AnishMahto commented on code in PR #58645:
URL: https://github.com/apache/spark/pull/58645#discussion_r4020277867
##########
sql/pipelines/src/main/scala/org/apache/spark/sql/pipelines/autocdc/Scd2VersionMap.scala:
##########
@@ -77,15 +83,69 @@ private[pipelines] object Scd2VersionMap {
/**
* Schema of the version map: `Map(String, Boolean)`.
*
- * Keys are dot-delimited paths to *leaf* columns that received a null value
in their
- * corresponding upsert event (e.g. `"address.city"`, `` "`has space`.city"
``). Paths
- * must be formatted by
[[org.apache.spark.sql.catalyst.util.QuotingUtils.quoted]] to
- * ensure segments that need quoting are back-tick escaped.
+ * Keys are compact JSON arrays of the name parts of *leaf* columns that
received a null
+ * value in their upsert event (e.g. `["address","city"]`). Keeping
+ * name parts separate distinguishes a nested path from a column whose name
contains dots
+ * and keeps persisted keys independent of SQL identifier quoting rules.
Name parts use the
+ * persisted target schema's canonical spelling.
*
- * Values indicate authorship. I.e, `true` => authored-null, `false` =>
unauthored-null.
+ * Values indicate authorship: `true` means authored-null, `false` means
unauthored-null.
+ * Null values never appear in the map.
*
* Lack of entry in the map for a null-valued leaf column implies the column
was
* schema-evolved with an unauthored-null.
*/
def mapType: MapType = MapType(StringType, BooleanType, valueContainsNull =
false)
+
+ /** Encodes a leaf path as the compact JSON string persisted as its version
map key. */
Review Comment:
Yeah it's a column name. Changed to using `QuotingUtils.quoteNameParts`.
My biggest concern about doing this earlier is that whatever serialization
method we use, it needs to be stable across all future spark releases, because
we will now be serializing/deserializing and persisting the canonical form into
the version map.
But I suppose `QuotingUtils.quoteNameParts` is likely stable enough, I
imagine other things would also break if it ever changed its quoting
logic/output format. I could consider adding some golden tests too to prevent
regression from future changes to `QuotingUtils`.
--
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]
