From: Jesper Pedersen <jesper.pedersen@redhat.com>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Floris Van Nee <florisvannee@optiver.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: James Coleman <jtc331@gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: Jeff Janes <jeff.janes@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Bhushan Uparkar <bhushan.uparkar@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Subject: Re: Index Skip Scan
Date: Thu, 11 Jul 2019 12:53:54 -0400
Message-ID: <994cbf82-333c-56d7-2860-9c8c85aba790@redhat.com> (raw)
In-Reply-To: <CAKJS1f9xBUCdqj4hT3gc0Vp=BimUnRoQe2ogDEGBhuLv+txz_g@mail.gmail.com>
References: <2685f020-f302-9bf6-1b5c-ebc2bcebc635@2ndquadrant.com>
<CA+q6zcVnRRqktw4cg2Vddy3S+VY3Tsmit0kausYtctvsTum7tA@mail.gmail.com>
<1559763453006.85615@Optiver.com>
<1559763570329.85227@Optiver.com>
<4d469bf2-49ff-bc3d-a428-6c7b5d87d526@redhat.com>
<CAKJS1f86FgODuUnHiQ25RKeuES4qTqeNxm1QbqJWrBoZxVGLiQ@mail.gmail.com>
<CA+q6zcXoYjCtyU-jFU_=p2DL_K3BHbBEEbJHaJTxdvRX3cU-Sg@mail.gmail.com>
<CA+q6zcVVigO_1KRBxvzKfnR593CEE7FKR-bwP-XcH6AVHOo4+g@mail.gmail.com>
<a8b5c2d9-7b23-e12b-fc6b-eee63dc8dacf@redhat.com>
<CA+hUKGKo30N5VNuRWhDuMGVZ3hTcv4J5RGXe286GRJZLk_jBYQ@mail.gmail.com>
<CAKJS1f_v5X1Sb42ShOhsLzuMA6iX52PcBwpf4P-L9H_GDYA3-Q@mail.gmail.com>
<CA+hUKG+oY0u1YON4KHikmX0aO6tFBOF_LzZQOTX8bVUbe1j5mA@mail.gmail.com>
<CA+q6zcWC2BgJwFf31xQUZ+FbP+J0zR8fM0sGVMuCYJDbAW_GAw@mail.gmail.com>
<1562769595280.16860@Optiver.com>
<CA+q6zcU66Gx954SjOT5nZRRbYG-gVVNJx5NSbn3Gn2aKwtaX1Q@mail.gmail.com>
<CAKJS1f9xBUCdqj4hT3gc0Vp=BimUnRoQe2ogDEGBhuLv+txz_g@mail.gmail.com>
Hi David,
On 7/11/19 7:38 AM, David Rowley wrote:
> The UniqueKeys idea is quite separate from pathkeys. Currently, a
> Path can have a List of PathKeys which define the order that the
> tuples will be read from the Plan node that's created from that Path.
> The idea with UniqueKeys is that a Path can also have a non-empty List
> of UniqueKeys to define that there will be no more than 1 row with the
> same value for the Paths UniqueKey column/exprs.
>
> As of now, if you look at standard_qp_callback() sets
> root->query_pathkeys, the idea here would be that the callback would
> also set a new List field named "query_uniquekeys" based on the
> group_pathkeys when non-empty and !root->query->hasAggs, or by using
> the distinct clause if it's non-empty. Then in build_index_paths()
> around the call to match_pathkeys_to_index() we'll probably also want
> to check if the index can support UniqueKeys that would suit the
> query_uniquekeys that were set during standard_qp_callback().
>
> As for setting query_uniquekeys in standard_qp_callback(), this should
> be simply a matter of looping over either group_pathkeys or
> distinct_pathkeys and grabbing the pk_eclass from each key and making
> a canonical UniqueKey from that. To have these canonical you'll need
> to have a new field in PlannerInfo named canon_uniquekeys which will
> do for UniqueKeys what canon_pathkeys does for PathKeys. So you'll
> need an equivalent of make_canonical_pathkey() in uniquekey.c
>
> With this implementation, the code that the patch adds in
> create_distinct_paths() can mostly disappear. You'd only need to look
> for a path that uniquekeys_contained_in() matches the
> root->query_uniquekeys and then just leave it to
> set_cheapest(distinct_rel); to pick the cheapest path.
>
> It would be wasted effort to create paths with UniqueKeys if there's
> multiple non-dead base rels in the query as the final rel in
> create_distinct_paths would be a join rel, so it might be worth
> checking that before creating such index paths in build_index_paths().
> However, down the line, we could carry the uniquekeys forward into the
> join paths if the join does not duplicate rows from the other side of
> the join... That's future stuff though, not for this patch, I don't
> think.
>
> I think a UniqueKey can just be a struct similar to PathKey, e.g be
> located in pathnodes.h around where PathKey is defined. Likely we'll
> need a uniquekeys.c file that has the equivalent to
> pathkeys_contained_in() ... uniquekeys_contained_in(), which return
> true if arg1 is a superset of arg2 rather than check for one set being
> a prefix of another. As you mention above: UniqueKeys { x, y } ==
> UniqueKeys { y, x }. That superset check could perhaps be optimized
> by sorting UniqueKey lists in memory address order, that'll save
> having a nested loop, but likely that's not going to be required for a
> first cut version. This would work since you'd want UniqueKeys to be
> canonical the same as PathKeys are (Notice that compare_pathkeys()
> only checks memory addresses of pathkeys and not equals()).
>
> I think the UniqueKey struct would only need to contain an
> EquivalenceClass field. I think all the other stuff that's in PathKey
> is irrelevant to UniqueKey.
>
Thanks for the feedback ! I'll work on these changes for the next
uniquekey patch.
Best regards,
Jesper
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: jesper.pedersen@redhat.com, david.rowley@2ndquadrant.com, 9erthalion6@gmail.com, florisvannee@optiver.com, thomas.munro@gmail.com, jtc331@gmail.com, rafia.pghackers@gmail.com, jeff.janes@gmail.com, pg@bowt.ie, tomas.vondra@2ndquadrant.com, thomas.munro@enterprisedb.com, bhushan.uparkar@gmail.com, a.korotkov@postgrespro.ru
Subject: Re: Index Skip Scan
In-Reply-To: <994cbf82-333c-56d7-2860-9c8c85aba790@redhat.com>
* 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