Strip only the enclosing quotes when parsing visibility query string values

[Dev-next-gen]

What changed?

sqlquery.ParseValue now unquotes string literals with ExtractStringValue, which drops exactly the enclosing pair of single quotes. It used strings.Trim(sqlValue, "'"), which drops every leading and trailing quote.

Why?

I was reading the visibility query converter and noticed that parseSQLVal (in the unified converter and in the legacy SQL one) rebuilds the literal as '%s' from expr.Val and hands it to ParseValue. Because of the Trim, a value that itself starts or ends with a quote loses it. Keyword01 = '''foo''' is a query for the value 'foo', but every store ends up filtering on foo:

TestSQLQueryConverter/{mysql8,postgres12,sqlite}
    expected: "TemporalNamespaceDivision is null and Keyword01 = '''foo'''"
    actual  : "TemporalNamespaceDivision is null and Keyword01 = 'foo'"
TestElasticsearchQueryConverter
    expected: ..."term":{"Keyword01":"'foo'"}...
    actual  : ..."term":{"Keyword01":"foo"}...

In the same way, WorkflowId = 'abc''' matches abc instead of abc', so the query returns executions the caller didn't ask for and misses the one it did. Quotes in the middle of a value ('foo''s bar') were already handled correctly, which is why the existing test case didn't catch this.

How did you test it?

• built
• run locally and tested manually
• covered by existing tests
• added new unit test(s)
• added new functional test(s)

I added three cases to TestConvertSqlValue and one to the shared queryConverterTestCases, which runs against the MySQL, PostgreSQL, SQLite and Elasticsearch converters (the legacy converter tests pick it up as well). Without the fix, all four stores fail as shown above. With it, go test -tags test_dep ./common/sqlquery/ ./common/persistence/visibility/... ./common/archiver/... ./service/history/workflow/matcher/... passes.

Potential risks

If ParseValue gets a lone ', it used to return an empty string and now rejects it as an unparsable value. None of the callers can produce that input: the converters always wrap expr.Val in a pair of quotes, and sqlparser.String never emits a bare quote.

The legacy Elasticsearch converter (used only when system.visibilityEnableUnifiedQueryConverter is off) goes through sqlparser.String, which escapes quotes MySQL-style, so 'foo''s bar' comes out as foo\'s bar there. That's the same before and after this change, and I didn't touch it.

AI tools used

[CLAassistant]

All committers have signed the CLA.