Add support for JOIN pushdown in Exasol Connector#26603
Conversation
44ce5f3 to
13bff5d
Compare
d4f2a08 to
d58709a
Compare
There was a problem hiding this comment.
Thank you very much for your review, @ebyhr ! 👍
I have implemented integration tests, as requested.
In order to make the integration tests work, I also had to add basic implementation of convertPredicate method, which is also a prerequisite for the upcoing JOIN_PUSHDOWN feature.
Unfortunately, I couldn't implement convertPredicate in a separate PR, because the implemented integration tests require both toWriteMapping and convertPredicate methods.
The integration tests testToWriteMappingForDecimalType have been implemented in TestExasolConnectorTest
toWriteMapping and convertPredicate are triggered for LEFT JOIN queries when assertJoinConditionallyPushedDown check is done in integration tests.
Different column types are used to provide different values for toWriteMapping parameters.
Unfortunately, there is no way to check the business-logic in more details for these integration tests. These tests only make sure that the checks are successful and the assertJoinConditionallyPushedDown and expectJoinPushdownOnEmptyProjection are successfully supported for LEFT JOIN queries with different column types. By triggering toWriteMapping and convertPredicate these integration tests indirectly test these methods.
For more fine-grained assertions, please, check TestExasolToWriteMapping unit tests.
toWriteMapping is also triggered for INSERT queries, but because Exasol Connector is read-only and doesn't support INSERT queries, we don't need to cover toWriteMapping with the tests for INSERT queries.
Basic implementations of toWriteMapping and convertPredicate are prerequisites for enabling the upcoming JOIN_PUSHDOWN feature.
In the next PR I will enable testJoinPushdown() in BaseJdbcConnectorTest, which requires basic implementations of toWriteMapping and convertPredicate to make testJoinPushdown() work and to enable JOIN_PUSHDOWN feature.
d58709a to
f0016d5
Compare
5733432 to
bf020f8
Compare
bf020f8 to
062b439
Compare
0d65c13 to
3d12b9d
Compare
7a45d31 to
3598382
Compare
3598382 to
6a352b7
Compare
|
Hi @ebyhr , |
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
6a352b7 to
33bf332
Compare
33bf332 to
2ff64a9
Compare
skyglass
left a comment
There was a problem hiding this comment.
@chenjian2664, thank you very much for your feedback and clarification! 👍
I actually didn't know that SUPPORT_JOIN_PUSHDOWN automatically enables all sub flags, but I checked the TestPostgreSqlConnectorTest and it only has SUPPORT_JOIN_PUSHDOWN flag, without any sub-flags.
Therefore, I have removed all sub-flags and kept only SUPPORT_JOIN_PUSHDOWN flag, as you suggested.
The PR was updated, please review again.
skyglass
left a comment
There was a problem hiding this comment.
Thank you very much for your review, @chenjian2664 !
I have answered your comments, hopefully it resolves the current PR.
Our customers would be very happy if we could continue further support of join and aggregate pushdown, but, unfortunately, this unresolved PR is blocking us from further development.
2ff64a9 to
92536b9
Compare
b8ee8dd to
f096fa1
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
|
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
|
Any updates on this PR? |
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
Description
Additional context and related issues