confluentinc / kafka-connect-jdbc

Kafka Connect connector for JDBC-compatible databases
Other
1.01k stars 953 forks source link

[CCDB-2031] Fix quote.sql.identifiers=never not working for Oracle and Postgres #1357

Closed Tanish0019 closed 10 months ago

Tanish0019 commented 11 months ago

Problem

CCDB-2031 When a user has configured the quote.sql.identifiers property to NEVER, then current implementation of the connector does not honor it in all cases. Method calls which build the SQL statement in code are using the ExpressionBuilder class to correctly quote the table names. However, method calls that directly invoke a jdbc method with the table name do not take this into account. They are case sensitive

Solution

Overrided parseTableIdentifier method for oracle and postgres dialect, to check for config value and make the whole table name uppercase in case of Oracle and lowercase in case of Postgres. Note: This issue might be prevalent in other dialects as well but this PR is only addressing these 2 dialects for now.

Also bumped postgres-embedding library version used in integration tests to make it compatible with m1 macs.

Does this solution apply anywhere else?
If yes, where?

Test Strategy

Testing done:

Release Plan

[CCDB-2031]: https://confluentinc.atlassian.net/browse/CCDB-2031?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
cla-assistant[bot] commented 11 months ago

CLA assistant check
All committers have signed the CLA.

ManasjyotiSharma commented 11 months ago

Hi @Tanish0019 Thanks for writing the PR. Can you please add corresponding UTs for each of the 2 impacted dialects?

Tanish0019 commented 11 months ago

@ManasjyotiSharma Yes, will be doing some manual testing as well and updating the tests soon. Meanwhile, can you take a look if the fix looks okay?

Tanish0019 commented 11 months ago

@ManasjyotiSharma I have added the tests, can you please take a look? Thanks

ManasjyotiSharma commented 11 months ago

@Tanish0019 There seems to be test failures; please take a look & do the needful before merging:

18:36:09 [INFO] 18:36:09 [ERROR] Errors: 18:36:09 [ERROR] OracleDatatypeIT.testQuoteIdentifierAlwaysConfig:306->BaseConnectorIT.waitForCommittedRecords:152->BaseConnectorIT.lambda$waitForCommittedRecords$2:164 » NoRetry 18:36:09 [ERROR] OracleDatatypeIT.testQuoteIdentifierNeverConfig:269->BaseConnectorIT.waitForCommittedRecords:152->BaseConnectorIT.lambda$waitForCommittedRecords$2:164 » NoRetry 18:36:09 [INFO] 18:36:09 [ERROR] Tests run: 22, Failures: 0, Errors: 2, Skipped: 0

Tanish0019 commented 11 months ago

@ManasjyotiSharma thanks for reviewing. I am aware why tests are failing. Will fix and then merge.