Skip to content

fix(RF03): only skip templated refs for the placeholder templater - #8561

Open
udsy19 wants to merge 2 commits into
sqlfluff:mainfrom
udsy19:fix/rf03-templated-placeholder-only-skip
Open

udsy19 wants to merge 2 commits into
sqlfluff:mainfrom
udsy19:fix/rf03-templated-placeholder-only-skip

Conversation

@udsy19

@udsy19 udsy19 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Brief summary of the change made

Fixes #8556.

RF03 (references.consistent) unconditionally skips consistency checking
for any unqualified reference where ref.is_templated is True. That guard
was added in #7615 to stop the placeholder templater's colon-style bind
parameters (e.g. :colname) — rendered as a bare identifier that is not a
real column — from being wrongly qualified/flagged. ref.is_templated is a
generic marker shared by every templater though, so the same guard also
silently exempted Jinja/dbt {{ var }}-rendered column names — genuine
identifiers the user intends to select — from ever being checked.

This threads templater_name (context.config.get("templater")) down
through _visit_queries/_check_references and narrows the skip to apply
only when the active templater is placeholder, matching what #7615 itself
scoped the exemption to in its PR body, review thread, and tests.

Impact: silent-wrong-result.

Entry point / who reaches this: any user running sqlfluff lint or
sqlfluff fix with RF03 enabled (it is on by default in the base rule
set) against SQL rendered by the Jinja templater — the default templater,
and the one used by every dbt project (sqlfluff is dbt's most common SQL
linter 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(...) }}-style
macro that expands to a bare column name), silently produces zero RF03
violations 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 placeholder templater's exemption (the case #7615
actually 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.py
to upstream/main (regression fixture kept) and re-ran the targeted test:

$ pytest test/rules/yaml_test_cases_test.py -k RF03 -q
FAILED test/rules/yaml_test_cases_test.py::test__rule_test_case[RF03_test_fail_jinja_templated_unqualified_reference_inconsistent]
1 failed, 60 passed, 2436 deselected in 5.44s

Restored the fix and re-ran the identical command:

$ pytest test/rules/yaml_test_cases_test.py -k RF03 -q
61 passed, 2436 deselected in 5.82s

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

  • Included test cases to demonstrate any code changes: added
    RF03_test_fail_jinja_templated_unqualified_reference_inconsistent to
    test/fixtures/rules/std_rule_cases/RF03.yml, which fails without this
    fix (1 failed, 60 passed) and passes with it (61 passed).
  • Added appropriate documentation for the change (behavior-narrowing bug
    fix; no public API or documented-behavior change to update).
  • Created GitHub issues for any relevant followup/future enhancements
    (none identified).

AI-assisted contribution disclosure

AI-assisted. The defect was located and the mechanism (the #7615 guard's
generic ref.is_templated check vs. its own PR-scoped intent) diagnosed
with AI assistance; the fix, the regression fixture, and the negative
control (reverting only RF03.py and confirming the new test fails while
the 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 placeholder templater bind parameters like :colname, which render as bare identifiers that aren't real columns. Since ref.is_templated is a generic marker shared by all templaters, the skip also exempted genuine Jinja/dbt {{ my_col }} references from ever being checked.

  • Narrowed the skip to the placeholder templater by threading its name through the rule.
  • Stopped attaching autofixes to templated references: a single {{ 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.
  • Added regression tests verifying that Jinja-rendered unqualified references are still flagged and never autofixed.

Written for commit 0d8518c. Summary will update on new commits.

Review in cubic

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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Results ✅

Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL   23346      0   100%

270 files skipped due to complete coverage.

# qualification consistency, not silently exempted just because it is
# templated.
fail_str: |
{% set my_col = 'customer_id' %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@rubytobi rubytobi added the awaiting feedback Cannot continue investigating until more information is provided. label Oct 3, 2026
…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>
@udsy19

udsy19 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

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 templater == "placeholder", so your {% set %} / dbt_utils.star() (jinja) example never hits that skip path. What the PR changes for jinja is that templated refs are now checked for consistency instead of being silently skipped — which is the intent, since {{ my_col }} is a real column the user selects.

I reproduced your example. {{ cols }} → customer_id, region lexes as two column references that both map back to the single {{ cols }} source slice. RF03 was flagging the inconsistency (good), but its auto-fix prepended the qualifier to the whole slice: orders.{{ cols }} → orders.customer_id, region, qualifying only the first column and silently leaving region unqualified. (In default config ignore_templated_areas=True masks it as "unfixable", but the rewrite is still wrong.)

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 SELECT {{ dbt_utils.star(...) }} FROM t (no qualified sibling) there's no inconsistency, so nothing is flagged.

This branch has not been deployed

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

Labels

awaiting feedback Cannot continue investigating until more information is provided.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RF03: unqualified Jinja-rendered column references are silently exempted from consistency checking

2 participants