PDGGK commented on code in PR #20506:
URL: https://github.com/apache/hudi/pull/20506#discussion_r4500297213
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/ProvidesHoodieConfig.scala:
##########
@@ -572,6 +572,17 @@ object ProvidesHoodieConfig {
HoodieCommonConfig.SCHEMA_EVOLUTION_ENABLE.defaultValue.toString)
).toBoolean
+ /**
+ * Whether a Spark SQL UPDATE or DELETE on the table is written as a prepped
write, which takes each
+ * record's key, partition path and file group from the meta columns read
along with it. A table that
+ * does not populate those meta columns has nothing to read there, so it
goes through the regular write
+ * path, which derives the keys with the key generator and locates the
records through the index.
+ */
+ def shouldUsePreppedSqlWrites(sparkSession: SparkSession,
hoodieCatalogTable: HoodieCatalogTable): Boolean =
Review Comment:
Thanks, I reproduced it: on 097bc81 that table ended up with both
`[1,a1,10.0]` and `[1,a1,11.0]` after the UPDATE, and id 1 was still there
after the DELETE.
I went with failing fast (336f25f).
`ProvidesHoodieConfig.validateSqlWriteMatchesStoredKeys` rejects an UPDATE or
DELETE when all of these hold:
- the table does not populate the record key;
- it uses a complex key generator with a single record key field and at most
one partition field;
- `KeyGenUtils.encodeSingleKeyFieldNameForComplexKeyGen` says the writer
will prefix the field name.
The check also runs with optimized writes off, because that setting sends
the statement down the same regular write path.
Fixing the mismatch itself would change how `RecordContext` regenerates keys
for every reader of these tables, so I kept that out of this PR. Both tests now
cover the case under both settings and check that the rows are unchanged. With
the check disabled, both fail on the missing exception.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/others/TestUpdateTable.scala:
##########
@@ -541,6 +541,46 @@ class TestUpdateTable extends HoodieSparkSqlTestBase {
}
}
+ test("Test Update Table Without Meta Fields") {
Review Comment:
Done in 336f25f: both notes are gone, and the partitioned array/map test now
uses the default `populateMetaFields = false`. Locally on Spark 3.5 it passes,
as do the two "Without Meta Fields" tests.
--
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]
