srielau commented on code in PR #58317:
URL: https://github.com/apache/spark/pull/58317#discussion_r3929588378
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala:
##########
@@ -752,15 +757,30 @@ case class FileSourceScanExec(
lazy val inputRDD: RDD[InternalRow] = {
val options = relation.options +
(FileFormat.OPTION_RETURNING_BATCH -> supportsColumnar.toString)
- val readFile: (PartitionedFile) => Iterator[InternalRow] =
- relation.fileFormat.buildReaderWithPartitionValues(
- sparkSession = relation.sparkSession,
- dataSchema = relation.dataSchema,
- partitionSchema = relation.partitionSchema,
- requiredSchema = requiredSchema,
- filters = pushedDownFilters,
- options = options,
- hadoopConf = getHadoopConf(relation.sparkSession, relation.options))
+ val hadoopConf = getHadoopConf(relation.sparkSession, relation.options)
+ val readFile: (PartitionedFile) => Iterator[InternalRow] =
relation.fileFormat match {
+ case format: OrcFileFormat
Review Comment:
Addressed in 0c49b66ffa8. Introduced a typed `CharVarcharScanMode`
(`PreserveNative`/`SparkStandard`) in Catalyst; `LogicalRelation`,
`DataSourceV2Relation`, `HiveTableRelation`, and `FileSourceScanExec` now carry
`Option[CharVarcharScanMode]`. Added a mode-aware
`FileFormat.buildReaderWithPartitionValues` overload whose default
implementation clones the per-call Hadoop configuration, writes an
engine-private entry with the explicit mode, and invokes the existing
seven-argument method virtually. `FileSourceScanExec` calls this overload
whenever the relation has a bound mode, regardless of the concrete file format,
so the ORC-specific format match is removed. `OrcFileFormat`'s public
seven-argument override reads the bridged entry and passes the mode to its
non-virtual reader builder, so an existing subclass keeps its override and a
delegating `super` call retains the analyzed mode instead of defaulting to
preserve-native. The Hadoop entry is only a bridge across the legacy signatu
re; the authoritative state remains the typed plan field and overload
parameter.
Extended the subclass regression with an over-length value: `SparkStandard`
raises `EXCEED_LIMIT_LENGTH`, while `PreserveNative` retains native ORC
truncation, covering both the row and vectorized readers.
One scoping note: I kept the V2 ORC internals
(`OrcScan`/`OrcScanBuilder`/`OrcPartitionReaderFactory`) on the resolved
boolean and translate the mode to it at the V2 push-down boundary, since those
are concrete ORC types with no virtual `FileFormat` dispatch (the reported gap
is V1-only). The typed mode lives on the relation carriers and the V1 scan +
`FileFormat` overload. Happy to type the V2 internals too if you'd prefer.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ApplyCharTypePadding.scala:
##########
@@ -51,34 +51,71 @@ object ApplyCharTypePadding extends Rule[LogicalPlan] {
}
override def apply(plan: LogicalPlan): LogicalPlan = {
+ val standardSemantics = conf.charVarcharStandardSemantics
+
+ // Bind into case-class state, not a TreeNodeTag: tags are dropped by
canonicalization
Review Comment:
Reworded in 0c49b66ffa8. The comment now notes that a `TreeNodeTag` survives
canonicalization (`TreeNode.makeCopy` calls `copyTagsFrom`) but does not
participate in structural plan equality / `sameResult`, which is the actual
reason a case-class field is used here instead of a tag.
--
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]
