Re: posgres 12 bug (partitioned table)

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Soumyadeep Chakraborty <soumyadeep2007(at)gmail(dot)com>
Cc: Pavel Biryukov <79166341370(at)yandex(dot)ru>, David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Re: posgres 12 bug (partitioned table)
Date: 2020-07-08 02:16:54
Message-ID: CA+HiwqF3xqwQbtqrRYO5c+0HvG69tOmdnbyC=G0cvqZ9J0SgHA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hi Soumyadeep,

On Wed, Jul 8, 2020 at 9:37 AM Soumyadeep Chakraborty
<soumyadeep2007(at)gmail(dot)com> wrote:
> On Tue, Jul 7, 2020 at 7:18 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > If I change this to return a "heap" slot for partitioned tables, just
> > like for foreign tables, the problem goes away (see the attached). In
> > fact, even make check-world passes, so I don't know why it isn't that
> > way to begin with.
>
> This is what I had thought of initially but I had taken a step back for 2
> reasons:
>
> 1. It is not mandatory for an AM to supply a heap tuple in the slot
> passed to any AM routine. With your patch, the slot passed to
> table_tuple_insert() inside ExecInsert() for instance is now expected to
> supply a heap tuple for the subsequent call to ExecProcessReturning().
> This can lead to garbage values for xmin, xmax, cmin and cmax. I tried
> applying your patch on Zedstore [1], a columnar AM, and consequently, I
> got incorrect values for xmin, xmax etc with the query reported in this
> issue.

Ah, I see. You might've noticed that ExecInsert() only ever sees leaf
partitions, because tuple routing would've switched the result
relation to a leaf partition by the time we are in ExecInsert(). So,
table_tuple_insert() always refers to a leaf partition's AM. Not
sure if you've also noticed but each leaf partition gets to own a slot
(PartitionRoutingInfo.pi_PartitionTupleSlot), but currently it is only
used if the leaf partition attribute numbers are not the same as the
root partitioned table. How about we also use it if the leaf
partition AM's table_slot_callbacks() differs from the root
partitioned table's slot's tts_ops? That would be the case, for
example, if the leaf partition is of Zedstore AM. In the more common
cases where all leaf partitions are of heap AM, this would mean the
original slot would be used as is, that is, if we accept hard-coding
table_slot_callbacks() to return a "heap" slot for partitioned tables
as I suggest.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Pavel Stehule 2020-07-08 04:13:45 Re: Bug: Very poor query optimization by Postgres
Previous Message Soumyadeep Chakraborty 2020-07-08 00:37:17 Re: posgres 12 bug (partitioned table)

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-07-08 02:25:26 Re: [doc] modifying unit from characters to bytes
Previous Message Fujii Masao 2020-07-08 02:15:48 Re: Modifying data type of slot_keep_segs from XLogRecPtr to XLogSegNo