Re: Error for WITH options on partitioned tables

Поиск
Список
Период
Сортировка
От Karina Litskevich
Тема Re: Error for WITH options on partitioned tables
Дата
Msg-id CACiT8iary_fUiRpU3=mkBMSqYaUAK8jVT4eO0cnbvVVVuFoSGA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Error for WITH options on partitioned tables  (David Zhang <david.zhang@highgo.ca>)
Ответы Re: Error for WITH options on partitioned tables  (Simon Riggs <simon.riggs@enterprisedb.com>)
Список pgsql-hackers
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/

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Simon Riggs
Дата:
Сообщение: Re: Allow single table VACUUM in transaction block
Следующее
От: Aleksander Alekseev
Дата:
Сообщение: [PATCH] Const'ify the arguments of ilist.c/ilist.h functions