Fix Bug of COPY (on_error set_null) — Array Indexing Mismatch

Core Problem

This thread reports a memory safety bug and correctness violation in the recently added COPY ... (on_error set_null) feature, which allows COPY FROM to set columns to NULL instead of erroring when data type conversion fails. The bug is an array indexing mismatch between where the domain_with_constraint boolean array is written and where it is read, leading to both out-of-bounds reads and incorrect domain constraint enforcement.

The Technical Details

The on_error set_null feature needs to track which columns have domain NOT NULL constraints, because when it decides to substitute NULL for a failed conversion, it must reject that substitution if the target column's type is a domain with a NOT NULL constraint (since ExecConstraints() only checks table-level NOT NULL, not domain-level constraints).

The bug manifests in two functions:

BeginCopyFrom() — Initialization (Write Side): The domain_with_constraint array is allocated with size list_length(cstate->attnumlist) — the number of columns specified in the COPY column list. It is then populated using foreach_current_index(attno), which yields sequential indices 0, 1, 2, ... within the column list iteration. So if you COPY t(b) and b is the second column (attnum=2), the domain constraint flag is stored at index 0.

CopyFromTextLikeOneRow() — Usage (Read Side): The array is accessed using m = attnum - 1, where attnum is the actual attribute number from the table definition. So for column b with attnum=2, it reads index 1 — a different index than where the value was stored.

Consequences

1. Out-of-bounds read: If the COPY column list is shorter than the table's attribute list, accessing domain_with_constraint[attnum - 1] can read beyond the allocated array. For example, a table with 2 columns where COPY specifies only 1 column allocates a 1-element array but may access index 1.

2. Incorrect domain constraint enforcement: Even when the access doesn't go out of bounds, it reads the wrong element, potentially allowing NULL values to be inserted into NOT NULL domain columns. The reporter demonstrates this with a concrete reproduction case: COPY t(b) FROM STDIN WITH (on_error set_null) where b is type dnn (a NOT NULL domain over int). Invalid input for b is silently converted to NULL and inserted, violating the domain constraint.

This is a data integrity violation — the fundamental guarantee that domain constraints are enforced is broken.

Proposed Solutions

The reporter identifies two possible fixes and selects option 1:

Option 1 (Chosen): Allocate by Table Attribute Count

Allocate domain_with_constraint using the full table tuple descriptor's attribute count (tupDesc->natts) and index it by attnum - 1. This makes it consistent with other per-attribute arrays in the COPY state like typioparams[], typiofunc[], etc., which are all indexed by physical attribute number.

Pros:

• Consistent with existing coding patterns in COPY internals — reduces cognitive overhead and future bug risk
• Both write and read sides use the same natural attnum - 1 indexing

Cons:

• Allocates slightly more memory when a column list subset is specified (e.g., a 100-column table with COPY specifying 3 columns would allocate 100 bools instead of 3)

The reporter correctly notes this tradeoff is negligible — it's a small boolean array with short lifetime.

Option 2 (Not Chosen): Fix the Read Side

Change CopyFromTextLikeOneRow() to access domain_with_constraint using the column list index rather than the attribute number. This would require maintaining a mapping or using foreach_current_index during the read loop.

This approach was rejected because it would make domain_with_constraint inconsistent with how all other per-attribute arrays are accessed in the same code path, creating a maintenance hazard.

Additional Cleanup

The patch also converts several palloc calls to palloc_array — the type-safe array allocation macro that was introduced to prevent allocation size calculation errors. This is a minor but welcome modernization.

Duplicate Report

Jian He points out that this same issue was independently reported in another thread (referenced via postgr.es/m/CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com), indicating the bug is readily discoverable through testing and confirming its validity.

Architectural Significance

This bug highlights a recurring pattern in PostgreSQL COPY internals: the tension between column-list-relative indexing and table-attribute-relative indexing. The COPY FROM path must handle partial column lists (where the user specifies a subset of columns), and any per-column metadata arrays must be carefully indexed. The existing convention — index by physical attribute number — exists precisely to avoid this class of bug, and the original on_error set_null patch violated that convention.

The on_error feature itself has been evolving: on_error ignore was added first, and on_error set_null is a newer addition that required tracking domain constraints as an additional concern. Each extension of this feature adds surface area for exactly this kind of indexing bug, suggesting that future enhancements should be especially careful about array allocation and access patterns in the COPY state.