zeroshade commented on code in PR #2390:
URL: https://github.com/apache/iceberg-go/pull/2390#discussion_r4469967029
##########
catalog/sql/sql.go:
##########
@@ -1421,54 +1424,94 @@ func validateNamespacePropertyUpdates(removals
[]string, updates iceberg.Propert
}
func (c *Catalog) ListNamespaces(ctx context.Context, parent table.Identifier)
([]table.Identifier, error) {
- tableQuery := c.db.NewSelect().Model((*sqlIcebergTable)(nil)).
- Column("table_namespace").Where("catalog_name = ?", c.name)
- nsQuery := c.db.NewSelect().Model((*sqlIcebergNamespaceProps)(nil)).
- Column("namespace").Where("catalog_name = ?", c.name)
+ return withReadTx(ctx, c.db, func(ctx context.Context, tx bun.Tx)
([]table.Identifier, error) {
+ children, err := c.childNamespacesInTx(ctx, tx, parent)
+ if err != nil {
+ return nil, fmt.Errorf("error listing namespaces for
'%s': %w", parent, err)
+ }
+ if len(parent) == 0 || len(children) > 0 {
+ return children, nil
+ }
- if len(parent) > 0 {
- _, exists, err := c.resolveNamespaceKey(ctx, parent)
+ _, exists, err := c.resolveNamespaceKeyInTx(ctx, tx, parent)
if err != nil {
return nil, err
}
if !exists {
return nil, fmt.Errorf("%w: %s",
catalog.ErrNoSuchNamespace, strings.Join(parent, "."))
}
- tableQuery = whereNamespaceParent(tableQuery,
"table_namespace", parent)
- nsQuery = whereNamespaceParent(nsQuery, "namespace", parent)
- }
-
- namespaces, err := withReadTx(ctx, c.db, func(ctx context.Context, tx
bun.Tx) ([]string, error) {
- var namespaces []string
-
- rows, err := tx.QueryContext(ctx, tableQuery.String()+" UNION
"+nsQuery.String())
- if err != nil {
- return nil, fmt.Errorf("error listing namespaces for
'%s': %w", parent, err)
- }
+ return children, nil
+ })
+}
- err = c.db.ScanRows(ctx, rows, &namespaces)
+// childNamespacesInTx returns the namespaces one level below parent, or the
+// distinct first levels when parent is empty, as Java's JdbcCatalog does.
+// A namespace counts if it has properties of its own or holds a table or view
+// anywhere below it. LIKE ignores case on some databases, so the levels are
+// compared exactly after the query.
+func (c *Catalog) childNamespacesInTx(ctx context.Context, tx bun.Tx, parent
table.Identifier) ([]table.Identifier, error) {
+ tableQuery := tx.NewSelect().Model((*sqlIcebergTable)(nil)).
+ Column("table_namespace").Where("catalog_name = ?", c.name)
+ nsQuery := tx.NewSelect().Model((*sqlIcebergNamespaceProps)(nil)).
+ Column("namespace").Where("catalog_name = ?", c.name)
+ if len(parent) > 0 {
+ tableQuery = whereNamespaceDescendant(tableQuery,
"table_namespace", parent)
+ nsQuery = whereNamespaceDescendant(nsQuery, "namespace", parent)
+ }
- return namespaces, err
- })
+ rows, err := tx.QueryContext(ctx, tableQuery.String()+" UNION
"+nsQuery.String())
if err != nil {
return nil, err
}
+ var namespaces []string
+ if err := c.db.ScanRows(ctx, rows, &namespaces); err != nil {
+ return nil, err
+ }
+
ret := make([]table.Identifier, 0, len(namespaces))
+ seen := make(map[string]struct{}, len(namespaces))
for _, n := range namespaces {
ident, err := namespaceFromString(n)
if err != nil {
return nil, err
}
- if len(parent) == 0 || (len(ident) >= len(parent) &&
slices.Equal(ident[:len(parent)], parent)) {
- ret = append(ret, ident)
+ if len(ident) <= len(parent) ||
!slices.Equal(ident[:len(parent)], parent) {
+ continue
+ }
+ ident = ident[:len(parent)+1]
+ key := namespaceToString(ident)
+ if _, ok := seen[key]; ok {
+ continue
}
+ seen[key] = struct{}{}
+ ret = append(ret, ident)
}
return ret, nil
}
+// resolveListableNamespaceKey is resolveNamespaceKey for read operations,
Review Comment:
`CheckNamespaceExists` uses this, but `CreateTable`, `RenameTable` (target
namespace) and `CreateView` still use the exact `resolveNamespaceKey`. After
`CreateNamespace(a.b)`, `CheckNamespaceExists(a)` is `true` while
`CreateTable(a.t)` returns `namespace does not exist`. Java's
`JdbcTableOperations` checks the same descendant-aware `namespaceExists`, and
only in strict mode.
##########
website/src/releases.md:
##########
@@ -42,6 +42,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
### Unreleased compatibility notes
+- **SQL catalog nested namespaces match the JDBC catalog.**
`sql.Catalog.ListNamespaces(ctx, parent)` returns only the direct children of
`parent`, no longer `parent` itself or deeper descendants, and
`ListNamespaces(ctx, nil)` returns the distinct first levels. A namespace that
has no row of its own but has child namespaces, such as `a` after
`CreateNamespace(a.b)`, now exists for listing, `CheckNamespaceExists` and
`LoadNamespaceProperties`; `CreateNamespace` and `UpdateNamespaceProperties`
still need its own row. `DropNamespace` returns `catalog.ErrNamespaceNotEmpty`
while child namespaces exist. Walk the hierarchy one level at a time, and drop
children before their parent.
Review Comment:
If `CreateTable` and `CreateView` keep the exact check, please list them
here next to `CreateNamespace` and `UpdateNamespaceProperties`. This line also
conflicts with `main`.
--
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]
