Skip to content

Comments

Check sqllogictests for any dangling config settings(#17914)#20474

Open
cj-zhukov wants to merge 1 commit intoapache:mainfrom
cj-zhukov:cj-zhukov/check-sqllogictests-for-any-dangling-config-settings
Open

Check sqllogictests for any dangling config settings(#17914)#20474
cj-zhukov wants to merge 1 commit intoapache:mainfrom
cj-zhukov:cj-zhukov/check-sqllogictests-for-any-dangling-config-settings

Conversation

@cj-zhukov
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added development-process Related to development process of DataFusion sqllogictest SQL Logic Tests (.slt) labels Feb 22, 2026
@cj-zhukov
Copy link
Contributor Author

High-Level Overview

This PR introduces a validation script to prevent dangling configuration settings in sqllogictest (.slt) files.

This PR Adds:

  • A new shell script ci/scripts/check_slt_configs.sh that scans all .slt files and detects DataFusion configuration options that are set to: set datafusion.<config> = true but are never reset back to: set datafusion.<config> = false
  • Updates to existing .slt files where dangling boolean configs were found.
    All identified configs have been explicitly reset to false to ensure files restore the default session state.
  • A new CI job: sqllogictest-config-check that runs the validation script and fails the workflow if any abandoned boolean configuration is detected.

Current Limitations:

  • Only checks boolean flags set to true and id does not validate non-boolean configuration changes (e.g. default_catalog, default_schema, etc.)
  • Does not respect ordering: It only verifies that a matching = false exists somewhere in the file and It does not ensure the reset occurs after the corresponding = true.
  • Does not validate toggle correctness: it does not verify correct pairing, nesting, or the number of enable/disable occurrence

Follow-Up Improvements:

  • Parse (.slt) files properly instead of relying on regex/grep.
  • Track configuration state transitions in order.
  • Validate correct pairing and restoration semantics.
  • Support non-boolean configuration values.
    This could be implemented in Rust using a proper parser (e.g., nom), similar to what is already used in datafusion-examples.
    I’m happy to work on a follow-up PR to evolve this validation into a more robust and structured implementation.

@cj-zhukov
Copy link
Contributor Author

@Jefffrey Since you originally opened this issue, could you please take a look at this PR?

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 sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant