difin commented on PR #6930: URL: https://github.com/apache/hive/pull/6930#issuecomment-6534600432
The tests cover the stats path without a lock and the lock decision itself. I think these cases are still missing: - ADDPROPS and a manual metadata_location change with engine.hive.lock-enabled=false. This is the only DDL path where the PR sends the CAS. testConcurrentIcebergCommitsAndHiveAlterTableCalls and testAlterTableWithMetadataLocation run with the default, which is lock enabled, so neither reaches it. A lock-disabled version of the concurrent-commit test would show the commit survives. For the manual change, a test should show a concurrent commit makes the alter fail instead of being overwritten. - A CAS rejection with hive.stats.reliable=true. insertDuringStatsUpdate with reliable stats should show what happens to the statement and its data. With the current code, I'd expect the INSERT to report failure after its snapshot is already committed (see my comment in HiveIcebergMetaHook). - Locks under different Iceberg catalog names. otherWriterLockStateDuringAlter always requests its lock with testTables.catalogName(). A variant with another catalog name, or the HMS default catalog, would show the lock doesn't block that writer, and that writer then sends no CAS either. - Direct SQL disabled (metastore.try.direct.sql=false). An unlocked stats alter, TOUCH or ADDPROPS would now fail in the JDO fallback. A test would pin down the error the user gets. -- 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]
