dolthub / go-mysql-server

A MySQL-compatible relational database with a storage agnostic query engine. Implemented in pure Go.
Apache License 2.0
2.34k stars 205 forks source link

Primary key column order changes column order on insert #2397

Closed pdufour closed 7 months ago

pdufour commented 7 months ago

The order of keys in the primary key line seem to be messing with the order columns are inserted.

For example:

Table structure:

CREATE TABLE store (
    col_a binary(16) NOT NULL,
    col_b varchar(50) NOT NULL,
    PRIMARY KEY (col_b,col_a)
)

Insert:

_, err = db.Exec(
"INSERT INTO `store` (`col_a`, `col_b`) VALUES (?, ?)",
"",
"foobarfoobarfoobarfoobar",
)

This gives the error:

error="string 'foobarfoobarfoobarfoobar' is too large for column 'binary(16)'"

Swap the column order back in the primary key line:

CREATE TABLE store (
    col_a binary(16) NOT NULL,
    col_b varchar(50) NOT NULL,
    PRIMARY KEY (col_a,col_b)
)

Doesn't throw an error.

While you would expect column keys specified in the PK to be in the same column order as the original table, this is not a requirement of mysql.

Complete reproducible example:

package main

import (
    gosql "database/sql"
    "fmt"
    "time"

    sqle "github.com/dolthub/go-mysql-server"
    "github.com/dolthub/go-mysql-server/memory"
    "github.com/dolthub/go-mysql-server/server"
    _ "github.com/go-sql-driver/mysql"
)

var (
    dbName    = "mydb"
    tableName = "mytable"
    address   = "localhost"
    port      = 3309
)

func main() {
    fmt.Println("Hello, World!")
    pro := createTestDatabase()
    engine := sqle.NewDefault(pro)

    config := server.Config{
        Protocol: "tcp",
        Address:  fmt.Sprintf("%s:%d", address, port),
    }
    s, err := server.NewServer(config, engine, memory.NewSessionBuilder(pro), nil)
    if err != nil {
        panic(err)
    }
    defer s.Close()
    go func() {
        if err = s.Start(); err != nil {
            panic(err)
        }
    }()

    // Insert table
    time.Sleep(1 * time.Second)
    db, err := gosql.Open("mysql", "root:@tcp(127.0.0.1:3309)/mydb")
    if err != nil {
        panic(err)
    }

    _, err = db.Exec(`
        CREATE TABLE store (
            col_a binary(16) NOT NULL,
            col_b varchar(50) NOT NULL,
            PRIMARY KEY (col_b,col_a)
        )
    `)

    if err != nil {
        panic(err)
    }

    _, err = db.Exec(
        "INSERT INTO `store` (`col_a`, `col_b`) VALUES (?, ?)",
        "",
        "foobarfoobarfoobarfoobar",
    )

    if err != nil {
        panic(err)
    }
}

func createTestDatabase() *memory.DbProvider {
    db := memory.NewDatabase("mydb")
    db.BaseDatabase.EnablePrimaryKeyIndexes()

    pro := memory.NewDBProvider(db)

    return pro
}
jycor commented 7 months ago

Hey @pdufour, thanks for reporting this issue!

I was able to reproduce the bug, and can confirm that the presence of the PRIMARY KEY definition (specifically the order) is messing with how we validate the column type length restrictions. Interestingly, this doesn't seem to be an issue in dolt.

This is a ScriptTest repro for whoever is going to tackle this issue:

var scripts = []queries.ScriptTest{
    {
        Name: "tmp script test",
        SetUpScript: []string{
            "create table t (a varchar(5), b varchar(100), primary key(b, a));",
        },
        Assertions: []queries.ScriptTestAssertion{
            {
                Query: "insert into t (b, a) values ('123456', '')",
                Expected: []sql.Row{
                },
            },
            {
                Query: "select a, b from t",
                Expected: []sql.Row{
                },
            },
        },
    },
}

We will get started on the fix ASAP.