On 8/19/23 02:49, Peter Geoghegan wrote:
> On Tue, Aug 8, 2023 at 11:36 AM Peter Geoghegan wrote:
>>> The only thing the patch does is it looks at clauses we decided not to
>>> treat as index quals, and do maybe still evaluate them on index. And I
>>> don't think I want to move these goalposts
Hi,
It took me a while but I finally got back to working on this patch. I
reread the summary of principles Peter shared in August, and I do agree
with all the main points - the overall qual hierarchy and the various
examples and counter-examples.
I'll respond to a couple things in this
On Mon, Aug 7, 2023 at 12:34 PM Tomas Vondra
wrote:
> On 8/7/23 02:38, Peter Geoghegan wrote:
> > This plan switchover isn't surprising in itself -- it's one of the most
> > important issues addressed by my SAOP patch. However, it *is* a little
> > surprising that your patch doesn't even manage
On Tue, Aug 8, 2023 at 11:36 AM Peter Geoghegan wrote:
> > The only thing the patch does is it looks at clauses we decided not to
> > treat as index quals, and do maybe still evaluate them on index. And I
> > don't think I want to move these goalposts much further.
>
> Avoiding the need for
On Thu, Aug 17, 2023 at 4:29 PM Jeff Davis wrote:
> On Wed, 2023-08-09 at 17:14 -0700, Peter Geoghegan wrote:
> > + Index quals are better than equivalent index filters because bitmap
> > index scans can only use index quals
>
> It seems there's consensus that:
>
> * Index Filters (Tomas's
On Wed, 2023-08-09 at 17:14 -0700, Peter Geoghegan wrote:
> + Index quals are better than equivalent index filters because bitmap
> index scans can only use index quals
It seems there's consensus that:
* Index Filters (Tomas's patch and the topic of this thread) are more
general, because they
On Wed, Aug 9, 2023 at 11:15 AM Tomas Vondra
wrote:
> Cool. I'll try to build my own set of examples that I find interesting
> either because it's what the patch aims to help with, or because I
> expect it to be problematic for some reason. And then we can compare.
That would be great. I
On 8/9/23 19:53, Peter Geoghegan wrote:
> On Wed, Aug 9, 2023 at 10:00 AM Tomas Vondra
> wrote:
>> Anyway, I find this discussion rather abstract and I'll probably forget
>> half the important cases by next week. Maybe it'd be good to build a set
>> of examples demonstrating the interesting
On Wed, Aug 9, 2023 at 10:00 AM Tomas Vondra
wrote:
> Anyway, I find this discussion rather abstract and I'll probably forget
> half the important cases by next week. Maybe it'd be good to build a set
> of examples demonstrating the interesting cases? We've already used a
> couple tenk1 queries
On 8/8/23 22:54, Peter Geoghegan wrote:
> On Tue, Aug 8, 2023 at 1:24 PM Tomas Vondra
> wrote:
>>> Assuming that that happens, then it immediately gives index scans a
>>> huge advantage over bitmap index scans. At that point it seems
>>> important to describe (in high level terms) where it is
On Wed, Aug 9, 2023 at 9:05 AM Tomas Vondra
wrote:
> But in the example you shared yesterday, the problem is not really about
> visibility checks. In fact, the index scan costing completely ignores
> the VM checks - it didn't matter before, and the patch did not change
> this. It's about the
On 8/8/23 23:03, Peter Geoghegan wrote:
> On Tue, Aug 8, 2023 at 1:49 PM Tomas Vondra
> wrote:
>> So we expect 1250 rows. If that was accurate, the index scan would have
>> to do 1250 heap fetches. It's just luck the index scan doesn't need to
>> do that. I don't this there's a chance to
On Tue, Aug 8, 2023 at 1:49 PM Tomas Vondra
wrote:
> So we expect 1250 rows. If that was accurate, the index scan would have
> to do 1250 heap fetches. It's just luck the index scan doesn't need to
> do that. I don't this there's a chance to improve this costing - if the
> inputs are this off, it
On Tue, Aug 8, 2023 at 1:24 PM Tomas Vondra
wrote:
> > Assuming that that happens, then it immediately gives index scans a
> > huge advantage over bitmap index scans. At that point it seems
> > important to describe (in high level terms) where it is that the
> > advantage is innate, and where
On 8/8/23 21:15, Peter Geoghegan wrote:
> On Tue, Aug 8, 2023 at 11:36 AM Peter Geoghegan wrote:
>> Assuming that that happens, then it immediately gives index scans a
>> huge advantage over bitmap index scans. At that point it seems
>> important to describe (in high level terms) where it is that
On 8/8/23 20:36, Peter Geoghegan wrote:
> On Tue, Aug 8, 2023 at 11:14 AM Tomas Vondra
> wrote:
>> I agree this patch shouldn't make it harder to improve these cases, but
>> TBH I don't quite see which part of the patch would do that? Which bit
>> are you objecting to? If we decide to change
On Tue, Aug 8, 2023 at 11:36 AM Peter Geoghegan wrote:
> Assuming that that happens, then it immediately gives index scans a
> huge advantage over bitmap index scans. At that point it seems
> important to describe (in high level terms) where it is that the
> advantage is innate, and where it's
On Tue, Aug 8, 2023 at 11:14 AM Tomas Vondra
wrote:
> I agree this patch shouldn't make it harder to improve these cases, but
> TBH I don't quite see which part of the patch would do that? Which bit
> are you objecting to? If we decide to change how match_clause_to_index()
> deals with these
On 8/8/23 19:43, Peter Geoghegan wrote:
> On Mon, Aug 7, 2023 at 9:21 PM Peter Geoghegan wrote:
>> I see that get_op_btree_interpretation() will treat != as a kind of
>> honorary member of an opfamily whose = operator has our != operator as
>> its negator. Perhaps we should be finding a way to
On Mon, Aug 7, 2023 at 9:21 PM Peter Geoghegan wrote:
> I see that get_op_btree_interpretation() will treat != as a kind of
> honorary member of an opfamily whose = operator has our != operator as
> its negator. Perhaps we should be finding a way to pass != quals into
> the index AM so that they
On Tue, Aug 8, 2023 at 7:28 AM Tomas Vondra
wrote:
> Are you sure? Because if I try with the 20230716 patch, I get this plan
> (after disabling bitmapscan):
> So the condition is recognized as index filter. Or did you mean
> something different?
No, I was just wrong about this (and about
On 8/8/23 06:21, Peter Geoghegan wrote:
> On Mon, Aug 7, 2023 at 3:18 PM Peter Geoghegan wrote:
>> Even my patch cannot always make SAOP clauses into index quals. There
>> are specific remaining gaps that I hope that your patch will still
>> cover. The simplest example is a similar NOT IN()
On 8/8/23 00:18, Peter Geoghegan wrote:
> On Mon, Aug 7, 2023 at 12:34 PM Tomas Vondra
> wrote:
>> But then we call get_index_paths/build_index_path a little bit later,
>> and that decides to skip "lower SAOP" (which seems a bit strange,
>> because the column is "after" the equality, but meh).
On Mon, Aug 7, 2023 at 3:18 PM Peter Geoghegan wrote:
> Even my patch cannot always make SAOP clauses into index quals. There
> are specific remaining gaps that I hope that your patch will still
> cover. The simplest example is a similar NOT IN() inequality, like
> this:
>
> select
> ctid, *
>
On Mon, Aug 7, 2023 at 12:34 PM Tomas Vondra
wrote:
> But then we call get_index_paths/build_index_path a little bit later,
> and that decides to skip "lower SAOP" (which seems a bit strange,
> because the column is "after" the equality, but meh). Anyway, at this
> point we already decided what's
On 8/7/23 02:38, Peter Geoghegan wrote:
> On Sun, Aug 6, 2023 at 3:28 PM Peter Geoghegan wrote:
>> I decided to verify my understanding by checking what would happen
>> when I ran the OR-heavy tenk1 regression test query against a
>> combination of your patch, and v7 of the OR-to-SAOP
On Sun, Aug 6, 2023 at 3:28 PM Peter Geoghegan wrote:
> I decided to verify my understanding by checking what would happen
> when I ran the OR-heavy tenk1 regression test query against a
> combination of your patch, and v7 of the OR-to-SAOP transformation
> patch. (To be clear, this is without my
On Sun, Aug 6, 2023 at 1:13 PM Peter Geoghegan wrote:
> Since you're not relying on the nbtree work at all here, really (just
> on the transformation process itself), the strategic risk that this
> adds to your project isn't too great. It's not like this ties the
> success of your patch to the
On Sun, Aug 6, 2023 at 10:23 AM Tomas Vondra
wrote:
> > The index AM is entitled to make certain assumptions of opclass
> > members -- assumptions that cannot be made during expression
> > evaluation.
> Thanks for reminding me, I keep forgetting about this.
I was almost certain that you already
On 8/5/23 02:53, Peter Geoghegan wrote:
> ...
>
>> Right. This however begs a question - why would we actually need to
>> check the visibility map before evaluating the index filter, when the
>> index tuple alone is clearly good enough for the bitmapOr plan?
>>
>> Because if we didn't need to do
On Fri, Aug 4, 2023 at 4:34 PM Tomas Vondra
wrote:
> Thanks for the clarification, I think I understand better now.
Honestly, it's gratifying to be understood at all in a discussion like
this one. Figuring out how to articulate some of my more subtle ideas
(without leaving out important nuances)
On 8/4/23 04:07, Peter Geoghegan wrote:
> On Thu, Aug 3, 2023 at 3:04 PM Tomas Vondra
> wrote:
>> Because my patch is all about reducing the heap pages, which are usually
>> the expensive part of the index scan. But you're right the "index scan"
>> with index filter may access more index pages,
On Fri, Aug 4, 2023 at 4:47 AM Tomas Vondra
wrote:
> Well, I presented multiple options, so "yes" doesn't really clarify
> which of them applies. But my understanding is you meant the index pages
> accesses.
Sorry. Your understanding of what I must have meant before was correct
-- your patch
On 8/4/23 02:08, Peter Geoghegan wrote:
> On Thu, Aug 3, 2023 at 3:04 PM Tomas Vondra
> wrote:
>> When you say "index page accesses" do you mean accesses to index pages,
>> or accesses to heap pages from the index scan?
>
> Yes, that's exactly what I mean. Note that that's the dominant cost
>
On Thu, Aug 3, 2023 at 3:04 PM Tomas Vondra
wrote:
> Because my patch is all about reducing the heap pages, which are usually
> the expensive part of the index scan. But you're right the "index scan"
> with index filter may access more index pages, because it has fewer
> "access predicates".
On Thu, Aug 3, 2023 at 3:04 PM Tomas Vondra
wrote:
> When you say "index page accesses" do you mean accesses to index pages,
> or accesses to heap pages from the index scan?
Yes, that's exactly what I mean. Note that that's the dominant cost
for the original BitmapOr plan.
As I said upthread,
On Thu, Aug 3, 2023 at 2:46 PM Tomas Vondra
wrote:
> Sure, having more choices means a risk of making mistakes. But does
> simply postponing the choices to runtime actually solves this?
It solves that one problem, yes. This is particularly important in
cases where we would otherwise get truly
On 8/3/23 21:21, Peter Geoghegan wrote:
> On Thu, Aug 3, 2023 at 11:17 AM Tomas Vondra
> wrote:
>> Not sure. I'm a bit confused about what exactly is so risky on the plan
>> produced with the patch.
>
> It's all about the worst case. In the scenarios that I'm concerned
> about, we can be
On 8/3/23 20:50, Peter Geoghegan wrote:
> On Thu, Aug 3, 2023 at 4:57 AM Tomas Vondra
> wrote:
>>> I understand that that's how the patch is structured. It is
>>> nevertheless possible (as things stand) that the patch will make the
>>> planner shift from a plan that uses "Access Predicates" to
On Thu, Aug 3, 2023 at 11:17 AM Tomas Vondra
wrote:
> Not sure. I'm a bit confused about what exactly is so risky on the plan
> produced with the patch.
It's all about the worst case. In the scenarios that I'm concerned
about, we can be quite sure that the saving from not using a BitmapOr
will
On Thu, Aug 3, 2023 at 4:57 AM Tomas Vondra
wrote:
> > I understand that that's how the patch is structured. It is
> > nevertheless possible (as things stand) that the patch will make the
> > planner shift from a plan that uses "Access Predicates" to the maximum
> > extent possible when scanning
On 8/3/23 18:47, Peter Geoghegan wrote:
> On Thu, Aug 3, 2023 at 4:20 AM Tomas Vondra
> wrote:
>> Which is just the 7 buffers ...
>>
>> Did I do something wrong?
>
> I think that it might have something to do with your autovacuum
> settings. Note that the plan that you've shown for the master
On Thu, Aug 3, 2023 at 4:20 AM Tomas Vondra
wrote:
> Which is just the 7 buffers ...
>
> Did I do something wrong?
I think that it might have something to do with your autovacuum
settings. Note that the plan that you've shown for the master branch
isn't the same one that appears in
On 8/3/23 03:32, Peter Geoghegan wrote:
> On Wed, Aug 2, 2023 at 6:48 AM Tomas Vondra
> wrote:
>> How come we don't know that until the execution time? Surely when
>> building the paths/plans, we match the clauses to the index keys, no? Or
>> are you saying that just having a scan key is not
On 8/3/23 07:26, Peter Geoghegan wrote:
> On Wed, Aug 2, 2023 at 6:32 PM Peter Geoghegan wrote:
>> I don't dispute the fact that this can only happen when the planner
>> believes (with good reason) that the expected cost will be lower. But
>> I maintain that there is a novel risk to be concerned
On Wed, Aug 2, 2023 at 6:32 PM Peter Geoghegan wrote:
> I don't dispute the fact that this can only happen when the planner
> believes (with good reason) that the expected cost will be lower. But
> I maintain that there is a novel risk to be concerned about, which is
> meaningfully distinct from
On Wed, Aug 2, 2023 at 6:48 AM Tomas Vondra
wrote:
> How come we don't know that until the execution time? Surely when
> building the paths/plans, we match the clauses to the index keys, no? Or
> are you saying that just having a scan key is not enough for it to be
> "access predicate"?
In
On 8/2/23 02:50, Peter Geoghegan wrote:
> On Mon, Jul 24, 2023 at 11:59 AM Peter Geoghegan wrote:
>>> That might be true but I'm not sure what to do about that unless we
>>> incorporate some "robustness" measure into the costing. If every
>>> measure we have says one plan is better, don't we have
On Mon, Jul 24, 2023 at 11:59 AM Peter Geoghegan wrote:
> > That might be true but I'm not sure what to do about that unless we
> > incorporate some "robustness" measure into the costing. If every
> > measure we have says one plan is better, don't we have to choose it?
>
> I'm mostly concerned
On Mon, Jul 24, 2023 at 11:37 AM Jeff Davis wrote:
> I see. You're concerned that lowering the cost of an index scan path
> too much, due to pushing down a clause as an Index Filter, could cause
> it to out-compete a more "robust" plan.
The optimizer correctly determines that 3 index scans (plus
Jeff Davis writes:
> On Mon, 2023-07-24 at 13:58 -0400, Tom Lane wrote:
>> Please do not put in any code that assumes that restriction clause
>> order is preserved, or encourages users to think it is.
> Agreed. I didn't mean to add any extra guarantee of preserving clause
> order; just to follow
On Mon, 2023-07-24 at 10:54 -0700, Peter Geoghegan wrote:
> The case in question shows a cheaper plan replacing a more expensive
> plan -- so it's a win by every conventional measure. But, the new
> plan
> is less robust in the sense that I described yesterday: it will be
> much slower than the
On Mon, 2023-07-24 at 13:58 -0400, Tom Lane wrote:
> Please do not put in any code that assumes that restriction clause
> order is preserved, or encourages users to think it is.
Agreed. I didn't mean to add any extra guarantee of preserving clause
order; just to follow the current way
Peter Geoghegan writes:
> On Mon, Jul 24, 2023 at 10:36 AM Jeff Davis wrote:
>> Could we start out conservatively and push down as an Index Filter
>> unless there is some other clause ahead of it that can't be pushed
>> down? That would allow users to have some control by writing clauses in
>>
On Mon, Jul 24, 2023 at 10:36 AM Jeff Davis wrote:
> I'm getting a bit lost in this discussion as well -- for the purposes
> of this feature, we only need to know whether to push down a clause as
> an Index Filter or not, right?
I think so.
> Could we start out conservatively and push down as
On Sun, 2023-07-23 at 14:04 +0200, Tomas Vondra wrote:
> But I'm getting a bit lost regarding what's the proposed costing
> strategy. It's hard to follow threads spanning days, with various
> other
> distractions, etc.
I'm getting a bit lost in this discussion as well -- for the purposes
of this
On Sun, Jul 23, 2023 at 5:04 AM Tomas Vondra
wrote:
> Sorry, I think I meant 'cost estimates', not the selectivity estimates.
> If you convert the original "simple" clauses into the more complex list,
> presumably we'd cost that differently, right? I may be entirely wrong,
> but my intuition is
On 7/21/23 21:17, Peter Geoghegan wrote:
> ...
>> But I was
>> thinking more about the costing part - if you convert the clauses in
>> some way, does that affect the reliability of estimates somehow?
>
> Obviously, it doesn't affect the selectivity at all. That seems most
> important (you
On Fri, Jul 21, 2023 at 4:52 AM Tomas Vondra
wrote:
> > (Actually, I'm glossing over a lot. The MDAM paper describes
> > techniques that'd make even the really challenging cases safe, through
> > a process of converting quals from conjunctive normal form into
> > disjunctive normal form. This is
On 7/21/23 05:32, Peter Geoghegan wrote:
> On Thu, Jul 20, 2023 at 4:35 AM Tomas Vondra
> wrote:
>> I think the SAOP patch may need to be much more careful about this, but
>> for this patch it's simpler because it doesn't really change any of the
>> index internals, or the indexscan in general.
>
On Thu, Jul 20, 2023 at 4:35 AM Tomas Vondra
wrote:
> I think the SAOP patch may need to be much more careful about this, but
> for this patch it's simpler because it doesn't really change any of the
> index internals, or the indexscan in general.
It's true that the SAOP patch needs relatively
On 7/19/23 23:38, Peter Geoghegan wrote:
> On Wed, Jul 19, 2023 at 1:46 PM Jeff Davis wrote:
>> On Wed, 2023-07-19 at 20:03 +0200, Tomas Vondra wrote:
>>> Makes sense, I also need to think about maybe not having duplicate
>>> clauses in the two lists. What annoys me on that it partially
>>>
On Wed, Jul 19, 2023 at 1:46 PM Jeff Davis wrote:
> On Wed, 2023-07-19 at 20:03 +0200, Tomas Vondra wrote:
> > Makes sense, I also need to think about maybe not having duplicate
> > clauses in the two lists. What annoys me on that it partially
> > prevents
> > the cost-based reordering done by
On Wed, 2023-07-19 at 20:03 +0200, Tomas Vondra wrote:
> Makes sense, I also need to think about maybe not having duplicate
> clauses in the two lists. What annoys me on that it partially
> prevents
> the cost-based reordering done by order_qual_clauses(). So maybe we
> should have three lists ...
On 7/19/23 19:17, Jeff Davis wrote:
> On Wed, 2023-07-19 at 11:16 +0200, Tomas Vondra wrote:
>> I wonder if Andres was right (in the index prefetch thread) that
>> splitting regular index scans and index-only scans may not be ideal.
>> In
>> a way, this patch moves those nodes closer, both in
On Wed, 2023-07-19 at 11:16 +0200, Tomas Vondra wrote:
> I wonder if Andres was right (in the index prefetch thread) that
> splitting regular index scans and index-only scans may not be ideal.
> In
> a way, this patch moves those nodes closer, both in capability and
> code
> (because now both use
On 7/19/23 01:22, Jeff Davis wrote:
> On Wed, 2023-07-19 at 00:36 +0200, Tomas Vondra wrote:
>>> * I'm confused about the relationship of an IOS to an index filter.
>>> It
>>> seems like the index filter only works for an ordinary index scan?
>>> Why
>>> is that?
>>
>> What would it do for IOS?
On Wed, 2023-07-19 at 00:36 +0200, Tomas Vondra wrote:
> > * I'm confused about the relationship of an IOS to an index filter.
> > It
> > seems like the index filter only works for an ordinary index scan?
> > Why
> > is that?
>
> What would it do for IOS?
The way it's presented is slightly
On 7/18/23 22:21, Jeff Davis wrote:
> Hi,
>
>
> On Sun, 2023-07-16 at 22:36 +0200, Tomas Vondra wrote:
>> This kept bothering me, so I looked at it today, and reworked it to
>> use
>> the IOS approach.
>
> Initial comments on patch 20230716:
>
> * check_index_filter() alredy looks at
Hi,
On Sun, 2023-07-16 at 22:36 +0200, Tomas Vondra wrote:
> This kept bothering me, so I looked at it today, and reworked it to
> use
> the IOS approach.
Initial comments on patch 20230716:
* check_index_filter() alredy looks at "canreturn", which should mean
that you don't need to later
On 7/15/23 16:20, Tomas Vondra wrote:
>
> ...
>
> 4) problems with opcintype != opckeytype (name_ops)
>
> While running the tests, I ran into an issue with name_ops, causing
> failures for \dT and other catalog queries. The root cause is that
> name_ops has opcintype = name, but opckeytype =
Hi,
here's a minor update of the patch, rebased to a current master and
addressing a couple issues reported by cfbot. Most are minor tweaks, but
the last one (4) is a somewhat more serious issue.
1) "tid" might have not been initialized in the IndexNext loop
2) add enable_indexonlyfilter GUC
On 6/21/23 18:17, James Coleman wrote:
> On Wed, Jun 21, 2023 at 11:28 AM Tomas Vondra
> wrote:
>>
>>
>>
>> On 6/21/23 14:45, James Coleman wrote:
>>> Hello,
>>>
>>> I've cc'd Jeff Davis on this due to a conversation we had at PGCon
>>> about applying filters on index tuples during index
On Wed, Jun 21, 2023 at 11:28 AM Tomas Vondra
wrote:
>
>
>
> On 6/21/23 14:45, James Coleman wrote:
> > Hello,
> >
> > I've cc'd Jeff Davis on this due to a conversation we had at PGCon
> > about applying filters on index tuples during index scans.
> >
> > I've also cc'd Andres Freund because I
On 6/21/23 14:45, James Coleman wrote:
> Hello,
>
> I've cc'd Jeff Davis on this due to a conversation we had at PGCon
> about applying filters on index tuples during index scans.
>
> I've also cc'd Andres Freund because I think this relates to his
> musing in [1] that:
>> One thing I have
Hello,
I've cc'd Jeff Davis on this due to a conversation we had at PGCon
about applying filters on index tuples during index scans.
I've also cc'd Andres Freund because I think this relates to his
musing in [1] that:
> One thing I have been wondering around this is whether we should not have
>
Hi,
I took a stab at this and implemented the trick with the VM - during
index scan, we also extract the filters that only need the indexed
attributes (just like in IOS). And then, during the execution we:
1) scan the index using the scan keys (as before)
2) if the heap page is all-visible,
> This isn't a problem for operators found in operator families, because
> we trust those to not have undesirable side effects like raising
> data-dependent errors. But it'd be an issue if we started to apply
> quals that aren't index quals directly to index rows before doing
> the heap liveness
On 2/15/23 16:18, Tom Lane wrote:
> Tomas Vondra writes:
>> On 2/15/23 09:57, Maxim Ivanov wrote:
>>> TLDR; additional index column B specified in CREATE INDEX .. (A)
>>> INCLUDE(B) is not used to filter rows in queries like WHERE B = $1
>>> ORDER BY A during IndexScan.
Tomas Vondra writes:
> On 2/15/23 09:57, Maxim Ivanov wrote:
>> TLDR; additional index column B specified in CREATE INDEX .. (A)
>> INCLUDE(B) is not used to filter rows in queries like WHERE B = $1
>> ORDER BY A during IndexScan. https://dbfiddle.uk/iehtq44L
> Seems doable, unless I'm missing
On 2/15/23 09:57, Maxim Ivanov wrote:
> Hi All,
>
> I'd like to report what seems to be a missing optimization
> opportunity or understand why it is not possible to achieve.
>
> TLDR; additional index column B specified in CREATE INDEX .. (A)
> INCLUDE(B) is not used to filter rows in
Hi All,
I'd like to report what seems to be a missing optimization opportunity or
understand why it is not possible to achieve.
TLDR; additional index column B specified in CREATE INDEX .. (A) INCLUDE(B) is
not used to filter rows in queries like WHERE B = $1 ORDER BY A during
IndexScan.
82 matches
Mail list logo
