Re: Error for WITH options on partitioned tables

Hi David,

> I am not very clear about why `build_reloptions` is removed in patch
> `v2-0002-better-error-message-for-setting-parameters-for-p.patch`, if
> you can help explain would be great.

"build_reloptions" parses "reloptions" and takes for it a list of allowed
options defined by the 5th argument "relopt_elems" and the 6th argument
"num_relopt_elems", which are NULL and 0 in the removed call. If "validate"
is false, it ignores options, which are not in the list, while parsing. If
"validate" is true, it "elog"s ERROR when it meets option, which is not in
the
allowed list.

As in the deleted call "build_reloptions" is called with an empty list of
allowed options, it does nothing (returns NULL) when "validate" is false,
and
"elog"s ERROR when "validate" is true and "reloptions" is not empty. That is
what the deleted comment above the deleted call about. This call is there
only
for validation. So as I wanted to make a specific error message for the
case of
partitioned tables, I added the validation in "partitioned_table_reloptions"
and saw no reason to call "build_reloptions" any more because it would just
return NULL in other cases.

> This generic error message is reported by
> `errdetail_relkind_not_supported`, and there is a similar error message
> for partitioned tables. Anyone knows if this can be an option for
> reporting this `fillfactor` parameter not supported error.

This error is reported by "ATSimplePermissions" and, as we can see in the
beginning of this function, it makes no difference between regular relations
and partitioned tables now. To make it report errors for partitioned tables
we
should add new "alter table target-type flag" and add it to a mask of each
"AlterTableType" if partitioned table is an allowed target for it (see that
huge switch-case in function "ATPrepCmd"). Then
"partitioned_table_reloptions"
may become useless and we also should check weather some other functions
become
useless. Maybe that is the right way, but it looks much harder than the
existing solutions, so I believe, before anyone began going this way, it's
better to know whether there are any pitfalls there.

Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/