Обсуждение: [HACKERS] error detail when partition not found
Simon pointed out in a nearby thread [0] that the detail part of partition-not-found error should show just the partition keys. I posted a patch on that thread [1], but to avoid confusion being caused by multitude of patches over there I'm re-posting it here. * What the patch does: Currently we show the whole row in the detail part of the error. CREATE TABLE measurement_year_month ( logdate date not null, peaktemp int, unitsales int ) PARTITION BY RANGE (EXTRACT(YEAR FROM logdate), EXTRACT(MONTH FROM logdate)); # INSERT INTO measurement_year_month VALUES ('2016-12-02', 1, 1); ERROR: no partition of relation "measurement_year_month" found for row DETAIL: Failing row contains (2016-12-02, 1, 1). Patch changes it look like the following: # INSERT INTO measurement_year_month VALUES ('2016-12-02', 1, 1); ERROR: no partition of relation "measurement_year_month" found for row DETAIL: Partition key of the failing row contains (date_part('year'::text, logdate), date_part('month'::text, logdate))=(2016, 12). It's similar to error detail shown when btree unique violation occurs: -- just to be clear, using LIKE won't make measurement partitioned too CREATE TABLE measurement (LIKE measurement_year_month); CREATE UNIQUE INDEX ON measurement (EXTRACT(YEAR FROM logdate), EXTRACT(MONTH FROM logdate)) # INSERT INTO measurement VALUES ('2016-12-02', 1, 1); INSERT 0 1 # INSERT INTO measurement VALUES ('2016-12-02', 1, 1); ERROR: duplicate key value violates unique constraint "measurement_date_part_date_part1_idx" DETAIL: Key (date_part('year'::text, logdate), date_part('month'::text, logdate))=(2016, 12) already exists. * Some of the implementation details of the patch here: The rules about which columns to show or whether to show the DETAIL at all are similar to those in BuildIndexValueDescription(): - if user has SELECT privilege on the whole table, simply go ahead - if user doesn't have SELECT privilege on the table, check that they can see all the columns in the key (no point in showing partial key); however abort on finding an expression for which we don't try finding out privilege situation of whatever columns may be in the expression Thanks, Amit [0] https://www.postgresql.org/message-id/CANP8%2BjJBpWocfKrbJcaf3iBt9E3U%3DWPE_NC8YE6rye%2BYJ1sYnQ%40mail.gmail.com [1] https://www.postgresql.org/message-id/2f8df068-9a49-d74a-30af-7cd17bdee181%40lab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
On Tue, Feb 21, 2017 at 7:28 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Simon pointed out in a nearby thread [0] that the detail part of > partition-not-found error should show just the partition keys. I posted a > patch on that thread [1], but to avoid confusion being caused by multitude > of patches over there I'm re-posting it here. Thanks. GetPartitionFailureData seems like a strange name for a datatype, particularly the "Get" part. How about PartitionRoutingFailureInfo? Or just two out parameters. Spelling: BuildSlotPartitinKeyDescription (in comment). ExecBuildSlotPartitionKeyDescription could have a comment saying that it's LIKE BuildIndexValueDescription() instead of copy-and-pasting the comments. And maybe BuildIndexValueDescription() could also get a comment saying that if we change anything there, we should check whether ExecBuildSlotPartitionKeyDescription() needs a similar change. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Thanks for the review. On 2017/02/27 2:39, Robert Haas wrote: > On Tue, Feb 21, 2017 at 7:28 AM, Amit Langote > <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> Simon pointed out in a nearby thread [0] that the detail part of >> partition-not-found error should show just the partition keys. I posted a >> patch on that thread [1], but to avoid confusion being caused by multitude >> of patches over there I'm re-posting it here. > > Thanks. GetPartitionFailureData seems like a strange name for a > datatype, particularly the "Get" part. How about > PartitionRoutingFailureInfo? Or just two out parameters. Went with two out parameters instead of a new struct. > Spelling: BuildSlotPartitinKeyDescription (in comment). Fixed. > ExecBuildSlotPartitionKeyDescription could have a comment saying that > it's LIKE BuildIndexValueDescription() instead of copy-and-pasting the > comments. And maybe BuildIndexValueDescription() could also get a > comment saying that if we change anything there, we should check > whether ExecBuildSlotPartitionKeyDescription() needs a similar change. OK, I modified the comments. Although, I kept comments that are a bit different. Updated patch is attached. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
On Mon, Feb 27, 2017 at 9:54 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > Updated patch is attached. Committed with one grammatical change to the comments. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2017/03/03 12:43, Robert Haas wrote: > On Mon, Feb 27, 2017 at 9:54 AM, Amit Langote > <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> Updated patch is attached. > > Committed with one grammatical change to the comments. Thanks. I've marked this as fixed on the open item wiki page. Thanks, Amit