Skip to content

GH-55493: [C++][Substrait] Use resolved output types for ProjectRel - #55494

Open
gvol wants to merge 2 commits into
apache:mainfrom
gvol:main
Open

gvol wants to merge 2 commits into
apache:mainfrom
gvol:main

Conversation

@gvol

@gvol gvol commented Nov 11, 2026 •

Copy link
Copy Markdown

Rationale for this change

  • ProjectRel conversion calls OutputType::type() on the kernel signature which only supports fixed output types.

  • Valid expressions such as case_when compute their output types from their inputs. Deserializing these projections aborts in Debug builds, and presumably leads to bad behavior in Release builds.

What changes are included in this PR?

  • Use the output type that Bind resolves for the projected field.

  • Add a regression test with an IfThen expression that maps to case_when. The test checks the converted schema and query results. It aborts in Debug mode without the corresponding fix.

Are these changes tested?

  • Added a new test which passes after the changes.

  • Ran other tests in Debug mode with the following command

cmake --build cpp/build-debug --target arrow-substrait-substrait-test -j 8
ARROW_TEST_DATA="$PWD/testing/data" PARQUET_TEST_DATA="$PWD/cpp/submodules/parquet-testing/data" cpp/build-debug/debug/arrow-substrait-substrait-test

Are there any user-facing changes?

There are no public API changes apart from the lack of abort().

This PR contains a "Critical Fix"., namely a bug that causes a crash (in Debug mode).

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

  • Human - I wrote some of the PR description and commit messages.
  • AI

Reviewed before submission by:

  • Human - though I am unfamiliar with the arrow code base, the change and test look sane to me.
  • AI
  • Not reviewed

Ivan Andrus added 2 commits November 10, 2026 19:44
Add a ProjectRel test using case_when, which computes its output
type from its inputs.  Check the converted schema and query results.

The test aborts without other changes.
OutputType::type() is only valid for fixed kernel outputs.
Use the type resolved by Bind for the projected field instead.

This fixes the assert in the test in the previous commit
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #55493 has been automatically assigned in GitHub to PR creator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant