search

sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.11k stars 1.29k forks source link

insights: query builder builds incorrect search queries when OR is used in search #37332

Open chwarwick opened 2 years ago

chwarwick commented 2 years ago

The following search query in an insight results in an incorrect search: .Command or .GitCommand file:vcs/git -file:_test.go lang:go

Generated the following query: (archived:no fork:no .Command OR archived:no fork:no file:vcs/git -file:_test.go lang:go .GitCommand)

This search is wrong because the .Command side of the or is not applying the additional file and lang parameters.

Work arounds:

  1. use parentheses - (.Command or .GitCommand) file:vcs/git -file:_test.go lang:go
  2. change ordering - file:vcs/git -file:_test.go lang:go .Command or .GitCommand
sourcegraph-bot-2 commented 2 years ago

Heads up @joelkw @felixfbecker @vovakulikov @unclejustin - the "team/code-insights" label was applied to this issue.

chwarwick commented 2 years ago

cc @coury-clark

chwarwick commented 2 years ago

Additional context, search via the UI provides the expected results, the difference I see is that the query passed to the api is:

context:global .Command or .GitCommand file:vcs/git -file:_test.go lang:go

This moves the .Command or .GitCommand out of the first position and causes it to be interpreted as:

context:global (.Command or .GitCommand) file:vcs/git -file:_test.go lang:go

instead of: 

.Command OR (.GitCommand file:vcs/git -file:_test.go lang:go)
coury-clark commented 2 years ago

We need to check if our manipulation of the tree (fork/archive) is causing this, or if this is otherwise just a bug in the parser. The examples above don't have the fork archive - does that mean this is from a non-manipulated query?

felixfbecker commented 2 years ago

I thought we explicitly don't support AND/OR operators, or do we now?

coury-clark commented 2 years ago

I thought we explicitly don't support AND/OR operators, or do we now?

I think that's only true for capture group insights, since the compute endpoint doesn't support that yet.

chwarwick commented 2 years ago

The manipulation of the tree doesn't cause it, it's dependent on the what the parser encounters first. If it's a parameter like context, repo, fork, etc... then it parses the way we expect and matches the search UI, however if it first encounters .Command or .GitCommand it parses differently.

My initial reaction is it's a bug in the query parsing but I could interpret query 

.Command or .GitCommand file:vcs/git -file:_test.go lang:go

As

.Command OR .GitCommand AND file:vcs/git AND -file:_test.go AND lang:go

In which case I think it's correct, and it's the search box that is returning results that I would expect.

coury-clark commented 2 years ago

@coury-clark

CristinaBirkel commented 2 years ago

Validate to see if it's a search issue, and if so file an issue with search.