You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2023/08/29 11:39:04 UTC
[spark] branch master updated: [SPARK-44967][SQL][CONNECT] Unit should be considered first before using Boolean for TreeNodeTag
This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 0a8a29807b5 [SPARK-44967][SQL][CONNECT] Unit should be considered first before using Boolean for TreeNodeTag
0a8a29807b5 is described below
commit 0a8a29807b5578686d4dad271593883eefe049db
Author: Jiaan Geng <be...@163.com>
AuthorDate: Tue Aug 29 19:38:45 2023 +0800
[SPARK-44967][SQL][CONNECT] Unit should be considered first before using Boolean for TreeNodeTag
### What changes were proposed in this pull request?
Currently, there are a lot of `TreeNodeTag[Boolean]` are defined.
In fact, developers don't require the boolean value boxed into `TreeNodeTag`, just want know it as a flag.
### Why are the changes needed?
Unit should be considered first before using Boolean for TreeNodeTag.
### Does this PR introduce _any_ user-facing change?
'No'.
Just update inner implementation.
### How was this patch tested?
N/A
### Was this patch authored or co-authored using generative AI tooling?
'No'.
Closes #42683 from beliefer/SPARK-44967.
Authored-by: Jiaan Geng <be...@163.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../spark/sql/connect/planner/SparkConnectPlanner.scala | 4 ++--
.../apache/spark/sql/catalyst/analysis/CheckAnalysis.scala | 14 +++++++-------
.../sql/catalyst/analysis/ColumnResolutionHelper.scala | 2 +-
.../scala/org/apache/spark/sql/catalyst/dsl/package.scala | 2 +-
.../org/apache/spark/sql/catalyst/expressions/Cast.scala | 2 +-
.../org/apache/spark/sql/catalyst/parser/AstBuilder.scala | 4 ++--
.../spark/sql/catalyst/plans/logical/LogicalPlan.scala | 2 +-
.../org/apache/spark/sql/catalyst/trees/TreeNode.scala | 2 ++
.../scala/org/apache/spark/sql/catalyst/util/package.scala | 2 +-
sql/core/src/main/scala/org/apache/spark/sql/Column.scala | 2 +-
.../execution/datasources/FileMetadataStructSuite.scala | 6 +++---
11 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
index 5b5018a1668..fbe877b4547 100644
--- a/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
+++ b/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala
@@ -1391,8 +1391,8 @@ class SparkConnectPlanner(val sessionHolder: SessionHolder) extends Logging {
if (attr.hasPlanId) {
expr.setTagValue(LogicalPlan.PLAN_ID_TAG, attr.getPlanId)
}
- if (attr.hasIsMetadataColumn) {
- expr.setTagValue(LogicalPlan.IS_METADATA_COL, attr.getIsMetadataColumn)
+ if (attr.hasIsMetadataColumn && attr.getIsMetadataColumn) {
+ expr.setTagValue(LogicalPlan.IS_METADATA_COL, ())
}
expr
}
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
index 0d663dfb5f9..13999f391d9 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
@@ -51,8 +51,8 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
*/
val extendedCheckRules: Seq[LogicalPlan => Unit] = Nil
- val DATA_TYPE_MISMATCH_ERROR = TreeNodeTag[Boolean]("dataTypeMismatchError")
- val INVALID_FORMAT_ERROR = TreeNodeTag[Boolean]("invalidFormatError")
+ val DATA_TYPE_MISMATCH_ERROR = TreeNodeTag[Unit]("dataTypeMismatchError")
+ val INVALID_FORMAT_ERROR = TreeNodeTag[Unit]("invalidFormatError")
/**
* Fails the analysis at the point where a specific tree node was parsed using a provided
@@ -247,7 +247,7 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
case checkRes: TypeCheckResult.DataTypeMismatch =>
hof.dataTypeMismatch(hof, checkRes)
case checkRes: TypeCheckResult.InvalidFormat =>
- hof.setTagValue(INVALID_FORMAT_ERROR, true)
+ hof.setTagValue(INVALID_FORMAT_ERROR, ())
hof.invalidFormat(checkRes)
}
@@ -273,10 +273,10 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
case e: Expression if e.checkInputDataTypes().isFailure =>
e.checkInputDataTypes() match {
case checkRes: TypeCheckResult.DataTypeMismatch =>
- e.setTagValue(DATA_TYPE_MISMATCH_ERROR, true)
+ e.setTagValue(DATA_TYPE_MISMATCH_ERROR, ())
e.dataTypeMismatch(e, checkRes)
case TypeCheckResult.TypeCheckFailure(message) =>
- e.setTagValue(DATA_TYPE_MISMATCH_ERROR, true)
+ e.setTagValue(DATA_TYPE_MISMATCH_ERROR, ())
val extraHint = extraHintForAnsiTypeCoercionExpression(operator)
e.failAnalysis(
errorClass = "DATATYPE_MISMATCH.TYPE_CHECK_FAILURE_WITH_HINT",
@@ -285,7 +285,7 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
"msg" -> message,
"hint" -> extraHint))
case checkRes: TypeCheckResult.InvalidFormat =>
- e.setTagValue(INVALID_FORMAT_ERROR, true)
+ e.setTagValue(INVALID_FORMAT_ERROR, ())
e.invalidFormat(checkRes)
}
@@ -858,7 +858,7 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog with QueryErrorsB
val nonAnsiPlan = getDefaultTypeCoercionPlan(plan)
var issueFixedIfAnsiOff = true
getAllExpressions(nonAnsiPlan).foreach(_.foreachUp {
- case e: Expression if e.getTagValue(DATA_TYPE_MISMATCH_ERROR).contains(true) &&
+ case e: Expression if e.getTagValue(DATA_TYPE_MISMATCH_ERROR).isDefined &&
e.checkInputDataTypes().isFailure =>
e.checkInputDataTypes() match {
case TypeCheckResult.TypeCheckFailure(_) | _: TypeCheckResult.DataTypeMismatch =>
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala
index 6af348328e7..d7b1f99f1ed 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala
@@ -512,7 +512,7 @@ trait ColumnResolutionHelper extends Logging {
}
val plan = planOpt.get
- val isMetadataAccess = u.getTagValue(LogicalPlan.IS_METADATA_COL).contains(true)
+ val isMetadataAccess = u.getTagValue(LogicalPlan.IS_METADATA_COL).isDefined
try {
if (!isMetadataAccess) {
plan.resolve(u.nameParts, conf.resolver)
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala
index 559822369d6..39e0f836e24 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala
@@ -134,7 +134,7 @@ package object dsl {
expr
} else {
val cast = Cast(expr, to)
- cast.setTagValue(Cast.USER_SPECIFIED_CAST, true)
+ cast.setTagValue(Cast.USER_SPECIFIED_CAST, ())
cast
}
}
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
index 08666330587..b975dc3c7a5 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
@@ -181,7 +181,7 @@ object Cast extends QueryErrorsBase {
/**
* A tag to decide if a CAST is specified by user.
*/
- val USER_SPECIFIED_CAST = new TreeNodeTag[Boolean]("user_specified_cast")
+ val USER_SPECIFIED_CAST = new TreeNodeTag[Unit]("user_specified_cast")
/**
* Returns true iff we can cast `from` type to `to` type.
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 b1f212b1739..f8a9ea268d2 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
@@ -2118,12 +2118,12 @@ class AstBuilder extends DataTypeAstBuilder with SQLConfHelper with Logging {
ctx.name.getType match {
case SqlBaseParser.CAST =>
val cast = Cast(expression(ctx.expression), dataType)
- cast.setTagValue(Cast.USER_SPECIFIED_CAST, true)
+ cast.setTagValue(Cast.USER_SPECIFIED_CAST, ())
cast
case SqlBaseParser.TRY_CAST =>
val cast = Cast(expression(ctx.expression), dataType, evalMode = EvalMode.TRY)
- cast.setTagValue(Cast.USER_SPECIFIED_CAST, true)
+ cast.setTagValue(Cast.USER_SPECIFIED_CAST, ())
cast
}
}
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
index a57c9d7a162..a5f4c4d5c51 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
@@ -196,7 +196,7 @@ object LogicalPlan {
// 3, resolve this expression with the matching node. If any error occurs, analyzer fallbacks
// to the old code path.
private[spark] val PLAN_ID_TAG = TreeNodeTag[Long]("plan_id")
- private[spark] val IS_METADATA_COL = TreeNodeTag[Boolean]("is_metadata_col")
+ private[spark] val IS_METADATA_COL = TreeNodeTag[Unit]("is_metadata_col")
}
/**
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
index 9e605a45414..d5d7f17bfe3 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
@@ -53,6 +53,8 @@ import org.apache.spark.util.collection.BitSet
private class MutableInt(var i: Int)
// A tag of a `TreeNode`, which defines name and type
+// Note: In general, if developers only care about its tagging capabilities,
+// then Unit should be considered first before using Boolean.
case class TreeNodeTag[T](name: String)
// A functor that always returns true.
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala
index 98eb2ade985..f50e84ff8d3 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala
@@ -104,7 +104,7 @@ package object util extends Logging {
PrettyAttribute(usePrettyExpression(e.child) + "." + e.field.name, e.dataType)
case r: InheritAnalysisRules =>
PrettyAttribute(r.makeSQLString(r.parameters.map(toPrettySQL)), r.dataType)
- case c: Cast if !c.getTagValue(Cast.USER_SPECIFIED_CAST).getOrElse(false) =>
+ case c: Cast if c.getTagValue(Cast.USER_SPECIFIED_CAST).isEmpty =>
PrettyAttribute(usePrettyExpression(c.child).sql, c.dataType)
case p: PythonFuncExpression => PrettyPythonUDF(p.name, p.dataType, p.children)
}
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala
index 8d4cb50dfa2..39e4815fc57 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala
@@ -1171,7 +1171,7 @@ class Column(val expr: Expression) extends Logging {
*/
def cast(to: DataType): Column = withExpr {
val cast = Cast(expr, CharVarcharUtils.replaceCharVarcharWithStringForCast(to))
- cast.setTagValue(Cast.USER_SPECIFIED_CAST, true)
+ cast.setTagValue(Cast.USER_SPECIFIED_CAST, ())
cast
}
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala
index 15055a276fa..c782104f4f9 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/FileMetadataStructSuite.scala
@@ -1035,10 +1035,10 @@ class FileMetadataStructSuite extends QueryTest with SharedSparkSession {
spark.range(end = 10).write.format("parquet").save(path.toString)
// Add the tag to the base Dataframe before selecting a metadata column.
- val customTag = TreeNodeTag[Boolean]("customTag")
+ val customTag = TreeNodeTag[Unit]("customTag")
val baseDf = spark.read.format("parquet").load(path.toString)
val tagsPut = baseDf.queryExecution.analyzed.collect {
- case rel: LogicalRelation => rel.setTagValue(customTag, true)
+ case rel: LogicalRelation => rel.setTagValue(customTag, ())
}
assert(tagsPut.nonEmpty)
@@ -1047,7 +1047,7 @@ class FileMetadataStructSuite extends QueryTest with SharedSparkSession {
// Expect the tag in the analyzed and optimized plan after querying a metadata column.
def isTaggedRelation(plan: LogicalPlan): Boolean = plan match {
- case rel: LogicalRelation => rel.getTagValue(customTag).getOrElse(false)
+ case rel: LogicalRelation => rel.getTagValue(customTag).isDefined
case _ => false
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org