Hi Alexander,
On 06/18/2018 01:31 PM, Alexander Korotkov wrote:
<jesper.peder...@redhat.com> wrote:
Should the patch continue to "piggy-back" on T_IndexOnlyScan, or should
a new node (T_IndexSkipScan) be created ? If latter, then there likely
will be functionality that needs to be refactored into shared code
between the nodes.
Is skip scan only possible for index-only scan? I guess, that no. We
could also make plain index scan to behave like a skip scan. And it
should be useful for accelerating DISTINCT ON clause. Thus, we might
have 4 kinds of index scan: IndexScan, IndexOnlyScan, IndexSkipScan,
IndexOnlySkipScan. So, I don't think I like index scan nodes to
multiply this way, and it would be probably better to keep number
nodes smaller. But I don't insist on that, and I would like to hear
other opinions too.
Yes, there are likely other use-cases for Index Skip Scan apart from the
simplest form. Which sort of suggests that having dedicated nodes would
be better in the long run.
My goal is to cover the simplest form, which can be handled by extending
the T_IndexOnlyScan node, or by having common functions that both use.
We can always improve the functionality with future patches.
Which is the best way to deal with the amcanbackward functionality ? Do
people see another alternative to Robert's idea of adding a flag to the
scan.
Supporting amcanbackward seems to be basically possible, but rather
complicated and not very efficient. So, it seems to not worth
implementing, at least in the initial version. > And then the question
should how index access method report that it supports both skip scan
and backward scan, but not both together? What about turning
amcanbackward into a function which takes (bool skipscan) argument?
Therefore, this function will return whether backward scan is
supported depending of whether skip scan is used.
The feedback from Robert Haas seems to suggest that it was a requirement
for the patch to be considered.
I wasn't planning on making this a patch submission for the July
CommitFest due to the reasons mentioned above, but can do so if people
thinks it is best. The patch is based on master/4c8156.
Please, register it on commitfest. If even there wouldn't be enough
of time for this patch on July commitfest, it's no problem to move it.
Based on the feedback from Andrew and Michael I won't register this
thread until the September CF.
Thanks for your feedback !
Best regards,
Jesper
