Skip to content

Enable debug assertions in CI.#20832

Open
stuhood wants to merge 1 commit intoapache:mainfrom
paradedb:stuhood.enable-debug-assertions-in-ci
Open

Enable debug assertions in CI.#20832
stuhood wants to merge 1 commit intoapache:mainfrom
paradedb:stuhood.enable-debug-assertions-in-ci

Conversation

@stuhood
Copy link

@stuhood stuhood commented Mar 9, 2026

Which issue does this PR close?

Rationale for this change

CI uses release profiles to allow tests to run more quickly (and potentially also to provide more coverage for the most realistic case for end users of the system). But debug assertions can expose real bugs / mistaken assumptions, and so they should be covered in CI as well.

What changes are included in this PR?

Adjust the Rust build profiles which are used in CI to enable debug assertions. Split out a ci-release profile from release in order to avoid shipping debug assertions to end users of the CLI tool.

Are these changes tested?

Yes, by CI.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the development-process Related to development process of DataFusion label Mar 9, 2026
@alamb
Copy link
Contributor

alamb commented Mar 9, 2026

if we go this route we should also leave some breadcrumbs in the code to know when we can revert the changes too

It might make more sense to just fix the upstream bug

@stuhood
Copy link
Author

stuhood commented Mar 9, 2026

if we go this route we should also leave some breadcrumbs in the code to know when we can revert the changes too

It might make more sense to just fix the upstream bug

I believe that we want to enable this regardless of the upstream bug fixes, because this is about preventing further debug-assertion-failures from landing.

@mbutrovich
Copy link
Contributor

+1 from me, I just raised this issue with @andygrove on Comet today when we are adding debug_assert around some code and I commented that "we don't run that in CI anyway."

I love debug_assert sprinkle them everywhere! Thanks @stuhood. It should already be enabled for [profile.ci] though, right? I don't mind it being explicit, just curious.

@stuhood
Copy link
Author

stuhood commented Mar 9, 2026

This did not fail because sqllogictest-sqlite did not run for some reason: likely because .github/workflows/labeler/labeler-config.yml will not match for the edits that have been made.

But this PR will/should fail until the arrow upgrade fix for #20689 is actually merged.

@stuhood
Copy link
Author

stuhood commented Mar 9, 2026

It should already be enabled for [profile.ci] though, right? I don't mind it being explicit, just curious.

Yea, it looks like you are right: https://doc.rust-lang.org/cargo/reference/profiles.html#dev

I'm happy to remove it, or to leave it explicit.

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

Labels

development-process Related to development process of DataFusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Debug assertions currently failing on main

3 participants