Re: POC, WIP: OR-clause support for indexes
От | Alena Rybakina |
---|---|
Тема | Re: POC, WIP: OR-clause support for indexes |
Дата | |
Msg-id | 4b31e52e-2660-aa39-3c58-0c401e43f274@yandex.ru обсуждение исходный текст |
Ответ на | Re: POC, WIP: OR-clause support for indexes (Andrey Lepikhov <a.lepikhov@postgrespro.ru>) |
Ответы |
Re: POC, WIP: OR-clause support for indexes
|
Список | pgsql-hackers |
Hi! Thank you for your detailed review, your changes have greatly helped to improve this patch. On 06.07.2023 13:20, Andrey Lepikhov wrote: > On 6/7/2023 03:06, Alena Rybakina wrote: >>> I corrected this constant in the patch. > The patch don't apply cleanly: it contains some trailing spaces. I fixed it. > > Also, quick glance into the code shows some weak points; > 1. transformBoolExprOr should have input type BoolExpr. Agreed. > 2. You can avoid the switch operator at the beginning of the function, > because you only need one option. Agreed. > 3. Stale comments: RestrictIinfos definitely not exists at this point. Yes, unfortunately, I missed this from the previous version when I tried to perform such a transformation at the index creation stage. > 4. I don't know, you really need to copy the expr or not, but it is > better to do as late, as possible. Yes, I agree with you, copying "expr" is not necessary in this patch > 5. You assume, that leftop is non-constant and rightop - constant. Why? Agreed, It was too presumptuous on my part and I agree with your changes. > 6.I doubt about equivalence operator. Someone can invent a custom '=' > operator with another semantics, than usual. May be better to check > mergejoinability? Yes, I agree with you, and I haven't thought about it before. But I haven't found any functions to arrange this in PostgreSQL, but using mergejoinability turns out to be more beautiful here. > 7. I don't know how to confidently identify constant expressions at > this level. So, I guess, You can only merge here expressions like > "F(X)=Const", not an 'F(X)=ConstExpression'. I see, you can find solution for this case, thank you for this, and I think it's reliable enough. On 07.07.2023 05:43, Andrey Lepikhov wrote: > On 6/7/2023 03:06, Alena Rybakina wrote: >>> The test was performed on the same benchmark database generated by 2 >>> billion values. >>> >>> I corrected this constant in the patch. > In attempt to resolve some issues had mentioned in my previous letter > I used op_mergejoinable to detect mergejoinability of a clause. > Constant side of the expression is detected by call of > eval_const_expressions() and check each side on the Const type of node. > > See 'diff to diff' in attachment. I notices you remove condition for checking equal operation. strcmp(strVal(linitial((arg)->name)), "=") == 0 Firstly, it is noticed me not correct, but a simple example convinced me otherwise: postgres=# explain analyze select x from a where x=1 or x>5 or x<3 or x=2; QUERY PLAN -------------------------------------------------------------------------------------------------------- Seq Scan on a (cost=0.00..2291.00 rows=97899 width=4) (actual time=0.038..104.168 rows=99000 loops=1) Filter: ((x > '5'::numeric) OR (x < '3'::numeric) OR (x = ANY ('{1,2}'::numeric[]))) Rows Removed by Filter: 1000 Planning Time: 9.938 ms Execution Time: 113.457 ms (5 rows) It surprises me that such check I can write such similar way: eval_const_expressions(NULL, orqual). Yes, I see we can remove this code: bare_orarg = transformExprRecurse(pstate, (Node *)arg); bare_orarg = coerce_to_boolean(pstate, bare_orarg, "OR"); because we will provide similar manipulation in this: foreach(l, gentry->consts) { Node *rexpr = (Node *) lfirst(l); rexpr = coerce_to_common_type(pstate, rexpr, scalar_type, "IN"); aexprs = lappend(aexprs, rexpr); } -- Regards, Alena Rybakina Postgres Professional
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Damir BelyalovДата:
Сообщение: Re: Implement missing join selectivity estimation for range types