Skip to content

Fix: dont try to serialize engine adapter to sql in macro template method#5455

Merged
georgesittas merged 2 commits intoSQLMesh:mainfrom
z3z1ma:fix/dont-ser-engine
Oct 2, 2025
Merged

Fix: dont try to serialize engine adapter to sql in macro template method#5455
georgesittas merged 2 commits intoSQLMesh:mainfrom
z3z1ma:fix/dont-ser-engine

Conversation

@z3z1ma
Copy link
Contributor

@z3z1ma z3z1ma commented Sep 29, 2025

No idea why this is happening but its a doozy, completely tanks sqlmesh. Seems to happen in blueprint model path (but do your due diligence if you investigate this). So anyone using blueprints would [probably] have memory spikes and program lock ups.

image
(Pdb) pp self.locals
{'__sqlmesh__blueprint__vars__': {'object': Literal(this='account', is_string=True)},
 'default_catalog': 'segment-warehouse-236622',
 'engine_adapter': <sqlmesh.core.engine_adapter.bigquery.BigQueryEngineAdapter object at 0x34d7ae540>,
 'execution_date': datetime.date(2025, 9, 29),
 'execution_ds': '2025-09-29',
 'execution_dt': datetime.datetime(2025, 9, 29, 22, 25, 52, 957000, tzinfo=datetime.timezone.utc),
 'execution_dtntz': datetime.datetime(2025, 9, 29, 22, 25, 52, 957000),
 'execution_epoch': 1759184752.957,
 'execution_hour': 22,
 'execution_millis': 1759184752957,
 'execution_ts': '2025-09-29 22:25:52.957000',
 'execution_tstz': '2025-09-29 22:25:52.957000+00:00',
 'latest_date': datetime.date(2025, 9, 29),
 'latest_ds': '2025-09-29',
 'latest_dt': datetime.datetime(2025, 9, 29, 22, 25, 52, 957000, tzinfo=datetime.timezone.utc),
 'latest_dtntz': datetime.datetime(2025, 9, 29, 22, 25, 52, 957000),
 'latest_epoch': 1759184752.957,
 'latest_hour': 22,
 'latest_millis': 1759184752957,
 'latest_ts': '2025-09-29 22:25:52.957000',
 'latest_tstz': '2025-09-29 22:25:52.957000+00:00',
 'model_kind_name': 'VIEW',
 'runtime_stage': 'evaluating',
 'selected_models': set(),
 'snapshot': Snapshot<name: "segment-warehouse-236622"."analytics_normalized"."lkp_gong_call__account", dev_version_: 637983665, change_category: 1, fingerprint: SnapshotFingerprint<181920018, data: 3700948483, meta: 4068885469, pdata: 2940760196, pmeta: 396221053>, forward_only: True, node: SqlModel<analytics_normalized.lkp_gong_call__account>, parents: (SnapshotId<"segment-warehouse-236622"."analytics_staging"."stg_gong__call_context": 4083958684>, SnapshotId<"segment-warehouse-236622"."analytics_staging"."stg_gong__calls": 3436451818>), intervals: [(1577836800000, 1759017600000)], created_ts: 1758822746641, updated_ts: 1759054546933, ttl: in 3 days, version: novde, unpaused_ts: 1759050900000>,
 'snapshot_table_exists': True,
 'this_model': Table(
  this=Identifier(this='lkp_gong_call__account', quoted=True),
  db=Identifier(this='analytics_normalized', quoted=True),
  catalog=Identifier(this='segment-warehouse-236622', quoted=True),
  _comments=[
    segment-warehouse-236622.analytics_normalized.lkp_gong_call__account])}

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.

Good catch, thanks! This looks reasonable but will take a quick look to see if more things need to be excluded. The selected_models set seems a bit off too.

@georgesittas
Copy link
Contributor

@z3z1ma actually, did you figure out what causes the memory spike? I'm not sure I understand yet. I had assumed that the issue was convert_sql(v, self.dialect) when v is the engine adapter, but it appears that the exp.convert just raises in that case and returns the adapter itself:

(Pdb) convert_sql(base_mapping["engine_adapter"], self.dialect)
<sqlmesh.core.engine_adapter.duckdb.DuckDBEngineAdapter object at 0x10cd53650>

@alex-harness
Copy link

alex-harness commented Oct 1, 2025

Its something hitting the __dict__ path in exp.convert.
If not engine adapter - did you try the snapshot?

Let me think, I bet if you had a snapshot from a blueprint model, there is some janky recursion/condition that blows things up recursively going into __dict__ as it does.

Try it on a blueprint model snapshot with 100 or so variants at least. That's where it was easy to see the downhill

@georgesittas
Copy link
Contributor

Gotcha. Will revisit this tomorrow morning, thanks for following up. That theory sounds plausible.

@georgesittas
Copy link
Contributor

georgesittas commented Oct 2, 2025

Your theory is correct @z3z1ma:

> /Users/georgesittas/Code/tobiko/sqlmesh/.venv/lib/python3.13/site-packages/sqlglot/expressions.py(9186)convert()
-> return Struct(
(Pdb) value
Snapshot<name: "memory"."testing_schema"."t", dev_version_: 379305693, change_category: 1, fingerprint: SnapshotFingerprint<3570873088, data: 292153945, meta: 2702926817, pdata:
2167820958, pmeta: 542419559>, node: SqlModel<testing_schema.t: m.sql>, parents: (SnapshotId<"memory"."testing_schema"."other": 844781549>,), created_ts: 1759408538101, updated_ts:
1759408538101, ttl: in 1 week, version: 379305693>
(Pdb)
(Pdb) value.__dict__.items()
dict_items([('name', '"memory"."testing_schema"."t"'), ('dev_version_', '379305693'), ('change_category', BREAKING), ('fingerprint', SnapshotFingerprint<3570873088, data: 292153945,
meta: 2702926817, pdata: 2167820958, pmeta: 542419559>), ('previous_versions', ()), ('base_table_name_override', None), ('dev_table_suffix', 'dev'), ('table_naming_convention',
SCHEMA_AND_TABLE), ('forward_only', False), ('physical_schema_', None), ('node', SqlModel<testing_schema.t: m.sql>), ('parents', (SnapshotId<"memory"."testing_schema"."other":
844781549>,)), ('intervals', []), ('dev_intervals', []), ('pending_restatement_intervals', []), ('created_ts', 1759408538101), ('updated_ts', 1759408538101), ('ttl', 'in 1 week'),
('version', '379305693'), ('unpaused_ts', None), ('effective_from', None), ('migrated', False), ('unrestorable', False), ('next_auto_restatement_ts', None), ('last_altered_ts', None),
('dev_last_altered_ts', None), ('identifier', '3570873088'), ('snapshot_id', SnapshotId<"memory"."testing_schema"."t": 3570873088>), ('fully_qualified_table', Table(
  this=Identifier(this='t', quoted=True),
  db=Identifier(this='testing_schema', quoted=True),
  catalog=Identifier(this='memory', quoted=True)))])

It eventually does fail, probably because to_identifier(k) or the recursive convert fails. The same thing happens for the engine adapter, since it contains a __dict__ attribute.

@georgesittas georgesittas merged commit 4933d29 into SQLMesh:main Oct 2, 2025
30 of 32 checks passed
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.

3 participants