Skip to content

DDIR: ordering, source shapes and scalar operations for SNB reads - #863

Merged
frankmcsherry merged 4 commits into
master-nextfrom
codex/ddir-ldbc-prerequisites
Sep 7, 2026
Merged

DDIR: ordering, source shapes and scalar operations for SNB reads#863
frankmcsherry merged 4 commits into
master-nextfrom
codex/ddir-ldbc-prerequisites

Conversation

@frankmcsherry

@frankmcsherry frankmcsherry commented Sep 7, 2026

Copy link
Copy Markdown
Member

DDIR: ordering, source shapes and scalar operations for the SNB reads

Base: master-next. Part one of the benchmark foundation; the runnable
benchmark is the separate, stacked PR #864.

This is the engine-prerequisite part of the LDBC work. The runnable benchmark
follows separately.

  1. Make Corgi Min and Collect agree with DDIR's observable signed-integer
    and lexicographic list ordering. Physical arrangement-key ordering is not
    changed, nor is Corgi's general structural ordering.
  2. Allow explicit shapes on external inputs/imports. Empty strings/lists and
    inactive SUM lanes cannot reveal their full shape from rows. These are
    execution-time contracts, enforced by assertions at the boundary; they are
    not transactional feed validation or a new static type system. A mismatched
    row can panic a dataflow worker on either backend and take down the shared
    server, disconnecting all clients. A feed acknowledgement does not certify
    its shape. There is no per-program failure isolation; this foundation assumes
    trusted programs and shape-correct data. This assertion adds a fatal failure
    path to the Vec backend; Corgi's transcoding could already panic on misfit rows.
  3. Add scalar operations needed by the read catalogue: integer division, list
    append, value-only projection for collect, declared empty-list constructors,
    and explicit floating conversion/arithmetic. Pin Corgi to the merged public
    division revision be00398; no local dependency override is required.

The list-ordering adapter is not performance-neutral. The reviewer measured
100,000 rows, comparing the old length-first sort against rank construction
plus sorting the ranks:

List length Alphabet size Old New Ratio
4 1000 4.3 ms 13.4 ms 3.1x
16 1000 4.3 ms 34.6 ms 8.1x
4 4 5.0 ms 11.5 ms 2.3x
16 4 16.5 ms 49.1 ms 3.0x

These are reviewer-supplied measurements, not independently reproduced here
and not whole-query slowdown ratios. Min/Collect use this ranker for
list-valued subcolumns, including nested lists and strings represented as lists.
Non-list values do not run the list ranker. It eagerly ranks every element,
including unused tails, then scans all lists and allocates scratch per prefix
round until prefixes distinguish values or reach their ends. The fixed-length
lists of scalar elements measured here already have the same length-first and
lexicographic order, so these cases expose implementation overhead rather than
an unavoidable cost of changing the answer. Ranking optimization and server
admission/isolation are separate follow-ups.

The floating representation uses an existing one-variant SUM carrying encoded
f64 bits, with explicit operators and total ordering. There is no new nominal
runtime type: a user's single-variant integer newtype also passes the structural
checks for fneg and binary floating operators, and its payload is interpreted
as encoded f64 bits. This is a type-checking limitation. The representation also
differs from LDBC's Float32 API.
Integer division returns zero on a zero divisor and wraps i64::MIN / -1,
matching the selected Corgi operation.

A declared constructor can supply the element shape for empty list(), but
Corgi's shape inference does not propagate between append arguments:
append(list(), xs) is rejected even when xs has a known element shape.

The branch is replayed onto master-next at 660ff8da. The only conflict was
the dependency pin, resolved to the merged revision. Benchmark definitions,
data, profiling hooks and optimizer experiments are not in this diff.

Before the review cleanup, 118 Rust library/integration tests passed (four
existing heavy tests remained ignored). The unchanged downstream LDBC fixture passed the local
CI command matrix on both backends: four-query panel at one/four workers,
all-41 mixed at one/four workers, and all-41 maintained at four workers.
These are local macOS checks, not a claim that hosted CI ran. Full-suite checks
use the same logical-plan Python evaluator, not an independent LDBC oracle.
An optimized server build and the same downstream fixture matrix also pass
with CARGO_PROFILE_RELEASE_DEBUG=0 (opt-level 3, LTO, one codegen unit retained).
This does not claim that the ignored heavy Rust checks or all hosted CI jobs ran.

Review cleanup (5d08c7b2) makes unary eligibility an exhaustive match, fixes
the has_shape indentation and documents the limitations above; it changes no
runtime semantics and adds no tests. Fresh checks pass: all 53 library tests,
all eight server integration tests, the benchmark's Python checks, and all 41
reads on both backends with four workers in mixed and maintained modes using
the rebuilt optimized server and tiny witness. The reviewer's ranking timings
and fuzz campaigns were not rerun.

The separately reproduced upstream chunk merge-horizon issue is fixed in #862.
This PR neither introduces nor includes that fix; small-fixture passes must
not be taken as evidence that the issue is resolved.

Squash merging is fine. Preserve this branch's old tip before merging, then
replay only the benchmark branch's own commits onto the actual squash merge.

Keep unary eligibility exhaustive, correct the has_shape indentation, and document empty-list inference and the structural floating representation. Disclose fatal ascription assertions and the cost of lexicographic list ranking without changing either behavior.
@frankmcsherry
frankmcsherry merged commit b83dc32 into master-next Sep 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant