Repository navigation
Researching - #3568
Open
Hydrocharged wants to merge 68 commits into
Open
Researching#3568Hydrocharged wants to merge 68 commits into
Hydrocharged wants to merge 68 commits into
Conversation
Contributor
|
Hydrocharged
force-pushed
the
daylon/rust-port
branch
from
October 8, 2026 22:45
0df5f63 to
4efbc0d
Compare
Contributor
|
Mini sysbench, main 34f8e0f (Go) against 288c0a0 (Rust), with Postgres 15 for comparison: Reads
Writes
|
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
Hydrocharged
force-pushed
the
daylon/rust-port
branch
from
October 9, 2026 01:20
41ed83c to
f09a0d8
Compare
…regates, and user-defined forms, polymorphic user functions, and positional PL/pgSQL parameters
…ngs of the storage fixtures, and left column numbers out of the node client comparison
…ptions, reported view updatability in information_schema, and deferred view privilege checks to use
…mits by date never ties them
…rms, the two-array json_object forms, and jsonb_delete, and expanded composite results of functions in FROM
…ors, and the tsvector functions that need no parser
…RDER BY, and LIMIT
…eprocess_expression does
…Scan paths, and preprocess_aggrefs
|
Diff SummaryThe run covers core query behavior across joins, filtering, grouping, ordering, subqueries, indexing, window results, NULL and missing values, and set expansion, including both normal workflows and boundary cases. Most exercised behavior remains healthy, but a newly supported correlated query form still fails during planning rather than returning results. Merge with caution — the PR introduces a medium-severity failure in correlated quantified subqueries, making a supported query pattern unusable, while the separate NULL-boundary panic is unrelated to this PR and is a flag for later. The attributable issue is limited in scope but should be weighed before merging. Tests run by Ito
Tests that are no longer relevantBelow are tests that previously ran and are no longer relevant:
Additional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟡 NULL LIMIT crashes the SQL session
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
| e | ||
| } | ||
|
|
||
| /// process_sublinks turns each subquery expression of an expression over a row of `width` columns into a SubPlan, |
There was a problem hiding this comment.
🆕 New Failure: identified in this diff run
Correlated ANY subqueries fail during planning
What failed: Running a query with a correlated ANY subquery shows a planner error and returns no results. The other correlated checks and the independent IN/count checks return the expected values.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: Applications using a correlated ANY subquery receive a database error instead of query results. Other tested subquery forms continue to work, and no data loss or corruption was observed.
- Steps to Reproduce:
- Create the sub_outer and sub_inner SQL fixtures with outer keys and matching inner keys.
- Run a SELECT that includes
o.k = ANY (SELECT i.k FROM sub_inner i WHERE i.k >= o.k)together with the correlated EXISTS and scalar subqueries. - Observe that planning stops with
expression.subqueryAnyExpr: expected right child to return 3 values but returned 2instead of returning one result per outer row.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: The PR changed crates/sql/src/optimizer/subselect.rs to process every
AnySubquerythroughmake_subplanat lines 328-335.make_subplanreplaces correlated outer references with SubPlan arguments at lines 72-83, andbuild_subplanstores the linked AnySubquery and its arguments at lines 131-158. The PR also addedSubPlanin crates/sql/src/expr.rs at lines 132-160; itsevalrecomputes arguments for the current enclosing row and then evaluates the linked subquery. This is the production path exercised by the failing correlated quantified query. The observed planner error says the Any expression receives a right child with two values when it expects three, which is a result-shape/field-mapping failure in this newly added subquery planning path, not a database setup failure. A targeted fix should preserve the single-column right-hand result for this scalar ANY query and only construct field comparisons when the subquery actually returns a matching row shape. - Why this is likely a bug: The SQL form is a normal correlated quantified subquery: its right side returns one column, and the outer value should be compared with that column for each outer row. Instead, the planner raises an internal result-shape error before execution. The repository's newly added subquery code explicitly claims to support AnySubquery and per-row SubPlan arguments, and the same run proves that the database and fixtures can execute the neighboring subquery forms. The smallest practical remediation is to correct the correlated ANY target/result mapping in the new SubPlan path, then add a regression query covering a one-column correlated ANY subquery.
Relevant code
crates/sql/src/optimizer/subselect.rs:328-335
pub fn process_sublinks(ctx: &mut Ctx<'_>, e: Expr, width: usize) -> Expr {
match e {
link @ (Expr::Exists(_) | Expr::Scalar(_) | Expr::ArraySubquery(..) | Expr::AnySubquery(..)) => {
let link = link.map_children(&mut |c| process_sublinks(ctx, c, width));
make_subplan(ctx, link, false, width)
}crates/sql/src/optimizer/subselect.rs:131-158
fn build_subplan(ctx: &Ctx<'_>, mut link: Expr, plan: Plan, mut path: Option<Rc<Path>>, args: Vec<Expr>) -> SubPlan {
let init_plan = args.is_empty() && !matches!(link, Expr::AnySubquery(..));
let use_hash_table = match (&link, &path) {
(Expr::AnySubquery(test, _, false), Some(path)) => {
args.is_empty() && subpath_is_hashable(path) && testexpr_is_hashable(test)
}
_ => false,
};
...
*link.subquery_mut().expect("a subquery expression") = crate::plan::share_subquery(plan, uncorrelated);
let mut subplan = SubPlan { link, args, init_plan, planned: true, startup_cost: 0.0, per_call_cost: 0.0 };crates/sql/src/expr.rs:147-152
fn eval(&self, ctx: &mut Ctx<'_>, row: &[Value]) -> Result<Value> {
let args = self.args.iter().map(|a| a.eval(ctx, row)).collect::<Result<Vec<Value>>>()?;
self.link.eval_sublink(ctx, row, &args)
}Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Correlated ANY subqueries fail during planning**
**What failed:** Running a query with a correlated ANY subquery shows a planner error and returns no results. The other correlated checks and the independent IN/count checks return the expected values.
- **Impact:** Applications using a correlated ANY subquery receive a database error instead of query results. Other tested subquery forms continue to work, and no data loss or corruption was observed.
- **Steps to reproduce:**
1. Create the sub_outer and sub_inner SQL fixtures with outer keys and matching inner keys.
2. Run a SELECT that includes `o.k = ANY (SELECT i.k FROM sub_inner i WHERE i.k >= o.k)` together with the correlated EXISTS and scalar subqueries.
3. Observe that planning stops with `expression.subqueryAnyExpr: expected right child to return 3 values but returned 2` instead of returning one result per outer row.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR changed crates/sql/src/optimizer/subselect.rs to process every `AnySubquery` through `make_subplan` at lines 328-335. `make_subplan` replaces correlated outer references with SubPlan arguments at lines 72-83, and `build_subplan` stores the linked AnySubquery and its arguments at lines 131-158. The PR also added `SubPlan` in crates/sql/src/expr.rs at lines 132-160; its `eval` recomputes arguments for the current enclosing row and then evaluates the linked subquery. This is the production path exercised by the failing correlated quantified query. The observed planner error says the Any expression receives a right child with two values when it expects three, which is a result-shape/field-mapping failure in this newly added subquery planning path, not a database setup failure. A targeted fix should preserve the single-column right-hand result for this scalar ANY query and only construct field comparisons when the subquery actually returns a matching row shape.
- **Why this is likely a bug:** The SQL form is a normal correlated quantified subquery: its right side returns one column, and the outer value should be compared with that column for each outer row. Instead, the planner raises an internal result-shape error before execution. The repository's newly added subquery code explicitly claims to support AnySubquery and per-row SubPlan arguments, and the same run proves that the database and fixtures can execute the neighboring subquery forms. The smallest practical remediation is to correct the correlated ANY target/result mapping in the new SubPlan path, then add a regression query covering a one-column correlated ANY subquery.
**Relevant code:**
`crates/sql/src/optimizer/subselect.rs:328-335`
~~~rust
pub fn process_sublinks(ctx: &mut Ctx<'_>, e: Expr, width: usize) -> Expr {
match e {
link @ (Expr::Exists(_) | Expr::Scalar(_) | Expr::ArraySubquery(..) | Expr::AnySubquery(..)) => {
let link = link.map_children(&mut |c| process_sublinks(ctx, c, width));
make_subplan(ctx, link, false, width)
}
~~~
`crates/sql/src/optimizer/subselect.rs:131-158`
~~~rust
fn build_subplan(ctx: &Ctx<'_>, mut link: Expr, plan: Plan, mut path: Option<Rc<Path>>, args: Vec<Expr>) -> SubPlan {
let init_plan = args.is_empty() && !matches!(link, Expr::AnySubquery(..));
let use_hash_table = match (&link, &path) {
(Expr::AnySubquery(test, _, false), Some(path)) => {
args.is_empty() && subpath_is_hashable(path) && testexpr_is_hashable(test)
}
_ => false,
};
...
*link.subquery_mut().expect("a subquery expression") = crate::plan::share_subquery(plan, uncorrelated);
let mut subplan = SubPlan { link, args, init_plan, planned: true, startup_cost: 0.0, per_call_cost: 0.0 };
~~~
`crates/sql/src/expr.rs:147-152`
~~~rust
fn eval(&self, ctx: &mut Ctx<'_>, row: &[Value]) -> Result<Value> {
let args = self.args.iter().map(|a| a.eval(ctx, row)).collect::<Result<Vec<Value>>>()?;
self.link.eval_sublink(ctx, row, &args)
}
~~~…tch, and DiscreteKnapsack
|
Diff SummaryThe run broadly exercised SQL query correctness across set operations, recursion, aggregation, grouping, joins, correlated subqueries, ordering, boundary conditions, and recovery after errors. Coverage included normal results, duplicate and NULL handling, type combinations, planner-sensitive cases, and adversarial edge conditions, with most tested behavior appearing healthy. Merge with caution — the PR introduces a medium-severity type-handling defect in mixed numeric set-operation queries, causing ordinary filters to fail and affecting valid user queries without an explicit cast. The issue is attributable to this PR and should be considered a merge risk, while the remaining target-build and coverage observations are not additional product failures. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
There was a problem hiding this comment.
🆕 New Failure: identified in this diff run
Mixed numeric set queries reject normal filters
What failed: Filtering the mixed numeric UNION with x >= 2 raises an operator error. The UNION and EXCEPT result columns are reported as text rather than a common numeric type, even though the same rows work when explicitly cast back to bigint.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: Queries that combine different numeric types with UNION or EXCEPT can fail when users apply a normal numeric filter. Users can work around this by adding an explicit cast, but the unmodified query does not return its expected rows.
- Steps to Reproduce:
- Run SELECT x FROM (SELECT 1::smallint AS x UNION SELECT 2::bigint UNION SELECT NULL::bigint) u WHERE x >= 2 OR x IS NULL;.
- Inspect the type with SELECT x, pg_typeof(x) FROM (SELECT 1::smallint AS x UNION SELECT 2::bigint UNION SELECT NULL::bigint) u;.
- Repeat with an EXCEPT query that combines integer and bigint leaves, then inspect pg_typeof(x).
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: The normal planner path establishes a common type for each set-operation column in crates/sql/src/plan.rs:869-899. It calls common_type for both branches, coerces each branch to the selected type, and stores that type in the result columns before constructing Plan::SetOp. The PR's optimizer reconstruction then rebuilds a Query in crates/sql/src/optimizer/query.rs:193-211. At line 208 it takes col_types from rtable.first().coltypes and assigns those types to every set-operation node at lines 209 and 234-240, rather than carrying the set operation's already-resolved common output type through the reconstructed query. The outer set-operation columns are represented as relation-zero variables at query.rs:210, and crates/sql/src/optimizer/nodefuncs.rs:34-40 resolves those variables from parse.set_operations.col_types. The resulting metadata is inconsistent with the original common-type contract, so the executor exposes text values and the comparison binder rejects text >= integer. The smallest targeted fix is to preserve the common output type vector from the planned set operation when constructing set_operations and use it consistently for the reconstructed target list and relation-zero variables; avoid a broad optimizer rewrite.
- Why this is likely a bug: The failure is deterministic for ordinary SQL and is independently confirmed by the type inspection: both UNION and EXCEPT return pg_typeof(x) as text for numeric inputs, while typed post-materialization references return bigint and the casted workaround returns the expected rows. The repository's planner contract says set-operation columns use the common type of both branches, and existing EXCEPT tests expect an integer result column. This is therefore a production type-propagation defect in the PR's new optimizer path, not a browser or connection problem. A targeted correction to carry the common column types into the reconstructed Query should restore numeric predicates without changing set-operation duplicate or direction semantics.
Relevant code
crates/sql/src/plan.rs:869-899
plan_set_operation documents that set-operation columns use the common types of both branches, calls common_type for each pair, coerces each branch to those types, and stores them in the result columns.crates/sql/src/optimizer/query.rs:193-211
set_operation_query reconstructs the set-operation Query, derives col_types from rtable.first().coltypes, propagates them, and creates relation-zero output variables.crates/sql/src/optimizer/nodefuncs.rs:33-42
query_expr_type resolves a relation-zero variable from parse.set_operations.col_types, so incorrect reconstructed metadata controls the type seen by outer predicates.crates/sql/src/optimizer/prepunion.rs:341-347
generate_setop_tlist builds output variables from col_types and assumes the binder already coerced every input to the set-operation column types.crates/tests/tests/scripts/union.rs:35-50
Existing EXCEPT coverage expects the result column to retain the integer type (INT4), establishing that set-operation output is typed rather than generic text.Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Mixed numeric set queries reject normal filters**
**What failed:** Filtering the mixed numeric UNION with x >= 2 raises an operator error. The UNION and EXCEPT result columns are reported as text rather than a common numeric type, even though the same rows work when explicitly cast back to bigint.
- **Impact:** Queries that combine different numeric types with UNION or EXCEPT can fail when users apply a normal numeric filter. Users can work around this by adding an explicit cast, but the unmodified query does not return its expected rows.
- **Steps to reproduce:**
1. Run SELECT x FROM (SELECT 1::smallint AS x UNION SELECT 2::bigint UNION SELECT NULL::bigint) u WHERE x >= 2 OR x IS NULL;.
2. Inspect the type with SELECT x, pg_typeof(x) FROM (SELECT 1::smallint AS x UNION SELECT 2::bigint UNION SELECT NULL::bigint) u;.
3. Repeat with an EXCEPT query that combines integer and bigint leaves, then inspect pg_typeof(x).
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The normal planner path establishes a common type for each set-operation column in crates/sql/src/plan.rs:869-899. It calls common_type for both branches, coerces each branch to the selected type, and stores that type in the result columns before constructing Plan::SetOp. The PR's optimizer reconstruction then rebuilds a Query in crates/sql/src/optimizer/query.rs:193-211. At line 208 it takes col_types from rtable.first().coltypes and assigns those types to every set-operation node at lines 209 and 234-240, rather than carrying the set operation's already-resolved common output type through the reconstructed query. The outer set-operation columns are represented as relation-zero variables at query.rs:210, and crates/sql/src/optimizer/nodefuncs.rs:34-40 resolves those variables from parse.set_operations.col_types. The resulting metadata is inconsistent with the original common-type contract, so the executor exposes text values and the comparison binder rejects text >= integer. The smallest targeted fix is to preserve the common output type vector from the planned set operation when constructing set_operations and use it consistently for the reconstructed target list and relation-zero variables; avoid a broad optimizer rewrite.
- **Why this is likely a bug:** The failure is deterministic for ordinary SQL and is independently confirmed by the type inspection: both UNION and EXCEPT return pg_typeof(x) as text for numeric inputs, while typed post-materialization references return bigint and the casted workaround returns the expected rows. The repository's planner contract says set-operation columns use the common type of both branches, and existing EXCEPT tests expect an integer result column. This is therefore a production type-propagation defect in the PR's new optimizer path, not a browser or connection problem. A targeted correction to carry the common column types into the reconstructed Query should restore numeric predicates without changing set-operation duplicate or direction semantics.
**Relevant code:**
`crates/sql/src/plan.rs:869-899`
~~~rust
plan_set_operation documents that set-operation columns use the common types of both branches, calls common_type for each pair, coerces each branch to those types, and stores them in the result columns.
~~~
`crates/sql/src/optimizer/query.rs:193-211`
~~~rust
set_operation_query reconstructs the set-operation Query, derives col_types from rtable.first().coltypes, propagates them, and creates relation-zero output variables.
~~~
`crates/sql/src/optimizer/nodefuncs.rs:33-42`
~~~rust
query_expr_type resolves a relation-zero variable from parse.set_operations.col_types, so incorrect reconstructed metadata controls the type seen by outer predicates.
~~~
`crates/sql/src/optimizer/prepunion.rs:341-347`
~~~rust
generate_setop_tlist builds output variables from col_types and assumes the binder already coerced every input to the set-operation column types.
~~~
`crates/tests/tests/scripts/union.rs:35-50`
~~~rust
Existing EXCEPT coverage expects the result column to retain the integer type (INT4), establishing that set-operation output is typed rather than generic text.
~~~…ins, and streamed single-reference recursive CTE scans
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
1 participant
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.