+1 (non-binding)

Thanks for preparing the release candidate, Voon.

I validated release-1.2.1-rc1 (609be82e4e55) as a member of the Apache
XTable (incubating) community. I tested a local source build with Spark 3.x
and Scala 2.12.

The tests passed for three XTable changes:

   - Table version 9 support (#835). The full xtable-core suite passed,
   including column stats for partitioned tables with externally registered
   files.
   - Hudi secondary indexes for Iceberg tables (#935). The full xtable-core
   suite passed, including file and row position checks across inserts,
   updates, and deletes.
   - Iceberg support through HoodieTableFormat (#894). The module passed 25
   integration tests and 7 unit tests, with one skip for a known limitation.

For Hudi, I also tested the record index and secondary index directly on
local COW and MOR partitioned tables. Queries that used the indexes
returned the same results as queries with data skipping and the metadata
table disabled. These checks covered inserts, updates, and deletes.

For externally written Parquet files without record keys, I tested the
backport in 6165fbe20c15. Positional record keys, secondary key mappings,
deletes during replace commits, and validation checks all behaved as
expected.

One compatibility concern does not block my vote. HoodieCleanStat exposed
two public constructors in 1.2.0. In 1.2.1-rc1, it removes the
seven-argument constructor and makes the eleven-argument constructor
non-public. This breaks source and binary compatibility for callers that
use these constructors. XTable now uses the builder, but a release note
could help other consumers.

Thanks,
Vinish

Reply via email to