Repository navigation
exec, plan-codec: existence join for IN/EXISTS subqueries (P2-T115 unit 1) - #237
Merged
Merged
Conversation
General subquery decorrelation needs two more join forms in the executor: a mark join for EXISTS/IN subqueries that are not top-level filter conjuncts, and a single-match left outer join for correlated scalar subqueries without an aggregate. This change adds both to the existing hash join build/probe code and its grace-spill path, with their plan data and one new codec feature. The analyzer does not produce them yet. MarkExists emits every probe row once with a non-null boolean mark (TRUE on a match, else FALSE). MarkIn (one key, no null-safe keys) is TRUE on a match, else NULL when the probe key is NULL or the build side holds a NULL key, else FALSE; an empty build side gives FALSE for every row, NULL keys included, matching Spark's default nullInEmptyListBehavior=false. A residual condition, when present, decides matches as in the other join types (NULL is not a match). A LeftOuter join with single_match set fails the query with SCALAR_SUBQUERY_TOO_MANY_ROWS (SQLSTATE 21000) when a probe row matches more than one build row. Weighted build sides are rejected exactly like the other non-inner join types; probe weights pass through. The plan codec carries the new join types and the single-match flag of HashJoinNode under one new semantic feature, mark_join. Nested-loop joins refuse mark joins; reorientation and role-reversed grace joins never apply to them. Unverified: diff coverage was not tool-measured (new paths are exercised by the tests below, except one defensive unreachable arm in the mark tail). The legacy nullInEmptyListBehavior=true semantics (NULL for NULL IN (empty)) is deliberately not followed: the task pins FALSE for an empty build. The mark output column is named mark; no analyzer consumer exists yet. Signed-off-by: techysneha14 <343708768+shehatathe@users.noreply.github.com>
Spark 4.2 plans EXISTS and IN outside a filter conjunct as an ExistenceJoin whose exists column is never NULL: k IN (subquery) is false, not NULL, when the subquery holds a NULL or k is NULL, in the SELECT list, under OR, in CASE, GROUP BY, aggregate FILTER and join conditions alike. NOT IN is the negated existence join on (k = c) OR isnull(k = c), by nested loops when uncorrelated and with the correlation keys as hash keys otherwise. Replace MarkExists and MarkIn with one Existence join type that follows this definition. A probe row's exists depends only on the build rows with its key, so the build-wide NULL and emptiness facts are gone, the residual condition decides per pair, and the join is correct broadcast or partitioned. The nested-loop join runs it too. The plan format follows design P2-T166: JOIN_TYPE_EXISTENCE under the existence_join feature, the single-match flag under its own join_single_match feature, and an ignorable distinct_build flag, validated in the decoder and at instantiation. Orientation no longer turns a single-match left outer join into a right outer join. Tests use a reference model written from Spark's definition, replay the Spark 4.2.0 results in the shapes Spark plans, replay in-null-semantics and exists-outside-filter from tests/spark-golden, and check co-partitioned execution against one join. Signed-off-by: gopal tathe <44522021+techgopal@users.noreply.github.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Join and plan contracts crates/krishiv-exec/src/join/mod.rs, crates/krishiv-exec/src/physical/mod.rs, proto/krishiv/plan/v1/plan.proto, crates/krishiv-plan-codec/src/proto/krishiv.plan.v1.pb.rs |
The join and plan definitions add Existence, single_match, and distinct_build. They define existence output as probe columns plus a non-nullable Boolean and constrain single-match and distinct-build configurations. |
Planning and plan instantiation crates/krishiv-exec/src/physical/instantiate.rs, crates/krishiv-exec/src/planner/join.rs, crates/krishiv-exec/src/planner/orient.rs, crates/krishiv-exec/tests/planner/physical.rs |
Plan construction initializes the new flags. Orientation does not flip single-match left outer joins. Instantiation rejects distinct-build plans that do not meet the eligibility rules. |
Existence and single-match execution crates/krishiv-exec/src/join/nested.rs, crates/krishiv-exec/src/join/probe.rs, crates/krishiv-exec/tests/hash_join/*, crates/krishiv-exec/benches/hash_join.rs, crates/krishiv-exec/examples/join_index_matrix.rs, crates/krishiv-exec/src/join/grace.rs |
Hash and nested-loop joins emit existence results. Hash joins in single-match mode return a scalar-subquery error when a probe row has multiple qualifying matches. Tests cover null handling, residuals, spilling, weights, partitions, randomized inputs, and Spark-shaped cases. |
Plan encoding and feature checks crates/krishiv-plan-codec/src/{decode.rs,encode.rs,features.rs}, crates/krishiv-plan-codec/tests/{common/inventory.rs,features.rs,roundtrip.rs} |
The codec reads and writes the new join type and flags. Feature checks classify existence and single-match joins, and decoding rejects invalid flag combinations. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Possibly related PRs
- KrishivAI/krishiv-engine#166: Adds hash-join orientation and right-side join variants. This change updates the orientation contract for existence joins and single-match left outer joins.
Merge Risk: 
7bc05
-
No actionable issue is established for the new join behavior or plan compatibility. The change appears mergeable after normal checks.
Security architecture risk: 
7bc05
-
The new join modes change query results and saved-plan compatibility. Validation and reader checks constrain the change, and no introduced security vulnerability was established. Cancellation, concurrent reuse, and production exposure remain incompletely verified.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The demonstrated exposure is callers already able to supply executable plans or construct joins, with effects on query results, cardinality errors, and query spill resources. These paths do not establish a new tenant, credential, or service authority boundary; production exposure remains unverified.
Trust Boundaries and Controls
- observed — Serialized feature declarations are not accepted as sufficient authority to enable semantics: the decoder checks reader support and independently derives the features used by the decoded plan, rejecting mismatches and understated legacy reader requirements.
Resilience and Maintainability Implications
- observed — Spill ownership has drop-based cleanup, and the added duplicate-match spill test asserts both memory release and spill-file deletion after failure. Forced-spill existence tests compare results in both key layouts. These checks do not establish cancellation-mid-replay or concurrent-reuse behavior.
- observed — Whole-input-batch tail output predates this PR in semi, anti, and unmatched outer joins. The new existence tail follows that pattern and emits no more than its input rows. The documented batch-size discrepancy is therefore not established as a new maximum-scope security exposure.
Hardening Proposals
- proposed — Consider enforcing and asserting the configured output-batch bound across all tail-emitting joins, including grace replay. This would strengthen resource-containment guarantees for an existing behavior, not remediate a verified PR-introduced vulnerability.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Description check The description provides a detailed summary, task reference, Spark behavior, and test coverage. It does not include the required Tests run section with commands and result lines, and it omits the requ… Add a Tests run section with the exact commands executed and their result lines. Add and complete the repository checklist, including DCO, licensing, test coverage, error handling, distributed execution, API, dependency, and performance ite… Docstring Coverage Docstring coverage is 55.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 22 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly identifies the main change: adding existence joins for IN/EXISTS subqueries in the execution and plan-codec components. It is concise and specific. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Description check
-
Explanation
The description provides a detailed summary, task reference, Spark behavior, and test coverage. It does not include the required Tests run section with commands and result lines, and it omits the required checklist.
Resolution
Add a Tests run section with the exact commands executed and their result lines. Add and complete the repository checklist, including DCO, licensing, test coverage, error handling, distributed execution, API, dependency, and performance items.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 55.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 22 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance1 warning
Comparing 
Footnotes
|
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
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.
Existence and single-match hash joins (P2-T115 unit 1, design P2-T166 §3.2)
Adds the physical join forms general subquery decorrelation needs; the analyzer does not produce them yet.
JoinType::Existence(Spark'sExistenceJoin): every probe row once plus a non-nullable booleanexists, TRUE when some build row makes the whole condition (keys and residual) TRUE. Runs in the hash join (in memory and grace spill, both key formats) and the nested-loop join.single_matchon aLeftOuterhash join: a second match for a probe row fails withSCALAR_SUBQUERY_TOO_MANY_ROWS(SQLSTATE 21000). Orientation never flips a single-match join toRightOuter.JOIN_TYPE_EXISTENCE(featureexistence_join),single_match(featurejoin_single_match),distinct_build(ignorable, validated: left semi/anti/existence without a residual; the executor still stores every row).Spark 4.2.0 results (ANSI on and off identical)
t(id,k)=(1,1),(2,2),(3,NULL),(4,4),un={1,NULL},nn={1,2},e={},c(g,k)=(1,1),(2,NULL),(4,9),(3,5):k IN (SELECT k FROM un)k IN nnk IN ek NOT IN unk NOT IN nnk NOT IN e(k IN un) IS NULLCASE WHEN k IN un THEN 'y' WHEN NOT (k IN un) THEN 'n' ELSE 'u' END(k IN un) OR k = 2k IN (SELECT c.k FROM c WHERE c.g = t.id)k NOT IN (SELECT c.k FROM c WHERE c.g = t.id)EXISTS (SELECT 1 FROM c WHERE c.g = t.id AND c.k = t.k)The same values appear under
ORin WHERE, in GROUP BY, in an aggregate FILTER and in a join ON condition. An IN subquery in ORDER BY is rejected (UNSUPPORTED_IN_EXISTS_SUBQUERY). Spark's plans:INis a hashExistenceJoinon the key; uncorrelatedNOT INis NOT of a nested-loopExistenceJoinon(k = s.k) OR isnull(k = s.k); correlatedNOT INis NOT of a hashExistenceJoinon the correlation keys with that residual. A literal NULL probe and one-row literal subqueries are folded before any join (in-nullability.sql.out); that belongs to the analyzer unit.Tests replay these statements in Spark's plan shapes, plus
in-null-semantics.sql.outandexists-outside-filter.sql.outfromtests/spark-golden. A reference model written from Spark's definition is checked on edge cases, random data, spill, and co-partitioned execution.