[SPARK-28109][SQL] Fix TRIM(type trimStr FROM str) returns incorrect value #24911

wangyum · 2019-06-19T16:09:41Z

What changes were proposed in this pull request?

SPARK-28093 fixed TRIM/LTRIM/RTRIM('str', 'trimStr') returns an incorrect value, but that fix introduced a new bug, TRIM(type trimStr FROM str) returns an incorrect value. This pr fix this issue.

How was this patch tested?

unit tests and manual tests:
Before this PR:

spark-sql> SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx');
Tom	z
spark-sql> SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx');
bar
spark-sql> SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest');
test	xyz
spark-sql> SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz');
testxyz
spark-sql> SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD');
XxyLAST WORD
spark-sql> SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx');
test	xy
spark-sql> SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx');
xyztest
spark-sql> SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy');
TURNERyxX

After this PR:

spark-sql> SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx');
Tom     Tom
spark-sql> SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx');
bar     bar
spark-sql> SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest');
test    test
spark-sql> SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz');
testxyz testxyz
spark-sql> SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD');
XxyLAST WORD    XxyLAST WORD
spark-sql> SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx');
test    test
spark-sql> SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx');
xyztest xyztest
spark-sql> SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy');
TURNERyxX       TURNERyxX

And PostgreSQL:

postgres=# SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx');
 btrim | btrim
-------+-------
 Tom   | Tom
(1 row)

postgres=# SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx');
 btrim | btrim
-------+-------
 bar   | bar
(1 row)

postgres=# SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest');
 ltrim | ltrim
-------+-------
 test  | test
(1 row)

postgres=# SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz');
  ltrim  |  ltrim
---------+---------
 testxyz | testxyz
(1 row)

postgres=# SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD');
    ltrim     |    ltrim
--------------+--------------
 XxyLAST WORD | XxyLAST WORD
(1 row)

postgres=# SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx');
 rtrim | rtrim
-------+-------
 test  | test
(1 row)

postgres=# SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx');
  rtrim  |  rtrim
---------+---------
 xyztest | xyztest
(1 row)

postgres=# SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy');
   rtrim   |   rtrim
-----------+-----------
 TURNERyxX | TURNERyxX
(1 row)

dongjoon-hyun · 2019-06-19T16:36:08Z

@wangyum . This is about new syntax. This is irrelevant to branch-2.4/2.3 PR, isn't it?

dongjoon-hyun · 2019-06-19T16:42:22Z

~~The new bug is only for SPARK-28075 at master branch, right?~~
Got it. This is not about new one.

dongjoon-hyun · 2019-06-19T16:50:56Z

cc @gatorsmile

dongjoon-hyun · 2019-06-19T17:01:59Z

sql/core/src/test/resources/sql-tests/inputs/string-functions.sql

+SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD');
+SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx');
+SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx');
+SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy');


Thank you for the new test coverage!

SparkQA · 2019-06-19T19:24:51Z

Test build #106680 has finished for PR 24911 at commit 57cb08a.

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

dongjoon-hyun · 2019-06-19T19:44:27Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala


-    assertEqual(
+    intercept("select ltrim(both 'S' from 'SS abc S'", "mismatched input 'from' expecting {')'")
+    intercept("select rtrim(trailing 'S' from 'SS abc S'", "mismatched input 'from' expecting {')'")


This is the change of error message, but it looks okay. The previous error message is the following.

spark-sql> select ltrim(both 'S' from 'SS abc S'; Error in query: missing ')' at '<EOF>'(line 1, pos 37) == SQL == select ltrim(both 'S' from 'SS abc S' -------------------------------------^^^

dongjoon-hyun

+1, LGTM.
Merged to master.

dongjoon-hyun · 2019-06-19T19:47:11Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

-      } else {
-        funcID
-      }
-    }


The old one was a hacky way.

…rameter order issue ## What changes were proposed in this pull request? This pr backport #24902 and #24911 to branch-2.4. ## How was this patch tested? unit tests Closes #24907 from wangyum/SPARK-28093-branch-2.4. Authored-by: Yuming Wang <yumwang@ebay.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…rameter order issue ## What changes were proposed in this pull request? This pr backport #24902 and #24911 to branch-2.3. ## How was this patch tested? unit tests Closes #24908 from wangyum/SPARK-28093-branch-2.3. Authored-by: Yuming Wang <yumwang@ebay.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…value ## What changes were proposed in this pull request? [SPARK-28093](https://issues.apache.org/jira/browse/SPARK-28093) fixed `TRIM/LTRIM/RTRIM('str', 'trimStr')` returns an incorrect value, but that fix introduced a new bug, `TRIM(type trimStr FROM str)` returns an incorrect value. This pr fix this issue. ## How was this patch tested? unit tests and manual tests: Before this PR: ```sql spark-sql> SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx'); Tom z spark-sql> SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx'); bar spark-sql> SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest'); test xyz spark-sql> SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz'); testxyz spark-sql> SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD'); XxyLAST WORD spark-sql> SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx'); test xy spark-sql> SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx'); xyztest spark-sql> SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy'); TURNERyxX ``` After this PR: ```sql spark-sql> SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx'); Tom Tom spark-sql> SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx'); bar bar spark-sql> SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest'); test test spark-sql> SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz'); testxyz testxyz spark-sql> SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD'); XxyLAST WORD XxyLAST WORD spark-sql> SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx'); test test spark-sql> SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx'); xyztest xyztest spark-sql> SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy'); TURNERyxX TURNERyxX ``` And PostgreSQL: ```sql postgres=# SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx'); btrim | btrim -------+------- Tom | Tom (1 row) postgres=# SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx'); btrim | btrim -------+------- bar | bar (1 row) postgres=# SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest'); ltrim | ltrim -------+------- test | test (1 row) postgres=# SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz'); ltrim | ltrim ---------+--------- testxyz | testxyz (1 row) postgres=# SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST WORD'); ltrim | ltrim --------------+-------------- XxyLAST WORD | XxyLAST WORD (1 row) postgres=# SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx'); rtrim | rtrim -------+------- test | test (1 row) postgres=# SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx'); rtrim | rtrim ---------+--------- xyztest | xyztest (1 row) postgres=# SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy'); rtrim | rtrim -----------+----------- TURNERyxX | TURNERyxX (1 row) ``` Closes apache#24911 from wangyum/SPARK-28109. Authored-by: Yuming Wang <yumwang@ebay.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…rameter order issue ## What changes were proposed in this pull request? This pr backport apache#24902 and apache#24911 to branch-2.4. ## How was this patch tested? unit tests Closes apache#24907 from wangyum/SPARK-28093-branch-2.4. Authored-by: Yuming Wang <yumwang@ebay.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

Create a Trim expression

57cb08a

dongjoon-hyun reviewed Jun 19, 2019

View reviewed changes

dongjoon-hyun approved these changes Jun 19, 2019

View reviewed changes

dongjoon-hyun reviewed Jun 19, 2019

View reviewed changes

dongjoon-hyun mentioned this pull request Jun 19, 2019

[SPARK-28093][SPARK-28109][SQL][2.4] Fix TRIM/LTRIM/RTRIM function parameter order issue #24907

Closed

dongjoon-hyun closed this in fe5145e Jun 19, 2019

wangyum deleted the SPARK-28109 branch June 20, 2019 08:26

wangyum mentioned this pull request Jun 21, 2019

[SPARK-28093][SPARK-28109][SQL][2.3] Fix TRIM/LTRIM/RTRIM function parameter order issue #24908

Closed

dongjoon-hyun added the SQL label Feb 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-28109][SQL] Fix TRIM(type trimStr FROM str) returns incorrect value #24911

[SPARK-28109][SQL] Fix TRIM(type trimStr FROM str) returns incorrect value #24911

wangyum commented Jun 19, 2019 •

edited

Loading

dongjoon-hyun commented Jun 19, 2019

dongjoon-hyun commented Jun 19, 2019 •

edited

Loading

dongjoon-hyun commented Jun 19, 2019

dongjoon-hyun Jun 19, 2019

SparkQA commented Jun 19, 2019

dongjoon-hyun Jun 19, 2019 •

edited

Loading

dongjoon-hyun left a comment

dongjoon-hyun Jun 19, 2019

[SPARK-28109][SQL] Fix TRIM(type trimStr FROM str) returns incorrect value #24911

[SPARK-28109][SQL] Fix TRIM(type trimStr FROM str) returns incorrect value #24911

Conversation

wangyum commented Jun 19, 2019 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

dongjoon-hyun commented Jun 19, 2019

dongjoon-hyun commented Jun 19, 2019 • edited Loading

dongjoon-hyun commented Jun 19, 2019

dongjoon-hyun Jun 19, 2019

Choose a reason for hiding this comment

SparkQA commented Jun 19, 2019

dongjoon-hyun Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun Jun 19, 2019

Choose a reason for hiding this comment

wangyum commented Jun 19, 2019 •

edited

Loading

dongjoon-hyun commented Jun 19, 2019 •

edited

Loading

dongjoon-hyun Jun 19, 2019 •

edited

Loading