fix(search_jobs): fail validation for repo searches#64300
Conversation
| // /internal/search/search.go. We duplicate the check here to make sure we fail | ||
| // early and not during execution. | ||
| func validateSearcherParams(b query.Basic, inputs *search.Inputs, resultTypes result.Types) error { | ||
| r := toTextPatternInfo(b, resultTypes, inputs.Features, inputs.DefaultLimit()) |
There was a problem hiding this comment.
What if we performed this validation as part of toTextPatternInfo to avoid having to duplicate this logic? It seems nice for searcher too to report errors early?
There was a problem hiding this comment.
Sounds good to me. This makes the change a bit more invasive though, but it should work.
jtibshirani
left a comment
There was a problem hiding this comment.
Looks good! Checking I understand: this should have no difference on searcher or structural search behavior, right? Because we just shuffled the validation to another place, but even before it was happening for every query sent to searcher.
Yes! |
This closes a pretty big gap in the query validation for Search Jobs. We don't support repo search yet and searcher returned errors during execution, complaining about missing patterns. With this PR we fail during validation so users cannot even create these kinds of jobs. See new test cases.
Test plan
Updated unit tests