Skip to content

feat: add hms list namespaces#135

Merged
jackye1995 merged 3 commits into
lance-format:mainfrom
wojiaodoubao:hms-list-namespace
Jul 18, 2025
Merged

feat: add hms list namespaces#135
jackye1995 merged 3 commits into
lance-format:mainfrom
wojiaodoubao:hms-list-namespace

Conversation

@wojiaodoubao

@wojiaodoubao wojiaodoubao commented Jul 16, 2025 •
edited by jackye1995
Loading

Copy link
Copy Markdown
Collaborator

Closes #102

@github-actions github-actions Bot added enhancement New feature or request java Java features labels Jul 16, 2025
geruh
geruh reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/util/PageUtils.java Outdated
geruh
geruh reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/test/java/com/lancedb/lance/namespace/TestHMSNamespace.java Outdated
geruh
geruh reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/test/java/com/lancedb/lance/namespace/TestHMSNamespace.java
}

@Test
public void testListNamespacesV3() throws Exception {

@geruh geruh Jul 17, 2025

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.

wdyt about adding a test for an empty database?

@wojiaodoubao wojiaodoubao Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think "hive.mydb" and "mycatalog.default" should cover empty databases. Please correct me if I miss anything.

geruh
geruh reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/util/PageUtils.java Outdated
geruh
geruh reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/hive/LanceHMSNamespace.java
} else {
throw new RuntimeException(String.format("Namespace parent {} is too long.", parent));
}
} catch (TException e) {

@geruh geruh Jul 17, 2025

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.

what about metaexception

@wojiaodoubao wojiaodoubao Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Since MetaException is derived from TException, it will be caught and wrapped with RuntimeException then throw. Do you mean we shouldn't wrap MetaException with a RuntimeException?

jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/conf/ConfKey.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java Outdated

public class CoreKeys {
public static final String CATALOG_IMPL = "lance.namespace.catalog-impl";
public static final String CATALOG_IMPL_DEFAULT =

@jackye1995 jackye1995 Jul 17, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: NS_IMPL_DEFAULT

jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/conf/ConfKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/conf/ConfKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/conf/ConfKeys.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/util/PageUtils.java Outdated
jackye1995
jackye1995 reviewed Jul 17, 2025
View reviewed changes
Comment thread java/pom.xml Outdated
geruh
geruh suggested changes Jul 17, 2025
View reviewed changes
Comment thread java/lance-namespace-hms/src/test/java/com/lancedb/lance/namespace/TestHiveNamespace.java
assertEquals(Sets.newHashSet(), namespace.listNamespaces(request).getNamespaces());

// Case 5: non-existing catalog, database, table.
request.setParent(Lists.list("nonexistedcatalog"));

@geruh geruh Jul 17, 2025

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.

HiveMetastore client returns an empty list if the catalog doesn't exist, and that's why we get an empty response for non existent catalogs.

Related to the spec, I see that the ListNamespace operation doesn’t define a not found exception for listing with parent. Is this intentional @jackye1995

@jackye1995 jackye1995 Jul 18, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

that seems like a miss, but maybe we can add that in a separated PR.

Comment thread java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/hive/LanceHiveNamespace.java
jackye1995
jackye1995 approved these changes Jul 18, 2025
View reviewed changes

@jackye1995 jackye1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for the work @wojiaodoubao! There are some minor comments but I think we can address in later PRs along the way, let's go! And thanks @geruh for the review!

@jackye1995 jackye1995 merged commit 9f1f23b into lance-format:main Jul 18, 2025
5 checks passed
@wojiaodoubao wojiaodoubao mentioned this pull request Jul 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jackye1995 jackye1995 jackye1995 approved these changes
+1 more reviewer
@geruh geruh geruh requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request java Java features

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Java HiveNamespace ListNamespaces

3 participants

@wojiaodoubao @geruh @jackye1995