Re: [HACKERS] UPDATE of partition key

2017-11-13 Thread David Rowley
On 10 November 2017 at 16:42, Amit Khandekar wrote: [ update-partition-key_v23.patch ] Hi Amit, Thanks for working on this. I'm looking forward to seeing this go in. So... I've signed myself up to review the patch, and I've just had a look at it, (after first reading

Re: [HACKERS] UPDATE of partition key

2017-11-09 Thread Amit Khandekar
On 9 November 2017 at 09:27, Thomas Munro wrote: > On Wed, Nov 8, 2017 at 5:57 PM, Amit Khandekar wrote: >> On 8 November 2017 at 07:55, Thomas Munro >> wrote: >>> On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas

Re: [HACKERS] UPDATE of partition key

2017-11-08 Thread Thomas Munro
On Wed, Nov 8, 2017 at 5:57 PM, Amit Khandekar wrote: > On 8 November 2017 at 07:55, Thomas Munro > wrote: >> On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas wrote: >>> The changes to trigger.c still make me

Re: [HACKERS] UPDATE of partition key

2017-11-07 Thread Thomas Munro
On Wed, Nov 8, 2017 at 5:57 PM, Amit Khandekar wrote: > Thomas, can you please try the attached incremental patch > regress_locale_changes.patch and check if the test passes ? The patch > is to be applied on the main v22 patch. If the test passes, I will > include these

Re: [HACKERS] UPDATE of partition key

2017-11-07 Thread Amit Khandekar
On 8 November 2017 at 07:55, Thomas Munro wrote: > On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas wrote: >> The changes to trigger.c still make me super-nervous. Hey THOMAS >> MUNRO, any chance you could review that part? > > Looking, but

Re: [HACKERS] UPDATE of partition key

2017-11-07 Thread Thomas Munro
On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas wrote: > The changes to trigger.c still make me super-nervous. Hey THOMAS > MUNRO, any chance you could review that part? Looking, but here's one silly thing that jumped out at me while getting started with this patch. I cannot

Re: [HACKERS] UPDATE of partition key

2017-11-06 Thread Amit Langote
On 2017/11/07 14:40, Amit Khandekar wrote: > On 7 November 2017 at 00:33, Robert Haas wrote: > >> Also, +1 for Amit Langote's idea of trying to merge >> mt_perleaf_childparent_maps with mt_persubplan_childparent_maps. > > Currently I am trying to see if it simplifies

Re: [HACKERS] UPDATE of partition key

2017-11-06 Thread Amit Khandekar
On 7 November 2017 at 00:33, Robert Haas wrote: > Also, +1 for Amit Langote's idea of trying to merge > mt_perleaf_childparent_maps with mt_persubplan_childparent_maps. Currently I am trying to see if it simplifies things if we do that. We will be merging these arrays

Re: [HACKERS] UPDATE of partition key

2017-11-06 Thread Robert Haas
On Wed, Oct 25, 2017 at 11:40 AM, Amit Khandekar wrote: > Below I have addressed the remaining review comments : The changes to trigger.c still make me super-nervous. Hey THOMAS MUNRO, any chance you could review that part? + /* The caller must have already locked

Re: [HACKERS] UPDATE of partition key

2017-11-02 Thread Amit Langote
Hi Amit. Thanks a lot for updated patches and sorry that I couldn't get to looking at your emails sooner. Note that I'm replying here to both of your emails, but looking at only the latest v22 patch. On 2017/10/24 0:15, Amit Khandekar wrote: > On 16 October 2017 at 08:28, Amit Langote

Re: [HACKERS] UPDATE of partition key

2017-10-15 Thread Amit Langote
Hi Amit. On 2017/10/04 22:51, Amit Khandekar wrote: > Main patch : > update-partition-key_v20.patch Guess you're already working on it but the patch needs a rebase. A couple of hunks in the patch to execMain.c and nodeModifyTable.c fail. Meanwhile a few comments: +void

Re: [HACKERS] UPDATE of partition key

2017-10-12 Thread Amit Langote
On 2017/10/13 6:18, Robert Haas wrote: > Is anybody still reviewing the main patch here? (It would be good if > the answer is "yes".) I am going to try to look at the latest version over the weekend and early next week. Thanks, Amit -- Sent via pgsql-hackers mailing list

Re: [HACKERS] UPDATE of partition key

2017-10-12 Thread Robert Haas
On Wed, Oct 4, 2017 at 9:51 AM, Amit Khandekar wrote: > Preparatory patches : > 0001-Prepare-for-re-using-UPDATE-result-rels-during-tuple.patch > 0002-Prevent-a-redundant-ConvertRowtypeExpr-node.patch > Main patch : > update-partition-key_v20.patch Committed 0001 with a

Re: [HACKERS] UPDATE of partition key

2017-10-03 Thread Robert Haas
On Tue, Oct 3, 2017 at 8:16 AM, Amit Khandekar wrote: > While writing this down, I observed that after multi-level partition > tree expansion was introduced, the child table expressions are not > converted directly from the root. Instead, they are converted from > their

Re: [HACKERS] UPDATE of partition key

2017-10-03 Thread Amit Khandekar
On 30 September 2017 at 01:26, Robert Haas wrote: > On Fri, Sep 29, 2017 at 3:53 PM, Robert Haas wrote: >> On Fri, Sep 22, 2017 at 1:57 AM, Amit Khandekar >> wrote: >>> The patch for the above change is : >>>

Re: [HACKERS] UPDATE of partition key

2017-09-29 Thread Robert Haas
On Fri, Sep 22, 2017 at 1:57 AM, Amit Khandekar wrote: > The patch for the above change is : > 0002-Prevent-a-redundant-ConvertRowtypeExpr-node.patch Thinking about this a little more, I'm wondering about how this case arises. I think that for this patch to avoid

Re: [HACKERS] UPDATE of partition key

2017-09-29 Thread amul sul
On Wed, Sep 13, 2017 at 4:24 PM, amul sul wrote: > > > On Sun, Sep 10, 2017 at 8:47 AM, Amit Kapila > wrote: >> >> On Fri, Sep 8, 2017 at 4:51 PM, amul sul wrote: >> > On Thu, May 18, 2017 at 9:13 AM, Amit Kapila

Re: [HACKERS] UPDATE of partition key

2017-09-28 Thread Amit Khandekar
Below are some performance figures. Overall, there does not appear to be a noticeable difference in the figures in partition key updates with and without row movement (which is surprising), and non-partition-key updates with and without the patch. All the values are in milliseconds.

Re: [HACKERS] UPDATE of partition key

2017-09-22 Thread Amit Khandekar
On 21 September 2017 at 19:52, amul sul wrote: > On Wed, Sep 20, 2017 at 9:27 PM, Amit Khandekar > wrote: >> >> On 20 September 2017 at 00:06, Robert Haas wrote: >> > On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar

Re: [HACKERS] UPDATE of partition key

2017-09-21 Thread amul sul
On Wed, Sep 20, 2017 at 9:27 PM, Amit Khandekar wrote: > On 20 September 2017 at 00:06, Robert Haas wrote: > > On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar > wrote: > >> [ new patch ] > 86 - (event ==

Re: [HACKERS] UPDATE of partition key

2017-09-20 Thread Amit Khandekar
On 20 September 2017 at 00:06, Robert Haas wrote: > On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar > wrote: >> [ new patch ] > > This already fails to apply again. In general, I think it would be a > good idea to break this up into a patch series

Re: [HACKERS] UPDATE of partition key

2017-09-19 Thread Robert Haas
On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar wrote: > [ new patch ] This already fails to apply again. In general, I think it would be a good idea to break this up into a patch series rather than have it as a single patch. That would allow some bits to be applied

Re: [HACKERS] UPDATE of partition key

2017-09-19 Thread Dilip Kumar
On Tue, Sep 19, 2017 at 1:15 PM, Amit Khandekar wrote: > On 18 September 2017 at 20:45, Dilip Kumar wrote: >> Please find few more comments. >> >> + * in which they appear in the PartitionDesc. Also, extract the >> + * partition key columns of the

Re: [HACKERS] UPDATE of partition key

2017-09-19 Thread Amit Khandekar
On 18 September 2017 at 20:45, Dilip Kumar wrote: > Please find few more comments. > > + * in which they appear in the PartitionDesc. Also, extract the > + * partition key columns of the root partitioned table. Those of the > + * child partitions would be collected during

Re: [HACKERS] UPDATE of partition key

2017-09-18 Thread Dilip Kumar
On Mon, Sep 18, 2017 at 11:29 AM, Dilip Kumar wrote: > On Fri, Sep 15, 2017 at 4:55 PM, Amit Khandekar > wrote: >> On 12 September 2017 at 12:39, Amit Khandekar wrote: >>> On 12 September 2017 at 11:57, Dilip Kumar

Re: [HACKERS] UPDATE of partition key

2017-09-17 Thread Dilip Kumar
On Fri, Sep 15, 2017 at 4:55 PM, Amit Khandekar wrote: > On 12 September 2017 at 12:39, Amit Khandekar wrote: >> On 12 September 2017 at 11:57, Dilip Kumar wrote: >>> On Tue, Sep 12, 2017 at 11:15 AM, Amit Khandekar

Re: [HACKERS] UPDATE of partition key

2017-09-13 Thread amul sul
On Sun, Sep 10, 2017 at 8:47 AM, Amit Kapila wrote: > On Fri, Sep 8, 2017 at 4:51 PM, amul sul wrote: > > On Thu, May 18, 2017 at 9:13 AM, Amit Kapila > > wrote: > >> > >> On Wed, May 17, 2017 at 5:17 PM, Robert Haas

Re: [HACKERS] UPDATE of partition key

2017-09-12 Thread Amit Khandekar
On 12 September 2017 at 11:57, Dilip Kumar wrote: > On Tue, Sep 12, 2017 at 11:15 AM, Amit Khandekar > wrote: > >> But the statement level trigger function can refer to OLD TABLE and >> NEW TABLE, which will contain all the OLD rows and NEW rows >>

Re: [HACKERS] UPDATE of partition key

2017-09-12 Thread Dilip Kumar
On Tue, Sep 12, 2017 at 11:15 AM, Amit Khandekar wrote: > But the statement level trigger function can refer to OLD TABLE and > NEW TABLE, which will contain all the OLD rows and NEW rows > respectively. So the updated rows of the partitions (including the > moved ones)

Re: [HACKERS] UPDATE of partition key

2017-09-11 Thread Amit Khandekar
On 11 September 2017 at 21:12, Dilip Kumar wrote: > On Thu, Sep 7, 2017 at 11:41 AM, Amit Khandekar > wrote: >> On 6 September 2017 at 21:47, Dilip Kumar wrote: > >> Actually, since transition tables came in, the functions

Re: [HACKERS] UPDATE of partition key

2017-09-11 Thread Dilip Kumar
On Thu, Sep 7, 2017 at 11:41 AM, Amit Khandekar wrote: > On 6 September 2017 at 21:47, Dilip Kumar wrote: > Actually, since transition tables came in, the functions like > ExecARUpdateTriggers() or ExecARInsertTriggers() have this additional >

Re: [HACKERS] UPDATE of partition key

2017-09-09 Thread Amit Kapila
On Fri, Sep 8, 2017 at 4:51 PM, amul sul wrote: > On Thu, May 18, 2017 at 9:13 AM, Amit Kapila > wrote: >> >> On Wed, May 17, 2017 at 5:17 PM, Robert Haas >> wrote: >> > On Wed, May 17, 2017 at 6:29 AM, Amit Kapila

Re: [HACKERS] UPDATE of partition key

2017-09-08 Thread amul sul
On Thu, May 18, 2017 at 9:13 AM, Amit Kapila wrote: > On Wed, May 17, 2017 at 5:17 PM, Robert Haas > wrote: > > On Wed, May 17, 2017 at 6:29 AM, Amit Kapila > wrote: > >> I think we can do this even without using an

Re: [HACKERS] UPDATE of partition key

2017-09-08 Thread Amit Langote
On 2017/09/08 18:57, Robert Haas wrote: >> As mentioned by Amit Langote in the above mail thread, he is going to >> do changes for making RelationGetPartitionDispatchInfo() return the >> leaf partitions in depth-first order. Once that is done, I will then >> remove the hash table method for

Re: [HACKERS] UPDATE of partition key

2017-09-08 Thread Robert Haas
On Thu, Sep 7, 2017 at 6:17 AM, Amit Khandekar wrote: > On 3 September 2017 at 17:10, Amit Khandekar wrote: >> After recent commit 30833ba154, now the partitions are expanded in >> depth-first order. It didn't seem worthwhile rebasing my partition

Re: [HACKERS] UPDATE of partition key

2017-09-07 Thread Amit Khandekar
On 3 September 2017 at 17:10, Amit Khandekar wrote: > After recent commit 30833ba154, now the partitions are expanded in > depth-first order. It didn't seem worthwhile rebasing my partition > walker changes onto the latest code. So in the attached patch, I have > removed

Re: [HACKERS] UPDATE of partition key

2017-09-06 Thread Dilip Kumar
On Mon, Sep 4, 2017 at 10:52 AM, Amit Khandekar wrote: > On 4 September 2017 at 07:43, Amit Kapila wrote: > Oops sorry. Now attached. I have done some basic testing and initial review of the patch. I have some comments/doubts. I will continue

Re: [HACKERS] UPDATE of partition key

2017-09-03 Thread Amit Kapila
On Sun, Sep 3, 2017 at 5:10 PM, Amit Khandekar wrote: > On 31 August 2017 at 14:15, Amit Khandekar wrote: >> Thanks Dilip. I am working on rebasing the patch. Particularly, the >> partition walker in my patch depended on the fact that all the

Re: [HACKERS] UPDATE of partition key

2017-09-03 Thread Amit Khandekar
On 31 August 2017 at 14:15, Amit Khandekar wrote: > Thanks Dilip. I am working on rebasing the patch. Particularly, the > partition walker in my patch depended on the fact that all the tables > get opened (and then closed) while creating the tuple routing info. > But in

Re: [HACKERS] UPDATE of partition key

2017-08-31 Thread Amit Khandekar
Thanks Dilip. I am working on rebasing the patch. Particularly, the partition walker in my patch depended on the fact that all the tables get opened (and then closed) while creating the tuple routing info. But in HEAD, now only the partitioned tables get opened. So need some changes in my patch.

Re: [HACKERS] UPDATE of partition key

2017-08-31 Thread Dilip Kumar
On Fri, Aug 11, 2017 at 10:44 AM, Amit Khandekar wrote: > On 4 August 2017 at 22:28, Amit Khandekar wrote: >>> I am planning to review and test this patch, Seems like this patch needs to be rebased. [dilip@localhost postgresql]$ patch -p1 <

Re: [HACKERS] UPDATE of partition key

2017-08-07 Thread Rajkumar Raghuwanshi
On Fri, Aug 4, 2017 at 10:28 PM, Amit Khandekar wrote: > > > > Below are the TODOS at this point : > > > > Fix for bug reported by Rajkumar about update with join. > > I had explained the root issue of this bug here : [1] > > Attached patch includes the fix, which is

Re: [HACKERS] UPDATE of partition key

2017-08-04 Thread Amit Khandekar
> > Below are the TODOS at this point : > > Fix for bug reported by Rajkumar about update with join. I had explained the root issue of this bug here : [1] Attached patch includes the fix, which is explained below. Currently in the patch, there is a check if the tuple is concurrently deleted by

Re: [HACKERS] UPDATE of partition key

2017-08-04 Thread Amit Langote
On 2017/08/02 19:49, Amit Khandekar wrote: > On 2 August 2017 at 14:38, Amit Langote wrote: >>> One approach I had considered was to have find_inheritance_children() >>> itself lock the children in bound order, so that everyone will have >>> bound-ordered oids, but

Re: [HACKERS] UPDATE of partition key

2017-08-02 Thread Amit Khandekar
On 2 August 2017 at 14:38, Amit Langote wrote: > On 2017/07/29 2:45, Amit Khandekar wrote: >> On 28 July 2017 at 20:10, Robert Haas wrote: >>> On Wed, Jul 26, 2017 at 2:13 AM, Amit Langote wrote: I checked that we get the same result

Re: [HACKERS] UPDATE of partition key

2017-08-02 Thread Amit Langote
On 2017/07/29 2:45, Amit Khandekar wrote: > On 28 July 2017 at 20:10, Robert Haas wrote: >> On Wed, Jul 26, 2017 at 2:13 AM, Amit Langote wrote: >>> I checked that we get the same result relation order with both the >>> patches, but I would like to highlight a notable

Re: [HACKERS] UPDATE of partition key

2017-07-28 Thread Amit Khandekar
On 28 July 2017 at 20:10, Robert Haas wrote: > On Wed, Jul 26, 2017 at 2:13 AM, Amit Langote > wrote: >> Sorry to be responding this late to the Amit's make_resultrel_ordered >> patch itself, but I agree that we should teach the planner to

Re: [HACKERS] UPDATE of partition key

2017-07-28 Thread Robert Haas
On Wed, Jul 26, 2017 at 2:13 AM, Amit Langote wrote: > Sorry to be responding this late to the Amit's make_resultrel_ordered > patch itself, but I agree that we should teach the planner to *always* > expand partitioned tables in the partition bound order. Sounds

Re: [HACKERS] UPDATE of partition key

2017-07-26 Thread Amit Khandekar
On 26 July 2017 at 02:37, Robert Haas wrote: > Is there any real benefit in this "walker" interface? It looks to me > like it might be simpler to just change things around so that it > returns a list of OIDs, like find_all_inheritors, but generated > differently. Then if

Re: [HACKERS] UPDATE of partition key

2017-07-26 Thread Amit Langote
On 2017/07/25 21:55, Rajkumar Raghuwanshi wrote: > Got one more observation : update... returning is not working with whole > row reference. please take a look. > > postgres=# create table part (a int, b int) partition by range(a); > CREATE TABLE > postgres=# create table part_p1 partition of

Re: [HACKERS] UPDATE of partition key

2017-07-26 Thread Etsuro Fujita
On 2017/07/26 6:07, Robert Haas wrote: On Thu, Jul 13, 2017 at 1:09 PM, Amit Khandekar wrote: Attached is a WIP patch (make_resultrels_ordered.patch) that generates the result rels in canonical order. This patch is kept separate from the update-partition-key patch, and

Re: [HACKERS] UPDATE of partition key

2017-07-26 Thread Amit Langote
On 2017/07/26 6:07, Robert Haas wrote: > On Thu, Jul 13, 2017 at 1:09 PM, Amit Khandekar > wrote: >> Attached is a WIP patch (make_resultrels_ordered.patch) that generates >> the result rels in canonical order. This patch is kept separate from >> the update-partition-key

Re: [HACKERS] UPDATE of partition key

2017-07-25 Thread Robert Haas
On Thu, Jul 13, 2017 at 1:09 PM, Amit Khandekar wrote: > Attached is a WIP patch (make_resultrels_ordered.patch) that generates > the result rels in canonical order. This patch is kept separate from > the update-partition-key patch, and can be applied on master branch.

Re: [HACKERS] UPDATE of partition key

2017-07-25 Thread Rajkumar Raghuwanshi
On Tue, Jul 25, 2017 at 3:54 PM, Amit Khandekar wrote: > On 25 July 2017 at 15:02, Rajkumar Raghuwanshi > wrote: > > On Mon, Jul 24, 2017 at 11:23 AM, Amit Khandekar > > > wrote: > >> > >> > >> Attached

Re: [HACKERS] UPDATE of partition key

2017-07-25 Thread Amit Khandekar
On 25 July 2017 at 15:02, Rajkumar Raghuwanshi wrote: > On Mon, Jul 24, 2017 at 11:23 AM, Amit Khandekar > wrote: >> >> >> Attached update-partition-key_v13.patch now contains this >> make_resultrels_ordered.patch changes. >> > > I

Re: [HACKERS] UPDATE of partition key

2017-07-25 Thread Rajkumar Raghuwanshi
On Mon, Jul 24, 2017 at 11:23 AM, Amit Khandekar wrote: > > Attached update-partition-key_v13.patch now contains this > make_resultrels_ordered.patch changes. > > I have applied attach patch and got below observation. Observation : if join producing multiple output rows

Re: [HACKERS] UPDATE of partition key

2017-07-23 Thread Amit Khandekar
On 13 July 2017 at 22:39, Amit Khandekar wrote: > Attached is a WIP patch (make_resultrels_ordered.patch) that generates > the result rels in canonical order. This patch is kept separate from > the update-partition-key patch, and can be applied on master branch. Attached

Re: [HACKERS] UPDATE of partition key

2017-07-13 Thread Amit Khandekar
On 5 July 2017 at 15:12, Amit Khandekar wrote: > Like I mentioned upthread... in expand_inherited_rtentry(), if we > replace find_all_inheritors() with something else that returns oids in > canonical order, that will change the order in which children tables > get locked,

Re: [HACKERS] UPDATE of partition key

2017-07-05 Thread Amit Khandekar
On 4 July 2017 at 15:23, Amit Khandekar wrote: > On 4 July 2017 at 14:48, Amit Khandekar wrote: >> On 4 July 2017 at 14:38, Amit Langote wrote: >>> On 2017/07/04 17:25, Etsuro Fujita wrote: On 2017/07/03 18:54,

Re: [HACKERS] UPDATE of partition key

2017-07-04 Thread Amit Khandekar
On 4 July 2017 at 14:48, Amit Khandekar wrote: > On 4 July 2017 at 14:38, Amit Langote wrote: >> On 2017/07/04 17:25, Etsuro Fujita wrote: >>> On 2017/07/03 18:54, Amit Langote wrote: On 2017/07/02 20:10, Robert Haas wrote: > That

Re: [HACKERS] UPDATE of partition key

2017-07-04 Thread Amit Khandekar
On 4 July 2017 at 14:38, Amit Langote wrote: > On 2017/07/04 17:25, Etsuro Fujita wrote: >> On 2017/07/03 18:54, Amit Langote wrote: >>> On 2017/07/02 20:10, Robert Haas wrote: That seems pretty easy to do - just have expand_inherited_rtentry() notice

Re: [HACKERS] UPDATE of partition key

2017-07-04 Thread Amit Langote
On 2017/07/04 17:25, Etsuro Fujita wrote: > On 2017/07/03 18:54, Amit Langote wrote: >> On 2017/07/02 20:10, Robert Haas wrote: >>> That >>> seems pretty easy to do - just have expand_inherited_rtentry() notice >>> that it's got a partitioned table and call >>> RelationGetPartitionDispatchInfo()

Re: [HACKERS] UPDATE of partition key

2017-07-04 Thread Etsuro Fujita
On 2017/07/03 18:54, Amit Langote wrote: On 2017/07/02 20:10, Robert Haas wrote: But that seems like it wouldn't be too hard to fix: let's have expand_inherited_rtentry() expand the partitioned table in the same order that will be used by ExecSetupPartitionTupleRouting(). That's really what

Re: [HACKERS] UPDATE of partition key

2017-07-03 Thread Amit Langote
On 2017/07/02 20:10, Robert Haas wrote: > On Fri, Jun 30, 2017 at 4:20 PM, Robert Haas wrote: >> I don't think the approach of building a hash table to figure out >> which result rels have already been created is a good one. That too >> feels like something that the

Re: [HACKERS] UPDATE of partition key

2017-07-02 Thread Robert Haas
On Fri, Jun 30, 2017 at 4:20 PM, Robert Haas wrote: > I don't think the approach of building a hash table to figure out > which result rels have already been created is a good one. That too > feels like something that the planner should be figuring out and the > executor

Re: [HACKERS] UPDATE of partition key

2017-06-30 Thread Thomas Munro
On Fri, Jun 30, 2017 at 12:01 AM, Amit Khandekar wrote: > On 29 June 2017 at 07:42, Amit Langote wrote: >> Hi Amit, >> >> On 2017/06/28 20:43, Amit Khandekar wrote: >>> In attached patch v12 >> >> The patch no longer applies and fails to

Re: [HACKERS] UPDATE of partition key

2017-06-30 Thread Robert Haas
On Thu, Jun 29, 2017 at 3:52 PM, Amit Khandekar wrote: > So to conclude, I think, we can do this : > > Scenario 1 : > Only one partitioned table : the root; rest all are leaf partitions. > In this case, it is definitely efficient to just check the root > partition key,

Re: [HACKERS] UPDATE of partition key

2017-06-29 Thread Amit Khandekar
On 22 June 2017 at 01:41, Robert Haas wrote: >>> +for (i = 0; i < num_rels; i++) >>> +{ >>> +ResultRelInfo *resultRelInfo = _rels[i]; >>> +Relationrel = resultRelInfo->ri_RelationDesc; >>> +Bitmapset *expr_attrs = NULL; >>> + >>>

Re: [HACKERS] UPDATE of partition key

2017-06-29 Thread Amit Khandekar
On 29 June 2017 at 07:42, Amit Langote wrote: > Hi Amit, > > On 2017/06/28 20:43, Amit Khandekar wrote: >> In attached patch v12 > > The patch no longer applies and fails to compile after the following > commit was made yesterday: > > commit

Re: [HACKERS] UPDATE of partition key

2017-06-28 Thread Amit Langote
Hi Amit, On 2017/06/28 20:43, Amit Khandekar wrote: > In attached patch v12 The patch no longer applies and fails to compile after the following commit was made yesterday: commit 501ed02cf6f4f60c3357775eb07578aebc912d3a Author: Andrew Gierth Date: Wed Jun 28

Re: [HACKERS] UPDATE of partition key

2017-06-28 Thread Amit Khandekar
On 22 June 2017 at 01:57, Robert Haas wrote: > On Wed, Jun 21, 2017 at 1:38 PM, Amit Khandekar > wrote: Yep, it's more appropriate to use ModifyTableState->rootResultRelationInfo->ri_RelationDesc somehow. That is, if answer to the

Re: [HACKERS] UPDATE of partition key

2017-06-28 Thread Amit Khandekar
On 26 June 2017 at 08:37, Amit Khandekar wrote: > On 22 June 2017 at 01:41, Robert Haas wrote: Second, it will amount to a functional bug if you get a different answer than the planner did. >>> >>> Actually, the per-leaf WCOs are meant to

Re: [HACKERS] UPDATE of partition key

2017-06-25 Thread Amit Khandekar
On 22 June 2017 at 01:41, Robert Haas wrote: >>> Second, it will amount to a functional bug if you get a >>> different answer than the planner did. >> >> Actually, the per-leaf WCOs are meant to be executed on the >> destination partitions where the tuple is moved, while

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Robert Haas
On Wed, Jun 21, 2017 at 1:38 PM, Amit Khandekar wrote: >>> Yep, it's more appropriate to use >>> ModifyTableState->rootResultRelationInfo->ri_RelationDesc somehow. That >>> is, if answer to the question I raised above is positive. > > From what I had checked earlier when

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Robert Haas
On Wed, Jun 21, 2017 at 1:37 PM, Amit Khandekar wrote: >> e.g. the table is partitioned on order number, and you do UPDATE >> lineitem SET product_code = 'K372B' WHERE product_code = 'K372'. > > This query does not update order number, so here there is no >

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Amit Khandekar
On 21 June 2017 at 20:14, Robert Haas wrote: > On Wed, Jun 21, 2017 at 5:28 AM, Amit Langote > wrote:>> The comment "UPDATE/DELETE > cases are handled above" is referring to >>> the code that initializes the WCOs generated by the planner.

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Amit Khandekar
On 21 June 2017 at 00:23, Robert Haas wrote: > On Tue, Jun 20, 2017 at 2:54 AM, Amit Khandekar > wrote: >>> I guess I don't see why it should work like this. In the INSERT case, >>> we must build withCheckOption objects for each partition because

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Robert Haas
On Wed, Jun 21, 2017 at 5:28 AM, Amit Langote wrote:>> The comment "UPDATE/DELETE cases are handled above" is referring to >> the code that initializes the WCOs generated by the planner. You've >> modified the comment in your patch, but the associated code: your >>

Re: [HACKERS] UPDATE of partition key

2017-06-21 Thread Amit Langote
On 2017/06/21 3:53, Robert Haas wrote: > On Tue, Jun 20, 2017 at 2:54 AM, Amit Khandekar > wrote: >>> I guess I don't see why it should work like this. In the INSERT case, >>> we must build withCheckOption objects for each partition because those >>> partitions don't

Re: [HACKERS] UPDATE of partition key

2017-06-20 Thread Robert Haas
On Tue, Jun 20, 2017 at 2:54 AM, Amit Khandekar wrote: >> I guess I don't see why it should work like this. In the INSERT case, >> we must build withCheckOption objects for each partition because those >> partitions don't appear in the plan otherwise -- but in the UPDATE

Re: [HACKERS] UPDATE of partition key

2017-06-20 Thread Amit Khandekar
On 20 June 2017 at 03:46, Robert Haas wrote: > On Thu, Jun 15, 2017 at 1:36 PM, Amit Khandekar > wrote: >> Attached patch v10 fixes the above. In the existing code, where it >> builds WCO constraints for each leaf partition; with the patch, that >>

Re: [HACKERS] UPDATE of partition key

2017-06-19 Thread Amit Khandekar
On 20 June 2017 at 03:42, Thomas Munro wrote: > Just a thought: If I understand correctly this new array of tuple > conversion maps is the same as mtstate->mt_transition_tupconv_maps in > my patch transition-tuples-from-child-tables-v11.patch (hopefully soon > to be

Re: [HACKERS] UPDATE of partition key

2017-06-19 Thread Robert Haas
On Thu, Jun 15, 2017 at 1:36 PM, Amit Khandekar wrote: > Attached patch v10 fixes the above. In the existing code, where it > builds WCO constraints for each leaf partition; with the patch, that > code now is applicable to row-movement-updates as well. I guess I don't see

Re: [HACKERS] UPDATE of partition key

2017-06-19 Thread Thomas Munro
On Fri, Jun 16, 2017 at 5:36 AM, Amit Khandekar wrote: > There is another issue I discovered. The row-movement works fine if > the destination leaf partition has different attribute ordering than > the root : the existing insert-tuple-routing mapping handles that. But > if

Re: [HACKERS] UPDATE of partition key

2017-06-18 Thread Amit Khandekar
When I tested partition-key-update on a partitioned table having no child partitions, it crashed. This is because there is an Assert(mtstate->mt_num_partitions > 0) for creating the partition-to-root map, which fails if there are no partitions under the partitioned table. Actually we should skp

Re: [HACKERS] UPDATE of partition key

2017-06-15 Thread Amit Khandekar
On 13 June 2017 at 15:40, Amit Khandekar wrote: > While rebasing my patch for the below recent commit, I realized that a > similar issue exists for the uptate-tuple-routing patch as well : > > commit 78a030a441966d91bc7e932ef84da39c3ea7d970 > Author: Tom Lane

Re: [HACKERS] UPDATE of partition key

2017-06-13 Thread Amit Khandekar
While rebasing my patch for the below recent commit, I realized that a similar issue exists for the uptate-tuple-routing patch as well : commit 78a030a441966d91bc7e932ef84da39c3ea7d970 Author: Tom Lane Date: Mon Jun 12 23:29:44 2017 -0400 Fix confusion about number of

Re: [HACKERS] UPDATE of partition key

2017-06-09 Thread Amit Kapila
On Fri, Jun 9, 2017 at 7:48 PM, Amit Khandekar wrote: > On 9 June 2017 at 19:10, Amit Kapila wrote: >> On Thu, Jun 8, 2017 at 10:40 PM, Robert Haas wrote: >>> On Thu, Jun 8, 2017 at 7:01 AM, Amit Kapila

Re: [HACKERS] UPDATE of partition key

2017-06-09 Thread Amit Khandekar
On 9 June 2017 at 19:10, Amit Kapila wrote: > On Thu, Jun 8, 2017 at 10:40 PM, Robert Haas wrote: >> On Thu, Jun 8, 2017 at 7:01 AM, Amit Kapila wrote: >>> On Thu, Jun 8, 2017 at 1:33 AM, Robert Haas

Re: [HACKERS] UPDATE of partition key

2017-06-09 Thread Amit Kapila
On Thu, Jun 8, 2017 at 10:40 PM, Robert Haas wrote: > On Thu, Jun 8, 2017 at 7:01 AM, Amit Kapila wrote: >> On Thu, Jun 8, 2017 at 1:33 AM, Robert Haas wrote: >>> On Wed, Jun 7, 2017 at 5:46 AM, Amit Kapila

Re: [HACKERS] UPDATE of partition key

2017-06-08 Thread Amit Khandekar
On 7 June 2017 at 20:19, Amit Khandekar wrote: > On 7 June 2017 at 16:42, Amit Khandekar wrote: >> The column bitmap set returned by GetUpdatedColumns() refer to >> attribute numbers w.r.t. to the root partition. And the >> mstate->resultRelInfo[]

Re: [HACKERS] UPDATE of partition key

2017-06-08 Thread Robert Haas
On Thu, Jun 8, 2017 at 7:01 AM, Amit Kapila wrote: > On Thu, Jun 8, 2017 at 1:33 AM, Robert Haas wrote: >> On Wed, Jun 7, 2017 at 5:46 AM, Amit Kapila wrote: >>> As far as I understand, it is to ensure that for deleted

Re: [HACKERS] UPDATE of partition key

2017-06-08 Thread Amit Kapila
On Thu, Jun 8, 2017 at 1:33 AM, Robert Haas wrote: > On Wed, Jun 7, 2017 at 5:46 AM, Amit Kapila wrote: >> As far as I understand, it is to ensure that for deleted rows, nothing >> more needs to be done. For example, see the below check in >>

Re: [HACKERS] UPDATE of partition key

2017-06-07 Thread Robert Haas
On Wed, Jun 7, 2017 at 5:46 AM, Amit Kapila wrote: > As far as I understand, it is to ensure that for deleted rows, nothing > more needs to be done. For example, see the below check in > ExecUpdate/ExecDelete. > if (!ItemPointerEquals(tupleid, )) > { > .. > } > .. > >

Re: [HACKERS] UPDATE of partition key

2017-06-07 Thread Amit Khandekar
On 7 June 2017 at 16:42, Amit Khandekar wrote: > The column bitmap set returned by GetUpdatedColumns() refer to > attribute numbers w.r.t. to the root partition. And the > mstate->resultRelInfo[] have attnos w.r.t. to the leaf partitions. So > we need to do something

Re: [HACKERS] UPDATE of partition key

2017-06-07 Thread Amit Khandekar
On 6 June 2017 at 23:52, Robert Haas wrote: > On Fri, Jun 2, 2017 at 7:07 AM, Amit Khandekar wrote: >> So, according to that, below would be the logic : >> >> Run partition constraint check on the original NEW row. >> If it succeeds : >> { >>

Re: [HACKERS] UPDATE of partition key

2017-06-07 Thread Amit Kapila
On Tue, Jun 6, 2017 at 11:54 PM, Robert Haas wrote: > On Mon, Jun 5, 2017 at 2:51 AM, Amit Kapila wrote: >>> Greg/Amit's idea of using the CTID field rather than an infomask bit >>> seems like a possibly promising approach. Not everything that

Re: [HACKERS] UPDATE of partition key

2017-06-06 Thread Robert Haas
On Mon, Jun 5, 2017 at 2:51 AM, Amit Kapila wrote: >> Greg/Amit's idea of using the CTID field rather than an infomask bit >> seems like a possibly promising approach. Not everything that needs >> bit-space can use the CTID field, so using it is a little less likely >>

Re: [HACKERS] UPDATE of partition key

2017-06-06 Thread Robert Haas
On Fri, Jun 2, 2017 at 7:07 AM, Amit Khandekar wrote: > So, according to that, below would be the logic : > > Run partition constraint check on the original NEW row. > If it succeeds : > { > Fire BR UPDATE trigger on the original partition. > Run partition

Re: [HACKERS] UPDATE of partition key

2017-06-05 Thread Amit Khandekar
On 5 June 2017 at 11:27, Amit Kapila wrote: > On Fri, Jun 2, 2017 at 4:37 PM, Amit Khandekar wrote: >> On 2 June 2017 at 01:17, Robert Haas wrote: >>> On Thu, Jun 1, 2017 at 7:41 AM, Amit Khandekar

  1   2   >