-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Hive: Fix HiveViewOperations does not update view query upon alteration #14831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveViewOperations.java
Outdated
Show resolved
Hide resolved
|
@stuxuhai Thanks for the PR! Can we add a test? |
|
@huaxingao Thx! Unit test has been added. |
|
Could you fix a checkstyle error? You can run: |
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveViewOperations.java
Show resolved
Hide resolved
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveViewOperations.java
Outdated
Show resolved
Hide resolved
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCommits.java
Outdated
Show resolved
Hide resolved
|
@ebyhr thanks,fixed |
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCommits.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| public void testViewQueryIsUpdatedOnCommit() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you slightly update the test to have
@Test
public void testViewQueryIsUpdatedOnCommit() throws Exception {
HiveViewOperations ops = (HiveViewOperations) ((BaseView) view).operations();
assertThat(view.currentVersion().representations())
.containsExactly(
ImmutableSQLViewRepresentation.builder().sql(VIEW_QUERY).dialect("hive").build());
Table hmsTable = ops.loadHmsTable();
assertThat(hmsTable.getViewOriginalText()).isEqualTo(VIEW_QUERY);
assertThat(hmsTable.getViewExpandedText()).isEqualTo(VIEW_QUERY);
String newQuery = "select * from ns.tbl2 limit 10";
view =
catalog
.buildView(VIEW_IDENTIFIER)
.withSchema(SCHEMA)
.withDefaultNamespace(NS)
.withQuery("hive", newQuery)
.replace();
assertThat(view.currentVersion().representations())
.containsExactly(
ImmutableSQLViewRepresentation.builder().sql(newQuery).dialect("hive").build());
Table updatedHmsTable = ops.loadHmsTable();
assertThat(updatedHmsTable.getViewOriginalText()).isEqualTo(newQuery);
assertThat(updatedHmsTable.getViewExpandedText()).isEqualTo(newQuery);
}
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCommits.java
Show resolved
Hide resolved
nastra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @stuxuhai
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCommits.java
Outdated
Show resolved
Hide resolved
|
@stuxuhai I've pushed some minor changes to your branch. There's a flaky test that causes CI failures, so please rebase when you get a chance |
35865d8 to
7831b81
Compare
|
@nastra Rebased on top of the latest changes. Thanks! |
The current implementation of
HiveViewOperations'sdoCommitmethod correctly updates the view properties and the Iceberg metadata location in the Hive Metastore (HMS) when a view is altered. However, it fails to update the actual SQL query string stored in the HMS.This bug causes severe inconsistency:
This fix guarantees that the view definition in the Hive Metastore is always synchronized with the canonical Iceberg view definition, enhancing compatibility and reliability when using external systems that rely on HMS for query metadata.