Обсуждение: [COMMITTERS] pgsql: Give a better error for duplicate entries in VACUUM/ANALYZEcolu
Give a better error for duplicate entries in VACUUM/ANALYZE column list. Previously, the code didn't think about this case and would just try to analyze such a column twice. That would fail at the point of inserting the second version of the pg_statistic row, with obscure error messsages like "duplicate key value violates unique constraint" or "tuple already updated by self", depending on context and PG version. We could allow the case by ignoring duplicate column specifications, but it seems better to reject it explicitly. The bogus error messages seem like arguably a bug, so back-patch to all supported versions. Nathan Bossart, per a report from Michael Paquier, and whacked around a bit by me. Discussion: https://postgr.es/m/E061A8E3-5E3D-494D-94F0-E8A9B312BBFC@amazon.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/71480501057fee9fa3649b072173ff10e2b842d0 Modified Files -------------- src/backend/commands/analyze.c | 25 ++++++++++++++++++------- src/test/regress/expected/vacuum.out | 5 +++++ src/test/regress/sql/vacuum.sql | 5 +++++ 3 files changed, 28 insertions(+), 7 deletions(-) -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Give a better error for duplicate entries inVACUUM/ANALYZE colu
От
Peter Eisentraut
Дата:
On 9/21/17 18:13, Tom Lane wrote: > Give a better error for duplicate entries in VACUUM/ANALYZE column list. > > Previously, the code didn't think about this case and would just try to > analyze such a column twice. In the error message, we should write "specified more than once" instead of "specified twice", because that could otherwise look a bit silly: VACUUM ANALYZE vaccluster(i,i,i); ERROR: column "i" of relation "vaccluster" is specified twice (Also, the "is" doesn't seem to fit there.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/21/17 18:13, Tom Lane wrote:
>> Give a better error for duplicate entries in VACUUM/ANALYZE column list.
> In the error message, we should write "specified more than once" instead
> of "specified twice", because that could otherwise look a bit silly:
> VACUUM ANALYZE vaccluster(i,i,i);
> ERROR: column "i" of relation "vaccluster" is specified twice
OK.
> (Also, the "is" doesn't seem to fit there.)
Hm, reads fine to me, and I'd still rather include "is" in the
revised wording. Anybody else agree with Peter's wording?
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Give a better error for duplicate entries inVACUUM/ANALYZE colu
От
Peter Eisentraut
Дата:
On 9/25/17 15:09, Tom Lane wrote: > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes: >> On 9/21/17 18:13, Tom Lane wrote: >>> Give a better error for duplicate entries in VACUUM/ANALYZE column list. > >> In the error message, we should write "specified more than once" instead >> of "specified twice", because that could otherwise look a bit silly: >> VACUUM ANALYZE vaccluster(i,i,i); >> ERROR: column "i" of relation "vaccluster" is specified twice > > OK. > >> (Also, the "is" doesn't seem to fit there.) > > Hm, reads fine to me, and I'd still rather include "is" in the > revised wording. Anybody else agree with Peter's wording? Note a big deal. I'm just working off existing error messages: msgid "parameter \"%s\" specified more than once" msgid "column name \"%s\" specified more than once" msgid "column \"%s\" specified more than once" msgid "filter variable \"%s\" specified more than once" msgid "option \"%s\" provided more than once" msgid "parameter name \"%s\" used more than once" msgid "storage type specified more than once" msgid "procedure number %d for (%s,%s) appears more than once" msgid "operator number %d for (%s,%s) appears more than once" msgid "publication name \"%s\" used more than once" msgid "relation \"%s\" would be inherited from more than once" msgid "column \"%s\" appears more than once in partition key" msgid "column name \"%s\" appears more than once in USING clause" msgid "common column name \"%s\" appears more than once in left table" msgid "common column name \"%s\" appears more than once in right table" msgid "WITH query name \"%s\" specified more than once" msgid "recursive reference to query \"%s\" must not appear more than once" msgid "XML attribute name \"%s\" appears more than once" msgid "argument name \"%s\" used more than once" msgid "table name \"%s\" specified more than once" -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: Give a better error for duplicate entries inVACUUM/ANALYZE colu
От
"David G. Johnston"
Дата:
On 9/25/17 15:09, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> On 9/21/17 18:13, Tom Lane wrote:
>>> Give a better error for duplicate entries in VACUUM/ANALYZE column list.
>
>> In the error message, we should write "specified more than once" instead
>> of "specified twice", because that could otherwise look a bit silly:
>> VACUUM ANALYZE vaccluster(i,i,i);
>> ERROR: column "i" of relation "vaccluster" is specified twice
>
> OK.
>
>> (Also, the "is" doesn't seem to fit there.)
>
> Hm, reads fine to me, and I'd still rather include "is" in the
> revised wording. Anybody else agree with Peter's wording?
Note a big deal. I'm just working off existing error messages:
About half of those, especially the "appears" ones, seem unhelpful for deciding whether to add "is" here; "is appears" just doesn't work.
I think the added length due to the "of relation %", makes dropping the 'is' sound more odd than those like "column % specified"
The middle ground would be writing: column "i" of relation "vaccluster" appears more than once; I'm good with using appears instead of deciding between [is] specified.
I'm not seeing that we have a formal distinction between "specified" and "appears"...
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Sep 25, 2017 at 1:50 PM, Peter Eisentraut <
> peter.eisentraut@2ndquadrant.com> wrote:
>> On 9/25/17 15:09, Tom Lane wrote:
>>> Hm, reads fine to me, and I'd still rather include "is" in the
>>> revised wording. Anybody else agree with Peter's wording?
>> Note a big deal. I'm just working off existing error messages:
> About half of those, especially the "appears" ones, seem unhelpful for
> deciding whether to add "is" here; "is appears" just doesn't work.
Peter's evidence is pretty conclusive that using "is" is not consistent
with our message style precedents, but there's still room to choose
which precedent to follow ;-).
> The middle ground would be writing: column "i" of relation "vaccluster"
> appears more than once; I'm good with using appears instead of deciding
> between [is] specified.
Yeah, I like "appears more than once" too. It does not leave one feeling
that "is" has been left out. Also, "specified" seems like an
unnecessarily formal word here. Per Mark Twain, "Don't use a five-dollar
word when a fifty-cent word will do."
I'll make this change in the back-patched patch, but Peter's list suggests
that it'd be worth trying to make a lot of these other usages more
consistent in HEAD.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers