You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "stuhood (via GitHub)" <gi...@apache.org> on 2026/03/09 20:25:26 UTC
[PR] Enable debug assertions in CI. [datafusion]
stuhood opened a new pull request, #20832:
URL: https://github.com/apache/datafusion/pull/20832
## Which issue does this PR close?
- Closes #20831.
## 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).
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "stuhood (via GitHub)" <gi...@apache.org>.
stuhood commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4027048631
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4027020228
+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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich merged PR #20832:
URL: https://github.com/apache/datafusion/pull/20832
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4026797188
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4038074313
> > 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.
Now most rust tests is running in `--profile ci`, however this new `--profile ci-release` profile looks more reasonable to me 🤔 . Maybe their compilation time is longer then running time, so `--profile ci` makes CI faster? If someone can confirm this, we can add the explanation to https://github.com/apache/datafusion/blob/86cb815e03e5244a3a2c9757171c252a3a65181d/Cargo.toml#L230
However, for the extended SQLite test suite (which includes many test cases), the workload should be dominated by test execution time rather than compilation time, so this `ci-release` profile is likely necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "stuhood (via GitHub)" <gi...@apache.org>.
stuhood commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4026831084
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "stuhood (via GitHub)" <gi...@apache.org>.
stuhood commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4027029835
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 should fail until the arrow upgrade unblocking #20689 is actually merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Enable debug assertions in CI. [datafusion]
Posted by "stuhood (via GitHub)" <gi...@apache.org>.
stuhood commented on PR #20832:
URL: https://github.com/apache/datafusion/pull/20832#issuecomment-4128028394
With #21044 in, this should now be unblocked: I've rebased it.
@alamb, @mbutrovich: mind taking another look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org