Skip to content

Set proper join vars type for the RHS field query in OLAP #18028

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.

Sign up for GitHub

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

Merged
systay merged 7 commits into from
Mar 25, 2025
Merged

Set proper join vars type for the RHS field query in OLAP #18028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
54fd3a7
correctly execute the join's RHS field query in stream
frouioui Mar 25, 2025
8ddeff0
fix unit tests
frouioui Mar 25, 2025
7182f78
fix executor test empty stream
frouioui Mar 25, 2025
b9eafb2
feat: make sure to handle more types
systay Mar 25, 2025
4ff32ab
feat: handle types better when handling empty results
systay Mar 25, 2025
4021757
fix: don't stop processing LHS rows too early
systay Mar 25, 2025
3ac2b12
test: update expectations
systay Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,87 @@ func TestAliasesInOuterJoinQueries(t *testing.T) {
}
}

func TestJoinTypes(t *testing.T) {
columns := []string{
"id",
"msg",
"keyspace_id",
"tinyint_unsigned",
"bool_signed",
"smallint_unsigned",
"mediumint_unsigned",
"int_unsigned",
"float_unsigned",
"double_unsigned",
"decimal_unsigned",
"t_date",
"t_datetime",
"t_datetime_micros",
"t_time",
"t_timestamp",
"c8",
"c16",
"c24",
"c32",
"c40",
"c48",
"c56",
"c63",
"c64",
"json_col",
"text_col",
"data",
"tinyint_min",
"tinyint_max",
"tinyint_pos",
"tinyint_neg",
"smallint_min",
"smallint_max",
"smallint_pos",
"smallint_neg",
"medint_min",
"medint_max",
"medint_pos",
"medint_neg",
"int_min",
"int_max",
"int_pos",
"int_neg",
"bigint_min",
"bigint_max",
"bigint_pos",
"bigint_neg",
}

mcmp, closer := start(t)
defer closer()

// Insert data into the 2 tables
mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)")
mcmp.Exec("insert into all_types(id) values (1)")

for _, mode := range []string{"oltp", "olap"} {
mcmp.Run(mode, func(mcmp *utils.MySQLCompare) {
utils.Exec(t, mcmp.VtConn, fmt.Sprintf("set workload = %s", mode))
// No result from the RHS, but the RHS uses LHS's values in a few places
// There used to be instances where the query sent to vttablet looked like this:
//
// "select tbl.unq_col + tbl.id + :t1_id1 /* INT64 */ as col from tbl where 1 != 1"
// {"t1_id1": {"type": "NULL_TYPE", "value": ""}, "t1_id2": {"type": "NULL_TYPE", "value": ""}, "tbl_id": {"type": "INT64", "value": 90}}
//
// Because we were hardcoding the join vars to NULL when sending the RHS field query iff there were no results from the RHS
// leading to DECIMAL/FLOAT64 types returned by MySQL as we are doing "tbl.unq_col + null + null"

for _, column := range columns {
query := fmt.Sprintf("select t1.id1 as t0, tbl.%s+tbl.id+t1.id1 as col from t1 join all_types tbl where tbl.id > 90", column)
mcmp.Run(column, func(mcmp *utils.MySQLCompare) {
mcmp.ExecWithColumnCompare(query)
})
}
})
}
}

func TestAlterTableWithView(t *testing.T) {
mcmp, closer := start(t)
defer closer()
Expand Down
53 changes: 53 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,56 @@ create table tbl_enum_set
set_col set('a', 'b', 'c', 'd', 'e', 'f', 'g'),
primary key (id)
) Engine = InnoDB;

