WITH ORDINALITY planner improvements
On Mon, Jul 29, 2013 at 1:02 PM, Greg Stark <stark@mit.edu> wrote:
On Mon, Jul 29, 2013 at 8:56 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Unless LATERAL provides a way to do lock-step iteration through a pair
(or more) of functions I don't think we can get rid of SRFs [in select target lists] yetYou don't even need lateral. This works fine:
postgres=# select * from generate_series(1,10) with ordinality as
a(a,o) natural full outer join generate_series(1,5) with ordinality as
b(b,o) ;o | a | b
----+----+---
1 | 1 | 1
2 | 2 | 2
3 | 3 | 3
4 | 4 | 4
5 | 5 | 5
6 | 6 |
7 | 7 |
8 | 8 |
9 | 9 |
10 | 10 |
(10 rows)However it occurs to me that the plan isn't ideal:
postgres=# explain select * from generate_series(1,10) with ordinality
as a(a,o) natural full outer join generate_series(1,5) with ordinality
as b(b,o) ;
QUERY PLAN
---------------------------------------------------------------------------------------
Merge Full Join (cost=119.66..199.66 rows=5000 width=24)
Merge Cond: (a.o = b.o)
-> Sort (cost=59.83..62.33 rows=1000 width=12)
Sort Key: a.o
-> Function Scan on generate_series a (cost=0.00..10.00
rows=1000 width=12)
-> Sort (cost=59.83..62.33 rows=1000 width=12)
Sort Key: b.o
-> Function Scan on generate_series b (cost=0.00..10.00
rows=1000 width=12)
(8 rows)I think all that's required to avoid the sorts is teaching the planner
that the Path has a PathKey of the ordinal column. I can look at that
later but I'll go ahead and commit it without it at first. I wonder if
it's also useful to teach the planner that the column is unique.
So I took a crack at this. But it's not working.
To generate the pathkey for the ordinality column I basically exported
make_pathkey_from_sortop() through a thin wrapper called
make_pathkey_for_functionscan() and passed the Var from the
reltargetlist and hard coded int8gt's oid. That might stand to be
generalized a bit but in any case it doesn't matter because it doesn't
work.
My best guess of what's going wrong is that the eclass is being
generated too late and instead of getting the canonical eclass it's
getting a freshly minted one that doesn't match the eclass for the
merge join pathkey. But as I said, that's just a guess. I'm missing a
high level view of the order of operations between turning parser
column references into vars into eclasses and what has to be done
when. My best guess is that what's needed is marking the ordinality
column with a sortref early on but again, as I said, I'm just
guessing.
--
greg
--
greg
Attachments:
ordinality-path.diffapplication/octet-stream; name=ordinality-path.diffDownload
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
***************
*** 1268,1274 **** set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
required_outer = rel->lateral_relids;
/* Generate appropriate path */
! add_path(rel, create_functionscan_path(root, rel, required_outer));
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
--- 1268,1274 ----
required_outer = rel->lateral_relids;
/* Generate appropriate path */
! add_path(rel, create_functionscan_path(root, rel, required_outer, rte->funcordinality));
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
*** a/src/backend/optimizer/path/pathkeys.c
--- b/src/backend/optimizer/path/pathkeys.c
***************
*** 782,787 **** make_pathkeys_for_sortclauses(PlannerInfo *root,
--- 782,820 ----
}
/****************************************************************************
+ * PATHKEYS AND FUNCTION SCANS
+ ****************************************************************************/
+
+ /*
+ * make_pathkeys_for_functionscan
+ * Generate a pathkeys list that represents the sort order specified by a
+ * specific column of a function scan. This is used for WITH ORDINALITY
+ *
+ * This is basically exporting make_pathkey_from_sortop for callers that know
+ * exactly what expression they want to generate a pathkey list for.
+ */
+ List *
+ make_pathkeys_for_functionscan(PlannerInfo *root,
+ Expr *expr,
+ Oid sortop)
+
+ {
+ List *pathkeys = NIL;
+ PathKey *pathkey;
+ pathkey = make_pathkey_from_sortop(root,
+ expr,
+ sortop,
+ false, /* nulls first */
+ 0, /* sortref */
+ true);
+
+ pathkeys = lappend(pathkeys, pathkey);
+ return pathkeys;
+ }
+
+
+
+ /****************************************************************************
* PATHKEYS AND MERGECLAUSES
****************************************************************************/
*** a/src/backend/optimizer/util/pathnode.c
--- b/src/backend/optimizer/util/pathnode.c
***************
*** 27,33 ****
#include "parser/parsetree.h"
#include "utils/lsyscache.h"
#include "utils/selfuncs.h"
!
typedef enum
{
--- 27,35 ----
#include "parser/parsetree.h"
#include "utils/lsyscache.h"
#include "utils/selfuncs.h"
! #include "catalog/pg_opfamily.h"
! #include "catalog/pg_operator.h"
! #include "catalog/pg_type.h"
typedef enum
{
***************
*** 1623,1629 **** create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
*/
Path *
create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
! Relids required_outer)
{
Path *pathnode = makeNode(Path);
--- 1625,1631 ----
*/
Path *
create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
! Relids required_outer, bool funcordinality)
{
Path *pathnode = makeNode(Path);
***************
*** 1631,1637 **** create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
pathnode->parent = rel;
pathnode->param_info = get_baserel_parampathinfo(root, rel,
required_outer);
! pathnode->pathkeys = NIL; /* for now, assume unordered result */
cost_functionscan(pathnode, root, rel, pathnode->param_info);
--- 1633,1647 ----
pathnode->parent = rel;
pathnode->param_info = get_baserel_parampathinfo(root, rel,
required_outer);
! if (funcordinality) {
! /* ordered by ordinality column */
! pathnode->pathkeys =
! make_pathkeys_for_functionscan(root,
! llast(rel->reltargetlist),
! INT8GT);
! } else {
! pathnode->pathkeys = NIL; /* No information about ordering */
! }
cost_functionscan(pathnode, root, rel, pathnode->param_info);
*** a/src/include/catalog/pg_operator.h
--- b/src/include/catalog/pg_operator.h
***************
*** 171,182 **** DESCR("greater than or equal");
--- 171,184 ----
DATA(insert OID = 410 ( "=" PGNSP PGUID b t t 20 20 16 410 411 int8eq eqsel eqjoinsel ));
DESCR("equal");
+ #define INT8EQ 410
DATA(insert OID = 411 ( "<>" PGNSP PGUID b f f 20 20 16 411 410 int8ne neqsel neqjoinsel ));
DESCR("not equal");
DATA(insert OID = 412 ( "<" PGNSP PGUID b f f 20 20 16 413 415 int8lt scalarltsel scalarltjoinsel ));
DESCR("less than");
DATA(insert OID = 413 ( ">" PGNSP PGUID b f f 20 20 16 412 414 int8gt scalargtsel scalargtjoinsel ));
DESCR("greater than");
+ #define INT8GT 413
DATA(insert OID = 414 ( "<=" PGNSP PGUID b f f 20 20 16 415 413 int8le scalarltsel scalarltjoinsel ));
DESCR("less than or equal");
DATA(insert OID = 415 ( ">=" PGNSP PGUID b f f 20 20 16 414 412 int8ge scalargtsel scalargtjoinsel ));
*** a/src/include/optimizer/pathnode.h
--- b/src/include/optimizer/pathnode.h
***************
*** 70,76 **** extern UniquePath *create_unique_path(PlannerInfo *root, RelOptInfo *rel,
extern Path *create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
List *pathkeys, Relids required_outer);
extern Path *create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
! Relids required_outer);
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel,
Relids required_outer);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel,
--- 70,76 ----
extern Path *create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
List *pathkeys, Relids required_outer);
extern Path *create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
! Relids required_outer, bool funcordinality);
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel,
Relids required_outer);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel,
*** a/src/include/optimizer/paths.h
--- b/src/include/optimizer/paths.h
***************
*** 174,179 **** extern List *build_join_pathkeys(PlannerInfo *root,
--- 174,182 ----
extern List *make_pathkeys_for_sortclauses(PlannerInfo *root,
List *sortclauses,
List *tlist);
+ extern List * make_pathkeys_for_functionscan(PlannerInfo *root,
+ Expr *expr,
+ Oid sortop);
extern void initialize_mergeclause_eclasses(PlannerInfo *root,
RestrictInfo *restrictinfo);
extern void update_mergeclause_eclasses(PlannerInfo *root,
However it occurs to me that the plan isn't ideal:
postgres=# explain select * from generate_series(1,10) with ordinality
as a(a,o) natural full outer join generate_series(1,5) with ordinality
as b(b,o) ;
QUERY PLAN
----------------------------------------------------------------------
----------------- Merge Full Join (cost=119.66..199.66 rows=5000
width=24)
Merge Cond: (a.o = b.o)
-> Sort (cost=59.83..62.33 rows=1000 width=12)
Sort Key: a.o
-> Function Scan on generate_series a (cost=0.00..10.00
rows=1000 width=12)
-> Sort (cost=59.83..62.33 rows=1000 width=12)
Sort Key: b.o
-> Function Scan on generate_series b (cost=0.00..10.00
rows=1000 width=12)
(8 rows)
I think all that's required to avoid the sorts is teaching the planner
that the Path has a PathKey of the ordinal column. I can look at that
later but I'll go ahead and commit it without it at first. I wonder if
it's also useful to teach the planner that the column is unique.So I took a crack at this. But it's not working.
To generate the pathkey for the ordinality column I basically exported
make_pathkey_from_sortop() through a thin wrapper called
make_pathkey_for_functionscan() and passed the Var from the reltargetlist and
hard coded int8gt's oid. That might stand to be generalized a bit but in any
case it doesn't matter because it doesn't work.
I think the patch is buggy in some ways. I've reworked on the patch. (I did
nothing on the uniqueness of an ordinality column. We should do something on
that?) Attached is a reworked version of the patch. With the reworked version,
we get a plan for the above query:
postgres=# EXPLAIN SELECT * FROM generate_series(1,10) WITH ORDINALITY AS a(a,o)
NATURAL FULL OUTER JOIN generate_series(1,5) WITH ORDINALITY AS b(b,o);
QUERY PLAN
--------------------------------------------------------------------------------
-------
Merge Full Join (cost=0.01..97.50 rows=5000 width=24)
Merge Cond: (a.o = b.o)
-> Function Scan on generate_series a (cost=0.00..10.00 rows=1000 width=12)
-> Materialize (cost=0.00..12.50 rows=1000 width=12)
-> Function Scan on generate_series b (cost=0.00..10.00 rows=1000
width=12)
(5 rows)
My best guess of what's going wrong is that the eclass is being generated too
late and instead of getting the canonical eclass it's getting a freshly minted
one that doesn't match the eclass for the merge join pathkey. But as I said,
that's just a guess. I'm missing a high level view of the order of operations
between turning parser column references into vars into eclasses and what has
to be done when. My best guess is that what's needed is marking the ordinality
column with a sortref early on but again, as I said, I'm just guessing.
You can find the discussion about this in src/backend/optimizer/README (Part:
Order of processing for EquivalenceClasses and PathKeys).
Thanks,
Best regards,
Etsuro Fujita
Attachments:
ordinality-path-20130813.patchapplication/octet-stream; name=ordinality-path-20130813.patchDownload
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71e3058..d113c71 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1985,6 +1985,7 @@ _copyRangeTblEntry(const RangeTblEntry *from)
COPY_NODE_FIELD(funccoltypmods);
COPY_NODE_FIELD(funccolcollations);
COPY_SCALAR_FIELD(funcordinality);
+ COPY_SCALAR_FIELD(funcordinality_attno);
COPY_NODE_FIELD(values_lists);
COPY_NODE_FIELD(values_collations);
COPY_STRING_FIELD(ctename);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 3183ccf..8d2ce43 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2236,6 +2236,7 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
COMPARE_NODE_FIELD(funccoltypmods);
COMPARE_NODE_FIELD(funccolcollations);
COMPARE_SCALAR_FIELD(funcordinality);
+ COMPARE_SCALAR_FIELD(funcordinality_attno);
COMPARE_NODE_FIELD(values_lists);
COMPARE_NODE_FIELD(values_collations);
COMPARE_STRING_FIELD(ctename);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index bad2239..a2e12ac 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2384,6 +2384,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
WRITE_NODE_FIELD(funccoltypmods);
WRITE_NODE_FIELD(funccolcollations);
WRITE_BOOL_FIELD(funcordinality);
+ WRITE_INT_FIELD(funcordinality_attno);
break;
case RTE_VALUES:
WRITE_NODE_FIELD(values_lists);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index aad63e5..eb36a57 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1224,6 +1224,7 @@ _readRangeTblEntry(void)
READ_NODE_FIELD(funccoltypmods);
READ_NODE_FIELD(funccolcollations);
READ_BOOL_FIELD(funcordinality);
+ READ_INT_FIELD(funcordinality_attno);
break;
case RTE_VALUES:
READ_NODE_FIELD(values_lists);
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 4b8a73d..63d53bf 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1259,6 +1259,7 @@ static void
set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
Relids required_outer;
+ List *pathkeys;
/*
* We don't support pushing join clauses into the quals of a function
@@ -1267,8 +1268,11 @@ set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
*/
required_outer = rel->lateral_relids;
+ /* Generate pathkeys for a function scan */
+ pathkeys = build_function_pathkeys(root, rel, rte);
+
/* Generate appropriate path */
- add_path(rel, create_functionscan_path(root, rel, required_outer));
+ add_path(rel, create_functionscan_path(root, rel, pathkeys, required_outer));
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 6724996..9f3f261 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -18,6 +18,7 @@
#include "postgres.h"
#include "access/skey.h"
+#include "catalog/pg_type.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "nodes/plannodes.h"
@@ -26,6 +27,7 @@
#include "optimizer/paths.h"
#include "optimizer/tlist.h"
#include "utils/lsyscache.h"
+#include "utils/typcache.h"
static PathKey *make_canonical_pathkey(PlannerInfo *root,
@@ -699,6 +701,64 @@ convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
}
/*
+ * build_function_pathkeys
+ * Generate a single-PathKey list that represents the sort order specified
+ * by a specific column of a function scan.
+ */
+List *
+build_function_pathkeys(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
+{
+ TypeCacheEntry *typentry;
+ Oid sortop;
+ Oid opfamily;
+ Oid opcintype;
+ int16 strategy;
+ Expr *expr;
+ PathKey *cpathkey = NULL;
+
+ if (rte->funcordinality == false ||
+ has_useful_pathkeys(root, rel) == false)
+ return NIL;
+
+ Assert(rte->funcordinality_attno > 0);
+
+ /* Find the sort operator using the type cache */
+ typentry = lookup_type_cache(INT8OID, TYPECACHE_LT_OPR);
+ sortop = typentry->lt_opr;
+
+ /* Find the operator in pg_amop --- failure shouldn't happen */
+ if (!get_ordering_op_properties(sortop,
+ &opfamily, &opcintype, &strategy))
+ elog(ERROR, "operator %u is not a valid ordering operator",
+ sortop);
+
+ /* Build a representation for ordinality column */
+ expr = (Expr *) makeVar(rel->relid,
+ rte->funcordinality_attno,
+ INT8OID,
+ -1,
+ InvalidOid,
+ 0);
+
+ /* OK, Try to make a canonical pathkey for ordinality column */
+ cpathkey = make_pathkey_from_sortinfo(root,
+ expr,
+ opfamily,
+ opcintype,
+ InvalidOid,
+ false,
+ false,
+ 0,
+ rel->relids,
+ false);
+
+ if (cpathkey != NULL)
+ return truncate_useless_pathkeys(root, rel, list_make1(cpathkey));
+ else
+ return NIL;
+}
+
+/*
* build_join_pathkeys
* Build the path keys for a join relation constructed by mergejoin or
* nestloop join. This is normally the same as the outer path's keys.
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 64b1705..a7169ef 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1623,7 +1623,7 @@ create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
*/
Path *
create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
- Relids required_outer)
+ List *pathkeys, Relids required_outer)
{
Path *pathnode = makeNode(Path);
@@ -1631,7 +1631,7 @@ create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
pathnode->parent = rel;
pathnode->param_info = get_baserel_parampathinfo(root, rel,
required_outer);
- pathnode->pathkeys = NIL; /* for now, assume unordered result */
+ pathnode->pathkeys = pathkeys;
cost_functionscan(pathnode, root, rel, pathnode->param_info);
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 39922d3..dd53c7a 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1228,6 +1228,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
Alias *alias = rangefunc->alias;
List *coldeflist = rangefunc->coldeflist;
Alias *eref;
+ AttrNumber ordinality_attno = 0;
rte->rtekind = RTE_FUNCTION;
rte->relid = InvalidOid;
@@ -1275,11 +1276,15 @@ addRangeTableEntryForFunction(ParseState *pstate,
Assert(tupdesc);
/* Build the column alias list */
buildRelationAliases(tupdesc, alias, eref, rangefunc->ordinality);
+
+ ordinality_attno = tupdesc->natts + 1;
}
else if (functypclass == TYPEFUNC_SCALAR)
{
/* Base data type, i.e. scalar */
buildScalarFunctionAlias(funcexpr, funcname, alias, eref, rangefunc->ordinality);
+
+ ordinality_attno = 2;
}
else if (functypclass == TYPEFUNC_RECORD)
{
@@ -1334,6 +1339,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
*/
rte->lateral = lateral;
rte->funcordinality = rangefunc->ordinality;
+ rte->funcordinality_attno = ordinality_attno;
rte->inh = false; /* never true for functions */
rte->inFromCl = inFromCl;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 51fef68..49a7017 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -772,6 +772,7 @@ typedef struct RangeTblEntry
List *funccoltypmods; /* integer list of column typmods */
List *funccolcollations; /* OID list of column collation OIDs */
bool funcordinality; /* is this called WITH ORDINALITY? */
+ AttrNumber funcordinality_attno; /* attno of ordinality column */
/*
* Fields valid for a values RTE (else NIL):
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index bc68789..bc9bc34 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -70,7 +70,7 @@ extern UniquePath *create_unique_path(PlannerInfo *root, RelOptInfo *rel,
extern Path *create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
List *pathkeys, Relids required_outer);
extern Path *create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
- Relids required_outer);
+ List *pathkeys, Relids required_outer);
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel,
Relids required_outer);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel,
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 9ef93c7..3a2ace8 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -167,6 +167,8 @@ extern List *build_index_pathkeys(PlannerInfo *root, IndexOptInfo *index,
ScanDirection scandir);
extern List *convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
List *subquery_pathkeys);
+extern List *build_function_pathkeys(PlannerInfo *root, RelOptInfo *rel,
+ RangeTblEntry *rte);
extern List *build_join_pathkeys(PlannerInfo *root,
RelOptInfo *joinrel,
JoinType jointype,
I wrote:
I've reworked on the patch.
Attached is an updated version of the patch. In that version the code for the
newly added function build_function_pathkeys() has been made more simple by
using the macro INTEGER_BTREE_FAM_OID.
Thanks,
Best regards,
Etsuro Fujita
Attachments:
ordinality-path-20130815.patchapplication/octet-stream; name=ordinality-path-20130815.patchDownload
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71e3058..d113c71 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1985,6 +1985,7 @@ _copyRangeTblEntry(const RangeTblEntry *from)
COPY_NODE_FIELD(funccoltypmods);
COPY_NODE_FIELD(funccolcollations);
COPY_SCALAR_FIELD(funcordinality);
+ COPY_SCALAR_FIELD(funcordinality_attno);
COPY_NODE_FIELD(values_lists);
COPY_NODE_FIELD(values_collations);
COPY_STRING_FIELD(ctename);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 3183ccf..8d2ce43 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2236,6 +2236,7 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
COMPARE_NODE_FIELD(funccoltypmods);
COMPARE_NODE_FIELD(funccolcollations);
COMPARE_SCALAR_FIELD(funcordinality);
+ COMPARE_SCALAR_FIELD(funcordinality_attno);
COMPARE_NODE_FIELD(values_lists);
COMPARE_NODE_FIELD(values_collations);
COMPARE_STRING_FIELD(ctename);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index bad2239..a2e12ac 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2384,6 +2384,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
WRITE_NODE_FIELD(funccoltypmods);
WRITE_NODE_FIELD(funccolcollations);
WRITE_BOOL_FIELD(funcordinality);
+ WRITE_INT_FIELD(funcordinality_attno);
break;
case RTE_VALUES:
WRITE_NODE_FIELD(values_lists);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index aad63e5..eb36a57 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1224,6 +1224,7 @@ _readRangeTblEntry(void)
READ_NODE_FIELD(funccoltypmods);
READ_NODE_FIELD(funccolcollations);
READ_BOOL_FIELD(funcordinality);
+ READ_INT_FIELD(funcordinality_attno);
break;
case RTE_VALUES:
READ_NODE_FIELD(values_lists);
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 4b8a73d..63d53bf 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1259,6 +1259,7 @@ static void
set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
Relids required_outer;
+ List *pathkeys;
/*
* We don't support pushing join clauses into the quals of a function
@@ -1267,8 +1268,11 @@ set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
*/
required_outer = rel->lateral_relids;
+ /* Generate pathkeys for a function scan */
+ pathkeys = build_function_pathkeys(root, rel, rte);
+
/* Generate appropriate path */
- add_path(rel, create_functionscan_path(root, rel, required_outer));
+ add_path(rel, create_functionscan_path(root, rel, pathkeys, required_outer));
/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 6724996..6a701fa 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -18,6 +18,8 @@
#include "postgres.h"
#include "access/skey.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_type.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "nodes/plannodes.h"
@@ -699,6 +701,51 @@ convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
}
/*
+ * build_function_pathkeys
+ * Generate a single-PathKey list that represents the sort order specified
+ * by a specific column of a function scan.
+ */
+List *
+build_function_pathkeys(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
+{
+ Expr *expr;
+ PathKey *cpathkey = NULL;
+
+ if (has_useful_pathkeys(root, rel) == false)
+ return NIL;
+
+ if (rte->funcordinality == false)
+ return NIL;
+
+ Assert(rte->funcordinality_attno > 0);
+
+ /* Build a representation for ordinality column */
+ expr = (Expr *) makeVar(rel->relid,
+ rte->funcordinality_attno,
+ INT8OID,
+ -1,
+ InvalidOid,
+ 0);
+
+ /* OK, Try to make a canonical pathkey for ordinality column */
+ cpathkey = make_pathkey_from_sortinfo(root,
+ expr,
+ INTEGER_BTREE_FAM_OID,
+ INT8OID,
+ InvalidOid,
+ false,
+ false,
+ 0,
+ rel->relids,
+ false);
+
+ if (cpathkey != NULL)
+ return truncate_useless_pathkeys(root, rel, list_make1(cpathkey));
+ else
+ return NIL;
+}
+
+/*
* build_join_pathkeys
* Build the path keys for a join relation constructed by mergejoin or
* nestloop join. This is normally the same as the outer path's keys.
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 64b1705..a7169ef 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1623,7 +1623,7 @@ create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
*/
Path *
create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
- Relids required_outer)
+ List *pathkeys, Relids required_outer)
{
Path *pathnode = makeNode(Path);
@@ -1631,7 +1631,7 @@ create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
pathnode->parent = rel;
pathnode->param_info = get_baserel_parampathinfo(root, rel,
required_outer);
- pathnode->pathkeys = NIL; /* for now, assume unordered result */
+ pathnode->pathkeys = pathkeys;
cost_functionscan(pathnode, root, rel, pathnode->param_info);
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 39922d3..dd53c7a 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1228,6 +1228,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
Alias *alias = rangefunc->alias;
List *coldeflist = rangefunc->coldeflist;
Alias *eref;
+ AttrNumber ordinality_attno = 0;
rte->rtekind = RTE_FUNCTION;
rte->relid = InvalidOid;
@@ -1275,11 +1276,15 @@ addRangeTableEntryForFunction(ParseState *pstate,
Assert(tupdesc);
/* Build the column alias list */
buildRelationAliases(tupdesc, alias, eref, rangefunc->ordinality);
+
+ ordinality_attno = tupdesc->natts + 1;
}
else if (functypclass == TYPEFUNC_SCALAR)
{
/* Base data type, i.e. scalar */
buildScalarFunctionAlias(funcexpr, funcname, alias, eref, rangefunc->ordinality);
+
+ ordinality_attno = 2;
}
else if (functypclass == TYPEFUNC_RECORD)
{
@@ -1334,6 +1339,7 @@ addRangeTableEntryForFunction(ParseState *pstate,
*/
rte->lateral = lateral;
rte->funcordinality = rangefunc->ordinality;
+ rte->funcordinality_attno = ordinality_attno;
rte->inh = false; /* never true for functions */
rte->inFromCl = inFromCl;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 51fef68..49a7017 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -772,6 +772,7 @@ typedef struct RangeTblEntry
List *funccoltypmods; /* integer list of column typmods */
List *funccolcollations; /* OID list of column collation OIDs */
bool funcordinality; /* is this called WITH ORDINALITY? */
+ AttrNumber funcordinality_attno; /* attno of ordinality column */
/*
* Fields valid for a values RTE (else NIL):
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 9686229..0033a3c 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -70,7 +70,7 @@ extern UniquePath *create_unique_path(PlannerInfo *root, RelOptInfo *rel,
extern Path *create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel,
List *pathkeys, Relids required_outer);
extern Path *create_functionscan_path(PlannerInfo *root, RelOptInfo *rel,
- Relids required_outer);
+ List *pathkeys, Relids required_outer);
extern Path *create_valuesscan_path(PlannerInfo *root, RelOptInfo *rel,
Relids required_outer);
extern Path *create_ctescan_path(PlannerInfo *root, RelOptInfo *rel,
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 9ef93c7..3a2ace8 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -167,6 +167,8 @@ extern List *build_index_pathkeys(PlannerInfo *root, IndexOptInfo *index,
ScanDirection scandir);
extern List *convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
List *subquery_pathkeys);
+extern List *build_function_pathkeys(PlannerInfo *root, RelOptInfo *rel,
+ RangeTblEntry *rte);
extern List *build_join_pathkeys(PlannerInfo *root,
RelOptInfo *joinrel,
JoinType jointype,
On Thu, Aug 15, 2013 at 07:25:17PM +0900, Etsuro Fujita wrote:
I wrote:
I've reworked on the patch.
Attached is an updated version of the patch. In that version the code for the
newly added function build_function_pathkeys() has been made more simple by
using the macro INTEGER_BTREE_FAM_OID.
Is this patch to be applied?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian <bruce@momjian.us> writes:
On Thu, Aug 15, 2013 at 07:25:17PM +0900, Etsuro Fujita wrote:
Attached is an updated version of the patch. In that version the code for the
newly added function build_function_pathkeys() has been made more simple by
using the macro INTEGER_BTREE_FAM_OID.
Is this patch to be applied?
It hasn't been reviewed AFAIK.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(2014/02/01 8:01), Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Thu, Aug 15, 2013 at 07:25:17PM +0900, Etsuro Fujita wrote:
Attached is an updated version of the patch. In that version the code for the
newly added function build_function_pathkeys() has been made more simple by
using the macro INTEGER_BTREE_FAM_OID.Is this patch to be applied?
It hasn't been reviewed AFAIK.
Thanks for picking this up.
I think it doesn't need to be reviewed anymore because the same feature
has been commited in [1]/messages/by-id/E1Vjel1-0007x9-TR@gemulon.postgresql.org if I understand correctly.
[1]: /messages/by-id/E1Vjel1-0007x9-TR@gemulon.postgresql.org
/messages/by-id/E1Vjel1-0007x9-TR@gemulon.postgresql.org
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers