fix(frontend): align role rule merge semantics#24936
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
I found one substantive issue in the new rewrite-rule merge behavior.
The current implementation now OR-merges many single-table SELECT rules as long as the projection list text matches, for example select a from db1.t1 where .... That is not semantics-preserving in general. It is only safe for row-identity-preserving projections (effectively select * / exact table-star forms). For partial projections like select a ..., two different base rows that satisfy different role predicates but project to the same a value will now both be returned, whereas the previous UNION DISTINCT-style behavior would have deduplicated the projected result.
What makes this higher confidence is that the new tests explicitly bless this for select a ... and select a as x .... So the PR is not just changing implementation shape — it is also locking in a user-visible semantic change for multi-role rewrite results. I think OR-merging should be restricted to projections that are guaranteed row-preserving, and non-identity projections should keep a semantics-preserving fallback instead of being treated as mergeable.
What type of PR is this?
Which issue(s) this PR fixes:
issue #23271
What this PR does / why we need it:
This ports the relevant behavior from #24792 to 3.0-dev:
Validation performed:
Go test notes:
go test -mod=vendor ./pkg/frontend -run TestMergeRewriteRulesis blocked by the local repository vendor mismatch betweengo.modandvendor/modules.txt.GOFLAGS=-mod=mod go test ...is blocked by the local environment missingusearch.hforgithub.com/unum-cloud/usearch/golang.