Re: [PATCH] Add schema and table names to partition error

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