AnishMahto commented on code in PR #58645:
URL: https://github.com/apache/spark/pull/58645#discussion_r4019990718
##########
sql/pipelines/src/main/scala/org/apache/spark/sql/pipelines/autocdc/Scd2BatchProcessor.scala:
##########
@@ -1445,23 +1508,37 @@ object Scd2BatchProcessor {
private[pipelines] def computeTrackedHistoryColumns(
schema: StructType,
changeArgs: ChangeArgs,
- resolver: Resolver): Seq[String] = {
- val keyColNames = changeArgs.keys.map(_.name)
-
- val eligibleSchema = StructType(schema.fields.filterNot { field =>
- reservedFrameworkColNames.exists(resolver(_, field.name)) ||
- keyColNames.exists(resolver(_, field.name))
- })
-
+ resolver: Resolver): Seq[String] =
ColumnSelection
.applyToSchema(
schemaName = "trackHistorySelection",
- schema = eligibleSchema,
+ schema = computeUserDataSchema(schema, changeArgs, resolver),
columnSelection = changeArgs.trackHistorySelection,
resolver = resolver
)
.fieldNames
.toImmutableArraySeq
+
+ /**
+ * The subset of `schema` that is user data a column selection may act on:
every field that is
+ * neither a framework reserved column nor one of [[ChangeArgs.keys]]. Field
order is preserved.
+ *
+ * Both [[ChangeArgs.trackHistorySelection]] and
[[ChangeArgs.ignoreNullSelection]] resolve
+ * against this, so an exclude-list in either cannot pick up a key or a
framework column, and
+ * an include-list naming one fails as not found.
+ *
+ * `schema` is expected to have already been narrowed by
[[ChangeArgs.columnSelection]] and then
Review Comment:
Agree, and on further inspection I realized this function's name/scaladoc is
over stating what this function actually cares about.
The function is unopinionated on what columns the input schema contains. It
also doesn't actually care about the concept of "user data schema" - it only
cares to filter out keys and reserved column names. Whether the result of that
filtering is considered user data schema or not by callers is irrelevant.
So I decided to simply decompose this into two more focused functions;
`filterOutKeyColumns` and `filterOutReservedFrameworkColumns`.
Unit/integration tests are then responsible for actually verifying whether
the constructed version maps only contain whatever is considered user data
schema.
--
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]
