-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Seed and generic test support for DuneSQL #3676
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.
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
Conversation
|
After this is merged and you migrate a model that uses seed tests you should do the following steps to allow the seed test to run in DuneSQL.
|
| count(model_{{seed_matching_columns[0]}}) as `result_model`, | ||
| 1 as `expected_seed`, | ||
| cast(count(model_{{seed_matching_columns[0]}}) as varchar) as result_model, | ||
| cast(1 as varchar) as expected_seed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the casts here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In spark these columns had all kind of types (depending on the type of the column we're comparing), but for trino we need to unify them to 1 type (hence the varchar choice).
Note these casted columns are not used for the actual comparison, they are just included in the result table so you can easily see the cause of errors in the test results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, makes sense. Could we add a comment on this? E.g., cast all columns to varchar to unify column types
* seed test support for dunesql * forgot one _legacy * add comment
_legacyversion_legacyversioncheck_seedgeneric macro and testsvarbinaryin seed columns for DuneSQL seeds