Changeset: 6e01d555b38d for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/6e01d555b38d
Modified Files:
sql/server/rel_exp.c
sql/server/rel_exp.h
sql/server/rel_optimizer.c
sql/server/rel_select.c
sql/server/rel_unnest.c
sql/test/SQLancer/Tests/sqlancer18.test
Branch: Jan2022
Log Message:
make sure we know on which relation we need to group (ie the outer or the
inner).
If unclear and within the where, we should give an error.
diffs (truncated from 316 to 300 lines):
diff --git a/sql/server/rel_exp.c b/sql/server/rel_exp.c
--- a/sql/server/rel_exp.c
+++ b/sql/server/rel_exp.c
@@ -1541,20 +1541,20 @@ distinct_rel(sql_exp *e, const char **rn
}
int
-rel_has_exp(sql_rel *rel, sql_exp *e)
+rel_has_exp(sql_rel *rel, sql_exp *e, bool subexp)
{
- if (rel_find_exp(rel, e) != NULL)
+ if (rel_find_exp_and_corresponding_rel(rel, e, subexp, NULL, NULL))
return 0;
return -1;
}
int
-rel_has_exps(sql_rel *rel, list *exps)
+rel_has_exps(sql_rel *rel, list *exps, bool subexp)
{
if (list_empty(exps))
return 0;
for (node *n = exps->h; n; n = n->next)
- if (rel_has_exp(rel, n->data) >= 0)
+ if (rel_has_exp(rel, n->data, subexp) >= 0)
return 0;
return -1;
}
@@ -1565,15 +1565,21 @@ rel_has_all_exps(sql_rel *rel, list *exp
if (list_empty(exps))
return 1;
for (node *n = exps->h; n; n = n->next)
- if (rel_has_exp(rel, n->data) < 0)
+ if (rel_has_exp(rel, n->data, false) < 0)
return 0;
return 1;
}
+static int
+rel_has_exp2(sql_rel *rel, sql_exp *e)
+{
+ return rel_has_exp(rel, e, false);
+}
+
sql_rel *
find_rel(list *rels, sql_exp *e)
{
- node *n = list_find(rels, e, (fcmp)&rel_has_exp);
+ node *n = list_find(rels, e, (fcmp)&rel_has_exp2);
if (n)
return n->data;
return NULL;
@@ -1586,7 +1592,7 @@ find_one_rel(list *rels, sql_exp *e)
sql_rel *fnd = NULL;
for(n = rels->h; n; n = n->next) {
- if (rel_has_exp(n->data, e) == 0) {
+ if (rel_has_exp(n->data, e, false) == 0) {
if (fnd)
return NULL;
fnd = n->data;
@@ -1658,7 +1664,7 @@ exps_find_prop(list *exps, rel_prop kind
}
static sql_exp *
-rel_find_exp_and_corresponding_rel_(sql_rel *rel, sql_exp *e, sql_rel **res)
+rel_find_exp_and_corresponding_rel_(sql_rel *rel, sql_exp *e, bool subexp,
sql_rel **res)
{
sql_exp *ne = NULL;
@@ -1683,7 +1689,7 @@ rel_find_exp_and_corresponding_rel_(sql_
*res = rel;
return ne;
case e_convert:
- return rel_find_exp_and_corresponding_rel_(rel, e->l, res);
+ return rel_find_exp_and_corresponding_rel_(rel, e->l, subexp,
res);
case e_aggr:
case e_func:
if (e->l) {
@@ -1691,8 +1697,10 @@ rel_find_exp_and_corresponding_rel_(sql_
node *n = l->h;
ne = n->data;
- while (ne != NULL && n != NULL) {
- ne = rel_find_exp_and_corresponding_rel_(rel,
n->data, res);
+ while ((subexp || ne != NULL) && n != NULL) {
+ ne = rel_find_exp_and_corresponding_rel_(rel,
n->data, subexp, res);
+ if (subexp && ne)
+ break;
n = n->next;
}
return ne;
@@ -1708,8 +1716,10 @@ rel_find_exp_and_corresponding_rel_(sql_
node *n = l->h;
ne = n->data;
- while (ne != NULL && n != NULL) {
- ne = rel_find_exp_and_corresponding_rel_(rel,
n->data, res);
+ while ((subexp || ne != NULL) && n != NULL) {
+ ne = rel_find_exp_and_corresponding_rel_(rel,
n->data, subexp, res);
+ if (subexp && ne)
+ break;
n = n->next;
}
return ne;
@@ -1720,9 +1730,9 @@ rel_find_exp_and_corresponding_rel_(sql_
}
sql_exp *
-rel_find_exp_and_corresponding_rel(sql_rel *rel, sql_exp *e, sql_rel **res,
bool *under_join)
+rel_find_exp_and_corresponding_rel(sql_rel *rel, sql_exp *e, bool subexp,
sql_rel **res, bool *under_join)
{
- sql_exp *ne = rel_find_exp_and_corresponding_rel_(rel, e, res);
+ sql_exp *ne = rel_find_exp_and_corresponding_rel_(rel, e, subexp, res);
if (rel && !ne) {
switch(rel->op) {
@@ -1732,9 +1742,9 @@ rel_find_exp_and_corresponding_rel(sql_r
case op_join:
case op_semi:
case op_anti:
- ne = rel_find_exp_and_corresponding_rel(rel->l, e, res,
under_join);
+ ne = rel_find_exp_and_corresponding_rel(rel->l, e,
subexp, res, under_join);
if (!ne && is_join(rel->op))
- ne = rel_find_exp_and_corresponding_rel(rel->r,
e, res, under_join);
+ ne = rel_find_exp_and_corresponding_rel(rel->r,
e, subexp, res, under_join);
if (ne && under_join)
*under_join = true;
break;
@@ -1743,7 +1753,7 @@ rel_find_exp_and_corresponding_rel(sql_r
break;
default:
if (!is_project(rel->op) && rel->l)
- ne = rel_find_exp_and_corresponding_rel(rel->l,
e, res, under_join);
+ ne = rel_find_exp_and_corresponding_rel(rel->l,
e, subexp, res, under_join);
}
}
return ne;
@@ -1752,7 +1762,7 @@ rel_find_exp_and_corresponding_rel(sql_r
sql_exp *
rel_find_exp(sql_rel *rel, sql_exp *e)
{
- return rel_find_exp_and_corresponding_rel(rel, e, NULL, NULL);
+ return rel_find_exp_and_corresponding_rel(rel, e, false, NULL, NULL);
}
int
diff --git a/sql/server/rel_exp.h b/sql/server/rel_exp.h
--- a/sql/server/rel_exp.h
+++ b/sql/server/rel_exp.h
@@ -129,7 +129,7 @@ extern unsigned int exp_card(sql_exp *e)
extern const char *exp_find_rel_name(sql_exp *e);
extern sql_exp *rel_find_exp(sql_rel *rel, sql_exp *e);
-extern sql_exp *rel_find_exp_and_corresponding_rel(sql_rel *rel, sql_exp *e,
sql_rel **res, bool *under_join);
+extern sql_exp *rel_find_exp_and_corresponding_rel(sql_rel *rel, sql_exp *e,
bool subexp, sql_rel **res, bool *under_join);
extern int exp_cmp( sql_exp *e1, sql_exp *e2);
extern int exp_equal( sql_exp *e1, sql_exp *e2);
@@ -170,10 +170,10 @@ extern int exp_has_sideeffect(sql_exp *e
extern sql_exp *exps_find_prop(list *exps, rel_prop kind);
-/* returns 0 when the relation contain the passed expression else < 0 */
-extern int rel_has_exp(sql_rel *rel, sql_exp *e);
-/* return 0 when the relation contain atleast one of the passed expressions
else < 0 */
-extern int rel_has_exps(sql_rel *rel, list *e);
+/* returns 0 when the relation contain the passed expression (or sub
expressions if subexp is set) else < 0 */
+extern int rel_has_exp(sql_rel *rel, sql_exp *e, bool subexp);
+/* return 0 when the relation contain atleast one of the passed expressions
(or sub expressions if subexp is set) else < 0 */
+extern int rel_has_exps(sql_rel *rel, list *e, bool subexp);
/* return 1 when the relation contains all of the passed expressions else 0 */
extern int rel_has_all_exps(sql_rel *rel, list *e);
diff --git a/sql/server/rel_optimizer.c b/sql/server/rel_optimizer.c
--- a/sql/server/rel_optimizer.c
+++ b/sql/server/rel_optimizer.c
@@ -386,7 +386,7 @@ static sql_exp *
joinexp_col(sql_exp *e, sql_rel *r)
{
if (e->type == e_cmp) {
- if (rel_has_exp(r, e->l) >= 0)
+ if (rel_has_exp(r, e->l, false) >= 0)
return e->l;
return e->r;
}
@@ -394,6 +394,12 @@ joinexp_col(sql_exp *e, sql_rel *r)
return NULL;
}
+static int
+rel_has_exp2(sql_rel *r, sql_exp *e)
+{
+ return rel_has_exp(r, e, false);
+}
+
static sql_column *
table_colexp(sql_exp *e, sql_rel *r)
{
@@ -827,8 +833,8 @@ order_joins(visitor *v, list *rels, list
node *ln, *rn, *en;
cje = djn->data;
- ln = list_find(n_rels, cje->l, (fcmp)&rel_has_exp);
- rn = list_find(n_rels, cje->r, (fcmp)&rel_has_exp);
+ ln = list_find(n_rels, cje->l, (fcmp)&rel_has_exp2);
+ rn = list_find(n_rels, cje->r, (fcmp)&rel_has_exp2);
if (ln && rn) {
assert(0);
@@ -992,7 +998,7 @@ push_in_join_down(mvc *sql, list *rels,
if (e->type == e_cmp && e->flag ==
cmp_equal) {
/* in values are on
the right of the join */
- if (rel_has_exp(r, e->r) >= 0)
+ if (rel_has_exp(r, e->r, false)
>= 0)
je = e;
}
}
@@ -4293,7 +4299,7 @@ gen_push_groupby_down(mvc *sql, sql_rel
if (exp_is_atom(ce))
list_append(aliases, ce);
else if (ce->type == e_column) {
- if (rel_has_exp(cl, ce) == 0) /* collect
aliases outside groupby */
+ if (rel_has_exp(cl, ce, false) == 0) /* collect
aliases outside groupby */
list_append(aliases, ce);
else
list_append(aggrs, ce);
@@ -4301,7 +4307,7 @@ gen_push_groupby_down(mvc *sql, sql_rel
list *args = ce->l;
/* check args are part of left/right */
- if (!list_empty(args) && rel_has_exps(cl, args)
== 0)
+ if (!list_empty(args) && rel_has_exps(cl, args,
false) == 0)
return rel;
if (rel->op != op_join && exp_aggr_is_count(ce))
ce->p = prop_create(sql->sa,
PROP_COUNT, ce->p);
@@ -5283,7 +5289,7 @@ find_projection_for_join2semi(sql_rel *r
return has_nil(e) ?
MAY_HAVE_DUPLICATE_NULLS : ALL_VALUES_DISTINCT;
if ((is_simple_project(rel->op) ||
is_groupby(rel->op) || is_inter(rel->op) || is_except(rel->op)) &&
- (found =
rel_find_exp_and_corresponding_rel(rel->l, e, &res, &underjoin)) && !underjoin)
{ /* grouping column on inner relation */
+ (found =
rel_find_exp_and_corresponding_rel(rel->l, e, false, &res, &underjoin)) &&
!underjoin) { /* grouping column on inner relation */
if (need_distinct(res) &&
list_length(res->exps) == 1)
return ALL_VALUES_DISTINCT;
if (is_unique(found))
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
@@ -3395,7 +3395,7 @@ static sql_exp *
sql_rel *groupby = rel ? *rel : NULL, *sel = NULL, *gr, *og = NULL,
*res = groupby;
sql_rel *subquery = NULL;
list *exps = NULL;
- bool is_grouping = !strcmp(aname, "grouping"), has_args = false, found
= false;
+ bool is_grouping = !strcmp(aname, "grouping"), has_args = false, found
= false, used_rel = false;
if (!query_has_outer(query)) {
if (!groupby) {
@@ -3443,6 +3443,7 @@ static sql_exp *
if (!e)
return NULL;
+ used_rel |= (rel_has_exp(gl, e, true) == 0);
has_args = true;
if (gl && gl != ogl) {
if (gl->grouped) {
@@ -3506,6 +3507,9 @@ static sql_exp *
return
sql_error(sql, ERR_GROUPBY, SQLSTATE(42000) "SELECT: subquery uses ungrouped
column \"%s.%s\" from outer query", exp_relname(e), exp_name(e));
return
sql_error(sql, ERR_GROUPBY, SQLSTATE(42000) "SELECT: subquery uses ungrouped
column from outer query");
}
+ } else if (!used_rel &&
is_sql_where(of)) {
+ char *uaname =
SA_NEW_ARRAY(sql->ta, char, strlen(aname) + 1);
+ return sql_error(sql,
02, SQLSTATE(42000) "%s: aggregate functions not allowed in WHERE clause",
toUpperCopy(uaname, aname));
} else if (!is_sql_aggr(of)) {
set_outer(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
@@ -1945,9 +1945,9 @@ exp_reset_card_and_freevar_set_physical_
sql_exp *le = NULL, *re = NULL;
bool underjoinl = false, underjoinr = false;
- le = rel_find_exp_and_corresponding_rel(rel->l, e,
NULL, &underjoinl);
+ le = rel_find_exp_and_corresponding_rel(rel->l, e,
false, NULL, &underjoinl);
if (!is_simple_project(rel->op) && !is_inter(rel->op)
&& !is_except(rel->op) && !is_semi(rel->op) && rel->r) {
- re = rel_find_exp_and_corresponding_rel(rel->r,
e, NULL, &underjoinr);
+ re = rel_find_exp_and_corresponding_rel(rel->r,
e, false, NULL, &underjoinr);
/* if the expression is found under a join, the
cardinality expands to multi */
e->card =
MAX(le?underjoinl?CARD_MULTI:le->card:CARD_ATOM,
re?underjoinr?CARD_MULTI:re->card:CARD_ATOM);
} else if (e->card == CARD_ATOM) { /* unnested columns
vs atoms */
diff --git a/sql/test/SQLancer/Tests/sqlancer18.test
b/sql/test/SQLancer/Tests/sqlancer18.test
--- a/sql/test/SQLancer/Tests/sqlancer18.test
+++ b/sql/test/SQLancer/Tests/sqlancer18.test
@@ -672,17 +672,10 @@ statement ok
create or replace view v102(vc0) as (select 1 from v101)
statement ok
-savepoint sp1
-
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
