Skip to content

Fix nested selected link joins #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dodok8 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix nested selected link joins #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c7e67f
Fix nested selected link joins
dodok8 Jun 25, 2026
48dda43
Fix selected link alias collisions
dodok8 Jun 25, 2026
a68dfc9
Avoid selected link alias collisions with root paths
dodok8 Jun 25, 2026
991cd76
Preserve multi link cardinality under nullable sources
dodok8 Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
298 changes: 266 additions & 32 deletions engine/sqlite-query-plan/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,27 @@ pub fn plan_select(ir: &SelectQuery) -> SQLiteSelectPlan {
let selected_column_names = selected_field_column_names(ir.shape());
let mut select_aliases = SQLiteComputedAliasAllocator::new(selected_column_names.clone());
let mut join_aliases = SQLiteJoinAliasAllocator::new(selected_link_aliases(ir.shape()));
let selected_shape_aliases =
plan_selected_shape_aliases(ir.shape(), root_path_aliases(ir), &mut join_aliases);
let planned_shape_values = plan_shape_values(
ir.shape(),
"root",
false,
&[],
&selected_shape_aliases,
&mut select_aliases,
&mut join_aliases,
);
let selected_values = planned_shape_values.values;
let mut result_aliases = SQLiteComputedAliasAllocator::new(selected_column_names);
let result_shape = plan_result_shape(ir.shape(), "root", false, &mut result_aliases);
let result_shape = plan_result_shape(
ir.shape(),
"root",
false,
&[],
&selected_shape_aliases,
&mut result_aliases,
);

let planned_orders: Vec<PlannedOrder> = ir
.order_by()
Expand All @@ -66,13 +77,8 @@ pub fn plan_select(ir: &SelectQuery) -> SQLiteSelectPlan {
None => (None, vec![]),
};

let mut joins: Vec<SQLiteJoin> = ir
.shape()
.fields()
.into_iter()
.filter(|field| field.child_shape().is_some())
.map(SQLiteJoin::selected_single_link)
.collect();
let mut joins =
plan_selected_shape_joins(ir.shape(), "root", false, &[], &selected_shape_aliases);

joins.extend(planned_shape_values.joins);
joins.extend(filter_joins);
Expand Down Expand Up @@ -395,6 +401,158 @@ impl SQLiteJoinAliasAllocator {
}
}

struct SQLiteSelectedShapeAliases {
aliases: Vec<SQLiteSelectedShapeAlias>,
}

struct SQLiteSelectedShapeAlias {
shape_path: Vec<usize>,
sql_alias: String,
}

impl SQLiteSelectedShapeAliases {
fn alias_for_path(&self, shape_path: &[usize]) -> &str {
self.aliases
.iter()
.find(|alias| alias.shape_path == shape_path)
.expect("selected shape alias should exist for nested field")
.sql_alias
.as_str()
}
}

fn plan_selected_shape_aliases(
shape: &query_ir::ResolvedShape,
reserved_root_path_aliases: Vec<String>,
join_aliases: &mut SQLiteJoinAliasAllocator,
) -> SQLiteSelectedShapeAliases {
let mut aliases = Vec::new();
let mut used_aliases = vec!["root".to_string()];
used_aliases.extend(reserved_root_path_aliases);
collect_selected_shape_aliases(shape, &[], &mut used_aliases, &mut aliases, join_aliases);
Comment on lines +430 to +432

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Reserve root-path aliases in generated alias allocation

These root-path aliases are added only to used_aliases, but the join_aliases allocator used below does not reserve them. If a valid root link named like the allocator prefix, such as __gelite_join_0, is referenced by a filter or order clause, then a later nested selected-link conflict can call next_alias() and reuse __gelite_join_0 for a different selected join, producing duplicate SQL table aliases. Add these root-path aliases to the allocator's reserved set, or check used_aliases after generating an alias.

Useful? React with 👍 / 👎.


SQLiteSelectedShapeAliases { aliases }
}

fn collect_selected_shape_aliases(
shape: &query_ir::ResolvedShape,
shape_path: &[usize],
used_aliases: &mut Vec<String>,
aliases: &mut Vec<SQLiteSelectedShapeAlias>,
join_aliases: &mut SQLiteJoinAliasAllocator,
) {
for (index, item) in shape.items().iter().enumerate() {
let query_ir::ResolvedShapeItem::Field(field) = item else {
continue;
};

let Some(child_shape) = field.child_shape() else {
continue;
};

let mut child_path = shape_path.to_vec();
child_path.push(index);
let preferred_alias = field.output_name();
let conflicts_with_existing_alias = used_aliases
.iter()
.any(|used_alias| used_alias == preferred_alias);
let sql_alias = if !shape_path.is_empty() && conflicts_with_existing_alias {
join_aliases.next_alias()
} else {
preferred_alias.to_string()
Comment on lines +459 to +462

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reserve root link aliases before nested aliases

When a root-level selected link appears after an earlier nested selected link with the same field name, this condition ignores the detected conflict because shape_path is empty, so both joins get the same SQL alias. For example, with Post.best_friend and User.best_friend, select Post { author: { best_friend: { name } }, best_friend: { name } } first assigns the nested author.best_friend alias best_friend, then assigns the root Post.best_friend alias best_friend too, producing two different joins with the same table alias. Root aliases need to win up front, or this conflict must allocate a fresh alias.

Useful? React with 👍 / 👎.

};

used_aliases.push(sql_alias.clone());
aliases.push(SQLiteSelectedShapeAlias {
shape_path: child_path.clone(),
sql_alias,
});
collect_selected_shape_aliases(
child_shape,
&child_path,
used_aliases,
aliases,
join_aliases,
);
}
}

fn root_path_aliases(ir: &SelectQuery) -> Vec<String> {
let mut aliases = Vec::new();

if let Some(filter) = ir.filter() {
collect_root_path_aliases_from_expr(filter, &mut aliases);
}

for order in ir.order_by() {
collect_root_path_aliases_from_value(order.value(), &mut aliases);
}

collect_root_computed_path_aliases(ir.shape(), &mut aliases);

aliases
}

fn collect_root_computed_path_aliases(shape: &query_ir::ResolvedShape, aliases: &mut Vec<String>) {
for item in shape.items() {
let query_ir::ResolvedShapeItem::Computed(computed) = item else {
continue;
};

collect_root_path_aliases_from_value(computed.value(), aliases);
}
}

fn collect_root_path_aliases_from_expr(expr: &Expr, aliases: &mut Vec<String>) {
match expr {
Expr::Compare(compare) => {
collect_root_path_aliases_from_value(compare.left(), aliases);
collect_root_path_aliases_from_value(compare.right(), aliases);
}
Expr::IsNull(value) | Expr::IsNotNull(value) => {
collect_root_path_aliases_from_value(value, aliases);
}
Expr::In(in_expr) => {
collect_root_path_aliases_from_value(in_expr.left(), aliases);
for value in in_expr.right() {
collect_root_path_aliases_from_value(value, aliases);
}
}
Expr::And(left, right) | Expr::Or(left, right) => {
collect_root_path_aliases_from_expr(left, aliases);
collect_root_path_aliases_from_expr(right, aliases);
}
Expr::Not(inner) => collect_root_path_aliases_from_expr(inner, aliases),
}
}

fn collect_root_path_aliases_from_value(value: &query_ir::ValueExpr, aliases: &mut Vec<String>) {
match value {
query_ir::ValueExpr::Path(path) => collect_root_path_alias_from_path(path, aliases),
query_ir::ValueExpr::Literal(_) => {}
query_ir::ValueExpr::Arithmetic(arithmetic) => {
collect_root_path_aliases_from_value(arithmetic.left(), aliases);
collect_root_path_aliases_from_value(arithmetic.right(), aliases);
}
query_ir::ValueExpr::UnaryArithmetic(unary) => {
collect_root_path_aliases_from_value(unary.operand(), aliases);
}
}
}

fn collect_root_path_alias_from_path(path: &query_ir::ResolvedPath, aliases: &mut Vec<String>) {
let Some(first_step) = path.steps().first() else {
return;
};

if matches!(
first_step.kind(),
query_ir::ResolvedPathStepKind::Link { .. }
) {
aliases.push(first_step.field().name().to_string());
}
}

fn selected_field_column_names(shape: &query_ir::ResolvedShape) -> Vec<String> {
let mut column_names = Vec::new();
collect_selected_field_column_names(shape, false, &mut column_names);
Expand Down Expand Up @@ -443,17 +601,21 @@ fn plan_shape_values(
shape: &query_ir::ResolvedShape,
source_alias: &str,
source_nullable: bool,
shape_path: &[usize],
selected_shape_aliases: &SQLiteSelectedShapeAliases,
computed_aliases: &mut SQLiteComputedAliasAllocator,
join_aliases: &mut SQLiteJoinAliasAllocator,
) -> PlannedShapeValues {
let mut values = Vec::new();
let mut joins = Vec::new();

for item in shape.items() {
for (index, item) in shape.items().iter().enumerate() {
match item {
query_ir::ResolvedShapeItem::Field(field) => match field.child_shape() {
Some(child_shape) => {
let nested_alias = field.output_name();
let mut child_path = shape_path.to_vec();
child_path.push(index);
let nested_alias = selected_shape_aliases.alias_for_path(&child_path);
let child_id_field = FieldRef::new(
schema_model::FieldId::new(1),
child_shape.source_object_type().clone(),
Expand All @@ -470,6 +632,8 @@ fn plan_shape_values(
child_shape,
nested_alias,
source_nullable || field.cardinality() == Cardinality::Optional,
&child_path,
selected_shape_aliases,
computed_aliases,
join_aliases,
);
Expand Down Expand Up @@ -502,28 +666,80 @@ fn plan_shape_values(
PlannedShapeValues { values, joins }
}

fn plan_selected_shape_joins(
shape: &query_ir::ResolvedShape,
source_alias: &str,
source_nullable: bool,
shape_path: &[usize],
selected_shape_aliases: &SQLiteSelectedShapeAliases,
) -> Vec<SQLiteJoin> {
let mut joins = Vec::new();

for (index, item) in shape.items().iter().enumerate() {
let query_ir::ResolvedShapeItem::Field(field) = item else {
continue;
};

let Some(child_shape) = field.child_shape() else {
continue;
};

let mut child_path = shape_path.to_vec();
child_path.push(index);
let target_alias = selected_shape_aliases.alias_for_path(&child_path);
let join_cardinality = path_step_join_cardinality(source_nullable, field.cardinality());
Comment thread
dodok8 marked this conversation as resolved.

joins.push(SQLiteJoin::selected_single_link_with_alias(
source_alias,
field,
target_alias,
join_cardinality,
));
joins.extend(plan_selected_shape_joins(
child_shape,
target_alias,
source_nullable || field.cardinality() == Cardinality::Optional,
&child_path,
selected_shape_aliases,
));
}

joins
}

fn plan_result_shape(
shape: &query_ir::ResolvedShape,
source_alias: &str,
include_identity: bool,
shape_path: &[usize],
selected_shape_aliases: &SQLiteSelectedShapeAliases,
computed_aliases: &mut SQLiteComputedAliasAllocator,
) -> SQLiteResultShapePlan {
let fields = shape
.items()
.iter()
.map(|item| match item {
.enumerate()
.map(|(index, item)| match item {
query_ir::ResolvedShapeItem::Field(field) => match field.child_shape() {
Some(child_shape) => SQLiteResultField {
output_name: field.output_name().to_string(),
cardinality: field.cardinality(),
value: None,
nested_shape: Some(plan_result_shape(
child_shape,
field.output_name(),
true,
computed_aliases,
)),
},
Some(child_shape) => {
let mut child_path = shape_path.to_vec();
child_path.push(index);
let nested_alias = selected_shape_aliases.alias_for_path(&child_path);

SQLiteResultField {
output_name: field.output_name().to_string(),
cardinality: field.cardinality(),
value: None,
nested_shape: Some(plan_result_shape(
child_shape,
nested_alias,
true,
&child_path,
selected_shape_aliases,
computed_aliases,
)),
}
}
None => SQLiteResultField {
output_name: field.output_name().to_string(),
cardinality: field.cardinality(),
Expand Down Expand Up @@ -955,10 +1171,10 @@ fn path_step_join_cardinality(
current_nullable: bool,
step_cardinality: Cardinality,
) -> Cardinality {
if current_nullable {
Cardinality::Optional
} else {
step_cardinality
match (current_nullable, step_cardinality) {
(_, Cardinality::Many) => Cardinality::Many,
(true, _) => Cardinality::Optional,
(false, cardinality) => cardinality,
}
}

Expand Down Expand Up @@ -1292,26 +1508,44 @@ pub struct SQLiteJoin {
}

impl SQLiteJoin {
pub fn selected_single_link(shape_field: &query_ir::ResolvedShapeField) -> Self {
pub fn selected_single_link(
source_alias: &str,
shape_field: &query_ir::ResolvedShapeField,
cardinality: Cardinality,
) -> Self {
Self::selected_single_link_with_alias(
source_alias,
shape_field,
shape_field.output_name(),
cardinality,
)
}

fn selected_single_link_with_alias(
source_alias: &str,
shape_field: &query_ir::ResolvedShapeField,
target_alias: &str,
cardinality: Cardinality,
) -> Self {
let child_shape = shape_field
.child_shape()
.expect("selected link field must have child shape");

let field = shape_field.field().clone();

Self {
kind: SQLiteJoinKind::for_single_link(shape_field.cardinality()),
source_alias: "root".to_string(),
kind: SQLiteJoinKind::for_single_link(cardinality),
source_alias: source_alias.to_string(),
target_table: child_shape
.source_object_type()
.name()
.to_ascii_lowercase()
.to_string(),
target_alias: shape_field.output_name().to_string(),
target_alias: target_alias.to_string(),
on: SQLiteJoinCondition {
left_alias: "root".to_string(),
left_alias: source_alias.to_string(),
left_column: format!("{}_id", field.name()),
right_alias: shape_field.output_name().to_string(),
right_alias: target_alias.to_string(),
right_column: "id".to_string(),
},
reason: SQLiteJoinReason::SelectedSingleLink { field },
Expand Down
Loading