From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com> |
Subject: | Re: No-op case in ExecEvalConvertRowtype |
Date: | 2017-04-07 01:58:59 |
Message-ID: | e929bc34-f562-aded-ac13-7e1efa545995@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2017/04/07 8:36, Tom Lane wrote:
> I wrote:
>> I propose to deal with this by reverting 3838074f8 in toto, and then
>> trying to clarify that comment, and maybe adding a regression test case
>> based on the example I showed earlier so that it will be a little more
>> obvious if someone breaks this again.
>> However, I see that 3838074f8 touches some partitioning code, which
>> makes me wonder if there's anything in the partitioning logic that
>> really depends on this erroneous "optimization".
Definitely misread the comment there, but was mystified why the tests
didn't break. The partitioning tuple-routing code optionally avoids
converting tuples by using this optimization. Since TupleDesc.tdtypeid of
the parent and the partition to which a tuple is routed are never the
same, tuples would always have to be converted before 3838074f8. One of
the earlier versions of that patch introduced a consider_typeid parameter
for which only ExecEvalConvertRowtype() passed true.
> After further poking around, I've concluded that that approach is probably
> an overreaction. Of the dozen or so callers of convert_tuples_by_position
> and convert_tuples_by_name, it seems that ExecEvalConvertRowtype is the
> only one that really needs the correct composite-datum headers in the
> converted tuple; and even for it, forcing use of do_convert_tuple is
> a pretty expensive, brute-force way to get that result. Ashutosh's
> proposal to use heap_copy_tuple_as_datum when no column rearrangement
> is required should be substantially more efficient.
>
> So I now think it's okay to remove consideration of matching the target
> rowtype OID from the tupconvert.c functions, although we have to realize
> that that is effectively an API change for them, one which has a definite
> potential for biting third-party callers.
And I see that just in 3f902354b08 lets the partition tuple-routing code
keep utilizing that optimization.
Thanks,
Amit
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2017-04-07 01:59:40 | Re: Performance improvement for joins where outer side is unique |
Previous Message | Andrew Dunstan | 2017-04-07 01:56:05 | Re: Time to change pg_regress diffs to unified by default? |