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 crash related to ordinalityNode props #32596

Merged
merged 1 commit into from Nov 27, 2018

Conversation

Projects
None yet
3 participants
@RaduBerinde
Copy link
Member

RaduBerinde commented Nov 26, 2018

Addresses a crash in the heuristic planner, caused by not properly
copying the source properties in ordinalityNode.

Fixes #31911.

Release note: None

@RaduBerinde RaduBerinde requested a review from knz Nov 26, 2018

@RaduBerinde RaduBerinde requested review from cockroachdb/sql-execution-prs as code owners Nov 26, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Nov 26, 2018

This change is Reviewable

@knz

knz approved these changes Nov 27, 2018

Copy link
Member

knz left a comment

:lgtm: with nit see below

Also there needs to be a release note (bug fix).

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/sql/ordinality.go, line 145 at r1 (raw file):

		// currently the only case where this happens we consider it's not
		// worth the hassle and just use the source ordering.
		o.props = srcProps.copy()

nit: you can hoist the call to copy() and drop one of the two if branches.

@RaduBerinde RaduBerinde force-pushed the RaduBerinde:fix-ordinality-planner-crash branch from 2703165 to 23c8d6c Nov 27, 2018

sql: fix crash related to ordinalityNode props
Addresses a crash in the heuristic planner, caused by not properly
copying the source properties in ordinalityNode.

Fixes #31911.

Release note (bug fix): fixed a crash caused by WITH ORDINALITY in
some cases.

@RaduBerinde RaduBerinde force-pushed the RaduBerinde:fix-ordinality-planner-crash branch from 23c8d6c to f310dd0 Nov 27, 2018

@RaduBerinde
Copy link
Member

RaduBerinde left a comment

Done, thanks.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)

@RaduBerinde

This comment has been minimized.

Copy link
Member

RaduBerinde commented Nov 27, 2018

bors r+

craig bot pushed a commit that referenced this pull request Nov 27, 2018

Merge #32596
32596: sql: fix crash related to ordinalityNode props r=RaduBerinde a=RaduBerinde

Addresses a crash in the heuristic planner, caused by not properly
copying the source properties in ordinalityNode.

Fixes #31911.

Release note: None

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Nov 27, 2018

Build succeeded

@craig craig bot merged commit f310dd0 into cockroachdb:master Nov 27, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@RaduBerinde RaduBerinde deleted the RaduBerinde:fix-ordinality-planner-crash branch Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment