Skip to content

INSTR: case-sensitive for nonbinary strings (should fold), and StringWrapper substring clobbers haystack (returns 1) #3650

Description

@LaPingvino

Two defects in Instr (sql/expression/function/substring.go), found while investigating #3649 (LOCATE). Unlike LOCATE, INSTR already counts positions in characters correctly — these are separate divergences:

1. INSTR is case-sensitive for nonbinary strings (MySQL: insensitive)

MySQL 8.4 manual (INSTR): "This function is multibyte safe, and is case-sensitive only if at least one argument is a binary string."

SELECT INSTR('xyza','A');
-- MySQL / MariaDB 12.2: 4
-- Dolt 2.2.2:           0

findSubsequence compares runes exactly (text[i+j] != subtext[j]) with no case folding.

2. Copy-paste bug: substring's StringWrapper branch clobbers the haystack

In Instr.Eval, the sql.StringWrapper case of the substring argument assigns to the wrong variable:

var subtext []rune
switch substr := substr.(type) {
...
case sql.StringWrapper:
    s, err := substr.Unwrap(ctx)
    if err != nil {
        return nil, err
    }
    text = []rune(s)      // <-- should be subtext

When the substring arrives wrapped (e.g. Dolt's out-of-band TextStorage values), subtext stays nil/empty and text is overwritten with the needle — findSubsequence(text, subtext) with an empty needle returns 0, so INSTR returns 1 unconditionally for any wrapped substring argument.

(While fixing, the two functions could share one rune-based, case-folding-aware search core — see the fix-shape discussion in #3649.)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions