On 2026-10-05 Mo 3:05 PM, shihao zhong wrote:
Hi,I used Opus to analyze the new features in PG 19, and this is one of the things it found. COPY (query) TO with FORMAT json can name the JSON keys after the scanned table's columns, not the query's. create temp table u1 (a int, b int); create temp table u2 (b int, a int); insert into u1 values (10, 1); insert into u2 values (20, 2); copy (select * from u1 union all select * from u2) to stdout (format json); {"a":10,"b":1} {"b":20,"a":2} The query's columns are (a, b), so the second row should be {"a":20,"b":2}. row_to_json() over the same query gives that. Column aliases are lost the same way, when the query returns all columns of a table in order. copy (select a as x, b as y from u1) to stdout (format json); {"a":10,"b":1} I expected {"x":10,"y":1}. The attached copy-json-query-columns.sql runs both cases.
Yes, definitely a bug.
I think the reason is CopyToJsonOneRow() rebuilds the tuple with the query's descriptor only when the slot's tdtypeid is RECORDOID. Then the scan node that does not project returns a slot with the table's row type, so the datum goes to composite_to_json(). 0001 always forms the tuple with the query's descriptor on the query path. That is one heap_form_tuple() per row in place of the tuple copy the old path made. I have not measured it. Another fix is to pass the descriptor to composite_to_json() and skip the extra tuple, but that changes json.c and looks like too much for 19.
Your solution is apparently correct, but when I measured it there was a performance hit of 4% to 7% in some cases, which we really don't want if we can avoid it. So I (and Opus) came up with the attached patch.heap_copy_tuple_as_datum() stamps the copy with whatever descriptor it's given, so we can keep the copy and just pass it the query's descriptor. That is safe because a scan only skips projection when tlist_matches_tupdesc() holds, and that excludes dropped columns and columns with missing values, so the tuple's physical layout matches the query's descriptor. With that change the timings are indistinguishable from master. Virtual slots still use heap_form_tuple() as in your patch.
Please test this out and see if you can break it again ;-) cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
From d839ebe3f14d8ef27a2679a4ccb31a8b0730162c Mon Sep 17 00:00:00 2001 From: Andrew Dunstan <[email protected]> Date: Mon, 5 Oct 2026 16:28:25 -0400 Subject: [PATCH v2] COPY TO FORMAT JSON: use the query's column names as keys CopyToJsonOneRow() stamped the row datum with the query's blessed descriptor only when the output slot's descriptor was of type RECORD. When the top plan node is a scan that does not project, the slot carries the scanned table's row type instead, so composite_to_json() took the keys from the table rather than from the query. For example, with u1 (a int, b int) containing (10, 1) and u2 (b int, a int) containing (20, 2): copy (select * from u1 union all select * from u2) to stdout (format json); {"a":10,"b":1} {"b":20,"a":2} The query's columns are (a, b), so the second row should be {"a":20,"b":2}, which is what row_to_json() over the same query produces. Column aliases were lost in the same way. Fix by always stamping the datum with the query's descriptor. For a virtual slot, form the tuple from the slot's values as before. For any other slot, copy the slot's tuple with heap_copy_tuple_as_datum(), passing the query's descriptor, so this costs no more than the old code did. That is safe because a scan returns its tuple unprojected only when tlist_matches_tupdesc() holds, which rules out dropped columns and columns with missing values, so the tuple's physical layout matches the query's descriptor. Add regression tests for column aliases over an unprojected scan, UNION ALL over tables with different column orders, and a table with a column that has a missing value. Bug introduced in 7dadd38cda9. Discussion: https://postgr.es/m/cagrkxqqhjxgufhpz86+dxgaqzt37hikduuarytfbn2_3mry...@mail.gmail.com Backpatch-through: 19 --- src/backend/commands/copyto.c | 44 ++++++++++++++++++++---------- src/test/regress/expected/copy.out | 13 +++++++++ src/test/regress/sql/copy.sql | 10 +++++++ 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 5850608a3fb..a187d8ec284 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -395,22 +395,38 @@ CopyToJsonOneRow(CopyToState cstate, TupleTableSlot *slot) else { /* - * Full table or query without column list. For queries, the slot's - * TupleDesc may carry RECORDOID, which is not registered in the type - * cache and would cause composite_to_json's lookup_rowtype_tupdesc - * call to fail. Build a HeapTuple stamped with the blessed - * descriptor so the type can be looked up correctly. + * Full table or query without column list. For a query, the slot's + * descriptor is either an unregistered RECORD type, which + * composite_to_json's lookup_rowtype_tupdesc() cannot look up, or, + * when the top plan node does not project, the row type of a scanned + * table, whose column names need not match the query's. Either way, + * the datum must be stamped with the query's blessed descriptor. + * + * A virtual slot has no physical tuple, so form one directly. + * Otherwise copy the slot's tuple and stamp the copy with the query's + * descriptor. That is safe because the tuple's physical layout + * matches the query's result descriptor: a scan returns its scan + * tuple unprojected only if tlist_matches_tupdesc() holds, which + * rules out dropped columns and columns with missing values. */ - if (!cstate->rel && slot->tts_tupleDescriptor->tdtypeid == RECORDOID) - { - HeapTuple tup = heap_form_tuple(cstate->tupDesc, - slot->tts_values, - slot->tts_isnull); - - rowdata = HeapTupleGetDatum(tup); - } - else + if (cstate->rel) rowdata = ExecFetchSlotHeapTupleDatum(slot); + else if (TTS_IS_VIRTUAL(slot)) + rowdata = HeapTupleGetDatum(heap_form_tuple(cstate->tupDesc, + slot->tts_values, + slot->tts_isnull)); + else + { + HeapTuple tup; + bool shouldFree; + + Assert(slot->tts_tupleDescriptor->natts == cstate->tupDesc->natts); + + tup = ExecFetchSlotHeapTuple(slot, false, &shouldFree); + rowdata = heap_copy_tuple_as_datum(tup, cstate->tupDesc); + if (shouldFree) + heap_freetuple(tup); + } } composite_to_json(rowdata, cstate->json_buf, false); diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out index 0af0b646921..979bef5f3f2 100644 --- a/src/test/regress/expected/copy.out +++ b/src/test/regress/expected/copy.out @@ -117,6 +117,19 @@ copy (select * from copytest) to stdout (format json); {"style":"Unix","test":"abc\ndef","filler":2} {"style":"Mac","test":"abc\rdef","filler":3} {"style":"esc\\ape","test":"a\\r\\\r\\\n\\nb","filler":4} +-- keys are the query's column names, also when a scan does not project +copy (select a as x, b as y, c as z from copytest_order) to stdout (format json); +{"x":1,"y":2,"z":3} +create temp table copytest_order2 (c int, b int, a int); +insert into copytest_order2 values (4, 5, 6); +copy (select * from copytest_order union all select * from copytest_order2) + to stdout (format json); +{"a":1,"b":2,"c":3} +{"a":4,"b":5,"c":6} +-- columns with missing values force a projection +alter table copytest_order2 add column d int default 7; +copy (select * from copytest_order2) to stdout (format json); +{"c":4,"b":5,"a":6,"d":7} -- all of the following should yield error copy copytest to stdout (format json, delimiter '|'); ERROR: cannot specify DELIMITER in JSON mode diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql index 14da3c5ec37..6d3923fb3f0 100644 --- a/src/test/regress/sql/copy.sql +++ b/src/test/regress/sql/copy.sql @@ -101,6 +101,16 @@ copy copytest to stdout json; copy copytest to stdout (format json); copy (select * from copytest) to stdout (format json); +-- keys are the query's column names, also when a scan does not project +copy (select a as x, b as y, c as z from copytest_order) to stdout (format json); +create temp table copytest_order2 (c int, b int, a int); +insert into copytest_order2 values (4, 5, 6); +copy (select * from copytest_order union all select * from copytest_order2) + to stdout (format json); +-- columns with missing values force a projection +alter table copytest_order2 add column d int default 7; +copy (select * from copytest_order2) to stdout (format json); + -- all of the following should yield error copy copytest to stdout (format json, delimiter '|'); copy copytest to stdout (format json, null '\N'); -- 2.43.0
