From 2185c8a88644e627645356094d69f0c7f1ce8258 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Thu, 23 Jun 2016 11:33:01 +1200 Subject: [PATCH] #752 - Refactor: SQL generation for @History support with Postgres, MySql (views/triggers based) such that it more closely aligns with SQL 2011 based SQL --- .../config/dbplatform/DbHistorySupport.java | 10 +++-- .../dbplatform/DbStandardHistorySupport.java | 5 +-- .../dbplatform/DbViewHistorySupport.java | 5 +-- .../com/avaje/ebeaninternal/api/SpiQuery.java | 8 ++-- .../server/core/InternalConfiguration.java | 2 +- .../server/core/OrmQueryRequest.java | 7 ++++ .../ebeaninternal/server/persist/Binder.java | 14 +++---- .../server/query/CQueryBuilder.java | 42 +++++++++---------- .../server/query/CQueryEngine.java | 2 +- .../server/query/CQueryHistorySupport.java | 7 ++-- .../server/query/CQueryPredicates.java | 18 ++------ .../server/query/DefaultDbSqlContext.java | 28 ++++++------- .../server/query/SqlTreeBuilder.java | 2 +- .../server/query/SqlTreeNodeBean.java | 11 +++-- .../server/querydefn/DefaultOrmQuery.java | 20 +++------ .../dbplatform/MySqlHistorySupportTest.java | 32 ++++++++++++++ .../PostgresHistorySupportTest.java | 38 +++++++++++++++++ 17 files changed, 148 insertions(+), 103 deletions(-) create mode 100644 src/test/java/com/avaje/ebean/config/dbplatform/MySqlHistorySupportTest.java create mode 100644 src/test/java/com/avaje/ebean/config/dbplatform/PostgresHistorySupportTest.java diff --git a/src/main/java/com/avaje/ebean/config/dbplatform/DbHistorySupport.java b/src/main/java/com/avaje/ebean/config/dbplatform/DbHistorySupport.java index 173b4ba93..f9dc35fdd 100644 --- a/src/main/java/com/avaje/ebean/config/dbplatform/DbHistorySupport.java +++ b/src/main/java/com/avaje/ebean/config/dbplatform/DbHistorySupport.java @@ -6,11 +6,13 @@ package com.avaje.ebean.config.dbplatform; public interface DbHistorySupport { /** - * Return true if the 'As of' predicate is part of the from clause - * (more standard sql2011). So true for Oracle total recall and false - * for Postgres and MySql (where we use views and history tables). + * Return true if the implementation is SQL2011 standards based. + *

+ * Non standards based means we need to add additional predicates into the + * JOIN ON clause and add an additional predicate for the base table. + *

