Skip to content

fix(ir): make each '_' in a rule body a fresh anonymous variable - #219

Merged
jsam merged 2 commits into
mainfrom
fm/il-fix-wildcard
Oct 3, 2026
Merged

jsam merged 2 commits into
mainfrom
fm/il-fix-wildcard

Conversation

@jsam

@jsam jsam commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

In a rule body, every _ must be a fresh anonymous variable. Lowering named a placeholder column after its relation and position (_ph_{relation}_{i}). When two body atoms read the same relation with _ at the same position, both scans got the same column name, and the join used it as a key. Results were silently wrong, for session and persistent rules alike:

+user[(1,"ann",true),(2,"bob",false),(3,"cy",true)]
+w(A,B) <- user(A,_,_), user(B,_,_)
?w(A,B)                              -- 3 rows (diagonal) instead of 9

+e[(1,2,"a"),(2,3,"b"),(3,4,"c")]
+two(X,Z) <- e(X,Y,_), e(Y,Z,_)
?two(X,Z)                            -- 0 rows instead of (1,3),(2,4)

The plain query ?user(A,_,_), user(B,_,_) looked correct only because the query path renames the goal atom's _ to _p{i} before lowering.

Fix

build_scan now names each placeholder per body atom, _ph_a{atom}_c{i}, the same scheme constants already use. The _ph_ prefix is unchanged, so antijoin key inference still skips placeholder columns.

Not a #206 regression

This is a long-standing bug. The _ph_{relation}_{i} naming dates from 2024 (5cf9a2c). The new regression tests fail 7/7 on 4dd8fe9, the commit just before #206 (277c7f0), the same as on main.

Tests

  • tests/wildcard_rule_tests.rs covers both repros above, plus session rules, persistent rules after a restart, wildcards in negated atoms, wildcards under an aggregate, and wildcards in a recursive rule. All 7 tests fail on main without the fix and pass with it.
  • ir_builder unit test: in a self-join, the _ columns are not join keys.
  • Snapshot spec: examples/iql/06_joins/79_wildcard_fresh_per_atom.iql.
  • Two existing snapshots had recorded the wrong rows and are corrected:
    • 18_advanced_patterns/25_running_total: the running count is now 1..5 instead of all 1s.
    • 18_advanced_patterns/30_self_comparison: higher_than now returns its 5 rows instead of none.
  • The debug snapshots 33_meta/17_debug_join and 26_debug_complex show the new placeholder column names.

