Sql support#5
Open
llambeau wants to merge 43 commits into
Open
Conversation
Captures architecture decisions for bringing SQL support to bmg.js: monorepo with separate packages, standalone predicate library, typed AST, async-only SqlRelation, cursor streaming, Docker-based integration tests. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Move core package to packages/bmg/. Set up pnpm-workspace.yaml with root-level test/build scripts that delegate to all packages. Replace microbundle with tsup for building — fixes a pre-existing build failure where microbundle's bundled babel couldn't parse `export type` syntax with newer @babel/parser versions. Update CI workflow, CLAUDE.md, and package exports for new structure. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Standalone predicate library with:
- Typed AST (discriminated unions): eq, neq, lt, lte, gt, gte, in,
and, or, not, tautology, contradiction
- Pred builder helpers for ergonomic construction
- evaluate() — run predicates against JS objects
- toSql() — compile to parameterized SQL (Postgres and SQLite dialects)
- isPredicate() type guard for runtime detection
- fromObject() to convert { key: value } to conjunction of equalities
86 tests covering builder, evaluation, SQL compilation, and type guards.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The Predicate type now accepts structured predicate AST nodes alongside plain objects and functions. toPredicateFunc() detects structured predicates via isPredicate() and evaluates them using evaluate(). This prepares the ground for SqlRelation which will compile structured predicates to SQL rather than evaluating them in memory. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
On CI, the predicate package dist/ doesn't exist when bmg tests run (no build step before tests). Add a vitest alias so bmg resolves @enspirit/predicate directly from TypeScript source. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Phase 2 of the SQL support roadmap. Provides: - Typed SQL AST (discriminated unions): SelectExpr, UnionExpr, ExceptExpr, IntersectExpr, with JOINs, GROUP BY, ORDER BY, LIMIT/OFFSET, aggregates - SqlBuilder for ergonomic AST construction with auto-generated table qualifiers (t1, t2, ...) and fromSelf() subquery wrapping - compile() — AST to parameterized SQL with Postgres and SQLite dialects - compilePredicate() — compiles @enspirit/predicate AST nodes to SQL with qualified column names (supports dotted pre-qualified refs) 38 tests covering all SQL constructs: SELECT, DISTINCT, WHERE, GROUP BY, ORDER BY, LIMIT/OFFSET, JOINs (INNER/LEFT/CROSS), set operations (UNION/EXCEPT/INTERSECT), subqueries, aliases, aggregates, literals. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Phase 3: processor functions that rewrite the SQL AST, mirroring the Ruby bmg Processor classes: - processWhere — add/merge WHERE clause (wraps in subquery for GROUP BY) - processProject / processAllbut — clip select list + add DISTINCT - processRename — change column aliases in select list - processExtend — add columns referencing existing columns - processConstants — add literal value columns - processRequalify — regenerate table qualifiers for binary ops - processJoin — INNER/LEFT JOIN with select list and WHERE merging - processMerge — UNION/EXCEPT/INTERSECT 23 new tests covering each processor and chained operations. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Phase 4 core: SqlRelation<T> implements AsyncRelation<T> backed by SQL. - DatabaseAdapter interface with query(), stream(), and dialect - BmgSql<T>(adapter, table, attrs?) factory for creating SQL relations - SqlRelation pushes to SQL: restrict, project, allbut, rename, extend, constants, union, minus, intersect (when both operands share adapter) - Falls back to BaseAsyncRelation (in-memory) for: function predicates, prefix, suffix, transform, group, ungroup, wrap, unwrap, image, summarize, matching, not_matching, cross_product - toSql() for SQL inspection without executing - Cursor-based async iteration via adapter.stream() 24 new tests with mock adapter verifying SQL push-down, fallback, chaining, and terminal operations. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
PostgresAdapter wraps the pg (node-postgres) library and implements
DatabaseAdapter:
- query() for batch execution via Pool.query()
- stream() for cursor-based streaming via DECLARE CURSOR / FETCH
- close() to shut down the connection pool
- Uses PostgresDialect for $1 parameterized queries
Also re-exports BmgSql and SqlRelation for convenience so users
only need: import { PostgresAdapter, BmgSql } from '@enspirit/bmg-pg'
4 unit tests with mock pool.
Docker-based integration tests deferred to a later phase.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Imran-imtiaz48
left a comment
Imran-imtiaz48
left a comment
There was a problem hiding this comment.
The implementation of PostgresAdapter is generally clean and aligns well with the DatabaseAdapter interface. The constructor logic correctly supports both an existing Pool instance and a PoolConfig, which makes the adapter flexible for different integration scenarios. The query() method is straightforward and properly delegates execution to pool.query, returning typed rows, which keeps the abstraction simple and consistent. The stream() implementation using PostgreSQL cursors is a solid approach for handling large result sets without loading everything into memory, and the use of AsyncIterable integrates nicely with modern JavaScript async iteration patterns. Resource management is handled carefully through the cleanup() function, ensuring that cursors are closed, transactions are committed, and clients are released back to the pool even when errors occur. However, there are a few areas that could be improved. The declareSQL conditional inside init() currently produces the same SQL in both branches, making the condition unnecessary and potentially confusing. Error handling could also be strengthened, particularly around transaction management, where a ROLLBACK might be safer than COMMIT if an error occurs during streaming. Additionally, the cursor naming strategy using Date.now() and Math.random() is acceptable but not guaranteed to avoid collisions in highly concurrent environments. The tests cover basic functionality such as dialect assignment, query delegation, parameter passing, and pool shutdown, but they do not validate the streaming behavior, which is the most complex part of the adapter. Adding unit tests or integration tests for cursor-based streaming would significantly improve confidence in the implementation. Overall, the code structure is clear, readable, and maintainable, but tightening error handling, removing redundant logic, and expanding test coverage would make the adapter more robust.
21 integration tests in packages/bmg-pg/tests/integration/ that exercise the full stack against a real Postgres: - Basic read, one() - restrict: plain object, Pred.eq/gt/and/in, chained - project, allbut, rename, constants - union, minus, intersect (same-adapter set ops) - join, left_join (with suppliers + shipments tables) - Fallback: function predicate → in-memory, correct results - toSql() inspection - Cursor-based async iteration - Chained: restrict → project → rename → toArray No gating — tests fail hard if Postgres is unavailable. Separated from unit tests via vitest config: - `pnpm test` runs unit tests only (excludes integration/) - `pnpm test:integration` runs integration tests only docker-compose.yml at root for local dev. CI workflow adds a separate integration job with Postgres service container. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…fset Phase 5: additional processors and SqlRelation wiring. New processors: - processSummarize — GROUP BY + aggregates (count, sum, avg, min, max) - processSemiJoin — matching/not_matching via EXISTS/NOT EXISTS subquery - processOrderBy — ORDER BY clause - processLimitOffset — LIMIT/OFFSET clause SqlRelation now pushes to SQL: - summarize() when all aggregators are SQL-compilable - matching()/not_matching() with array keys on same-adapter relations Compiler extended with EXISTS subquery support for semi-joins. 9 new processor unit tests, 4 new integration tests (summarize, matching, not_matching against real Postgres). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Phase 6: Runtime type system.
- RelationType tracks attribute names and candidate keys at runtime
- project/allbut propagate type info through operator chains
- processProject/processAllbut skip DISTINCT when a key is preserved
- BmgSql() accepts optional { keys: [['sid']] } for key declaration
Type propagation through: restrict (same type), project, allbut,
rename, extend, join, summarize.
15 new RelationType unit tests, 5 new DISTINCT optimization tests
verifying key-aware behavior end-to-end.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Rewrite README to cover: - Package overview table (bmg, predicate, bmg-sql, bmg-pg) - In-memory quick start (unchanged) - PostgreSQL quick start with full working example - Structured predicates (Pred.eq, Pred.and, Pred.in, etc.) - SQL push-down vs transparent fallback explanation - Joins and set operations across SQL relations - Aggregation via summarize - Development commands (test, integration, build) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
|
@llambeau interesting. A few major comments: 1/ I wonder whether using an external SQL-AST lib, e.g. https://github.com/tobilg/polyglot or https://github.com/taozhi8833998/node-sql-parser, would not be a better strategy here 2/ RelationType must then be moved to core, like bmg-rb ; it would then provide the foundation for optimization even with Memory-relations 3/ We most certainly need to (im)port black-box SQL tests from bmg-rb for some guard rails on Claude. |
Prepares packages/bmg-sql/tests/black-box/ with README, INDEX, and 19 per-operator tracking docs (one per source YAML in bmg-rb spec/integration/sequel/base/). All 89 cases transcribed from upstream SHA fa8c7e0 with description, original Ruby, expected SQLite, and warnings flagging unsupported operators and API divergences. No tests ported yet — this is the tracking base for the porting work. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Self-directed plan: one operator per iteration, one commit per iteration, simple-to-complex order (14 supported operators, 5 blocked). Includes loop steps, fixture spec, SQL normalizer spec, type-level assertion rule for type-reshaping operators, and stop conditions. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
First iteration of the porting loop. Adds shared helpers (mock-adapter.ts, fixtures.ts) and base.test.ts with the one portable case (base.01). Case base.02 (subquery source) is blocked pending a raw-SQL relation factory. Also revises CLAUDE_PLAN.md: drops the SQL normalizer in favor of asserting bmg-sql's native output directly with .toBe(), keeping bmg-rb's SQL in the .md tracking docs as the semantic reference. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Single case: column-aliasing extend pushed down to SQL as
`<col> AS <alias>`. Uses bmg-sql's string-reference form
(`{ supplier_id: 'sid' }`).
Type quirk flagged in extend.md: string-ref form infers the
literal type ('sid') for the new attribute instead of the
underlying column's value type. Runtime is correct; only the
static type is imprecise. Test documents current behavior via
expectTypeOf.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Covers three variants: - Project preserving key (no DISTINCT) - Project dropping key (DISTINCT emitted) - allbut then project (DISTINCT propagates) RelationType-driven DISTINCT optimization verified end-to-end. No bmg-sql source changes. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
union.01 (self-union) and union.02 (chained) ported. bmg-sql wraps operands in parens and left-nests chains — different syntax, same query. union.03 (UNION ALL via `all: true`) blocked: processMerge accepts the flag but Relation.union() in bmg core has no options argument. Exposing it requires cross-package API changes (core types + Memory + async Base + SqlRelation). Flagged for user review. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Self-minus, chained minus, and minus+summarize all ported. Divergence: bmg-sql wraps the EXCEPT as a derived table (subquery in FROM) rather than a CTE (WITH t2 AS ...). Same query semantically — engines plan them identically. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
All four cases ported including the duplicate in the upstream YAML. Adds the pattern for asserting both `sql` and `params` when the case involves restrict literals — bmg-sql parameterizes (uses `?` placeholder) where bmg-rb inlines. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
allbut.01-03 and allbut.05 match bmg-rb semantically. allbut.05 uses Pred.or from @enspirit/predicate for OR-restrict. allbut.04 divergent: bmg-sql emits DISTINCT where bmg-rb does not. bmg-rb recognizes that restrict on key-prefix (`sid='S1'` on supplies keyed by [sid,pid]) functionally restores uniqueness via pid, so allbut(sid) is safe without DISTINCT. bmg-sql's RelationType only tracks whole-key preservation — fixing this needs functional-dependency tracking via restrict equalities. Same result set, redundant DISTINCT only. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Anti-join via NOT EXISTS. Cases .01-.03 ported. Case .04 (against a raw-SQL relation) blocked on subquery factory. bmg-sql emits `SELECT 1 AS "_exists"` inside EXISTS where bmg-rb uses `SELECT *`, and adds vacuous `WHERE 1=1` in the empty-key case. Semantically equivalent for EXISTS evaluation. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Semi-join via EXISTS. Cases .01-.05 match bmg-rb semantically.
matching.06 surfaced a real bmg-sql bug: when the right-side of
matching is itself a join, the inner JOIN ... ON clause emits
wrong aliases ("t1"."pid" = "t1"."pid" instead of
"t7"."pid" = "t8"."pid"). Query returns incorrect results
against a real DB. Flagged via it.fails() with the CORRECT
expected SQL — when fixed in processSemiJoin, the test will
invert and alert us.
matching.07 (native SQL relation) blocked on subquery factory.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
All 8 left_join cases blocked on a cross-cutting bmg-sql bug surfaced during this iteration: buildJoinPredicate resolves both operand aliases via each relation's own SqlBuilder, so both start at `t1`. processJoin renames the right side's table via processRequalify (e.g. → `t2`), but the already-built ON predicate isn't rewritten — it still references `t1` on both sides, producing e.g. `ON "t1"."sid" = "t1"."sid"`. Scope: affects `join`, `left_join`, and matching-against-a-join (i.e. matching.06). Query results are incorrect on a real DB. Fix requires API reshape in processJoin (pass keys in and build the predicate post-requalify, or return old→new qualifier map and rewrite the predicate). Non-trivial — stop condition #3 triggered. Stopping loop per CLAUDE_PLAN.md; awaiting user decision on direction. Test file has all 8 cases as it.todo with clear blocker notes. Tracking: left_join.md "Blocker" section, INDEX.md "New bmg-sql bug surfaced by porting", CLAUDE_PLAN.md Stopped? flag set. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
WHERE push-down works for the simple cases (.01, .02, .06). Porting surfaced four distinct gaps in bmg-sql / @enspirit/predicate: - NULL-in-IN (.03-.05): @enspirit/predicate's toSql emits IN (?, NULL, ?) with a NULL param, which silently drops NULL rows in standard SQL. bmg-rb splits NULL out via OR and collapses single-value / single-NULL IN lists. Fix belongs in the predicate package. - Alias-in-WHERE (.07): restrict after rename leaves the predicate on the SELECT alias (firstname) instead of resolving back to the underlying column (name). Emits invalid SQL in Postgres/standard SQL. Needs rename-aware processWhere. - UNION push-down (.10, .11): post-union restrict wraps the UNION in a derived table rather than pushing into each branch. Same result, different shape. - Predicate.match (.08, .09): blocked — LIKE predicate not implemented in @enspirit/predicate. All divergent cases are snapshotted, so future fixes will flip the assertions and alert us. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Ported .01-.04, .06, .08. GROUP BY + standard aggregates push down
cleanly, and the post-summarize restrict wrap (.06) uses a derived
table — same precedent as minus.03 (bmg-rb uses WITH; same semantics).
Settled the bmg-rb `:qty => :sum` convention: bmg-js equivalent is the
verbose AggregatorSpec `{ qty: { op: 'sum', attr: 'qty' } }`. The
short form `{ qty: 'sum' }` produces SUM(*) in bmg-sql — not the same
thing. Documented at the top of summarize.test.ts.
summarize.05 surfaces the same join-alias bug as matching.06: the
join predicate compiles to `t1.sid = t1.sid`. Marked it.fails
with the correct expected SQL so the fix alerts us.
summarize.07 is blocked (it.todo): three layered bugs in the same
processJoin / processRequalify family — the join-alias bug fires, the
inner subquery is not requalified (inner `t1` collides with outer `t1`),
and the outer WHERE references the wrong alias. Not expressible as a
single correct-SQL snapshot without fixing the processor family.
summarize.09 and .10 are blocked: `distinct_count` is not in bmg
core's AggregatorName union nor in bmg-sql's compilableOps allowlist,
even though the SQL AST already emits COUNT(DISTINCT x). Unblocking
is a cross-package change.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
All 4 cases blocked at the type-system level. bmg core's Transformation type is strictly JS-function-based (TransformFunc = (value: unknown) => unknown), with no type-token channel comparable to bmg-rb's class literals (String, Integer, Date). bmg-sql's SqlRelation.transform() just falls back to in-memory — there is no processTransform and no CAST emission in the AST/compiler. Unblocking is a cross-package feature, not a test-porting fix: 1. Typed-token extension to core's Transformation (shape TBD). 2. processTransform in bmg-sql emitting CAST(col AS t) and the date(col) special case for transform.04. 3. Dialect hook for the varchar spelling. Test file follows the left_join.test.ts precedent — a describe block with it.todo per case plus an explanatory comment keeping the blocker visible on the tracker. This completes option (b) from CLAUDE_PLAN: all non-join operators iterated. Remaining gap is the cross-cutting join-alias bug blocking left_join (0/8), join (0/14), matching.06, summarize.05, summarize.07. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Option (b) is exhausted: all non-join operators iterated (43/89 cases ported, 7 divergent, 2 known-bug, 24 blocked). Loop paused pending the join-alias refactor in processJoin / processRequalify — belongs on its own branch with its own plan, not bundled here. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SQL `IN (NULL, ...)` does not match NULL rows (NULL IN NULL is
UNKNOWN), so `restrict(Pred.in('col', [null, ...]))` was silently
dropping NULL rows when compiled to SQL — a correctness bug, not
just a divergence from bmg-rb.
Fix partitions nulls out of InPredicate.values:
- all-null list → `col IS NULL`
- mixed list → `(col IS NULL OR col IN (non-null values))`
- no-null list → unchanged `col IN (values)`
The combined form is parenthesized so it binds correctly inside AND
and NOT (kind is still 'in', so the caller's kind-based paren logic
would otherwise miss it).
Landed in two places — predicate compilation is duplicated across
@enspirit/predicate's toSql.ts and bmg-sql's compile.ts. Both get
the same fix; a future cleanup could consolidate.
Black-box test flips:
- restrict.03: divergent → ported
- restrict.04: divergent → ported (minor cosmetic: bmg-sql keeps
`IN (?)` for single value; bmg-rb degrades to `=`. Same query.)
- restrict.05: divergent → ported (`IS NULL` collapse)
Adds 7 new predicate toSql tests covering null/mixed/AND/NOT cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
bmg core: - AggregatorName += 'distinct_count' (types.ts, lib-definitions.ts) - AggregatorResult routes distinct_count to `number` alongside count - applyAggregator (sync + async) implements distinct_count as Set size over non-null values of the attribute bmg-sql: - compilableOps += 'distinct_count' in SqlRelation.summarize() - AST & compile.ts already knew how to emit COUNT(DISTINCT col) Black-box flips: - summarize.09: todo → ported (verbose form, result named qty) - summarize.10: todo → ported (result named count) CHANGELOG updated — user-facing addition. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds Relation.page(ordering, pageNumber, { pageSize? }) to:
- bmg core Relation<T> interface (types.ts + lib-definitions.ts)
- MemoryRelation (sync) via new sync/operators/page.ts
- AsyncRelation interface + BaseAsyncRelation (async/types.ts +
async/Relation/Base.ts + async/operators/page.ts)
- SqlRelation, wiring to existing processOrderBy + processLimitOffset
(no new processor needed)
Default pageSize = 20; pages are 1-indexed. Ordering accepts either
a bare attribute name (ASC) or a [name, 'asc' | 'desc'] tuple:
r.page(['name', 'sid'], 1, { pageSize: 2 })
r.page([['status', 'desc'], 'name'], 2)
processOrderBy change: wrap complex SELECTs (GROUP BY, LIMIT,
computed columns) in a subquery before applying ORDER BY. Mirrors
processWhere's isComplex wrap. Without this, summarize().page()
would emit `ORDER BY "t1"."qty"` after `MAX("t1"."qty") AS "qty"`,
resolving to the raw column (invalid in most dialects after GROUP
BY) instead of the aggregate alias. bmg-rb uses a CTE wrap for the
same reason — we use a derived table (same precedent as minus.03
and summarize.06).
Black-box flips:
- page.01 through page.05: blocked → ported
- page.04 verifies that findColumnRef resolves the renamed alias
back to the underlying column in ORDER BY (ORDER BY "t1"."sid"
not "t1"."id"), matching bmg-rb's push-down
5 sync in-memory tests + 5 black-box SQL tests added. CHANGELOG
updated (new user-facing operator).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds a new AST node `RawSubqueryRef` (opaque SQL fragment + alias +
optional bind params) and a `BmgSql.fromSubquery` factory that
builds a SELECT projecting the given attrs from that subquery:
BmgSql.fromSubquery<T>(
adapter,
'SELECT sid FROM suppliers WHERE city = ?',
['sid'],
{ params: ['London'] },
)
The fragment is embedded verbatim, aliased as t1, and composes with
every existing operator (restrict, matching, not_matching, join,
summarize, etc.). Params are bound in the order they appear in the
compiled SQL.
Wired the new TableSpec variant through:
- compile.ts: compileTableSpec + getTableAlias
- processors.ts: getAlias + requalifyTableSpec
- index.ts: export RawSubqueryRef
Black-box flips (3 cases):
- base.02: blocked → ported
- matching.07: blocked → ported (with 'London' bind param)
- not_matching.04: blocked → ported
fixtures.ts helper now exposes the shared `adapter` so
fromSubquery-built relations stay compatible with the fixture
relations (same adapter instance check in extractCompatibleExpr).
CHANGELOG updated — user-facing addition on bmg-sql.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The plan was stale in several places after A→D landed: - Current state totals conflicted (43 in one bullet, 53 in another) - "Blocked" section still listed page, base.02, matching.07, not_matching.04 as blocked even though unblockers C and D resolved them - Operator-order checklist had stale case counts (not_matching 3/4, matching 5/7, summarize 7/10, base 1/2) - Completion notes was still a placeholder Changes: - Current state: single consistent totals block (53/89, 30 blocked via it.todo) - New "Unblocker pass (A→D)" section capturing the four commits, the explicit relaxation of stop conditions #3 / #5, and the precedent this sets for future bounded unblocker work - New "Next up" section ranking remaining work by effort × value, with the join-alias refactor called out as the single biggest lever (~25 cases) - Updated operator-order checkboxes, plus a new "Operators added outside the original 14" subsection for `page` - "Blocked" section split into "Still blocked" and "Resolved by the unblocker pass" - Completion notes filled in with final tally and pointers forward Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Pre-existing bug: buildJoinPredicate (in relation.ts) built the ON predicate using each relation's own SqlBuilder, so both operands started at t1. processRequalify then renamed the right side's tables but not the already-built on predicate, producing ON t1.sid = t1.sid — wrong results on a real DB. Blocked ~25 black-box cases across join.*, left_join.*, matching.06, summarize.05/.07. Three scoped fixes, co-located in the processor family: 1. processJoin(left, right, keys, kind, builder) now takes keys instead of a pre-built predicate and builds ON *after* processRequalify via a new buildJoinOn helper. buildJoinOn resolves each key's qualifier from the respective select list, so multi-way joins (a.join(b, [x]).join(c, [y])) emit correct refs even when y lives on a nested table. 2. requalifyTableSpec for inner_join / left_join now also requalifies the on predicate via a new requalifyPredicate walk that splits AttrRef names on '.' and remaps the qualifier prefix. Fixes nested-join-under-EXISTS (matching.06) and layered-summarize joins (summarize.07). 3. buildSelectQualifier (compile.ts) now looks up each attr's qualifier in the SELECT list, falling back to the left table alias only if absent. Fixes WHERE predicates that reference right-side join columns (summarize.07, left_join.07, and the general restrict-after-join pattern). Removed now-unused helpers from SqlRelation: buildJoinPredicate, getLeftAlias, getRightAlias, getSpecAlias. Ports: - join.test.ts: new file with 14 cases (10 ported, 4 blocked on prefix push-down / CROSS JOIN push-down) - left_join.test.ts: rewrote from stubs to 8 cases (6 ported, 2 blocked on defaults/COALESCE API) - matching.06: flipped from it.fails to ported - summarize.05: flipped from it.fails to ported - summarize.07: flipped from it.todo to ported Net: +19 ported cases (53 → 72/89). Tracking docs (INDEX.md, per-file .md, CLAUDE_PLAN.md) refreshed to reflect the unblock. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SqlRelation.prefix() and .suffix() now compile to SQL instead of falling back to in-memory. Implementation is a thin layer over rename: when relType is known, build a renaming map covering every attr (respecting except) and delegate to this.rename(). While porting join.02 / join.10 (which combine rename + prefix), a pre-existing bug surfaced: JOIN ON and WHERE clauses were referencing the SELECT alias name, not the underlying physical column. That's invalid SQL in most dialects — aliases aren't visible in ON/WHERE at the same query level. Fix: buildJoinOn and buildSelectQualifier now look up each attribute's underlying (qualifier, column) pair via the select list, falling back to (fallback_alias, name) only for attrs not contributed by any select item. Ports: - prefix.test.ts (new): prefix.01 — SELECT with AS 'supplier_<attr>' - suffix.test.ts (new): suffix.01 — symmetric with prefix - join.02 / join.10: flipped from it.todo to ported - left_join.02: expected SQL corrected (t1.sid = t2.sid, not t2.id) - restrict.07: flipped from divergent-snapshot to ported (t1.name, not t1.firstname) Net: +4 ported cases (72 → 76/89). restrict.07 no longer divergent. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
SqlRelation.cross_join / .cross_product / .join(other, []) now compile to CROSS JOIN instead of falling back to in-memory. - New processCrossJoin processor: parallel to processJoin but emits a cross_join TableSpec (no ON) and skips buildJoinOn. - Cross-join compile output switches from the comma form (FROM a, b) to the explicit CROSS JOIN form for consistency with bmg-rb. - join(other, []) (empty keys) now routes to processCrossJoin rather than the fallback, matching bmg-rb's "empty join keys means cross product" semantic. - Existing compile.test.ts CROSS JOIN expectation updated. Ports: - join.08 — flipped from it.todo to ported - join.09 — flipped to ported (divergent on FROM reordering: bmg-rb reorders cross-first, bmg-sql emits in source order; same keys picked by both) Net: +2 ported cases (76 → 78/89). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
left_join now accepts an optional `defaults` map. For unmatched
right-side rows (or matched rows where a right attribute is null), the
default value is substituted instead of null. Usage:
suppliers.left_join(supplies, ['sid'], { pid: 'P9', qty: 0 })
Cross-package wiring:
- bmg core (sync + async): `left_join` operator and Relation /
AsyncRelation methods accept a fourth `defaults` parameter. The
merge step substitutes defaults when the right value is
null/undefined. `defaults` attrs that aren't in the right side are
still added to the result (so defaults can provide attrs an empty
right relation doesn't carry).
- Types: new `LeftJoinDefaults<R>` exported from types.ts.
- bmg-sql: `SqlRelation.left_join` passes `defaults` through to
`processJoin`, then wraps each defaulted select item in
`COALESCE(expr, literal)` via a new `applyLeftJoinDefaults` helper.
Literals parameterize through the standard param path, so e.g.
`{pid: 'P9'}` becomes `COALESCE(t.pid, ?)` + param `'P9'`.
Unit tests added:
- tests/operators/left_join.test.ts: two cases covering the unmatched
path and the matched-with-null path.
Ports:
- left_join.03 — flipped from it.todo to ported
- left_join.08 — flipped from it.todo to ported (divergent: bmg-rb
uses CTE, bmg-sql uses derived-table wrap — same precedent as
minus.03 / summarize.06 / summarize.07)
Net: +2 ported cases (78 → 80/89).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Current state: 80/89 ported (4 divergent, 9 blocked via it.todo) - New "Unblocker pass 2" section capturing items 1-3 (prefix/suffix, CROSS JOIN, LEFT JOIN defaults) with case-impact breakdown - Next up: reranked (LIKE predicate, transform, UNION ALL, constants, union push-down into branches) - Operator checklist: left_join 8/8, join 14/14, restrict 9/11 - Added prefix (1/1) and suffix (1/1) to "Operators added outside the original 14" - Blocked list: dropped prefix/suffix/CROSS JOIN/LEFT JOIN defaults (all resolved by pass 2); added UNION ALL and transform Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Cross-package feature spanning @enspirit/predicate, @enspirit/bmg-js,
and @enspirit/bmg-sql. Enables substring matching via LIKE.
@enspirit/predicate:
- New MatchPredicate kind (types.ts) with pattern and optional
caseSensitive flag (default true).
- Pred.match(attr, pattern, opts?) builder.
- Evaluator returns false when the left value isn't a string; does a
toUpperCase-style compare when caseSensitive is false.
- toSql emits `attr LIKE $1 ESCAPE '\'` (or UPPER(attr) LIKE UPPER($1)
ESCAPE '\' for case-insensitive). Pattern is wrapped %...% by the
compiler; the param value carries the wrapped form.
- Guards updated to recognize the new kind.
@enspirit/bmg-js (core):
- New rxmatch(attrs, pattern, { caseSensitive? }) operator on sync and
async relations. Compiled as restrict(or(match(...), match(...), ...))
under the hood.
- New RxmatchOptions type export.
- Core unit tests added (5 cases).
@enspirit/bmg-sql:
- compilePredicate gets a 'match' branch matching the predicate
package's shape (LIKE ... ESCAPE '\' with optional UPPER wrapping).
- requalifyPredicate handles the new kind.
- SqlRelation.rxmatch pushes down via processWhere (builds the same
or(match...) predicate directly).
Ports:
- rxmatch.01, rxmatch.02 (new test file)
- restrict.08, restrict.09 flipped from it.todo to ported
Net: +4 ported cases (80 → 84/89).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
UNION ALL option (cross-package):
- Relation.union / AsyncRelation.union now accept an options arg:
{ all?: boolean }. Default false (existing UNION behavior). When
true, skip deduplication.
- In-memory: straight concatenation. Caller is responsible for the
relation invariant — use only when branches are known disjoint or
duplicates are acceptable.
- bmg-sql: plumbs the flag through to processMerge's pre-existing
`all: boolean` parameter, emitting `UNION ALL`.
- New UnionOptions type export in core.
- Unit test added (bmg/tests/operators/union.test.ts).
Ports:
- union.03 — flipped from it.todo to ported.
- constants.01 — ported (processConstants was already wired up; only
the snapshot test was missing).
Net: +2 ported cases. Totals recounted: 85/89 (prior totals were off
by 1; corrected in INDEX + CLAUDE_PLAN).
Remaining blocked: 4 transform.* cases (type-token API).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The biggest remaining black-box blocker: bmg core's Transformation
type was strictly JS-function-based, so no transform could push down
to SQL. This adds a declarative token channel.
bmg core:
- New TransformToken ('string' | 'integer' | 'date') and TransformStep
(= TransformFunc | TransformToken). Transformation now accepts
tokens anywhere a function was accepted.
- In-memory interpretation: 'string' → String(v), 'integer' →
Math.trunc(Number(v)), 'date' → new Date(v). Null/undefined pass
through untouched.
- Shared helper support/applyTransform.ts factored out of the sync
and async operators so they stay in lockstep.
- 5 core unit tests covering single token, pipeline, bare-token,
and mixed function + token pipelines.
bmg-sql:
- New CastExpr AST node ('cast', expr, type) compiling to
CAST(expr AS type). type is opaque (emitted verbatim) so dialect-
specific spellings flow through unchanged.
- New processTransform processor that wraps select items in
CAST (for 'string' / 'integer') or func_call date() (for 'date')
according to the pipeline. Tokens chain by nesting.
- SqlRelation.transform now routes to processTransform via a
toTokenSpec helper that distinguishes token-only transforms (push
down) from any-function transforms (fall back to in-memory).
- Compile's func_call branch emits the function name as-given
(callers pick casing). Updated applyLeftJoinDefaults to use
uppercase 'COALESCE' so existing left_join snapshots keep their
shape.
- requalifyScalar handles 'cast'; hasComputedAttributes treats
'cast' as computed (so subsequent ops wrap via ensureSelect).
Ports:
- transform.01-04 all flipped from it.todo to ported.
- .01: CAST(col AS varchar(255))
- .02: bare token shorthand (applies to all attrs)
- .03: nested CAST pipeline [string, integer]
- .04: date(CAST(...)) composition
Net: +4 ported cases (85 → 89/89). Every bmg-rb black-box case is
now covered.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
processWhere now recognizes `union` / `intersect` / `except` at the
top of an expression and recursively pushes the predicate into both
branches, rather than falling into the ensureSelect/fromSelf path
that wraps the set op in a derived table. Applies the identity:
σ_p(A ∪ B) = σ_p(A) ∪ σ_p(B) (and likewise for ∩, \)
One fewer subquery hop; matches bmg-rb's per-branch push-down shape.
Ports:
- restrict.10 — flipped from divergent to ported (exact bmg-rb match)
- restrict.11 — still divergent, but closer to bmg-rb: per-branch
push-down is now done, only missing the aggressive contradiction
folding (`city='Paris' AND city='London'` → empty). Snapshot
updated and the divergence note sharpened.
Net: divergent snapshots shrunk (~5 → ~4). Total ported unchanged
(89/89 — still all covered).
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
3 participants
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.
No description provided.