properly scope table references when binding#5673
properly scope table references when binding#5673pedrocarlo wants to merge 5 commits intotursodatabase:mainfrom
Conversation
1bd7830 to
7635065
Compare
|
glad youre making something like this. will read maybe tomorrow. i think we should try to solve all of our scoped binding problems in one go, eg. allowing correlated subs in all the positions that we've currently disabled due to scope issues |
|
yeah, I still need to read bit more of the PR to see if I'm actually solving all scoping problems. I need some more sqltests to prove this. As we don't have this "spec" written somewhere of all the scoping behaviours we expect, the AI and me cannot be 100% sure we did the right thing. |
7635065 to
3ac8a56
Compare
…ied name for the table being referenced
|
Ok I'm still brainstorming a bit how we can try and solve this correctly. I'm also looking into datafusion and seeing if we can get some more interesting ideas from them. |
Description
This PR replaces ad-hoc column name binding in
core/translate/expr.rswith a composable trait-based scope system.The previous binder used flat behavior flags and direct iteration over
joined_tables + outer_query_refs, which made clause-specific scoping fragile. This refactor introduces explicit scope composition at each call site so each SQL clause can define its own name-resolution precedence and visibility.Design
New module
core/translate/scope.rswith:ScopetraitResolvedColumnandLookupResultFullTableScope(joined tables, then outer refs)LocalTableScope(joined tables only)AliasScope(result-column alias expansion)EmptyScope(resolves nothing)ChainedScope<I, O>combinator for precedence layeringBinder API change
bind_and_rewrite_exprsignature changed to:BindingBehavior.Expr::IdandExpr::Qualifiednow delegate toScope.Expr::DoublyQualifiedandExpr::FunctionCallStaras binder special-cases (usingscope.table_references_mut()when needed).Scoping behavior changes
GROUP BY/HAVING/ORDER BYalias chains now useAliasScope -> LocalTableScopewhere appropriate, preventing accidental outer-reference capture.WHEREand general expression contexts continue to use full table visibility viaFullTableScope(with optional alias fallback depending on clause semantics).Motivation and context
I caught some scoping issues with differential fuzzer where a
GROUP BYin an inner query was resolving a column from an outer query that had the same name but was from a different table. So I wanted to try and solve this once and for all. Look at the.sqltestfor an exampleDescription of AI Usage
Planned with Claude and executed by Codex