Validation

  • make test: fmt, clippy and unit/integration tests pass. In the snapshot suite, 1149 pass and 3 fail. The 3 failures are already on main and unrelated to this PR: 33_meta/04_help, 33_meta/11_help_specific and 39_meta_complete/07_help_comprehensive. feat(storage): consistent online checkpoint export (.backup) (#48) #216 added .backup to the client help without regenerating them.

jsam added 2 commits October 3, 2026 22:07
Lowering named a placeholder column after its relation and position
(_ph_{relation}_{i}), so two body atoms over one relation with '_' at the
same position joined on it. w(A,B) <- user(A,_,_), user(B,_,_) returned
only the diagonal and two(X,Z) <- e(X,Y,_), e(Y,Z,_) returned nothing,
for session and persistent rules alike. Name placeholders per atom
(_ph_a{atom}_c{i}), like constants already are.

The naming dates from 2024, so this is long-standing, not a #206
regression.
25_running_total and 30_self_comparison had recorded the wrong rows
(counts of 1, no results) produced by joined placeholders. The debug
snapshots show the new per-atom placeholder column names.
@jsam

jsam commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Performance gate (make perf-gate, 10 rounds, shared host under load)

Verdict: INCONCLUSIVE, with no FAIL. Every required metric's point estimate is within budget. The change is to compile-time column naming only. Query latency is unchanged: bound_query p50 1.003, p99 0.970; cheap_query p50 1.006, p99 0.954. The INCONCLUSIVE intervals and INVALID sample counts come from host load (load average 12–22, server CPUs unpinned; see #198).

Full report

Performance gate: INCONCLUSIVE

Policy provisional: cost budget p50 +5%, p99 +10%, throughput -5%; 95% interval of the median paired per-round ratio. Budgets are the plan's proposed ceilings, pending captain decision D1; tightening is always allowed.

  • baseline: e732cd5258c74d9247e99b2697a0ae080ccf9042 (binary sha256 a163777901b8)
  • candidate: 2d9e8067acc4d0d517dcd302ab41183c17f732fe (binary sha256 d1025f8dccd7)
  • profile standard, 10 rounds, fixtures cheap_query, bound_query, insert_single, insert_batch, delta_single, delta_fanout, delta_first, interference
  • host computeinstance-e00sx5ndp6kwb7pqfv: AMD EPYC-Genoa Processor (32 CPUs, 125 GB), kernel 6.11.0-1016-nvidia, governor unknown, data on ext4 (/home/sam/.treehouse/inputlayer-6d7bcd/1/inputlayer/target/perf-gate/runs/20261003T223557Z/servers), server CPUs unpinned
  • load average: start 12.56 16.18 22.71 4/1920 2891625, end 11.93 9.45 15.27 13/2033 3127361
  • build: rustc 1.99.0 (b940084d7 2026-09-28); cargo build --release --all-features

Required metrics

metric baseline candidate cost ratio interval budget baseline spread status
bound_query.concurrent_latency_us.p99 12.872 ms 12.810 ms 0.995 0.925..1.025 1.10 6.4% PASS
bound_query.latency_us.p50 8.312 ms 8.294 ms 1.003 0.986..1.021 1.05 4.3% PASS
bound_query.latency_us.p99 9.007 ms 8.733 ms 0.970 0.941..1.010 1.10 19.9% PASS
bound_query.queries_per_sec 836/s 815/s 0.975 0.957..1.096 1.05 22.4% INCONCLUSIVE
cheap_query.latency_us.p50 0.432 ms 0.445 ms 1.006 0.975..1.069 1.05 10.4% INCONCLUSIVE
cheap_query.latency_us.p99 0.524 ms 0.516 ms 0.954 0.829..1.114 1.10 39.1% INCONCLUSIVE
cheap_query.queries_per_sec 9467/s 9392/s 0.996 0.963..1.034 1.05 12.9% PASS
delta_fanout.delta_us.p50 13.256 ms 12.403 ms 0.921 0.872..0.951 1.05 519.2% PASS
delta_fanout.delta_us.p99 22.252 ms 22.058 ms 0.968 0.718..1.394 1.10 864.8% INCONCLUSIVE
delta_fanout.last_agent_us.p99 26.940 ms 25.454 ms 0.921 0.661..1.372 1.10 860.2% INCONCLUSIVE
delta_first.first_delta_us.p50 40.674 ms 3.835 ms 0.094 0.091..0.096 1.05 1.1% PASS
delta_first.warm_delta_us.p50 4.559 ms 4.535 ms 0.980 0.951..1.063 1.05 31.4% INCONCLUSIVE
delta_single.delta_us.p50 7.151 ms 5.476 ms 0.764 0.735..0.884 1.05 50.5% PASS
delta_single.delta_us.p99 17.552 ms 12.487 ms 0.687 0.576..1.088 1.10 90.2% PASS
insert_batch.ack_us.p50 8.086 ms 3.376 ms 0.402 0.363..0.463 1.05 51.0% PASS
insert_batch.facts_per_sec 43052/s 192880/s 0.224 0.203..0.235 1.05 5.1% PASS
insert_single.ack_us.p50 7.495 ms 1.425 ms 0.186 0.143..0.206 1.05 470.0% PASS
insert_single.ack_us.p99 42.136 ms 4.875 ms 0.116 0.073..0.162 1.10 37.1% PASS
insert_single.concurrent_facts_per_sec 178/s 743/s 0.237 0.224..0.262 1.05 11.4% PASS
insert_single.facts_per_sec 47/s 643/s 0.072 0.069..0.077 1.05 16.5% PASS
interference.delta_us.p50 7.242 ms 6.018 ms 0.830 0.794..0.877 1.05 274.6% PASS
interference.delta_us.p99 15.208 ms 13.152 ms 0.663 0.488..1.118 1.10 896.9% INCONCLUSIVE

Diagnostic metrics (not gated)

metric baseline candidate cost ratio interval budget baseline spread status
bound_query.concurrent_latency_us.p50 8.302 ms 8.552 ms 1.030 1.000..1.038 1.05 8.2% PASS
cheap_query.concurrent_latency_us.p50 0.810 ms 0.818 ms 0.995 0.961..1.050 1.05 14.6% INCONCLUSIVE
cheap_query.concurrent_latency_us.p99 1.335 ms 1.313 ms 0.988 0.944..1.080 1.10 13.1% PASS
delta_fanout.ack_us.p50 5.663 ms 4.315 ms 0.771 0.713..0.783 1.05 1217.5% PASS
delta_fanout.ack_us.p99 11.193 ms 9.973 ms 0.856 0.707..1.118 1.10 1547.1% INCONCLUSIVE
delta_fanout.last_agent_us.p50 19.027 ms 16.717 ms 0.868 0.834..0.887 1.05 445.9% PASS
delta_fanout.subscribe_us.p50 3.496 ms 3.510 ms 1.006 0.923..1.022 1.05 34.5% PASS
delta_fanout.subscribe_us.p99 - - - - 1.10 - INVALID: baseline round 1: 64 samples, need 100
delta_first.first_delta_us.p99 - - - - 1.10 - INVALID: baseline round 1: 40 samples, need 100
delta_first.subscribe_us.p50 3.538 ms 3.510 ms 0.992 0.987..1.017 1.05 5.4% PASS
delta_first.subscribe_us.p99 - - - - 1.10 - INVALID: baseline round 1: 40 samples, need 100
delta_first.warm_delta_us.p99 - - - - 1.10 - INVALID: baseline round 1: 40 samples, need 100
delta_single.ack_us.p50 11.139 ms 3.142 ms 0.286 0.237..0.407 1.05 92.5% PASS
delta_single.ack_us.p99 42.102 ms 10.171 ms 0.242 0.216..0.349 1.10 6.4% PASS
delta_single.last_agent_us.p50 7.151 ms 5.476 ms 0.764 0.735..0.884 1.05 50.5% PASS
delta_single.last_agent_us.p99 17.552 ms 12.487 ms 0.687 0.576..1.088 1.10 90.2% PASS
delta_single.subscribe_us.p50 - - - - 1.05 - INVALID: baseline round 1: 1 samples, need 20
delta_single.subscribe_us.p99 - - - - 1.10 - INVALID: baseline round 1: 1 samples, need 100
insert_batch.ack_us.p99 - - - - 1.10 - INVALID: baseline round 1: 20 samples, need 100
insert_single.concurrent_ack_us.p50 40.119 ms 4.915 ms 0.132 0.121..0.539 1.05 86.6% PASS
insert_single.concurrent_ack_us.p99 42.045 ms 13.243 ms 0.317 0.196..0.330 1.10 1.7% PASS
interference.ack_us.p50 10.618 ms 3.447 ms 0.338 0.238..0.552 1.05 334.3% PASS
interference.ack_us.p99 42.059 ms 10.220 ms 0.241 0.156..0.260 1.10 247.7% PASS
interference.long_request_us.p50 187.781 ms - - - 1.05 0.0% INVALID: baseline round 2: 19 samples, need 20
interference.long_request_us.p99 - - - - 1.10 - INVALID: baseline round 1: 20 samples, need 100

@jsam
jsam marked this pull request as ready for review October 3, 2026 22:47
@jsam
jsam merged commit a6fd628 into main Oct 3, 2026
14 checks passed
@jsam
jsam deleted the fm/il-fix-wildcard branch October 3, 2026 22:47
jsam added a commit that referenced this pull request Oct 4, 2026
…ry request (#249)

The release-checkpoint performance gate fails on main: cheap_query serial
p50 is 1.110x the approved baseline e732cd5 on the 32-vCPU runner
(115 -> 126 us). Concurrent latency and throughput did not move. Bisected
to #214 (per-connection request pipelining), made worse by #222.

Cause: the pipeline runs requests in a `FuturesUnordered`. Its `poll_next`
wakes its own task and returns `Pending` after any poll that polled every
future in the set. With one request in flight, which is every serial
client, that happens on each request's first poll. The connection task
wakes itself, tokio schedules that as a yield, and a yield wakes an idle
worker to take the task. Each request therefore costs one more context
switch (4.0 -> 5.0 per query) and a hand-off of the connection task to
another core. #222 added no work per query but made the hand-off dearer:
user cycles per query rose 15% at the same instruction count.

Fix: keep one never-woken future in the set. After the pipeline's first
poll it is never polled again, so no poll reaches every future and the
forced yield no longer fires. The yield after futures that wake
themselves still applies. Pipelining, ordering, deadlines, cancellation
and the credential fence are unchanged. Nothing is added per request: no
lock, atomic or allocation. Each connection gets one more set node.

Bisect (cheap_query, `?edge(1, Y)`, serial). Server counters per query
from perf stat. Ratios are against e732cd5, from an interleaved probe
with one pinned server per commit, 40 rounds:

| commit | PR | user instr/query | ctx switches/query | p50 ratio |
|---|---|---|---|---|
| e732cd5 | baseline | 1.282M | 4.0 | 1.000 |
| d75ce6e | #208 atomic programs | 1.279M | 4.0 | 0.993 |
| 4578fe6 | #215 notification stream | 1.271M | 4.0 | 0.991 |
| 71a4d24 | #216 online backup | 1.275M | 4.0 | 1.016 |
| 2f621c5 | #200 first-update latency | 1.272M | 4.0 | 1.009 |
| e96df5c | #207 | 1.279M | 4.0 | 0.998 |
| 4dd8fe9 | #209 shard index | 1.272M | 4.0 | 0.987 |
| 277c7f0 | #206 | 1.274M | 4.0 | 1.008 |
| 3554c98 | #214 pipelining, deadlines | 1.296M | 5.0 | 1.020 |
| 3492a59 | #213 | 1.294M | 5.0 | 1.030 |
| 2ba9a5a | #212 provenance | 1.297M | 5.0 | 1.029 |
| 9ed61b4 | #204 key expiry | 1.297M | 5.0 | 1.027 |
| be35ee3 | #221 | 1.297M | 5.0 | 1.013 |
| c98a590 | #222 bounded streaming | 1.298M | 5.0 | 1.047 |
| a6fd628 | #219 wildcard fix | 1.297M | 5.0 | 1.061 |
| b7f7ad6 | #220 WAL durability | 1.300M | 5.0 | 1.056 |
| de54807 | #223 shared standing eval | 1.299M | 5.0 | 1.071 |
| 55ec34d | #225 checkpoint-1 fixes | 1.300M | 5.0 | 1.045 |
| 23712b8 | main | 1.300M | 5.0 | 1.064 |
| this change | | 1.300M | 4.0 | 1.033 |

Confirming A/Bs with the gate's own fixture (fresh pinned servers each
round, 40 rounds, ratio [95% interval]):

- #214 vs its parent 277c7f0: 1.039 [1.027..1.049]
- #222 vs its parent be35ee3: 1.025 [1.006..1.044]
- A/A 3554c98: 1.010 [1.002..1.025]
- e732cd5 -> main: 1.048 [1.028..1.066]
- main -> this change: 0.978 [0.958..0.989]
- e732cd5 -> this change: 1.021 [1.009..1.044], PASS (budget 1.05)

User cycles per query (perf stat, same instruction count): e732cd5 958K,
be35ee3 1053K, c98a590 1206K, this change 1006K.

Full gate, all fixtures, 20 rounds, servers pinned to CPUs 2-15:

- main -> this change: no metric worse. Every required metric is PASS, or
  INCONCLUSIVE with an interval that includes 1.0. cheap_query p50 0.979
  [0.953..1.007].
- e732cd5 -> this change: cheap_query p50 1.029 [1.019..1.072],
  INCONCLUSIVE on a busy host (load average up to 22); bound_query p50
  1.022, PASS. The insert and delta metrics improved since the baseline.

The shared host is about 4x slower per query than the CI runner, so the
same hand-off is a smaller share of each query here than there.

The remaining ~1.7% of user instructions per query since #214 is the
pipeline's own per-request bookkeeping: the boxed request future (one
4-8 KiB allocation), its set node, the request control and the in-flight
maps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant