tanmayrauth commented on code in PR #1507:
URL: https://github.com/apache/iceberg-go/pull/1507#discussion_r3633958271
##########
catalog/hive/hive.go:
##########
@@ -776,6 +777,16 @@ func (c *Catalog) ListNamespaces(ctx context.Context,
parent table.Identifier) (
}
// CreateNamespace creates a new namespace in the catalog.
+// defaultNamespaceLocation derives <warehouse>/<db>.db, the metastore's
Review Comment:
The helper landed between this doc comment and the function, so `//
CreateNamespace creates a new namespace...` now documents
`defaultNamespaceLocation` and `CreateNamespace` is left undocumented — `go
doc` and linters will render it on the wrong symbol. Move
`defaultNamespaceLocation` below `CreateNamespace`, or move this comment line
down so it sits directly above `func (c *Catalog) CreateNamespace`.
##########
catalog/hive/hive_test.go:
##########
@@ -399,6 +399,28 @@ func TestHiveCreateNamespace(t *testing.T) {
mockClient.AssertExpectations(t)
}
+func TestDefaultNamespaceLocation(t *testing.T) {
+ require.Equal(t, "s3://warehouse/db.db",
defaultNamespaceLocation("s3://warehouse", "db"))
+ require.Equal(t, "s3://warehouse/db.db",
defaultNamespaceLocation("s3://warehouse/", "db"))
+ require.Empty(t, defaultNamespaceLocation("", "db"))
+}
+
+func TestHiveCreateNamespaceDerivesLocationFromWarehouse(t *testing.T) {
Review Comment:
Nice to also cover precedence: warehouse configured *and* an explicit
`location` prop, asserting the explicit value wins. Today the only
explicit-location test runs on a catalog with no warehouse, so the `if
db.LocationUri == ""` branch is never exercised with a non-empty warehouse — a
refactor that moved the
derivation ahead of the props loop would override a user's explicit
location and no test would catch it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
