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

Migration: Add scroll_depth to imported_pages #4908

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

RobertJoonas
Copy link
Contributor

@RobertJoonas RobertJoonas commented Dec 16, 2024

Changes

This PR only contains the migration adding the scroll_depth UInt8 column to the imported_pages ClickHouse table.

The column will have a default value of 255 which hints that the value is non-existent (a valid scroll depth has to be from 0 to 100). The alternatives default values are:

  • 0 - cannot use because it can be a valid value in rare cases
  • NULL - bad performance in ClickHouse

When querying scroll depth from imported data, we'll simply filter out rows where scroll_depth is more than 100.

Copy link

Preview environment👷🏼‍♀️🏗️
PR-4908

Copy link
Contributor

@apata apata 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 irreversible actually? Seems to me that we could drop it just as easily as we add it.

@RobertJoonas
Copy link
Contributor Author

Is it irreversible actually? Seems to me that we could drop it just as easily as we add it.

You're right - I just figured that rolling it back would not be necessary in any case. The column has a default value, and it's not in the schema yet, so there shouldn't by any issues. I'll also run it on staging first and complete a full export -> import flow.

@RobertJoonas
Copy link
Contributor Author

The column has a default value, and it's not in the schema yet, so there shouldn't by any issues

Famous last words... Why not add it: a6e92ff

@RobertJoonas RobertJoonas force-pushed the scroll-depth-imported-pages-migration branch from b1057f4 to 7d27c5b Compare December 16, 2024 15:40
@RobertJoonas RobertJoonas added this pull request to the merge queue Dec 17, 2024
Merged via the queue into master with commit e82ae65 Dec 17, 2024
17 checks passed
@RobertJoonas RobertJoonas deleted the scroll-depth-imported-pages-migration branch December 17, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants