Changeset: 7fe10e6fa740 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=7fe10e6fa740
Modified Files:
sql/server/rel_select.c
sql/server/sql_mvc.c
sql/server/sql_mvc.h
sql/server/sql_parser.y
sql/test/miscellaneous/Tests/groupby_expressions.sql
sql/test/miscellaneous/Tests/groupby_expressions.stable.err
sql/test/miscellaneous/Tests/groupby_expressions.stable.out
Branch: groupby-expressions
Log Message:
Using symbol comparison instead of string comparison for group by expressions.
This simplifies search for expressions in the projection phase.
diffs (truncated from 303 to 300 lines):
diff --git a/sql/server/rel_select.c b/sql/server/rel_select.c
--- a/sql/server/rel_select.c
+++ b/sql/server/rel_select.c
@@ -3120,14 +3120,6 @@ rel_unop(mvc *sql, sql_rel **rel, symbol
sql_subtype *t = NULL;
int type = (ek.card == card_loader)?F_LOADER:((ek.card ==
card_none)?F_PROC:F_FUNC);
- if (*rel && (*rel)->card == CARD_AGGR) { //group by expression case,
handle it before
- sql_exp *exp = stack_get_groupby_expression(sql, se);
- if (sql->errstr[0] != '\0')
- return NULL;
- if (exp)
- return exp_column(sql->sa, exp_relname(exp),
exp_name(exp), exp_subtype(exp), exp->card, has_nil(exp), is_intern(exp));
- }
-
if (sname)
s = mvc_bind_schema(sql, sname);
@@ -3435,14 +3427,6 @@ rel_binop(mvc *sql, sql_rel **rel, symbo
if (!s)
return NULL;
- if (*rel && (*rel)->card == CARD_AGGR) { //group by expression case,
handle it before
- sql_exp *exp = stack_get_groupby_expression(sql, se);
- if (sql->errstr[0] != '\0')
- return NULL;
- if (exp)
- return exp_column(sql->sa, exp_relname(exp),
exp_name(exp), exp_subtype(exp), exp->card, has_nil(exp), is_intern(exp));
- }
-
l = rel_value_exp(sql, rel, dl->next->data.sym, f, iek);
r = rel_value_exp(sql, rel, dl->next->next->data.sym, f, iek);
if (!l || !r)
@@ -3520,14 +3504,6 @@ rel_nop(mvc *sql, sql_rel **rel, symbol
exp_kind iek = {type_value, card_column, FALSE};
int err = 0;
- if (*rel && (*rel)->card == CARD_AGGR) { //group by expression case,
handle it before
- sql_exp *exp = stack_get_groupby_expression(sql, se);
- if (sql->errstr[0] != '\0')
- return NULL;
- if (exp)
- return exp_column(sql->sa, exp_relname(exp),
exp_name(exp), exp_subtype(exp), exp->card, has_nil(exp), is_intern(exp));
- }
-
for (; ops; ops = ops->next, nr_args++) {
sql_exp *e = rel_value_exp(sql, rel, ops->data.sym, fs, iek);
sql_subtype *tpe;
@@ -3826,18 +3802,6 @@ rel_aggr(mvc *sql, sql_rel **rel, symbol
d = l->h->next->next;
}
- if (*rel && (*rel)->card == CARD_AGGR) { //group by expression case,
handle it before
- sql_exp *exp = stack_get_groupby_expression(sql, se);
- if (sql->errstr[0] != '\0')
- return NULL;
- if (exp) {
- sql_exp *res = exp_column(sql->sa, exp_relname(exp),
exp_name(exp), exp_subtype(exp), exp->card, has_nil(exp), is_intern(exp));
- if (distinct)
- set_distinct(res);
- return res;
- }
- }
-
if (sname)
s = mvc_bind_schema(sql, sname);
return _rel_aggr( sql, rel, distinct, s, aname, d, f);
@@ -5093,6 +5057,22 @@ rel_value_exp2(mvc *sql, sql_rel **rel,
if (THRhighwater())
return sql_error(sql, 10, SQLSTATE(42000) "SELECT: too many
nested operators");
+ if (*rel && (*rel)->card == CARD_AGGR) { //group by expression case,
handle it before
+ sql_exp *exp = stack_get_groupby_expression(sql, se);
+ if (sql->errstr[0] != '\0')
+ return NULL;
+ if (exp) {
+ sql_exp *res = exp_column(sql->sa, exp_relname(exp),
exp_name(exp), exp_subtype(exp), exp->card, has_nil(exp), is_intern(exp));
+ if(se->token == SQL_AGGR) {
+ dlist *l = se->data.lval;
+ int distinct = l->h->next->data.i_val;
+ if (distinct)
+ set_distinct(res);
+ }
+ return res;
+ }
+ }
+
switch (se->token) {
case SQL_OP:
return rel_op(sql, se, ek);
diff --git a/sql/server/sql_mvc.c b/sql/server/sql_mvc.c
--- a/sql/server/sql_mvc.c
+++ b/sql/server/sql_mvc.c
@@ -1628,21 +1628,10 @@ sql_var*
stack_push_groupby_expression(mvc *sql, symbol *def, sql_exp *exp)
{
sql_var* res = NULL;
- char *err = NULL;
sql_groupby_expression *sge = MNEW(sql_groupby_expression);
if(sge) {
- sge->sdef = symbol2string(sql, def, 1, &err);
- if (!sge->sdef) {
- if (err) {
- (void) sql_error(sql, 02, SQLSTATE(42000)
"SELECT: incorrect expression '%s'", err);
- _DELETE(err);
- _DELETE(sge);
- return NULL;
- }
- _DELETE(sge);
- return NULL;
- }
+ sge->sdef = def;
sge->token = def->token;
sge->exp = exp;
@@ -1657,19 +1646,9 @@ stack_push_groupby_expression(mvc *sql,
sql_exp*
stack_get_groupby_expression(mvc *sql, symbol *def)
{
- char *err = NULL, *sdef = symbol2string(sql, def, 1, &err);
-
if(sql->has_groupby_expressions) {
- if (!sdef) {
- if (err) {
- (void) sql_error(sql, 02, SQLSTATE(42000)
"SELECT: incorrect expression '%s'", err);
- _DELETE(err);
- return NULL;
- }
- return NULL;
- }
for (int i = sql->topvars-1; i >= 0; i--) {
- if (!sql->vars[i].frame && sql->vars[i].exp &&
sql->vars[i].exp->token == def->token && strcmp(sql->vars[i].exp->sdef,
sdef)==0) {
+ if (!sql->vars[i].frame && sql->vars[i].exp &&
sql->vars[i].exp->token == def->token && symbol_cmp(sql->vars[i].exp->sdef,
def)==0) {
return sql->vars[i].exp->exp;
}
}
@@ -1757,10 +1736,8 @@ stack_pop_until(mvc *sql, int top)
c_delete(v->name);
VALclear(&v->a.data);
v->a.data.vtype = 0;
- if(v->exp) {
- _DELETE(v->exp->sdef);
+ if(v->exp)
_DELETE(v->exp);
- }
v->wdef = NULL;
}
}
@@ -1778,10 +1755,8 @@ stack_pop_frame(mvc *sql)
table_destroy(v->t);
else if (v->rel)
rel_destroy(v->rel);
- else if(v->exp) {
- _DELETE(v->exp->sdef);
+ else if(v->exp)
_DELETE(v->exp);
- }
v->wdef = NULL;
}
if (sql->topvars && sql->vars[sql->topvars].name)
diff --git a/sql/server/sql_mvc.h b/sql/server/sql_mvc.h
--- a/sql/server/sql_mvc.h
+++ b/sql/server/sql_mvc.h
@@ -68,7 +68,7 @@
#define mod_locked 16
typedef struct sql_groupby_expression {
- char *sdef;
+ symbol *sdef;
tokens token;
sql_exp *exp;
} sql_groupby_expression;
diff --git a/sql/server/sql_parser.y b/sql/server/sql_parser.y
--- a/sql/server/sql_parser.y
+++ b/sql/server/sql_parser.y
@@ -496,7 +496,7 @@ int yydebug=1;
routine_body
table_function_column_list
select_target_list
- simple_scalar_exp_commalist
+ search_condition_commalist
external_function_name
with_list
window_specification
@@ -3646,12 +3646,12 @@ opt_table_name:
opt_group_by_clause:
/* empty */ { $$ = NULL; }
- | sqlGROUP BY simple_scalar_exp_commalist { $$ = _symbol_create_list(
SQL_GROUPBY, $3 );}
- ;
-
-simple_scalar_exp_commalist:
- simple_scalar_exp { $$ =
append_symbol(L(), $1); }
- | simple_scalar_exp_commalist ',' simple_scalar_exp { $$ = append_symbol($1,
$3); }
+ | sqlGROUP BY search_condition_commalist { $$ = _symbol_create_list(
SQL_GROUPBY, $3 );}
+ ;
+
+search_condition_commalist:
+ search_condition { $$ = append_symbol(L(),
$1); }
+ | search_condition_commalist ',' search_condition { $$ = append_symbol($1,
$3); }
;
column_ref_commalist:
@@ -4260,7 +4260,7 @@ window_ident_clause:
window_partition_clause:
/* empty */ { $$ = NULL; }
- | PARTITION BY simple_scalar_exp_commalist
+ | PARTITION BY search_condition_commalist
{ $$ = _symbol_create_list( SQL_GROUPBY, $3 ); }
;
diff --git a/sql/test/miscellaneous/Tests/groupby_expressions.sql
b/sql/test/miscellaneous/Tests/groupby_expressions.sql
--- a/sql/test/miscellaneous/Tests/groupby_expressions.sql
+++ b/sql/test/miscellaneous/Tests/groupby_expressions.sql
@@ -14,10 +14,16 @@ select cast(sum(("aa"+1) + ("bb"+2)) as
select cast("aa"+1 as bigint) from "groupings" group by "aa"+1 having "aa"+1 >
2;
select cast("aa"+1 as bigint) from "groupings" group by "aa"+1 order by "aa"+1;
---select count(*) from "groupings" group by case when "aa" > 1 then "aa" else
"aa" + 10 end;
---select (case when "aa" > 1 then "aa" else "aa" * 4 end) from "groupings"
group by (case when "aa" > 1 then "aa" else "aa" * 4 end);
+select count(*) from "groupings" group by "aa" > 1;
+select "aa" > 1 from "groupings" group by "aa" > 1;
+select count(*) from "groupings" group by case when "aa" > 1 then "aa" else
"aa" + 10 end;
+select case when "aa" > 1 then "aa" else "aa" * 4 end from "groupings" group
by case when "aa" > 1 then "aa" else "aa" * 4 end;
+
+select cast(sum("aa"+"bb") as bigint) from "groupings" group by "aa"+"bb";
+select cast(sum("aa"+3452) as bigint) from "groupings" group by "aa"+"bb";
rollback;
+select "aa"+3452 from "groupings" group by "aa"+"bb"; --error
select count(*) from "groupings" group by rank() over (); --error
select count(*) from "groupings" having rank() over (); --error
select count(*) from "groupings" order by rank() over (); --error TODO?
diff --git a/sql/test/miscellaneous/Tests/groupby_expressions.stable.err
b/sql/test/miscellaneous/Tests/groupby_expressions.stable.err
--- a/sql/test/miscellaneous/Tests/groupby_expressions.stable.err
+++ b/sql/test/miscellaneous/Tests/groupby_expressions.stable.err
@@ -30,7 +30,11 @@ stderr of test 'groupby_expressions` in
# 14:21:56 > "mclient" "-lsql" "-ftest" "-tnone" "-Eutf-8" "-i" "-e"
"--host=/var/tmp/mtest-21747" "--port=32012"
# 14:21:56 >
-MAPI = (monetdb) /var/tmp/mtest-23848/.s.monetdb.34914
+MAPI = (monetdb) /var/tmp/mtest-28161/.s.monetdb.36698
+QUERY = select "aa"+3452 from "groupings" group by "aa"+"bb"; --error
+ERROR = !SELECT: no such aggregate 'sql_add'
+CODE = 42000
+MAPI = (monetdb) /var/tmp/mtest-28161/.s.monetdb.36698
QUERY = select count(*) from "groupings" group by rank() over (); --error
ERROR = !RANK: window function 'rank' not allowed in GROUP BY clause
CODE = 42000
diff --git a/sql/test/miscellaneous/Tests/groupby_expressions.stable.out
b/sql/test/miscellaneous/Tests/groupby_expressions.stable.out
--- a/sql/test/miscellaneous/Tests/groupby_expressions.stable.out
+++ b/sql/test/miscellaneous/Tests/groupby_expressions.stable.out
@@ -134,6 +134,48 @@ Ready.
% 1 # length
[ 2 ]
[ 3 ]
+#select count(*) from "groupings" group by "aa" > 1;
+% sys.L3 # table_name
+% L3 # name
+% bigint # type
+% 1 # length
+[ 2 ]
+[ 1 ]
+#select "aa" > 1 from "groupings" group by "aa" > 1;
+% sys.L1 # table_name
+% L1 # name
+% boolean # type
+% 5 # length
+[ false ]
+[ true ]
+#select count(*) from "groupings" group by case when "aa" > 1 then "aa" else
"aa" + 10 end;
+% sys.L3 # table_name
+% L3 # name
+% bigint # type
+% 1 # length
+[ 2 ]
+[ 1 ]
+#select case when "aa" > 1 then "aa" else "aa" * 4 end from "groupings" group
by case when "aa" > 1 then "aa" else "aa" * 4 end;
+% sys.L1 # table_name
+% L1 # name
+% bigint # type
+% 1 # length
+[ 4 ]
+[ 2 ]
+#select cast(sum("aa"+"bb") as bigint) from "groupings" group by "aa"+"bb";
+% sys.L4 # table_name
+% L4 # name
+% bigint # type
+% 1 # length
+[ 2 ]
+[ 8 ]
+#select cast(sum("aa"+3452) as bigint) from "groupings" group by "aa"+"bb";
+% sys.L4 # table_name
+% L4 # name
+% bigint # type
+% 4 # length
+[ 3453 ]
+[ 6907 ]
_______________________________________________
checkin-list mailing list
checkin-list@monetdb.org
https://www.monetdb.org/mailman/listinfo/checkin-list
