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
signature.asc
Description: PGP signature