Merge pull request #214 from svladykin/viewcost

Sub-query or view cost could be wrong for cached ViewIndex

h2/src/main/org/h2/index/ViewIndex.java

@@ -6,6 +6,7 @@
 package org.h2.index;
 
 import java.util.ArrayList;
+import java.util.concurrent.TimeUnit;
 import org.h2.api.ErrorCode;
 import org.h2.command.Prepared;
 import org.h2.command.dml.Query;
@@ -26,9 +27,6 @@ import org.h2.table.TableFilter;
 import org.h2.table.TableView;
 import org.h2.util.IntArray;
 import org.h2.util.New;
-import org.h2.util.SmallLRUCache;
-import org.h2.util.SynchronizedVerifier;
-import org.h2.util.Utils;
 import org.h2.value.Value;
 
 /**
@@ -37,16 +35,22 @@ import org.h2.value.Value;
  */
 public class ViewIndex extends BaseIndex implements SpatialIndex {
 
+    private static final long MAX_AGE_NANOS =
+            TimeUnit.MILLISECONDS.toNanos(Constants.VIEW_COST_CACHE_MAX_AGE);
+
     private final TableView view;
     private final String querySQL;
     private final ArrayList<Parameter> originalParameters;
-    private final SmallLRUCache<IntArray, CostElement> costCache =
-        SmallLRUCache.newInstance(Constants.VIEW_INDEX_CACHE_SIZE);
     private boolean recursive;
     private final int[] indexMasks;
     private Query query;
     private final Session createSession;
 
+    /**
+     * The time in nanoseconds when this index (and its cost) was calculated.
+     */
+    private final long evaluatedAt;
+
     /**
      * Constructor for the original index in {@link TableView}.
      *
@@ -65,6 +69,8 @@ public class ViewIndex extends BaseIndex implements SpatialIndex {
         columns = new Column[0];
         this.createSession = null;
         this.indexMasks = null;
+        // this is a main index of TableView, it does not need eviction time stamp 
+        evaluatedAt = Long.MIN_VALUE;
     }
 
     /**
@@ -91,6 +97,10 @@ public class ViewIndex extends BaseIndex implements SpatialIndex {
         if (!recursive) {
             query = getQuery(session, masks, filters, filter, sortOrder);
         }
+        // we don't need eviction for recursive views since we can't calculate their cost
+        // if it is a sub-query we don't need eviction as well because the whole ViewIndex cache
+        // is getting dropped in Session.prepareLocal
+        evaluatedAt = recursive || view.getTopQuery() != null ? Long.MAX_VALUE : System.nanoTime();
     }
 
     @Override
@@ -106,6 +116,12 @@ public class ViewIndex extends BaseIndex implements SpatialIndex {
         return createSession;
     }
 
+    public boolean isExpired() {
+        assert evaluatedAt != Long.MIN_VALUE : "must not be called for main index of TableView";
+        return !recursive && view.getTopQuery() == null &&
+                System.nanoTime() - evaluatedAt > MAX_AGE_NANOS;
+    }
+
     @Override
     public String getPlanSQL() {
         return query == null ? null : query.getPlanSQL();
@@ -126,72 +142,10 @@ public class ViewIndex extends BaseIndex implements SpatialIndex {
         throw DbException.getUnsupportedException("VIEW");
     }
 
-    /**
-     * A calculated cost value.
-     */
-    static class CostElement {
-
-        /**
-         * The time in milliseconds when this cost was calculated.
-         */
-        long evaluatedAt;
-
-        /**
-         * The cost.
-         */
-        double cost;
-    }
-
     @Override
-    public synchronized double getCost(Session session, int[] masks,
+    public double getCost(Session session, int[] masks,
             TableFilter[] filters, int filter, SortOrder sortOrder) {
-        if (recursive) {
-            return 1000;
-        }
-        IntArray masksArray = new IntArray(masks == null ?
-                Utils.EMPTY_INT_ARRAY : masks);
-        SynchronizedVerifier.check(costCache);
-        CostElement cachedCost = costCache.get(masksArray);
-        if (cachedCost != null) {
-            long time = System.currentTimeMillis();
-            if (time < cachedCost.evaluatedAt + Constants.VIEW_COST_CACHE_MAX_AGE) {
-                return cachedCost.cost;
-            }
-        }
-        Query q = prepareSubQuery(querySQL, session, masks, filters, filter, sortOrder);
-        if (masks != null) {
-            for (int idx = 0; idx < masks.length; idx++) {
-                int mask = masks[idx];
-                if (mask == 0) {
-                    continue;
-                }
-                int nextParamIndex = q.getParameters().size() + view.getParameterOffset();
-                if ((mask & IndexCondition.EQUALITY) != 0) {
-                    Parameter param = new Parameter(nextParamIndex);
-                    q.addGlobalCondition(param, idx, Comparison.EQUAL_NULL_SAFE);
-                } else if ((mask & IndexCondition.SPATIAL_INTERSECTS) != 0) {
-                    Parameter param = new Parameter(nextParamIndex);
-                    q.addGlobalCondition(param, idx, Comparison.SPATIAL_INTERSECTS);
-                } else {
-                    if ((mask & IndexCondition.START) != 0) {
-                        Parameter param = new Parameter(nextParamIndex);
-                        q.addGlobalCondition(param, idx, Comparison.BIGGER_EQUAL);
-                    }
-                    if ((mask & IndexCondition.END) != 0) {
-                        Parameter param = new Parameter(nextParamIndex);
-                        q.addGlobalCondition(param, idx, Comparison.SMALLER_EQUAL);
-                    }
-                }
-            }
-            String sql = q.getPlanSQL();
-            q = prepareSubQuery(sql, session, masks, filters, filter, sortOrder);
-        }
-        double cost = q.getCost();
-        cachedCost = new CostElement();
-        cachedCost.evaluatedAt = System.currentTimeMillis();
-        cachedCost.cost = cost;
-        costCache.put(masksArray, cachedCost);
-        return cost;
+        return recursive ? 1000 : query.getCost();
     }
 
     @Override

h2/src/main/org/h2/table/TableView.java

@@ -235,15 +235,15 @@ public class TableView extends Table {
     @Override
     public PlanItem getBestPlanItem(Session session, int[] masks,
             TableFilter[] filters, int filter, SortOrder sortOrder) {
-        PlanItem item = new PlanItem();
-        item.cost = index.getCost(session, masks, filters, filter, sortOrder);
         final CacheKey cacheKey = new CacheKey(masks, this);
         Map<Object, ViewIndex> indexCache = session.getViewIndexCache(topQuery != null);
         ViewIndex i = indexCache.get(cacheKey);
-        if (i == null) {
+        if (i == null || i.isExpired()) {
             i = new ViewIndex(this, index, session, masks, filters, filter, sortOrder);
             indexCache.put(cacheKey, i);
         }
+        PlanItem item = new PlanItem();
+        item.cost = i.getCost(session, masks, filters, filter, sortOrder);
         item.setIndex(i);
         return item;
     }