Fix JOIN clause to reference the correct table in SQL query generation

[treyfel]

• Corrected the SQL JOIN clause in JdbcJoinOperator#createSqlClause to use the right table name when forming the JOIN statement, ensuring the SQL query generated is not a self-join.

• Updated the expected SQL query in JdbcJoinOperatorTest#testWithHsqldb to match the corrected JOIN clause logic.

[mspruc]

ty for your contribution,

I'm not entirely sure I understand the issue you are trying to fix here, JdbcJoinOperator.createSqlClause() now uses the right table name twice, but what about the left table name then?

Can I ask what issue prompted the change? If you have a stack trace or something I can also take a look at it.

[kamir]

Please check if I got it right and if my suggestions is in line with your goal.
And please clarify the goal if it is something else.
Cheers, Mirko

[kamir]

I think the problem was in the test class:

SELECT * FROM testA JOIN testA ON testB.a=testA.a;

In JdbcJoinOperator.java‎
the code looks good.

[treyfel]

ty for your contribution,

I'm not entirely sure I understand the issue you are trying to fix here, JdbcJoinOperator.createSqlClause() now uses the right table name twice, but what about the left table name then?

The left table name comes from the tableSource JdbcTableSource.createSqlClause().

Can I ask what issue prompted the change? If you have a stack trace or something I can also take a look at it.

There was no initial issue as the tests we’re green but when working on something regarding join operations on the jdbc platform, I noticed the test checks for a self join syntax, which I assume was not intended.

[zkaoudi]

+1 for this PR.
The left table is created as a "FROM $leftTableName" statement in the source operator. In the proposed change the "JOIN" should be done on the right table as a string of "JOIN $rightTableName". Then the equality "ON" comes where it doesn't matter the order of the predicates.

@treyfel maybe you can split up the return string into two strings (join part and predicates part) so that it's not confusing with a single return statement

[mspruc]

let's merge this, I still have some outstanding issues with how we handle createSqlClause() and .withSqlImplementation() as ive previously mentioned in #643 but I have something for that in the pipeline, for now this should be fine @zkaoudi @kamir

[zkaoudi]

Hi @kamir, can you merge the PR? The suggested changes are correct