On Thu, Sep 05, 2019 at 03:52:48PM +0900, Michael Paquier wrote: > Right, there was this part. This brings also the point of having one > interface for the backend as all the error messages for the backend > are actually the same, with the most simple name being that: > pg_strtoint(value, size, error_ok).
I have been looking at that for the last couple of days. First, I
have consolidated all the error strings in a single routine like this
one, except that error_ok is not really needed if you take this
approach: callers that don't care about failures could just call the
set of routines in common/string.c and be done with it.
Attached is an update of my little module that I used to check that
the refactoring was done correctly (regression tests attached), it
also includes a couple of routines to check the performance difference
between one approach and the other, with focus on two things:
- Is pg_strtouint64 really improved?
- How much do we lose by moving to a common interface in the backend
with pg_strtoint?
The good news is that removing strtol from pg_strtouint64 really
improves the performance as already reported, and with one billion
calls in a tight loop you see a clear difference:
=# select pg_strtouint64_old_check('10000', 1000000000);
pg_strtouint64_old_check
--------------------------
10000
(1 row)
Time: 15576.539 ms (00:15.577)
=# select pg_strtouint64_new_check('10000', 1000000000);
pg_strtouint64_new_check
--------------------------
10000
(1 row)
Time: 8820.544 ms (00:08.821)
So the new implementation is more than 40% faster with this
micro-benchmark on my Debian box.
The bad news is that a pg_strtoint() wrapper is not a good idea:
=# select pg_strtoint_check('10000', 1000000000, 4);
pg_strtoint_check
-------------------
10000
(1 row)
Time: 11178.101 ms (00:11.178)
=# select pg_strtoint32_check('10000', 1000000000);
pg_strtoint32_check
---------------------
10000
(1 row)
Time: 9252.894 ms (00:09.253)
So trying to consolidate all error messages leads to a 15% hit with
this test, which sucks.
So, it seems to me that if we want to have a consolidation of
everything, we basically need to have the generic error messages for
the backend directly into common/string.c where the routines are
refactored, and I think that we should do the following based on the
patch attached:
- Just remove pg_strtoint(), and move the error generation logic in
common/string.c.
- Add an error_ok flag for all the pg_strto[u]int{16|32|64} routines,
which generates errors only for FRONTEND is not defined.
- Use pg_log_error() for the frontend errors.
If those errors are added everywhere, we would basically have no code
paths in the frontend or the backend (for the unsigned part only)
which use them yet. Another possibility would be to ignore the
error_ok flag in those cases, but that's inconsistent. My take would
be here to have a more generic error message, like that:
"value \"%s\" is out of range for [unsigned] {16|32|64}-bit integer"
"invalid input syntax for [unsigned] {16|32|64}-bit integer: \"%s\"\n"
I do not suggest to change the messages for the backend for signed
entries though, as these are linked to the data types used.
Attached is an update of my toy module, and an updated patch with what
I have done up to now. This stuff already does a lot, so for now I
have not worked on the removal strtoint() in string.c yet. We could
just do that with a follow-up patch and make it use the new conversion
routines once we are sure that they are in a correct shape. As
strtoint() makes use of strtol(), switching to the new routines will
be much faster anyway...
Fabien, any thoughts?
--
Michael
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 221b47298c..28891ba337 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -79,6 +79,7 @@
#include "utils/builtins.h"
#include "utils/hashutils.h"
#include "utils/memutils.h"
+#include "common/string.h"
PG_MODULE_MAGIC;
@@ -1022,7 +1023,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
/* parse command tag to retrieve the number of affected rows. */
if (completionTag &&
strncmp(completionTag, "COPY ", 5) == 0)
- rows = pg_strtouint64(completionTag + 5, NULL, 10);
+ (void) pg_strtouint64(completionTag + 5, &rows);
else
rows = 0;
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index adf0490f85..9583845d7e 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -306,7 +306,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
/* internal error */
elog(ERROR, "check_foreign_key: too short %d (< 5) list of arguments", nargs);
- nrefs = pg_strtoint32(args[0]);
+ nrefs = pg_strtoint(args[0], 4);
if (nrefs < 1)
/* internal error */
elog(ERROR, "check_foreign_key: %d (< 1) number of references specified", nrefs);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 2c0ae395ba..8e75d52b06 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -21,6 +21,7 @@
#include "catalog/heap.h"
#include "catalog/pg_type.h"
#include "commands/trigger.h"
+#include "common/string.h"
#include "executor/executor.h"
#include "executor/spi_priv.h"
#include "miscadmin.h"
@@ -2338,8 +2339,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
CreateTableAsStmt *ctastmt = (CreateTableAsStmt *) stmt->utilityStmt;
if (strncmp(completionTag, "SELECT ", 7) == 0)
- _SPI_current->processed =
- pg_strtouint64(completionTag + 7, NULL, 10);
+ (void) pg_strtouint64(completionTag + 7, &_SPI_current->processed);
else
{
/*
@@ -2361,8 +2361,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
else if (IsA(stmt->utilityStmt, CopyStmt))
{
Assert(strncmp(completionTag, "COPY ", 5) == 0);
- _SPI_current->processed = pg_strtouint64(completionTag + 5,
- NULL, 10);
+ (void) pg_strtouint64(completionTag + 5, &_SPI_current->processed);
}
}
diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c
index a9bd47d937..593a5ef54e 100644
--- a/src/backend/libpq/pqmq.c
+++ b/src/backend/libpq/pqmq.c
@@ -286,10 +286,10 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
edata->hint = pstrdup(value);
break;
case PG_DIAG_STATEMENT_POSITION:
- edata->cursorpos = pg_strtoint32(value);
+ edata->cursorpos = pg_strtoint(value, 4);
break;
case PG_DIAG_INTERNAL_POSITION:
- edata->internalpos = pg_strtoint32(value);
+ edata->internalpos = pg_strtoint(value, 4);
break;
case PG_DIAG_INTERNAL_QUERY:
edata->internalquery = pstrdup(value);
@@ -316,7 +316,7 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
edata->filename = pstrdup(value);
break;
case PG_DIAG_SOURCE_LINE:
- edata->lineno = pg_strtoint32(value);
+ edata->lineno = pg_strtoint(value, 4);
break;
case PG_DIAG_SOURCE_FUNCTION:
edata->funcname = pstrdup(value);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 764e3bb90c..17cde42b4d 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -34,6 +34,7 @@
#include "fmgr.h"
#include "miscadmin.h"
+#include "common/string.h"
#include "nodes/extensible.h"
#include "nodes/parsenodes.h"
#include "nodes/plannodes.h"
@@ -80,7 +81,7 @@
#define READ_UINT64_FIELD(fldname) \
token = pg_strtok(&length); /* skip :fldname */ \
token = pg_strtok(&length); /* get field value */ \
- local_node->fldname = pg_strtouint64(token, NULL, 10)
+ (void) pg_strtouint64(token, &local_node->fldname)
/* Read a long integer field (anything written as ":fldname %ld") */
#define READ_LONG_FIELD(fldname) \
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 1baf7ef31f..8af5e083ee 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -25,10 +25,10 @@
#include "parser/parse_expr.h"
#include "parser/parse_relation.h"
#include "utils/builtins.h"
-#include "utils/int8.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
#include "utils/varbit.h"
+#include "common/string.h"
static void pcb_error_callback(void *arg);
@@ -496,7 +496,7 @@ make_const(ParseState *pstate, Value *value, int location)
case T_Float:
/* could be an oversize integer as well as a float ... */
- if (scanint8(strVal(value), true, &val64))
+ if (pg_strtoint64(strVal(value), &val64) == PG_STRTOINT_OK)
{
/*
* It might actually fit in int32. Probably only INT_MIN can
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 6eba08a920..de52f538a7 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -341,7 +341,7 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID *primary_tli)
ntuples, nfields, 3, 1)));
}
primary_sysid = pstrdup(PQgetvalue(res, 0, 0));
- *primary_tli = pg_strtoint32(PQgetvalue(res, 0, 1));
+ *primary_tli = pg_strtoint(PQgetvalue(res, 0, 1), 4);
PQclear(res);
return primary_sysid;
@@ -487,7 +487,7 @@ libpqrcv_endstreaming(WalReceiverConn *conn, TimeLineID *next_tli)
if (PQnfields(res) < 2 || PQntuples(res) != 1)
ereport(ERROR,
(errmsg("unexpected result set after end-of-streaming")));
- *next_tli = pg_strtoint32(PQgetvalue(res, 0, 0));
+ *next_tli = pg_strtoint(PQgetvalue(res, 0, 0), 4);
PQclear(res);
/* the result set should be followed by CommandComplete */
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 9c08757fca..fa41edaa96 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -12,6 +12,7 @@
*/
#include "postgres.h"
+#include "common/string.h"
#include "catalog/pg_publication.h"
#include "fmgr.h"
@@ -22,7 +23,6 @@
#include "replication/pgoutput.h"
#include "utils/inval.h"
-#include "utils/int8.h"
#include "utils/memutils.h"
#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -113,7 +113,7 @@ parse_output_parameters(List *options, uint32 *protocol_version,
errmsg("conflicting or redundant options")));
protocol_version_given = true;
- if (!scanint8(strVal(defel->arg), true, &parsed))
+ if (pg_strtoint64(strVal(defel->arg), &parsed) != PG_STRTOINT_OK)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid proto_version")));
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index f18e761af8..a7b80a83d4 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -2460,13 +2460,13 @@ prsd_headline(PG_FUNCTION_ARGS)
char *val = defGetString(defel);
if (pg_strcasecmp(defel->defname, "MaxWords") == 0)
- max_words = pg_strtoint32(val);
+ max_words = pg_strtoint(val, 4);
else if (pg_strcasecmp(defel->defname, "MinWords") == 0)
- min_words = pg_strtoint32(val);
+ min_words = pg_strtoint(val, 4);
else if (pg_strcasecmp(defel->defname, "ShortWord") == 0)
- shortword = pg_strtoint32(val);
+ shortword = pg_strtoint(val, 4);
else if (pg_strcasecmp(defel->defname, "MaxFragments") == 0)
- max_fragments = pg_strtoint32(val);
+ max_fragments = pg_strtoint(val, 4);
else if (pg_strcasecmp(defel->defname, "StartSel") == 0)
prs->startsel = pstrdup(val);
else if (pg_strcasecmp(defel->defname, "StopSel") == 0)
diff --git a/src/backend/utils/adt/arrayutils.c b/src/backend/utils/adt/arrayutils.c
index 6170e35ac2..60557402dc 100644
--- a/src/backend/utils/adt/arrayutils.c
+++ b/src/backend/utils/adt/arrayutils.c
@@ -226,7 +226,7 @@ ArrayGetIntegerTypmods(ArrayType *arr, int *n)
result = (int32 *) palloc(*n * sizeof(int32));
for (i = 0; i < *n; i++)
- result[i] = pg_strtoint32(DatumGetCString(elem_values[i]));
+ result[i] = pg_strtoint(DatumGetCString(elem_values[i]), 4);
pfree(elem_values);
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 6515fc8ec6..15f3babc05 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -26,7 +26,6 @@
#include "libpq/pqformat.h"
#include "utils/builtins.h"
#include "utils/cash.h"
-#include "utils/int8.h"
#include "utils/numeric.h"
#include "utils/pg_locale.h"
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 755ca6e277..f8cc5bc48a 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -92,7 +92,6 @@
#include "utils/datetime.h"
#include "utils/float.h"
#include "utils/formatting.h"
-#include "utils/int8.h"
#include "utils/memutils.h"
#include "utils/numeric.h"
#include "utils/pg_locale.h"
diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 04825fc77d..8007cc5839 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -64,7 +64,7 @@ int2in(PG_FUNCTION_ARGS)
{
char *num = PG_GETARG_CSTRING(0);
- PG_RETURN_INT16(pg_strtoint16(num));
+ PG_RETURN_INT16(pg_strtoint(num, 2));
}
/*
@@ -269,7 +269,7 @@ int4in(PG_FUNCTION_ARGS)
{
char *num = PG_GETARG_CSTRING(0);
- PG_RETURN_INT32(pg_strtoint32(num));
+ PG_RETURN_INT32(pg_strtoint(num, 4));
}
/*
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 0ff9394a2f..d7e1a04c7d 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -23,7 +23,6 @@
#include "nodes/nodeFuncs.h"
#include "nodes/supportnodes.h"
#include "optimizer/optimizer.h"
-#include "utils/int8.h"
#include "utils/builtins.h"
@@ -47,99 +46,14 @@ typedef struct
* Formatting and conversion routines.
*---------------------------------------------------------*/
-/*
- * scanint8 --- try to parse a string into an int8.
- *
- * If errorOK is false, ereport a useful error message if the string is bad.
- * If errorOK is true, just return "false" for bad input.
- */
-bool
-scanint8(const char *str, bool errorOK, int64 *result)
-{
- const char *ptr = str;
- int64 tmp = 0;
- bool neg = false;
-
- /*
- * Do our own scan, rather than relying on sscanf which might be broken
- * for long long.
- *
- * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate
- * value as a negative number.
- */
-
- /* skip leading spaces */
- while (*ptr && isspace((unsigned char) *ptr))
- ptr++;
-
- /* handle sign */
- if (*ptr == '-')
- {
- ptr++;
- neg = true;
- }
- else if (*ptr == '+')
- ptr++;
-
- /* require at least one digit */
- if (unlikely(!isdigit((unsigned char) *ptr)))
- goto invalid_syntax;
-
- /* process digits */
- while (*ptr && isdigit((unsigned char) *ptr))
- {
- int8 digit = (*ptr++ - '0');
-
- if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) ||
- unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
- goto out_of_range;
- }
-
- /* allow trailing whitespace, but not other trailing chars */
- while (*ptr != '\0' && isspace((unsigned char) *ptr))
- ptr++;
-
- if (unlikely(*ptr != '\0'))
- goto invalid_syntax;
-
- if (!neg)
- {
- /* could fail if input is most negative number */
- if (unlikely(tmp == PG_INT64_MIN))
- goto out_of_range;
- tmp = -tmp;
- }
-
- *result = tmp;
- return true;
-
-out_of_range:
- if (!errorOK)
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("value \"%s\" is out of range for type %s",
- str, "bigint")));
- return false;
-
-invalid_syntax:
- if (!errorOK)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
- errmsg("invalid input syntax for type %s: \"%s\"",
- "bigint", str)));
- return false;
-}
-
/* int8in()
*/
Datum
int8in(PG_FUNCTION_ARGS)
{
- char *str = PG_GETARG_CSTRING(0);
- int64 result;
+ char *str = PG_GETARG_CSTRING(0);
- (void) scanint8(str, false, &result);
- PG_RETURN_INT64(result);
+ PG_RETURN_INT64(pg_strtoint(str, 8));
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index a00db3ce7a..bc02958950 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -39,7 +39,6 @@
#include "utils/float.h"
#include "utils/guc.h"
#include "utils/hashutils.h"
-#include "utils/int8.h"
#include "utils/numeric.h"
#include "utils/sortsupport.h"
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index 70138feb29..1d8b6be9bc 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -19,6 +19,7 @@
#include <ctype.h>
#include "common/int.h"
+#include "common/string.h"
#include "utils/builtins.h"
/*
@@ -110,153 +111,86 @@ pg_atoi(const char *s, int size, int c)
}
/*
- * Convert input string to a signed 16 bit integer.
+ * pg_strtoint
*
- * Allows any number of leading or trailing whitespace characters. Will throw
- * ereport() upon bad input format or overflow.
+ * Convert input string to a signed integer of the wanted size.
*
- * NB: Accumulate input as a negative number, to deal with two's complement
- * representation of the most negative number, which can't be represented as a
- * positive number.
+ * 'size' is the sizeof() the desired integral result (2, 4, or 8 bytes).
+ *
+ * This throws ereport() upon bad input format or overflow.
*/
-int16
-pg_strtoint16(const char *s)
+int64
+pg_strtoint(const char *s, int size)
{
- const char *ptr = s;
- int16 tmp = 0;
- bool neg = false;
+ pg_strtoint_status status = PG_STRTOINT_OK;
+ int64 result;
+ char *type;
- /* skip leading spaces */
- while (likely(*ptr) && isspace((unsigned char) *ptr))
- ptr++;
-
- /* handle sign */
- if (*ptr == '-')
+ switch (size)
{
- ptr++;
- neg = true;
- }
- else if (*ptr == '+')
- ptr++;
+ case 2:
+ {
+ int16 res16;
- /* require at least one digit */
- if (unlikely(!isdigit((unsigned char) *ptr)))
- goto invalid_syntax;
+ status = pg_strtoint16(s, &res16);
+ result = (int64) res16;
+ break;
+ }
+ case 4:
+ {
+ int32 res32;
- /* process digits */
- while (*ptr && isdigit((unsigned char) *ptr))
- {
- int8 digit = (*ptr++ - '0');
-
- if (unlikely(pg_mul_s16_overflow(tmp, 10, &tmp)) ||
- unlikely(pg_sub_s16_overflow(tmp, digit, &tmp)))
- goto out_of_range;
+ status = pg_strtoint32(s, &res32);
+ result = (int64) res32;
+ break;
+ }
+ case 8:
+ status = pg_strtoint64(s, &result);
+ break;
+ default:
+ elog(ERROR, "unsupported result size: %d", size);
+ break;
}
- /* allow trailing whitespace, but not other trailing chars */
- while (*ptr != '\0' && isspace((unsigned char) *ptr))
- ptr++;
+ if (likely(status == PG_STRTOINT_OK))
+ return result;
- if (unlikely(*ptr != '\0'))
- goto invalid_syntax;
-
- if (!neg)
+ switch (size)
{
- /* could fail if input is most negative number */
- if (unlikely(tmp == PG_INT16_MIN))
- goto out_of_range;
- tmp = -tmp;
+ case 2:
+ type = "smallint";
+ break;
+ case 4:
+ type = "integer";
+ break;
+ case 8:
+ type = "bigint";
+ break;
+ default:
+ Assert(false);
+ break;
}
- return tmp;
-
-out_of_range:
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("value \"%s\" is out of range for type %s",
- s, "smallint")));
-
-invalid_syntax:
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
- errmsg("invalid input syntax for type %s: \"%s\"",
- "smallint", s)));
-
- return 0; /* keep compiler quiet */
-}
-
-/*
- * Convert input string to a signed 32 bit integer.
- *
- * Allows any number of leading or trailing whitespace characters. Will throw
- * ereport() upon bad input format or overflow.
- *
- * NB: Accumulate input as a negative number, to deal with two's complement
- * representation of the most negative number, which can't be represented as a
- * positive number.
- */
-int32
-pg_strtoint32(const char *s)
-{
- const char *ptr = s;
- int32 tmp = 0;
- bool neg = false;
-
- /* skip leading spaces */
- while (likely(*ptr) && isspace((unsigned char) *ptr))
- ptr++;
-
- /* handle sign */
- if (*ptr == '-')
+ switch (status)
{
- ptr++;
- neg = true;
+ case PG_STRTOINT_SYNTAX_ERROR:
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("invalid input syntax for type %s: \"%s\"",
+ type, s)));
+ break;
+
+ case PG_STRTOINT_RANGE_ERROR:
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("value \"%s\" is out of range for type %s",
+ s, type)));
+ break;
+
+ case PG_STRTOINT_OK:
+ Assert(false);
+ break;
}
- else if (*ptr == '+')
- ptr++;
-
- /* require at least one digit */
- if (unlikely(!isdigit((unsigned char) *ptr)))
- goto invalid_syntax;
-
- /* process digits */
- while (*ptr && isdigit((unsigned char) *ptr))
- {
- int8 digit = (*ptr++ - '0');
-
- if (unlikely(pg_mul_s32_overflow(tmp, 10, &tmp)) ||
- unlikely(pg_sub_s32_overflow(tmp, digit, &tmp)))
- goto out_of_range;
- }
-
- /* allow trailing whitespace, but not other trailing chars */
- while (*ptr != '\0' && isspace((unsigned char) *ptr))
- ptr++;
-
- if (unlikely(*ptr != '\0'))
- goto invalid_syntax;
-
- if (!neg)
- {
- /* could fail if input is most negative number */
- if (unlikely(tmp == PG_INT32_MIN))
- goto out_of_range;
- tmp = -tmp;
- }
-
- return tmp;
-
-out_of_range:
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("value \"%s\" is out of range for type %s",
- s, "integer")));
-
-invalid_syntax:
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
- errmsg("invalid input syntax for type %s: \"%s\"",
- "integer", s)));
return 0; /* keep compiler quiet */
}
@@ -543,25 +477,3 @@ pg_ltostr(char *str, int32 value)
return end;
}
-
-/*
- * pg_strtouint64
- * Converts 'str' into an unsigned 64-bit integer.
- *
- * This has the identical API to strtoul(3), except that it will handle
- * 64-bit ints even where "long" is narrower than that.
- *
- * For the moment it seems sufficient to assume that the platform has
- * such a function somewhere; let's not roll our own.
- */
-uint64
-pg_strtouint64(const char *str, char **endptr, int base)
-{
-#ifdef _MSC_VER /* MSVC only */
- return _strtoui64(str, endptr, base);
-#elif defined(HAVE_STRTOULL) && SIZEOF_LONG < 8
- return strtoull(str, endptr, base);
-#else
- return strtoul(str, endptr, base);
-#endif
-}
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index e5c7e5c7ee..749fc44623 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -37,7 +37,6 @@
#include "utils/builtins.h"
#include "utils/date.h"
#include "utils/hashutils.h"
-#include "utils/int8.h"
#include "utils/lsyscache.h"
#include "utils/rangetypes.h"
#include "utils/timestamp.h"
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index d36156f4e4..bdb6ab5e84 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -5626,8 +5626,8 @@ text_format(PG_FUNCTION_ARGS)
str = OutputFunctionCall(&typoutputinfo_width, value);
- /* pg_strtoint32 will complain about bad data or overflow */
- width = pg_strtoint32(str);
+ /* pg_strtoint will complain about bad data or overflow */
+ width = (int) pg_strtoint(str, 4);
pfree(str);
}
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index e9020ad231..84a9d39795 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -23,6 +23,7 @@
*-------------------------------------------------------------------------
*/
+#include "common/string.h"
#include "fe_utils/psqlscan_int.h"
/* context information for reporting errors in expressions */
@@ -205,7 +206,7 @@ notnull [Nn][Oo][Tt][Nn][Uu][Ll][Ll]
return MAXINT_PLUS_ONE_CONST;
}
{digit}+ {
- if (!strtoint64(yytext, true, &yylval->ival))
+ if (pg_strtoint64(yytext, &yylval->ival) != PG_STRTOINT_OK)
expr_yyerror_more(yyscanner, "bigint constant overflow",
strdup(yytext));
return INTEGER_CONST;
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ed7652bfbf..6581dc4de4 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -34,6 +34,7 @@
#include "postgres_fe.h"
#include "common/int.h"
#include "common/logging.h"
+#include "common/string.h"
#include "fe_utils/conditional.h"
#include "getopt_long.h"
#include "libpq-fe.h"
@@ -664,116 +665,6 @@ usage(void)
progname, progname);
}
-/* return whether str matches "^\s*[-+]?[0-9]+$" */
-static bool
-is_an_int(const char *str)
-{
- const char *ptr = str;
-
- /* skip leading spaces; cast is consistent with strtoint64 */
- while (*ptr && isspace((unsigned char) *ptr))
- ptr++;
-
- /* skip sign */
- if (*ptr == '+' || *ptr == '-')
- ptr++;
-
- /* at least one digit */
- if (*ptr && !isdigit((unsigned char) *ptr))
- return false;
-
- /* eat all digits */
- while (*ptr && isdigit((unsigned char) *ptr))
- ptr++;
-
- /* must have reached end of string */
- return *ptr == '\0';
-}
-
-
-/*
- * strtoint64 -- convert a string to 64-bit integer
- *
- * This function is a slightly modified version of scanint8() from
- * src/backend/utils/adt/int8.c.
- *
- * The function returns whether the conversion worked, and if so
- * "*result" is set to the result.
- *
- * If not errorOK, an error message is also printed out on errors.
- */
-bool
-strtoint64(const char *str, bool errorOK, int64 *result)
-{
- const char *ptr = str;
- int64 tmp = 0;
- bool neg = false;
-
- /*
- * Do our own scan, rather than relying on sscanf which might be broken
- * for long long.
- *
- * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate
- * value as a negative number.
- */
-
- /* skip leading spaces */
- while (*ptr && isspace((unsigned char) *ptr))
- ptr++;
-
- /* handle sign */
- if (*ptr == '-')
- {
- ptr++;
- neg = true;
- }
- else if (*ptr == '+')
- ptr++;
-
- /* require at least one digit */
- if (unlikely(!isdigit((unsigned char) *ptr)))
- goto invalid_syntax;
-
- /* process digits */
- while (*ptr && isdigit((unsigned char) *ptr))
- {
- int8 digit = (*ptr++ - '0');
-
- if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) ||
- unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
- goto out_of_range;
- }
-
- /* allow trailing whitespace, but not other trailing chars */
- while (*ptr != '\0' && isspace((unsigned char) *ptr))
- ptr++;
-
- if (unlikely(*ptr != '\0'))
- goto invalid_syntax;
-
- if (!neg)
- {
- if (unlikely(tmp == PG_INT64_MIN))
- goto out_of_range;
- tmp = -tmp;
- }
-
- *result = tmp;
- return true;
-
-out_of_range:
- if (!errorOK)
- fprintf(stderr,
- "value \"%s\" is out of range for type bigint\n", str);
- return false;
-
-invalid_syntax:
- if (!errorOK)
- fprintf(stderr,
- "invalid input syntax for type bigint: \"%s\"\n", str);
- return false;
-}
-
/* convert string to double, detecting overflows/underflows */
bool
strtodouble(const char *str, bool errorOK, double *dv)
@@ -1283,11 +1174,13 @@ getVariable(CState *st, char *name)
return var->svalue;
}
+
/* Try to convert variable to a value; return false on failure */
static bool
makeVariableValue(Variable *var)
{
size_t slen;
+ int64 ival;
if (var->value.type != PGBT_NO_VALUE)
return true; /* no work */
@@ -1320,15 +1213,16 @@ makeVariableValue(Variable *var)
{
setBoolValue(&var->value, false);
}
- else if (is_an_int(var->svalue))
+
+ /*
+ * Attempt an int64 conversion.
+ *
+ * A too-large but syntactically correct int64 will be parsed as a double
+ * below.
+ */
+ else if (pg_strtoint64(var->svalue, &ival) == PG_STRTOINT_OK)
{
- /* if it looks like an int, it must be an int without overflow */
- int64 iv;
-
- if (!strtoint64(var->svalue, false, &iv))
- return false;
-
- setIntValue(&var->value, iv);
+ setIntValue(&var->value, ival);
}
else /* type should be double */
{
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index c4a1e298e0..da3f8fa055 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -160,7 +160,6 @@ extern void syntax_error(const char *source, int lineno, const char *line,
const char *cmd, const char *msg,
const char *more, int col) pg_attribute_noreturn();
-extern bool strtoint64(const char *str, bool errorOK, int64 *pi);
extern bool strtodouble(const char *str, bool errorOK, double *pd);
#endif /* PGBENCH_H */
diff --git a/src/common/string.c b/src/common/string.c
index c9b8482cb0..788ea19b8e 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -22,6 +22,7 @@
#endif
#include "common/string.h"
+#include "common/int.h"
/*
@@ -58,6 +59,376 @@ strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base)
}
+/*
+ * pg_strtoint16
+ *
+ * Convert input string to a signed 16-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * NB: Accumulate input as a negative number, to deal with two's complement
+ * representation of the most negative number, which can't be represented as a
+ * positive number.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtoint16(const char *s, int16 *result)
+{
+ const char *ptr = s;
+ int16 tmp = 0;
+ bool neg = false;
+
+ /* skip leading spaces */
+ while (likely(*ptr) && isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '-')
+ {
+ ptr++;
+ neg = true;
+ }
+ else if (*ptr == '+')
+ ptr++;
+
+ /* require at least one digit */
+ if (unlikely(!isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ int8 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_s16_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_sub_s16_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace, but not other trailing chars */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ if (!neg)
+ {
+ /* could fail if input is most negative number */
+ if (unlikely(tmp == PG_INT16_MIN))
+ return PG_STRTOINT_RANGE_ERROR;
+ tmp = -tmp;
+ }
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
+/*
+ * pg_strtoint32
+ *
+ * Convert input string to a signed 32-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * NB: Accumulate input as a negative number, to deal with two's complement
+ * representation of the most negative number, which can't be represented as a
+ * positive number.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtoint32(const char *s, int32 *result)
+{
+ const char *ptr = s;
+ int32 tmp = 0;
+ bool neg = false;
+
+ /* skip leading spaces */
+ while (likely(*ptr) && isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '-')
+ {
+ ptr++;
+ neg = true;
+ }
+ else if (*ptr == '+')
+ ptr++;
+
+ /* require at least one digit */
+ if (unlikely(!isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ int8 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_s32_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_sub_s32_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace, but not other trailing chars */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ if (!neg)
+ {
+ /* could fail if input is most negative number */
+ if (unlikely(tmp == PG_INT32_MIN))
+ return PG_STRTOINT_RANGE_ERROR;
+ tmp = -tmp;
+ }
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
+/*
+ * pg_strtoint64
+ *
+ * Convert input string to a signed 32-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * NB: Accumulate input as a negative number, to deal with two's complement
+ * representation of the most negative number, which can't be represented as a
+ * positive number.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtoint64(const char *str, int64 *result)
+{
+ const char *ptr = str;
+ int64 tmp = 0;
+ bool neg = false;
+
+ /*
+ * Do our own scan, rather than relying on sscanf which might be broken
+ * for long long.
+ *
+ * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate
+ * value as a negative number.
+ */
+
+ /* skip leading spaces */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '-')
+ {
+ ptr++;
+ neg = true;
+ }
+ else if (*ptr == '+')
+ ptr++;
+
+ /* require at least one digit */
+ if (unlikely(!isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ int64 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace, but not other trailing chars */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ if (!neg)
+ {
+ if (unlikely(tmp == PG_INT64_MIN))
+ return PG_STRTOINT_RANGE_ERROR;
+ tmp = -tmp;
+ }
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
+/*
+ * pg_strtouint16
+ *
+ * Convert input string to an unsigned 16-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtouint16(const char *str, uint16 *result)
+{
+ const char *ptr = str;
+ uint16 tmp = 0;
+
+ /* skip leading spaces */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '+')
+ ptr++;
+ else if (unlikely(*ptr == '-'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* require at least one digit */
+ if (!unlikely(isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ uint8 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_u16_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_add_u16_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* but not other trailing chars */
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
+/*
+ * pg_strtouint32
+ *
+ * Convert input string to an unsigned 32-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtouint32(const char *str, uint32 *result)
+{
+ const char *ptr = str;
+ uint32 tmp = 0;
+
+ /* skip leading spaces */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '+')
+ ptr++;
+ else if (unlikely(*ptr == '-'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* require at least one digit */
+ if (!unlikely(isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ uint8 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_u32_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_add_u32_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* but not other trailing chars */
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
+/*
+ * pg_strtouint64
+ *
+ * Convert input string to an unsigned 64-bit integer. Allows any number of
+ * leading or trailing whitespace characters.
+ *
+ * The function returns immediately if the conversion failed with a status
+ * value to let the caller handle the error. On success, the result is
+ * stored in "*result".
+ */
+pg_strtoint_status
+pg_strtouint64(const char *str, uint64 *result)
+{
+ const char *ptr = str;
+ uint64 tmp = 0;
+
+ /* skip leading spaces */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* handle sign */
+ if (*ptr == '+')
+ ptr++;
+ else if (unlikely(*ptr == '-'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* require at least one digit */
+ if (!unlikely(isdigit((unsigned char) *ptr)))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ /* process digits, we know that we have one ahead per the last check */
+ do
+ {
+ uint64 digit = (*ptr++ - '0');
+
+ if (unlikely(pg_mul_u64_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_add_u64_overflow(tmp, digit, &tmp)))
+ return PG_STRTOINT_RANGE_ERROR;
+ }
+ while (isdigit((unsigned char) *ptr));
+
+ /* allow trailing whitespace */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ /* but not other trailing chars */
+ if (unlikely(*ptr != '\0'))
+ return PG_STRTOINT_SYNTAX_ERROR;
+
+ *result = tmp;
+ return PG_STRTOINT_OK;
+}
+
/*
* pg_clean_ascii -- Replace any non-ASCII chars with a '?' char
*
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 94f653fdd7..eb0f273d82 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -11,8 +11,26 @@
#define COMMON_STRING_H
extern bool pg_str_endswith(const char *str, const char *end);
-extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
- int base);
+extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr,
+ int base);
+
+/*
+ * Set of routines for conversion from string to various integer types.
+ */
+typedef enum pg_strtoint_status
+{
+ PG_STRTOINT_OK,
+ PG_STRTOINT_SYNTAX_ERROR,
+ PG_STRTOINT_RANGE_ERROR
+} pg_strtoint_status;
+
+extern pg_strtoint_status pg_strtoint16(const char *str, int16 *result);
+extern pg_strtoint_status pg_strtoint32(const char *str, int32 *result);
+extern pg_strtoint_status pg_strtoint64(const char *str, int64 *result);
+extern pg_strtoint_status pg_strtouint16(const char *str, uint16 *result);
+extern pg_strtoint_status pg_strtouint32(const char *str, uint32 *result);
+extern pg_strtoint_status pg_strtouint64(const char *str, uint64 *result);
+
extern void pg_clean_ascii(char *str);
extern int pg_strip_crlf(char *str);
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 937ddb7ef0..472a97c8b9 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -43,14 +43,12 @@ extern int namestrcmp(Name name, const char *str);
/* numutils.c */
extern int32 pg_atoi(const char *s, int size, int c);
-extern int16 pg_strtoint16(const char *s);
-extern int32 pg_strtoint32(const char *s);
+extern int64 pg_strtoint(const char *s, int size);
extern void pg_itoa(int16 i, char *a);
extern void pg_ltoa(int32 l, char *a);
extern void pg_lltoa(int64 ll, char *a);
extern char *pg_ltostr_zeropad(char *str, int32 value, int32 minwidth);
extern char *pg_ltostr(char *str, int32 value);
-extern uint64 pg_strtouint64(const char *str, char **endptr, int base);
/* oid.c */
extern oidvector *buildoidvector(const Oid *oids, int n);
diff --git a/src/include/utils/int8.h b/src/include/utils/int8.h
deleted file mode 100644
index 4836095d93..0000000000
--- a/src/include/utils/int8.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * int8.h
- * Declarations for operations on 64-bit integers.
- *
- *
- * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/include/utils/int8.h
- *
- * NOTES
- * These data types are supported on all 64-bit architectures, and may
- * be supported through libraries on some 32-bit machines. If your machine
- * is not currently supported, then please try to make it so, then post
- * patches to the postgresql.org hackers mailing list.
- *
- *-------------------------------------------------------------------------
- */
-#ifndef INT8_H
-#define INT8_H
-
-extern bool scanint8(const char *str, bool errorOK, int64 *result);
-
-#endif /* INT8_H */
overflow.tar.gz
Description: application/gzip
signature.asc
Description: PGP signature
