Identity projection
Hello.
A very simple query shown below on partitioned tables can run for
four times as long as that on non-partitioned tables when the
whole data can be loaded onto memory.
* QUERY : SELECT * FROM t;
* EXEC TREE : Result(Append(SeqScan)) for partitioned
: SeqScan for non-partitioned
One of the cause seems to be the Result node attached over an
Append node does projection regardless of the necessity.
When executing the query for non-partitioned tables, the top node
receives the tuples in relation heap. Meanwhile, for paritioned
one, the Result node executes projection which is redundant.
This patch reduces run time of such queries by 45% when result
recored has 30 columns and seems to have no harm for performance.
You can see the details of performance measurment later in this
message.
=====
Discussion underlying this patch.
As far as I saw, identity projection can be done by returning
econtext->ecxt_outertuple in ExecProject(). node->ps.ps_projInfo
== NULL also can indicate to do so but I preferred to capsulate
them into ExecProject() rather than scattering codes outside.
# 'identity projection' here means that the projected tuple
# contains all orignal attributes in original order.
The next thing to think is when to do so.
ExecBuildProjectionInfo() which controls direct mapping of tuple
slot had been seemed suitable to do that. But I couldn't find
apropriate condition for judging properly whether the lower tuple
slot can be thrown up to the upper node.
Then, I found set_upper_references() seems suitable. The types
and positions of tlist attributes can be examined in quite clear
way there.
More aggressively, Result node could be pruned for
non-projection-capable plans when it is an identity projection.
But set_upper_references() is called far after here so the
projection is unidentified as identity projection or not at the
moment.
Pros and Cons.
The most important advantage of this patch is significant
reduction of runtime for some part of queries returns many
columns for partitioned tables, and union ops.
The most poor point of this patch, I suppose, might be adding
similar examination for tlists, say, exam for direct mapping in
set_upper_references().. This may be implemented smarter than
this patch.
Additionally to that, two steps of copying the flag
tlist_lower_cogruent from Plan node to PlanState node and finally
onto ProjectionInfo seems somewhat ugly..
=====
Patch summary.
This patch introduces 'identity projection' in the process below.
1. set_upper_references() judges the 'congruency' (is this right
wording?) between subplan->targetlist and plan->tagetlist. And
stores the result in plan->tlist_lower_congruent.
2. ExecAssignProjectionInfo() passes tlist_lower_congruent to
ExecBuildProjectionInfo() and the ProjectionInfo created is
marked as 'identity' according to the handed
tlist_lower_congruent. I suppose ExecAssignProjectionInfo()
may be called with tlist_lower_congruent == true only from
ExecInitResult(), ExecInitGroup() and ExecInitAgg().
3. ExecProject() does 'identity projection' if needed.
======
Performance measurement details.
The effect of this patch becomes greater as more columns exists.
The reduction of execution time was up to 45% for select * for a
partitioned table which has 30 columns, and 14% for single
column.
Exec time for non-partitioned (child) table seemes not to be
influenced by this patch.
The detail of the result follows. The result figures are the
averages for ten times run.
OS CentOS6.3
CPU Inter Core i7 965 @ 3.2GHz
Memory 6GB
shared_buffers=2GB # No block read occurred on select.
### TEST FOR RESULT NODE
# CREATE TABLE parent (c01 int, c02 numeric,... cxx int);
# CREATE TABLE child () INHERITS(parent);
# INSERT INTO child (SELECT n, floor(random() * 10000),..
# FROM generate_series(0, 10000000 - 1) n);
30 columns
EXPLAIN ANALYZE SELECT * FROM parent;
original: 4868 ms
patched : 2691 ms (-45%)
EXPLAIN ANALYZE SELECT * FROM child;
original: 1252 ms
patched : 1125 ms (-10%)
15 columns
EXPLAIN ANALYZE SELECT * FROM parent;
original: 3785 ms
patched : 2685 ms (-29%)
EXPLAIN ANALYZE SELECT * FROM child0;
original: 1108 ms
patched : 1091 ms (-1.5%)
2 columns
EXPLAIN ANALYZE SELECT * FROM parent;
original: 3785 ms
patched : 2560 ms (-32%)
EXPLAIN ANALYZE SELECT * FROM child;
original: 1108 ms
patched : 973 ms (-12%)
1 column
EXPLAIN ANALYZE SELECT * FROM parent;
original: 2998 ms
patched : 2593 ms (-14%)
EXPLAIN ANALYZE SELECT * FROM child;
original: 1141 ms
patched : 969 ms (-15%)
### TEST FOR GROUP NODE
# CREATE TABLE tbl (c01 int, c02 numeric, c03 int);
# INSERT INTO tbl (SELECT n, floor(random() * 10000),..
# FROM generate_series(0, 10000000 - 1) n);
3 columns
EXPLAIN ANALYZE SELECT * FROM tbl GROUP BY c01, c02, c03;
original: 860 ms
patched : 775 ms (-9.9%)
Negative check - additional time to process identity check on planning.
# CREATE OR REPLACE FUNCTION test () RETURNS int AS $BODY$
DECLARE
r int;
BEGIN
r := 0;
LOOP
PERFORM * FROM parenta LIMIT 1;
r := r + 1;
EXIT WHEN r > 100000;
END LOOP;
RETURN 1;
END;
$BODY$ LANGUAGE plpgsql;
EXPLAIN ANALYZE SELECT test()
original: 2695 ms
patched : 2622 ms (-3%)
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
identproj_v1_20120913.patchtext/x-patch; charset=us-asciiDownload+44-3
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
This patch reduces run time of such queries by 45% when result
recored has 30 columns and seems to have no harm for performance.
This patch seems quite unsafe to me: it's not generally okay for
a plan node to return a slot that doesn't belong to it, because of
tuple-lifespan issues. It's possible that Result in particular
could do that safely, but if so we ought to hack nodeResult.c for
it, not the generic projection machinery.
Something I'd been considering in connection with another example
is teaching the planner not to generate a Result node in the first
place, if the node is just doing an identity projection. There
are a couple of ways that could be done --- one is to get setrefs.c
to remove the node on-the-fly, similar to what it does with
SubqueryScan. But it might be better just to check whether the
node is actually needed before creating it in the first place.
Another point here is that the projection code already special-cases
simple projections, so it's a bit hard to believe that it's as slow as
you suggest above. I wonder if your test case is confusing that
optimization somehow.
regards, tom lane
Hello, Thank you for suggestions.
This patch reduces run time of such queries by 45% when result
recored has 30 columns and seems to have no harm for performance.This patch seems quite unsafe to me: it's not generally okay for
a plan node to return a slot that doesn't belong to it, because of
tuple-lifespan issues. It's possible that Result in particular
could do that safely, but if so we ought to hack nodeResult.c for
it, not the generic projection machinery.
Hmm..
Concerning tuple-lifespan, almost every types of node which may
do projection has the route for no-projInfo. This patch for
nodeResult eventually does the same thing. If they are special
cases and the operation could not be done generally, I should
follow the real(or hidden amoung code lines?) lifespan
regulation...
From the another point of view, execution nodes may hold
tupleslots which palloc'ed in projInfos of them just after
receiving a result tuple from their childs. And thy are finally
free'd in next projection on the same node or ExecEndNode() after
the fiish of processing the entire current query. The life of the
contents in the slots should be valid until next projection in
upper nodes or sending the result tuple. The execution tree is
bottom-spreaded and every node in it could not be executed under
different ancestors, and no multi-threaded execution..
The above is the figure from my view. And I suppose these
facts(is it correct?) are enough to ensure the tuple-lifeplan.
And concerning genericity of 'identity projection', .. Perhaps
you're right. I capsulated the logic into ExecProject but it is
usable only from a few kind of nodes.. I'll revert modification
on ExecProject and do identity projection on each nodes which can
do that.
Something I'd been considering in connection with another example
is teaching the planner not to generate a Result node in the first
place, if the node is just doing an identity projection. There
are a couple of ways that could be done --- one is to get setrefs.c
to remove the node on-the-fly, similar to what it does with
SubqueryScan. But it might be better just to check whether the
node is actually needed before creating it in the first place.
I completely agree with the last sentence regarding Result
node. As I described in the previous message, it was a bit hard
to find the way to do that. I'll seek for that with more effort.
Another point here is that the projection code already special-cases
simple projections, so it's a bit hard to believe that it's as slow as
you suggest above. I wonder if your test case is confusing that
optimization somehow.
The whole table is on memory and query is very simple and the
number of columns is relatively larger in this case. This is
because I intended to improve retrieving a large part of
partitioned table with many columns.
In this case, the executor shuttles up and down in shallow tree
and every level does almost nothing, but the result node does
pfree/palloc and direct mapping up to 30 columns which seems
rather heavy in the whole this execution. I could found no sign
of failure of optimization in that so simple execution tree...
And the effect of cource becomes smaller for fewer columns or
more complex queries, or queries on tables hanging out of memory
onto disks.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Although I said as following, the gain seems a bit larger... I'll
recheck the testing conditions...
Another point here is that the projection code already special-cases
simple projections, so it's a bit hard to believe that it's as slow as
you suggest above. I wonder if your test case is confusing that
optimization somehow.The whole table is on memory and query is very simple and the
number of columns is relatively larger in this case. This is
because I intended to improve retrieving a large part of
partitioned table with many columns.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
This patch seems quite unsafe to me: it's not generally okay for
a plan node to return a slot that doesn't belong to it, because of
tuple-lifespan issues. It's possible that Result in particular
could do that safely, but if so we ought to hack nodeResult.c for
it, not the generic projection machinery.
Concerning tuple-lifespan, almost every types of node which may
do projection has the route for no-projInfo.
Huh? Maybe I'm missing something, but AFAICS the only place where
ExecProject is conditionally bypassed is execScan.c, that is relation
scan nodes. When execScan does skip the projection, what it returns
instead is its scan-tuple slot --- that is, a different slot that still
belongs to the scan node. So nothing changes from the standpoint of
slot ownership or lifespan. Skipping the projection step in a non-leaf
plan node would be an entirely different matter.
From the another point of view, execution nodes may hold
tupleslots which palloc'ed in projInfos of them just after
receiving a result tuple from their childs. And thy are finally
free'd in next projection on the same node or ExecEndNode() after
the fiish of processing the entire current query. The life of the
contents in the slots should be valid until next projection in
upper nodes or sending the result tuple. The execution tree is
bottom-spreaded and every node in it could not be executed under
different ancestors, and no multi-threaded execution..
That's not so true as it used to be, with lazy evaluation of CTEs.
I just had to fix a planner bug that amounted to assuming that plan
tree execution is strictly top-down when it no longer is, so I'm
uncomfortable with adding new assumptions of that kind to the executor.
regards, tom lane
Hello, sorry for long absense,
# I had unexpected and urgent time-consuming tasks... :-(
I have had a bit more precise inspection by two aspects, and they
seemd showing that the difference should be the execution time of
ExecProject.
I'll be able to back fully next week with reviesed patch, and to
take some other pathes to review...
============
Although I said as following, the gain seems a bit larger... I'll
recheck the testing conditions...
I had inspected more precisely on two aspects maginifying the
effect of this patch by putting 300 columns into table.
First, explain analyze says the difference caused by this patch
is only in the actual time of Result node.
orig$ psql -c 'explain analyze select * from parenta'
QUERY PLAN
--------------------------------------------------------------------------
Result (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.013.. *2406.792* rows=1000000 loops=1)
-> Append (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011..412.749 rows=1000000 loops=1)
-> Seq Scan on parenta (cost=0.00..0.00 rows=1 width=1228)
(actual time=0.001..0.001 rows=0 loops=1)
-> Seq Scan on childa000 parenta
(cost=0.00..176667.00 rows=1000000 width=1202)
(actual time=0.009..334.633 rows=1000000 loops=1)
Total runtime: 2446.474 ms
(5 rows)
patched$ psql -c 'explain analyze select * from parenta'
QUERY PLAN
--------------------------------------------------------------------------
Result (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011.. *507.239* rows=1000000 loops=1)
-> Append (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011..419.420 rows=1000000 loops=1)
-> Seq Scan on parenta (cost=0.00..0.00 rows=1 width=1228)
(actual time=0.000..0.000 rows=0 loops=1)
-> Seq Scan on childa000 parenta
(cost=0.00..176667.00 rows=1000000 width=1202)
(actual time=0.010..335.721 rows=1000000 loops=1)
Total runtime: 545.879 ms
(5 rows)
Second, the results of configure --enable-profiling shows that
the exectime of ExecProject chages greately. This is consistent
with what explain showed.
orig:
% cumulative self self total
time seconds seconds calls s/call s/call name
60.29 1.26 1.26 1000005 0.00 0.00 slot_deform_tuple
!> 30.14 1.89 0.63 1000000 0.00 0.00 ExecProject
3.35 1.96 0.07 3000004 0.00 0.00 ExecProcNode
0.96 1.98 0.02 1000002 0.00 0.00 ExecScan
0.96 2.00 0.02 166379 0.00 0.00 TerminateBufferIO
0.48 2.01 0.01 3000004 0.00 0.00 InstrStartNode
0.48 2.02 0.01 3000004 0.00 0.00 InstrStopNode
!> 0.48 2.03 0.01 1000001 0.00 0.00 ExecResult
0.48 2.04 0.01 830718 0.00 0.00 LWLockAcquire
0.48 2.05 0.01 506834 0.00 0.00 hash_search_with_hash_value
0.48 2.06 0.01 341656 0.00 0.00 LockBuffer
0.48 2.07 0.01 168383 0.00 0.00 ReadBuffer_common
0.48 2.08 0.01 4 0.00 0.00 InstrEndLoop
0.48 2.09 0.01 ExecCleanTargetListLength
0.00 2.09 0.00 2000005 0.00 0.00 MemoryContextReset
patched:
% cumulative self self total
time seconds seconds calls ms/call ms/call name
23.08 0.03 0.03 3000004 0.00 0.00 ExecProcNode
15.38 0.05 0.02 1000002 0.00 0.00 heapgettup_pagemode
15.38 0.07 0.02 830718 0.00 0.00 LWLockAcquire
7.69 0.08 0.01 2000005 0.00 0.00 MemoryContextReset
7.69 0.09 0.01 1000002 0.00 0.00 ExecScan
7.69 0.10 0.01 1000000 0.00 0.00 ExecStoreTuple
7.69 0.11 0.01 841135 0.00 0.00 LWLockRelease
7.69 0.12 0.01 168383 0.00 0.00 ReadBuffer_common
7.69 0.13 0.01 168383 0.00 0.00 UnpinBuffer
0.00 0.13 0.00 3000004 0.00 0.00 InstrStartNode
...
!> 0.00 0.13 0.00 1000001 0.00 0.00 ExecResult
!> 0.00 0.13 0.00 1000000 0.00 0.00 ExecProject
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Kyotaro HORIGUCHI wrote:
Hello, sorry for long absense,
# I had unexpected and urgent time-consuming tasks... :-(
I have had a bit more precise inspection by two aspects, and they
seemd showing that the difference should be the execution time of
ExecProject.I'll be able to back fully next week with reviesed patch, and to
take some other pathes to review...
Hi, I've marked this patch as Returned with Feedback (thanks Tom).
Please submit an updated version to the upcoming commitfest. Thanks.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Ah. It's too late. I'll re-submit updated versions of my patches
left alone in the last CF.
Hi, I've marked this patch as Returned with Feedback (thanks Tom).
Please submit an updated version to the upcoming commitfest. Thanks.
I'm sorry and thank you.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane..
The following is the result of performance test posted before in
order to show the source of the gain.
regards,
--
--
Kyotaro Horiguchi
NTT Open Source Software Center
At Fri, 05 Oct 2012 16:04:16 +0900, Kyotaro HORIGUCHI wrote in <20121005.160416.256387378.horiguchi.kyotaro@lab.ntt.co.jp>
Although I said as following, the gain seems a bit larger... I'll
recheck the testing conditions...I had inspected more precisely on two aspects maginifying the
effect of this patch by putting 300 columns into table.First, explain analyze says the difference caused by this patch
is only in the actual time of Result node.orig$ psql -c 'explain analyze select * from parenta'
QUERY PLAN
--------------------------------------------------------------------------
Result (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.013.. *2406.792* rows=1000000 loops=1)
-> Append (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011..412.749 rows=1000000 loops=1)
-> Seq Scan on parenta (cost=0.00..0.00 rows=1 width=1228)
(actual time=0.001..0.001 rows=0 loops=1)
-> Seq Scan on childa000 parenta
(cost=0.00..176667.00 rows=1000000 width=1202)
(actual time=0.009..334.633 rows=1000000 loops=1)
Total runtime: 2446.474 ms
(5 rows)patched$ psql -c 'explain analyze select * from parenta'
QUERY PLAN
--------------------------------------------------------------------------
Result (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011.. *507.239* rows=1000000 loops=1)
-> Append (cost=0.00..176667.00 rows=1000001 width=1202)
(actual time=0.011..419.420 rows=1000000 loops=1)
-> Seq Scan on parenta (cost=0.00..0.00 rows=1 width=1228)
(actual time=0.000..0.000 rows=0 loops=1)
-> Seq Scan on childa000 parenta
(cost=0.00..176667.00 rows=1000000 width=1202)
(actual time=0.010..335.721 rows=1000000 loops=1)
Total runtime: 545.879 ms
(5 rows)Second, the results of configure --enable-profiling shows that
the exectime of ExecProject chages greately. This is consistent
with what explain showed.orig:
% cumulative self self total
time seconds seconds calls s/call s/call name
60.29 1.26 1.26 1000005 0.00 0.00 slot_deform_tuple!> 30.14 1.89 0.63 1000000 0.00 0.00 ExecProject
3.35 1.96 0.07 3000004 0.00 0.00 ExecProcNode
0.96 1.98 0.02 1000002 0.00 0.00 ExecScan
0.96 2.00 0.02 166379 0.00 0.00 TerminateBufferIO
0.48 2.01 0.01 3000004 0.00 0.00 InstrStartNode
0.48 2.02 0.01 3000004 0.00 0.00 InstrStopNode!> 0.48 2.03 0.01 1000001 0.00 0.00 ExecResult
0.48 2.04 0.01 830718 0.00 0.00 LWLockAcquire
0.48 2.05 0.01 506834 0.00 0.00 hash_search_with_hash_value
0.48 2.06 0.01 341656 0.00 0.00 LockBuffer
0.48 2.07 0.01 168383 0.00 0.00 ReadBuffer_common
0.48 2.08 0.01 4 0.00 0.00 InstrEndLoop
0.48 2.09 0.01 ExecCleanTargetListLength
0.00 2.09 0.00 2000005 0.00 0.00 MemoryContextResetpatched:
% cumulative self self total
time seconds seconds calls ms/call ms/call name
23.08 0.03 0.03 3000004 0.00 0.00 ExecProcNode
15.38 0.05 0.02 1000002 0.00 0.00 heapgettup_pagemode
15.38 0.07 0.02 830718 0.00 0.00 LWLockAcquire
7.69 0.08 0.01 2000005 0.00 0.00 MemoryContextReset
7.69 0.09 0.01 1000002 0.00 0.00 ExecScan
7.69 0.10 0.01 1000000 0.00 0.00 ExecStoreTuple
7.69 0.11 0.01 841135 0.00 0.00 LWLockRelease
7.69 0.12 0.01 168383 0.00 0.00 ReadBuffer_common
7.69 0.13 0.01 168383 0.00 0.00 UnpinBuffer
0.00 0.13 0.00 3000004 0.00 0.00 InstrStartNode...
!> 0.00 0.13 0.00 1000001 0.00 0.00 ExecResult
!> 0.00 0.13 0.00 1000000 0.00 0.00 ExecProject
==============================
Attachments:
identproj_v2_20121112.patchtext/x-patch; charset=us-asciiDownload+44-8
On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane..The following is the result of performance test posted before in
order to show the source of the gain.
Hmm, this reminds me of the discussion on removing useless Limit nodes:
http://archives.postgresql.org/pgsql-performance/2012-12/msg00127.php.
The optimization on Group, WindowAgg and Agg nodes doesn't seem that
important, the cost of doing the aggregation/grouping is likely
overwhelming the projection cost, and usually you do projection in
grouping/aggregation anyway. But makes sense for Result.
For Result, I think you should aim to remove the useless Result node
from the plan altogether. And do the same for useless Limit nodes.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Kyotaro,
Are you planning to update this patch based on Heikki's comments? The
patch is listed in the commitfest and we're trying to make some
progress through all of those patches.
Thanks,
Stephen
* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
Show quoted text
On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane..The following is the result of performance test posted before in
order to show the source of the gain.Hmm, this reminds me of the discussion on removing useless Limit
nodes: http://archives.postgresql.org/pgsql-performance/2012-12/msg00127.php.The optimization on Group, WindowAgg and Agg nodes doesn't seem that
important, the cost of doing the aggregation/grouping is likely
overwhelming the projection cost, and usually you do projection in
grouping/aggregation anyway. But makes sense for Result.For Result, I think you should aim to remove the useless Result node
from the plan altogether. And do the same for useless Limit nodes.- Heikki
On Friday, December 14, 2012 5:11 PM Heikki Linnakangas wrote:
On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
Hello, This is new version of identity projection patch.
Reverted projectionInfo and ExecBuildProjectionInfo. Identity
projection is recognized directly in ExecGroup, ExecResult, and
ExecWindowAgg. nodeAgg is reverted because I couldn't make it
sane..The following is the result of performance test posted before in
order to show the source of the gain.Hmm, this reminds me of the discussion on removing useless Limit nodes:
http://archives.postgresql.org/pgsql-performance/2012-12/msg00127.php.The optimization on Group, WindowAgg and Agg nodes doesn't seem that
important, the cost of doing the aggregation/grouping is likely
overwhelming the projection cost, and usually you do projection in
grouping/aggregation anyway. But makes sense for Result.For Result, I think you should aim to remove the useless Result node
from the plan altogether.
I was reviewing this patch and found that it can be move forward as follows:
There can be 2 ways to remove result node
a. Remove the Result plan node in case it is not required - This is same as
currently it does for SubqueryScan.
We can check if the result plan is trivial (with logic similar to
trivial_subqueryscan()), then remove result node.
b. to avoid adding it to Plan node in case it is not required -
For this, in grouping_planner() currently it checks if the plan is
projection capable (is_projection_capable_plan()),
we can enhance this check such that it also check projection is really
required depending if the targetlist contains
any non Var element.
Please suggest which way is more preferable and if one of above 2 seems to
be okay,
I can update the patch on behalf of Kyotaro-san incase they don't have time
currently.
And do the same for useless Limit nodes.
Is it okay, if this can be done as part of separate patch?
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila <amit.kapila@huawei.com> writes:
There can be 2 ways to remove result node
a. Remove the Result plan node in case it is not required - This is same as
currently it does for SubqueryScan.
We can check if the result plan is trivial (with logic similar to
trivial_subqueryscan()), then remove result node.
b. to avoid adding it to Plan node in case it is not required -
For this, in grouping_planner() currently it checks if the plan is
projection capable (is_projection_capable_plan()),
we can enhance this check such that it also check projection is really
required depending if the targetlist contains
any non Var element.
Please suggest which way is more preferable and if one of above 2 seems to
be okay,
Adding a result node only to remove it again seems a bit expensive.
It'd be better not to generate the node in the first place. (There's
a technical reason to generate a temporary SubqueryScan, which is to
keep Var numbering in the subplan separate from that in the upper plan;
but AFAICS that doesn't apply to Result.)
An advantage of removing useless Results at setrefs.c time is that we
can be sure it will be applied to all Result nodes. However, we might
be able to do it the other way with only one point-of-change if we hack
make_result itself to check whether the proposed tlist is an identity.
Note that "contains non Var element" is the wrong test for this anyway
--- the question is does the tlist have the same expressions in the same
order as the tlist of the Result's child node.regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
Amit Kapila <amit.kapila@huawei.com> writes:
There can be 2 ways to remove result node
a. Remove the Result plan node in case it is not required - This issame as
currently it does for SubqueryScan.
We can check if the result plan is trivial (with logic similar to
trivial_subqueryscan()), then remove result node.b. to avoid adding it to Plan node in case it is not required -
For this, in grouping_planner() currently it checks if the plan is
projection capable (is_projection_capable_plan()),
we can enhance this check such that it also check projection isreally
required depending if the targetlist contains
any non Var element.Please suggest which way is more preferable and if one of above 2
seems to
be okay,
Adding a result node only to remove it again seems a bit expensive.
It'd be better not to generate the node in the first place. (There's
a technical reason to generate a temporary SubqueryScan, which is to
keep Var numbering in the subplan separate from that in the upper plan;
but AFAICS that doesn't apply to Result.)An advantage of removing useless Results at setrefs.c time is that we
can be sure it will be applied to all Result nodes. However, we might
be able to do it the other way with only one point-of-change if we hack
make_result itself to check whether the proposed tlist is an identity.
So for this, if a,b,c (below mentioned conds.) are true then don't create
Result Plan, just return subplan
a. subplan is NOT NULL and
b. resconstantqual is NULL and
c. compare expr of each TargetEntry for proposed tlist with subplan target
Note that "contains non Var element" is the wrong test for this anyway --- the question is does the tlist have the same expressions in the same order as the tlist of the Result's child node.
As per my understanding, currently in code wherever Result node can be
avoided,
it calls function is_projection_capable_plan(), so we can even enhance
is_projection_capable_plan()
so that it can also verify the expressions of tlists. But for this we need
to change at all places
from where is_projection_capable_plan() is called.
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila <amit.kapila@huawei.com> writes:
On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
As per my understanding, currently in code wherever Result node can be
avoided,
it calls function is_projection_capable_plan(), so we can even enhance
is_projection_capable_plan()
so that it can also verify the expressions of tlists. But for this we need
to change at all places
from where is_projection_capable_plan() is called.
Hm. Really there's a whole dance that typically goes on, which is like
if (!is_projection_capable_plan(result_plan))
{
result_plan = (Plan *) make_result(root,
sub_tlist,
NULL,
result_plan);
}
else
{
/*
* Otherwise, just replace the subplan's flat tlist with
* the desired tlist.
*/
result_plan->targetlist = sub_tlist;
}
Perhaps we could encapsulate this whole sequence into a function called
say assign_targetlist_to_plan(), which would have the responsibility to
decide whether a Result node needs to be inserted.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Friday, February 08, 2013 11:06 PM Tom Lane wrote:
Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
As per my understanding, currently in code wherever Result node can be
avoided,
it calls function is_projection_capable_plan(), so we can even enhance
is_projection_capable_plan()
so that it can also verify the expressions of tlists. But for this we need
to change at all places
from where is_projection_capable_plan() is called.
Hm. Really there's a whole dance that typically goes on, which is like
if (!is_projection_capable_plan(result_plan))
....
Perhaps we could encapsulate this whole sequence into a function called
say assign_targetlist_to_plan(), which would have the responsibility to
decide whether a Result node needs to be inserted.
If we want to encapsulate whole of above logic in assign_targetlist_to_plan(),
then the responsibility of new functionwill be much higher, because the code that
assigns targetlist is not same at all places.
For example
Related code in prepare_sort_from_pathkeys() is as below where it needs to append junk entry to target list.
if (!adjust_tlist_in_place && !is_projection_capable_plan(lefttree))
{ /* copy needed so we don't modify input's tlist below */
tlist = copyObject(tlist);
lefttree = (Plan *) make_result(root, tlist, NULL, lefttree);
}
/* Don't bother testing is_projection_capable_plan again */
adjust_tlist_in_place = true;
/*
* Add resjunk entry to input's tlist
*/
tle = makeTargetEntry(sortexpr,
list_length(tlist) + 1,
NULL,
true);
tlist = lappend(tlist, tle);
lefttree->targetlist = tlist; /* just in case NIL before */
Similar kind of code is there in grouping_planner for the case of activeWindows.Now we can change the code such that places where any new target entry has to be added to target list, move that part of code before calling assign_targetlist_to_plan or pass extra parameters to assign_targetlist_to_plan, so that it can accomodate all such cases. The story doesn't ends there, in some places it has to make a copy of targetlist before assigning it to plan's targetlist.
How about if just enhance the code as below:
if (!is_projection_capable_plan(result_plan) && compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
{
result_plan = (Plan *) make_result(root,
sub_tlist,
NULL,
result_plan);
where the new function will be something as below:
bool
compare_tlist_exprs(List *tlist1, List *tlist2) {
ListCell *lp, *lc;
if (list_length(tlist1) != list_length(tlist2))
return false; /* tlists not same length */
forboth(lp, tlist1, lc, tlist2) {
TargetEntry *ptle = (TargetEntry *) lfirst(lp);
TargetEntry *ctle = (TargetEntry *) lfirst(lc);
if(!equal(ptle->expr,ctle->expr))
return false;
}
return true;
}
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Saturday, February 09, 2013 6:56 AM Amit kapila wrote:
Friday, February 08, 2013 11:06 PM Tom Lane wrote:
Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
As per my understanding, currently in code wherever Result node can be
avoided,
Hm. Really there's a whole dance that typically goes on, which is like
if (!is_projection_capable_plan(result_plan))
....
Perhaps we could encapsulate this whole sequence into a function called
say assign_targetlist_to_plan(), which would have the responsibility to
decide whether a Result node needs to be inserted.
if (!is_projection_capable_plan(result_plan) && compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
Sorry, the check I suggested in last mail should be as below:
if (!is_projection_capable_plan(result_plan) && !compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit kapila <amit.kapila@huawei.com> writes:
if (!is_projection_capable_plan(result_plan) && compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
Sorry, the check I suggested in last mail should be as below:
if (!is_projection_capable_plan(result_plan) && !compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
You know, I was thinking that compare_tlist_exprs() was a pretty
unhelpfully-chosen name for a function returning boolean, and this
thinko pretty much proves the point. It'd be better to call it
something like equivalent_tlists(), tlists_are_equivalent(), etc.
(I'm not caring for the emphasis on the exprs either, because I think
it'll also be necessary to compare resjunk fields for instance.)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Saturday, February 09, 2013 9:03 AM Tom Lane wrote:
Amit kapila <amit.kapila@huawei.com> writes:
if (!is_projection_capable_plan(result_plan) && compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
Sorry, the check I suggested in last mail should be as below:
if (!is_projection_capable_plan(result_plan) && !compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
You know, I was thinking that compare_tlist_exprs() was a pretty
unhelpfully-chosen name for a function returning boolean, and this
thinko pretty much proves the point. It'd be better to call it
something like equivalent_tlists(), tlists_are_equivalent(), etc.
(I'm not caring for the emphasis on the exprs either, because I think
it'll also be necessary to compare resjunk fields for instance.)
The fields which cannot be compared are resname, resorigtbl, resorigcol as these gets cleared in planner.
I am not sure about fields resno and ressortgroupref, but I will check in more detail before sending patch.
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Saturday, February 09, 2013 9:03 AM Tom Lane wrote:
Amit kapila <amit.kapila@huawei.com> writes:
if (!is_projection_capable_plan(result_plan) &&
compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
Sorry, the check I suggested in last mail should be as below:
if (!is_projection_capable_plan(result_plan) &&
!compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
You know, I was thinking that compare_tlist_exprs() was a pretty
unhelpfully-chosen name for a function returning boolean, and this
thinko pretty much proves the point. It'd be better to call it
something like equivalent_tlists(), tlists_are_equivalent(), etc.
(I'm not caring for the emphasis on the exprs either, because I think
it'll also be necessary to compare resjunk fields for instance.)
We cannot directly compare expressions in target list as even if expressions
are equal, below node (ex. APPEND) will
not do projection, and hence expr will not be evaluated.
So I have checked if both expr's are Var and have same varattno. I have
included tests of expr and resjunk in equivalent_tlists().
I am not able see the need of comparison for resno and ressortgroupref, so I
have not include the check for them in equivalent_tlists().
Updated patch is attached with this mail.
Conclusion of Test
---------------------
1. The performance improvement is very good (upto 60%) for table with 30
columns
2. The performance improvement is good (upto 36%) even when table contains 2
columns
Configuration
------------------
Os - Suse -11.2
CPU - Intel(R) Xeon(R) L5408 @ 2.13GHz
Ram - 24G
shared_buffers - 4GB
Test Data
-----------
30 Columns
----------
CREATE TABLE tbl_parent (c01 int, c02 numeric, c03 int, c04 int, c05 int,
c06 int, c07 int, c08 int, c09 int, c10 int,
c11 numeric, c12 numeric, c13 numeric, c14 numeric, c15
numeric, c16 numeric, c17 numeric, c18 numeric, c19 numeric, c20 numeric,
c21 int, c22 numeric, c23 int, c24 int, c25 int, c26
int, c27 int, c28 int, c29 int, c30 int);
CREATE TABLE tbl_child () INHERITS(tbl_parent);
INSERT INTO tbl_child (SELECT n, floor(random() * 10000), n+2, n+3, n+4,
n+5, n+6, n+7, n+8, n+9,
n+10, n+11, n+12, n+13, n+14, n+15, n+16, n+17,
n+18, n+19,
n+20, n+21, n+22, n+23, n+24, n+25, n+26, n+27,
n+28, n+29 FROM generate_series(0, 10000000 - 1) n);
EXPLAIN ANALYZE SELECT * FROM tbl_parent;
Original -
9995.1994 ms
Patched -
3817.268 ms (~61%)
10 Columns
--------------
CREATE TABLE tbl_parent (c01 int, c02 numeric, c03 int, c04 int, c05 int,
c06 numeric, c07 numeric, c08 numeric, c09 numeric, c10 numeric);
CREATE TABLE tbl_child () INHERITS(tbl_parent);
INSERT INTO tbl_child (SELECT n, floor(random() * 10000), n+2, n+3, n+4,
n+5, n+6, n+7, n+8, n+9 FROM generate_series(0, 10000000 - 1) n);
EXPLAIN ANALYZE SELECT * FROM tbl_parent;
Original -
6404.2992 ms
Patched -
3374.2356 ms (~47%)
2 Columns
---------
CREATE TABLE tbl_parent (c01 numeric, c02 int);
CREATE TABLE tbl_child () INHERITS(tbl_parent);
INSERT INTO tbl_child (SELECT floor(random() * 10000), n FROM
generate_series(0, 10000000 - 1) n);
EXPLAIN ANALYZE SELECT * FROM tbl_parent;
Original -
4952.5758 ms
Patched -
3162.2286 ms (~36%)
With Regards,
Amit Kapila.