On Mon, Jan 13, 2025 at 2:47 AM Yura Sokolov <y.soko...@postgrespro.ru> wrote: > Вс, 12 янв. 2025 г. в 21:39, Alexander Korotkov <aekorot...@gmail.com>: >> >> On Fri, Nov 29, 2024 at 9:54 AM Alexander Korotkov <aekorot...@gmail.com> >> wrote: >> > On Fri, Nov 29, 2024 at 7:51 AM Alena Rybakina >> > <a.rybak...@postgrespro.ru> wrote: >> > > >> > > On 29.11.2024 03:04, Alexander Korotkov wrote: >> > > > On Thu, Nov 28, 2024 at 9:33 PM Alena Rybakina >> > > > <a.rybak...@postgrespro.ru> wrote: >> > > >> On 28.11.2024 22:28, Ranier Vilela wrote: >> > > >> >> > > >> Em qui., 28 de nov. de 2024 às 16:03, Alena Rybakina >> > > >> <a.rybak...@postgrespro.ru> escreveu: >> > > >>> Hi! Thank you for the case. >> > > >>> >> > > >>> On 28.11.2024 21:00, Alexander Lakhin wrote: >> > > >>>> Hello Alexander, >> > > >>>> >> > > >>>> 21.11.2024 09:34, Alexander Korotkov wrote: >> > > >>>>> I'm going to push this if no objections. >> > > >>>> Please look at the following query, which triggers an error after >> > > >>>> ae4569161: >> > > >>>> SET random_page_cost = 1; >> > > >>>> CREATE TABLE tbl(u UUID); >> > > >>>> CREATE INDEX idx ON tbl USING HASH (u); >> > > >>>> SELECT COUNT(*) FROM tbl WHERE u = >> > > >>>> '00000000000000000000000000000000' OR >> > > >>>> u = '11111111111111111111111111111111'; >> > > >>>> >> > > >>>> ERROR: XX000: ScalarArrayOpExpr index qual found where not allowed >> > > >>>> LOCATION: ExecIndexBuildScanKeys, nodeIndexscan.c:1625 >> > > >>>> >> > > >>>> >> > > >>> I found out what the problem is index scan method was not generated. >> > > >>> We >> > > >>> need to check this during OR clauses for SAOP transformation. >> > > >>> >> > > >>> There is a patch to fix this problem. >> > > >> Hi. >> > > >> Thanks for the quick fix. >> > > >> >> > > >> But I wonder if it is not possible to avoid all if the index is >> > > >> useless? >> > > >> Maybe moving your fix to the beginning of the function? >> > > >> >> > > >> diff --git a/src/backend/optimizer/path/indxpath.c >> > > >> b/src/backend/optimizer/path/indxpath.c >> > > >> index d827fc9f4d..5ea0b27d01 100644 >> > > >> --- a/src/backend/optimizer/path/indxpath.c >> > > >> +++ b/src/backend/optimizer/path/indxpath.c >> > > >> @@ -3248,6 +3248,10 @@ match_orclause_to_indexcol(PlannerInfo *root, >> > > >> Assert(IsA(orclause, BoolExpr)); >> > > >> Assert(orclause->boolop == OR_EXPR); >> > > >> >> > > >> + /* Ignore index if it doesn't support index scans */ >> > > >> + if(!index->amsearcharray) >> > > >> + return NULL; >> > > >> + >> > > >> >> > > >> Agree. I have updated the patch >> > > >> >> > > >> /* >> > > >> * Try to convert a list of OR-clauses to a single SAOP expression. >> > > >> Each >> > > >> * OR entry must be in the form: (indexkey operator constant) or >> > > >> (constant >> > > >> >> > > >> The test bug: >> > > >> EXPLAIN SELECT COUNT(*) FROM tbl WHERE u = >> > > >> '00000000000000000000000000000000' OR u = >> > > >> '11111111111111111111111111111111'; >> > > >> QUERY PLAN >> > > >> ---------------------------------------------------------------------------------------------------------------------------------- >> > > >> Aggregate (cost=12.46..12.47 rows=1 width=8) >> > > >> -> Bitmap Heap Scan on tbl (cost=2.14..12.41 rows=18 width=0) >> > > >> Recheck Cond: ((u = '00000000-0000-0000-0000-000000000000'::uuid) OR >> > > >> (u = '11111111-1111-1111-1111-111111111111'::uuid)) >> > > >> -> BitmapOr (cost=2.14..2.14 rows=18 width=0) >> > > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0) >> > > >> Index Cond: (u = '00000000-0000-0000-0000-000000000000'::uuid) >> > > >> -> Bitmap Index Scan on idx (cost=0.00..1.07 rows=9 width=0) >> > > >> Index Cond: (u = '11111111-1111-1111-1111-111111111111'::uuid) >> > > >> (8 rows) >> > > > I slightly revised the fix and added similar check to >> > > > group_similar_or_args(). Could you, please, review that before >> > > > commit? >> > > > >> > > I agree with changes. Thank you! >> > >> > Andrei, Alena, thank you for the feedback. Pushed! >> >> I think we should give some more attention to the patch enabling OR to >> SAOP transformation for joins (first time posted in [1]). I think we >> tried to only work with Const and Param, because we were previously >> working during parse stage. So, at that stage if we have the clause >> like "a.x = 1 OR a.x = b.x OR b.x = 2", then we don't know if we >> should transform it into "a.x = ANY(1, b.x) OR b.x = 2" or into "a.x >> =1 OR b.x = ANY(a.x, 2)". But if we do the transformation during the >> index matching, we would actually be able to try the both and select >> the best. > > > But why not “a.x = ANY(1, b.x) OR b.x = ANY(a.x, 2)” ? Looks strange, but > correct ))
That could probably work for a parse stage, but as you can check that approach has a lot of other problems. As we do during index matching, that doesn't matter. I just wanted to state that nothing in the current approach prevent us from working the same way for joins. ------ Regards, Alexander Korotkov Supabase
