Skip to content

Umbrella: API and dependency cleanup before the v1.0.0 freeze #2062

Description

@laskoviymishka

Background

We are heading toward a v1.0.0 release of iceberg-go. Under Go semver, once we tag 1.0.0 any breaking change to the public API means a new /v2 module path, so the pre-1.0 window is the one cheap chance to fix API and layering warts. This is an umbrella issue to collect those breaking cleanups in one place so we can agree on scope and sequence them, rather than discovering them right after the freeze.

Two efforts already in flight roll up under this:

The intent is to keep things additive or behind facades where we can, and be explicit about the few genuinely breaking changes that have to land before the tag. Measurements and the detailed Arrow analysis live in #2044. Thanks @nssalian and @zeroshade for the early review; I have folded your notes into the sections below.

Status

1. Dependency footprint and layering

Goal: a consumer that only reads/writes metadata or talks to a REST catalog should not link a cloud SDK it never uses, or the Substrait execution engine.

2. Public API consistency (breaking, so pre-1.0 or /v2 only)

  • io.IO: Open and Remove take no context.Context, while BulkRemovableIO.DeleteFiles does. Thread ctx through the base interface for a consistent, cancellable IO surface. (prototype ready)
  • Catalog capabilities: view operations and RegisterTable are not part of the core Catalog interface and are supported unevenly across the REST / SQL / Hive / Glue / Hadoop implementations. Decide whether these become optional capability interfaces (e.g. a ViewableCatalog) so support is discoverable and uniform.
  • table write API: the string-path and DataFile method pairs overlap (AddFiles / AddDataFiles, ReplaceDataFiles / ReplaceDataFilesWithDataFiles, ReplaceFiles / ReplaceFilesWithDeleteFiles). Settle on canonical names before we freeze all of them.
  • PartitionField.SourceID() reads ambiguously next to the SourceIDs field now that a field can have multiple source ids. Rename or clarify.
  • Schema.FindFieldByIDRef / FieldsRef: as @nssalian noted, the internal.SchemaRef{} argument already gates these to in-module callers (external modules cannot construct a type under internal/), so it is a deliberate sentinel rather than a leak. The action is to pin the intended shape (keep the sentinel, or move to an unexported entry point) before someone opens a PR, not to "hide" it.

3. Deprecations to remove at the freeze

Thanks @nssalian for tracing these. They are not equal cost, so splitting:

Clean deletes (batchable in one PR at the tag):

  • DataFileBuilder.DistinctValueCounts (distinct_counts / field 111 is deprecated in the spec; no non-test callers in-repo).
  • io/gocloud ParseAWSConfig / ParseGCSConfig redirect shims (only referenced by io/gocloud/compat_test.go; superseded by the per-cloud s3.ParseAWSConfig / gcs.ParseGCSConfig).
  • WitMaxConcurrency (the typo'd alias of WithMaxConcurrency, which is the one we keep) and RewriteFiles.Apply (superseded by RewriteFiles.ApplyResult).

Migration (separate PR):

  • Transform.ToHumanStr: ToHumanStrType (used by PartitionToPath) delegates to ToHumanStr, and every transform implements it, so removing it means inverting that delegation across every transform. That is a migration, not a delete, and deserves its own PR.

Out of scope for v1 release

To keep the freeze achievable, we would not block v1 on these:

cc @zeroshade @nssalian @slachiewicz

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions