Redesign tablesample method API, and do extensive code review.

The original implementation of TABLESAMPLE modeled the tablesample method
API on index access methods, which wasn't a good choice because, without
specialized DDL commands, there's no way to build an extension that can
implement a TSM.  (Raw inserts into system catalogs are not an acceptable
thing to do, because we can't undo them during DROP EXTENSION, nor will
pg_upgrade behave sanely.)  Instead adopt an API more like procedural
language handlers or foreign data wrappers, wherein the only SQL-level
support object needed is a single handler function identified by having
a special return type.  This lets us get rid of the supporting catalog
altogether, so that no custom DDL support is needed for the feature.

Adjust the API so that it can support non-constant tablesample arguments
(the original coding assumed we could evaluate the argument expressions at
ExecInitSampleScan time, which is undesirable even if it weren't outright
unsafe), and discourage sampling methods from looking at invisible tuples.
Make sure that the BERNOULLI and SYSTEM methods are genuinely repeatable
within and across queries, as required by the SQL standard, and deal more
honestly with methods that can't support that requirement.

Make a full code-review pass over the tablesample additions, and fix
assorted bugs, omissions, infelicities, and cosmetic issues (such as
failure to put the added code stanzas in a consistent ordering).
Improve EXPLAIN's output of tablesample plans, too.

Back-patch to 9.5 so that we don't have to support the original API
in production.

1 files changed, 134 insertions, 56 deletions

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index e90e1d68e3a..4e490b23b4e 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -18,8 +18,8 @@
 #include "miscadmin.h"
 
 #include "access/heapam.h"
+#include "access/tsmapi.h"
 #include "catalog/catalog.h"
-#include "access/htup_details.h"
 #include "catalog/heap.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_type.h"
@@ -43,7 +43,7 @@
 #include "utils/guc.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
-#include "utils/syscache.h"
+
 
 /* Convenience macro for the most common makeNamespaceItem() case */
 #define makeDefaultNSItem(rte)	makeNamespaceItem(rte, true, true, false, true)
@@ -63,6 +63,8 @@ static RangeTblEntry *transformRangeSubselect(ParseState *pstate,
 						RangeSubselect *r);
 static RangeTblEntry *transformRangeFunction(ParseState *pstate,
 					   RangeFunction *r);
+static TableSampleClause *transformRangeTableSample(ParseState *pstate,
+						  RangeTableSample *rts);
 static Node *transformFromClauseItem(ParseState *pstate, Node *n,
 						RangeTblEntry **top_rte, int *top_rti,
 						List **namespace);
@@ -423,40 +425,6 @@ transformJoinOnClause(ParseState *pstate, JoinExpr *j, List *namespace)
 	return result;
 }
 
-static RangeTblEntry *
-transformTableSampleEntry(ParseState *pstate, RangeTableSample *rv)
-{
-	RangeTblEntry *rte = NULL;
-	CommonTableExpr *cte = NULL;
-	TableSampleClause *tablesample = NULL;
-
-	/* if relation has an unqualified name, it might be a CTE reference */
-	if (!rv->relation->schemaname)
-	{
-		Index		levelsup;
-
-		cte = scanNameSpaceForCTE(pstate, rv->relation->relname, &levelsup);
-	}
-
-	/* We first need to build a range table entry */
-	if (!cte)
-		rte = transformTableEntry(pstate, rv->relation);
-
-	if (!rte ||
-		(rte->relkind != RELKIND_RELATION &&
-		 rte->relkind != RELKIND_MATVIEW))
-		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("TABLESAMPLE clause can only be used on tables and materialized views"),
-				 parser_errposition(pstate, rv->relation->location)));
-
-	tablesample = ParseTableSample(pstate, rv->method, rv->repeatable,
-								   rv->args, rv->relation->location);
-	rte->tablesample = tablesample;
-
-	return rte;
-}
-
 /*
  * transformTableEntry --- transform a RangeVar (simple relation reference)
  */
@@ -748,6 +716,109 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
 	return rte;
 }
 
+/*
+ * transformRangeTableSample --- transform a TABLESAMPLE clause
+ *
+ * Caller has already transformed rts->relation, we just have to validate
+ * the remaining fields and create a TableSampleClause node.
+ */
+static TableSampleClause *
+transformRangeTableSample(ParseState *pstate, RangeTableSample *rts)
+{
+	TableSampleClause *tablesample;
+	Oid			handlerOid;
+	Oid			funcargtypes[1];
+	TsmRoutine *tsm;
+	List	   *fargs;
+	ListCell   *larg,
+			   *ltyp;
+
+	/*
+	 * To validate the sample method name, look up the handler function, which
+	 * has the same name, one dummy INTERNAL argument, and a result type of
+	 * tsm_handler.  (Note: tablesample method names are not schema-qualified
+	 * in the SQL standard; but since they are just functions to us, we allow
+	 * schema qualification to resolve any potential ambiguity.)
+	 */
+	funcargtypes[0] = INTERNALOID;
+
+	handlerOid = LookupFuncName(rts->method, 1, funcargtypes, true);
+
+	/* we want error to complain about no-such-method, not no-such-function */
+	if (!OidIsValid(handlerOid))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("tablesample method %s does not exist",
+						NameListToString(rts->method)),
+				 parser_errposition(pstate, rts->location)));
+
+	/* check that handler has correct return type */
+	if (get_func_rettype(handlerOid) != TSM_HANDLEROID)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("function %s must return type \"tsm_handler\"",
+						NameListToString(rts->method)),
+				 parser_errposition(pstate, rts->location)));
+
+	/* OK, run the handler to get TsmRoutine, for argument type info */
+	tsm = GetTsmRoutine(handlerOid);
+
+	tablesample = makeNode(TableSampleClause);
+	tablesample->tsmhandler = handlerOid;
+
+	/* check user provided the expected number of arguments */
+	if (list_length(rts->args) != list_length(tsm->parameterTypes))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_TABLESAMPLE_ARGUMENT),
+		  errmsg_plural("tablesample method %s requires %d argument, not %d",
+						"tablesample method %s requires %d arguments, not %d",
+						list_length(tsm->parameterTypes),
+						NameListToString(rts->method),
+						list_length(tsm->parameterTypes),
+						list_length(rts->args)),
+				 parser_errposition(pstate, rts->location)));
+
+	/*
+	 * Transform the arguments, typecasting them as needed.  Note we must also
+	 * assign collations now, because assign_query_collations() doesn't
+	 * examine any substructure of RTEs.
+	 */
+	fargs = NIL;
+	forboth(larg, rts->args, ltyp, tsm->parameterTypes)
+	{
+		Node	   *arg = (Node *) lfirst(larg);
+		Oid			argtype = lfirst_oid(ltyp);
+
+		arg = transformExpr(pstate, arg, EXPR_KIND_FROM_FUNCTION);
+		arg = coerce_to_specific_type(pstate, arg, argtype, "TABLESAMPLE");
+		assign_expr_collations(pstate, arg);
+		fargs = lappend(fargs, arg);
+	}
+	tablesample->args = fargs;
+
+	/* Process REPEATABLE (seed) */
+	if (rts->repeatable != NULL)
+	{
+		Node	   *arg;
+
+		if (!tsm->repeatable_across_queries)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				  errmsg("tablesample method %s does not support REPEATABLE",
+						 NameListToString(rts->method)),
+					 parser_errposition(pstate, rts->location)));
+
+		arg = transformExpr(pstate, rts->repeatable, EXPR_KIND_FROM_FUNCTION);
+		arg = coerce_to_specific_type(pstate, arg, FLOAT8OID, "REPEATABLE");
+		assign_expr_collations(pstate, arg);
+		tablesample->repeatable = (Expr *) arg;
+	}
+	else
+		tablesample->repeatable = NULL;
+
+	return tablesample;
+}
+
 
 /*
  * transformFromClauseItem -
@@ -844,6 +915,33 @@ transformFromClauseItem(ParseState *pstate, Node *n,
 		rtr->rtindex = rtindex;
 		return (Node *) rtr;
 	}
+	else if (IsA(n, RangeTableSample))
+	{
+		/* TABLESAMPLE clause (wrapping some other valid FROM node) */
+		RangeTableSample *rts = (RangeTableSample *) n;
+		Node	   *rel;
+		RangeTblRef *rtr;
+		RangeTblEntry *rte;
+
+		/* Recursively transform the contained relation */
+		rel = transformFromClauseItem(pstate, rts->relation,
+									  top_rte, top_rti, namespace);
+		/* Currently, grammar could only return a RangeVar as contained rel */
+		Assert(IsA(rel, RangeTblRef));
+		rtr = (RangeTblRef *) rel;
+		rte = rt_fetch(rtr->rtindex, pstate->p_rtable);
+		/* We only support this on plain relations and matviews */
+		if (rte->relkind != RELKIND_RELATION &&
+			rte->relkind != RELKIND_MATVIEW)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("TABLESAMPLE clause can only be applied to tables and materialized views"),
+				   parser_errposition(pstate, exprLocation(rts->relation))));
+
+		/* Transform TABLESAMPLE details and attach to the RTE */
+		rte->tablesample = transformRangeTableSample(pstate, rts);
+		return (Node *) rtr;
+	}
 	else if (IsA(n, JoinExpr))
 	{
 		/* A newfangled join expression */
@@ -1165,26 +1263,6 @@ transformFromClauseItem(ParseState *pstate, Node *n,
 
 		return (Node *) j;
 	}
-	else if (IsA(n, RangeTableSample))
-	{
-		/* Tablesample reference */
-		RangeTableSample *rv = (RangeTableSample *) n;
-		RangeTblRef *rtr;
-		RangeTblEntry *rte = NULL;
-		int			rtindex;
-
-		rte = transformTableSampleEntry(pstate, rv);
-
-		/* assume new rte is at end */
-		rtindex = list_length(pstate->p_rtable);
-		Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-		*top_rte = rte;
-		*top_rti = rtindex;
-		*namespace = list_make1(makeDefaultNSItem(rte));
-		rtr = makeNode(RangeTblRef);
-		rtr->rtindex = rtindex;
-		return (Node *) rtr;
-	}
 	else
 		elog(ERROR, "unrecognized node type: %d", (int) nodeTag(n));
 	return NULL;				/* can't get here, keep compiler quiet */