(2018/07/12 13:38), Ashutosh Bapat wrote:
On Thu, Jul 12, 2018 at 9:02 AM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
(2018/07/11 20:02), Ashutosh Bapat wrote:
On Wed, Jul 11, 2018 at 1:23 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
Actually, even if we could create such an index on the child table and
the
targetlist had the ConvertRowtypeExpr, the planner would still not be
able
to use an index-only scan with that index; because check_index_only would
not consider that an index-only scan is possible for that index because
that
index is an expression index and that function currently does not
consider
that index expressions are able to be returned back in an index-only
scan.
That behavior of the planner might be improved in future, though.
Right and when we do so, not having ConvertRowtypeExpr in the
targetlist will be a problem.
Yeah, but I don't think that that's unsolvable; because in that case the CRE
as an index expression could be converted back to the whole-row Var's
rowtype by adding another CRE to the index expression for that conversion, I
suspect that that special handling could allow us to support an index-only
scan even when having the whole-row Var instead of the CRE in the
targetlist.
I am not able to understand this. Can you please provide an example?
Sorry, my explanation was not good. Let me try. We can't create an
inherited index on the whole-row reference in a partitioned table, but
actually, we can directly create the corresponding index on a child table:
postgres=# create table pt (a int, b text) partition by list (a);
CREATE TABLE
postgres=# create table ptp1 (b text, a int check (a in (1)));
CREATE TABLE
postgres=# alter table pt attach partition ptp1 for values in (1);
ALTER TABLE
postgres=# create index ptp1_conv_wr_idx on ptp1 ((ptp1::pt));
CREATE INDEX
postgres=# insert into pt values (1, 'foo');
INSERT 0 1
postgres=# select *, ptp1 as wr, ptp1::pt as conv_wr from ptp1;
b | a | wr | conv_wr
-----+---+---------+---------
foo | 1 | (foo,1) | (1,foo)
(1 row)
In this example, the value of the whole-row reference to the child table
ptp1 for that record is ('foo',1), and that of the index expression for
that record is (1,'foo'). Those have different column orders, but the
latter could be mapped to the former by a technique like
do_convert_tuple. So, what I suspect is: by producing the value of the
whole-row reference from that of the index expression like that, we
could support index-only scans with such an index in the case where we
have the whole-row reference in the targetlist, not the index expression
itself.
(Having said that, I'm not 100% sure we need to solve that
problem when we improve the planner, because there doesn't seem to me to be
enough use-case to justify making the code complicated for that.) Anyway, I
think that that would be a matter of future versions of PG.
I am afraid that a fix which just works only till we change the other
parts of the system is useful only till that time. At that time, it
needs to be replaced with a different fix.
I agree on that point.
If that time is long
enough, that's ok. In this case, I agree that if we haven't heard from
users till now that they need to create indexes on whole-row
expressions, there's chance that we will never implement this feature.
I don't object to adding that feature to future versions of PG if required.
So, I think it is safe to have whole-row
Vars instead of ConvertRowtypeExprs in the targetlist.
when it's looking for an expression, it finds a whole-row expression
so it think it needs to add a ConvertRowtypeExpr on that. But when the
plan is created, there is ConvertRowtypeExpr already, but there is no
way to know that a new ConvertRowtypeExpr is not needed anymore. So,
we may have two ConvertRowtypeExprs giving wrong results.
Maybe I'm missing something, but I don't think that we need to worry about
that, because in the approach I proposed, we only add CREs above whole-row
Vars in the targetlists for subplans of an Append/MergeAppend for a
partitioned relation at plan creation time.
There's a patch in an adjacent thread started by David Rowley to rip
out Append/MergeAppend when there is only one subplan. So, your
solution won't work there.
Thanks for sharing that information! I skimmed the thread. I haven't
yet caught up with all the discussions there, so I think I'm missing
something, but it looks like that we haven't yet reached any consensus
on the way to go. In my opinion, I like the approach mentioned in [1].
And if we go that way, my patch seems to fit into that, because in
that approach the Append/MergeAppend could be removed after adjusting
the targetlists for its subplans in
create_append_plan/create_merge_append_plan. Anyway, I'd like to join
in that work for PG12.
Best regards,
Etsuro Fujita
[1] https://www.postgresql.org/message-id/1867.1509032831%40sss.pgh.pa.us
