dolthub/go-mysql-server

Panic when executing sql statements from external command line tool after startup

cold-runner opened this issue · 7 comments

cold-runner commented 
package main

import (
	sqle "github.com/dolthub/go-mysql-server"
	"github.com/dolthub/go-mysql-server/memory"
	"github.com/dolthub/go-mysql-server/server"
)

func main() {
	engine := sqle.NewDefault(memory.NewDBProvider())
	config := server.Config{
		Protocol: "tcp",
		Address:  "127.0.0.1:4408",
	}
	s, err := server.NewDefaultServer(config, engine)

	if err != nil {
		panic(err)
	}
	if err := s.Start(); err != nil {
		panic(err)
	}
}

When I start the server and connect through the mycli client tool and do the create table statement
2024-02-24_16-51
The terminal log is as follows

ERRO[0064] mysql_server caught panic:
interface conversion: sql.Session is *sql.BaseSession, not *memory.Session
/home/huaiyu/environment/go/1.22.0/src/runtime/iface.go:262 (0x410ab0)
        panicdottypeE: panic(&TypeAssertionError{iface, have, want, ""})
/home/huaiyu/environment/go/1.22.0/src/runtime/iface.go:272 (0x410a69)
        panicdottypeI: panicdottypeE(t, want, iface)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/memory/session.go:50 (0xcbd9bd)
        com/dolthub/go-mysql-server/memory.SessionFromContext: return ctx.Session.(*Session)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/memory/database.go:251 (0xcbd91b)
        com/dolthub/go-mysql-server/memory.(*BaseDatabase).CreateTable: sess := SessionFromContext(ctx)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/ddl.go:905 (0xf64bd3)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildCreateTable: err = creatable.CreateTable(ctx, n.Name(), n.CreateSchema, n.Collation, n.Comment)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:82 (0xf8cc2c)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExecNoAnalyze: return b.buildCreateTable(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:27 (0xf89d2e)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExec: iter, err := b.buildNodeExecNoAnalyze(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/builder.go:33 (0xfbb896)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).Build: return b.buildNodeExec(ctx, n, r)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/transaction.go:305 (0xfbb881)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildTransactionCommittingNode: iter, err := b.Build(ctx, n.Child(), row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:48 (0xf8a239)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExecNoAnalyze: return b.buildTransactionCommittingNode(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:27 (0xf89d2e)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExec: iter, err := b.buildNodeExecNoAnalyze(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/builder.go:33 (0xf8ff5e)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).Build: return b.buildNodeExec(ctx, n, r)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/other.go:326 (0xf8ff49)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildQueryProcess: iter, err := b.Build(ctx, n.Child(), row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:58 (0xf8d1a4)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExecNoAnalyze: return b.buildQueryProcess(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/node_builder.gen.go:27 (0xf89d2e)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).buildNodeExec: iter, err := b.buildNodeExecNoAnalyze(ctx, n, row)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/sql/rowexec/builder.go:33 (0xf5cd44)
        com/dolthub/go-mysql-server/sql/rowexec.(*BaseBuilder).Build: return b.buildNodeExec(ctx, n, r)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/engine.go:402 (0x1112161)
        com/dolthub/go-mysql-server.(*Engine).QueryWithBindings: iter, err := e.Analyzer.ExecBuilder.Build(ctx, analyzed, nil)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/server/handler.go:834 (0x1156f0a)
        com/dolthub/go-mysql-server/server.(*Handler).executeQuery: return h.e.QueryWithBindings(ctx, query, parsed, bindings)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/server/handler.go:367 (0x1151ca5)
        com/dolthub/go-mysql-server/server.(*Handler).doQuery: schema, rowIter, err := queryExec(ctx, query, parsed, analyzedPlan, bindings)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/server/handler.go:565 (0x1153b04)
        com/dolthub/go-mysql-server/server.(*Handler).errorWrappedDoQuery: remainder, err := h.doQuery(c, query, parsed, nil, mode, h.executeQuery, bindings, callback)
/home/huaiyu/go/pkg/mod/github.com/dolthub/go-mysql-server@v0.18.0/server/handler.go:284 (0x1151365)
        com/dolthub/go-mysql-server/server.(*Handler).ComQuery: _, err := h.errorWrappedDoQuery(c, query, nil, MultiStmtModeOff, nil, callback)
/home/huaiyu/go/pkg/mod/github.com/dolthub/vitess@v0.0.0-20240228192915-d55088cef56a/go/mysql/conn.go:1462 (0x9279f1)
        com/dolthub/vitess/go/mysql.(*Conn).execQuery: err = handler.ComQuery(c, query, resultsCB)
/home/huaiyu/go/pkg/mod/github.com/dolthub/vitess@v0.0.0-20240228192915-d55088cef56a/go/mysql/conn.go:972 (0x922f24)
        com/dolthub/vitess/go/mysql.(*Conn).handleNextCommand: for query, err = c.execQuery(query, handler, multiStatements); err == nil && query != ""; {
/home/huaiyu/go/pkg/mod/github.com/dolthub/vitess@v0.0.0-20240228192915-d55088cef56a/go/mysql/server.go:526 (0x93d1ca)
        com/dolthub/vitess/go/mysql.(*Listener).handle: err := c.handleNextCommand(l.handler)
/home/huaiyu/environment/go/1.22.0/src/runtime/asm_amd64.s:1695 (0x473020)
        goexit: BYTE    $0x90   // NOP 
INFO[0064] NewConnection                                 DisableClientMultiStatements=false connectionID=5
WARN[0064] error running query                           connectTime="2024-03-04 01:40:16.527164195 +0800 CST m=+64.402182622" connectionDb=new_db connectionID=5 error="table with name first_table already exists"
dvilaverde commented

I'm seeing the same. For some reason I can no longer use s, err := server.NewDefaultServer(config, engine) and need to use server.NewServer, specifically to set the SessionBuilder in order to avoid using the server.DefaultSessionBuilder.

Here is an example that works:

pro := sql.NewDatabaseProvider(dbs...)
	engine := sqle.NewDefault(
		pro,
	)

config := server.Config{
		Protocol: "tcp",
		Address:  fmt.Sprintf("localhost:%d", port),
	}

s, err := server.NewServer(config, engine, func(ctx context.Context, conn *mysql.Conn, addr string) (sql.Session, error) {
		builder, err := server.DefaultSessionBuilder(ctx, conn, addr)
		if err != nil {
			return nil, err
		}
		return memory.NewSession(builder.(*sql.BaseSession), pro), nil
	}, nil)
fulghum commented

@cold-runner and @dvilaverde – thank you for taking the time to report this one to us. 🙏 The most recent release of go-mysql-server contains several correctness fixes to the memory database implementation and also includes support for the in-memory implementation to integrate with the table rewrite hook in the engine. As part of that work, the way to instantiate an in-memory server changed so that the session provider function must now be specified.

As part of that work, it looks like we updated the example in the _example folder, but we didn't update the main project README. Apologies that we missed that! I've just fixed the main project README with the correct, updated code for instantiating an in-memory test server.

I'll go ahead and resolve this issue, since these changes were intended and the README was out of date, but please feel free to comment if you have any problems moving to the new instantiation code and we'll be more than happy to help. Apologies for the snag and thank you for letting us know the README was out of sync with the latest release.

twhiteman commented

Can the NewDefaultServer method be updated to create a valid session builder!? As right now it results in the above panic, otherwise, what is the purpose of NewDefaultServer anymore?

fulghum commented

Good question about NewDefaultServer. It looks like the default session builder that is used in NewDefaultServer now uses BaseSession, but code in the in-memory engine requires memory.Session. We might be able to change the in-memory engine to work with that default session builder somehow, but I haven't looked at it deeply.

Tagging @zachmu for his opinion here, since he's got some context form being on the commits that changed the sample code to this new interface.

zachmu commented

Thanks for the feedback. The NewDefaultServer() method is pretty useless now, because the memory implementation now requires its own Session type for the server to work correctly. I'll do a pass on this and either fix or remove NewDefaultServer.

zachmu commented

The tip of main fixes this issue, but I'll leave this open until we do a patch release.