Merge pull request #4885 from nocodb/fix/null-sort-order

fix: Set sort order of nulls in postgres

packages/nc-gui/components/smartsheet/Grid.vue

@@ -418,7 +418,18 @@ async function clearCell(ctx: { row: number; col: number } | null, skipUpdate =
     return
   }
 
+  // handle Checkbox and rating fields in a special way
+  switch (columnObj.uidt) {
+    case UITypes.Checkbox:
+      rowObj.row[columnObj.title] = false
+      break
+    case UITypes.Rating:
+      rowObj.row[columnObj.title] = 0
+      break
+    default:
       rowObj.row[columnObj.title] = null
+      break
+  }
 
   if (!skipUpdate) {
     // update/save cell value
@@ -663,7 +674,8 @@ const closeAddColumnDropdown = () => {
           <thead ref="tableHead">
           <tr class="nc-grid-header border-1 bg-gray-100 sticky top[-1px]">
             <th data-testid="grid-id-column">
-                <div class="w-full h-full bg-gray-100 flex min-w-[70px] pl-5 pr-1 items-center" data-testid="nc-check-all">
+              <div class="w-full h-full bg-gray-100 flex min-w-[70px] pl-5 pr-1 items-center"
+                   data-testid="nc-check-all">
                 <template v-if="!readOnly">
                   <div class="nc-no-label text-gray-500" :class="{ hidden: selectedAllRecords }">#</div>
                   <div
@@ -885,7 +897,8 @@ const closeAddColumnDropdown = () => {
               </div>
             </a-menu-item>
 
-            <a-menu-item v-if="contextMenuTarget" data-testid="context-menu-item-copy" @click="copyValue(contextMenuTarget)">
+            <a-menu-item v-if="contextMenuTarget" data-testid="context-menu-item-copy"
+                         @click="copyValue(contextMenuTarget)">
               <div v-e="['a:row:copy']" class="nc-project-menu-item">
                 <!-- Copy -->
                 {{ $t('general.copy') }}

packages/nc-gui/utils/filterUtils.ts

@@ -41,13 +41,13 @@ export const comparisonOpList: {
     text: 'is empty',
     value: 'empty',
     ignoreVal: true,
-    excludedTypes: [UITypes.Checkbox],
+    excludedTypes: [UITypes.Checkbox, UITypes.Rating],
   },
   {
     text: 'is not empty',
     value: 'notempty',
     ignoreVal: true,
-    excludedTypes: [UITypes.Checkbox],
+    excludedTypes: [UITypes.Checkbox, UITypes.Rating],
   },
   {
     text: 'is null',

packages/nocodb-sdk/src/lib/sqlUi/MssqlUi.ts

@@ -1084,6 +1084,7 @@ export class MssqlUi {
       case 'Checkbox':
         colProp.dt = 'tinyint';
         colProp.dtxp = 1;
+        colProp.cdf = '0';
         break;
       case 'MultiSelect':
         colProp.dt = 'text';
@@ -1150,6 +1151,7 @@ export class MssqlUi {
         break;
       case 'Rating':
         colProp.dt = 'int';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'varchar';

packages/nocodb-sdk/src/lib/sqlUi/MysqlUi.ts

@@ -597,7 +597,6 @@ export class MysqlUi {
   }
 
   static onCheckboxChangeAI(col) {
-    console.log(col);
     if (
       col.dt === 'int' ||
       col.dt === 'bigint' ||
@@ -977,6 +976,7 @@ export class MysqlUi {
       case 'Checkbox':
         colProp.dt = 'tinyint';
         colProp.dtxp = 1;
+        colProp.cdf = '0';
         break;
       case 'MultiSelect':
         colProp.dt = 'set';
@@ -1049,6 +1049,7 @@ export class MysqlUi {
         break;
       case 'Rating':
         colProp.dt = 'int';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'varchar';

packages/nocodb-sdk/src/lib/sqlUi/OracleUi.ts

@@ -824,6 +824,7 @@ export class OracleUi {
       case 'Checkbox':
         colProp.dt = 'tinyint';
         colProp.dtxp = 1;
+        colProp.cdf = '0';
         break;
       case 'MultiSelect':
         colProp.dt = 'varchar2';
@@ -890,6 +891,7 @@ export class OracleUi {
         break;
       case 'Rating':
         colProp.dt = 'integer';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'varchar';

packages/nocodb-sdk/src/lib/sqlUi/PgUi.ts

@@ -1596,6 +1596,7 @@ export class PgUi {
         break;
       case 'Checkbox':
         colProp.dt = 'bool';
+        colProp.cdf = 'false';
         break;
       case 'MultiSelect':
         colProp.dt = 'text';
@@ -1662,6 +1663,7 @@ export class PgUi {
         break;
       case 'Rating':
         colProp.dt = 'smallint';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'character varying';

packages/nocodb-sdk/src/lib/sqlUi/SnowflakeUi.ts

@@ -688,6 +688,7 @@ export class SnowflakeUi {
         break;
       case 'Checkbox':
         colProp.dt = 'BOOLEAN';
+        colProp.cdf = '0';
         break;
       case 'MultiSelect':
         colProp.dt = 'TEXT';
@@ -700,7 +701,6 @@ export class SnowflakeUi {
         break;
       case 'Date':
         colProp.dt = 'DATE';
-
         break;
       case 'Year':
         colProp.dt = 'INT';
@@ -754,6 +754,7 @@ export class SnowflakeUi {
         break;
       case 'Rating':
         colProp.dt = 'SMALLINT';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'VARCHAR';

packages/nocodb-sdk/src/lib/sqlUi/SqliteUi.ts

@@ -788,9 +788,8 @@ export class SqliteUi {
         colProp.dt = 'text';
         break;
       case 'Checkbox':
-        // colProp.dt = 'tinyint';
-        // colProp.dtxp = 1;
         colProp.dt = 'boolean';
+        colProp.cdf = '0';
         break;
       case 'MultiSelect':
         colProp.dt = 'text';
@@ -857,6 +856,7 @@ export class SqliteUi {
         break;
       case 'Rating':
         colProp.dt = 'integer';
+        colProp.cdf = '0';
         break;
       case 'Formula':
         colProp.dt = 'varchar';

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/conditionV2.ts

@@ -303,11 +303,11 @@ const parseConditionV2 = async (
             } else {
               val = val.startsWith('%') || val.endsWith('%') ? val : `%${val}%`;
             }
-            qb = qb.whereNot(
-              field,
-              qb?.client?.config?.client === 'pg' ? 'ilike' : 'like',
-              val
-            );
+            if (qb?.client?.config?.client === 'pg') {
+              qb = qb.whereRaw('??::text not ilike ?', [field, val]);
+            } else {
+              qb = qb.whereNot(field, 'like', val);
+            }
             break;
           case 'allof':
           case 'anyof':

packages/nocodb/src/lib/db/sql-data-mapper/lib/sql/sortV2.ts

@@ -29,6 +29,9 @@ export default async function sortV2(
     const column = await sort.getColumn();
     if (!column) continue;
     const model = await column.getModel();
+
+    const nulls = sort.direction === 'desc' ? 'LAST' : 'FIRST';
+
     switch (column.uidt) {
       case UITypes.Rollup:
         {
@@ -39,7 +42,7 @@ export default async function sortV2(
             })
           ).builder;
 
-          qb.orderBy(builder, sort.direction || 'asc');
+          qb.orderBy(builder, sort.direction || 'asc', nulls);
         }
         break;
       case UITypes.Formula:
@@ -55,7 +58,7 @@ export default async function sortV2(
               column
             )
           ).builder;
-          qb.orderBy(builder, sort.direction || 'asc');
+          qb.orderBy(builder, sort.direction || 'asc', nulls);
         }
         break;
       case UITypes.Lookup:
@@ -178,7 +181,7 @@ export default async function sortV2(
                 break;
             }
 
-            qb.orderBy(selectQb, sort.direction || 'asc');
+            qb.orderBy(selectQb, sort.direction || 'asc', nulls);
           }
         }
         break;
@@ -206,7 +209,7 @@ export default async function sortV2(
               ])
             );
 
-          qb.orderBy(selectQb, sort.direction || 'asc');
+          qb.orderBy(selectQb, sort.direction || 'asc', nulls);
         }
         break;
       case UITypes.SingleSelect: {
@@ -214,17 +217,23 @@ export default async function sortV2(
         if (clientType === 'mysql' || clientType === 'mysql2') {
           qb.orderBy(
             sanitize(knex.raw('CONCAT(??)', [column.column_name])),
-            sort.direction || 'asc'
+            sort.direction || 'asc',
+            nulls
           );
         } else if (clientType === 'mssql') {
           qb.orderBy(
             sanitize(
               knex.raw('CAST(?? AS VARCHAR(MAX))', [column.column_name])
             ),
-            sort.direction || 'asc'
+            sort.direction || 'asc',
+            nulls
           );
         } else {
-          qb.orderBy(sanitize(column.column_name), sort.direction || 'asc');
+          qb.orderBy(
+            sanitize(column.column_name),
+            sort.direction || 'asc',
+            nulls
+          );
         }
         break;
       }
@@ -233,22 +242,32 @@ export default async function sortV2(
         if (clientType === 'mysql' || clientType === 'mysql2') {
           qb.orderBy(
             sanitize(knex.raw('CONCAT(??)', [column.column_name])),
-            sort.direction || 'asc'
+            sort.direction || 'asc',
+            nulls
           );
         } else if (clientType === 'mssql') {
           qb.orderBy(
             sanitize(
               knex.raw('CAST(?? AS VARCHAR(MAX))', [column.column_name])
             ),
-            sort.direction || 'asc'
+            sort.direction || 'asc',
+            nulls
           );
         } else {
-          qb.orderBy(sanitize(column.column_name), sort.direction || 'asc');
+          qb.orderBy(
+            sanitize(column.column_name),
+            sort.direction || 'asc',
+            nulls
+          );
         }
         break;
       }
       default:
-        qb.orderBy(sanitize(column.column_name), sort.direction || 'asc');
+        qb.orderBy(
+          sanitize(column.column_name),
+          sort.direction || 'asc',
+          nulls
+        );
         break;
     }
   }

tests/playwright/tests/columnCheckbox.spec.ts

@@ -2,7 +2,6 @@ import { test } from '@playwright/test';
 import { DashboardPage } from '../pages/Dashboard';
 import setup from '../setup';
 import { ToolbarPage } from '../pages/Dashboard/common/Toolbar';
-import { isPg } from '../setup/db';
 
 test.describe('Checkbox - cell, filter, sort', () => {
   let dashboard: DashboardPage, toolbar: ToolbarPage;
@@ -87,10 +86,10 @@ test.describe('Checkbox - cell, filter, sort', () => {
     // Filter column
     await verifyFilter({ opType: 'is checked', result: ['1a', '1c', '1f'] });
     await verifyFilter({ opType: 'is not checked', result: ['1b', '1d', '1e'] });
-    await verifyFilter({ opType: 'is equal', value: '0', result: ['1b'] });
+    await verifyFilter({ opType: 'is equal', value: '0', result: ['1b', '1d', '1e'] });
     await verifyFilter({ opType: 'is not equal', value: '1', result: ['1b', '1d', '1e'] });
-    await verifyFilter({ opType: 'is null', result: ['1d', '1e'] });
-    await verifyFilter({ opType: 'is not null', result: ['1a', '1b', '1c', '1f'] });
+    await verifyFilter({ opType: 'is null', result: [] });
+    await verifyFilter({ opType: 'is not null', result: ['1a', '1b', '1c', '1d', '1e', '1f'] });
 
     // Sort column
     await toolbar.sort.add({
@@ -98,11 +97,7 @@ test.describe('Checkbox - cell, filter, sort', () => {
       isAscending: true,
       isLocallySaved: false,
     });
-    if (isPg(context)) {
-      await validateRowArray(['1b', '1a', '1c', '1f', '1d', '1e']);
-    } else {
-      await validateRowArray(['1d', '1e', '1b', '1a', '1c', '1f']);
-    }
+    await validateRowArray(['1b', '1d', '1e', '1a', '1c', '1f']);
     await toolbar.sort.reset();
 
     // sort descending & validate
@@ -111,11 +106,7 @@ test.describe('Checkbox - cell, filter, sort', () => {
       isAscending: false,
       isLocallySaved: false,
     });
-    if (isPg(context)) {
-      await validateRowArray(['1d', '1e', '1a', '1c', '1f', '1b']);
-    } else {
     await validateRowArray(['1a', '1c', '1f', '1b', '1d', '1e']);
-    }
     await toolbar.sort.reset();
 
     // wait for 10 seconds

tests/playwright/tests/columnRating.spec.ts

@@ -0,0 +1,106 @@
+import { test } from '@playwright/test';
+import { DashboardPage } from '../pages/Dashboard';
+import setup from '../setup';
+import { ToolbarPage } from '../pages/Dashboard/common/Toolbar';
+
+test.describe('Rating - cell, filter, sort', () => {
+  let dashboard: DashboardPage, toolbar: ToolbarPage;
+  let context: any;
+
+  // define validateRowArray function
+  async function validateRowArray(value: string[]) {
+    const length = value.length;
+    for (let i = 0; i < length; i++) {
+      await dashboard.grid.cell.verify({
+        index: i,
+        columnHeader: 'Title',
+        value: value[i],
+      });
+    }
+  }
+
+  async function verifyFilter(param: { opType: string; value?: string; result: string[] }) {
+    await toolbar.clickFilter();
+    await toolbar.filter.add({
+      columnTitle: 'rating',
+      opType: param.opType,
+      value: param.value,
+      isLocallySaved: false,
+    });
+    await toolbar.clickFilter();
+
+    // verify filtered rows
+    await validateRowArray(param.result);
+    // Reset filter
+    await toolbar.filter.reset();
+  }
+
+  test.beforeEach(async ({ page }) => {
+    context = await setup({ page });
+    dashboard = new DashboardPage(page, context.project);
+    toolbar = dashboard.grid.toolbar;
+  });
+
+  test('Rating', async () => {
+    // close 'Team & Auth' tab
+    await dashboard.closeTab({ title: 'Team & Auth' });
+
+    await dashboard.treeView.createTable({ title: 'Sheet1' });
+
+    await dashboard.grid.addNewRow({ index: 0, value: '1a' });
+    await dashboard.grid.addNewRow({ index: 1, value: '1b' });
+    await dashboard.grid.addNewRow({ index: 2, value: '1c' });
+    await dashboard.grid.addNewRow({ index: 3, value: '1d' });
+    await dashboard.grid.addNewRow({ index: 4, value: '1e' });
+    await dashboard.grid.addNewRow({ index: 5, value: '1f' });
+
+    // Create Rating column
+    await dashboard.grid.column.create({
+      title: 'rating',
+      type: 'Rating',
+    });
+
+    // In cell insert
+    await dashboard.grid.cell.rating.select({ index: 0, columnHeader: 'rating', rating: 2 });
+    await dashboard.grid.cell.rating.select({ index: 2, columnHeader: 'rating', rating: 1 });
+    await dashboard.grid.cell.rating.select({ index: 5, columnHeader: 'rating', rating: 0 });
+
+    // column values
+    // 1a : 3
+    // 1b : 0
+    // 1c : 2
+    // 1d : 0
+    // 1e : 0
+    // 1f : 1
+
+    // Filter column
+    await verifyFilter({ opType: 'is equal', value: '3', result: ['1a'] });
+    await verifyFilter({ opType: 'is not equal', value: '3', result: ['1b', '1c', '1d', '1e', '1f'] });
+    await verifyFilter({ opType: 'is like', value: '2', result: ['1c'] });
+    await verifyFilter({ opType: 'is not like', value: '2', result: ['1a', '1b', '1d', '1e', '1f'] });
+    await verifyFilter({ opType: 'is null', result: [] });
+    await verifyFilter({ opType: 'is not null', result: ['1a', '1b', '1c', '1d', '1e', '1f'] });
+    // await verifyFilter({ opType: '>', value: '1', result: ['1a', '1c'] });
+    await verifyFilter({ opType: '>=', value: '1', result: ['1a', '1c', '1f'] });
+    // await verifyFilter({ opType: '<', value: '1', result: [] });
+    await verifyFilter({ opType: '<=', value: '1', result: ['1b', '1d', '1e', '1f'] });
+
+    // Sort column
+    await toolbar.sort.add({
+      columnTitle: 'rating',
+      isAscending: true,
+      isLocallySaved: false,
+    });
+    await validateRowArray(['1b', '1d', '1e', '1f', '1c', '1a']);
+    await toolbar.sort.reset();
+
+    // sort descending & validate
+    await toolbar.sort.add({
+      columnTitle: 'rating',
+      isAscending: false,
+      isLocallySaved: false,
+    });
+    await validateRowArray(['1a', '1c', '1f', '1b', '1d', '1e']);
+    await toolbar.sort.reset();
+  });
+});