Warn on table/column identifier case issues

.changeset/short-bats-sin.md

@@ -0,0 +1,5 @@
+---
+'@powersync/service-sync-rules': minor
+---
+
+Warn when identifiers are automatically convererted to lower case.

packages/sync-rules/src/SqlDataQuery.ts

@@ -76,6 +76,7 @@ export class SqlDataQuery {
       sql,
       schema: querySchema
     });
+    tools.checkSpecificNameCase(tableRef);
     const filter = tools.compileWhereClause(where);
 
     const inputParameterNames = filter.inputParameters!.map((p) => p.key);

packages/sync-rules/src/SqlParameterQuery.ts

@@ -96,6 +96,7 @@ export class SqlParameterQuery {
       supports_parameter_expressions: true,
       schema: querySchema
     });
+    tools.checkSpecificNameCase(tableRef);
     const where = q.where;
     const filter = tools.compileWhereClause(where);
 
@@ -118,6 +119,9 @@ export class SqlParameterQuery {
 
     for (let column of q.columns ?? []) {
       const name = tools.getSpecificOutputName(column);
+      if (column.alias != null) {
+        tools.checkSpecificNameCase(column.alias);
+      }
       if (tools.isTableRef(column.expr)) {
         rows.lookup_columns.push(column);
         const extractor = tools.compileRowValueExtractor(column.expr);

packages/sync-rules/src/StaticSqlParameterQuery.ts

@@ -39,6 +39,9 @@ export class StaticSqlParameterQuery {
     }
 
     for (let column of columns) {
+      if (column.alias != null) {
+        tools.checkSpecificNameCase(column.alias);
+      }
       const name = tools.getSpecificOutputName(column);
       const extractor = tools.compileParameterValueExtractor(column.expr);
       if (isClauseError(extractor)) {

packages/sync-rules/src/sql_filters.ts

@@ -1,4 +1,4 @@
-import { Expr, ExprRef, NodeLocation, SelectedColumn } from 'pgsql-ast-parser';
+import { Expr, ExprRef, Name, NodeLocation, QName, QNameAliased, SelectedColumn, parse } from 'pgsql-ast-parser';
 import { nil } from 'pgsql-ast-parser/src/utils.js';
 import { ExpressionType, TYPE_NONE } from './ExpressionType.js';
 import { SqlRuleError } from './errors.js';
@@ -182,6 +182,7 @@ export class SqlTools {
       const value = staticValue(expr);
       return staticValueClause(value);
     } else if (expr.type == 'ref') {
+      this.checkRefCase(expr);
       const column = expr.name;
       if (column == '*') {
         return this.error('* not supported here', expr);
@@ -512,6 +513,61 @@ export class SqlTools {
     }
   }
 
+  public checkRefCase(ref: ExprRef) {
+    if (ref.table != null) {
+      this.checkSpecificNameCase(ref.table);
+    }
+    this.checkColumnNameCase(ref);
+  }
+
+  private checkColumnNameCase(expr: ExprRef) {
+    if (expr.name.toLowerCase() != expr.name) {
+      // name is not lower case, must be quoted
+      return;
+    }
+
+    let location = expr._location;
+    if (location == null) {
+      return;
+    }
+    const tableLocation = expr.table?._location;
+    if (tableLocation != null) {
+      // exp._location contains the entire expression.
+      // We use this to remove the "table" part.
+      location = { start: tableLocation.end + 1, end: location.end };
+    }
+    const source = this.sql.substring(location.start, location.end);
+    if (source.toLowerCase() != source) {
+      // source is not lower case, while parsed is lower-case
+      this.warn(`Unquoted identifiers are converted to lower-case. Use "${source}" instead.`, location);
+    }
+  }
+
+  /**
+   * Check the case of a table name or any alias.
+   */
+  public checkSpecificNameCase(expr: Name | QName | QNameAliased) {
+    if ((expr as QNameAliased).alias != null || (expr as QName).schema != null) {
+      // We cannot properly distinguish alias and schema from the name itself,
+      // without building our own complete parser, so we ignore this for now.
+      return;
+    }
+    if (expr.name.toLowerCase() != expr.name) {
+      // name is not lower case, which means it is already quoted
+      return;
+    }
+
+    const location = expr._location;
+    if (location == null) {
+      return;
+    }
+    const source = this.sql.substring(location.start, location.end);
+    if (source.toLowerCase() != source) {
+      // source is not lower case
+      this.warn(`Unquoted identifiers are converted to lower-case. Use "${source}" instead.`, location);
+    }
+  }
+
   private checkRef(table: string, ref: ExprRef) {
     if (this.schema) {
       const type = this.schema.getType(table, ref.name);

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

@@ -194,4 +194,64 @@ describe('data queries', () => {
     const query = SqlDataQuery.fromSql('mybucket', ['org_id'], sql);
     expect(query.errors[0].message).toMatch(/Parameter match expression is not allowed here/);
   });
+
+  test('case-sensitive queries (1)', () => {
+    const sql = 'SELECT * FROM Assets';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Assets" instead.` }
+    ]);
+  });
+
+  test('case-sensitive queries (2)', () => {
+    const sql = 'SELECT *, Name FROM assets';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Name" instead.` }
+    ]);
+  });
+
+  test('case-sensitive queries (3)', () => {
+    const sql = 'SELECT * FROM assets WHERE Archived = False';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Archived" instead.` }
+    ]);
+  });
+
+  test.skip('case-sensitive queries (4)', () => {
+    // Cannot validate table alias yet
+    const sql = 'SELECT * FROM assets as myAssets';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "myAssets" instead.` }
+    ]);
+  });
+
+  test.skip('case-sensitive queries (5)', () => {
+    // Cannot validate table alias yet
+    const sql = 'SELECT * FROM assets myAssets';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "myAssets" instead.` }
+    ]);
+  });
+
+  test.skip('case-sensitive queries (6)', () => {
+    // Cannot validate anything with a schema yet
+    const sql = 'SELECT * FROM public.ASSETS';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "ASSETS" instead.` }
+    ]);
+  });
+
+  test.skip('case-sensitive queries (7)', () => {
+    // Cannot validate schema yet
+    const sql = 'SELECT * FROM PUBLIC.assets';
+    const query = SqlDataQuery.fromSql('mybucket', [], sql);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "PUBLIC" instead.` }
+    ]);
+  });
 });

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

@@ -376,7 +376,10 @@ describe('parameter queries', () => {
     // Postgres and/or SQLite.
     const sql = 'SELECT users.userId AS user_id FROM users WHERE users.userId = token_parameters.user_id';
     const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
-    expect(query.errors).toEqual([]);
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "userId" instead.` },
+      { message: `Unquoted identifiers are converted to lower-case. Use "userId" instead.` }
+    ]);
     query.id = '1';
 
     expect(query.evaluateParameterRow({ userId: 'user1' })).toEqual([]);
@@ -389,6 +392,46 @@ describe('parameter queries', () => {
     ]);
   });
 
+  test('case-sensitive parameter queries (3)', () => {
+    const sql = 'SELECT user_id FROM users WHERE Users.user_id = token_parameters.user_id';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Users" instead.` }
+    ]);
+  });
+
+  test('case-sensitive parameter queries (4)', () => {
+    const sql = 'SELECT Users.user_id FROM users WHERE user_id = token_parameters.user_id';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Users" instead.` }
+    ]);
+  });
+
+  test('case-sensitive parameter queries (5)', () => {
+    const sql = 'SELECT user_id FROM Users WHERE user_id = token_parameters.user_id';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "Users" instead.` }
+    ]);
+  });
+
+  test('case-sensitive parameter queries (6)', () => {
+    const sql = 'SELECT userId FROM users';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "userId" instead.` }
+    ]);
+  });
+
+  test('case-sensitive parameter queries (7)', () => {
+    const sql = 'SELECT user_id as userId FROM users';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "userId" instead.` }
+    ]);
+  });
+
   test('dynamic global parameter query', () => {
     const sql = "SELECT workspaces.id AS workspace_id FROM workspaces WHERE visibility = 'public'";
     const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;

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

@@ -82,6 +82,14 @@ describe('static parameter queries', () => {
     expect(query.getStaticBucketIds(normalizeTokenParameters({ user_id: 'user1' }))).toEqual(['mybucket["user1"]']);
   });
 
+  test('case-sensitive queries (1)', () => {
+    const sql = 'SELECT request.user_id() as USER_ID';
+    const query = SqlParameterQuery.fromSql('mybucket', sql) as SqlParameterQuery;
+    expect(query.errors).toMatchObject([
+      { message: `Unquoted identifiers are converted to lower-case. Use "USER_ID" instead.` }
+    ]);
+  });
+
   describe('dangerous queries', function () {
     function testDangerousQuery(sql: string) {
       test(sql, function () {