| From: | Joao Detomini <joao(dot)detomini(at)enterprisedb(dot)com> |
|---|---|
| To: | andrew(at)dunslane(dot)net |
| Cc: | shihao zhong <zhong950419(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Joe Conway <mail(at)joeconway(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com> |
| Subject: | Re: [PG19] COPY (query) TO ... (FORMAT json) uses the table's column names |
| Date: | 2026-10-05 22:37:08 |
| Message-ID: | CABH8dKzCFYg9ZVyh6hKPuJOpCMm1Rdvwmrr6jo2g8LSVpeO=vg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Andrew,
Thanks for v2. I tried to break it and couldn't.
I compared COPY (query) TO STDOUT (format json) against row_to_json()
over the same query on about 37 shapes (fast defaults, dropped and
added columns, inheritance, partitions with different column order,
toasted values, joins, Sort/Limit/LockRows over a plain scan), with
parallel plans forced, on an assert-enabled build. The output was
identical every time, and the new Assert never fired.
It also fixes a case not in the report or the tests: a view with
renamed columns, or a CTE with a column alias list, selected through
an inheritance parent. Before, the parent's rows used the table's
column names and the child's rows used the query's, in the same
result.
Regards,
João Marcelo
Em seg., 5 de out. de 2026 às 18:05, Andrew Dunstan <andrew(at)dunslane(dot)net>
escreveu:
>
> 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 | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-10-05 23:33:55 | Re: Logical replication: lost updates/deletes and invalid log messages caused by SnapshotDirty + concurrent updates |
| Previous Message | Mihail Nikalayeu | 2026-10-05 22:36:00 | Re: [BUG?] check_exclusion_or_unique_constraint false negative |