Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #214 from VoltDB/ENG-3304

ENG-3304 fix value type bugs uncovered by initial DECODE implementation
  • Loading branch information...
commit 655d7db3cfbfe3f5555cf3d6bb5b6b6834eba214 2 parents 077f356 + 377502c
@vtkstef vtkstef authored
View
17 src/ee/common/NValue.hpp
@@ -1882,8 +1882,8 @@ inline uint16_t NValue::getTupleStorageSize(const ValueType type) {
return sizeof(TTInt);
default:
char message[128];
- snprintf(message, 128, "NValue::getTupleStorageSize() unrecognized type"
- " '%d'", type);
+ snprintf(message, 128, "NValue::getTupleStorageSize() unsupported type '%s'",
+ getTypeName(type).c_str());
throw SerializableEEException(VOLT_EE_EXCEPTION_TYPE_EEEXCEPTION,
message);
}
@@ -2174,7 +2174,8 @@ inline void NValue::serializeToTupleStorage(void *storage, const bool isInlined,
break;
default:
char message[128];
- snprintf(message, 128, "NValue::serializeToTupleStorage() unrecognized type '%d'", type);
+ snprintf(message, 128, "NValue::serializeToTupleStorage() unrecognized type '%s'",
+ getTypeName(type).c_str());
throw SQLException(SQLException::data_exception_most_specific_type_mismatch,
message);
}
@@ -2252,8 +2253,8 @@ inline void NValue::deserializeFrom(SerializeInput &input, const ValueType type,
}
default:
char message[128];
- snprintf(message, 128, "NValue::deserializeFrom() unrecognized type '%d'",
- type);
+ snprintf(message, 128, "NValue::deserializeFrom() unrecognized type '%s'",
+ getTypeName(type).c_str());
throw SerializableEEException(VOLT_EE_EXCEPTION_TYPE_EEEXCEPTION,
message);
}
@@ -2722,8 +2723,8 @@ inline NValue NValue::op_subtract(const NValue rhs) const {
break;
}
throwDynamicSQLException("Promotion of %s and %s failed in op_subtract.",
- getTypeName(getValueType()).c_str(),
- getTypeName(rhs.getValueType()).c_str());
+ getValueTypeString().c_str(),
+ rhs.getValueTypeString().c_str());
}
inline NValue NValue::op_add(const NValue rhs) const {
@@ -2750,7 +2751,7 @@ inline NValue NValue::op_add(const NValue rhs) const {
}
throwDynamicSQLException("Promotion of %s and %s failed in op_add.",
getValueTypeString().c_str(),
- getValueTypeString().c_str());
+ rhs.getValueTypeString().c_str());
}
inline NValue NValue::op_multiply(const NValue rhs) const {
View
2  src/frontend/org/voltdb/VoltTableRow.java
@@ -729,7 +729,7 @@ final void validateColumnType(int columnIndex, VoltType... types) {
throw new RuntimeException("VoltTableRow is in an invalid state. Consider calling advanceRow().");
if ((columnIndex >= getColumnCount()) || (columnIndex < 0)) {
- throw new IndexOutOfBoundsException("Column index " + columnIndex + " is type greater than the number of columns");
+ throw new IndexOutOfBoundsException("Column index " + columnIndex + " is greater than the number of columns");
}
final VoltType columnType = getColumnType(columnIndex);
for (VoltType type : types)
View
6 src/hsqldb19b3/org/hsqldb_voltpatches/ExpressionColumn.java
@@ -845,7 +845,11 @@ VoltXMLElement voltGetXML(Session session) throws HSQLParseException
}
else if (isParam) {
exp.name = "value";
- exp.attributes.put("type", Types.getTypeName(dataType.typeCode));
+ // This eliminates a NullPointerException which MAY be a sign of insufficient type inference,
+ // but there MAY be cases where a parameter type can't legitimately be inferred, so let it go.
+ if (dataType != null) {
+ exp.attributes.put("type", Types.getTypeName(dataType.typeCode));
+ }
exp.attributes.put("isparam", "true");
}
else {
View
21 src/hsqldb19b3/org/hsqldb_voltpatches/ExpressionOp.java
@@ -524,22 +524,27 @@ public Object getValue(Session session) {
VoltXMLElement voltGetXML(Session session) throws HSQLParseException
{
String element = null;
+ boolean unsupported = false;
switch (opType) {
case OpTypes.LIMIT: element = "limit"; break;
//TODO: Enable these as they are supported in VoltDB.
// They appear to be a complete set as supported by the other methods in this module.
- case OpTypes.ALTERNATIVE :
- case OpTypes.CASEWHEN :
- case OpTypes.CAST :
- case OpTypes.ORDER_BY :
- case OpTypes.SIMPLE_COLUMN :
- case OpTypes.TABLE :
- case OpTypes.VALUE :
- case OpTypes.ZONE_MODIFIER :
+ case OpTypes.ALTERNATIVE : unsupported = true; element = "alternative"; break;
+ case OpTypes.CASEWHEN : unsupported = true; element = "case"; break;
+ case OpTypes.CAST : unsupported = true; element = "cast (possibly implied)"; break;
+ case OpTypes.ORDER_BY : unsupported = true; element = "order by"; break;
+ case OpTypes.SIMPLE_COLUMN : unsupported = true; element = "simple column"; break;
+ case OpTypes.TABLE : unsupported = true; element = "tablen"; break;
+ case OpTypes.VALUE : unsupported = true; element = "value"; break;
+ case OpTypes.ZONE_MODIFIER : unsupported = true; element = "zone modifier"; break;
default:
throw new HSQLParseException("Unsupported Expression Operation: " +
String.valueOf(opType));
}
+ if (unsupported) {
+ throw new HSQLParseException(element + " operation is not supported");
+ }
+
VoltXMLElement exp = new VoltXMLElement("operation");
// We want to keep track of which expressions are the same in the XML output
View
84 src/hsqldb19b3/org/hsqldb_voltpatches/FunctionForVoltDB.java
@@ -206,6 +206,83 @@ public void resolveTypes(Session session, Expression parent) {
nodes[1].dataType = Type.SQL_VARCHAR;
}
break;
+
+ /*
+ * Infer parameter types to make the types of the 1st, 2nd, and (if not the last) 4th, 6th, etc.
+ * arguments to DECODE as consistent as possible,
+ * and the types of the 3rd, 5th, 7th, etc. and LAST arguments as consistent as possible.
+ * Punt to inferring VARCHAR if the other arguments give no clue or are inconsistent
+ * -- the VoltDB EE complains about NULL-typed parameters but is somewhat forgiving about
+ * mixed argument types.
+ */
+ case FunctionId.FUNC_VOLT_DECODE:
+ // Track whether parameter type hinting is needed for either key or value arguments.
+ // For simplicity(?), parameters are not tracked explicitly (by position)
+ // or even by category (key vs. value). So, if any parameter hinting is required at all,
+ // all arguments get re-checked.
+ boolean needParamType = false;
+ Type inputTypeInferred = null;
+ Type resultTypeInferred = null;
+
+ for (int ii = 0; ii < nodes.length; ii++) {
+ Type argType = nodes[ii].dataType;
+ if (argType == null) {
+ // A param here means work to do, below.
+ if (nodes[ii].isParam) {
+ needParamType = true;
+ }
+ continue;
+ }
+ // Except for the first and the optional last/"default" argument,
+ // the arguments alternate between candidate inputs and candidate results.
+ if ((((ii % 2) == 0) || ii == nodes.length-1) && (ii != 0)) {
+ // These arguments represent candidate result values
+ // that may hint at the result type or require hinting from the other result values.
+ if (resultTypeInferred == null) {
+ resultTypeInferred = argType; // Take the first result type hint.
+ } else if (resultTypeInferred != argType) {
+ resultTypeInferred = Type.SQL_VARCHAR; // Discard contradictory hints.
+ }
+ } else {
+ // These arguments represent candidate input keys
+ // that may hint at the input type or may require hinting from the other input keys.
+ if (inputTypeInferred == null) {
+ inputTypeInferred = argType; // Take the first input type hint.
+ } else if (inputTypeInferred != argType) {
+ inputTypeInferred = Type.SQL_VARCHAR; // Discard contradictory hints, falling back to string type.
+ }
+ }
+ }
+
+ // With any luck, there are no parameter "?" arguments to worry about.
+ if ( ! needParamType) {
+ break;
+ }
+
+ // No luck, try to infer the parameters' types.
+ // Punt to guessing VARCHAR for lack of better information.
+ if (inputTypeInferred == null) {
+ inputTypeInferred = Type.SQL_VARCHAR;
+ }
+ if (resultTypeInferred == null) {
+ resultTypeInferred = Type.SQL_VARCHAR;
+ }
+
+ for (int ii = 0; ii < nodes.length; ii++) {
+ Type argType = nodes[ii].dataType;
+ if ((argType != null) || ! nodes[ii].isParam) {
+ continue;
+ }
+ // This is the same test as above for determining that the argument
+ // is a candidate result vs. a candidate input.
+ if ((((ii % 2) == 0) || ii == nodes.length-1) && (ii != 0)) {
+ nodes[ii].dataType = resultTypeInferred;
+ } else {
+ nodes[ii].dataType = inputTypeInferred;
+ }
+ }
+ break;
+
default:
break;
}
@@ -214,7 +291,7 @@ public void resolveTypes(Session session, Expression parent) {
if (nodes[i] != null) {
if (i >= paramTypes.length) {
// TODO support type checking for variadic functions
- continue;
+ break;
}
if (paramTypes[i] == null) {
continue; // accept all argument types
@@ -228,7 +305,10 @@ public void resolveTypes(Session session, Expression parent) {
dataType = m_def.getDataType();
if (dataType == null && nodes.length > 0) {
- Expression like_child = nodes[0];
+ if (parameterArg < 0 || parameterArg >= nodes.length) {
+ throw Error.error(ErrorCode.X_42565); // incompatible data type (so says the error -- we're missing one, actually)
+ }
+ Expression like_child = nodes[parameterArg];
if (like_child != null) {
dataType = like_child.dataType;
}
View
6 src/hsqldb19b3/org/hsqldb_voltpatches/StatementQuery.java
@@ -42,6 +42,7 @@
import org.hsqldb_voltpatches.lib.OrderedHashSet;
import org.hsqldb_voltpatches.result.Result;
import org.hsqldb_voltpatches.result.ResultMetaData;
+import org.hsqldb_voltpatches.types.Type;
/**
* Implementation of Statement for query expressions.<p>
@@ -438,7 +439,10 @@ else if ((otherCol.opType != OpTypes.SIMPLE_COLUMN) &&
parameter.attributes.put("index", String.valueOf(i));
ExpressionColumn param = parameters[i];
parameter.attributes.put("id", param.getUniqueId(session));
- parameter.attributes.put("type", Types.getTypeName(param.getDataType().typeCode));
+ Type paramType = param.getDataType();
+ if (paramType != null) {
+ parameter.attributes.put("type", Types.getTypeName(paramType.typeCode));
+ }
}
// scans
View
139 tests/frontend/org/voltdb/regressionsuites/TestFunctionsForVoltDBSuite.java
@@ -26,7 +26,6 @@
import java.io.IOException;
import org.voltdb.BackendTarget;
-import org.voltdb.VoltProcedure;
import org.voltdb.VoltTable;
import org.voltdb.VoltType;
import org.voltdb.client.Client;
@@ -43,15 +42,6 @@
public class TestFunctionsForVoltDBSuite extends RegressionSuite {
- /**
- * Inner class procedure to see if we can invoke it.
- */
- public static class InnerProc extends VoltProcedure {
- public long run() {
- return 0L;
- }
- }
-
/** Procedures used by this suite */
static final Class<?>[] PROCEDURES = { Insert.class };
@@ -307,6 +297,48 @@ public void testDECODE() throws NoConnectionsException, IOException, ProcCallExc
assertEquals(1, result.getRowCount());
assertTrue(result.advanceRow());
assertEquals("where",result.getString(1));
+
+ // param cases
+ // For project.addStmtProcedure("DECODE_PARAM_INFER_STRING", "select desc, DECODE (desc,?,?,desc) from P1 where id = ?");
+ cr = client.callProcedure("DECODE_PARAM_INFER_STRING", "Gateway", "You got it!", 4);
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals("You got it!",result.getString(1));
+
+ // For project.addStmtProcedure("DECODE_PARAM_INFER_INT", "select desc, DECODE (id,?,?,id) from P1 where id = ?");
+ cr = client.callProcedure("DECODE_PARAM_INFER_INT", 4, -4, 4);
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals(-4,result.getLong(1));
+
+ // For project.addStmtProcedure("DECODE_PARAM_INFER_DEFAULT", "select desc, DECODE (?,?,?,?) from P1 where id = ?");
+ cr = client.callProcedure("DECODE_PARAM_INFER_DEFAULT", "Gateway", "Gateway", "You got it!", "You ain't got it!", 4);
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals("You got it!",result.getString(1));
+
+ // For project.addStmtProcedure("DECODE_PARAM_INFER_CONFLICTING", "select desc, DECODE (id,1,?,2,99,'99') from P1 where id = ?");
+ cr = client.callProcedure("DECODE_PARAM_INFER_CONFLICTING", "贾鑫?贾鑫!", 1);
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals("贾鑫?贾鑫!",result.getString(1));
+
+ // For project.addStmtProcedure("DECODE_PARAM_INFER_CONFLICTING", "select desc, DECODE (id,1,?,2,99,'99') from P1 where id = ?");
+ try {
+ cr = client.callProcedure("DECODE_PARAM_INFER_CONFLICTING", 1000, 1);
+ fail("Should have thrown unfortunate type error.");
+ } catch (ProcCallException pce) {
+ String msg = pce.getMessage();
+ assertTrue(msg.contains("TYPE ERROR FOR PARAMETER 0"));
+ }
}
public void testDECODENoDefault() throws NoConnectionsException, IOException, ProcCallException {
@@ -349,6 +381,52 @@ public void testDECODEVeryLong() throws NoConnectionsException, IOException, Pro
assertEquals("where",result.getString(1));
}
+ public void testDECODEAsInput() throws NoConnectionsException, IOException, ProcCallException {
+ System.out.println("STARTING DECODE No Default");
+ Client client = getClient();
+ ClientResponse cr;
+ VoltTable result;
+
+ cr = client.callProcedure("P1.insert", 1, "zheng", 10, 1.1);
+ cr = client.callProcedure("P1.insert", 2, "li", 10, 1.1);
+ cr = client.callProcedure("P1.insert", 3, null, 10, 1.1);
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+
+ // use DECODE as string input to operator
+ cr = client.callProcedure("@AdHoc", "select desc || DECODE(id, 1, ' is the 1', ' is not the 1') from P1 where id = 2");
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals("li is not the 1",result.getString(0));
+
+ // use DECODE as integer input to operator
+ cr = client.callProcedure("@AdHoc", "select id + DECODE(desc, 'li', 0, -2*id) from P1 where id = 2");
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals(2,result.getLong(0));
+
+ // use DECODE as integer input to operator, with unused incompatible option
+ cr = client.callProcedure("@AdHoc", "select id + DECODE(id, 2, 0, 'incompatible') from P1 where id = 2");
+ assertEquals(ClientResponse.SUCCESS, cr.getStatus());
+ result = cr.getResults()[0];
+ assertEquals(1, result.getRowCount());
+ assertTrue(result.advanceRow());
+ assertEquals(2,result.getLong(0));
+
+ // use DECODE as integer input to operator, with used incompatible option
+ try {
+ cr = client.callProcedure("@AdHoc", "select id + DECODE(id, 1, 0, 'incompatible') from P1 where id = 2");
+ fail();
+ } catch (ProcCallException pce) {
+ String message = pce.getMessage();
+ // It's about that string argument to the addition operator.
+ assertTrue(message.contains("varchar"));
+ }
+ }
+
public void testFIELDFunction() throws Exception {
final String jstemplate = "{\n" +
@@ -381,7 +459,7 @@ public void testFIELDFunction() throws Exception {
cr = client.callProcedure("JS1.insert",8, null);
cr = client.callProcedure("JS1.insert",9, "{\"id\":9, \"贾鑫Vo\":\"分かりません\"}");
- cr = client.callProcedure("IdProc", "id","1");
+ cr = client.callProcedure("IdFieldProc", "id","1");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(1, result.getRowCount());
@@ -389,7 +467,7 @@ public void testFIELDFunction() throws Exception {
assertEquals(1L,result.getLong(0));
try {
- cr = client.callProcedure("IdProc", "id", 1);
+ cr = client.callProcedure("IdFieldProc", "id", 1);
fail("parameter check failed");
}
catch ( ProcCallException pcex) {
@@ -397,28 +475,28 @@ public void testFIELDFunction() throws Exception {
}
try {
- cr = client.callProcedure("IdProc", 1, "1");
+ cr = client.callProcedure("IdFieldProc", 1, "1");
fail("parameter check failed");
}
catch ( ProcCallException pcex) {
assertTrue(pcex.getMessage().contains("TYPE ERROR FOR PARAMETER 0"));
}
- cr = client.callProcedure("IdProc", "tag", "three");
+ cr = client.callProcedure("IdFieldProc", "tag", "three");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(1, result.getRowCount());
assertTrue(result.advanceRow());
assertEquals(3L,result.getLong(0));
- cr = client.callProcedure("IdProc", "bool", "false");
+ cr = client.callProcedure("IdFieldProc", "bool", "false");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(1, result.getRowCount());
assertTrue(result.advanceRow());
assertEquals(4L,result.getLong(0));
- cr = client.callProcedure("IdProc", "贾鑫Vo", "分かりません");
+ cr = client.callProcedure("IdFieldProc", "贾鑫Vo", "分かりません");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(1, result.getRowCount());
@@ -441,7 +519,7 @@ public void testFIELDFunction() throws Exception {
assertTrue(result.advanceRow());
assertEquals(8L,result.getLong(0));
- cr = client.callProcedure("InnerProc", "贾鑫Vo" ,"wakarimasen");
+ cr = client.callProcedure("InnerFieldProc", "贾鑫Vo" ,"wakarimasen");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(3, result.getRowCount());
@@ -452,7 +530,7 @@ public void testFIELDFunction() throws Exception {
assertTrue(result.advanceRow());
assertEquals(3L,result.getLong(0));
- cr = client.callProcedure("IdProc", "arr" ,"[1,2,3,4]");
+ cr = client.callProcedure("IdFieldProc", "arr" ,"[1,2,3,4]");
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
result = cr.getResults()[0];
assertEquals(3, result.getRowCount());
@@ -482,16 +560,15 @@ public void testFIELDFunctionWithInvalidJSON() throws Exception {
assertEquals(ClientResponse.SUCCESS, cr.getStatus());
try {
- cr = client.callProcedure("BadIdProc", 1, "id", "1");
+ cr = client.callProcedure("BadIdFieldProc", 1, "id", "1");
fail("document validity check failed");
}
catch(ProcCallException pcex) {
- assertTrue(pcex.getMessage().contains(
- "'{\"id\":1 \"bool\": false}' is not valid JSON"
- ));
+ String msg = pcex.getMessage();
+ assertTrue(msg.contains("'{\"id\":1 \"bool\": false}' is not valid JSON"));
}
try {
- cr = client.callProcedure("BadIdProc", 2, "id", "2");
+ cr = client.callProcedure("BadIdFieldProc", 2, "id", "2");
fail("document validity check failed");
}
catch(ProcCallException pcex) {
@@ -531,21 +608,21 @@ public TestFunctionsForVoltDBSuite(String name) {
"CREATE PROCEDURE FieldProc AS\n" +
" SELECT FIELD(DOC, ?) AS JFIELD FROM JS1 WHERE FIELD( DOC, ?) = ?\n" +
";\n" +
- "CREATE PROCEDURE IdProc AS\n" +
- " SELECT ID FROM JS1 WHERE FIELD( DOC, ?) = ?\n" +
+ "CREATE PROCEDURE IdFieldProc AS\n" +
+ " SELECT ID FROM JS1 WHERE FIELD( DOC, ?) = ? ORDER BY ID\n" +
";\n" +
- "CREATE PROCEDURE InnerProc AS\n" +
- " SELECT ID FROM JS1 WHERE FIELD(FIELD(DOC, 'inner'), ?) = ?\n" +
+ "CREATE PROCEDURE InnerFieldProc AS\n" +
+ " SELECT ID FROM JS1 WHERE FIELD(FIELD(DOC, 'inner'), ?) = ? ORDER BY ID\n" +
";\n" +
"CREATE PROCEDURE NullFieldProc AS\n" +
- " SELECT ID FROM JS1 WHERE FIELD( DOC, ?) IS NULL\n" +
+ " SELECT ID FROM JS1 WHERE FIELD( DOC, ?) IS NULL ORDER BY ID\n" +
";\n" +
"CREATE TABLE JSBAD (\n" +
" ID INTEGER NOT NULL,\n" +
" DOC VARCHAR(8192),\n" +
" PRIMARY KEY(ID))\n" +
";\n" +
- "CREATE PROCEDURE BadIdProc AS\n" +
+ "CREATE PROCEDURE BadIdFieldProc AS\n" +
" SELECT ID FROM JSBAD WHERE ID = ? AND FIELD(DOC, ?) = ?\n" +
";\n" +
"";
@@ -575,6 +652,10 @@ public TestFunctionsForVoltDBSuite(String name) {
"'a','a'," +
"'a','a'," +
"'where') from P1 where id = ?");
+ project.addStmtProcedure("DECODE_PARAM_INFER_STRING", "select desc, DECODE (desc,?,?,desc) from P1 where id = ?");
+ project.addStmtProcedure("DECODE_PARAM_INFER_INT", "select desc, DECODE (id,?,?,id) from P1 where id = ?");
+ project.addStmtProcedure("DECODE_PARAM_INFER_DEFAULT", "select desc, DECODE (?,?,?,?) from P1 where id = ?");
+ project.addStmtProcedure("DECODE_PARAM_INFER_CONFLICTING", "select desc, DECODE (id,1,?,2,99,'贾鑫') from P1 where id = ?");
// Test OCTET_LENGTH
project.addStmtProcedure("OCTET_LENGTH", "select desc, OCTET_LENGTH (desc) from P1 where id = ?");
// Test POSITION and CHAR_LENGTH

0 comments on commit 655d7db

Please sign in to comment.
Something went wrong with that request. Please try again.