Skip to content

Search: Fix error when passing double-anchored, single-word arguments to repo:has.description

Warren Gifford requested to merge tl/repo-has-description-anchors into main

Created by: tbliu98

Currently, passing a double-anchored regex pattern without any spaces (e.g. ^foo$) to repo:has.description causes a SQL error because within parseDescriptionPattern() the exact return value from parseIncludePattern() is always ignored, which results in a malformed SQL query when exact is non-nil. This was the result of an incorrect assumption by me that because regex patterns passed to repo:has.description are always "fuzzed" there will never be an exact string match. However, if the given pattern is something like ^foo$ then the fuzzed pattern will be (?:^foo$), which does have an exact string match. This PR fixes the error by:

  1. Not ignoring the value for exact returned by parseIncludePattern()
  2. Adding start and end anchors to each element of exact, then adding the resulting array to the database query condition, if exact is non-nil.

Example: repo:has.description(^foo$) will now only match repo descriptions that are an exact match of the string foo. Currently this query will result in a SQL error.

NOTE: Patterns that are double-anchored but containing spaces currently do not cause an error, because we are fuzzing the pattern passed to repo:has.description up-front -- basically we replace spaces with .*?. For repo:has.description(^foo bar$), the pattern ^foo bar$ is actually parsed as (?:^foo).*?(?:bar$), and exact will be nil for this pattern, which is why it doesn't cause an error. If you search for repo:has.description(^foo bar$) you'll get repo descriptions that start with foo and end with bar, with an indeterminate number of characters in between. I'm not yet sure if that is the exact behavior we want, but regex search appears to work similarly: check out the results for ^foo bar$.

In any case this fixes the very obvious and visible bug, and behavior can continue to be refined.

Test plan

Manually verify in traces that the argument passed to the db query condition for patterns like ^foo$ (double-anchored, single-word) is correct. Relying on existing unit tests for parseIncludePattern().

Merge request reports

Loading