Three correctness bugs in the DML executor that the 4.7 audit
surfaced:
1. RunInsert logged the transaction BEFORE dbAppend() and validation.
LogRecord captured the PREVIOUS row's RecNo, and a CHECK/FK
violation that rolled back via dbDelete() still left a spurious
INSERT entry in the log pointing at the wrong record. Move
LogRecord to after all field puts and all validators pass, so
the log only records committed INSERTs at the correct RecNo.
2. RunUpdate (fallback path) skipped CHECK and FK validation entirely
— only RunInsert validated. An UPDATE could violate the same
constraints INSERT protects against. Add the same validator calls
after FieldPut, with a captured aPrevVals snapshot so the in-
memory record can roll back cleanly on failure. Gated by
SqlLoadConstraints to skip the validator (and its recursive
five_SQL) for tables without SQL-level metadata — tables created
via plain dbCreate see no change.
3. RunDelete had no transaction logging at all — a BEGIN / DELETE /
ROLLBACK cycle silently lost the row. Add LogRecord("DELETE")
before dbDelete so undo can re-surface it. (A full FK-cascade
check on delete would require parent→child scanning; deferred.)
The fast-path SqlBulkUpdate branch still bypasses per-record
validation by design (documented) — it's gated by
`! ::oTxn:IsActive()`, so txn-active queries always take the
validated fallback.
FiveSql2 43/43 (including SAVEPOINT + ROLLBACK TO and all four CHECK/
FK tests), Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Opus 4.7 audit of the codebase surfaced several items that Opus 4.6
sessions left behind. This pass removes what's definitively dead and
fixes one trivial defensive bug; the real logic bugs (transaction
ordering, missing RunUpdate/RunDelete validation) come in a separate
commit.
Deletions:
- `_FiveSql2/src/TSqlParser_orig.prg` (1173 lines) — superseded by
`TSqlParser2.prg` (Pratt). Production never instantiates the old
parser; the only callers were the comparison/benchmark test files
also being removed.
- `_FiveSql2/test/test_parser_cmp.prg` — compared orig vs Pratt AST,
useless now that orig is gone.
- `_FiveSql2/test/bench_parser.prg` — benched both, same reason.
- `_FiveSql2/Makefile` `test_cmp:` and `bench:` targets referenced
the removed files.
- `TSqlIndex.prg` methods `ApplyScope`, `ClearScope`, `ApplySeek`,
`IndexInfo`, `CreateTempIndex`, `DropTempIndex` — each declared in
the class header and implemented (~165 lines total) but zero
callers anywhere in `_FiveSql2/` or `hbrtl/`. Class declarations
removed alongside the bodies.
Small fixes:
- `TSqlDDL.prg:179-180` stale comment claiming Five doesn't support
`@byref` — false since commit e95afad (2026-04-13) wired @byref
via RefCell. The same method uses @nPos correctly elsewhere.
- `hbrt/class.go:tryBinaryOp` defensive nil-check on AsArray().
IsObject() checks the type tag; a corrupted Value with tag=Object
but ptr=nil would crash on `.Class`. Correct construction paths
never hit this, but the guard is cheap.
Compat tests: FiveSql2 43/43, Harbour compat 56/56, Go test ALL PASS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CTE tables now materialise via MEMRDD (no file on disk), yet the
RunSelect cleanup loop was still stat-ing __cte_<name>.dbf for every
CTE in every CTE query. Profile after the FetchRow rewrite pinned
HbFileExists at 20.28% of total CPU — pure waste when MEMRDD is the
common path.
Add s_lCteDiskSeen flag, set only when the legacy DBFNTX fallback in
RunSelect actually opens a pre-existing __cte_<name>.dbf (line 1247
path — rare, only for sub-executors referencing a CTE by name on a
crashed-prior-run .dbf). Cleanup runs only when the flag is set.
pprof delta (full bench with cache enabled):
rawsyscalln: 25.56% → 8.50% (~17 points removed)
HbFileExists: 20.28% → 0% (dropped out of top)
Wall-clock unchanged (ENOENT stats are kernel-cached on Darwin), but
this removes the last visible avoidable syscall. What's left in the
profile (kevent, madvise, pthread_cond_*) is Go runtime + scheduler
overhead that application code can't touch.
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TSqlExecutor:FetchRow was the per-row workhorse for aggregation,
HAVING, and window queries. Even with the pre-built aFetchCache
binding columns to (nWA, nFPos), the PRG FOR loop paid one method
dispatch per column per row (dbSelectArea, FieldGet, AllTrim,
AAdd) — profile pinned it at ~30% of B4 CPU.
SqlFetchRowFast collapses the cache-path loop into a single Go
call:
- bound entry: SelectByNum + area.GetValue directly
- unbound (aggregate/expression): self:EvalExpr via Send
- character values: TrimSpace inline
The PRG FetchRow keeps its original cache-miss fallback path
unchanged for rare queries where aFetchCache isn't built.
Bench deltas (median of 3 steady runs, 1000 iters):
B4_GROUP_HAVING 418 → 327 us -22% (1.28x)
B9_ROW_NUMBER 191 → 120 us -37% (1.59x)
B10_RANK_PART 228 → 135 us -41% (1.69x)
B11_SUM_OVER 249 → 156 us -37% (1.60x)
B14_COUNT 235 → 219 us -7%
B15_CTE_WIN_JOIN 1577 → 1452 us -8%
Single-table SELECT (B1-B3, B5-B7, B8) stays flat — those already
hit the column-binding fast path and don't need aggregate dispatch.
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
B4 GROUP+HAVING profile showed SqlIsAggName at ~9% of CPU —
SqlEvalFunc checks it for every function in every row, and the
PRG body was two string allocations + a substring scan:
RETURN ("," + c + ",") $ ("," + AGG_FUNCTIONS + ",")
Replace with a hash lookup against the existing aggFuncSet map
in hbrtl/sqlexpr.go (already populated for SqlExprHasAgg, same
AGG_FUNCTIONS list). Upper-casing skips the allocation when the
input is already upper, which it almost always is in practice.
Bench deltas (median of 3 steady runs, 1000 iters):
B4_GROUP_HAVING 447 → 418 us -6.5%
B14_COUNT 252 → 235 us -7%
B15_CTE_WIN_JOIN 1595 → 1577 us -1%
Other benches unchanged (no aggregate calls per row).
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AcquireTemp now returns the purpose string (upper-cased table name)
as the alias when available, and falls back to FA_#### only when the
same purpose is already in-flight this query — i.e., self-joins.
Previously every call returned a fresh FA_####, so the WA cache
(keyed by alias) could never hit on JOIN queries and the file got
reopened every iteration.
Bench deltas vs prior HEAD:
B6_INNER_JOIN 217 → 130 us -40% (1.67x)
B15_CTE_WIN_JOIN 1678 → 1595 us -5%
Single-table benches unchanged — they were already hitting the
cache via the table-name alias path.
B8 recursive CTE stays flat: its sub-executors at nDepth>1 still
cycle through fresh purposes that don't stabilise across queries.
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the SELECT WA cache landed, pprof showed HbFileExists → os.Stat
at 28% of remaining CPU — the RunSelect cleanup loop was stat-ing
__view_<table>.dbf for every table in every query, even on the
common view-free path.
Track view materialisation with a TSqlIndex.lViewUsed flag set in
OpenTable when CheckView produces a temp. The cleanup loop now
runs only when the flag is set, then resets it. View-using queries
are unaffected.
pprof delta:
rawsyscalln: 2.14s → 1.41s (48% → 32% of total CPU)
os.Stat: 1.24s → 0.49s (28% → 11%)
Wall-clock bench numbers stayed within plus-or-minus 3% noise (stats
are cheap when the target file does not exist, so CPU savings do not
translate directly to end-to-end time) but this removes the next
biggest syscall waste visible in the profile.
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TSqlExecutor:OpenTable now hands lifetime to the WA cache for stable
aliases (user-supplied or table-named). CloseOpened skips those
entries, so the DBF mmap stays alive across queries instead of being
unmapped + re-opened 1000 times a bench. Previously the WA cache only
covered DML (INSERT/UPDATE/DELETE) — SELECT was still paying the full
dbUseArea/dbCloseArea syscall bill every query (profile showed
rtlDbCloseArea + munmap at ~30% of total CPU).
AcquireTemp-generated aliases (FA_####) are excluded — they change
every query (self-joins, nested depth), so caching them would just
leak entries for no reuse. JOIN / recursive CTE regressions from an
earlier unrestricted version are gone.
Bench deltas vs prior HEAD (median of 3 steady runs, 1000 iters):
B1_SELECT_STAR 82 → 41 us -50% (2.0x)
B2_WHERE_FILTER 78 → 35 us -55% (2.2x)
B3_ORDER_BY 90 → 48 us -47% (1.88x)
B5_DISTINCT 75 → 32 us -57% (2.34x)
B7_CTE_SIMPLE 120 → 77 us -36% (1.56x)
B9_ROW_NUMBER 239 → 194 us -19%
B10_RANK_PART 276 → 233 us -16%
B11_SUM_OVER 296 → 252 us -15%
B4_GROUP_HAVING 498 → 450 us -10%
Others flat (JOIN / recursive CTE / DML already covered).
FiveSql2 43/43, Harbour compat 56/56.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SqlOrderBy: Go sort.Slice for ORDER BY, 10-50x faster than PRG ASort.
SqlGroupBy: Go map-based GROUP BY accumulation (ready for integration).
TryBuildSortSpec detects simple ORDER BY columns and routes to Go.
Fallback to PRG for complex ORDER BY expressions.
43/43 + 41/41 verify + 51/51 compat + go test ALL PASS.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
FindColIdx2 searched for bare column name (e.g. 'AMOUNT') but
aFieldNames now contains qualified names ('o.amount') from the
Go join fast path. Added fallback: try xArg[2] (the full AST name)
when the bare name misses. Fixes SUM/AVG/MIN/MAX aggregation after
Go-native hash join.
Verified: 41/41 correctness tests pass (verify_correctness.prg).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Q2 Running total regressed 100ms→6.7s from the frame-aware rewrite.
Default frame (UNBOUNDED PRECEDING to CURRENT ROW) now uses O(N)
incremental path; general per-row-frame loop only for custom frames.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
--- #15 RIGHT JOIN O(N*M) → O(N+M) via matched RecNo set ---
--- #19 s_nRCJSeq modular counter (% 100000) ---
--- #20 Implicit column alias without AS keyword ---
Validation: 43/43 + 51/51 + go test ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
--- #12 Window frame spec now honoured ---
Parser parsed ROWS BETWEEN ... AND ... but discarded the result.
Now stores hFrame in a 6th slot on ND_WINDOW nodes via AAdd.
ApplyWindowFunctions reads it and computes per-row frame boundaries
via SqlFrameOffset helper. Unified SUM/AVG/COUNT/MIN/MAX into one
frame-aware CASE branch.
--- #6 EXISTS LIMIT mutation removed ---
Removed direct parse-tree mutation (hQuery["limit"] := 1) that
would corrupt reuse. Semi-join lift handles the fast case.
Validation: 43/43 + 51/51 + go test ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Continues the static-analysis sweep from 7babfb7.
--- #3 Resolve NIL ambiguity (HIGH) ---
ResolveFromOuter returned NIL for both "column not found" and
"column value is NULL". Callers tested `xVal != NIL` to decide
success, which silently dropped legitimate NULL outer-row values
in correlated subqueries. Added a by-reference lFound flag so
callers distinguish the two cases.
--- #14 Multi-level LEFT JOIN null-fill (MEDIUM) ---
LEFT JOIN null-fill only fired at the last join level
(`nIdx >= Len(aJoins)`). For `a LEFT JOIN b ON ... JOIN c ON ...`
where b had no match, the null-fill for b was skipped and the
outer row was dropped entirely. Now recurses into subsequent joins
when the match fails, so the base case can still emit a row with
NULLs for b's columns.
--- #18 UNION/INTERSECT/EXCEPT applied after LIMIT (MEDIUM) ---
SQL standard requires set operations before ORDER BY / DISTINCT /
OFFSET / LIMIT. Reordered to:
RIGHT JOIN pass → UNION/INTERSECT/EXCEPT → DISTINCT → ORDER BY
→ OFFSET → LIMIT.
Previously LIMIT clipped the first SELECT before UNION merged the
second's rows, producing more rows than intended.
--- #22 DATEADD month overflow (LOW) ---
`DATEADD('MONTH', 1, '2024-01-31')` produced `SToD("20240231")`
(Feb 31) → empty date. Now normalizes month overflow/underflow
into year rollover and clamps the day to the target month's last
day. Year addition also handles Feb 29 → Feb 28 on non-leap years.
--- #23 VIEW temp file leak (LOW) ---
TSqlIndex:CheckView creates `__view_<table>.dbf` temp files that
were never cleaned up. Added post-scan cleanup in RunSelect's
close section (after CTE cleanup) that erases matching temp files.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Systematic bug-hunt driven by an automated analysis of all FiveSql2
source files. Each fix is targeted — no speculative refactoring.
--- #1 CLASSDATA hSubCache leaked across queries (CRITICAL) ---
CLASSDATA hSubCache INIT { => } SHARED
shared one hash across ALL TSqlExecutor instances. A non-correlated
subquery cached in query A was silently returned for an unrelated
query B if the subquery text happened to produce the same cache key.
Converted to instance DATA initialized in New().
--- #5+#21 IS NULL / COALESCE treated empty string as NULL (HIGH) ---
RETURN xL == NIL .OR. ( ValType(xL) == "C" .AND. Empty(AllTrim(xL)) )
SQL standard: '' is a valid non-NULL value. Removed the empty-string
check from both IS NULL evaluation and COALESCE skip logic.
--- #4 Multiple ? parameters all returned first value (HIGH) ---
ND_PAR nodes had no index — EvalExpr always returned ::aParams[1].
Parser now stamps each ? with a sequential 1-based index in xNode[2].
EvalExpr uses it to return the correct ::aParams[n].
--- #10+#11 SqlEvalRowExpr missing / and || operators, single-arg
function eval (MEDIUM) ---
Division and string concatenation fell through to RETURN NIL in the
row-expression evaluator used by recursive CTEs and aggregate
ComputeAgg. Also, multi-argument functions like SUBSTR(x,2,3) only
received the first argument. Both fixed.
--- #9 SUM/AVG/MIN/MAX of all NULLs returned 0 instead of NULL
(MEDIUM) ---
SQL standard requires NULL. Changed the aggregate return path to
return NIL when nCount == 0 (SUM/AVG) or when xMin/xMax == NIL.
--- #8 MIN/MAX used SqlCoerceNum for comparison (MEDIUM) ---
Strings and dates were coerced to numbers (Val()) before comparing,
making MIN('banana') == MIN('apple') == 0. Switched to SqlCmpLt
which handles type-appropriate comparison.
--- #7 SqlExprHasAgg only checked top-level node (MEDIUM) ---
Expressions like `salary + COUNT(*)` were not detected as containing
an aggregate because the top node was ND_BIN, not ND_FN. Made the
function recursive — walks ND_BIN, ND_UNI, ND_FN args, ND_CASE
branches.
--- #13 SELECT * only expanded first table in JOINs (MEDIUM) ---
`SELECT * FROM orders o JOIN customers c ON ...` only included
fields from orders. Changed the expansion loop to iterate ALL
entries in ::aTables.
--- #2 s_aOuterStack not unwound on subquery error (HIGH) ---
SubqueryCached's PushOuter/PopOuter pair was not protected by
BEGIN SEQUENCE. A runtime error inside the subquery left a stale
entry on the module-level outer stack, corrupting all subsequent
queries' correlated column resolution. Wrapped in SEQUENCE/RECOVER.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A scalar correlated subquery with a JOIN inside:
SELECT e.name,
(SELECT SUM(o.qty * p.price)
FROM ord o INNER JOIN prod p ON o.prod_id = p.id
WHERE o.emp_id = e.id) AS revenue
FROM emp e WHERE e.dept = 'SALES'
returned wrong values (equal to SUM(qty) instead of SUM(qty*price))
or zero for all but the first outer row. Root cause was a triple
interaction between three independent bugs.
--- Bug 1: Subquery cache leaked across five_SQL invocations ---
hSubCorrCache, aSubCacheSlots, aSemiJoinSlots, nSubCacheSeq were
declared as DATA ... INIT { => } / {} / 0. In Five's compiled output,
hash/array INIT literals may share the same backing instance across
New() calls, so the cache from query A (SUM qty, no join) was still
there when query B ran, providing a hit on the same key — returning
A's cached (wrong) value instead of re-executing B's subquery.
Fix: explicit initialization in New().
--- Bug 2: aJoins alias mutation across subquery invocations ---
RunSelect's join-alias sync loop mutated aJoins[i][3] from the
user alias ("p") to the depth-suffixed temp alias ("FA_0003").
aJoins was a direct reference into hQuery["joins"], so the mutation
persisted across re-executions of the same hQuery. On the 2nd call,
the sync loop couldn't find a matching aTables entry because the
stale temp alias ("FA_0003") didn't match the new one ("FA_0005").
The join table's workarea was positioned wrong → empty join result.
Fix: deep-clone both ::aTables and aJoins at the start of RunSelect
so each invocation starts from the parsed originals.
--- Bug 3: SqlCollectCols stripped alias prefixes ---
When adding hidden columns for complex aggregate arguments (e.g.
SUM(o.qty * p.price)), SqlCollectCols returned bare names like
"qty" and "price" instead of qualified "o.qty" / "p.price". In a
JOIN context, unqualified "price" routed FetchRow to the first
table (ord) instead of prod — FieldPos returned 0, the column was
silently NIL, and the multiplication collapsed to qty*1 = qty.
Fix: new SqlCollectColExprs returns the original ND_COL AST nodes
with qualified names preserved. The hidden-column loop now inserts
these directly so FetchRow's dot-qualified path resolves to the
correct workarea via FindWA.
--- Verification ---
Deterministic 5-emp / 6-order / 3-product test:
Expected revenues per emp:
Emp 1: 2*10 + 3*20 = 80 → got 80.00 ✓
Emp 2: 1*10 + 4*30 = 130 → got 130.00 ✓
Emp 3: 5*20 = 100 → got 100.00 ✓
Emp 4: no orders = 0 → got 0 ✓
Emp 5: 7*10 = 70 → got 70.00 ✓
Also verified SUM(qty*2) and SUM(p.price) variants.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Correlated EXISTS with high-cardinality keys was stuck at O(outer × inner)
because memoization couldn't amortize across unique correlation values.
H3 in the subquery stress bench:
SELECT e.name FROM emp e
WHERE EXISTS (SELECT 1 FROM ord WHERE ord.emp_id = e.id AND ord.qty > 15)
500 outer rows × 500 distinct e.id values × 5000-row ord scan = 10s,
with no path to improvement from caching the subquery result.
Fix: detect the semi-join shape on the subquery and rewrite it at
runtime into a non-correlated DISTINCT scan whose result is cached
as a hash set. Each outer row then becomes an O(1) hash probe.
--- What we lift ---
SELECT ... FROM inner_table
WHERE inner.col = outer.col [AND other_non_correlated_preds]
Shape constraints (all must hold):
- single table, no JOIN
- no GROUP BY, no HAVING, no UNION
- WHERE is an AND tree containing an equi-term where one side is
a column with an alias prefix from the subquery's own FROM
and the other is a column from an outer alias
- the remaining AND terms (non-correlated residue) have no
outer references of their own — rules out patterns like
`WHERE e2.dept = e.dept AND e2.salary > e.salary` where the
second term can't live without the outer context
--- How the lift works ---
1. Walk the WHERE as a flat AND-term list
2. Find and remove the first correlated equi-term, remember the
inner column name and outer column reference
3. Verify residue is non-correlated via a recursive AST walker
(SemiJoinHasOuterRef) — bail to fallback if not
4. Clone hQuery with:
columns = {DISTINCT inner.col}
where = residue (or NIL)
distinct = .T.
limit / top / order_by / group_by / having cleared
5. Run the cloned subquery once via a nested TSqlExecutor — no
PushOuter because it's now non-correlated
6. Build a hash set keyed on SqlValToStr(each distinct inner value)
7. Per EXISTS probe: Resolve the outer column reference, look up
in the hash set
Cached in ::aSemiJoinSlots indexed by xSubNode identity so the
analysis + lifted scan runs exactly once per subquery expression.
Subqueries that don't match the shape store the sentinel "NO" so
subsequent probes skip re-analysis and fall through to the existing
SubqueryCached + LIMIT 1 path.
NOT EXISTS works through the same path — lNegate flag just flips
the final hash-lookup result.
--- Bench (emp=500, prod=100, ord=5k) ---
Pattern Before After Speedup
────────────────────────────────────────────────────────────
H3 EXISTS correlated 10.0s 4.5ms ~2200x
H8 NOT EXISTS self-join 900ms 890ms same (can't lift:
remainder
`e2.salary > e.salary`
is correlated)
H11 Scalar + EXISTS + derived 3.2s 1.0s 3.2x
H8 correctly falls through to the non-lifted path because the
remainder outer-reference check (SemiJoinHasOuterRef) rejects the
`e2.salary > e.salary` term. The 5-row answer is still correct.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
- H3 returns 125 rows (matches pre-change correct result)
- H8 returns 5 rows (matches pre-change correct result)
Known pre-existing bug, unrelated: H7 (scalar correlated subquery
with inner INNER JOIN) returns zero for rows 2..N — workarea state
leaks between consecutive subquery invocations. Not touched here,
filed for follow-up.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extreme subquery stress bench (12 patterns spanning scalar-in-SELECT,
nested correlation, EXISTS, NOT IN, derived tables, self-joins, and
mixed combinations) exposed three weaknesses in the post-ROLLUP state:
1. EXISTS / NOT EXISTS evaluated the full subquery result per outer
row, even though it only needs to know whether any row matches.
2. EXISTS was routed through a separate code path that bypassed the
correlated-memoization cache from 2d90236.
3. The previous SubqueryCached identified each subquery node by
mutating slot 6 on the ast array via ASize — which interacted
badly with downstream code paths expecting the original shape
(derived-table queries panicked on ArrayPop after the ASize).
Fixes:
* EXISTS / NOT EXISTS now route through SubqueryCached the same way
ND_SUB in WHERE does, so correlated EXISTS predicates memoize on
outer free-variable values when the cardinality is low.
* The EXISTS handler plants `hQuery["limit"] := 1` on the subquery
before the first execution. EXISTS doesn't care about the rest
of the result rows, so dropping the scan cap saves full-scan
cost in the common case.
* A new early-termination branch in RunSelect's scan loop exits
the `WHILE !Eof()` as soon as aRows reaches nLimit, guarded by
the same "no ORDER BY / GROUP BY / agg / DISTINCT" precondition
(those need the full input). This is what makes the LIMIT 1
injection actually pay off — before, LIMIT was only applied via
ASize after the full materialized scan.
* SubqueryCached no longer mutates the parse tree. Instead of
ASize-ing the node and stashing cache metadata in slot 6, it
keeps a per-executor aSubCacheSlots list of
{xSubNode, {id, aFreeVars}} pairs and identifies nodes by
Harbour's reference-equality `==` on arrays. O(n) lookup in n =
number of distinct subqueries in the query, which is ≤ 4 or so
for all realistic queries, so the linear scan is free. Fixes the
derived-table ArrayPop panic.
Bench impact (emp=500, prod=100, ord=5k — subquery hell):
Pattern Before After Δ
───────────────────────────────────────────────────────
H3 Correlated EXISTS 13.3s 10.0s 1.3x
H7 Scalar-in-SELECT + JOIN 362ms 2ms 181x
H8 NOT EXISTS self-join 1.8s 900ms 2.0x
H11 Scalar + EXISTS + derived 13.7s 3.2s 4.3x
(H1, H2, H5, H6, H9, H10, H12 unchanged at 3–72ms)
H7's 181x is the scalar-in-SELECT-list memoization payoff — each
dept's revenue subquery used to run 100 times (once per SALES emp),
now runs once per distinct dept.
H3's 1.3x is the best we can do without semi-join lift: 500 outer
rows × 500 unique correlation keys = 500 cache misses, and the 375
rows whose correlation finds no match must scan the full ord table
to confirm emptiness. Fixing that needs the optimizer to rewrite
`WHERE EXISTS (SELECT 1 FROM ord WHERE ord.emp_id = e.id AND ...)`
into `WHERE e.id IN (SELECT DISTINCT emp_id FROM ord WHERE ...)`,
which is a real query-rewrite feature left for a follow-up.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two SQL:2013 features that were stubs or bugs. Both ship together
because they share testing infrastructure (the SQL:2013 analytics
bench).
--- 1. ROLLUP / CUBE / GROUPING SETS (TSqlAgg) ---
The parser has recognized these for a while, storing them as
`ND_FN "ROLLUP"` / "CUBE" / "GROUPING SETS" nodes inside the
GROUP BY list. GroupBy never actually expanded them — it treated
the ND_FN as an opaque group term, which meant every row hashed
into the empty bucket and the query returned a single row.
New TSqlAgg:ExpandGroupingSets walks the aGroupBy array and
expands each ROLLUP / CUBE / GSETS modifier into a list of flat
grouping sets by cross-product with the surrounding plain terms:
GROUP BY ROLLUP(a, b, c) → {(a,b,c), (a,b), (a), ()}
GROUP BY CUBE(a, b) → {(a,b), (a), (b), ()}
GROUP BY GROUPING SETS((a,b),()) → as-is
GROUP BY x, ROLLUP(a, b) → {(x,a,b), (x,a), (x)}
When the expansion produces more than one set, GroupBy recurses
once per set (passing the plain flat set) and NILs out SELECT
columns that aren't in the current set — the standard subtotal
placeholder. Fast path (no ROLLUP/CUBE/GSETS node) short-circuits
to the original single-pass logic.
Correctness check: `SELECT region, SUM(amount) FROM sales GROUP BY
ROLLUP(region)` on a 5-region dataset now returns 6 rows (5
per-region subtotals + 1 grand total row with region=NIL). Was 1.
--- 2. Correlated subquery memoization (TSqlExecutor) ---
Committed 9e0f82c fixed a silent caching bug that made correlated
subqueries return the first outer-row's result for every subsequent
row, at the cost of dropping caching entirely — every outer row
re-executed the subquery. For Q8 in the SQL:2013 bench (1000 emps,
correlated on 3 distinct depts) that was 4.9 seconds.
The right answer is to memoize per outer-key, not globally. This
commit adds:
- TSqlExecutor:CollectFreeVars(hQ): walks a subquery's WHERE,
columns, and HAVING for ND_COL references whose alias prefix
isn't one of the subquery's own FROM tables. Those are the
outer columns the subquery actually depends on.
- TSqlExecutor:SubqueryCached(xSubNode): runs the free-var
analysis once per distinct AST node (memoized onto a 6th slot
on the node), builds a cache key from the current values of
those free vars via ::Resolve(), looks up in ::hSubCorrCache,
executes on miss. Non-correlated subqueries end up with an
empty free-var list → single cache entry → same behavior as
the old CacheSubquery fast path.
- ND_SUB and ND_SUB-in-IN handlers route through SubqueryCached
instead of the split cache/push-outer logic.
Plus a correctness fix that SubqueryCached surfaced: when a
subquery runs at nDepth > 1, TSqlExecutor rewrites each FROM
table's alias to a depth-suffixed temp (so concurrent opens of
the same file don't collide). Previously the original user-written
alias was only preserved in aTables[i][3] for single-char aliases.
Multi-char aliases like `emp e2` lost their original after the
rename, so FindWA("E2") failed, Resolve("e2.dept") returned NIL,
and `WHERE e2.dept = e1.dept` evaluated NIL=NIL → every row was
filtered out → subquery AVG returned 0 → outer `salary > 0` was
trivially true for everyone. Now we always stash the original
alias in [3] before the rename.
--- Bench (SQL:2013 analytics, 10 queries, emp=1k, sales=20k) ---
Query Before After Δ
────────────────────────────────────────────────────────
Q6 RECURSIVE hierarchy (prev fix) 30ms
Q7 ROLLUP subtotals 86ms, 1 row 106ms, 6 rows (correct)
Q8 Correlated subquery 4933ms 20ms ~245x
(all other queries unchanged at 4–230ms)
Q8 30-row sanity regression test (emp.dept in {A,B,C}, deterministic
salaries so hand-computed averages are 155/810/1765):
SELECT name, dept, salary FROM emp e1
WHERE salary > (SELECT AVG(salary) FROM emp e2 WHERE e2.dept = e1.dept)
Before: 30 rows (wrong — returns all)
After: 15 rows (correct — 5 above each dept's average)
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes uncovered by a SQL:2013 analytics benchmark covering the
query patterns people actually run on DBF data (OLAP, BI, hierarchy
traversal).
--- Fix 1: correlated subquery was silently wrong ---
EvalExpr's ND_SUB handler only pushed the outer context when
`s_aOuterStack` was already non-empty — otherwise it routed the
subquery through CacheSubquery, which stores the first result under
a key derived from the subquery's syntax tokens. For a correlated
subquery in a top-level WHERE:
SELECT name, dept, salary FROM emp e1
WHERE salary > (SELECT AVG(salary) FROM emp e2 WHERE e2.dept = e1.dept)
the first outer row saw an empty stack, cached the result, and
every subsequent outer row got the same cached value regardless of
e1.dept. The query returned all 1000 employees instead of the 505
who actually beat their department's average.
Fix: always PushOuter + Run, no cache. Correctness over caching.
Trade-off: non-correlated scalar subqueries now re-execute per
outer row. A proper per-outer-key memoization is deferred — it
requires walking the subquery AST to collect free variables.
--- Fix 2: WITH RECURSIVE hierarchy join was O(m*n) ---
RecCteJoin (the in-memory join used when a recursive CTE's step
references both a real table and the CTE frontier) ran a flat
nested loop: for each DBF row × each prev-iteration row, build a
combined row buffer and run SqlEvalRowExpr on the ON condition.
For a 4-level 1000-employee hierarchy that's ~1M ON evaluations,
~4.6 seconds.
Fix: detect the shape `dbfAlias.col = cteAlias.col` at join-setup
time, build a PRG hash on the CTE frontier keyed by its join column
(aPrevRows is always small — at most the last iteration's emitted
rows), then scan the DBF side once and probe the hash. Complex ON
predicates fall through to the original nested loop.
--- Bench (SQL:2013 analytics, emp=1k, sales=20k, evt=30k) ---
Query Before After Speedup
──────────────────────────────────────────────────────────────
RECURSIVE hierarchy 4-level 4603ms 30ms ~150x
Correlated subquery (all emp) 10ms ❌ 4933ms ✓ (correct)
Other SQL:2013 queries (ROW_NUMBER top-N, running total, moving
average, DENSE_RANK, LAG, NTILE, gaps-and-islands) are all in the
expected 10–230ms range for these dataset sizes, unchanged by
this commit.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Known follow-ups (not in this commit):
- Q7 ROLLUP(col) parses but isn't expanded in GroupBy — returns
a single grand-total row instead of per-value + total. Grouping
sets implementation is a separate feature.
- Correlated subquery memoization by outer free-variable key
would bring Q8 from 4.9s back to ~50ms for small cardinality
correlations — requires AST free-var analysis.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
FiveSql2's HashJoin only recognized bare equi-terms (xOnCond[1]=ND_BIN,
xOnCond[2]="="), so a compound ON predicate like
ON e.dept_id = t.dept_id AND e.salary = t.max_sal
fell through to the nested-loop ELSE branch:
dbSelectArea(nInnerWA)
dbGoTop()
WHILE !Eof()
IF SqlIsTrue(EvalExpr(xOnCond))
JoinRecurse(...)
ENDIF
dbSkip()
ENDDO
That's O(outer × inner) per outer row, re-evaluating the full AND tree
every probe. Query Q7 in the complex benchmark (CTE top_emp joined back
to emp on compound key) ran at 4.6 seconds for 100 inner × 10k outer.
Fix has two pieces:
1. **Probe-term extraction in JoinRecurse**: when xOnCond is an AND,
walk the left-associative chain looking for the first equi-term
(`a.x = b.x`). Use that as the hash-probe key, drive the normal
hash-join code path through it.
2. **Post-filter in HashJoin**: after a hash match, if the *original*
xOnCond was compound, re-evaluate the full predicate with
EvalExpr to drop matches that satisfied the hash key but not the
rest of the AND (e.g. same dept but different salary). Bare equi-
joins still skip the re-eval — the hash match is conclusive.
Bench (10k × 100 × compound ON predicate):
Query Before After Speedup
─────────────────────────────────────────────────────────
Q7 CTE + JOIN compound ON 4573ms 209ms 21.9x
Still works for the existing bare equi case (43-test unchanged) and
the 3-way JOIN case (no regression). Falls back to the generic nested
loop only when no probe-term can be extracted at all.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
- Q7 result: 100 rows (correct)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Surfaced by complex-query benchmarking. Query like:
SELECT d.name AS dept, COUNT(*) AS n, SUM(o.amount) AS total
FROM dept d INNER JOIN emp e ON ... INNER JOIN ord o ON ...
GROUP BY d.name
returned exactly 1 row instead of 100. Removing the AS aliases made
it work correctly. Semantic bug, not a performance issue.
Root cause: TSqlAgg:GroupBy resolved each GROUP BY column by calling
FindColIdx against aFN — the output alias list. For GROUP BY d.name
with d.name AS dept, the group expression's column name was looked
up in {"dept","n","total"} and missed. FindColIdx returned 0, every
row got an empty group key, and the hash collapsed everything into
one bucket.
Fix: new FindGroupIdx walks aCols (SELECT list expressions) instead,
matching the GROUP BY column against each SELECT item's source
expression ND_COL name. Handles qualified refs (d.name -> NAME) and
falls back to FindColIdx for cases where GROUP BY uses a column not
in the SELECT list.
Also hoisted the resolution out of the per-row loop — GROUP BY
columns resolve once into aGroupIdx[] so each row just indexes.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- Complex bench Q4: 1 row -> 100 rows (correct)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Complex-query benchmarking turned up two hot paths that the earlier
SqlScan/SqlEach work didn't touch: multi-table JOIN and nested-scan
row fetching. This commit hits both.
--- Part 1: SqlHashBuild — Go-native hash-join build ---
FiveSql2's HashJoin previously built the inner-side hash in PRG:
WHILE !Eof()
xVal := FieldGet(nFPos)
cKey := SqlValToStr(xVal)
IF !hb_HHasKey(hHash, cKey) ; hHash[cKey] := {} ; ENDIF
AAdd(hHash[cKey], RecNo())
dbSkip()
ENDDO
That loop runs at ~40μs per row from class dispatch + hb_HHasKey
lookups + AAdd growth + SqlValToStr formatting. On a 50k-row inner
table that's ~2 seconds wasted on what should be a sub-50ms
housekeeping op.
New hbrtl.SqlHashBuild does the same thing in one Go-native pass:
- Direct *dbf.DBFArea loop (no interface dispatch, same devirt as
SqlScan)
- Go `map[string][]int64` accumulates RecNos by key — one
allocation per distinct key
- Inline ASCII-only digit formatter for numeric keys (strconv.Itoa
is allocation-heavy for small ints)
- CHAR keys are right-trimmed to match SqlCmpEq semantics so the
hash probe matches what EvalExpr would compute
- Final Five hash is built once from Keys/Values/Order slices
directly, skipping the per-key hb_HSet path
HashJoin now calls `SqlHashBuild(nFPos)` instead of running the
PRG loop.
--- Part 2: TSqlExecutor:BuildFetchCache ---
The JOIN fallback loop calls FetchRow per row. FetchRow was already
column-ref-aware but did the string parse (`At + SubStr + Upper`)
and `::FindWA` linear scan every single invocation. For a 50k-row
join emitting 50k result rows, that's ~200k redundant resolutions.
New BuildFetchCache walks the SELECT list once before the scan and
pre-binds each plain-column expression to `{nWA, nFPos}`. FetchRow's
new fast path checks ::aFetchCache and jumps straight to
`dbSelectArea + FieldGet` when bound. Complex exprs (functions,
CASE, subqueries) still fall through to EvalExpr.
::aFetchCache is set right before the join WHILE loop and cleared
after — no cross-query bleed.
--- Bench (50k ord × 10k emp × 100 dept, 3-run steady state) ---
Query Before After Speedup
────────────────────────────────────────────────────────────
2-way INNER JOIN, 10k rows 91ms 68ms 1.34x
2-way JOIN + GROUP BY 110ms 94ms 1.17x
3-way INNER JOIN COUNT 2610ms 610ms 4.28x
3-way JOIN + GROUP BY 2860ms 830ms 3.45x
The 3-way speedup is almost entirely SqlHashBuild. The 2-way case
benefits from the fetch cache because its per-row cost is dominated
by FetchRow (no second hash build to amortize).
--- Limits still standing ---
CTE + JOIN queries (Q7 in bench_complex: ~4.5s) aren't affected by
either optimization — CTE materialization goes through a different
path that writes/reads a temp DBF. Follow-up target.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wires the new SqlEach RTL into FiveSql2's front-end so users write
the SQL they know and opt into streaming with a familiar Harbour
code block — no manual RTL plumbing.
API:
/* Existing array form — unchanged, 43-test still green */
aR := five_SQL( "SELECT name FROM t" )
/* New block form — zero intermediate rows, 2x raw PRG */
five_SQL( "SELECT id, name FROM t WHERE salary > 50000", NIL,
{|nID, cName| Process(nID, cName)} )
Parameter order (cSQL, aParams, bBlock) keeps backward compatibility
with every existing call site. Passing NIL for aParams when only a
block is needed is standard Harbour idiom.
Routing:
* TFiveSQL:Execute now takes an optional bBlock parameter and
stores it on TSqlExecutor as ::bRowBlock.
* TSqlExecutor:RunSelect's existing Go fast path (same guards as
before: single table, no JOIN/GROUP/aggregate, plain column
projections, WHERE compilable via SqlExprToPrg) branches on
::bRowBlock:
- block present → SqlEach streams rows through the block
- block absent → SqlScan materializes into aRows (current path)
* Post-processing (GROUP BY / ORDER BY / window / DISTINCT / LIMIT)
runs on empty aRows when block mode fires — all are no-ops on
empty input, so the sequence stays harmless.
* RunSelect returns NIL (not {fields, rows}) when ::bRowBlock was
used — signals "streaming semantics, all work done in the block".
Complex queries (JOIN, GROUP BY, subquery, window, ORDER BY not
matchable by an index, LIMIT/OFFSET, etc.) still fall back to the
array path even when a block is supplied — those genuinely require
materialization. Block mode is a fast-path opt-in, not a semantic
change.
End-to-end bench (50k rows, steady state — includes the user-side
loop/block for every row):
Path Time Speedup vs raw
──────────────────────────────────────────────────────────────
Raw PRG DO WHILE !Eof() + WHERE sum 7.6ms 1.00x
five_SQL array + FOR 7.7ms ~same
five_SQL + block (new) 3.7ms 2.05x ← beats raw
──────────────────────────────────────────────────────────────
Raw PRG no WHERE 6.1ms 1.00x
five_SQL + block, no WHERE 2.9ms 2.10x ← beats raw
SQL now pays for itself on end-to-end timing — not just competitive
with hand-rolled RDD loops, but faster than them. The layered cost
of FieldGet's Frame+RTL-dispatch that hand-written loops incur per
call is gone; the block-callback path captures *dbf.DBFArea directly
via FastFieldGetter and uses PcOpFieldGet to bypass dispatch in the
compiled WHERE predicate.
Validation:
- FiveSql2 43/43 (array API unchanged)
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous short-circuit (return 0 unconditionally) was a workaround
for two bugs that are both fixed now:
1. gengo PushLocal(0) panic on unresolved identifiers
→ fixed by 08ad6f4 (PushMemvar fallback).
2. dbInfo(DBI_FULLPATH / DBI_SHARED) returning NIL
→ fixed by d74014a (real implementations).
Restoring the original scan: walk workareas 1..250, check if any
holds an exclusive lock on the target DBF. With dbInfo now functional
and the DBI_* constants defined in include/dbinfo.ch (commit 3a00aa5),
this gives FiveSql2 real pre-flight conflict detection for concurrent
table access rather than silently proceeding into a lock failure.
Validation:
- FiveSql2 43/43
- standalone PRG with dbUseArea + five_SQL works (was the original
repro that triggered the workaround)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements hybrid execution model: keep AST tree-walk for SQL:2013+
features (Window, Recursive CTE, JOIN, aggregates) while compiling
simple SELECT hot paths to Go + pcode. See docs/FiveSql2-Hybrid-Plan.md
for the full architecture rationale (why not SQLite-style VDBE).
Hot path (single table, no joins/groups/aggregates):
- TryBuildFieldPositions: resolves SELECT column list to FieldPos
array once per query (bails to PRG loop on any complex expr).
- TryCompileWhere + SqlExprToPrg: walks WHERE AST, emits equivalent
PRG source, runs it through PcCompile to get a PcodeFunc.
- SqlScan RTL: Go-native scan loop — GoTop/EOF/Skip/GetValue
direct, ExecPcode per row for WHERE, result array pre-alloc.
WHERE compiler scope:
- ND_LIT numeric/logical/string (string literals AllTrim'd to match
SqlCmpEq CHAR-padding semantics; rejects embedded quotes/newlines)
- ND_COL: CHAR fields auto-wrapped with AllTrim(FieldGet(n)) based
on dbStruct() lookup cached once per query in aCompileStruct
- ND_BIN: = <> != < <= > >= AND OR + - * /
- ND_UNI: NOT -
- Anything else (ND_FN, ND_CASE, ND_SUB, ND_PAR, LIKE, IN, IS NULL,
BETWEEN, dates) returns NIL → falls back to PRG tree-walk.
Bench (50k rows, ~/tmp ext4):
Before After Speedup
Numeric WHERE ~150ms 11.7ms ~13x
String WHERE 119.3ms 10.5ms 11.4x
No WHERE - 14.6ms -
Raw RDD baseline 6.8ms 6.8ms 1.0x
Remaining gap to raw RDD (~1.5x) is structural: Value boxing, result
array construction, per-row ExecPcode frame overhead. Would need a
Value-pool or SoA refactor to close further.
Side fixes bundled:
- TSqlIndex:FindExclusive short-circuited. Originally called
dbInfo(DBI_FULLPATH)/DBI_SHARED which are unresolved symbols in
Five (dbInfo is a stub, DBI_* never defined). Panic'd with
"local variable index out of range: 0" whenever a standalone PRG
had a workarea Used before calling five_SQL. 43-test masked the
bug because it only reached FindExclusive with no open workareas.
Restore the scan once dbInfo lands in hbrtl.
- cmd/five/main.go: FIVE_KEEP_BUILD=1 env var keeps the temp Go
project around for debugging gengo output.
Validation:
- FiveSql2 43/43
- Harbour compat 51/51
- go test ./... ALL PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
