Skip to content

Fix!: Pass the model dialect when computing a hash for the column types#4067

Merged
izeigerman merged 1 commit intomainfrom
fix-pass-dialect-column-types-hash
Apr 1, 2025
Merged

Fix!: Pass the model dialect when computing a hash for the column types#4067
izeigerman merged 1 commit intomainfrom
fix-pass-dialect-column-types-hash

Conversation

@izeigerman
Copy link
Collaborator

Otherwise, there can be a mismatch between the hash computed using the original instance of a model and the deserialized one when the type is constructed using the default dialect. (eg. exp.DataType.build("int")).

Example:

In [6]: exp.DataType.build("int").sql("bigquery")
Out[6]: 'INT64'

In [7]: exp.DataType.build("int64", dialect="bigquery")
Out[7]: DataType(this=Type.BIGINT, nested=False)

@izeigerman izeigerman requested a review from a team April 1, 2025 05:46
@izeigerman izeigerman force-pushed the fix-pass-dialect-column-types-hash branch from 89faa87 to 503b839 Compare April 1, 2025 05:49
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.

Is it correct that we don't need a migration script for this because state's not changed?

@izeigerman
Copy link
Collaborator Author

@georgesittas We do need a migration script to trigger the fingerprint re-compute, it just the test didn't catch it, cause we don't have this problem there. Good catch

@izeigerman izeigerman force-pushed the fix-pass-dialect-column-types-hash branch from 503b839 to 0fb7e71 Compare April 1, 2025 18:09
@izeigerman izeigerman force-pushed the fix-pass-dialect-column-types-hash branch from 0fb7e71 to 6a63673 Compare April 1, 2025 18:10
@izeigerman izeigerman merged commit 6350d24 into main Apr 1, 2025
22 checks passed
@izeigerman izeigerman deleted the fix-pass-dialect-column-types-hash branch April 1, 2025 18:33
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