Skip to content

fix: remove arbitrary physical candidate limits - #730

Open
zzylol wants to merge 175 commits into
mainfrom
fix/708-mask-routing
Open

zzylol wants to merge 175 commits into
mainfrom
fix/708-mask-routing

Conversation

@zzylol

@zzylol zzylol commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #708. Stacked on #749; review this PR after the backend plan/SDS refactor.

Physical alternatives are sets of materialization keys, so neither the 64-candidate rejection nor the 63-set enumeration cutoff reflects a representation limit. This change removes both limits and the redundant deduplication for distinct large-inventory choices.

With 100 optional keys, candidate generation now produces 202 materialization choices plus native execution, and selection considers every supplied alternative. Search remains exhaustive through four keys; larger inventories consider all/none and every singleton/complement pair (2 + 2N). That larger search is intentionally non-exhaustive and may increase compilation and pricing work.

This branch includes the updated #749 foundation. Its review diff contains only control_plane/src/physical/workload_cost.rs and control_plane/src/physical/workload_cost/materialization_candidates.rs.

Verification on the stacked branch: cargo test -p control_plane --locked passed (426 library tests and integration targets); cargo fmt --all --check and git diff --check passed.

@zzylol
zzylol force-pushed the fix/708-mask-routing branch from d8995dc to 16f75d2 Compare September 21, 2026 17:31
@zzylol
zzylol changed the base branch from main to refactor/backend-plan-split September 21, 2026 17:31
zzylol and others added 29 commits September 28, 2026 17:27
CompileAndPublishPhysicalPlanRequest gained a required `dataset_identity`
field in this branch, but two api_tests fixtures build their request body as
JSON by hand and were never updated, so both failed deserialization with
`missing field dataset_identity` before reaching the handler.

Take the value from the planning snapshot's own `environment.dataset_identity`
rather than inventing one, so the fixture keeps describing the same dataset the
rest of the snapshot describes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zzylol
zzylol changed the base branch from refactor/backend-plan-split to main September 28, 2026 18:42
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.

Why are physical alternatives capped at 64?

1 participant