Skip to content

Commit

Permalink
Merge pull request #3563 from VoltDB/ENG-10111-max-predicates
Browse files Browse the repository at this point in the history
Handle stack overflow error condition for queries due to deep expression tree
  • Loading branch information
hhakimi53 committed May 20, 2016
2 parents 842a240 + 4facd0e commit 23a9410
Show file tree
Hide file tree
Showing 22 changed files with 599 additions and 79 deletions.
37 changes: 13 additions & 24 deletions src/frontend/org/voltdb/compiler/AsyncCompilerAgentHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -244,32 +244,21 @@ else if (work.invocationName.startsWith("@AdHoc")) {
private byte[] addDDLToCatalog(Catalog oldCatalog, byte[] oldCatalogBytes, String[] adhocDDLStmts)
throws IOException, VoltCompilerException
{
VoltCompilerReader ddlReader = null;
try {
InMemoryJarfile jarfile = CatalogUtil.loadInMemoryJarFile(oldCatalogBytes);

StringBuilder sb = new StringBuilder();
compilerLog.info("Applying the following DDL to cluster:");
for (String stmt : adhocDDLStmts) {
compilerLog.info("\t" + stmt);
sb.append(stmt);
sb.append(";\n");
}
String newDDL = sb.toString();
compilerLog.trace("Adhoc-modified DDL:\n" + newDDL);
InMemoryJarfile jarfile = CatalogUtil.loadInMemoryJarFile(oldCatalogBytes);

VoltCompiler compiler = new VoltCompiler();
compiler.compileInMemoryJarfileWithNewDDL(jarfile, newDDL, oldCatalog);
return jarfile.getFullJarBytes();
}
finally {
if (ddlReader != null) {
try {
ddlReader.close();
}
catch (IOException ioe) {}
}
StringBuilder sb = new StringBuilder();
compilerLog.info("Applying the following DDL to cluster:");
for (String stmt : adhocDDLStmts) {
compilerLog.info("\t" + stmt);
sb.append(stmt);
sb.append(";\n");
}
String newDDL = sb.toString();
compilerLog.trace("Adhoc-modified DDL:\n" + newDDL);

VoltCompiler compiler = new VoltCompiler();
compiler.compileInMemoryJarfileWithNewDDL(jarfile, newDDL, oldCatalog);
return jarfile.getFullJarBytes();
}

private byte[] modifyCatalogClasses(byte[] oldCatalogBytes, String deletePatterns,
Expand Down
11 changes: 10 additions & 1 deletion src/frontend/org/voltdb/compiler/PlannerTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,16 @@ synchronized AdHocPlannedStatement planSql(String sqlIn, StatementPartitioning p
//////////////////////
// OUTPUT THE RESULT
//////////////////////
CorePlan core = new CorePlan(plan, m_catalogHash);
CorePlan core = null;
try {
core = new CorePlan(plan, m_catalogHash);
}
catch (StackOverflowError error) {
//
String msg = "Encountered stack overflow error. "
+ "Try reducing the number of predicate expressions in the query.";
throw new RuntimeException(msg);
}
AdHocPlannedStatement ahps = new AdHocPlannedStatement(plan, core);

// do not put wrong parameter explain query into cache
Expand Down
13 changes: 11 additions & 2 deletions src/frontend/org/voltdb/compiler/StatementCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,18 @@ static boolean compileFromSqlTextAndUpdateCatalog(VoltCompiler compiler, HSQLInt
*/
static byte[] writePlanBytes(VoltCompiler compiler, PlanFragment fragment, AbstractPlanNode planGraph)
throws VoltCompilerException {
String json = null;
// get the plan bytes
PlanNodeList node_list = new PlanNodeList(planGraph);
String json = node_list.toJSONString();
try {
PlanNodeList node_list = new PlanNodeList(planGraph);
json = node_list.toJSONString();
}
catch (StackOverflowError error) {
//
String msg = "Encountered stack overflow error. "
+ "Try reducing the number of predicate expressions in the query.";
throw compiler.new VoltCompilerException(msg);
}
compiler.captureDiagnosticJsonFragment(json);
// Place serialized version of PlanNodeTree into a PlanFragment
byte[] jsonBytes = json.getBytes(Charsets.UTF_8);
Expand Down
13 changes: 10 additions & 3 deletions src/frontend/org/voltdb/compiler/VoltCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,8 @@ private InMemoryJarfile compileInternal(
fw.close();
m_reportPath = file.getAbsolutePath();
}
} catch (IOException e) {
}
catch (IOException e) {
e.printStackTrace();
return null;
}
Expand Down Expand Up @@ -2279,10 +2280,16 @@ public void compileInMemoryJarfileWithNewDDL(InMemoryJarfile jarfile, String new
m_classLoader = originalClassLoader;

if (canonicalDDLReader != null) {
try { canonicalDDLReader.close(); } catch (IOException ioe) {}
try {
canonicalDDLReader.close();
}
catch (IOException ioe) {}
}
if (newDDLReader != null) {
try { newDDLReader.close(); } catch (IOException ioe) {}
try {
newDDLReader.close();
}
catch (IOException ioe) {}
}
}
}
Expand Down
36 changes: 25 additions & 11 deletions src/frontend/org/voltdb/planner/QueryPlanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,16 @@ public void parse() throws PlanningErrorException {
// this is much easier to parse than SQL and is checked against the catalog
try {
m_xmlSQL = m_HSQL.getXMLCompiledStatement(m_sql);
} catch (HSQLParseException e) {
}
catch (HSQLParseException e) {
// XXXLOG probably want a real log message here
throw new PlanningErrorException(e.getMessage());
}
catch (StackOverflowError error) {
String msg = "Encountered stack overflow error. " +
"Try reducing the number of predicate expressions in the query.";
throw new PlanningErrorException(msg);
}

if (m_isUpsert) {
assert(m_xmlSQL.name.equalsIgnoreCase("INSERT"));
Expand Down Expand Up @@ -271,7 +277,7 @@ public CompiledPlan plan() throws PlanningErrorException {
}
// fall through to try replan without parameterization.
}
catch (Exception e) {
catch (Exception | StackOverflowError e) {
// ignore any errors planning with parameters
// fall through to re-planning without them
m_hasExceptionWhenParameterized = true;
Expand All @@ -282,17 +288,25 @@ public CompiledPlan plan() throws PlanningErrorException {
}
}

// if parameterization isn't requested or if it failed, plan here
CompiledPlan plan = compileFromXML(m_xmlSQL, null);
if (plan == null) {
if (m_debuggingStaticModeToRetryOnError) {
plan = compileFromXML(m_xmlSQL, null);
CompiledPlan plan = null;
try {
// if parameterization isn't requested or if it failed, plan here
plan = compileFromXML(m_xmlSQL, null);
if (plan == null) {
if (m_debuggingStaticModeToRetryOnError) {
plan = compileFromXML(m_xmlSQL, null);
}
throw new PlanningErrorException(m_recentErrorMsg);
}
throw new PlanningErrorException(m_recentErrorMsg);
}

if (m_isUpsert) {
replacePlanForUpsert(plan);
if (m_isUpsert) {
replacePlanForUpsert(plan);
}
}
catch (StackOverflowError error) {
String msg = "Encountered stack overflow error. " +
"Try reducing the number of predicate expressions in the query.";
throw new PlanningErrorException(msg);
}

return plan;
Expand Down
6 changes: 4 additions & 2 deletions src/frontend/org/voltdb/plannodes/PlanNodeList.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ public String toJSONString() {
stringer.value(node.getPlanNodeId().intValue());
}
stringer.endArray(); //end execution list
} else {
}
else {
stringer.key(Members.EXECUTE_LISTS.name()).array();
for (List<AbstractPlanNode> list : m_executeLists) {
stringer.object().key(Members.EXECUTE_LIST.name()).array();
Expand All @@ -174,7 +175,8 @@ public String toJSONString() {
}

stringer.endObject(); //end PlanNodeList
} catch (JSONException e) {
}
catch (JSONException e) {
// HACK ugly ugly to make the JSON handling
// in QueryPlanner generate a JSONException for a plan we know
// here that we can't serialize. Making this method throw
Expand Down
4 changes: 2 additions & 2 deletions src/hsqldb19b3/org/hsqldb_voltpatches/ErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public interface ErrorCode {
int M_SERVER_SECURE_VERIFY_2 = 137; // Server certificate has empty Common Name
int M_SERVER_SECURE_VERIFY_3 = 139; // Certificate Common Name[$$] does not match host name[$$]

//
//
int INVALID_LIMIT = 153; // ; in LIMIT, OFFSET or FETCH

//
Expand Down Expand Up @@ -496,7 +496,7 @@ public interface ErrorCode {
int X_42512 = 5512; // invalid property value
int X_42513 = 5513; // property cannot be changed

// constraint defintition issues
// constraint definition issues
int X_42520 = 5520; // SET NULL requires nullable column
int X_42521 = 5521; // SET DEFAULT requires column default expression for
int X_42522 = 5522; // a UNIQUE constraint already exists on the set of columns
Expand Down
13 changes: 13 additions & 0 deletions tests/frontend/org/voltdb/AdHocQueryTester.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public static void setUpSchema(VoltProjectBuilder builder,
"create view V_REPPED1 (REPPEDVAL, num_rows, sum_bigint) as " +
"select REPPEDVAL, count(*), sum(NONPART) from REPPED1 group by REPPEDVAL;" +

"create table long_query_table (id INTEGER NOT NULL, NAME VARCHAR(16));" +
"";

builder.addLiteralSchema(schema);
Expand Down Expand Up @@ -266,4 +267,16 @@ protected void runAllAdHocSPtests(int hashableA, int hashableB, int hashableC, i
// TODO: Three-way join test cases are probably required to cover all code paths through AccessPaths.
}

protected static String getQueryForLongQueryTable(int numberOfPredicates) {
StringBuilder string = new StringBuilder("SELECT count(*) FROM long_query_table ");
if (numberOfPredicates > 0) {
string.append("WHERE ID = 123 ");
for (int i = 1; i < numberOfPredicates; i++) {
string.append("AND ID > 100 ");
}
}
string.append(";");
return string.toString();
}

}
39 changes: 39 additions & 0 deletions tests/frontend/org/voltdb/TestAdHocQueries.java
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,45 @@ public void testAdHocWithParams() throws Exception {
}
}

@Test
public void testAdHocQueryForStackOverFlowCondition() throws IOException, Exception {
System.out.println("Starting testLongAdHocQuery");

VoltDB.Configuration config = setUpSPDB();
ServerThread localServer = new ServerThread(config);
localServer.start();
localServer.waitForInitialization();

m_client = ClientFactory.createClient();
m_client.createConnection("localhost", config.m_port);

String sql = getQueryForLongQueryTable(750);
try {
m_client.callProcedure("@AdHoc", sql);
fail("Query was expected to generate stack over flow error");
}
catch (Exception exception) {
System.out.println(exception.getMessage());
String expectedMsg;
expectedMsg = "Encountered stack overflow error. " +
"Try reducing the number of predicate expressions in the query.";
boolean foundMsg = exception.getMessage().contains(expectedMsg);
assert foundMsg : exception.getMessage();
}
finally {
if (m_client != null) {
m_client.close();
}
m_client = null;

if (localServer != null) {
localServer.shutdown();
localServer.join();
}
localServer = null;
}
}

private void verifyIncorrectParameterMessage(TestEnv env, String adHocQuery, Object[] params) {
int expected = 0;
for (int i=0; i < adHocQuery.length(); i++ ) {
Expand Down
54 changes: 54 additions & 0 deletions tests/frontend/org/voltdb/compiler/TestProcCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,60 @@ public void testIndexOnConstant() throws Exception {
assertEquals(2, countLinesMatching(lines, "^\\s*\\[TABLE SCAN]\\sselect.*indexed_replicated_blah.*"));
}

protected static String getQueryForFoo(int numberOfPredicates) {
StringBuilder string = new StringBuilder("SELECT * FROM FOO ");
if (numberOfPredicates > 0) {
string.append("WHERE ID = 10 ");
for (int i = 1; i < numberOfPredicates; i++) {
string.append("AND ID > 1 ");
}
}
string.append(";");
return string.toString();
}

public void testStmntWithLotsOfPredicates() throws Exception {
String simpleSchema = "Create Table foo ( " +
"id BIGINT DEFAULT 0 NOT NULL, " +
"name VARCHAR(255) NOT NULL, " +
"PRIMARY KEY(id));";

VoltProjectBuilder builder = new VoltProjectBuilder();
builder.addLiteralSchema(simpleSchema);

// generate with query with lot of predicates for stored procedure
String sql = getQueryForFoo(350);
builder.addStmtProcedure("StmntWithPredicates", sql, null);

boolean success = builder.compile(Configuration.getPathToCatalogForTest("lots_of_predicates.jar"));
assert(success);
}

public void testStmntForStackOverflowCondition() throws Exception {
String schema = "Create Table foo ( " +
"id BIGINT DEFAULT 0 NOT NULL, " +
"name VARCHAR(255) NOT NULL, " +
"PRIMARY KEY(id));";

VoltProjectBuilder builder = new VoltProjectBuilder();
ByteArrayOutputStream capturer = new ByteArrayOutputStream();
PrintStream capturing = new PrintStream(capturer);
builder.setCompilerDebugPrintStream(capturing);
builder.addLiteralSchema(schema);

// Test for stored procedure which have more than max allowable predicates in the
// results in error and does not crash or hang the system
String sql = getQueryForFoo(1800);
builder.addStmtProcedure("StmntForStaclOverFlow", sql, null);

boolean success = builder.compile(Configuration.getPathToCatalogForTest("max_plus_predicates.jar"));
assert(!success);
String captured = capturer.toString("UTF-8");
String errMsg = "Encountered stack overflow error. " +
"Try reducing the number of predicate expressions in the query";
assert(captured.contains(errMsg));
}

private int countLinesMatching(String[] lines, String pattern) {
int count = 0;
for (String string : lines) {
Expand Down
20 changes: 20 additions & 0 deletions tests/frontend/org/voltdb/planner/TestAdHocPlans.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,26 @@ public void testSP() throws Exception {
runAllAdHocSPtests(0, 1, 2, 3);
}

public void testAdHocQueryForStackOverFlowCondition() throws NoConnectionsException, IOException, ProcCallException {
// query with max predicates in where clause
String sql = getQueryForLongQueryTable(300);
runQueryTest(sql, 1, 1, 1, VALIDATING_SP_RESULT);

// generate query with lots of predicate to simulate stack overflow when parsing the expression
sql = getQueryForLongQueryTable(2000);
try {
runQueryTest(sql, 1, 1, 1, VALIDATING_SP_RESULT);
fail("Query was expected to generate stack over flow error");
}
catch (Exception exception) {
String expectedMsg;
expectedMsg = "Encountered stack overflow error. " +
"Try reducing the number of predicate expressions in the query";
boolean foundMsg = exception.getMessage().contains(expectedMsg);
assertTrue(foundMsg);
}
}

/**
* For planner-only testing, most of the args are ignored.
*/
Expand Down
1 change: 1 addition & 0 deletions tests/sqlcmd/baselines/simple/exec.errbaseline
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[max_plus_predicates]: Failed to plan for statement (sql) select count(*) FROM t WHERE c > ? and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0 and c > 0; Error: "Encountered stack overflow error. Try reducing the number of predicate expressions in the query."
Loading

0 comments on commit 23a9410

Please sign in to comment.