From: | Chris Bandy <bandy(dot)chris(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Amit Langote <amitlangote09(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Add schema and table names to partition error |
Date: | 2020-03-04 05:18:51 |
Message-ID: | b129e6a8-ccc5-c8eb-01e0-0847ef0390d0@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 3/3/20 10:08 AM, Alvaro Herrera wrote:
>> +\set VERBOSITY verbose
>> +-- no partitions
>> +CREATE TABLE pterr1 (x int, y int, PRIMARY KEY (x, y)) PARTITION BY RANGE (y);
>> +INSERT INTO pterr1 VALUES (10, 10);
>> +ERROR: 23514: no partition of relation "pterr1" found for row
>> +DETAIL: Partition key of the failing row contains (y) = (10).
>> +SCHEMA NAME: public
>> +TABLE NAME: pterr1
>> +LOCATION: ExecFindPartition, execPartition.c:349
>
> This won't work well, because people would be forced to update the .out
> file whenever the execPartition.c file changed to add or remove lines
> before the one with the error call.
I agree. I expected that and should have made it more clear that I
didn't intend for those tests to be committed. Others in the thread
suggested I include some form of test, even if it didn't live past
review. That being said...
> Maybe if you want to verify the
> schema/table names, use a plpgsql function to extract them, using
> GET STACKED DIAGNOSTICS TABLE_NAME = ...
> in an exception block?
This is a great idea and the result looks much cleaner than I expected.
I have no reservations about committing the attached tests.
> I'm not sure that this *needs* to be tested, though. Don't we already
> verify that errtable() works, elsewhere?
I looked for tests that might target errtable() or errtableconstraint()
but found none. Perhaps someone who knows the tests better could answer
this question.
> I don't suppose you mean to
> test that every single ereport() call that includes errtable() contains
> a TABLE NAME item.
Correct. I intend only to test the few calls I'm touching in this
thread. It might be worthwhile for someone to perform a more thorough
review of existing errors, however. The documentation seems to say that
every error in SQLSTATE class 23 has one of these fields filled[1]. The
errors in these patches are in that class but lacked any fields.
[1] https://www.postgresql.org/docs/current/errcodes-appendix.html
Thanks,
Chris
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Add-schema-and-table-names-to-partition-errors.patch | text/x-patch | 3.1 KB |
v3-0002-Tests-for-partition-error-fields.patch | text/x-patch | 5.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2020-03-04 05:19:54 | Re: logical replication empty transactions |
Previous Message | Michael Paquier | 2020-03-04 05:15:10 | Re: reindex concurrently and two toast indexes |