srielau commented on code in PR #58317:
URL: https://github.com/apache/spark/pull/58317#discussion_r3960964836
##########
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:
Fixed in 50940524276. FileSourceScanExec now removes the reserved bridge
entry from the Hadoop configuration immediately after relation options are
copied. A bound mode is then added only by
FileFormat.buildReaderWithPartitionValues typed overload on its cloned per-call
configuration, preserving virtual dispatch and delegating super calls. Added an
unbound V1 ORC regression for invalid, SparkStandard, and PreserveNative caller
values; all are ignored and the focused ORC tests pass.
##########
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:
Fixed in 50940524276. Added CacheManager.recacheByV2Relation for the
catalog-less mutation path. It compares the written and cached
DataSourceV2Relation with only charVarcharScanMode cleared, preserving all
existing sameResult identity while invalidating both bound variants.
DataSourceV2Strategy now uses it only for the TableProvider branch. Added an
end-to-end SimpleWritableDataSource append/recache regression for
PreserveNative and SparkStandard; the focused DataSourceV2Suite test passes.
##########
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:
Fixed in 50940524276. The test comment now refers to the two bound scan
modes instead of the obsolete Some(false)/Some(true) representation.
##########
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:
Fixed in 50940524276. The comment now states only the stable reason:
TreeNodeTag values do not participate in structural equality / sameResult,
while the case-class field does.
--
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]
