On Thu, Sep 14, 2017 at 12:59 AM, Amit Langote
wrote:
> Since Jeevan Ladhe mentioned this patch [1] earlier this week, sending the
> rebased patches here for consideration. Actually there are only 2 patches
> now, because 0002 above is rendered unnecessary by ecfe59e50fb [2].
Committed 0001 and
On 2017/08/07 11:05, Amit Langote wrote:
> By the way, bulk of 0004 is refactoring which it seems is what Jeevan's
> default partition patch set also includes as one of the patches [1]. It
> got a decent amount review from Ashutosh. I broke it down into a separate
> patch, so that the patch to ad
On 2017/08/05 11:05, Robert Haas wrote:
> On Thu, Aug 3, 2017 at 8:45 PM, Amit Langote
> wrote:
>>> 0003 needs a rebase.
>>
>> Rebased patch attached.
>
> Committed.
Thank you.
> I think 0004 is a new feature, so I'm leaving that for v11.
Sure.
By the way, bulk of 0004 is refactoring which it
On Thu, Aug 3, 2017 at 8:45 PM, Amit Langote
wrote:
>> 0003 needs a rebase.
>
> Rebased patch attached.
Committed. I think 0004 is a new feature, so I'm leaving that for v11.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers
On 2017/08/04 3:29, Robert Haas wrote:
> On Thu, Aug 3, 2017 at 1:04 AM, Amit Langote
> wrote:
>> Alright, attached updated 0001 does that.
>
> Committed 0001 and 0002.
Thanks.
> 0003 needs a rebase.
Rebased patch attached.
Thanks,
Amit
From f069845c027acc36aab4790d6d6afbf50bba803e Mon Sep 17
On Thu, Aug 3, 2017 at 1:04 AM, Amit Langote
wrote:
> Alright, attached updated 0001 does that.
Committed 0001 and 0002. 0003 needs a rebase.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@post
On 2017/08/02 20:35, Robert Haas wrote:
> On Tue, Aug 1, 2017 at 9:44 PM, Amit Langote
> wrote:
>> I too dislike the shape of attachRel. How about we rename attachRel to
>> attachrel? So, attachrel_children, attachrel_constr, etc. It's still
>> long though... :)
>
> OK, I can live with that, I
On Tue, Aug 1, 2017 at 9:44 PM, Amit Langote
wrote:
> I too dislike the shape of attachRel. How about we rename attachRel to
> attachrel? So, attachrel_children, attachrel_constr, etc. It's still
> long though... :)
OK, I can live with that, I guess.
--
Robert Haas
EnterpriseDB: http://www.e
On 2017/08/02 10:27, Robert Haas wrote:
> On Tue, Aug 1, 2017 at 9:23 PM, Amit Langote
> wrote:
>> Since ATExecAttachPartition() deals with the possibility that the table
>> being attached itself might be partitioned, someone reading the code might
>> find it helpful to get some clue about whose p
On Tue, Aug 1, 2017 at 9:23 PM, Amit Langote
wrote:
> Since ATExecAttachPartition() deals with the possibility that the table
> being attached itself might be partitioned, someone reading the code might
> find it helpful to get some clue about whose partitions/children a
> particular piece of code
Thanks for reviewing.
On 2017/08/02 2:54, Robert Haas wrote:
> On Mon, Jul 31, 2017 at 11:10 PM, Amit Langote
> wrote:
>> OK, these cosmetic changes are now in attached patch 0001.
>
> Regarding 0001:
>
> -List *childrels;
> +List *attachRel_children;
>
> I sorta don't see
On Mon, Jul 31, 2017 at 11:10 PM, Amit Langote
wrote:
> OK, these cosmetic changes are now in attached patch 0001.
Regarding 0001:
-List *childrels;
+List *attachRel_children;
I sorta don't see why this is necessary, or better.
/* It's safe to skip the validation scan
Thanks for taking a look at this.
On 2017/08/01 6:26, Robert Haas wrote:
> On Wed, Jul 26, 2017 at 9:50 PM, Amit Langote
> wrote:
>> At least patch 0001 does address a bug. Not sure if we can say that 0002
>> addresses a bug; it implements a feature that might be a
>> nice-to-have-in-PG-10.
>
>
On Wed, Jul 26, 2017 at 9:50 PM, Amit Langote
wrote:
> At least patch 0001 does address a bug. Not sure if we can say that 0002
> addresses a bug; it implements a feature that might be a
> nice-to-have-in-PG-10.
I think 0001 is actually several fixes that should be separated:
- Cosmetic fixes,
Thanks for looking at this again.
On 2017/07/26 23:31, Ashutosh Bapat wrote:
> On Wed, Jul 12, 2017 at 7:17 AM, Amit Langote
> wrote:
>>
>> Thanks for the review. Patch updated taking care of the comments.
>
> The patches still apply and compile. make check runs well. I do not
> have any furthe
On Wed, Jul 12, 2017 at 7:17 AM, Amit Langote
wrote:
>
> Thanks for the review. Patch updated taking care of the comments.
The patches still apply and compile. make check runs well. I do not
have any further review comments. Given that they address a bug,
should we consider those for v10?
--
B
On 2017/07/11 19:49, Ashutosh Bapat wrote:
> On Tue, Jul 4, 2017 at 9:51 AM, Amit Langote
> wrote:
>
>>
>> Attached updated patches.
>
> There's an extra "we" in
> +* Note that attachRel's OID is in this list. If it's partitioned, we
> +* we don't need to schedule it to be scann
On Tue, Jul 4, 2017 at 9:51 AM, Amit Langote
wrote:
>
> Attached updated patches.
There's an extra "we" in
+* Note that attachRel's OID is in this list. If it's partitioned, we
+* we don't need to schedule it to be scanned (would be a noop anyway
And some portions of the commen
Thanks for the review.
On 2017/07/03 20:13, Ashutosh Bapat wrote:
> Thanks for working on the previous comments. The code really looks good now.
> On Fri, Jun 23, 2017 at 2:29 PM, Amit Langote
> wrote:
>>
>>> Don't we need an exclusive lock to
>>> make sure that the constraints are not changed wh
Thanks for working on the previous comments. The code really looks good now.
On Fri, Jun 23, 2017 at 2:29 PM, Amit Langote
wrote:
>
>> Don't we need an exclusive lock to
>> make sure that the constraints are not changed while we are validating those?
>
> If I understand your question correctly, y
Thanks for the review again.
On 2017/06/22 19:55, Ashutosh Bapat wrote:
> On Thu, Jun 15, 2017 at 4:06 PM, Amit Langote
> wrote:
>>
>> Anyway, I tried to implement the refactoring in patch 0002, which is not
>> all of the patch 0001 that Jeevan posted. Please take a look. I wondered
>> if we sh
On Thu, Jun 15, 2017 at 4:06 PM, Amit Langote
wrote:
>
> Anyway, I tried to implement the refactoring in patch 0002, which is not
> all of the patch 0001 that Jeevan posted. Please take a look. I wondered
> if we should emit a NOTICE when an individual leaf partition validation
> can be skipped?
On 2017/06/15 18:05, Ashutosh Bapat wrote:
> On Thu, Jun 15, 2017 at 2:30 PM, Amit Langote
> wrote:
>> On 2017/06/15 17:53, Ashutosh Bapat wrote:
>>> On Thu, Jun 15, 2017 at 2:12 PM, Amit Langote wrote:
> Both of the above comments are not related to the bug that is being
> fixed, but
>>>
On Thu, Jun 15, 2017 at 2:30 PM, Amit Langote
wrote:
> On 2017/06/15 17:53, Ashutosh Bapat wrote:
>> On Thu, Jun 15, 2017 at 2:12 PM, Amit Langote wrote:
Both of the above comments are not related to the bug that is being fixed,
but
they apply to the same code where the bug exists.
On 2017/06/15 17:53, Ashutosh Bapat wrote:
> On Thu, Jun 15, 2017 at 2:12 PM, Amit Langote wrote:
>>> Both of the above comments are not related to the bug that is being fixed,
>>> but
>>> they apply to the same code where the bug exists. So instead of fixing it
>>> twice, may be we should expand
On Thu, Jun 15, 2017 at 2:12 PM, Amit Langote
wrote:
> Thanks for the review.
>
> On 2017/06/15 16:08, Ashutosh Bapat wrote:
>> On Thu, Jun 15, 2017 at 10:46 AM, Amit Langote wrote:
>>> If we end up having to perform the validation scan and the table being
>>> attached is a partitioned table, we w
Thanks for the review.
On 2017/06/15 16:08, Ashutosh Bapat wrote:
> On Thu, Jun 15, 2017 at 10:46 AM, Amit Langote wrote:
>> If we end up having to perform the validation scan and the table being
>> attached is a partitioned table, we will scan its leaf partitions. Each
>> of those leaf partition
On Thu, Jun 15, 2017 at 10:46 AM, Amit Langote
wrote:
> Thanks for taking a look.
>
> On 2017/06/14 20:06, Ashutosh Bapat wrote:
>> On Wed, Jun 14, 2017 at 9:20 AM, Amit Langote
>> wrote:
>>>
>>> By the way, I mentioned an existing problem in one of the earlier emails
>>> on this thread about dif
Thanks for taking a look.
On 2017/06/14 20:06, Ashutosh Bapat wrote:
> On Wed, Jun 14, 2017 at 9:20 AM, Amit Langote
> wrote:
>>
>> By the way, I mentioned an existing problem in one of the earlier emails
>> on this thread about differing attribute numbers in the table being
>> attached causing p
On Wed, Jun 14, 2017 at 6:15 AM, Ashutosh Bapat
wrote:
> PFA patch set addressing comments by Tom and Amit.
LGTM. Committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To ma
On Wed, Jun 14, 2017 at 9:20 AM, Amit Langote
wrote:
>
> By the way, I mentioned an existing problem in one of the earlier emails
> on this thread about differing attribute numbers in the table being
> attached causing predicate_implied_by() to give up due to structural
> inequality of Vars. To c
PFA patch set addressing comments by Tom and Amit.
0001 is same as Robert's patch.
On Wed, Jun 14, 2017 at 7:20 AM, Robert Haas wrote:
> On Tue, Jun 13, 2017 at 5:28 PM, Tom Lane wrote:
>> Robert Haas writes:
>>> OK, I think I see the problem here. predicate_implied_by() and
>>> predicate_ref
On 2017/06/14 5:36, Robert Haas wrote:
> On Tue, Jun 13, 2017 at 10:24 AM, Robert Haas wrote:
>> I think that's going to come as an unpleasant surprise to more than
>> one user. I'm not sure exactly how we need to restructure things here
>> so that this works properly, and maybe modifying
>> pred
On 2017/06/13 23:24, Robert Haas wrote:
> On Mon, Jun 12, 2017 at 4:09 AM, Amit Langote
> wrote:
>> On 2017/06/09 20:49, Ashutosh Bapat wrote:
>>> May be we should pass a flag to predicate_implied_by() to handle NULL
>>> behaviour for CHECK constraints. Partitioning has shown that it needs
>>> to
On Tue, Jun 13, 2017 at 5:28 PM, Tom Lane wrote:
> Robert Haas writes:
>> OK, I think I see the problem here. predicate_implied_by() and
>> predicate_refuted_by() differ in what they assume about the predicate
>> evaluating to NULL, but both of them assume that if the clause
>> evaluates to NULL
Robert Haas writes:
> OK, I think I see the problem here. predicate_implied_by() and
> predicate_refuted_by() differ in what they assume about the predicate
> evaluating to NULL, but both of them assume that if the clause
> evaluates to NULL, that's equivalent to false. So there's actually no
>
On Tue, Jun 13, 2017 at 10:24 AM, Robert Haas wrote:
> I think that's going to come as an unpleasant surprise to more than
> one user. I'm not sure exactly how we need to restructure things here
> so that this works properly, and maybe modifying
> predicate_implied_by() isn't the right thing at a
On Mon, Jun 12, 2017 at 4:09 AM, Amit Langote
wrote:
> On 2017/06/09 20:49, Ashutosh Bapat wrote:
>> May be we should pass a flag to predicate_implied_by() to handle NULL
>> behaviour for CHECK constraints. Partitioning has shown that it needs
>> to use predicate_implied_by() for comparing constra
On 2017/06/09 20:49, Ashutosh Bapat wrote:
> May be we should pass a flag to predicate_implied_by() to handle NULL
> behaviour for CHECK constraints. Partitioning has shown that it needs
> to use predicate_implied_by() for comparing constraints and there may
> be other cases that can come up in fut
On 2017/06/09 20:47, Ashutosh Bapat wrote:
> Thanks for the long explanation. I guess, this should be written in
> comments somewhere in the code there. I see following comment in
> ATExecAttachPartition()
> --
> *
> * There is a case in which we cannot rely on just the result of the
>
May be we should pass a flag to predicate_implied_by() to handle NULL
behaviour for CHECK constraints. Partitioning has shown that it needs
to use predicate_implied_by() for comparing constraints and there may
be other cases that can come up in future. Instead of handling it
outside predicate_impli
On Fri, Jun 9, 2017 at 10:31 AM, Amit Langote
wrote:
> On 2017/06/08 19:25, Ashutosh Bapat wrote:
>> On Thu, Jun 8, 2017 at 3:13 PM, Amit Langote
I think this code could be removed entirely in light of commit
3ec76ff1f2cf52e9b900349957b42d28128b7bc7.
>>>
>>> I am assuming you think that
On 2017/06/08 18:43, Amit Langote wrote:
> On 2017/06/08 1:44, Robert Haas wrote:
>> On Wed, Jun 7, 2017 at 7:47 AM, Ashutosh Bapat
>> wrote:
>>> In ATExecAttachPartition() there's following code
>>>
>>> 13715 partnatts = get_partition_natts(key);
>>> 13716 for (i = 0; i < partnatt
On 2017/06/08 19:25, Ashutosh Bapat wrote:
> On Thu, Jun 8, 2017 at 3:13 PM, Amit Langote
>>> I think this code could be removed entirely in light of commit
>>> 3ec76ff1f2cf52e9b900349957b42d28128b7bc7.
>>
>> I am assuming you think that because now we emit IS NOT NULL constraint
>> internally for
On Thu, Jun 8, 2017 at 3:13 PM, Amit Langote
wrote:
> On 2017/06/08 1:44, Robert Haas wrote:
>> On Wed, Jun 7, 2017 at 7:47 AM, Ashutosh Bapat
>> wrote:
>>> In ATExecAttachPartition() there's following code
>>>
>>> 13715 partnatts = get_partition_natts(key);
>>> 13716 for (i = 0;
On 2017/06/08 1:44, Robert Haas wrote:
> On Wed, Jun 7, 2017 at 7:47 AM, Ashutosh Bapat
> wrote:
>> In ATExecAttachPartition() there's following code
>>
>> 13715 partnatts = get_partition_natts(key);
>> 13716 for (i = 0; i < partnatts; i++)
>> 13717 {
>> 13718 A
On Wed, Jun 7, 2017 at 7:47 AM, Ashutosh Bapat
wrote:
> In ATExecAttachPartition() there's following code
>
> 13715 partnatts = get_partition_natts(key);
> 13716 for (i = 0; i < partnatts; i++)
> 13717 {
> 13718 AttrNumber partattno;
> 13719
> 13720
In ATExecAttachPartition() there's following code
13715 partnatts = get_partition_natts(key);
13716 for (i = 0; i < partnatts; i++)
13717 {
13718 AttrNumber partattno;
13719
13720 partattno = get_partition_col_attnum(key, i);
13721
13722
48 matches
Mail list logo
