AddFiles: Add dry_run - #40167
AddFiles: Add dry_run#40167claudevdm wants to merge 6 commits into
Conversation
49f0b21 to
f436398
Compare
|
R: @ahmedabu98 |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40167 +/- ##
============================================
- Coverage 58.47% 58.46% -0.01%
+ Complexity 13593 13591 -2
============================================
Files 2576 2576
Lines 269266 269266
Branches 11079 11079
============================================
- Hits 157441 157427 -14
- Misses 105863 105871 +8
- Partials 5962 5968 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ommitting or registering
SchemaEvolutionConfig.setDryRun(true) (provider key dry_run) turns
AddFiles into a report-only transform: the read side runs as usual
(footers, distinct schemas), then DryRunReport emits one Row per
distinct schema plus one summary row on a new output, dry_run_report.
Why: before enabling evolution on a large import, a user wants to know
what the options would do to the table and which files would be
refused, without touching anything. Running the real transform under
FAIL_PIPELINE answers only "would it fail", and only for the first
failure.
The verdicts come from CommitSchemaUnion.plan, the one step that
decides everything a commit does before writing anything. A Plan says
which distinct file schemas are merged (SchemaToMerge), which are
refused and why (IncompatibleSchema), and the schema the table ends
with. It is an EvolutionPlan against an existing table (base schema
snapshot, name-mapping repair) or a CreationPlan when the table is
missing (partition spec and sort order resolved against the union, or
the problem that blocks creation). commitOnce dispatches to evolve or
create, which fail or warn under the handling mode and then write; the
dry run turns the same plan into rows, so a check added to the plan
reaches both and the report cannot drift from the commit. A schema that
is fine against the table but conflicts with another schema of the
input is therefore reported with the blame a real run assigns.
Real-run changes that come with planning first: partition or sort
fields that do not fit the union fail with a message naming them,
before any catalog write and under either handling (the per-file
fallback creation throws the same error, so it could never be routed);
problems are reported before the transaction is opened; every
transaction is checked against the one base snapshot; planning retries
like committing; a window without schemas plans nothing. Settings
(config, the handling resolved for the mode, NewTableSettings) replaces
the three loose arguments both DoFns carried.
Report rows (REPORT_SCHEMA), told apart by row_type:
row_type schema | create | unreadable | unchecked | summary
schema_key short murmur3 key of the schema JSON on schema rows
schema the canonical schema JSON; on the create row, the
union the table would be created with
num_files files the row covers
changes ARRAY<STRING>: the SchemaDelta descriptions on schema
rows; "create <optional|required> <name> <type>" per
column on the create row (pins shown required); the
totals line and table-level changes on the summary
allowed whether a real run would accept it
reason why not, else ""; the consequence on the summary
would_create_table false whenever a real run would not create the
table, including when it would fail first
Provider: the dry_run_report output only exists when dry_run is set,
so existing YAML pipelines that enumerate outputs are unaffected.
f436398 to
2d2d5de
Compare
ahmedabu98
left a comment
There was a problem hiding this comment.
Left some comments.
Really cool to see how it looks like in the YAML test
ahmedabu98
left a comment
There was a problem hiding this comment.
Just some doc nits. Everything else LGTM, thanks!
|
|
||
| @SchemaFieldDescription( | ||
| "When true, nothing is committed or registered: the transform reads the files' schemas" | ||
| + " and emits a dry_run_report output with one row that describes what a real run" |
There was a problem hiding this comment.
| + " and emits a dry_run_report output with one row that describes what a real run" | |
| + " and emits a `dry_run_report` output with one row that describes what a real run" |
| @SchemaFieldDescription( | ||
| "When true, nothing is committed or registered: the transform reads the files' schemas" | ||
| + " and emits a dry_run_report output with one row that describes what a real run" | ||
| + " would do. Its allowed field is true when every file schema can be merged and the" |
There was a problem hiding this comment.
| + " would do. Its allowed field is true when every file schema can be merged and the" | |
| + " would do. Its `allowed` field is true when every file schema can be merged and the" |
| "When true, nothing is committed or registered: the transform reads the files' schemas" | ||
| + " and emits a dry_run_report output with one row that describes what a real run" | ||
| + " would do. Its allowed field is true when every file schema can be merged and the" | ||
| + " configuration raises no problem; otherwise its reason field says what a real run" |
There was a problem hiding this comment.
| + " configuration raises no problem; otherwise its reason field says what a real run" | |
| + " configuration raises no problem; otherwise its `reason` field says what a real run" |
| + " and emits a dry_run_report output with one row that describes what a real run" | ||
| + " would do. Its allowed field is true when every file schema can be merged and the" | ||
| + " configuration raises no problem; otherwise its reason field says what a real run" | ||
| + " would do about it (fail, or route the files to the error output). Its schemas" |
There was a problem hiding this comment.
| + " would do about it (fail, or route the files to the error output). Its schemas" | |
| + " would do about it (fail, or route the files to the error output). Its `schemas`" |
| + " would do about it (fail, or route the files to the error output). Its schemas" | ||
| + " field lists each distinct file schema with the changes a real run would make for" | ||
| + " it and, when it cannot be merged, why. The output only exists when this is set;" | ||
| + " consume it as input: <this transform's name>.dry_run_report. Against a missing" |
There was a problem hiding this comment.
| + " consume it as input: <this transform's name>.dry_run_report. Against a missing" | |
| + " consume it as input: `<this transform's name>.dry_run_report`. Against a missing" |
AddFiles:
dry_runreports what the schema pre-pass would doSchemaEvolutionConfig.setDryRun(true)(dry_run: truein YAML) turns AddFiles into a report-only transform. The read side runs as usual (footers, distinct schemas); instead of committing and registering, a newdry_run_reportoutput gets one row per distinct file schema plus acreaterow, marker rows for unreadable and ORC/Avro files, and a summary row, told apart by arow_typecolumn.snapshotsanderrorsare empty. The output only exists whendry_runis set, so existing YAML pipelines are unaffected.The diff has two halves. Reading them in this order is easiest.
1.
CommitSchemaUnion: the commit now plans before it writes.Everything a commit decides is pulled into a
Plan, built byplan(): which distinct file schemas get merged (SchemaToMerge), which are refused and why (IncompatibleSchema), the schema the table ends with, and any configuration problems. It is anEvolutionPlanagainst an existing table (classify and fold on scratch transactions, all against one snapshot of the table schema, plus whether the name mapping gets repaired) or aCreationPlanwhen the table is missing (fold for creation, then the partition spec and sort order resolved against the union, or the problem that blocks creation).commitOnceonly dispatches toevolveorcreate; each fails or warns under the handling mode first, then writes.DryRunReportturns the same plan into rows. A check added to the plan reaches both, so the report cannot drift from the commit. In particular a schema that is fine against the table but conflicts with another schema of the input is reported with the blame a real run assigns.Because these types are now shared with the report, some merged code was renamed and tidied:
Acceptedis nowSchemaToMergeandIncompatibleis nowIncompatibleSchema(each is a whole distinct file schema, not a column change);TableCreationis nowNewTableSettings;reportIncompatibleis nowfailOrWarnOnIncompatibleSchemas, since it can end the run.Settings(the config, the handling resolved for the pipeline mode, the new-table settings) replaces the three loose arguments thatcommit,CommitSchemaOnceand nowDryRunReportcarried.Verdictsholder tracks where each schema stands, and staging returns theConflictit found.2. The dry run itself.
DryRunReport(new), theexpandwiring inAddFiles, thedry_runknob inSchemaEvolutionConfigand the YAML provider, the marker keys inReadFooterSchema, the transform-level tests inAddFilesTest, and a YAML fixture that runs a dry run against a missing table, writes the report to JSON and asserts the full rows.Report rows
row_typeschema,create,unreadable,uncheckedorsummaryschema_keyschemarows, to group by; empty on the othersschemacreaterow, the union the table would be created with; empty on the othersnum_fileschangesSchemaDeltadescriptions onschemarows; onecreate optional <name> <type>per column on thecreaterow; the totals line plus any table-level change (name mapping) on the summaryallowedreasonwould_create_tableThank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.