Repository navigation
fix(search): normalize KQL path once - #3516
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
🟢 Coverage 100.00% diff coverage · +0.01% coverage variation
Metric Results Coverage variation ✅ +0.01% coverage variation (-1.00%) Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (a303d7f) Report Missing Report Missing Report Missing Head commit (e1f313b) 92020 (+1) 23227 (+12) 25.24% (+0.01%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3516) 7 7 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
bab439b to
f6a1820
Compare
The trailing slash is trimmed in the normalizer instead of in each compiler; an empty path is the space root.
| if FieldIsPath(node.Key) { | ||
| node.Value = strings.TrimSuffix(node.Value, "/") | ||
| if node.Value == "" { | ||
| node.Value = "." | ||
| } | ||
| } |
There was a problem hiding this comment.
Should we also fix cases like /parent or parent while we're at it?
Claude suggests
| if FieldIsPath(node.Key) { | |
| node.Value = strings.TrimSuffix(node.Value, "/") | |
| if node.Value == "" { | |
| node.Value = "." | |
| } | |
| } | |
| if FieldIsPath(node.Key) && !strings.ContainsAny(node.Value, "*?") { | |
| node.Value = utils.MakeRelativePath(node.Value) | |
| } |
and parity cases
{id: 11, query: `path:"/parent"`, want: []string{"parent", "child.jpg"}},
{id: 12, query: `path:"parent"`, want: []string{"parent", "child.jpg"}},
which would make it consistent with the scope handling in the backends.
There was a problem hiding this comment.
Done in 4a720c1, but without the wildcard guard: all path terms start with ./, so a wildcard without the prefix (path:"parent/*") matches nothing. With MakeRelativePath applied unconditionally, parent/* and ./parent/* behave the same, like parent and ./parent. PATH-11 to PATH-13 pin it for both engines.
There was a problem hiding this comment.
You are right, I was wrong that the prefix never changes wildcard semantics: a leading * or ? used to cover the ., and anchoring it to ./ breaks */parent. e1f313b keeps your exemption for values that start with a wildcard and pins */parent, ?/parent, */Documents and */parent/ as PATH-14 to PATH-17; all four fail on both engines without it.
Description
The trailing slash of a KQL
path:value was trimmed in the bleve compiler and again in the OpenSearch transpiler. The normalizer now trims it once for both engines. An empty value,path:"/", was a term query on the empty string and matched nothing; it now means the space root, consistent with the scope handling.Types of changes