No-op case in ExecEvalConvertRowtype

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>
Subject: No-op case in ExecEvalConvertRowtype
Date: 2017-04-06 09:05:52
Message-ID: CAFjFpRfvHABV6+oVvGcshF8rHn+1LfRUhj7Jz1CDZ4gPUwehBg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,
In ExecEvalConvertRowtype(), if the input row doesn't require any
conversion, we simply return that row as is.
2820 /*
2821 * No-op if no conversion needed (not clear this can happen here).
2822 */
2823 if (op->d.convert_rowtype.map == NULL)
2824 return;

If the type of input row is different from that of the output row, the
tree further up would expect typeid in the tuple header to be that of
the output row.

Rajkumar reported a crash while testing with multi-level
partition-wise join with a SELECT clause containing the whole row
reference. In case of multi-level partitioning, per the latest
proposed patch in [1] a whole-row reference on parent translates into
a nested ConvertRowtypeExpr with as many ConvertRowtypeExpr
decorations as there are levels in the partitioning hierarchy. Each
ConvertRowtypeExpr converts a row from lower partition to the row type
of parent. Since the tuples returned by lower ConvertRowtypeExpr were
not stamped with output type, following assertion in the same function
tripped.

2800 Assert(HeapTupleHeaderGetTypeId(tuple) == indesc->tdtypeid ||
2801 HeapTupleHeaderGetTypeId(tuple) == RECORDOID);

I tried to create a testcase where this assertion would fail without
multi-level partitioned table, but could not construct one. But I
think this looks like a bug.

PFA the patch to fix this issue by copying the tuple with the output
type stamped in tuple header. Since the tuple could be an on-disk
tuple, it looks like copying is inevitable. But I am not sure if this
is the right fix or if there is any other way to avoid copying.
Comments are welcome.

[1] https://www.postgresql.org/message-id/CAFjFpRfBipiEzP739PFDrzxrWG-UKThHj3tyHkwCeE2hDwP78A%40mail.gmail.com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
ConvertRowtypeNoop.patch application/octet-stream 1.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kuntal Ghosh 2017-04-06 09:34:13 Re: strange parallel query behavior after OOM crashes
Previous Message Jeff Davis 2017-04-06 08:43:20 Range Merge Join v1