MIME-Version: 1.0
References: <CALvfUkBxTYy5uWPFVwpk_7ii2zgT07t3d-yR_cy4sfrrLU=kcg@mail.gmail.com>
 <fc1e39f8-274f-4be6-8a70-9fb0d9fe84a2@aklaver.com> <CAFCRh-_LhP5Ln4-beVz=1pQ9VycU3hAxnJHXhPHZRmX0BSCVEQ@mail.gmail.com>
 <CAFCRh--Vhx0105S7SWAYHGcVMTSRtbcoArDUGSbYSWRUGunW_g@mail.gmail.com>
 <CAKFQuwZb3zaSqkOvGXRRLZEW61az+4xyHZ7tBPR91JfrcdCKvA@mail.gmail.com>
 <3853387.1701096982@sss.pgh.pa.us> <CAFCRh-_GdiUvjd5z5FfvTfhruOnYqBu163XU47zZE8RNATCJGQ@mail.gmail.com>
 <3a98decf-3fe3-4b49-9b68-fda01338872c@sedlakovi.org> <CALvfUkCq3pvkmniWEKZwkbA6hNNvkANF3OamMfhC170RgY+HJg@mail.gmail.com>
 <24e3ee88-ec1e-421b-89ae-8a47ee0d2df1@joeconway.com> <20231201230958.GA1786735@nathanxps13>
In-Reply-To: <20231201230958.GA1786735@nathanxps13>
From: Davin Shearer <davin@apache.org>
Date: Fri, 1 Dec 2023 22:00:29 -0500
Message-ID: <CALvfUkByf6eou6J36-2Hw7Kdro80JD_Ch1AeUDAjsX8RpcA==A@mail.gmail.com>
Subject: Re: Emitting JSON to file using COPY TO
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Joe Conway <mail@joeconway.com>, Nathan Bossart <nathandbossart@gmail.com>
Content-Type: multipart/alternative; boundary="00000000000042df92060b7e17c0"
Archived-At: <https://www.postgresql.org/message-id/CALvfUkByf6eou6J36-2Hw7Kdro80JD_Ch1AeUDAjsX8RpcA%3D%3DA%40mail.gmail.com>
Precedence: bulk

--00000000000042df92060b7e17c0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

I'm really glad to see this taken up as a possible new feature and will
definitely use it if it gets released.  I'm impressed with how clean,
understandable, and approachable the postgres codebase is in general and
how easy it is to read and understand this patch.

