Make executor's Range Table an array instead of a List

Started by David Rowleyover 7 years ago6 messages

david.rowley@2ndquadrant.com

over 7 years ago

1 attachment(s)

Continuing along on my adventures of making performance improvements
for partitioned tables, I discovered that getrelid is quite slow in
InitPlan() during UPDATEs and DELETEs when there are many
resultRelations to process. A List is a pretty poor choice for this
data structure since we already know how big it is and we pretty much
only ever perform lookups by the List's Nth element.

Converting this to an array obviously improves that O(n) List lookup
into an O(1) operation.

Benchmark against today's master with 300 partitions using prepared statements:

CREATE TABLE partbench (id BIGINT NOT NULL, i1 INT NOT NULL, i2 INT
NOT NULL, i3 INT NOT NULL, i4 INT NOT NULL, i5 INT NOT NULL) PARTITION
BY RANGE (id);
\o /dev/null
select 'CREATE TABLE partbench' || x::text || ' PARTITION OF partbench
FOR VALUES FROM (' || (x*100000)::text || ') TO (' ||
((x+1)*100000)::text || ');' from generate_Series(0,299) x;
\gexec
\o

ALTER SYSTEM SET plan_cache_mode = 'force_generic_plan'; -- minimise
planning overhead.

update.sql:

\set p_id 29999999
update partbench set i1 = i1+1 where id = :p_id;

pgbench -n -M prepared -T 60 -f update.sql postgres

Unpatched:

tps = 558.708604 (excluding connections establishing)
tps = 556.128341 (excluding connections establishing)
tps = 569.096393 (excluding connections establishing)

Patched:

tps = 683.271356 (excluding connections establishing)
tps = 678.693578 (excluding connections establishing)
tps = 694.446564 (excluding connections establishing)

(about 22% improvement)

Of course, it would be much nicer if PlannedStmt->rtable was already
an array and we didn't have to convert it to one during executor
startup. We'd need to build an array that's a Node type for that to
work. I did suggest something ([1]/messages/by-id/CAKJS1f_2SnXhPVa6eWjzy2O9A=ocwgd0Cj-LQeWpGtrWqbUSDA@mail.gmail.com) last year in this regard, but I
felt like it might be a bit of an uphill struggle to gain enough
community support to make that work. So I feel like this patch is
quite worthwhile in the meantime.

The (fairly small) patch is attached.

I'll add this to the September 'fest.

[1]: /messages/by-id/CAKJS1f_2SnXhPVa6eWjzy2O9A=ocwgd0Cj-LQeWpGtrWqbUSDA@mail.gmail.com

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

v1-0001-Change-the-executor-s-range-table-from-a-List-to-.patchapplication/octet-stream; name=v1-0001-Change-the-executor-s-range-table-from-a-List-to-.patchDownload

From 86d15e8f2f5138ffa61ae57947baf10077b91563 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Thu, 23 Aug 2018 18:48:21 +1200
Subject: [PATCH v1] Change the executor's range table from a List to an array

When the range table contained many entries performing an rt_fetch or a
getrelid could become quite expensive due to List's O(n) lookup time.
With an array the lookup is O(1).  This improves performance quite a
bit for partitioned table UPDATEs and DELETEs which have many result
relations.

When looking for a RangeTblEntry in the executor now the new
exec_rt_fetch macro must be used. rt_fetch is no longer valid.
---
 contrib/postgres_fdw/postgres_fdw.c      | 12 ++++++------
 src/backend/commands/copy.c              | 12 +++++++++++-
 src/backend/commands/trigger.c           |  2 +-
 src/backend/executor/execExprInterp.c    |  6 +++---
 src/backend/executor/execMain.c          | 29 +++++++++++++++++++++--------
 src/backend/executor/execParallel.c      | 10 +++++++++-
 src/backend/executor/execUtils.c         |  7 ++++---
 src/backend/executor/nodeLockRows.c      |  2 +-
 src/backend/replication/logical/worker.c |  3 ++-
 src/include/nodes/execnodes.h            | 19 ++++++++++++++++++-
 src/include/parser/parsetree.h           | 10 ----------
 11 files changed, 76 insertions(+), 36 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 0803c30a48..2fb3efaf3c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1345,7 +1345,7 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
 		rtindex = fsplan->scan.scanrelid;
 	else
 		rtindex = bms_next_member(fsplan->fs_relids, -1);
-	rte = rt_fetch(rtindex, estate->es_range_table);
+	rte = exec_rt_fetch(rtindex, estate->es_range_table_array);
 	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
 	/* Get info about foreign table. */
@@ -1732,8 +1732,8 @@ postgresBeginForeignModify(ModifyTableState *mtstate,
 										FdwModifyPrivateRetrievedAttrs);
 
 	/* Find RTE. */
-	rte = rt_fetch(resultRelInfo->ri_RangeTableIndex,
-				   mtstate->ps.state->es_range_table);
+	rte = exec_rt_fetch(resultRelInfo->ri_RangeTableIndex,
+						mtstate->ps.state->es_range_table_array);
 
 	/* Construct an execution state. */
 	fmstate = create_foreign_modify(mtstate->ps.state,
@@ -2037,7 +2037,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate,
 	 * correspond to this partition if it is one of the UPDATE subplan target
 	 * rels; in that case, we can just use the existing RTE as-is.
 	 */
-	rte = list_nth(estate->es_range_table, resultRelation - 1);
+	rte = exec_rt_fetch(resultRelation, estate->es_range_table_array);
 	if (rte->relid != RelationGetRelid(rel))
 	{
 		rte = copyObject(rte);
@@ -2397,7 +2397,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
 	 * ExecCheckRTEPerms() does.
 	 */
 	rtindex = estate->es_result_relation_info->ri_RangeTableIndex;
-	rte = rt_fetch(rtindex, estate->es_range_table);
+	rte = exec_rt_fetch(rtindex, estate->es_range_table_array);
 	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
 	/* Get info about foreign table. */
@@ -5757,7 +5757,7 @@ conversion_error_callback(void *arg)
 			RangeTblEntry *rte;
 			Var		   *var = (Var *) tle->expr;
 
-			rte = rt_fetch(var->varno, estate->es_range_table);
+			rte = exec_rt_fetch(var->varno, estate->es_range_table_array);
 
 			if (var->varattno == 0)
 				is_wholerow = true;
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 9bc67ce60f..f3d423b8a1 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2333,6 +2333,8 @@ CopyFrom(CopyState cstate)
 	bool		has_before_insert_row_trig;
 	bool		has_instead_insert_row_trig;
 	bool		leafpart_use_multi_insert = false;
+	int			i;
+	ListCell   *lc;
 
 #define MAX_BUFFERED_TUPLES 1000
 #define RECHECK_MULTI_INSERT_THRESHOLD 1000
@@ -2482,7 +2484,15 @@ CopyFrom(CopyState cstate)
 	estate->es_result_relations = resultRelInfo;
 	estate->es_num_result_relations = 1;
 	estate->es_result_relation_info = resultRelInfo;
-	estate->es_range_table = cstate->range_table;
+
+	estate->es_range_table_size = list_length(cstate->range_table);
+	estate->es_range_table_array = (RangeTblEntry **) palloc(sizeof(RangeTblEntry *) *
+												estate->es_range_table_size);
+
+	/* Populate the range table array */
+	i = 0;
+	foreach(lc, cstate->range_table)
+		estate->es_range_table_array[i++] = lfirst_node(RangeTblEntry, lc);
 
 	/* Set up a tuple slot too */
 	myslot = ExecInitExtraTupleSlot(estate, tupDesc);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 2436692eb8..6184e21878 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -75,7 +75,7 @@ static int	MyTriggerDepth = 0;
  * to be changed, however.
  */
 #define GetUpdatedColumns(relinfo, estate) \
-	(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols)
+	(exec_rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table_array)->updatedCols)
 
 /* Local function prototypes */
 static void ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid);
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 9d6e25aae5..9a5589552a 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3961,10 +3961,10 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 		 * perhaps other places.)
 		 */
 		if (econtext->ecxt_estate &&
-			variable->varno <= list_length(econtext->ecxt_estate->es_range_table))
+			variable->varno <= econtext->ecxt_estate->es_range_table_size)
 		{
-			RangeTblEntry *rte = rt_fetch(variable->varno,
-										  econtext->ecxt_estate->es_range_table);
+			RangeTblEntry *rte = exec_rt_fetch(variable->varno,
+											   econtext->ecxt_estate->es_range_table_array);
 
 			if (rte->eref)
 				ExecTypeSetColNames(output_tupdesc, rte->eref->colnames);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index c583e020a0..c8dec6722d 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -107,9 +107,9 @@ static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
  * to be changed, however.
  */
 #define GetInsertedColumns(relinfo, estate) \
-	(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->insertedCols)
+	(exec_rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table_array)->insertedCols)
 #define GetUpdatedColumns(relinfo, estate) \
