Re: No-op case in ExecEvalConvertRowtype

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, 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-06 23:36:37
Message-ID: 18193.1491521797@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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".

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.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Beena Emerson 2017-04-06 23:38:15 Re: increasing the default WAL segment size
Previous Message Beena Emerson 2017-04-06 23:33:41 Re: increasing the default WAL segment size