zeroshade commented on issue #2062:
URL: https://github.com/apache/iceberg-go/issues/2062#issuecomment-6476657503
## Proposal: split `Catalog` into capability interfaces
This is the decision for the "Catalog capabilities" item in section 2.
`Catalog` has 17 methods. Adding a method after v1 breaks every implementer,
including third-party catalogs, so the shape needs to be settled before the
tag. The split below composes the interface rather than stripping it, so
existing callers see the same method set.
### What we have today
- **Optional interfaces already in place:** `TransactionalCatalog`,
`RegisterableCatalog` and `RefreshableCredentialCatalog`, checked with a type
assertion. `PurgeTable` is embedded through `PurgeableCatalog`.
- **Views have no interface.** Each backend has concrete methods:
| Backend | List / Load / Create / Drop / CheckExists | Rename / Register
view | `Name()` |
|---|---|---|---|
| REST | ✓ | ✓ / ✓ | ✓ |
| Hive | ✓ | – | – |
| SQL | ✓, but with different signatures | – | ✓ |
| Glue, Hadoop | – | – | – |
- **SQL's view signatures differ** from REST and Hive:
- `CreateView` takes `(ctx, ident, *iceberg.Schema, view.Representation,
iceberg.Properties) error` instead of `(ctx, ident, *view.Version,
*iceberg.Schema, ...CreateViewOpt) (*view.View, error)`.
- `LoadView` returns `view.Metadata` instead of `*view.View`.
A shared view interface can't cover SQL until those are aligned.
### Proposed shape
This mirrors Java's `Catalog` / `SupportsNamespaces` / `ViewCatalog`:
```go
// Core table operations.
type TableCatalog interface {
CreateTable(...); LoadTable(...); CommitTable(...)
ListTables(...); DropTable(...); RenameTable(...); CheckTableExists(...)
PurgeableCatalog
}
type NamespaceCatalog interface {
ListNamespaces(...); CreateNamespace(...); DropNamespace(...)
CheckNamespaceExists(...); LoadNamespaceProperties(...);
UpdateNamespaceProperties(...)
}
// What every backend implements. New methods after v1 go into new optional
// interfaces, never here.
type Catalog interface {
io.Closer
Name() string
CatalogType() Type
TableCatalog
NamespaceCatalog
}
// Optional, checked with a type assertion like RegisterableCatalog.
type ViewCatalog interface {
ListViews(...); LoadView(...) (*view.View, error); CreateView(...)
(*view.View, error)
DropView(...); CheckViewExists(...) (bool, error)
}
```
**Decisions:**
- **`Name()` joins the core interface.** REST and SQL already have it, and
Hive, Glue and Hadoop store the name but don't expose it. Adding it breaks
third-party implementers, which is why it has to happen before v1.
- **Namespaces stay part of `Catalog`.** All five backends implement them,
and making them optional would force a type assertion for a universal feature.
`NamespaceCatalog` exists so code can accept only what it needs.
- **`RenameView` and `RegisterView` stay out of `ViewCatalog`.** Only REST
has them; they can become separate optional interfaces later.
- **The `Catalog` doc comment should state the rule** that new methods after
v1 go into new optional interfaces.
### Compatibility
| Change | Breaking? |
|---|---|
| Split `Catalog` into embedded `TableCatalog` / `NamespaceCatalog` | No:
same method set |
| Add `ViewCatalog`; REST and Hive satisfy it as-is | No: additive |
| Add `Name()` to `Catalog`, implemented by Hive, Glue and Hadoop | Yes, for
third-party `Catalog` implementations |
| Align SQL `CreateView` / `LoadView` with REST and Hive | Yes, for
`sql.Catalog` callers |
### Suggested PRs
1. **`refactor(catalog)!:` split the interface, add `Name()` and
`ViewCatalog`.**
- Changes: `catalog/catalog.go`, plus `Name()` on Hive, Glue and Hadoop.
- Add compile-time checks such as `var _ catalog.ViewCatalog =
(*Catalog)(nil)` in REST and Hive.
- Add a release note for `Name()`.
2. **`refactor(catalog/sql)!:` align SQL view signatures with
`ViewCatalog`.**
- Add the SQL compile-time check and a release note.
These can be claimed separately. PR 2 depends on PR 1. Please claim here
before starting.
--
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]