Skip to content

optimize everything even without schema #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tobymao merged 4 commits into from
Feb 16, 2023
Merged

optimize everything even without schema #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ac07d7
optimize everything even without schema
tobymao Feb 16, 2023
e7def44
Adjust settings for the diffing algorithm in the categorizer to bette…
izeigerman Feb 16, 2023
7573b82
always annotate types
tobymao Feb 16, 2023
4b604fd
Merge branch 'main' into toby/optimize_all
tobymao Feb 16, 2023
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
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"requests",
"rich",
"ruamel.yaml",
"sqlglot>=11.1.0",
"sqlglot>=11.1.2",
],
extras_require={
"dev": [
Expand Down
5 changes: 2 additions & 3 deletions sqlmesh/core/model/definition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from astor import to_source
from pydantic import Field
from sqlglot import exp
from sqlglot.optimizer.annotate_types import annotate_types
from sqlglot.optimizer.scope import traverse_scope
from sqlglot.schema import MappingSchema
from sqlglot.time import format_time
Expand Down Expand Up @@ -575,9 +574,9 @@ def columns_to_types(self) -> t.Dict[str, exp.DataType]:
return self.columns_to_types_

if self._columns_to_types is None:
query = annotate_types(self._query_renderer.render())
Copy link
Member

@izeigerman izeigerman Feb 16, 2023

Choose a reason for hiding this comment

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

This is problematic. If the optimizer fails because of something other than type annotation we'll ignore the error in the QueryRenderer but will then get an obscure error while trying to add a time filter:

sqlmesh/core/model/definition.py:544: in render_query
    return self._query_renderer.render(
sqlmesh/core/renderer.py:188: in render
    self.filter_time_column(node, *dates[0:2])
sqlmesh/core/renderer.py:216: in filter_time_column
    low = self._time_converter(start)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = Model<name: a, query: SELECT 1, ds>, time = datetime.datetime(1970, 1, 1, 0, 0, tzinfo=datetime.timezone.utc)

    def convert_to_time_column(self, time: TimeLike) -> exp.Expression:
        """Convert a TimeLike object to the same time format and type as the model's time column."""
        if self.time_column:
            if self.time_column.format:
                time = to_datetime(time).strftime(self.time_column.format)

            time_column_type = self.columns_to_types[self.time_column.column]
>           if time_column_type.this in exp.DataType.TEXT_TYPES:
E           AttributeError: 'NoneType' object has no attribute 'this'

Copy link
Contributor Author

@tobymao tobymao Feb 16, 2023

Choose a reason for hiding this comment

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

annotate types doesn’t work well on unoptimizable sql.

self._columns_to_types = {
expression.alias_or_name: expression.type for expression in query.expressions
expression.alias_or_name: expression.type
for expression in self._query_renderer.render().expressions
}

return self._columns_to_types
Expand Down
33 changes: 13 additions & 20 deletions sqlmesh/core/renderer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from jinja2 import Environment
from sqlglot import exp, parse_one
from sqlglot.errors import SchemaError, SqlglotError
from sqlglot.errors import OptimizeError, SchemaError, SqlglotError
from sqlglot.optimizer import optimize
from sqlglot.optimizer.annotate_types import annotate_types
from sqlglot.optimizer.expand_laterals import expand_laterals
Expand All @@ -30,7 +30,6 @@
qualify_tables,
qualify_columns,
expand_laterals,
annotate_types,
)


Expand Down Expand Up @@ -141,24 +140,18 @@ def render(
except MacroEvalError as ex:
raise_config_error(f"Failed to resolve macro for query. {ex}", self._path)

if self._schema:
# This takes care of expanding star projections

try:
self._query_cache[cache_key] = optimize(
self._query_cache[cache_key],
schema=self._schema,
rules=RENDER_OPTIMIZER_RULES,
)
except SchemaError:
pass
except SqlglotError as ex:
raise_config_error(f"Invalid model query. {ex}", self._path)

self._columns_to_types = {
expression.alias_or_name: expression.type
for expression in self._query_cache[cache_key].expressions
}
try:
self._query_cache[cache_key] = optimize(
self._query_cache[cache_key],
schema=self._schema,
rules=RENDER_OPTIMIZER_RULES,
)
except (SchemaError, OptimizeError):
pass
except SqlglotError as ex:
raise_config_error(f"Invalid model query. {ex}", self._path)

self._query_cache[cache_key] = annotate_types(self._query_cache[cache_key])

query = self._query_cache[cache_key]

Expand Down
4 changes: 2 additions & 2 deletions sqlmesh/core/snapshot/categorizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import typing as t

from sqlglot import exp
from sqlglot.diff import Insert, Keep, diff
from sqlglot.diff import ChangeDistiller, Insert, Keep

from sqlmesh.core.snapshot.definition import Snapshot, SnapshotChangeCategory

Expand Down Expand Up @@ -33,7 +33,7 @@ def categorize_change(new: Snapshot, old: Snapshot) -> t.Optional[SnapshotChange
):
return None

edits = diff(old_model.render_query(), new_model.render_query())
edits = ChangeDistiller(t=0.5).diff(old_model.render_query(), new_model.render_query())
inserted_expressions = {e.expression for e in edits if isinstance(e, Insert)}

for edit in edits:
Expand Down
62 changes: 31 additions & 31 deletions tests/core/test_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,15 @@ def test_load(assert_exp_eq):
@pytest.mark.parametrize(
"query, error",
[
("sum(x)::int", "must have inferrable names"),
("CAST(x + 1 AS INT)", "must have inferrable names"),
("y::int, x::int AS y", "duplicate"),
("sum(x)::int -- annotation", "must have inferrable names"),
],
)
def test_model_validation(query, error):
expressions = parse(
f"""
MODEL (
name db.table,
kind FULL,
);

SELECT {query}
Expand Down Expand Up @@ -473,25 +471,27 @@ def test_render_query(assert_exp_eq):
assert_exp_eq(
model.render_query(start="2020-10-28", end="2020-10-28"),
"""
SELECT y
FROM x
SELECT
x.y AS y
FROM x AS x
WHERE
y <= '2020-10-28'
AND y <= TIME_STR_TO_TIME('2020-10-28T23:59:59.999000+00:00')
AND y >= '2020-10-28'
AND y >= TIME_STR_TO_TIME('2020-10-28T00:00:00+00:00')
x.y <= '2020-10-28'
AND x.y <= TIME_STR_TO_TIME('2020-10-28T23:59:59.999000+00:00')
AND x.y >= '2020-10-28'
AND x.y >= TIME_STR_TO_TIME('2020-10-28T00:00:00+00:00')
""",
)
assert_exp_eq(
model.render_query(start="2020-10-28", end=to_datetime("2020-10-29")),
"""
SELECT y
FROM x
SELECT
x.y AS y
FROM x AS x
WHERE
y <= '2020-10-28'
AND y <= TIME_STR_TO_TIME('2020-10-28T23:59:59.999000+00:00')
AND y >= '2020-10-28'
AND y >= TIME_STR_TO_TIME('2020-10-28T00:00:00+00:00')
x.y <= '2020-10-28'
AND x.y <= TIME_STR_TO_TIME('2020-10-28T23:59:59.999000+00:00')
AND x.y >= '2020-10-28'
AND x.y >= TIME_STR_TO_TIME('2020-10-28T00:00:00+00:00')
""",
)

Expand Down Expand Up @@ -685,13 +685,13 @@ def test_filter_time_column(assert_exp_eq):
model.render_query(start="2021-01-01", end="2021-01-01", latest="2021-01-01"),
"""
SELECT
id::INT AS id,
name::TEXT AS name,
price::DOUBLE AS price,
ds::TEXT AS ds
FROM raw.items
items.id::INT AS id,
items.name::TEXT AS name,
items.price::DOUBLE AS price,
items.ds::TEXT AS ds
FROM raw.items AS items
WHERE
CAST(ds AS TEXT) <= '20210101' AND CAST(ds as TEXT) >= '20210101'
CAST(items.ds AS TEXT) <= '20210101' AND CAST(items.ds AS TEXT) >= '20210101'
""",
)

Expand Down Expand Up @@ -720,13 +720,13 @@ def test_filter_time_column(assert_exp_eq):
model.render_query(start="2021-01-01", end="2021-01-01", latest="2021-01-01"),
"""
SELECT
id::INT AS id,
name::TEXT AS name,
price::DOUBLE AS price,
ds::TEXT AS ds
FROM raw.items
items.id::INT AS id,
items.name::TEXT AS name,
items.price::DOUBLE AS price,
items.ds::TEXT AS ds
FROM raw.items AS items
WHERE
CAST(ds AS TEXT) <= '20210101' AND CAST(ds as TEXT) >= '20210101'
CAST(items.ds AS TEXT) <= '20210101' AND CAST(items.ds as TEXT) >= '20210101'
""",
)

Expand Down Expand Up @@ -761,11 +761,11 @@ def test_parse_model(assert_exp_eq):
model.render_query(),
"""
SELECT
CAST(id AS INT) AS id,
ds
FROM x
CAST(x.id AS INT) AS id,
x.ds AS ds
FROM x AS x
WHERE
ds <= '1970-01-01' AND ds >= '1970-01-01'
x.ds <= '1970-01-01' AND x.ds >= '1970-01-01'
""",
)

Expand Down
2 changes: 1 addition & 1 deletion tests/core/test_snapshot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def test_fingerprint(model: Model, parent_model: Model):
fingerprint = fingerprint_from_model(model, models={})

original_fingerprint = SnapshotFingerprint(
data_hash="3118027933",
data_hash="2278368927",
metadata_hash="3589467163",
)

Expand Down
6 changes: 3 additions & 3 deletions tests/dbt/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,18 @@ def test_variables(assert_exp_eq):

# Case 2: using a defined variable without a default value
defined_variables["foo"] = 6
assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), "SELECT 6")
assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), 'SELECT 6 AS "6"')

# Case 3: using a defined variable with a default value
model_config._variables["foo"] = True
model_config.sql = "SELECT {{ var('foo', 5) }}"

assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), "SELECT 6")
assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), 'SELECT 6 AS "6"')

# Case 4: using an undefined variable with a default value
del defined_variables["foo"]

assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), "SELECT 5")
assert_exp_eq(model_config.to_sqlmesh(**kwargs).render_query(), 'SELECT 5 AS "5"')


def test_source_config(sushi_dbt_project: Project):
Expand Down