Skip to content

Commit

Permalink
Fixed a bug in the PlanOptimizer where we would incorrectly set the D…
Browse files Browse the repository at this point in the history
…istinctPlanNode target column. Enabled TestTPCCSuite (without cluster testing for now)
  • Loading branch information
apavlo committed Mar 19, 2012
1 parent 51f93ce commit 090c553
Show file tree
Hide file tree
Showing 16 changed files with 186 additions and 88 deletions.
2 changes: 0 additions & 2 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,6 @@ TEST CASES
<exclude name="**/TestRollbackSuite.class" />
<exclude name="**/TestSaveRestoreSysprocSuite.class" />
<exclude name="**/TestSqlUpdateSuite.class" />
<exclude name="**/TestTPCCSuite.class" />

<!-- Misc VoltDB stuff -->
<exclude name='org/voltdb/client/TestDistributer*'/>
Expand Down Expand Up @@ -1011,7 +1010,6 @@ TEST CASES
<exclude name="**/TestRollbackSuite.class" />
<exclude name="**/TestSaveRestoreSysprocSuite.class" />
<exclude name="**/TestSqlUpdateSuite.class" />
<exclude name="**/TestTPCCSuite.class" />

<!-- Brown Tests -->
<include name='edu/brown/**/Test*.class'/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public Catalog createCatalog() throws IOException {
public Catalog createCatalog(boolean fkeys, boolean full_catalog) throws IOException {
// compile a catalog
if (full_catalog) {
this.addProcedures(this.procedures);
// this.addProcedures(this.procedures);
} else {
// The TPC-E catalog takes a long time load, so we have the ability
// to just compile the schema and the first procedure to make things load faster
Expand Down
1 change: 1 addition & 0 deletions src/frontend/edu/brown/hstore/HStoreSite.java
Original file line number Diff line number Diff line change
Expand Up @@ -1947,6 +1947,7 @@ public void deleteTransaction(final Long txn_id, final Status status) {
TxnCounter.REJECTED.inc(catalog_proc);
break;
case ABORT_UNEXPECTED:
case ABORT_GRACEFUL:
// TODO: Make new counter?
break;
default:
Expand Down
15 changes: 7 additions & 8 deletions src/frontend/edu/brown/optimizer/PlanOptimizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public class PlanOptimizer {
* The list of PlanNodeTypes that we do not want to try to optimize
*/
private static final PlanNodeType TO_IGNORE[] = { PlanNodeType.AGGREGATE, PlanNodeType.NESTLOOP, };
private static final String BROKEN_SQL[] = { "FROM CUSTOMER, FLIGHT, RESERVATION", // Airline
// DeleteReservation.GetCustomerReservation
"SELECT imb_ib_id, ib_bid", // AuctionMark NewBid.getMaxBidId
private static final String BROKEN_SQL[] = {
// "FROM CUSTOMER, FLIGHT, RESERVATION", // Airline DeleteReservation.GetCustomerReservation
// "SELECT imb_ib_id, ib_bid", // AuctionMark NewBid.getMaxBidId
};

/**
Expand Down Expand Up @@ -124,11 +124,10 @@ public AbstractPlanNode optimize(final String sql, final AbstractPlanNode root)
AbstractPlanNode new_root = root;
if (trace.get())
LOG.trace("BEFORE: " + sql + "\n" + StringUtil.box(PlanNodeUtil.debug(root)));
// if (PlanNodeUtil.isDistributedQuery(root) &&
// sql.contains("SELECT f_id FROM FLIGHT ORDER BY F_DEPART_TIME DESC LIMIT 10000"))
// {
// LOG.debug("LET 'ER RIP!");
// }
// if (PlanNodeUtil.isDistributedQuery(root) &&
if (sql.contains("DISTINCT")) {
LOG.debug("LET 'ER RIP!");
}

// STEP #1:
// Populate the PlanOptimizerState with the information that we will
Expand Down
8 changes: 4 additions & 4 deletions src/frontend/edu/brown/optimizer/PlanOptimizerState.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ public void clearDirtyNodes() {
}

public void markDirty(AbstractPlanNode node) {
if (debug.get())
LOG.debug("Marking " + node + " as dirty");
if (trace.get())
LOG.trace("Marking " + node + " as dirty");
this.dirtyPlanNodes.add(node);
}

Expand Down Expand Up @@ -167,8 +167,8 @@ protected void addPlanNodeColumn(AbstractPlanNode node, Column catalog_col) {
if (this.planNodeColumns.containsKey(node) == false) {
this.planNodeColumns.put(node, new HashSet<Column>());
}
if (debug.get())
LOG.debug(String.format("Referenced Columns %s -> %s", node, catalog_col));
if (trace.get())
LOG.trace(String.format("Referenced Columns %s -> %s", node, catalog_col));
this.planNodeColumns.get(node).add(catalog_col);
}

Expand Down
48 changes: 31 additions & 17 deletions src/frontend/edu/brown/optimizer/PlanOptimizerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,17 @@ protected void callback(AbstractPlanNode element) {
// JOIN
// ---------------------------------------------------
if (element instanceof AbstractJoinPlanNode) {
if ((state.areChildrenDirty(element) || force) && PlanOptimizerUtil.updateJoinsColumns(state, (AbstractJoinPlanNode) element) == false) {
if ((state.areChildrenDirty(element) || force) &&
PlanOptimizerUtil.updateJoinsColumns(state, (AbstractJoinPlanNode) element) == false) {
this.stop();
return;
}
// ---------------------------------------------------
// ORDER BY
// ---------------------------------------------------
} else if (element instanceof OrderByPlanNode) {
if ((state.areChildrenDirty(element) || force) && PlanOptimizerUtil.updateOrderByColumns(state, (OrderByPlanNode) element) == false) {
if ((state.areChildrenDirty(element) || force) &&
PlanOptimizerUtil.updateOrderByColumns(state, (OrderByPlanNode) element) == false) {
this.stop();
return;
}
Expand All @@ -297,7 +299,8 @@ protected void callback(AbstractPlanNode element) {
// AGGREGATE
// ---------------------------------------------------
else if (element instanceof AggregatePlanNode) {
if ((state.areChildrenDirty(element) || force) && PlanOptimizerUtil.updateAggregateColumns(state, (AggregatePlanNode) element) == false) {
if ((state.areChildrenDirty(element) || force) &&
PlanOptimizerUtil.updateAggregateColumns(state, (AggregatePlanNode) element) == false) {
this.stop();
return;
}
Expand All @@ -306,7 +309,8 @@ else if (element instanceof AggregatePlanNode) {
// DISTINCT
// ---------------------------------------------------
else if (element instanceof DistinctPlanNode) {
if ((state.areChildrenDirty(element) || force) && PlanOptimizerUtil.updateDistinctColumns(state, (DistinctPlanNode) element) == false) {
if ((state.areChildrenDirty(element) || force) &&
PlanOptimizerUtil.updateDistinctColumns(state, (DistinctPlanNode) element) == false) {
this.stop();
return;
}
Expand All @@ -315,7 +319,8 @@ else if (element instanceof DistinctPlanNode) {
// PROJECTION
// ---------------------------------------------------
else if (element instanceof ProjectionPlanNode) {
if ((state.areChildrenDirty(element) || force) && PlanOptimizerUtil.updateProjectionColumns(state, (ProjectionPlanNode) element) == false) {
if ((state.areChildrenDirty(element) || force) &&
PlanOptimizerUtil.updateProjectionColumns(state, (ProjectionPlanNode) element) == false) {
this.stop();
return;
}
Expand Down Expand Up @@ -390,14 +395,23 @@ public static boolean updateDistinctColumns(final PlanOptimizerState state, Dist
// // the guid
// node.setDistinctColumnGuid(new_pc.guid());

for (Integer guid : node.getOutputColumnGUIDs()) {
node.setDistinctColumnGuid(guid);
PlanColumn found = null;
for (Integer new_guid : node.getOutputColumnGUIDs()) {
PlanColumn new_pc = state.plannerContext.get(new_guid);
assert (new_pc != null);
if (new_pc.equals(orig_pc, true, true)) {
found = new_pc;
break;
}
} // FOR
assert(found != null) :
"Failed to find DistinctColumn " + orig_pc + " in " + node + " output columns";
node.setDistinctColumnGuid(found.guid());

state.markDirty(node);
// if (debug.get())
// LOG.debug(String.format("Updated %s with proper distinct column guid: ORIG[%d] => NEW[%d]",
// node, orig_guid, new_pc.guid()));
if (debug.get())
LOG.debug(String.format("Updated %s with proper distinct column guid: ORIG[%d] => NEW[%d]",
node, orig_guid, found.guid()));

return (true);
}
Expand Down Expand Up @@ -893,9 +907,9 @@ public static boolean updateJoinsColumns(final PlanOptimizerState state, final A
if (trace.get())
LOG.trace("New Inner Input GUIDs: " + inner_new_input_guids);

// ---------------------------------------------------
// NEST LOOP INDEX
// ---------------------------------------------------
// ---------------------------------------------------
// NEST LOOP INDEX
// ---------------------------------------------------
} else {
// Otherwise, just grab all of the columns for the target table in
// the inline scan
Expand Down Expand Up @@ -1235,10 +1249,10 @@ else if (element instanceof OrderByPlanNode) {
// it
if (exists == false) {
ref_columns.add(above_pc);
if (debug.get())
LOG.debug(String.format("Added PlanColumn #%d to list of referenced columns. [%s]", col_guid, above_pc.getDisplayName()));
} else if (debug.get()) {
LOG.debug("Skipped PlanColumn #" + col_guid + " because it already exists.");
if (trace.get())
LOG.trace(String.format("Added PlanColumn #%d to list of referenced columns. [%s]", col_guid, above_pc.getDisplayName()));
} else if (trace.get()) {
LOG.trace("Skipped PlanColumn #" + col_guid + " because it already exists.");
}
}
} // FOR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ protected void callback(AbstractPlanNode element) {
// ---------------------------------------------------
// Distributed NestLoopIndexPlanNode
// This is where the NestLoopIndexPlanNode immediately passes
// its
// intermediate results to a SendPlanNode
// its intermediate results to a SendPlanNode
// ---------------------------------------------------
else if (element instanceof NestLoopIndexPlanNode && element.getParent(0) instanceof SendPlanNode) {
assert (state.join_node_index.size() == state.join_tbl_mapping.size()) : "Join data structures don't have the same size";
Expand Down
5 changes: 3 additions & 2 deletions src/frontend/org/voltdb/ClientResponseImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,10 @@ public String getAppStatusString() {
@Override
public String toString() {
Map<String, Object> m = new ListOrderedMap<String, Object>();
m.put("Status", this.status + " / " + this.statusString);
m.put("Status", this.status +
(this.statusString == null || this.statusString.isEmpty() ? "" : " / " + this.statusString));
m.put("Handle", this.clientHandle);
m.put("Timestamp", this.requestCounter);
m.put("RequestCounter", this.requestCounter);
m.put("Throttle", this.throttle);
m.put("SinglePartition", this.singlepartition);
m.put("BasePartition", this.basePartition);
Expand Down
5 changes: 5 additions & 0 deletions src/frontend/org/voltdb/compiler/VoltProjectBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ protected String getStmtProcedureSQL(String name) {
return (null);
}

public void clearProcedures() {
m_procedures.clear();
m_procInfoOverrides.clear();
}

/**
* Create a single statement procedure that only has one query
* The input parameters to the SQL statement will be automatically passed
Expand Down
28 changes: 27 additions & 1 deletion tests/frontend/edu/brown/optimizer/TestPlanOptimizerTPCC.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
import org.voltdb.catalog.Procedure;
import org.voltdb.catalog.Statement;
import org.voltdb.catalog.Table;
import org.voltdb.planner.PlanColumn;
import org.voltdb.planner.PlannerContext;
import org.voltdb.plannodes.AbstractPlanNode;
import org.voltdb.plannodes.AbstractScanPlanNode;
import org.voltdb.plannodes.DistinctPlanNode;
import org.voltdb.plannodes.IndexScanPlanNode;
import org.voltdb.plannodes.ProjectionPlanNode;
import org.voltdb.types.PlanNodeType;
Expand All @@ -28,7 +31,30 @@ public class TestPlanOptimizerTPCC extends BasePlanOptimizerTestCase {

@Override
protected void setUp() throws Exception {
super.setUp(ProjectType.TPCC);
super.setUp(ProjectType.TPCC);
}

/**
* testProjectionPushdownDistinctOffset
*/
public void testProjectionPushdownDistinctOffset() throws Exception {
Procedure catalog_proc = this.getProcedure(slev.class);
Statement catalog_stmt = this.getStatement(catalog_proc, "GetStockCount");

// Grab the root node of the multi-partition query plan tree for this Statement
AbstractPlanNode root = PlanNodeUtil.getRootPlanNodeForStatement(catalog_stmt, true);
assertNotNull(root);

// Make sure that the DistinctPlanNode's target column is OL_I_ID
Collection<DistinctPlanNode> dist_nodes = PlanNodeUtil.getPlanNodes(root, DistinctPlanNode.class);
assertEquals(1, dist_nodes.size());
DistinctPlanNode dist_node = CollectionUtil.first(dist_nodes);
assertNotNull(dist_node);

int col_guid = dist_node.getDistinctColumnGuid();
PlanColumn pc = PlannerContext.singleton().get(col_guid);
assertNotNull(pc);
assertEquals("OL_I_ID", pc.getDisplayName());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ public class SelectAll extends VoltProcedure {

public VoltTable[] run() {
voltQueueSQL(warehouse);
// voltQueueSQL(district);
// voltQueueSQL(item);
// voltQueueSQL(customer);
// voltQueueSQL(history);
// voltQueueSQL(stock);
// voltQueueSQL(orders);
// voltQueueSQL(new_order);
// voltQueueSQL(order_line);
voltQueueSQL(district);
voltQueueSQL(item);
voltQueueSQL(customer);
voltQueueSQL(history);
voltQueueSQL(stock);
voltQueueSQL(orders);
voltQueueSQL(new_order);
voltQueueSQL(order_line);
return voltExecuteSQL();
}
}
37 changes: 21 additions & 16 deletions tests/frontend/org/voltdb/benchmark/tpcc/procedures/slev.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,27 +62,32 @@
)
public class slev extends VoltProcedure {

public final SQLStmt GetOId = new SQLStmt("SELECT D_NEXT_O_ID FROM DISTRICT WHERE D_W_ID = ? AND D_ID = ?;");
public final SQLStmt GetOId = new SQLStmt(
"SELECT D_NEXT_O_ID " +
" FROM DISTRICT " +
" WHERE D_W_ID = ? AND D_ID = ?;"
);

public final SQLStmt GetStockCount = new SQLStmt(
"SELECT COUNT(DISTINCT(OL_I_ID)) FROM ORDER_LINE, STOCK " +
"WHERE OL_W_ID = ? AND " +
"OL_D_ID = ? AND " +
"OL_O_ID < ? AND " +
"OL_O_ID >= ? AND " +
"S_W_ID = ? AND " +
"S_I_ID = OL_I_ID AND " +
"S_QUANTITY < ?;");
"SELECT COUNT(DISTINCT(OL_I_ID)) " +
" FROM ORDER_LINE, STOCK " +
" WHERE OL_W_ID = ? " +
" AND OL_D_ID = ? " +
" AND OL_O_ID < ? " +
" AND OL_O_ID >= ? " +
" AND S_W_ID = ? " +
" AND S_I_ID = OL_I_ID " +
" AND S_QUANTITY < ?;"
);

public VoltTable[] run(short w_id, byte d_id, int threshold) {

voltQueueSQL(GetOId, w_id, d_id);
if (false) { // FIXME
final VoltTable result = voltExecuteSQL()[0];
final long o_id = result.asScalarLong(); //if invalid (i.e. no matching o_id), we expect a fail here.
voltQueueSQL(GetStockCount, w_id, d_id, o_id, o_id - 20, w_id, threshold);
}
//return assumes that o_id is a temporary variable, and that stock_count is a necessarily returned variable.
final VoltTable result = voltExecuteSQL()[0];
long o_id = result.asScalarLong(); //if invalid (i.e. no matching o_id), we expect a fail here.
voltQueueSQL(GetStockCount, w_id, d_id, o_id, o_id - 20, w_id, threshold);

// Return assumes that o_id is a temporary variable,
// and that stock_count is a necessarily returned variable.
return voltExecuteSQL();
}
}
5 changes: 5 additions & 0 deletions tests/frontend/org/voltdb/regressionsuites/LocalCluster.java
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ public int getNodeCount()
{
return m_siteCount;
}

@Override
public int getPartitionCount() {
return (m_siteCount * m_partitionPerSite);
}

@Override
public void finalize() throws Throwable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ public int getNodeCount()
{
return 1;
}

@Override
public int getPartitionCount() {
return (m_partitionCount);
}

@Override
public List<String> shutDown() throws InterruptedException {
Expand Down

0 comments on commit 090c553

Please sign in to comment.