Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd854720

Browse files
committed
postgres_fdw: Tighten up allowed values for batch_size, fetch_size options.
Previously the values such as '100$%$#$#', '9,223,372,' were accepted andtreated as valid integers for postgres_fdw options batch_size and fetch_size.Whereas this is not the case with fdw_startup_cost and fdw_tuple_cost optionsfor which an error is thrown. This was because endptr was not usedwhile converting strings to integers using strtol.This commit changes the logic so that it uses parse_int functioninstead of strtol as it serves the purpose by returning false in caseif it is unable to convert the string to integer. Note thatthis function also rounds off the values such as '100.456' to 100 and'100.567' or '100.678' to 101.While on this, use parse_real for fdw_startup_cost and fdw_tuple_cost options.Since parse_int and parse_real are being used for reloptions and GUCs,it is more appropriate to use in postgres_fdw rather than using strtoland strtod directly.Back-patch to v14.Author: Bharath RupireddyReviewed-by: Ashutosh Bapat, Tom Lane, Kyotaro Horiguchi, Fujii MasaoDiscussion:https://postgr.es/m/CALj2ACVMO6wY5Pc4oe1OCgUOAtdjHuFsBDw8R5uoYR86eWFQDA@mail.gmail.com
1 parent9fd8557 commitd854720

File tree

5 files changed

+82
-35
lines changed

5 files changed

+82
-35
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10480,3 +10480,22 @@ DROP TABLE result_tbl;
1048010480
DROP TABLE join_tbl;
1048110481
ALTER SERVER loopback OPTIONS (DROP async_capable);
1048210482
ALTER SERVER loopback2 OPTIONS (DROP async_capable);
10483+
-- ===================================================================
10484+
-- test invalid server and foreign table options
10485+
-- ===================================================================
10486+
-- Invalid fdw_startup_cost option
10487+
CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw
10488+
OPTIONS(fdw_startup_cost '100$%$#$#');
10489+
ERROR: invalid value for floating point option "fdw_startup_cost": 100$%$#$#
10490+
-- Invalid fdw_tuple_cost option
10491+
CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw
10492+
OPTIONS(fdw_tuple_cost '100$%$#$#');
10493+
ERROR: invalid value for floating point option "fdw_tuple_cost": 100$%$#$#
10494+
-- Invalid fetch_size option
10495+
CREATE FOREIGN TABLE inv_fsz (c1 int )
10496+
SERVER loopback OPTIONS (fetch_size '100$%$#$#');
10497+
ERROR: invalid value for integer option "fetch_size": 100$%$#$#
10498+
-- Invalid batch_size option
10499+
CREATE FOREIGN TABLE inv_bsz (c1 int )
10500+
SERVER loopback OPTIONS (batch_size '100$%$#$#');
10501+
ERROR: invalid value for integer option "batch_size": 100$%$#$#

‎contrib/postgres_fdw/option.c

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include"commands/extension.h"
2121
#include"postgres_fdw.h"
2222
#include"utils/builtins.h"
23+
#include"utils/guc.h"
2324
#include"utils/varlena.h"
2425

2526
/*
@@ -119,41 +120,50 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
119120
strcmp(def->defname,"fdw_tuple_cost")==0)
120121
{
121122
/* these must have a non-negative numeric value */
122-
doubleval;
123-
char*endp;
123+
char*value;
124+
doublereal_val;
125+
boolis_parsed;
124126

125-
val=strtod(defGetString(def),&endp);
126-
if (*endp||val<0)
127+
value=defGetString(def);
128+
is_parsed=parse_real(value,&real_val,0,NULL);
129+
130+
if (!is_parsed)
131+
ereport(ERROR,
132+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
133+
errmsg("invalid value for floating point option \"%s\": %s",
134+
def->defname,value)));
135+
136+
if (real_val<0)
127137
ereport(ERROR,
128-
(errcode(ERRCODE_SYNTAX_ERROR),
129-
errmsg("%s requires a non-negativenumeric value",
138+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
139+
errmsg("\"%s\" requires a non-negativefloating point value",
130140
def->defname)));
131141
}
132142
elseif (strcmp(def->defname,"extensions")==0)
133143
{
134144
/* check list syntax, warn about uninstalled extensions */
135145
(void)ExtractExtensionList(defGetString(def), true);
136146
}
137-
elseif (strcmp(def->defname,"fetch_size")==0)
147+
elseif (strcmp(def->defname,"fetch_size")==0||
148+
strcmp(def->defname,"batch_size")==0)
138149
{
139-
intfetch_size;
150+
char*value;
151+
intint_val;
152+
boolis_parsed;
153+
154+
value=defGetString(def);
155+
is_parsed=parse_int(value,&int_val,0,NULL);
140156

141-
fetch_size=strtol(defGetString(def),NULL,10);
142-
if (fetch_size <=0)
157+
if (!is_parsed)
143158
ereport(ERROR,
144-
(errcode(ERRCODE_SYNTAX_ERROR),
145-
errmsg("%s requires a non-negative integer value",
146-
def->defname)));
147-
}
148-
elseif (strcmp(def->defname,"batch_size")==0)
149-
{
150-
intbatch_size;
159+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
160+
errmsg("invalid value for integer option \"%s\": %s",
161+
def->defname,value)));
151162

152-
batch_size=strtol(defGetString(def),NULL,10);
153-
if (batch_size <=0)
163+
if (int_val <=0)
154164
ereport(ERROR,
155-
(errcode(ERRCODE_SYNTAX_ERROR),
156-
errmsg("%s requires a non-negative integer value",
165+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
166+
errmsg("\"%s\" requires a non-negative integer value",
157167
def->defname)));
158168
}
159169
elseif (strcmp(def->defname,"password_required")==0)

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5024,7 +5024,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
50245024

