Fix -> operator on missing values to return null (#193)

* Fix -> on missing value to return null. * Improve error message if value is not a json array. * Add changeset. * Tweak test. * Fix comments in tests. * Add test for json_each + json_keys. * Fix typo.
powersync-ja · Feb 4, 2025 · e26e434 · e26e434
1 parent f4898a3
commit e26e434
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 10 deletions.
diff --git a/.changeset/fast-monkeys-approve.md b/.changeset/fast-monkeys-approve.md
@@ -0,0 +1,5 @@
+---
+'@powersync/service-sync-rules': patch
+---
+
+Fix -> operator on missing values to return null.
diff --git a/packages/sync-rules/src/TableValuedFunctions.ts b/packages/sync-rules/src/TableValuedFunctions.ts
@@ -27,7 +27,7 @@ export const JSON_EACH: TableValuedFunction = {
       throw new Error('Expected JSON string');
     }
     if (!Array.isArray(values)) {
-      throw new Error('Expected an array');
+      throw new Error(`Expected an array, got ${valueString}`);
     }
 
     return values.map((v) => {

diff --git a/packages/sync-rules/src/sql_functions.ts b/packages/sync-rules/src/sql_functions.ts
@@ -260,7 +260,7 @@ const iif: DocumentedSqlFunction = {
   parameters: [
     { name: 'x', type: ExpressionType.ANY, optional: false },
     { name: 'y', type: ExpressionType.ANY, optional: false },
-    { name: 'z', type: ExpressionType.ANY, optional: false },
+    { name: 'z', type: ExpressionType.ANY, optional: false }
   ],
   getReturnType() {
     return ExpressionType.ANY;
@@ -865,7 +865,10 @@ export function jsonExtract(sourceValue: SqliteValue, path: SqliteValue, operato
     value = value[c];
   }
   if (operator == '->') {
-    // -> must always stringify
+    // -> must always stringify, except when it's null
+    if (value == null) {
+      return null;
+    }
     return JSONBig.stringify(value);
   } else {
     // Plain scalar value - simple conversion.

diff --git a/packages/sync-rules/test/src/sql_functions.test.ts b/packages/sync-rules/test/src/sql_functions.test.ts
@@ -12,6 +12,10 @@ describe('SQL functions', () => {
     expect(fn.json_extract(JSON.stringify({ foo: 42 }), '$')).toEqual('{"foo":42}');
     expect(fn.json_extract(`{"foo": 42.0}`, '$')).toEqual('{"foo":42.0}');
     expect(fn.json_extract(`{"foo": true}`, '$')).toEqual('{"foo":true}');
+    // SQLite gives null instead of 'null'. We should match that, but it's a breaking change.
+    expect(fn.json_extract(`{"foo": null}`, '$.foo')).toEqual('null');
+    // Matches SQLite
+    expect(fn.json_extract(`{}`, '$.foo')).toBeNull();
   });
 
   test('->>', () => {
@@ -23,6 +27,10 @@ describe('SQL functions', () => {
     expect(jsonExtract(`{"foo": 42.0}`, 'foo', '->>')).toEqual(42.0);
     expect(jsonExtract(`{"foo": 42.0}`, '$', '->>')).toEqual(`{"foo":42.0}`);
     expect(jsonExtract(`{"foo": true}`, '$.foo', '->>')).toEqual(1n);
+    // SQLite gives null instead of 'null'. We should match that, but it's a breaking change.
+    expect(jsonExtract(`{"foo": null}`, '$.foo', '->>')).toEqual('null');
+    // Matches SQLite
+    expect(jsonExtract(`{}`, '$.foo', '->>')).toBeNull();
   });
 
   test('->', () => {
@@ -34,6 +42,10 @@ describe('SQL functions', () => {
     expect(jsonExtract(`{"foo": 42.0}`, 'foo', '->')).toEqual('42.0');
     expect(jsonExtract(`{"foo": 42.0}`, '$', '->')).toEqual(`{"foo":42.0}`);
     expect(jsonExtract(`{"foo": true}`, '$.foo', '->')).toEqual('true');
+    // SQLite gives 'null' instead of null. We should match that, but it's a breaking change.
+    expect(jsonExtract(`{"foo": null}`, '$.foo', '->')).toBeNull();
+    // Matches SQLite
+    expect(jsonExtract(`{}`, '$.foo', '->')).toBeNull();
   });
 
   test('json_array_length', () => {
@@ -127,13 +139,13 @@ describe('SQL functions', () => {
   test('iif', () => {
     expect(fn.iif(null, 1, 2)).toEqual(2);
     expect(fn.iif(0, 1, 2)).toEqual(2);
-    expect(fn.iif(1, "first", "second")).toEqual("first");
-    expect(fn.iif(0.1, "is_true", "is_false")).toEqual("is_true");
-    expect(fn.iif("a", "is_true", "is_false")).toEqual("is_false");
-    expect(fn.iif(0n, "is_true", "is_false")).toEqual("is_false");
-    expect(fn.iif(2n, "is_true", "is_false")).toEqual("is_true");
-    expect(fn.iif(new Uint8Array([]), "is_true", "is_false")).toEqual("is_false");
-    expect(fn.iif(Uint8Array.of(0x61, 0x62, 0x43), "is_true", "is_false")).toEqual("is_false");
+    expect(fn.iif(1, 'first', 'second')).toEqual('first');
+    expect(fn.iif(0.1, 'is_true', 'is_false')).toEqual('is_true');
+    expect(fn.iif('a', 'is_true', 'is_false')).toEqual('is_false');
+    expect(fn.iif(0n, 'is_true', 'is_false')).toEqual('is_false');
+    expect(fn.iif(2n, 'is_true', 'is_false')).toEqual('is_true');
+    expect(fn.iif(new Uint8Array([]), 'is_true', 'is_false')).toEqual('is_false');
+    expect(fn.iif(Uint8Array.of(0x61, 0x62, 0x43), 'is_true', 'is_false')).toEqual('is_false');
   });
 
   test('upper', () => {

diff --git a/packages/sync-rules/test/src/table_valued_function_queries.test.ts b/packages/sync-rules/test/src/table_valued_function_queries.test.ts
@@ -42,6 +42,43 @@ describe('table-valued function queries', () => {
     expect(query.getStaticBucketIds(new RequestParameters({ sub: '' }, {}))).toEqual([]);
   });
 
+  test('json_each(array param not present)', function () {
+    const sql = "SELECT json_each.value as v FROM json_each(request.parameters() -> 'array_not_present')";
+    const query = SqlParameterQuery.fromSql('mybucket', sql, {
+      ...PARSE_OPTIONS,
+      accept_potentially_dangerous_queries: true
+    }) as StaticSqlParameterQuery;
+    expect(query.errors).toEqual([]);
+    expect(query.bucket_parameters).toEqual(['v']);
+
+    expect(query.getStaticBucketIds(new RequestParameters({ sub: '' }, {}))).toEqual([]);
+  });
+
+  test('json_each(array param not present, ifnull)', function () {
+    const sql = "SELECT json_each.value as v FROM json_each(ifnull(request.parameters() -> 'array_not_present', '[]'))";
+    const query = SqlParameterQuery.fromSql('mybucket', sql, {
+      ...PARSE_OPTIONS,
+      accept_potentially_dangerous_queries: true
+    }) as StaticSqlParameterQuery;
+    expect(query.errors).toEqual([]);
+    expect(query.bucket_parameters).toEqual(['v']);
+
+    expect(query.getStaticBucketIds(new RequestParameters({ sub: '' }, {}))).toEqual([]);
+  });
+
+  test('json_each on json_keys', function () {
+    const sql = `SELECT value FROM json_each(json_keys('{"a": [], "b": 2, "c": null}'))`;
+    const query = SqlParameterQuery.fromSql('mybucket', sql, PARSE_OPTIONS) as StaticSqlParameterQuery;
+    expect(query.errors).toEqual([]);
+    expect(query.bucket_parameters).toEqual(['value']);
+
+    expect(query.getStaticBucketIds(new RequestParameters({ sub: '' }, {}))).toEqual([
+      'mybucket["a"]',
+      'mybucket["b"]',
+      'mybucket["c"]'
+    ]);
+  });
+
   test('json_each with fn alias', function () {
     const sql = "SELECT e.value FROM json_each(request.parameters() -> 'array') e";
     const query = SqlParameterQuery.fromSql('mybucket', sql, {