Repository navigation
add WRITE_TRUNCATE_DATA write_disposition to the bigquery python sdk - #40622
GaryPWhite wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR adds BigQueryDisposition.WRITE_TRUNCATE_DATA to the Python BigQuery I/O connector. However, BigQueryBatchFileLoads in bigquery_file_loads.py was not updated to handle WRITE_TRUNCATE_DATA when copying from temporary tables or updating destination schemas, which can lead to data loss on multi-partition loads and broken schema_update_options behavior.
🔍 General Feedback
- Critical (
bigquery_file_loads.py): InBigQueryBatchFileLoads._load_data(sdks/python/apache_beam/io/gcp/bigquery_file_loads.py:970),if self.write_disposition in ('WRITE_EMPTY', 'WRITE_TRUNCATE'):groups temporary table load job IDs by destination table soTriggerCopyJobstruncates the destination table only once on the first copy job and usesWRITE_APPENDfor the rest. Because'WRITE_TRUNCATE_DATA'is missing from this check, copy jobs from multiple temp tables will run in parallel as single-element batches across workers—each issuingWRITE_TRUNCATE_DATAand overwriting data copied by previous jobs (see #24535). - Schema updates (
bigquery_file_loads.py): InUpdateDestinationSchema.process(sdks/python/apache_beam/io/gcp/bigquery_file_loads.py:368),self._write_disposition not in ('WRITE_TRUNCATE', 'WRITE_APPEND')will causeUpdateDestinationSchemato return early forWRITE_TRUNCATE_DATA, skippingschemaUpdateOptionson the destination table prior to copying temp tables. - Storage Write API (
bigquery.py):WriteToBigQuerywithmethod=STORAGE_WRITE_APIdelegates to the Java cross-language expansion service, whereBigQueryIO.Write.WriteDispositiononly definesWRITE_TRUNCATE,WRITE_APPEND, andWRITE_EMPTY. PassingWRITE_TRUNCATE_DATAwithSTORAGE_WRITE_APIwill fail with a JavaIllegalArgumentExceptionduring expansion; consider validating this upfront in Python. - Testing: Please add unit tests covering
WRITE_TRUNCATE_DATA(e.g., addingparam(write_disposition=BigQueryDisposition.WRITE_TRUNCATE_DATA)toTestBigQueryFileLoads.test_multiple_partition_files_write_dispositionsinbigquery_file_loads_test.pyand testingBigQueryDisposition.validate_write/BigQueryWriteFninbigquery_test.py).
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
| @staticmethod | ||
| def validate_write(disposition): | ||
| values = ( | ||
| BigQueryDisposition.WRITE_TRUNCATE_DATA, |
There was a problem hiding this comment.
🔴 Critical: Allowing WRITE_TRUNCATE_DATA here enables it for BigQueryBatchFileLoads, but sdks/python/apache_beam/io/gcp/bigquery_file_loads.py only checks for ('WRITE_EMPTY', 'WRITE_TRUNCATE') in BigQueryBatchFileLoads._load_data (line 970) and ('WRITE_TRUNCATE', 'WRITE_APPEND') in UpdateDestinationSchema.process (line 368).
Without including 'WRITE_TRUNCATE_DATA' in BigQueryBatchFileLoads._load_data, loads that spill into multiple temporary tables will not be grouped by destination before TriggerCopyJobs. As a result, parallel copy jobs will each execute with WRITE_TRUNCATE_DATA and overwrite one another's data. Please update both checks in bigquery_file_loads.py.
| and append actions occur as one atomic update upon job completion. | ||
| table. * WRITE_TRUNCATE_DATA: If the table already exists, BigQuery | ||
| overwrites the table data, but does not use schema and table constraints | ||
| from the source table.* WRITE_APPEND: If the table already exists, |
There was a problem hiding this comment.
🟡 Medium: bigquery_v2_messages.py is an autogenerated Apitools client file (# NOTE: This file is autogenerated and should not be edited by hand.), and writeDisposition is an unvalidated _messages.StringField(8), so editing this docstring has no runtime effect. In addition, lines 5445–5450 introduce trailing whitespace that will fail lint checks (table.* WRITE_APPEND: ). Consider reverting manual edits to this file or removing the trailing whitespace.
| - BigQueryDisposition.CREATE_NEVER: fail the write if does not exist. | ||
| write_disposition: A string describing what happens if the table has | ||
| already some data. Possible values are: | ||
| - BigQueryDisposition.WRITE_TRUNCATE_DATA: delete table data but preserve table metadata. |
There was a problem hiding this comment.
🟢 Low: This line is 98 characters long (and line 2133 is 105 characters), exceeding the 80-character line length limit enforced by lint checks.
|
Assigning reviewers: R: @damccorm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
still coming back to this with some of gem/copilots comments folks, putting it down for the night :) |
|
Sounds good, converting to draft for now - please comment when it is ready |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40622 +/- ##
=============================================
+ Coverage 56.16% 79.92% +23.75%
=============================================
Files 1124 488 -636
Lines 178336 82539 -95797
Branches 1489 0 -1489
=============================================
- Hits 100170 65972 -34198
+ Misses 75645 16567 -59078
+ Partials 2521 0 -2521
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:
|
I've noticed when using the python SDK for Dataflow pipelines, we can't use the
WRITE_TRUNCATE_DATAwrite disposition. I believe this change is as simple as adding the disposition for use, adding it tovalidate_writeand the streaming insert prevention list.I've also added the change to
CHANGES.md. AFAIK there's no issue open for this right now.LMK if there's anything missing.