cloud-fan commented on code in PR #58317:
URL: https://github.com/apache/spark/pull/58317#discussion_r4009047658
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileFormat.scala:
##########
@@ -266,6 +295,25 @@ object FileFormat {
*/
val OPTION_RETURNING_BATCH = "returning_batch"
+ /**
+ * Engine-private Hadoop configuration entry that transports the analyzed
CHAR/VARCHAR scan mode
+ * across the legacy [[FileFormat.buildReaderWithPartitionValues]]
signature. It is written by the
+ * mode-aware overload and read by formats that honor first-class
CHAR/VARCHAR types. This is not
+ * a public option; the authoritative state is the typed plan field and
overload parameter.
+ */
+ val CHAR_VARCHAR_SCAN_MODE = "__spark_sql_char_varchar_scan_mode"
+
+ /** Writes the CHAR/VARCHAR scan mode into `conf` under
[[CHAR_VARCHAR_SCAN_MODE]]. */
+ private[sql] def setCharVarcharScanMode(
+ conf: Configuration, mode: CharVarcharScanMode): Unit = {
+ conf.set(CHAR_VARCHAR_SCAN_MODE, mode.toString)
+ }
+
+ /** Reads the CHAR/VARCHAR scan mode from `conf`, or `None` if no mode was
bridged in. */
+ private[sql] def charVarcharScanMode(conf: Configuration):
Option[CharVarcharScanMode] = {
+ Option(conf.get(CHAR_VARCHAR_SCAN_MODE)).map(CharVarcharScanMode.fromName)
Review Comment:
Confirmed: caller-provided bridge values are removed and only the typed
overload authors the private mode on its cloned configuration. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3960197738","thread_id":"inline:3960197738","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ApplyCharTypePadding.scala:
##########
@@ -51,34 +51,74 @@ object ApplyCharTypePadding extends Rule[LogicalPlan] {
}
override def apply(plan: LogicalPlan): LogicalPlan = {
+ val standardSemantics = conf.charVarcharStandardSemantics
+ val scanMode = CharVarcharScanMode(standardSemantics)
+
+ // Bind into case-class state, not a TreeNodeTag: `TreeNode.makeCopy`
calls `copyTagsFrom`,
+ // so a tag survives canonicalization, but it does not participate in
structural plan
Review Comment:
Confirmed: the final comment now states only that TreeNodeTag does not
participate in structural equality or sameResult. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3960197749","thread_id":"inline:3960197749","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/SaveIntoDataSourceCommandSuite.scala:
##########
@@ -69,7 +70,29 @@ class SaveIntoDataSourceCommandSuite extends
SharedSparkSession {
saveIntoDataSource(2)
checkAnswer(loadData, Row(0) :: Row(1) :: Nil)
+ spark.catalog.clearCache()
FakeV1DataSource.data = null
+
+ // Bound modes store Some(false)/Some(true) on LogicalRelation, so recache
must match
Review Comment:
Confirmed: the test comment now uses the current two-bound-mode
representation. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3960197755","thread_id":"inline:3960197755","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala:
##########
@@ -116,7 +116,10 @@ case class DataSourceV2Relation(
catalog: Option[CatalogPlugin],
identifier: Option[Identifier],
options: CaseInsensitiveStringMap,
- timeTravelSpec: Option[TimeTravelSpec] = None)
+ timeTravelSpec: Option[TimeTravelSpec] = None,
+ // Bound at analysis so sameResult / cache reuse distinguish preserve-only
vs standard
+ // CHAR/VARCHAR scans. None means the relation was not analyzed under
first-class types.
+ charVarcharScanMode: Option[CharVarcharScanMode] = None)
Review Comment:
Confirmed: SharedState relation-cache hits clear only the prior scan mode
for fresh-session rebinding, while persisted plans retain captured semantics.
Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3962201879","thread_id":"inline:3962201879","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharScanMode.scala:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.util
+
+/**
+ * The CHAR/VARCHAR scan mode bound to a relation (and its scan) during
analysis.
+ *
+ * A relation carries `Option[CharVarcharScanMode]`: `None` means the relation
was not analyzed
+ * under first-class CHAR/VARCHAR types (native reader behavior), while a
`Some` value pins the
+ * mode so that `sameResult` / cache reuse keep the two variants distinct.
+ */
+sealed trait CharVarcharScanMode
+
+object CharVarcharScanMode {
+ /**
+ * Preserve the native, constrained CHAR/VARCHAR types of the source (e.g.
native ORC
+ * padding/truncation). Corresponds to preserve-only semantics.
+ */
+ case object PreserveNative extends CharVarcharScanMode
+
+ /**
+ * Request physical STRING from the source so Spark observes the original
value and applies
+ * standard CHAR/VARCHAR length checks. Corresponds to standard semantics.
+ */
+ case object SparkStandard extends CharVarcharScanMode
+
+ /** Maps the boolean `spark.sql.charVarcharStandardSemantics` value to the
typed mode. */
+ def apply(standardSemantics: Boolean): CharVarcharScanMode =
+ if (standardSemantics) SparkStandard else PreserveNative
+
+ /** Parses a mode from its `toString` name; the inverse of
[[CharVarcharScanMode.toString]]. */
+ def fromName(name: String): CharVarcharScanMode = name match {
+ case "PreserveNative" => PreserveNative
+ case "SparkStandard" => SparkStandard
+ case other => throw new IllegalArgumentException(s"Unknown
CharVarcharScanMode: $other")
Review Comment:
Confirmed: the Scaladoc now names
spark.sql.charVarchar.standardSemantics.enabled exactly. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3962201885","thread_id":"inline:3962201885","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileFormat.scala:
##########
@@ -165,6 +166,34 @@ trait FileFormat {
}
}
+ /**
+ * Same as [[buildReaderWithPartitionValues]] but also carries the analyzed
CHAR/VARCHAR scan
+ * mode. [[FileSourceScanExec]] calls this overload whenever the relation
has a bound mode,
+ * regardless of the concrete file format.
+ *
+ * The default implementation bridges the mode across the legacy
seven-argument signature: it
+ * clones the per-call Hadoop configuration, writes an engine-private entry
with the explicit
+ * mode, then invokes the seven-argument method virtually. A format that
honors first-class
+ * CHAR/VARCHAR types (e.g.
[[org.apache.spark.sql.execution.datasources.orc.OrcFileFormat]])
+ * reads that entry in its seven-argument override, so existing subclasses
keep their override
+ * and a call to `super` retains the bound mode. The Hadoop entry is only a
transport across the
+ * legacy signature; the authoritative state remains the typed plan field
and this parameter.
+ */
+ def buildReaderWithPartitionValues(
Review Comment:
Confirmed: the typed mode, bridge key, and overload are private[sql], while
the existing public FileFormat hook remains intact. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:3962201889","thread_id":"inline:3962201889","verdict_sha256":"9e8299073fe3a3fd317d35d14ddb819ddbbaf0c580a01f233a538f0836de3d9e"}
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
