Bug Report: unexpected error text type with an unknown/unsupported collation cannot be hashed

[Jolg42]

Overview of the Issue

I found out that the newest image version of vitess/vttestserver:mysql80 (same for mysql57) started to fail some tests in prisma/prisma#20608

From what I understand, this is what comes out from the main branch, labeled as v18.0 (dev)

This is unexpected because only the docker image version changed, and the tests started to fail with

text type with an unknown/unsupported collation cannot be hashed

It looks like it might be related to the use of BINARY in the SQL (see reproduction repo below)

Originally posted on a #shared-planetscale Slack Channel

Reproduction Steps

See minimal reproduction repository
https://github.com/Jolg42/repro-prisma-vitess-error

Binary Version

See docker image
`mysql` says `Server version: 8.0.21-vitess Version: 18.0.0-SNAPSHOT (Git revision ee7b9e7fbaef309d5cb39991252d88418092ccd9 branch 'main') built on Tue Aug  8 17:13:58 UTC 2023 by vitess@buildkitsandbox using go1.20.5 linux/amd64`

Operating System and Environment details

See docker image.
Could be reproduced locally on macOS and remotely on Linux/Ubuntu with GitHub Actions

Log Fragments

No response

[dbussink]

It looks like it might be related to the use of BINARY in the SQL (see reproduction repo below)

This is indeed part of the trigger. Fwiw, the usage of BINARY makes the queries also less efficient, I have no idea why Prisma decides to add these as they are not really useful to from the surface.

The core of the issue here is actually due to the usage of prepared statements. In general in MySQL prepared statements are not super useful and also with Vitess their usage is very limited. They only provide marginal benefits if you re-run the same query very often.

For schema inspection queries like these, doing client side interpolation is almost always much faster and reduces the number of roundtrips significantly. This is I think also something that Prisma could consider to change as well.

Nonetheless, there is really a bug here with prepared statements. I've reduced the reproduction repository to the following Go script:

package main

import (
	"database/sql"
	"time"

	_ "github.com/go-sql-driver/mysql"
)

// ...

func main() {
	db, err := sql.Open("mysql", "root@tcp(127.0.0.1:15306)/commerce?interpolateParams=false")
	if err != nil {
		panic(err)
	}
	// See "Important settings" section.
	db.SetConnMaxLifetime(time.Minute * 3)
	db.SetMaxOpenConns(10)
	db.SetMaxIdleConns(10)
	_, err = db.Query(`SELECT DISTINCT
                BINARY table_info.table_name AS table_name,
                table_info.create_options AS create_options,
                table_info.table_comment AS table_comment
              FROM information_schema.tables AS table_info
              JOIN information_schema.columns AS column_info
                  ON BINARY column_info.table_name = BINARY table_info.table_name
              WHERE
                  table_info.table_schema = ?
                  AND column_info.table_schema = ?
                  -- Exclude views.
                  AND table_info.table_type = 'BASE TABLE'
              ORDER BY BINARY table_info.table_name`, "commerce", "commerce")
	if err != nil {
		panic(err)
	}
}

When running this against the examples/local/101_initial_cluster.sh setup locally it fails with the same error:

go run .
panic: Error 1815 (HY000): text type with an unknown/unsupported collation cannot be hashed

goroutine 1 [running]:
main.main()
	/Users/dirkjan/tmp/gotest/main.go:35 +0x105
exit status 2

@vitessio/query-serving This is probably something in the wheelhouse of you all. The reason that it seems to only trigger with prepared statements, is that somehow if we don't use those, we end up sending it straight to MySQL and avoid the problem it seems, but if we prepare it we don't (which is beyond my understanding of the query planner).

When we plan it, the semantic analysis doesn't know the type of the BINARY table_info.table_name AS table_name expression which is needed for the Distinct node from the plan. We end up here:

vitess/go/vt/vtgate/engine/distinct.go

Lines 122 to 133 in a30b19e

	hashcode, err := evalengine.NullsafeHashcode(col, checkCol.Collation, col.Type())
	if err != nil {
	if err != evalengine.UnsupportedCollationHashError || checkCol.WsCol == nil {
	return 0, err
	}
	checkCol = checkCol.SwitchToWeightString()
	pt.checkCols[i] = checkCol
	hashcode, err = evalengine.NullsafeHashcode(inputRow[checkCol.Col], checkCol.Collation, col.Type())
	if err != nil {
	return 0, err
	}
	}

We have the collations.Unknown collation.ID here because of the missing semantic analysis for the type of the binary conversion function. That means we end up triggering the following logic:

https://github.com/vitessio/vitess/blob/main/go/vt/vtgate/evalengine/api_hash.go#L48-L50

We also don't have a weight string column available .WsCol, so we also can't fall back to that. As mentioned above, this whole path only gets triggered when using prepared statements.

[Jolg42]

@dbussink Note, I checked why we used BINARY and there is a reason, like pointed out by my colleague Tom: for the sort order (so the ordering matches between the database and application code)
I opened a PR to remove the cast, and it failed our tests.

[dbussink]

@dbussink Note, I checked why we used BINARY and there is a reason, like pointed out by my colleague Tom: for the sort order (so the ordering matches between the database and application code)
I opened a PR to remove the cast, and it failed our tests.

Ah ok, I'd probably say the right fix is then to change the app code, but that's not up to me really 😄.

The main problem performance wise is that changing the collation on the columns means that MySQL can't use the indexes to do the join, so it performs not very optimal. These are usually queries I guess for small data sizes so it might be as big of a problem in practice.

[tomhoule]

Aah yes, that's a good point that didn't jump to mind for me. The information schema tables should be of a manageable size, but I agree that could be a problem. The alternative solution would be re-sorting the result set on the rust side (or tweaking the comparator in our binary searches).

[Jolg42]

Note: I can confirm this is fixed in the latest version of vitess/vttestserver:mysql80. Thank you, @systay and all 🙌🏼

I spotted 2 PRs opened to backport this, adding them here for info:
#13864
#13863