From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: florisvannee@optiver.com
Cc: pg@bowt.ie
Cc: jesper.pedersen@redhat.com
Cc: david.rowley@2ndquadrant.com
Cc: alvherre@2ndquadrant.com
Cc: thomas.munro@gmail.com
Cc: jtc331@gmail.com
Cc: rafia.pghackers@gmail.com
Cc: jeff.janes@gmail.com
Cc: bhushan.uparkar@gmail.com
Cc: pgsql-hackers@postgresql.org
Cc: a.korotkov@postgrespro.ru
Subject: Re: Index Skip Scan
Date: Sat, 8 Feb 2020 15:56:35 +0100
Message-ID: <20200208145635.5cgmvgz2gsyvqpgj@development> (raw)
In-Reply-To: <20200207162543.zlj2tvcwy7jku6vu@localhost>
References: <20200205163730.eqvoxh6eb3do52r5@localhost>
<20200206.102450.1514793516830965023.horikyota.ntt@gmail.com>
<20200206105707.fuidmdsi6oapieoz@localhost>
<20200206.212220.642002310227620136.horikyota.ntt@gmail.com>
<20200207162543.zlj2tvcwy7jku6vu@localhost>
OK,
A couple more comments based on quick review of the patch, particularly
the part related to planning:
1) create_skipscan_unique_path has one assert commented out. Either it's
something we want to enforce, or we should remove it.
/*Assert(distinctPrefixKeys <= list_length(pathnode->path.pathkeys));*/
2) I wonder if the current costing model is overly optimistic. We simply
copy the startup cost from the IndexPath, which seems fine. But for
total cost we do this:
pathnode->path.total_cost = basepath->startup_cost * numGroups;
which seems a bit too simplistic. The startup cost is pretty much just
the cost to find the first item in the index, but surely we need to do
more to find the next group - we need to do comparisons to skip some of
the items, etc. If we think that's unnecessary, we need to explain it in
a comment or somthing.
3) I don't think we should make planning dependent on hasDeclareCursor.
4) I'm not quite sure how sensible it's to create a new IndexPath in
create_skipscan_unique_path. On the one hand it works, but I don't think
any other path is constructed like this so I wonder if we're missing
something. Perhaps it'd be better to just add a new path node on top of
the IndexPath, and then handle this in create_plan. We already do
something similar for Bitmap Index Scans, where we create a different
executor node from IndexPath depending on the parent node.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, 9erthalion6@gmail.com, horikyota.ntt@gmail.com, florisvannee@optiver.com, pg@bowt.ie, jesper.pedersen@redhat.com, david.rowley@2ndquadrant.com, alvherre@2ndquadrant.com, thomas.munro@gmail.com, jtc331@gmail.com, rafia.pghackers@gmail.com, jeff.janes@gmail.com, bhushan.uparkar@gmail.com, a.korotkov@postgrespro.ru
Subject: Re: Index Skip Scan
In-Reply-To: <20200208145635.5cgmvgz2gsyvqpgj@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox