Skip to content

Fix nested selected link joins#27

Open
dodok8 wants to merge 4 commits into
mainfrom
issue-26-fix-nested-selected-link-joins
Open

Fix nested selected link joins#27
dodok8 wants to merge 4 commits into
mainfrom
issue-26-fix-nested-selected-link-joins

Conversation

@dodok8

@dodok8 dodok8 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix nested selected single-link shape planning so descendant selected links join from the selected parent alias instead of the root source, while keeping SQL aliases unique when selected link names repeat or collide with root-relative path aliases.

Closes #26

Scope

  • Recursively plan selected single-link child shapes in sqlite-query-plan.
  • Carry the selected parent alias into nested selected link planning.
  • Assign planner-owned SQL aliases when repeated nested selected link names would collide.
  • Reserve root-relative path aliases before assigning nested selected link aliases.
  • Reuse the selected-shape alias plan across joins, selected values, and result-shape references.
  • Keep Many cardinality distinct from nullable parent sources so unsupported multi-link shapes do not get lowered as single-link joins.
  • Use LEFT JOIN for descendant selected joins under optional selected sources so root rows are preserved.
  • Add planner fixtures and tests for required, optional, repeated-name, root-path-collision, and multi-cardinality nested selected shapes.
  • Add query-pipeline execution coverage for Post.author.best_friend and repeated User.best_friend.best_friend nested selection.

Spec and plan alignment

List the spec/ and plan/ files checked for this change. If the change
intentionally diverges from them, explain why and include the required document
update in this pull request.

  • spec/query.md: selected relation fields require nested shapes, and the query pipeline must preserve nested result shapes.
  • spec/sqlite-query-plan.md: SQLite Plan owns aliases, joins, selected value slots, result-shaping metadata, and inner/left join choices for required and optional links. Multi-link nested shapes are specified for follow-up planning rather than single-link joins.
  • plan/sqlite-query-plan-implementation-plan.md: the change stays inside sqlite-query-plan and keeps SQL string rendering in sqlite-query-sqlgen.
  • No intentional spec divergence.

Tests

List the automated and manual checks performed.

  • cargo test --workspace
  • Other: cargo test -p sqlite-query-plan sqlite_select_plan_can_join_nested_selected_single_link
  • Other: cargo test -p sqlite-query-plan sqlite_select_plan_uses_left_join_for_nested_selected_link_under_optional_source
  • Other: cargo test -p sqlite-query-plan sqlite_select_plan_uses_unique_aliases_for_repeated_nested_selected_link_names
  • Other: cargo test -p sqlite-query-plan sqlite_select_plan_avoids_root_path_alias_collision_with_nested_selected_link
  • Other: cargo test -p sqlite-query-plan sqlite_select_plan_preserves_multi_link_cardinality_under_optional_source
  • Other: cargo test --test select_execution select_pipeline_executes_nested_selected_single_link_shape
  • Other: cargo test --test select_execution select_pipeline_executes_repeated_nested_selected_single_link_names

Review notes

Call out API boundaries, temporary rules, migration concerns, or follow-up work
that reviewers should inspect closely.

  • The fix is scoped to SQLite planning. The resolver and backend-independent IR contracts are unchanged.
  • The query-pipeline fixture still uses raw SQL inserts because Gelite does not yet have an insert pipeline.
  • Nested result shaping still uses the current flat selected-value execution path; this PR fixes the join source, join kind, and alias identity needed by that path.
  • Multi-link selected shapes remain unsupported in the SQLite select planner until follow-up fetch planning is implemented.

AI assistance

Disclose AI tool usage according to AI_POLICY.md.

  • No AI tools were used.
  • AI tools were used and are disclosed here and in the relevant commit
    messages with Assisted-by trailers.

Tools and models:

  • Codex:gpt-5.5

Plan selected single-link joins recursively so nested result shapes use the parent selected alias as the join source. Preserve parent rows under optional selected sources by lowering descendant joins to LEFT joins.

Assisted-by: Codex:gpt-5.5
@dodok8 dodok8 marked this pull request as ready for review June 25, 2026 07:01
@dodok8

dodok8 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c7e67f23c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread engine/sqlite-query-plan/src/lib.rs Outdated
Assign planner-owned SQL aliases to repeated nested selected links and reuse the alias plan across joins, selected values, and result-shape references.

Assisted-by: Codex:gpt-5.5
@dodok8

dodok8 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48dda43463

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread engine/sqlite-query-plan/src/lib.rs Outdated
Comment thread engine/sqlite-query-plan/src/lib.rs
dodok8 added 2 commits June 25, 2026 17:05
Reserve root path aliases before assigning nested selected link SQL aliases so selected joins do not collide with filter or ordering joins synthesized from the root source.

Assisted-by: Codex:gpt-5.5
Keep Many cardinality distinct from optional source nullability so unsupported multi-link selected shapes do not get planned as single-link joins.

Assisted-by: Codex:gpt-5.5
@sonarqubecloud

Copy link
Copy Markdown

@dodok8

dodok8 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 991cd76e33

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +459 to +462
let sql_alias = if !shape_path.is_empty() && conflicts_with_existing_alias {
join_aliases.next_alias()
} else {
preferred_alias.to_string()

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 👍 / 👎.

Comment on lines +430 to +432
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);

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 👍 / 👎.

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.

bug: nested selected link shapes miss recursive SQLite joins

1 participant