Skip to content

Support Unicode collation expansions - #3801

Open
elianddb wants to merge 1 commit into
mainfrom
elian/11506
Open

Support Unicode collation expansions#3801
elianddb wants to merge 1 commit into
mainfrom
elian/11506

Conversation

@elianddb

@elianddb elianddb commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@elianddb elianddb changed the title sql: support Unicode collation expansions Support Unicode collation expansions Sep 4, 2026
@itoqa

itoqa Bot commented Sep 4, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: e587585: 14 test cases ran, 4 failed ❌, 10 passed ✅.

Summary

The run broadly covers Unicode text equality and ordering across lookups, joins, grouping, hashing, sorting, padding, and case-sensitive or binary comparisons. It also exercises adversarial malformed-text inputs, revealing weaknesses in error handling and edge-case comparison semantics.

Not safe to merge yet — the PR introduces a high-impact correctness issue in binary text comparisons and additional medium-impact failures involving case-sensitive matching and malformed input acceptance, which can produce incorrect query results or silently accept invalid data. All reported failures are attributable to this PR, so they are merge blockers rather than unrelated caveats.

Tests run by Ito

View full run

Result Severity Type Description
High severity Rev The database says ß and ss are equal in a binary collation. The equality check should keep these different byte sequences separate, just as DISTINCT does.
Medium severity Malformed A comparison using invalid UTF-8 returned no error instead of rejecting the malformed input.
Medium severity Rev The database says uppercase Œ is equal to lowercase oe. A case-sensitive comparison should not erase that case difference.
Medium severity Rev The database accepts two identical strings with invalid text and reports them as equal. It should reject the invalid text with a malformed-string error.
General Equivalent values are placed together and each gets a window count of 2. The separate value gets a count of 1.
General Strings with sharp S compared correctly with their two-letter forms, including when another character followed them. Changing that following character changed the ordering consistently, and all rows remained in the sorted result.
General Equivalent spellings stay equal when trailing spaces are added, while a later non-space character is still compared correctly in both directions.
Comparison Sharp S and the oe ligature matched their two-letter forms, and German phonebook comparisons matched umlauts with ae and oe. Sharp S also stayed below x, so equality and ordering both worked as expected.
Comparison The general collation kept sharp S (ß) different from ss. Equality returned false, ß sorted before ss, and the reverse ordering check also returned false as expected.
Hashing Looking up the stored sharp S with ss returned the row, and both spellings appeared in one group with a count of 2.
Padding The two equivalent spellings compare as equal even when one has trailing spaces, in both operand orders.
Rev The join matched ss with the sharp-S spelling, and the indexed lookup found the sharp-S row when searching for ss.
Rev All five expansion pairs compared equal in SQL, and ORDER BY returned all 10 inserted rows in a stable order without errors or missing data.
Window A window query grouped ss and sharp S together, while x stayed in its own group.

Tip

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

Comment thread sql/weight_scanner.go
return 0, false, nil
}

var r rune

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

Medium severity Invalid text is accepted during comparison

