From df3d295fb655ce5c1e4c6784c78a945bb0936d80 Mon Sep 17 00:00:00 2001 From: Trey Spiller Date: Wed, 4 Jun 2025 16:52:25 -0500 Subject: [PATCH 1/3] Add typo suggestions to MODEL block field errors --- sqlmesh/core/model/common.py | 25 +++++++++++++++++++++++-- sqlmesh/core/model/definition.py | 7 +++++-- sqlmesh/core/model/kind.py | 4 +++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/sqlmesh/core/model/common.py b/sqlmesh/core/model/common.py index 694e6d572a..8f2d9e8ff8 100644 --- a/sqlmesh/core/model/common.py +++ b/sqlmesh/core/model/common.py @@ -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 @@ -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] + + 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: diff --git a/sqlmesh/core/model/definition.py b/sqlmesh/core/model/definition.py index 23346a15a2..db61b09c8e 100644 --- a/sqlmesh/core/model/definition.py +++ b/sqlmesh/core/model/definition.py @@ -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.", @@ -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: diff --git a/sqlmesh/core/model/kind.py b/sqlmesh/core/model/kind.py index c858ac44fa..6395b13f99 100644 --- a/sqlmesh/core/model/kind.py +++ b/sqlmesh/core/model/kind.py @@ -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() From ec3202f943c01466d7d4170c6b106fd40fd26049 Mon Sep 17 00:00:00 2001 From: Trey Spiller Date: Wed, 4 Jun 2025 17:45:14 -0500 Subject: [PATCH 2/3] Add tests --- tests/core/test_model.py | 108 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/core/test_model.py b/tests/core/test_model.py index 932775f831..40466a02b7 100644 --- a/tests/core/test_model.py +++ b/tests/core/test_model.py @@ -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") From 51dc7dffb7bb05437b9ac490572b65ebcb2f4ca5 Mon Sep 17 00:00:00 2001 From: Trey Spiller Date: Wed, 4 Jun 2025 18:40:05 -0500 Subject: [PATCH 3/3] Remove kind opening parenthesis --- sqlmesh/core/model/kind.py | 2 +- tests/core/test_model.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sqlmesh/core/model/kind.py b/sqlmesh/core/model/kind.py index 6395b13f99..f58127dcdf 100644 --- a/sqlmesh/core/model/kind.py +++ b/sqlmesh/core/model/kind.py @@ -1016,7 +1016,7 @@ def create_model_kind(v: t.Any, dialect: str, defaults: t.Dict[str, t.Any]) -> M return actual_kind_type(**props) validate_extra_and_required_fields( - kind_type, set(props), f"MODEL block 'kind {name}(' field" + kind_type, set(props), f"MODEL block 'kind {name}' field" ) return kind_type(**props) diff --git a/tests/core/test_model.py b/tests/core/test_model.py index 40466a02b7..7a2f808e12 100644 --- a/tests/core/test_model.py +++ b/tests/core/test_model.py @@ -629,7 +629,7 @@ def test_model_field_name_suggestions(): 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'?" + == "Invalid field name present in the MODEL block 'kind INCREMENTAL_BY_TIME_RANGE' field: 'batch_sizes'. Did you mean 'batch_size'?" ) # multiple fields @@ -675,7 +675,7 @@ def test_model_required_field_missing(): 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." + == "Please add required field 'time_column' to the MODEL block 'kind INCREMENTAL_BY_TIME_RANGE' field." )