Re: Declarative partitioning

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Ildar Musin <i(dot)musin(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Declarative partitioning
Date: 2016-07-04 12:31:05
Message-ID: CAFjFpRdthtY81Q0JkXP=Ou8_69_b_FZMYdM0mHpAPQG7JDb1qw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amit,
I observed that the ChangeVarNodes call at line 1229 in
get_relation_constraints() (code below) changes the varno in the cached
copy of partition key expression, which is undesirable. The reason for this
seems to be that the RelationGetPartitionCheckQual() returns the copy of
partition key expression directly from the cache. This is mostly because
get_check_qual_for_range() directly works on cached copy of partition key
expressions, which it should never.

1223 /* Append partition check quals, if any */
1224 pcqual = RelationGetPartitionCheckQual(relation);
1225 if (pcqual)
1226 {
1227 /* Fix Vars to have the desired varno */
1228 if (varno != 1)
1229 ChangeVarNodes((Node *) pcqual, 1, varno, 0);
1230
1231 result = list_concat(result, pcqual);
1232 }

Because of this, the first time through the partition key expressions are
valid, but then onwards they are restamped with the varno of the first
partition.

Please add testcases to your patch to catch such types of issues.

On Mon, Jun 27, 2016 at 3:56 PM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
> wrote:

>
> Hi Ashutosh,
>
> On 2016/06/24 23:08, Ashutosh Bapat wrote:
> > Hi Amit,
> > I tried creating 2-level partitioned table and tried to create simple
> table
> > using CTAS from the partitioned table. It gives a cache lookup error.
> > Here's the test
> > CREATE TABLE pt1_l (a int, b varchar, c int) PARTITION BY RANGE(a);
> > CREATE TABLE pt1_l_p1 PARTITION OF pt1_l FOR VALUES START (1) END (250)
> > INCLUSIVE PARTITION BY RANGE(b);
> > CREATE TABLE pt1_l_p2 PARTITION OF pt1_l FOR VALUES START (251) END (500)
> > INCLUSIVE PARTITION BY RANGE(((a+c)/2));
> > CREATE TABLE pt1_l_p3 PARTITION OF pt1_l FOR VALUES START (501) END (600)
> > INCLUSIVE PARTITION BY RANGE(c);
> > CREATE TABLE pt1_l_p1_p1 PARTITION OF pt1_l_p1 FOR VALUES START
> ('000001')
> > END ('000125') INCLUSIVE;
> > CREATE TABLE pt1_l_p1_p2 PARTITION OF pt1_l_p1 FOR VALUES START
> ('000126')
> > END ('000250') INCLUSIVE;
> > CREATE TABLE pt1_l_p2_p1 PARTITION OF pt1_l_p2 FOR VALUES START (251) END
> > (375) INCLUSIVE;
> > CREATE TABLE pt1_l_p2_p2 PARTITION OF pt1_l_p2 FOR VALUES START (376) END
> > (500) INCLUSIVE;
> > CREATE TABLE pt1_l_p3_p1 PARTITION OF pt1_l_p3 FOR VALUES START (501) END
> > (550) INCLUSIVE;
> > CREATE TABLE pt1_l_p3_p2 PARTITION OF pt1_l_p3 FOR VALUES START (551) END
> > (600) INCLUSIVE;
> > INSERT INTO pt1_l SELECT i, to_char(i, 'FM000000'), i FROM
> > generate_series(1, 600, 2) i;
> > CREATE TABLE upt1_l AS SELECT * FROM pt1_l;
> >
> > The last statement gives error "ERROR: cache lookup failed for function
> > 0". Let me know if this problem is reproducible.
>
> Thanks for the test case. I can reproduce the error. It has to do with
> partition key column (b) being of type varchar whereas the default
> operator family for the type being text_ops and there not being an
> =(varchar, varchar) operator in text_ops.
>
> When creating a plan for SELECT * FROM pt1_l, which is an Append plan,
> partition check quals are generated internally to be used for constraint
> exclusion - such as, b >= '000001' AND b < '000125'. Individual OpExpr's
> are generated by using information from PartitionKey of the parent
> (PartitionKey) and pg_partition entry of the partition. When choosing the
> operator to use for =, >=, <, etc., opfamily and typid of corresponding
> columns are referred. As mentioned above, in this case, they happened to
> be text_ops and varchar, respectively, for column b. There doesn't exist
> an operator =(varchar, varchar) in text_ops, so InvalidOid is returned by
> get_opfamily_member which goes unchecked until the error in question
> occurs.
>
> I have tried to fix this in attached updated patch by using operator class
> input type for operator resolution in cases where column type didn't help.
>
> Thanks,
> Amit
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2016-07-04 12:47:15 Re: Reviewing freeze map code
Previous Message Andrew Borodin 2016-07-04 11:22:37 Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]