Skip to content

fix(query): return result projection mismatch error#19602

Open
sundy-li wants to merge 3 commits intodatabendlabs:mainfrom
sundy-li:issue-19568-20260324-1925
Open

fix(query): return result projection mismatch error#19602
sundy-li wants to merge 3 commits intodatabendlabs:mainfrom
sundy-li:issue-19568-20260324-1925

Conversation

@sundy-li
Copy link
Copy Markdown
Member

@sundy-li sundy-li commented Mar 24, 2026

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

  • return a structured DataStructMissMatch error instead of panicking when the rendered result projection schema disagrees with the bound result columns
  • add a pipeline regression test for the mismatch path
  • narrow this PR to the runtime error-path improvement instead of claiming to fix the nullable-propagation bug from #19568

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Performance Improvement
  • Other (please describe):

Related to #19568


This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Mar 24, 2026
@sundy-li sundy-li added the agent-reviewable Ready for agent review label Mar 24, 2026
Copy link
Copy Markdown
Member Author

@sundy-li sundy-li left a comment

Choose a reason for hiding this comment

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

The runtime change is reasonable, but the new query-level regression does not cover it. This still needs an end-to-end test that actually executes one of the failing queries and verifies we return DATA_STRUCT_MISS_MATCH instead of hitting the old assertion path.

@sundy-li
Copy link
Copy Markdown
Member Author

Review note: the runtime change in builder_project.rs is reasonable, but the new SQL regression still does not cover the production path changed in this PR. test_result_projection_schema_matches_nullable_join_outputs only builds a physical plan and inspects output_schema(). Since this PR only changes PipelineBuilder::build_result_projection, that test would also pass on main and does not reproduce #19568 end-to-end. Please replace or supplement it with a test that actually executes one of the failing queries through the interpreter/pipeline and asserts we return DATA_STRUCT_MISS_MATCH instead of hitting the old assertion path.

@sundy-li sundy-li added the agent-changed Changed by agent label Mar 24, 2026
@sundy-li sundy-li removed the agent-changed Changed by agent label Mar 24, 2026
unreachable!("expected query plan");
};

for column in &mut bind_context.columns {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This loop manufactures the mismatch by mutating bind_context after planning, so the test only proves the new guard rejects corrupted state. It still does not reproduce the production bug from #19568, where a real query loses nullability somewhere between planning and pipeline rendering. Since the PR still says Fixes #19568, please replace or supplement this with a genuine end-to-end reproducer for the reported query shape, or include the actual nullable-propagation fix instead of only the defensive error path.

@sundy-li
Copy link
Copy Markdown
Member Author

Blocking point: this PR adds a safer DATA_STRUCT_MISS_MATCH error path, but it still does not prove that the real nullable-propagation bug from #19568 is fixed. The current query-level test mutates bind_context after planning, so it validates a synthetic mismatch rather than the reported production path. Please either add a genuine end-to-end reproducer for the failing outer/range join query shape or include the actual propagation fix before closing #19568 with this PR.

@sundy-li
Copy link
Copy Markdown
Member Author

Blocking review:

  • src/query/service/tests/it/sql/exec/mod.rs:207 currently fails clippy::replace_box, which is why linux / check is red on this head. Please replace the allocation with *column.data_type = column.data_type.remove_nullable();.
  • The query-level regression still mutates bind_context after planning, so it only validates a synthetic mismatch path. That proves the new DATA_STRUCT_MISS_MATCH branch is reachable, but it still does not show that the real nullable-propagation bug from #19568 is fixed end-to-end for the failing outer/range join shapes. Please add a genuine reproducer or narrow the PR scope/body instead of closing #19568.

@sundy-li sundy-li added the agent-changed Changed by agent label Mar 25, 2026
@sundy-li sundy-li removed the agent-changed Changed by agent label Mar 25, 2026
@sundy-li
Copy link
Copy Markdown
Member Author

Addressed the blocking review in 8c2341ef36 by removing the synthetic select-interpreter regression, keeping the direct pipeline mismatch regression, and narrowing the PR body so it no longer claims to fix #19568. This branch now only covers the runtime error-path change plus the red linux / check lint cleanup.

@sundy-li sundy-li added agent-approved Approved by agent and removed agent-reviewable Ready for agent review labels Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-approved Approved by agent pr-bugfix this PR patches a bug in codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant