Fixed Issue #3326

[Hydrocharged]

Fixes #3326.

Stacked on #3349.

[itoqa]

Commit: 61288aa: 13 test cases ran, 1 failed ❌, 12 passed ✅.

Summary

The run covers byte-to-text conversion and decoding across common formats and character sets, including valid inputs, malformed and boundary inputs, error classification, session recovery, and result types. Core valid behavior and most defensive handling are healthy, but one malformed multibyte-input edge case still produces incorrect replacement text instead of an error.

Merge with caution — the PR has a medium-severity correctness issue where malformed multibyte input can be silently altered rather than rejected, although valid conversions and session stability remain intact. No unrelated findings are driving the verdict.

Tests run by Ito

Result	Severity	Type	Description
❌		Rev	The UTF-8 cases for bytes C3, E2 82, and FF correctly returned invalid byte sequence errors, and a valid query worked afterward. The truncated EUC_JP byte A4 returned the replacement character instead of failing as required for an incomplete multibyte sequence.
✅	—	General	Invalid bytes were rejected at the beginning, middle, and end of the input, while Latin-1 decoded each position without replacement text.
✅	—	General	Hex input with spaces, a final separator, or no digits returns the expected bytes without adding whitespace. An odd number of effective digits is rejected with the expected error.
✅	—	General	Incomplete escape sequences return a bytea syntax error, while complete octal and doubled-backslash sequences return the expected bytes without crashing the session.
✅	—	General	Decoded Latin1, UTF-8, and Japanese text kept the expected characters and lengths. Invalid UTF-8 returned the expected error, and the same SQL session continued working afterward.
✅	—	General	Malformed hex, base64, and escape input returned the expected PostgreSQL errors, and the same session continued working after each failure.
✅	—	Convert	The database returned hello for UTF-8 bytes and é for Latin-1 bytes without an error.
✅	—	Convert	The database rejects an unknown encoding name with a clear error and returns no text.
✅	—	Decode	The database returned the expected bytes for base64, escape, and hexadecimal input without errors.
✅	—	Decode	Using a dollar sign in base64 input shows an error and returns no partial decoded value.
✅	—	Decode	Using an unsupported format returns an error that names the format, and no decoded value is returned.
✅	—	Rev	Recognized but unsupported encodings show a feature-not-supported error, while an unknown encoding name shows an invalid-encoding error.
✅	—	Rev	The SQL functions returned the expected byte data and text, with the correct result types. The prepared-query check could not run because this local server does not support prepared statements.

Tip

Reply with @itoqa to send us feedback on this test run.

[itoqa]

Truncated EUC-JP returns replacement text

What failed: The UTF-8 cases for bytes C3, E2 82, and FF correctly returned invalid byte sequence errors, and a valid query worked afterward. The truncated EUC_JP byte A4 returned the replacement character instead of failing as required for an incomplete multibyte sequence.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug

• Severity: Medium
• Impact: A user who submits truncated EUC-JP data can receive a replacement character instead of an error, so the converted text is silently wrong. The issue is limited to malformed multibyte input and does not affect valid conversions.
• Steps to Reproduce:
  1. Connect to the local Doltgres server as the postgres user.
  2. Run SELECT convert_from('\xa4'::bytea, 'EUC_JP');.
  3. Observe that the query returns a replacement character instead of an invalid byte sequence error.
  4. Run SELECT convert_from('\x6869'::bytea, 'UTF8'); afterward and confirm the connection still works.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The PR adds server/functions/convert_from.go and registers it from server/functions/init.go. In convert_from_bytea_name.Callable, lines 50-65 resolve the requested PostgreSQL encoding and distinguish pass-through, unsupported, and decoder-backed encodings. For EUC_JP, server/functions/encoding.go lines 42-45 select japanese.EUCJP as the encoder. The decoder branch at server/functions/convert_from.go lines 67-72 calls source.encoder.NewDecoder().Bytes(input), discards the exact decoder output semantics, and returns string(converted) whenever Bytes returns no Go error. That behavior lets x/text produce a replacement character for the truncated input instead of converting the malformed sequence into the PostgreSQL CharacterNotInRepertoire error that the surrounding function promises. The smallest practical fix is to use strict malformed-input handling for decoder-backed source encodings, or explicitly detect incomplete/invalid source sequences before returning converted text, and return the existing invalid-byte-sequence error on rejection.
• Why this is likely a bug: The failure is reproducible through the normal SQL API with a user-supplied bytea value, not through a mock or test-only patch. The test’s UTF-8 cases demonstrate the intended contract: malformed bytes must fail, no replacement text may be returned, and the same session must remain usable. The EUC_JP result violates the same contract by silently changing malformed input into a visible character. Because the PR introduced both the EUC_JP decoder registration and the conversion branch that returns its output, the issue is a direct PR regression and can be fixed locally by making that branch reject malformed or truncated source sequences.

Relevant code

server/functions/convert_from.go:50-72

source := lookupPostgresEncoding(encodingName)
...
converted, err := source.encoder.NewDecoder().Bytes(input)
if err != nil {
    return nil, pgerror.WithCandidateCode(fmt.Errorf(`invalid byte sequence for encoding "%s"`, source.name), pgcode.CharacterNotInRepertoire)
}
return string(converted), nil

server/functions/encoding.go:42-45

var postgresEncodings = []postgresEncoding{
    {name: "SQL_ASCII", id: 0, aliases: []string{"SQLASCII"}, passThrough: true},
    {name: "EUC_JP", id: 1, encoder: japanese.EUCJP},

Evidence Package

• text/REV-6/REV-6-truncated-euc-jp-evidence.txt — 3 KB

Copy prompt for an agent

Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Truncated EUC-JP returns replacement text**

**What failed:** The UTF-8 cases for bytes C3, E2 82, and FF correctly returned invalid byte sequence errors, and a valid query worked afterward. The truncated EUC_JP byte A4 returned the replacement character instead of failing as required for an incomplete multibyte sequence.

- **Impact:** A user who submits truncated EUC-JP data can receive a replacement character instead of an error, so the converted text is silently wrong. The issue is limited to malformed multibyte input and does not affect valid conversions.
- **Steps to reproduce:**
  1. Connect to the local Doltgres server as the postgres user.
  2. Run SELECT convert_from('\xa4'::bytea, 'EUC_JP');.
  3. Observe that the query returns a replacement character instead of an invalid byte sequence error.
  4. Run SELECT convert_from('\x6869'::bytea, 'UTF8'); afterward and confirm the connection still works.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR adds server/functions/convert_from.go and registers it from server/functions/init.go. In convert_from_bytea_name.Callable, lines 50-65 resolve the requested PostgreSQL encoding and distinguish pass-through, unsupported, and decoder-backed encodings. For EUC_JP, server/functions/encoding.go lines 42-45 select japanese.EUCJP as the encoder. The decoder branch at server/functions/convert_from.go lines 67-72 calls source.encoder.NewDecoder().Bytes(input), discards the exact decoder output semantics, and returns string(converted) whenever Bytes returns no Go error. That behavior lets x/text produce a replacement character for the truncated input instead of converting the malformed sequence into the PostgreSQL CharacterNotInRepertoire error that the surrounding function promises. The smallest practical fix is to use strict malformed-input handling for decoder-backed source encodings, or explicitly detect incomplete/invalid source sequences before returning converted text, and return the existing invalid-byte-sequence error on rejection.
- **Why this is likely a bug:** The failure is reproducible through the normal SQL API with a user-supplied bytea value, not through a mock or test-only patch. The test’s UTF-8 cases demonstrate the intended contract: malformed bytes must fail, no replacement text may be returned, and the same session must remain usable. The EUC_JP result violates the same contract by silently changing malformed input into a visible character. Because the PR introduced both the EUC_JP decoder registration and the conversion branch that returns its output, the issue is a direct PR regression and can be fixed locally by making that branch reject malformed or truncated source sequences.

**Relevant code:**

`server/functions/convert_from.go:50-72`

~~~go
source := lookupPostgresEncoding(encodingName)
...
converted, err := source.encoder.NewDecoder().Bytes(input)
if err != nil {
    return nil, pgerror.WithCandidateCode(fmt.Errorf(`invalid byte sequence for encoding "%s"`, source.name), pgcode.CharacterNotInRepertoire)
}
return string(converted), nil
~~~

`server/functions/encoding.go:42-45`

~~~go
var postgresEncodings = []postgresEncoding{
    {name: "SQL_ASCII", id: 0, aliases: []string{"SQLASCII"}, passThrough: true},
    {name: "EUC_JP", id: 1, encoder: japanese.EUCJP},
~~~

[zachmu]

Probably worth addresssing now

[github-actions]

	Main	PR
Total	42090	42090
Successful	19854	19866
Failures	22236	22224
Partial Successes1	5440	5441

	Main	PR
Successful	47.1703%	47.1989%
Failures	52.8297%	52.8011%

${\color{lightgreen}Progressions (11)}$

aggregates

QUERY: insert into bytea_test_table values(decode('ff','hex'));

QUERY: insert into bytea_test_table values(decode('aa','hex'));

constraints

QUERY: SELECT * FROM DEFAULTEXPR_TBL;

strings

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: SELECT decode('1234567890abcdef00', 'hex');

QUERY: SELECT decode(encode(('\x' || repeat('1234567890abcdef0001', 7))::bytea,
                     'base64'), 'base64');

QUERY: SELECT decode(encode('\x1234567890abcdef00', 'escape'), 'escape');

subselect

QUERY: select count(*) from tenk1 t
where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0);

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[coffeegoddd]

@Hydrocharged DOLT

read_tests	from_latency	to_latency	percent_change
covering_index_scan_postgres	2.43	2.43	0.0
groupby_scan_postgres	78.6	78.6	0.0
index_join_postgres	2.26	2.26	0.0
index_join_scan_postgres	1.61	1.61	0.0
index_scan_postgres	458.96	467.3	1.82
oltp_point_select	0.37	0.37	0.0
oltp_read_only	6.43	6.32	-1.71
select_random_points	0.73	0.73	0.0
select_random_ranges	1.04	1.04	0.0
table_scan_postgres	467.3	467.3	0.0
types_table_scan_postgres	1170.65	1170.65	0.0

write_tests	from_latency	to_latency	percent_change
oltp_delete_insert_postgres	6.67	6.67	0.0
oltp_insert	3.36	3.36	0.0
oltp_read_write	13.46	13.46	0.0
oltp_update_index	3.55	3.55	0.0
oltp_update_non_index	3.25	3.25	0.0
oltp_write_only	7.04	7.04	0.0
types_delete_insert_postgres	7.17	7.17	0.0

[zachmu]

Change itself looks good, but please change the PR description to describe what actually changed / is implemented rather than just an issue number. For this one, should be "Implements convert_from and decode"

[zachmu]

Probably worth addresssing now

[zachmu]

This test belongs in function_test