Hi, On 2025-12-15 21:56:12 +0900, Amit Langote wrote: > On Thu, Dec 11, 2025 at 12:41 AM Robert Haas <[email protected]> wrote: > > On Tue, Dec 9, 2025 at 6:08 PM Andres Freund <[email protected]> wrote: > > > On 2025-12-09 16:40:17 -0500, Robert Haas wrote: > > > > On Fri, Aug 29, 2025 at 4:38 AM Julien Tachoires <[email protected]> > > > > wrote: > > > > Potentially, there could be a performance problem > > > > > > I think the big performance hazard with this is repeated deforming. The > > > scankey infrastructure deforms attributes one-by-one *and* it does not > > > "persist" the work of deforming for later accesses. So if you e.g. have > > > something like > > > > > > SELECT sum(col_29) FROM tbl WHERE col_30 = common_value; > > > or > > > SELECT * FROM tbl WHERE col_30 = common_value; > > > > > > we'll now deform col_30 in isolation for the ScanKey evaluation and then > > > we'll > > > deform columns 1-29 in the slot (because we always deform all the leading > > > columns), during projection. > > > > Hmm, this is a good point, and I agree that it's a huge challenge for > > this patch set. Repeated tuple deforming is REALLY expensive, which is > > why we've spent so much energy trying to use slots in an as many > > places as possible. I find it easy to believe that HeapKeyTest's loop > > over heap_getattr() is going to prohibitively painful and that this > > code will need to somehow also be slot-ified for this to be a viable > > project. > > > > > I don't really see this being viable without first tackling two nontrivial > > > projects: > > > > > > 1) Make slot deforming for expressions & projections selective, i.e. don't > > > deform all the leading columns, but only ones that will eventually be > > > needed > > > 2) Perform ScanKey evaluation in slot form, to be able to cache the > > > deforming > > > and to make deforming of multiple columns sufficiently efficient. > > > > IOW, I agree that we probably need to do #2. I am not entirely sure > > about #1. > > I'm also curious to understand why Andres sees #1 as a prerequisite > for qual pushdown.
I suspect you'll not see a whole lot of gain without it. When I experimented with it, a good portion (but not all!) of the gain seemed to be from just deforming the immediately required columns - but also a lot of the visible regressions were from that. > > We still hold a pin, though, which I think means very little can > > change. More items can be added to the page, so we might want to > > refresh the number of items on the page at least when we think we're > > done, but I believe that any sort of more invasive page rearrangement > > would be precluded by the pin. > > > > I kind of wonder if it would be good to make a change along the lines > > of v4-0001 even if this patch set doesn't move forward overall, or > > will need a lot of slot-ification to do so. It seems weird to me that > > we're OK with calling out to arbitrary code with a buffer lock held, > > and even weirder that whether or not we do that depends on whether > > SO_ALLOW_PAGEMODE was set. I don't think a difference of this kind > > between pagemode behavior and non-pagemode behavior would survive > > review if someone proposed it today; the fact that it works the way it > > does is probably an artifact of this mechanism having been added > > twenty years ago when the project was in a very different place. > > One maybe crazy thought: what about only enabling qual pushdown when > pagemode is used, since it already processes all tuples on a page in > one locked phase? That raises the question of whether there's a class > of quals simple enough (built-in ops?) that evaluating them alongside > visibility checking would be acceptable, with lock held that is -- but > it would avoid the lock churn and racy loop termination issues with > v4-0001. I think that's the wrong direction to go. We shouldn't do more under the lock, we should be less. You certainly couldn't just use builtin ops, as some of them *do* other catalog lookups, which would lead to deadlock potential. I think it's also just unnecessary to try to do anything under a lock here, it's not hard to first do all the visibility checks while locked and then, after unlocking, filter the tuples based on quals. Greetings, Andres Freund
