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

[CALCITE-500] Fix EnumerableJoin to build the left side and probe the right #691

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@atris
Copy link
Contributor

commented May 17, 2018

No description provided.

Atri Sharma added some commits May 17, 2018

Atri Sharma Atri Sharma
CALCITE-500 -- Change cost model of EnumerableJoin to put smaller set…
… as second. This will ensure that smaller side is built and larger side is probed
Atri Sharma Atri Sharma
@atris

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2018

@julianhyde Can you please help review? Thanks

@atris atris changed the title CALCITE-500 -- Change cost model of EnumerableJoin to put smaller set… CALCITE-500 -- Fix EnumerableJoin to build the left side and probe the right May 17, 2018

@zinking

This comment has been minimized.

Copy link
Contributor

commented May 22, 2018

you will need a test, and some descriptions.

@michaelmior

This comment has been minimized.

Copy link
Member

commented May 22, 2018

Also, the two commits should be squashed together.

@atris

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2018

@zabetak

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

I haven't looked a lot in this issue but the fix seems wrong. I think it breaks LEFT, RIGHT, and FULL outer joins.

@atris

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2018

@vlsi

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

Yes, it is not valid to just swap rightExpression and leftExpression.
leftResult.physType.generateAccessor(leftKeys) and friends should be replaced as well.

@atris

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2018

@zabetak

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

I would be inclined to change the cost function so that left and right inputs are set correctly. That way the implement method would not need to be changed at all.

@vlsi

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

@zabetak , that depends on the background :)

For instance, in Oracle DB the first input is used to build the table.

@vlsi vlsi changed the title CALCITE-500 -- Fix EnumerableJoin to build the left side and probe the right [CALCITE-500] Fix EnumerableJoin to build the left side and probe the right Jan 9, 2019

@hsyuan

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

@atris Are you going to continue working on this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.