#3552 Column alias required for SQL Server + findCount + distinct (#3553)

* #3552 Column alias required for SQL Server + findCount + distinct

- Modifies CQueryBuilder with the main fix which is to use the database platform selectCountWithColumnAlias for all distinct queries [and later disable it for single column case]

- Modifies SqlTreeBuilder because we need to NOT use the column alias for the count distinct SINGLE COLUMN case. So change here to:
  - build the root SqlTreeNode first as part of constructor
  - use rootNode.isSingleProperty() to NOT use column alias for the single column case
  - Hence need to initialise DefaultDbSqlContext AFTER the root SqlTreeNode

  - Modifies DefaultDbSqlContext with no effective change, just columnIndex++ ONLY when we are actually using column alias

 - Modifies H2Platform to also use selectCountWithColumnAlias true

* #3552 selectCountWithColumnAlias true for MariaDB and MySql

Plus fix TestRowCount for Oracle platform
This commit is contained in:
Rob Bygrave
2025-02-08 19:33:03 +13:00
committed by GitHub
parent c38c75fdec
commit 60aa899a34
7 changed files with 47 additions and 16 deletions
@@ -238,12 +238,14 @@ final class CQueryBuilder {
query.setMaxRows(0);
boolean countDistinct = query.isDistinct();
boolean useColumnAlias = selectCountWithColumnAlias;
boolean withAgg = false;
if (!countDistinct) {
withAgg = includesAggregation(request, query);
if (!withAgg) {
// minimise select clause for standard count
query.setSelectId();
useColumnAlias = false;
}
}
@@ -256,7 +258,7 @@ final class CQueryBuilder {
}
predicates.prepare(true);
SqlTree sqlTree = createSqlTree(request, predicates, selectCountWithColumnAlias && withAgg);
SqlTree sqlTree = createSqlTree(request, predicates, useColumnAlias);
if (SpiQuery.TemporalMode.CURRENT == query.temporalMode()) {
sqlTree.addSoftDeletePredicate(query);
}
@@ -295,9 +295,8 @@ final class DefaultDbSqlContext implements DbSqlContext {
if (useColumnAlias) {
sb.append(' ');
sb.append(columnAliasPrefix);
sb.append(columnIndex);
sb.append(columnIndex++);
}
columnIndex++;
}
@Override
@@ -47,7 +47,7 @@ public final class SqlTreeBuilder {
private final boolean disableLazyLoad;
private final boolean readOnly;
private final SpiQuery.TemporalMode temporalMode;
private SqlTreeNode rootNode;
private final SqlTreeNode rootNode;
private boolean sqlDistinct;
private final boolean platformDistinctNoLobs;
private final SqlTreeCommon common;
@@ -73,6 +73,7 @@ public final class SqlTreeBuilder {
this.alias = null;
this.ctx = null;
this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, readOnly, null);
this.rootNode = buildRootNode(desc);
}
/**
@@ -100,12 +101,13 @@ public final class SqlTreeBuilder {
this.alias = new SqlTreeAlias(request.baseTableAlias(), temporalMode);
this.distinctOnPlatform = builder.isPlatformDistinctOn();
this.platformDistinctNoLobs = builder.isPlatformDistinctNoLobs();
this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, readOnly, includeJoin);
this.rootNode = buildRootNode(desc);
String fromForUpdate = builder.fromForUpdate(query);
CQueryHistorySupport historySupport = builder.historySupport(query);
CQueryDraftSupport draftSupport = builder.draftSupport(query);
String colAlias = subQuery ? null : columnAliasPrefix;
String colAlias = subQuery || rootNode.isSingleProperty() ? null : columnAliasPrefix;
this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate);
this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, readOnly, includeJoin);
}
/**
@@ -113,7 +115,6 @@ public final class SqlTreeBuilder {
*/
public SqlTree build() {
// build the appropriate chain of SelectAdapter's
buildRoot(desc);
// build the actual String
String distinctOn = null;
String selectSql = null;
@@ -214,8 +215,8 @@ public final class SqlTreeBuilder {
return ctx.content();
}
private void buildRoot(STreeType desc) {
rootNode = buildSelectChain(null, null, desc, null);
private SqlTreeNode buildRootNode(STreeType desc) {
SqlTreeNode root = buildSelectChain(null, null, desc, null);
if (!rawSql) {
alias.addJoin(queryDetail.getFetchPaths(), desc);
alias.addJoin(predicates.predicateIncludes(), desc);
@@ -224,6 +225,7 @@ public final class SqlTreeBuilder {
alias.buildAlias();
predicates.parseTableAlias(alias);
}
return root;
}
/**
@@ -14,10 +14,10 @@ import java.util.List;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class TestRowCount extends BaseTestCase {
class TestRowCount extends BaseTestCase {
@Test
public void test() {
void test() {
ResetBasicData.reset();
LoggedSql.start();
@@ -46,7 +46,7 @@ public class TestRowCount extends BaseTestCase {
}
@Test
public void find_count_distinct_singleProperty() {
void find_count_distinct_singleProperty() {
ResetBasicData.reset();
Query<Customer> query = DB.find(Customer.class)
@@ -57,13 +57,13 @@ public class TestRowCount extends BaseTestCase {
int count = query.findCount();
assertThat(sqlOf(query)).contains("select count(distinct t0.anniversary) from o_customer t0 where t0.status = ?");
// never see column alias used with count distinct on single column
assertThat(query.getGeneratedSql()).contains("select count(distinct t0.anniversary) from o_customer t0 where t0.status = ?");
assertThat(count).isGreaterThan(0);
}
@Test
public void find_count_distinct_multipleProperties() {
void find_count_distinct_multipleProperties() {
ResetBasicData.reset();
Query<Customer> query = DB.find(Customer.class)
@@ -72,7 +72,32 @@ public class TestRowCount extends BaseTestCase {
int count = query.findCount();
assertThat(sqlOf(query)).contains("select count(*) from ( select distinct t0.anniversary, t0.status from o_customer t0)");
if (isSqlServer() || isH2() || isMariaDB() || isMySql()) {
assertThat(query.getGeneratedSql()).contains("select count(*) from ( select distinct t0.anniversary c0, t0.status c1 from o_customer t0)");
} else {
assertThat(query.getGeneratedSql()).contains("select count(*) from ( select distinct t0.anniversary, t0.status from o_customer t0)");
}
assertThat(count).isGreaterThan(0);
}
@Test
void find_count_distinct_multiplePropertiesSameColumn() {
ResetBasicData.reset();
Query<Customer> query = DB.find(Customer.class)
.setDistinct(true)
.select("id, anniversary, status")
.fetch("billingAddress", "id, city")
.fetch("shippingAddress", "id, city");
int count = query.findCount();
if (isSqlServer() || isH2() || isMariaDB() || isMySql()) {
// must use column alias
assertThat(query.getGeneratedSql()).contains("select count(*) from ( select distinct t0.anniversary c0, t0.status c1, t1.id c2, t1.city c3, t2.id c4, t2.city c5 from o_customer t0 left join o_address t1 on t1.id = t0.billing_address_id left join o_address t2 on t2.id = t0.shipping_address_id)");
} else {
assertThat(query.getGeneratedSql()).contains("select count(*) from ( select distinct t0.anniversary, t0.status, t1.id, t1.city, t2.id, t2.city from o_customer t0 left join o_address t1 on t1.id = t0.billing_address_id left join o_address t2 on t2.id = t0.shipping_address_id)");
}
assertThat(count).isGreaterThan(0);
}
@@ -24,6 +24,7 @@ public class H2Platform extends DatabasePlatform {
this.dbEncrypt = new H2DbEncrypt();
this.historySupport = new H2HistorySupport();
this.nativeUuidType = true;
this.selectCountWithColumnAlias = true;
this.supportsDeleteTableAlias = true;
this.inlineSqlUpdateLimit = true;
this.dbDefaultValue.setNow("now()");
@@ -16,6 +16,7 @@ public class MariaDbPlatform extends BaseMySqlPlatform {
super();
this.platform = Platform.MARIADB;
this.sequenceBatchMode = false;
this.selectCountWithColumnAlias = true;
// for MariaDB probably turn off forwardOnlyHintOnFindIterate with later driver
// this.forwardOnlyHintOnFindIterate = false;
this.historySupport = new MariaDbHistorySupport();
@@ -13,6 +13,7 @@ public abstract class BaseMySqlPlatform extends DatabasePlatform {
public BaseMySqlPlatform() {
super();
this.useExtraTransactionOnIterateSecondaryQueries = true;
this.selectCountWithColumnAlias = true;
this.selectCountWithAlias = true;
this.supportsSavepointId = false;
this.inlineSqlUpdateLimit = true;