fix(mysql): resolve comma-separated relations in dot-scoped completion - #2088
Conversation
|
Thank you, @mvanhorn, for contributing this fix and for adding focused regression coverage for #1949. The positive case is valid, and I re-ran both the targeted suite (183 tests) and the full MySQL reactor suite (610 MySQL tests); they pass. Before this is merged, I found a few cases that need to be addressed in 1. A same-depth
|
Addresses review feedback on OtterMind#2088: - Replace the backward same-depth FROM search with a single forward state scan per query block, so sibling subqueries and sibling CTE bodies no longer reuse an earlier block's FROM. This also removes the O(n^2) comma classification: separators are precomputed once, then read in O(1). - Recognize ON DUPLICATE KEY UPDATE, FOR UPDATE/SHARE and LOCK IN SHARE MODE contextually rather than treating raw ON/FOR/UPDATE as boundaries. - Scope resolution to the statement containing the cursor. - Add negative regression tests for sibling subqueries, the second CTE body, multiple statements, ON DUPLICATE KEY UPDATE and FOR UPDATE/SHARE. - Assert the captured metadata request is COLUMN, scoped to orders/b, with an empty prefix. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
|
All of these are handled in 705e543 -- apologies, I pushed it without following up here. The backward same-depth The boundary list is contextual rather than raw tokens: One side effect worth mentioning: separators are precomputed once and then read in O(1), so the per-comma backward scan is gone as well. Your three cases are in as negative tests, along with |
openai0229
left a comment
There was a problem hiding this comment.
Thank you, @mvanhorn, for the thoughtful and thorough follow-up. I re-reviewed 705e543: the query-block-aware separator scan addresses the earlier scope and performance concerns, the requested negative cases and metadata-request assertions are present, and I verified the branch on top of the current main with the full MySQL reactor suite (618 tests) passing.
This is a solid, focused fix for #1949. Approved, and thank you again for the contribution.
|
Thanks for merging the dot-scoped completion fix. Appreciate you taking the regression contract for multi-relation MySQL queries along with it. |
Related issue
Closes #1949
Summary
Issue #1949 asks for an explicit regression contract around MySQL dot-scoped
completion in multi-relation queries: after typing the second relation's alias,
b.should offer only that relation's columns. Writing the contract surfaced areal gap in
MysqlSqlCompletionRelationScopeResolver.isRelationBoundarydid not treat a,as a relation separator, so acomma-separated relation list was only partially resolved: in
select * from users u join orders o on u.id = o.user_id, payments p where p.the
payments prelation was never registered, andp.fell back to the firstrelation's columns. The new
isFromRelationSeparatorhelper accepts a commaonly when it sits inside the
FROMclause at the same paren depth: it finds thenearest preceding
FROMand refuses when a clause boundary (ON,WHERE,GROUP,HAVING,ORDER,LIMIT,UNION) intervenes. That keeps commas inselect lists, function arguments, and
WINDOWdefinitions from being mistakenfor relations.
The change stays inside the shared community MySQL plugin. No completion
pipeline rework, no UI rendering change, no Local or commercial-edition code.
Affected surfaces
Verification
Note:
-DskipTests=falsealone is not enough on this build;maven.test.skipis set upstream and skips test compilation, so the tests silently do not run.
The command above sets both flags.
main(the second relation'salias resolves to the first relation's columns) and pass with the resolver
change.
Risk and compatibility
chat2db-community-mysql; no Local or commercial-edition code touched, per the issue's non-goals.FROMclause at the same paren depth, so select-list commas, function-argument commas, andWINDOWdefinition commas keep their existing behavior. A negative test pins theWINDOWcase.Reviewer map
MysqlSqlCompletionRelationScopeResolver.isFromRelationSeparatorand the newMySqlLexer.COMMAcase inisRelationBoundary.FROMclause would be read as a relation separator and pollute the completion scope.doesNotTreatNamedWindowAsRelationis the guard for that class.MySqlLexer.COMMAcase inisRelationBoundary; the helper becomes unreachable and completion returns to the previous behavior.Contributor declaration
AI assistance: AI was used for assistance.