Date: Sun, 4 Nov 2018 21:44:27 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Surafel Temesgen <surafel3000@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_dump multi VALUES INSERT
Message-ID: <20181105024427.GF18594@tamriel.snowman.net>
References: 
 <CALAY4q9kumSdnRBzvRJvSRf2+BH20YmSvzqOkvwpEmodD-xv6g@mail.gmail.com>
 <27171.1539719339@sss.pgh.pa.us>
 <20181017190528.GD4184@tamriel.snowman.net>
 <20181105023013.GD1718@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="SnV5plBeK2Ge1I9g"
Content-Disposition: inline
In-Reply-To: <20181105023013.GD1718@paquier.xyz>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


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

Greetings,

* Michael Paquier (michael@paquier.xyz) wrote:
> On Wed, Oct 17, 2018 at 03:05:28PM -0400, Stephen Frost wrote:
> > The point of it is that it makes loading into other RDBMS faster.  Yes,
> > it has many of the same issues as our COPY does, but we support it
> > because it's much faster.  The same is true here, just for other
> > databases, so I'm +1 on the general idea.
>=20
> Well, the patch author has mentioned that he cares about also being able
> to detect errors when processing the dump, which multi inserts make that
> easier to check for.  However, even if you specify --data-only you still
> need to worry about the first SET commands ahead, which requires manual
> handling of the dump...

That's hardly a serious complication..

> I am honestly not convinced that it is worth complicating pg_dump for
> that, as there is no guarantee either that the DDLs generated by pg_dump
> will be compatible with what other systems expect.  This kind of
> compatibility for fetch and reload can also be kind of tricky with
> portability issues, so I'd rather let this stuff being handled correctly
> by other tools like pgloader or others rather than expecting to get this
> stuff half-baked within Postgres core tools.

I can see an argument for not wanting to complicate pg_dump, but we've
explicitly stated that the purpose of --inserts is to facilitate
restoring into other database systems and I don't agree that we should
just punt on that entirely.  For better or worse, there's an awful lot
of weight put behind things which are in core and we should take care to
do what we can to make those things better, especially when someone is
proposing a patch to improve the situation.

Sure, the patch might need work or have other issues, but that's an
opportunity for us to provide feedback to the author and encourage them
to improve the patch.

As for the other things that make it difficult to use pg_dump to get a
result out that can be loaded into other database systems- let's try to
improve on that too.  Having an option to skip the 'setup' bits, such as
the SET commands, certainly wouldn't be hard.

I certainly don't see us adding code to pg_dump to handle 'fetch and
reload' into some non-PG system, or, really, even into a PG system, and
that certainly isn't something the patch does, so I don't think it's a
particularly interesting argument.  Users can handle that as needed
themselves.

In other words, none of the arguments put forth really seem to be a
reason to reject the general idea of this patch, so I'm still +1 on
that.  Having just glanced over the patch quickly, I think I would have
done something like '--inserts=3D100' as the way to enable it instead of
adding a new option though.  Not that I feel very strongly about it.

Thanks!

Stephen

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJb366LAAoJEO1sijiDR2RVD14P/jUlgXZmIPOaAd2Nbdsbf68B
0OawoEWSBweZsFSulreYsrHDHA9EHNQ1RjcJB2DEsV0hxSxCUB0ivH635hRYkXvt
gpPwlcT1+BqM8zZpawNHa8FEMr8ly1xMfoLdduo61gIDivKbfGI5OoMjn3h8HD3r
Hr6kQZo5+Lpohte+yhJrGkTzgDy8gHyAlD+Ub8K9PfC+kN7Jm5E7TwxKyY9CZunD
/uASug4NJpE6RJP7SKgVgPiorc0w1HJ4lzWWBGu9IfVWtxx/PxNzQlId1r6x0VXX
o3F+wJNVO19WPEYwWJt9PiaC2+JgVi4lkQIOSzbe/piNQt/G8KlelnoontMEh86g
OFUXurpRiPhNB9z9K1/gL4XnEMv6bWJRdNjxS1CgYV/tcF/SIpB1TE224xp/ectw
VXTvitwvPWZ0ue9s334cL8Z+PVVpHz5iIPk0C+yfVxh3zfEPedyc3qNpNAx53iPP
9nsHkuzkD12ypMU5wdx87MjQeQ26jiL27I4uSdH161hPWbE1Qcs5fOLXnw7hWo1a
4NBlArZfpam0N3ZWpcSrSgi4kqdUD7kyyhqPdDzbQM6qhBXYQD9+EcLmvY489NFh
NJOu72fk9mredHFNUlceEOnudXaM3Czh8T/IiNkuQ1W6F3Tvjdh00+mt95gI7YFT
ohMpfyxnXTLRLllmNAYu
=P9DD
-----END PGP SIGNATURE-----

--SnV5plBeK2Ge1I9g--