Skip to content

Commit

Permalink
MONDRIAN: Fixes failing tests in AggregationManager. Using an aggrega…
Browse files Browse the repository at this point in the history 
…tion table with a caption column would trigger a carthesian product of tuples to be returned because of a non-required group-by inclusion of the caption column. Also fixes an issue where an alias was not assigned to a column coming from an aggregate table when using native tuples. Also disables some aggregate table tests when native crossjoins are disabled, since those tests were never designed to work without native tuples resolution.

[git-p4: depot-paths = "//open/mondrian/": change = 14628]
  • Loading branch information
@lucboudreau
lucboudreau committed Sep 28, 2011
1 parent a3a5d9f commit ded843e
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 54 deletions.
12 changes: 5 additions & 7 deletions src/main/mondrian/rolap/SqlTupleReader.java
View file
Expand Up @@ -975,7 +975,11 @@ protected void addLevelMemberSql(
int bitPos = starColumn.getBitPosition();
AggStar.Table.Column aggColumn = aggStar.lookupColumn(bitPos);
String q = aggColumn.generateExprString(sqlQuery);
sqlQuery.addSelectGroupBy(q, starColumn.getInternalType());
if (needsGroupBy) {
sqlQuery.addSelectGroupBy(q, starColumn.getInternalType());
} else {
sqlQuery.addSelect(q, starColumn.getInternalType());
}
sqlQuery.addOrderBy(q, true, false, true);
aggColumn.getTable().addToFrom(sqlQuery, false, true);
continue;
Expand Down Expand Up @@ -1023,16 +1027,10 @@ protected void addLevelMemberSql(

if (captionSql != null) {
alias = sqlQuery.addSelect(captionSql, null);
if (needsGroupBy) {
sqlQuery.addGroupBy(captionSql, alias);
}
}

if (!ordinalSql.equals(keySql)) {
alias = sqlQuery.addSelect(ordinalSql, null);
if (needsGroupBy) {
sqlQuery.addGroupBy(ordinalSql, alias);
}
}

constraint.addLevelConstraint(
Expand Down
2 changes: 1 addition & 1 deletion src/main/mondrian/rolap/sql/SqlQuery.java
View file
Expand Up @@ -490,7 +490,7 @@ public String addSelectGroupBy(
final String expression,
SqlStatement.Type type)
{
final String alias = addSelect(expression, type, null);
final String alias = addSelect(expression, type);
addGroupBy(expression, alias);
return alias;
}
Expand Down
65 changes: 19 additions & 46 deletions testsrc/main/mondrian/rolap/TestAggregationManager.java
View file
Expand Up @@ -894,6 +894,9 @@ public void testAggMembers() {
{
return;
}
if (!(MondrianProperties.instance().EnableNativeCrossJoin.get())) {
return;
}
SqlPattern[] patterns = {
new SqlPattern(
Dialect.DatabaseProduct.ACCESS,
Expand All @@ -914,7 +917,7 @@ public void testAggMembers() {
+ "where `agg_c_14_sales_fact_1997`.`the_year` = 1998 "
+ "and `agg_c_14_sales_fact_1997`.`store_id` = `store`.`store_id` "
+ "group by `store`.`store_country` "
+ "order by ISNULL(`store`.`store_country`), `store`.`store_country` ASC",
+ "order by ISNULL(`store`.`store_country`) ASC, `store`.`store_country` ASC",
26)};

assertQuerySql(
Expand Down Expand Up @@ -1263,6 +1266,9 @@ public void testOrdinalExprAggTuplesAndChildren() {
{
return;
}
if (!(MondrianProperties.instance().EnableNativeCrossJoin.get())) {
return;
}
TestContext.instance().flushSchemaCache();

String cube = "<Cube name=\"Sales_Prod_Ord\">\n"
Expand Down Expand Up @@ -1317,44 +1323,7 @@ public void testOrdinalExprAggTuplesAndChildren() {
SqlPattern[] patterns = {
new SqlPattern(
ACCESS_MYSQL,
"select "
+ "`product_class`.`product_family` as `c0`, "
+ "`product_class`.`product_department` as `c1`, "
+ "`product_class`.`product_category` as `c2`, "
+ "`product_class`.`product_department` as `c3`, "
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender` as `c4` "
+ "from "
+ "`agg_g_ms_pcat_sales_fact_1997` as "
+ "`agg_g_ms_pcat_sales_fact_1997`, "
+ "`product_class` as `product_class` "
+ "where "
+ "`product_class`.`product_family` = "
+ "`agg_g_ms_pcat_sales_fact_1997`.`product_family` "
+ "and `product_class`.`product_department` = "
+ "`agg_g_ms_pcat_sales_fact_1997`.`product_department` "
+ "and `product_class`.`product_category` = "
+ "`agg_g_ms_pcat_sales_fact_1997`.`product_category` "
+ "and (`agg_g_ms_pcat_sales_fact_1997`.`product_category`"
+ " = 'Meat' "
+ "and "
+ "`agg_g_ms_pcat_sales_fact_1997`.`product_department`"
+ " = 'Deli' "
+ "and `agg_g_ms_pcat_sales_fact_1997`.`product_family`"
+ " = 'Food') "
+ "and (`agg_g_ms_pcat_sales_fact_1997`.`gender` = 'M') "
+ "group by "
+ "`product_class`.`product_family`, "
+ "`product_class`.`product_department`, "
+ "`product_class`.`product_category`, "
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender` "
+ "order by ISNULL(`product_class`.`product_family`), "
+ "`product_class`.`product_family` ASC, "
+ "ISNULL(`product_class`.`product_department`), "
+ "`product_class`.`product_department` ASC, "
+ "ISNULL(`product_class`.`product_category`), "
+ "`product_class`.`product_category` ASC, "
+ "ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`), "
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender` ASC",
"select `agg_g_ms_pcat_sales_fact_1997`.`product_family` as `c0`, `agg_g_ms_pcat_sales_fact_1997`.`product_department` as `c1`, `product_class`.`product_category` as `c2`, `product_class`.`product_department` as `c3`, `agg_g_ms_pcat_sales_fact_1997`.`gender` as `c4` from `agg_g_ms_pcat_sales_fact_1997` as `agg_g_ms_pcat_sales_fact_1997`, `product_class` as `product_class` where `product_class`.`product_category` = `agg_g_ms_pcat_sales_fact_1997`.`product_category` and (`agg_g_ms_pcat_sales_fact_1997`.`product_category` = 'Meat' and `agg_g_ms_pcat_sales_fact_1997`.`product_department` = 'Deli' and `agg_g_ms_pcat_sales_fact_1997`.`product_family` = 'Food') and (`agg_g_ms_pcat_sales_fact_1997`.`gender` = 'M') group by `agg_g_ms_pcat_sales_fact_1997`.`product_family`, `agg_g_ms_pcat_sales_fact_1997`.`product_department`, `product_class`.`product_category`, `agg_g_ms_pcat_sales_fact_1997`.`gender` order by ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`product_family`) ASC, `agg_g_ms_pcat_sales_fact_1997`.`product_family` ASC, ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`product_department`) ASC, `agg_g_ms_pcat_sales_fact_1997`.`product_department` ASC, ISNULL(`product_class`.`product_category`) ASC, `product_class`.`product_category` ASC, ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`) ASC, `agg_g_ms_pcat_sales_fact_1997`.`gender` ASC",
null)
};

Expand Down Expand Up @@ -1405,7 +1374,9 @@ public void testAggregatingTuples() {
{
return;
}

if (!(MondrianProperties.instance().EnableNativeCrossJoin.get())) {
return;
}
// flush cache, to be sure sql is executed
TestContext.instance().flushSchemaCache();

Expand Down Expand Up @@ -1436,9 +1407,9 @@ public void testAggregatingTuples() {
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender`, "
+ "`agg_g_ms_pcat_sales_fact_1997`.`marital_status` "
+ "order by "
+ "ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`), "
+ "ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`) ASC, "
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender` ASC, "
+ "ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`marital_status`), "
+ "ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`marital_status`) ASC, "
+ "`agg_g_ms_pcat_sales_fact_1997`.`marital_status` ASC",
null)
};
Expand Down Expand Up @@ -1480,9 +1451,9 @@ public void testAggregatingTuples() {
+ "group by "
+ "`store`.`store_country`, `store`.`store_state` "
+ "order by "
+ "ISNULL(`store`.`store_country`), "
+ "ISNULL(`store`.`store_country`) ASC, "
+ "`store`.`store_country` ASC, "
+ "ISNULL(`store`.`store_state`), "
+ "ISNULL(`store`.`store_state`) ASC, "
+ "`store`.`store_state` ASC",
null)
};
Expand Down Expand Up @@ -1514,7 +1485,9 @@ public void testCollapsedChildren() {
{
return;
}

if (!(MondrianProperties.instance().EnableNativeCrossJoin.get())) {
return;
}
// flush cache to be sure sql is executed
TestContext.instance().flushSchemaCache();

Expand All @@ -1527,7 +1500,7 @@ public void testCollapsedChildren() {
+ "as `agg_g_ms_pcat_sales_fact_1997` "
+ "group by "
+ "`agg_g_ms_pcat_sales_fact_1997`.`gender`"
+ " order by ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`), `agg_g_ms_pcat_sales_fact_1997`.`gender` ASC",
+ " order by ISNULL(`agg_g_ms_pcat_sales_fact_1997`.`gender`) ASC, `agg_g_ms_pcat_sales_fact_1997`.`gender` ASC",
null)
};

Expand Down

0 comments on commit ded843e

Please sign in to comment.