50255025
if (strcmp(def->defname,"fetch_size")==0)
50265026
{
5027-
fetch_size=strtol(defGetString(def),NULL,10);
5027+
(void)parse_int(defGetString(def),&fetch_size,0,NULL);
50285028
break;
50295029
}
50305030
}
@@ -5034,7 +5034,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
50345034

50355035
if (strcmp(def->defname,"fetch_size")==0)
50365036
{
5037-
fetch_size=strtol(defGetString(def),NULL,10);
5037+
(void)parse_int(defGetString(def),&fetch_size,0,NULL);
50385038
break;
50395039
}
50405040
}
@@ -5801,14 +5801,16 @@ apply_server_options(PgFdwRelationInfo *fpinfo)
58015801
if (strcmp(def->defname,"use_remote_estimate")==0)
58025802
fpinfo->use_remote_estimate=defGetBoolean(def);
58035803
elseif (strcmp(def->defname,"fdw_startup_cost")==0)
5804-
fpinfo->fdw_startup_cost=strtod(defGetString(def),NULL);
5804+
(void)parse_real(defGetString(def),&fpinfo->fdw_startup_cost,0,
5805+
NULL);
58055806
elseif (strcmp(def->defname,"fdw_tuple_cost")==0)
5806-
fpinfo->fdw_tuple_cost=strtod(defGetString(def),NULL);
5807+
(void)parse_real(defGetString(def),&fpinfo->fdw_tuple_cost,0,
5808+
NULL);
58075809
elseif (strcmp(def->defname,"extensions")==0)
58085810
fpinfo->shippable_extensions=
58095811
ExtractExtensionList(defGetString(def), false);
58105812
elseif (strcmp(def->defname,"fetch_size")==0)
5811-
fpinfo->fetch_size=strtol(defGetString(def),NULL,10);
5813+
(void)parse_int(defGetString(def),&fpinfo->fetch_size,0,NULL);
58125814
elseif (strcmp(def->defname,"async_capable")==0)
58135815
fpinfo->async_capable=defGetBoolean(def);
58145816
}
@@ -5831,7 +5833,7 @@ apply_table_options(PgFdwRelationInfo *fpinfo)
58315833
if (strcmp(def->defname,"use_remote_estimate")==0)
58325834
fpinfo->use_remote_estimate=defGetBoolean(def);
58335835
elseif (strcmp(def->defname,"fetch_size")==0)
5834-
fpinfo->fetch_size=strtol(defGetString(def),NULL,10);
5836+
(void)parse_int(defGetString(def),&fpinfo->fetch_size,0,NULL);
58355837
elseif (strcmp(def->defname,"async_capable")==0)
58365838
fpinfo->async_capable=defGetBoolean(def);
58375839
}
@@ -7341,7 +7343,7 @@ get_batch_size_option(Relation rel)
73417343

73427344
if (strcmp(def->defname,"batch_size")==0)
73437345
{
7344-
batch_size=strtol(defGetString(def),NULL,10);
7346+
(void)parse_int(defGetString(def),&batch_size,0,NULL);
73457347
break;
73467348
}
73477349
}

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3339,3 +3339,19 @@ DROP TABLE join_tbl;
33393339

33403340
ALTER SERVER loopback OPTIONS (DROP async_capable);
33413341
ALTER SERVER loopback2 OPTIONS (DROP async_capable);
3342+
3343+
-- ===================================================================
3344+
-- test invalid server and foreign table options
3345+
-- ===================================================================
3346+
-- Invalid fdw_startup_cost option
3347+
CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw
3348+
OPTIONS(fdw_startup_cost'100$%$#$#');
3349+
-- Invalid fdw_tuple_cost option
3350+
CREATE SERVER inv_scst FOREIGN DATA WRAPPER postgres_fdw
3351+
OPTIONS(fdw_tuple_cost'100$%$#$#');
3352+
-- Invalid fetch_size option
3353+
CREATE FOREIGN TABLE inv_fsz (c1int )
3354+
SERVER loopback OPTIONS (fetch_size'100$%$#$#');
3355+
-- Invalid batch_size option
3356+
CREATE FOREIGN TABLE inv_bsz (c1int )
3357+
SERVER loopback OPTIONS (batch_size'100$%$#$#');

‎doc/src/sgml/postgres-fdw.sgml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,11 @@ OPTIONS (ADD password_required 'false');
266266
<term><literal>fdw_startup_cost</literal> (<type>floating point</type>)</term>
267267
<listitem>
268268
<para>
269-
This option, which can be specified for a foreign server, is anumeric
270-
value that is added to the estimated startup cost of any foreign-table
271-
scan on that server. This represents the additional overhead of
272-
establishing a connection, parsing and planning the query on the
273-
remote side, etc.
269+
This option, which can be specified for a foreign server, is afloating
270+
pointvalue that is added to the estimated startup cost of any
271+
foreign-tablescan on that server. This represents the additional
272+
overhead ofestablishing a connection, parsing and planning the query on
273+
theremote side, etc.
274274
The default value is <literal>100</literal>.
275275
</para>
276276
</listitem>
@@ -280,8 +280,8 @@ OPTIONS (ADD password_required 'false');
280280
<term><literal>fdw_tuple_cost</literal> (<type>floating point</type>)</term>
281281
<listitem>
282282
<para>
283-
This option, which can be specified for a foreign server, is anumeric
284-
value that is used as extra cost per-tuple for foreign-table
283+
This option, which can be specified for a foreign server, is afloating
284+
pointvalue that is used as extra cost per-tuple for foreign-table
285285
scans on that server. This represents the additional overhead of
286286
data transfer between servers. You might increase or decrease this
287287
number to reflect higher or lower network delay to the remote server.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp