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

Add config option and session property to restore legacy ORDER BY behavior #6871

Merged
merged 1 commit into from
Dec 14, 2016

Conversation

martint
Copy link
Contributor

@martint martint commented Dec 13, 2016

We recently made a change to the column resolution rules for ORDER BY to
make them compliant with ANSI SQL. In order to ease the transition from
the old semantics, we now add a config option and session property that
controls the behavior.

The session property is "legacy_order_by". The config option is
"deprecated.legacy-order-by".

This will be removed in a not-too-distant version.

Fixes #6867

@martint martint requested a review from cberner December 13, 2016 22:42
Copy link
Contributor

@cberner cberner left a comment

Choose a reason for hiding this comment

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

Make sure Travis passes, as you missed the tests. Otherwise looks good

@@ -52,6 +52,7 @@
private boolean optimizeSingleDistinct = true;
private boolean pushTableWriteThroughUnion = true;
private boolean legacyArrayAgg;
private boolean legacyOrderBy;
Copy link
Contributor

Choose a reason for hiding this comment

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

You forgot to add tests for this

@martint
Copy link
Contributor Author

martint commented Dec 14, 2016

Failure seems to be unrelated.

…avior

We recently made a change to the column resolution rules for ORDER BY to
make them compliant with ANSI SQL. In order to ease the transition from
the old semantics, we now add a config option and session property that
controls the behavior.

The session property is "legacy_order_by". The config option is
"deprecated.legacy-order-by".
@martint martint closed this Dec 14, 2016
@martint martint deleted the orderby branch December 14, 2016 00:38
@martint martint merged commit 3449578 into prestodb:master Dec 14, 2016
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.

2 participants