Re: [HACKERS] fix possible optimizations in ATExecAttachPartition()

[Jeevan Ladhe]

On Wed, Jun 14, 2017 at 2:12 AM, Robert Haas  wrote:

> On Tue, Jun 13, 2017 at 5:22 AM, Amit Langote
>  wrote:
> > Yeah, I was thinking the same while writing the patch posted on the
> thread
> > "A bug in mapping attributes in ATExecAttachPartition()" [1].  That patch
> > adds the break you mention in 2, but didn't do anything about point 1.
> >
> > In any case, +1 to your patch.  I'll rebase if someone decides to commit
> > it first.
>
> If the patch I posted in
> https://www.postgresql.org/message-id/CA%2BTgmoYmW9VwCWDpe7eXUxeKmAKOxm
> g8itgFkB5UTQTq4SnTjQ%40mail.gmail.com
> gets committed, all of this code will be gone entirely, so this will
> be moot.  If we decide to repair the existing broken logic rather than
> ripping it out entirely then this is probably a good idea, but I hope
> that's not what happens.
>

That seems a better option to me too.
+1

Regards,
Jeevan Ladhe

Re: [HACKERS] fix possible optimizations in ATExecAttachPartition()

[Robert Haas]

On Tue, Jun 13, 2017 at 5:22 AM, Amit Langote
 wrote:
> Yeah, I was thinking the same while writing the patch posted on the thread
> "A bug in mapping attributes in ATExecAttachPartition()" [1].  That patch
> adds the break you mention in 2, but didn't do anything about point 1.
>
> In any case, +1 to your patch.  I'll rebase if someone decides to commit
> it first.

If the patch I posted in
https://www.postgresql.org/message-id/CA%2BTgmoYmW9VwCWDpe7eXUxeKmAKOxmg8itgFkB5UTQTq4SnTjQ%40mail.gmail.com
gets committed, all of this code will be gone entirely, so this will
be moot.  If we decide to repair the existing broken logic rather than
ripping it out entirely then this is probably a good idea, but I hope
that's not what happens.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fix possible optimizations in ATExecAttachPartition()

[Amit Langote]

On 2017/06/13 18:08, Jeevan Ladhe wrote:
> Hi,
> 
> I was doing some testing for my default partitioning work, and I realized
> that
> there seem to be a some optimization possible (and I think we should really
> have
> this optimization) for logic around handling the "key IS NOT NULL"
> constraints
> followed by predicate_implied_by() call.
> 
> 1. Consider, if predicate_implied_by() returns false, in that case
> skip_validate
> is left set to false, and we really do not want to do lots of stuff followed
> this decision just to prove that the scan cannot be avoided(which is already
> decided in this case) if there is no NOT NULL constraint on the partition
> key
> column.
> 
> 2. Further, if we have already decided not to skip the scan, we should
> really have
> a break in following if block:
> 
> if (!partition_accepts_null &&
> (partattno == 0 ||
> !bms_is_member(partattno, not_null_attrs)))
> skip_validate = false;
> 
> 
> I have made above changes in the attached patch.
> PFA, and let me know if I am missing something here.

Yeah, I was thinking the same while writing the patch posted on the thread
"A bug in mapping attributes in ATExecAttachPartition()" [1].  That patch
adds the break you mention in 2, but didn't do anything about point 1.

In any case, +1 to your patch.  I'll rebase if someone decides to commit
it first.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/a5df1cf6-dc51-1720-2abe-7957502b2cf9%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

[HACKERS] fix possible optimizations in ATExecAttachPartition()

[Jeevan Ladhe]

Hi,

I was doing some testing for my default partitioning work, and I realized
that
there seem to be a some optimization possible (and I think we should really
have
this optimization) for logic around handling the "key IS NOT NULL"
constraints
followed by predicate_implied_by() call.

1. Consider, if predicate_implied_by() returns false, in that case
skip_validate
is left set to false, and we really do not want to do lots of stuff followed
this decision just to prove that the scan cannot be avoided(which is already
decided in this case) if there is no NOT NULL constraint on the partition
key
column.

2. Further, if we have already decided not to skip the scan, we should
really have
a break in following if block:

if (!partition_accepts_null &&
(partattno == 0 ||
!bms_is_member(partattno, not_null_attrs)))
skip_validate = false;


I have made above changes in the attached patch.
PFA, and let me know if I am missing something here.

Regards,
Jeevan Ladhe


fix_atexecattahpartition.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers