Skip to content

Commit 9a92b6c

Browse files
committed
Merge branch 'main' into feat/invalidate-cleanup-snapshots
Resolves conflict in sqlmesh/core/console.py by taking upstream's canonical IPython None-check fix (#5933) over our redundant duplicate of the same fix. Signed-off-by: mday-io <mdaytn@gmail.com>
2 parents 925011a + 6a563f5 commit 9a92b6c

16 files changed

Lines changed: 813 additions & 22 deletions

File tree

sqlmesh/core/console.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,11 +2920,10 @@ def __init__(
29202920

29212921
super().__init__(console, **kwargs)
29222922

2923-
shell = get_ipython()
2924-
user_ns_display = (
2925-
shell.user_ns.get("display", ipython_display) if shell else ipython_display
2923+
ipython = get_ipython()
2924+
self.display = display or (
2925+
ipython.user_ns.get("display", ipython_display) if ipython else ipython_display
29262926
)
2927-
self.display = display or user_ns_display
29282927
self.missing_dates_output = widgets.Output()
29292928
self.dynamic_options_after_categorization_output = widgets.VBox()
29302929

sqlmesh/core/context.py

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@
122122
filter_tests_by_patterns,
123123
)
124124
from sqlmesh.core.user import User
125-
from sqlmesh.utils import UniqueKeyDict, Verbosity
125+
from sqlmesh.utils import CorrelationId, UniqueKeyDict, Verbosity
126126
from sqlmesh.utils.concurrency import concurrent_apply_to_values
127127
from sqlmesh.utils.dag import DAG
128128
from sqlmesh.utils.date import (
@@ -815,6 +815,9 @@ def run(
815815
engine_type=self.snapshot_evaluator.adapter.dialect,
816816
state_sync_type=self.state_sync.state_type(),
817817
)
818+
snapshot_evaluator = self.snapshot_evaluator.set_correlation_id(
819+
CorrelationId.from_run_id(analytics_run_id)
820+
)
818821
self._load_materializations()
819822

820823
env_check_attempts_num = max(
@@ -867,6 +870,7 @@ def _has_environment_changed() -> bool:
867870
select_models=select_models,
868871
circuit_breaker=_has_environment_changed,
869872
no_auto_upstream=no_auto_upstream,
873+
snapshot_evaluator=snapshot_evaluator,
870874
)
871875
done = True
872876
except CircuitBreakerError:
@@ -2616,8 +2620,9 @@ def _run(
26162620
select_models: t.Optional[t.Collection[str]],
26172621
circuit_breaker: t.Optional[t.Callable[[], bool]],
26182622
no_auto_upstream: bool,
2623+
snapshot_evaluator: t.Optional[SnapshotEvaluator] = None,
26192624
) -> CompletionStatus:
2620-
scheduler = self.scheduler(environment=environment)
2625+
scheduler = self.scheduler(environment=environment, snapshot_evaluator=snapshot_evaluator)
26212626
snapshots = scheduler.snapshots
26222627

26232628
if select_models is not None:
@@ -3107,10 +3112,17 @@ def _cleanup_environments(
31073112
expired_env = self.state_reader.get_environment(expired_env_summary.name)
31083113

31093114
if expired_env:
3115+
cleanup_default_adapter, cleanup_engine_adapters, failure = (
3116+
self._cleanup_adapters_for_environment(expired_env)
3117+
)
3118+
if failure:
3119+
logger.warning(failure)
3120+
failures.append(failure)
3121+
continue
31103122
failures.extend(
31113123
cleanup_expired_views(
3112-
default_adapter=self.engine_adapter,
3113-
engine_adapters=self.engine_adapters,
3124+
default_adapter=cleanup_default_adapter,
3125+
engine_adapters=cleanup_engine_adapters,
31143126
environments=[expired_env],
31153127
console=self.console,
31163128
)
@@ -3122,6 +3134,62 @@ def _cleanup_environments(
31223134
self.state_sync.delete_expired_environments(current_ts=current_ts, name=name)
31233135
return failures
31243136

3137+
def _cleanup_adapters_for_environment(
3138+
self, environment: Environment
3139+
) -> t.Tuple[EngineAdapter, t.Dict[str, EngineAdapter], t.Optional[str]]:
3140+
"""Create cleanup-scoped adapters for an expired environment.
3141+
3142+
Persisted catalog-qualified view names indicate that virtual catalog injection was active,
3143+
so cleanup can clone only the selected adapters with the historical catalog and leave the
3144+
context's adapters unchanged.
3145+
"""
3146+
engine_adapters = self.engine_adapters
3147+
default_adapter = self.engine_adapter
3148+
catalogs_by_gateway: t.Dict[str, t.Set[str]] = collections.defaultdict(set)
3149+
3150+
for snapshot in environment.snapshots:
3151+
if not snapshot.is_model or snapshot.is_symbolic:
3152+
continue
3153+
3154+
gateway = (
3155+
snapshot.model_gateway
3156+
if environment.gateway_managed and snapshot.model_gateway in engine_adapters
3157+
else self.selected_gateway
3158+
)
3159+
adapter = engine_adapters.get(gateway, default_adapter)
3160+
catalog = snapshot.qualified_view_name.catalog_for_environment(
3161+
environment.naming_info, dialect=adapter.dialect
3162+
)
3163+
if catalog and adapter.supports_virtual_catalog() is True:
3164+
catalogs_by_gateway[gateway].add(catalog)
3165+
3166+
for gateway, catalogs in catalogs_by_gateway.items():
3167+
if len(catalogs) > 1:
3168+
catalogs_description = ", ".join(f"'{catalog}'" for catalog in sorted(catalogs))
3169+
return (
3170+
default_adapter,
3171+
engine_adapters,
3172+
(
3173+
f"Failed to clean up expired environment '{environment.name}': gateway "
3174+
f"'{gateway}' references multiple virtual catalogs: {catalogs_description}"
3175+
),
3176+
)
3177+
3178+
cleanup_engine_adapters = engine_adapters.copy()
3179+
cleanup_default_adapter = default_adapter
3180+
for gateway, catalogs in catalogs_by_gateway.items():
3181+
cleanup_adapter = engine_adapters.get(gateway, default_adapter).with_settings()
3182+
cleanup_adapter.inject_virtual_catalog(gateway)
3183+
# inject_virtual_catalog() may initialize adapter-specific state in addition to
3184+
# _default_catalog. Override only the cleanup clone with the catalog persisted in the
3185+
# expired environment so historical names pass SINGLE_CATALOG_ONLY validation.
3186+
cleanup_adapter._default_catalog = next(iter(catalogs))
3187+
cleanup_engine_adapters[gateway] = cleanup_adapter
3188+
if gateway == self.selected_gateway:
3189+
cleanup_default_adapter = cleanup_adapter
3190+
3191+
return cleanup_default_adapter, cleanup_engine_adapters, None
3192+
31253193
def _try_connection(self, connection_name: str, validator: t.Callable[[], None]) -> None:
31263194
connection_name = connection_name.capitalize()
31273195
try:

sqlmesh/core/engine_adapter/base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ def with_settings(self, **kwargs: t.Any) -> EngineAdapter:
178178
"query_execution_tracker": kwargs.pop(
179179
"query_execution_tracker", self._query_execution_tracker
180180
),
181+
"pre_ping": kwargs.pop("pre_ping", self._pre_ping),
181182
**self._extra_config,
182183
**kwargs,
183184
}

sqlmesh/core/engine_adapter/clickhouse.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ def _build_table_properties_exp(
850850
primary_key_vals = []
851851
if isinstance(primary_key, (exp.Tuple, exp.Array)):
852852
primary_key_vals = primary_key.expressions
853-
if isinstance(ordered_by_raw, exp.Paren):
853+
if isinstance(primary_key, exp.Paren):
854854
primary_key_vals = [primary_key.this]
855855

856856
if not primary_key_vals:

sqlmesh/core/macros.py

Lines changed: 93 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -965,17 +965,84 @@ def generate_surrogate_key(
965965
)
966966
)
967967

968+
concat = exp.func("CONCAT", *string_fields)
969+
# The argument is always a string; annotating it here lets generators that
970+
# split string/binary hash semantics (Presto, Trino) wrap the encode.
971+
concat.type = exp.DataType.build("text")
972+
968973
func = exp.func(
969974
hash_function.name,
970-
exp.func("CONCAT", *string_fields),
975+
concat,
971976
dialect=evaluator.dialect,
972977
)
973978
if isinstance(func, exp.MD5Digest):
974979
func = exp.MD5(this=func.this)
980+
elif isinstance(func, exp.SHA2Digest):
981+
# Same split as MD5/MD5Digest: the surrogate key must be a hex string,
982+
# not a binary digest, on every dialect.
983+
func = exp.SHA2(this=func.this, length=func.args.get("length"))
984+
elif isinstance(func, exp.Anonymous) and _is_presto_family(evaluator.dialect):
985+
# Athena runs the Trino engine, so sha256() takes varbinary there too,
986+
# but its parser has no SHA256/SHA512 entry: exp.func returns an
987+
# Anonymous node, so neither branch above fires and the surrogate key
988+
# keeps the bare SHA256(varchar) form reported in #5871. Unlike the
989+
# probe below, this is not a pin-era workaround — Athena still parses
990+
# to Anonymous on sqlglot versions that carry tobymao/sqlglot#7824.
991+
#
992+
# Anonymous is the catch-all for every unrecognised function name, and
993+
# hash_function is caller-supplied, so the name is checked rather than
994+
# assumed: an unknown hash must pass through untouched.
995+
length = _SHA2_DIGEST_LENGTHS.get(func.name.upper())
996+
if length is not None:
997+
func = exp.SHA2(this=concat, length=exp.Literal.number(length))
998+
999+
if isinstance(func, exp.SHA2) and _sha2_renders_binary(evaluator.dialect):
1000+
# Presto/Trino render a bare SHA256(varchar) for exp.SHA2 on sqlglot
1001+
# versions without tobymao/sqlglot#7824: a type error on Trino, and
1002+
# binary rather than string semantics where it runs. Build the
1003+
# hex-string form explicitly, mirroring what those generators do for
1004+
# MD5: LOWER(TO_HEX(SHA256(TO_UTF8(...)))). The probe keeps this
1005+
# branch inert once sqlglot renders the hex form natively, so the
1006+
# expression is never wrapped twice.
1007+
return exp.Lower(
1008+
this=exp.Hex(
1009+
this=exp.SHA2(
1010+
this=exp.Encode(this=func.this, charset=exp.Literal.string("utf-8")),
1011+
length=func.args.get("length"),
1012+
)
1013+
)
1014+
)
9751015

9761016
return func
9771017

9781018

1019+
# Dialects that model string and binary hashes separately, so a bare
1020+
# SHA256(varchar) is a type error rather than a hex-string surrogate key.
1021+
# Athena is on the list because it runs the Trino engine.
1022+
_PRESTO_FAMILY = frozenset({"presto", "trino", "athena"})
1023+
1024+
# The SHA-2 digest widths a surrogate key may ask for, by function name.
1025+
_SHA2_DIGEST_LENGTHS = {"SHA256": 256, "SHA512": 512}
1026+
1027+
1028+
def _is_presto_family(dialect: DialectType) -> bool:
1029+
"""Whether this dialect is Presto, Trino or Athena."""
1030+
return (str(dialect) if dialect else "").split(",")[0].strip().lower() in _PRESTO_FAMILY
1031+
1032+
1033+
@lru_cache(maxsize=None)
1034+
def _sha2_renders_binary(dialect: DialectType) -> bool:
1035+
"""Whether this dialect renders exp.SHA2 as a bare binary-semantics call.
1036+
1037+
Only the Presto family models string and binary hashes separately; other
1038+
dialects' SHA256(varchar) already returns a hex string.
1039+
"""
1040+
if not _is_presto_family(dialect):
1041+
return False
1042+
probe = exp.SHA2(this=exp.column("_sqlmesh_probe"), length=exp.Literal.number(256))
1043+
return "TO_HEX" not in probe.sql(dialect=dialect)
1044+
1045+
9791046
@macro()
9801047
def safe_add(_: MacroEvaluator, *fields: exp.Expr) -> exp.Case:
9811048
"""Adds numbers together, substitutes nulls for 0s and only returns null if all fields are null.
@@ -1379,15 +1446,17 @@ def resolve_template(
13791446
"""
13801447
Generates either a String literal or an exp.Table representing a physical table location, based on rendering the provided template String literal.
13811448
1382-
Note: It relies on the @this_model variable being available in the evaluation context (@this_model resolves to an exp.Table object
1383-
representing the current physical table).
1449+
Note: It relies on the @this_model variable being available in the evaluation context. @this_model usually resolves to an
1450+
exp.Table object representing the current physical table, but in an audit on a model with a time column it resolves to a
1451+
subquery that selects from that table and filters it down to the audited time range. In that case the placeholders below
1452+
are resolved against the physical table the subquery selects from.
13841453
Therefore, the @resolve_template macro must be used at creation or evaluation time and not at load time.
13851454
13861455
Args:
13871456
template: Template string literal. Can contain the following placeholders:
1388-
@{catalog_name} -> replaced with the catalog of the exp.Table returned from @this_model
1389-
@{schema_name} -> replaced with the schema of the exp.Table returned from @this_model
1390-
@{table_name} -> replaced with the name of the exp.Table returned from @this_model
1457+
@{catalog_name} -> replaced with the catalog of the physical table @this_model refers to
1458+
@{schema_name} -> replaced with the schema of the physical table @this_model refers to
1459+
@{table_name} -> replaced with the name of the physical table @this_model refers to
13911460
mode: What to return.
13921461
'literal' -> return an exp.Literal string
13931462
'table' -> return an exp.Table
@@ -1400,9 +1469,26 @@ def resolve_template(
14001469
>>> evaluator.locals.update({"this_model": exp.to_table("test_catalog.sqlmesh__test.test__test_model__2517971505")})
14011470
>>> evaluator.transform(parse_one(sql)).sql()
14021471
"'s3://data-bucket/prod/test_catalog/sqlmesh__test/test__test_model__2517971505'"
1472+
1473+
The same template resolves to the same location when @this_model is the time-filtered
1474+
subquery that audits on models with a time column receive:
1475+
1476+
>>> table = exp.to_table("test_catalog.sqlmesh__test.test__test_model__2517971505")
1477+
>>> subquery = exp.select("*").from_(table).where(exp.column("ds").eq("2020-01-01")).subquery()
1478+
>>> evaluator.locals.update({"this_model": subquery})
1479+
>>> evaluator.transform(parse_one(sql)).sql()
1480+
"'s3://data-bucket/prod/test_catalog/sqlmesh__test/test__test_model__2517971505'"
14031481
"""
14041482
if "this_model" in evaluator.locals:
1405-
this_model = exp.to_table(evaluator.locals["this_model"], dialect=evaluator.dialect)
1483+
this_model_expr = evaluator.locals["this_model"]
1484+
if isinstance(this_model_expr, exp.Subquery):
1485+
# Audits on models with a time column render @this_model as a subquery that filters the
1486+
# physical table on the audited time range, so resolve against the table it selects from
1487+
from_ = this_model_expr.unnest().args.get("from_")
1488+
if from_ is not None and isinstance(from_.this, exp.Table):
1489+
this_model_expr = from_.this
1490+
1491+
this_model = exp.to_table(this_model_expr, dialect=evaluator.dialect)
14061492
template_str: str = template.this
14071493
result = (
14081494
template_str.replace("@{catalog_name}", this_model.catalog)

sqlmesh/core/schema_loader.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def create_external_models_file(
4545
external_model_fqns = set()
4646

4747
for fqn, model in models.items():
48-
if model.kind.is_external:
48+
if model.kind.is_external and model._path == path:
4949
external_model_fqns.add(fqn)
5050
for dep in model.depends_on:
5151
if dep not in known_models:

sqlmesh/utils/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,10 @@ def __str__(self) -> str:
409409
def from_plan_id(cls, plan_id: str) -> CorrelationId:
410410
return CorrelationId(JobType.PLAN, plan_id)
411411

412+
@classmethod
413+
def from_run_id(cls, run_id: str) -> CorrelationId:
414+
return CorrelationId(JobType.RUN, run_id)
415+
412416

413417
def get_source_columns_to_types(
414418
columns_to_types: t.Dict[str, exp.DataType],

tests/core/engine_adapter/integration/test_integration_bigquery.py

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import typing as t
2+
from unittest import mock
3+
24
import pytest
5+
import time_machine
36
from pathlib import Path
47
from sqlglot import exp
58
from sqlglot.optimizer.qualify_columns import quote_identifiers
@@ -441,7 +444,7 @@ def test_table_diff_table_name_matches_column_name(ctx: TestContext):
441444
assert row_diff.full_match_count == 1
442445

443446

444-
def test_correlation_id_in_job_labels(ctx: TestContext):
447+
def test_plan_correlation_id_in_job_labels(ctx: TestContext):
445448
model_name = ctx.table("test")
446449

447450
sqlmesh = ctx.create_context()
@@ -469,3 +472,55 @@ def test_correlation_id_in_job_labels(ctx: TestContext):
469472
labels = adapter._job_params.get("labels")
470473
correlation_id = CorrelationId.from_plan_id(plan.plan_id)
471474
assert labels == {correlation_id.job_type.value.lower(): correlation_id.job_id}
475+
476+
477+
@time_machine.travel("2023-01-08 15:00:00 UTC")
478+
def test_run_correlation_id_in_job_labels(ctx: TestContext):
479+
run_id = "test_run_id"
480+
model_name = ctx.table("run_test")
481+
482+
sqlmesh = ctx.create_context()
483+
sqlmesh.upsert_model(
484+
load_sql_based_model(
485+
d.parse(
486+
f"""
487+
MODEL (
488+
name {model_name},
489+
kind INCREMENTAL_BY_TIME_RANGE (
490+
time_column event_ts
491+
),
492+
cron '@daily',
493+
start '2023-01-07'
494+
);
495+
SELECT 1 AS col, '2023-01-07' AS event_ts
496+
"""
497+
)
498+
)
499+
)
500+
sqlmesh.plan(auto_apply=True, no_prompts=True)
501+
502+
captured_evaluators: t.List = []
503+
original_scheduler = sqlmesh.scheduler
504+
505+
def scheduler_wrapper(
506+
environment: t.Optional[str] = None,
507+
snapshot_evaluator: t.Optional[t.Any] = None,
508+
):
509+
if snapshot_evaluator is not None:
510+
captured_evaluators.append(snapshot_evaluator)
511+
return original_scheduler(environment=environment, snapshot_evaluator=snapshot_evaluator)
512+
513+
with time_machine.travel("2023-01-09 00:00:00 UTC"):
514+
with mock.patch(
515+
"sqlmesh.core.context.analytics.collector.on_run_start", return_value=run_id
516+
):
517+
with mock.patch.object(sqlmesh, "scheduler", scheduler_wrapper):
518+
sqlmesh.run()
519+
520+
assert captured_evaluators
521+
adapter = t.cast(BigQueryEngineAdapter, captured_evaluators[-1].adapter)
522+
523+
assert adapter.correlation_id is not None
524+
labels = adapter._job_params.get("labels")
525+
correlation_id = CorrelationId.from_run_id(run_id)
526+
assert labels == {correlation_id.job_type.value.lower(): correlation_id.job_id}

tests/core/engine_adapter/test_base.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3522,6 +3522,15 @@ def test_pre_ping(mocker: MockerFixture, make_mocked_engine_adapter: t.Callable)
35223522
adapter._connection_pool.get().close.assert_called_once()
35233523

35243524

3525+
def test_with_settings_preserves_pre_ping(make_mocked_engine_adapter: t.Callable):
3526+
adapter = make_mocked_engine_adapter(EngineAdapter, pre_ping=True)
3527+
assert adapter.with_settings()._pre_ping is True
3528+
assert adapter.with_settings(pre_ping=False)._pre_ping is False
3529+
3530+
adapter = make_mocked_engine_adapter(EngineAdapter)
3531+
assert adapter.with_settings()._pre_ping is False
3532+
3533+
35253534
@pytest.mark.parametrize(
35263535
"partitioned_by",
35273536
[

0 commit comments

Comments
 (0)