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

[cold-runner]

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

[dvilaverde]

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]

@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]

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]

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]

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]

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