From df422c9673316b6a7ecdb96e314aeabd660985ad Mon Sep 17 00:00:00 2001 From: "Brian S. O'Neill" Date: Fri, 29 Jun 2007 05:03:09 +0000 Subject: Fixed IllegalArgumentException with optimized join queries of the form "a.b = ? & (a.c = ? | a.d = ?)". --- src/main/java/com/amazon/carbonado/Trigger.java | 33 ++++++++ .../com/amazon/carbonado/filter/ClosedFilter.java | 20 +++++ .../java/com/amazon/carbonado/filter/Filter.java | 56 ++++++++++++++ .../com/amazon/carbonado/filter/OpenFilter.java | 20 +++++ .../amazon/carbonado/filter/PropertyFilter.java | 20 +++++ .../com/amazon/carbonado/qe/FilteringScore.java | 22 +----- .../amazon/carbonado/qe/JoinedQueryExecutor.java | 11 ++- .../carbonado/raw/GenericEncodingStrategy.java | 12 ++- .../java/com/amazon/carbonado/raw/RawSupport.java | 12 ++- .../amazon/carbonado/repo/jdbc/JDBCSupport.java | 3 + .../carbonado/repo/sleepycat/BDBStorage.java | 26 +++++-- .../com/amazon/carbonado/spi/TriggerManager.java | 87 +++++++++++++++++++++- 12 files changed, 284 insertions(+), 38 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/amazon/carbonado/Trigger.java b/src/main/java/com/amazon/carbonado/Trigger.java index df737e8..7c1bbc9 100644 --- a/src/main/java/com/amazon/carbonado/Trigger.java +++ b/src/main/java/com/amazon/carbonado/Trigger.java @@ -18,6 +18,9 @@ package com.amazon.carbonado; +import com.amazon.carbonado.lob.Blob; +import com.amazon.carbonado.lob.Clob; + /** * Callback mechanism to allow custom code to run when a storable is * persisted. By default, the methods defined in this class do @@ -283,6 +286,36 @@ public abstract class Trigger { public void failedDelete(S storable, Object state) { } + /** + * Called after a Blob is loaded. Override to return an adapted Blob which + * can listen for changes. By default, the original Blob is returned + * unmodified. + * + * @param storable storable which contains Blob property + * @param name property name of Blob + * @param blob non-null Blob property instance + * @return adapted Blob + * @since 1.2 + */ + public Blob adaptBlob(S storable, String name, Blob blob) { + return blob; + } + + /** + * Called after a Clob is loaded. Override to return an adapted Clob which + * can listen for changes. By default, the original Clob is returned + * unmodified. + * + * @param storable storable which contains Clob property + * @param name property name of Clob + * @param clob non-null Clob property instance + * @return adapted Clob + * @since 1.2 + */ + public Clob adaptClob(S storable, String name, Clob clob) { + return clob; + } + /** * Call to quickly abort a "try" operation, returning false to the * caller. This method should not be called by a non-try trigger method, diff --git a/src/main/java/com/amazon/carbonado/filter/ClosedFilter.java b/src/main/java/com/amazon/carbonado/filter/ClosedFilter.java index f5f1273..598f149 100644 --- a/src/main/java/com/amazon/carbonado/filter/ClosedFilter.java +++ b/src/main/java/com/amazon/carbonado/filter/ClosedFilter.java @@ -19,6 +19,8 @@ package com.amazon.carbonado.filter; import java.io.IOException; +import java.util.Collections; +import java.util.List; import com.amazon.carbonado.Storable; @@ -46,6 +48,24 @@ public class ClosedFilter extends Filter { return getOpenFilter(getStorableType()); } + /** + * @since 1.1.1 + */ + @Override + public List> disjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + + /** + * @since 1.1.1 + */ + @Override + public List> conjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + @Override public FilterValues initialFilterValues() { return null; diff --git a/src/main/java/com/amazon/carbonado/filter/Filter.java b/src/main/java/com/amazon/carbonado/filter/Filter.java index b262822..cbc37dc 100644 --- a/src/main/java/com/amazon/carbonado/filter/Filter.java +++ b/src/main/java/com/amazon/carbonado/filter/Filter.java @@ -19,7 +19,11 @@ package com.amazon.carbonado.filter; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Map; + import org.cojen.util.SoftValuedHashMap; import org.cojen.util.WeakCanonicalSet; import org.cojen.util.WeakIdentityMap; @@ -360,6 +364,32 @@ public abstract class Filter implements Appender { return bind().dnf(); } + /** + * Splits the filter from its disjunctive normal form. Or'ng the filters + * together produces the full disjunctive normal form. + * + * @return unmodifiable list of sub filters which don't perform any 'or' + * operations + * @since 1.1.1 + */ + public List> disjunctiveNormalFormSplit() { + final List> list = new ArrayList>(); + + disjunctiveNormalForm().accept(new Visitor() { + public Object visit(AndFilter filter, Object param) { + list.add(filter); + return null; + } + + public Object visit(PropertyFilter filter, Object param) { + list.add(filter); + return null; + } + }, null); + + return Collections.unmodifiableList(list); + } + final Filter dnf() { Filter filter = this; if (!filter.isDisjunctiveNormalForm()) { @@ -390,6 +420,32 @@ public abstract class Filter implements Appender { return bind().cnf(); } + /** + * Splits the filter from its conjunctive normal form. And'ng the filters + * together produces the full conjunctive normal form. + * + * @return unmodifiable list of sub filters which don't perform any 'and' + * operations + * @since 1.1.1 + */ + public List> conjunctiveNormalFormSplit() { + final List> list = new ArrayList>(); + + conjunctiveNormalForm().accept(new Visitor() { + public Object visit(OrFilter filter, Object param) { + list.add(filter); + return null; + } + + public Object visit(PropertyFilter filter, Object param) { + list.add(filter); + return null; + } + }, null); + + return Collections.unmodifiableList(list); + } + final Filter cnf() { Filter filter = this; if (!filter.isConjunctiveNormalForm()) { diff --git a/src/main/java/com/amazon/carbonado/filter/OpenFilter.java b/src/main/java/com/amazon/carbonado/filter/OpenFilter.java index deb68d2..7604a16 100644 --- a/src/main/java/com/amazon/carbonado/filter/OpenFilter.java +++ b/src/main/java/com/amazon/carbonado/filter/OpenFilter.java @@ -19,6 +19,8 @@ package com.amazon.carbonado.filter; import java.io.IOException; +import java.util.Collections; +import java.util.List; import com.amazon.carbonado.Storable; @@ -46,6 +48,24 @@ public class OpenFilter extends Filter { return getClosedFilter(getStorableType()); } + /** + * @since 1.1.1 + */ + @Override + public List> disjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + + /** + * @since 1.1.1 + */ + @Override + public List> conjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + @Override public FilterValues initialFilterValues() { return null; diff --git a/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java b/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java index 71e575f..9f12095 100644 --- a/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java +++ b/src/main/java/com/amazon/carbonado/filter/PropertyFilter.java @@ -19,6 +19,8 @@ package com.amazon.carbonado.filter; import java.io.IOException; +import java.util.Collections; +import java.util.List; import org.cojen.classfile.TypeDesc; @@ -113,6 +115,24 @@ public class PropertyFilter extends Filter { } } + /** + * @since 1.1.1 + */ + @Override + public List> disjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + + /** + * @since 1.1.1 + */ + @Override + public List> conjunctiveNormalFormSplit() { + // Yes, the Java compiler really wants me to do a useless cast. + return Collections.singletonList((Filter) this); + } + public R accept(Visitor visitor, P param) { return visitor.visit(this, param); } diff --git a/src/main/java/com/amazon/carbonado/qe/FilteringScore.java b/src/main/java/com/amazon/carbonado/qe/FilteringScore.java index 7722873..2a9f354 100644 --- a/src/main/java/com/amazon/carbonado/qe/FilteringScore.java +++ b/src/main/java/com/amazon/carbonado/qe/FilteringScore.java @@ -270,27 +270,7 @@ public class FilteringScore { * together produces the original filter. */ static List> split(Filter filter) { - if (filter == null) { - return null; - } - - filter = filter.conjunctiveNormalForm(); - - final List> list = new ArrayList>(); - - filter.accept(new Visitor() { - public Object visit(OrFilter filter, Object param) { - list.add(filter); - return null; - } - - public Object visit(PropertyFilter filter, Object param) { - list.add(filter); - return null; - } - }, null); - - return list; + return filter == null ? null : filter.conjunctiveNormalFormSplit(); } private final OrderedProperty[] mIndexProperties; diff --git a/src/main/java/com/amazon/carbonado/qe/JoinedQueryExecutor.java b/src/main/java/com/amazon/carbonado/qe/JoinedQueryExecutor.java index 0f4e64d..d947ed7 100644 --- a/src/main/java/com/amazon/carbonado/qe/JoinedQueryExecutor.java +++ b/src/main/java/com/amazon/carbonado/qe/JoinedQueryExecutor.java @@ -21,6 +21,7 @@ package com.amazon.carbonado.qe; import java.io.IOException; import java.util.Comparator; +import java.util.List; import java.util.Map; import org.cojen.classfile.ClassFile; @@ -394,13 +395,17 @@ public class JoinedQueryExecutor private static OrderingList expectedOrdering(StorageAccess access, Filter filter, OrderingList ordering) { + List> split = filter.disjunctiveNormalFormSplit(); + Comparator comparator = CompositeScore.fullComparator(); CompositeScore bestScore = null; for (StorableIndex index : access.getAllIndexes()) { - CompositeScore candidateScore = CompositeScore.evaluate(index, filter, ordering); - if (bestScore == null || comparator.compare(candidateScore, bestScore) < 0) { - bestScore = candidateScore; + for (Filter sub : split) { + CompositeScore candidateScore = CompositeScore.evaluate(index, sub, ordering); + if (bestScore == null || comparator.compare(candidateScore, bestScore) < 0) { + bestScore = candidateScore; + } } } diff --git a/src/main/java/com/amazon/carbonado/raw/GenericEncodingStrategy.java b/src/main/java/com/amazon/carbonado/raw/GenericEncodingStrategy.java index d741c08..4c295dd 100644 --- a/src/main/java/com/amazon/carbonado/raw/GenericEncodingStrategy.java +++ b/src/main/java/com/amazon/carbonado/raw/GenericEncodingStrategy.java @@ -1462,8 +1462,8 @@ public class GenericEncodingStrategy { /** * Generates code to get a Lob from a locator from RawSupport. RawSupport - * instance and long locator must be on the stack. Result is a Lob on the - * stack, which may be null. + * instance, Storable instance, property name and long locator must be on + * the stack. Result is a Lob on the stack, which may be null. */ private void getLobFromLocator(CodeAssembler a, StorablePropertyInfo info) { if (!info.isLob()) { @@ -1481,7 +1481,8 @@ public class GenericEncodingStrategy { } a.invokeInterface(TypeDesc.forClass(RawSupport.class), name, - type, new TypeDesc[] {TypeDesc.LONG}); + type, new TypeDesc[] {TypeDesc.forClass(Storable.class), + TypeDesc.STRING, TypeDesc.LONG}); } ///////////////////////////////////////////////////////////////////////////////// @@ -1601,6 +1602,11 @@ public class GenericEncodingStrategy { if (info.isLob()) { // Need RawSupport instance for getting Lob from locator. pushRawSupport(a, instanceVar); + + // Also need to pass this stuff along when getting Lob. + a.loadThis(); + a.loadConstant(info.getPropertyName()); + // Locator is encoded as a long. storageType = TypeDesc.LONG; } diff --git a/src/main/java/com/amazon/carbonado/raw/RawSupport.java b/src/main/java/com/amazon/carbonado/raw/RawSupport.java index 63577d7..b83e548 100644 --- a/src/main/java/com/amazon/carbonado/raw/RawSupport.java +++ b/src/main/java/com/amazon/carbonado/raw/RawSupport.java @@ -73,8 +73,12 @@ public interface RawSupport extends MasterSupport { /** * Returns the Blob for the given locator, returning null if not found. + * + * @param storable storable that contains Blob + * @param name name of Blob property + * @param locator Blob locator */ - Blob getBlob(long locator) throws FetchException; + Blob getBlob(S storable, String name, long locator) throws FetchException; /** * Returns the locator for the given Blob, returning zero if null. @@ -85,8 +89,12 @@ public interface RawSupport extends MasterSupport { /** * Returns the Clob for the given locator, returning null if not found. + * + * @param storable storable that contains Blob + * @param name name of Clob property + * @param locator Clob locator */ - Clob getClob(long locator) throws FetchException; + Clob getClob(S storable, String name, long locator) throws FetchException; /** * Returns the locator for the given Clob, returning zero if null. diff --git a/src/main/java/com/amazon/carbonado/repo/jdbc/JDBCSupport.java b/src/main/java/com/amazon/carbonado/repo/jdbc/JDBCSupport.java index 304c816..7c212cc 100644 --- a/src/main/java/com/amazon/carbonado/repo/jdbc/JDBCSupport.java +++ b/src/main/java/com/amazon/carbonado/repo/jdbc/JDBCSupport.java @@ -34,6 +34,9 @@ import com.amazon.carbonado.gen.MasterSupport; public interface JDBCSupport extends MasterSupport { public JDBCRepository getJDBCRepository(); + // FIXME: Lob convert methods need to take Storable and property name. With + // this, the optional Lob adapting trigger must be invoked. + /** * @param loader used to reload Blob outside original transaction */ diff --git a/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBStorage.java b/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBStorage.java index c2d1844..03874aa 100644 --- a/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBStorage.java +++ b/src/main/java/com/amazon/carbonado/repo/sleepycat/BDBStorage.java @@ -684,9 +684,14 @@ abstract class BDBStorage implements Storage, Storag return database; } - Blob getBlob(long locator) throws FetchException { + Blob getBlob(S storable, String name, long locator) throws FetchException { try { - return mRepository.getLobEngine().getBlobValue(locator); + Blob blob = mRepository.getLobEngine().getBlobValue(locator); + Trigger trigger = mTriggerManager.getAdaptLobTrigger(); + if (trigger != null) { + blob = trigger.adaptBlob(storable, name, blob); + } + return blob; } catch (RepositoryException e) { throw e.toFetchException(); } @@ -702,9 +707,14 @@ abstract class BDBStorage implements Storage, Storag } } - Clob getClob(long locator) throws FetchException { + Clob getClob(S storable, String name, long locator) throws FetchException { try { - return mRepository.getLobEngine().getClobValue(locator); + Clob clob = mRepository.getLobEngine().getClobValue(locator); + Trigger trigger = mTriggerManager.getAdaptLobTrigger(); + if (trigger != null) { + clob = trigger.adaptClob(storable, name, clob); + } + return clob; } catch (RepositoryException e) { throw e.toFetchException(); } @@ -1075,16 +1085,16 @@ abstract class BDBStorage implements Storage, Storag } } - public Blob getBlob(long locator) throws FetchException { - return mStorage.getBlob(locator); + public Blob getBlob(S storable, String name, long locator) throws FetchException { + return mStorage.getBlob(storable, name, locator); } public long getLocator(Blob blob) throws PersistException { return mStorage.getLocator(blob); } - public Clob getClob(long locator) throws FetchException { - return mStorage.getClob(locator); + public Clob getClob(S storable, String name, long locator) throws FetchException { + return mStorage.getClob(storable, name, locator); } public long getLocator(Clob clob) throws PersistException { diff --git a/src/main/java/com/amazon/carbonado/spi/TriggerManager.java b/src/main/java/com/amazon/carbonado/spi/TriggerManager.java index a67b134..c4caf72 100644 --- a/src/main/java/com/amazon/carbonado/spi/TriggerManager.java +++ b/src/main/java/com/amazon/carbonado/spi/TriggerManager.java @@ -32,6 +32,9 @@ import com.amazon.carbonado.Storable; import com.amazon.carbonado.Trigger; import com.amazon.carbonado.TriggerFactory; +import com.amazon.carbonado.lob.Blob; +import com.amazon.carbonado.lob.Clob; + /** * Used by Storage implementations to manage triggers and consolidate them into * single logical triggers. This class is thread-safe and ensures that changes @@ -44,6 +47,7 @@ public class TriggerManager extends Trigger { private static final int FOR_INSERT = 1; private static final int FOR_UPDATE = 2; private static final int FOR_DELETE = 4; + private static final int FOR_ADAPT_LOB = 8; private static final Method BEFORE_INSERT_METHOD, @@ -62,7 +66,10 @@ public class TriggerManager extends Trigger { BEFORE_TRY_DELETE_METHOD, AFTER_DELETE_METHOD, AFTER_TRY_DELETE_METHOD, - FAILED_DELETE_METHOD; + FAILED_DELETE_METHOD, + + ADAPT_BLOB_METHOD, + ADAPT_CLOB_METHOD; static { Class triggerClass = Trigger.class; @@ -87,6 +94,11 @@ public class TriggerManager extends Trigger { AFTER_DELETE_METHOD = triggerClass.getMethod("afterDelete", TWO_PARAMS); AFTER_TRY_DELETE_METHOD = triggerClass.getMethod("afterTryDelete", TWO_PARAMS); FAILED_DELETE_METHOD = triggerClass.getMethod("failedDelete", TWO_PARAMS); + + ADAPT_BLOB_METHOD = triggerClass + .getMethod("adaptBlob", Object.class, String.class, Blob.class); + ADAPT_CLOB_METHOD = triggerClass + .getMethod("adaptClob", Object.class, String.class, Clob.class); } catch (NoSuchMethodException e) { Error error = new NoSuchMethodError(); error.initCause(e); @@ -97,6 +109,7 @@ public class TriggerManager extends Trigger { private final ForInsert mForInsert = new ForInsert(); private final ForUpdate mForUpdate = new ForUpdate(); private final ForDelete mForDelete = new ForDelete(); + private final ForAdaptLob mForAdaptLob = new ForAdaptLob(); public TriggerManager() { } @@ -143,6 +156,18 @@ public class TriggerManager extends Trigger { return forDelete.isEmpty() ? null : forDelete; } + /** + * Returns a consolidated trigger to call for adapt LOB operations, or null + * if none. If not null, the consolidated trigger is not a snapshot -- it + * will change as the set of triggers in this manager changes. + * + * @since 1.2 + */ + public Trigger getAdaptLobTrigger() { + ForAdaptLob forAdaptLob = mForAdaptLob; + return forAdaptLob.isEmpty() ? null : forAdaptLob; + } + public boolean addTrigger(Trigger trigger) { if (trigger == null) { throw new IllegalArgumentException(); @@ -161,6 +186,9 @@ public class TriggerManager extends Trigger { if ((types & FOR_DELETE) != 0) { retValue |= mForDelete.add(trigger); } + if ((types & FOR_ADAPT_LOB) != 0) { + retValue |= mForAdaptLob.add(trigger); + } return retValue; } @@ -183,6 +211,9 @@ public class TriggerManager extends Trigger { if ((types & FOR_DELETE) != 0) { retValue |= mForDelete.remove(trigger); } + if ((types & FOR_ADAPT_LOB) != 0) { + retValue |= mForAdaptLob.remove(trigger); + } return retValue; } @@ -207,6 +238,7 @@ public class TriggerManager extends Trigger { mForInsert.localDisable(); mForUpdate.localDisable(); mForDelete.localDisable(); + mForAdaptLob.localDisable(); } /** @@ -217,6 +249,7 @@ public class TriggerManager extends Trigger { mForInsert.localEnable(); mForUpdate.localEnable(); mForDelete.localEnable(); + mForAdaptLob.localEnable(); } @Override @@ -294,6 +327,16 @@ public class TriggerManager extends Trigger { mForDelete.failedDelete(storable, state); } + @Override + public Blob adaptBlob(S storable, String name, Blob blob) { + return mForAdaptLob.adaptBlob(storable, name, blob); + } + + @Override + public Clob adaptClob(S storable, String name, Clob clob) { + return mForAdaptLob.adaptClob(storable, name, clob); + } + /** * Determines which operations the given trigger overrides. */ @@ -329,6 +372,12 @@ public class TriggerManager extends Trigger { types |= FOR_DELETE; } + if (overridesMethod(triggerClass, ADAPT_BLOB_METHOD) || + overridesMethod(triggerClass, ADAPT_CLOB_METHOD)) + { + types |= FOR_ADAPT_LOB; + } + return types; } @@ -897,4 +946,40 @@ public class TriggerManager extends Trigger { } } } + + private static class ForAdaptLob extends ForSomething { + @Override + public Blob adaptBlob(S storable, String name, Blob blob) { + if (isLocallyDisabled()) { + return blob; + } + + Trigger[] triggers = mTriggers; + + int length = triggers.length; + + for (int i=0; i[] triggers = mTriggers; + + int length = triggers.length; + + for (int i=0; i