Skip to content
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

Delete column from Materialized View #65

Closed
wants to merge 7 commits into from

Conversation

kukukaka
Copy link
Contributor

Add a new use case to explain how to remove a column from a Materialized View.

@kukukaka kukukaka requested a review from xavijam November 17, 2023 17:50
@xavijam
Copy link
Member

xavijam commented Nov 20, 2023

There is a problem with your git connection, take a look to the documentation, specially:

In case you need to override commit value you can use tb init --git --override-commit <commit_sha>.

@kukukaka kukukaka linked an issue Nov 21, 2023 that may be closed by this pull request
set -e

tb deploy
../utils/populate_with_backfill.sh "date" "analytics_sessions_mv_v1" "analytics_sessions_v1" "analytics_sessions_1" "analytics_sessions_mv" "" "timestamp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beware the double underscore in analytics_sessions_mv__v1, since we use that suffix as convention for VERSION and we are about to deprecate it, I'd directly use a different .datasource name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted the double underscore and just kept a single one _v1 to just work as a new resource but I keep getting an error in the backfill that I'm not sure how to solve.

Looking at the action log, it's detecting the change as renames. I'm not sure if it's the issue but is not what I was trying to do:

renamed: delete_column_materialized_view/datasources/analytics_sessions_mv.datasource -> delete_column_materialized_view/datasources/analytics_sessions_mv_v1.datasource
renamed: delete_column_materialized_view/pipes/analytics_sessions.pipe -> delete_column_materialized_view/pipes/analytics_sessions_v1.pipe

And then at line 182 I'm getting a:
Pipe 'analytics_sessions_v1' not found

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To debug the issue I went to the Environment related to this PR tmp_ci__65 and I've seen the changes have not been deployed.

The reason is tb deploy does not deploy renamed resources (we have an issue about it). Rename is tricky because one thing is renaming a resource that keeps the same content (schema or nodes) and a different thing is renaming a resource with a change which should be a new resource + drop the old one.

For now, if we don't support rename tb deploy should raise an error.

I'm going to see if I can undo the rename

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know, and good to document it @alrocar

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After creating new resources instead of renaming, fixture tests started to fail, it's because we are using a pre-release version of the CLI and we need to hide the version warning.

It can be done adding TB_VERSION_WARNING=0 as an env variable in the .tinyenv file, if you look at the previous version it had it, I guess tb release generate --semver preserves content of .tinyenv file so maybe it was modified by hand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you a lot.

@kukukaka kukukaka force-pushed the delete_column_materialized_view branch from 332a7b8 to 52b48d5 Compare November 21, 2023 15:36
@alrocar
Copy link
Member

alrocar commented Nov 21, 2023

One last comment populate_with_backfill just works if the target datasource has a DateTime column to do the backfill, in this case the column date is a Date, so when looking for the min date (here) we are loosing data when doing the backfill.

Let me think how to make it work better for this specific case.

@xavijam xavijam closed this Mar 14, 2024
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.

Use case - Remove column Materialized with downstream
3 participants