[release-19.0-github] Improve UNION query merging

[arthurschreiber]

Description

This is a backport of vitessio#18289.

Related Issue(s)

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[Copilot]

Pull Request Overview

Backport of UNION query merging improvements to release-19.0, enabling more aggressive merging of UNION and UNION ALL on sharded keyspaces and deduplicating IN-list bind variables.

• Added new test cases covering IN-list, mixed IN/=, and repeated IN scenarios in select_cases.json and normalizer_test.go
• Extended tryMergeUnionShardedRouting in union_merging.go to merge on EqualUnique, Equal, and IN op codes
• Enhanced normalizer (normalizer.go) to cache tuple bind-vars (tupleVals) and updated route planning (route_planning.go) to compare ValTuple and ListArg
• Updated executor tests (executor_select_test.go) to expect merged UNION ALL pushdown

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
select_cases.json	Added test cases for UNION merging with IN lists and mixed filters
union_merging.go	Expanded merge criteria to include EqualUnique, Equal, and IN
route_planning.go	Added support for ValTuple and ListArg equality in gen4ValEqual
executor_select_test.go	Updated expected queries for merged UNION ALL scenarios
normalizer_test.go	Added test for repeated IN clause bind variable deduplication
normalizer.go	Implemented caching of tuple bind variables to avoid duplicate binds

Comments suppressed due to low confidence (2)

go/vt/sqlparser/normalizer.go:323

• This := shadows the outer bvname, so after this block bvname may remain empty. Use a new var (e.g. existing, found := nz.tupleVals[string(key)]) and then assign bvname = existing to avoid shadowing.

if bvname, ok := nz.tupleVals[string(key)]; !ok {

go/vt/sqlparser/normalizer.go:50

• [nitpick] The name tupleVals is a bit vague; consider renaming to something like tupleBindNameCache to clarify that it maps serialized tuple keys to bind-variable names.

tupleVals map[string]string

[Copilot]

[nitpick] The repeated direct comparisons with fallthrough could be refactored into a helper (e.g. isMergeableOp(op)) or a slice lookup to improve readability and reduce duplication.

[Copilot]

[nitpick] Consider combining the ValTuple and ListArg cases into a single case sqlparser.ValTuple, sqlparser.ListArg: block to reduce duplication and centralize the tuple/list comparison logic.

Suggested change

	case sqlparser.ValTuple:
	if b, ok := b.(sqlparser.ValTuple); ok {
	return gen4ValuesEqual(ctx, a, b)
	}
	return false
	case sqlparser.ListArg:
	if b, ok := b.(sqlparser.ListArg); ok {
	return a == b
	}
	case sqlparser.ValTuple, sqlparser.ListArg:
	if b, ok := b.(type); ok && a == b {
	switch a := a.(type) {
	case sqlparser.ValTuple:
	return gen4ValuesEqual(ctx, a, b)
	case sqlparser.ListArg:
	return a == b
	}
	}
	return false

[arthurschreiber]

This suggestion makes no sense and is not valid Go code. b.(type) can not be used outside of a switch statement.