Removed the cached ParameterMappings in PrefetchQueryPlanner because …

…this doesn't work in TPC-C NewOrder where the Statement identifiers are going to be the same (but the offsets are different).
cw75 · May 25, 2013 · 7e1d38a · 7e1d38a
1 parent 15f443e
commit 7e1d38a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 25 deletions.
diff --git a/src/frontend/edu/brown/hstore/specexec/PrefetchQueryPlanner.java b/src/frontend/edu/brown/hstore/specexec/PrefetchQueryPlanner.java
@@ -16,6 +16,7 @@
 import org.voltdb.catalog.Procedure;
 import org.voltdb.catalog.Statement;
 import org.voltdb.catalog.StmtParameter;
+import org.voltdb.exceptions.ServerFaultException;
 import org.voltdb.messaging.FastSerializer;
 
 import com.google.protobuf.ByteString;
@@ -50,8 +51,6 @@ public class PrefetchQueryPlanner {
         LoggerUtil.attachObserver(LOG, debug, trace);
     }
 
-    private final Map<Integer, ParameterMapping[][]> mappingsCache = new HashMap<Integer, ParameterMapping[][]>();
-
     private final PartitionEstimator p_estimator;
     private final int[] partitionSiteXref;
     private final CatalogContext catalogContext;
@@ -130,24 +129,8 @@ protected BatchPlanner addPlanner(Procedure catalog_proc,
         BatchPlanner planner = new BatchPlanner(prefetchStmts, prefetchStmts.length, catalog_proc, this.p_estimator);
         planner.setPrefetchFlag(true);
 
-        // For each Statement in this batch, cache its ParameterMappings objects
-        ParameterMapping mappings[][] = new ParameterMapping[prefetchStmts.length][]; 
-        for (int i = 0; i < prefetchStmts.length; i++) {
-            CountedStatement counted_stmt = prefetchable.get(i);
-            mappings[i] = new ParameterMapping[counted_stmt.statement.getParameters().size()];
-            for (StmtParameter catalog_param : counted_stmt.statement.getParameters().values()) {
-                Collection<ParameterMapping> pmSets = this.catalogContext.paramMappings.get(
-                                                                counted_stmt.statement,
-                                                                counted_stmt.counter,
-                                                                catalog_param);
-                assert(pmSets != null) : String.format("Unexpected %s for %s", counted_stmt, catalog_param);
-                mappings[i][catalog_param.getIndex()] = CollectionUtil.first(pmSets);
-            } // FOR (StmtParameter)
-        } // FOR (CountedStatement)
-
         int batchId = VoltProcedure.getBatchHashCode(prefetchStmts, prefetchStmts.length);
         this.planners.get().put(batchId, planner);
-        this.mappingsCache.put(batchId, mappings);
 
         if (debug.val)
             LOG.debug(String.format("%s Prefetch Statements: %s",
@@ -194,8 +177,6 @@ public TransactionInitRequest.Builder[] plan(LocalTransaction ts, ParameterSet p
         ParameterSet prefetchParams[] = new ParameterSet[planner.getBatchSize()];
         int prefetchCounters[] = new int[planner.getBatchSize()]; 
         ByteString prefetchParamsSerialized[] = new ByteString[prefetchParams.length];
-        ParameterMapping mappings[][] = this.mappingsCache.get(hashcode);
-        assert(mappings != null) : "Missing cached ParameterMappings for " + ts.getProcedure();
 
         // Makes a list of ByteStrings containing the ParameterSets that we need
         // to send over to the remote sites so that they can execute our
@@ -213,14 +194,31 @@ public TransactionInitRequest.Builder[] plan(LocalTransaction ts, ParameterSet p
             // ProcParameter to the StmtParameter. This relies on a
             // ParameterMapping already being installed in the catalog
             for (StmtParameter catalog_param : counted_stmt.statement.getParameters().values()) {
-                ParameterMapping pm = mappings[i][catalog_param.getIndex()];
+                Collection<ParameterMapping> pmSets = this.catalogContext.paramMappings.get(
+                                                                counted_stmt.statement,
+                                                                counted_stmt.counter,
+                                                                catalog_param);
+                assert(pmSets != null) : String.format("Unexpected %s for %s", counted_stmt, catalog_param);
+                ParameterMapping pm = CollectionUtil.first(pmSets);
                 assert(pm != null) :
                     String.format("Unexpected null %s for %s [%s]",
                                   ParameterMapping.class.getSimpleName(),
                                   catalog_param.fullName(), counted_stmt);
+                assert(pm.statement_index == counted_stmt.counter) :
+                    String.format("Mismatch StmtCounter for %s - Expected[%d] != Actual[%d]\n%s",
+                                  counted_stmt, counted_stmt.counter, pm.statement_index, pm);
 
                 if (pm.procedure_parameter.getIsarray()) {
-                    stmt_params[catalog_param.getIndex()] = ParametersUtil.getValue(procParams, pm);
+                    try {
+                        stmt_params[catalog_param.getIndex()] = ParametersUtil.getValue(procParams, pm);
+                    } catch (Throwable ex) {
+                        String msg = String.format("Unable to get %s value for %s in %s\n" +
+                        		                   "ProcParams: %s\nParameterMapping: %s",
+                                                   catalog_param.fullName(), ts, counted_stmt,
+                                                   procParams, pm);
+                        LOG.error(msg, ex);
+                        throw new ServerFaultException(msg, ex, ts.getTransactionId());
+                    }
                 }
                 else {
                     ProcParameter catalog_proc_param = pm.procedure_parameter;
@@ -276,7 +274,7 @@ public TransactionInitRequest.Builder[] plan(LocalTransaction ts, ParameterSet p
                 depTracker.addTransaction(ts); 
             }
             // HACK: Attach the prefetch params in the transaction handle in case we need to use it locally
-            if (site_id == local_site_id) {
+            if (site_id == local_site_id && ts.hasPrefetchParameters() == false) {
                 ts.attachPrefetchParameters(prefetchParams);
             }
 

diff --git a/src/frontend/edu/brown/hstore/txns/AbstractTransaction.java b/src/frontend/edu/brown/hstore/txns/AbstractTransaction.java
@@ -1118,7 +1118,7 @@ public void attachPrefetchQueries(List<WorkFragment> fragments, List<ByteString>
     }
 
     public boolean hasPrefetchParameters() {
-        return (this.prefetch.params != null);
+        return (this.prefetch != null && this.prefetch.params != null);
     }
 
     public void attachPrefetchParameters(ParameterSet params[]) {

diff --git a/src/frontend/org/voltdb/SQLStmt.java b/src/frontend/org/voltdb/SQLStmt.java
@@ -76,7 +76,7 @@ public SQLStmt(Statement catalog_stmt, CatalogMap<PlanFragment> fragments) {
 
     protected void computeHashCode() {
         if (this.catStmt != null) {
-            this.hashCode = this.catStmt.fullName().hashCode();
+            this.hashCode = this.catStmt.hashCode();
         } else {
             this.hashCode = this.sqlText.hashCode();
         }