From e4f7fa7fadc7ebf94ff21b2cc68186349745e210 Mon Sep 17 00:00:00 2001
From: Martin McKeaveney <martinmckeaveney@gmail.com>
Date: Thu, 16 Jan 2025 15:07:32 +0000
Subject: [PATCH] more consistent null handling

---
 packages/backend-core/src/sql/sql.ts          | 17 +++-----
 .../server/src/api/routes/tests/row.spec.ts   | 43 +++++++++++++++++++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts
index fb7cfdec1b..b91a331740 100644
--- a/packages/backend-core/src/sql/sql.ts
+++ b/packages/backend-core/src/sql/sql.ts
@@ -1162,20 +1162,14 @@ class InternalBuilder {
         const direction =
           value.direction === SortOrder.ASCENDING ? "asc" : "desc"
 
-        // TODO: figure out a way to remove this conditional, not relying on
-        // the defaults of each datastore.
-        let nulls: "first" | "last" | undefined = undefined
-        if (
-          this.client === SqlClient.POSTGRES ||
-          this.client === SqlClient.ORACLE
-        ) {
-          nulls = value.direction === SortOrder.ASCENDING ? "first" : "last"
-        }
+        let nulls: "first" | "last" =
+          value.direction === SortOrder.ASCENDING ? "first" : "last"
 
         if (this.isAggregateField(key)) {
-          query = query.orderByRaw(`?? ??`, [
+          query = query.orderByRaw(`?? ?? nulls ??`, [
             this.rawQuotedIdentifier(key),
             this.knex.raw(direction),
+            this.knex.raw(nulls as string),
           ])
         } else {
           let composite = `${aliased}.${key}`
@@ -1186,9 +1180,10 @@ class InternalBuilder {
               this.knex.raw(nulls as string),
             ])
           } else {
-            query = query.orderByRaw(`?? ??`, [
+            query = query.orderByRaw(`?? ?? nulls ??`, [
               this.rawQuotedIdentifier(composite),
               this.knex.raw(direction),
+              this.knex.raw(nulls as string),
             ])
           }
         }
diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts
index db5fcbaebb..101b9928f4 100644
--- a/packages/server/src/api/routes/tests/row.spec.ts
+++ b/packages/server/src/api/routes/tests/row.spec.ts
@@ -3650,6 +3650,49 @@ if (descriptions.length) {
         })
       })
 
+      describe("Fields with spaces", () => {
+        let table: Table
+        let otherTable: Table
+        let relatedRow: Row, mainRow: Row
+
+        beforeAll(async () => {
+          otherTable = await config.api.table.save(defaultTable())
+          table = await config.api.table.save(
+            saveTableRequest({
+              schema: {
+                links: {
+                  name: "links",
+                  fieldName: "links",
+                  type: FieldType.LINK,
+                  tableId: otherTable._id!,
+                  relationshipType: RelationshipType.ONE_TO_MANY,
+                },
+                "nameWithSpace ": {
+                  name: "nameWithSpace ",
+                  type: FieldType.STRING,
+                },
+              },
+            })
+          )
+          relatedRow = await config.api.row.save(otherTable._id!, {
+            name: generator.word(),
+            description: generator.paragraph(),
+          })
+          mainRow = await config.api.row.save(table._id!, {
+            "nameWithSpace ": generator.word(),
+            tableId: table._id!,
+            links: [relatedRow._id],
+          })
+        })
+
+        it("Successfully returns rows that have spaces in their field names", async () => {
+          const { rows } = await config.api.row.search(table._id!)
+          expect(rows.length).toBe(1)
+          const row = rows[0]
+          expect(row["nameWithSpace "]).toBeDefined()
+        })
+      })
+
       if (!isInternal && !isOracle) {
         describe("bigint ids", () => {
           let table1: Table, table2: Table