From 122aa10d33680d2e36ed975233e7865e07fa6e3c Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Thu, 30 Jul 2026 07:40:31 +0000 Subject: [PATCH v2 1/3] Make tid parsing consistent across libcs A tid consists of two values: a block number and an offset number. A tid can be parsed from a string representation of the form "(block,offset)". This parsing goes through tidin(), which uses strtoul() to parse integer values. strtoul() can set two different values for errno: EINVAL and ERANGE. Quoting the man page for EINVAL: > The value of base is not supported or no conversion could be performed > (the last feature is not portable across all platforms). Emphasis on the portion in parentheses. Take the following example: strtoul("", &endptr, 10); On glibc, the output value is 0, errno *is not* set, and endptr points to the empty string. On Apple libc, the output value is 0, errno *is* set to EINVAL, and endptr points to the empty string. With inputs of either "(,1)" or "(1,0)" on glibc, tidin() would produce a tid of (0,1) or (1,0) respectively. On Apple libc, tidin() would throw an error. This is inconsistent behavior across platforms, which I don't think is in the interest of Postgres. Queries should be portable across platforms to keep applications easier to write. Fixes: https://www.postgresql.org/message-id/19584-e60c446ba6f57c9c%40postgresql.org # ------------------------ >8 ------------------------ Do not modify or # remove the line above. Everything below it will be ignored. # # On branch strtoul Changes to be committed: modified: # src/backend/utils/adt/tid.c modified: # src/test/regress/expected/tid.out modified: # src/test/regress/sql/tid.sql # # Changes not staged for commit: modified: # src/backend/access/transam/xlog.c modified: # src/backend/access/transam/xlogrecovery.c modified: # src/backend/replication/pgoutput/pgoutput.c modified: # src/bin/pg_combinebackup/pg_combinebackup.c # diff --git c/src/backend/utils/adt/tid.c i/src/backend/utils/adt/tid.c index 9257886f1da..a97873f91ba 100644 --- c/src/backend/utils/adt/tid.c +++ i/src/backend/utils/adt/tid.c @@ -73,7 +73,7 @@ tidin(PG_FUNCTION_ARGS) errno = 0; cvt = strtoul(coord[0], &badp, 10); - if (errno || *badp != DELIM) + if (badp == coord[0] || errno || *badp != DELIM) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", @@ -95,8 +95,7 @@ tidin(PG_FUNCTION_ARGS) #endif cvt = strtoul(coord[1], &badp, 10); - if (errno || *badp != RDELIM || - cvt > USHRT_MAX) + if (badp == coord[1] || errno || *badp != RDELIM || cvt > USHRT_MAX) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", diff --git c/src/test/regress/expected/tid.out i/src/test/regress/expected/tid.out index 3497a77688b..45cc61fb1ca 100644 --- c/src/test/regress/expected/tid.out +++ i/src/test/regress/expected/tid.out @@ -17,6 +17,14 @@ SELECT '(1,65536)'::tid; -- error ERROR: invalid input syntax for type tid: "(1,65536)" LINE 1: SELECT '(1,65536)'::tid; ^ +SELECT '(,1)'::tid; -- error +ERROR: invalid input syntax for type tid: "(,1)" +LINE 1: SELECT '(,1)'::tid; + ^ +SELECT '(1,)'::tid; -- error +ERROR: invalid input syntax for type tid: "(1,)" +LINE 1: SELECT '(1,)'::tid; + ^ -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid'); pg_input_is_valid diff --git c/src/test/regress/sql/tid.sql i/src/test/regress/sql/tid.sql index c0a70be5cbd..51d00b92074 100644 --- c/src/test/regress/sql/tid.sql +++ i/src/test/regress/sql/tid.sql @@ -8,6 +8,8 @@ SELECT SELECT '(4294967296,1)'::tid; -- error SELECT '(1,65536)'::tid; -- error +SELECT '(,1)'::tid; -- error +SELECT '(1,)'::tid; -- error -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid'); Signed-off-by: Tristan Partin --- src/backend/utils/adt/tid.c | 5 ++--- src/test/regress/expected/tid.out | 8 ++++++++ src/test/regress/sql/tid.sql | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index 9257886f1da..a97873f91ba 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -73,7 +73,7 @@ tidin(PG_FUNCTION_ARGS) errno = 0; cvt = strtoul(coord[0], &badp, 10); - if (errno || *badp != DELIM) + if (badp == coord[0] || errno || *badp != DELIM) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", @@ -95,8 +95,7 @@ tidin(PG_FUNCTION_ARGS) #endif cvt = strtoul(coord[1], &badp, 10); - if (errno || *badp != RDELIM || - cvt > USHRT_MAX) + if (badp == coord[1] || errno || *badp != RDELIM || cvt > USHRT_MAX) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", diff --git a/src/test/regress/expected/tid.out b/src/test/regress/expected/tid.out index 3497a77688b..45cc61fb1ca 100644 --- a/src/test/regress/expected/tid.out +++ b/src/test/regress/expected/tid.out @@ -17,6 +17,14 @@ SELECT '(1,65536)'::tid; -- error ERROR: invalid input syntax for type tid: "(1,65536)" LINE 1: SELECT '(1,65536)'::tid; ^ +SELECT '(,1)'::tid; -- error +ERROR: invalid input syntax for type tid: "(,1)" +LINE 1: SELECT '(,1)'::tid; + ^ +SELECT '(1,)'::tid; -- error +ERROR: invalid input syntax for type tid: "(1,)" +LINE 1: SELECT '(1,)'::tid; + ^ -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid'); pg_input_is_valid diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql index c0a70be5cbd..51d00b92074 100644 --- a/src/test/regress/sql/tid.sql +++ b/src/test/regress/sql/tid.sql @@ -8,6 +8,8 @@ SELECT SELECT '(4294967296,1)'::tid; -- error SELECT '(1,65536)'::tid; -- error +SELECT '(,1)'::tid; -- error +SELECT '(1,)'::tid; -- error -- Also try it with non-error-throwing API SELECT pg_input_is_valid('(0)', 'tid'); -- Tristan Partin https://tristan.partin.io