fix(ST03): exempt data-modifying CTEs from unused_cte#2676
Open
skyf0l wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ST03 (
structure.unused_cte) reported a violation for any CTE whose name never appears in aTableReference, even when the CTE body is a data-modifying statement (INSERT/UPDATE/DELETE/MERGE). In Postgres a data-modifying CTE is executed for its side effects whether or not the primary query references it, so such a CTE is never "unused".This is a false positive on a common, idiomatic pattern: running several deletes/inserts as one atomic statement / round-trip.
Before this change all three of these CTEs were reported as unused, and there is no inline way to silence it.
Why
SQLFluff (which sqruff tracks for rule behaviour) already exempts these via
_is_data_modifying_cte. sqruff simply never ported that exemption. This PR brings sqruff in line.How
In
RuleST03::eval, before pushing a violation for a remaining CTE, inspect itscte_definition_segmentwithrecursive_crawlforInsertStatement/UpdateStatement/DeleteStatement/MergeStatement. If found, skip the CTE.Tests
Added Postgres fixtures to
ST03.yml:test_pass_postgres_data_modifying_delete_cte— single unreferenced DELETE CTE passes.test_pass_postgres_data_modifying_multiple_delete_ctes— several unreferenced DELETE CTEs in one statement pass.test_pass_postgres_data_modifying_insert_cte— unreferenced INSERT CTE passes.test_pass_postgres_data_modifying_update_cte— unreferenced UPDATE CTE passes.test_fail_postgres_unused_select_cte_alongside_data_modifying— a genuinely unused SELECT CTE next to an exempt data-modifying CTE is still flagged, so the exemption does not over-reach.All existing ST03 cases continue to pass.