Skip to content

Conversation

@deniskuzZ
Copy link
Member

@deniskuzZ deniskuzZ commented Jan 10, 2026

What changes were proposed in this pull request?

  • Added support for V3 format in the Hive Iceberg test-suite;
  • Removed redundancy, structured the test utils;
  • Fixed framework coordinating concurrent query execution (HiveIcebergStorageHandlerStub setup, TestUtilPhaser) used in TestConflictingDataFiles & TestOptimisticRetry

Why are the changes needed?

Additional test coverage for v3 format

Does this PR introduce any user-facing change?

No

How was this patch tested?

Jenkins
No production code changes, just test framework

@deniskuzZ
Copy link
Member Author

Can you please check the spoless issues. I'm getting failure because of that with this PR

-import·java.util.stream.Collectors;

please check again

This is causing checkstyle issues in HiveIcebergOutputCommitter.java

import static org.apache.iceberg.mr.hive.HiveTableUtil.generateJobLocation;
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.5.0:check (validate) on project hive-iceberg-handler: Failed during checkstyle execution: There is 1 error reported by Checkstyle 11.1.0 with /Users/raghav/Desktop/apache/hive/master/iceberg/iceberg-handler/../checkstyle/checkstyle.xml ruleset. -> [Help 1]

hm, it seems to be fixed now, build was green

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds V3 format support to the Hive Iceberg test suite and restructures the test framework for better maintainability. The changes include parameterization improvements to enable testing across format versions (V1, V2, V3), reorganization of test utilities into proper package structures, and fixes to the concurrent test framework used for testing optimistic concurrency and table locking scenarios.

Changes:

  • Extended test parameterization to support format versions 1-3 with configurable filtering
  • Moved test utilities to dedicated test and test.utils packages for better organization
  • Fixed concurrent testing framework (HiveIcebergStorageHandlerStub, TestUtilPhaser, WithMockedStorageHandler)

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TestHiveIcebergVectorization.java Added V2-only filtering for vectorization tests
HiveIcebergTestUtils.java Moved to test.utils package, updated stream operations to modern API
HiveIcebergStorageHandlerTestUtils.java Moved to test.utils package, changed visibility to public
WithMockedStorageHandler.java New annotation/rule for mocking storage handler in concurrent tests
TestUtilPhaser.java New comprehensive phaser utility for coordinating concurrent test execution
HiveIcebergStorageHandlerStub.java New stub implementation supporting both ext-locking and barrier synchronization modes
TestTables.java Moved to test package, added getCatalog() accessor
TestHiveShell.java Moved to test package
CustomTestHiveAuthorizerFactory.java Moved to test package
TestOptimisticRetry.java Updated to use new concurrent testing framework
TestConflictingDataFiles.java Updated to use new concurrent testing framework with proper synchronization
Multiple test files Updated imports and added format version filtering
HiveTableUtil.java Added utility methods for job/file locations, improved documentation
HiveIcebergOutputCommitter.java Refactored to use HiveTableUtil methods for location generation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

@deniskuzZ
Copy link
Member Author

@Aggarwal-Raghav , @difin, @kokila-19 any final comments, or shall we merge?

@kokila-19
Copy link
Contributor

LGTM +1, It can be merged.

@deniskuzZ deniskuzZ merged commit e302ff6 into apache:master Jan 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants