Skip to content

AddFiles: Dynamic destinations - #40852

Open
claudevdm wants to merge 4 commits into
apache:masterfrom
claudevdm:addfiles-dynamic-dest4
Open

claudevdm wants to merge 4 commits into
apache:masterfrom
claudevdm:addfiles-dynamic-dest4

Conversation

@claudevdm

@claudevdm claudevdm commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Completes dynamic destinations for IcebergIO.AddFiles: one transform registers files into many
tables, choosing each file's table from its own path.

path_pattern:  gs://lake/export/{table}/**
table:         default.{table}

gs://lake/export/orders/part-0.parquet     ->  default.orders
gs://lake/export/customers/part-0.parquet  ->  default.customers

Builds on #40749 (PathPattern, RouteFiles) and #40817 (the destination-table key threaded
through the registration chain), both merged. Those added the primitives and the plumbing with the
key held constant; this makes the key vary, and adds the YAML surface.

  1. SchemaCommitVerdict (53 lines): the value the pre-pass produces per table.
  2. CommitSchemaUnion: commit now returns that verdict instead of a long; tryCommit
    wraps it so no failure escapes.
  3. CommitSchemaPerTable (87): runs the pre-pass per table, emits one verdict each.
  4. TableGate (79): the barrier and filter. The three-outcome process method is the heart
    of the change.
  5. AddFiles.expand: the if (pathPattern != null) branch wiring the above together; the
    else is the existing single-table path.
  6. AddFilesSchemaTransformProvider: the path_pattern option and its three validations.

How it fits together

RouteFiles              path -> KV<table, path>, or an error row if nothing matched
  |
distinctSchemasPerTable Combine.perKey of the existing CollectDistinctSchemas
  |
CommitSchemaPerTable    one schema union per table; emits a verdict, never throws
  |
verdictView             View.asMap -> Map<table, SchemaCommitVerdict>
  |
TableGate               holds every file until all verdicts exist, then releases the
  |                     files of tables that were not refused
(the keyed registration chain from #40817)

The pre-pass is the same CollectDistinctSchemas CombineFn, lifted with Combine.perKey. The
dynamic path is the existing pipeline with a key that varies, not a second implementation.

The error output is unchanged for existing pipelines. ERROR_SCHEMA keeps its two fields; a
second DYNAMIC_ERROR_SCHEMA adds table and is chosen per instance in expand. An earlier
draft widened the single schema, which broke the public errors shape for every user and needed
a breaking-change entry plus a fix to Beam's own iceberg_add_files.yaml. The price of the split
is that the output shape depends on configuration, so the path_pattern field description says
so explicitly.

FAIL_PIPELINE becomes "refuse this table" under path_pattern. tryCommit catches
RuntimeException, so an IncompatibleSchemaException refuses one table and reports its files
rather than failing the run — registering 1000 tables and losing all of them to one bad table
isn't a usable feature. The option name is then a mild overstatement in this mode, which the
field description states. A distinct REFUSE_TABLE value would be more honest and more surface;
I'm happy to go that way instead.

error_handling is mandatory with path_pattern. Two of the three error producers are
structural, an unmatched path, and a refused table, so dropping the error collection can lose a
whole table's files silently. That's the one outcome this feature must not have, so the provider
refuses the combination rather than warning.

A table with no verdict is released, not refused. No verdict means no file of that table
yielded a readable schema, so the pre-pass never saw it. Releasing is safe because
getOrCreateTable rethrows NoSuchTableException when evolution is on, so each file becomes its
own MISSING_TABLE_ERROR row instead of one generic reason for the whole table, and no table is
ever created from a single file's schema. Only the pre-pass creates tables in dynamic mode.

Why catch (RuntimeException) rather than an enumerated set. RESTException parents both
permanent (Forbidden, BadRequest, UnprocessableEntity) and transient (ServiceUnavailable,
ServiceFailure) subclasses, so there's no clean permanent set to list, and anything an
enumeration missed would fail the pipeline — the outcome this exists to prevent.
NoSuchNamespaceException extends RuntimeException rather than NotFoundException, so
catch-by-parent would miss it too. It does also catch bugs in this code, which is why the
LOG.warn carries the throwable.

No retry wraps it, and the comment in tryCommit records why: Iceberg's REST client already
retries 429 and 503 (ExponentialHttpRequestRetryStrategy, 5 attempts by default) and
deliberately does not retry 500/502/504, since the commit may have landed. withRetry is for
stale state, where reloading the table is what makes a retry meaningful.

Dynamic destinations is batch-only, and that follows from three existing checks rather than a
new one: path_pattern requires schema evolution, and expand already refuses evolution on
unbounded input. Stated here because no single check says it.

@github-actions github-actions Bot added python java io yaml iceberg Iceberg IO connector size/xl XL: >=1000 lines changed labels Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.91%. Comparing base (2213e54) to head (f1a53ab).
⚠️ Report is 43 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #40852       +/-   ##
===========================================
+ Coverage        0   79.91%   +79.91%     
===========================================
  Files           0      493      +493     
  Lines           0    83479    +83479     
===========================================
+ Hits            0    66709    +66709     
- Misses          0    16770    +16770     
Flag Coverage Δ
python 79.91% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claudevdm claudevdm changed the title Addfiles dynamic dest4 AddFiles: Dynamic destinations Oct 27, 2026
@claudevdm
claudevdm marked this pull request as ready for review October 27, 2026 21:18
@claudevdm
claudevdm requested a review from ahmedabu98 October 27, 2026 21:18
@claudevdm

Copy link
Copy Markdown
Collaborator Author

R: @ahmedabu98

@github-actions

Copy link
Copy Markdown
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant