diff --git a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java index 6ded164..ed99a72 100644 --- a/src/java/com/samskivert/jdbc/depot/BuildVisitor.java +++ b/src/java/com/samskivert/jdbc/depot/BuildVisitor.java @@ -21,6 +21,7 @@ package com.samskivert.jdbc.depot; import java.sql.SQLException; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -83,8 +84,10 @@ public abstract class BuildVisitor implements ExpressionVisitor throws Exception { fieldOverride.getOverride().accept(this); - _builder.append(" as "); - appendField(fieldOverride.getField()); + if (_aliasFields) { + _builder.append(" as "); + appendIdentifier(fieldOverride.getField()); + } } public void visit (WhereCondition whereCondition) @@ -97,13 +100,7 @@ public abstract class BuildVisitor implements ExpressionVisitor if (ii > 0) { _builder.append(" and "); } - // TODO: This cannot work properly until the Depot architecture has been expanded - // so that every field reference is expanded according to context such as shadowOf - // and FieldOverride. For now, this works out of sheer luck - we have no ambiguous - // references to trip us up... but it must be fixed soon. - // appendTableAbbreviation(pClass); - // _builder.append("."); - appendColumn(pClass, keyFields[ii]); + appendRhsColumn(pClass, keyFields[ii]); _builder.append(values[ii] == null ? " is null " : " = ? "); } } @@ -126,13 +123,13 @@ public abstract class BuildVisitor implements ExpressionVisitor } else { _builder.append(" and "); } - appendColumn(key.getPersistentClass(), entry.getKey()); + appendRhsColumn(key.getPersistentClass(), entry.getKey()); _builder.append(entry.getValue() == null ? " is null " : " = ? "); } if (!first) { _builder.append(" and "); } - appendColumn(key.getPersistentClass(), key.getMultiField()); + appendRhsColumn(key.getPersistentClass(), key.getMultiField()); _builder.append(" in ("); Comparable[] values = key.getMultiValues(); @@ -210,9 +207,7 @@ public abstract class BuildVisitor implements ExpressionVisitor public void visit (ColumnExp columnExp) throws Exception { - appendTableAbbreviation(columnExp.getPersistentClass()); - _builder.append("."); - appendColumn(columnExp.getPersistentClass(), columnExp.getField()); + appendRhsColumn(columnExp.getPersistentClass(), columnExp.getField()); } public void visit (Not not) @@ -317,104 +312,76 @@ public abstract class BuildVisitor implements ExpressionVisitor } _builder.append("select "); - Computed entityComputed = pClass.getAnnotation(Computed.class); - - // iterate over the fields we're filling in and figure out whence each one comes - boolean skip = true; - for (String field : selectClause.getFields()) { - if (!skip) { - _builder.append(", "); - } - skip = false; - - // first, see if there's a field override - FieldOverride override = selectClause.lookupOverride(field); - if (override != null) { - override.accept(this); - continue; - } - - // figure out the class we're selecting from unless we're otherwise overriden: - // for a concrete record, simply use the corresponding table; for a computed one, - // default to the shadowed concrete record, or null if there isn't one - - Class tableClass; - if (entityComputed == null) { - tableClass = pClass; - } else if (!PersistentRecord.class.equals(entityComputed.shadowOf())) { - tableClass = entityComputed.shadowOf(); - } else { - tableClass = null; - } - - // handle the field-level @Computed annotation, if there is one - FieldMarshaller fm = _types.getMarshaller(pClass).getFieldMarshaller(field); - - Computed fieldComputed = fm.getComputed(); - if (fieldComputed != null) { - // check if the computed field has a literal SQL definition - if (fieldComputed.fieldDefinition().length() > 0) { - _builder.append(fieldComputed.fieldDefinition()).append(" as "); - appendField(field); - continue; - } - - // or if we can simply ignore the field - if (!fieldComputed.required()) { - skip = true; - continue; - } - - // else see if there's an overriding shadowOf definition - if (fieldComputed.shadowOf() != null) { - tableClass = fieldComputed.shadowOf(); - } - } - - // if we get this far we hopefully have a table to select from - if (tableClass != null) { - appendTableAbbreviation(tableClass); - _builder.append("."); - appendColumn(tableClass, field); - continue; - } - - // else owie + if (_overrides.containsKey(pClass)) { throw new IllegalArgumentException( - "Persistent field has no definition [class=" + - selectClause.getPersistentClass() + ", field=" + field + "]"); + "Can not yet nest SELECTs on the same persistent record."); } - if (selectClause.getFromOverride() != null) { - selectClause.getFromOverride().accept(this); + Map overrideMap = new HashMap(); + for (FieldOverride override : selectClause.getFieldOverrides()) { + overrideMap.put(override.getField(), override); + } + _overrides.put(pClass, overrideMap); - } else if (_types.getTableName(pClass) != null) { - _builder.append(" from "); - appendTableName(pClass); - _builder.append(" as "); - appendTableAbbreviation(pClass); + try { + // iterate over the fields we're filling in and figure out whence each one comes + boolean skip = true; - } else { - throw new SQLException("Query on @Computed entity with no FromOverrideClause."); - } + // while expanding column names in the SELECT query, do aliasing + _aliasFields = true; - for (Join clause : selectClause.getJoinClauses()) { - clause.accept(this); - } - if (selectClause.getWhereClause() != null) { - selectClause.getWhereClause().accept(this); - } - if (selectClause.getGroupBy() != null) { - selectClause.getGroupBy().accept(this); - } - if (selectClause.getOrderBy() != null) { - selectClause.getOrderBy().accept(this); - } - if (selectClause.getLimit() != null) { - selectClause.getLimit().accept(this); - } - if (selectClause.getForUpdate() != null) { - selectClause.getForUpdate().accept(this); + for (String field : selectClause.getFields()) { + if (!skip) { + _builder.append(", "); + } + skip = false; + + int len = _builder.length(); + appendRhsColumn(pClass, field); + + // if nothing was added, don't add a comma + if (_builder.length() == len) { + skip = true; + } + } + + // then stop + _aliasFields = false; + + if (selectClause.getFromOverride() != null) { + selectClause.getFromOverride().accept(this); + + } else if (_types.getTableName(pClass) != null) { + _builder.append(" from "); + appendTableName(pClass); + _builder.append(" as "); + appendTableAbbreviation(pClass); + + } else { + throw new SQLException("Query on @Computed entity with no FromOverrideClause."); + } + + for (Join clause : selectClause.getJoinClauses()) { + clause.accept(this); + } + if (selectClause.getWhereClause() != null) { + selectClause.getWhereClause().accept(this); + } + if (selectClause.getGroupBy() != null) { + selectClause.getGroupBy().accept(this); + } + if (selectClause.getOrderBy() != null) { + selectClause.getOrderBy().accept(this); + } + if (selectClause.getLimit() != null) { + selectClause.getLimit().accept(this); + } + if (selectClause.getForUpdate() != null) { + selectClause.getForUpdate().accept(this); + } + + } finally { + _overrides.remove(pClass); } if (isInner) { _builder.append(")"); @@ -440,7 +407,7 @@ public abstract class BuildVisitor implements ExpressionVisitor if (ii > 0) { _builder.append(", "); } - appendColumn(pClass, fields[ii]); + appendLhsColumn(pClass, fields[ii]); _builder.append(" = "); if (pojo != null) { @@ -479,7 +446,7 @@ public abstract class BuildVisitor implements ExpressionVisitor if (ii > 0) { _builder.append(", "); } - appendColumn(pClass, fields[ii]); + appendLhsColumn(pClass, fields[ii]); } _builder.append(") values("); @@ -497,23 +464,134 @@ public abstract class BuildVisitor implements ExpressionVisitor _builder.append(")"); } - protected abstract void appendTableName (Class type); - protected abstract void appendTableAbbreviation (Class type); - protected abstract void appendColumn (Class type, String field); - protected abstract void appendField (String field); + protected abstract void appendIdentifier (String field); + + protected void appendTableName (Class type) + { + appendIdentifier(_types.getTableName(type)); + } + + protected void appendTableAbbreviation (Class type) + { + appendIdentifier(_types.getTableAbbreviation(type)); + } + + // Constructs a name used for assignment in e.g. INSERT/UPDATE. This is the SQL + // equivalent of an lvalue; something that can appear to the left of an equals sign. + // We do not prepend this identifier with a table abbreviation, nor do we expand + // field overrides, shadowOf declarations, or the like: it is just a column name. + protected void appendLhsColumn (Class type, String field) + throws Exception + { + DepotMarshaller dm = _types.getMarshaller(type); + FieldMarshaller fm = dm.getFieldMarshaller(field); + if (dm == null) { + throw new IllegalArgumentException( + "Unknown field on persistent record [record=" + type + ", field=" + field + "]"); + } + + appendIdentifier(fm.getColumnName()); + } + + // Appends an expression for the given field on the given persistent record; this can + // appear in a SELECT list, in WHERE clauses, etc, etc. + protected void appendRhsColumn (Class type, String field) + throws Exception + { + DepotMarshaller dm = _types.getMarshaller(type); + FieldMarshaller fm = dm.getFieldMarshaller(field); + if (dm == null) { + throw new IllegalArgumentException( + "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) { + // overrides point to a raw column even when that column is overridden + // for the purposes of the query itself + _ignoreOverrides = true; + override.accept(this); + _ignoreOverrides = false; + return; + } + } + } + + Computed entityComputed = dm.getComputed(); + + // figure out the class we're selecting from unless we're otherwise overriden: + // for a concrete record, simply use the corresponding table; for a computed one, + // default to the shadowed concrete record, or null if there isn't one + Class tableClass; + if (entityComputed == null) { + tableClass = type; + + } else if (!PersistentRecord.class.equals(entityComputed.shadowOf())) { + tableClass = entityComputed.shadowOf(); + + } else { + tableClass = null; + } + + // handle the field-level @Computed annotation, if there is one + Computed fieldComputed = fm.getComputed(); + if (fieldComputed != null) { + // check if the computed field has a literal SQL definition + if (fieldComputed.fieldDefinition().length() > 0) { + _builder.append(fieldComputed.fieldDefinition()); + if (_aliasFields) { + _builder.append(" as "); + appendIdentifier(field); + } + return; + } + + // or if we can simply ignore the field + if (!fieldComputed.required()) { + return; + } + + // else see if there's an overriding shadowOf definition + if (fieldComputed.shadowOf() != null) { + tableClass = fieldComputed.shadowOf(); + } + } + + // if we get this far we hopefully have a table to select from + if (tableClass != null) { + appendTableAbbreviation(tableClass); + _builder.append("."); + appendIdentifier(fm.getColumnName()); + return; + } + + // else owie + throw new IllegalArgumentException( + "Persistent field has no definition [class=" + type + ", field=" + field + "]"); + } protected BuildVisitor (DepotTypes types) { _types = types; - _builder = new StringBuilder(); - _innerClause = false; } protected DepotTypes _types; /** A StringBuilder to hold the constructed SQL. */ - protected StringBuilder _builder; + protected StringBuilder _builder = new StringBuilder(); + + /** A mapping of field overrides per persistent record. */ + protected Map, Map> _overrides = + new HashMap, Map>(); /** A flag that's set to true for inner SELECT's */ - protected boolean _innerClause; + protected boolean _innerClause = false; + + protected boolean _ignoreOverrides = false; + + protected boolean _aliasFields = false; } diff --git a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java index 6e53be1..60aa1ed 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java +++ b/src/java/com/samskivert/jdbc/depot/DepotMarshaller.java @@ -31,7 +31,6 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -75,8 +74,8 @@ public class DepotMarshaller Entity entity = pclass.getAnnotation(Entity.class); // see if this is a computed entity - Computed computed = pclass.getAnnotation(Computed.class); - if (computed == null) { + _computed = pclass.getAnnotation(Computed.class); + if (_computed == null) { // if not, this class has a corresponding SQL table _tableName = _pclass.getName(); _tableName = _tableName.substring(_tableName.lastIndexOf(".")+1); @@ -212,6 +211,15 @@ public class DepotMarshaller return _pclass; } + /** + * Returns the @Computed annotation definition of this entity, or null if none. + */ + + public Computed getComputed () + { + return _computed; + } + /** * Returns the name of the table in which persistent instances of our class are stored. By * default this is the classname of the persistent object without the package. @@ -958,6 +966,9 @@ public class DepotMarshaller /** The name of our persistent object table. */ protected String _tableName; + /** The @Computed annotation of this entity, or null. */ + protected Computed _computed; + /** A field marshaller for each persistent field in our object. */ protected Map _fields = new HashMap(); diff --git a/src/java/com/samskivert/jdbc/depot/DepotRepository.java b/src/java/com/samskivert/jdbc/depot/DepotRepository.java index 653b975..cea2dcd 100644 --- a/src/java/com/samskivert/jdbc/depot/DepotRepository.java +++ b/src/java/com/samskivert/jdbc/depot/DepotRepository.java @@ -150,20 +150,6 @@ public abstract class DepotRepository boolean useExplicit = (marsh.getTableName() == null) || !marsh.hasPrimaryKey() || !_ctx.isUsingCache(); - // TODO: This is a very conservative approach where all complicated queries are handled - // TODO: with the traditional single-pass query. The double pass algorithm can handle a - // TODO: much larger class of queries, but only after some engine upgrades: specifically, - // TODO: the field references used in the WHERE clause of the Entity query in WithCache - // TODO: cannot be the trivial expansion currently used in the Key class, but must rather - // TODO: undergo the same treatment SELECT fields currently do in SQLQueryBuilder. This - // TODO: is probably best done with an additional method in QueryBuilderContext, or perhaps - // TODO: a much larger switch to using a visitor pattern for generating the SQL of the - // TODO: clause hierarchies. - for (QueryClause clause : clauses) { - useExplicit |= (clause instanceof FieldOverride || - clause instanceof Join || - 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/MySQLBuilder.java b/src/java/com/samskivert/jdbc/depot/MySQLBuilder.java index 6ff33fb..0add1f8 100644 --- a/src/java/com/samskivert/jdbc/depot/MySQLBuilder.java +++ b/src/java/com/samskivert/jdbc/depot/MySQLBuilder.java @@ -29,8 +29,6 @@ import java.sql.SQLException; import java.sql.Statement; import java.sql.Time; import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.List; import java.util.Set; import com.samskivert.Log; @@ -106,12 +104,7 @@ public class MySQLBuilder _builder.append(_types.getTableAbbreviation(type)); } - protected void appendColumn (Class type, String field) - { - _builder.append(_types.getColumnName(type, field)); - } - - protected void appendField (String field) + protected void appendIdentifier (String field) { _builder.append(field); } diff --git a/src/java/com/samskivert/jdbc/depot/PostgreSQLBuilder.java b/src/java/com/samskivert/jdbc/depot/PostgreSQLBuilder.java index 99899fa..4d6cf60 100644 --- a/src/java/com/samskivert/jdbc/depot/PostgreSQLBuilder.java +++ b/src/java/com/samskivert/jdbc/depot/PostgreSQLBuilder.java @@ -62,7 +62,7 @@ public class PostgreSQLBuilder public void visit (FullTextMatch match) throws Exception { - appendField("ftsCol_" + match.getName()); + appendIdentifier("ftsCol_" + match.getName()); _builder.append(" @@ TO_TSQUERY('default', ?)"); } @@ -71,22 +71,7 @@ public class PostgreSQLBuilder super(types); } - protected void appendTableName (Class type) - { - _builder.append("\"").append(_types.getTableName(type)).append("\""); - } - - protected void appendTableAbbreviation (Class type) - { - _builder.append("\"").append(_types.getTableAbbreviation(type)).append("\""); - } - - protected void appendColumn (Class type, String field) - { - _builder.append("\"").append(_types.getColumnName(type, field)).append("\""); - } - - protected void appendField (String field) + protected void appendIdentifier (String field) { _builder.append("\"").append(field).append("\""); }