Add Join operator support for Generic JDBC platform#708
Add Join operator support for Generic JDBC platform#708mohit-devlogs wants to merge 4 commits intoapache:mainfrom
Conversation
zkaoudi
left a comment
There was a problem hiding this comment.
thank you @mohit-devlogs. That was fast :) Could you please also add a unit test for the operator to make sure it works as it is supposed to?
Thank you! I will add a unit test for the operator and update the PR shortly. |
|
Can you expand the unit test to include the operator being executed? |
Thank you for the suggestion. |
|
I have extended the unit test to execute the GenericJdbcJoinOperator via JdbcExecutor against an in-memory HSQLDB instance. However, I am facing the following issue when I run the code: java.sql.SQLSyntaxErrorException: user lacks privilege or object not found: ID1 This issue occurs when the |
|
@mohit-devlogs can you check and compare with this test: |
|
@zkaoudi I adapted the test to execute the operator using an in-memory HSQLDB instance. java.sql.SQLSyntaxErrorException: user lacks privilege or object not found: A This appears to come from the SQL generated by GenericJdbcJoinOperator, where the column reference becomes detached from its table alias and HSQLDB interprets it as a table identifier. |
|
Yes, there should be something wrong with the genericjdbc implementation. As the |
Thanks for the hint. The issue was caused by GenericJdbcTableSource interpreting the first constructor argument as jdbcName, which led to configuration lookups like wayang.A.jdbc.url. |
|
|
||
| source1.addTargetPlatform(GenericJdbcPlatform.getInstance()); | ||
| source2.addTargetPlatform(GenericJdbcPlatform.getInstance()); | ||
| join.addTargetPlatform(JavaPlatform.getInstance()); |
There was a problem hiding this comment.
The join happens in Java for this test. So it's not the genericjdbcjoin operator that is being tested.
| } | ||
|
|
||
| long cardinality = resultSet.getLong(1); | ||
|
|
There was a problem hiding this comment.
no need to introduce all these line breaks
|
I think there is a deeper issue with the jdbc executor, so it would be enough if your test just checks if the right SQL statement is created, just like the |
Thanks for the clarification. I understand that the test should only verify the SQL generation, similar to JdbcJoinOperatorTest, rather than executing the query. I am currently adjusting the test accordingly. While doing so, I also noticed the issue in the JDBC executor related to stages with multiple sources (which affects JOIN operations). I am investigating it to understand whether it needs to be addressed or if the test should simply focus on SQL generation as suggested. I will update the test and push the changes shortly. |
… execution pipeline
|
I have updated the tests and pushed the changes. The GenericJdbcJoinOperatorTest now verifies SQL generation correctly, and I also adjusted the GenericJdbcFilterOperatorTest to include the downstream SqlToStreamOperator stage so that the JdbcExecutor can build the SQL query pipeline properly. Both tests pass locally in the wayang-generic-jdbc module. Please let me know if any further changes are required. |
This PR adds JOIN operator support for the Generic JDBC platform.
The JOIN operator is currently only implemented for wayang-postgres. This PR extends this to make it work on any JDBC database.
Implementation:
This PR extends the feature set of JOIN operators.
Closes #707