[SPARK-30826][SQL] Respect reference case in `StringStartsWith` pushed down to parquet by MaxGekk · Pull Request #27574 · apache/spark

MaxGekk · 2020-02-14T06:40:30Z

What changes were proposed in this pull request?

In the PR, I propose to convert the attribute name of StringStartsWith pushed down to the Parquet datasource to column reference via the nameToParquetField map. Similar conversions are performed for other source filters pushed down to parquet.

Why are the changes needed?

This fixes the bug described in SPARK-30826. The query from an external table:

CREATE TABLE t1 (col STRING)
USING parquet
OPTIONS (path '$path')

created on top of written parquet files by Seq("42").toDF("COL").write.parquet(path) returns wrong empty result:

spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show
+---+
|col|
+---+
+---+

Does this PR introduce any user-facing change?

Yes. After the changes the result is correct for the example above:

spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show
+---+
|col|
+---+
| 42|
+---+

How was this patch tested?

Added a test to ParquetFilterSuite

SparkQA · 2020-02-14T08:05:01Z

Test build #118396 has finished for PR 27574 at commit 5e632df.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2020-02-14T08:06:29Z

jenkins, retest this, please

Ngone51

LGTM. It'd be better if we can reduce the scope of PR title to make it more compatible with the fix.

cloud-fan · 2020-02-14T11:15:09Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala

          if pushDownStartWith && canMakeFilterOn(name, prefix) =>
        Option(prefix).map { v =>
-          FilterApi.userDefined(binaryColumn(name),
+          FilterApi.userDefined(binaryColumn(nameToParquetField(name).fieldName),


good catch!

SparkQA · 2020-02-14T12:55:52Z

Test build #118401 has finished for PR 27574 at commit 5e632df.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-02-14T14:29:13Z

does 2.4 also have this bug?

MaxGekk · 2020-02-14T15:10:32Z

@cloud-fan Yes, 2.4 has the bug too.

…d down to parquet ### What changes were proposed in this pull request? In the PR, I propose to convert the attribute name of `StringStartsWith` pushed down to the Parquet datasource to column reference via the `nameToParquetField` map. Similar conversions are performed for other source filters pushed down to parquet. ### Why are the changes needed? This fixes the bug described in [SPARK-30826](https://issues.apache.org/jira/browse/SPARK-30826). The query from an external table: ```sql CREATE TABLE t1 (col STRING) USING parquet OPTIONS (path '$path') ``` created on top of written parquet files by `Seq("42").toDF("COL").write.parquet(path)` returns wrong empty result: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ +---+ ``` ### Does this PR introduce any user-facing change? Yes. After the changes the result is correct for the example above: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ | 42| +---+ ``` ### How was this patch tested? Added a test to `ParquetFilterSuite` Closes #27574 from MaxGekk/parquet-StringStartsWith-case-sens. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 8b73b92) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

cloud-fan · 2020-02-15T12:11:40Z

thanks, merging to master/3.0/2.4!

…d down to parquet ### What changes were proposed in this pull request? In the PR, I propose to convert the attribute name of `StringStartsWith` pushed down to the Parquet datasource to column reference via the `nameToParquetField` map. Similar conversions are performed for other source filters pushed down to parquet. ### Why are the changes needed? This fixes the bug described in [SPARK-30826](https://issues.apache.org/jira/browse/SPARK-30826). The query from an external table: ```sql CREATE TABLE t1 (col STRING) USING parquet OPTIONS (path '$path') ``` created on top of written parquet files by `Seq("42").toDF("COL").write.parquet(path)` returns wrong empty result: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ +---+ ``` ### Does this PR introduce any user-facing change? Yes. After the changes the result is correct for the example above: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ | 42| +---+ ``` ### How was this patch tested? Added a test to `ParquetFilterSuite` Closes #27574 from MaxGekk/parquet-StringStartsWith-case-sens. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 8b73b92) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

HyukjinKwon

Nice. LGTM

…d down to parquet ### What changes were proposed in this pull request? In the PR, I propose to convert the attribute name of `StringStartsWith` pushed down to the Parquet datasource to column reference via the `nameToParquetField` map. Similar conversions are performed for other source filters pushed down to parquet. ### Why are the changes needed? This fixes the bug described in [SPARK-30826](https://issues.apache.org/jira/browse/SPARK-30826). The query from an external table: ```sql CREATE TABLE t1 (col STRING) USING parquet OPTIONS (path '$path') ``` created on top of written parquet files by `Seq("42").toDF("COL").write.parquet(path)` returns wrong empty result: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ +---+ ``` ### Does this PR introduce any user-facing change? Yes. After the changes the result is correct for the example above: ```scala spark.sql("SELECT * FROM t1 WHERE col LIKE '4%'").show +---+ |col| +---+ | 42| +---+ ``` ### How was this patch tested? Added a test to `ParquetFilterSuite` Closes apache#27574 from MaxGekk/parquet-StringStartsWith-case-sens. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

MaxGekk added 3 commits February 14, 2020 09:08

Add a test

0e2779c

Bug fix

cb4c50c

Add JIRA id to test title

5e632df

Ngone51 approved these changes Feb 14, 2020

View reviewed changes

MaxGekk changed the title ~~[SPARK-30826][SQL] Fix wrong result after the StringStartsWith filter pushed down to parquet~~ [SPARK-30826][SQL] Respect reference case in StringStartsWith pushed down to parquet Feb 14, 2020

MaxGekk requested review from cloud-fan, gengliangwang and wangyum February 14, 2020 11:11

cloud-fan reviewed Feb 14, 2020

View reviewed changes

cloud-fan approved these changes Feb 14, 2020

View reviewed changes

wangyum approved these changes Feb 14, 2020

View reviewed changes

cloud-fan closed this in 8b73b92 Feb 15, 2020

HyukjinKwon reviewed Feb 17, 2020

View reviewed changes

MaxGekk deleted the parquet-StringStartsWith-case-sens branch June 5, 2020 19:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-30826][SQL] Respect reference case in `StringStartsWith` pushed down to parquet#27574

[SPARK-30826][SQL] Respect reference case in `StringStartsWith` pushed down to parquet#27574
MaxGekk wants to merge 3 commits intoapache:masterfrom
MaxGekk:parquet-StringStartsWith-case-sens

MaxGekk commented Feb 14, 2020

Uh oh!

SparkQA commented Feb 14, 2020

Uh oh!

MaxGekk commented Feb 14, 2020

Uh oh!

Ngone51 left a comment

Uh oh!

cloud-fan Feb 14, 2020

Uh oh!

SparkQA commented Feb 14, 2020

Uh oh!

cloud-fan commented Feb 14, 2020

Uh oh!

MaxGekk commented Feb 14, 2020

Uh oh!

cloud-fan commented Feb 15, 2020

Uh oh!

HyukjinKwon left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

MaxGekk commented Feb 14, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Feb 14, 2020

Uh oh!

MaxGekk commented Feb 14, 2020

Uh oh!

Ngone51 left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 14, 2020

Uh oh!

cloud-fan commented Feb 14, 2020

Uh oh!

MaxGekk commented Feb 14, 2020

Uh oh!

cloud-fan commented Feb 15, 2020

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants