gengliangwang commented on code in PR #59974:
URL: https://github.com/apache/spark/pull/59974#discussion_r4434284322
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtilsSuite.scala:
##########
@@ -44,4 +50,87 @@ class PushDownUtilsSuite extends SparkFunSuite {
assert(pushed.size === 1)
intercept[SparkNumberFormatException](pushed.head.eval(failing))
}
+
+ private val innerStruct =
+ StructType.fromDDL("x int, y struct<p: string, q: array<int>>, z string")
+ private val commented = new MetadataBuilder().putString("comment",
"kept").build()
+ private val tableSchema = new StructType()
+ .add("id", LongType, nullable = false)
+ .add("s", innerStruct)
+ .add("arr", ArrayType(innerStruct))
+ .add("m", MapType(StringType, innerStruct))
+ .add(StructField("Mixed", StringType, nullable = true, commented))
+
+ /**
+ * Runs [[PushDownUtils.pruneColumns]] with nested schema pruning enabled on
a relation over
+ * `tableSchema`, and returns the required schema pushed to the scan builder.
+ */
+ private def pushedSchema(
+ query: Seq[AttributeReference] => (Seq[NamedExpression],
Seq[Expression])): StructType = {
+ val relation = DataSourceV2Relation.create(
+ new TestTable(CatalogV2Util.structTypeToV2Columns(tableSchema)), None,
None)
+ val (projects, filters) = query(relation.output)
+ val builder = new RecordingScanBuilder
+ withSQLConf(SQLConf.NESTED_SCHEMA_PRUNING_ENABLED.key -> "true") {
+ PushDownUtils.pruneColumns(builder, relation, projects, filters)
+ }
+ builder.requiredSchema
+ }
+
+ test("SPARK-60768: whole-column reads push the needed columns in table
order") {
Review Comment:
**[P1] New SPARK-60768 unit tests pass with the fix reverted**
The new unit tests check only the schema pushed to the scan builder. For
whole-column reads, the old nested-pruning path pushes exactly the same schema,
so none of them can tell whether `pruneSchema` was skipped. That skip is the
whole point of the PR (it removes the per-query nested-pruning cost), and
nothing guards it.
**Failure scenario:** Restore the old PushDownUtils hunk (`if
rootFields.nonEmpty pruneSchema(...) else new StructType()`). For (mixed, s,
id2, id; id>1, IsNotNull(m)), pruneSchema still yields id,s,m,Mixed in table
order with the same nullability and metadata, and the empty-reference case
still yields an empty StructType. All three tests stay green, so a regression
that brings the nested-pruning cost back would go unnoticed.
--
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]
