spark icon indicating copy to clipboard operation
spark copied to clipboard

[SPARK-48172][SQL] Fix escaping issues in JDBC Dialects

Open mihailom-db opened this issue 1 year ago • 3 comments

What changes were proposed in this pull request?

Special case escaping for MySQL and fix issues with redundant escaping for ' character.

Why are the changes needed?

When pushing down startsWith, endsWith and contains they are converted to LIKE. This requires addition of escape characters for these expressions. Unfortunately, MySQL uses ESCAPE '\' syntax instead of ESCAPE '' which would cause errors when trying to push down.

Does this PR introduce any user-facing change?

Yes

How was this patch tested?

Tests for each existing dialect.

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

No.

mihailom-db avatar May 07 '24 12:05 mihailom-db

Existing tests.

I believe that it cannot be included under the current coverage. Can we add some new tests?

yaooqinn avatar May 07 '24 12:05 yaooqinn

@cloud-fan Could you take a look at this fix?

mihailom-db avatar May 09 '24 09:05 mihailom-db

LGTM except @cloud-fan 's comment.

beliefer avatar May 11 '24 03:05 beliefer

merged to master/3.5/3.4!

cloud-fan avatar May 14 '24 15:05 cloud-fan

@mihailom-db Is the GA failure related to this pr? image https://github.com/panbingkun/spark/actions/runs/9086615518/job/24972712440

panbingkun avatar May 14 '24 23:05 panbingkun

From the GA's results, it seems that only MySQL is good, and everything else has problems, not only the support of data type LONGTEXT cc @beliefer @yaooqinn @HyukjinKwon @cloud-fan

panbingkun avatar May 15 '24 01:05 panbingkun

When I change the data type from LONGTEXT to varchar(100), it seems that there are different problems in MsSqlServerIntegrationSuite, DB2IntegrationSuite, PostgresIntegrationSuite , eg: image image image

panbingkun avatar May 15 '24 01:05 panbingkun

Thank @panbingkun,I reverted this in https://github.com/apache/spark/commit/4ff5ca81cffbd1940c864144ca8fbba54b605e4e

yaooqinn avatar May 15 '24 02:05 yaooqinn

Thank @panbingkun,I reverted this in 4ff5ca8

Thanks! @yaooqinn

@mihailom-db After fix it, you can resubmit :)

panbingkun avatar May 15 '24 02:05 panbingkun