From 635d1d86732ee86baa545fe31c9b44bf73dfead2 Mon Sep 17 00:00:00 2001 From: Edmund Horner Date: Fri, 12 Oct 2018 16:29:44 +1300 Subject: [PATCH 4/5] Tid Scan results are ordered The planner now knows that the results of a Tid path are ordered by ctid, so queries that rely on that order no longer need a separate sort. This improves cases such as "ORDER BY ctid ASC/DESC", as well as "SELECT MIN(ctid)/MAX(ctid)". Tid Scans can now be Backward. --- src/backend/commands/explain.c | 48 ++++++++----- src/backend/executor/nodeTidscan.c | 13 +++- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 2 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/path/pathkeys.c | 33 +++++++++ src/backend/optimizer/path/tidpath.c | 42 +++++++++-- src/backend/optimizer/plan/createplan.c | 9 ++- src/backend/optimizer/util/pathnode.c | 4 +- src/include/nodes/plannodes.h | 1 + src/include/nodes/relation.h | 1 + src/include/optimizer/pathnode.h | 3 +- src/include/optimizer/paths.h | 3 + src/test/regress/expected/tidscan.out | 123 +++++++++++++++++++++++++++++++- src/test/regress/sql/tidscan.sql | 39 +++++++++- 15 files changed, 292 insertions(+), 31 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index c0d0168..73ff03b 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -111,6 +111,7 @@ static void show_foreignscan_info(ForeignScanState *fsstate, ExplainState *es); static void show_eval_params(Bitmapset *bms_params, ExplainState *es); static const char *explain_get_index_name(Oid indexId); static void show_buffer_usage(ExplainState *es, const BufferUsage *usage); +static void show_scan_direction(ExplainState *es, ScanDirection scandir); static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir, ExplainState *es); static void ExplainScanTarget(Scan *plan, ExplainState *es); @@ -1271,7 +1272,6 @@ ExplainNode(PlanState *planstate, List *ancestors, case T_SeqScan: case T_SampleScan: case T_BitmapHeapScan: - case T_TidScan: case T_SubqueryScan: case T_FunctionScan: case T_TableFuncScan: @@ -1280,6 +1280,10 @@ ExplainNode(PlanState *planstate, List *ancestors, case T_WorkTableScan: ExplainScanTarget((Scan *) plan, es); break; + case T_TidScan: + show_scan_direction(es, ((TidScan *) plan)->scandir); + ExplainScanTarget((Scan *) plan, es); + break; case T_ForeignScan: case T_CustomScan: if (((Scan *) plan)->scanrelid > 0) @@ -2893,45 +2897,57 @@ show_buffer_usage(ExplainState *es, const BufferUsage *usage) } /* - * Add some additional details about an IndexScan or IndexOnlyScan + * Show the direction of a scan. */ static void -ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir, - ExplainState *es) +show_scan_direction(ExplainState *es, ScanDirection scandir) { - const char *indexname = explain_get_index_name(indexid); - if (es->format == EXPLAIN_FORMAT_TEXT) { - if (ScanDirectionIsBackward(indexorderdir)) + if (ScanDirectionIsBackward(scandir)) appendStringInfoString(es->str, " Backward"); - appendStringInfo(es->str, " using %s", indexname); } else { - const char *scandir; + const char *scandirstr; - switch (indexorderdir) + switch (scandir) { case BackwardScanDirection: - scandir = "Backward"; + scandirstr = "Backward"; break; case NoMovementScanDirection: - scandir = "NoMovement"; + scandirstr = "NoMovement"; break; case ForwardScanDirection: - scandir = "Forward"; + scandirstr = "Forward"; break; default: - scandir = "???"; + scandirstr = "???"; break; } - ExplainPropertyText("Scan Direction", scandir, es); - ExplainPropertyText("Index Name", indexname, es); + ExplainPropertyText("Scan Direction", scandirstr, es); } } /* + * Add some additional details about an IndexScan or IndexOnlyScan + */ +static void +ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir, + ExplainState *es) +{ + const char *indexname = explain_get_index_name(indexid); + + show_scan_direction(es, indexorderdir); + + if (es->format == EXPLAIN_FORMAT_TEXT) + appendStringInfo(es->str, " using %s", indexname); + else + ExplainPropertyText("Index Name", indexname, es); +} + +/* * Show the target of a Scan node */ static void diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index a3b5970..4f05938 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -816,6 +816,15 @@ TidNext(TidScanState *node) numRanges = node->tss_NumTidRanges; + /* If the plan direction is backward, invert the direction. */ + if (ScanDirectionIsBackward(((TidScan *) node->ss.ps.plan)->scandir)) + { + if (ScanDirectionIsForward(direction)) + direction = BackwardScanDirection; + else if (ScanDirectionIsBackward(direction)) + direction = ForwardScanDirection; + } + tuple = NULL; for (;;) { @@ -824,9 +833,7 @@ TidNext(TidScanState *node) if (!node->tss_inScan) { /* Initialize or advance scan position, depending on direction. */ - bool bBackward = ScanDirectionIsBackward(direction); - - if (bBackward) + if (ScanDirectionIsBackward(direction)) { if (node->tss_CurrentTidRange < 0) { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index cab02a4..8c05bdd 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -580,6 +580,7 @@ _copyTidScan(const TidScan *from) * copy remainder of node */ COPY_NODE_FIELD(tidquals); + COPY_SCALAR_FIELD(scandir); return newnode; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 647665a..8295a09 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -616,6 +616,7 @@ _outTidScan(StringInfo str, const TidScan *node) _outScanInfo(str, (const Scan *) node); WRITE_NODE_FIELD(tidquals); + WRITE_ENUM_FIELD(scandir, ScanDirection); } static void @@ -1892,6 +1893,7 @@ _outTidPath(StringInfo str, const TidPath *node) _outPathInfo(str, (const Path *) node); WRITE_NODE_FIELD(tidquals); + WRITE_ENUM_FIELD(scandir, ScanDirection); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index e117867..5bd01d5 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1846,6 +1846,7 @@ _readTidScan(void) ReadCommonScan(&local_node->scan); READ_NODE_FIELD(tidquals); + READ_ENUM_FIELD(scandir, ScanDirection); READ_DONE(); } diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index ec66cb9..b08830f 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -18,6 +18,9 @@ #include "postgres.h" #include "access/stratnum.h" +#include "access/sysattr.h" +#include "catalog/pg_operator.h" +#include "catalog/pg_type.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "nodes/plannodes.h" @@ -848,6 +851,36 @@ build_join_pathkeys(PlannerInfo *root, return truncate_useless_pathkeys(root, joinrel, outer_pathkeys); } +/* + * build_tidscan_pathkeys + * Build the path keys corresponding to ORDER BY ctid ASC|DESC. + */ +List * +build_tidscan_pathkeys(PlannerInfo *root, + RelOptInfo *rel, + ScanDirection scandir) +{ + int opno; + Expr *expr; + List *pathkeys; + + opno = ScanDirectionIsForward(scandir) ? TIDLessOperator : TIDGreaterOperator; + expr = (Expr *) makeVar(rel->relid, + SelfItemPointerAttributeNumber, + TIDOID, + -1, + InvalidOid, + 0); + pathkeys = build_expression_pathkey(root, + expr, + NULL, + opno, + rel->relids, + true); + + return pathkeys; +} + /**************************************************************************** * PATHKEYS AND SORT CLAUSES ****************************************************************************/ diff --git a/src/backend/optimizer/path/tidpath.c b/src/backend/optimizer/path/tidpath.c index 3290294..38c58fa 100644 --- a/src/backend/optimizer/path/tidpath.c +++ b/src/backend/optimizer/path/tidpath.c @@ -357,12 +357,17 @@ TidQualFromBaseRestrictinfo(RelOptInfo *rel) * create_tidscan_paths * Create paths corresponding to direct TID scans of the given rel. * + * Path keys will be set to "CTID ASC" by default, or "CTID DESC" if it + * looks more useful. + * * Candidate paths are added to the rel's pathlist (using add_path). */ void create_tidscan_paths(PlannerInfo *root, RelOptInfo *rel) { Relids required_outer; + List *pathkeys = NIL; + ScanDirection scandir = ForwardScanDirection; List *tidquals; /* @@ -374,8 +379,37 @@ create_tidscan_paths(PlannerInfo *root, RelOptInfo *rel) tidquals = TidQualFromBaseRestrictinfo(rel); - /* If there are tidquals, then it's worth generating a tidscan path. */ - if (tidquals) - add_path(rel, (Path *) create_tidscan_path(root, rel, tidquals, - required_outer)); + /* + * Look for a suitable direction by trying both forward and backward + * pathkeys. But don't set any pathkeys if neither direction helps the + * scan (we don't want to generate tid paths for everything). + */ + if (has_useful_pathkeys(root, rel)) + { + pathkeys = build_tidscan_pathkeys(root, rel, ForwardScanDirection); + if (!pathkeys_contained_in(pathkeys, root->query_pathkeys)) + { + pathkeys = build_tidscan_pathkeys(root, rel, BackwardScanDirection); + if (pathkeys_contained_in(pathkeys, root->query_pathkeys)) + scandir = BackwardScanDirection; + else + pathkeys = NIL; + } + } + else if (tidquals) + { + /* + * Otherwise, default to a forward scan -- but only if tid quals were + * found (we don't want to generate tid paths for everything). + */ + pathkeys = build_tidscan_pathkeys(root, rel, ForwardScanDirection); + } + + /* + * If there are tidquals or some useful pathkeys were found, then it's + * worth generating a tidscan path. + */ + if (tidquals || pathkeys) + add_path(rel, (Path *) create_tidscan_path(root, rel, tidquals, pathkeys, + scandir, required_outer)); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index e2c0bce..66513fe 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -184,7 +184,7 @@ static BitmapHeapScan *make_bitmap_heapscan(List *qptlist, List *bitmapqualorig, Index scanrelid); static TidScan *make_tidscan(List *qptlist, List *qpqual, Index scanrelid, - List *tidquals); + List *tidquals, ScanDirection scandir); static SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual, Index scanrelid, @@ -3115,7 +3115,8 @@ create_tidscan_plan(PlannerInfo *root, TidPath *best_path, scan_plan = make_tidscan(tlist, scan_clauses, scan_relid, - tidquals); + tidquals, + best_path->scandir); copy_generic_path_info(&scan_plan->scan.plan, &best_path->path); @@ -5176,7 +5177,8 @@ static TidScan * make_tidscan(List *qptlist, List *qpqual, Index scanrelid, - List *tidquals) + List *tidquals, + ScanDirection scandir) { TidScan *node = makeNode(TidScan); Plan *plan = &node->scan.plan; @@ -5187,6 +5189,7 @@ make_tidscan(List *qptlist, plan->righttree = NULL; node->scan.scanrelid = scanrelid; node->tidquals = tidquals; + node->scandir = scandir; return node; } diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index d50d86b..fcfdaa1 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -1186,6 +1186,7 @@ create_bitmap_or_path(PlannerInfo *root, */ TidPath * create_tidscan_path(PlannerInfo *root, RelOptInfo *rel, List *tidquals, + List *pathkeys, ScanDirection scandir, Relids required_outer) { TidPath *pathnode = makeNode(TidPath); @@ -1198,9 +1199,10 @@ create_tidscan_path(PlannerInfo *root, RelOptInfo *rel, List *tidquals, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel; pathnode->path.parallel_workers = 0; - pathnode->path.pathkeys = NIL; /* always unordered */ + pathnode->path.pathkeys = pathkeys; pathnode->tidquals = tidquals; + pathnode->scandir = scandir; cost_tidscan(&pathnode->path, root, rel, tidquals, pathnode->path.param_info); diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index bc9ff54..6a1a83d 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -486,6 +486,7 @@ typedef struct TidScan { Scan scan; List *tidquals; /* qual(s) involving CTID = something */ + ScanDirection scandir; } TidScan; /* ---------------- diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 895849f..4baf14e 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -1243,6 +1243,7 @@ typedef struct TidPath { Path path; List *tidquals; + ScanDirection scandir; } TidPath; /* diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index 81abcf5..735d185 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -63,7 +63,8 @@ extern BitmapOrPath *create_bitmap_or_path(PlannerInfo *root, RelOptInfo *rel, List *bitmapquals); extern TidPath *create_tidscan_path(PlannerInfo *root, RelOptInfo *rel, - List *tidquals, Relids required_outer); + List *tidquals, List *pathkeys, ScanDirection scandir, + Relids required_outer); extern AppendPath *create_append_path(PlannerInfo *root, RelOptInfo *rel, List *subpaths, List *partial_subpaths, Relids required_outer, diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index cafde30..a4b0a6a 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -211,6 +211,9 @@ extern List *build_join_pathkeys(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype, List *outer_pathkeys); +extern List *build_tidscan_pathkeys(PlannerInfo *root, + RelOptInfo *rel, + ScanDirection scandir); extern List *make_pathkeys_for_sortclauses(PlannerInfo *root, List *sortclauses, List *tlist); diff --git a/src/test/regress/expected/tidscan.out b/src/test/regress/expected/tidscan.out index 8083909..8dcbf99 100644 --- a/src/test/regress/expected/tidscan.out +++ b/src/test/regress/expected/tidscan.out @@ -176,7 +176,6 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) UPDATE tidscan SET id = -id WHERE CURRENT OF c RETURNING *; ERROR: cursor "c" is not positioned on a row ROLLBACK; -DROP TABLE tidscan; -- tests for tidrangescans CREATE TABLE tidrangescan(id integer, data text); INSERT INTO tidrangescan SELECT i,repeat('x', 100) FROM generate_series(1,1000) AS s(i); @@ -427,3 +426,125 @@ SELECT ctid FROM tidrangescan_empty WHERE ctid > '(9, 0)'; ------ (0 rows) +-- check that ordering on a tidscan doesn't require a sort +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid; + QUERY PLAN +--------------------------------------------------------------- + Tid Scan on tidscan + TID Cond: (ctid = ANY ('{"(0,2)","(0,1)","(0,3)"}'::tid[])) +(2 rows) + +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid; + ctid +------- + (0,1) + (0,2) + (0,3) +(3 rows) + +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid DESC; + QUERY PLAN +--------------------------------------------------------------- + Tid Scan Backward on tidscan + TID Cond: (ctid = ANY ('{"(0,2)","(0,1)","(0,3)"}'::tid[])) +(2 rows) + +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid DESC; + ctid +------- + (0,3) + (0,2) + (0,1) +(3 rows) + +-- ordering with no quals should use tid range scan +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidrangescan ORDER BY ctid ASC; + QUERY PLAN +-------------------------- + Tid Scan on tidrangescan +(1 row) + +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidrangescan ORDER BY ctid DESC; + QUERY PLAN +----------------------------------- + Tid Scan Backward on tidrangescan +(1 row) + +-- min/max +EXPLAIN (COSTS OFF) +SELECT MIN(ctid) FROM tidrangescan; + QUERY PLAN +-------------------------------------------- + Result + InitPlan 1 (returns $0) + -> Limit + -> Tid Scan on tidrangescan + Filter: (ctid IS NOT NULL) +(5 rows) + +SELECT MIN(ctid) FROM tidrangescan; + min +------- + (0,1) +(1 row) + +EXPLAIN (COSTS OFF) +SELECT MAX(ctid) FROM tidrangescan; + QUERY PLAN +------------------------------------------------- + Result + InitPlan 1 (returns $0) + -> Limit + -> Tid Scan Backward on tidrangescan + Filter: (ctid IS NOT NULL) +(5 rows) + +SELECT MAX(ctid) FROM tidrangescan; + max +-------- + (9,10) +(1 row) + +EXPLAIN (COSTS OFF) +SELECT MIN(ctid) FROM tidrangescan WHERE ctid > '(5,0)'; + QUERY PLAN +------------------------------------------------- + Result + InitPlan 1 (returns $0) + -> Limit + -> Tid Scan on tidrangescan + TID Cond: (ctid > '(5,0)'::tid) + Filter: (ctid IS NOT NULL) +(6 rows) + +SELECT MIN(ctid) FROM tidrangescan WHERE ctid > '(5,0)'; + min +------- + (5,1) +(1 row) + +EXPLAIN (COSTS OFF) +SELECT MAX(ctid) FROM tidrangescan WHERE ctid < '(5,0)'; + QUERY PLAN +------------------------------------------------- + Result + InitPlan 1 (returns $0) + -> Limit + -> Tid Scan Backward on tidrangescan + TID Cond: (ctid < '(5,0)'::tid) + Filter: (ctid IS NOT NULL) +(6 rows) + +SELECT MAX(ctid) FROM tidrangescan WHERE ctid < '(5,0)'; + max +-------- + (4,10) +(1 row) + +-- clean up +DROP TABLE tidscan; +DROP TABLE tidrangescan; diff --git a/src/test/regress/sql/tidscan.sql b/src/test/regress/sql/tidscan.sql index 02b094a..8f437e8 100644 --- a/src/test/regress/sql/tidscan.sql +++ b/src/test/regress/sql/tidscan.sql @@ -63,8 +63,6 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) UPDATE tidscan SET id = -id WHERE CURRENT OF c RETURNING *; ROLLBACK; -DROP TABLE tidscan; - -- tests for tidrangescans CREATE TABLE tidrangescan(id integer, data text); @@ -140,3 +138,40 @@ SELECT ctid FROM tidrangescan_empty WHERE ctid < '(1, 0)'; EXPLAIN (COSTS OFF) SELECT ctid FROM tidrangescan_empty WHERE ctid > '(9, 0)'; SELECT ctid FROM tidrangescan_empty WHERE ctid > '(9, 0)'; + +-- check that ordering on a tidscan doesn't require a sort +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid; +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid; + +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid DESC; +SELECT ctid FROM tidscan WHERE ctid = ANY(ARRAY['(0,2)', '(0,1)', '(0,3)']::tid[]) ORDER BY ctid DESC; + +-- ordering with no quals should use tid range scan +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidrangescan ORDER BY ctid ASC; + +EXPLAIN (COSTS OFF) +SELECT ctid FROM tidrangescan ORDER BY ctid DESC; + +-- min/max +EXPLAIN (COSTS OFF) +SELECT MIN(ctid) FROM tidrangescan; +SELECT MIN(ctid) FROM tidrangescan; + +EXPLAIN (COSTS OFF) +SELECT MAX(ctid) FROM tidrangescan; +SELECT MAX(ctid) FROM tidrangescan; + +EXPLAIN (COSTS OFF) +SELECT MIN(ctid) FROM tidrangescan WHERE ctid > '(5,0)'; +SELECT MIN(ctid) FROM tidrangescan WHERE ctid > '(5,0)'; + +EXPLAIN (COSTS OFF) +SELECT MAX(ctid) FROM tidrangescan WHERE ctid < '(5,0)'; +SELECT MAX(ctid) FROM tidrangescan WHERE ctid < '(5,0)'; + +-- clean up +DROP TABLE tidscan; +DROP TABLE tidrangescan; -- 2.7.4