*/ - boolean isBindWithFromClause(); + boolean isStandardsBased(); /** * Return the number of columns bound in a 'As Of' predicate. diff --git a/src/main/java/com/avaje/ebean/config/dbplatform/DbStandardHistorySupport.java b/src/main/java/com/avaje/ebean/config/dbplatform/DbStandardHistorySupport.java index 6d98b48c9..b2935d4d3 100644 --- a/src/main/java/com/avaje/ebean/config/dbplatform/DbStandardHistorySupport.java +++ b/src/main/java/com/avaje/ebean/config/dbplatform/DbStandardHistorySupport.java @@ -5,11 +5,8 @@ package com.avaje.ebean.config.dbplatform; */ public abstract class DbStandardHistorySupport implements DbHistorySupport { - /** - * Return true as with sql2011 the 'as of timestamp' clause included in from or join clause. - */ @Override - public boolean isBindWithFromClause() { + public boolean isStandardsBased() { return true; } diff --git a/src/main/java/com/avaje/ebean/config/dbplatform/DbViewHistorySupport.java b/src/main/java/com/avaje/ebean/config/dbplatform/DbViewHistorySupport.java index 9d9f0a7d6..fdabd33d9 100644 --- a/src/main/java/com/avaje/ebean/config/dbplatform/DbViewHistorySupport.java +++ b/src/main/java/com/avaje/ebean/config/dbplatform/DbViewHistorySupport.java @@ -9,11 +9,8 @@ package com.avaje.ebean.config.dbplatform; */ public abstract class DbViewHistorySupport implements DbHistorySupport { - /** - * Return false for view based implementations where we append extra 'as of' predicates to the end. - */ @Override - public boolean isBindWithFromClause() { + public boolean isStandardsBased() { return false; } diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java b/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java index d8020a540..5cb45d3e0 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiQuery.java @@ -223,14 +223,14 @@ public interface SpiQuery extends Query { Timestamp getAsOf(); /** - * Add a table alias for a @History entity involved in a 'As Of' query. + * Increment the counter of tables used in 'As Of' query. */ - void addAsOfTableAlias(String tableAlias); + void incrementAsOfTableCount(); /** - * Return the list of table alias involved in a 'As Of' query that have @History support. + * Return the table alias used for the base table. */ - List getAsOfTableAlias(); + int getAsOfTableCount(); void addSoftDeletePredicate(String softDeletePredicate); diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/InternalConfiguration.java b/src/main/java/com/avaje/ebeaninternal/server/core/InternalConfiguration.java index bba89a1c1..45eceb8a4 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/InternalConfiguration.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/InternalConfiguration.java @@ -235,7 +235,7 @@ public class InternalConfiguration { if (historySupport == null) { return new Binder(typeManager, 0, false, jsonHandler, dataTimeZone); } - return new Binder(typeManager, historySupport.getBindCount(), historySupport.isBindWithFromClause(), jsonHandler, dataTimeZone); + return new Binder(typeManager, historySupport.getBindCount(), historySupport.isStandardsBased(), jsonHandler, dataTimeZone); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/com/avaje/ebeaninternal/server/core/OrmQueryRequest.java index 754637407..bfaf3207c 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/OrmQueryRequest.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/OrmQueryRequest.java @@ -513,4 +513,11 @@ public final class OrmQueryRequest extends BeanRequest implements BeanQueryRe public boolean isAuditReads() { return !query.isDisableReadAudit() && beanDescriptor.isReadAuditing(); } + + /** + * Return the base table alias for this query. + */ + public String getBaseTableAlias() { + return query.getAlias() == null ? beanDescriptor.getBaseTableAlias() : query.getAlias(); + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/persist/Binder.java b/src/main/java/com/avaje/ebeaninternal/server/persist/Binder.java index 2de9ee82a..fec91ab45 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/persist/Binder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/persist/Binder.java @@ -33,7 +33,7 @@ public class Binder { private final int asOfBindCount; - private final boolean bindAsOfWithFromClause; + private final boolean asOfStandardsBased; private final DbExpressionHandler dbExpressionHandler; @@ -42,12 +42,12 @@ public class Binder { /** * Set the PreparedStatement with which to bind variables to. */ - public Binder(TypeManager typeManager, int asOfBindCount, boolean bindAsOfWithFromClause, + public Binder(TypeManager typeManager, int asOfBindCount, boolean asOfStandardsBased, DbExpressionHandler dbExpressionHandler, DataTimeZone dataTimeZone) { this.typeManager = typeManager; this.asOfBindCount = asOfBindCount; - this.bindAsOfWithFromClause = bindAsOfWithFromClause; + this.asOfStandardsBased = asOfStandardsBased; this.dbExpressionHandler = dbExpressionHandler; this.dataTimeZone = dataTimeZone; } @@ -60,12 +60,10 @@ public class Binder { } /** - * Return true if the 'as of' predicates are in the from/join clause in which case the timestamp is - * bound early (before all the other predicates ala Oracle). Return false if the 'as of' predicates are - * appended to the end of the predicates and the timestamp is bound last (Postgres, MySql). + * Return true if the 'as of' history support is SQL2011 standards based. */ - public boolean isBindAsOfWithFromClause() { - return bindAsOfWithFromClause; + public boolean isAsOfStandardsBased() { + return asOfStandardsBased; } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java index 4fd0a8f15..28dbacc7f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java @@ -472,12 +472,14 @@ public class CQueryBuilder { hasWhere = true; } + int asOfCount = query.getAsOfTableCount(); + if (asOfCount > 0 && !historySupport.isStandardsBased()) { + hasWhere = appendWhere(hasWhere, sb); + sb.append(historySupport.getAsOfPredicate(request.getBaseTableAlias())); + } + if (request.isFindById() || query.getId() != null) { - if (hasWhere) { - sb.append(" and "); - } else { - sb.append(" where "); - } + appendWhere(hasWhere, sb); BeanDescriptor desc = request.getBeanDescriptor(); String idSql = desc.getIdBinderIdSql(); @@ -515,24 +517,6 @@ public class CQueryBuilder { sb.append(dbFilterMany); } - List asOfTableAlias = query.getAsOfTableAlias(); - if (asOfTableAlias != null && !historySupport.isBindAtFromClause()) { - // append the effective date predicates for each table alias - // that maps to a @History entity involved in this query - // Do this when history using separate tables/views (PG, MySql etc) - if (!hasWhere) { - sb.append(" where "); - } else { - sb.append("and "); - } - for (int i = 0; i < asOfTableAlias.size(); i++) { - if (i > 0) { - sb.append(" and "); - } - sb.append(historySupport.getAsOfPredicate(asOfTableAlias.get(i))); - } - } - if (!query.isIncludeSoftDeletes()) { List softDeletePredicates = query.getSoftDeletePredicates(); if (softDeletePredicates != null) { @@ -565,6 +549,18 @@ public class CQueryBuilder { } + /** + * Append where or and based on the hasWhere flag. + */ + private boolean appendWhere(boolean hasWhere, StringBuilder sb) { + if (hasWhere) { + sb.append(" and "); + } else { + sb.append(" where "); + } + return true; + } + /** * Convert the dbOrderBy clause to be safe for adding to select. This is done when 'distinct' is * used. diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java index 38719d274..839cb47df 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryEngine.java @@ -206,7 +206,7 @@ public class CQueryEngine { SpiQuery query = request.getQuery(); String sysPeriodLower = getSysPeriodLower(query); - if (query.isVersionsBetween() && !historySupport.isBindAtFromClause()) { + if (query.isVersionsBetween() && !historySupport.isStandardsBased()) { // just add as normal predicates using the lower bound query.where().gt(sysPeriodLower, query.getVersionStart()); query.where().lt(sysPeriodLower, query.getVersionEnd()); diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryHistorySupport.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryHistorySupport.java index 4f30e4929..3b5e8e683 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryHistorySupport.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryHistorySupport.java @@ -31,11 +31,10 @@ public class CQueryHistorySupport { } /** - * Return true if the bind of 'as of' timestamp occurs with the from clause - * rather than at the end. + * Return true if the underlying history support is standards based. */ - public boolean isBindAtFromClause() { - return dbHistorySupport.isBindWithFromClause(); + public boolean isStandardsBased() { + return dbHistorySupport.isStandardsBased(); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java index 0393ce572..467743f09 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java @@ -126,7 +126,7 @@ public class CQueryPredicates { updateProperties.bind(binder, dataBind); } - if (query.isVersionsBetween() && binder.isBindAsOfWithFromClause()) { + if (query.isVersionsBetween() && binder.isAsOfStandardsBased()) { // sql2011 based versions between timestamp syntax Timestamp start = query.getVersionStart(); Timestamp end = query.getVersionEnd(); @@ -136,13 +136,13 @@ public class CQueryPredicates { dataBind.append(", "); } - List historyTableAlias = query.getAsOfTableAlias(); - if (historyTableAlias != null && binder.isBindAsOfWithFromClause()) { + int asOfTableCount = query.getAsOfTableCount(); + if (asOfTableCount > 0) { // bind the asOf value for each table alias as part of the from/join clauses // there is one effective date predicate per table alias Timestamp asOf = query.getAsOf(); dataBind.append("asOf ").append(asOf); - for (int i = 0; i < historyTableAlias.size() * binder.getAsOfBindCount(); i++) { + for (int i = 0; i < asOfTableCount * binder.getAsOfBindCount(); i++) { binder.bindObject(dataBind, asOf); } dataBind.append(", "); @@ -167,16 +167,6 @@ public class CQueryPredicates { filterMany.bind(dataBind); } - if (historyTableAlias != null && !binder.isBindAsOfWithFromClause()) { - // bind the asOf value for each table alias after all the normal predicates - // there is one effective date predicate per table alias - Timestamp asOf = query.getAsOf(); - dataBind.append(" asOf ").append(asOf); - for (int i = 0; i < historyTableAlias.size() * binder.getAsOfBindCount(); i++) { - binder.bindObject(dataBind, asOf); - } - } - if (having != null) { having.bind(dataBind); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java b/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java index 8b5795136..b105c868b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java @@ -106,8 +106,8 @@ public class DefaultDbSqlContext implements DbSqlContext { tableJoins.add(joinKey); - sb.append(" "); - sb.append(type); + sb.append(" ").append(type); + boolean addAsOfOnClause = false; if (draftSupport != null) { appendTable(table, draftSupport.getDraftTable(table)); @@ -117,32 +117,32 @@ public class DefaultDbSqlContext implements DbSqlContext { } else { // check if there is an associated history table and if so // use the unionAll view - we expect an additional predicate to match - appendTable(table, historySupport.getAsOfView(table)); + String asOfView = historySupport.getAsOfView(table); + appendTable(table, asOfView); + if (asOfView != null) { + addAsOfOnClause = !historySupport.isStandardsBased(); + } } sb.append(a2); sb.append(" on "); - for (int i = 0; i < cols.length; i++) { TableJoinColumn pair = cols[i]; if (i > 0) { sb.append(" and "); } - - sb.append(a2); - sb.append(".").append(pair.getForeignDbColumn()); + sb.append(a2).append(".").append(pair.getForeignDbColumn()); sb.append(" = "); - sb.append(a1); - sb.append(".").append(pair.getLocalDbColumn()); + sb.append(a1).append(".").append(pair.getLocalDbColumn()); } - // add on any inheritance where clause if (inheritance != null && inheritance.length() > 0) { - sb.append(" and "); - sb.append(a2); - sb.append("."); - sb.append(inheritance); + sb.append(" and ").append(a2).append(".").append(inheritance); + } + + if (addAsOfOnClause) { + sb.append(" and ").append(historySupport.getAsOfPredicate(a2)); } sb.append(" "); diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java index 93eaf9087..0c86a253b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java @@ -111,7 +111,7 @@ public class SqlTreeBuilder { this.queryDetail = query.getDetail(); this.predicates = predicates; - this.alias = new SqlTreeAlias(request.getQuery().getAlias() == null ? request.getBeanDescriptor().getBaseTableAlias() : request.getQuery().getAlias()); + this.alias = new SqlTreeAlias(request.getBaseTableAlias()); this.ctx = new DefaultDbSqlContext(alias, tableAliasPlaceHolder, columnAliasPrefix, !subQuery, historySupport, draftSupport); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java index 0e04d693c..8829c10cb 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java @@ -81,7 +81,7 @@ public class SqlTreeNodeBean implements SqlTreeNode { * Table alias set if this bean node includes a join to a intersection * table and that table has history support. */ - protected String intersectionAsOfTableAlias; + private boolean intersectionAsOfTableAlias; /** * Construct for Raw SQL. @@ -501,11 +501,10 @@ public class SqlTreeNodeBean implements SqlTreeNode { // if history on this bean type add it's alias // for each alias we add an effect date predicate if (desc.isHistorySupport()) { - query.addAsOfTableAlias(baseTableAlias); + query.incrementAsOfTableCount(); } - if (intersectionAsOfTableAlias != null) { - // adds the 'as of' predicate for this intersection table - query.addAsOfTableAlias(intersectionAsOfTableAlias); + if (intersectionAsOfTableAlias) { + query.incrementAsOfTableCount(); } for (int i = 0; i < children.length; i++) { children[i].addAsOfTableAlias(query); @@ -531,7 +530,7 @@ public class SqlTreeNodeBean implements SqlTreeNode { TableJoin manyToManyJoin = manyProp.getIntersectionTableJoin(); manyToManyJoin.addJoin(joinType, parentAlias, alias2, ctx); if (!manyProp.isExcludedFromHistory()) { - intersectionAsOfTableAlias = alias2; + intersectionAsOfTableAlias = true; } return nodeBeanProp.addJoin(joinType, alias2, alias, ctx); diff --git a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java index ec89dbbff..9760e23c5 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/com/avaje/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -144,10 +144,7 @@ public class DefaultOrmQuery implements SpiQuery { private DefaultExpressionList havingExpressions; - /** - * The list of table alias associated with @History entity beans. - */ - private List asOfTableAlias; + private int asOfTableCount; /** * Set for flashback style 'as of' query. @@ -281,21 +278,14 @@ public class DefaultOrmQuery implements SpiQuery { return softDeletePredicates; } - /** - * This table alias is for a @History entity involved in the query and as - * such we need to add a 'as of predicate' to the query using this alias. - */ @Override - public void addAsOfTableAlias(String tableAlias) { - if (asOfTableAlias == null) { - asOfTableAlias = new ArrayList(); - } - asOfTableAlias.add(tableAlias); + public void incrementAsOfTableCount() { + asOfTableCount++; } @Override - public List getAsOfTableAlias() { - return asOfTableAlias; + public int getAsOfTableCount() { + return asOfTableCount; } @Override diff --git a/src/test/java/com/avaje/ebean/config/dbplatform/MySqlHistorySupportTest.java b/src/test/java/com/avaje/ebean/config/dbplatform/MySqlHistorySupportTest.java new file mode 100644 index 000000000..53f03710d --- /dev/null +++ b/src/test/java/com/avaje/ebean/config/dbplatform/MySqlHistorySupportTest.java @@ -0,0 +1,32 @@ +package com.avaje.ebean.config.dbplatform; + + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class MySqlHistorySupportTest { + + private MySqlHistorySupport support = new MySqlHistorySupport(); + + @Test + public void getAsOfPredicate() { + + String asOfPredicate = support.getAsOfPredicate("t0", "sys_period"); + assertEquals(asOfPredicate, "(t0.sys_period_start <= ? and (t0.sys_period_end is null or t0.sys_period_end > ?))"); + } + + @Test + public void getLower() throws Exception { + + String lower = support.getSysPeriodLower("t0", "sys_period"); + assertEquals(lower, "t0.sys_period_start"); + } + + @Test + public void getUpper() throws Exception { + + String upper = support.getSysPeriodUpper("t0", "sys_period"); + assertEquals(upper, "t0.sys_period_end"); + } +} \ No newline at end of file diff --git a/src/test/java/com/avaje/ebean/config/dbplatform/PostgresHistorySupportTest.java b/src/test/java/com/avaje/ebean/config/dbplatform/PostgresHistorySupportTest.java new file mode 100644 index 000000000..79a87069f --- /dev/null +++ b/src/test/java/com/avaje/ebean/config/dbplatform/PostgresHistorySupportTest.java @@ -0,0 +1,38 @@ +package com.avaje.ebean.config.dbplatform; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class PostgresHistorySupportTest { + + private PostgresHistorySupport support = new PostgresHistorySupport(); + + @Test + public void getBindCount() throws Exception { + + assertEquals(support.getBindCount(), 1); + } + + @Test + public void getAsOfPredicate() throws Exception { + + String asOfPredicate = support.getAsOfPredicate("t0", "sys_period"); + assertEquals(asOfPredicate, "t0.sys_period @> ?::timestamptz"); + } + + @Test + public void getSysPeriodLower() throws Exception { + + String lower = support.getSysPeriodLower("t0", "sys_period"); + assertEquals(lower, "lower(t0.sys_period)"); + } + + @Test + public void getSysPeriodUpper() throws Exception { + + String upper = support.getSysPeriodUpper("t0", "sys_period"); + assertEquals(upper, "upper(t0.sys_period)"); + } + +} \ No newline at end of file