diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 25112df916..b510322c4e 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -10480,3 +10480,22 @@ DROP TABLE result_tbl; DROP TABLE join_tbl; ALTER SERVER loopback OPTIONS (DROP async_capable); ALTER SERVER loopback2 OPTIONS (DROP async_capable); +-- =================================================================== +-- test invalid server and foreign table options +-- =================================================================== +-- Invalid fdw_startup_cost option +CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw + OPTIONS(fdw_startup_cost '100$%$#$#'); +ERROR: invalid value for floating point option "fdw_startup_cost": 100$%$#$# +-- Invalid fdw_tuple_cost option +CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw + OPTIONS(fdw_tuple_cost '100$%$#$#'); +ERROR: invalid value for floating point option "fdw_tuple_cost": 100$%$#$# +-- Invalid fetch_size option +CREATE FOREIGN TABLE inv_fsz (c1 int ) + SERVER loopback OPTIONS (fetch_size '100$%$#$#'); +ERROR: invalid value for integer option "fetch_size": 100$%$#$# +-- Invalid batch_size option +CREATE FOREIGN TABLE inv_bsz (c1 int ) + SERVER loopback OPTIONS (batch_size '100$%$#$#'); +ERROR: invalid value for integer option "batch_size": 100$%$#$# diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 672b55a808..4593cbc540 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -20,6 +20,7 @@ #include "commands/extension.h" #include "postgres_fdw.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/varlena.h" /* @@ -119,14 +120,23 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) strcmp(def->defname, "fdw_tuple_cost") == 0) { /* these must have a non-negative numeric value */ - double val; - char *endp; + char *value; + double real_val; + bool is_parsed; - val = strtod(defGetString(def), &endp); - if (*endp || val < 0) + value = defGetString(def); + is_parsed = parse_real(value, &real_val, 0, NULL); + + if (!is_parsed) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s requires a non-negative numeric value", + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for floating point option \"%s\": %s", + def->defname, value))); + + if (real_val < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" requires a non-negative floating point value", def->defname))); } else if (strcmp(def->defname, "extensions") == 0) @@ -134,26 +144,26 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) /* check list syntax, warn about uninstalled extensions */ (void) ExtractExtensionList(defGetString(def), true); } - else if (strcmp(def->defname, "fetch_size") == 0) + else if (strcmp(def->defname, "fetch_size") == 0 || + strcmp(def->defname, "batch_size") == 0) { - int fetch_size; + char *value; + int int_val; + bool is_parsed; - fetch_size = strtol(defGetString(def), NULL, 10); - if (fetch_size <= 0) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s requires a non-negative integer value", - def->defname))); - } - else if (strcmp(def->defname, "batch_size") == 0) - { - int batch_size; + value = defGetString(def); + is_parsed = parse_int(value, &int_val, 0, NULL); - batch_size = strtol(defGetString(def), NULL, 10); - if (batch_size <= 0) + if (!is_parsed) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s requires a non-negative integer value", + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid value for integer option \"%s\": %s", + def->defname, value))); + + if (int_val <= 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" requires a non-negative integer value", def->defname))); } else if (strcmp(def->defname, "password_required") == 0) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index a5afac47eb..5cf10402a2 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -5024,7 +5024,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel, if (strcmp(def->defname, "fetch_size") == 0) { - fetch_size = strtol(defGetString(def), NULL, 10); + (void) parse_int(defGetString(def), &fetch_size, 0, NULL); break; } } @@ -5034,7 +5034,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel, if (strcmp(def->defname, "fetch_size") == 0) { - fetch_size = strtol(defGetString(def), NULL, 10); + (void) parse_int(defGetString(def), &fetch_size, 0, NULL); break; } } @@ -5801,14 +5801,16 @@ apply_server_options(PgFdwRelationInfo *fpinfo) if (strcmp(def->defname, "use_remote_estimate") == 0) fpinfo->use_remote_estimate = defGetBoolean(def); else if (strcmp(def->defname, "fdw_startup_cost") == 0) - fpinfo->fdw_startup_cost = strtod(defGetString(def), NULL); + (void) parse_real(defGetString(def), &fpinfo->fdw_startup_cost, 0, + NULL); else if (strcmp(def->defname, "fdw_tuple_cost") == 0) - fpinfo->fdw_tuple_cost = strtod(defGetString(def), NULL); + (void) parse_real(defGetString(def), &fpinfo->fdw_tuple_cost, 0, + NULL); else if (strcmp(def->defname, "extensions") == 0) fpinfo->shippable_extensions = ExtractExtensionList(defGetString(def), false); else if (strcmp(def->defname, "fetch_size") == 0) - fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); + (void) parse_int(defGetString(def), &fpinfo->fetch_size, 0, NULL); else if (strcmp(def->defname, "async_capable") == 0) fpinfo->async_capable = defGetBoolean(def); } @@ -5831,7 +5833,7 @@ apply_table_options(PgFdwRelationInfo *fpinfo) if (strcmp(def->defname, "use_remote_estimate") == 0) fpinfo->use_remote_estimate = defGetBoolean(def); else if (strcmp(def->defname, "fetch_size") == 0) - fpinfo->fetch_size = strtol(defGetString(def), NULL, 10); + (void) parse_int(defGetString(def), &fpinfo->fetch_size, 0, NULL); else if (strcmp(def->defname, "async_capable") == 0) fpinfo->async_capable = defGetBoolean(def); } @@ -7341,7 +7343,7 @@ get_batch_size_option(Relation rel) if (strcmp(def->defname, "batch_size") == 0) { - batch_size = strtol(defGetString(def), NULL, 10); + (void) parse_int(defGetString(def), &batch_size, 0, NULL); break; } } diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 95862e38ed..911f171d81 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3339,3 +3339,19 @@ DROP TABLE join_tbl; ALTER SERVER loopback OPTIONS (DROP async_capable); ALTER SERVER loopback2 OPTIONS (DROP async_capable); + +-- =================================================================== +-- test invalid server and foreign table options +-- =================================================================== +-- Invalid fdw_startup_cost option +CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw + OPTIONS(fdw_startup_cost '100$%$#$#'); +-- Invalid fdw_tuple_cost option +CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw + OPTIONS(fdw_tuple_cost '100$%$#$#'); +-- Invalid fetch_size option +CREATE FOREIGN TABLE inv_fsz (c1 int ) + SERVER loopback OPTIONS (fetch_size '100$%$#$#'); +-- Invalid batch_size option +CREATE FOREIGN TABLE inv_bsz (c1 int ) + SERVER loopback OPTIONS (batch_size '100$%$#$#'); diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index d96c3d0f0c..44acf7c586 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -266,11 +266,11 @@ OPTIONS (ADD password_required 'false'); fdw_startup_cost - This option, which can be specified for a foreign server, is a numeric - value that is added to the estimated startup cost of any foreign-table - scan on that server. This represents the additional overhead of - establishing a connection, parsing and planning the query on the - remote side, etc. + This option, which can be specified for a foreign server, is a floating + point value that is added to the estimated startup cost of any + foreign-table scan on that server. This represents the additional + overhead of establishing a connection, parsing and planning the query on + the remote side, etc. The default value is 100. @@ -280,8 +280,8 @@ OPTIONS (ADD password_required 'false'); fdw_tuple_cost - This option, which can be specified for a foreign server, is a numeric - value that is used as extra cost per-tuple for foreign-table + This option, which can be specified for a foreign server, is a floating + point value that is used as extra cost per-tuple for foreign-table scans on that server. This represents the additional overhead of data transfer between servers. You might increase or decrease this number to reflect higher or lower network delay to the remote server.