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, 44 insertions, 50 deletions

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 5112cac9017..51391f6a4e0 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -32,7 +32,6 @@
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_proc.h"
-#include "catalog/pg_tablesample_method.h"
 #include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
@@ -349,8 +348,6 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 			 int prettyFlags);
 static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 			 int prettyFlags, int wrapColumn);
-static void get_tablesample_def(TableSampleClause *tablesample,
-					deparse_context *context);
 static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
 			  TupleDesc resultDesc,
 			  int prettyFlags, int wrapColumn, int startIndent);
@@ -416,6 +413,8 @@ static void get_column_alias_list(deparse_columns *colinfo,
 static void get_from_clause_coldeflist(RangeTblFunction *rtfunc,
 						   deparse_columns *colinfo,
 						   deparse_context *context);
+static void get_tablesample_def(TableSampleClause *tablesample,
+					deparse_context *context);
 static void get_opclass_name(Oid opclass, Oid actual_datatype,
 				 StringInfo buf);
 static Node *processIndirection(Node *node, deparse_context *context,
@@ -4235,50 +4234,6 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 	heap_close(ev_relation, AccessShareLock);
 }
 
-/* ----------
- * get_tablesample_def			- Convert TableSampleClause back to SQL
- * ----------
- */
-static void
-get_tablesample_def(TableSampleClause *tablesample, deparse_context *context)
-{
-	StringInfo	buf = context->buf;
-	HeapTuple	tuple;
-	Form_pg_tablesample_method tsm;
-	char	   *tsmname;
-	int			nargs;
-	ListCell   *l;
-
-	/* Load the tablesample method */
-	tuple = SearchSysCache1(TABLESAMPLEMETHODOID, ObjectIdGetDatum(tablesample->tsmid));
-	if (!HeapTupleIsValid(tuple))
-		ereport(ERROR,
-				(errcode(ERRCODE_UNDEFINED_OBJECT),
-				 errmsg("cache lookup failed for tablesample method %u",
-						tablesample->tsmid)));
-
-	tsm = (Form_pg_tablesample_method) GETSTRUCT(tuple);
-	tsmname = NameStr(tsm->tsmname);
-	appendStringInfo(buf, " TABLESAMPLE %s (", quote_identifier(tsmname));
-
-	ReleaseSysCache(tuple);
-
-	nargs = 0;
-	foreach(l, tablesample->args)
-	{
-		if (nargs++ > 0)
-			appendStringInfoString(buf, ", ");
-		get_rule_expr((Node *) lfirst(l), context, true);
-	}
-	appendStringInfoChar(buf, ')');
-
-	if (tablesample->repeatable != NULL)
-	{
-		appendStringInfoString(buf, " REPEATABLE (");
-		get_rule_expr(tablesample->repeatable, context, true);
-		appendStringInfoChar(buf, ')');
-	}
-}
 
 /* ----------
  * get_query_def			- Parse back one query parsetree
@@ -8781,9 +8736,6 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
 								 only_marker(rte),
 								 generate_relation_name(rte->relid,
 														context->namespaces));
-
-				if (rte->tablesample)
-					get_tablesample_def(rte->tablesample, context);
 				break;
 			case RTE_SUBQUERY:
 				/* Subquery RTE */
@@ -8963,6 +8915,10 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
 			/* Else print column aliases as needed */
 			get_column_alias_list(colinfo, context);
 		}
+
+		/* Tablesample clause must go after any alias */
+		if (rte->rtekind == RTE_RELATION && rte->tablesample)
+			get_tablesample_def(rte->tablesample, context);
 	}
 	else if (IsA(jtnode, JoinExpr))
 	{
@@ -9163,6 +9119,44 @@ get_from_clause_coldeflist(RangeTblFunction *rtfunc,
 }
 
 /*
+ * get_tablesample_def			- print a TableSampleClause
+ */
+static void
+get_tablesample_def(TableSampleClause *tablesample, deparse_context *context)
+{
+	StringInfo	buf = context->buf;
+	Oid			argtypes[1];
+	int			nargs;
+	ListCell   *l;
+
+	/*
+	 * We should qualify the handler's function name if it wouldn't be
+	 * resolved by lookup in the current search path.
+	 */
+	argtypes[0] = INTERNALOID;
+	appendStringInfo(buf, " TABLESAMPLE %s (",
+					 generate_function_name(tablesample->tsmhandler, 1,
+											NIL, argtypes,
+											false, NULL, EXPR_KIND_NONE));
+
+	nargs = 0;
+	foreach(l, tablesample->args)
+	{
+		if (nargs++ > 0)
+			appendStringInfoString(buf, ", ");
+		get_rule_expr((Node *) lfirst(l), context, false);
+	}
+	appendStringInfoChar(buf, ')');
+
+	if (tablesample->repeatable != NULL)
+	{
+		appendStringInfoString(buf, " REPEATABLE (");
+		get_rule_expr((Node *) tablesample->repeatable, context, false);
+		appendStringInfoChar(buf, ')');
+	}
+}
+
+/*
  * get_opclass_name			- fetch name of an index operator class
  *
  * The opclass name is appended (after a space) to buf.