Hive: Fix lock selection during table creation to respect table properties by s-sanjay · Pull Request #14236 · apache/iceberg

s-sanjay · 2025-10-02T10:36:22Z

Summary

HIVE_LOCK_ENABLED table property was not respected during table creation. When creating a new table, the lock type was falling back to configuration defaults regardless of what is set in table property

Background

PR #10016 changed lockObject() to use base metadata for lock decisions to ensure concurrent comitters agree on the lock. However, when creating a new table, base is null, which caused the lock decision to fall back to configuration defaults instead of using the table's property set in the SQL.

Changes

HiveTableOperations.java: Changed the caller in doCommit() to pass base != null ? base : metadata to lockObject(), ensuring new tables use their metadata properties for lock decisions and only falling back to conf defaults when this is not set ( preserving the same behavior as existing tables )

Test plan

Added comprehensive unit and end-to-end tests covering both NoLock and MetastoreLock scenarios during table creation
TestHiveCommits.java: Added four test cases:
- testCreateTableWithNoLockEndToEnd: End-to-end test for table creation with HIVE_LOCK_ENABLED=false
- testCreateTableWithLockEndToEnd: End-to-end test for table creation with HIVE_LOCK_ENABLED=true
- testCreateTableWithNoLock: Unit test verifying NoLock is used during table creation with the property set to false
- testCreateTableWithLock: Unit test verifying MetastoreLock is used during table creation with default/true setting
Existing test testChangeLockWithAlterTable continues to verify that lock changes on existing tables use base metadata

…rties When creating a new table passed metadata is null, the lock decision now uses the new metadata parameter ( since old version is null for creation ) instead of falling back to configuration defaults. Configuration default is still used when the table property is not set. This ensures the HIVE_LOCK_ENABLED table property is respected during table creation.

s-sanjay · 2025-10-12T15:30:24Z

@pvary @singhpk234 can you take a look ?

singhpk234 · 2025-10-12T17:16:11Z

    boolean updateHiveTable = false;

-    HiveLock lock = lockObject(base);
+    HiveLock lock = lockObject(base != null ? base : metadata);


looks reasonable to me, we do similar thing in glue
https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java#L154-L156

…ommits.java Adressing typo Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com>

…ommits.java Co-authored-by: Yuya Ebihara <ebyhry@gmail.com>

s-sanjay · 2025-10-15T13:35:20Z

@pvary @singhpk234 can you take a look ?

@singhpk234 Thanks for the comments, I have addressed them, can you take another look ?

deniskuzZ

lgtm +1

deniskuzZ · 2025-10-16T20:10:55Z

+              SCHEMA,
+              PartitionSpec.unpartitioned(),
+              catalog.defaultWarehouseLocation(newTableIdentifier),
+              lockEnabled ? ImmutableMap.of() : ImmutableMap.of(HIVE_LOCK_ENABLED, "false"));


minor, why not be explicit: ImmutableMap.of(HIVE_LOCK_ENABLED, String.valueOf(lockEnabled)

specifically wanted to test the default behavior where lock is enabled.. Ideally there are 3 scenarios

Default - no property - lock is set ( falls back to global conf )

Lock is explicitly disabled

Lock is explicitly enabled
The test is testing 1 and 2. 3 is being tested in the other test
We can add 1,2,3 in both these tests..

Done.. Added @NullSource so we test all 3 cases for both

deniskuzZ · 2025-10-16T20:13:55Z

+          .as("Lock mechanism should use (%s)", expectedLockClass.getSimpleName())
+          .isInstanceOf(expectedLockClass);
+    } finally {
+      catalog.dropTable(newTableIdentifier, true);


why do you need to drop, when there is no table create here?

commit ends up creating a new table... so dropping it here

… and Lock property missing ( which can implicitly mean lock is enabled ), unless disabled via global property

pvary · 2025-10-20T10:38:23Z

Merged to main.
Thanks for the PR @s-sanjay and @singhpk234, @ebyhr, @deniskuzZ for the review!

…rties (apache#14236)

github-actions Bot added the hive label Oct 2, 2025

s-sanjay mentioned this pull request Oct 2, 2025

Bug: HIVE_LOCK_ENABLED table property not respected during table creation #14237

Closed

3 tasks

deniskuzZ reviewed Oct 2, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java Outdated

s-sanjay force-pushed the fix-hive-lock-table-creation branch 2 times, most recently from 26188b6 to e67954d Compare October 4, 2025 16:19

deniskuzZ reviewed Oct 6, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java Outdated

s-sanjay force-pushed the fix-hive-lock-table-creation branch from e67954d to 2ff05d8 Compare October 7, 2025 05:05

singhpk234 reviewed Oct 12, 2025

View reviewed changes

ebyhr reviewed Oct 13, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java Outdated

s-sanjay and others added 3 commits October 14, 2025 16:36

Update hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveC…

5a53442

…ommits.java Adressing typo Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com>

Update hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveC…

a5e2d19

…ommits.java Co-authored-by: Yuya Ebihara <ebyhry@gmail.com>

Make the test parameterized, fix typo in test name

fd1e9ac

deniskuzZ approved these changes Oct 16, 2025

View reviewed changes

deniskuzZ reviewed Oct 16, 2025

View reviewed changes

s-sanjay requested review from ebyhr and singhpk234 October 17, 2025 03:46

Test all 3 combinations of Lock being explicitly enabled and disabled…

16d544c

… and Lock property missing ( which can implicitly mean lock is enabled ), unless disabled via global property

pvary reviewed Oct 17, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java Outdated

pvary reviewed Oct 17, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveTable.java Outdated

pvary reviewed Oct 17, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java

pvary reviewed Oct 17, 2025

View reviewed changes

Comment thread hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveTable.java

Fix table name and change assertThat to directly use AtomicReference

7c436a5

s-sanjay requested a review from pvary October 18, 2025 03:39

pvary approved these changes Oct 20, 2025

View reviewed changes

pvary merged commit c0250c7 into apache:main Oct 20, 2025
42 checks passed

thomaschow pushed a commit to thomaschow/iceberg that referenced this pull request Jan 19, 2026

Hive: Fix lock selection during table creation to respect table prope…

757faa0

…rties (apache#14236)

talatuyarer pushed a commit to talatuyarer/iceberg that referenced this pull request Apr 1, 2026

Hive: Fix lock selection during table creation to respect table prope…

5cb1049

…rties (apache#14236)

Uh oh!

Conversation

s-sanjay commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Changes

Test plan

Uh oh!

Uh oh!

Uh oh!

s-sanjay commented Oct 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

singhpk234 Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

s-sanjay commented Oct 15, 2025

Uh oh!

deniskuzZ left a comment

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

s-sanjay Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

s-sanjay Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

s-sanjay Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pvary commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

s-sanjay commented Oct 2, 2025 •

edited

Loading

s-sanjay Oct 17, 2025 •

edited

Loading