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

Amit,

On 3/11/20 6:29 AM, Amit Kapila wrote:
> On Tue, Mar 3, 2020 at 10:05 AM Chris Bandy <bandy.chris@gmail.com> wrote:
>>
>> On 3/1/20 10:09 PM, Amit Langote wrote:
>>> Hi Chris,
>>>
>>> On Mon, Mar 2, 2020 at 8:51 AM Chris Bandy <bandy.chris@gmail.com> wrote:
>>>> On 3/1/20 5:14 AM, Amit Kapila wrote:
>>>>> On Sun, Mar 1, 2020 at 10:10 AM Amit Langote <amitlangote09@gmail.com> wrote:
>>>>>>
>>>>>> There are couple more instances in src/backend/command/tablecmds.c
>>>>>> where partition constraint is checked:
>>>>>>
>>>>>> Maybe, better fix these too for completeness.
>>>>>
>>>>> Another thing we might need to see is which of these can be
>>>>> back-patched.  We should also try to write the tests for cases we are
>>>>> changing even if we don't want to commit those.
>>>>
>>>> I don't have any opinion on back-patching. Existing tests pass. I wasn't
>>>> able to find another test that checks the constraint field of errors.
>>>> There's a little bit in the tests for psql, but that is about the the
>>>> \errverbose functionality rather than specific errors and their fields.
>>>
>>> Actually, it's not a bad idea to use \errverbose to test this.
>>>
>>
>> I've added a second patch with tests that cover three of the five errors
>> touched by the first patch. Rather than \errverbose, I simply \set
>> VERBOSITY verbose. I could not find a way to exclude the location field
>> from the output, so those lines will be likely be out of date soon--if
>> not already.
>>
>> I couldn't find a way to exercise the errors in tablecmds.c. Does anyone
>> know how to instigate a table rewrite that would violate partition
>> constraints? I tried:
>>
> 
> When I tried to apply your patch on HEAD with patch -p1 <
> <path_to_patch>, I am getting below errors
> 
> (Stripping trailing CRs from patch; use --binary to disable.)
> can't find file to patch at input line 17
> Perhaps you used the wrong -p or --strip option?
> The text leading up to this was:
> ..
> 
> I have tried with git am as well, but it failed.  I am not sure what
> is the reason.  Can you please once check at your end?

Yes, sorry. This set (and v3 and v4) should work with -p0. Any following
patches from me will use the normal -p1.

> Also, see, if
> it applies till 9.5 as I think we should backpatch this.
> 
> IIUC, this patch is mainly to get the table name, schema name in case
> of the error paths, so that your application can handle errors in case
> partition constraint violation, right?

Yes, that is correct. Which also means it doesn't apply to 9.5 (no
partitions!) Later in this thread I created a test that covers all
integrity violation errors.[1] *That* can be backpatched, if you'd like.

For an approach limited to partitions only, I recommend looking at v4
rather than v2 or v3.[2]

[1]: https://postgresql.org/message-id/0731def8-978e-0285-04ee-582762729b38%40gmail.com
[2]: https://postgresql.org/message-id/7985cf2f-5082-22d9-1bb4-6b280150eeae%40gmail.com

Thanks,
Chris