Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/07 8:25), Robert Haas wrote: On Thu, Apr 5, 2018 at 6:21 AM, Etsuro Fujita wrote: Attached is an updated version of the patch set plus the patch in [1]. Patch 0003_foreign-routing-fdwapi-6.patch can be applied on top of patch

Re: [HACKERS] Add support for tuple routing to foreign partitions

On Sun, Apr 8, 2018 at 5:37 AM, Andres Freund wrote: > On 2018-04-06 19:25:20 -0400, Robert Haas wrote: >> On Thu, Apr 5, 2018 at 6:21 AM, Etsuro Fujita >> wrote: >> > Attached is an updated version of the patch set plus the patch in [1]. >> >

Re: [HACKERS] Add support for tuple routing to foreign partitions

On 2018-04-06 19:25:20 -0400, Robert Haas wrote: > On Thu, Apr 5, 2018 at 6:21 AM, Etsuro Fujita > wrote: > > Attached is an updated version of the patch set plus the patch in [1]. Patch > > 0003_foreign-routing-fdwapi-6.patch can be applied on top of patch > >

Re: [HACKERS] Add support for tuple routing to foreign partitions

On Thu, Apr 5, 2018 at 6:21 AM, Etsuro Fujita wrote: > Attached is an updated version of the patch set plus the patch in [1]. Patch > 0003_foreign-routing-fdwapi-6.patch can be applied on top of patch > 0001_postgres-fdw-refactoring-6.patch and >

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/05 16:31), Amit Langote wrote: Might be a good idea to attach the bug-fix patch here as well, and perhaps add numbers to the file names like: 0001_postgres-fdw-refactoring-5.patch 0002_BUGFIX-copy-from-check-constraint-fix.patch 0003_foreign-routing-fdwapi-5.patch OK Just one minor

Re: [HACKERS] Add support for tuple routing to foreign partitions

On 2018/04/05 16:31, Amit Langote wrote: > Fuiita-san, Oops, sorry about misspelling your name here, Fujita-san. - Amit

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fuiita-san, On 2018/04/05 15:56, Etsuro Fujita wrote: > (2018/04/05 15:37), Amit Langote wrote: >> I noticed that the 2nd patch (foreign-routing-fdwapi-5.patch) fails to >> apply to copy.c: > > I forgot to mention this: the second patch is created on top of the first > patch

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/05 15:37), Amit Langote wrote: On 2018/04/05 15:02, Etsuro Fujita wrote: (2018/04/04 19:31), Etsuro Fujita wrote: Attached is an updated version of the patch set: * As before, patch foreign-routing-fdwapi-4.patch is created on top of patch postgres-fdw-refactoring-4.patch and the

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, On 2018/04/05 15:02, Etsuro Fujita wrote: > (2018/04/04 19:31), Etsuro Fujita wrote: >> Attached is an updated version of the patch set: >> * As before, patch foreign-routing-fdwapi-4.patch is created on top of >> patch postgres-fdw-refactoring-4.patch and the bug-fix patch [1]. > >

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/04 19:31), Etsuro Fujita wrote: Attached is an updated version of the patch set: * As before, patch foreign-routing-fdwapi-4.patch is created on top of patch postgres-fdw-refactoring-4.patch and the bug-fix patch [1]. I did a bit of cleanup and comment-rewording to patch

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/03 22:01), Etsuro Fujita wrote: Attached is an updated version of the patch. Patch foreign-routing-fdwapi-3.patch is created on top of patch postgres-fdw-refactoring-3.patch and the bug-fix patch [1]. One thing I noticed about patch foreign-routing-fdwapi-3.patch is this bug: the

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/03 13:59), Amit Langote wrote: On 2018/04/02 21:29, Etsuro Fujita wrote: (2018/04/02 18:49), Amit Langote wrote: I looked at the new patch. It looks good overall, although I have one question -- IIUC, BeginForeignInsert() performs actions that are equivalent of performing

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/03 13:32), Amit Langote wrote: On 2018/04/02 21:26, Etsuro Fujita wrote: We wouldn't need that for foreign partitions (When DO NOTHING with an inference specification or DO UPDATE on a partitioned table containing foreign partitions, the planner would throw an error before we get to

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, On 2018/04/02 21:29, Etsuro Fujita wrote: > (2018/04/02 18:49), Amit Langote wrote: >> I looked at the new patch.  It looks good overall, although I have one >> question -- IIUC, BeginForeignInsert() performs actions that are >> equivalent of performing PlanForeignModify() +

Re: [HACKERS] Add support for tuple routing to foreign partitions

On 2018/04/02 21:26, Etsuro Fujita wrote: > (2018/03/30 22:28), Alvaro Herrera wrote: >> Etsuro Fujita wrote: >> >>> Now we have ON CONFLICT for partitioned tables, which requires the >>> conversion map to be computed in ExecInitPartitionInfo, so I updated the >>> patch so that we keep the former

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/04/02 18:49), Amit Langote wrote: 2. If I understand the description you provided in [1] correctly, the point of having ri_PartitionIsValid and ExecInitExtraPartitionInfo() is to avoid possibly-redundantly performing following two steps in ExecSetupPartitionTupleRouting() for *reused*

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/30 22:28), Alvaro Herrera wrote: Etsuro Fujita wrote: Now we have ON CONFLICT for partitioned tables, which requires the conversion map to be computed in ExecInitPartitionInfo, so I updated the patch so that we keep the former step in ExecInitPartitionInfo and

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, Thanks for updating the patch. On 2018/03/30 21:56, Etsuro Fujita wrote: > I modified the patch to use the existing API ExecForeignInsert instead of > that API and removed that API including this doc. OK. >> 2. If I understand the description you provided in [1] correctly, the >>

Re: [HACKERS] Add support for tuple routing to foreign partitions

Etsuro Fujita wrote: > Now we have ON CONFLICT for partitioned tables, which requires the > conversion map to be computed in ExecInitPartitionInfo, so I updated the > patch so that we keep the former step in ExecInitPartitionInfo and > ExecSetupPartitionTupleRouting and so that we just init the

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/20 21:31), Alvaro Herrera wrote: Amit Langote wrote: 2. If I understand the description you provided in [1] correctly, the point of having ri_PartitionIsValid and ExecInitExtraPartitionInfo() is to avoid possibly-redundantly performing following two steps in

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/23 20:55), Etsuro Fujita wrote: (2018/03/23 4:09), Robert Haas wrote: 1. It still doesn't work for COPY, because COPY isn't going to have a ModifyTableState. I really think it ought to be possible to come up with an API that can handle both INSERT and COPY; I don't think it should be

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/19 20:25), Amit Langote wrote: On 2018/02/27 21:01, Etsuro Fujita wrote: Attached is a new version of the patch set. * Comments postgres-fdw-refactoring-1.patch: 1. Just a minor nitpick: wouldn't it be better to call it create_foreign_modify_state just like its "finish" counterpart

Re: [HACKERS] Add support for tuple routing to foreign partitions

On Fri, Mar 23, 2018 at 8:22 AM, Etsuro Fujita wrote: >> I think for bulk >> inserts we'll need an API that says "here's a row, store it or buffer >> it as you like" and then another API that says "flush any buffered >> rows to the actual table and perform any

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/23 21:02), Robert Haas wrote: On Fri, Mar 23, 2018 at 7:55 AM, Etsuro Fujita wrote: Maybe I'm missing something, but I think the proposed FDW API could be used for the COPY case as well with some modifications to core. If so, my question is: should we

Re: [HACKERS] Add support for tuple routing to foreign partitions

On Fri, Mar 23, 2018 at 7:55 AM, Etsuro Fujita wrote: > Maybe I'm missing something, but I think the proposed FDW API could be used > for the COPY case as well with some modifications to core. If so, my > question is: should we support COPY into foreign tables as

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/03/23 4:09), Robert Haas wrote: On Tue, Feb 27, 2018 at 7:01 AM, Etsuro Fujita Attached is a new version of the patch set. I took a look at this patch set today but I really don't think we should commit something so minimal. It's got at least four issues that I see: 1. It still

Re: [HACKERS] Add support for tuple routing to foreign partitions

On Tue, Feb 27, 2018 at 7:01 AM, Etsuro Fujita wrote: > Changes other than that are: > > * Fixed typo and revised code/comments > * Added more regression tests > * Added docs > > Attached is a new version of the patch set. I took a look at this patch set today but I

Re: [HACKERS] Add support for tuple routing to foreign partitions

Hi, Amit Langote wrote: > 2. If I understand the description you provided in [1] correctly, the > point of having ri_PartitionIsValid and ExecInitExtraPartitionInfo() is to > avoid possibly-redundantly performing following two steps in > ExecSetupPartitionTupleRouting() for *reused* partition

Re: [HACKERS] Add support for tuple routing to foreign partitions

Hi Amit, (2018/03/20 15:57), Amit Langote wrote: On 2018/03/19 20:25, Amit Langote wrote: That's all I have for now. Will reply to your previous email. While testing this patch, I noticed a crash when performing EXPLAIN on update of a partition tree containing foreign partitions. Crash

Re: [HACKERS] Add support for tuple routing to foreign partitions

On 2018/03/19 20:25, Amit Langote wrote: > That's all I have for now. While testing this patch, I noticed a crash when performing EXPLAIN on update of a partition tree containing foreign partitions. Crash occurs in postgresEndForeignRouting() due to the following Assert failing:

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, Thanks for sending the updated patches. On 2018/02/27 21:01, Etsuro Fujita wrote: > (2018/02/21 20:54), Etsuro Fujita wrote: >> void >> BeginForeignRouting(); >> >> Prepare for a tuple-routing operation on a foreign table. This is called >> from ExecSetupPartitionTupleRouting and

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/02/21 20:54), Etsuro Fujita wrote: void BeginForeignRouting(); Prepare for a tuple-routing operation on a foreign table. This is called from ExecSetupPartitionTupleRouting and ExecInitPartitionInfo. I modified execPartition.c so that this callback routine is called from a single

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/02/23 16:38), Amit Langote wrote: On Thu, Feb 22, 2018 at 8:49 PM, Etsuro Fujita wrote: This would introduce an asymmetry (we can move tuples from plain partitions to foreign partitions, but the reverse is not true), but I am thinking that it would be

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, On Thu, Feb 22, 2018 at 8:49 PM, Etsuro Fujita wrote: > (2018/02/22 11:52), Amit Langote wrote: >> I wonder why partition_index needs to be made part of this API? > > The reason for that is because I think the FDW might want to look at the > partition

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/02/22 11:52), Amit Langote wrote: On 2018/02/21 20:54, Etsuro Fujita wrote: void BeginForeignRouting(ModifyTableState *mtstate, ResultRelInfo *resultRelInfo, int partition_index); Prepare for a tuple-routing operation on a foreign table. This is

Re: [HACKERS] Add support for tuple routing to foreign partitions

Fujita-san, On 2018/02/21 20:54, Etsuro Fujita wrote: > (2018/02/02 19:33), Etsuro Fujita wrote: >> (2018/01/25 23:33), Stephen Frost wrote: >>> I'm afraid a good bit of this patch is now failing to apply. I don't >>> have much else to say except to echo the performance concern that Amit >>>

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/02/02 19:33), Etsuro Fujita wrote: (2018/01/25 23:33), Stephen Frost wrote: I'm afraid a good bit of this patch is now failing to apply. I don't have much else to say except to echo the performance concern that Amit Langote raised about expanding the inheritence tree twice. To address

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/01/25 23:33), Stephen Frost wrote: I'm afraid a good bit of this patch is now failing to apply. I don't have much else to say except to echo the performance concern that Amit Langote raised about expanding the inheritence tree twice. To address that concern, I'm thinking to redesign the

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2018/02/02 19:33), Etsuro Fujita wrote: (2018/01/25 23:33), Stephen Frost wrote: I'm afraid a good bit of this patch is now failing to apply. I don't have much else to say except to echo the performance concern that Amit Langote raised about expanding the inheritence tree twice. To address

Re: [HACKERS] Add support for tuple routing to foreign partitions

Etsuro, * Etsuro Fujita (fujita.ets...@lab.ntt.co.jp) wrote: > (2017/12/18 23:25), Alvaro Herrera wrote: > >InitResultRelInfo becomes unintelligible after this patch -- it was > >straightforward but adding partition_root makes things shaky. Please > >add a proper comment indicating what each

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2017/12/18 23:25), Alvaro Herrera wrote: InitResultRelInfo becomes unintelligible after this patch -- it was straightforward but adding partition_root makes things shaky. Please add a proper comment indicating what each argument is. I was thiking that the comment I added to the definition of

Re: [HACKERS] Add support for tuple routing to foreign partitions

On 2017/12/18 23:25, Alvaro Herrera wrote: > (I wonder why > this function needs a local variable "partition_check" -- seems > pointless). Before 15ce775faa4 [1], there were more than one line where partition_check was being set, but maybe it still didn't have to be a separate variable. Thanks,

Re: [HACKERS] Add support for tuple routing to foreign partitions

InitResultRelInfo becomes unintelligible after this patch -- it was straightforward but adding partition_root makes things shaky. Please add a proper comment indicating what each argument is. (I wonder why this function needs a local variable "partition_check" -- seems pointless). -- Álvaro

Re: [HACKERS] Add support for tuple routing to foreign partitions

(2017/12/18 18:14), Amit Langote wrote: I noticed that this patch introduces a partition_rels field in ModifyTable, which contains the RT indexes of *all* leaf partitions in the partition tree. That means we now expand the partition inheritance tree in the planner even in the INSERT case,

Re: [HACKERS] Add support for tuple routing to foreign partitions

Hi Maksim, (2017/12/12 17:57), Maksim Milyutin wrote: Your patch already is not applied on master. Please rebase it. Done. Please find attached an updated version of the patch. Best regards, Etsuro Fujita *** a/contrib/file_fdw/input/file_fdw.source ---

Re: [HACKERS] Add support for tuple routing to foreign partitions

Hi, Fujita-san! On 24.11.2017 16:03, Etsuro Fujita wrote: (2017/10/27 20:00), Etsuro Fujita wrote: Please find attached an updated version of the patch. Amit rebased this patch and sent me the rebased version off list. Thanks for that, Amit! One thing I noticed I overlooked is about this