On Thu, Sep 10, 2020 at 02:31:32PM +0200, Daniel Gustafsson wrote:
> Given how unintrusive this optimization is, +1 from me to go ahead with this
> patch. pg_dump tests pass. Personally I would've updated the nearby comments
> to reflect why the check for dataOnly is there, but MMV there. I'm moving this
> patch to Ready for Committer.
We need two comments here. I would suggest something like:
"Skip default/check for a data-only dump, as this is only needed for
dumps of the table schema."
Better wording is of course welcome.
> I'm fairly sure there is a lot more we can do to improve the performance of
> data-only dumps, but this nicely chips away at the problem.
I was looking at that, and wondered about cases like the following,
artistic, thing:
CREATE FUNCTION check_data_zz() RETURNS boolean
LANGUAGE sql STABLE STRICT
AS $$select count(a) > 0 from zz$$;
CREATE TABLE public.yy (
a integer,
CONSTRAINT yy_check CHECK (check_data_zz())
);
CREATE TABLE zz (a integer);
INSERT INTO zz VALUES (1);
INSERT INTO yy VALUES (1);
Even on HEAD, this causes the data load to fail because yy's data is
inserted before zz, so keeping track of the CHECK dependency could
have made sense for --data-only if we were to make a better work at
detecting the dependency between both tables and made sure that zz
data needs to appear before yy, but it is not like this would happen
easily in pg_dump, and we document it this way (see the warning about
dump/reload in ddl.sgml for check constraints). In short, I think
that this patch looks like a good thing to have.
--
Michael
