Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cold-runner opened this issue Mar 3, 2024 · 7 comments · Fixed by #2390
Closed
Assignees

Comments

@cold-runner
Copy link

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
Copy link

dvilaverde commented Mar 4, 2024

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
Copy link
Contributor

fulghum commented Mar 4, 2024

@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
Copy link

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
Copy link
Contributor

fulghum commented Mar 11, 2024

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.

@fulghum fulghum reopened this Mar 11, 2024
@zachmu
Copy link
Member

zachmu commented Mar 11, 2024

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
Copy link
Member

zachmu commented Mar 13, 2024

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

@zachmu
Copy link
Member

zachmu commented Apr 9, 2024

This is in the latest release:

https://github.com/dolthub/go-mysql-server/releases/tag/v0.18.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants