On 28/01/15 09:41, Kyotaro HORIGUCHI wrote:
As an issue related to size esmation, I got a explain result as
following,
=# explain (analyze on, buffers on) select a from t1 tablesample system(10) where
a < 50000;
QUERY PLAN
--------------------------------------------------------------------------------
Sample Scan on t1 (cost=0.00..301.00 rows=10000 width=4) (actual
time=0.015..2
.920 rows=4294 loops=1)
Filter: (a < 50000)
Rows Removed by Filter: 5876
Buffers: shared hit=45
actual rows is large as twice as the estimated. tsm_system_cost
estimates the number of the result rows using baserel->tuples,
not using baserel->rows so it doesn't reflect the scan quals. Did
the code come from some other intention?
No, that's a bug.
4.
SampleNext()
a.
Isn't it better to code SampleNext() similar to SeqNext() and
IndexNext(), which encapsulate the logic to get the tuple in
another function(tbs_next() or something like that)?
Possibly, my thinking was that unlike the index_getnext() and
heap_getnext(), this function would not be called from any other
place so adding one more layer of abstraction didn't seem useful.
And it would mean creating new ScanDesc struct, etc.
I think adding additional abstraction would simplify the function
and reduce the chance of discrepency, I think we need to almost
create a duplicate version of code for heapgetpage() method.
Yeah, I agree that we need to build structure like ScanDesc, but
still it will be worth to keep code consistent.
Well similar, not same as we are not always fetching whole page or doing
visibility checks on all tuples in the page, etc. But I don't see why it
can't be inside nodeSamplescan. If you look at bitmap heap scan, that
one is also essentially somewhat modified sequential scan and does
everything within the node nodeBitmapHeapscan because the algorithm is
not used anywhere else, just like sample scan.
As a general opinion, I'll vote for Amit's comment, since three
or four similar instances seems to be a enough reason to abstract
it. On the other hand, since the scan processes are distributed
in ExecProcNode by the nodeTag of scan nodes, reunioning of the
control by abstraction layer after that could be said to
introducing useless annoyance. In short, bastraction at the level
of *Next() would bring the necessity of additional changes around
the execution node distribution.
Yes, that's my view too. I would generally be for that change also and
it would be worth it if the code was used in more than one place, but as
it is it seems like it will just add code/complexity for no real
benefit. It would make sense in case we used sequential scan node
instead of the new node as Amit also suggested, but I remain unconvinced
that mixing sampling and sequential scan into single scan node would be
a good idea.
How will it get smoothen for cases when let us say
more than 50% of tuples are not visible. I think its
due to Postgres non-overwritting storage architecture
that we maintain multiple version of rows in same heap,
otherwise for different kind of architecture (like mysql/oracle)
where multiple row versions are not maintained in same
heap, the same function (with same percentage) can return
entirely different number of rows.
I don't know how else to explain, if we loop over every tuple in the
relation and return it with equal probability then visibility checks
don't matter as the percentage of visible tuples will be same in the
result as in the relation.
Surely it migh yield the effectively the same result. Even so,
this code needs exaplation comment about the nature aroud the
code, or write code as MMVC people won't get confused, I suppose.
Yes it does, but as I am failing to explain even here, it's not clear to
me what to write there. From my point of view it's just effect of the
essential property of bernoulli sampling which is that every element has
equal probability of being included in the sample. It comes from the
fact that we do bernoulli trial (in the code it's the while
(sampler_random_fract(sampler->randstate) > probability) loop) on every
individual tuple.
This means that the ratio of visible and invisible tuples should be same
in the sample as it is in the relation. We then just skip the invisible
tuples and get the correct percentage of the visible ones (this has
performance benefit of not having to do visibility check on all tuples).
If that wasn't true than the bernoulli sampling would just simply not
work as intended as the same property is the reason why it's used in
statistics - the trends should look the same in sample as they look in
the source population.
Obviously there is some variation in practice as we don't have perfect
random generator but that's independent of the algorithm.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
