> This seems to be a harmless enough change, and if we're going to do it
> we should do it now.
+1
> OTOH, any application that relies on a particular field ordering in JSON
> data is broken by design, IMNSHO.
This should be done for the sake of consistent behavior when the COPY
TO command is supplied with an explicit list.
My comments on the patch:
1/
+ *
+ * This fires when the user gave an explicit column list (which may
+ * subset or reorder columns) or when the default list excludes
+ * generated columns.
*/
This comment is not needed.
But I think we should add a comment explaining the condition here.
- if (rel && list_length(cstate->attnumlist) < tupDesc->natts)
+ if (rel && (attnamelist != NIL ||
+ list_length(cstate->attnumlist) < tupDesc->natts))
{
Also maybe simplify this comment
2/
+-- column list that reorders all columns must be honored in the JSON output,
+-- like text/CSV (the keys must appear in the requested order, not the table's
+-- physical order)
Should we just add tests for all the output types?
copy2.sql has a reordering test for text
-- column reordering
COPY rls_t1 (b, a) TO stdout;
but that is just incidental for rls testing.
--
Sami Imseih
Amazon Web Services (AWS)
