Bruce Momjian <bruce@momjian.us> writes:
> On Mon, Aug 25, 2014 at 11:30:53PM +0200, Andres Freund wrote:
>> I still think this is a bad idea and should rather be reverted.
> Uh, which idea don't you like. I thought the discussion had a clear
> conclusion.
The discussion seemed to have a consensus that an error would be a good
thing, but the problem is that this implementation is complete junk.
transformRelOptions has *no* business throwing an error, because it has
no idea what the set of valid options is; notice the patch didn't even
touch the comment it falsified:
* Note that this is not responsible for determining whether the options
* are valid, but it does check that namespaces for all the options given are
The kluge that was used instead of actual knowledge (ie look at the
relOptions array) isn't an adequate answer. This coding will allow
options that aren't valid for the particular relkind, and it will probably
reject extension options that should be allowed, and it certainly has no
clue whether the option is valid in the particular option namespace.
A larger point is that we could easily consider RESET as meaning
"remove this option *if it's applied to this relation*", which would
mean that resetting a nonexistent option shouldn't be an error.
If we don't define the action that way, then should RESET foo, where
foo is a valid option that's not been set on the particular table,
be an error? If not, what's the argument for allowing that case
and not this one? Do we need a RESET IF EXISTS to cover that?
Please revert and return the patch for further work/discussion.
We had consensus on a vague idea, not the details of this particular
patch.
regards, tom lane
