Skip to content

Conversation

TerukiUeno
Copy link

@TerukiUeno TerukiUeno commented Sep 23, 2025

Fixes: #1984

Solution

This PR introduces a new --disable-assertions command-line flag that allows users to completely disable all assertions during compilation and execution. This includes both manual assertions (defined with type: "assertion") and built-in inline assertions (uniqueKey, nonNull, and rowConditions).

Key Implementation Details:

  1. CLI Flag: Added --disable-assertions option to both compile and run commands with the description: "Disables all assertions including built-in assertions (uniqueKey, nonNull, rowConditions) and manual assertions (type: assertion)."

  2. Core Integration: The flag is passed through the compilation pipeline:

    • CLI → ProjectConfigOptions.disableAssertionsProjectConfig.disableAssertionsSession.projectConfig.disableAssertions
  3. Assertion Disabling Logic: When disableAssertions is true, all assertions (both manual and inline) are automatically marked as disabled: true during their construction phase.

Test

  • Verify that bazel test //core/... passes

  • Ensure ./scripts/lint passes

  • Test the complete workflow by running both ./scripts/run compile --disable-assertions and ./scripts/run run --disable-assertions --dry-run on a personal project, confirming that:

    • All manual assertions (type: "assertion") are properly disabled
    • All built-in assertions (uniqueKey, nonNull, rowConditions) are properly disabled

Copy link

google-cla bot commented Sep 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@TerukiUeno TerukiUeno force-pushed the add_option_to_skip_all_assertions branch from 09047c6 to b9ff7c3 Compare September 23, 2025 13:06
@TerukiUeno TerukiUeno marked this pull request as ready for review September 28, 2025 14:10
@TerukiUeno TerukiUeno requested a review from a team as a code owner September 28, 2025 14:10
@TerukiUeno TerukiUeno requested review from kolina and removed request for a team September 28, 2025 14:10
@kolina
Copy link
Contributor

kolina commented Oct 1, 2025

/gcbrun

@kolina
Copy link
Contributor

kolina commented Oct 3, 2025

/gcbrun

@kolina
Copy link
Contributor

kolina commented Oct 19, 2025

/gcbrun

{
statement:
"create or replace table `dataform-open-source.dataform.example` as \n\nselect 1 as testValue2",
`create or replace table \`${DEFAULT_DATABASE}.dataform.example\` as \n\nselect 1 as testValue2`,
Copy link
Contributor

@kolina kolina Oct 20, 2025

Choose a reason for hiding this comment

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

It seems that this triggered our linter checks for SQL injection, see logs

You can fix it by adding such comments in corresponding places in the test

Copy link
Author

Choose a reason for hiding this comment

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

@kolina
Apologies for the oversight. I’ve fixed it.
fix linter error for SQL injection

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: option to skip built-in assertions

2 participants