From 4f4743807cb66a2676295819a62de6f10f9f598e Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Wed, 10 Apr 2019 01:25:43 +1200 Subject: [PATCH] #1671 - Use table alias with delete query for Postgres, Oracle, MySql --- .../config/dbplatform/DatabasePlatform.java | 17 +++++-- .../config/dbplatform/h2/H2Platform.java | 1 + .../dbplatform/oracle/OraclePlatform.java | 1 + .../dbplatform/postgres/PostgresPlatform.java | 1 + .../server/query/CQueryBuilder.java | 46 +++++++++++-------- src/test/java/io/ebean/BaseTestCase.java | 4 ++ .../IsEmptyExpressionQueryTest.java | 29 +++++++++++- .../org/tests/delete/TestDeleteByQuery.java | 23 ++++++++-- 8 files changed, 94 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java index 035673c10..16ab2592a 100644 --- a/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java @@ -49,6 +49,8 @@ public class DatabasePlatform { */ protected boolean useExtraTransactionOnIterateSecondaryQueries; + protected boolean supportsDeleteTableAlias; + /** * The behaviour used when ending a read only transaction at read committed isolation level. */ @@ -176,10 +178,10 @@ public class DatabasePlatform { * findIterate() and findVisit(). */ protected boolean forwardOnlyHintOnFindIterate; - + /** * If set then use the CONCUR_UPDATABLE hint when creating ResultSets. - * + * * This is {@code false} for HANA */ protected boolean supportsResultSetConcurrencyModeUpdatable = true; @@ -306,6 +308,13 @@ public class DatabasePlatform { return supportsNativeIlike; } + /** + * Return true if the platform supports delete statements with table alias. + */ + public boolean isSupportsDeleteTableAlias() { + return supportsDeleteTableAlias; + } + /** * Return the maximum table name length. *

