Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning - Mailing list pgsql-hackers

From David Rowley
Subject Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning
Date
Msg-id CAKJS1f_yfpgPQ0oBe82kKRnTA8kh_nm7f8wPhbxhyNnTdEGG7A@mail.gmail.com
Whole thread Raw
In response to Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
Responses Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning
Re: [Sender Address Forgery]Re: [Sender Address Forgery]Re: [HACKERS]path toward faster partition pruning
List pgsql-hackers
Thanks for addressing that list.

Just one thing to reply on before I look at the updated version:

On 11 January 2018 at 22:52, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/01/10 10:55, David Rowley wrote:
>> One more thing I discovered while troubleshooting a bug Beena reported
>> in the run-time partition pruning patch is that
>> classify_partition_bounding_keys properly does;
>>
>> if (EXPR_MATCHES_PARTKEY(leftop, partattno, partexpr))
>>     constexpr = rightop;
>> else if (EXPR_MATCHES_PARTKEY(rightop, partattno, partexpr))
>>     constexpr = leftop;
>> else
>>     /* Clause not meant for this column. */
>>     continue;
>>
>> for OpExpr clauses, but does not do the same for leftop for the
>> ScalarArrayOpExpr test.
>
> I'm not sure why we'd need to do that?  Does the syntax of clauses that
> use a ScalarArrayOpExpr() allow them to have the partition key on RHS?

No, but there's no test to ensure the leftop matches the partition key.

There's just:

ScalarArrayOpExpr *saop = (ScalarArrayOpExpr *) clause;
Oid saop_op = saop->opno;
Oid saop_opfuncid = saop->opfuncid;
Oid saop_coll = saop->inputcollid;
Node   *leftop = (Node *) linitial(saop->args),
   *rightop = (Node *) lsecond(saop->args);
List   *elem_exprs,
   *elem_clauses;
ListCell *lc1;
bool negated = false;

/*
* In case of NOT IN (..), we get a '<>', which while not
* listed as part of any operator family, we are able to
* handle the same if its negator is indeed a part of the
* partitioning operator family.
*/
if (!op_in_opfamily(saop_op, partopfamily))
{
Oid negator = get_negator(saop_op);
int strategy;
Oid lefttype,
righttype;


if (!OidIsValid(negator))
continue;
get_op_opfamily_properties(negator, partopfamily, false,
   &strategy,
   &lefttype, &righttype);
if (strategy == BTEqualStrategyNumber)
negated = true;
}

Since there's nothing to reject the clause that does not match the
partition key, the IN's left operand might be of any random type, and
may well not be in partopfamily, so when it comes to looking up
get_op_opfamily_properties() you'll hit: elog(ERROR, "operator %u is
not a member of opfamily %u", opno, opfamily);

Still looking at the v17 patch here, but I also don't see a test to
see if the IsBooleanOpfamily(partopfamily) is checking it matches the
partition key.

> Can you point me to the email where Beena reported the problem in question?

https://www.postgresql.org/message-id/CAOG9ApERiop7P=GRkqQKa82AuBKjxN3qVixie3WK4WqQpEjS6g@mail.gmail.com

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


pgsql-hackers by date:

Previous
From: Tatsuro Yamada
Date:
Subject: Minor code improvement to estimate_path_cost_size in postgres_fdw
Next
From: David Rowley
Date:
Subject: Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning