diff --git a/src/java/com/samskivert/jdbc/depot/BindVisitor.java b/src/java/com/samskivert/jdbc/depot/BindVisitor.java index 06ca6f1..cf46fa3 100644 --- a/src/java/com/samskivert/jdbc/depot/BindVisitor.java +++ b/src/java/com/samskivert/jdbc/depot/BindVisitor.java @@ -26,7 +26,7 @@ import java.util.Set; import com.samskivert.jdbc.depot.Key.WhereCondition; import com.samskivert.jdbc.depot.clause.DeleteClause; -import com.samskivert.jdbc.depot.clause.FieldOverride; +import com.samskivert.jdbc.depot.clause.FieldDefinition; import com.samskivert.jdbc.depot.clause.ForUpdate; import com.samskivert.jdbc.depot.clause.FromOverride; import com.samskivert.jdbc.depot.clause.GroupBy; @@ -63,7 +63,7 @@ public class BindVisitor implements ExpressionVisitor { } - public void visit (FieldOverride fieldOverride) + public void visit (FieldDefinition fieldOverride) throws Exception { } diff --git a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java index d3ea095..26e858d 100644 --- a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java +++ b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java @@ -29,6 +29,7 @@ import java.util.Set; import com.samskivert.jdbc.depot.Key.WhereCondition; import com.samskivert.jdbc.depot.annotation.Computed; import com.samskivert.jdbc.depot.clause.DeleteClause; +import com.samskivert.jdbc.depot.clause.FieldDefinition; import com.samskivert.jdbc.depot.clause.FieldOverride; import com.samskivert.jdbc.depot.clause.ForUpdate; import com.samskivert.jdbc.depot.clause.FromOverride; @@ -81,13 +82,13 @@ public abstract class BuildVisitor implements ExpressionVisitor } } - public void visit (FieldOverride fieldOverride) + public void visit (FieldDefinition definition) throws Exception { - fieldOverride.getOverride().accept(this); - if (_aliasFields) { + definition.getDefinition().accept(this); + if (_enableAliasing) { _builder.append(" as "); - appendIdentifier(fieldOverride.getField()); + appendIdentifier(definition.getField()); } } @@ -280,9 +281,7 @@ public abstract class BuildVisitor implements ExpressionVisitor _builder.append(" as "); appendTableAbbreviation(join.getJoinClass()); _builder.append(" on "); - _ignoreOverrides = true; join.getJoinCondition().accept(this); - _ignoreOverrides = false; } public void visit (Limit limit) @@ -315,23 +314,23 @@ public abstract class BuildVisitor implements ExpressionVisitor } _builder.append("select "); - if (_overrides.containsKey(pClass)) { + if (_definitions.containsKey(pClass)) { throw new IllegalArgumentException( "Can not yet nest SELECTs on the same persistent record."); } - Map overrideMap = new HashMap(); - for (FieldOverride override : selectClause.getFieldOverrides()) { - overrideMap.put(override.getField(), override); + Map definitionMap = new HashMap(); + for (FieldDefinition definition : selectClause.getFieldDefinitions()) { + definitionMap.put(definition.getField(), definition); } - _overrides.put(pClass, overrideMap); + _definitions.put(pClass, definitionMap); try { // iterate over the fields we're filling in and figure out whence each one comes boolean skip = true; - // while expanding column names in the SELECT query, do aliasing - _aliasFields = true; + // while expanding column names in the SELECT query, do aliasing and expansion + _enableAliasing = _enableOverrides = true; for (String field : selectClause.getFields()) { if (!skip) { @@ -349,7 +348,7 @@ public abstract class BuildVisitor implements ExpressionVisitor } // then stop - _aliasFields = false; + _enableAliasing = _enableOverrides = false; if (selectClause.getFromOverride() != null) { selectClause.getFromOverride().accept(this); @@ -384,7 +383,7 @@ public abstract class BuildVisitor implements ExpressionVisitor } } finally { - _overrides.remove(pClass); + _definitions.remove(pClass); } if (isInner) { _builder.append(")"); @@ -508,19 +507,36 @@ public abstract class BuildVisitor implements ExpressionVisitor "Unknown field on persistent record [record=" + type + ", field=" + field + "]"); } - if (!_ignoreOverrides) { - Map fieldOverrides = _overrides.get(type); - if (fieldOverrides != null) { - // first, see if there's a field override - FieldOverride override = fieldOverrides.get(field); - if (override != null) { + Map fieldOverrides = _definitions.get(type); + if (fieldOverrides != null) { + // first, see if there's a field override + FieldDefinition override = fieldOverrides.get(field); + + if (override != null) { + boolean useOverride; + if (override instanceof FieldOverride) { + if (fm.getComputed() != null || dm.getComputed() != null) { + throw new IllegalArgumentException( + "FieldOverride cannot be used on @Computed field: " + field); + } + useOverride = _enableOverrides; + } else if (fm.getComputed() == null && dm.getComputed() == null) { + throw new IllegalArgumentException( + "FieldDefinition must not be used on concrete field: " + field); + } else { + useOverride = true; + } + + if (useOverride) { // If a FieldOverride's target is in turn another FieldOverride, the second // one is ignored. As an example, when creating ItemRecords from CloneRecords, // we make Item.itemId = Clone.itemId. We also make Item.parentId = Item.itemId // and would be dismayed to find Item.parentID = Item.itemId = Clone.itemId. - _ignoreOverrides = true; + + boolean saved = _enableOverrides; + _enableOverrides = false; override.accept(this); - _ignoreOverrides = false; + _enableOverrides = saved; return; } } @@ -548,7 +564,7 @@ public abstract class BuildVisitor implements ExpressionVisitor // check if the computed field has a literal SQL definition if (fieldComputed.fieldDefinition().length() > 0) { _builder.append(fieldComputed.fieldDefinition()); - if (_aliasFields) { + if (_enableAliasing) { _builder.append(" as "); appendIdentifier(field); } @@ -590,13 +606,13 @@ public abstract class BuildVisitor implements ExpressionVisitor protected StringBuilder _builder = new StringBuilder(); /** A mapping of field overrides per persistent record. */ - protected Map, Map> _overrides = - new HashMap, Map>(); + protected Map, Map> _definitions= + new HashMap, Map>(); /** A flag that's set to true for inner SELECT's */ protected boolean _innerClause = false; - protected boolean _ignoreOverrides = false; + protected boolean _enableOverrides = false; - protected boolean _aliasFields = false; + protected boolean _enableAliasing = false; } diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java index 24e254e..172bb43 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java +++ b/src/java/com/samskivert/jdbc/depot/DepotRepository.java @@ -37,6 +37,8 @@ import com.samskivert.jdbc.JDBCUtil; import com.samskivert.jdbc.depot.Modifier.*; import com.samskivert.jdbc.depot.clause.DeleteClause; +import com.samskivert.jdbc.depot.clause.FieldOverride; +import com.samskivert.jdbc.depot.clause.FromOverride; import com.samskivert.jdbc.depot.clause.InsertClause; import com.samskivert.jdbc.depot.clause.QueryClause; import com.samskivert.jdbc.depot.clause.UpdateClause; @@ -147,6 +149,14 @@ public abstract class DepotRepository boolean useExplicit = (marsh.getTableName() == null) || !marsh.hasPrimaryKey() || !_ctx.isUsingCache(); + // TODO: The two-pass query still doesn't do well in circumstances where we do tricky + // TODO: things such as override the table from whence the primary key fields come, so + // TODO: somewhat conservatively we fall back on the traditional explicit approach on + // TODO: any query involving FromOverrides or FieldOverrides. FieldDefinitions should + // TODO: not present a problem, however. + for (QueryClause clause : clauses) { + useExplicit |= (clause instanceof FieldOverride || clause instanceof FromOverride); + } return _ctx.invoke(useExplicit ? new FindAllQuery.Explicitly(_ctx, type, clauses) : new FindAllQuery.WithCache(_ctx, type, clauses)); diff --git a/src/java/com/samskivert/jdbc/depot/FindAllQuery.java b/src/java/com/samskivert/jdbc/depot/FindAllQuery.java index dd6deaf..eddfb50 100644 --- a/src/java/com/samskivert/jdbc/depot/FindAllQuery.java +++ b/src/java/com/samskivert/jdbc/depot/FindAllQuery.java @@ -35,6 +35,7 @@ import com.samskivert.util.ArrayUtil; import com.samskivert.jdbc.DatabaseLiaison; import com.samskivert.jdbc.JDBCUtil; +import com.samskivert.jdbc.depot.clause.FieldDefinition; import com.samskivert.jdbc.depot.clause.FieldOverride; import com.samskivert.jdbc.depot.clause.Join; import com.samskivert.jdbc.depot.clause.QueryClause; @@ -102,7 +103,7 @@ public abstract class FindAllQuery // a select subset of query clauses are preserved for the entity query QueryClause[] newClauses = new QueryClause[_clauses.length + 1]; for (int ii = 0; ii < _clauses.length; ii ++) { - if (_clauses[ii] instanceof Join || _clauses[ii] instanceof FieldOverride) { + if (_clauses[ii] instanceof Join || _clauses[ii] instanceof FieldDefinition) { newClauses[jj ++] = _clauses[ii]; } } diff --git a/src/java/com/samskivert/jdbc/depot/clause/FieldDefinition.java b/src/java/com/samskivert/jdbc/depot/clause/FieldDefinition.java new file mode 100644 index 0000000..f757874 --- /dev/null +++ b/src/java/com/samskivert/jdbc/depot/clause/FieldDefinition.java @@ -0,0 +1,94 @@ +// +// $Id$ +// +// samskivert library - useful routines for java programs +// Copyright (C) 2006-2007 Michael Bayne, Pär Winzell +// +// This library is free software; you can redistribute it and/or modify it +// under the terms of the GNU Lesser General Public License as published +// by the Free Software Foundation; either version 2.1 of the License, or +// (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +package com.samskivert.jdbc.depot.clause; + +import java.util.Collection; + +import com.samskivert.io.PersistenceException; +import com.samskivert.jdbc.depot.PersistentRecord; +import com.samskivert.jdbc.depot.expression.ColumnExp; +import com.samskivert.jdbc.depot.expression.ExpressionVisitor; +import com.samskivert.jdbc.depot.expression.LiteralExp; +import com.samskivert.jdbc.depot.expression.SQLExpression; + +/** + * Supplies a definition for a computed field of the persistent object we're creating. + * + * Thus the select portion of a query can include a reference to a different column in a different + * table through a {@link ColumnExp}, or a literal expression such as COUNT(*) through a + * {@link LiteralExp}. + * + * @see FieldOverride + */ +public class FieldDefinition extends QueryClause +{ + public FieldDefinition (String field, String str) + throws PersistenceException + { + this(field, new LiteralExp(str)); + } + + public FieldDefinition (String field, Class pClass, String pCol) + throws PersistenceException + { + this(field, new ColumnExp(pClass, pCol)); + } + + public FieldDefinition (String field, SQLExpression override) + throws PersistenceException + { + _field = field; + _definition = override; + } + + /** + * The field we're defining. The Query object uses this for indexing. + */ + public String getField () + { + return _field; + } + + public SQLExpression getDefinition () + { + return _definition; + } + + // from SQLExpression + public void addClasses (Collection> classSet) + { + _definition.addClasses(classSet); + } + + // from SQLExpression + public void accept (ExpressionVisitor visitor) + throws Exception + { + visitor.visit(this); + } + + /** The name of the field on the persistent object to override. */ + protected String _field; + + /** The defining expression. */ + protected SQLExpression _definition; + +} diff --git a/src/java/com/samskivert/jdbc/depot/clause/FieldOverride.java b/src/java/com/samskivert/jdbc/depot/clause/FieldOverride.java index 5d625b3..94811c3 100644 --- a/src/java/com/samskivert/jdbc/depot/clause/FieldOverride.java +++ b/src/java/com/samskivert/jdbc/depot/clause/FieldOverride.java @@ -3,7 +3,7 @@ // // samskivert library - useful routines for java programs // Copyright (C) 2006-2007 Michael Bayne, Pär Winzell -// +// // This library is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License as published // by the Free Software Foundation; either version 2.1 of the License, or @@ -20,12 +20,9 @@ package com.samskivert.jdbc.depot.clause; -import java.util.Collection; - import com.samskivert.io.PersistenceException; import com.samskivert.jdbc.depot.PersistentRecord; import com.samskivert.jdbc.depot.expression.ColumnExp; -import com.samskivert.jdbc.depot.expression.ExpressionVisitor; import com.samskivert.jdbc.depot.expression.LiteralExp; import com.samskivert.jdbc.depot.expression.SQLExpression; @@ -37,57 +34,23 @@ import com.samskivert.jdbc.depot.expression.SQLExpression; * table through a {@link ColumnExp}, or a literal expression such as COUNT(*) through a * {@link LiteralExp}. */ -public class FieldOverride extends QueryClause +public class FieldOverride extends FieldDefinition { public FieldOverride (String field, String str) throws PersistenceException { - this(field, new LiteralExp(str)); + super(field, str); } public FieldOverride (String field, Class pClass, String pCol) throws PersistenceException { - this(field, new ColumnExp(pClass, pCol)); + super(field, pClass, pCol); } public FieldOverride (String field, SQLExpression override) throws PersistenceException { - _field = field; - _override = override; + super(field, override); } - - /** - * The field we're overriding. The Query object uses this for indexing. - */ - public String getField () - { - return _field; - } - - public SQLExpression getOverride () - { - return _override; - } - - // from SQLExpression - public void addClasses (Collection> classSet) - { - _override.addClasses(classSet); - } - - // from SQLExpression - public void accept (ExpressionVisitor visitor) - throws Exception - { - visitor.visit(this); - } - - /** The name of the field on the persistent object to override. */ - protected String _field; - - /** The overriding expression. */ - protected SQLExpression _override; - } diff --git a/src/java/com/samskivert/jdbc/depot/clause/SelectClause.java b/src/java/com/samskivert/jdbc/depot/clause/SelectClause.java index fbc44a3..8bca190 100644 --- a/src/java/com/samskivert/jdbc/depot/clause/SelectClause.java +++ b/src/java/com/samskivert/jdbc/depot/clause/SelectClause.java @@ -67,8 +67,8 @@ public class SelectClause extends QueryClause } else if (clause instanceof Join) { _joinClauses.add((Join) clause); - } else if (clause instanceof FieldOverride) { - _disMap.put(((FieldOverride) clause).getField(), ((FieldOverride) clause)); + } else if (clause instanceof FieldDefinition) { + _disMap.put(((FieldDefinition) clause).getField(), ((FieldDefinition) clause)); } else if (clause instanceof OrderBy) { if (_orderBy != null) { @@ -101,12 +101,12 @@ public class SelectClause extends QueryClause } } - public FieldOverride lookupOverride (String field) + public FieldDefinition lookupDefinition (String field) { return _disMap.get(field); } - public Collection getFieldOverrides () + public Collection getFieldDefinitions () { return _disMap.values(); } @@ -169,7 +169,7 @@ public class SelectClause extends QueryClause for (Join join : _joinClauses) { join.addClasses(classSet); } - for (FieldOverride override : _disMap.values()) { + for (FieldDefinition override : _disMap.values()) { override.addClasses(classSet); } } @@ -181,7 +181,7 @@ public class SelectClause extends QueryClause } /** Persistent class fields mapped to field override clauses. */ - protected Map _disMap = new HashMap(); + protected Map _disMap = new HashMap(); /** The persistent class this select defines. */ protected Class _pClass; diff --git a/src/java/com/samskivert/jdbc/depot/expression/ExpressionVisitor.java b/src/java/com/samskivert/jdbc/depot/expression/ExpressionVisitor.java index acb24c1..7c08059 100644 --- a/src/java/com/samskivert/jdbc/depot/expression/ExpressionVisitor.java +++ b/src/java/com/samskivert/jdbc/depot/expression/ExpressionVisitor.java @@ -26,7 +26,7 @@ import com.samskivert.jdbc.depot.MultiKey; import com.samskivert.jdbc.depot.PersistentRecord; import com.samskivert.jdbc.depot.clause.DeleteClause; -import com.samskivert.jdbc.depot.clause.FieldOverride; +import com.samskivert.jdbc.depot.clause.FieldDefinition; import com.samskivert.jdbc.depot.clause.ForUpdate; import com.samskivert.jdbc.depot.clause.FromOverride; import com.samskivert.jdbc.depot.clause.GroupBy; @@ -50,7 +50,7 @@ import com.samskivert.jdbc.depot.operator.SQLOperator.MultiOperator; */ public interface ExpressionVisitor { - public void visit (FieldOverride fieldOverride) + public void visit (FieldDefinition fieldOverride) throws Exception; public void visit (WhereCondition whereCondition) throws Exception;