Skip to content

Commit f5838d5

Browse files
FrancescAltedclaude
andcommitted
Record the dictionary rank sort in the assessment
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d41d92e commit f5838d5

1 file changed

Lines changed: 45 additions & 6 deletions

File tree

‎plans/string-flavours-assessment.md‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,10 @@ Also note `group_by` is *fastest* on dictionary (81 ms vs 217 ms for `<U`, 217 m
181181
codes are already integers, so there is nothing to factorize. This was true even before the decode
182182
fix, because `group_by` was the only caller that used the batched `decode_batch()` path.
183183

184-
One optimization deliberately left on the table: `_build_lex_keys` (`ctable.py:11292`) still decodes
185-
a dictionary column to an object array and lexsorts that. Sorting the int32 *ranks* instead — the
186-
trick `create_index` already uses — measured ~4× faster again (107 ms → 27 ms per 200 k rows). Not
187-
done, because it is a tuning item rather than a bug once the decode is O(D) instead of O(N).
184+
~~One optimization deliberately left on the table: `_build_lex_keys` (`ctable.py:11292`) still
185+
decodes a dictionary column to an object array and lexsorts that. Sorting the int32 *ranks* instead
186+
— the trick `create_index` already uses — measured ~4× faster again (107 ms → 27 ms per 200 k
187+
rows).~~ — **done**, see ¹³.
188188

189189
### Could utf8 have an index? Yes, and the pattern is already in-tree
190190

@@ -332,8 +332,11 @@ Found while measuring, unrelated to the utf8 decision:
332332
- ~~`create_index` accepts any of the five kinds on a rank-indexed column and silently builds one
333333
that is never consulted~~ — **fixed**, see §⁹.
334334
- ~~Nested (dotted) utf8 leaves cannot be filtered~~ — **fixed**, see ¹².
335-
- Still open: `_build_lex_keys` could sort dictionary ranks instead of decoded strings (~4×), and
336-
the `where("c == 'x'")` string form still bypasses the index for both flavours.
335+
- ~~`_build_lex_keys` could sort dictionary ranks instead of decoded strings (~4×)~~ — **done**,
336+
see ¹³.
337+
- Still open: the `where("c == 'x'")` string form bypasses the index for both flavours, deliberately
338+
(see ⁷ — substituting a precomputed mask measured *slower*; the real fix is teaching `plan_query`
339+
to consume index positions).
337340

338341
⁶ `blosc2.lazyexpr` over a bare `DictionaryColumn` returns the **capacity-padded** slot array —
339342
1 048 576 rows for a 3-row table. Not a container bug: `DictionaryColumn.__len__` is documented as
@@ -711,3 +714,39 @@ written — alias map threaded through `_rewrite_nested_expression`, three call
711714
`==` started passing, which looked like success. It was not: the mask rewrite had simply removed the
712715
dotted name from the expression entirely, and `startswith` still failed. A plan's diagnosis is worth
713716
re-deriving before its prescription is implemented, and one passing case is not a route check.
717+
718+
---
719+
720+
## ¹³ Dictionary `sort_by` on ranks — what shipped
721+
722+
The last tuning item from the list above. A row's alphabetical rank orders exactly as its decoded
723+
value does, which is the whole basis of the FULL rank index; `_build_lex_keys` was the one caller
724+
still paying for the strings. It now maps codes through `_dict_code_to_rank` — extracted from the
725+
index builder so the two cannot drift — and hands `np.lexsort` an int32 key.
726+
727+
200 k rows, cardinality 5 000, single ascending key:
728+
729+
| | before | after |
730+
|---|---|---|
731+
| key construction | 106.2 ms | **21.4 ms** |
732+
| `sort_by(view=True)` | 246.7 ms | **156.9 ms** |
733+
| `sort_by(view=True, ascending=False)` | 273.5 ms | **156.2 ms** |
734+
| `sort_by()` (materializes) | 298.5 ms | 206.5 ms |
735+
736+
Key construction matches the ~4× the assessment predicted; end-to-end is ~1.6×, because the key was
737+
about 40 % of `sort_by`. Descending gains a little more than ascending: an object key could not be
738+
negated, so it went through a double `argsort` that an int32 key does not need.
739+
740+
**A second copy of the key builder turned up** in `_sorted_small_copy_from_live_positions` — the
741+
filtered-table path — carrying the same decode, its own subtly different dtype list (`USO` against
742+
`USOT`, so no `StringDType` sort keys), and a `KeyError` for a computed sort column. It also
743+
gathered the codes and then threw them away to re-read and decode the column. It now calls
744+
`_build_lex_keys` with the arrays it already gathered, which deletes the duplicate and fixes all
745+
three divergences.
746+
747+
Verification worth noting, since a sort is easy to get subtly wrong: the new tests were run against
748+
the *old* implementation too. The two behaviour tests (nulls last in both directions, rank keys
749+
composing with a secondary key, filtered path agreeing) pass on both, so they pin behaviour rather
750+
than the change; only the route assertion — that the value key is `int32` — fails on the old code.
751+
Separately, a fuzz over 5 table sizes × 7 column/direction combinations × with and without deleted
752+
rows was checked against a plain Python `sorted()` reference.

0 commit comments

Comments
 (0)