From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>
Subject: Re: WITH clause in CREATE STATISTICS
Date: Fri, 12 May 2017 20:03:04 -0400
Message-ID: <20075.1494633784@sss.pgh.pa.us> (raw)
In-Reply-To: <20170512221010.nglatgt5azzdxjlj@alvherre.pgsql>
References: <20170512221010.nglatgt5azzdxjlj@alvherre.pgsql>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> Hm. That would behave less than desirably if getObjectClass() could
>> return a value that wasn't a member of the enum, but it's hard to
>> credit that happening. I guess I'd vote for removing the default:
>> case from all of the places that have "switch (getObjectClass(object))",
>> as forgetting to add an entry seems like a much larger hazard.
> Ignoring the one in alter.c's AlterObjectNamespace_oid, which only
> handles a small subset of the enum values, that gives the following
> warnings:
> /pgsql/source/master/src/backend/catalog/dependency.c: In function 'doDeletion':
> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_ROLE' not handled in switch [-Wswitch]
> switch (getObjectClass(object))
> ^
> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_DATABASE' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_TBLSPACE' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_SUBSCRIPTION' not handled in switch [-Wswitch]
Hm. I think it's intentional that shared objects are not handled there,
but I wonder whether the lack of SUBSCRIPTION represents a bug.
Anyway I think it would be fine to add those to the switch as explicit
error cases.
> /pgsql/source/master/src/backend/commands/tablecmds.c: In function 'ATExecAlterColumnType':
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_AM' not handled in switch [-Wswitch]
> switch (getObjectClass(&foundObject))
> ^
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_STATISTIC_EXT' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_EVENT_TRIGGER' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_PUBLICATION' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_PUBLICATION_REL' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_SUBSCRIPTION' not handled in switch [-Wswitch]
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_TRANSFORM' not handled in switch [-Wswitch]
All of those ought to go into the category that prints
"unexpected object depending on column", I think; certainly
"unrecognized object class" is not a better report, and the fact that
we've evidently not touched this switch in the last few additions of
object types provides fodder for the idea that the default: is unhelpful.
(Not to mention that not having OCLASS_STATISTIC_EXT here is an outright
bug as of today...) So losing the default: case here seems good to me.
Alternatively, we could drop the explicit listing of oclasses we're not
expecting to see, and let the default: case be "unexpected object
depending on column". Treating the default separately is only of value if
we'd expect getObjectDescription to fail, but there's no good reason for
this code to be passing judgment on whether that might happen.
What do we want this to do about statistics objects? We could either
throw a feature-not-implemented error or try to update the stats
object for the new column type.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@2ndquadrant.com, simon@2ndquadrant.com, andrew.dunstan@2ndquadrant.com, tomas.vondra@2ndquadrant.com, dean.a.rasheed@gmail.com
Subject: Re: WITH clause in CREATE STATISTICS
In-Reply-To: <20075.1494633784@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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