create table all_types
(
id bigint not null,
msg varchar(64),
keyspace_id bigint(20) unsigned,
tinyint_unsigned TINYINT,
bool_signed BOOL,
smallint_unsigned SMALLINT,
mediumint_unsigned MEDIUMINT,
int_unsigned INT,
float_unsigned FLOAT(10, 2),
double_unsigned DOUBLE(16, 2),
decimal_unsigned DECIMAL,
t_date DATE,
t_datetime DATETIME,
t_datetime_micros DATETIME(6),
t_time TIME,
t_timestamp TIMESTAMP,
c8 bit(8) DEFAULT NULL,
c16 bit(16) DEFAULT NULL,
c24 bit(24) DEFAULT NULL,
c32 bit(32) DEFAULT NULL,
c40 bit(40) DEFAULT NULL,
c48 bit(48) DEFAULT NULL,
c56 bit(56) DEFAULT NULL,
c63 bit(63) DEFAULT NULL,
c64 bit(64) DEFAULT NULL,
json_col JSON,
text_col TEXT,
data longblob,
tinyint_min TINYINT,
tinyint_max TINYINT,
tinyint_pos TINYINT,
tinyint_neg TINYINT,
smallint_min SMALLINT,
smallint_max SMALLINT,
smallint_pos SMALLINT,
smallint_neg SMALLINT,
medint_min MEDIUMINT,
medint_max MEDIUMINT,
medint_pos MEDIUMINT,
medint_neg MEDIUMINT,
int_min INT,
int_max INT,
int_pos INT,
int_neg INT,
bigint_min BIGINT,
bigint_max BIGINT,
bigint_pos BIGINT,
bigint_neg BIGINT,
primary key (id)
) Engine = InnoDB;
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/vschema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@
"name": "hash"
}
]
},
"all_types": {
"column_vindexes": [
{
"column": "id",
"name": "hash"
}
]
}
}
}
10 changes: 7 additions & 3 deletions go/vt/vtgate/engine/join.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ func bindvarForType(field *querypb.Field) *querypb.BindVariable {
size := max(1, int(field.ColumnLength-field.Decimals))
scale := max(1, int(field.Decimals))
bv.Value = append(append(bytes.Repeat([]byte{'0'}, size), byte('.')), bytes.Repeat([]byte{'0'}, scale)...)
case querypb.Type_JSON:
bv.Value = []byte(`""`) // empty json object
default:
return sqltypes.NullBindVariable
}
Expand Down Expand Up @@ -176,16 +178,18 @@ func (jn *Join) TryStreamExecute(ctx context.Context, vcursor VCursor, bindVars
mu.Lock()
defer mu.Unlock()
if fieldsSent.CompareAndSwap(false, true) {
for k := range jn.Vars {
joinVars[k] = sqltypes.NullBindVariable
for k, v := range jn.Vars {
joinVars[k] = bindvarForType(lresult.Fields[v])
}
result := &sqltypes.Result{}
rresult, err := jn.Right.GetFields(ctx, vcursor, combineVars(bindVars, joinVars))
if err != nil {
return err
}
result.Fields = joinFields(lresult.Fields, rresult.Fields, jn.Cols)
return callback(result)
if err := callback(result); err != nil {
return err
}
}
return nil
})
Expand Down
4 changes: 3 additions & 1 deletion go/vt/vtgate/executor_select_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2757,10 +2757,12 @@ func TestEmptyJoinStream(t *testing.T) {
sbc1.SetResults([]*sqltypes.Result{{
Fields: []*querypb.Field{
{Name: "id", Type: sqltypes.Int32, Charset: collations.CollationBinaryID, Flags: uint32(querypb.MySqlFlag_NUM_FLAG)},
{Name: "col", Type: sqltypes.Int32, Charset: collations.CollationBinaryID, Flags: uint32(querypb.MySqlFlag_NUM_FLAG)},
},
}, {
Fields: []*querypb.Field{
{Name: "id", Type: sqltypes.Int32, Charset: collations.CollationBinaryID, Flags: uint32(querypb.MySqlFlag_NUM_FLAG)},
{Name: "col", Type: sqltypes.Int32, Charset: collations.CollationBinaryID, Flags: uint32(querypb.MySqlFlag_NUM_FLAG)},
},
}})
result, err := executorStream(ctx, executor, "select u1.id, u2.id from user u1 join user u2 on u2.id = u1.col where u1.id = 1")
Expand All @@ -2771,7 +2773,7 @@ func TestEmptyJoinStream(t *testing.T) {
}, {
Sql: "select u2.id from `user` as u2 where 1 != 1",
BindVariables: map[string]*querypb.BindVariable{
"u1_col": sqltypes.NullBindVariable,
"u1_col": sqltypes.Int32BindVariable(0),
},
}}
utils.MustMatch(t, wantQueries, sbc1.Queries)
Expand Down
Loading