Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Jun 30, 2025

Which issue does this PR close?

Rationale for this change

When query involves e.g. UNION ALL, it may produces record batches with incompatible schema. For example, one union branch may produce a nullable field while the order may produce a non-null field. Before the change, convert_batches success or failure depended on ordering of record batches returned by the query, and thus could lead (and did lead) to test flakiness.

What changes are included in this PR?

This changes schema source used by the convert_batches. Instead of peaking the first schema, it used the declared schema from the data frame, which should be correct with respect to nullability.

Are these changes tested?

yes

Are there any user-facing changes?

no

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jun 30, 2025
When query involves e.g. UNION ALL, it may produces record batches with
incompatible schema. For example, one union branch may produce a
nullable field while the order may produce a non-null field. Before the
change, `convert_batches` success or failure depended on ordering of
record batches returned by the query, and thus could lead (and did lead)
to test flakiness. This changes schema source used by the
`convert_batches`. Instead of peaking the first schema, it used the
declared schema from the data frame, which should be correct with
respect to nullability.
@findepi findepi force-pushed the findepi/fix-spurious-failure-in-convert-batches-test-helper-aff883 branch from 8fa74c0 to 36f21cf Compare June 30, 2025 14:41
@alamb
Copy link
Contributor

alamb commented Jun 30, 2025

THANK YOU

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @findepi -- this makes sense to me. I have hit the random failure in datafusion/sqllogictest/test_files/union_by_name.slt before so this is a nice welcome addition

@findepi findepi merged commit df9f096 into apache:main Jul 1, 2025
27 checks passed
@findepi findepi deleted the findepi/fix-spurious-failure-in-convert-batches-test-helper-aff883 branch July 1, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky SLT test union_by_name.slt:343

2 participants