This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-2.3 by this push:
new 3a0b0c1 Revert "[SPARK-28093][SPARK-28109][SQL][2.3] Fix
TRIM/LTRIM/RTRIM function parameter order issue"
3a0b0c1 is described below
commit 3a0b0c186f28d71168a2fd965947dd76d10cecaa
Author: Yuming Wang <[email protected]>
AuthorDate: Mon Jun 24 09:05:31 2019 -0700
Revert "[SPARK-28093][SPARK-28109][SQL][2.3] Fix TRIM/LTRIM/RTRIM function
parameter order issue"
## What changes were proposed in this pull request?
This reverts commit 1201a0a7.
The parameter orders might be different in different vendors. We can change
it in 3.x to make it more consistent with the other vendors. However, it could
break the existing customers' workloads if they already use trim.
## How was this patch tested?
N/A
Closes #24944 from wangyum/SPARK-28093-REVERT-2.3.
Authored-by: Yuming Wang <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../apache/spark/sql/catalyst/parser/SqlBase.g4 | 6 +-
.../catalyst/expressions/stringExpressions.scala | 6 +-
.../spark/sql/catalyst/parser/AstBuilder.scala | 43 +++++++-------
.../expressions/StringExpressionsSuite.scala | 11 ----
.../sql/catalyst/parser/PlanParserSuite.scala | 24 +++-----
.../parser/TableIdentifierParserSuite.scala | 2 +-
.../sql-tests/inputs/string-functions.sql | 12 +---
.../sql-tests/results/string-functions.sql.out | 66 +---------------------
8 files changed, 40 insertions(+), 130 deletions(-)
diff --git
a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
index 15fd48b..5fa75fe 100644
---
a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
+++
b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
@@ -582,12 +582,12 @@ primaryExpression
| '(' query ')'
#subqueryExpression
| qualifiedName '(' (setQuantifier? argument+=expression (','
argument+=expression)*)? ')'
(OVER windowSpec)?
#functionCall
+ | qualifiedName '(' trimOption=(BOTH | LEADING | TRAILING)
argument+=expression
+ FROM argument+=expression ')'
#functionCall
| value=primaryExpression '[' index=valueExpression ']'
#subscript
| identifier
#columnReference
| base=primaryExpression '.' fieldName=identifier
#dereference
| '(' expression ')'
#parenthesizedExpression
- | TRIM '(' trimOption=(BOTH | LEADING | TRAILING)
(trimStr=valueExpression)?
- FROM srcStr=valueExpression ')'
#trim
;
constant
@@ -735,7 +735,6 @@ nonReserved
| VIEW | REPLACE
| IF
| POSITION
- | TRIM
| NO | DATA
| START | TRANSACTION | COMMIT | ROLLBACK | IGNORE
| SORT | CLUSTER | DISTRIBUTE | UNSET | TBLPROPERTIES | SKEWED | STORED |
DIRECTORIES | LOCATION
@@ -873,7 +872,6 @@ TRAILING: 'TRAILING';
IF: 'IF';
POSITION: 'POSITION';
-TRIM: 'TRIM';
EQ : '=' | '==';
NSEQ: '<=>';
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
index 1166e77..c855581 100755
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
@@ -714,7 +714,7 @@ case class StringTrim(
trimStr: Option[Expression] = None)
extends String2TrimExpression {
- def this(srcStr: Expression, trimStr: Expression) = this(srcStr,
Option(trimStr))
+ def this(trimStr: Expression, srcStr: Expression) = this(srcStr,
Option(trimStr))
def this(srcStr: Expression) = this(srcStr, None)
@@ -814,7 +814,7 @@ case class StringTrimLeft(
trimStr: Option[Expression] = None)
extends String2TrimExpression {
- def this(srcStr: Expression, trimStr: Expression) = this(srcStr,
Option(trimStr))
+ def this(trimStr: Expression, srcStr: Expression) = this(srcStr,
Option(trimStr))
def this(srcStr: Expression) = this(srcStr, None)
@@ -917,7 +917,7 @@ case class StringTrimRight(
trimStr: Option[Expression] = None)
extends String2TrimExpression {
- def this(srcStr: Expression, trimStr: Expression) = this(srcStr,
Option(trimStr))
+ def this(trimStr: Expression, srcStr: Expression) = this(srcStr,
Option(trimStr))
def this(srcStr: Expression) = this(srcStr, None)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
index 418e029..bdc357d 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
@@ -1186,28 +1186,29 @@ class AstBuilder(conf: SQLConf) extends
SqlBaseBaseVisitor[AnyRef] with Logging
}
/**
- * Create a Trim expression.
- */
- override def visitTrim(ctx: TrimContext): Expression = withOrigin(ctx) {
- val srcStr = expression(ctx.srcStr)
- val trimStr = Option(ctx.trimStr).map(expression)
- ctx.trimOption.getType match {
- case SqlBaseParser.BOTH =>
- StringTrim(srcStr, trimStr)
- case SqlBaseParser.LEADING =>
- StringTrimLeft(srcStr, trimStr)
- case SqlBaseParser.TRAILING =>
- StringTrimRight(srcStr, trimStr)
- case other =>
- throw new ParseException("Function trim doesn't support with " +
- s"type $other. Please use BOTH, LEADING or TRAILING as trim type",
ctx)
- }
- }
-
- /**
* Create a (windowed) Function expression.
*/
override def visitFunctionCall(ctx: FunctionCallContext): Expression =
withOrigin(ctx) {
+ def replaceFunctions(
+ funcID: FunctionIdentifier,
+ ctx: FunctionCallContext): FunctionIdentifier = {
+ val opt = ctx.trimOption
+ if (opt != null) {
+ if (ctx.qualifiedName.getText.toLowerCase(Locale.ROOT) != "trim") {
+ throw new ParseException(s"The specified function
${ctx.qualifiedName.getText} " +
+ s"doesn't support with option ${opt.getText}.", ctx)
+ }
+ opt.getType match {
+ case SqlBaseParser.BOTH => funcID
+ case SqlBaseParser.LEADING => funcID.copy(funcName = "ltrim")
+ case SqlBaseParser.TRAILING => funcID.copy(funcName = "rtrim")
+ case _ => throw new ParseException("Function trim doesn't support
with " +
+ s"type ${opt.getType}. Please use BOTH, LEADING or Trailing as
trim type", ctx)
+ }
+ } else {
+ funcID
+ }
+ }
// Create the function call.
val name = ctx.qualifiedName.getText
val isDistinct = Option(ctx.setQuantifier()).exists(_.DISTINCT != null)
@@ -1219,7 +1220,9 @@ class AstBuilder(conf: SQLConf) extends
SqlBaseBaseVisitor[AnyRef] with Logging
case expressions =>
expressions
}
- val function = UnresolvedFunction(visitFunctionName(ctx.qualifiedName),
arguments, isDistinct)
+ val funcId = replaceFunctions(visitFunctionName(ctx.qualifiedName), ctx)
+ val function = UnresolvedFunction(funcId, arguments, isDistinct)
+
// Check if the function is evaluated in a windowed context.
ctx.windowSpec match {
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
index 45b93ec..97ddbeb 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
@@ -466,9 +466,6 @@ class StringExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
// scalastyle:on
checkEvaluation(StringTrim(Literal("a"), Literal.create(null,
StringType)), null)
checkEvaluation(StringTrim(Literal.create(null, StringType),
Literal("a")), null)
-
- checkEvaluation(StringTrim(Literal("yxTomxx"), Literal("xyz")), "Tom")
- checkEvaluation(StringTrim(Literal("xxxbarxxx"), Literal("x")), "bar")
}
test("LTRIM") {
@@ -493,10 +490,6 @@ class StringExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
// scalastyle:on
checkEvaluation(StringTrimLeft(Literal.create(null, StringType),
Literal("a")), null)
checkEvaluation(StringTrimLeft(Literal("a"), Literal.create(null,
StringType)), null)
-
- checkEvaluation(StringTrimLeft(Literal("zzzytest"), Literal("xyz")),
"test")
- checkEvaluation(StringTrimLeft(Literal("zzzytestxyz"), Literal("xyz")),
"testxyz")
- checkEvaluation(StringTrimLeft(Literal("xyxXxyLAST WORD"), Literal("xy")),
"XxyLAST WORD")
}
test("RTRIM") {
@@ -522,10 +515,6 @@ class StringExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
// scalastyle:on
checkEvaluation(StringTrimRight(Literal("a"), Literal.create(null,
StringType)), null)
checkEvaluation(StringTrimRight(Literal.create(null, StringType),
Literal("a")), null)
-
- checkEvaluation(StringTrimRight(Literal("testxxzx"), Literal("xyz")),
"test")
- checkEvaluation(StringTrimRight(Literal("xyztestxxzx"), Literal("xyz")),
"xyztest")
- checkEvaluation(StringTrimRight(Literal("TURNERyxXxy"), Literal("xy")),
"TURNERyxX")
}
test("FORMAT") {
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala
index 687f5fa..812bfdd 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala
@@ -18,7 +18,7 @@
package org.apache.spark.sql.catalyst.parser
import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
-import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, UnresolvedAlias,
UnresolvedAttribute, UnresolvedFunction, UnresolvedGenerator,
UnresolvedInlineTable, UnresolvedRelation, UnresolvedSubqueryColumnAliases,
UnresolvedTableValuedFunction}
+import org.apache.spark.sql.catalyst.analysis.{AnalysisTest,
UnresolvedAttribute, UnresolvedFunction, UnresolvedGenerator,
UnresolvedInlineTable, UnresolvedRelation, UnresolvedSubqueryColumnAliases,
UnresolvedTableValuedFunction}
import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.plans._
import org.apache.spark.sql.catalyst.plans.logical._
@@ -652,26 +652,20 @@ class PlanParserSuite extends AnalysisTest {
}
test("TRIM function") {
- def assertTrimPlans(inputSQL: String, expectedExpression: Expression):
Unit = {
- comparePlans(
- parsePlan(inputSQL),
- Project(Seq(UnresolvedAlias(expectedExpression)), OneRowRelation())
- )
- }
- 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 {')'")
+ intercept("select ltrim(both 'S' from 'SS abc S'", "missing ')' at
'<EOF>'")
+ intercept("select rtrim(trailing 'S' from 'SS abc S'", "missing ')' at
'<EOF>'")
- assertTrimPlans(
+ assertEqual(
"SELECT TRIM(BOTH '@$%&( )abc' FROM '@ $ % & ()abc ' )",
- StringTrim(Literal("@ $ % & ()abc "), Some(Literal("@$%&( )abc")))
+ OneRowRelation().select('TRIM.function("@$%&( )abc", "@ $ % & ()abc "))
)
- assertTrimPlans(
+ assertEqual(
"SELECT TRIM(LEADING 'c []' FROM '[ ccccbcc ')",
- StringTrimLeft(Literal("[ ccccbcc "), Some(Literal("c []")))
+ OneRowRelation().select('ltrim.function("c []", "[ ccccbcc "))
)
- assertTrimPlans(
+ assertEqual(
"SELECT TRIM(TRAILING 'c&^,.' FROM 'bc...,,,&&&ccc')",
- StringTrimRight(Literal("bc...,,,&&&ccc"), Some(Literal("c&^,.")))
+ OneRowRelation().select('rtrim.function("c&^,.", "bc...,,,&&&ccc"))
)
}
}
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
index 50e17b1..cc80a41 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala
@@ -51,7 +51,7 @@ class TableIdentifierParserSuite extends SparkFunSuite {
"rollup", "row", "rows", "set", "smallint", "table", "timestamp", "to",
"trigger",
"true", "truncate", "update", "user", "values", "with", "regexp", "rlike",
"bigint", "binary", "boolean", "current_date", "current_timestamp",
"date", "double", "float",
- "int", "smallint", "timestamp", "at", "position", "both", "leading",
"trailing", "trim")
+ "int", "smallint", "timestamp", "at", "position", "both", "leading",
"trailing")
val hiveStrictNonReservedKeyword = Seq("anti", "full", "inner", "left",
"semi", "right",
"natural", "union", "intersect", "except", "database", "on", "join",
"cross", "select", "from",
diff --git a/sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
b/sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
index f63dbec..4113734 100644
--- a/sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
+++ b/sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
@@ -46,14 +46,4 @@ FROM (
encode(string(id + 2), 'utf-8') col3,
encode(string(id + 3), 'utf-8') col4
FROM range(10)
-);
-
--- trim/ltrim/rtrim
-SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx');
-SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx');
-SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest');
-SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz');
-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');
+)
diff --git
a/sql/core/src/test/resources/sql-tests/results/string-functions.sql.out
b/sql/core/src/test/resources/sql-tests/results/string-functions.sql.out
index aace15f..d5f8705 100644
--- a/sql/core/src/test/resources/sql-tests/results/string-functions.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/string-functions.sql.out
@@ -1,5 +1,5 @@
-- Automatically generated by SQLQueryTestSuite
--- Number of queries: 23
+-- Number of queries: 15
-- !query 0
@@ -161,67 +161,3 @@ struct<plan:string>
== Physical Plan ==
*Project [concat(cast(id#xL as string), cast(encode(cast((id#xL + 2) as
string), utf-8) as string), cast(encode(cast((id#xL + 3) as string), utf-8) as
string)) AS col#x]
+- *Range (0, 10, step=1, splits=2)
-
-
--- !query 15
-SELECT trim('yxTomxx', 'xyz'), trim(BOTH 'xyz' FROM 'yxTomxx')
--- !query 15 schema
-struct<trim(yxTomxx, xyz):string,trim(yxTomxx, xyz):string>
--- !query 15 output
-Tom Tom
-
-
--- !query 16
-SELECT trim('xxxbarxxx', 'x'), trim(BOTH 'x' FROM 'xxxbarxxx')
--- !query 16 schema
-struct<trim(xxxbarxxx, x):string,trim(xxxbarxxx, x):string>
--- !query 16 output
-bar bar
-
-
--- !query 17
-SELECT ltrim('zzzytest', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytest')
--- !query 17 schema
-struct<ltrim(zzzytest, xyz):string,ltrim(zzzytest, xyz):string>
--- !query 17 output
-test test
-
-
--- !query 18
-SELECT ltrim('zzzytestxyz', 'xyz'), trim(LEADING 'xyz' FROM 'zzzytestxyz')
--- !query 18 schema
-struct<ltrim(zzzytestxyz, xyz):string,ltrim(zzzytestxyz, xyz):string>
--- !query 18 output
-testxyz testxyz
-
-
--- !query 19
-SELECT ltrim('xyxXxyLAST WORD', 'xy'), trim(LEADING 'xy' FROM 'xyxXxyLAST
WORD')
--- !query 19 schema
-struct<ltrim(xyxXxyLAST WORD, xy):string,ltrim(xyxXxyLAST WORD, xy):string>
--- !query 19 output
-XxyLAST WORD XxyLAST WORD
-
-
--- !query 20
-SELECT rtrim('testxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'testxxzx')
--- !query 20 schema
-struct<rtrim(testxxzx, xyz):string,rtrim(testxxzx, xyz):string>
--- !query 20 output
-test test
-
-
--- !query 21
-SELECT rtrim('xyztestxxzx', 'xyz'), trim(TRAILING 'xyz' FROM 'xyztestxxzx')
--- !query 21 schema
-struct<rtrim(xyztestxxzx, xyz):string,rtrim(xyztestxxzx, xyz):string>
--- !query 21 output
-xyztest xyztest
-
-
--- !query 22
-SELECT rtrim('TURNERyxXxy', 'xy'), trim(TRAILING 'xy' FROM 'TURNERyxXxy')
--- !query 22 schema
-struct<rtrim(TURNERyxXxy, xy):string,rtrim(TURNERyxXxy, xy):string>
--- !query 22 output
-TURNERyxX TURNERyxX
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