-	(rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols)
+	(exec_rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table_array)->updatedCols)
 
 /* end of local decls */
 
@@ -806,7 +806,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	CmdType		operation = queryDesc->operation;
 	PlannedStmt *plannedstmt = queryDesc->plannedstmt;
 	Plan	   *plan = plannedstmt->planTree;
-	List	   *rangeTable = plannedstmt->rtable;
+	RangeTblEntry **rangeTable;
 	EState	   *estate = queryDesc->estate;
 	PlanState  *planstate;
 	TupleDesc	tupType;
@@ -816,12 +816,24 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	/*
 	 * Do permissions checks
 	 */
-	ExecCheckRTPerms(rangeTable, true);
+	ExecCheckRTPerms(plannedstmt->rtable, true);
 
 	/*
 	 * initialize the node's execution state
 	 */
-	estate->es_range_table = rangeTable;
+
+	/* Build the range table array from the rtable List */
+	estate->es_range_table_size = list_length(plannedstmt->rtable);
+	rangeTable = (RangeTblEntry **) palloc(sizeof(RangeTblEntry *) *
+										   estate->es_range_table_size);
+	estate->es_range_table_array = rangeTable;
+
+	/* Populate the range table array */
+	i = 0;
+	foreach(l, plannedstmt->rtable)
+		estate->es_range_table_array[i++] = lfirst_node(RangeTblEntry, l);
+
+
 	estate->es_plannedstmt = plannedstmt;
 
 	/*
@@ -3068,7 +3080,7 @@ EvalPlanQualBegin(EPQState *epqstate, EState *parentestate)
 		/*
 		 * We already have a suitable child EPQ tree, so just reset it.
 		 */
-		int			rtsize = list_length(parentestate->es_range_table);
+		int			rtsize = parentestate->es_range_table_size;
 		PlanState  *planstate = epqstate->planstate;
 
 		MemSet(estate->es_epqScanDone, 0, rtsize * sizeof(bool));
@@ -3113,7 +3125,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
 	MemoryContext oldcontext;
 	ListCell   *l;
 
-	rtsize = list_length(parentestate->es_range_table);
+	rtsize = parentestate->es_range_table_size;
 
 	epqstate->estate = estate = CreateExecutorState();
 
@@ -3136,7 +3148,8 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
 	estate->es_direction = ForwardScanDirection;
 	estate->es_snapshot = parentestate->es_snapshot;
 	estate->es_crosscheck_snapshot = parentestate->es_crosscheck_snapshot;
-	estate->es_range_table = parentestate->es_range_table;
+	estate->es_range_table_array = parentestate->es_range_table_array;
+	estate->es_range_table_size = parentestate->es_range_table_size;
 	estate->es_plannedstmt = parentestate->es_plannedstmt;
 	estate->es_junkFilter = parentestate->es_junkFilter;
 	estate->es_output_cid = parentestate->es_output_cid;
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index ee0f07a81e..48bb965bd3 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -144,6 +144,8 @@ ExecSerializePlan(Plan *plan, EState *estate)
 {
 	PlannedStmt *pstmt;
 	ListCell   *lc;
+	List	   *rtable;
+	int			i;
 
 	/* We can't scribble on the original plan, so make a copy. */
 	plan = copyObject(plan);
@@ -179,7 +181,13 @@ ExecSerializePlan(Plan *plan, EState *estate)
 	pstmt->dependsOnRole = false;
 	pstmt->parallelModeNeeded = false;
 	pstmt->planTree = plan;
-	pstmt->rtable = estate->es_range_table;
+
+	/* Transform the range table array into List form */
+	rtable = NIL;
+	for (i = 0; i < estate->es_range_table_size; i++)
+		rtable = lappend(rtable, estate->es_range_table_array[i]);
+
+	pstmt->rtable = rtable;
 	pstmt->resultRelations = NIL;
 	pstmt->nonleafResultRelations = NIL;
 
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 5b3eaec80b..099f0fe5c6 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -105,7 +105,8 @@ CreateExecutorState(void)
 	estate->es_direction = ForwardScanDirection;
 	estate->es_snapshot = InvalidSnapshot;	/* caller must initialize this */
 	estate->es_crosscheck_snapshot = InvalidSnapshot;	/* no crosscheck */
-	estate->es_range_table = NIL;
+	estate->es_range_table_array = NULL;
+	estate->es_range_table_size = 0;
 	estate->es_plannedstmt = NULL;
 
 	estate->es_junkFilter = NULL;
@@ -672,7 +673,7 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
 	}
 
 	/* Open the relation and acquire lock as needed */
-	reloid = getrelid(scanrelid, estate->es_range_table);
+	reloid = getrelid(scanrelid, estate->es_range_table_array);
 	rel = heap_open(reloid, lockmode);
 
 	/*
@@ -881,7 +882,7 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
 		ListCell   *l;
 		Index		rti = lfirst_int(lc);
 		bool		is_result_rel = false;
-		Oid			relid = getrelid(rti, estate->es_range_table);
+		Oid			relid = getrelid(rti, estate->es_range_table_array);
 
 		/* If this is a result relation, already locked in InitPlan */
 		foreach(l, stmt->nonleafResultRelations)
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 30de8a95ab..6db345ae7a 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -400,7 +400,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
 	/*
 	 * Create workspace in which we can remember per-RTE locked tuples
 	 */
-	lrstate->lr_ntables = list_length(estate->es_range_table);
+	lrstate->lr_ntables = estate->es_range_table_size;
 	lrstate->lr_curtuples = (HeapTuple *)
 		palloc0(lrstate->lr_ntables * sizeof(HeapTuple));
 
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 2054abe653..5e7aa7c6c5 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -199,7 +199,8 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
 	rte->rtekind = RTE_RELATION;
 	rte->relid = RelationGetRelid(rel->localrel);
 	rte->relkind = rel->localrel->rd_rel->relkind;
-	estate->es_range_table = list_make1(rte);
+	estate->es_range_table_array = &rte;
+	estate->es_range_table_size = 1;
 
 	resultRelInfo = makeNode(ResultRelInfo);
 	InitResultRelInfo(resultRelInfo, rel->localrel, 1, NULL, 0);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 41fa2052a2..c9e5194056 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -477,7 +477,8 @@ typedef struct EState
 	ScanDirection es_direction; /* current scan direction */
 	Snapshot	es_snapshot;	/* time qual to use */
 	Snapshot	es_crosscheck_snapshot; /* crosscheck time qual for RI */
-	List	   *es_range_table; /* List of RangeTblEntry */
+	struct RangeTblEntry **es_range_table_array;	/* 0-based range table */
+	int			es_range_table_size;	/* size of the range table array */
 	PlannedStmt *es_plannedstmt;	/* link to top of plan tree */
 	const char *es_sourceText;	/* Source text from QueryDesc */
 
@@ -573,6 +574,22 @@ typedef struct EState
 	struct JitContext *es_jit;
 } EState;
 
+/*
+ *		exec_rt_fetch
+ *
+ * NB: this will crash and burn if handed an out-of-range RT index
+ */
+#define exec_rt_fetch(rangetblidx, rangetbl) rangetbl[(rangetblidx) - 1]
+
+/* XXX moved from parsetree.h. okay??
+ *		getrelid
+ *
+ *		Given the range index of a relation, return the corresponding
+ *		relation OID.  Note that InvalidOid will be returned if the
+ *		RTE is for a non-relation-type RTE.
+ */
+#define getrelid(rangeindex,rangetable) \
+	(exec_rt_fetch(rangeindex, rangetable)->relid)
 
 /*
  * ExecRowMark -
diff --git a/src/include/parser/parsetree.h b/src/include/parser/parsetree.h
index dd9ae658ac..fe16d7d1fa 100644
--- a/src/include/parser/parsetree.h
+++ b/src/include/parser/parsetree.h
@@ -31,16 +31,6 @@
 #define rt_fetch(rangetable_index, rangetable) \
 	((RangeTblEntry *) list_nth(rangetable, (rangetable_index)-1))
 
-/*
- *		getrelid
- *
- *		Given the range index of a relation, return the corresponding
- *		relation OID.  Note that InvalidOid will be returned if the
- *		RTE is for a non-relation-type RTE.
- */
-#define getrelid(rangeindex,rangetable) \
-	(rt_fetch(rangeindex, rangetable)->relid)
-
 /*
  * Given an RTE and an attribute number, return the appropriate
  * variable name or alias for that attribute of that RTE.
-- 
2.16.2.windows.1

Amit Langote

amitlangote09@gmail.com

over 7 years ago

In reply to: David Rowley (#1)

Re: Make executor's Range Table an array instead of a List

(sorry David for the same email twice, I didn't hit Reply All at first)

On Thu, Aug 23, 2018 at 7:58 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:

Continuing along on my adventures of making performance improvements
for partitioned tables, I discovered that getrelid is quite slow in
InitPlan() during UPDATEs and DELETEs when there are many
resultRelations to process. A List is a pretty poor choice for this
data structure since we already know how big it is and we pretty much
only ever perform lookups by the List's Nth element.

Converting this to an array obviously improves that O(n) List lookup
into an O(1) operation.

Benchmark against today's master with 300 partitions using prepared statements:

CREATE TABLE partbench (id BIGINT NOT NULL, i1 INT NOT NULL, i2 INT
NOT NULL, i3 INT NOT NULL, i4 INT NOT NULL, i5 INT NOT NULL) PARTITION
BY RANGE (id);
\o /dev/null
select 'CREATE TABLE partbench' || x::text || ' PARTITION OF partbench
FOR VALUES FROM (' || (x*100000)::text || ') TO (' ||
((x+1)*100000)::text || ');' from generate_Series(0,299) x;
\gexec
\o

ALTER SYSTEM SET plan_cache_mode = 'force_generic_plan'; -- minimise
planning overhead.

update.sql:

\set p_id 29999999
update partbench set i1 = i1+1 where id = :p_id;

pgbench -n -M prepared -T 60 -f update.sql postgres

Unpatched:

tps = 558.708604 (excluding connections establishing)
tps = 556.128341 (excluding connections establishing)
tps = 569.096393 (excluding connections establishing)

Patched:

tps = 683.271356 (excluding connections establishing)
tps = 678.693578 (excluding connections establishing)
tps = 694.446564 (excluding connections establishing)

(about 22% improvement)

Of course, it would be much nicer if PlannedStmt->rtable was already
an array and we didn't have to convert it to one during executor
startup. We'd need to build an array that's a Node type for that to
work. I did suggest something ([1]) last year in this regard, but I
felt like it might be a bit of an uphill struggle to gain enough
community support to make that work. So I feel like this patch is
quite worthwhile in the meantime.

The (fairly small) patch is attached.

One of the patches I sent last week does the same thing, among a
couple of other things with regard to handling relations in the
executor. On a cursory look at the patch, your take of it looks
better than mine. Will test tomorrow. Here is a link to my email:

/messages/by-id/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

4th of my patches implements the array'fication of executor's range table.

Thanks,
Amit

David Rowley

david.rowley@2ndquadrant.com

over 7 years ago

In reply to: Amit Langote (#2)

Re: Make executor's Range Table an array instead of a List

On 24 August 2018 at 02:26, Amit Langote <amitlangote09@gmail.com> wrote:

One of the patches I sent last week does the same thing, among a
couple of other things with regard to handling relations in the
executor. On a cursory look at the patch, your take of it looks
better than mine. Will test tomorrow. Here is a link to my email:

/messages/by-id/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

4th of my patches implements the array'fication of executor's range table.

Sorry, didn't realise. I'll withdraw this and review yours during the
upcoming 'fest.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 7 years ago

In reply to: David Rowley (#3)

Re: Make executor's Range Table an array instead of a List

On 2018/08/24 7:22, David Rowley wrote:

On 24 August 2018 at 02:26, Amit Langote <amitlangote09@gmail.com> wrote:

One of the patches I sent last week does the same thing, among a
couple of other things with regard to handling relations in the
executor. On a cursory look at the patch, your take of it looks
better than mine. Will test tomorrow. Here is a link to my email:

/messages/by-id/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

4th of my patches implements the array'fication of executor's range table.

Sorry, didn't realise. I'll withdraw this and review yours during the
upcoming 'fest.

Since your patch implemented the idea in a better manner, I'm thinking I
should merge it with mine. Is that okay with you?

Thanks,
Amit

David Rowley

david.rowley@2ndquadrant.com

over 7 years ago

In reply to: Amit Langote (#4)

Re: Make executor's Range Table an array instead of a List

On 4 September 2018 at 19:31, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2018/08/24 7:22, David Rowley wrote:

On 24 August 2018 at 02:26, Amit Langote <amitlangote09@gmail.com> wrote:

One of the patches I sent last week does the same thing, among a
couple of other things with regard to handling relations in the
executor. On a cursory look at the patch, your take of it looks
better than mine. Will test tomorrow. Here is a link to my email:

/messages/by-id/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

4th of my patches implements the array'fication of executor's range table.

Sorry, didn't realise. I'll withdraw this and review yours during the
upcoming 'fest.

Since your patch implemented the idea in a better manner, I'm thinking I
should merge it with mine. Is that okay with you?

Feel free to do that. I've not yet looked at your patch. I was
planning on looking at your partition planning performance
improvements patches first.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Amit Langote

Langote_Amit_f8@lab.ntt.co.jp

over 7 years ago

In reply to: David Rowley (#5)

Re: Make executor's Range Table an array instead of a List

On 2018/09/04 17:14, David Rowley wrote:

On 4 September 2018 at 19:31, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2018/08/24 7:22, David Rowley wrote:

On 24 August 2018 at 02:26, Amit Langote <amitlangote09@gmail.com> wrote:

One of the patches I sent last week does the same thing, among a
couple of other things with regard to handling relations in the
executor. On a cursory look at the patch, your take of it looks
better than mine. Will test tomorrow. Here is a link to my email:

/messages/by-id/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp

4th of my patches implements the array'fication of executor's range table.

Sorry, didn't realise. I'll withdraw this and review yours during the
upcoming 'fest.

Since your patch implemented the idea in a better manner, I'm thinking I
should merge it with mine. Is that okay with you?

Feel free to do that.

Okay, will do.

I've not yet looked at your patch. I was
planning on looking at your partition planning performance
improvements patches first.

Great, thanks.

Regards,
Amit