pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
15+ messages / 4 participants
[nested] [flat]
* BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
@ 2026-02-13 07:46 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 15+ messages in thread
From: PG Bug reporting form @ 2026-02-13 07:46 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: ranvis@gmail.com
The following bug has been logged on the website:
Bug reference: 19406
Logged by: SATŌ Kentarō
Email address: ranvis@gmail.com
PostgreSQL version: 15.16
Operating system: Linux 5.14.0-611.27.1.el9_7.x86_64
Description:
After upgrading from PostgreSQL 15.15 to 15.16, substring(text) raises:
>ERROR: invalid byte sequence for encoding "UTF8": 0xe6 0x97
on valid UTF-8 text stored in a TOAST-compressed column.
The same SQL works on PostgreSQL 15.15.
The data is valid UTF-8. The problem occurs only when operating on a
TOAST-compressed value.
Steps to Reproduce:
$ psql
psql (15.16)
Type "help" for help.
user=> \x
Expanded display is on.
user=> \set VERBOSITY verbose
user=> create table toast_repro (data text);
CREATE TABLE
user=> insert into toast_repro select repeat('日向', 2000); -- e6 97 a5 e5 90
91
INSERT 0 1
user=> select pg_column_size(data), octet_length(data) from toast_repro;
-[ RECORD 1 ]--+------
pg_column_size | 153
octet_length | 12000
user=> select substring(data from 1 for 2) from toast_repro;
-[ RECORD 1 ]---
substring | 日向
user=> select substring(data from 1 for 1) from toast_repro;
ERROR: 22021: invalid byte sequence for encoding "UTF8": 0xe6 0x97
LOCATION: report_invalid_encoding_int, mbutils.c:1796
user=> select substring('日向' from 1 for 1);
-[ RECORD 1 ]-
substring | 日
user=> select substring(data::varchar(100) from 1 for 1) from toast_repro;
-[ RECORD 1 ]-
substring | 日
user=> select substring(data from 1 for 1) from ( select data || '' as data
from toast_repro) t;
-[ RECORD 1 ]-
substring | 日
Environment:
version | PostgreSQL 15.16 on x86_64-pc-linux-gnu, compiled by gcc (GCC)
11.5.0 20240719 (Red Hat 11.5.0-11), 64-bit
server_encoding | UTF8
client_encoding | UTF8
lc_collate | ja_JP.UTF-8
lc_ctype | ja_JP.UTF-8
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
@ 2026-02-13 17:27 ` Noah Misch <noah@leadboat.com>
2026-02-13 21:58 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 15+ messages in thread
From: Noah Misch @ 2026-02-13 17:27 UTC (permalink / raw)
To: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: thomas.munro@gmail.com
On Fri, Feb 13, 2026 at 07:46:22AM +0000, PG Bug reporting form wrote:
> After upgrading from PostgreSQL 15.15 to 15.16, substring(text) raises:
> >ERROR: invalid byte sequence for encoding "UTF8": 0xe6 0x97
> on valid UTF-8 text stored in a TOAST-compressed column.
> user=> select substring(data from 1 for 1) from toast_repro;
> ERROR: 22021: invalid byte sequence for encoding "UTF8": 0xe6 0x97
Thanks for the report. That is a bug and a regression; I regret missing it
during review. The substring operation works by taking a 4-byte slice from
the toasted value (4 bytes being the max length of a UTF8 char in PostgreSQL),
the finding the actual first character within those bytes. However, it
incorrectly requires those four bytes to be a valid UTF8 string. I'll start
on a fix.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-13 21:58 ` Thomas Munro <thomas.munro@gmail.com>
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Munro @ 2026-02-13 21:58 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sat, Feb 14, 2026 at 6:27 AM Noah Misch <noah@leadboat.com> wrote:
> On Fri, Feb 13, 2026 at 07:46:22AM +0000, PG Bug reporting form wrote:
> > After upgrading from PostgreSQL 15.15 to 15.16, substring(text) raises:
> > >ERROR: invalid byte sequence for encoding "UTF8": 0xe6 0x97
> > on valid UTF-8 text stored in a TOAST-compressed column.
>
> > user=> select substring(data from 1 for 1) from toast_repro;
> > ERROR: 22021: invalid byte sequence for encoding "UTF8": 0xe6 0x97
>
> Thanks for the report. That is a bug and a regression; I regret missing it
> during review. The substring operation works by taking a 4-byte slice from
> the toasted value (4 bytes being the max length of a UTF8 char in PostgreSQL),
> the finding the actual first character within those bytes. However, it
> incorrectly requires those four bytes to be a valid UTF8 string. I'll start
> on a fix.
Ack. Also looking into this.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-13 22:48 ` Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
1 sibling, 1 reply; 15+ messages in thread
From: Noah Misch @ 2026-02-13 22:48 UTC (permalink / raw)
To: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: thomas.munro@gmail.com
On Fri, Feb 13, 2026 at 09:27:02AM -0800, Noah Misch wrote:
> On Fri, Feb 13, 2026 at 07:46:22AM +0000, PG Bug reporting form wrote:
> > After upgrading from PostgreSQL 15.15 to 15.16, substring(text) raises:
> > >ERROR: invalid byte sequence for encoding "UTF8": 0xe6 0x97
> > on valid UTF-8 text stored in a TOAST-compressed column.
>
> > user=> select substring(data from 1 for 1) from toast_repro;
> > ERROR: 22021: invalid byte sequence for encoding "UTF8": 0xe6 0x97
>
> Thanks for the report. That is a bug and a regression; I regret missing it
> during review. The substring operation works by taking a 4-byte slice from
> the toasted value (4 bytes being the max length of a UTF8 char in PostgreSQL),
> the finding the actual first character within those bytes. However, it
> incorrectly requires those four bytes to be a valid UTF8 string. I'll start
> on a fix.
Attached. I may add some more tests, e.g. a toasted invalid string where the
detoasted length is less than the slice we request. This version is viable,
however.
I audited the other pg_mbstrlen_with_len(), and I think they're all okay with
an error if the input has an incomplete char. Hence, those don't need changes
beyond what we're already released. Most pass either parser input or an
existing datum with its len. text_position_get_match_pos() is the most subtle
caller, and I think it's fine.
I audited other uses of slice detoast. The only other one is bytea substring,
which is obviously indifferent to character encoding.
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, that yields 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. Fix that incidentally. That and stopping the char count
early might improve performance.
Back-patch to v14 (all supported versions).
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..d0bc9e5 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size != -1 ?
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1) :
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..d850664 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -388,6 +394,16 @@ SELECT SUBSTRING('a' SIMILAR U&'\00AC' ESCAPE U&'\00A7');
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
-- Levenshtein distance metric: exercise character length cache.
SELECT U&"real\00A7_name" FROM (select 1) AS x(real_name);
ERROR: column "real§_name" does not exist
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..1b2178b 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -222,6 +223,11 @@ DROP FUNCTION test_text_to_bytea;
-- substring slow path: multi-byte escape char vs. multi-byte pattern char.
SELECT SUBSTRING('a' SIMILAR U&'\00AC' ESCAPE U&'\00A7');
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
-- Levenshtein distance metric: exercise character length cache.
SELECT U&"real\00A7_name" FROM (select 1) AS x(real_name);
-- JSON errcontext: truncate long data.
Attachments:
toast-slice-mblen-v1.patch (text/plain, 7.5K) download
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, that yields 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. Fix that incidentally. That and stopping the char count
early might improve performance.
Back-patch to v14 (all supported versions).
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..d0bc9e5 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size != -1 ?
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1) :
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..d850664 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -388,6 +394,16 @@ SELECT SUBSTRING('a' SIMILAR U&'\00AC' ESCAPE U&'\00A7');
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
-- Levenshtein distance metric: exercise character length cache.
SELECT U&"real\00A7_name" FROM (select 1) AS x(real_name);
ERROR: column "real§_name" does not exist
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..1b2178b 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -222,6 +223,11 @@ DROP FUNCTION test_text_to_bytea;
-- substring slow path: multi-byte escape char vs. multi-byte pattern char.
SELECT SUBSTRING('a' SIMILAR U&'\00AC' ESCAPE U&'\00A7');
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
-- Levenshtein distance metric: exercise character length cache.
SELECT U&"real\00A7_name" FROM (select 1) AS x(real_name);
-- JSON errcontext: truncate long data.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-14 00:21 ` Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-17 18:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 15+ messages in thread
From: Noah Misch @ 2026-02-14 00:21 UTC (permalink / raw)
To: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: thomas.munro@gmail.com
On Fri, Feb 13, 2026 at 02:48:04PM -0800, Noah Misch wrote:
> On Fri, Feb 13, 2026 at 09:27:02AM -0800, Noah Misch wrote:
> > On Fri, Feb 13, 2026 at 07:46:22AM +0000, PG Bug reporting form wrote:
> > > After upgrading from PostgreSQL 15.15 to 15.16, substring(text) raises:
> > > >ERROR: invalid byte sequence for encoding "UTF8": 0xe6 0x97
> > > on valid UTF-8 text stored in a TOAST-compressed column.
> >
> > > user=> select substring(data from 1 for 1) from toast_repro;
> > > ERROR: 22021: invalid byte sequence for encoding "UTF8": 0xe6 0x97
> >
> > Thanks for the report. That is a bug and a regression; I regret missing it
> > during review. The substring operation works by taking a 4-byte slice from
> > the toasted value (4 bytes being the max length of a UTF8 char in PostgreSQL),
> > the finding the actual first character within those bytes. However, it
> > incorrectly requires those four bytes to be a valid UTF8 string. I'll start
> > on a fix.
>
> Attached. I may add some more tests, e.g. a toasted invalid string where the
> detoasted length is less than the slice we request.
Tests already covered that in particular, but I added some other tests. I
think this is now ready. Review welcome. I have a Valgrind test run ongoing.
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, that yields 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. Fix that incidentally. That and stopping the char count
early might improve performance.
Back-patch to v14 (all supported versions).
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..99a5470 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size == -1 ?
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..cac1cb7 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -375,7 +381,43 @@ NOTICE: MULE_INTERNAL LC2: \x908283 -> {9470595} -> \x908283 = OK
t
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+
+(1 row)
+
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+ substring
+-----------
+ 🚀
+(1 row)
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..782f90f 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -212,7 +213,23 @@ INSERT INTO encoding_tests VALUES
SELECT COUNT(test_encoding(encoding, description, input)) > 0
FROM encoding_tests;
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
Attachments:
toast-slice-mblen-v2.patch (text/plain, 8.6K) download
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, that yields 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. Fix that incidentally. That and stopping the char count
early might improve performance.
Back-patch to v14 (all supported versions).
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..99a5470 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size == -1 ?
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..cac1cb7 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -375,7 +381,43 @@ NOTICE: MULE_INTERNAL LC2: \x908283 -> {9470595} -> \x908283 = OK
t
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+
+(1 row)
+
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+ substring
+-----------
+ 🚀
+(1 row)
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..782f90f 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -212,7 +213,23 @@ INSERT INTO encoding_tests VALUES
SELECT COUNT(test_encoding(encoding, description, input)) > 0
FROM encoding_tests;
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-14 05:38 ` Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 15+ messages in thread
From: Noah Misch @ 2026-02-14 05:38 UTC (permalink / raw)
To: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: thomas.munro@gmail.com
On Fri, Feb 13, 2026 at 04:21:13PM -0800, Noah Misch wrote:
> Review welcome. I have a Valgrind test run ongoing.
Valgrind found the complaint below, but I think this an instrumentation
problem. I've added a fix for that instrumentation. I also made minor edits
to the log message of the main patch, hence v3.
The release team is preparing to announce a 2026-02-26 out-of-cycle release in
light of this regression. I plan to push these fixes at 2026-02-14T20:00+0000
to unblock that formal announcement.
==00:00:01:11.756 3464664== VALGRINDERROR-BEGIN
==00:00:01:11.756 3464664== Unaddressable byte(s) found during client check request
==00:00:01:11.769 3464664== at 0xC6076A: pg_mblen_with_len (mbutils.c:1115)
==00:00:01:11.769 3464664== by 0xC07CD3: pg_mbcharcliplen_chars (varlena.c:807)
==00:00:01:11.770 3464664== by 0xC07AAD: text_substring (varlena.c:732)
==00:00:01:11.770 3464664== by 0xC07797: text_substr (varlena.c:553)
==00:00:01:11.770 3464664== by 0x779688: ExecInterpExpr (execExprInterp.c:953)
==00:00:01:11.770 3464664== by 0x77BBD6: ExecInterpExprStillValid (execExprInterp.c:2299)
==00:00:01:11.770 3464664== by 0x7DBBC4: ExecEvalExprNoReturn (executor.h:423)
==00:00:01:11.770 3464664== by 0x7DBC73: ExecEvalExprNoReturnSwitchContext (executor.h:464)
==00:00:01:11.770 3464664== by 0x7DBCD3: ExecProject (executor.h:496)
==00:00:01:11.770 3464664== by 0x7DC134: ExecScanExtended (execScan.h:234)
==00:00:01:11.770 3464664== by 0x7DC474: ExecSeqScanWithProject (nodeSeqscan.c:162)
==00:00:01:11.770 3464664== by 0x794561: ExecProcNodeFirst (execProcnode.c:469)
==00:00:01:11.770 3464664== Address 0x19340a5e is 12,062 bytes inside a block of size 12,064 alloc'd
==00:00:01:11.770 3464664== at 0x4C29F73: malloc (vg_replace_malloc.c:309)
==00:00:01:11.770 3464664== by 0xC79F03: AllocSetAllocLarge (aset.c:756)
==00:00:01:11.770 3464664== by 0xC7AA4C: AllocSetAlloc (aset.c:1033)
==00:00:01:11.770 3464664== by 0xC8B37E: palloc (mcxt.c:1408)
==00:00:01:11.770 3464664== by 0x4B0182: detoast_attr_slice (detoast.c:324)
==00:00:01:11.770 3464664== by 0xC5177F: pg_detoast_datum_slice (fmgr.c:1825)
==00:00:01:11.770 3464664== by 0xC07A11: text_substring (varlena.c:716)
==00:00:01:11.770 3464664== by 0xC07797: text_substr (varlena.c:553)
==00:00:01:11.770 3464664== by 0x779688: ExecInterpExpr (execExprInterp.c:953)
==00:00:01:11.770 3464664== by 0x77BBD6: ExecInterpExprStillValid (execExprInterp.c:2299)
==00:00:01:11.770 3464664== by 0x7DBBC4: ExecEvalExprNoReturn (executor.h:423)
==00:00:01:11.770 3464664== by 0x7DBC73: ExecEvalExprNoReturnSwitchContext (executor.h:464)
==00:00:01:11.770 3464664==
==00:00:01:11.770 3464664== VALGRINDERROR-END
{
<insert_a_suppression_name_here>
Memcheck:User
fun:pg_mblen_with_len
fun:pg_mbcharcliplen_chars
fun:text_substring
fun:text_substr
fun:ExecInterpExpr
fun:ExecInterpExprStillValid
fun:ExecEvalExprNoReturn
fun:ExecEvalExprNoReturnSwitchContext
fun:ExecProject
fun:ExecScanExtended
fun:ExecSeqScanWithProject
fun:ExecProcNodeFirst
}
2026-02-13 21:03:38.905 PST client backend[3464664] pg_regress/encoding ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
2026-02-13 21:03:38.905 PST client backend[3464664] pg_regress/encoding STATEMENT: SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
**00:00:01:11.771 3464664** Valgrind detected 1 error(s) during execution of "SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;"
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, 8 bytes spans 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. For avoidance of doubt, stop detoasting extra.
Back-patch to v14, like the above commit. For applications using
SUBSTRING() on non-ASCII column values, consider applying this to your
copy of any of the February 12, 2026 releases.
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..99a5470 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size == -1 ?
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..cac1cb7 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -375,7 +381,43 @@ NOTICE: MULE_INTERNAL LC2: \x908283 -> {9470595} -> \x908283 = OK
t
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+
+(1 row)
+
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+ substring
+-----------
+ 🚀
+(1 row)
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..782f90f 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -212,7 +213,23 @@ INSERT INTO encoding_tests VALUES
SELECT COUNT(test_encoding(encoding, description, input)) > 0
FROM encoding_tests;
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
From: Noah Misch <noah@leadboat.com>
pg_mblen_range, pg_mblen_with_len: Valgrind after encoding ereport.
The prior order caused spurious Valgrind errors. They're spurious
because the ereport(ERROR) stops subsequent code from accessing the
memory in question. pg_mblen_cstr() ordered the checks correctly, but
these other two did not. Back-patch to v14, like commit
1e7fe06c10c0a8da9dd6261a6be8d405dc17c728.
Reviewed-by: FIXME
Discussion: https://postgr.es/m/FIXME
Backpatch-through: 14
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index a5a7348..f3f94d4 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1086,15 +1086,16 @@ pg_mblen_range(const char *mbstr, const char *end)
int length = pg_wchar_table[DatabaseEncoding->encoding].mblen((const unsigned char *) mbstr);
Assert(end > mbstr);
+
+ if (unlikely(mbstr + length > end))
+ report_invalid_encoding_db(mbstr, length, end - mbstr);
+
#ifdef VALGRIND_EXPENSIVE
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, end - mbstr);
#else
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, length);
#endif
- if (unlikely(mbstr + length > end))
- report_invalid_encoding_db(mbstr, length, end - mbstr);
-
return length;
}
@@ -1109,15 +1110,16 @@ pg_mblen_with_len(const char *mbstr, int limit)
int length = pg_wchar_table[DatabaseEncoding->encoding].mblen((const unsigned char *) mbstr);
Assert(limit >= 1);
+
+ if (unlikely(length > limit))
+ report_invalid_encoding_db(mbstr, length, limit);
+
#ifdef VALGRIND_EXPENSIVE
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, limit);
#else
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, length);
#endif
- if (unlikely(length > limit))
- report_invalid_encoding_db(mbstr, length, limit);
-
return length;
}
Attachments:
toast-slice-mblen-v3.patch (text/plain, 8.7K) download
From: Noah Misch <noah@leadboat.com>
Fix SUBSTRING() for toasted multibyte characters.
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() does
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, 8 bytes spans 2 complete chars and 1 partial char.
Fix this by replacing this pg_mbstrlen_with_len() call with a string
traversal that differs by stopping upon finding as many chars as the
substring could need. This also makes SUBSTRING() stop raising an
encoding error if the incomplete char is past the end of the substring.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. For avoidance of doubt, stop detoasting extra.
Back-patch to v14, like the above commit. For applications using
SUBSTRING() on non-ASCII column values, consider applying this to your
copy of any of the February 12, 2026 releases.
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd71..99a5470 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -133,6 +133,7 @@ static text *text_substring(Datum str,
int32 start,
int32 length,
bool length_not_specified);
+static int pg_mbcharcliplen_chars(const char *mbstr, int len, int limit);
static text *text_overlay(text *t1, text *t2, int sp, int sl);
static int text_position(text *t1, text *t2, Oid collid);
static void text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state);
@@ -586,7 +587,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -684,11 +685,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
+ * Ending at position 1, exclusive, obviously yields an empty
+ * string. A zero or negative value can happen if the start was
+ * negative or one. SQL99 says to return a zero-length string.
*/
- if (E < 1)
+ if (E <= 1)
return cstring_to_text("");
/*
@@ -698,11 +699,11 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
L1 = E - S1;
/*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any longer than the
+ * inclusive end position times the encoding max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -726,8 +727,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
+ slice_strlen =
+ (slice_size == -1 ?
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice), slice_len, E - 1));
/*
* Check that the start position wasn't > slice_strlen. If so, SQL99
@@ -783,6 +786,35 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
}
/*
+ * pg_mbcharcliplen_chars -
+ * Mirror pg_mbcharcliplen(), except return value unit is chars, not bytes.
+ *
+ * This mirrors all the dubious historical behavior, so it's static to
+ * discourage proliferation. The assertions are specific to the one caller.
+ */
+static int
+pg_mbcharcliplen_chars(const char *mbstr, int len, int limit)
+{
+ int nch = 0;
+ int l;
+
+ Assert(len > 0);
+ Assert(limit > 0);
+ Assert(pg_database_encoding_max_length() > 1);
+
+ while (len > 0 && *mbstr)
+ {
+ l = pg_mblen_with_len(mbstr, len);
+ nch++;
+ if (nch == limit)
+ break;
+ len -= l;
+ mbstr += l;
+ }
+ return nch;
+}
+
+/*
* textoverlay
* Replace specified substring of first string with second
*
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38c..cac1cb7 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -375,7 +381,43 @@ NOTICE: MULE_INTERNAL LC2: \x908283 -> {9470595} -> \x908283 = OK
t
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+
+(1 row)
+
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+ substring
+-----------
+ 🚀
+(1 row)
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0..782f90f 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -212,7 +213,23 @@ INSERT INTO encoding_tests VALUES
SELECT COUNT(test_encoding(encoding, description, input)) > 0
FROM encoding_tests;
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
mblen-valgrind-after-report-v1.patch (text/plain, 1.7K) download
From: Noah Misch <noah@leadboat.com>
pg_mblen_range, pg_mblen_with_len: Valgrind after encoding ereport.
The prior order caused spurious Valgrind errors. They're spurious
because the ereport(ERROR) stops subsequent code from accessing the
memory in question. pg_mblen_cstr() ordered the checks correctly, but
these other two did not. Back-patch to v14, like commit
1e7fe06c10c0a8da9dd6261a6be8d405dc17c728.
Reviewed-by: FIXME
Discussion: https://postgr.es/m/FIXME
Backpatch-through: 14
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index a5a7348..f3f94d4 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1086,15 +1086,16 @@ pg_mblen_range(const char *mbstr, const char *end)
int length = pg_wchar_table[DatabaseEncoding->encoding].mblen((const unsigned char *) mbstr);
Assert(end > mbstr);
+
+ if (unlikely(mbstr + length > end))
+ report_invalid_encoding_db(mbstr, length, end - mbstr);
+
#ifdef VALGRIND_EXPENSIVE
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, end - mbstr);
#else
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, length);
#endif
- if (unlikely(mbstr + length > end))
- report_invalid_encoding_db(mbstr, length, end - mbstr);
-
return length;
}
@@ -1109,15 +1110,16 @@ pg_mblen_with_len(const char *mbstr, int limit)
int length = pg_wchar_table[DatabaseEncoding->encoding].mblen((const unsigned char *) mbstr);
Assert(limit >= 1);
+
+ if (unlikely(length > limit))
+ report_invalid_encoding_db(mbstr, length, limit);
+
#ifdef VALGRIND_EXPENSIVE
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, limit);
#else
VALGRIND_CHECK_MEM_IS_DEFINED(mbstr, length);
#endif
- if (unlikely(length > limit))
- report_invalid_encoding_db(mbstr, length, limit);
-
return length;
}
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-14 08:15 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Munro @ 2026-02-14 08:15 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sat, Feb 14, 2026 at 6:38 PM Noah Misch <noah@leadboat.com> wrote:
> [mblen-valgrind-after-report-v1.patch]
LGTM. The new valgrind check should clearly be after the new non-local exit.
Studying the other patch...
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
@ 2026-02-14 19:07 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Munro @ 2026-02-14 19:07 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sat, Feb 14, 2026 at 9:15 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sat, Feb 14, 2026 at 6:38 PM Noah Misch <noah@leadboat.com> wrote:
> > [mblen-valgrind-after-report-v1.patch]
>
> LGTM. The new valgrind check should clearly be after the new non-local exit.
>
> Studying the other patch...
/*
- * Total slice size in bytes can't be any
longer than the start
- * position plus substring length times the
encoding max length.
- * If that overflows, we can just use -1.
+ * Total slice size in bytes can't be any
longer than the
+ * inclusive end position times the encoding
max length. If that
+ * overflows, we can just use -1.
*/
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
Isn't it still conceptually "exclusive", but adjusted to be zero-indexed?
/* Now we can get the actual length of the slice in MB
characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
-
slice_len);
+ slice_strlen =
+ (slice_size == -1 ?
+ pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
+ pg_mbcharcliplen_chars(VARDATA_ANY(slice),
slice_len, E - 1));
Comment presumably needs adjustment to say that we only count as far
as we need to, and why.
There is something a bit strange about all this, though.
pg_mbstrlen_with_len(..., -1) returns 0, so if you ask for characters
that really exist past 2^29 (~500 million), you must get an empty
string, right? That's hard to reach, pre-existing and out of scope
for the immediate problem report, except ... now we're contorting the
code even further to keep it.
The outline I had come up with before seeing your patch was: let's
just delete it. The position search can check bounds incrementally,
following our general approach. This avoids the reported problem by
ditching the pre-flight scan through the slice (up to 4x more
pg_mblen_XXX calls and memory access than we strictly need), and also
the special cases for empty strings since they already fall out of the
general behaviour (am I missing something?), not leaving much code
behind. As far as I can see so far, the only user-visible side-effect
requires corruption: substring() moves from the
internal-NUL-is-terminator category to internal-NUL-is-character
category, but that's an implementation detail.
When I saw your patch yesterday, I initially abandoned the thought,
thinking that your idea looked more conservative, but after sleeping
on it and reflecting again on these oddities, I have merged my draft
implementation with your tests, ancient detoasting fence post
observation and commit message, just to see if you think this approach
might be worth considering further.
Attachments:
v2tm-0001-Fix-SUBSTRING-for-toasted-multibyte-characters.patch (application/x-patch, 13.0K) download
From 9f03e4d23356e8012e8a190bddd81059dc88471b Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Sat, 14 Feb 2026 21:07:38 +1300
Subject: [PATCH v2tm 1/2] Fix SUBSTRING() for toasted multibyte characters.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 1e7fe06c10c0a8da9dd6261a6be8d405dc17c728 changed
pg_mbstrlen_with_len() to ereport(ERROR) if the input ends in an
incomplete character. Most callers want that. text_substring() did
not. It detoasts the most bytes it could possibly need to get the
requested number of characters. For example, to extract up to 2 chars
from UTF8, it needs to detoast 8 bytes. In a string of 3-byte UTF8
chars, 8 bytes spans 2 complete chars and 1 partial char.
Fix by removing the pg_mbstrlen_with_len() call. An encoding error is
still raised if input runs out in the middle of a multi-byte sequence
that is needed for the result, but that can be detected incrementally.
This is consistent with the general philosophy of the above commit,
which was to raise errors on a just-in-time basis. Before the above
commit, SUBSTRING() never raised an encoding error.
SUBSTRING() has long been detoasting enough for one more char than
needed, because it did not distinguish exclusive and inclusive end
position. For avoidance of doubt, stop detoasting extra.
A long standing overflow bug affecting positions exceeding 2^29 is also
addressed, though such large numbers would be unlikely to be used in
real applications.
Back-patch to v14, like the above commit. For applications using
SUBSTRING() on non-ASCII column values, consider applying this to your
copy of any of the February 12, 2026 releases.
Reported-by: SATŌ Kentarō <ranvis@gmail.com>
Reviewed-by: FIXME
Bug: #19406
Discussion: https://postgr.es/m/19406-9867fddddd724fca@postgresql.org
Backpatch-through: 14
---
src/backend/utils/adt/varlena.c | 103 +++++++------------------
src/test/regress/expected/encoding.out | 72 ++++++++++++-----
src/test/regress/sql/encoding.sql | 26 +++++--
3 files changed, 102 insertions(+), 99 deletions(-)
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index dbecd7160d6..5e3951ab0e1 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -586,7 +586,7 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
- int32 E; /* end position */
+ int32 E; /* end position, exclusive */
/*
* SQL99 says S can be zero or negative (which we don't document), but we
@@ -644,34 +644,27 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
* detoasting, so we'll grab a conservatively large slice now and go
* back later to do the right thing
*/
- int32 slice_start;
int32 slice_size;
- int32 slice_strlen;
- int32 slice_len;
+ const char *slice_end;
text *slice;
- int32 E1;
int32 i;
char *p;
char *s;
text *ret;
- /*
- * We need to start at position zero because there is no way to know
- * in advance which byte offset corresponds to the supplied start
- * position.
- */
- slice_start = 0;
-
- if (length_not_specified) /* special case - get length to end of
- * string */
- slice_size = L1 = -1;
+ if (length_not_specified) /* special case - get whole string */
+ {
+ slice_size = -1;
+ E = PG_INT32_MAX;
+ }
else if (length < 0)
{
/* SQL99 says to throw an error for E < S, i.e., negative length */
ereport(ERROR,
(errcode(ERRCODE_SUBSTRING_ERROR),
errmsg("negative substring length not allowed")));
- slice_size = L1 = -1; /* silence stupider compilers */
+ slice_size = -1; /* silence stupider compilers */
+ E = PG_INT32_MAX;
}
else if (pg_add_s32_overflow(S, length, &E))
{
@@ -679,30 +672,17 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
* L could be large enough for S + L to overflow, in which case
* the substring must run to end of string.
*/
- slice_size = L1 = -1;
+ slice_size = -1;
+ E = PG_INT32_MAX;
}
else
{
/*
- * A zero or negative value for the end position can happen if the
- * start was negative or one. SQL99 says to return a zero-length
- * string.
- */
- if (E < 1)
- return cstring_to_text("");
-
- /*
- * if E is past the end of the string, the tuple toaster will
- * truncate the length for us
+ * Total slice size in bytes can't be any longer than the end
+ * position (made zero-based) times the encoding max length. If
+ * that overflows, we can just use -1.
*/
- L1 = E - S1;
-
- /*
- * Total slice size in bytes can't be any longer than the start
- * position plus substring length times the encoding max length.
- * If that overflows, we can just use -1.
- */
- if (pg_mul_s32_overflow(E, eml, &slice_size))
+ if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
slice_size = -1;
}
@@ -712,63 +692,34 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
*/
if (VARATT_IS_COMPRESSED(DatumGetPointer(str)) ||
VARATT_IS_EXTERNAL(DatumGetPointer(str)))
- slice = DatumGetTextPSlice(str, slice_start, slice_size);
+ slice = DatumGetTextPSlice(str, 0, slice_size);
else
slice = (text *) DatumGetPointer(str);
- /* see if we got back an empty string */
- slice_len = VARSIZE_ANY_EXHDR(slice);
- if (slice_len == 0)
- {
- if (slice != (text *) DatumGetPointer(str))
- pfree(slice);
- return cstring_to_text("");
- }
-
- /* Now we can get the actual length of the slice in MB characters */
- slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
- slice_len);
-
- /*
- * Check that the start position wasn't > slice_strlen. If so, SQL99
- * says to return a zero-length string.
- */
- if (S1 > slice_strlen)
- {
- if (slice != (text *) DatumGetPointer(str))
- pfree(slice);
- return cstring_to_text("");
- }
-
- /*
- * Adjust L1 and E1 now that we know the slice string length. Again
- * remember that S1 is one based, and slice_start is zero based.
- */
- if (L1 > -1)
- E1 = Min(S1 + L1, slice_start + 1 + slice_strlen);
- else
- E1 = slice_start + 1 + slice_strlen;
+ p = VARDATA_ANY(slice);
+ slice_end = p + VARSIZE_ANY_EXHDR(slice);
/*
- * Find the start position in the slice; remember S1 is not zero based
+ * Find the start position in the slice; remember S1 is not zero
+ * based. If we run out of input first, we'll return an empty string.
*/
- p = VARDATA_ANY(slice);
- for (i = 0; i < S1 - 1; i++)
- p += pg_mblen_unbounded(p);
+ for (i = 1; i < S1 && p < slice_end; i++)
+ p += pg_mblen_range(p, slice_end);
/* hang onto a pointer to our start position */
s = p;
/*
* Count the actual bytes used by the substring of the requested
- * length.
+ * length, again giving up if we run out of input.
*/
- for (i = S1; i < E1; i++)
- p += pg_mblen_unbounded(p);
+ for (i = S1; i < E && p < slice_end; i++)
+ p += pg_mblen_range(p, slice_end);
ret = (text *) palloc(VARHDRSZ + (p - s));
SET_VARSIZE(ret, VARHDRSZ + (p - s));
- memcpy(VARDATA(ret), s, (p - s));
+ if (s < p)
+ memcpy(VARDATA(ret), s, (p - s));
if (slice != (text *) DatumGetPointer(str))
pfree(slice);
diff --git a/src/test/regress/expected/encoding.out b/src/test/regress/expected/encoding.out
index ea1f38cff41..eec879b05ec 100644
--- a/src/test/regress/expected/encoding.out
+++ b/src/test/regress/expected/encoding.out
@@ -63,7 +63,13 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+ substring
+-----------
+ caf
+(1 row)
+
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
SELECT reverse(truncated) FROM regress_encoding;
ERROR: invalid byte sequence for encoding "UTF8": 0xc3
@@ -84,6 +90,13 @@ SELECT length(with_nul) FROM regress_encoding;
4
(1 row)
+SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
+ regexp_replace
+----------------
+ é
+(1 row)
+
+-- NUL = character
SELECT substring(with_nul, 3, 1) FROM regress_encoding;
substring
-----------
@@ -96,25 +109,12 @@ SELECT substring(with_nul, 4, 1) FROM regress_encoding;
é
(1 row)
-SELECT substring(with_nul, 5, 1) FROM regress_encoding;
- substring
------------
-
-(1 row)
-
-SELECT convert_to(substring(with_nul, 5, 1), 'UTF8') FROM regress_encoding;
- convert_to
-------------
- \x
-(1 row)
-
-SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
- regexp_replace
-----------------
- é
+SELECT substring(with_nul, 5, 1) = test_bytea_to_text('\x00') FROM regress_encoding;
+ ?column?
+----------
+ t
(1 row)
--- NUL = character
SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;
with_nul | reverse | reverse
----------+---------+---------
@@ -375,7 +375,43 @@ NOTICE: MULE_INTERNAL LC2: \x908283 -> {9470595} -> \x908283 = OK
t
(1 row)
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+
+(1 row)
+
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+ substring
+-----------
+ …
+(1 row)
+
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+ERROR: invalid byte sequence for encoding "UTF8": 0xe2 0x80
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+ substring
+-----------
+ 🚀
+(1 row)
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
diff --git a/src/test/regress/sql/encoding.sql b/src/test/regress/sql/encoding.sql
index b9543c0cb32..7de95f49947 100644
--- a/src/test/regress/sql/encoding.sql
+++ b/src/test/regress/sql/encoding.sql
@@ -40,7 +40,8 @@ SELECT reverse(good) FROM regress_encoding;
-- invalid short mb character = error
SELECT length(truncated) FROM regress_encoding;
-SELECT substring(truncated, 1, 1) FROM regress_encoding;
+SELECT substring(truncated, 1, 3) FROM regress_encoding;
+SELECT substring(truncated, 1, 4) FROM regress_encoding;
SELECT reverse(truncated) FROM regress_encoding;
-- invalid short mb character = silently dropped
SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
@@ -51,12 +52,11 @@ SELECT regexp_replace(truncated, '^caf(.)$', '\1') FROM regress_encoding;
-- NUL = terminator
SELECT length(with_nul) FROM regress_encoding;
-SELECT substring(with_nul, 3, 1) FROM regress_encoding;
-SELECT substring(with_nul, 4, 1) FROM regress_encoding;
-SELECT substring(with_nul, 5, 1) FROM regress_encoding;
-SELECT convert_to(substring(with_nul, 5, 1), 'UTF8') FROM regress_encoding;
SELECT regexp_replace(with_nul, '^caf(.)$', '\1') FROM regress_encoding;
-- NUL = character
+SELECT substring(with_nul, 3, 1) FROM regress_encoding;
+SELECT substring(with_nul, 4, 1) FROM regress_encoding;
+SELECT substring(with_nul, 5, 1) = test_bytea_to_text('\x00') FROM regress_encoding;
SELECT with_nul, reverse(with_nul), reverse(reverse(with_nul)) FROM regress_encoding;
-- If a corrupted string contains NUL in the tail bytes of a multibyte
@@ -212,7 +212,23 @@ INSERT INTO encoding_tests VALUES
SELECT COUNT(test_encoding(encoding, description, input)) > 0
FROM encoding_tests;
+-- substring fetches a slice of a toasted value; unused tail of that slice is
+-- an incomplete char (bug #19406)
+CREATE TABLE toast_3b_utf8 (c text);
+INSERT INTO toast_3b_utf8 VALUES (repeat(U&'\2026', 4000));
+SELECT SUBSTRING(c FROM 1 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- diagnose incomplete char iff within the substring
+UPDATE toast_3b_utf8 SET c = c || test_bytea_to_text('\xe280');
+SELECT SUBSTRING(c FROM 4000 FOR 1) FROM toast_3b_utf8;
+SELECT SUBSTRING(c FROM 4001 FOR 1) FROM toast_3b_utf8;
+-- substring needing last byte of its slice_size
+ALTER TABLE toast_3b_utf8 RENAME TO toast_4b_utf8;
+UPDATE toast_4b_utf8 SET c = repeat(U&'\+01F680', 3000);
+SELECT SUBSTRING(c FROM 3000 FOR 1) FROM toast_4b_utf8;
+
DROP TABLE encoding_tests;
+DROP TABLE toast_4b_utf8;
DROP FUNCTION test_encoding;
DROP FUNCTION test_text_to_wchars;
DROP FUNCTION test_mblen_func;
--
2.47.3
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
@ 2026-02-14 19:33 ` Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Noah Misch @ 2026-02-14 19:33 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sun, Feb 15, 2026 at 08:07:22AM +1300, Thomas Munro wrote:
> On Sat, Feb 14, 2026 at 9:15 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > On Sat, Feb 14, 2026 at 6:38 PM Noah Misch <noah@leadboat.com> wrote:
> > > [mblen-valgrind-after-report-v1.patch]
> >
> > LGTM. The new valgrind check should clearly be after the new non-local exit.
> >
> > Studying the other patch...
Thank you.
> /*
> - * Total slice size in bytes can't be any
> longer than the start
> - * position plus substring length times the
> encoding max length.
> - * If that overflows, we can just use -1.
> + * Total slice size in bytes can't be any
> longer than the
> + * inclusive end position times the encoding
> max length. If that
> + * overflows, we can just use -1.
> */
> - if (pg_mul_s32_overflow(E, eml, &slice_size))
> + if (pg_mul_s32_overflow(E - 1, eml, &slice_size))
> slice_size = -1;
>
> Isn't it still conceptually "exclusive", but adjusted to be zero-indexed?
Since it's discrete (being an integer), "up to E, exclusive" and "up to E - 1,
inclusive" are the same thing. My comment may not be the optimal way to
express that.
> /* Now we can get the actual length of the slice in MB
> characters */
> - slice_strlen = pg_mbstrlen_with_len(VARDATA_ANY(slice),
> -
> slice_len);
> + slice_strlen =
> + (slice_size == -1 ?
> + pg_mbstrlen_with_len(VARDATA_ANY(slice), slice_len) :
> + pg_mbcharcliplen_chars(VARDATA_ANY(slice),
> slice_len, E - 1));
>
> Comment presumably needs adjustment to say that we only count as far
> as we need to, and why.
Changed to:
/*
* Now we can get the actual length of the slice in MB characters,
* stopping at the end of the substring. Continuing beyond the
* substring end could find an incomplete character attributable
* solely to DatumGetTextPSlice() chopping in the middle of a
* character, and it would be superfluous work at best.
*/
> There is something a bit strange about all this, though.
> pg_mbstrlen_with_len(..., -1) returns 0, so if you ask for characters
> that really exist past 2^29 (~500 million), you must get an empty
> string, right? That's hard to reach, pre-existing and out of scope
> for the immediate problem report, except ... now we're contorting the
> code even further to keep it.
- slice_size is the amount we *requested* from the toaster. It can be -1,
which retrieves the max available.
- slice_len is the amount *returned* from the toaster. It's nonnegative.
Does a behavior specific to strings >2^29 still exist?
> The outline I had come up with before seeing your patch was: let's
> just delete it. The position search can check bounds incrementally,
> following our general approach. This avoids the reported problem by
> ditching the pre-flight scan through the slice (up to 4x more
> pg_mblen_XXX calls and memory access than we strictly need), and also
> the special cases for empty strings since they already fall out of the
> general behaviour (am I missing something?), not leaving much code
> behind.
Like you, I made a note that it's wasteful to make two mblen passes over the
string. I'm only seeing a 50% reduction in mblen calls, not an 80% reduction,
but I didn't look too closely. I guessed such a change would be less clearly
correct, so I figured it would be less suitable for back branches. Hence, I
didn't draft it.
> As far as I can see so far, the only user-visible side-effect
> requires corruption: substring() moves from the
> internal-NUL-is-terminator category to internal-NUL-is-character
> category, but that's an implementation detail.
That does carry some risk, not necessarily too much to accept.
> When I saw your patch yesterday, I initially abandoned the thought,
> thinking that your idea looked more conservative, but after sleeping
> on it and reflecting again on these oddities, I have merged my draft
> implementation with your tests, ancient detoasting fence post
> observation and commit message, just to see if you think this approach
> might be worth considering further.
My first impression, hurried due to the commit ETA in 30 minutes, is that this
is less conservative and should hold for master-only.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-14 20:10 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 20:19 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Munro @ 2026-02-14 20:10 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sun, Feb 15, 2026 at 8:33 AM Noah Misch <noah@leadboat.com> wrote:
> - slice_len is the amount *returned* from the toaster. It's nonnegative.
Ah, right, that makes more sense.
> > The outline I had come up with before seeing your patch was: let's
> > just delete it. The position search can check bounds incrementally,
> > following our general approach. This avoids the reported problem by
> > ditching the pre-flight scan through the slice (up to 4x more
> > pg_mblen_XXX calls and memory access than we strictly need), and also
> > the special cases for empty strings since they already fall out of the
> > general behaviour (am I missing something?), not leaving much code
> > behind.
>
> Like you, I made a note that it's wasteful to make two mblen passes over the
> string. I'm only seeing a 50% reduction in mblen calls, not an 80% reduction,
> but I didn't look too closely. I guessed such a change would be less clearly
> correct, so I figured it would be less suitable for back branches. Hence, I
> didn't draft it.
I was comparing to unpatched master, but yeah of course your patch
already gets part of the way there.
> My first impression, hurried due to the commit ETA in 30 minutes, is that this
> is less conservative and should hold for master-only.
Got it. Will add it to the pile of master-only fallout from this area.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
@ 2026-02-14 20:19 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 22:46 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Munro @ 2026-02-14 20:19 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
Also, LGTM.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 20:19 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
@ 2026-02-14 22:46 ` Noah Misch <noah@leadboat.com>
2026-02-16 18:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: Noah Misch @ 2026-02-14 22:46 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Sun, Feb 15, 2026 at 09:19:04AM +1300, Thomas Munro wrote:
> Also, LGTM.
Pushed as commit 9f4fd11
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 20:19 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 22:46 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-16 18:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-02-17 04:20 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2026-02-16 18:21 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Thomas Munro <thomas.munro@gmail.com>; ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
Noah Misch <noah@leadboat.com> writes:
> Pushed as commit 9f4fd11
Various BF animals are complaining
varlena.c: In function 'text_substring':
varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
int32 E; /* end position, exclusive */
^
and I've also seen that locally depending on which gcc version and -O
level I'm using. Could we silence that?
bollworm | 2026-02-16 09:32:03 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
camel | 2026-02-16 09:32:30 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
chimaera | 2026-02-16 11:40:29 | /home/debian/20-chimaera/buildroot/HEAD/pgsql.build/../pgsql/src/backend/utils/adt/varlena.c:739:5: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
comma | 2026-02-16 14:55:48 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
conchuela | 2026-02-16 15:20:01 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
flea | 2026-02-16 09:31:52 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
krait | 2026-02-16 09:50:17 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
pipit | 2026-02-15 02:36:02 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
urocryon | 2026-02-15 01:45:07 | varlena.c:739:5: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
wireworm | 2026-02-16 15:20:49 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
ziege | 2026-02-15 01:18:25 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 20:19 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Thomas Munro <thomas.munro@gmail.com>
2026-02-14 22:46 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-16 18:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-02-17 04:20 ` Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 15+ messages in thread
From: Noah Misch @ 2026-02-17 04:20 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; ranvis@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Feb 16, 2026 at 01:21:45PM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Pushed as commit 9f4fd11
>
> Various BF animals are complaining
>
> varlena.c: In function 'text_substring':
> varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> int32 E; /* end position, exclusive */
> ^
>
> and I've also seen that locally depending on which gcc version and -O
> level I'm using. Could we silence that?
>
>
> bollworm | 2026-02-16 09:32:03 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> camel | 2026-02-16 09:32:30 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> chimaera | 2026-02-16 11:40:29 | /home/debian/20-chimaera/buildroot/HEAD/pgsql.build/../pgsql/src/backend/utils/adt/varlena.c:739:5: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> comma | 2026-02-16 14:55:48 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> conchuela | 2026-02-16 15:20:01 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> flea | 2026-02-16 09:31:52 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> krait | 2026-02-16 09:50:17 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> pipit | 2026-02-15 02:36:02 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> urocryon | 2026-02-15 01:45:07 | varlena.c:739:5: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> wireworm | 2026-02-16 15:20:49 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
> ziege | 2026-02-15 01:18:25 | varlena.c:590:9: warning: 'E' may be used uninitialized in this function [-Wmaybe-uninitialized]
Thanks. I've pushed commit 8cef93d.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-13 22:48 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Re: BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 Noah Misch <noah@leadboat.com>
@ 2026-02-17 18:15 ` Noah Misch <noah@leadboat.com>
1 sibling, 0 replies; 15+ messages in thread
From: Noah Misch @ 2026-02-17 18:15 UTC (permalink / raw)
To: ranvis@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: thomas.munro@gmail.com
On Fri, Feb 13, 2026 at 04:21:13PM -0800, Noah Misch wrote:
> On Fri, Feb 13, 2026 at 02:48:04PM -0800, Noah Misch wrote:
> > Attached. I may add some more tests, e.g. a toasted invalid string where the
> > detoasted length is less than the slice we request.
>
> Tests already covered that in particular, but I added some other tests.
I cast a wider net for string function bugs specific to toasted strings and/or
multibyte text. That yielded the attached. This did reproduce $SUBJECT, but
it didn't find additional bugs. The method is crude, but I'm archiving it
here so folks know the testing happened and in case someone pursues similar
testing in the future.
From: Noah Misch <noah@leadboat.com>
Roughly extend string function tests to multibyte+toasted inputs.
Not for commit. This operates by two mechanisms:
- ascii2utf8sql.pl: mechanically translate selected src/test/regress/sql
files to replace string literals with multibyte string literals. This
is a cheap way to extend existing string function coverage to
multibyte inputs.
- fmgr.c, pg_lzcompress.c: automatically toast-compress function
arguments of type "text". Don't avoid compression just because the
input is small.
Tests are not intended to pass. Setup a server for installcheck, then
test with:
sh run.sh
make installcheck-parallel; grep +ERROR src/test/regress/regression.diffs
Many additional errors appear, due to the crudity of this change. The
key is the presence or absence of additional "invalid byte sequence for
encoding" errors. The master branch gets 3 of those errors before
commit 9f4fd11, and it gets 0 after that regression fix. The query
"SELECT SUBSTRING(repeat(U&'\2026', 4000) FROM 1 FOR 1);" is another
example that reaches the error after these changes (and with a revert of
commit 9f4fd11). It doesn't normally reach toast compression, but these
changes make it do so.
Gemini 3 wrote much this.
Discussion: https://postgr.es/m/FIXME
diff --git a/ascii2utf8sql.pl b/ascii2utf8sql.pl
new file mode 100644
index 0000000..03c4e3f
--- /dev/null
+++ b/ascii2utf8sql.pl
@@ -0,0 +1,310 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use utf8;
+
+# Ensure standard filehandles handle UTF-8 properly
+binmode STDIN, ":encoding(UTF-8)";
+binmode STDOUT, ":encoding(UTF-8)";
+
+my %map;
+my %regex_metachars = map { $_ => 1 } split //, '<.*+?|()[]{}^$\\-:=';
+
+# Exact strings that should bypass the transformation
+my %denylist = (
+ 'UTF8' => 1,
+ '\x' => 1);
+
+# Helper to map uppercase/lowercase pairs identically across byte boundaries
+sub add_pair
+{
+ my ($u_ascii, $l_ascii, $u_uni, $l_uni) = @_;
+ $map{$u_ascii} = chr($u_uni);
+ $map{$l_ascii} = chr($l_uni);
+}
+
+# 1. Letters mapping (Preserving Postgres casing rules)
+# A-C: 1-byte
+add_pair('A', 'a', 0x0041, 0x0061);
+add_pair('B', 'b', 0x0042, 0x0062);
+add_pair('C', 'c', 0x0043, 0x0063);
+
+# D-K: 2-byte (Latin Extended-A)
+add_pair('D', 'd', 0x010A, 0x010B); # Ċ/ċ
+add_pair('E', 'e', 0x0112, 0x0113); # Ē/ē
+add_pair('F', 'f', 0x011E, 0x011F); # Ğ/ğ
+add_pair('G', 'g', 0x0122, 0x0123); # Ģ/ģ
+add_pair('H', 'h', 0x0128, 0x0129); # Ĩ/ĩ
+add_pair('I', 'i', 0x012A, 0x012B); # Ī/ī
+add_pair('J', 'j', 0x0132, 0x0133); # IJ/ij
+add_pair('K', 'k', 0x0136, 0x0137); # Ķ/ķ
+
+# L-S: 3-byte (Fullwidth Latin)
+for my $i (11 .. 18)
+{
+ add_pair(chr(0x41 + $i), chr(0x61 + $i), 0xFF21 + $i, 0xFF41 + $i);
+}
+
+# T-Z: 4-byte (Deseret)
+for my $i (19 .. 25)
+{
+ add_pair(chr(0x41 + $i), chr(0x61 + $i), 0x10400 + $i, 0x10428 + $i);
+}
+
+# 2. Digits mapping
+$map{'0'} = '0'; # 1-byte
+$map{'1'} = chr(0x0661); # 2-byte (Arabic-Indic)
+$map{'2'} = chr(0x0662);
+$map{'3'} = chr(0x0663);
+$map{'4'} = chr(0xFF14); # 3-byte (Fullwidth)
+$map{'5'} = chr(0xFF15);
+$map{'6'} = chr(0xFF16);
+$map{'7'} = chr(0x1D7E9); # 4-byte (Math Sans-Serif)
+$map{'8'} = chr(0x1D7EA);
+$map{'9'} = chr(0x1D7EB);
+
+# 3. Other ASCII characters
+$map{'\\'} = '\\'; # Lock backslash to 1-byte to protect E'\n' escapes
+my @symbols;
+for my $i (32 .. 126)
+{
+ my $c = chr($i);
+ push @symbols, $c if $c !~ /[A-Za-z0-9']/ && !exists $map{$c};
+}
+
+my $base2_sym = 0x00A1; # ¡ (2-byte)
+my $base3_sym = 0x2010; # ‐ (3-byte)
+my $base4_sym = 0x1D300; # 𝌀 (4-byte)
+
+for my $i (0 .. $#symbols)
+{
+ my $c = $symbols[$i];
+ if ($i < 3)
+ {
+ $map{$c} = $c;
+ }
+ elsif ($i < 13)
+ {
+ $map{$c} = chr($base2_sym++);
+ }
+ elsif ($i < 23)
+ {
+ $map{$c} = chr($base3_sym++);
+ }
+ else
+ {
+ $map{$c} = chr($base4_sym++);
+ }
+}
+
+sub transform_string
+{
+ my ($text, $mode) = @_;
+
+ return $text if $mode eq 'control';
+ return $text if $denylist{$text};
+
+ # For format strings, protect PostgreSQL %-escapes
+ # (e.g. %s, %I, %L, %% and parametrized ones like %1$s, %-10s)
+ if ($mode eq 'format')
+ {
+ my $res = "";
+ # Scans the string chunk by chunk, matching anything up to a format specifier or end of string.
+ # The specifier regex captures: %, optional -, numbers, $, *, and the final type letter (s, I, L, or %).
+ while ($text =~ /\G(.*?)(%[-0-9\$*]*[sIL%]|$)/gcs)
+ {
+ my $pre = $1;
+ my $spec = $2;
+
+ # Recursively apply 'normal' transformation to the text between format specifiers
+ $res .= transform_string($pre, 'normal') if length $pre;
+ $res .= $spec if length $spec;
+ }
+ return $res;
+ }
+
+ my $res = "";
+ my $is_regex = ($mode eq 'regex');
+ my @chars = split //, $text;
+
+ for (my $i = 0; $i < @chars; $i++)
+ {
+ my $c = $chars[$i];
+
+ # Pass through doubled single quotes untouched
+ if ($c eq "'" && $i + 1 < @chars && $chars[ $i + 1 ] eq "'")
+ {
+ $res .= "''";
+ $i++;
+ next;
+ }
+
+ if ($is_regex)
+ {
+ # Skip transformation for escaped regex sequences (e.g., \d, \s)
+ if ($c eq '\\' && $i + 1 < @chars)
+ {
+ $res .= $c . $chars[ $i + 1 ];
+ $i++;
+ next;
+ }
+ if ($regex_metachars{$c})
+ {
+ $res .= $c;
+ next;
+ }
+ }
+
+ $res .= $map{$c} // $c;
+ }
+ return $res;
+}
+
+# Slurp the entire input stream
+local $/ = undef;
+my $sql = <STDIN>;
+my $out = "";
+
+# Stack tracks nested function calls: { name => 'regexp_replace', arg => 0 }
+my @stack;
+my $last_id = "";
+
+pos($sql) = 0;
+
+# The regex evaluates alternatives in order.
+# It matches comments and dollar-quotes first, returning them unmodified.
+# It only captures and transforms single-quoted strings.
+while (
+ $sql =~ m/\G(
+ (--.*?\n) # $2: single-line comment
+ | (\/\*.*?\*\/) # $3: multi-line comment
+ | (\$([a-zA-Z0-9_]*)\$.*?\$\5\$) # $4: dollar quote, $5: tag
+ | (([EeuU]&?)?('(?:[^']|'')*')) # $6: full string, $7: prefix, $8: content
+ | ([a-zA-Z_][a-zA-Z0-9_]*) # $9: identifier
+ | ([(),]) # $10: punctuation
+ | (\s+) # $11: whitespace
+ | (.) # $12: other
+)/gsx)
+{
+ if (defined $2)
+ {
+ $out .= $2;
+ $last_id = "";
+ }
+ elsif (defined $3)
+ {
+ $out .= $3;
+ $last_id = "";
+ }
+ elsif (defined $4)
+ {
+ $out .= $4;
+ $last_id = "";
+ }
+ elsif (defined $6)
+ {
+ my $prefix = $7 // "";
+ my $literal = $8;
+ $literal =~ s/^'//;
+ $literal =~ s/'$//;
+
+ my $mode = 'normal';
+
+ # 1. Apply function argument rules based on the stack
+ if (@stack)
+ {
+ my $func = lc($stack[-1]{name});
+ my $arg_idx = $stack[-1]{arg};
+
+ # Apply arg index rules for regexp functions
+ if ($func =~ /^regexp_/)
+ {
+ if ($arg_idx == 0)
+ {
+ $mode = 'normal';
+ }
+ elsif ($arg_idx == 1)
+ {
+ $mode = 'regex';
+ }
+ elsif ($arg_idx == 2 && $func eq 'regexp_replace')
+ {
+ $mode = 'normal';
+ }
+ else
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'encode')
+ {
+ if ($arg_idx == 0 || $arg_idx == 1)
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'decode')
+ {
+ if ($arg_idx == 1)
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'format')
+ {
+ if ($arg_idx == 0)
+ {
+ $mode = 'format';
+ }
+ }
+ }
+
+ # 2. Look ahead in the unparsed text to see if it's cast to bytea or regclass
+ my $remainder = substr($sql, pos($sql));
+ if ($remainder =~ /^\s*::\s*(?:bytea|regclass)\b/i)
+ {
+ $mode = 'control';
+ }
+
+ $out .= $prefix . "'" . transform_string($literal, $mode) . "'";
+ $last_id = "";
+ }
+ elsif (defined $9)
+ {
+ $out .= $9;
+ # Exclude common SQL syntax keywords from being treated as function names
+ if ($9 !~ /^(from|for|as|and|or|not|in)$/i)
+ {
+ $last_id = $9;
+ }
+ }
+ elsif (defined $10)
+ {
+ my $p = $10;
+ $out .= $p;
+ if ($p eq '(')
+ {
+ push @stack, { name => $last_id, arg => 0 };
+ }
+ elsif ($p eq ',')
+ {
+ $stack[-1]{arg}++ if @stack;
+ }
+ elsif ($p eq ')')
+ {
+ pop @stack if @stack;
+ }
+ $last_id = "";
+ }
+ elsif (defined $11)
+ {
+ $out .= $11;
+ }
+ elsif (defined $12)
+ {
+ $out .= $12;
+ $last_id = "";
+ }
+}
+
+print $out;
diff --git a/run.sh b/run.sh
new file mode 100644
index 0000000..b000c34
--- /dev/null
+++ b/run.sh
@@ -0,0 +1,24 @@
+# To let coverage gaps inform which files to translate, I used bpftrace to get
+# the call stacks that reach a pg_mblen* call w/ retval=1 but reach no such
+# call with retval!=1.
+
+cd src/test/regress
+# fairly clean
+tests='text strings regex'
+# much noisier
+tests="$tests arrays collate.icu.utf8 tsdicts tsearch tsrf tstypes"
+# expects v15+; v14 lacks test_setup
+runnable=test_setup
+for t in $tests; do
+ perl ../../../ascii2utf8sql.pl <sql/$t.sql >sql/$t.utf8.sql
+ # TODO script not ready for expected outputs: deletes more than it should
+ perl ../../..//ascii2utf8sql.pl <expected/$t.out >expected/$t.utf8.out
+ runnable="$runnable $t.utf8"
+done
+cd ../../..
+make installcheck-tests TESTS="$runnable tablespace" "$@"
+echo '==== all added errors'
+echo '==== (includes false positives from flawed munging of expected output)'
+grep '^[+]ERROR' src/test/regress/regression.diffs | sort | uniq -c
+echo '==== all examples of goal error: invalid byte sequence for encoding'
+grep 'invalid byte sequence for encoding' src/test/regress/regression.diffs
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 4e26df7..9b51df5 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -34,11 +34,188 @@
#include "utils/lsyscache.h"
#include "utils/syscache.h"
+#include "access/xact.h"
+#include "access/toast_compression.h"
+#include "access/toast_internals.h"
+#include "utils/fmgroids.h"
+
+/* https://gemini.google.com/u/1/app/dc868a99fe48c7e8 */
+/* stack of nested function pointers */
+#define MAX_NESTED_CALLS 1024
+static PGFunction fn_addr_stack[MAX_NESTED_CALLS];
+static int fn_addr_stack_ptr = 0;
+
+/*
+ *
+ * The wrapper function that receives the fully populated FunctionCallInfo,
+ * compresses varlenas in place, and calls the original function.
+ */
+static Datum
+my_function_wrapper(FunctionCallInfo fcinfo)
+{
+ PGFunction orig_fn;
+ Datum result;
+ bool modified = false;
+
+ /* Allocate backup array on the stack (max args in Postgres is 100) */
+ Datum saved_values[100];
+
+ if (fn_addr_stack_ptr > 0)
+ orig_fn = fn_addr_stack[fn_addr_stack_ptr - 1];
+ else
+ elog(ERROR, "test_toast_hook: fn_addr_stack empty");
+
+ /* Backup and compress */
+ for (int i = 0; i < fcinfo->nargs; i++)
+ {
+ saved_values[i] = fcinfo->args[i].value;
+
+ if (!fcinfo->args[i].isnull && get_fn_expr_argtype(fcinfo->flinfo, i) == TEXTOID)
+ {
+ Datum val = fcinfo->args[i].value;
+
+ if (!VARATT_IS_EXTENDED(DatumGetPointer(val)))
+ {
+ Datum cval = toast_compress_datum(val, TOAST_PGLZ_COMPRESSION);
+
+ if (cval)
+ {
+ fcinfo->args[i].value = cval;
+ modified = true;
+ }
+ }
+ }
+ }
+
+ /* Execute */
+ result = orig_fn(fcinfo);
+
+ /* Restore the original pointers to protect the ExprState cache */
+ if (modified)
+ {
+ for (int i = 0; i < fcinfo->nargs; i++)
+ {
+ fcinfo->args[i].value = saved_values[i];
+ }
+ }
+
+ return result;
+}
+
+/*
+ * Force Postgres to route all function calls through fmgr_security_definer.
+ */
+#define MAX_CACHED_OIDS 65536
+/*
+ * 0 = unknown, 1 = hook needed, 2 = hook not needed
+ *
+ * Exempt functions used in a search of the PROCOID system, to avoid infinite
+ * recursion. We know none of these have text args. This list is probably
+ * incomplete, since associated relcache rebuilds are possible but rare.
+ * FIXME test w/ debug_discard_caches.
+ */
+static uint8 oid_hook_cache[MAX_CACHED_OIDS] = {
+ [F_BTHANDLER] = 2,
+ [F_BTOIDCMP] = 2,
+ [F_BTTEXTCMP] = 2,
+ [F_HEAP_TABLEAM_HANDLER] = 2,
+ [F_INT2GT] = 2,
+ [F_OIDEQ] = 2,
+};
+
+static bool
+my_needs_fmgr_hook(Oid fn_oid)
+{
+ bool needs_hook = false;
+ HeapTuple tup;
+
+ /* Fast path: check local cache */
+ if (fn_oid < MAX_CACHED_OIDS)
+ {
+ if (oid_hook_cache[fn_oid] == 1)
+ return true;
+ if (oid_hook_cache[fn_oid] == 2)
+ return false;
+ }
+
+ /*
+ * * If we cannot access the catalog safely, assume false to prevent
+ * crashes. The cache will remain 'unknown' and will be populated during
+ * the next in-transaction execution.
+ */
+ if (!IsTransactionState())
+ return false;
+
+ /*
+ * So far, infinite recursion has always been associated with a btree
+ * opclass member relevant to the PROCOID syscache or its recursive
+ * dependencies. The solution has been to add the member to the
+ * oid_hook_cache initializer.
+ */
+ if (stack_is_too_deep())
+ elog(ERROR, "stack depth for fn_oid=%u", fn_oid);
+
+ /* Safe to query the catalog to inspect the function's arguments */
+ tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
+
+ if (HeapTupleIsValid(tup))
+ {
+ Form_pg_proc procForm = (Form_pg_proc) GETSTRUCT(tup);
+
+ /* Check if any argument requires TEXTOID */
+ for (int i = 0; i < procForm->pronargs; i++)
+ {
+ if (procForm->proargtypes.values[i] == TEXTOID)
+ {
+ needs_hook = true;
+ break;
+ }
+ }
+ ReleaseSysCache(tup);
+ }
+
+ /* Populate the cache */
+ if (fn_oid < MAX_CACHED_OIDS)
+ {
+ oid_hook_cache[fn_oid] = needs_hook ? 1 : 2;
+ }
+
+ return needs_hook;
+}
+
+/*
+ * Hook the execution cycle to swap the function pointer.
+ */
+static void
+my_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum *private_data)
+{
+ if (event == FHET_START)
+ {
+ /* Stash the real function pointer */
+ if (fn_addr_stack_ptr < MAX_NESTED_CALLS)
+ fn_addr_stack[fn_addr_stack_ptr++] = flinfo->fn_addr;
+ else
+ elog(ERROR, "test_toast_hook: MAX_NESTED_CALLS exceeded");
+
+ /* Replace the execution pointer with our argument-mutating wrapper */
+ flinfo->fn_addr = my_function_wrapper;
+ }
+ else if (event == FHET_END || event == FHET_ABORT)
+ {
+ /* Pop the stack AND restore the original function pointer */
+ if (fn_addr_stack_ptr > 0)
+ {
+ fn_addr_stack_ptr--;
+ flinfo->fn_addr = fn_addr_stack[fn_addr_stack_ptr];
+ }
+ }
+}
+
/*
* Hooks for function calls
*/
-PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook = NULL;
-PGDLLIMPORT fmgr_hook_type fmgr_hook = NULL;
+PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook = my_needs_fmgr_hook;
+PGDLLIMPORT fmgr_hook_type fmgr_hook = my_fmgr_hook;
/*
* Hashtable for fast lookup of external C functions
@@ -164,7 +341,9 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
finfo->fn_mcxt = mcxt;
finfo->fn_expr = NULL; /* caller may set this later */
- if ((fbp = fmgr_isbuiltin(functionId)) != NULL)
+ if ((fbp = fmgr_isbuiltin(functionId)) != NULL &&
+ (!IsNormalProcessingMode() ||
+ !FmgrHookIsNeeded(functionId)))
{
/*
* Fast path for builtin functions: don't bother consulting pg_proc
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 75d529d..aec17c6 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -233,7 +233,6 @@ static const PGLZ_Strategy strategy_default_data = {
10 /* Lower good match size by 10% at every loop
* iteration */
};
-const PGLZ_Strategy *const PGLZ_strategy_default = &strategy_default_data;
static const PGLZ_Strategy strategy_always_data = {
@@ -246,6 +245,7 @@ static const PGLZ_Strategy strategy_always_data = {
6 /* Look harder for a good match */
};
const PGLZ_Strategy *const PGLZ_strategy_always = &strategy_always_data;
+const PGLZ_Strategy *const PGLZ_strategy_default = &strategy_always_data;
/* ----------
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index f38688b..6e0b628 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -1455,13 +1455,13 @@ ERROR: regexp_split_to_array() does not support the "global" option
-- change NULL-display back
\pset null ''
-- E021-11 position expression
-SELECT POSITION('4' IN '1234567890') = '4' AS "4";
+SELECT POSITION('4' IN '1234567890') = $int4$4$int4$ AS "4";
4
---
t
(1 row)
-SELECT POSITION('5' IN '1234567890') = '5' AS "5";
+SELECT POSITION('5' IN '1234567890') = $int4$5$int4$ AS "5";
5
---
t
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index d8a0973..b26ae4d 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -395,9 +395,9 @@ SELECT regexp_split_to_array('thE QUick bROWn FOx jUMPs ovEr The lazy dOG', 'e',
\pset null ''
-- E021-11 position expression
-SELECT POSITION('4' IN '1234567890') = '4' AS "4";
+SELECT POSITION('4' IN '1234567890') = $int4$4$int4$ AS "4";
-SELECT POSITION('5' IN '1234567890') = '5' AS "5";
+SELECT POSITION('5' IN '1234567890') = $int4$5$int4$ AS "5";
SELECT POSITION('\x11'::bytea IN ''::bytea) = 0 AS "0";
SELECT POSITION('\x33'::bytea IN '\x1122'::bytea) = 0 AS "0";
Attachments:
ascii2utf8sql-v0.1.patch (text/plain, 16.8K) download
From: Noah Misch <noah@leadboat.com>
Roughly extend string function tests to multibyte+toasted inputs.
Not for commit. This operates by two mechanisms:
- ascii2utf8sql.pl: mechanically translate selected src/test/regress/sql
files to replace string literals with multibyte string literals. This
is a cheap way to extend existing string function coverage to
multibyte inputs.
- fmgr.c, pg_lzcompress.c: automatically toast-compress function
arguments of type "text". Don't avoid compression just because the
input is small.
Tests are not intended to pass. Setup a server for installcheck, then
test with:
sh run.sh
make installcheck-parallel; grep +ERROR src/test/regress/regression.diffs
Many additional errors appear, due to the crudity of this change. The
key is the presence or absence of additional "invalid byte sequence for
encoding" errors. The master branch gets 3 of those errors before
commit 9f4fd11, and it gets 0 after that regression fix. The query
"SELECT SUBSTRING(repeat(U&'\2026', 4000) FROM 1 FOR 1);" is another
example that reaches the error after these changes (and with a revert of
commit 9f4fd11). It doesn't normally reach toast compression, but these
changes make it do so.
Gemini 3 wrote much this.
Discussion: https://postgr.es/m/FIXME
diff --git a/ascii2utf8sql.pl b/ascii2utf8sql.pl
new file mode 100644
index 0000000..03c4e3f
--- /dev/null
+++ b/ascii2utf8sql.pl
@@ -0,0 +1,310 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use utf8;
+
+# Ensure standard filehandles handle UTF-8 properly
+binmode STDIN, ":encoding(UTF-8)";
+binmode STDOUT, ":encoding(UTF-8)";
+
+my %map;
+my %regex_metachars = map { $_ => 1 } split //, '<.*+?|()[]{}^$\\-:=';
+
+# Exact strings that should bypass the transformation
+my %denylist = (
+ 'UTF8' => 1,
+ '\x' => 1);
+
+# Helper to map uppercase/lowercase pairs identically across byte boundaries
+sub add_pair
+{
+ my ($u_ascii, $l_ascii, $u_uni, $l_uni) = @_;
+ $map{$u_ascii} = chr($u_uni);
+ $map{$l_ascii} = chr($l_uni);
+}
+
+# 1. Letters mapping (Preserving Postgres casing rules)
+# A-C: 1-byte
+add_pair('A', 'a', 0x0041, 0x0061);
+add_pair('B', 'b', 0x0042, 0x0062);
+add_pair('C', 'c', 0x0043, 0x0063);
+
+# D-K: 2-byte (Latin Extended-A)
+add_pair('D', 'd', 0x010A, 0x010B); # Ċ/ċ
+add_pair('E', 'e', 0x0112, 0x0113); # Ē/ē
+add_pair('F', 'f', 0x011E, 0x011F); # Ğ/ğ
+add_pair('G', 'g', 0x0122, 0x0123); # Ģ/ģ
+add_pair('H', 'h', 0x0128, 0x0129); # Ĩ/ĩ
+add_pair('I', 'i', 0x012A, 0x012B); # Ī/ī
+add_pair('J', 'j', 0x0132, 0x0133); # IJ/ij
+add_pair('K', 'k', 0x0136, 0x0137); # Ķ/ķ
+
+# L-S: 3-byte (Fullwidth Latin)
+for my $i (11 .. 18)
+{
+ add_pair(chr(0x41 + $i), chr(0x61 + $i), 0xFF21 + $i, 0xFF41 + $i);
+}
+
+# T-Z: 4-byte (Deseret)
+for my $i (19 .. 25)
+{
+ add_pair(chr(0x41 + $i), chr(0x61 + $i), 0x10400 + $i, 0x10428 + $i);
+}
+
+# 2. Digits mapping
+$map{'0'} = '0'; # 1-byte
+$map{'1'} = chr(0x0661); # 2-byte (Arabic-Indic)
+$map{'2'} = chr(0x0662);
+$map{'3'} = chr(0x0663);
+$map{'4'} = chr(0xFF14); # 3-byte (Fullwidth)
+$map{'5'} = chr(0xFF15);
+$map{'6'} = chr(0xFF16);
+$map{'7'} = chr(0x1D7E9); # 4-byte (Math Sans-Serif)
+$map{'8'} = chr(0x1D7EA);
+$map{'9'} = chr(0x1D7EB);
+
+# 3. Other ASCII characters
+$map{'\\'} = '\\'; # Lock backslash to 1-byte to protect E'\n' escapes
+my @symbols;
+for my $i (32 .. 126)
+{
+ my $c = chr($i);
+ push @symbols, $c if $c !~ /[A-Za-z0-9']/ && !exists $map{$c};
+}
+
+my $base2_sym = 0x00A1; # ¡ (2-byte)
+my $base3_sym = 0x2010; # ‐ (3-byte)
+my $base4_sym = 0x1D300; # 𝌀 (4-byte)
+
+for my $i (0 .. $#symbols)
+{
+ my $c = $symbols[$i];
+ if ($i < 3)
+ {
+ $map{$c} = $c;
+ }
+ elsif ($i < 13)
+ {
+ $map{$c} = chr($base2_sym++);
+ }
+ elsif ($i < 23)
+ {
+ $map{$c} = chr($base3_sym++);
+ }
+ else
+ {
+ $map{$c} = chr($base4_sym++);
+ }
+}
+
+sub transform_string
+{
+ my ($text, $mode) = @_;
+
+ return $text if $mode eq 'control';
+ return $text if $denylist{$text};
+
+ # For format strings, protect PostgreSQL %-escapes
+ # (e.g. %s, %I, %L, %% and parametrized ones like %1$s, %-10s)
+ if ($mode eq 'format')
+ {
+ my $res = "";
+ # Scans the string chunk by chunk, matching anything up to a format specifier or end of string.
+ # The specifier regex captures: %, optional -, numbers, $, *, and the final type letter (s, I, L, or %).
+ while ($text =~ /\G(.*?)(%[-0-9\$*]*[sIL%]|$)/gcs)
+ {
+ my $pre = $1;
+ my $spec = $2;
+
+ # Recursively apply 'normal' transformation to the text between format specifiers
+ $res .= transform_string($pre, 'normal') if length $pre;
+ $res .= $spec if length $spec;
+ }
+ return $res;
+ }
+
+ my $res = "";
+ my $is_regex = ($mode eq 'regex');
+ my @chars = split //, $text;
+
+ for (my $i = 0; $i < @chars; $i++)
+ {
+ my $c = $chars[$i];
+
+ # Pass through doubled single quotes untouched
+ if ($c eq "'" && $i + 1 < @chars && $chars[ $i + 1 ] eq "'")
+ {
+ $res .= "''";
+ $i++;
+ next;
+ }
+
+ if ($is_regex)
+ {
+ # Skip transformation for escaped regex sequences (e.g., \d, \s)
+ if ($c eq '\\' && $i + 1 < @chars)
+ {
+ $res .= $c . $chars[ $i + 1 ];
+ $i++;
+ next;
+ }
+ if ($regex_metachars{$c})
+ {
+ $res .= $c;
+ next;
+ }
+ }
+
+ $res .= $map{$c} // $c;
+ }
+ return $res;
+}
+
+# Slurp the entire input stream
+local $/ = undef;
+my $sql = <STDIN>;
+my $out = "";
+
+# Stack tracks nested function calls: { name => 'regexp_replace', arg => 0 }
+my @stack;
+my $last_id = "";
+
+pos($sql) = 0;
+
+# The regex evaluates alternatives in order.
+# It matches comments and dollar-quotes first, returning them unmodified.
+# It only captures and transforms single-quoted strings.
+while (
+ $sql =~ m/\G(
+ (--.*?\n) # $2: single-line comment
+ | (\/\*.*?\*\/) # $3: multi-line comment
+ | (\$([a-zA-Z0-9_]*)\$.*?\$\5\$) # $4: dollar quote, $5: tag
+ | (([EeuU]&?)?('(?:[^']|'')*')) # $6: full string, $7: prefix, $8: content
+ | ([a-zA-Z_][a-zA-Z0-9_]*) # $9: identifier
+ | ([(),]) # $10: punctuation
+ | (\s+) # $11: whitespace
+ | (.) # $12: other
+)/gsx)
+{
+ if (defined $2)
+ {
+ $out .= $2;
+ $last_id = "";
+ }
+ elsif (defined $3)
+ {
+ $out .= $3;
+ $last_id = "";
+ }
+ elsif (defined $4)
+ {
+ $out .= $4;
+ $last_id = "";
+ }
+ elsif (defined $6)
+ {
+ my $prefix = $7 // "";
+ my $literal = $8;
+ $literal =~ s/^'//;
+ $literal =~ s/'$//;
+
+ my $mode = 'normal';
+
+ # 1. Apply function argument rules based on the stack
+ if (@stack)
+ {
+ my $func = lc($stack[-1]{name});
+ my $arg_idx = $stack[-1]{arg};
+
+ # Apply arg index rules for regexp functions
+ if ($func =~ /^regexp_/)
+ {
+ if ($arg_idx == 0)
+ {
+ $mode = 'normal';
+ }
+ elsif ($arg_idx == 1)
+ {
+ $mode = 'regex';
+ }
+ elsif ($arg_idx == 2 && $func eq 'regexp_replace')
+ {
+ $mode = 'normal';
+ }
+ else
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'encode')
+ {
+ if ($arg_idx == 0 || $arg_idx == 1)
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'decode')
+ {
+ if ($arg_idx == 1)
+ {
+ $mode = 'control';
+ }
+ }
+ elsif ($func eq 'format')
+ {
+ if ($arg_idx == 0)
+ {
+ $mode = 'format';
+ }
+ }
+ }
+
+ # 2. Look ahead in the unparsed text to see if it's cast to bytea or regclass
+ my $remainder = substr($sql, pos($sql));
+ if ($remainder =~ /^\s*::\s*(?:bytea|regclass)\b/i)
+ {
+ $mode = 'control';
+ }
+
+ $out .= $prefix . "'" . transform_string($literal, $mode) . "'";
+ $last_id = "";
+ }
+ elsif (defined $9)
+ {
+ $out .= $9;
+ # Exclude common SQL syntax keywords from being treated as function names
+ if ($9 !~ /^(from|for|as|and|or|not|in)$/i)
+ {
+ $last_id = $9;
+ }
+ }
+ elsif (defined $10)
+ {
+ my $p = $10;
+ $out .= $p;
+ if ($p eq '(')
+ {
+ push @stack, { name => $last_id, arg => 0 };
+ }
+ elsif ($p eq ',')
+ {
+ $stack[-1]{arg}++ if @stack;
+ }
+ elsif ($p eq ')')
+ {
+ pop @stack if @stack;
+ }
+ $last_id = "";
+ }
+ elsif (defined $11)
+ {
+ $out .= $11;
+ }
+ elsif (defined $12)
+ {
+ $out .= $12;
+ $last_id = "";
+ }
+}
+
+print $out;
diff --git a/run.sh b/run.sh
new file mode 100644
index 0000000..b000c34
--- /dev/null
+++ b/run.sh
@@ -0,0 +1,24 @@
+# To let coverage gaps inform which files to translate, I used bpftrace to get
+# the call stacks that reach a pg_mblen* call w/ retval=1 but reach no such
+# call with retval!=1.
+
+cd src/test/regress
+# fairly clean
+tests='text strings regex'
+# much noisier
+tests="$tests arrays collate.icu.utf8 tsdicts tsearch tsrf tstypes"
+# expects v15+; v14 lacks test_setup
+runnable=test_setup
+for t in $tests; do
+ perl ../../../ascii2utf8sql.pl <sql/$t.sql >sql/$t.utf8.sql
+ # TODO script not ready for expected outputs: deletes more than it should
+ perl ../../..//ascii2utf8sql.pl <expected/$t.out >expected/$t.utf8.out
+ runnable="$runnable $t.utf8"
+done
+cd ../../..
+make installcheck-tests TESTS="$runnable tablespace" "$@"
+echo '==== all added errors'
+echo '==== (includes false positives from flawed munging of expected output)'
+grep '^[+]ERROR' src/test/regress/regression.diffs | sort | uniq -c
+echo '==== all examples of goal error: invalid byte sequence for encoding'
+grep 'invalid byte sequence for encoding' src/test/regress/regression.diffs
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 4e26df7..9b51df5 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -34,11 +34,188 @@
#include "utils/lsyscache.h"
#include "utils/syscache.h"
+#include "access/xact.h"
+#include "access/toast_compression.h"
+#include "access/toast_internals.h"
+#include "utils/fmgroids.h"
+
+/* https://gemini.google.com/u/1/app/dc868a99fe48c7e8 */
+/* stack of nested function pointers */
+#define MAX_NESTED_CALLS 1024
+static PGFunction fn_addr_stack[MAX_NESTED_CALLS];
+static int fn_addr_stack_ptr = 0;
+
+/*
+ *
+ * The wrapper function that receives the fully populated FunctionCallInfo,
+ * compresses varlenas in place, and calls the original function.
+ */
+static Datum
+my_function_wrapper(FunctionCallInfo fcinfo)
+{
+ PGFunction orig_fn;
+ Datum result;
+ bool modified = false;
+
+ /* Allocate backup array on the stack (max args in Postgres is 100) */
+ Datum saved_values[100];
+
+ if (fn_addr_stack_ptr > 0)
+ orig_fn = fn_addr_stack[fn_addr_stack_ptr - 1];
+ else
+ elog(ERROR, "test_toast_hook: fn_addr_stack empty");
+
+ /* Backup and compress */
+ for (int i = 0; i < fcinfo->nargs; i++)
+ {
+ saved_values[i] = fcinfo->args[i].value;
+
+ if (!fcinfo->args[i].isnull && get_fn_expr_argtype(fcinfo->flinfo, i) == TEXTOID)
+ {
+ Datum val = fcinfo->args[i].value;
+
+ if (!VARATT_IS_EXTENDED(DatumGetPointer(val)))
+ {
+ Datum cval = toast_compress_datum(val, TOAST_PGLZ_COMPRESSION);
+
+ if (cval)
+ {
+ fcinfo->args[i].value = cval;
+ modified = true;
+ }
+ }
+ }
+ }
+
+ /* Execute */
+ result = orig_fn(fcinfo);
+
+ /* Restore the original pointers to protect the ExprState cache */
+ if (modified)
+ {
+ for (int i = 0; i < fcinfo->nargs; i++)
+ {
+ fcinfo->args[i].value = saved_values[i];
+ }
+ }
+
+ return result;
+}
+
+/*
+ * Force Postgres to route all function calls through fmgr_security_definer.
+ */
+#define MAX_CACHED_OIDS 65536
+/*
+ * 0 = unknown, 1 = hook needed, 2 = hook not needed
+ *
+ * Exempt functions used in a search of the PROCOID system, to avoid infinite
+ * recursion. We know none of these have text args. This list is probably
+ * incomplete, since associated relcache rebuilds are possible but rare.
+ * FIXME test w/ debug_discard_caches.
+ */
+static uint8 oid_hook_cache[MAX_CACHED_OIDS] = {
+ [F_BTHANDLER] = 2,
+ [F_BTOIDCMP] = 2,
+ [F_BTTEXTCMP] = 2,
+ [F_HEAP_TABLEAM_HANDLER] = 2,
+ [F_INT2GT] = 2,
+ [F_OIDEQ] = 2,
+};
+
+static bool
+my_needs_fmgr_hook(Oid fn_oid)
+{
+ bool needs_hook = false;
+ HeapTuple tup;
+
+ /* Fast path: check local cache */
+ if (fn_oid < MAX_CACHED_OIDS)
+ {
+ if (oid_hook_cache[fn_oid] == 1)
+ return true;
+ if (oid_hook_cache[fn_oid] == 2)
+ return false;
+ }
+
+ /*
+ * * If we cannot access the catalog safely, assume false to prevent
+ * crashes. The cache will remain 'unknown' and will be populated during
+ * the next in-transaction execution.
+ */
+ if (!IsTransactionState())
+ return false;
+
+ /*
+ * So far, infinite recursion has always been associated with a btree
+ * opclass member relevant to the PROCOID syscache or its recursive
+ * dependencies. The solution has been to add the member to the
+ * oid_hook_cache initializer.
+ */
+ if (stack_is_too_deep())
+ elog(ERROR, "stack depth for fn_oid=%u", fn_oid);
+
+ /* Safe to query the catalog to inspect the function's arguments */
+ tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
+
+ if (HeapTupleIsValid(tup))
+ {
+ Form_pg_proc procForm = (Form_pg_proc) GETSTRUCT(tup);
+
+ /* Check if any argument requires TEXTOID */
+ for (int i = 0; i < procForm->pronargs; i++)
+ {
+ if (procForm->proargtypes.values[i] == TEXTOID)
+ {
+ needs_hook = true;
+ break;
+ }
+ }
+ ReleaseSysCache(tup);
+ }
+
+ /* Populate the cache */
+ if (fn_oid < MAX_CACHED_OIDS)
+ {
+ oid_hook_cache[fn_oid] = needs_hook ? 1 : 2;
+ }
+
+ return needs_hook;
+}
+
+/*
+ * Hook the execution cycle to swap the function pointer.
+ */
+static void
+my_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum *private_data)
+{
+ if (event == FHET_START)
+ {
+ /* Stash the real function pointer */
+ if (fn_addr_stack_ptr < MAX_NESTED_CALLS)
+ fn_addr_stack[fn_addr_stack_ptr++] = flinfo->fn_addr;
+ else
+ elog(ERROR, "test_toast_hook: MAX_NESTED_CALLS exceeded");
+
+ /* Replace the execution pointer with our argument-mutating wrapper */
+ flinfo->fn_addr = my_function_wrapper;
+ }
+ else if (event == FHET_END || event == FHET_ABORT)
+ {
+ /* Pop the stack AND restore the original function pointer */
+ if (fn_addr_stack_ptr > 0)
+ {
+ fn_addr_stack_ptr--;
+ flinfo->fn_addr = fn_addr_stack[fn_addr_stack_ptr];
+ }
+ }
+}
+
/*
* Hooks for function calls
*/
-PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook = NULL;
-PGDLLIMPORT fmgr_hook_type fmgr_hook = NULL;
+PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook = my_needs_fmgr_hook;
+PGDLLIMPORT fmgr_hook_type fmgr_hook = my_fmgr_hook;
/*
* Hashtable for fast lookup of external C functions
@@ -164,7 +341,9 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
finfo->fn_mcxt = mcxt;
finfo->fn_expr = NULL; /* caller may set this later */
- if ((fbp = fmgr_isbuiltin(functionId)) != NULL)
+ if ((fbp = fmgr_isbuiltin(functionId)) != NULL &&
+ (!IsNormalProcessingMode() ||
+ !FmgrHookIsNeeded(functionId)))
{
/*
* Fast path for builtin functions: don't bother consulting pg_proc
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index 75d529d..aec17c6 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -233,7 +233,6 @@ static const PGLZ_Strategy strategy_default_data = {
10 /* Lower good match size by 10% at every loop
* iteration */
};
-const PGLZ_Strategy *const PGLZ_strategy_default = &strategy_default_data;
static const PGLZ_Strategy strategy_always_data = {
@@ -246,6 +245,7 @@ static const PGLZ_Strategy strategy_always_data = {
6 /* Look harder for a good match */
};
const PGLZ_Strategy *const PGLZ_strategy_always = &strategy_always_data;
+const PGLZ_Strategy *const PGLZ_strategy_default = &strategy_always_data;
/* ----------
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index f38688b..6e0b628 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -1455,13 +1455,13 @@ ERROR: regexp_split_to_array() does not support the "global" option
-- change NULL-display back
\pset null ''
-- E021-11 position expression
-SELECT POSITION('4' IN '1234567890') = '4' AS "4";
+SELECT POSITION('4' IN '1234567890') = $int4$4$int4$ AS "4";
4
---
t
(1 row)
-SELECT POSITION('5' IN '1234567890') = '5' AS "5";
+SELECT POSITION('5' IN '1234567890') = $int4$5$int4$ AS "5";
5
---
t
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index d8a0973..b26ae4d 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -395,9 +395,9 @@ SELECT regexp_split_to_array('thE QUick bROWn FOx jUMPs ovEr The lazy dOG', 'e',
\pset null ''
-- E021-11 position expression
-SELECT POSITION('4' IN '1234567890') = '4' AS "4";
+SELECT POSITION('4' IN '1234567890') = $int4$4$int4$ AS "4";
-SELECT POSITION('5' IN '1234567890') = '5' AS "5";
+SELECT POSITION('5' IN '1234567890') = $int4$5$int4$ AS "5";
SELECT POSITION('\x11'::bytea IN ''::bytea) = 0 AS "0";
SELECT POSITION('\x33'::bytea IN '\x1122'::bytea) = 0 AS "0";
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2026-02-17 18:15 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-13 07:46 BUG #19406: substring(text) fails on valid UTF-8 toasted value in PostgreSQL 15.16 PG Bug reporting form <noreply@postgresql.org>
2026-02-13 17:27 ` Noah Misch <noah@leadboat.com>
2026-02-13 21:58 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-13 22:48 ` Noah Misch <noah@leadboat.com>
2026-02-14 00:21 ` Noah Misch <noah@leadboat.com>
2026-02-14 05:38 ` Noah Misch <noah@leadboat.com>
2026-02-14 08:15 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:07 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 19:33 ` Noah Misch <noah@leadboat.com>
2026-02-14 20:10 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 20:19 ` Thomas Munro <thomas.munro@gmail.com>
2026-02-14 22:46 ` Noah Misch <noah@leadboat.com>
2026-02-16 18:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-02-17 04:20 ` Noah Misch <noah@leadboat.com>
2026-02-17 18:15 ` Noah Misch <noah@leadboat.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox