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

[SPARK-48172][SQL][FOLLOWUP] Fix escaping issues in JDBCDialects #46807

Closed
wants to merge 3 commits into from

Conversation

mihailom-db
Copy link
Contributor

What changes were proposed in this pull request?

Removal of stripMargin from the code in DockerJDBCIntegrationV2Suite.

Why are the changes needed?

#46588
Given PR was merged to master/3.5/3.4. This PR broke daily jobs for OracleIntegrationSuite. Upon inspection, it was noted that 3.4 and 3.5 are run with JDK8 while master is run with JDK21 and stripMargin was behaving differently in those cases. Upon removing stripMargin and spliting INSERT INTO statements into multiple lines, all integration tests have passed.

Does this PR introduce any user-facing change?

No, only loading of the test data was changed to follow language requirements.

How was this patch tested?

Existing suite was aborted in the job and now it is running.

Was this patch authored or co-authored using generative AI tooling?

No

Closes #46806

@github-actions github-actions bot added the SQL label May 30, 2024
@mihailom-db
Copy link
Contributor Author

@milastdbx Please take a look at this PR, problems were in #46588 as 3.4 and 3.5 are run with different JDK version.

connection.prepareStatement("INSERT INTO pattern_testing_table "
+ "VALUES ('special_character_quote''_present')")
.executeUpdate()
connection.prepareStatement("INSERT INTO pattern_testing_table "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can these *not* values be collapsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collapsing them now would require changes in the ordering of results of tests in V2JDBCTest, as collapsing might reorder inputs, so maybe it is better to keep this change scoped to fixing the JDK problem for CI runs.

@yaooqinn yaooqinn changed the title [FOLLOWUP][SPARK-48172][SQL] Fix escaping issues in JDBCDialects [SPARK-48172][SQL][FOLLOWUP] Fix escaping issues in JDBCDialects May 31, 2024
@yaooqinn yaooqinn closed this in 4360ec7 May 31, 2024
yaooqinn pushed a commit that referenced this pull request May 31, 2024
### What changes were proposed in this pull request?
Removal of stripMargin from the code in `DockerJDBCIntegrationV2Suite`.

### Why are the changes needed?
#46588
Given PR was merged to master/3.5/3.4. This PR broke daily jobs for `OracleIntegrationSuite`. Upon inspection, it was noted that 3.4 and 3.5 are run with JDK8 while master is run with JDK21 and stripMargin was behaving differently in those cases. Upon removing stripMargin and spliting `INSERT INTO` statements into multiple lines, all integration tests have passed.

### Does this PR introduce _any_ user-facing change?
No, only loading of the test data was changed to follow language requirements.

### How was this patch tested?
Existing suite was aborted in the job and now it is running.

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #46806

Closes #46807 from mihailom-db/FixOracleMaster.

Authored-by: Mihailo Milosevic <mihailo.milosevic@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit 4360ec7)
Signed-off-by: Kent Yao <yao@apache.org>
yaooqinn pushed a commit that referenced this pull request May 31, 2024
### What changes were proposed in this pull request?
Removal of stripMargin from the code in `DockerJDBCIntegrationV2Suite`.

### Why are the changes needed?
#46588
Given PR was merged to master/3.5/3.4. This PR broke daily jobs for `OracleIntegrationSuite`. Upon inspection, it was noted that 3.4 and 3.5 are run with JDK8 while master is run with JDK21 and stripMargin was behaving differently in those cases. Upon removing stripMargin and spliting `INSERT INTO` statements into multiple lines, all integration tests have passed.

### Does this PR introduce _any_ user-facing change?
No, only loading of the test data was changed to follow language requirements.

### How was this patch tested?
Existing suite was aborted in the job and now it is running.

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #46806

Closes #46807 from mihailom-db/FixOracleMaster.

Authored-by: Mihailo Milosevic <mihailo.milosevic@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit 4360ec7)
Signed-off-by: Kent Yao <yao@apache.org>
@yaooqinn
Copy link
Member

Merged to master/3.5/3.4. Thank you @mihailom-db

riyaverm-db pushed a commit to riyaverm-db/spark that referenced this pull request Jun 7, 2024
### What changes were proposed in this pull request?
Removal of stripMargin from the code in `DockerJDBCIntegrationV2Suite`.

### Why are the changes needed?
apache#46588
Given PR was merged to master/3.5/3.4. This PR broke daily jobs for `OracleIntegrationSuite`. Upon inspection, it was noted that 3.4 and 3.5 are run with JDK8 while master is run with JDK21 and stripMargin was behaving differently in those cases. Upon removing stripMargin and spliting `INSERT INTO` statements into multiple lines, all integration tests have passed.

### Does this PR introduce _any_ user-facing change?
No, only loading of the test data was changed to follow language requirements.

### How was this patch tested?
Existing suite was aborted in the job and now it is running.

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#46806

Closes apache#46807 from mihailom-db/FixOracleMaster.

Authored-by: Mihailo Milosevic <mihailo.milosevic@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
szehon-ho pushed a commit to szehon-ho/spark that referenced this pull request Aug 7, 2024
### What changes were proposed in this pull request?
Removal of stripMargin from the code in `DockerJDBCIntegrationV2Suite`.

### Why are the changes needed?
apache#46588
Given PR was merged to master/3.5/3.4. This PR broke daily jobs for `OracleIntegrationSuite`. Upon inspection, it was noted that 3.4 and 3.5 are run with JDK8 while master is run with JDK21 and stripMargin was behaving differently in those cases. Upon removing stripMargin and spliting `INSERT INTO` statements into multiple lines, all integration tests have passed.

### Does this PR introduce _any_ user-facing change?
No, only loading of the test data was changed to follow language requirements.

### How was this patch tested?
Existing suite was aborted in the job and now it is running.

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#46806

Closes apache#46807 from mihailom-db/FixOracleMaster.

Authored-by: Mihailo Milosevic <mihailo.milosevic@databricks.com>
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit 4360ec7)
Signed-off-by: Kent Yao <yao@apache.org>
(cherry picked from commit 1d74d9dd0f832a5e5c03f6b808cb3a3b1b379556)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants