[jdatcmd]

Fixes #2045.

What was wrong

pgcolumnar.import_arrow passed each Arrow UTF8 value straight to the column type's input function. textin, varcharin, json_in and jsonb_in do not check the encoding, because core checks client text before they run, and a file the backend reads skips that step. So a stream holding ok, the bytes ff fe and a lone c3 imported 3 rows into a text column, and upper() and ORDER BY on it then failed with invalid byte sequence for encoding "UTF8". read_parquet already refuses the same bytes (pq_text_cell_ok).

The fix

imp_scalar_at now checks every UTF8 value with pg_encoding_verifymbstr(GetDatabaseEncoding(), ...) before any input function sees it, in a new imp_text_check. It mirrors pq_text_cell_ok: an all-ASCII value skips the verifier, and a bad one raises 22021 with the same message shape, naming the Arrow column and the first bad bytes. A NUL byte is refused too, as in the Parquet reader. The check sits in one place, above both text branches. Each import node now carries its target column's name, so an element of a text[] is named the same way.

One change from the brief, said plainly: the brief expected json and jsonb to validate already. They do not. On main, invalid bytes inside a JSON string imported 3 rows into json and into jsonb (red output below). The check runs before every input function, so json and jsonb now refuse such values too. bytea is unchanged and still keeps the raw bytes.

Red: the new tests on main's code (PG18)

E  AssertionError: import_arrow refuses invalid UTF-8 for a text column with 22021, naming the column, and stores no row: got '3|3' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "t": 0xff|0'
E  AssertionError: import_arrow refuses invalid UTF-8 for a varchar(10) column with 22021, naming the column, and stores no row: got '3|3' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "v": 0xff|0'
E  AssertionError: import_arrow refuses a value ending in a lone UTF-8 lead byte (c3) with 22021, and stores no row: got '2|2' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "t": 0xc3|0'
E  AssertionError: import_arrow refuses invalid UTF-8 inside a list<string> for a text[] column with 22021: got '1|1' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "a": 0xff|0'
E  AssertionError: import_arrow refuses invalid UTF-8 inside a JSON string for a json column, and stores no row: got '3|3' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "j": 0xff|0'
E  AssertionError: import_arrow refuses invalid UTF-8 inside a JSON string for a jsonb column, and stores no row: got '3|3' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "j": 0xff|0'
E  AssertionError: import_arrow refuses a value holding a NUL byte with 22021, and stores no row: got '2|2' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "t": 0x00|0'
accounting: 22 pass + 7 fail + 0 unrun = 29
7 failed, 4 passed

Each reading is <what import_arrow returned>|<rows stored>. The 4 that passed on main are the controls: valid multibyte UTF-8 into text and varchar, bytea keeping raw bytes, and valid JSON into json and jsonb.

Green

test/pytest/test_arrow_import_encoding.py:

PostgreSQL 18.6:  accounting: 29 pass + 0 fail + 0 unrun = 29   11 passed
PostgreSQL 17.11: accounting: 29 pass + 0 fail + 0 unrun = 29   11 passed

Existing Arrow files (test_arrow_import.py test_arrow_nested_import.py test_arrow_nested.py test_arrow_export.py test_fuzz_arrow.py), on PG18 and on PG17:

accounting: 140 pass + 0 fail + 2 unrun = 142
57 passed

The 2 unrun are test_arrow_export.py's over-1 GB arm, which declines without PGC_RUN_OVER_1GB=1. test_parallel_copy*.py do not use Arrow, so they were not run.

Builds on PG15, 16, 17, 18 and 19: exit 0, 0 warnings each.

Guard half, --pgc-expect-tests 863: accounting: 2300 pass + 0 fail + 0 unrun = 2300, 863 passed.

pgc_ledger.py census-commits --against origin/main: 0 refused. No commit changes check_ledger.tsv.

Mutant

With the new call disabled (if (n->kind == A_UTF8 && false)), the text check fails again on PG18:

E  AssertionError: import_arrow refuses invalid UTF-8 for a text column with 22021, naming the column, and stores no row: got '3|3' want '22021 invalid byte sequence for encoding "UTF8" in Arrow column "t": 0xff|0'
1 failed

Registries

• expected_tests.txt: cluster_tests 5347 -> 5358, by collection (6221 collected, less guard_tests 863).
• TESTS.md: section 356 names every new test.
• CHANGELOG.md: an entry under Unreleased, Fixed.

🤖 Generated with Claude Code

[OffgridwithJD]

Reviewed at b8306aa.

Verified (PG 18.6, a private prefix copy, new worktree)

• Build: make + make install, 0 warnings.
• Green at the head: pytest test_arrow_import_encoding.py gives 11 passed, accounting: 29 pass + 0 fail + 0 unrun = 29.
• Red with main's src/columnar_arrow.c restored, rebuilt and reinstalled: 7 failed, 4 passed, accounting: 22 pass + 7 fail + 0 unrun = 29. These are the same 7 as in the PR body, and the 4 controls pass.
• Fix restored, then the new file with test_arrow_import.py test_arrow_nested_import.py test_arrow_nested.py test_arrow_export.py test_fuzz_arrow.py: 68 passed, accounting: 169 pass + 0 fail + 2 unrun = 171. The 2 unrun are the over-1 GB arm.
• Registry: --collect-only gives 6221 tests collected. 6221 - 863 = 5358 matches cluster_tests 5358. TESTS.md section 356 is present. test/docs_style.sh: PASSED.
• Code read: imp_text_check reads body + dOff + start for vlen bytes. That range is already proven inside the data buffer, because imp_check_bounds checks that the offsets are non-negative and non-decreasing and that the last offset is within bufLen[dataBuf], so the new read adds no out-of-bounds risk. The bad-byte report is capped at 8 bytes, and buf[41] fits 8 x "0xNN " plus the NUL. The fast path also catches a NUL byte, and the verifier refuses NUL too.

Blocking

• None.

Non-blocking

• Merge-order note: #2039 also adds TESTS.md section 356 and also bumps cluster_tests from 5347 (to 5351). Whichever lands second has to renumber its section to 357 and restate the count (5347 + 4 + 11 = 5362).
• In a non-UTF8 database (for example LATIN1), the verifier accepts any non-NUL byte, so Arrow UTF-8 multibyte text is still stored unconverted as mojibake. That is the same as main and outside this PR's scope. A possible follow-up is to convert with pg_any_to_server(..., PG_UTF8) instead of only verifying.
• The message says "Arrow column" but prints the target column's name. That is fine if import matches columns by name. If it matches by position, "column" alone would be more accurate.

Approving at b8306aa.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Thanks for the approval at b8306aa8. #2036 and #2041 landed and took TESTS.md sections 356 and 357, so I merged main in at cccdffac.

The one hand resolution, in TESTS.md: this PR's test_arrow_import_encoding.py section is renumbered to 358, in both the table of contents (anchor included) and the heading. Main's 356 and 357 are kept. restate.py recounted expected_tests.txt by collection: cluster_tests 5361.

Checked on the merge: test_arrow_import_encoding.py on PG18 gives 11 passed (29 checks). census-commits: 0 refused. Please re-approve this merge-only head.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD This needs your re-approval at the new head. I merged main after #2042 landed, and this branch's TESTS.md section is now 359; no code changed. On the merge I ran test_docs_cover_the_corpus.py, test_harness_deps.py, test_sanitizer_gate.py and this PR's own test file on PG18, and all passed. The PR now links its tracking issue.

[OffgridwithJD]

Approved at 990adb7. Since my approval at b8306aa, the only changes are merges of main.

The change itself is unchanged. The code and test diff against the merge base is byte-identical to what I approved at b8306aa. I compared objstore/, src/ and the new test file between the two. CI is 15 of 15 on this head.

The merges. They only re-register the change on 17d9287: TESTS.md §359 and the cluster_tests line.

Needs another restate. #2043 merged as 85cd464 and took §359, so this head now shows CONFLICTING. It also collides with the other of #2039/#2040 on the same lines. After rebasing onto main, this goes to §360 or later; my docs-audit PR also wants a number, and whichever lands later takes the next one. Ping me and I'll re-approve the restated head quickly.

