feat: support sortBy on joined table

feat: update ts definition for sortBy method

fix: throw error when sort by is used on joined table with loki

test: add integration test on sort on joined table

Author: mlecoq

CHANGELOG-Unreleased.md

@@ -17,6 +17,7 @@
 - [adapters] Adapter objects now returns `dbName`
 - [TypeScript] Add unsafeExecute method
 - [TypeScript] Add localStorage property to Database
+- [Query] Add `Q.sortBy({column:'columnName', table:'tableName'})` to support sorting on joined tables
 
 ### Performance
 

src/QueryDescription/index.d.ts

@@ -49,10 +49,12 @@ export type On = $RE<{
 export type SortOrder = 'asc' | 'desc'
 export const asc: SortOrder
 export const desc: SortOrder
+export type SortColumn = ColumnName | $RE<{column: ColumnName, table: TableName<any>}>
 export type SortBy = $RE<{
   type: 'sortBy',
   sortColumn: ColumnName,
   sortOrder: SortOrder,
+  table: TableName<any>,
 }>
 export type Take = $RE<{
   type: 'take',
@@ -193,7 +195,7 @@ export function and(...clauses: Where[]): And
 
 export function or(...clauses: Where[]): Or
 
-export function sortBy(sortColumn: ColumnName, sortOrder: SortOrder): SortBy
+export function sortBy(sortColumn: SortColumn, sortOrder: SortOrder): SortBy
 
 export function take(count: number): Take
 

src/QueryDescription/index.js

@@ -59,10 +59,12 @@ export type On = $RE<{
 export type SortOrder = 'asc' | 'desc'
 export const asc: SortOrder = 'asc'
 export const desc: SortOrder = 'desc'
+export type SortColumn = ColumnName | $RE<{ column: ColumnName, table: TableName<any> }>
 export type SortBy = $RE<{
   type: 'sortBy',
   sortColumn: ColumnName,
   sortOrder: SortOrder,
+  table?: TableName<any>,
 }>
 export type Take = $RE<{
   type: 'take',
@@ -319,12 +321,16 @@ export function or(...clauses: Where[]): Or {
   return { type: 'or', conditions: clauses }
 }
 
-export function sortBy(sortColumn: ColumnName, sortOrder: SortOrder = asc): SortBy {
+export function sortBy(sortColumn: SortColumn, sortOrder: SortOrder = asc): SortBy {
   invariant(
     sortOrder === 'asc' || sortOrder === 'desc',
     `Invalid sortOrder argument received in Q.sortBy (valid: asc, desc)`,
   )
-  return { type: 'sortBy', sortColumn: checkName(sortColumn), sortOrder }
+
+  const sortCol = typeof sortColumn === 'object' ? sortColumn.column : sortColumn
+  const table = typeof sortColumn === 'object' ? sortColumn.table : undefined
+
+  return { type: 'sortBy', sortColumn: checkName(sortCol), sortOrder, table }
 }
 
 export function take(count: number): Take {

src/QueryDescription/test.js

@@ -477,6 +477,19 @@ describe('buildQueryDescription', () => {
       sortBy: [{ type: 'sortBy', sortColumn: 'sortable_column', sortOrder: 'desc' }],
     })
   })
+  it('supports sorting query on joined table', () => {
+    const query = Q.buildQueryDescription([
+      Q.sortBy({ column: 'sortable_column', table: 'joinedTable' }, Q.desc),
+    ])
+    expect(query).toEqual({
+      where: [],
+      joinTables: [],
+      nestedJoinTables: [],
+      sortBy: [
+        { type: 'sortBy', sortColumn: 'sortable_column', sortOrder: 'desc', table: 'joinedTable' },
+      ],
+    })
+  })
   it('does not support skip operator without take operator', () => {
     expect(() => {
       Q.buildQueryDescription([Q.skip(100)])

src/__tests__/databaseTests.js

@@ -991,6 +991,7 @@ function joinTest(
     skipCount?: boolean,
     skipLoki?: boolean,
     skipSqlite?: boolean,
+    checkOrder?: boolean,
   }>,
 ): void {
   joinTests.push(options)
@@ -1140,6 +1141,28 @@ joinTest({
     { id: 'n6' }, // bad TT
   ],
 })
+joinTest({
+  name: `can perform Q.sort on joined table`,
+  query: [
+    Q.experimentalJoinTables(['tag_assignments']),
+    Q.sortBy({ column: 'text1', table: 'tag_assignments' }),
+  ],
+  extraRecords: {
+    tag_assignments: [
+      { id: 'tt1', text1: 'z', task_id: 'm6' },
+      { id: 'tt2', text1: 'y', task_id: 'm8' },
+      { id: 'tt3', text1: 'x', task_id: 'm7' },
+      { id: 'tt4', text1: 'w', task_id: 'n5' },
+      { id: 'tt5', text1: 'v', task_id: 'n6' },
+      { id: 'tt6', text1: 'u', task_id: 'm2' },
+      { id: 'tt7', text1: 'z', task_id: 'm2' },
+    ],
+  },
+  matching: [{ id: 'm2' }, { id: 'n6' }, { id: 'n5' }, { id: 'm7' }, { id: 'm8' }, { id: 'm6' }],
+  nonMatching: [],
+  checkOrder: true,
+  skipLoki: true,
+})
 joinTest({
   name: `can perform Q.on's nested in Q.on`,
   query: [

src/adapters/lokijs/worker/executeQuery.js

@@ -1,5 +1,7 @@
 // @flow
 
+import { invariant } from '../../../utils/common'
+
 import type { SerializedQuery } from '../../../Query'
 
 import type { DirtyRaw } from '../../../RawRecord'
@@ -31,6 +33,10 @@ function performQuery(query: SerializedQuery, loki: Loki): LokiResultset {
   // Step three: sort, skip, take
   const { sortBy, take, skip } = query.description
   if (sortBy.length) {
+    if (process.env.NODE_ENV !== 'production') {
+      invariant(!sortBy.some((sort) => sort.table), 'sortBy is not supported on joined table')
+    }
+
     resultset = resultset.compoundsort(
       sortBy.map(({ sortColumn, sortOrder }) => [sortColumn, sortOrder === 'desc']),
     )

src/adapters/sqlite/encodeQuery/index.js

@@ -196,7 +196,7 @@ const encodeOrderBy = (table: TableName<any>, sortBys: SortBy[]) => {
   }
   const orderBys = sortBys
     .map((sortBy) => {
-      return `"${table}"."${sortBy.sortColumn}" ${sortBy.sortOrder}`
+      return `"${sortBy.table ?? table}"."${sortBy.sortColumn}" ${sortBy.sortOrder}`
     })
     .join(', ')
   return ` order by ${orderBys}`

src/adapters/sqlite/encodeQuery/test.js

@@ -256,6 +256,11 @@ describe('SQLite encodeQuery', () => {
       `select "tasks".* from "tasks" where "tasks"."_status" is not 'deleted' order by "tasks"."sortable_column" desc, "tasks"."sortable_column2" asc`,
     )
   })
+  it('encodes order by clause on table', () => {
+    expect(encoded([Q.sortBy({ column: 'sortable_column', table: 'table' }, Q.desc)])).toBe(
+      `select "tasks".* from "tasks" where "tasks"."_status" is not 'deleted' order by "table"."sortable_column" desc`,
+    )
+  })
   it('encodes limit clause', () => {
     expect(encoded([Q.take(100)])).toBe(
       `select "tasks".* from "tasks" where "tasks"."_status" is not 'deleted' limit 100`,