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

optbuilder: disable RANGE window mode with offsets and NULLS LAST #94342

Merged
merged 1 commit into from
Dec 28, 2022

Conversation

yuzefovich
Copy link
Member

This commit makes it so that we return a regular error when building the window frame with RANGE mode with offsets when NULLS LAST ordering is requested. The execution engine assumes that exactly one column is included in the ordering for such a frame, and we recently fixed how we handle NULLS LAST (by projecting another column), so at the moment with such queries we encounter a scary-looking internal error, and now we'll get a regular error. Teaching the execution engine about this is not exactly trivial, so for now we'll just prohibit such queries.

Addresses: #94032.

Epic: None

Release note (bug fix): Previously, CockroachDB could encounter an internal error when evaluating window functions with RANGE window frame mode with OFFSET PRECEDING or OFFSET FOLLOWING boundary when ORDER BY clause has NULLS LAST option. This will now result in a regular error since the feature is marked as unsupported.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

# Regression test for an internal error with RANGE mode with offsets and NULLS
# LAST (currently unsupported due to limitations in the execution engine
# #94032).
build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe leave a TODO here with an issue number.

This commit makes it so that we return a regular error when building the
window frame with RANGE mode with offsets when NULLS LAST ordering is
requested. The execution engine assumes that exactly one column is
included in the ordering for such a frame, and we recently fixed how we
handle NULLS LAST (by projecting another column), so at the moment with
such queries we encounter a scary-looking internal error, and now we'll
get a regular error. Teaching the execution engine about this is not
exactly trivial, so for now we'll just prohibit such queries.

Epic: None

Release note (bug fix): Previously, CockroachDB could encounter an
internal error when evaluating window functions with RANGE window frame
mode with OFFSET PRECEDING or OFFSET FOLLOWING boundary when ORDER BY
clause has NULLS LAST option. This will now result in a regular error
since the feature is marked as unsupported.
Copy link
Member Author

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)


pkg/sql/opt/optbuilder/testdata/window line 1543 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

Maybe leave a TODO here with an issue number.

Done.

@craig
Copy link
Contributor

craig bot commented Dec 27, 2022

Build failed:

@yuzefovich
Copy link
Member Author

Seems like a flake.

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 27, 2022

Build failed:

@yuzefovich
Copy link
Member Author

Another flake.

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 28, 2022

Build succeeded:

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.

3 participants