Smaller fixes, more tests

Author: simolus3

packages/sync-rules/src/compiler/sqlite.ts

@@ -247,8 +247,9 @@ export class PostgresToSqlite {
         let rightHandSideOfIs: SqlExpression<ExpressionInput>;
 
         switch (expr.op) {
-          case '+':
           case '-':
+            return this.invalidExpression(expr, 'Unary minus is not currently supported');
+          case '+':
             return { type: 'unary', operator: expr.op, operand: this.translateNodeWithLocation(expr.operand) };
           case 'NOT':
             return { type: 'unary', operator: 'not', operand: this.translateNodeWithLocation(expr.operand) };

packages/sync-rules/src/sync_plan/engine/javascript.ts

@@ -151,9 +151,9 @@ class ExpressionToJavaScriptFunction
         return operand;
       case 'not':
         return (input) => sqliteNot(operand(input));
-      case '~':
-      case '-':
-        throw new Error(`unary operator not supported: ${expr.operator}`);
+      // case '~':
+      // case '-':
+      //   throw new Error(`unary operator not supported: ${expr.operator}`);
     }
   }
 
@@ -173,10 +173,13 @@ class ExpressionToJavaScriptFunction
     return (input) => {
       const evaluatedValue = value(input);
 
-      return (
-        sqliteBool(evaluateOperator('>=', evaluatedValue, low(input))) &&
-        sqliteBool(evaluateOperator('<=', evaluatedValue, high(input)))
-      );
+      const geqLow = evaluateOperator('>=', evaluatedValue, low(input));
+      const leqHigh = evaluateOperator('<=', evaluatedValue, high(input));
+      if (geqLow == null || leqHigh == null) {
+        return null;
+      }
+
+      return sqliteBool(geqLow) && sqliteBool(leqHigh);
     };
   }
 

packages/sync-rules/src/sync_plan/expression.ts

@@ -26,7 +26,7 @@ export type SqlExpression<Data> =
  */
 export type ExternalData<Data> = { type: 'data'; source: Data };
 
