Re: Remove invalid SS2/SS3 handling from EUC-KR routines

From: SungJun Jang <sjjang112233(at)gmail(dot)com>
To: John Naylor <johncnaylorls(at)gmail(dot)com>
Cc: assam258(at)gmail(dot)com, Michael Paquier <michael(at)paquier(dot)xyz>, Junwang Zhao <zhjwpku(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, li(dot)evan(dot)chao(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, Tatsuo Ishii <ishii(at)postgresql(dot)org>, thomas(dot)munro(at)gmail(dot)com
Subject: Re: Remove invalid SS2/SS3 handling from EUC-KR routines
Date: 2026-10-06 01:40:59
Message-ID: CAE+cgNiwK81_CsuUk36yjH6vr415kKB9Qnmcphd2Ksocf1ek2Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi John,

Thanks for looking at this.

> Doesn't that just contradict what you said above? I've set it back to
> waiting on author.

Right, v3 had no tests. v4 is attached, split as discussed:

0001 adds tests to euc_kr.sql and records what master does today.
0002 is the v3 change (code unchanged, rebased) plus the
expected-output changes.

> While looking, I've found another behavior change that should be
> called out -- data type width calculations can change query plans:

Thanks, I had missed that. I get the same numbers here. The source
is type_maximum_size(), which multiplies the typmod by
pg_encoding_max_length(). It has two callers, so there are two
effects in EUC_KR databases:

- get_typavgwidth(): the width estimates in your example.

- heapam_relation_needs_toast_table(): a new table whose columns all
have a bounded width might no longer get a TOAST table. For
"create table t (v varchar(700))", master creates one and v4 does
not.

I went through the other readers of maxmblen as well. They either
compare it against 1 or use it to size buffers. The one visible edge
I found is the length limit in lpad()/rpad()/translate():
lpad('x', 400000000) fails with "requested length too large" on
master and succeeds with v4.

One correction to how this was described earlier in the thread: 0x8E
is not affected. pg_euc_mblen() already returned 2 for SS2, and the
wchar value is built the same way, so only 0x8F changes (3 bytes to
2). That is visible in error messages:

SELECT convert_from('\x8fa1a1', 'EUC_KR');
master: invalid byte sequence for encoding "EUC_KR": 0x8f 0xa1 0xa1
v4: invalid byte sequence for encoding "EUC_KR": 0x8f 0xa1

0001 has a test for this, so 0002 changes two expected lines rather
than the one I announced in May: the maximum length and this message.
Compared to what I proposed then, 0001 also has a regexp_replace()
call, to reach pg_euckr2wchar_with_len() when run in an EUC_KR
database.

The 0002 commit message now lists these effects instead of claiming
that pg_encoding_max_length() is the only one.

Testing: after each patch, the regress suite passes in a UTF8
database, and the whole parallel_schedule passes in an EUC_KR database
(pg_regress --encoding=EUC_KR --no-locale). Comparing the EUC_KR
results before and after 0002, the two lines above are the only
differences.

Given the plan changes, I think this is for master only.

Regards,
SungJun Jang

2026년 9월 25일 (금) 오후 8:00, John Naylor <johncnaylorls(at)gmail(dot)com>님이 작성:

> On Thu, Aug 20, 2026 at 12:11 PM Henson Choi <assam258(at)gmail(dot)com> wrote:
> > From your message of May 13:
> >
> > > Does this structure work for you, or would you prefer a different
> > > approach?
> >
> > It works. 0001 baseline capture, then 0002 with the fix, is a good
> > idea.
>
> +1
>
> > That is what I had asked for. I have now reviewed v3 and I think it
> > should go in as it stands. CI is green, so I am marking this Ready for
> > Committer.
>
> Doesn't that just contradict what you said above? I've set it back to
> waiting on author.
>
> While looking, I've found another behavior change that should be
> called out -- data type width calculations can change query plans:
>
> createdb -T template0 -E EUC_KR --locale=C euc_kr
>
> psql -d euc_kr -X <<'EOF'
> create table t(c char(10), v varchar(10));
> explain select c from t;
> explain select v from t;
> EOF
>
> master:
> QUERY PLAN
> -----------------------------------------------------
> Seq Scan on t (cost=0.00..18.50 rows=850 width=34)
> (1 row)
>
> QUERY PLAN
> -----------------------------------------------------
> Seq Scan on t (cost=0.00..18.50 rows=850 width=33)
> (1 row)
>
> v3:
> QUERY PLAN
> ------------------------------------------------------
> Seq Scan on t (cost=0.00..20.70 rows=1070 width=24)
> (1 row)
>
> QUERY PLAN
> ------------------------------------------------------
> Seq Scan on t (cost=0.00..20.70 rows=1070 width=24)
> (1 row)
>
> --
> John Naylor
> Amazon Web Services
>

Attachment Content-Type Size
v4-0002-Make-EUC-KR-encoding-routines-self-contained.patch application/octet-stream 6.5 KB
v4-0001-Add-tests-for-EUC_KR-encoding-properties.patch application/octet-stream 3.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shihao zhong 2026-10-06 01:48:58 Re: [PG19]pg_verifybackup never finishes on a gzip-compressed tar backup
Previous Message Manu 2026-10-06 01:29:09 Re: Progress reporting: a debug trace and a test framework