Avoid using lcons and list_delete_first in plan_union_children()
Hi,
In function plan_union_children(),
I found the lcons and list_delete_first here is easy to be replaced by lappend and list_delete_last.
And I also found a previous commit do similar thing, so I try to improve this one.
Previous commit:
d97b714a219959a50f9e7b37ded674f5132f93f3
Best regards,
houzj
Attachments:
0001-Avoid-using-lcons-and-list_delete_first.patchapplication/octet-stream; name=0001-Avoid-using-lcons-and-list_delete_first.patchDownload
From 34a1de41f654f92b61b2d78725b0b86c41c24bf3 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Tue, 1 Dec 2020 05:42:45 -0500
Subject: [PATCH] Avoid using lcons and list_delete_first
---
src/backend/optimizer/prep/prepunion.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 745f443..28bb96a 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -886,9 +886,9 @@ plan_union_children(PlannerInfo *root,
while (pending_rels != NIL)
{
- Node *setOp = linitial(pending_rels);
+ Node *setOp = llast(pending_rels);
- pending_rels = list_delete_first(pending_rels);
+ pending_rels = list_delete_last(pending_rels);
if (IsA(setOp, SetOperationStmt))
{
@@ -899,8 +899,8 @@ plan_union_children(PlannerInfo *root,
equal(op->colTypes, top_union->colTypes))
{
/* Same UNION, so fold children into parent */
- pending_rels = lcons(op->rarg, pending_rels);
- pending_rels = lcons(op->larg, pending_rels);
+ pending_rels = lappend(pending_rels, op->rarg);
+ pending_rels = lappend(pending_rels, op->larg);
continue;
}
}
--
1.8.3.1
On 01/12/2020 12:52, Hou, Zhijie wrote:
Hi,
In function plan_union_children(),
I found the lcons and list_delete_first here is easy to be replaced by lappend and list_delete_last.And I also found a previous commit do similar thing, so I try to improve this one.
Previous commit:
d97b714a219959a50f9e7b37ded674f5132f93f3
This doesn't matter much in practice. I was able to measure a
performance difference with a very extreme example: "SELECT 1 UNION
SELECT 2 UNION ... UNION SELECT 10000". That was about 30% faster with
this patch.
I don't see any downside, though. And like the commit message of
d97b714a21, it's better to have good examples than bad ones in the code
base, if someone copy-pastes it to somewhere where it matters.
But then again, I'm not excited about changing these one by one. If this
is worth changing, what about the one in simplify_or_arguments()? Or in
CopyMultiInsertInfoFlush()?
Perhaps it would make sense to invent a new Queue abstraction for this.
Or maybe it would be overkill..
- Heikki