-export type UnaryOperator = 'not' | '~' | '+' | '-';
+export type UnaryOperator = 'not' | '+'; //| '-' | '~';
 
 export type UnaryExpression<Data> = {
   type: 'unary';

packages/sync-rules/src/sync_plan/expression_to_sql.ts

@@ -221,7 +221,7 @@ const binaryPrecedence: Record<BinaryOperator, Precedence> = {
 
 const unaryPrecedence: Record<UnaryOperator, Precedence> = {
   not: Precedence.not,
-  '~': Precedence.unary,
-  '+': Precedence.unary,
-  '-': Precedence.unary
+  //'~': Precedence.unary,
+  '+': Precedence.unary
+  //'-': Precedence.unary
 };

packages/sync-rules/test/src/sync_plan/engine/engine.test.ts

@@ -3,6 +3,7 @@ import * as sqlite from 'node:sqlite';
 import { describe, expect, onTestFinished, test } from 'vitest';
 import {
   CompatibilityContext,
+  CompatibilityEdition,
   javaScriptExpressionEngine,
   nodeSqliteExpressionEngine,
   SqliteValue
@@ -14,19 +15,22 @@ import {
 } from '../../../../src/sync_plan/engine/scalar_expression_engine.js';
 import { BinaryOperator } from '../../../../src/sync_plan/expression.js';
 
-const compatibility = CompatibilityContext.FULL_BACKWARDS_COMPATIBILITY;
-
 describe('sqlite', () => {
-  defineEngineTests(false, () => nodeSqliteExpressionEngine(sqlite, compatibility));
+  defineEngineTests(false, (c) => nodeSqliteExpressionEngine(sqlite, c));
 });
 
 describe('javascript', () => {
-  defineEngineTests(true, () => javaScriptExpressionEngine(compatibility));
+  defineEngineTests(true, javaScriptExpressionEngine);
 });
 
-function defineEngineTests(isJavaScript: boolean, createEngine: () => ScalarExpressionEngine) {
+function defineEngineTests(
+  isJavaScript: boolean,
+  createEngine: (compatibility: CompatibilityContext) => ScalarExpressionEngine
+) {
+  let compatibility = CompatibilityContext.FULL_BACKWARDS_COMPATIBILITY;
+
   function prepare(stmt: ScalarStatement) {
-    const engine = createEngine();
+    const engine = createEngine(compatibility);
     onTestFinished(() => engine.close());
     return engine.prepareEvaluator(stmt);
   }
@@ -60,6 +64,12 @@ function defineEngineTests(isJavaScript: boolean, createEngine: () => ScalarExpr
     expect(prepare({ outputs: [{ type: 'lit_null' }] }).evaluate([])).toStrictEqual([[null]]);
   });
 
+  test('not null', () => {
+    expect(
+      prepare({ outputs: [{ type: 'unary', operator: 'not', operand: { type: 'lit_null' } }] }).evaluate([])
+    ).toStrictEqual([[isJavaScript ? 1n : null]]);
+  });
+
   test('literal double', () => {
     expect(prepare({ outputs: [{ type: 'lit_double', value: 3 }] }).evaluate([])).toStrictEqual([[3]]);
   });
@@ -125,6 +135,10 @@ function defineEngineTests(isJavaScript: boolean, createEngine: () => ScalarExpr
     expect(stmt.evaluate([1, 2, 3])).toStrictEqual([[0n]]);
     expect(stmt.evaluate([4, 2, 3])).toStrictEqual([[0n]]);
     expect(stmt.evaluate([1, 1, 1])).toStrictEqual([[1n]]);
+
+    expect(stmt.evaluate([null, 3, 4])).toStrictEqual([[null]]);
+    expect(stmt.evaluate([4, null, 4])).toStrictEqual([[null]]);
+    expect(stmt.evaluate([4, 3, null])).toStrictEqual([[null]]);
   });
 
   test('scalar in', () => {
@@ -261,12 +275,6 @@ function defineEngineTests(isJavaScript: boolean, createEngine: () => ScalarExpr
     expectFunction('unixepoch', ['now'], null);
     expectFunction('datetime', ['now'], null);
 
-    const jsonObject = JSON.stringify({ foo: { bar: ['baz'] } });
-
-    // Legacy JSON behavior, support paths without $. prefix
-    expectFunction('->', [jsonObject, 'foo.bar'], '["baz"]');
-    expectFunction('->>', [jsonObject, 'foo.bar.0'], 'baz');
-
     const point = '010100000097900F7A36FB40C0F4FDD478E9D63240';
     expectFunction('st_x', [point], -33.9626);
     expectFunction('st_y', [point], 18.8395);
@@ -275,4 +283,18 @@ function defineEngineTests(isJavaScript: boolean, createEngine: () => ScalarExpr
     expectFunction('st_asgeojson', [geography], '{"type":"Point","coordinates":[-33.9626,18.8395]}');
     expectFunction('st_astext', [geography], 'POINT(-33.9626 18.8395)');
   });
+
+  test('legacy and fixed JSON behavior', () => {
+    const jsonObject = JSON.stringify({ foo: { bar: ['baz'] }, 'foo.bar': ['another'] });
+    compatibility = CompatibilityContext.FULL_BACKWARDS_COMPATIBILITY;
+
+    // Legacy JSON behavior, support paths without $. prefix
+    expectFunction('->', [jsonObject, 'foo.bar'], '["baz"]');
+    expectFunction('->>', [jsonObject, 'foo.bar.0'], 'baz');
+
+    // New JSON behavior, require $. syntax.
+    compatibility = new CompatibilityContext({ edition: CompatibilityEdition.SYNC_STREAMS });
+    expectFunction('->', [jsonObject, 'foo.bar'], '["another"]');
+    expectFunction('->>', [jsonObject, 'foo.bar.0'], null);
+  });
 }