8000 Fix asymmetry in setting EquivalenceClass.ec_sortref · postgrespro/postgres@199012a · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 199012a

    Browse files
    akorotkovakorotkov
    committed
    Fix asymmetry in setting EquivalenceClass.ec_sortref
    0452b46 made get_eclass_for_sort_expr() always set EquivalenceClass.ec_sortref if it's not done yet. This leads to an asymmetric situation when whoever first looks for the EquivalenceClass sets the ec_sortref. It is also counterintuitive that get_eclass_for_sort_expr() performs modification of data structures. This commit makes make_pathkeys_for_sortclauses_extended() responsible for setting EquivalenceClass.ec_sortref. Now we set the EquivalenceClass.ec_sortref's needed to explore alternative GROUP BY ordering specifically during building pathkeys by the list of grouping clauses. Discussion: https://postgr.es/m/17037754-f187-4138-8285-0e2bfebd0dea%40postgrespro.ru Reported-by: Tom Lane Author: Andrei Lepikhov Reviewed-by: Alexander Korotkov, Pavel Borisov
    1 parent f654f00 commit 199012a

    File tree

    6 files changed

    +92
    -19
    lines changed

    6 files changed

    +92
    -19
    lines changed

    ‎src/backend/optimizer/path/equivclass.c

    Lines changed: 1 addition & 12 deletions
    Original file line numberDiff line numberDiff line change
    @@ -652,18 +652,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
    652652

    653653
    if (opcintype == cur_em->em_datatype &&
    654654
    equal(expr, cur_em->em_expr))
    655-
    {
    656-
    /*
    657-
    * Match!
    658-
    *
    659-
    * Copy the sortref if it wasn't set yet. That may happen if
    660-
    * the ec was constructed from a WHERE clause, i.e. it doesn't
    661-
    * have a target reference at all.
    662-
    */
    663-
    if (cur_ec->ec_sortref == 0 && sortref > 0)
    664-
    cur_ec->ec_sortref = sortref;
    665-
    return cur_ec;
    666-
    }
    655+
    return cur_ec; /* Match! */
    667656
    }
    668657
    }
    669658

    ‎src/backend/optimizer/path/pathkeys.c

    Lines changed: 16 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1355,7 +1355,8 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
    13551355
    &sortclauses,
    13561356
    tlist,
    13571357
    false,
    1358-
    &sortable);
    1358+
    &sortable,
    1359+
    false);
    13591360
    /* It's caller error if not all clauses were sortable */
    13601361
    Assert(sortable);
    13611362
    return result;
    @@ -1379,13 +1380,17 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
    13791380
    * to remove any clauses that can be proven redundant via the eclass logic.
    13801381
    * Even though we'll have to hash in that case, we might as well not hash
    13811382
    * redundant columns.)
    1383+
    *
    1384+
    * If set_ec_sortref is true then sets the value of the pathkey's
    1385+
    * EquivalenceClass unless it's already initialized.
    13821386
    */
    13831387
    List *
    13841388
    make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
    13851389
    List **sortclauses,
    13861390
    List *tlist,
    13871391
    bool remove_redundant,
    1388-
    bool *sortable)
    1392+
    bool *sortable,
    1393+
    bool set_ec_sortref)
    13891394
    {
    13901395
    List *pathkeys = NIL;
    13911396
    ListCell *l;
    @@ -1409,6 +1414,15 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
    14091414
    sortcl->nulls_first,
    14101415
    sortcl->tleSortGroupRef,
    14111416
    true);
    1417+
    if (pathkey->pk_eclass->ec_sortref == 0 && set_ec_sortref)
    1418+
    {
    1419+
    /*
    1420+
    * Copy the sortref if it hasn't been set yet. That may happen if
    1421+
    * the EquivalenceClass was constructed from a WHERE clause, i.e.
    1422+
    * it doesn't have a target reference at all.
    1423+
    */
    1424+
    pathkey->pk_eclass->ec_sortref = sortcl->tleSortGroupRef;
    1425+
    }
    14121426

    14131427
    /* Canonical form eliminates redundant ordering keys */
    14141428
    if (!pathkey_is_redundant(pathkey, pathkeys))

    ‎src/backend/optimizer/plan/planner.c

    Lines changed: 12 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -3395,12 +3395,17 @@ standard_qp_callback(PlannerInfo *root, void *extra)
    33953395
    */
    33963396
    bool sortable;
    33973397

    3398+
    /*
    3399+
    * Convert group clauses into pathkeys. Set the ec_sortref field of
    3400+
    * EquivalenceClass'es if it's not set yet.
    3401+
    */
    33983402
    root->group_pathkeys =
    33993403
    make_pathkeys_for_sortclauses_extended(root,
    34003404
    &root->processed_groupClause,
    34013405
    tlist,
    34023406
    true,
    3403-
    &sortable);
    3407+
    &sortable,
    3408+
    true);
    34043409
    if (!sortable)
    34053410
    {
    34063411
    /* Can't sort; no point in considering aggregate ordering either */
    @@ -3450,7 +3455,8 @@ standard_qp_callback(PlannerInfo *root, void *extra)
    34503455
    &root->processed_distinctClause,
    34513456
    tlist,
    34523457
    true,
    3453-
    &sortable);
    3458+
    &sortable,
    3459+
    false);
    34543460
    if (!sortable)
    34553461
    root->distinct_pathkeys = NIL;
    34563462
    }
    @@ -3476,7 +3482,8 @@ standard_qp_callback(PlannerInfo *root, void *extra)
    34763482
    &groupClauses,
    34773483
    tlist,
    34783484
    false,
    3479-
    &sortable);
    3485+
    &sortable,
    3486+
    false);
    34803487
    if (!sortable)
    34813488
    root->setop_pathkeys = NIL;
    34823489
    }
    @@ -6061,7 +6068,8 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
    60616068
    &wc->partitionClause,
    60626069
    tlist,
    60636070
    true,
    6064-
    &sortable);
    6071+
    &sortable,
    6072+
    false);
    60656073

    60666074
    Assert(sortable);
    60676075
    }

    ‎src/include/optimizer/paths.h

    Lines changed: 2 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -239,7 +239,8 @@ extern List *make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
    239239
    List **sortclauses,
    240240
    List *tlist,
    241241
    bool remove_redundant,
    242-
    bool *sortable);
    242+
    bool *sortable,
    243+
    bool set_ec_sortref);
    243244
    extern void initialize_mergeclause_eclasses(PlannerInfo *root,
    244245
    RestrictInfo *restrictinfo);
    245246
    extern void update_mergeclause_eclasses(PlannerInfo *root,

    ‎src/test/regress/expected/aggregates.out

    Lines changed: 47 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2917,6 +2917,53 @@ GROUP BY c1.w, c1.z;
    29172917
    5.0000000000000000
    29182918
    (2 rows)
    29192919

    2920+
    -- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause
    2921+
    -- ordering. Also, here we check that it doesn't depend on the initial clause
    2922+
    -- order in the GROUP-BY list.
    2923+
    EXPLAIN (COSTS OFF)
    2924+
    SELECT c1.y,c1.x FROM group_agg_pk c1
    2925+
    JOIN group_agg_pk c2
    2926+
    ON c1.x = c2.x
    2927+
    GROUP BY c1.y,c1.x,c2.x;
    2928+
    QUERY PLAN
    2929+
    -----------------------------------------------------
    2930+
    Group
    2931+
    Group Key: c1.x, c1.y
    2932+
    -> Incremental Sort
    2933+
    Sort Key: c1.x, c1.y
    2934+
    Presorted Key: c1.x
    2935+
    -> Merge Join
    2936+
    Merge Cond: (c1.x = c2.x)
    2937+
    -> Sort
    2938+
    Sort Key: c1.x
    2939+
    -> Seq Scan on group_agg_pk c1
    2940+
    -> Sort
    2941+
    Sort Key: c2.x
    2942+
    -> Seq Scan on group_agg_pk c2
    2943+
    (13 rows)
    2944+
    2945+
    EXPLAIN (COSTS OFF)
    2946+
    SELECT c1.y,c1.x FROM group_agg_pk c1
    2947+
    JOIN group_agg_pk c2
    2948+
    ON c1.x = c2.x
    2949+
    GROUP BY c1.y,c2.x,c1.x;
    2950+
    QUERY PLAN
    2951+
    -----------------------------------------------------
    2952+
    Group
    2953+
    Group Key: c2.x, c1.y
    2954+
    -> Incremental Sort
    2955+
    Sort Key: c2.x, c1.y
    2956+
    Presorted Key: c2.x
    2957+
    -> Merge Join
    2958+
    Merge Cond: (c1.x = c2.x)
    2959+
    -> Sort
    2960+
    Sort Key: c1.x
    2961+
    -> Seq Scan on group_agg_pk c1
    2962+
    -> Sort
    2963+
    Sort Key: c2.x
    2964+
    -> Seq Scan on group_agg_pk c2
    2965+
    (13 rows)
    2966+
    29202967
    RESET enable_nestloop;
    29212968
    RESET enable_hashjoin;
    29222969
    DROP TABLE group_agg_pk;

    ‎src/test/regress/sql/aggregates.sql

    Lines changed: 14 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1263,6 +1263,20 @@ SELECT avg(c1.f ORDER BY c1.x, c1.y)
    12631263
    FROM group_agg_pk c1 JOIN group_agg_pk c2 ON c1.x = c2.x
    12641264
    GROUP BY c1.w, c1.z;
    12651265

    1266+
    -- Pathkeys, built in a subtree, can be used to optimize GROUP-BY clause
    1267+
    -- ordering. Also, here we check that it doesn't depend on the initial clause
    1268+
    -- order in the GROUP-BY list.
    1269+
    EXPLAIN (COSTS OFF)
    1270+
    SELECT c1.y,c1.x FROM group_agg_pk c1
    1271+
    JOIN group_agg_pk c2
    1272+
    ON c1.x = c2.x
    1273+
    GROUP BY c1.y,c1.x,c2.x;
    1274+
    EXPLAIN (COSTS OFF)
    1275+
    SELECT c1.y,c1.x FROM group_agg_pk c1
    1276+
    JOIN group_agg_pk c2
    1277+
    ON c1.x = c2.x
    1278+
    GROUP BY c1.y,c2.x,c1.x;
    1279+
    12661280
    RESET enable_nestloop;
    12671281
    RESET enable_hashjoin;
    12681282
    DROP TABLE group_agg_pk;

    0 commit comments

    Comments
     (0)
    0