Re: [HACKERS] WITH clause in CREATE STATISTICS - Mailing list pgsql-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
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_TBLSPACE' not
handledin switch [-Wswitch] 

> /pgsql/source/master/src/backend/catalog/dependency.c:1106:2: warning: enumeration value 'OCLASS_SUBSCRIPTION' not
handledin 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
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_EVENT_TRIGGER' not
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_PUBLICATION' not
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_PUBLICATION_REL' not
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_SUBSCRIPTION' not
handledin switch [-Wswitch] 
> /pgsql/source/master/src/backend/commands/tablecmds.c:9112:3: warning: enumeration value 'OCLASS_TRANSFORM' not
handledin 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