Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorder joins before finalizing subqueries #1458

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

max-hoffman
Copy link
Contributor

We cannot set field indexes for subquery expressions until the parent scope has been finalized.

The query that surfaced this bug was a subquery expression with a dependency on a join child, whose order is rearranged after we finalize subquery. Ex: the query below swaps the schema from under the subquery expression; y and is_one should match, but we reorder the join for a lookup, changing the scope schema from [xy,uv] -> [uv,xy]. The subquery expression now selects the wrong column for comparison:

> select y, (select 1 where y = 1) is_one
  from xy join uv on x = v
  order by y;
+---+--------+
| y | is_one |
+---+--------+
| 0 | 1      |
| 0 | 1      |
| 1 | NULL   |
| 1 | NULL   |
+---+--------+

We select the correct field now when the subquery is finalized after the parent scope is fixed:

> select y, (select 1 where y = 1) is_one
  from xy join uv on x = v
  order by y;
+---+--------+
| y | is_one |
+---+--------+
| 0 | NULL   |
| 0 | NULL   |
| 1 | 1      |
| 1 | 1      |
+---+--------+

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

sql/analyzer/analyzer.go Outdated Show resolved Hide resolved
@max-hoffman max-hoffman merged commit 250562a into main Dec 7, 2022
@max-hoffman max-hoffman deleted the max/join-reorder-subqe-scope-index branch December 7, 2022 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants