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

distsql: support tuples in streams #15938

Closed
RaduBerinde opened this issue May 15, 2017 · 4 comments
Closed

distsql: support tuples in streams #15938

RaduBerinde opened this issue May 15, 2017 · 4 comments
Assignees
Labels
A-sql-execution Relating to SQL execution. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Milestone

Comments

@RaduBerinde
Copy link
Member

We currently don't support tuples in DistSQL streams. We don't have a ColumnType_Kind for it and no way to encode it over the wire. This makes certain queries non-distributed.

For example: SELECT (k, v) FROM t.

Another example is more subtle: SELECT k, v FROM t ORDER BY (k, v). Here the extra parens cause a tuple to be created for each row and used as the sort key. So this query is not supported by DistSQL, as opposed to the equivalent SELECT k, v FROM t ORDER BY k, v. Granted, this case should be probably converted to latter version before distsql planning.

CC @asubiotto

@RaduBerinde RaduBerinde added this to the 1.1 milestone May 15, 2017
@asubiotto asubiotto self-assigned this May 16, 2017
@vivekmenezes vivekmenezes modified the milestones: 1.2, 1.1 Aug 10, 2017
@vivekmenezes vivekmenezes removed this from the 2.0 milestone Feb 9, 2018
@petermattis petermattis added this to the 2.1 milestone Feb 21, 2018
@jordanlewis jordanlewis added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-execution Relating to SQL execution. labels Apr 25, 2018
@rjnn
Copy link
Contributor

rjnn commented May 2, 2018

Supporting tuples in DistSQL requires two things:

  1. propagating the appropriate type information via ColumnType. This was fairly straightforward.
  2. encoding/decoding the tuple types over the wire. for now I'm working on adding only a value encoding, so it would not be order preserving. This is a little more work, but still seems straightforward.

The other part mentioned - converting tuplified queries into untuplified versions - are not strictly necessary for supporting tuples in distsql, so should probably be tracked by the optimizer team.

@RaduBerinde
Copy link
Member Author

Great! A value encoding is functionally sufficient.

The other part wasn't meant to be a task for this issue, it was just an example where tuples can pop up unexpectedly. I am filing an issue for the optimizer.

@rjnn
Copy link
Contributor

rjnn commented May 2, 2018

Glad to see you think a value encoding is sufficient. I was afraid there might be an edge case where it wouldn't suffice. I think optimization wise pushing tuple construction as far down towards the end of the query would probably be sufficient - the value encoding precludes execution optimizations that require orderings between values, but that's about it... Anything that requires a tuple can be accomplished by keeping multiple columns as far as I can see, and constructing the tuple at the very end.

@RaduBerinde
Copy link
Member Author

the value encoding precludes execution optimizations that require orderings between values, but that's about it

Exactly, if we need ordering, we will just decode and compare. Unclear how important this optimization is anyway.

rjnn pushed a commit to rjnn/cockroach that referenced this issue May 23, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 23, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 23, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 23, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 24, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 24, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 29, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 29, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
rjnn pushed a commit to rjnn/cockroach that referenced this issue May 31, 2018
Closes cockroachdb#15938.

Release note (performance improvement): using tuples in a query no
longer reverts you to single node local SQL execution.
@craig craig bot closed this as completed in 7751b46 Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

6 participants