Changeset: 683532f29d99 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/683532f29d99
Added Files:
sql/test/rel-optimizers/optimize-proj/Tests/groupby-cse.test
Modified Files:
sql/server/rel_optimize_proj.c
sql/test/BugTracker-2015/Tests/crash_in_reduce_groupby.Bug-3818.test
sql/test/miscellaneous/Tests/groupby_error.test
sql/test/miscellaneous/Tests/simple_selects.test
Branch: Jun2023
Log Message:
Backported all changes from optimizer-testing branch to Jun2023 branch.
diffs (truncated from 638 to 300 lines):
diff --git a/sql/server/rel_optimize_proj.c b/sql/server/rel_optimize_proj.c
--- a/sql/server/rel_optimize_proj.c
+++ b/sql/server/rel_optimize_proj.c
@@ -1703,46 +1703,53 @@ rel_groupby_cse(visitor *v, sql_rel *rel
{
if (is_groupby(rel->op) && !list_empty(rel->r)) {
sql_rel *l = rel->l;
- int needed = 0;
-
- for (node *n=((list*)rel->r)->h; n ; n = n->next) {
- sql_exp *e = n->data;
- e->used = 0; /* we need to use this flag, clean it
first */
- }
+
+ /* for every group expression e1 */
for (node *n=((list*)rel->r)->h; n ; n = n->next) {
sql_exp *e1 = n->data;
+ /* it's good to examine the same expression in the
subrelation e.g. in case it's an alias */
/* TODO maybe cover more cases? Here I only look at the
left relation */
- sql_exp *e3 = e1->type == e_column ?
exps_find_exp(l->exps, e1) : NULL;
-
+ sql_exp *e1_sub = e1->type == e_column ?
exps_find_exp(l->exps, e1) : NULL;
+
+ /* for every other group expression */
for (node *m=n->next; m; m = m->next) {
sql_exp *e2 = m->data;
- sql_exp *e4 = e2->type == e_column ?
exps_find_exp(l->exps, e2) : NULL;
-
- if (exp_match_exp(e1, e2) || exp_refers(e1, e2)
|| (e3 && e4 && (exp_match_exp(e3, e4) || exp_refers(e3, e4)))) {
- e2->used = 1; /* flag it as being
removed */
- needed = 1;
+ sql_exp *e2_sub = e2->type == e_column ?
exps_find_exp(l->exps, e2) : NULL;
+
+ /* check if the expression are the same */
+ if (exp_match_exp(e1, e2) || exp_refers(e1, e2)
|| (e1_sub && e2_sub && (exp_match_exp(e1_sub, e2_sub) || exp_refers(e1_sub,
e2_sub)))) {
+
+ /* use e2 from rel->exps instead of e2
from the rel->r as it can have an alias from the higher rel */
+ sql_exp *e2_in_exps = (e2->alias.rname
== e2->l && e2->alias.name == e2->r) ?
+ exps_bind_column2(rel->exps,
e2->l, e2->r, NULL) :
+ exps_bind_column(rel->exps,
e2->alias.name, NULL, NULL, 0);
+ assert(e2_in_exps);
+
+ /* same as e2 */
+ sql_exp *e1_in_exps = (e1->alias.rname
== e1->l && e1->alias.name == e1->r) ?
+ exps_bind_column2(rel->exps,
e1->l, e1->r, NULL) :
+ exps_bind_column(rel->exps,
e1->alias.name, NULL, NULL, 0);
+ assert(e1_in_exps);
+
+ /* write e2 as an e1 alias since the
expressions are the same */
+ sql_exp* e2_as_e1_alias =
exp_copy(v->sql, e1_in_exps);
+ /* NOTE: it is important to get the
rname (exp->l) and name (exp->r) from e2 IN the exps
+ * (e2_in_exps), and not from e2, since
it could carry an alias from the higher rel */
+ exp_setalias(e2_as_e1_alias,
e2_in_exps->l, e2_in_exps->r);
+
+ /* replace e2 with e2_as_e1_alias in
expressions list */
+ node *e2_exps_node =
list_find(rel->exps, e2_in_exps, NULL);
+ list_append_before(rel->exps,
e2_exps_node, e2_as_e1_alias);
+ list_remove_node(rel->exps, NULL,
e2_exps_node);
+
+ /* finally remove e2 from the groups'
list (->r) since it's redundant */
+ node *e2_r_node = list_find(rel->r, e2,
NULL);
+ list_remove_node(rel->r, NULL,
e2_r_node);
+
+ v->changes++;
}
}
}
-
- if (!needed)
- return rel;
-
- if (!is_simple_project(l->op) || !list_empty(l->r) ||
rel_is_ref(l) || need_distinct(l))
- rel->l = l = rel_project(v->sql->sa, l,
rel_projections(v->sql, l, NULL, 1, 1));
-
- for (node *n=((list*)rel->r)->h; n ; ) {
- node *next = n->next;
- sql_exp *e = n->data;
-
- if (e->used) { /* remove unecessary grouping columns */
- e->used = 0;
- list_append(l->exps, e);
- list_remove_node(rel->r, NULL, n);
- v->changes++;
- }
- n = next;
- }
}
return rel;
}
diff --git
a/sql/test/BugTracker-2015/Tests/crash_in_reduce_groupby.Bug-3818.test
b/sql/test/BugTracker-2015/Tests/crash_in_reduce_groupby.Bug-3818.test
--- a/sql/test/BugTracker-2015/Tests/crash_in_reduce_groupby.Bug-3818.test
+++ b/sql/test/BugTracker-2015/Tests/crash_in_reduce_groupby.Bug-3818.test
@@ -27,10 +27,8 @@ GROUP BY cods, elrik, ether, jaelen, sor
----
project (
| group by (
-| | project (
-| | | table("sys"."t2a") [ "t2a"."tib0" NOT NULL UNIQUE ]
-| | ) [ "t2a"."tib0" NOT NULL UNIQUE, tinyint(8) "0" as "elrik", tinyint(8)
"0" as "ether", tinyint(8) "0" as "jaelen", tinyint(8) "0" as "sora" ]
-| ) [ tinyint(8) "0" as "cods" ] [ "cods" NOT NULL, "elrik" NOT NULL, "ether"
NOT NULL, "jaelen" NOT NULL, "sora" NOT NULL, "sys"."min" no nil ("t2a"."tib0"
NOT NULL UNIQUE) NOT NULL as "%1"."%1" ]
+| | table("sys"."t2a") [ "t2a"."tib0" NOT NULL UNIQUE ]
+| ) [ tinyint(8) "0" as "cods" ] [ "cods" NOT NULL, "cods" NOT NULL as
"elrik", "cods" NOT NULL as "ether", "cods" NOT NULL as "jaelen", "cods" NOT
NULL as "sora", "sys"."min" no nil ("t2a"."tib0" NOT NULL UNIQUE) NOT NULL as
"%1"."%1" ]
) [ "cods" NOT NULL UNIQUE, "elrik" NOT NULL, "ether" NOT NULL, "jaelen" NOT
NULL, "sora" NOT NULL, "%1"."%1" NOT NULL ]
statement ok
diff --git a/sql/test/miscellaneous/Tests/groupby_error.test
b/sql/test/miscellaneous/Tests/groupby_error.test
--- a/sql/test/miscellaneous/Tests/groupby_error.test
+++ b/sql/test/miscellaneous/Tests/groupby_error.test
@@ -415,7 +415,7 @@ project (
| | | table("sys"."tab1") [ "tab1"."part" UNIQUE as "myalias0"."part",
"tab1"."tet" UNIQUE as "myalias0"."tet" ],
| | | table("sys"."tab2") [ "tab2"."myk" as "myalias1"."myk", "tab2"."ups"
UNIQUE as "myalias1"."ups" ]
| | ) [ ("myalias0"."part" UNIQUE) = ("myalias1"."myk") ]
-| ) [ "myalias1"."ups", "myalias0"."part", "myalias0"."tet" as "track" ] [
"myalias0"."part", "track", "myalias0"."tet", "sys"."count"() NOT NULL as
"%1"."%1" ]
+| ) [ "myalias1"."ups", "myalias0"."part", "myalias0"."tet" as "track" ] [
"myalias0"."part", "track", "track" as "myalias0"."tet", "sys"."count"() NOT
NULL as "%1"."%1" ]
) [ "myalias0"."part" as "part", "myalias0"."tet" as "tet", "%1"."%1" NOT NULL
as "mycount", "track" ]
query IIII rowsort
diff --git a/sql/test/miscellaneous/Tests/simple_selects.test
b/sql/test/miscellaneous/Tests/simple_selects.test
--- a/sql/test/miscellaneous/Tests/simple_selects.test
+++ b/sql/test/miscellaneous/Tests/simple_selects.test
@@ -912,8 +912,8 @@ plan SELECT x AS x, x AS x1 FROM (VALUES
----
project (
| group by (
-| | [ [ tinyint(2) "1", tinyint(2) "2", tinyint(2) "3" ] as "x"."x", "x"."x"
NOT NULL as "x1" ]
-| ) [ "x"."x" NOT NULL ] [ "x"."x" NOT NULL, "x1" NOT NULL ]
+| | [ [ tinyint(2) "1", tinyint(2) "2", tinyint(2) "3" ] as "x"."x" ]
+| ) [ "x"."x" NOT NULL ] [ "x"."x" NOT NULL, "x"."x" NOT NULL as "x1" ]
) [ "x"."x" NOT NULL UNIQUE as "x", "x1" NOT NULL ]
query II rowsort
diff --git a/sql/test/rel-optimizers/optimize-proj/Tests/groupby-cse.test
b/sql/test/rel-optimizers/optimize-proj/Tests/groupby-cse.test
new file mode 100644
--- /dev/null
+++ b/sql/test/rel-optimizers/optimize-proj/Tests/groupby-cse.test
@@ -0,0 +1,502 @@
+statement ok
+create table foo (c1 int, c2 int, c3 int)
+
+statement ok
+INSERT INTO
+ foo
+VALUES
+ (1,1,13),
+ (1,2,17),
+ (2,1,11),
+ (1,1,15)
+
+# bar tables is required for the examples that uses join ops
+statement ok
+create table bar (c1 int, c2 int, c3 int)
+
+statement ok
+INSERT INTO
+ bar
+VALUES
+ (10,1,13),
+ (10,2,17),
+ (20,2,11),
+ (10,3,16)
+
+## GROUPBY <--input-- BASETABLE ########################################
+
+# the same expression should be removed from the GROUPBY op list of
+# group expressions (->l)
+query T nosort
+PLAN
+SELECT
+ c1,
+ c1,
+ count(c3)
+FROM
+ foo
+GROUP BY
+ c1,
+ c1
+----
+project (
+| group by (
+| | table("sys"."foo") [ "foo"."c1", "foo"."c3" ]
+| ) [ "foo"."c1" ] [ "foo"."c1", "sys"."count" no nil ("foo"."c3") NOT NULL as
"%1"."%1" ]
+) [ "foo"."c1" UNIQUE, "foo"."c1" UNIQUE, "%1"."%1" NOT NULL ]
+
+query III nosort gb-over-bt
+SELECT
+ c1,
+ c1,
+ count(c3)
+FROM
+ foo
+GROUP BY
+ c1,
+ c1
+----
+1
+1
+3
+2
+2
+1
+
+# same as before
+query T nosort
+PLAN
+SELECT
+ c1 AS c1_alias1,
+ c1 AS c1_alias2,
+ count(c3)
+FROM
+ foo
+GROUP BY
+ c1_alias1,
+ c1_alias2
+----
+project (
+| group by (
+| | table("sys"."foo") [ "foo"."c1", "foo"."c3" ]
+| ) [ "foo"."c1" as "c1_alias1" ] [ "c1_alias1", "c1_alias1" as "c1_alias2",
"sys"."count" no nil ("foo"."c3") NOT NULL as "%1"."%1" ]
+) [ "c1_alias1" UNIQUE, "c1_alias2", "%1"."%1" NOT NULL ]
+
+query III rowsort gb-over-bt
+SELECT
+ c1 AS c1_alias1,
+ c1 AS c1_alias2,
+ count(c3)
+FROM
+ foo
+GROUP BY
+ c1_alias1,
+ c1_alias2
+----
+1
+1
+3
+2
+2
+1
+
+## GROUPBY <--input-- TABLE ############################################
+
+statement ok
+CREATE FUNCTION tableReturnUDF ()
+RETURNS TABLE(a1 int, a2 int, a3 int)
+BEGIN
+ RETURN SELECT c1, c2, c3 FROM foo;
+END;
+
+# the same expression should be removed from the GROUPBY op list of
+# group expressions (->l) exactly like in the case of basetable
+query T nosort
+PLAN
+SELECT
+ a1,
+ a1,
+ count(a3)
+FROM
+ tableReturnUDF()
+GROUP BY
+ a1,
+ a1
+----
+project (
+| group by (
+| | table ("sys"."tablereturnudf"(),
+| | ) [ "%1"."a1", "%1"."a2", "%1"."a3" ]
+| ) [ "%1"."a1" ] [ "%1"."a1", "sys"."count" no nil ("%1"."a3") NOT NULL as
"%2"."%2" ]
+) [ "%1"."a1" UNIQUE, "%1"."a1" UNIQUE, "%2"."%2" NOT NULL ]
+
+query III rowsort
+SELECT
+ a1,
+ a1,
+ count(a3)
+FROM
+ tableReturnUDF()
+GROUP BY
+ a1,
+ a1
+----
+1
+1
+3
+2
+2
+1
+
+## GROUPBY <--input-- PROJECT ##########################################
+
+## in this example the order by groups are aliases of the same column
+## coming from the inner projection operator
+query T nosort
+PLAN
+SELECT
+ c1_alias1,
+ c1_alias2,
+ c2,
+ count(c1_alias2)
+FROM (
+ SELECT
+ c1 AS c1_alias1,
_______________________________________________
checkin-list mailing list -- [email protected]
To unsubscribe send an email to [email protected]
