Skip to content

Fix: update sushi.marketing so it does not check scd time column types#3988

Closed
georgesittas wants to merge 1 commit intomainfrom
jo/fix_ci
Closed

Fix: update sushi.marketing so it does not check scd time column types#3988
georgesittas wants to merge 1 commit intomainfrom
jo/fix_ci

Conversation

@georgesittas
Copy link
Contributor

This commit changed how DuckDB parses DATETIME and TIMESTAMP; these types are now not timezone-aware.

The marketing sushi model started failing due to this change, because there's an assertion about its columns & their types. The catch is that the "expected" types for valid_from and valid_to in this mapping are parsed using DuckDB, so the resulting DataType instances are different than the actual ones:

# Expected
('valid_from', DataType(this=Type.TIMESTAMPNTZ, nested=False)),
('valid_to', DataType(this=Type.TIMESTAMPNTZ, nested=False))

# Actual
('valid_from', DataType(this=Type.TIMESTAMP, nested=False)),
('valid_to', DataType(this=Type.TIMESTAMP, nested=False))

Here's where the SCD2 types are constructed: https://github.com/TobikoData/sqlmesh/blob/cdd97c51a14eb1f4441228a6cbd479fd77587849/sqlmesh/core/model/kind.py#L674

Just putting this here for discussion, but not sure yet if we should change the above DataType.build call in the SCD kind's time_data_type to build a TIMESTAMPNTZ instead.

@georgesittas georgesittas requested review from a team, eakmanrq and tobymao March 13, 2025 16:38
@tobymao
Copy link
Contributor

tobymao commented Mar 13, 2025

@eakmanrq

@georgesittas
Copy link
Contributor Author

Closing this for now as I temporarily reverted the DuckDB changes upstream, see #3989.

@georgesittas georgesittas deleted the jo/fix_ci branch March 13, 2025 18:45
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.

2 participants