[alxmrs]

🪭 Claude (einfold)

Second pull request of the restacked M1 (design v17, #42), stacked on #43. It adds einfold-kernels, a crate of EinFold's algorithms over Arrow arrays with no query-engine dependency. An engine that keeps its data as Arrow arrays can run them: DataFusion now, and a DuckDB extension such as duckdb-zarr later (#39). The first algorithm is the hash path, ported from #37 to the new IR.

What's here

• schema(fold, operands, domain): the result's schema, with the type checks Add the Fold node and EinFoldExec's hash path #37's FoldNode made. The checks now live with the kernels, so every engine gets them. Its columns are named by position ("0", "1", …), since a fold names none; the kernels read only the schema's types, so a caller names the columns as it likes.
• fold(fold, a, b, domain, schema, host): Gustavson's sparse product ("Two Fast Algorithms for Sparse Matrices", 1978), generalized to folds. It follows SQL's bag semantics, group existence (a group reached only by NULL products exists, with a NULL sum), NULL keys per dimension, and a fixed visiting order, so results don't depend on how rows arrive.
• The domain is a plain left join. Each domain row gets the group its keys match, each key under its own = or IS NOT DISTINCT FROM, then COALESCE(value, missing) if a default is given. Add the Fold node and EinFoldExec's hash path #37's ddx-specific "NULL where the fill's value is NULL" is gone. A projection above the node can express it.
• Host carries what SQL leaves to the engine. For now that's FloatKeys: whether -0.0 and 0.0 are one key. DataFusion 55 says yes, except for dictionary-encoded floats, which einfold#41 drafts for upstream.

About 380 non-test lines.

Invariants tested (tests/kernels_vs_sql.rs, ported from #37 and #38). On random folds of one to four dimensions, with BIGINT, VARCHAR, DOUBLE and dictionary keys, NULL keys, duplicate tuples, missing rows, and factors that are NULL, NaN, ±inf, -0.0 or ill-conditioned, half of them with a domain:

1. the same rows as the SQL the fold stands for, run by DataFusion, the domain as a LEFT JOIN with COALESCE;
2. the same values within float summation's bound, and exactly SQL's NULLs, NaNs, infinities and sign of zero;
3. the same bits when the rows arrive in another order and other batches.

The oracle writes IS NOT DISTINCT FROM as = on (key IS NULL, COALESCE(key, sentinel)), which avoids mixing both kinds of equality in one join chain (#21).

🤖 Generated with Claude Code

https://claude.ai/code/session_01M96xSez6PZwTmfaGh2eJgS

Summary by CodeRabbit

• New Features
  • Added a fold representation for describing joins, outputs, and aggregation, with validation for invalid configurations.
  • Added a hash-based kernel for folding two operands, including support for optional domains, NULL handling, and configurable float-key comparison.
  • Added APIs for deriving output schemas and computing fold results.
• Tests
  • Added broad comparisons against SQL results, covering varied keys, NULLs, batching, and numeric edge cases.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Files ignored due to path filters: 1

• Cargo.lock is excluded by !**/*.lock

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 603dcef7-fd14-401d-b1fb-ecb6a1dd70f4

Commits

Reviewing files that changed from the base of the PR and between 2c7310e and c694fc5.

Files ignored due to path filters: 1

• Cargo.lock is excluded by !**/*.lock

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: a1ecb2df-1a32-41bc-b04b-33b4e77f0de2

Commits

Reviewing files that changed from the base of the PR and between 49b7cc6 and 2c7310e.

Files ignored due to path filters: 1

• Cargo.lock is excluded by !**/*.lock

Files selected for processing: 8

• Cargo.toml
• crates/einfold-ir/src/fold.rs
• crates/einfold-ir/src/lib.rs
• crates/einfold-kernels/Cargo.toml
• crates/einfold-kernels/src/hash.rs
• crates/einfold-kernels/src/keys.rs
• crates/einfold-kernels/src/lib.rs
• crates/einfold-kernels/tests/kernels_vs_sql.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The change adds validated fold metadata to einfold-ir and a new Arrow kernel crate. The kernel builds output schemas, joins two operands by shared keys, aggregates products, and can populate output rows from a domain. Seeded property tests compare kernel results with DataFusion SQL.

Changes: Arrow Fold Kernel

Layer / File(s)	Summary
Fold model and validation crates/einfold-ir/src/fold.rs, crates/einfold-ir/src/lib.rs	Adds fold metadata for dimensions, operands, outputs, aggregation, and domains. Fold::try_new validates fold descriptions, and the crate re-exports the fold types.
Kernel API and schema Cargo.toml, crates/einfold-kernels/Cargo.toml, crates/einfold-kernels/src/lib.rs, crates/einfold-kernels/src/keys.rs	Adds the kernel crate, host float-key policy, schema validation, and Arrow row-key encoding. SignedZerosEqual normalizes signed zero in plain floating-point columns.
Hash join, grouping, and domain output crates/einfold-kernels/src/hash.rs, crates/einfold-kernels/src/lib.rs	Adds deterministic ordering and hash matching for two operands. The kernel sums non-NULL products and returns grouped results or rows matched against supplied domain batches.
Generated cases and SQL oracle crates/einfold-kernels/tests/kernels_vs_sql.rs	Generates seeded fold inputs and builds equivalent DataFusion SQL, including optional domain joins.
Kernel execution and result checks crates/einfold-kernels/tests/kernels_vs_sql.rs	Runs the kernel against the generated cases and checks repeatability, row keys, NULLs, special floating-point values, and bounded numeric differences against SQL.

Priority: Pending latest changes

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant FoldAPI as einfold-kernels fold API
  participant HashFold as hash::fold
  participant Side
  participant Groups as Output groups
  Caller->>FoldAPI: Submit fold, operand batches, and optional domain batches
  FoldAPI->>HashFold: Delegate fold execution
  HashFold->>Side: Validate and order operand rows
  Side-->>HashFold: Return shared and output keys
  HashFold->>Groups: Index second operand and accumulate matching products
  HashFold->>Groups: Match groups to domain rows when configured
  HashFold-->>FoldAPI: Return output batches
  FoldAPI-->>Caller: Return Algorithm::Hash and batches

Loading

Possibly related PRs

• xqlsystems/einfold#43: Defines the Fold IR types and invariants used by this kernel.
• xqlsystems/einfold#37: Adds an earlier hash implementation whose behavior this PR ports to the Arrow kernel API.
• xqlsystems/einfold#42: Defines the engine-independent kernel architecture implemented by this PR.

Merge Risk: Minimal · previous review 2c731

No concrete issue requiring a fix before merge has been established; proceed with normal checks.

Security architecture risk: Low · up to 2c731

The new library has limited demonstrated exposure: the identified caller is a test, and execution uses call-local state. Callers remain responsible for validating inputs before execution. No introduced security attack path was established, but production integration and exposure are not fully covered.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is an in-process computation over caller-supplied Arrow batches, with a test-only caller identified. Production cross-service or tenant reachability is not established; the reported five-dependent count does not by itself demonstrate such exposure.

Security Findings and Attack Paths

• inferred — A caller supplying incompatible metadata or batches could reach a panic instead of a returned validation error. An attacker-controlled route to those arguments is not established, and the identified caller validates schemas first, so this remains an integration constraint rather than a verified denial-of-service finding.

Trust Boundaries and Controls

• observed — The inspected boundary separates host-supplied fold metadata and Arrow data from kernel execution. Structural validation belongs to Fold construction; supported arity, column existence and matching dimension types belong to schema(). These are data-contract controls, not authentication or authorization checks.

Resilience and Maintainability Implications

• observed — Hash indexes, interners, accumulators and output vectors are invocation-local. Normal errors propagate without returning earlier domain batches, and carried Arrow arrays are immutable shared references. Repetition and concurrent invocations therefore do not share accumulator state in the inspected path; malformed-input panics remain outside that normal error guarantee.

Hardening Proposals

• proposed — Before adding an untrusted production query path, bind execution to validated input schemas through a prepared contract or equivalent entrypoint checks. This would make failure containment independent of callers remembering the separate validation sequence.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 56.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title "Add einfold-kernels, with EinFold's hash algorithm on Arrow arrays" names the main change: a new einfold-kernels crate whose first algorithm is the hash fold path over Arrow arrays. The raw…
Description check		The description follows the template. It has the SPDX/template comments, a 'What and why' section, and a 'How it is tested' section. It explains the purpose (a crate of EinFold algorithms over Arrow a…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "Add einfold-kernels, with EinFold's hash algorithm on Arrow arrays" names the main change: a new einfold-kernels crate whose first algorithm is the hash fold path over Arrow arrays. The raw summary confirms the new crate, the schema and fold entry points, and the hash implementation. The title is specific and accurate, and it is a single sentence of 9 words.

Full details: Description check

Explanation

The description follows the template. It has the SPDX/template comments, a 'What and why' section, and a 'How it is tested' section. It explains the purpose (a crate of EinFold algorithms over Arrow arrays with no query-engine dependency), the schema and fold API, the domain semantics, the Host float-key policy, and the invariants tested. The 'Closes #' issue link is left blank, and the template's AGENTS.md reference (identity line, issue link, design review) is only partly covered. The 'Closes #' line is an unfilled placeholder. The description also refers to other PRs and issues (#37, #38, #39, #41, #42, #43, #21) without giving the context needed to understand them. Because the core sections are complete, the check passes, but the author should fill in the issue link.

Full details: Docstring Coverage

Explanation

Docstring coverage is 56.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 62 functions across 6 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[alxmrs]

🪭 Claude (einfold)

A later idea for this hash path, from Haskell: discrimination instead of sorting and hashing. I'm not proposing it for this PR. It's worth a spike once the hash path matters for speed.

Today the hash path sorts each operand by its rows' contents (O(n log n)), only to make its order of additions deterministic, then builds and probes hash tables. Fritz Henglein's generic discrimination (ICFP 2008) sorts and partitions by distribution, a generalized radix sort, in worst-case linear time, with no comparisons and no hashing. Henglein and Larsen then built SQL-style equijoins on it, discrimination-based joins, with Cartesian products kept symbolic rather than formed. That is our fold over a join, and they found it competitive with SQL engines. Edward Kmett's Haskell package discrimination implements it: linear-time sorting, grouping and joins.

Why it fits:

• Determinism for free. A discriminator's output order depends only on the keys' values, so there's no separate sort for it.
• Already in the right form. Our keys are already Arrow's row format, byte strings that compare as the host compares keys, which is exactly what a byte-wise discriminator partitions.

Why it waits:

• With the host's own plan as the fallback (Detect folds and fills in DataFusion's plans #48), the hash path runs only for explicit FoldNodes and in tests.
• The first M1 hash kernel ran at 0.11–0.6× of DataFusion. A faster hash path is only worth having if it beats the host, measured.

So: a spike, timed against DataFusion's own hash join and aggregate on M1's shapes, before any change here.

• Henglein, Generic Discrimination: Sorting and Partitioning Unshared Data in Linear Time, ICFP 2008
• Henglein and Larsen, Generic multiset programming with discrimination-based joins and symbolic Cartesian products, Higher-Order and Symbolic Computation, 2011
• Kmett, discrimination

[alxmrs]

🪤 Claude (fuzz adversary)

No bugs found in the hash algorithm. Two searches ran it on inputs its own test doesn't draw:

• Add a simulation soak of einfold in DataFusion, run nightly #65 runs every fold that detection finds through einfold's kernels (DetectFolds::without_fallback), so the hash path sees detection's real plans: comma joins, CTE operands, UNION ALL, a fold of a fold, and domains. Keys come in eight types, including NaNs of both signs and two payloads, -0.0 beside 0.0, dictionary-encoded floats and text (some NULLs as a NULL dictionary value), and UInt64 keys past BIGINT. NULL slots hold random garbage values, and batches include empty ones. It matched DataFusion bit for bit with exact values, and within the summation bound with inexact ones.
• Test the dense algorithm at its block edges against an oracle #64 checks it against an independent nested-loop oracle, on folds moved off the dense path by one row that joins nothing.

I also checked by hand that NaN payloads and dictionary-encoded -0.0/0.0 match DataFusion's join and GROUP BY (FloatKeys::SignedZerosEqual leaving dictionaries alone is right for joins too, not only for grouping).

[alxmrs]

🪭 Claude (einfold)

A later idea for this hash path, from Haskell: discrimination instead of sorting and hashing. I'm not proposing it for this PR. It's worth a spike once the hash path matters for speed.

Today the hash path sorts each operand by its rows' contents (O(n log n)), only to make its order of additions deterministic, then builds and probes hash tables. Fritz Henglein's generic discrimination (ICFP 2008) sorts and partitions by distribution, a generalized radix sort, in worst-case linear time, with no comparisons and no hashing. Henglein and Larsen then built SQL-style equijoins on it, discrimination-based joins, with Cartesian products kept symbolic rather than formed. That is our fold over a join, and they found it competitive with SQL engines. Edward Kmett's Haskell package discrimination implements it: linear-time sorting, grouping and joins.

Why it fits:

• Determinism for free. A discriminator's output order depends only on the keys' values, so there's no separate sort for it.
• Already in the right form. Our keys are already Arrow's row format, byte strings that compare as the host compares keys, which is exactly what a byte-wise discriminator partitions.

Why it waits:

• With the host's own plan as the fallback (Detect folds and fills in DataFusion's plans #48), the hash path runs only for explicit FoldNodes and in tests.
• The first M1 hash kernel ran at 0.11–0.6× of DataFusion. A faster hash path is only worth having if it beats the host, measured.

So: a spike, timed against DataFusion's own hash join and aggregate on M1's shapes, before any change here.

• Henglein, Generic Discrimination: Sorting and Partitioning Unshared Data in Linear Time, ICFP 2008
• Henglein and Larsen, Generic multiset programming with discrimination-based joins and symbolic Cartesian products, Higher-Order and Symbolic Computation, 2011
• Kmett, discrimination

I would actually like to explore this path early! Before merging down the stack, let's try this alternative implementation and evaluate it. At the very least, maybe we could guard this by a flag (though, it sounds worthy of being our primary impl).

[alxmrs]

After learning more by reading PRs, I think it could be factored as another algorithm.

[alxmrs]

🪭 Claude (einfold)

Agreed, and as its own algorithm. Planning it showed a factoring I like: discrimination turns any keys into dense ranks. Henglein's discriminator partitions keys by their bytes, a radix pass per byte with no comparisons and no hashing, so it can give each distinct key a rank, 0..k, in key order, in linear time. With ranks in place of keys:

• dense data runs the dense algorithm (Add EinFold's dense algorithm to einfold-kernels #45), which today needs keys that are already small non-negative integers. Text, float, dictionary and sparse integer keys would all qualify;
• sparse data runs Gustavson's algorithm over rows bucketed by rank with a counting sort, still with no hashing;
• the order of additions is fixed by the keys' values, so the hash path's O(n log n) sort for determinism goes away.

Keys are already in Arrow's row format, byte strings that compare as the host compares keys, which is what a byte-wise discriminator needs.

Plan: spike S30, on main as spikes go. It times discrimination plus dense or Gustavson against einfold's hash path and against DataFusion, on M1's shapes with text keys, sparse integer keys and dense integer keys, at n = 10,000 and 50,000, from Arrow rows to Arrow rows. If it wins, it becomes Algorithm::Discriminate in a PR on this stack, before you merge it.

[alxmrs]

🪭 Claude (einfold)

Spike S30 is done (report, on main). Discrimination is worth it.

It ranks each dimension's keys by radix discrimination: integers as order-preserving u64s, other keys in Arrow's row format. Then it runs Gustavson's product over buckets made by counting sorts. It's timed against einfold_kernels::fold at #57 and against DataFusion, from Arrow rows to Arrow rows, on 12 threads:

• against the hash path: 9–34× faster everywhere. 1.6 s becomes 51 ms on the matrix product with sparse integer keys at n = 50,000;
• against DataFusion: 1.1–2.7× faster in 11 of 12 cases, with text, sparse and dense integer keys, on ddx's n × 16 · 16 × 8 product and on a graph-like sparse product with 3.2 million output rows. The exception is the matrix product with sparse integer keys at 50,000: 51–56 ms against 41–49;
• against the dense algorithm: 3× slower on dense integer keys, so dense stays first where it applies.

What I'd propose for the stack, before you merge it: Algorithm::Discriminate in einfold-kernels, replacing the hash algorithm. That's one PR on top of #44, or stacked after #57 if you'd rather not rewrite under the PRs you've reviewed. It needs what the spike skipped:

• NULL keys under = and IS NOT DISTINCT FROM;
• the host's comparison of -0.0 and 0.0;
• domains, and a deterministic tiebreak for duplicate key tuples.

kernels_vs_sql.rs and #64's metamorphic test already cover all of those, so they'd check it as it is.

One choice is yours: once discrimination beats the host on a fold, should EinFoldExec run it rather than the host's plan where the dense algorithm declines? That would move the fallback from "always the host" to "the host only where we haven't measured a win". On these shapes it would be 1.1–2.7×, but not on every shape, as the one exception shows.

[alxmrs]

🪭 Claude (einfold)

Decision: discrimination is the default where the dense algorithm declines (now on #68, with the design updated on #42). Per your note, I surveyed the queries einfold is meant to serve, then chose.

Where a fold appears that the dense algorithm can't take, it's almost always a sparse product on integer ids, the shape where S30 measured discrimination beating DataFusion 1.3–1.9×, with integer and text keys alike:

• xarray-sql's geospatial cases (the page). Case 08, regridding, is SUM(s.value * w.weight) over a sparse weight table on integer cell ids. Case 09, warp, is the same weight join on coordinate keys. Cases 01–07 have no fold: per-row arithmetic, single-table GROUP BYs, a range join, and AVG(POWER(…)).
• GraphCast (xarray-sql#270, demo 3). Message passing is node features joined to edge tables on integer node ids, 100k–200k edges, summed per receiver. The MLPs are dense integer matrix products, which the dense algorithm takes.
• NanoGPT (xarray-sql#269, demo 2). The embedding gradients are GROUP BY token over a batch, sparse on integer ids. The rest is dense matrix products and attention, which are the dense and fused algorithms.
• ddx's gradients: dense, or joined back to their domain.

The shape where discrimination lost, a matrix product whose keys are sparse integers or text, doesn't appear in these queries. The closest is the warp's two-column coordinate keys. Those take discrimination's byte-by-byte path, so ranking each column separately is the next improvement there.

What stays on the host's plan: a fold joined back to its domain, where only the host's plan is right if the domain's keys turn out not to be unique; and pipelines, which have no general algorithm yet. DetectFolds::with_host_fallback() gives back the old behavior, never slower than the host's plan, for anyone who wants it.

[alxmrs]

LGTM