Hello Thomas,
+extern bool pg_strtoint64(const char *str, bool errorOK, int64 *result);
+extern uint64 pg_strtouint64(const char *str, char **endptr, int base);
One of these things is not like the other.
Indeed.
I agree that it is unfortunate, and it was bothering me a little as well.
Let's see... the int64 version is used only by pgbench and is being
promoted to common where it can be used by more code.
No and yes.
The pgbench code was a copy of server-side internal "scanint8", so it is
used both by pgbench and the server-side handling of "int8", it is used
significantly, taking advantage of its versatile error reporting feature
on both sides.
With a name like that, wouldn't it make sense to bring it into line with
the uint64 interface, and then move pgbench's error reporting stuff back
into pgbench?
That would need moving the server-side error handling as well, which I
would not really be happy with.
The uint64 one derives its shape from the family of standard functions
like strtol() so I think it wins.
Yep, it cannot be changed either.
I do not think that changing the error handling capability is appropriate,
it is really a feature of the function. The function could try to use an
internal pg_strtoint64 which would look like the other unsigned version,
but then it would not differentiate the various error conditions (out of
range vs syntax error).
The compromise I can offer is to change the name of the first one, say to
"pg_scanint8" to reflect its former backend name. Attached a v4 which does
a renaming so as to avoid the name similarity but signature difference. I
also made both error messages identical.
--
Fabien.
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ba57628f6f..5c3536b117 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;
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 8eedb613a1..e8744f054c 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"
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 6c2626ee62..3735268e3a 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"
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 1baf7ef31f..9a66ad41bd 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_scanint8(strVal(value), true, &val64))
{
/*
* It might actually fit in int32. Probably only INT_MIN can
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index d317fd7006..673f25a014 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 "replication/logical.h"
@@ -20,7 +21,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"
@@ -111,7 +111,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_scanint8(strVal(defel->arg), true, &parsed))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid proto_version")));
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index c92e9d5046..618660f372 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 206576d4bd..1f17987e85 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/int8.c b/src/backend/utils/adt/int8.c
index 0ff9394a2f..5992f4d425 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -18,12 +18,12 @@
#include <math.h>
#include "common/int.h"
+#include "common/string.h"
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "nodes/nodeFuncs.h"
#include "nodes/supportnodes.h"
#include "optimizer/optimizer.h"
-#include "utils/int8.h"
#include "utils/builtins.h"
@@ -47,89 +47,6 @@ 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
@@ -138,7 +55,7 @@ int8in(PG_FUNCTION_ARGS)
char *str = PG_GETARG_CSTRING(0);
int64 result;
- (void) scanint8(str, false, &result);
+ (void) pg_scanint8(str, false, &result);
PG_RETURN_INT64(result);
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 201784bbf6..02bdf278c2 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..f5d488c569 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -543,25 +543,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 cb16d701d8..3941cb82b5 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/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index e9020ad231..bfd3800318 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_scanint8(yytext, true, &yylval->ival))
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 8b84658ccd..ebc1964957 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"
@@ -669,7 +670,7 @@ is_an_int(const char *str)
{
const char *ptr = str;
- /* skip leading spaces; cast is consistent with strtoint64 */
+ /* skip leading spaces; cast is consistent with pg_scanint8 */
while (*ptr && isspace((unsigned char) *ptr))
ptr++;
@@ -689,90 +690,6 @@ is_an_int(const char *str)
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)
@@ -1324,7 +1241,7 @@ makeVariableValue(Variable *var)
/* if it looks like an int, it must be an int without overflow */
int64 iv;
- if (!strtoint64(var->svalue, false, &iv))
+ if (!pg_scanint8(var->svalue, false, &iv))
return false;
setIntValue(&var->value, iv);
@@ -5020,18 +4937,15 @@ set_random_seed(const char *seed)
else
{
/* parse unsigned-int seed value */
- unsigned long ulseed;
- char garbage;
-
- /* Don't try to use UINT64_FORMAT here; it might not work for sscanf */
- if (sscanf(seed, "%lu%c", &ulseed, &garbage) != 1)
+ char *garbage;
+ iseed = pg_strtouint64(seed, &garbage, 10);
+ if (*garbage != '\0')
{
fprintf(stderr,
"unrecognized random seed option \"%s\": expecting an unsigned integer, \"time\" or \"rand\"\n",
seed);
return false;
}
- iseed = (uint64) ulseed;
}
if (seed != NULL)
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 b01a56ceaa..b51641038e 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -22,6 +22,7 @@
#endif
#include "common/string.h"
+#include "common/int.h"
/*
@@ -57,6 +58,123 @@ strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base)
return (int) val;
}
+/*
+ * pg_scanint8 -- convert a string to 64-bit integer
+ *
+ * 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
+pg_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)
+ {
+ if (unlikely(tmp == PG_INT64_MIN))
+ goto out_of_range;
+ tmp = -tmp;
+ }
+
+ *result = tmp;
+ return true;
+
+out_of_range:
+ if (!errorOK)
+#ifdef FRONTEND
+ fprintf(stderr,
+ "value \"%s\" is out of range for type %s\n", str, "bigint");
+#else
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("value \"%s\" is out of range for type %s",
+ str, "bigint")));
+#endif
+ return false;
+
+invalid_syntax:
+ if (!errorOK)
+#ifdef FRONTEND
+ fprintf(stderr,
+ "invalid input syntax for type %s: \"%s\"\n", "bigint", str);
+#else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("invalid input syntax for type %s: \"%s\"",
+ "bigint", str)));
+#endif
+
+ return false;
+}
+
+/*
+ * 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
+}
+
/*
* 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 77f31337ca..3d551d2934 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -13,6 +13,9 @@
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 bool pg_scanint8(const char *str, bool errorOK, int64 *result);
+extern uint64 pg_strtouint64(const char *str, char **endptr, int base);
extern void pg_clean_ascii(char *str);
+
#endif /* COMMON_STRING_H */
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index a3cd7d26fa..e014a45705 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -50,7 +50,6 @@ 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 */
