Merge pull request #3991 from katzyn/json

Fix SQL nulls in JSON

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,8 @@ <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>Issue #3966: Unable to insert null into a JSON column
+</li>
 <li>Issue #3554: Regression in 2.1.214 when joining 2 recursive CTE containing bind values
 </li>
 <li>PR #3989: Refactor CTE-related code and reduce recompilations

h2/src/main/org/h2/command/Parser.java

@@ -2503,7 +2503,7 @@ private Query parseQueryExpression() {
                 }
                 query = parseQueryExpressionBodyAndEndOfQuery(start);
                 query.setNeverLazy(true);
-                query.setWithClause(queryScope.tableSubqeries);
+                query.setWithClause(queryScope.tableSubqueries);
             } finally {
                 queryScope = outerQueryScope;
             }
@@ -6961,12 +6961,12 @@ private void parseSingleCommonTableExpression(boolean isPotentiallyRecursive) {
              */
             Table recursiveTable = new ShadowTable(database.getMainSchema(), cteName, columns.toArray(new Column[0]));
             BitSet outerUsedParameters = openParametersScope();
-            queryScope.tableSubqeries.put(cteName, recursiveTable);
+            queryScope.tableSubqueries.put(cteName, recursiveTable);
             try {
                 withQuery = parseQuery();
             } finally {
                 queryParameters = closeParametersScope(outerUsedParameters);
-                queryScope.tableSubqeries.remove(cteName);
+                queryScope.tableSubqueries.remove(cteName);
             }
             columnTemplateList = QueryExpressionTable.createQueryColumnTemplateList(cols, withQuery);
             sql = withQuery.getPlanSQL(DEFAULT_SQL_FLAGS);
@@ -6989,7 +6989,7 @@ private void parseSingleCommonTableExpression(boolean isPotentiallyRecursive) {
             sql = withQuery.getPlanSQL(DEFAULT_SQL_FLAGS);
         }
         read(CLOSE_PAREN);
-        queryScope.tableSubqeries.put(cteName, new CTE(cteName, withQuery, StringUtils.cache(sql),
+        queryScope.tableSubqueries.put(cteName, new CTE(cteName, withQuery, StringUtils.cache(sql),
                 queryParameters, columnTemplateList.toArray(new Column[0]), session, isPotentiallyRecursive,
                 queryScope));
     }
@@ -7894,7 +7894,7 @@ private Table readTableOrView(String tableName, boolean resolveMaterializedView)
 
     private Table getWithSubquery(String name) {
         for (QueryScope queryScope = this.queryScope; queryScope != null; queryScope = queryScope.parent) {
-            Table tableSubquery = queryScope.tableSubqeries.get(name);
+            Table tableSubquery = queryScope.tableSubqueries.get(name);
             if (tableSubquery != null) {
                 return tableSubquery;
             }

h2/src/main/org/h2/command/QueryScope.java

@@ -9,15 +9,30 @@
 
 import org.h2.table.Table;
 
+/**
+ * The scope of identifiers for a query with the WITH clause.
+ */
 public final class QueryScope {
 
+    /**
+     * The scope of a parent query with the WITH clause.
+     */
     public final QueryScope parent;
 
-    public final LinkedHashMap<String, Table> tableSubqeries;
+    /**
+     * The elements of the WITH list.
+     */
+    public final LinkedHashMap<String, Table> tableSubqueries;
 
+    /**
+     * Creates new instance of a query scope.
+     *
+     * @param parent
+     *            parent scope, or {@code null}
+     */
     public QueryScope(QueryScope parent) {
         this.parent = parent;
-        tableSubqeries = new LinkedHashMap<>();
+        tableSubqueries = new LinkedHashMap<>();
     }
 
 }

h2/src/main/org/h2/expression/Format.java

@@ -9,6 +9,7 @@
 import org.h2.value.TypeInfo;
 import org.h2.value.Value;
 import org.h2.value.ValueJson;
+import org.h2.value.ValueNull;
 
 /**
  * A format clause such as FORMAT JSON.
@@ -45,16 +46,27 @@ public Value getValue(SessionLocal session) {
      * @return the value with applied format
      */
     public Value getValue(Value value) {
+        return applyJSON(value);
+    }
+
+    /**
+     * Applies the JSON format to the specified value.
+     *
+     * @param value
+     *            the value
+     * @return the value with applied format
+     */
+    public static Value applyJSON(Value value) {
         switch (value.getValueType()) {
         case Value.NULL:
-            return ValueJson.NULL;
+            return ValueNull.INSTANCE;
         case Value.VARCHAR:
         case Value.VARCHAR_IGNORECASE:
         case Value.CHAR:
         case Value.CLOB:
             return ValueJson.fromJson(value.getString());
         default:
-            return value.convertTo(TypeInfo.TYPE_JSON);
+            return value.convertToJson(TypeInfo.TYPE_JSON, Value.CONVERT_TO, null);
         }
     }
 

h2/src/main/org/h2/expression/aggregate/Aggregate.java

@@ -693,7 +693,7 @@ public Value getAggregatedValue(SessionLocal session, Object aggregateData) {
                     throw DbException.getInvalidValueException("JSON_OBJECTAGG key", "NULL");
                 }
                 Value value = row[1];
-                if (value == ValueNull.INSTANCE) {
+                if (value == ValueNull.INSTANCE || value == ValueJson.NULL) {
                     if ((flags & JsonConstructorUtils.JSON_ABSENT_ON_NULL) != 0) {
                         continue;
                     }

h2/src/main/org/h2/expression/function/JsonConstructorFunction.java

@@ -66,7 +66,7 @@ private Value jsonObject(SessionLocal session, Expression[] args) {
                 throw DbException.getInvalidValueException("JSON_OBJECT key", "NULL");
             }
             Value value = args[i++].getValue(session);
-            if (value == ValueNull.INSTANCE) {
+            if (value == ValueNull.INSTANCE || value == ValueJson.NULL) {
                 if ((flags & JsonConstructorUtils.JSON_ABSENT_ON_NULL) != 0) {
                     continue;
                 } else {

h2/src/main/org/h2/res/help.csv

@@ -4592,8 +4592,12 @@ The allowed length is from 1 to 1,000,000,000 bytes.
 The length is a size constraint; only the actual data is persisted.
 
 To set a JSON value with ""java.lang.String"" in a PreparedStatement use a ""FORMAT JSON"" data format
-(""INSERT INTO TEST(ID, DATA) VALUES (?, ? FORMAT JSON)"").
-Without the data format VARCHAR values are converted to a JSON string values.
+(""INSERT INTO TEST(ID, DATA) VALUES (?, ? FORMAT JSON)"") or use
+""setObject(parameter, jsonText, H2Type.JSON)"" instead of ""setString()"".
+
+Without the data format VARCHAR values are converted to JSON string values.
+
+SQL/JSON null value ""JSON 'null'"" is distinct from the SQL null value ""NULL"".
 
 Order of object members is preserved as is.
 Duplicate object member names are allowed.

h2/src/main/org/h2/util/json/JsonConstructorUtils.java

@@ -46,7 +46,7 @@ public static void jsonObjectAppend(ByteArrayOutputStream baos, String key, Valu
             baos.write(',');
         }
         JSONByteArrayTarget.encodeString(baos, key).write(':');
-        byte[] b = value.convertTo(TypeInfo.TYPE_JSON).getBytesNoCopy();
+        byte[] b = value.convertToJson(TypeInfo.TYPE_JSON, Value.CONVERT_TO, null).getBytesNoCopy();
         baos.write(b, 0, b.length);
     }
 
@@ -89,7 +89,7 @@ public static Value jsonObjectFinish(ByteArrayOutputStream baos, int flags) {
      *            the flags ({@link #JSON_ABSENT_ON_NULL})
      */
     public static void jsonArrayAppend(ByteArrayOutputStream baos, Value value, int flags) {
-        if (value == ValueNull.INSTANCE) {
+        if (value == ValueNull.INSTANCE || value == ValueJson.NULL) {
             if ((flags & JSON_ABSENT_ON_NULL) != 0) {
                 return;
             }

h2/src/main/org/h2/value/Value.java

@@ -392,19 +392,19 @@ public abstract class Value extends VersionedValue<Value> implements HasSQL, Typ
      * Convert a value to the specified type without taking scale and precision
      * into account.
      */
-    static final int CONVERT_TO = 0;
+    public static final int CONVERT_TO = 0;
 
     /**
      * Cast a value to the specified type. The scale is set if applicable. The
      * value is truncated to a required precision.
      */
-    static final int CAST_TO = 1;
+    public static final int CAST_TO = 1;
 
     /**
      * Cast a value to the specified type for assignment. The scale is set if
      * applicable. If precision is too large an exception is thrown.
      */
-    static final int ASSIGN_TO = 2;
+    public static final int ASSIGN_TO = 2;
 
     /**
      * Returns name of the specified data type.
@@ -2384,7 +2384,20 @@ public final ValueGeometry convertToGeometry(ExtTypeInfoGeometry extTypeInfo) {
         return result;
     }
 
-    private ValueJson convertToJson(TypeInfo targetType, int conversionMode, Object column) {
+    /**
+     * Converts this value to a JSON value. May not be called on a NULL
+     * value.
+     *
+     * @param targetType
+     *            the type of the returned value
+     * @param conversionMode
+     *            conversion mode
+     * @param column
+     *            the column (if any), used to improve the error message if
+     *            conversion fails
+     * @return the JSON value
+     */
+    public ValueJson convertToJson(TypeInfo targetType, int conversionMode, Object column) {
         ValueJson v;
         switch (getValueType()) {
         case JSON:

h2/src/main/org/h2/value/ValueToObjectConverter.java

@@ -34,6 +34,7 @@
 import org.h2.api.ErrorCode;
 import org.h2.api.Interval;
 import org.h2.engine.Session;
+import org.h2.expression.Format;
 import org.h2.jdbc.JdbcArray;
 import org.h2.jdbc.JdbcBlob;
 import org.h2.jdbc.JdbcClob;
@@ -86,69 +87,72 @@ public static Value objectToValue(Session session, Object x, int type) {
             return ValueNull.INSTANCE;
         } else if (type == Value.JAVA_OBJECT) {
             return ValueJavaObject.getNoCopy(JdbcUtils.serialize(x, session.getJavaObjectSerializer()));
-        } else if (x instanceof Value) {
-            Value v = (Value) x;
+        }
+        Value v;
+        Class<?> clazz;
+        if (x instanceof Value) {
+            v = (Value) x;
             if (v instanceof ValueLob) {
                 session.addTemporaryLob((ValueLob) v);
             }
-            return v;
-        }
-        Class<?> clazz = x.getClass();
-        if (clazz == String.class) {
-            return ValueVarchar.get((String) x, session);
+        } else if ((clazz = x.getClass()) == String.class) {
+            v = ValueVarchar.get((String) x, session);
         } else if (clazz == Long.class) {
-            return ValueBigint.get((Long) x);
+            v = ValueBigint.get((Long) x);
         } else if (clazz == Integer.class) {
-            return ValueInteger.get((Integer) x);
+            v = ValueInteger.get((Integer) x);
         } else if (clazz == Boolean.class) {
-            return ValueBoolean.get((Boolean) x);
+            v = ValueBoolean.get((Boolean) x);
         } else if (clazz == Byte.class) {
-            return ValueTinyint.get((Byte) x);
+            v = ValueTinyint.get((Byte) x);
         } else if (clazz == Short.class) {
-            return ValueSmallint.get((Short) x);
+            v = ValueSmallint.get((Short) x);
         } else if (clazz == Float.class) {
-            return ValueReal.get((Float) x);
+            v = ValueReal.get((Float) x);
         } else if (clazz == Double.class) {
-            return ValueDouble.get((Double) x);
+            v = ValueDouble.get((Double) x);
         } else if (clazz == byte[].class) {
-            return ValueVarbinary.get((byte[]) x);
+            v = ValueVarbinary.get((byte[]) x);
         } else if (clazz == UUID.class) {
-            return ValueUuid.get((UUID) x);
+            v = ValueUuid.get((UUID) x);
         } else if (clazz == Character.class) {
-            return ValueChar.get(((Character) x).toString());
+            v = ValueChar.get(((Character) x).toString());
         } else if (clazz == LocalDate.class) {
-            return JSR310Utils.localDateToValue((LocalDate) x);
+            v = JSR310Utils.localDateToValue((LocalDate) x);
         } else if (clazz == LocalTime.class) {
-            return JSR310Utils.localTimeToValue((LocalTime) x);
+            v = JSR310Utils.localTimeToValue((LocalTime) x);
         } else if (clazz == LocalDateTime.class) {
-            return JSR310Utils.localDateTimeToValue((LocalDateTime) x);
+            v = JSR310Utils.localDateTimeToValue((LocalDateTime) x);
         } else if (clazz == Instant.class) {
-            return JSR310Utils.instantToValue((Instant) x);
+            v = JSR310Utils.instantToValue((Instant) x);
         } else if (clazz == OffsetTime.class) {
-            return JSR310Utils.offsetTimeToValue((OffsetTime) x);
+            v = JSR310Utils.offsetTimeToValue((OffsetTime) x);
         } else if (clazz == OffsetDateTime.class) {
-            return JSR310Utils.offsetDateTimeToValue((OffsetDateTime) x);
+            v = JSR310Utils.offsetDateTimeToValue((OffsetDateTime) x);
         } else if (clazz == ZonedDateTime.class) {
-            return JSR310Utils.zonedDateTimeToValue((ZonedDateTime) x);
+            v = JSR310Utils.zonedDateTimeToValue((ZonedDateTime) x);
         } else if (clazz == Interval.class) {
             Interval i = (Interval) x;
-            return ValueInterval.from(i.getQualifier(), i.isNegative(), i.getLeading(), i.getRemaining());
+            v = ValueInterval.from(i.getQualifier(), i.isNegative(), i.getLeading(), i.getRemaining());
         } else if (clazz == Period.class) {
-            return JSR310Utils.periodToValue((Period) x);
+            v = JSR310Utils.periodToValue((Period) x);
         } else if (clazz == Duration.class) {
-            return JSR310Utils.durationToValue((Duration) x);
-        }
-        if (x instanceof Object[]) {
-            return arrayToValue(session, x);
+            v = JSR310Utils.durationToValue((Duration) x);
+        } else if (x instanceof Object[]) {
+            v = arrayToValue(session, x);
         } else if (GEOMETRY_CLASS != null && GEOMETRY_CLASS.isAssignableFrom(clazz)) {
-            return ValueGeometry.getFromGeometry(x);
+            v = ValueGeometry.getFromGeometry(x);
         } else if (x instanceof BigInteger) {
-            return ValueNumeric.get((BigInteger) x);
+            v = ValueNumeric.get((BigInteger) x);
         } else if (x instanceof BigDecimal) {
-            return ValueNumeric.getAnyScale((BigDecimal) x);
+            v = ValueNumeric.getAnyScale((BigDecimal) x);
         } else {
-            return otherToValue(session, x);
+            v = otherToValue(session, x);
+        }
+        if (type == Value.JSON) {
+            v = Format.applyJSON(v);
         }
+        return v;
     }
 
     private static Value otherToValue(Session session, Object x) {

h2/src/test/org/h2/test/db/TestPersistentCommonTableExpressions.java

@@ -103,7 +103,7 @@ private void testPersistentRecursiveTableInCreateView() throws Exception {
                 +"      FROM my_tree mt                                                                        \n"
                 +"INNER JOIN tree_cte mtc ON mtc.child_fk = mt.parent_fk                                       \n"
                 +"),                                                                                           \n"
-                +"unused_cte(unUsedColumn) AS ( SELECT 1 AS unUsedColumn )                                                   \n"
+                +"unused_cte(unUsedColumn) AS ( SELECT 1 AS unUsedColumn )                                     \n"
                 +"SELECT sub_tree_root_id, tree_level, parent_fk, child_fk FROM tree_cte;                      \n";
 
         String withQuery = "SELECT * FROM v_my_tree";
@@ -227,7 +227,7 @@ private void testPersistentRecursiveTableInCreateViewDropAllObjects() throws Exc
                 +"      FROM my_tree mt                                                                        \n"
                 +"INNER JOIN tree_cte mtc ON mtc.child_fk = mt.parent_fk                                       \n"
                 +"),                                                                                           \n"
-                +"unused_cte(unUsedColumn) AS ( SELECT 1 AS unUsedColumn )                                                   \n"
+                +"unused_cte(unUsedColumn) AS ( SELECT 1 AS unUsedColumn )                                     \n"
                 +"SELECT sub_tree_root_id, tree_level, parent_fk, child_fk FROM tree_cte;                      \n";
 
         String withQuery = "SELECT * FROM v_my_tree";

h2/src/test/org/h2/test/jdbc/TestPreparedStatement.java

@@ -917,11 +917,25 @@ private void testJson(Connection conn) throws SQLException {
         prep.setInt(1, 2);
         prep.setString(2, "[1]");
         prep.executeUpdate();
+        prep.setInt(1, 3);
+        prep.setString(2, null);
+        prep.executeUpdate();
+        prep = conn.prepareStatement("INSERT INTO TEST VALUES (?, ?)");
+        prep.setInt(1, 4);
+        prep.setObject(2, "[1]", H2Type.JSON);
+        prep.executeUpdate();
+        prep.setInt(1, 5);
+        prep.setObject(2, null, H2Type.JSON);
+        prep.executeUpdate();
         try (ResultSet rs = stat.executeQuery("SELECT J FROM TEST ORDER BY ID")) {
             assertTrue(rs.next());
             assertEquals("\"[1]\"", rs.getString(1));
-            assertTrue(rs.next());
-            assertEquals("[1]", rs.getString(1));
+            for (int i = 0; i < 2; i++) {
+                assertTrue(rs.next());
+                assertEquals("[1]", rs.getString(1));
+                assertTrue(rs.next());
+                assertEquals(null, rs.getString(1));
+            }
             assertFalse(rs.next());
         }
         stat.execute("DROP TABLE TEST");

h2/src/test/org/h2/test/scripts/datatypes/json.sql

@@ -228,9 +228,9 @@ DROP TABLE TEST;
 > ok
 
 SELECT NULL FORMAT JSON, (NULL FORMAT JSON) IS NULL;
-> JSON 'null' FALSE
-> ----------- -----
-> null        FALSE
+> NULL TRUE
+> ---- ----
+> null TRUE
 > rows: 1
 
 CREATE MEMORY TABLE TEST(J JSON) AS VALUES ('["\u00A7''",{}]' FORMAT JSON);