Re: POC, WIP: OR-clause support for indexes

From: Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Nikolay Shaplov <dhyan(at)nataraj(dot)su>, pgsql-hackers(at)lists(dot)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie>, Marcos Pegoraro <marcos(at)f10(dot)com(dot)br>, teodor(at)sigaev(dot)ru, Peter Eisentraut <peter(at)eisentraut(dot)org>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: 2025-01-13 00:47:56
Message-ID: Mime4j.0.10773454d7da3b8f.1945d21b049@imap.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

<div>Вс, 12 янв. 2025 г. в 21:39, Alexander Korotkov &lt;aekorotkov(at)gmail(dot)com&gt;:<br></div><div><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov &lt;aekorotkov(at)gmail(dot)com&gt; wrote:
<br>&gt; On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina
<br>&gt; &lt;a(dot)rybakina(at)postgrespro(dot)ru&gt; wrote:
<br>&gt; &gt;
<br>&gt; &gt; On 29.11.2024 03:04, Alexander Korotkov wrote:
<br>&gt; &gt; &gt; On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina
<br>&gt; &gt; &gt; &lt;a(dot)rybakina(at)postgrespro(dot)ru&gt; wrote:
<br>&gt; &gt; &gt;&gt; On 28.11.2024 22:28, Ranier Vilela wrote:
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina &lt;a(dot)rybakina(at)postgrespro(dot)ru&gt; escreveu:
<br>&gt; &gt; &gt;&gt;&gt; Hi! Thank you for the case.
<br>&gt; &gt; &gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt; On 28.11.2024 21:00, Alexander Lakhin wrote:
<br>&gt; &gt; &gt;&gt;&gt;&gt; Hello Alexander,
<br>&gt; &gt; &gt;&gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt;&gt; 21.11.2024 09:34, Alexander Korotkov wrote:
<br>&gt; &gt; &gt;&gt;&gt;&gt;&gt; I'm going to push this if no objections.
<br>&gt; &gt; &gt;&gt;&gt;&gt; Please look at the following query, which triggers an error after
<br>&gt; &gt; &gt;&gt;&gt;&gt; ae4569161:
<br>&gt; &gt; &gt;&gt;&gt;&gt; SET random_page_cost = 1;
<br>&gt; &gt; &gt;&gt;&gt;&gt; CREATE TABLE tbl(u UUID);
<br>&gt; &gt; &gt;&gt;&gt;&gt; CREATE INDEX idx ON tbl USING HASH (u);
<br>&gt; &gt; &gt;&gt;&gt;&gt; SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000' OR
<br>&gt; &gt; &gt;&gt;&gt;&gt; u = '11111111111111111111111111111111';
<br>&gt; &gt; &gt;&gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt;&gt; ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed
<br>&gt; &gt; &gt;&gt;&gt;&gt; LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625
<br>&gt; &gt; &gt;&gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt; I found out what the problem is index scan method was not generated. We
<br>&gt; &gt; &gt;&gt;&gt; need to check this during OR clauses for SAOP transformation.
<br>&gt; &gt; &gt;&gt;&gt;
<br>&gt; &gt; &gt;&gt;&gt; There is a patch to fix this problem.
<br>&gt; &gt; &gt;&gt; Hi.
<br>&gt; &gt; &gt;&gt; Thanks for the quick fix.
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; But I wonder if it is not possible to avoid all if the index is useless?
<br>&gt; &gt; &gt;&gt; Maybe moving your fix to the beginning of the function?
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; diff --git a/src/backend/optimizer/path/indxpath.<wbr>c b/src/backend/optimizer/path/indxpath.c
<br>&gt; &gt; &gt;&gt; index d827fc9f4d..5ea0b27d01 100644
<br>&gt; &gt; &gt;&gt; --- a/src/backend/optimizer/path/indxpath.c
<br>&gt; &gt; &gt;&gt; +++ b/src/backend/optimizer/path/indxpath.c
<br>&gt; &gt; &gt;&gt; @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root,
<br>&gt; &gt; &gt;&gt; Assert(IsA(orclause, BoolExpr));
<br>&gt; &gt; &gt;&gt; Assert(orclause-&gt;boolop == OR_EXPR);
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; + /* Ignore index if it doesn't support index scans */
<br>&gt; &gt; &gt;&gt; + if(!index-&gt;amsearcharray)
<br>&gt; &gt; &gt;&gt; + return NULL;
<br>&gt; &gt; &gt;&gt; +
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; Agree. I have updated the patch
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; /*
<br>&gt; &gt; &gt;&gt; * Try to convert a list of OR-clauses to a single SAOP expression. Each
<br>&gt; &gt; &gt;&gt; * OR entry must be in the form: (indexkey operator constant) or (constant
<br>&gt; &gt; &gt;&gt;
<br>&gt; &gt; &gt;&gt; The test bug:
<br>&gt; &gt; &gt;&gt; EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = '00000000000000000000000000000000'<wbr> OR u = '11111111111111111111111111111111';
<br>&gt; &gt; &gt;&gt; QUERY PLAN
<br>&gt; &gt; &gt;&gt; --------------------------------<wbr>--------------------------------<wbr>--------------------------------<wbr>----------------------------------
<br>&gt; &gt; &gt;&gt; Aggregate (cost=12.46..12.47 rows=1 width=8)
<br>&gt; &gt; &gt;&gt; -&gt; Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0)
<br>&gt; &gt; &gt;&gt; Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'<wbr>::uuid) OR (u = '11111111-1111-1111-1111-111111111111'<wbr>::uuid))
<br>&gt; &gt; &gt;&gt; -&gt; BitmapOr (cost=2.14..2.14 rows=18 width=0)
<br>&gt; &gt; &gt;&gt; -&gt; Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
<br>&gt; &gt; &gt;&gt; Index Cond: (u = '00000000-0000-0000-0000-000000000000'<wbr>::uuid)
<br>&gt; &gt; &gt;&gt; -&gt; Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0)
<br>&gt; &gt; &gt;&gt; Index Cond: (u = '11111111-1111-1111-1111-111111111111'<wbr>::uuid)
<br>&gt; &gt; &gt;&gt; (8 rows)
<br>&gt; &gt; &gt; I slightly revised the fix and added similar check to
<br>&gt; &gt; &gt; group_similar_or_args(). Could you, please, review that before
<br>&gt; &gt; &gt; commit?
<br>&gt; &gt; &gt;
<br>&gt; &gt; I agree with changes. Thank you!
<br>&gt;
<br>&gt; Andrei, Alena, thank you for the feedback. Pushed!
<br>
<br>I think we should give some more attention to the patch enabling OR to
<br>SAOP transformation for joins (first time posted in [1]). I think we
<br>tried to only work with Const and Param, because we were previously
<br>working during parse stage. So, at that stage if we have the clause
<br>like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we
<br>should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x
<br>=1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the
<br>index matching, we would actually be able to try the both and select
<br>the best.
</blockquote><div dir="auto"><br></div><div dir="auto">But why not “a.x = ANY(1, b.x) OR b.x = ANY(a.x, 2)” ? Looks strange, but correct ))</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;" dir="auto"><br>The revised patch is attached. Most notably it revises
<br>group_similar_or_args() to have the same notion of const-ness as
<br>others. In that function we split potential index key and constant
<br>early to save time on enumerating all possible index keys. But it
<br>appears to be possible to split by relids bitmapsets: index key should
<br>use our relid, while const shouldn't. Other that that, comments,
<br>commit message and naming are revised.
<br>
<br>Links.
<br>1. https://www.postgresql.org/message-<wbr>id/CAPpHfdu9QJ%3DGbua3CUUH2KKG_8urakJTen4JD47PGh9wWP%3DQxQ%40mail.<wbr>gmail.com
<br>
<br>------
<br>Regards,
<br>Alexander Korotkov
<br>Supabase
<br></blockquote></div></div>

Attachment Content-Type Size
unknown_filename text/html 6.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michail Nikolaev 2025-01-13 01:02:00 Issue with markers in isolation tester? Or not?
Previous Message Ajin Cherian 2025-01-13 00:16:41 Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility.