Merge pull request #1494 from katzyn/misc

Fixes for MERGE USING and other changes

h2/src/docsrc/help/help.csv

@@ -3604,7 +3604,7 @@ ENVELOPE(X)
 "
 
 "Functions (Numeric)","ABS","
-ABS (numeric|interval)
+ABS(numeric|interval)
 ","
 Returns the absolute value of a specified value.
 The returned value is of the same data type as the parameter.
@@ -3939,9 +3939,9 @@ CALL SECURE_RAND(16)
 "
 
 "Functions (Numeric)","SIGN","
-SIGN ( { numeric } )
+SIGN(numeric|interval)
 ","
-Returns -1 if the value is smaller 0, 0 if zero, and otherwise 1.
+Returns -1 if the value is smaller than 0, 0 if zero, and otherwise 1.
 ","
 SIGN(VALUE)
 "

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,16 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>Issue #1493: MERGE statement fails when it updates more than one row
+</li>
+<li>Issue #1492: Unnecessary restriction on MERGE USING statement when ON clause doesn't reference any target table columns
+</li>
+<li>Issue #1491: Unnecessary restriction on MERGE USING statement when ON predicate doesn't match inserted row
+</li>
+<li>Issue #1490: NullPointerException when running invalid MERGE statement
+</li>
+<li>Issue #1488: Improve documentation of window and some other functions
+</li>
 <li>Issue #1485: Default window frame in presence of ORDER BY is RANGE .., not ROWS
 </li>
 <li>PR #1484: New tests, reimplemented EXCLUDE clause, and assorted changes

h2/src/main/org/h2/command/dml/MergeUsing.java

@@ -6,9 +6,7 @@
 package org.h2.command.dml;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
-import java.util.HashSet;
 
 import org.h2.api.ErrorCode;
 import org.h2.api.Trigger;
@@ -19,7 +17,6 @@ import org.h2.engine.Session;
 import org.h2.expression.ConditionAndOr;
 import org.h2.expression.Expression;
 import org.h2.expression.ExpressionColumn;
-import org.h2.expression.ExpressionVisitor;
 import org.h2.message.DbException;
 import org.h2.result.ResultInterface;
 import org.h2.result.Row;
@@ -103,7 +100,6 @@ public class MergeUsing extends Prepared {
     private Table targetTable;
     private TableFilter targetTableFilter;
     private Column[] columns;
-    private Column[] keys;
     private final ArrayList<Expression[]> valuesExpressionList = Utils.newSmallArrayList();
     private Query query;
 
@@ -214,59 +210,35 @@ public class MergeUsing extends Prepared {
             }
         } else {
             if (insertCommand != null) {
-                int count = insertCommand.update();
-                if (!isTargetRowFound()) {
-                    throw DbException.get(ErrorCode.GENERAL_ERROR_1,
-                            "Expected to find key after row inserted, but none found. "
-                                    + "Insert does not match ON condition.:"
-                                    + targetTable.getSQL() + ":source row="
-                                    + Arrays.asList(sourceRow.getValueList()));
-                }
-                countUpdatedRows += count;
+                countUpdatedRows += insertCommand.update();
             }
         }
     }
 
     private boolean isTargetRowFound() {
+        boolean matched = false;
         try (ResultInterface rows = targetMatchQuery.query(0)) {
-            if (!rows.next()) {
-                return false;
-            }
-            Value targetRowId = rows.currentRow()[0];
-            Integer number = targetRowidsRemembered.get(targetRowId);
-            // throw and exception if we have processed this _ROWID_ before...
-            if (number != null) {
-                throw DbException.get(ErrorCode.DUPLICATE_KEY_1,
-                        "Merge using ON column expression, " +
-                        "duplicate _ROWID_ target record already updated, deleted or inserted:_ROWID_="
-                                + targetRowId + ":in:"
-                                + targetTableFilter.getTable()
-                                + ":conflicting source row number:"
-                                + number);
-            }
-            // remember the source column values we have used before (they
-            // are the effective ON clause keys
-            // and should not be repeated
-            targetRowidsRemembered.put(targetRowId, sourceQueryRowNumber);
-            if (rows.next()) {
-                int rowCount;
-                if (rows.isLazy()) {
-                    for (rowCount = 2; rows.next(); rowCount++) {
-                    }
-                } else {
-                    rowCount = rows.getRowCount();
+            while (rows.next()) {
+                Value targetRowId = rows.currentRow()[0];
+                Integer number = targetRowidsRemembered.get(targetRowId);
+                // throw and exception if we have processed this _ROWID_ before...
+                if (number != null) {
+                    throw DbException.get(ErrorCode.DUPLICATE_KEY_1,
+                            "Merge using ON column expression, " +
+                            "duplicate _ROWID_ target record already updated, deleted or inserted:_ROWID_="
+                                    + targetRowId + ":in:"
+                                    + targetTableFilter.getTable()
+                                    + ":conflicting source row number:"
+                                    + number);
                 }
-                throw DbException.get(ErrorCode.DUPLICATE_KEY_1,
-                        "Duplicate key updated "
-                                + rowCount
-                                + " rows at once, only 1 expected:_ROWID_="
-                                + targetRowId + ":in:"
-                                + targetTableFilter.getTable()
-                                + ":conflicting source row number:"
-                                + targetRowidsRemembered.get(targetRowId));
+                // remember the source column values we have used before (they
+                // are the effective ON clause keys
+                // and should not be repeated
+                targetRowidsRemembered.put(targetRowId, sourceQueryRowNumber);
+                matched = true;
             }
-            return true;
         }
+        return matched;
     }
 
     // Use the regular merge syntax as our plan SQL
@@ -278,17 +250,7 @@ public class MergeUsing extends Prepared {
             buff.appendExceptFirst(", ");
             buff.append(c.getSQL());
         }
-        buff.append(')');
-        if (keys != null) {
-            buff.append(" KEY(");
-            buff.resetCount();
-            for (Column c : keys) {
-                buff.appendExceptFirst(", ");
-                buff.append(c.getSQL());
-            }
-            buff.append(')');
-        }
-        buff.append('\n');
+        buff.append(')').append('\n');
         if (!valuesExpressionList.isEmpty()) {
             buff.append("VALUES ");
             int row = 0;
@@ -322,15 +284,6 @@ public class MergeUsing extends Prepared {
         onCondition.mapColumns(sourceTableFilter, 2, Expression.MAP_INITIAL);
         onCondition.mapColumns(targetTableFilter, 1, Expression.MAP_INITIAL);
 
-        if (keys == null) {
-            keys = buildColumnListFromOnCondition(targetTableFilter.getTable());
-        }
-        if (keys.length == 0) {
-            throw DbException.get(ErrorCode.COLUMN_NOT_FOUND_1,
-                    "No references to target columns found in ON clause:"
-                            + targetTableFilter.toString());
-        }
-
         // only do the optimize now - before we have already gathered the
         // unoptimized column data
         onCondition = onCondition.optimize(session);
@@ -392,13 +345,6 @@ public class MergeUsing extends Prepared {
         targetMatchQuery.prepare();
     }
 
-    private Column[] buildColumnListFromOnCondition(Table table) {
-        HashSet<Column> columns = new HashSet<>();
-        ExpressionVisitor visitor = ExpressionVisitor.getColumnsVisitor(columns, table);
-        onCondition.isEverything(visitor);
-        return columns.toArray(new Column[0]);
-    }
-
     private Expression appendOnCondition(Update updateCommand) {
         if (updateCommand.getCondition() == null) {
             return onCondition;

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

@@ -317,9 +317,15 @@ public class ExpressionColumn extends Expression {
             }
             return true;
         case ExpressionVisitor.GET_COLUMNS1:
+            if (column == null) {
+                throw DbException.get(ErrorCode.COLUMN_NOT_FOUND_1, getSQL());
+            }
             visitor.addColumn1(column);
             return true;
         case ExpressionVisitor.GET_COLUMNS2:
+            if (column == null) {
+                throw DbException.get(ErrorCode.COLUMN_NOT_FOUND_1, getSQL());
+            }
             visitor.addColumn2(column);
             return true;
         default:

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

@@ -5,6 +5,11 @@
  */
 package org.h2.value;
 
+import static org.h2.util.DateTimeUtils.NANOS_PER_DAY;
+import static org.h2.util.DateTimeUtils.NANOS_PER_HOUR;
+import static org.h2.util.DateTimeUtils.NANOS_PER_MINUTE;
+import static org.h2.util.DateTimeUtils.NANOS_PER_SECOND;
+
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 
@@ -169,28 +174,28 @@ public class ValueInterval extends Value {
         }
         long l = leading;
         switch (type) {
-        case Value.INTERVAL_SECOND:
-            if (r >= 1_000_000_000) {
+        case INTERVAL_SECOND:
+            if (r >= NANOS_PER_SECOND) {
                 l++;
-                r -= 1_000_000_000;
+                r -= NANOS_PER_SECOND;
             }
             break;
-        case Value.INTERVAL_DAY_TO_SECOND:
-            if (r >= DateTimeUtils.NANOS_PER_DAY) {
+        case INTERVAL_DAY_TO_SECOND:
+            if (r >= NANOS_PER_DAY) {
                 l++;
-                r -= DateTimeUtils.NANOS_PER_DAY;
+                r -= NANOS_PER_DAY;
             }
             break;
-        case Value.INTERVAL_HOUR_TO_SECOND:
-            if (r >= 3_600_000_000_000L) {
+        case INTERVAL_HOUR_TO_SECOND:
+            if (r >= NANOS_PER_HOUR) {
                 l++;
-                r -= 3_600_000_000_000L;
+                r -= NANOS_PER_HOUR;
             }
             break;
-        case Value.INTERVAL_MINUTE_TO_SECOND:
-            if (r >= 60_000_000_000L) {
+        case INTERVAL_MINUTE_TO_SECOND:
+            if (r >= NANOS_PER_MINUTE) {
                 l++;
-                r -= 60_000_000_000L;
+                r -= NANOS_PER_MINUTE;
             }
             break;
         }

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

@@ -228,9 +228,4 @@ public class ValueTime extends Value {
         return ValueTime.fromNanos((long) (nanos / v.getDouble()));
     }
 
-    @Override
-    public int getSignum() {
-        return (int) (-nanos >>> 63);
-    }
-
 }

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

@@ -196,73 +196,6 @@ public class TestMergeUsing extends TestDb implements Trigger {
                 "SELECT 1 AS ID, 'Marcy'||X||X UNION ALL SELECT 1 AS ID, 'Marcy2'",
                 2);
 
-        // Multiple update on same row: SQL standard says duplicate or repeated
-        // updates in same statement should cause errors -but because first row
-        // is updated, delete then insert it's considered different
-        // One insert, one update one delete happens (on same row, which is
-        // okay), then another update (which is illegal)target table missing PK,
-        // no source or target alias
-        testMergeUsingException(
-                "CREATE TABLE PARENT AS (SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,1) );"
-                        + "CREATE TABLE SOURCE AS (SELECT 1 AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3)  );",
-                "MERGE INTO PARENT USING SOURCE ON (PARENT.ID = SOURCE.ID) WHEN MATCHED THEN " +
-                "UPDATE SET PARENT.NAME = SOURCE.NAME||SOURCE.ID WHERE PARENT.ID = 2 " +
-                "DELETE WHERE PARENT.ID = 1 WHEN NOT MATCHED THEN " +
-                "INSERT (ID, NAME) VALUES (SOURCE.ID, SOURCE.NAME)",
-                GATHER_ORDERED_RESULTS_SQL,
-                "SELECT 1 AS ID, 'Marcy'||X||X UNION ALL SELECT 1 AS ID, 'Marcy2'",
-                3,
-                "Unique index or primary key violation: \"Merge using " +
-                "ON column expression, duplicate _ROWID_ target record " +
-                "already updated, deleted or inserted:_ROWID_=2:in:PUBLIC.PARENT:conflicting source row number:2");
-        // Duplicate key updated 3 rows at once, only 1 expected
-        testMergeUsingException(
-                "CREATE TABLE PARENT AS (SELECT 1 AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3) );"
-                        + "CREATE TABLE SOURCE AS (SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3)  );",
-                "MERGE INTO PARENT USING SOURCE ON (PARENT.ID = SOURCE.ID) WHEN MATCHED THEN " +
-                "UPDATE SET PARENT.NAME = SOURCE.NAME||SOURCE.ID WHERE PARENT.ID = 2 " +
-                "DELETE WHERE PARENT.ID = 1 WHEN NOT MATCHED THEN " +
-                "INSERT (ID, NAME) VALUES (SOURCE.ID, SOURCE.NAME)",
-                GATHER_ORDERED_RESULTS_SQL,
-                "SELECT X AS ID, 'Marcy'||X||X AS NAME FROM SYSTEM_RANGE(2,2) UNION ALL " +
-                "SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(3,3)",
-                3, "Duplicate key updated 3 rows at once, only 1 expected");
-        // Missing target columns in ON expression
-        testMergeUsingException(
-                "CREATE TABLE PARENT AS (SELECT 1 AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3) );"
-                        + "CREATE TABLE SOURCE AS (SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3)  );",
-                "MERGE INTO PARENT USING SOURCE ON (1 = SOURCE.ID) WHEN MATCHED THEN " +
-                "UPDATE SET PARENT.NAME = SOURCE.NAME||SOURCE.ID WHERE PARENT.ID = 2 " +
-                "DELETE WHERE PARENT.ID = 1 WHEN NOT MATCHED THEN " +
-                "INSERT (ID, NAME) VALUES (SOURCE.ID, SOURCE.NAME)",
-                GATHER_ORDERED_RESULTS_SQL,
-                "SELECT X AS ID, 'Marcy'||X||X AS NAME FROM SYSTEM_RANGE(2,2) UNION ALL " +
-                "SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(3,3)",
-                3, "No references to target columns found in ON clause");
-        // Missing source columns in ON expression
-        testMergeUsingException(
-                "CREATE TABLE PARENT AS (SELECT 1 AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3) );"
-                        + "CREATE TABLE SOURCE AS (SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3)  );",
-                "MERGE INTO PARENT USING SOURCE ON (PARENT.ID = 1) WHEN MATCHED THEN " +
-                "UPDATE SET PARENT.NAME = SOURCE.NAME||SOURCE.ID WHERE PARENT.ID = 2 " +
-                "DELETE WHERE PARENT.ID = 1 WHEN NOT MATCHED THEN " +
-                "INSERT (ID, NAME) VALUES (SOURCE.ID, SOURCE.NAME)",
-                GATHER_ORDERED_RESULTS_SQL,
-                "SELECT X AS ID, 'Marcy'||X||X AS NAME FROM SYSTEM_RANGE(2,2) UNION ALL " +
-                "SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(3,3)",
-                3, "Duplicate key updated 3 rows at once, only 1 expected");
-        // Insert does not insert correct values with respect to ON condition
-        // (inserts ID value above 100, instead)
-        testMergeUsingException(
-                "CREATE TABLE PARENT AS (SELECT 1 AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(4,4) );"
-                        + "CREATE TABLE SOURCE AS (SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(1,3)  );",
-                "MERGE INTO PARENT USING SOURCE ON (PARENT.ID = SOURCE.ID) WHEN MATCHED THEN " +
-                "UPDATE SET PARENT.NAME = SOURCE.NAME||SOURCE.ID WHERE PARENT.ID = 2 " +
-                "DELETE WHERE PARENT.ID = 1 WHEN NOT MATCHED THEN " +
-                "INSERT (ID, NAME) VALUES (SOURCE.ID+100, SOURCE.NAME)",
-                GATHER_ORDERED_RESULTS_SQL,
-                "SELECT X AS ID, 'Marcy'||X AS NAME FROM SYSTEM_RANGE(4,4)", 1,
-                "Expected to find key after row inserted, but none found. Insert does not match ON condition.");
         // One insert, one update one delete happens, target table missing PK,
         // triggers update all NAME fields
         triggerTestingUpdateCount = 0;

h2/src/test/org/h2/test/scripts/dml/mergeUsing.sql

@@ -29,7 +29,7 @@ EXPLAIN PLAN
             UPDATE SET P.NAME = S.NAME WHERE 2 = 2 WHEN NOT
         MATCHED THEN
             INSERT (ID, NAME) VALUES (S.ID, S.NAME);
->> MERGE INTO PUBLIC.PARENT(ID, NAME) KEY(ID) SELECT X AS ID, ('Coco' || X) AS NAME FROM SYSTEM_RANGE(1, 2) /* PUBLIC.RANGE_INDEX */
+>> MERGE INTO PUBLIC.PARENT(ID, NAME) SELECT X AS ID, ('Coco' || X) AS NAME FROM SYSTEM_RANGE(1, 2) /* PUBLIC.RANGE_INDEX */
 
 DROP TABLE PARENT;
 > ok
@@ -208,5 +208,56 @@ SELECT * FROM TEST;
 > 30
 > rows: 3
 
+MERGE INTO TEST USING (SELECT 40) ON UNKNOWN_COLUMN = 1 WHEN NOT MATCHED THEN INSERT (ID) VALUES (40);
+> exception COLUMN_NOT_FOUND_1
+
+DROP TABLE TEST;
+> ok
+
+CREATE TABLE TEST(ID INT PRIMARY KEY, VALUE INT);
+> ok
+
+INSERT INTO TEST VALUES (1, 10), (2, 20);
+> update count: 2
+
+MERGE INTO TEST USING (SELECT 1) ON (ID < 0)
+    WHEN MATCHED THEN UPDATE SET VALUE = 30
+    WHEN NOT MATCHED THEN INSERT VALUES (3, 30);
+> update count: 1
+
+SELECT * FROM TEST;
+> ID VALUE
+> -- -----
+> 1  10
+> 2  20
+> 3  30
+> rows: 3
+
+MERGE INTO TEST USING (SELECT 1) ON (ID = ID)
+    WHEN MATCHED THEN UPDATE SET VALUE = 40
+    WHEN NOT MATCHED THEN INSERT VALUES (4, 40);
+> update count: 3
+
+SELECT * FROM TEST;
+> ID VALUE
+> -- -----
+> 1  40
+> 2  40
+> 3  40
+> rows: 3
+
+MERGE INTO TEST USING (SELECT 1) ON (1 = 1)
+    WHEN MATCHED THEN UPDATE SET VALUE = 50
+    WHEN NOT MATCHED THEN INSERT VALUES (5, 50);
+> update count: 3
+
+SELECT * FROM TEST;
+> ID VALUE
+> -- -----
+> 1  50
+> 2  50
+> 3  50
+> rows: 3
+
 DROP TABLE TEST;
 > ok

h2/src/test/org/h2/test/unit/TestDate.java

@@ -135,9 +135,6 @@ public class TestDate extends TestBase {
         assertEquals("1970-01-01", t1.getDate().toString());
         assertEquals("TIME '11:11:11'", t1.getSQL());
         assertEquals("TIME '11:11:11'", t1.toString());
-        assertEquals(1, t1.getSignum());
-        assertEquals(0, t1.multiply(ValueInt.get(0)).getSignum());
-        assertEquals(0, t1.subtract(t1).getSignum());
         assertEquals("05:35:35.5", t1.multiply(ValueDouble.get(0.5)).getString());
         assertEquals("22:22:22", t1.divide(ValueDouble.get(0.5)).getString());
         assertEquals(Value.TIME, t1.getType());