Changeset: c063a779dde4 for MonetDB URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=c063a779dde4 Modified Files: sql/server/rel_rel.c sql/server/rel_select.c sql/server/rel_unnest.c sql/test/BugTracker-2017/Tests/limit.Bug-6322.stable.out sql/test/BugTracker-2017/Tests/push_subslice.Bug-6322.stable.out sql/test/mergetables/Tests/sqlsmith-exists.stable.out sql/test/mergetables/Tests/sqlsmith-exists2.stable.out sql/test/subquery/Tests/subquery3.stable.err Branch: default Log Message:
fix output of sqlsmith-exists2 (ie removed too early and/or optimization) added checks for aggregation in group by and nested aggregation (without correlation) diffs (truncated from 338 to 300 lines): diff --git a/sql/server/rel_rel.c b/sql/server/rel_rel.c --- a/sql/server/rel_rel.c +++ b/sql/server/rel_rel.c @@ -2048,6 +2048,11 @@ rel_visitor(mvc *sql, sql_rel *rel, rel_ if (!rel) return rel; + if (rel->exps && (rel->exps = exps_rel_visitor(sql, rel->exps, rel_rewriter)) == NULL) + return NULL; + if ((is_groupby(rel->op) || is_simple_project(rel->op)) && rel->r && (rel->r = exps_rel_visitor(sql, rel->r, rel_rewriter)) == NULL) + return NULL; + switch(rel->op){ case op_basetable: case op_table: 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 @@ -2392,54 +2392,12 @@ rel_logical_exp(sql_query *query, sql_re } case SQL_AND: { - /* split into 2 lists, simle logical expressions and or's */ - list *nors = sa_list(sql->sa); - list *ors = sa_list(sql->sa); - symbol *lo = sc->data.lval->h->data.sym; symbol *ro = sc->data.lval->h->next->data.sym; - node *n; - - while (lo->token == SQL_AND) { - symbol *s; - - sc = lo; - lo = sc->data.lval->h->data.sym; - s = sc->data.lval->h->next->data.sym; - - if (s->token != SQL_OR) - list_prepend(nors, s); - else - list_prepend(ors, s); - } - if (lo->token != SQL_OR) - list_prepend(nors, lo); - else - list_prepend(ors, lo); - if (ro->token != SQL_OR) - append(nors, ro); - else - append(ors, ro); - - for(n=nors->h; n; n = n->next) { - symbol *lo = n->data; - rel = rel_logical_exp(query, rel, lo, f); - if (!rel) - return NULL; - } - for(n=ors->h; n; n = n->next) { - symbol *lo = n->data; - rel = rel_logical_exp(query, rel, lo, f); - if (!rel) - return NULL; - } - return rel; - /* rel = rel_logical_exp(query, rel, lo, f); if (!rel) return NULL; return rel_logical_exp(query, rel, ro, f); - */ } case SQL_FILTER: /* [ x,..] filter [ y,..] */ @@ -3243,6 +3201,7 @@ static sql_exp * exps = sa_list(sql->sa); if (args && args->data.sym) { + int all_aggr = query_has_outer(query); all_freevar = 1; for ( ; args; args = args->next ) { int base = (!groupby || !is_project(groupby->op) || is_base(groupby->op) || is_processed(groupby)); @@ -3257,10 +3216,27 @@ static sql_exp * } if (!e || !exp_subtype(e)) /* we also do not expect parameters here */ return NULL; + all_aggr &= (exp_card(e) <= CARD_AGGR && !exp_is_atom(e) && !is_func(e->type) && (!groupby->r || !exps_find_exp(groupby->r, e))); has_freevar |= exp_has_freevar(sql, e); all_freevar &= (is_freevar(e)>0); list_append(exps, e); } + if (all_aggr && !all_freevar) { + char *uaname = GDKmalloc(strlen(aname) + 1); + sql_exp *e = sql_error(sql, 02, SQLSTATE(42000) "%s: aggregate functions cannot be nested", + uaname ? toUpperCopy(uaname, aname) : aname); + if (uaname) + GDKfree(uaname); + return e; + } + if (is_sql_groupby(f) && !all_freevar) { + char *uaname = GDKmalloc(strlen(aname) + 1); + sql_exp *e = sql_error(sql, 02, SQLSTATE(42000) "%s: aggregate function '%s' not allowed in GROUP BY clause", + uaname ? toUpperCopy(uaname, aname) : aname, aname); + if (uaname) + GDKfree(uaname); + return e; + } } if (all_freevar) { /* case 2, ie use outer */ diff --git a/sql/server/rel_unnest.c b/sql/server/rel_unnest.c --- a/sql/server/rel_unnest.c +++ b/sql/server/rel_unnest.c @@ -1501,6 +1501,71 @@ rewrite_exp_rel(mvc *sql, sql_rel *rel, /* simplify expressions, such as not(not(x)) */ /* exp visitor */ + +static list * +exps_simplify_exp(mvc *sql, list *exps) +{ + if (list_empty(exps)) + return exps; + + int needed = 0; + for (node *n=exps->h; n && !needed; n = n->next) { + sql_exp *e = n->data; + + needed = (exp_is_true(sql, e) || exp_is_false(sql, e) || (is_compare(e->type) && e->flag == cmp_or)); + } + if (needed) { + list *nexps = sa_list(sql->sa); + sql->caching = 0; + for (node *n=exps->h; n; n = n->next) { + sql_exp *e = n->data; + + /* TRUE or X -> TRUE + * FALSE or X -> X */ + if (is_compare(e->type) && e->flag == cmp_or) { + list *l = e->l = exps_simplify_exp(sql, e->l); + list *r = e->r = exps_simplify_exp(sql, e->r); + + if (list_length(l) == 1) { + sql_exp *ie = l->h->data; + + if (exp_is_true(sql, ie)) { + continue; + } else if (exp_is_false(sql, ie)) { + nexps = list_merge(nexps, r, (fdup)NULL); + continue; + } + } else if (list_length(l) == 0) { /* left is true */ + continue; + } + if (list_length(r) == 1) { + sql_exp *ie = r->h->data; + + if (exp_is_true(sql, ie)) + continue; + else if (exp_is_false(sql, ie)) { + nexps = list_merge(nexps, l, (fdup)NULL); + continue; + } + } else if (list_length(r) == 0) { /* right is true */ + continue; + } + } + /* TRUE and X -> X */ + if (exp_is_true(sql, e)) { + continue; + /* FALSE and X -> FALSE */ + } else if (exp_is_false(sql, e)) { + return append(sa_list(sql->sa), e); + } else { + append(nexps, e); + } + } + return nexps; + } + return exps; +} + static sql_exp * rewrite_simplify_exp(mvc *sql, sql_rel *rel, sql_exp *e, int depth) { @@ -1540,7 +1605,8 @@ rewrite_simplify_exp(mvc *sql, sql_rel * /* TRUE or X -> TRUE * FALSE or X -> X */ if (is_compare(e->type) && e->flag == cmp_or) { - list *l = e->l, *r = e->r; + list *l = e->l = exps_simplify_exp(sql, e->l); + list *r = e->r = exps_simplify_exp(sql, e->r); sql->caching = 0; if (list_length(l) == 1) { @@ -1550,6 +1616,18 @@ rewrite_simplify_exp(mvc *sql, sql_rel * return ie; else if (exp_is_false(sql, ie) && list_length(r) == 1) return r->h->data; + } else if (list_length(l) == 0) { /* left is true */ + return exp_atom_bool(sql->sa, 1); + } + if (list_length(r) == 1) { + sql_exp *ie = r->h->data; + + if (exp_is_true(sql, ie)) + return ie; + else if (exp_is_false(sql, ie) && list_length(l) == 1) + return l->h->data; + } else if (list_length(r) == 0) { /* right is true */ + return exp_atom_bool(sql->sa, 1); } } } @@ -1562,33 +1640,8 @@ rewrite_simplify(mvc *sql, sql_rel *rel) if (!rel) return rel; - if (is_select(rel->op) && !list_empty(rel->exps)) { - int needed = 0; - for (node *n=rel->exps->h; n && !needed; n = n->next) { - sql_exp *e = n->data; - - needed = (exp_is_true(sql, e) || exp_is_false(sql, e)); - } - if (needed) { - list *nexps = sa_list(sql->sa); - sql->caching = 0; - for (node *n=rel->exps->h; n; n = n->next) { - sql_exp *e = n->data; - - /* TRUE and X -> X */ - if (exp_is_true(sql, e)) { - continue; - /* FALSE and X -> FALSE */ - } else if (exp_is_false(sql, e)) { - rel->exps = append(sa_list(sql->sa), e); - return rel; - } else { - append(nexps, e); - } - } - rel->exps = nexps; - } - } + if ((is_select(rel->op) || is_join(rel->op)) && !list_empty(rel->exps)) + rel->exps = exps_simplify_exp(sql, rel->exps); return rel; } diff --git a/sql/test/BugTracker-2017/Tests/limit.Bug-6322.stable.out b/sql/test/BugTracker-2017/Tests/limit.Bug-6322.stable.out --- a/sql/test/BugTracker-2017/Tests/limit.Bug-6322.stable.out +++ b/sql/test/BugTracker-2017/Tests/limit.Bug-6322.stable.out @@ -39,7 +39,7 @@ stdout of test 'limit.Bug-6322` in direc #where (true) # or ((select pc from sys.tracelog) # is not NULL); -% . # table_name +% sys. # table_name % c2 # name % int # type % 2 # length diff --git a/sql/test/BugTracker-2017/Tests/push_subslice.Bug-6322.stable.out b/sql/test/BugTracker-2017/Tests/push_subslice.Bug-6322.stable.out --- a/sql/test/BugTracker-2017/Tests/push_subslice.Bug-6322.stable.out +++ b/sql/test/BugTracker-2017/Tests/push_subslice.Bug-6322.stable.out @@ -47,7 +47,7 @@ stdout of test 'push_subslice.Bug-6322` #where (true) # or ((select pc from sys.tracelog) # is not NULL); -% . # table_name +% sys. # table_name % c2 # name % int # type % 4 # length diff --git a/sql/test/mergetables/Tests/sqlsmith-exists.stable.out b/sql/test/mergetables/Tests/sqlsmith-exists.stable.out --- a/sql/test/mergetables/Tests/sqlsmith-exists.stable.out +++ b/sql/test/mergetables/Tests/sqlsmith-exists.stable.out @@ -72,7 +72,7 @@ stdout of test 'sqlsmith-exists` in dire # ref_1.col5 as c12, # ref_2.col3 as c13, # case when exists ( -% ., ., ., . # table_name +% sys., sys., ., sys. # table_name % c0, c1, c2, c3 # name % int, int, tinyint, int # type % 1, 1, 1, 1 # length @@ -459,7 +459,7 @@ stdout of test 'sqlsmith-exists` in dire # ref_0.col1 AS c0 # ,89 AS c1 # ,ref_0.col2 AS c2 -% ., . # table_name +% ., sys. # table_name % c0, c1 # name % varchar, varchar # type % 0, 0 # length diff --git a/sql/test/mergetables/Tests/sqlsmith-exists2.stable.out b/sql/test/mergetables/Tests/sqlsmith-exists2.stable.out --- a/sql/test/mergetables/Tests/sqlsmith-exists2.stable.out +++ b/sql/test/mergetables/Tests/sqlsmith-exists2.stable.out @@ -81,6 +81,9 @@ stdout of test 'sqlsmith-exists2` in dir % c0 # name % int # type _______________________________________________ checkin-list mailing list checkin-list@monetdb.org https://www.monetdb.org/mailman/listinfo/checkin-list