Hi, Alexander! d4378c0005e61b1bb7
On Fri, 28 Mar 2025 at 03:18, Alexander Korotkov <aekorot...@gmail.com> wrote: > > Hi! > > On Mon, Mar 24, 2025 at 2:46 PM Alena Rybakina > <a.rybak...@postgrespro.ru> wrote: > > I agree with Andrey's changes and think we should fix this, because > > otherwise it might be inconvenient. > > For example, without this changes we will have to have different test > > output files for the same query for different versions of Postres in > > extensions if the whole change is only related to the order of column > > output for a transformation that was not applied. > > I agree with problem spotted by Andrei: it should be preferred to > preserve original order of clauses as much as possible. The approach > implemented in Andrei's patch seems fragile for me. Original order is > preserved if we didn't find any group. But once we find a single > group original order might be destroyed completely. > > The attached patch changes the reordering algorithm of > group_similar_or_args() in the following way. We reorder each group > of similar clauses so that the first item of the group stays in place, > but all the other items are moved after it. So, if there are no > similar clauses, the order of clauses stays the same. When there are > some groups, only required reordering happens while the rest of the > clauses remain in their places. With your patch, I've re-checked that there are no changes in the order of evaluation in plans compared to d4378c0005e61b1bb7 It might be good to also include Andrei's test from his last patch. i.e: +-- No OR-clause groupings should happen - no clause permutations in +-- the filtering conditions we should see in the EXPLAIN. +EXPLAIN (COSTS OFF) +SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2; + +-- OR clauses on the 'unique' column is grouped. So, clause permutation happened +-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths: index scan on +-- the 'hundred' column occupies the first position. +EXPLAIN (COSTS OFF) +SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2; I propose small changes for comments: s/To have this property,/To do so,/g s/in place, but all the/in place, and all the/g s/some groups, only/some groups,/g s/Resort/Re-sort/п The patch overall looks good to me. Regards, Pavel Borisov Supabase
