Re: Error for WITH options on partitioned tables - Mailing list pgsql-hackers

From David Zhang
Subject Re: Error for WITH options on partitioned tables
Date
Msg-id 2e22bba8-7a07-edec-9859-0b88a640acbd@highgo.ca
Whole thread Raw
In response to Re: Error for WITH options on partitioned tables  (Karina Litskevich <litskevichkarina@gmail.com>)
Responses Re: Error for WITH options on partitioned tables
Re: Error for WITH options on partitioned tables
List pgsql-hackers
Hi Karina,

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.

 From my observation, it seems the WITH option has different behavior 
when creating partitioned table and index. For example,

pgbench -i --partitions=2 --partition-method=range -d postgres

postgres=# create index idx_bid on pgbench_accounts using btree(bid) 
with (fillfactor = 90);
CREATE INDEX

postgres=# select relname, relkind, reloptions from pg_class where 
relnamespace=2200 order by oid;
           relname           | relkind |    reloptions
----------------------------+---------+------------------
  idx_bid                    | I       | {fillfactor=90}
  pgbench_accounts_1_bid_idx | i       | {fillfactor=90}
  pgbench_accounts_2_bid_idx | i       | {fillfactor=90}

I can see the `fillfactor` parameter has been added to the indexes, 
however, if I try to alter `fillfactor`, I got an error like below.
postgres=# alter index idx_bid set (fillfactor=40);
ERROR:  ALTER action SET cannot be performed on relation "idx_bid"
DETAIL:  This operation is not supported for partitioned indexes.

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.


Best regards,

David

On 2022-10-14 8:16 a.m., Karina Litskevich wrote:
> Hi, Simon!
>
> The new error message looks better. But I believe it is better to use
> "parameters" instead of "options" as it is called "storage parameters"
> in the documentation. I also believe it is better to report error in
> partitioned_table_reloptions() (thanks to Japin Li for mentioning this
> function) as it also fixes the error message in the following situation:
>
> test=# CREATE TABLE parted_col_comment (a int, b text) PARTITION BY 
> LIST (a);
> CREATE TABLE
> test=# ALTER TABLE parted_col_comment SET (fillfactor=100);
> ERROR:  unrecognized parameter "fillfactor"
>
> I attached the new versions of patches.
>
> I'm not sure about the errcode. May be it is better to report error with
> ERRCODE_INVALID_OBJECT_DEFINITION for CREATE TABLE and with
> ERRCODE_WRONG_OBJECT_TYPE for ALTER TABLE (as when you try "ALTER TABLE
> partitioned INHERIT nonpartitioned;" an ERROR with 
> ERRCODE_WRONG_OBJECT_TYPE
> is reported). Then either the desired code should be passed to
> partitioned_table_reloptions() or similar checks and ereport calls 
> should be
> placed in two different places. I'm not sure whether it is a good idea to
> change the code in one of these ways just to change the error code.
>
> Best regards,
> Karina Litskevich
> Postgres Professional: http://postgrespro.com/




pgsql-hackers by date:

Previous
From: ilya.v.gladyshev@gmail.com
Date:
Subject: Segfault on logical replication to partitioned table with foreign children
Next
From: Andres Freund
Date:
Subject: Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?