Skip to content

Feat!: Introduce Model linting#3787

Merged
VaggelisD merged 8 commits intomainfrom
vaggelisd/query_linter
Mar 6, 2025
Merged

Feat!: Introduce Model linting#3787
VaggelisD merged 8 commits intomainfrom
vaggelisd/query_linter

Conversation

@VaggelisD
Copy link
Contributor

@VaggelisD VaggelisD commented Feb 5, 2025

Fixes #3753

This PR introduces a new interface for a general purpose Model linter:

  • Each Rule checks for a specific lint error; If a violation is found, a RuleViolation is created with the proper context
  • Rules are grouped in RuleSets; Each RuleSet is a mapping from Rule class name -> Rule class.
  • The Linter applies the RuleSets on the model and outputs warnings / raises errors depending on their severity

The linter configuration along with the default values is the following:

gateways:
 ...

default_gateway: dev

model_defaults:
  ...

linter:
  enabled: False
  
  # Can be overridden with "ALL" or ["rule0", "rule1", ...., "ruleN"]
  rules: []
  warn_rules:
  ignored_rules: ["noselectstar", "ambiguousorinvalidcolumn", "invalidselectstarexpansion"]

This is to preserve the behavior of QueryRenderer today; For this matter, validate_query is deprecated and instead the corresponding checks are implemented as rules in the Linter.

On a model granularity, rules can be ignored by the new ignored_rules attribute:

MODEL (
  name sqlmesh_example.test_model,
  kind FULL,
  ignored_rules ["noselectstar"]
);

SELECT ...;

Users can implement their own lint rules in a linter/ directory by subclassing Rule; These will be automatically picked up at load time:

# linter/user.py

import typing as t

from sqlmesh.core.linter.rule import Rule, RuleViolation
from sqlmesh.core.model import Model

class NoMissingOwner(Rule):
    """Model owner always should be specified."""

    def check_model(self, model: Model) -> t.Optional[RuleViolation]:
        return self.violation() if not model.owner else None 

@VaggelisD VaggelisD marked this pull request as draft February 5, 2025 15:25
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from cbdeab5 to 8f456d4 Compare February 11, 2025 17:46
@VaggelisD VaggelisD changed the title Feat!: Introduce SQL linting Feat!: Introduce Model linting Feb 11, 2025
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from 8f456d4 to e60106e Compare February 11, 2025 17:47
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from e60106e to c90f22c Compare February 11, 2025 18:05
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch 3 times, most recently from 2c3e92e to 7116a75 Compare February 24, 2025 09:09
@VaggelisD VaggelisD marked this pull request as ready for review February 24, 2025 09:26
@VaggelisD VaggelisD requested a review from a team February 24, 2025 09:47
@VaggelisD
Copy link
Contributor Author

PS: v0071 is also defined here, I suspect Iaroslav's PR will be merged much sooner.

Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

Exciting stuff!

@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch 2 times, most recently from f9c6ae2 to 7beea0b Compare February 25, 2025 17:49
@VaggelisD VaggelisD requested a review from a team February 26, 2025 14:32
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from 0e7a4e0 to cef2ff6 Compare March 4, 2025 18:47
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from 6ed1217 to 0f6250d Compare March 6, 2025 09:05
@VaggelisD VaggelisD force-pushed the vaggelisd/query_linter branch from f4a1c6b to 8280209 Compare March 6, 2025 16:57
Copy link
Collaborator

@izeigerman izeigerman left a comment

Choose a reason for hiding this comment

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

Let's move violated_rules into the QueryRenderer. LGTM otherwise 👍 Let's make sure to follow-up with docs. Great work!

@VaggelisD VaggelisD merged commit 8de28b5 into main Mar 6, 2025
22 checks passed
@VaggelisD VaggelisD deleted the vaggelisd/query_linter branch March 6, 2025 21:11
Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

Nice!

afzaljasani pushed a commit that referenced this pull request Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linting rule to PREVENT select * in general

6 participants