Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 7 Dec 2023 09:38:59 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Junwang Zhao <zhjwpku@gmail.com>
Cc: Sutou Kouhei <kou@clear-code.com>, nathandbossart@gmail.com,
	pgsql-hackers@postgresql.org
Subject: Re: Make COPY format extendable: Extract COPY TO format
 implementations
Message-ID: <ZXEUIy6wl4jHy6Nm@paquier.xyz>
References: <CAEG8a3Jf7kPV3ez5OHu-pFGscKfVyd9KkubMF199etkfz=EPRg@mail.gmail.com>
 <20231206.151908.897707799247386849.kou@clear-code.com>
 <CAEG8a3K9dE2gt3+K+h=DwTqMenR84aeYuYS+cty3SR3LAeDBAQ@mail.gmail.com>
 <20231206.162834.1390394858102165899.kou@clear-code.com>
 <CAEG8a3LSRhK601Bn50u71BgfNWm4q3kv-o-KEq=hrbyLbY_EsA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="cvQUBATM3B8Y3T9j"
Content-Disposition: inline
In-Reply-To: <CAEG8a3LSRhK601Bn50u71BgfNWm4q3kv-o-KEq=hrbyLbY_EsA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZXEUIy6wl4jHy6Nm%40paquier.xyz>
Precedence: bulk


--cvQUBATM3B8Y3T9j
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 06, 2023 at 10:07:51PM +0800, Junwang Zhao wrote:
> I read the thread[1] you posted and I think Andres's suggestion sounds gr=
eat.
>=20
> Should we extract both *copy to* and *copy from* for the first step, in t=
hat
> case we can add the pg_copy_handler catalog smoothly later.
>=20
> Attached V4 adds 'extract copy from' and it passed the cirrus ci,
> please take a look.
>=20
> I added a hook *copy_from_end* but this might be removed later if not use=
d.
>=20
> [1]: https://www.postgresql.org/message-id/20180211211235.5x3jywe5z3lkgcs=
r%40alap3.anarazel.de

I was looking at the differences between v3 posted by Sutou-san and
v4 from you, seeing that:

+/* Routines for a COPY HANDLER implementation. */
+typedef struct CopyHandlerOps
 {
     /* Called when COPY TO is started. This will send a header. */
-    void        (*start) (CopyToState cstate, TupleDesc tupDesc);
+    void        (*copy_to_start) (CopyToState cstate, TupleDesc tupDesc);
=20
     /* Copy one row for COPY TO. */
-    void        (*one_row) (CopyToState cstate, TupleTableSlot *slot);
+    void        (*copy_to_one_row) (CopyToState cstate, TupleTableSlot *sl=
ot);
=20
     /* Called when COPY TO is ended. This will send a trailer. */
-    void        (*end) (CopyToState cstate);
-} CopyToFormatOps;
+    void        (*copy_to_end) (CopyToState cstate);
+
+    void        (*copy_from_start) (CopyFromState cstate, TupleDesc tupDes=
c);
+    bool        (*copy_from_next) (CopyFromState cstate, ExprContext *econ=
text,
+                                    Datum *values, bool *nulls);
+    void        (*copy_from_error_callback) (CopyFromState cstate);
+    void        (*copy_from_end) (CopyFromState cstate);
+} CopyHandlerOps;

And we've spent a good deal of time refactoring the copy code so as
the logic behind TO and FROM is split.  Having a set of routines that
groups both does not look like a step in the right direction to me,
and v4 is an attempt at solving two problems, while v3 aims to improve
one case.  It seems to me that each callback portion should be focused
on staying in its own area of the code, aka copyfrom*.c or copyto*.c.
--
Michael

--cvQUBATM3B8Y3T9j
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmVxFCMACgkQnvQgOdby
QH1tKA/+NGCDhrVdDossqqDxpQBFevQCaF68W+PnAyeLS8aivVQjibw4iv6I4kJS
Tt+YP0VmUY9rSJ8AEz7fMPikogv49qui1v74vjaezwkkyiwwOKseFgeL2kMZ9hQz
Gsozv72Dp+LlLM4DBGAH+RHEa4TYYjUDzHYjBsfMKCFlfIhgc0/KUYFGB4MwpPB+
r2TOzpCWKDVnAyzacXQEdm60CPJ8CAwRG8+iiu/ZvXgAp2B4S1+qoo/k7ahnb5qa
c7zTCgooEKycaWG7nPXEeguGBtXZ3J7omGILdd/1qXI13xHg9zK0SY7kEObryldF
iXxlaeIVpVXNFwpohV5JkXJgqQQUnv1zk4oNmjZS9LBcjpxuta6iBhUTK6pJYEIs
2JstC9TLW6Qz2Aozm2SnVbRuOPVcwnh5tp8fRPkxFpDXPmCRfujzsNCesh8RcUQP
51XnAIGkIeLshu1mknsNKqw/nNY01y03xV8cAbbxabEkkAnhcMzdjNKYDKACF594
JY9iP08RhIQ5Z1QtyShUTwWCz2/i/0tF8o1cxur24fxUiDzm1p7Zbj9PgMUjvbCI
VS1ZKGRkyp5WeL2t6YIEmmeMAkzLDodiCouQR/46gvmo0YQUk3IyqyafvHq8pxMe
ir3t+Ee8Zq9cyPXbRuz88gpkMuKICsw19XHQzDblKHXv0wlzS1w=
=T7+F
-----END PGP SIGNATURE-----

--cvQUBATM3B8Y3T9j--