What failed: A comparison using invalid UTF-8 returned no error instead of rejecting the malformed input.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Queries that compare malformed UTF-8 may return an ordering result instead of rejecting the invalid value. This can produce incorrect filtering or sorting for affected data, but there is no evidence of data loss or corruption.
  • Steps to Reproduce:
    1. Create a utf8mb4_unicode_ci StringType.
    2. Call StringType.Compare with string([]byte{0xff}) as one operand and a valid string such as "x" as the other.
    3. Check the returned error; it is nil instead of ErrCollationMalformedString.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PR added sql/weight_scanner.go and moved StringType.Compare in sql/types/strings.go to use it. In WeightScanner.Next, sql/weight_scanner.go:62-68 decodes the next rune and stores the decoded rune in r and the number of consumed bytes in read. The guard at sql/weight_scanner.go:69 checks read == utf8.RuneError, but utf8.RuneError is a rune value (U+FFFD), not a byte count. For []byte{0xff}, utf8.DecodeRuneInString returns r == utf8.RuneError and read == 1, so the condition is false; the scanner consumes the byte and returns a collation weight. StringType.Compare at sql/types/strings.go:311-331 propagates scanner errors when they occur, but receives nil here and returns an ordering result. The smallest fix is to change the guard to test r == utf8.RuneError together with read == 1 (or use the encoder's equivalent malformed-sequence contract), while retaining read == 0 as an error. The same validation must apply in both the encoder and UTF-8 fallback branches.
  • Why this is likely a bug: The focused direct probe reproduced the defect: StringType.Compare returned a nil error for string([]byte{0xff}) versus "x", while the test contract requires ErrCollationMalformedString and no comparison result. The control comparison tests still pass, so the failure is isolated to malformed-input detection rather than a general collation-ordering problem. The PR's new scanner is the direct production path for this input, and its incorrect type/value check explains the result. A targeted condition fix in WeightScanner.Next is sufficient; no broad redesign is needed.
Relevant code

sql/weight_scanner.go:62-71

var r rune
var read int
if ws.encoder != nil {
	r, read = ws.encoder.NextRune(ws.str)
} else {
	r, read = utf8.DecodeRuneInString(ws.str)
}
if read == 0 || read == utf8.RuneError {
	return 0, false, ErrCollationMalformedString.New("scanning")
}

sql/types/strings.go:311-331

iterA := sql.NewWeightScanner(t.collation, as)
iterB := sql.NewWeightScanner(t.collation, bs)
...
aWeight, aOk, aErr := iterA.Next()
if aErr != nil {
	return 0, aErr
}
bWeight, bOk, bErr := iterB.Next()
if bErr != nil {
	return 0, bErr
}

sql/errors.go:847-848

ErrCollationMalformedString = errors.NewKind("malformed string encountered while %s")
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Invalid text is accepted during comparison**

**What failed:** A comparison using invalid UTF-8 returned no error instead of rejecting the malformed input.

- **Impact:** Queries that compare malformed UTF-8 may return an ordering result instead of rejecting the invalid value. This can produce incorrect filtering or sorting for affected data, but there is no evidence of data loss or corruption.
- **Steps to reproduce:**
  1. Create a utf8mb4_unicode_ci StringType.
  2. Call StringType.Compare with string([]byte{0xff}) as one operand and a valid string such as "x" as the other.
  3. Check the returned error; it is nil instead of ErrCollationMalformedString.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR added sql/weight_scanner.go and moved StringType.Compare in sql/types/strings.go to use it. In WeightScanner.Next, sql/weight_scanner.go:62-68 decodes the next rune and stores the decoded rune in r and the number of consumed bytes in read. The guard at sql/weight_scanner.go:69 checks read == utf8.RuneError, but utf8.RuneError is a rune value (U+FFFD), not a byte count. For []byte{0xff}, utf8.DecodeRuneInString returns r == utf8.RuneError and read == 1, so the condition is false; the scanner consumes the byte and returns a collation weight. StringType.Compare at sql/types/strings.go:311-331 propagates scanner errors when they occur, but receives nil here and returns an ordering result. The smallest fix is to change the guard to test r == utf8.RuneError together with read == 1 (or use the encoder's equivalent malformed-sequence contract), while retaining read == 0 as an error. The same validation must apply in both the encoder and UTF-8 fallback branches.
- **Why this is likely a bug:** The focused direct probe reproduced the defect: StringType.Compare returned a nil error for string([]byte{0xff}) versus "x", while the test contract requires ErrCollationMalformedString and no comparison result. The control comparison tests still pass, so the failure is isolated to malformed-input detection rather than a general collation-ordering problem. The PR's new scanner is the direct production path for this input, and its incorrect type/value check explains the result. A targeted condition fix in WeightScanner.Next is sufficient; no broad redesign is needed.

**Relevant code:**

`sql/weight_scanner.go:62-71`

~~~go
var r rune
var read int
if ws.encoder != nil {
	r, read = ws.encoder.NextRune(ws.str)
} else {
	r, read = utf8.DecodeRuneInString(ws.str)
}
if read == 0 || read == utf8.RuneError {
	return 0, false, ErrCollationMalformedString.New("scanning")
}
~~~

`sql/types/strings.go:311-331`

~~~go
iterA := sql.NewWeightScanner(t.collation, as)
iterB := sql.NewWeightScanner(t.collation, bs)
...
aWeight, aOk, aErr := iterA.Next()
if aErr != nil {
	return 0, aErr
}
bWeight, bOk, bErr := iterB.Next()
if bErr != nil {
	return 0, bErr
}
~~~

`sql/errors.go:847-848`

~~~go
ErrCollationMalformedString = errors.NewKind("malformed string encountered while %s")
~~~

Comment thread sql/expansion.go

var collationExpanders = [len(collationArray)]func(r rune) []rune{}

// unicodeExpander expands runes for standard Unicode Collation Algorithm

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

High severity Binary comparison treats different text as equal

What failed: The database says ß and ss are equal in a binary collation. The equality check should keep these different byte sequences separate, just as DISTINCT does.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: High High severity
  • Impact: Queries using the utf8mb4_0900_bin collation can treat different values as equal, causing filters, joins, or lookups to return incorrect results. This can silently affect applications that rely on byte-sensitive comparisons.
  • Steps to Reproduce:
    1. Open a SQL session against the local MySQL-compatible endpoint.
    2. Compare _utf8mb4'ß' COLLATE utf8mb4_0900_bin with _utf8mb4'ss' COLLATE utf8mb4_0900_bin.
    3. Observe that the equality expression returns 1, although the binary collation should return 0.
    4. Run the matching DISTINCT query and observe that it returns a count of 2.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The local SQL check returned equality_result=1 for ß versus ss under utf8mb4_0900_bin, while the equivalent UNION ALL with COUNT(DISTINCT value) returned 2. In the PR-added sql/expansion.go, unicodeExpander maps the rune 'ß' to the two-rune sequence {'s', 's'} at lines 23-27. The init loop at lines 75-83 assigns that expander to every collation whose name contains '0900', which includes utf8mb4_0900_bin. The normal StringType.Compare path in sql/types/strings.go:311-355 constructs WeightScanners and compares their emitted weights; sql/weight_scanner.go:74-81 applies the configured expander before returning each rune weight. Therefore the binary collation's ß input is converted into the same comparison-weight sequence as ss. The separate binary fast path in sql/collations.go:873-879 only protects Collation_binary hashing and does not prevent utf8mb4_0900_bin from receiving the expansion in comparison. The targeted fix is to leave utf8mb4_0900_bin without an expander, or otherwise route that specific collation through raw byte-sensitive comparison, while retaining expansion for the intended linguistic UCA collations.
  • Why this is likely a bug: This is a production-code defect, not a browser or SQL setup problem. The SQL session reached the local engine and produced the specific wrong equality result, and the source provides a direct explanation: the new dispatch treats the binary 0900 collation as an expansion collation, so a character that must remain byte-distinct is rewritten to the weights for ss before comparison. The inconsistent DISTINCT count is an additional sign that equality semantics are wrong rather than that the two values are legitimately equivalent. Any query using utf8mb4_0900_bin equality can be affected, including predicates, joins, and indexed lookups. Removing the expander assignment for this binary collation is a small, focused correction and does not require changing the intended Unicode expansion mappings for linguistic collations.
Relevant code

sql/expansion.go:21-28

func unicodeExpander(r rune) []rune {
	switch r {
	case 'ß':
		return []rune{'s', 's'}

sql/expansion.go:75-83

for id, collation := range collationArray {
		if strings.Contains(collation.Name, "_0900_") {
			collationExpanders[id] = unicodeExpander
		}

sql/weight_scanner.go:74-81

if ws.expander != nil {
		if exp := ws.expander(r); len(exp) > 0 {
			ws.expanded = exp
			ws.expandedIdx = 1
			return ws.getRuneWeight(exp[0]), true, nil
		}

sql/types/strings.go:311-312

iterA := sql.NewWeightScanner(t.collation, as)
	iterB := sql.NewWeightScanner(t.collation, bs)
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**High severity — Binary comparison treats different text as equal**

**What failed:** The database says ß and ss are equal in a binary collation. The equality check should keep these different byte sequences separate, just as DISTINCT does.

- **Impact:** Queries using the utf8mb4_0900_bin collation can treat different values as equal, causing filters, joins, or lookups to return incorrect results. This can silently affect applications that rely on byte-sensitive comparisons.
- **Steps to reproduce:**
  1. Open a SQL session against the local MySQL-compatible endpoint.
  2. Compare _utf8mb4'ß' COLLATE utf8mb4_0900_bin with _utf8mb4'ss' COLLATE utf8mb4_0900_bin.
  3. Observe that the equality expression returns 1, although the binary collation should return 0.
  4. Run the matching DISTINCT query and observe that it returns a count of 2.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The local SQL check returned equality_result=1 for ß versus ss under utf8mb4_0900_bin, while the equivalent UNION ALL with COUNT(DISTINCT value) returned 2. In the PR-added sql/expansion.go, unicodeExpander maps the rune 'ß' to the two-rune sequence {'s', 's'} at lines 23-27. The init loop at lines 75-83 assigns that expander to every collation whose name contains '_0900_', which includes utf8mb4_0900_bin. The normal StringType.Compare path in sql/types/strings.go:311-355 constructs WeightScanners and compares their emitted weights; sql/weight_scanner.go:74-81 applies the configured expander before returning each rune weight. Therefore the binary collation's ß input is converted into the same comparison-weight sequence as ss. The separate binary fast path in sql/collations.go:873-879 only protects Collation_binary hashing and does not prevent utf8mb4_0900_bin from receiving the expansion in comparison. The targeted fix is to leave utf8mb4_0900_bin without an expander, or otherwise route that specific collation through raw byte-sensitive comparison, while retaining expansion for the intended linguistic UCA collations.
- **Why this is likely a bug:** This is a production-code defect, not a browser or SQL setup problem. The SQL session reached the local engine and produced the specific wrong equality result, and the source provides a direct explanation: the new dispatch treats the binary 0900 collation as an expansion collation, so a character that must remain byte-distinct is rewritten to the weights for ss before comparison. The inconsistent DISTINCT count is an additional sign that equality semantics are wrong rather than that the two values are legitimately equivalent. Any query using utf8mb4_0900_bin equality can be affected, including predicates, joins, and indexed lookups. Removing the expander assignment for this binary collation is a small, focused correction and does not require changing the intended Unicode expansion mappings for linguistic collations.

**Relevant code:**

`sql/expansion.go:21-28`

~~~go
func unicodeExpander(r rune) []rune {
	switch r {
	case 'ß':
		return []rune{'s', 's'}
~~~

`sql/expansion.go:75-83`

~~~go
for id, collation := range collationArray {
		if strings.Contains(collation.Name, "_0900_") {
			collationExpanders[id] = unicodeExpander
		}
~~~

`sql/weight_scanner.go:74-81`

~~~go
if ws.expander != nil {
		if exp := ws.expander(r); len(exp) > 0 {
			ws.expanded = exp
			ws.expandedIdx = 1
			return ws.getRuneWeight(exp[0]), true, nil
		}
~~~

`sql/types/strings.go:311-312`

~~~go
iterA := sql.NewWeightScanner(t.collation, as)
	iterB := sql.NewWeightScanner(t.collation, bs)
~~~

Comment thread sql/expansion.go

// unicodeExpander expands runes for standard Unicode Collation Algorithm
// (UCA) collations (such as utf8mb4_unicode_ci and 0900 collations).
func unicodeExpander(r rune) []rune {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

Medium severity Uppercase ligature loses its case

What failed: The database says uppercase Œ is equal to lowercase oe. A case-sensitive comparison should not erase that case difference.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Case-sensitive searches can treat uppercase Œ and lowercase oe as the same value. This can return incorrect matches for data using the affected collation and character pair.
  • Steps to Reproduce:
    1. Open a SQL session against the local MySQL-compatible endpoint.
    2. Compare _utf8mb4'Œ' COLLATE utf8mb4_0900_as_cs with _utf8mb4'oe' COLLATE utf8mb4_0900_as_cs.
    3. Observe that the comparison returns 1, even though the case-sensitive collation should return 0.
    4. Compare uppercase Œ with uppercase OE under the same collation and observe that it returns 0.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PR adds sql/expansion.go. In unicodeExpander, lines 27-28 put both lowercase œ and uppercase Œ in the same switch case and return the lowercase rune sequence {'o', 'e'} for both. The init loop at lines 75-83 assigns unicodeExpander to every collation whose name contains '0900', which includes utf8mb4_0900_as_cs. String comparison obtains weights through sql/weight_scanner.go:74-81, so the uppercase source rune is replaced by lowercase expansion weights before the case-sensitive sorter can distinguish it. The local SQL check returned 1 for Œ versus oe and 0 for Œ versus OE, matching this path. The targeted fix is to make the expansion selected for case-sensitive collations retain the uppercase weights, rather than mapping Œ to lowercase oe.
  • Why this is likely a bug: This is a production-code defect rather than a test setup issue. The local SQL result demonstrates the wrong equality in the normal query path, and the source shows why: the new PR mapping converts Œ into lowercase oe before utf8mb4_0900_as_cs applies its case-sensitive weights. The error affects any comparison using this collation, not just the recorded pair. A focused expansion change that preserves uppercase output for case-sensitive collations addresses the failing behavior without changing the intended case-insensitive expansion behavior.
Relevant code

sql/expansion.go:23-28

func unicodeExpander(r rune) []rune {
	switch r {
	case 'œ', 'Œ':
		return []rune{'o', 'e'}

sql/expansion.go:75-83

for id, collation := range collationArray {
		if strings.Contains(collation.Name, "_0900_") {
			collationExpanders[id] = unicodeExpander
		}

sql/weight_scanner.go:74-81

if ws.expander != nil {
		if exp := ws.expander(r); len(exp) > 0 {
			ws.expanded = exp
			ws.expandedIdx = 1
			return ws.getRuneWeight(exp[0]), true, nil
		}
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Uppercase ligature loses its case**

**What failed:** The database says uppercase Œ is equal to lowercase oe. A case-sensitive comparison should not erase that case difference.

- **Impact:** Case-sensitive searches can treat uppercase Œ and lowercase oe as the same value. This can return incorrect matches for data using the affected collation and character pair.
- **Steps to reproduce:**
  1. Open a SQL session against the local MySQL-compatible endpoint.
  2. Compare _utf8mb4'Œ' COLLATE utf8mb4_0900_as_cs with _utf8mb4'oe' COLLATE utf8mb4_0900_as_cs.
  3. Observe that the comparison returns 1, even though the case-sensitive collation should return 0.
  4. Compare uppercase Œ with uppercase OE under the same collation and observe that it returns 0.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR adds sql/expansion.go. In unicodeExpander, lines 27-28 put both lowercase œ and uppercase Œ in the same switch case and return the lowercase rune sequence {'o', 'e'} for both. The init loop at lines 75-83 assigns unicodeExpander to every collation whose name contains '_0900_', which includes utf8mb4_0900_as_cs. String comparison obtains weights through sql/weight_scanner.go:74-81, so the uppercase source rune is replaced by lowercase expansion weights before the case-sensitive sorter can distinguish it. The local SQL check returned 1 for Œ versus oe and 0 for Œ versus OE, matching this path. The targeted fix is to make the expansion selected for case-sensitive collations retain the uppercase weights, rather than mapping Œ to lowercase oe.
- **Why this is likely a bug:** This is a production-code defect rather than a test setup issue. The local SQL result demonstrates the wrong equality in the normal query path, and the source shows why: the new PR mapping converts Œ into lowercase oe before utf8mb4_0900_as_cs applies its case-sensitive weights. The error affects any comparison using this collation, not just the recorded pair. A focused expansion change that preserves uppercase output for case-sensitive collations addresses the failing behavior without changing the intended case-insensitive expansion behavior.

**Relevant code:**

`sql/expansion.go:23-28`

~~~go
func unicodeExpander(r rune) []rune {
	switch r {
	case 'œ', 'Œ':
		return []rune{'o', 'e'}
~~~

`sql/expansion.go:75-83`

~~~go
for id, collation := range collationArray {
		if strings.Contains(collation.Name, "_0900_") {
			collationExpanders[id] = unicodeExpander
		}
~~~

`sql/weight_scanner.go:74-81`

~~~go
if ws.expander != nil {
		if exp := ws.expander(r); len(exp) > 0 {
			ws.expanded = exp
			ws.expandedIdx = 1
			return ws.getRuneWeight(exp[0]), true, nil
		}
~~~

Comment thread sql/types/strings.go
}
aWeight := getRuneWeight(ar)
bWeight := getRuneWeight(br)
if as == bs {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

Medium severity Malformed text is accepted as equal

What failed: The database accepts two identical strings with invalid text and reports them as equal. It should reject the invalid text with a malformed-string error.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Database comparisons can silently report malformed text as equal when both values contain the same invalid UTF-8 bytes. This can produce incorrect results for queries or checks that receive malformed text.
  • Steps to Reproduce:
    1. Create a StringType using the utf8mb4_unicode_ci collation.
    2. Create two identical strings containing an invalid UTF-8 byte, such as 0xff.
    3. Call the string comparison with both invalid strings.
    4. Observe that it returns equality with no error instead of returning the malformed-string error.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The focused test returned cmp=0 and err=nil for two identical strings containing 0xff under utf8mb4_unicode_ci. In sql/types/strings.go:307-309, StringType.Compare returns equality as soon as the converted byte strings are identical. That return happens before sql.NewWeightScanner is called at lines 311-312. The new scanner in sql/weight_scanner.go:64-70 is the path that decodes input and returns ErrCollationMalformedString when the encoder reports utf8.RuneError or consumes zero bytes, so the early return prevents both operands from being validated. The PR diff explicitly adds the early-return lines in sql/types/strings.go and replaces the old comparison loop with scanner calls, making the changed fast path the direct cause. A targeted fix is to remove the early return or run scanner validation before returning equality; no broader comparison rewrite is required.
  • Why this is likely a bug: This is a production-code defect, not a test setup problem. The focused repository test exercised the real StringType.Compare implementation and observed equality with no error for identical malformed bytes. The source confirms that the newly added fast path skips the scanner's explicit malformed-byte check, while the test plan requires invalid encoded input to be rejected even when both operands are identical. Any caller that compares identical malformed strings can receive a false successful equality result. Validating the operands before the fast return preserves the optimization's intent without changing the normal expansion comparison behavior.
Relevant code

sql/types/strings.go:307-312

if as == bs {
		return 0, nil
	}

	iterA := sql.NewWeightScanner(t.collation, as)
	iterB := sql.NewWeightScanner(t.collation, bs)

sql/weight_scanner.go:64-70

if ws.encoder != nil {
		r, read = ws.encoder.NextRune(ws.str)
	} else {
		r, read = utf8.DecodeRuneInString(ws.str)
	}
	if read == 0 || read == utf8.RuneError {
		return 0, false, ErrCollationMalformedString.New("scanning")
	}
Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Malformed text is accepted as equal**

**What failed:** The database accepts two identical strings with invalid text and reports them as equal. It should reject the invalid text with a malformed-string error.

- **Impact:** Database comparisons can silently report malformed text as equal when both values contain the same invalid UTF-8 bytes. This can produce incorrect results for queries or checks that receive malformed text.
- **Steps to reproduce:**
  1. Create a StringType using the utf8mb4_unicode_ci collation.
  2. Create two identical strings containing an invalid UTF-8 byte, such as 0xff.
  3. Call the string comparison with both invalid strings.
  4. Observe that it returns equality with no error instead of returning the malformed-string error.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The focused test returned cmp=0 and err=nil for two identical strings containing 0xff under utf8mb4_unicode_ci. In sql/types/strings.go:307-309, StringType.Compare returns equality as soon as the converted byte strings are identical. That return happens before sql.NewWeightScanner is called at lines 311-312. The new scanner in sql/weight_scanner.go:64-70 is the path that decodes input and returns ErrCollationMalformedString when the encoder reports utf8.RuneError or consumes zero bytes, so the early return prevents both operands from being validated. The PR diff explicitly adds the early-return lines in sql/types/strings.go and replaces the old comparison loop with scanner calls, making the changed fast path the direct cause. A targeted fix is to remove the early return or run scanner validation before returning equality; no broader comparison rewrite is required.
- **Why this is likely a bug:** This is a production-code defect, not a test setup problem. The focused repository test exercised the real StringType.Compare implementation and observed equality with no error for identical malformed bytes. The source confirms that the newly added fast path skips the scanner's explicit malformed-byte check, while the test plan requires invalid encoded input to be rejected even when both operands are identical. Any caller that compares identical malformed strings can receive a false successful equality result. Validating the operands before the fast return preserves the optimization's intent without changing the normal expansion comparison behavior.

**Relevant code:**

`sql/types/strings.go:307-312`

~~~go
if as == bs {
		return 0, nil
	}

	iterA := sql.NewWeightScanner(t.collation, as)
	iterB := sql.NewWeightScanner(t.collation, bs)
~~~

`sql/weight_scanner.go:64-70`

~~~go
if ws.encoder != nil {
		r, read = ws.encoder.NextRune(ws.str)
	} else {
		r, read = utf8.DecodeRuneInString(ws.str)
	}
	if read == 0 || read == utf8.RuneError {
		return 0, false, ErrCollationMalformedString.New("scanning")
	}
~~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dolt: explicit Unicode-collation RANGE peers are split

1 participant