Merge pull request #42 from powersync-ja/validate-id-column

Validate id column for "select *"

Author: rkistner

.changeset/brown-horses-whisper.md

@@ -0,0 +1,5 @@
+---
+'@powersync/service-sync-rules': patch
+---
+
+Warn when id column is not present in "select \* from ..."

packages/sync-rules/src/ExpressionType.ts

@@ -67,4 +67,8 @@ export class ExpressionType {
   isNumericOnly() {
     return this.typeFlags != TYPE_NONE && (this.typeFlags & (TYPE_INTEGER | TYPE_REAL)) == this.typeFlags;
   }
+
+  isNone() {
+    return this.typeFlags == TYPE_NONE;
+  }
 }

packages/sync-rules/src/SqlDataQuery.ts

@@ -106,6 +106,8 @@ export class SqlDataQuery {
     rows.tools = tools;
 
     let hasId = false;
+    let hasWildcard = false;
+
     for (let column of q.columns ?? []) {
       const name = tools.getOutputName(column);
       if (name != '*') {
@@ -140,12 +142,28 @@ export class SqlDataQuery {
           }
         });
       }
-      if (name == 'id' || name == '*') {
+      if (name == 'id') {
         hasId = true;
+      } else if (name == '*') {
+        hasWildcard = true;
+        if (querySchema == null) {
+          // Not performing schema-based validation - assume there is an id
+          hasId = true;
+        } else {
+          const idType = querySchema.getType(alias, 'id');
+          if (!idType.isNone()) {
+            hasId = true;
+          }
+        }
       }
     }
     if (!hasId) {
-      rows.errors.push(new SqlRuleError(`Query must return an "id" column`, sql, q.columns?.[0]._location));
+      const error = new SqlRuleError(`Query must return an "id" column`, sql, q.columns?.[0]._location);
+      if (hasWildcard) {
+        // Schema-based validations are always warnings
+        error.type = 'warning';
+      }
+      rows.errors.push(error);
     }
     rows.errors.push(...tools.errors);
     return rows;

packages/sync-rules/test/src/data_queries.test.ts

@@ -142,6 +142,17 @@ describe('data queries', () => {
         type: 'warning'
       }
     ]);
+
+    const q4 = SqlDataQuery.fromSql('q4', [], 'SELECT * FROM other', schema);
+    expect(q4.errors).toMatchObject([
+      {
+        message: `Query must return an "id" column`,
+        type: 'warning'
+      }
+    ]);
+
+    const q5 = SqlDataQuery.fromSql('q5', [], 'SELECT other_id as id, * FROM other', schema);
+    expect(q5.errors).toMatchObject([]);
   });
 
   test('invalid query - invalid IN', function () {

packages/sync-rules/test/src/util.ts

@@ -32,6 +32,10 @@ export const BASIC_SCHEMA = new StaticSchema([
               { name: 'count', pg_type: 'int4' },
               { name: 'owner_id', pg_type: 'uuid' }
             ]
+          },
+          {
+            name: 'other',
+            columns: [{ name: 'other_id', pg_type: 'uuid' }]
           }
         ]
       }