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

sql: fix a bug with ordinality planning #39086

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

yuzefovich
Copy link
Member

Ordinality node appends an Int column so that it can write to it.
Previously, plan.ResultTypes were appended to before adding a
single group stage which internally used updated ResultTypes to
set up inputs to the ordinality processor. Now this behavior is
fixed and adding a single group stage takes care of updating
ResultTypes accordingly.

Fixes: #38995.

Release note: None

Ordinality node appends an Int column so that it can write to it.
Previously, plan.ResultTypes were appended to before adding a
single group stage which internally used updated ResultTypes to
set up inputs to the ordinality processor. Now this behavior is
fixed and adding a single group stage takes care of updating
ResultTypes accordingly.

Release note: None
@yuzefovich yuzefovich requested review from jordanlewis, rafiss and a team July 24, 2019 20:36
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich
Copy link
Member Author

What happens at the moment is that tableReader outputs a single column, but ordinality gets set up such that it thinks there are two input columns. In DistSQL it doesn't trigger any issues since ordinality will actually write into that second "imaginary" column, but with vectorized the behavior is different: ordinality will write into a third column and we will also append a fourth column for projection, so everything gets screwed up.

Copy link
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

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

:lgtm: Wow, nice catch.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @rafiss)

@yuzefovich
Copy link
Member Author

TFTR!

bors r+

craig bot pushed a commit that referenced this pull request Jul 25, 2019
39086: sql: fix a bug with ordinality planning r=yuzefovich a=yuzefovich

Ordinality node appends an Int column so that it can write to it.
Previously, plan.ResultTypes were appended to before adding a
single group stage which internally used updated ResultTypes to
set up inputs to the ordinality processor. Now this behavior is
fixed and adding a single group stage takes care of updating
ResultTypes accordingly.

Fixes: #38995.

Release note: None

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Jul 25, 2019

Build succeeded

@craig craig bot merged commit dc4f2fc into cockroachdb:master Jul 25, 2019
@yuzefovich yuzefovich deleted the ordinality branch July 30, 2019 04:25
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.

exec: probably a bug with ordinality
3 participants