Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions sqlmesh/core/model/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from pathlib import Path

from astor import to_source
from difflib import get_close_matches
from sqlglot import exp
from sqlglot.helper import ensure_list

Expand Down Expand Up @@ -267,13 +268,33 @@ def validate_extra_and_required_fields(
) -> None:
missing_required_fields = klass.missing_required_fields(provided_fields)
if missing_required_fields:
field_names = "'" + "', '".join(missing_required_fields) + "'"
raise_config_error(
f"Missing required fields {missing_required_fields} in the {entity_name}"
f"Please add required field{'s' if len(missing_required_fields) > 1 else ''} {field_names} to the {entity_name}."
)

extra_fields = klass.extra_fields(provided_fields)
if extra_fields:
raise_config_error(f"Invalid extra fields {extra_fields} in the {entity_name}")
extra_field_names = "'" + "', '".join(extra_fields) + "'"

all_fields = klass.all_fields()
close_matches = {}
for field in extra_fields:
matches = get_close_matches(field, all_fields, n=1)
if matches:
close_matches[field] = matches[0]
Comment on lines +278 to +285
Copy link
Copy Markdown
Contributor

@benfdking benfdking Jun 5, 2025

Choose a reason for hiding this comment

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

not important: This looks great to me and is a lovely bit of kit!

@izeigerman this is an example of the thing I talked about on Tuesday when I was saying building with sympathy for the "LSP"

To me, this is a perfect example of something that could be a diagnostic, e.g., if this were a structured error, we could

  1. Highlight it in line, e.g., under the misspelled word
  2. Suggest a code action that could correct the mistyped word to the 'close_match'

Actually even more than just for the LSP if you printed it correctly, you could print a link to the file with the right code line that IDE's would pick up as a link to the right file and place.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I hear you. Though, I don't believe all the relevant information is available here—for example, the position of the field in the code. So Trey wouldn't be able to express this sympathy in the PR even if he wanted to, since he doesn't have access to the original source, only the Pydantic model object.


if len(close_matches) == 1:
similar_msg = ". Did you mean " + "'" + "', '".join(close_matches.values()) + "'?"
else:
similar = [
f"- {field}: Did you mean '{match}'?" for field, match in close_matches.items()
]
similar_msg = "\n\n " + "\n ".join(similar) if similar else ""

raise_config_error(
f"Invalid field name{'s' if len(extra_fields) > 1 else ''} present in the {entity_name}: {extra_field_names}{similar_msg}"
)


def single_value_or_tuple(values: t.Sequence) -> exp.Identifier | exp.Tuple:
Expand Down
7 changes: 5 additions & 2 deletions sqlmesh/core/model/definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -2156,7 +2156,10 @@ def load_sql_based_model(
name = get_model_name(path)

if not name:
raise_config_error("Model must have a name", path)
raise_config_error(
"Please add the required 'name' field to the MODEL block at the top of the file.\n\n"
+ "Learn more at https://sqlmesh.readthedocs.io/en/stable/concepts/models/overview"
)
if "default_catalog" in meta_fields:
raise_config_error(
"`default_catalog` cannot be set on a per-model basis. It must be set at the connection level.",
Expand Down Expand Up @@ -2400,7 +2403,7 @@ def _create_model(
**kwargs: t.Any,
) -> Model:
validate_extra_and_required_fields(
klass, {"name", *kwargs} - {"grain", "table_properties"}, "model definition"
klass, {"name", *kwargs} - {"grain", "table_properties"}, "MODEL block"
)

for prop in PROPERTIES:
Expand Down
4 changes: 3 additions & 1 deletion sqlmesh/core/model/kind.py
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,9 @@ def create_model_kind(v: t.Any, dialect: str, defaults: t.Dict[str, t.Any]) -> M
actual_kind_type, _ = custom_materialization
return actual_kind_type(**props)

validate_extra_and_required_fields(kind_type, set(props), f"model kind '{name}'")
validate_extra_and_required_fields(
kind_type, set(props), f"MODEL block 'kind {name}' field"
)
return kind_type(**props)

name = (v.name if isinstance(v, exp.Expression) else str(v)).upper()
Expand Down
108 changes: 108 additions & 0 deletions tests/core/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,114 @@ def test_opt_out_of_time_column_in_partitioned_by():
assert model.partitioned_by == [exp.to_column('"b"')]


def test_model_no_name():
expressions = d.parse(
"""
MODEL (
dialect bigquery,
);

SELECT 1::int AS a, 2::int AS b;
"""
)

with pytest.raises(ConfigError) as ex:
load_sql_based_model(expressions)
assert (
str(ex.value)
== "Please add the required 'name' field to the MODEL block at the top of the file.\n\nLearn more at https://sqlmesh.readthedocs.io/en/stable/concepts/models/overview"
)


def test_model_field_name_suggestions():
# top-level field
expressions = d.parse(
"""
MODEL (
name db.table,
dialects bigquery,
);

SELECT 1::int AS a, 2::int AS b;
"""
)

with pytest.raises(ConfigError) as ex:
load_sql_based_model(expressions)
assert (
str(ex.value)
== "Invalid field name present in the MODEL block: 'dialects'. Did you mean 'dialect'?"
)

# kind field
expressions = d.parse(
"""
MODEL (
name db.table,
kind INCREMENTAL_BY_TIME_RANGE(
time_column a,
batch_sizes 1
),
);

SELECT 1::int AS a, 2::int AS b;
"""
)

with pytest.raises(ConfigError) as ex:
load_sql_based_model(expressions)
assert (
str(ex.value)
== "Invalid field name present in the MODEL block 'kind INCREMENTAL_BY_TIME_RANGE' field: 'batch_sizes'. Did you mean 'batch_size'?"
)

# multiple fields
expressions = d.parse(
"""
MODEL (
name db.table,
dialects bigquery,
descriptions 'a',
asdfasdf true
);

SELECT 1::int AS a, 2::int AS b;
"""
)

with pytest.raises(ConfigError) as ex:
load_sql_based_model(expressions)
ex_str = str(ex.value)
# field order is non-deterministic, so we can't test the output string directly
assert "Invalid field names present in the MODEL block: " in ex_str
assert "'descriptions'" in ex_str
assert "'dialects'" in ex_str
assert "'asdfasdf'" in ex_str
assert "- descriptions: Did you mean 'description'?" in ex_str
assert "- dialects: Did you mean 'dialect'?" in ex_str
assert "- asdfasdf: Did you mean " not in ex_str


def test_model_required_field_missing():
expressions = d.parse(
"""
MODEL (
name db.table,
kind INCREMENTAL_BY_TIME_RANGE (),
);

SELECT 1::int AS a, 2::int AS b;
"""
)

with pytest.raises(ConfigError) as ex:
load_sql_based_model(expressions)
assert (
str(ex.value)
== "Please add required field 'time_column' to the MODEL block 'kind INCREMENTAL_BY_TIME_RANGE' field."
)


def test_no_model_statement(tmp_path: Path):
# No name inference => MODEL (...) is required
expressions = d.parse("SELECT 1 AS x")
Expand Down