blob enginetests #1061
Merged
blob enginetests #1061
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a small set of blob enginetests. Binary types are passed as byte arrays. Prevent binary primary keys. Add filter for binary indexes ensuring an index prefix less between 0 and 1000 is set or implicit from table schema. Need more blob index tests. Need to split binary types into their own format, probably when collations are added (all binary types have fixed binary collation).
max-hoffman
force-pushed
the
max/blob-enginetests
branch
from
4a7916b
to
92c5d78
Compare
zachmu
approved these changes
Jun 21, 2022
| @@ -86,8 +86,6 @@ var ForeignKeyTests = []ScriptTest{ | |||
| SetUpScript: []string{ | |||
| "CREATE TABLE parent1 (pk BIGINT PRIMARY KEY, v1 CHAR(20), INDEX (v1));", | |||
| "CREATE TABLE parent2 (pk BIGINT PRIMARY KEY, v1 VARCHAR(20), INDEX (v1));", | |||
| "CREATE TABLE parent3 (pk BIGINT PRIMARY KEY, v1 BINARY(20), INDEX (v1));", | |||
There was a problem hiding this comment.
Why are these deleted? Binary and carbinary are valid index types
| "", // ssl_cipher | ||
| "", // x509_issuer | ||
| "", // x509_subject | ||
| []byte(""), // ssl_cipher |
There was a problem hiding this comment.
Some places you're using uint8 and others byte. Standardize on byte everywhere you're doing this
| func validateIndexType(cols []sql.IndexColumn, sch sql.Schema) error { | ||
| for _, c := range cols { | ||
| i := sch.IndexOfColName(c.Name) | ||
| if sql.IsBlob(sch[i].Type) { |
There was a problem hiding this comment.
This isn't matching the right types and your check is too restrictive as a result
func IsBlob(t Type) bool {
switch t.Type() {
case sqltypes.Binary, sqltypes.VarBinary, sqltypes.Blob:
return true
default:
return false
}
}You'll need to see what other places this is used, but I'm guessing it should be really be called IsByteType or similar
| return sql.ErrUnknownIndexColumn.New(idxCol.Name, idx.IndexName) | ||
| } | ||
|
|
||
| if sql.IsBlob(col.Type) { |
| // validatePkTypes prevents creating tables with blob primary keys | ||
| func validatePkTypes(tableSpec *plan.TableSpec) error { | ||
| for _, col := range tableSpec.Schema.Schema { | ||
| if col.PrimaryKey && sql.IsBlob(col.Type) { |
sql/errors.go
Outdated
| @@ -576,6 +576,15 @@ var ( | |||
|
|
|||
| // ErrSpatialTypeConversion is returned when one spatial type cannot be converted to the other spatial type | |||
| ErrSpatialTypeConversion = errors.NewKind("Cannot get geometry object from data you send to the GEOMETRY field") | |||
|
|
|||
| // ErrInvalidBinaryPrimaryKey is returned when attempting to create a primary key with a binary column | |||
| ErrInvalidBinaryPrimaryKey = errors.NewKind("invalid primary key on binary column '%s'") | |||
There was a problem hiding this comment.
Message / name needs adjusting
Binary columns are fine, it's specifically blob / text types that can't be used in keys
This was referenced Aug 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.