>
>
>> I believe it won't be needed as a regression test but DEBUGn
>> messages could be usable to see "what was sent to the remote
>> side". It can be shown to the console so easily done in
>> regression test. See the deocumentation for client_min_messages
>> for details. (It could receive far many messages then expected,
>> though, and maybe fragile for changing in other part.)
>>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>>
>>
> Ok, I'll see what debug-level messages reveal.
>
>
>
Here's a re-based patch. Notable changes since the last patch are:
- some changes had to move to postgres_fdw.h
- the regression tests are in their own script fetch_size.sql /
fetch_size.out. that should make things easier for the reviewer(s), even if
we later merge it into postgres_fdw.sql/.out.
- I put braces around a multi-line error ereport(). That might be a style
violation, but the statement seemed confusing without it.
- The fetch sql is reported as a DEBUG1 level ereport(), and confirms that
server level options are respected, and table level options are used in
favor of server-level options.
I left the DEBUG1-level message in this patch so that others can see the
output, but I assume we would omit that for production code, with
corresponding deletions from fetch_size.sql and fetch_size.out.
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index 3543312..5d50b30 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -10,7 +10,7 @@ SHLIB_LINK = $(libpq)
EXTENSION = postgres_fdw
DATA = postgres_fdw--1.0.sql
-REGRESS = postgres_fdw
+REGRESS = postgres_fdw fetch_size
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 380ac80..f482dcd 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -162,6 +162,12 @@ InitPgFdwOptions(void)
/* updatable is available on both server and table */
{"updatable", ForeignServerRelationId, false},
{"updatable", ForeignTableRelationId, false},
+ /*
+ * fetch_size is available on both server and table, the table setting
+ * overrides the server setting.
+ */
+ {"fetch_size", ForeignServerRelationId, false},
+ {"fetch_size", ForeignTableRelationId, false},
{NULL, InvalidOid, false}
};
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index f501c6c..d2595f6 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -68,7 +68,9 @@ enum FdwScanPrivateIndex
/* SQL statement to execute remotely (as a String node) */
FdwScanPrivateSelectSql,
/* Integer list of attribute numbers retrieved by the SELECT */
- FdwScanPrivateRetrievedAttrs
+ FdwScanPrivateRetrievedAttrs,
+ /* Integer representing the desired fetch_size */
+ FdwScanPrivateFetchSize
};
/*
@@ -126,6 +128,8 @@ typedef struct PgFdwScanState
/* working memory contexts */
MemoryContext batch_cxt; /* context holding current batch of tuples */
MemoryContext temp_cxt; /* context for per-tuple temporary data */
+
+ int fetch_size; /* number of tuples per fetch */
} PgFdwScanState;
/*
@@ -303,6 +307,8 @@ static HeapTuple make_tuple_from_result_row(PGresult *res,
List *retrieved_attrs,
MemoryContext temp_context);
static void conversion_error_callback(void *arg);
+static int get_fetch_size(ForeignTable *table,
+ ForeignServer *server);
/*
@@ -371,6 +377,8 @@ postgresGetForeignRelSize(PlannerInfo *root,
/* Look up foreign-table catalog info. */
fpinfo->table = GetForeignTable(foreigntableid);
fpinfo->server = GetForeignServer(fpinfo->table->serverid);
+ /* Look up any table-specific fetch size */
+ fpinfo->fetch_size = get_fetch_size(fpinfo->table,fpinfo->server);
/*
* Extract user-settable option values. Note that per-table setting of
@@ -1069,6 +1077,9 @@ postgresGetForeignPlan(PlannerInfo *root,
*/
fdw_private = list_make2(makeString(sql.data),
retrieved_attrs);
+ fdw_private = list_make3(makeString(sql.data),
+ retrieved_attrs,
+ makeInteger(fpinfo->fetch_size));
/*
* Create the ForeignScan node from target list, filtering expressions,
@@ -1147,6 +1158,8 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
FdwScanPrivateSelectSql));
fsstate->retrieved_attrs = (List *) list_nth(fsplan->fdw_private,
FdwScanPrivateRetrievedAttrs);
+ fsstate->fetch_size = intVal(list_nth(fsplan->fdw_private,
+ FdwScanPrivateFetchSize));
/* Create contexts for batches of tuples and per-tuple temp workspace. */
fsstate->batch_cxt = AllocSetContextCreate(estate->es_query_cxt,
@@ -2275,15 +2288,13 @@ fetch_more_data(ForeignScanState *node)
{
PGconn *conn = fsstate->conn;
char sql[64];
- int fetch_size;
int numrows;
int i;
- /* The fetch size is arbitrary, but shouldn't be enormous. */
- fetch_size = 100;
-
snprintf(sql, sizeof(sql), "FETCH %d FROM c%u",
- fetch_size, fsstate->cursor_number);
+ fsstate->fetch_size, fsstate->cursor_number);
+
+ ereport(DEBUG1, (errmsg_internal("Fetch Command: %s", sql)));
res = PQexec(conn, sql);
/* On error, report the original query, not the FETCH. */
@@ -2311,7 +2322,7 @@ fetch_more_data(ForeignScanState *node)
fsstate->fetch_ct_2++;
/* Must be EOF if we didn't get as many tuples as we asked for. */
- fsstate->eof_reached = (numrows < fetch_size);
+ fsstate->eof_reached = (numrows < fsstate->fetch_size);
PQclear(res);
res = NULL;
@@ -2695,8 +2706,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
* then just adjust rowstoskip and samplerows appropriately.
*/
- /* The fetch size is arbitrary, but shouldn't be enormous. */
- fetch_size = 100;
+ fetch_size = get_fetch_size(table,server);
/* Fetch some rows */
snprintf(fetch_sql, sizeof(fetch_sql), "FETCH %d FROM c%u",
@@ -3252,3 +3262,57 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
/* We didn't find any suitable equivalence class expression */
return NULL;
}
+
+static DefElem*
+get_option(List *options, char *optname)
+{
+ ListCell *lc;
+
+ foreach(lc, options)
+ {
+ DefElem *def = (DefElem *) lfirst(lc);
+
+ if (strcmp(def->defname, optname) == 0)
+ return def;
+ }
+ return NULL;
+}
+
+/*
+ * Scan the foreign sever and foreign table definitions for any explicit
+ * fetch_size options. Prefer table-specific option to server-wide option.
+ * If none are found, keep the previous default of 100
+ */
+static int
+get_fetch_size(ForeignTable *table,
+ ForeignServer *server)
+{
+ DefElem *def;
+ int fetch_size;
+
+ def = get_option(table->options, "fetch_size");
+ if (!def)
+ {
+ /*
+ * In the absence of table-specific fetch size,
+ * look for a server-specific one
+ */
+ def = get_option(server->options, "fetch_size");
+ }
+
+ if (def)
+ {
+ fetch_size = strtol(defGetString(def), NULL,10);
+ if (fetch_size < 0)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("invalid fetch size for foreign table \"%s\"",
+ get_rel_name(table->relid)),
+ errhint("fetch_size must be > 0.")));
+ }
+ }
+ else
+ fetch_size = 100;
+ return fetch_size;
+}
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index f243de8..fad6ae4 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -53,6 +53,8 @@ typedef struct PgFdwRelationInfo
ForeignTable *table;
ForeignServer *server;
UserMapping *user; /* only set in use_remote_estimate mode */
+
+ int fetch_size; /* fetch size for this remote table */
} PgFdwRelationInfo;
/* in postgres_fdw.c */
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5a829d5..19350c5 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -342,6 +342,34 @@
</sect3>
<sect3>
+ <title>Fetch Size Options</title>
+
+ <para>
+ By default, rows are fetched from the remote server 100 at a time.
+ This may be overridden using the following option:
+ </para>
+
+ <variablelist>
+
+ <varlistentry>
+ <term><literal>fetch_size</literal></term>
+ <listitem>
+ <para>
+ This option specifies the number of rows <filename>postgres_fdw</>
+ should get in each fetch operation. It can be specified for a foreign
+ table or a foreign server. The option specified on a table overrides
+ an option specified for the server.
+ The default is <literal>100</>.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+ </sect3>
+
+
+ <sect3>
<title>Importing Options</title>
<para>
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
