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: move indexJoiner into joinReader #40749

Closed
RaduBerinde opened this issue Sep 13, 2019 · 3 comments · Fixed by #51416
Closed

sql: move indexJoiner into joinReader #40749

RaduBerinde opened this issue Sep 13, 2019 · 3 comments · Fixed by #51416
Assignees
@helenmhe-zz

Comments

@RaduBerinde
Copy link
Member

The join reader has two somewhat distinct "modes" of operation (index join and general lookup join). The index join mode has some limitations: it expects that the input stream starts with the PK columns in order, and is not capable of "passing through" other values - we may have values for other columns available but the index join has to get and decode those again.

As we remove the heuristic planner, we can lift some of the restrictions around indexJoinNode (e.g. allow arbitrary (non-scanNode) input) and it is becoming a bit of a pain to deal with the limitations.

We should unify these two modes and make sure that any index-join-specific optimizations are carried over and can be applied whenever we are doing an index join (e.g. #40748, #39471).
@asubiotto asubiotto mentioned this issue Apr 14, 2020
Closed
5 tasks
@asubiotto asubiotto changed the title sql: improve join reader interface sql: move indexJoiner into joinReader Jun 2, 2020
@asubiotto
Copy link
Contributor

asubiotto commented Jun 2, 2020 •
edited
Loading

We're planning on getting rid of the indexJoiner processor and instantiate a joinReader for index joins instead. Both processors use the same spec. At that point I think we can delete the indexJoinerNode, does that sound good to you?

@RaduBerinde
Copy link
Member Author

Yeah, that'd be great!

@RaduBerinde
Copy link
Member Author

We would probably want to preserve the EXPLAIN output for index join, so we may want to plumb a flag just for cosmetic purposes.

@helenmhe-zz helenmhe-zz mentioned this issue Jul 14, 2020
Merged
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 15, 2020
@helenmhe
sql: refactor index join to use JoinReader
cf7a6e6 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 15, 2020
@helenmhe
s/sql/rowexec/: refactor index join to use JoinReader
bec58d5 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 16, 2020
@helenmhe
rowexec: refactor index join to use JoinReader
800a1ab 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 16, 2020
@helenmhe
rowexec: refactor index join to use JoinReader
5ef4872 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 21, 2020
@helenmhe
rowexec: refactor index join to use JoinReader
a9eace8 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 22, 2020
@helenmhe
rowexec: refactor index join to use JoinReader
c52f8bc 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
helenmhe-zz pushed a commit to helenmhe-zz/cockroach that referenced this issue Jul 22, 2020
@helenmhe
rowexec: refactor index join to use JoinReader
8e4af83 
Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: cockroachdb#40749

Release note: None
craig bot pushed a commit that referenced this issue Jul 28, 2020
@otan @rohany
@helenmhe @RaduBerinde
Merge #51183 #51240 #51416 #51537 #51923
c8559fe 
51183: geogen: introduce package to generate random geo types r=sumeerbhola a=otan

Generate random geo types in a new geogen package. This is immediately
utilised for all the randomized testing, including sqlsmith.

Release note: None

51240: sql: implement the `DROP TYPE` command r=rohany a=rohany

This PR implements the `DROP TYPE` command without support for
`CASCADE`. It lays the foundation of the dependency management between
types and tables that use them. With this, we can drop types when a
database is dropped.

Fixes #48363.

Release note: None

51416: rowexec: refactor index join to use JoinReader r=helenmhe a=helenmhe

Previously there was a separate processor specifically for index joins.
This commit instantiates a joinReader processor instead, so that we
can get rid of the indexJoiner processor and indexJoinerNode.

Resolves: #40749

Release note: None

51537: builtins: implement ST_Intersection and ST_Buffer for geography types r=sumeerbhola a=otan

PostGIS uses _ST_BestSRID to approximate a geography as a geometry to
perform ST_Buffer and ST_Intersection. We're obliged to do the same -
comments are inline.

Resolves #48812
Resolves #48389
Resolves #48390
Resolves #48391
Resolves #48398

Release note (sql change): Implements the ST_Buffer and ST_Intersection
functions for Geography types.

Release note (sql change): Implement ST_Intersection for string types.

51923: opt: move code that generates proto explain tree r=RaduBerinde a=RaduBerinde

Move the code that generates the `roachpb.ExplainTreePlanNode` tree
(shown in the UI) to exec/explain. It parallels the existing code for
generating other explain output.

Release note: None

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Helen He <helenhe.mit@gmail.com>
Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
@craig craig bot closed this as completed in ce0bde7 Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@helenmhe-zz helenmhe-zz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rowexec: refactor index join to use JoinReader helenmhe-zz/cockroach
4 participants
@jordanlewis @asubiotto @RaduBerinde @helenmhe-zz