I reviewed the patch (though I didn't build and test the code) and have a
concern with adding the '[' at the beginning and ']' at the end of the json
output.  Those are already added by `json_agg` (
https://www.postgresql.org/docs/current/functions-aggregate.html) as you
can see in my initial email.  Adding them in the COPY TO may be redundant
(e.g., [[{"key":"value"...}....]]).

I think COPY TO makes good sense to support, though COPY FROM maybe not so
much as JSON isn't necessarily flat and rectangular like CSV.

For my use-case, I'm emitting JSON files to Apache NiFi for processing, and
NiFi has superior handling of JSON (via JOLT parsers) versus CSV where
parsing is generally done with regex.  I want to be able to emit JSON using
a postgres function and thus COPY TO.

Definitely +1 for COPY TO.

I don't think COPY FROM will work out well unless the JSON is required to
be flat and rectangular.  I would vote -1 to leave it out due to the
necessary restrictions making it not generally useful.

Hope it helps,
Davin

On Fri, Dec 1, 2023 at 6:10=E2=80=AFPM Nathan Bossart <nathandbossart@gmail=
.com>
wrote:

> On Fri, Dec 01, 2023 at 02:28:55PM -0500, Joe Conway wrote:
> > I did a quick PoC patch (attached) -- if there interest and no hard
> > objections I would like to get it up to speed for the January commitfes=
t.
>
> Cool.  I would expect there to be interest, given all the other JSON
> support that has been added thus far.
>
> I noticed that, with the PoC patch, "json" is the only format that must b=
e
> quoted.  Without quotes, I see a syntax error.  I'm assuming there's a
> conflict with another json-related rule somewhere in gram.y, but I haven'=
t
> tracked down exactly which one is causing it.
>
> > 1. Is supporting JSON array format sufficient, or does it need to suppo=
rt
> > some other options? How flexible does the support scheme need to be?
>
> I don't presently have a strong opinion on this one.  My instinct would b=
e
> start with something simple, though.  I don't think we offer any special
> options for log_destination...
>
> > 2. This only supports COPY TO and we would undoubtedly want to support
> COPY
> > FROM for JSON as well, but is that required from the start?
>
> I would vote for including COPY FROM support from the start.
>
> > !     if (!cstate->opts.json_mode)
>
> I think it's unfortunate that this further complicates the branching in
> CopyOneRowTo(), but after some quick glances at the code, I'm not sure it=
's
> worth refactoring a bunch of stuff to make this nicer.
>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com
>

--00000000000042df92060b7e17c0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">I&#39;m really glad to see this taken up as a possible new=
 feature and will definitely use=C2=A0it if it gets=C2=A0released.=C2=A0 I&=
#39;m impressed with how clean, understandable, and approachable the postgr=
es codebase is in general and how easy it=C2=A0is to=C2=A0read and understa=
nd this patch.<div><br></div><div>I reviewed the patch (though I didn&#39;t=
 build and test the code) and have a concern with adding the &#39;[&#39; at=
 the beginning and &#39;]&#39; at the end of the json output.=C2=A0 Those a=
re already added by `json_agg` (<a href=3D"https://www.postgresql.org/docs/=
current/functions-aggregate.html">https://www.postgresql.org/docs/current/f=
unctions-aggregate.html</a>) as you can see in my initial email.=C2=A0 Addi=
ng them in the COPY TO may be redundant (e.g., [[{&quot;key&quot;:&quot;val=
ue&quot;...}....]]).</div><div><br></div><div>I think COPY TO makes good se=
nse to support, though COPY FROM maybe not so much as JSON isn&#39;t necess=
arily flat and rectangular like CSV.</div><div><br></div><div>For my use-ca=
se, I&#39;m emitting JSON files to Apache NiFi for processing, and NiFi has=
 superior handling of JSON (via JOLT parsers) versus CSV where parsing is g=
enerally done with regex.=C2=A0 I want to be able to emit JSON using a post=
gres function and thus COPY TO.</div><div><br></div><div>Definitely=C2=A0+1=
 for COPY TO.</div><div><br></div><div>I don&#39;t think COPY FROM will wor=
k out well unless the JSON is required to be flat and rectangular.=C2=A0 I =
would vote -1 to leave it out due to the necessary restrictions making it n=
ot generally useful.</div><div><br></div><div>Hope it helps,</div><div>Davi=
n</div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail=
_attr">On Fri, Dec 1, 2023 at 6:10=E2=80=AFPM Nathan Bossart &lt;<a href=3D=
"mailto:nathandbossart@gmail.com">nathandbossart@gmail.com</a>&gt; wrote:<b=
r></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex=
;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,20=
4,204);padding-left:1ex">On Fri, Dec 01, 2023 at 02:28:55PM -0500, Joe Conw=
ay wrote:<br>
&gt; I did a quick PoC patch (attached) -- if there interest and no hard<br=
>
&gt; objections I would like to get it up to speed for the January commitfe=
st.<br>
<br>
Cool.=C2=A0 I would expect there to be interest, given all the other JSON<b=
r>
support that has been added thus far.<br>
<br>
I noticed that, with the PoC patch, &quot;json&quot; is the only format tha=
t must be<br>
quoted.=C2=A0 Without quotes, I see a syntax error.=C2=A0 I&#39;m assuming =
there&#39;s a<br>
conflict with another json-related rule somewhere in gram.y, but I haven=
9;t<br>
tracked down exactly which one is causing it.<br>
<br>
&gt; 1. Is supporting JSON array format sufficient, or does it need to supp=
ort<br>
&gt; some other options? How flexible does the support scheme need to be?<b=
r>
<br>
I don&#39;t presently have a strong opinion on this one.=C2=A0 My instinct =
would be<br>
start with something simple, though.=C2=A0 I don&#39;t think we offer any s=
pecial<br>
options for log_destination...<br>
<br>
&gt; 2. This only supports COPY TO and we would undoubtedly want to support=
 COPY<br>
&gt; FROM for JSON as well, but is that required from the start?<br>
<br>
I would vote for including COPY FROM support from the start.<br>
<br>
&gt; !=C2=A0 =C2=A0 =C2=A0if (!cstate-&gt;opts.json_mode)<br>
<br>
I think it&#39;s unfortunate that this further complicates the branching in=
<br>
CopyOneRowTo(), but after some quick glances at the code, I&#39;m not sure =
it&#39;s<br>
worth refactoring a bunch of stuff to make this nicer.<br>
<br>
-- <br>
Nathan Bossart<br>
Amazon Web Services: <a href=3D"https://aws.amazon.com" rel=3D"noreferrer" =
target=3D"_blank">https://aws.amazon.com</a><br>
</blockquote></div>

--00000000000042df92060b7e17c0--