Skip to content

[hive] Support creating hive catalog table branch and reading branch data#3820

Merged
tsreaper merged 3 commits intoapache:masterfrom
discivigour:b5_hiveBranch
Aug 1, 2024
Merged

[hive] Support creating hive catalog table branch and reading branch data#3820
tsreaper merged 3 commits intoapache:masterfrom
discivigour:b5_hiveBranch

Conversation

@discivigour
Copy link
Copy Markdown
Contributor

Purpose

Support creating hive catalog table branch and reading branch data

Tests

  • HiveCatalogITCaseBase

API and Format

No changes.

Documentation

Copy link
Copy Markdown
Contributor

@tsreaper tsreaper left a comment

Choose a reason for hiding this comment

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

You forgot to support ALTER TABLE with branches in HiveCatalog. Please support this feature and add tests.

Comment on lines +203 to +205
if (!options.contains(CoreOptions.BRANCH)) {
options.set(CoreOptions.BRANCH, "");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: Don't set default value here. Use different constructor of SchemaManager below.

hiveShell.execute("SET paimon.branch=b2");
assertThat(hiveShell.executeQuery("SELECT * FROM t"))
.isEqualTo(Arrays.asList("4\tx1", "5\tx2"));
hiveShell.execute("SET paimon.branch=null");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test that after setting branch to null, we can read from default branch.

Comment on lines +414 to +423
Path tableLocation = getDataTableLocation(identifier);
return new SchemaManager(fileIO, tableLocation, branchName)
.latest()
.map(
s -> {
Options branchOptions = new Options(s.options());
branchOptions.set(CoreOptions.BRANCH, branchName);
return s.copy(branchOptions.toMap());
})
.orElseThrow(() -> new TableNotExistException(identifier));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move this into tableSchemaInFileSystem, so that it supports reading schema for branches. FileSystemCatalog can also reuse this method.

@tsreaper tsreaper merged commit 59143c1 into apache:master Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants