trilean-sql: compile some/every/fold(max|min) over a correlated table - #48
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…rences SqlCollectionBinding maps a some/every/fold node's collection key onto a correlated child table (table, join, and a per-collection columnFor for references inside item/filter), the collection-level counterpart of the existing columnFor mapping for a plain reference. Left unset, a tree using one of those kinds is refused exactly as before; this option is what a caller opts into to have it pushed down instead.
…onFor table Mirrors InvalidColumnError for the correlated table a some/every/fold node compiles against: a table name is an identifier that has to be written into the statement text rather than bound as a parameter, so this rejects the two shapes quoting cannot rescue, an empty name and an empty dot-separated segment.
…ity guard findUnpushablePredicate's some/every case and findUnpushableExpression's fold case now resolve the collection via a new resolveCollectionForGuard helper instead of refusing outright: a string collection key with collectionFor set recurses into item/filter (or combiner.item) using the resolved binding's own columnFor, exactly mirroring how the evaluator re-points its EvaluationContext at the collection item. Without options, or without collectionFor set, the refusal is unchanged from before. fold(reduce) is refused unconditionally regardless of collectionFor: it threads an arbitrary combine expression through the collection in a caller-chosen order, which has no general SQL translation. fold(max|min) additionally refuses a projected item whose static kind is text or boolean. SQL's MAX/MIN would happily order text lexicographically or booleans as 0/1 the moment two or more rows participate, where trilean's own compareValues refuses to order either past a single item -- a divergence a fixed pair of operands could prove but a collection's real cardinality, only known at query time, cannot rule out, so it is refused unconditionally rather than only when two or more rows are proven to participate. findUnpushableExpression now takes the compile options as a parameter (previously it took none at all), threaded through its four existing call sites, so a fold buried inside a comparison, textCompare, memberOf, or exists operand can resolve its own collection correctly.
Resolves the collection through the new compileCollection helper (quoting the correlated table via a new quoteTable, mirroring quoteColumn) and compiles item/filter against it, then aggregates each participating row's own vote with MAX/CASE to match the evaluator's OR/AND-fold absorption exactly: some is true the moment any row's item is true regardless of another row's indeterminacy, indeterminate only once no row voted true and at least one participated indeterminately, false otherwise -- every is the mirror image. item and filter are each compiled exactly once, never inlined twice, both for correctness (a NULL-propagating expression's meaning would otherwise differ between occurrences) and because a dialect's own bare placeholder cannot safely be reused. collectionFor is memoised for the duration of one compilation the same way columnFor already is, including a nested per-binding memoisation of each resolved binding's own columnFor. fold still refuses unconditionally for now, via the same safety net that already covers a guard/compiler allow-list mismatch elsewhere in this file -- its own compilation follows in the next commit.
Aggregates the projected item across the correlated collection with MAX/MIN directly, going NULL (indeterminate) the moment any participating row's filter or projected value is itself indeterminate -- matching the evaluator's own short-circuit for fold, which unlike some/every's OR/AND has no absorbing value at all. An empty participating set falls out of the same aggregate for free: MAX/MIN over zero rows is NULL in SQL, the same indeterminate result the evaluator reaches by its own separate domain-error path for a fold with nothing to seed a running extremum from. fold(reduce) stays refused unconditionally, with or without collectionFor set: it threads an arbitrary combine expression through the collection in a caller-chosen order, which has no general SQL translation.
…nslation Adds a Collections section covering SqlCollectionBinding, the some/every vote-aggregation and fold max/min aggregation each compile to, and the fold(reduce)-is-always-refused and text/boolean-ordering refusal cases. Moves some, every, and fold(max|min) from the refused table to the compiles table, and updates the API reference and error list to match.
Unit level: relocates some/every/fold(max|min) out of the unconditional refusal lists into a dedicated guard.test.ts describe block covering the collectionFor-absent/collectionFor-present split, the non-string collection key, filter/item resolving against the collection's own columnFor rather than the outer one, and the new text/boolean fold ordering refusal; compile.test.ts asserts the exact compiled SQL text for some/every (with and without a filter, and a conjunctive item locking in the single-witness-row shape) and fold max/min, plus fold(reduce)'s unconditional refusal. Integration level: adds a shared subject_tags correlated-table fixture (src/test-support/columns.ts) and a matching schema/seed/resolveCollection extension to all three integration suites, seeded to exercise an empty collection, a subject where every tag passes a threshold, one where only some do, one pairing a clean vote with an unknown-weight tag, and one whose sole tag has an unknown weight -- then runs some/every/fold trees against a real connection in each dialect and checks the matched rows agree with evaluatePredicate exactly, including the and-over-range hazard a naive per-column translation (rather than one combined boolean per row) would get wrong.
…r the 800-line cap compile.test.ts had grown to 954 lines of real code, over the new max-lines cap. Splits it by tested concern into four files (connectives through exists, some/every/fold, parameters through refusal, and the per-dialect and unimplemented-dialect behaviour), each comfortably under the cap. Extracts the compile() wrapper and the shared age fixtures (ADULT_AGE, ageOver, and their siblings) -- used across most of the original file rather than confined to one topic -- into a new compile-test-helpers.ts every split file imports from.
…the 800-line cap guard.test.ts had grown to 896 lines, over the new max-lines cap. Splits it by tested concern into four files (supported/unsupported node kinds, quantification over a collection, references and cross-dialect operand handling, and SQLite/PostgreSQL regex-pushdown dialect behaviour), each comfortably under the cap. Extracts the shared ageOver fixture -- used across the first two split files -- into a new guard-test-helpers.ts both import from.
… the 800-line cap test/integration/sqlite.test.ts had grown to 1003 lines, over the new max-lines cap. Splits it by tested concern into three files (predicate compilation, degenerate/adversarial fragments and the divergences the guard's refusals exist to prevent, and the some/every/fold correlated- collection suite), each comfortably under the cap. Extracts the shared in-memory database lifecycle -- schema, seeded rows, the resolver bridging a row to the evaluator's own notion of "known", and the agreeingRows comparison every case is built on -- into a new sqlite-test-support.ts all three split files import from. Each split file still gets its own isolated vitest module instance, so each opens and closes its own connection via that shared beforeAll/afterAll exactly as the unsplit file did for itself.
…er the 800-line cap test/integration/postgres.test.ts had grown to 829 lines, over the new max-lines cap. Splits it by tested concern into three files (predicate compilation, degenerate/adversarial fragments, and the some/every/fold correlated-collection suite), each comfortably under the cap. Extracts the shared container lifecycle -- schema, seeded rows, the resolver bridging a row to the evaluator's own notion of "known", and the agreeingRows comparison every case is built on -- into a new postgres-test-support.ts all three split files import from. Each split file still gets its own isolated vitest module instance, so each starts, seeds, and stops its own container via that shared beforeAll/afterAll exactly as the unsplit file did for itself. Kept a genuinely independent structural copy from the PGlite suite's own support module, per that suite's own stated design intent: a shared harness parameterised over both engines would make them agree by construction, which is the one thing this parity suite exists to avoid.
… the 800-line cap test/integration/pglite.test.ts had grown to 825 lines, over the new max-lines cap. Splits it by tested concern into three files (predicate compilation, degenerate/adversarial fragments, and the some/every/fold correlated-collection suite), each comfortably under the cap. Extracts the shared in-process database lifecycle -- schema, seeded rows, the resolver bridging a row to the evaluator's own notion of "known", and the agreeingRows comparison every case is built on -- into a new pglite-test-support.ts all three split files import from. Each split file still gets its own isolated vitest module instance, so each opens and closes its own PGlite instance via that shared beforeAll/afterAll exactly as the unsplit file did for itself. Kept a genuinely independent structural copy from the container-backed suite's own support module, per that suite's own stated design intent: a shared harness parameterised over both engines would make them agree by construction, which is the one thing this parity suite exists to avoid.
Mearman
force-pushed
the
feat/trilean-sql-quantifiers
branch
from
September 15, 2026 15:57
c8d6677 to
4952b3e
Compare
|
🎉 This PR is included in trilean-sql@2.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #47.
Adds a
collectionForoption (alongside the existingcolumnFor) that maps asome/every/foldnode'scollectionkey onto a correlated table --{ table, join, columnFor }, wherejoinis caller-authored raw SQL relating one row of the table to the outer row (a plain FK, a composite key, or an FK plus a literal discriminator for the EAV case the linked issue is actually about) and the collection-levelcolumnForresolves references inside that collection's ownitem/filter.some/everycompile to a correlated subquery that aggregates each participating row's own vote withMAX/CASE, built to match the evaluator's OR/AND-fold absorption exactly rather than a plainEXISTS/NOT EXISTS-- a naive two-valuedEXISTStranslation would collapse SQL's NULL handling into boolean logic and disagree withevaluatePredicatethe moment a row's filter or item is itself indeterminate.fold(max|min)compiles the same correlated subquery aggregated withMAX/MIN, goingNULLthe instant any participating row is indeterminate.fold(reduce)stays refused -- it threads an arbitrary combine expression through the collection in whatever order the caller's own resolver returns items, which has no general SQL translation.One thing not in the original issue write-up:
fold(max|min)also refuses a projected item that's statically known to be text or boolean. trilean's owncompareValuesrefuses to order either past a single item, but SQL'sMAX/MINwill happily order text lexicographically or booleans as 0/1 the moment two or more rows participate -- and since a collection's real row count is only known at query time, there's no way to prove in advance that only one row will ever show up, so it's refused unconditionally rather than only when a fixed pair of operands proves the mismatch (the same thingcomparealready does for a booleangt/lt).Every case is measured against a real connection, not just asserted as a string: added a shared
subject_tagsfixture across all three integration suites (Postgres via testcontainers, PGlite, better-sqlite3) seeded to cover an empty collection, a subject where every tag passes a threshold, one where only some do, one pairing a clean vote with a tag whose weight is unknown, and one whose sole tag has an unknown weight -- then run the compiled SQL andevaluatePredicateside by side and check they agree row for row, including the and-over-range case a naive per-column split (rather than one combined boolean per row) would get wrong.prepublishOnlyis green (lint, typecheck, unit + integration across all three dialects, build, publint, attw).