Fix some planning oversights in postgres_fdw.

Include eval costs of local conditions in remote-estimate mode, and don't
assume the remote eval cost is zero in local-estimate mode.  (The best
we can do with that at the moment is to assume a seqscan, which may well
be wildly pessimistic ... but zero won't do at all.)

To get a reasonable local estimate, we need to know the relpages count
for the remote rel, so improve the ANALYZE code to fetch that rather
than just setting the foreign table's relpages field to zero.

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 0293115054f07e8a54f62020484b7939b9590ef6..9816f550ca57cd9af4e07bc2f2bc1c96cde50ec7 100644 (file)
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -456,6 +456,29 @@ appendWhereClause(StringInfo buf,
        }
 }
 
+/*
+ * Construct SELECT statement to acquire size in blocks of given relation.
+ *
+ * Note: we use local definition of block size, not remote definition.
+ * This is perhaps debatable.
+ *
+ * Note: pg_relation_size() exists in 8.1 and later.
+ */
+void
+deparseAnalyzeSizeSql(StringInfo buf, Relation rel)
+{
+       Oid                     relid = RelationGetRelid(rel);
+       StringInfoData relname;
+
+       /* We'll need the remote relation name as a literal. */
+       initStringInfo(&relname);
+       deparseRelation(&relname, relid);
+
+       appendStringInfo(buf, "SELECT pg_catalog.pg_relation_size(");
+       deparseStringLiteral(buf, relname.data);
+       appendStringInfo(buf, "::pg_catalog.regclass) / %d", BLCKSZ);
+}
+
 /*
  * Construct SELECT statement to acquire sample rows of given relation.
  *

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index a3256179f2bee9db0446dbc7c3125177fe0fd3b4..99dc6aef135c018616a08a13e12b12cf8fd9bed2 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -259,6 +259,9 @@ postgresGetForeignRelSize(PlannerInfo *root,
        int                     width;
        Cost            startup_cost;
        Cost            total_cost;
+       Cost            run_cost;
+       QualCost        qpqual_cost;
+       Cost            cpu_per_tuple;
        List       *remote_conds;
        List       *param_conds;
        List       *local_conds;
@@ -349,6 +352,16 @@ postgresGetForeignRelSize(PlannerInfo *root,
                sel *= clauselist_selectivity(root, local_conds,
                                                                          baserel->relid, JOIN_INNER, NULL);
 
+               /*
+                * Add in the eval cost of those conditions, too.
+                */
+               cost_qual_eval(&qpqual_cost, param_conds, root);
+               startup_cost += qpqual_cost.startup;
+               total_cost += qpqual_cost.per_tuple * rows;
+               cost_qual_eval(&qpqual_cost, local_conds, root);
+               startup_cost += qpqual_cost.startup;
+               total_cost += qpqual_cost.per_tuple * rows;
+
                /* Report estimated numbers to planner. */
                baserel->rows = clamp_row_est(rows * sel);
                baserel->width = width;
@@ -367,18 +380,25 @@ postgresGetForeignRelSize(PlannerInfo *root,
                 * estimate of 10 pages, and divide by the column-datatype-based width
                 * estimate to get the corresponding number of tuples.
                 */
-               if (baserel->tuples <= 0)
+               if (baserel->pages == 0 && baserel->tuples == 0)
+               {
+                       baserel->pages = 10;
                        baserel->tuples =
                                (10 * BLCKSZ) / (baserel->width + sizeof(HeapTupleHeaderData));
+               }
 
                set_baserel_size_estimates(root, baserel);
 
-               /*
-                * XXX need to do something here to calculate sane startup and total
-                * cost estimates ... for the moment, we do this:
-                */
+               /* Cost as though this were a seqscan, which is pessimistic. */
                startup_cost = 0;
-               total_cost = baserel->rows * cpu_tuple_cost;
+               run_cost = 0;
+               run_cost += seq_page_cost * baserel->pages;
+
+               startup_cost += baserel->baserestrictcost.startup;
+               cpu_per_tuple = cpu_tuple_cost + baserel->baserestrictcost.per_tuple;
+               run_cost += cpu_per_tuple * baserel->tuples;
+
+               total_cost = startup_cost + run_cost;
        }
 
        /*
@@ -1068,9 +1088,62 @@ postgresAnalyzeForeignTable(Relation relation,
                                                        AcquireSampleRowsFunc *func,
                                                        BlockNumber *totalpages)
 {
-       *totalpages = 0;                        /* XXX this is probably a bad idea */
+       ForeignTable *table;
+       ForeignServer *server;
+       UserMapping *user;
+       PGconn     *conn;
+       StringInfoData sql;
+       PGresult   *volatile res = NULL;
+
+       /* Return the row-analysis function pointer */
        *func = postgresAcquireSampleRowsFunc;
 
+       /*
+        * 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.
+        */
+
+       /*
+        * Get the connection to use.  We do the remote access as the table's
+        * owner, even if the ANALYZE was started by some other user.
+        */
+       table = GetForeignTable(RelationGetRelid(relation));
+       server = GetForeignServer(table->serverid);
+       user = GetUserMapping(relation->rd_rel->relowner, server->serverid);
+       conn = GetConnection(server, user);
+
+       /*
+        * Construct command to get page count for relation.
+        */
+       initStringInfo(&sql);
+       deparseAnalyzeSizeSql(&sql, relation);
+
+       /* In what follows, do not risk leaking any PGresults. */
+       PG_TRY();
+       {
+               res = PQexec(conn, sql.data);
+               if (PQresultStatus(res) != PGRES_TUPLES_OK)
+                       pgfdw_report_error(ERROR, res, false, sql.data);
+
+               if (PQntuples(res) != 1 || PQnfields(res) != 1)
+                       elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query");
+               *totalpages = strtoul(PQgetvalue(res, 0, 0), NULL, 10);
+
+               PQclear(res);
+               res = NULL;
+       }
+       PG_CATCH();
+       {
+               if (res)
+                       PQclear(res);
+               PG_RE_THROW();
+       }
+       PG_END_TRY();
+
+       ReleaseConnection(conn);
+
        return true;
 }
 

diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 52d1d49b25e2a5268d4cdafc8aaebf150a5b29a3..940b81a7398f9d1047bb775348035527b3291be3 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -47,6 +47,7 @@ extern void appendWhereClause(StringInfo buf,
                                  bool has_where,
                                  List *exprs,
                                  PlannerInfo *root);
+extern void deparseAnalyzeSizeSql(StringInfo buf, Relation rel);
 extern void deparseAnalyzeSql(StringInfo buf, Relation rel);
 
 #endif   /* POSTGRES_FDW_H */