> On Nov 19, 2025, at 23:37, Heikki Linnakangas <[email protected]> wrote:
> 
> Here's a small patch to replace the int64 parsing code in pgbench with a call 
> to strtoi64(). Makes it a little simpler.
> 
> Spotted this while grepping for all the different integer parsing functions 
> we have. We could probably consolidate them some more, we still have quite a 
> different integer-parsing routines in the backend and in the frontend. But 
> this is one small, straightforward step in that direction.
> 
> - Heikki
> <0001-Use-strtoi64-in-pgbench-replacing-its-open-coded-imp.patch>

I have no concern if we decide to no longer support tailing spaces, while I 
still got a couple of small comments:

1
```
-/* return whether str matches "^\s*[-+]?[0-9]+$" */
+/*
+ * Return whether str matches "^\s*[-+]?[0-9]+$"
+ *
+ * This should agree with strtoint64() on what's accepted, ignoring overflows.
+ */
 static bool
 is_an_int(const char *str)
```

Here you added a comment saying "ignoring overflows”, yes, is_an_int() doesn’t 
check if the integer overflows.

But looking at where the function is called:
```
        else if (is_an_int(var->svalue))
        {
                /* 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);
        }
```

The comment says “it must be an int without overflow”, so this comment should 
be updated as well.

2
```
+       if (unlikely(errno != 0))
        {
-               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;
+               if (!errorOK)
+                       pg_log_error("value \"%s\" is out of range for type 
bigint", str);
+               return false;
        }
```

Here we log an “out out range” error when errno is not 0, which is inaccurate, 
we should check ERANGE.

strtoi64() maps to strtol()/strtoll(), the functions could return more errors 
than ERANGE.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to