pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
12+ messages / 3 participants
[nested] [flat]

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL_10_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/a2b1eb23496e9abd695036e3cbb1d39c05618692

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  5 +++++
3 files changed, 22 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL9_6_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/ea31541f564851611fee58fe2019c866ce0e8aa0

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  4 ++++
3 files changed, 21 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL9_4_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/b572b435ca67a2c032187d376baa271f9049b7cf

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  4 ++++
3 files changed, 21 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL9_2_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/e56facd8b3005aee30fe3d10ef82504d99a2f236

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  4 ++++
3 files changed, 21 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL9_3_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/a09d8be7ddaf3d5bccbd1cc1138895fde379d15e

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  4 ++++
3 files changed, 21 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-21 22:13  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-21 22:13 UTC (permalink / raw)
  To: pgsql-committers

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
------
REL9_5_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/122289a66b92799885917953907d5b3f3f9e6ac2

Modified Files
--------------
src/backend/commands/analyze.c       | 13 ++++++++++++-
src/test/regress/expected/vacuum.out |  5 +++++
src/test/regress/sql/vacuum.sql      |  4 ++++
3 files changed, 21 insertions(+), 1 deletion(-)


-- 
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-25 18:51  Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 12+ messages in thread

From: Peter Eisentraut @ 2017-09-25 18:51 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers

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



^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-25 19:09  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 12+ messages in thread

From: Tom Lane @ 2017-09-25 19:09 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: 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



^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-25 20:50  Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 12+ messages in thread

From: Peter Eisentraut @ 2017-09-25 20:50 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers

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



^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-25 21:15  David G. Johnston <david.g.johnston@gmail.com>
  parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 12+ messages in thread

From: David G. Johnston @ 2017-09-25 21:15 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers

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:
> > 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.

^ permalink  raw  reply  [nested|flat] 12+ messages in thread

* Re: pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu
@ 2017-09-26 19:09  Tom Lane <tgl@sss.pgh.pa.us>
  parent: David G. Johnston <david.g.johnston@gmail.com>
  0 siblings, 0 replies; 12+ messages in thread

From: Tom Lane @ 2017-09-26 19:09 UTC (permalink / raw)
  To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; pgsql-committers

"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



^ permalink  raw  reply  [nested|flat] 12+ messages in thread


end of thread, other threads:[~2017-09-26 19:09 UTC | newest]

Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-25 18:51 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-09-25 19:09   ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-25 20:50     ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-09-25 21:15       ` David G. Johnston <david.g.johnston@gmail.com>
2017-09-26 19:09         ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 22:13 pgsql: Give a better error for duplicate entries in VACUUM/ANALYZE colu Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox