On Fri, Apr 16, 2021 at 11:55 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Apr 06, 2021 at 02:25:05PM +0900, Amit Langote wrote: > > Attached is v5 that I am finishing with. Much more could be done but > I don't want to do something too invasive at this stage of the game. > There are a couple of extra relations in terms of relations opened for > a partitioned table within create_estate_for_relation() when > redirecting to the tuple routing, but my guess is that this would be > better in the long-term. >
Hmm, I am not sure if it is a good idea to open indexes needlessly especially when it is not done in the previous code. @@ -1766,8 +1771,11 @@ apply_handle_tuple_routing(ResultRelInfo *relinfo, slot_getallattrs(remoteslot); } MemoryContextSwitchTo(oldctx); + + ExecOpenIndices(partrelinfo_new, false); apply_handle_insert_internal(partrelinfo_new, estate, remoteslot_part); + ExecCloseIndices(partrelinfo_new); } It seems you forgot to call open indexes before apply_handle_delete_internal. I am not sure if it is a good idea to do the refactoring related to indexes or other things to fix a minor bug in commit 1375422c. It might be better to add a simple fix like what Hou-San has originally proposed [1] because even using ExecInitResultRelation might not be the best thing as it is again trying to open a range table which we have already opened in logicalrep_rel_open. OTOH, using ExecInitResultRelation might encapsulate the assignment we are doing outside. In general, it seems doing bigger refactoring or changes might lead to some bugs or unintended consequences, so if possible, we can try such refactoring as a separate patch. One argument against the proposed refactoring could be that with the previous code, we were trying to open the indexes just before it is required but with the new patch in some cases, they will be opened during the initial phase and for other cases, they are opened just before they are required. It might not necessarily be a bad idea to rearrange code like that but maybe there is a better way to do that. [1] - https://www.postgresql.org/message-id/OS0PR01MB571686F75FBDC219FF3DFF0D94769%40OS0PR01MB5716.jpnprd01.prod.outlook.com -- With Regards, Amit Kapila.