From: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [Sender Address Forgery]Re: [HACKERS] path toward faster partition pruning |
Date: | 2018-01-11 10:23:30 |
Message-ID: | CAKJS1f_yfpgPQ0oBe82kKRnTA8kh_nm7f8wPhbxhyNnTdEGG7A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(at)lab(dot)ntt(dot)co(dot)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?
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-01-11 10:40:04 | Re: [Sender Address Forgery]Re: [HACKERS] path toward faster partition pruning |
Previous Message | Tatsuro Yamada | 2018-01-11 10:14:33 | Minor code improvement to estimate_path_cost_size in postgres_fdw |