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.

Sign up for GitHub

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

Jump to bottom

sql: Parenthetical SELECTs throw away ORDER BY #4734

Closed
danhhz opened this issue Feb 29, 2016 · 4 comments
Closed

sql: Parenthetical SELECTs throw away ORDER BY #4734

danhhz opened this issue Feb 29, 2016 · 4 comments
Assignees
@danhhz
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Milestone
Beta

Comments

@danhhz
Copy link
Contributor

danhhz commented Feb 29, 2016

repro: https://github.com/paperstreet/cockroach/commits/parenselect_order

testdata/order_by:136 root: (SELECT a FROM t) ORDER BY a DESC
--- FAIL: TestLogic (0.06s)
  logic_test.go:659: testdata/order_by:136: expected:
    "5"
    "4"
    "3"
    "2"
    "1"
    but found:
    "1"
    "2"
    "3"
    "4"
    "5"
@danhhz danhhz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 29, 2016
@danhhz danhhz self-assigned this Feb 29, 2016
@danhhz danhhz added this to the Beta milestone Feb 29, 2016
@petermattis
Copy link
Collaborator

I imagine there is some silly oversight in the code. Definitely would be a good intro to the code to track down what is going on. Let us know if you need any guidance.

@danhhz
Copy link
Contributor Author

danhhz commented Mar 1, 2016

Ah, I should have mentioned that I did actually find the bug, was going to take a look at fixing it today

@danhhz danhhz closed this as completed Mar 1, 2016
@danhhz
Copy link
Contributor Author

danhhz commented Mar 1, 2016

Oops, wrong button : - /

@danhhz danhhz reopened this Mar 1, 2016
@tamird
Copy link
Contributor

tamird commented Mar 1, 2016

You can probably keep this issue open until the bug is fixed.

On Tue, Mar 1, 2016 at 10:34 AM, Dan Harrison notifications@github.com
wrote:

Ah, I should have mentioned that I did actually find the bug, was going to
take a look at fixing it today

—
Reply to this email directly or view it on GitHub
#4734 (comment)
.

danhhz added a commit to danhhz/cockroach that referenced this issue Mar 4, 2016
@danhhz
Support ORDER BY and LIMIT on all SELECT-like statements
91e03fd 
We were previously accepting these parses, but losing the sort and limit data. This commit adds a generalized sortNode and limitNode for any select-like, but keeps the optimization of passing them down to Select itself.

Closes cockroachdb#4734.
Closes cockroachdb#2691.
@danhhz danhhz mentioned this issue Mar 4, 2016
Merged
danhhz added a commit to danhhz/cockroach that referenced this issue Mar 4, 2016
@danhhz
Support ORDER BY and LIMIT on all SELECT-like statements
e3966de 
We were previously accepting these parses, but losing the sort and limit data.
This commit adds a generalized sortNode and limitNode for any select-like, but
keeps the optimization of passing them down to Select itself for index
selection.

Closes cockroachdb#4734.
Closes cockroachdb#2691.
danhhz added a commit to danhhz/cockroach that referenced this issue Mar 8, 2016
@danhhz
Support ORDER BY and LIMIT on all SelectStatements
18bf340 
We were previously accepting these parses, but losing the sort and limit data.
This commit adds a generalized sortNode and limitNode for any SelectStatement,
but keeps the optimization of passing them down to SelectClause itself for index
selection.

Closes cockroachdb#4734.
Closes cockroachdb#2691.
danhhz added a commit to danhhz/cockroach that referenced this issue Mar 9, 2016
@danhhz
Support ORDER BY and LIMIT on all SelectStatements
e3b714c 
We were previously accepting these parses, but losing the sort and limit data.
This commit adds a generalized sortNode and limitNode for any SelectStatement,
but keeps the optimization of passing them down to SelectClause itself for index
selection.

Closes cockroachdb#4734.
Closes cockroachdb#2691.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danhhz danhhz

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
Beta
Development

No branches or pull requests
3 participants
@danhhz @tamird @petermattis