On Tue, Apr 9, 2024 at 7:27 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Peter Geoghegan <p...@bowt.ie> writes: > > On Mon, Apr 8, 2024 at 10:12 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I don't know that I'd call it scary exactly, but I do think it > >> was premature. A week ago there was no consensus that it was > >> ready to commit, but Alexander pushed it (or half of it, anyway) > >> despite that. > > > Some of the most compelling cases for the transformation will involve > > path keys. If the transformation enables the optimizer to build a > > plain index scan (or index-only scan) with useful path keys, then that > > might well lead to a far superior plan compared to what's possible > > with BitmapOrs. > > I did not say it isn't a useful thing to have. I said the patch > did not appear ready to go in. > > > I understand that it'll still be possible to use OR expression > > evaluation in such cases, without applying the transformation (via > > filter quals), so in principle you don't need the transformation to > > get an index scan that can (say) terminate the scan early due to the > > presence of an "ORDER BY ... LIMIT n". But I suspect that that won't > > work out much of the time, because the planner will believe (rightly > > or wrongly) that the filter quals will incur too many heap page > > accesses. > > That's probably related to the fact that we don't have a mechanism > for evaluating non-indexed quals against columns that are retrievable > from the index. We really oughta work on getting that done. But > I've been keeping a side eye on the work to unify plain and index-only > scans, because that's going to touch a lot of the same code so it > doesn't seem profitable to pursue those goals in parallel. > > > Another problem (at least as I see it) with the new > > or_to_any_transform_limit GUC is that its design seems to have nothing > > to say about the importance of these sorts of cases. Most of these > > cases will only have 2 or 3 constants, just because that's what's most > > common in general. > > Yeah, that's one of the reasons I'm dubious that the committed > patch was ready.
While inventing this GUC, I was thinking more about avoiding regressions rather than about unleashing the full power of this optimization. But now I see that that wasn't good enough. And it was definitely hasty to commit to this shape. I apologize for this. Tom, I think you are way more experienced in this codebase than me. And, probably more importantly, more experienced in making decisions for planner development. If you see some way forward to polish this post-commit, Andrei and I are ready to work hard on this with you. If you don't see (or don't think that's good), let's revert this. ------ Regards, Alexander Korotkov
