This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new 1535b2b [SPARK-30886][SQL] Deprecate two-parameter TRIM/LTRIM/RTRIM
functions
1535b2b is described below
commit 1535b2bb51782a89b271b1ebe53273ab610b390b
Author: Dongjoon Hyun <[email protected]>
AuthorDate: Thu Mar 5 20:09:39 2020 -0800
[SPARK-30886][SQL] Deprecate two-parameter TRIM/LTRIM/RTRIM functions
### What changes were proposed in this pull request?
This PR aims to show a deprecation warning on two-parameter
TRIM/LTRIM/RTRIM function usages based on the community decision.
-
https://lists.apache.org/thread.html/r48b6c2596ab06206b7b7fd4bbafd4099dccd4e2cf9801aaa9034c418%40%3Cdev.spark.apache.org%3E
### Why are the changes needed?
For backward compatibility, SPARK-28093 is reverted. However, from Apache
Spark 3.0.0, we should give a safe guideline to use SQL syntax instead of the
esoteric function signatures.
### Does this PR introduce any user-facing change?
Yes. This shows a directional warning.
### How was this patch tested?
Pass the Jenkins with a newly added test case.
Closes #27643 from dongjoon-hyun/SPARK-30886.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit afb84e9d378003c57cd01d21cdb1a977ba25454b)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../spark/sql/catalyst/analysis/Analyzer.scala | 20 ++++++---
.../sql/catalyst/analysis/AnalysisSuite.scala | 52 ++++++++++++++++++++++
2 files changed, 66 insertions(+), 6 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
index 3cb754d..eadcd0f 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
@@ -19,6 +19,7 @@ package org.apache.spark.sql.catalyst.analysis
import java.util
import java.util.Locale
+import java.util.concurrent.atomic.AtomicBoolean
import scala.collection.mutable
import scala.collection.mutable.ArrayBuffer
@@ -1795,6 +1796,7 @@ class Analyzer(
* Replaces [[UnresolvedFunction]]s with concrete [[Expression]]s.
*/
object ResolveFunctions extends Rule[LogicalPlan] {
+ val trimWarningEnabled = new AtomicBoolean(true)
def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
case q: LogicalPlan =>
q transformExpressions {
@@ -1839,13 +1841,19 @@ class Analyzer(
}
AggregateExpression(agg, Complete, isDistinct, filter)
// This function is not an aggregate function, just return the
resolved one.
- case other =>
- if (isDistinct || filter.isDefined) {
- failAnalysis("DISTINCT or FILTER specified, " +
- s"but ${other.prettyName} is not an aggregate function")
- } else {
- other
+ case other if (isDistinct || filter.isDefined) =>
+ failAnalysis("DISTINCT or FILTER specified, " +
+ s"but ${other.prettyName} is not an aggregate function")
+ case e: String2TrimExpression if arguments.size == 2 =>
+ if (trimWarningEnabled.get) {
+ log.warn("Two-parameter TRIM/LTRIM/RTRIM function
signatures are deprecated." +
+ " Use SQL syntax `TRIM((BOTH | LEADING | TRAILING)?
trimStr FROM str)`" +
+ " instead.")
+ trimWarningEnabled.set(false)
}
+ e
+ case other =>
+ other
}
}
}
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
index d385133..8451b9b 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala
@@ -21,6 +21,7 @@ import java.util.{Locale, TimeZone}
import scala.reflect.ClassTag
+import org.apache.log4j.Level
import org.scalatest.Matchers
import org.apache.spark.api.python.PythonEvalType
@@ -768,4 +769,55 @@ class AnalysisSuite extends AnalysisTest with Matchers {
assert(message.startsWith(s"Max iterations ($maxIterations) reached for
batch Resolution, " +
s"please set '${SQLConf.ANALYZER_MAX_ITERATIONS.key}' to a larger
value."))
}
+
+ test("SPARK-30886 Deprecate two-parameter TRIM/LTRIM/RTRIM") {
+ Seq("trim", "ltrim", "rtrim").foreach { f =>
+ val logAppender = new LogAppender("deprecated two-parameter
TRIM/LTRIM/RTRIM functions")
+ def check(count: Int): Unit = {
+ val message = "Two-parameter TRIM/LTRIM/RTRIM function signatures are
deprecated."
+ assert(logAppender.loggingEvents.size == count)
+ assert(logAppender.loggingEvents.exists(
+ e => e.getLevel == Level.WARN &&
+ e.getRenderedMessage.contains(message)))
+ }
+
+ withLogAppender(logAppender) {
+ val testAnalyzer1 = new Analyzer(
+ new SessionCatalog(new InMemoryCatalog, FunctionRegistry.builtin,
conf), conf)
+
+ val plan1 = testRelation2.select(
+ UnresolvedFunction(f, $"a" :: Nil, isDistinct = false))
+ testAnalyzer1.execute(plan1)
+ // One-parameter is not deprecated.
+ assert(logAppender.loggingEvents.isEmpty)
+
+ val plan2 = testRelation2.select(
+ UnresolvedFunction(f, $"a" :: $"b" :: Nil, isDistinct = false))
+ testAnalyzer1.execute(plan2)
+ // Deprecation warning is printed out once.
+ check(1)
+
+ val plan3 = testRelation2.select(
+ UnresolvedFunction(f, $"b" :: $"a" :: Nil, isDistinct = false))
+ testAnalyzer1.execute(plan3)
+ // There is no change in the log.
+ check(1)
+
+ // New analyzer from new SessionState
+ val testAnalyzer2 = new Analyzer(
+ new SessionCatalog(new InMemoryCatalog, FunctionRegistry.builtin,
conf), conf)
+ val plan4 = testRelation2.select(
+ UnresolvedFunction(f, $"c" :: $"d" :: Nil, isDistinct = false))
+ testAnalyzer2.execute(plan4)
+ // Additional deprecation warning from new analyzer
+ check(2)
+
+ val plan5 = testRelation2.select(
+ UnresolvedFunction(f, $"c" :: $"d" :: Nil, isDistinct = false))
+ testAnalyzer2.execute(plan5)
+ // There is no change in the log.
+ check(2)
+ }
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
