Skip to content
36 changes: 36 additions & 0 deletions enginetest/queries/variable_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"math"

"github.com/dolthub/go-mysql-server/sql"
"github.com/dolthub/go-mysql-server/sql/types"
)

var VariableQueries = []ScriptTest{
Expand Down Expand Up @@ -60,6 +61,41 @@ var VariableQueries = []ScriptTest{
{uint64(2)},
},
},
{
Name: "variable scope is included in returned column name when explicitly provided",
Assertions: []ScriptTestAssertion{
{
Query: "select @@max_allowed_packet;",
Expected: []sql.Row{{1073741824}},
ExpectedColumns: sql.Schema{
{
Name: "@@max_allowed_packet",
Type: types.Uint64,
},
},
},
{
Query: "select @@session.max_allowed_packet;",
Expected: []sql.Row{{1073741824}},
ExpectedColumns: sql.Schema{
{
Name: "@@SESSION.max_allowed_packet",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the difference in case going to be an issue? If so might need a deeper fix

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could mean we need to add an alias in the select exprs, similar to what we do with other expressions today.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not including the scope is enough to fix the immediate customer issue, but the casing difference was sticking out to me, too. Instead of returning a boolean value for whether the scope was explicitly specified or not, we could return the explicitly specified scope and then we'd have the original casing that we could preserve.

Type: types.Uint64,
},
},
},
{
Query: "select @@global.max_allowed_packet;",
Expected: []sql.Row{{1073741824}},
ExpectedColumns: sql.Schema{
{
Name: "@@GLOBAL.max_allowed_packet",
Type: types.Uint64,
},
},
},
},
},
{
Name: "@@server_id",
Assertions: []ScriptTestAssertion{
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e
github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
github.com/dolthub/vitess v0.0.0-20240117061527-f9260279b3d3
github.com/dolthub/vitess v0.0.0-20240117191503-43038f0e7332
github.com/go-kit/kit v0.10.0
github.com/go-sql-driver/mysql v1.7.2-0.20231213112541-0004702b931d
github.com/gocraft/dbr/v2 v2.7.2
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9X
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
github.com/dolthub/vitess v0.0.0-20240110233415-e46007d964c0 h1:P8wb4dR5krirPa0swEJbEObc/I7GaAM/01nOnuQrl0c=
github.com/dolthub/vitess v0.0.0-20240110233415-e46007d964c0/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dolthub/vitess v0.0.0-20240117061527-f9260279b3d3 h1:nEwq2/8gTI2jm/4APIMTrWNDDRCn8AWJjrCbH+d7CJc=
github.com/dolthub/vitess v0.0.0-20240117061527-f9260279b3d3/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dolthub/vitess v0.0.0-20240117190712-1b506b108f54 h1:AjTW9LRaRq12Pu1tt0YG+vjfDduA59O7DhtyiNgY0Yw=
github.com/dolthub/vitess v0.0.0-20240117190712-1b506b108f54/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dolthub/vitess v0.0.0-20240117191503-43038f0e7332 h1:lkY6/i/jFn70eQWfZaxMhjlMtcTZiLErCBAoKcySzCQ=
github.com/dolthub/vitess v0.0.0-20240117191503-43038f0e7332/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw=
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU=
Expand Down
15 changes: 15 additions & 0 deletions server/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,20 @@ func schemaToFields(ctx *sql.Context, s sql.Schema) []*querypb.Field {
charset = uint32(charSetResults)
}

var flags querypb.MySqlFlag
if !c.Nullable {
flags = flags | querypb.MySqlFlag_NOT_NULL_FLAG
}
if c.AutoIncrement {
flags = flags | querypb.MySqlFlag_AUTO_INCREMENT_FLAG
}
if c.PrimaryKey {
flags = flags | querypb.MySqlFlag_PRI_KEY_FLAG
}
if types.IsUnsigned(c.Type) {
flags = flags | querypb.MySqlFlag_UNSIGNED_FLAG
}

fields[i] = &querypb.Field{
Name: c.Name,
OrgName: c.Name,
Expand All @@ -727,6 +741,7 @@ func schemaToFields(ctx *sql.Context, s sql.Schema) []*querypb.Field {
Type: c.Type.Type(),
Charset: charset,
ColumnLength: c.Type.MaxTextResponseByteLength(ctx),
Flags: uint32(flags),
}

if types.IsDecimal(c.Type) {
Expand Down
13 changes: 10 additions & 3 deletions sql/expression/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@ type SystemVar struct {
Name string
Collation sql.CollationID
Scope sql.SystemVariableScope
Explicit bool
}

var _ sql.Expression = (*SystemVar)(nil)
var _ sql.CollationCoercible = (*SystemVar)(nil)

// NewSystemVar creates a new SystemVar expression.
func NewSystemVar(name string, scope sql.SystemVariableScope) *SystemVar {
return &SystemVar{name, sql.CollationID(0), scope}
// NewSystemVar creates a new SystemVar expression for the system variable named |name| with the specified |scope|.
// When |explicit| is true, it indicates that the variable scope was explicitly specified, and not inferred.
func NewSystemVar(name string, scope sql.SystemVariableScope, explicit bool) *SystemVar {
return &SystemVar{name, sql.CollationID(0), scope, explicit}
}

// Children implements the sql.Expression interface.
Expand Down Expand Up @@ -100,6 +102,11 @@ func (v *SystemVar) Resolved() bool { return true }

// String implements the sql.Expression interface.
func (v *SystemVar) String() string {
// If the scope wasn't explicitly provided, then don't include it in the string representation
if v.Explicit == false {
return fmt.Sprintf("@@%s", v.Name)
}

switch v.Scope {
case sql.SystemVariableScope_Session:
return fmt.Sprintf("@@SESSION.%s", v.Name)
Expand Down
34 changes: 18 additions & 16 deletions sql/planbuilder/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,27 +98,27 @@ func (b *Builder) setExprsToExpressions(inScope *scope, e ast.SetVarExprs) []sql
}
switch strings.ToLower(expr.String()) {
case "'isolation level repeatable read'":
varToSet := expression.NewSystemVar("transaction_isolation", scope)
varToSet := expression.NewSystemVar("transaction_isolation", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral("REPEATABLE-READ", types.LongText))
continue
case "'isolation level read committed'":
varToSet := expression.NewSystemVar("transaction_isolation", scope)
varToSet := expression.NewSystemVar("transaction_isolation", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral("READ-COMMITTED", types.LongText))
continue
case "'isolation level read uncommitted'":
varToSet := expression.NewSystemVar("transaction_isolation", scope)
varToSet := expression.NewSystemVar("transaction_isolation", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral("READ-UNCOMMITTED", types.LongText))
continue
case "'isolation level serializable'":
varToSet := expression.NewSystemVar("transaction_isolation", scope)
varToSet := expression.NewSystemVar("transaction_isolation", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral("SERIALIZABLE", types.LongText))
continue
case "'read write'":
varToSet := expression.NewSystemVar("transaction_read_only", scope)
varToSet := expression.NewSystemVar("transaction_read_only", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral(false, types.Boolean))
continue
case "'read only'":
varToSet := expression.NewSystemVar("transaction_read_only", scope)
varToSet := expression.NewSystemVar("transaction_read_only", scope, true)
res[i] = expression.NewSetField(varToSet, expression.NewLiteral(true, types.Boolean))
continue
}
Expand Down Expand Up @@ -168,10 +168,12 @@ func (b *Builder) buildSysVar(colName *ast.ColName, scopeHint ast.SetScope) (sql
var varName string
var scope ast.SetScope
var err error
if table != "" {
varName, scope, err = ast.VarScope(table, col)
var explicit bool

if table == "" {
varName, scope, explicit, err = ast.VarScope(col)
} else {
varName, scope, err = ast.VarScope(col)
varName, scope, explicit, err = ast.VarScope(table, col)
}
if err != nil {
b.handleErr(err)
Expand All @@ -187,11 +189,11 @@ func (b *Builder) buildSysVar(colName *ast.ColName, scopeHint ast.SetScope) (sql
if !ok {
return nil, scope, false
}
return expression.NewSystemVar(varName, sql.SystemVariableScope_Global), scope, true
return expression.NewSystemVar(varName, sql.SystemVariableScope_Global, explicit), scope, true
case ast.SetScope_None, ast.SetScope_Session:
switch strings.ToLower(varName) {
case "character_set_database", "collation_database":
sysVar := expression.NewSystemVar(varName, sql.SystemVariableScope_Session)
sysVar := expression.NewSystemVar(varName, sql.SystemVariableScope_Session, explicit)
sysVar.Collation = sql.Collation_Default
if db, err := b.cat.Database(b.ctx, b.ctx.GetCurrentDatabase()); err == nil {
sysVar.Collation = plan.GetDatabaseCollation(b.ctx, db)
Expand All @@ -202,7 +204,7 @@ func (b *Builder) buildSysVar(colName *ast.ColName, scopeHint ast.SetScope) (sql
if err != nil {
return nil, scope, false
}
return expression.NewSystemVar(varName, sql.SystemVariableScope_Session), scope, true
return expression.NewSystemVar(varName, sql.SystemVariableScope_Session, explicit), scope, true
}
case ast.SetScope_User:
t, _, err := b.ctx.GetUserVariable(b.ctx, varName)
Expand All @@ -214,9 +216,9 @@ func (b *Builder) buildSysVar(colName *ast.ColName, scopeHint ast.SetScope) (sql
}
return expression.NewUserVar(varName), scope, true
case ast.SetScope_Persist:
return expression.NewSystemVar(varName, sql.SystemVariableScope_Persist), scope, true
return expression.NewSystemVar(varName, sql.SystemVariableScope_Persist, explicit), scope, true
case ast.SetScope_PersistOnly:
return expression.NewSystemVar(varName, sql.SystemVariableScope_PersistOnly), scope, true
return expression.NewSystemVar(varName, sql.SystemVariableScope_PersistOnly, explicit), scope, true
default: // shouldn't happen
err := fmt.Errorf("unknown set scope %v", scope)
b.handleErr(err)
Expand Down Expand Up @@ -325,9 +327,9 @@ func (b *Builder) simplifySetExpr(name *ast.ColName, varScope ast.SetScope, val
table := name.Qualifier.String()
col := name.Name.Lowered()
if table != "" {
varName, _, err = ast.VarScope(table, col)
varName, _, _, err = ast.VarScope(table, col)
} else {
varName, _, err = ast.VarScope(col)
varName, _, _, err = ast.VarScope(col)
}
if err != nil {
b.handleErr(err)
Expand Down
2 changes: 1 addition & 1 deletion sql/rowexec/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestPersistedSessionSetIterator(t *testing.T) {
sqlCtx, globals := newPersistedSqlContext()
s := plan.NewSet(
[]sql.Expression{
expression.NewSetField(expression.NewSystemVar(test.name, test.scope), expression.NewLiteral(int64(test.value), types.Int64)),
expression.NewSetField(expression.NewSystemVar(test.name, test.scope, true), expression.NewLiteral(int64(test.value), types.Int64)),
},
)

Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_bool.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,8 @@ func (t SystemBoolType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t SystemBoolType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t SystemBoolType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
Comment thread
fulghum marked this conversation as resolved.
}

// Promote implements the Type interface.
Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_double.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,8 @@ func (t systemDoubleType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t systemDoubleType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t systemDoubleType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,8 @@ func (t systemEnumType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t systemEnumType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t systemEnumType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_int.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,8 @@ func (t systemIntType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t systemIntType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t systemIntType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
Expand Down
5 changes: 5 additions & 0 deletions sql/types/system_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ func (t systemSetType) Equals(otherType sql.Type) bool {
return false
}

// MaxTextResponseByteLength implements the Type interface
func (t systemSetType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
func (t systemSetType) Promote() sql.Type {
return t
Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ func (t systemStringType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t systemStringType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t systemStringType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
Expand Down
5 changes: 2 additions & 3 deletions sql/types/system_uint.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ func (t systemUintType) Equals(otherType sql.Type) bool {
}

// MaxTextResponseByteLength implements the Type interface
func (t systemUintType) MaxTextResponseByteLength(_ *sql.Context) uint32 {
// system types are not sent directly across the wire
return 0
func (t systemUintType) MaxTextResponseByteLength(ctx *sql.Context) uint32 {
return t.UnderlyingType().MaxTextResponseByteLength(ctx)
}

// Promote implements the Type interface.
Expand Down
4 changes: 4 additions & 0 deletions sql/types/typecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,5 +186,9 @@ func IsTuple(t sql.Type) bool {

// IsUnsigned checks if t is an unsigned type.
func IsUnsigned(t sql.Type) bool {
if svt, ok := t.(sql.SystemVariableType); ok {
t = svt.UnderlyingType()
}

return t == Uint8 || t == Uint16 || t == Uint24 || t == Uint32 || t == Uint64
}
2 changes: 1 addition & 1 deletion sql/variables/system_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ var systemVars = map[string]sql.SystemVariable{
Scope: sql.SystemVariableScope_Both,
Dynamic: true,
SetVarHintApplies: false,
Type: types.NewSystemIntType("max_allowed_packet", 1024, 1073741824, false),
Type: types.NewSystemUintType("max_allowed_packet", 1024, 1073741824),
Default: int64(1073741824),
},
"max_connect_errors": {
Expand Down