diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index ea2139fbfa..48bce9744d 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -2370,10 +2370,11 @@ deparseAnalyzeSizeSql(StringInfo buf, Relation rel)
}
/*
- * Construct SELECT statement to acquire a number of rows of a relation.
+ * Construct SELECT statement to acquire the number of rows of a relation.
*
- * Note: Maybe this should compare relpages and current relation size
- * and adjust reltuples accordingly?
+ * Note: we just return the remote server's reltuples value, which might
+ * be off a good deal, but it doesn't seem worth working harder. See
+ * comments in postgresAcquireSampleRowsFunc.
*/
void
deparseAnalyzeTuplesSql(StringInfo buf, Relation rel)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index e2df32830f..8c2a0c6ca1 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4972,10 +4972,6 @@ postgresAnalyzeForeignTable(Relation relation,
/*
* postgresCountTuplesForForeignTable
* Count tuples in foreign table (just get pg_class.reltuples).
- *
- * Note: It's unclear how accurate reltuples is, maybe size that using
- * relpages and simple assumptions (1 tuples per page, ...)? Using
- * tsm_system_rows wold make this somewhat unnecessary.
*/
static double
postgresCountTuplesForForeignTable(Relation relation)
@@ -4985,14 +4981,7 @@ postgresCountTuplesForForeignTable(Relation relation)
PGconn *conn;
StringInfoData sql;
PGresult *volatile res = NULL;
- double reltuples = -1;
-
- /*
- * Now we have to get the number of pages. It's annoying that the ANALYZE
- * API requires us to return that now, because it forces some duplication
- * of effort between this routine and postgresAcquireSampleRowsFunc. But
- * it's probably not worth redefining that API at this point.
- */
+ volatile double reltuples = -1;
/*
* Get the connection to use. We do the remote access as the table's
@@ -5016,7 +5005,7 @@ postgresCountTuplesForForeignTable(Relation relation)
pgfdw_report_error(ERROR, res, conn, false, sql.data);
if (PQntuples(res) != 1 || PQnfields(res) != 1)
- elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query");
+ elog(ERROR, "unexpected result from deparseAnalyzeTuplesSql query");
reltuples = strtod(PQgetvalue(res, 0, 0), NULL);
}
PG_FINALLY();
@@ -5034,8 +5023,6 @@ postgresCountTuplesForForeignTable(Relation relation)
/*
* Acquire a random sample of rows from foreign table managed by postgres_fdw.
*
- * We fetch the whole table from the remote side and pick out some sample rows.
- *
* Selected rows are returned in the caller-allocated array rows[],
* which must have at least targrows entries.
* The actual number of rows selected is returned as the function result.
@@ -5058,16 +5045,14 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
ForeignServer *server;
UserMapping *user;
PGconn *conn;
+ int server_version_num;
+ PgFdwSamplingMethod method = ANALYZE_SAMPLE_AUTO; /* auto is default */
+ double sample_frac = -1.0;
+ double reltuples;
unsigned int cursor_number;
StringInfoData sql;
PGresult *volatile res = NULL;
ListCell *lc;
- int server_version_num;
-
- /* analyze sampling enabled by default, if available */
- PgFdwSamplingMethod method = ANALYZE_SAMPLE_AUTO;
- double sample_frac = -1.0;
- double reltuples;
/* Initialize workspace state */
astate.rel = relation;
@@ -5099,7 +5084,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
server_version_num = PQserverVersion(conn);
/*
- * Should we try do the sampling for analyze on the remote server?
+ * What sampling method should we use?
*/
foreach(lc, server->options)
{
@@ -5107,7 +5092,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
if (strcmp(def->defname, "analyze_sampling") == 0)
{
- char *value = defGetString(def);
+ char *value = defGetString(def);
if (strcmp(value, "off") == 0)
method = ANALYZE_SAMPLE_OFF;
@@ -5130,7 +5115,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
if (strcmp(def->defname, "analyze_sampling") == 0)
{
- char *value = defGetString(def);
+ char *value = defGetString(def);
if (strcmp(value, "off") == 0)
method = ANALYZE_SAMPLE_OFF;
@@ -5173,7 +5158,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
/*
* If we've decided to do remote sampling, calculate the sampling rate. We
- * need to get the number of tuples from the remote server, so we skip the
+ * need to get the number of tuples from the remote server, but skip that
* network round-trip if not needed.
*/
if (method != ANALYZE_SAMPLE_OFF)
@@ -5181,37 +5166,42 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
reltuples = postgresCountTuplesForForeignTable(relation);
/*
- * No rows or we expect to sample everything - disable sampling after
- * all (and make sure we don't divide by 0 in sample_frac calculation.)
+ * Remote's reltuples could be 0 or -1 if the table has never been
+ * vacuumed/analyzed. In that case, or if we expect to need all the
+ * rows, disable sampling after all.
*/
if ((reltuples <= 0) || (targrows >= reltuples))
method = ANALYZE_SAMPLE_OFF;
+ else
+ {
+ sample_frac = targrows / reltuples;
- /* Make sure we don't divide by 0 when calculating the rate. */
- sample_frac = targrows / Max(1.0, reltuples);
-
- /*
- * Let's sample a bit more (10%), we'll reduce the sample locally.
- *
- * XXX Not sure this is really necessary. If we don't trust the remote
- * sampling to sample the right number of rows, we should not use it.
- */
- sample_frac *= 1.1;
+ /*
+ * Let's sample a bit more (10%) to compensate for any small
+ * inaccuracy in the reltuples value. We'll reduce the sample
+ * locally if it's too large. Of course, this won't fix a big
+ * error in reltuples --- but big errors in it are most commonly
+ * underestimates due to the table having grown since the last
+ * ANALYZE. That will lead to overestimating the required
+ * sample_frac, which is fine.
+ */
+ sample_frac *= 1.1;
- /*
- * Ensure the sampling rate is between 0.0 and 1.0, even after the
- * 10% adjustment above.
- */
- sample_frac = Min(1.0, Max(0.0, sample_frac));
+ /*
+ * Ensure the sampling rate is between 0.0 and 1.0, even after the
+ * 10% adjustment above. (Clamping to 0.0 is just paranoia.)
+ */
+ sample_frac = Min(1.0, Max(0.0, sample_frac));
- /*
- * If we expect the sampling to reduce very few rows, just disable it
- * and read the whole remote table. We decide based on the number of
- * rows we expect to "eliminate" by sampling. If saving than 100 rows,
- * we disable sampling.
- */
- if (reltuples * (1 - sample_frac) < 100.0)
- method = ANALYZE_SAMPLE_OFF;
+ /*
+ * If we expect sampling to remove very few rows, just disable it
+ * and read the whole remote table; the overhead won't be worth
+ * it. We decide based on the number of rows we expect sampling
+ * to eliminate. If saving fewer than 100 rows, disable sampling.
+ */
+ if (reltuples * (1 - sample_frac) < 100.0)
+ method = ANALYZE_SAMPLE_OFF;
+ }
}
/*
@@ -5315,9 +5305,9 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
*totaldeadrows = 0.0;
/*
- * Without ANALYZE sampling, we've retrieved all living tuples from foreign
- * server, so just use that. Otherwise we have the reltuples estimate we
- * got from the remote side.
+ * Without sampling, we've retrieved all living tuples from foreign
+ * server, so report that as totalrows. Otherwise use the reltuples
+ * estimate we got from the remote side.
*/
if (method == ANALYZE_SAMPLE_OFF)
*totalrows = astate.samplerows;
@@ -5330,7 +5320,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
ereport(elevel,
(errmsg("\"%s\": table contains %.0f rows, %d rows in sample",
RelationGetRelationName(relation),
- astate.samplerows, astate.numrows)));
+ *totalrows, astate.numrows)));
return astate.numrows;
}
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index d44c8cdd71..d681d1675e 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -305,28 +305,6 @@ OPTIONS (ADD password_required 'false');
-
- analyze_sampling (text)
-
-
- This option determines if ANALYZE on a foreign
- table samples the data on the remote node, or reads and transfers
- all data and performs the sampling locally. The supported values
- are off, random,
- system, bernoulli and
- auto. off disables remote
- sampling, so all data are transferred and sampled locally.
- random performs remote sampling using
- random() function, while system
- and bernoulli rely on built-in TABLESAMPLE
- methods. random works on all server versions,
- while TABLESAMPLE is supported only since 9.5.
- auto checks the server version and picks the
- best remote sampling method automatically.
-
-
-
-
@@ -348,6 +326,41 @@ OPTIONS (ADD password_required 'false');
frequently updated, the local statistics will soon be obsolete.
+
+ The following option controls how such an ANALYZE
+ operation behaves:
+
+
+
+
+
+ analyze_sampling (text)
+
+
+ This option, which can be specified for a foreign table or a foreign
+ server, determines if ANALYZE on a foreign table
+ samples the data on the remote side, or reads and transfers all data
+ and performs the sampling locally. The supported values
+ are off, random,
+ system, bernoulli
+ and auto. off disables remote
+ sampling, so all data are transferred and sampled locally.
+ random performs remote sampling using the
+ random() function to choose returned rows,
+ while system and bernoulli rely
+ on the built-in TABLESAMPLE methods of those
+ names. random works on all remote server versions,
+ while TABLESAMPLE is supported only since 9.5.
+ auto (the default) picks the recommended sampling
+ method automatically; currently it means
+ either bernoulli or random
+ depending on the remote server version.
+
+
+
+
+
+