Re: pg_dump multi VALUES INSERT

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Surafel Temesgen <surafel3000(at)gmail(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_dump multi VALUES INSERT
Date: 2019-02-03 08:00:19
Message-ID: alpine.DEB.2.21.1902030841060.18649@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello David,

>> I do not understand why dump_inserts declaration has left the "flags for
>> options" section.
>
> I moved that because it's no longer just a flag. It now stores an int value.

Hmmm. Indeed, all th "int"s of this section should be "bool" instead. Now,
some "flags" do not appear although the culd (clear, createdb, blobs), so
the logic is kinda fuzzy anyway. Do as you wish.

>> I'd suggest not to rely on "atoi" because it does not check the argument
>> syntax, so basically anything is accepted, eg "1O" is 1;
>
> Seems like it's good enough for --jobs and --compress. Do you think
> those should be changed too? or what's the reason to hold
> --rows-per-insert to a different standard?

I think that there is a case for avoiding sloppy "good enough" programming
practices:-) Alas, as you point out, "atoi" is widely used. I'm campaining
to avoid adding more of them. There has been some push to actually remove
"atoi" when not appropriate, eg from "libpq". I'd suggest to consider
starting doing the right thing, and left fixing old patterns to another
patch.

>> There is a test, that is good! Charater "." should be backslashed in the
>> regexpr.
>
> Yeah, you're right. I wonder if we should fix the test of them in
> another patch.

From a software engineering perspective, I'd say that a feature and its
tests really belong to the same patch.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2019-02-03 08:11:24 Re: Synchronize with imath upstream
Previous Message Andrew Gierth 2019-02-03 07:43:57 Re: [Patch] Log10 and hyperbolic functions for SQL:2016 compliance