Hi Alexander,
On 06/18/2018 01:31 PM, Alexander Korotkov wrote:
> <jesper.pedersen@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