Repository navigation
Conversation
RF03 (references.consistent) silently stopped flagging unqualified/
qualified inconsistency for ANY templated reference, not just
placeholder-templater bind parameters (:colname). A Jinja/dbt
{{ my_col }} column reference is a genuine identifier the user
intends to select, and the current is_templated-only check hides
real qualification inconsistencies for it with no warning.
Narrow the skip added in 53dc9af (sqlfluff#7615) to the placeholder
templater specifically, which is the only templater whose rendered
bare identifiers are not real column references.
Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
Coverage Results ✅ |
| # qualification consistency, not silently exempted just because it is | ||
| # templated. | ||
| fail_str: | | ||
| {% set my_col = 'customer_id' %} |
There was a problem hiding this comment.
How will this work with multiple columns? e.g. {% set cols = 'customer_id, region' %} SELECT {{ cols }}, orders.total FROM orders doesn't dbt_utils.star() return such strings?
…olumns
A single templated expression can render to more than one column, e.g.
a Jinja/dbt `{{ cols }}` (or `dbt_utils.star()`) rendering to
`customer_id, region`. The rendered value is lexed as several column
references that all map back to the one template slice, so prepending a
qualifier only touches the first rendered column (`orders.{{ cols }}` ->
`orders.customer_id, region`), silently leaving the rest unqualified.
Still flag such references for single-table consistency, but attach no
fix so the SQL is left intact for the user to resolve. Update the Jinja
test to lint-only and add a multi-column case covering the scenario.
Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
|
Good catch — the multi-column case wasn't handled well, and I've pushed a fix. Scope note first: this PR's skip is gated on I reproduced your example. Fix: when a reference is templated, RF03 still flags the inconsistency but no longer attaches a fix — a templated expression can render to zero, one, or many columns sharing one slice, so a qualifier can't be inserted per-column safely; the user resolves it by hand. I converted the single-column test to lint-only and added a multi-column test for your exact case. For a pure |
Brief summary of the change made
Fixes #8556.
RF03(references.consistent) unconditionally skips consistency checkingfor any unqualified reference where
ref.is_templatedisTrue. That guardwas added in #7615 to stop the
placeholdertemplater's colon-style bindparameters (e.g.
:colname) — rendered as a bare identifier that is not areal column — from being wrongly qualified/flagged.
ref.is_templatedis ageneric marker shared by every templater though, so the same guard also
silently exempted Jinja/dbt
{{ var }}-rendered column names — genuineidentifiers the user intends to select — from ever being checked.
This threads
templater_name(context.config.get("templater")) downthrough
_visit_queries/_check_referencesand narrows the skip to applyonly when the active templater is
placeholder, matching what #7615 itselfscoped the exemption to in its PR body, review thread, and tests.
Impact: silent-wrong-result.
Entry point / who reaches this: any user running
sqlfluff lintorsqlfluff fixwithRF03enabled (it is on by default in the base ruleset) against SQL rendered by the Jinja templater — the default templater,
and the one used by every dbt project (
sqlfluffis dbt's most common SQLlinter integration). A single-table query with one qualified and one
unqualified column reference, where the unqualified reference comes from a
Jinja variable/macro (e.g.
{{ my_col }}, or any dbt{{ ref(...) }}-stylemacro that expands to a bare column name), silently produces zero
RF03violations instead of the two violations the byte-identical
non-templated query correctly reports. The user sees a clean lint pass on
code the rule is supposed to flag — the rule is not merely weaker here, it
is inverted from "flag both qualified and unqualified in the same select"
to "flag neither," specifically for the common Jinja/dbt path.
Are there any other side effects of this change that we should be aware of?
None expected. The
placeholdertemplater's exemption (the case #7615actually addressed) is unchanged — this only narrows the condition that
already existed from "any templater" to "the templater #7615's own PR
scoped it to."
Negative control — reverted only
src/sqlfluff/rules/references/RF03.pyto
upstream/main(regression fixture kept) and re-ran the targeted test:Restored the fix and re-ran the identical command:
The new case fails without the fix and passes with it; the other 60
pre-existing RF03 cases are unaffected in both directions.
Pull Request checklist
RF03_test_fail_jinja_templated_unqualified_reference_inconsistenttotest/fixtures/rules/std_rule_cases/RF03.yml, which fails without thisfix (
1 failed, 60 passed) and passes with it (61 passed).fix; no public API or documented-behavior change to update).
(none identified).
AI-assisted contribution disclosure
AI-assisted. The defect was located and the mechanism (the
#7615guard'sgeneric
ref.is_templatedcheck vs. its own PR-scoped intent) diagnosedwith AI assistance; the fix, the regression fixture, and the negative
control (reverting only
RF03.pyand confirming the new test fails whilethe other 60 remain green) were manually validated by execution before this
PR was opened.
Signed-off-by: Udaya Tejas udayatejas2004@gmail.com
Summary by cubic
Fixes RF03 (
references.consistent) silently skipping consistency checks for any templated unqualified reference, hiding real violations in Jinja/dbt queries.The existing skip was meant for
placeholdertemplater bind parameters like:colname, which render as bare identifiers that aren't real columns. Sinceref.is_templatedis a generic marker shared by all templaters, the skip also exempted genuine Jinja/dbt{{ my_col }}references from ever being checked.placeholdertemplater by threading its name through the rule.{{ cols }}template (e.g.dbt_utils.star()) can render to multiple columns, so inserting or deleting a qualifier would touch only the first rendered column and silently corrupt the SQL. Such references are still flagged and left for the user to fix by hand.Written for commit 0d8518c. Summary will update on new commits.