@@ -524,7 +533,7 @@ public class DatabasePlatform { public void setForwardOnlyHintOnFindIterate(boolean forwardOnlyHintOnFindIterate) { this.forwardOnlyHintOnFindIterate = forwardOnlyHintOnFindIterate; } - + /** * Return true if the ResultSet CONCUR_UPDATABLE Hint should be used on * createNativeSqlTree() PreparedStatements. @@ -535,7 +544,7 @@ public class DatabasePlatform { public boolean isSupportsResultSetConcurrencyModeUpdatable() { return supportsResultSetConcurrencyModeUpdatable; } - + /** * Set to true if the ResultSet CONCUR_UPDATABLE Hint should be used by default on createNativeSqlTree() PreparedStatements. */ diff --git a/src/main/java/io/ebean/config/dbplatform/h2/H2Platform.java b/src/main/java/io/ebean/config/dbplatform/h2/H2Platform.java index 1b093d880..d3418904d 100644 --- a/src/main/java/io/ebean/config/dbplatform/h2/H2Platform.java +++ b/src/main/java/io/ebean/config/dbplatform/h2/H2Platform.java @@ -23,6 +23,7 @@ public class H2Platform extends DatabasePlatform { this.dbEncrypt = new H2DbEncrypt(); this.historySupport = new H2HistorySupport(); this.nativeUuidType = true; + this.supportsDeleteTableAlias = true; this.dbDefaultValue.setNow("now()"); this.columnAliasPrefix = null; diff --git a/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java b/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java index 3eae9a8c6..cedd2a481 100644 --- a/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/oracle/OraclePlatform.java @@ -23,6 +23,7 @@ public class OraclePlatform extends DatabasePlatform { public OraclePlatform() { super(); this.platform = Platform.ORACLE; + this.supportsDeleteTableAlias = true; this.maxTableNameLength = 30; this.maxConstraintNameLength = 30; this.dbEncrypt = new OracleDbEncrypt(); diff --git a/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java b/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java index c8266fbec..938f5b3b3 100644 --- a/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java +++ b/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java @@ -30,6 +30,7 @@ public class PostgresPlatform extends DatabasePlatform { super(); this.platform = Platform.POSTGRES; this.supportsNativeIlike = true; + this.supportsDeleteTableAlias = true; this.selectCountWithAlias = true; this.blobDbType = Types.LONGVARBINARY; this.clobDbType = Types.VARCHAR; diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java index 037f53ed1..6c1151815 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java @@ -135,14 +135,22 @@ class CQueryBuilder { private String buildDeleteSql(OrmQueryRequest request, String rootTableAlias, CQueryPredicates predicates, SqlTree sqlTree) { + String alias = alias(rootTableAlias); if (!sqlTree.isIncludeJoins()) { - // simple - delete from table ... - return aliasStrip(buildSql("delete", request, predicates, sqlTree).getSql()); + if (dbPlatform.isSupportsDeleteTableAlias()) { + // delete from table ... + return aliasReplace(buildSql("delete", request, predicates, sqlTree).getSql(), alias); + } else if (dbPlatform.getPlatform() == Platform.MYSQL) { + return aliasReplace(buildSql("delete " + alias, request, predicates, sqlTree).getSql(), alias); + } else { + // simple - delete from table ... + return aliasStrip(buildSql("delete", request, predicates, sqlTree).getSql()); + } } // wrap as - delete from table where id in (select id ...) String sql = buildSql(null, request, predicates, sqlTree).getSql(); sql = request.getBeanDescriptor().getDeleteByIdInSql() + "in (" + sql + ")"; - sql = aliasReplace(sql, alias(rootTableAlias)); + sql = aliasReplace(sql, alias); return sql; } @@ -425,7 +433,7 @@ class CQueryBuilder { try { // For SqlServer we need either "selectMethod=cursor" in the connection string or fetch explicitly a cursorable // statement here by specifying ResultSet.CONCUR_UPDATABLE - PreparedStatement statement = connection.prepareStatement(sql,ResultSet.TYPE_FORWARD_ONLY, dbPlatform.isSupportsResultSetConcurrencyModeUpdatable() ? ResultSet.CONCUR_UPDATABLE : ResultSet.CONCUR_READ_ONLY); + PreparedStatement statement = connection.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, dbPlatform.isSupportsResultSetConcurrencyModeUpdatable() ? ResultSet.CONCUR_UPDATABLE : ResultSet.CONCUR_READ_ONLY); predicates.bind(statement, connection); ResultSet resultSet = statement.executeQuery(); @@ -702,21 +710,21 @@ class CQueryBuilder { } private String toSql(CountDistinctOrder orderBy) { - switch(orderBy) { - case ATTR_ASC: - return " order by r1.attribute_"; - case ATTR_DESC: - return " order by r1.attribute_ desc"; - case COUNT_ASC_ATTR_ASC: - return " order by count(*), r1.attribute_"; - case COUNT_ASC_ATTR_DESC: - return " order by count(*), r1.attribute_ desc"; - case COUNT_DESC_ATTR_ASC: - return " order by count(*) desc, r1.attribute_"; - case COUNT_DESC_ATTR_DESC: - return " order by count(*) desc, r1.attribute_ desc"; - default: - throw new IllegalArgumentException("Illegal enum: "+ orderBy); + switch (orderBy) { + case ATTR_ASC: + return " order by r1.attribute_"; + case ATTR_DESC: + return " order by r1.attribute_ desc"; + case COUNT_ASC_ATTR_ASC: + return " order by count(*), r1.attribute_"; + case COUNT_ASC_ATTR_DESC: + return " order by count(*), r1.attribute_ desc"; + case COUNT_DESC_ATTR_ASC: + return " order by count(*) desc, r1.attribute_"; + case COUNT_DESC_ATTR_DESC: + return " order by count(*) desc, r1.attribute_ desc"; + default: + throw new IllegalArgumentException("Illegal enum: " + orderBy); } } diff --git a/src/test/java/io/ebean/BaseTestCase.java b/src/test/java/io/ebean/BaseTestCase.java index 7f0577c5b..17dc0efc1 100644 --- a/src/test/java/io/ebean/BaseTestCase.java +++ b/src/test/java/io/ebean/BaseTestCase.java @@ -170,6 +170,10 @@ public abstract class BaseTestCase { return isH2() || isPostgres(); } + public boolean isPlatformSupportsDeleteTableAlias() { + return spiEbeanServer().getDatabasePlatform().isSupportsDeleteTableAlias(); + } + public boolean isPersistBatchOnCascade() { return spiEbeanServer().getDatabasePlatform().getPersistBatchOnCascade() != PersistBatch.NONE; } diff --git a/src/test/java/io/ebeaninternal/server/expression/IsEmptyExpressionQueryTest.java b/src/test/java/io/ebeaninternal/server/expression/IsEmptyExpressionQueryTest.java index feb486d84..4d60b5442 100644 --- a/src/test/java/io/ebeaninternal/server/expression/IsEmptyExpressionQueryTest.java +++ b/src/test/java/io/ebeaninternal/server/expression/IsEmptyExpressionQueryTest.java @@ -3,10 +3,14 @@ package io.ebeaninternal.server.expression; import io.ebean.BaseTestCase; import io.ebean.Ebean; import io.ebean.Query; +import org.ebeantest.LoggedSqlCollector; +import org.junit.Test; import org.tests.model.basic.Contact; import org.tests.model.basic.Customer; import org.tests.model.basic.ResetBasicData; -import org.junit.Test; +import org.tests.model.nofk.EUserNoFk; + +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -26,6 +30,29 @@ public class IsEmptyExpressionQueryTest extends BaseTestCase { assertThat(sqlOf(query)).contains("select t0.id from o_customer t0 where not exists (select 1 from contact x where x.customer_id = t0.id"); } + @Test + public void deleteQuery_isEmpty() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + Ebean.find(EUserNoFk.class) + .where().isEmpty("files") + .delete(); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(1); + + if (isPlatformSupportsDeleteTableAlias()) { + assertThat(sql.get(0)).contains("delete from euser_no_fk t0 where not exists (select 1 from efile_no_fk x where x.owner_user_id = t0.user_id)"); + } else if (isMySql()) { + assertThat(sql.get(0)).contains("delete t0 from euser_no_fk t0 where not exists (select 1 from efile_no_fk x where x.owner_user_id = t0.user_id)"); + } else { + assertThat(sql.get(0)).contains("delete from euser_no_fk where not exists (select 1 from efile_no_fk x where x.owner_user_id = user_id)"); + } + } + @Test public void isNotEmpty() { diff --git a/src/test/java/org/tests/delete/TestDeleteByQuery.java b/src/test/java/org/tests/delete/TestDeleteByQuery.java index 60c9043a1..a28561b19 100644 --- a/src/test/java/org/tests/delete/TestDeleteByQuery.java +++ b/src/test/java/org/tests/delete/TestDeleteByQuery.java @@ -50,7 +50,13 @@ public class TestDeleteByQuery extends BaseTestCase { loggedSql = LoggedSqlCollector.stop(); assertThat(loggedSql).hasSize(1); - assertThat(loggedSql.get(0)).contains("delete from bbookmark_user where name ="); + if (isPlatformSupportsDeleteTableAlias()) { + assertThat(loggedSql.get(0)).contains("delete from bbookmark_user t0 where t0.name ="); + } else if (isMySql()){ + assertThat(loggedSql.get(0)).contains("delete t0 from bbookmark_user t0 where t0.name ="); + } else { + assertThat(loggedSql.get(0)).contains("delete from bbookmark_user where name ="); + } server.find(BBookmarkUser.class).select("id").where().eq("name", "NotARealFirstName").delete(); @@ -102,8 +108,13 @@ public class TestDeleteByQuery extends BaseTestCase { Ebean.find(BBookmarkUser.class).setId(7000).delete(); List sql = LoggedSqlCollector.stop(); - assertThat(sql.get(0)).contains("delete from bbookmark_user where id = ?"); - assertThat(sql.get(1)).contains("delete from bbookmark_user where id = ?"); + if (isPlatformSupportsDeleteTableAlias()) { + assertThat(sql.get(0)).contains("delete from bbookmark_user t0 where t0.id = ?"); + assertThat(sql.get(1)).contains("delete from bbookmark_user t0 where t0.id = ?"); + } else if (!isMySql()) { + assertThat(sql.get(0)).contains("delete from bbookmark_user where id = ?"); + assertThat(sql.get(1)).contains("delete from bbookmark_user where id = ?"); + } // and note this is the easiest option Ebean.delete(BBookmarkUser.class, 7000); @@ -140,7 +151,11 @@ public class TestDeleteByQuery extends BaseTestCase { } List sql = LoggedSqlCollector.stop(); - assertThat(sql.get(0)).contains("delete from contact where id = ?"); + if (isPlatformSupportsDeleteTableAlias()) { + assertThat(sql.get(0)).contains("delete from contact t0 where t0.id = ?"); + } else if (!isMySql()){ + assertThat(sql.get(0)).contains("delete from contact where id = ?"); + } } @Test