Demonstration of sqlfluff plugin ignoring dialect specified in a configuration file #2571
Conversation
|
No issue mentions found. Please mention an issue in the pull request description. Use GitHub automation to close the issue when a PR is merged |
There was a problem hiding this comment.
Pull request overview
This PR demonstrates a configuration precedence issue in the sqlfluff plugin where the hardcoded --dialect ansi command-line argument overrides dialect settings specified in .sqlfluff configuration files.
- Adds test fixtures showing that postgres-specific linting rules should be applied when a
.sqlfluffconfig file specifiesdialect = postgres - Creates a reproducible test case to validate the issue and verify future fixes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
qlty-plugins/plugins/linters/sqlfluff/fixtures/postgres_config.in/test.sql |
Adds a simple SQL test file with double-quoted identifiers to demonstrate postgres-specific linting behavior |
qlty-plugins/plugins/linters/sqlfluff/fixtures/postgres_config.in/.sqlfluff |
Adds sqlfluff configuration file specifying postgres dialect to test config file precedence |
qlty-plugins/plugins/linters/sqlfluff/fixtures/__snapshots__/postgres_config_v3.5.0.shot |
Adds expected test output snapshot showing postgres-specific RF06 rule violations for unnecessary quoted identifiers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Coverage Impact - ubuntu-latest This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
|
Coverage Impact - macos-15 ⬇️ Merging this pull request will decrease total coverage on 🛟 Help
|
The sqlfluff plugin script is defined as follows:
When a dialect is specified as a command line option it takes precedence over the option in a configuration file. So if a repository has a configuration file defined as:
The dialect will be
ansi, ignoring the value in the configuration file. This PR intentionally introduces a failing test to demonstrate the behavior. The SQL in the PR is supposed to produce a lint issue (in postgres), but doesn't because the dialect is set to ansi.The sqlfluff plugin's use of a
--dialect ansieffectively creates a smart default; the sqlfluff linter itself does not have one. If you run sqlfluff (not the plugin), without a dialect (it can be passed as a command line argument or placed in a configuration file), sqlfluff raises an error.A fix for this plugin issue should retain the smart default for backwards compatibility. While removing the
--dialect ansifrom the script directive will cause the linter to successfully honor the dialect in configuration files, it will raise an error for any users of the plugin who haven't specified a dialect.One possible solution to maintain backwards compatibility would be initialize the plugin with a configuration file with a default dialect. This has its own issues -- e.g. if the customer has defined a dialect already, it should be honored. sqlfluff supports 5 different kinds of configuration files -- we could conditionally add the lowest precedent configuration file if it doesn't exist. In those cases it would be overridden by higher precedent configuration files, or not set if it already exists. This is pretty complex though.