From: Chris Bandy <bandy.chris@gmail.com>
Subject: Re: [PATCH] Add schema and table names to partition error
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
References: <0aa113a3-3c7f-db48-bcd8-f9290b2269ae@gmail.com>
 <CA+HiwqGRCm0HsXk_HOyWiPYbmy74QOdxQoHHohKUW1dBiFTtuw@mail.gmail.com>
 <CAA4eK1JZUxu9x0eQX7BCBGZ+9CvLr91V9OphHsSkE09jc2DQ8Q@mail.gmail.com>
 <bdeb5e60-6a86-6eeb-d073-f489bf3e8f55@gmail.com>
 <CA+HiwqE_1aDezgt4pBcUSU0UeUkowUUw-rE7faBkS07A-QtWuA@mail.gmail.com>
 <0acc36c9-1aef-1057-0c29-de473c850605@gmail.com>
 <CAA4eK1JVBuudeCY-+n6u+chYY=cyAgj1kf21eNiTG3KzjkDApA@mail.gmail.com>
Message-ID: <4037ab52-27e4-bbfb-d618-adbab81125e2@gmail.com>
Date: Wed, 11 Mar 2020 10:21:45 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: 
 <CAA4eK1JVBuudeCY-+n6u+chYY=cyAgj1kf21eNiTG3KzjkDApA@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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