Nit. The cluster_tests comment's arithmetic doesn't add up: 6221 tests collected, less guard_tests 863 gives 5358, but the line says 5361`. Main's guard_tests is no longer 863. The count is what CI collection checks, so only the comment is stale.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Thanks for approving 990adb7. #2043 and #2047 merged right after, so the branch conflicted and couldn't merge. It's now at 2d04c5f: main merged in, this PR's TESTS.md section renumbered to 360, and no code change. On the merge, test_docs_cover_the_corpus.py, test_harness_deps.py, test_arrow_import_encoding.py and test_arrow_import.py pass on PG18: 88 passed, 319 checks. Whichever of #2039 and #2040 merges second, I'll renumber to 361. Could you re-approve at this head?

[OffgridwithJD]

@jdatcmd Re-reviewed at 2d04c5f. Its code and tests are identical to what I approved at 990adb7: outside TESTS.md, expected_tests.txt and the CHANGELOG, the diff against main matches line for line. All 15 checks pass.

It conflicts with main again, though. #2050 merged as f2075fd and took TESTS.md §360, which this PR claims for test_arrow_import_encoding.py. Only TESTS.md conflicts; the CHANGELOG and expected_tests.txt merge cleanly.

• objstore: refuse a chunk size that wraps the response bound (heap overflow) #2039 and import_arrow: refuse text that is not valid in the database encoding #2040 both claim §360, so one should take §361 and the other §362. My docs(architecture): the module map names every source file and catalog #2052 claims §360 as well, and I'll restate it after both of yours.
• cluster_tests will need restating against whichever of objstore: refuse a chunk size that wraps the response bound (heap overflow) #2039/import_arrow: refuse text that is not valid in the database encoding #2040/iceberg: the remote containment guard reads "..;" and backslashes as a server may #2049 merges first. This PR alone takes it from 5352 to 5363.

Once it's restated I'll re-approve straight away, since there's no code left to review.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD I merged main into this branch again; the new head is 64bba93. Your approval was on an earlier commit, so please re-review.

What the merge brings in

#2049, #2050 and #2051. The only conflict was TESTS.md §360: main's test_docs_links_resolve.py keeps 360, and this PR's section is now 361. cluster_tests is 5372 by collection. The code is unchanged.

Full pytest suite at this head

• PG18: 6264 passed; accounting 11458 pass, 0 fail, 36 unrun.
• PG17: 6264 passed; accounting 11345 pass, 0 fail, 53 unrun.

The unrun tests need a real S3 endpoint, as before. census-commits: 0 refused.

#2039 and your #2052 also want §361. Whichever merges later renumbers.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD I merged main into this branch again after #2052 landed; the new head is d1cd1d0. Please re-review.

What the merge brings in

#2052. The only conflict was TESTS.md §361: main's test_architecture_map.py keeps 361, and this PR's section is now 362. cluster_tests is 5372 by collection, and guard_tests is main's 894. The code is unchanged.

Full pytest suite at this head

• PG18: 6266 passed; accounting 11464 pass, 0 fail, 36 unrun.
• PG17: 6266 passed; accounting 11351 pass, 0 fail, 53 unrun.

The unrun tests need a real S3 endpoint. census-commits: 0 refused.

#2039 also has a §362. Whichever merges second, I'll renumber.

🤖 Generated with Claude Code

[OffgridwithJD]

Adversarial re-review at d1cd1d0: approved.

What I checked

• Against my approval at 990adb7, the code is unchanged. Only the TESTS.md number moved (now §362).
• I built this head on PG 18.6. test_arrow_import_encoding.py and test_arrow_import.py pass: 27 passed.
• Mutant: with the imp_text_check call removed, 7 of 11 tests fail: text, varchar, the lone lead byte, text[], json, jsonb and NUL. The 4 controls pass, as they should.
• No bypass by Arrow type: n->kind comes from the target PostgreSQL type. So an Arrow Binary column into a text target is checked too, and only a bytea target (A_BINARY) skips the check. LargeUtf8, Utf8View, BinaryView and dictionary files are refused before decoding.
• Offsets: imp_check_bounds has already validated offsets as non-negative and non-decreasing within the buffer, so vlen >= 0 and the slice is in bounds before imp_text_check reads it.
• Fast path: an all-ASCII value containing a NUL takes the slow path, because '\0' is in the fast-path break condition, so it is refused. The NUL test pins that.
• Column names: imp_set_colname recurses, so a bad byte inside a struct or list names the top-level column.
• The ledger is right by collection: 6266 less 894 = 5372.

Non-blocking

• Docs: sql-reference.md's import_arrow section doesn't say a value invalid in the database encoding, or one holding a NUL, is refused with 22021. The Parquet section says so (around line 1511). One sentence would match it.
• Encoding (follow-up, shared with the Parquet reader): both readers verify the file's UTF-8 bytes against the database encoding without converting them. In a UTF8 database that is exactly right. In a LATIN1 database, any UTF-8 is "valid" and is stored as mojibake. In EUC_JP, a valid é is refused as an "invalid byte sequence". pg_any_to_server(s, len, PG_UTF8) would convert instead. Nothing in docs/ says a UTF8 database is assumed. This is not a regression, since it is better than storing the raw bytes, but it is worth an issue.

Numbering: #2039 also takes §362, so whichever merges second renumbers to §363 and restates (cluster_tests 5376 with both).

[jdatcmd]

@OffgridwithJD, please re-approve #2040 at f20ab5e8. Your approval is on d1cd1d00, which conflicted with main after #2039 merged.

What changed: f20ab5e8 merges main (673308ae, #2039) into the branch. I resolved conflicts by hand only in the count registries: test/pytest/TESTS.md (this PR's section is now 363) and the cluster test count (now 5376). No source or test code changed in the resolution.

What I ran on the merge. I ran the full pytest suite on PostgreSQL 18.6 and 17.11. No test failed on either version once the library-swapping files ran on their own.

• PG17: 6239 passed with -n 4. test_objstore_module.py, test_extension_upgrade.py and test_format_downgrade.py ran separately without -n: 31 + 5 passed.
• PG18: -n 4 passed 6194 and failed 76 in test_objstore_error_advice.py, test_objstore_http_read.py and test_objstore_listing.py. Each failure was pgcolumnar_objstore.so: file too short or "requires the object-store module". Run without -n, those three files passed (167), and so did test_objstore_module.py (31) and the two swap files (5).

Cause of the 76 failures. test_objstore_module.py renames and corrupts the installed pgcolumnar_objstore.so for a moment. Unlike test_extension_upgrade.py and test_format_downgrade.py, it does not decline under xdist, so any worker that loads the module at that moment fails. This is on main too and is not caused by this PR. I have not filed an issue; that decision is the owner's, because of the alpha5 tag rule.

Ledger census: 0 commits change test/check_ledger.tsv, 0 refused.