Date field formatting and orderBy.

[thruflo]

I have an ElectricCollection which loads tasks that have an inserted_at date field.

I'm parsing this when I sync the data:

const parser = {
  timestamp: (date: string) => new Date(date)
}

This is working, I see the date object in the collection data.

When I query it:

const { data } = useLiveQuery(query => (
    query
      .from({ taskCollection })
      .select('@id', '@title', '@inserted_at')
      .orderBy({'@inserted_at': 'desc'})
      .keyBy('@id')
  ))

1. I get the string version of the field back from the query
2. the orderBy doesn't work

The latter is confusing -- because I believe it was working and then stopped working. OrderBy works if I sort on another field. But not for inserted_at.

[gregermendle]

Care if I take a look at this?

[KyleAMathews]

Feel free!

[gregermendle]

Dug into this a bit and was curious if the timestamp parser above should in fact be timestamptz? Locally when testing on the todo app I can reproduce this issue when switching from timestamptz to timestamp. But I believe that is due to the fact that the todo schema is using a timestamp with a time zone.

[kevin-dp]

EDIT: Ignore this comment. While the explanation of orderBy in this comment is correct for D2TS, tanstack/db's orderBy works differently as it always passes an index column when using D2TS' orderBy operator. This is explained in my next comment.

---

TL;DR

The naming of the orderBy operator is confusing as it is not actually ordering the elements but rather re-arranging the stream to perform a topK operator.

Full answer

When using orderBy without an index column (like in your example) the query is compiled into a pipeline that uses the orderBy operator (cf. https://github.com/TanStack/db/blob/main/packages/db/src/query/order-by.ts#L236-L241). Looking into D2TS, the orderBy is rearranging the stream such that it can be passed to the topK operator (cf. https://github.com/electric-sql/d2ts/blob/main/packages/d2ts/src/operators/orderBy.ts#L56). So orderBy in fact does not order the elements but returns a stream of elements that are in the top K. Since the example snippet did not provide a limit nor an offset, we're in fact not really doing anything since K = infinity and offset = 0, hence, all elements from the input stream are in the output stream (without order).

If you're interested in the order of the elements then you need to add an index column in your select clause, like:

.select('@id', '@title', '@inserted_at', { index: { ORDER_INDEX: `numeric` } })

Now, this will be compiled into a pipeline that uses orderByWithIndex which will still compute a top K but now the result stream will add the index to the element, so your resulting collection will have an extra column "index" that is the index of that value in the sorted collection. Note that the order in which the (updated) values are emitted on the output stream is not necessarily the sorted order of the elements. However, i believe that the ts/db collection will take care of sorting the collection data based on that index column.

I did a quick test locally to order a collection using an index column like in the snippet above. I got the index property on the resulting collection and the collection was ordered by the index but the index is wrong. So (i believe) this is a bug we need to fix.

[kevin-dp]

Ignore my previous comment. Digging further in the tanstack/db source code i found that the query builder's select method seems to always add a default { _orderByIndex: { ORDER_INDEX: numeric } } to the array of selected columns (cf. https://github.com/TanStack/db/blob/main/packages/db/src/query/query-builder.ts#L269-L271). So it appears that even if we don't provide an index column, like in your original code snippet, it will use this default index instead. The collection should then sort the state based on that default index column. So, orderBy should indeed order the results (even when we're not explicitly passing an index column ourselves). However, the index column seems to be wrong in the query results.

The reason for the confusion is the else branch in the processOrderBy method (cf. https://github.com/TanStack/db/blob/main/packages/db/src/query/order-by.ts#L234-L243) which seems unreachable since tanstack/db always adds a default index column? cc. @samwillis

[samwillis]

@kevin-dp yes, currently only the first branch is used in ts/db - there is always a numeric index column.

db/packages/db/src/query/order-by.ts

Lines 188 to 210 in 1a3851d

	if (hasOrderIndexColumn) {
	if (orderIndexType === `numeric`) {
	// Use orderByWithIndex for numeric indices
	resultPipeline = resultPipeline.pipe(
	orderByWithIndex(valueExtractor, {
	limit: query.limit,
	offset: query.offset,
	comparator,
	}),
	map(([key, [value, index]]) => {
	// Add the index to the result
	// We add this to the main table alias for now
	// TODO: re are going to need to refactor the whole order by pipeline
	const result = {
	...(value as Record<string, unknown>),
	[mainTableAlias]: {
	...value[mainTableAlias],
	[orderIndexAlias]: index,
	},
	}
	return [key, result] as KeyedNamespacedRow
	})
	)

(this will change in future, but exactly how is TBD)

[kevin-dp]

Digging further into the issue, i found the source of the bug.
orderBy works fine when using ascending order but is wrong when using descending order.

The processOrderBy function roughly does this when sorting in descending order:

• extracts the value to sort
• if it is a string, inverts the string with String.fromCharCode(...[...val].map((c) => 65535 c.charCodeAt(0)))
• compares in ascending order using a.localeCompare(b)

The problem is that using localeCompare on inverted strings to compare in descending order doesn't work. Here's an example demonstrating that:

$ node
> const invert = (val) => String.fromCharCode(...[...val].map((c) => 65535  c.charCodeAt(0)))
> 'a' < 'b' // regular < check works fine
true
> invert('a') < invert('b')
false
> invert('a') > invert('b')
true

> 'a'.localeCompare('b') // localeCompare doesn't work with inversion
-1
> invert('a').localeCompare(invert('b')) // we're expecting 1
-1

The proper solution would be not to invert the strings but invert the result of localeCompare when the sorting order is descending. I will open a PR for that.

[KyleAMathews]

Fixed by #165