Fix load_tabular_data.sh depending on undefined behavior (which has started breaking) - #770
Open
dmannarino wants to merge 1 commit into
Open
Fix load_tabular_data.sh depending on undefined behavior (which has started breaking)#770dmannarino wants to merge 1 commit into
dmannarino wants to merge 1 commit into
Conversation
psql -c does not reliably support other statements after an embedded
COPY ... FROM STDIN unless the COPY is the last command in the string.
In practice this could abort the connection immediately ("unexpected
COPY_IN result, aborting connection") before any data was read, which
also surfaced as a spurious "Broken pipe" from the aws s3 cp side of
the pipe.
Restructure to feed psql one continuous script over its real stdin:
header SQL ending in COPY, the streamed S3 data (normalized to end in
exactly one newline via sed), an explicit \. terminator, then the
trailer SQL (INSERT ... COMMIT). Also add set -o pipefail so a failed
aws s3 cp is no longer masked by a later stage in the same pipe, and
-v ON_ERROR_STOP=1 to preserve fail-fast behavior on SQL errors.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #770 +/- ##
=======================================
Coverage 76.17% 76.18%
=======================================
Files 144 144
Lines 6834 6835 +1
=======================================
+ Hits 5206 5207 +1
Misses 1628 1628
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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The symptom:
test_table_source_asset_minimal(and any tabular data load usingload_tabular_data.sh) intermittently failed with the batch job reportingstatus: failed. The job logs showedBEGIN,CREATE TABLE, and bothALTER TABLEstatements succeeding, then immediatelyunexpected COPY_IN result, aborting connectionfrom psql, followed bydownload failed: ... [Errno 32] Broken pipefrom theaws s3 cpside of the pipe — with no data ever actually loaded.The issue:
load_tabular_data.shpipedaws s3 cp ... -intopsql -c "BEGIN; ...; COPY \"$TEMP_TABLE\" FROM STDIN ...; ...; INSERT ...; COMMIT;"— a single-cstring bundling multiple statements withCOPY FROM STDINin the middle rather than last. This is a documented limitation ofpsql -c: bundling more than one command, with aCOPYnot in the final position, produces "unexpected results" (per PostgreSQL's own maintainers), and here it manifested as psql aborting the connection the instant it entered copy mode, before reading any data.psqlclosing its end of the pipe first is also what produced the "Broken pipe" on theaws s3 cpside — a downstream symptom, not the root cause. Sincecreate_tabular_schema.shhas noCOPYstatement, it was unaffected, which is why only the load step failed.The fix:
Restructure the script to feed psql one continuous script over its real stdin instead of via
-c: header SQL ending in theCOPYstatement, then the streamed S3 data, then an explicit\.end-of-copy marker, then the trailer SQL (INSERT ... COMMIT) — the same pattern aspsql -f <(cat header.sql data.csv footer.sql), kept streaming rather than buffered. The data is piped throughsed -e '$a\'first so it always ends in exactly one newline (verified against both a normally-terminated and a missing-trailing-newline fixture), which keeps the\.marker on its own line without ever risking a spurious blank row that would breakCOPYon a multi-column table. Also addedset -o pipefail, since without it a failedaws s3 cpcould be silently masked by a later stage in the same pipe succeeding, andpsql -v ON_ERROR_STOP=1to preserve fail-fast behavior on SQL errors.Why now?:
The code has been (at least mostly) working for a few years but just started CONSISTENTLY breaking now. I would guess, but am not sure, that Ubuntu upgraded the PostgreSQL package to one which changed the undefined behavior in practice. Actually, Claude says that Ubuntu Noble did just go from PostgreSQL 15 to PostgreSQL 16.