Skip to content

fix(metric_alerts): Fix column type of TimeSeriesSnapshot.value to be an array of floats#19502

Merged
wedamija merged 1 commit into
masterfrom
danf/timeseriessnapshot_fix_value_type
Jun 23, 2020
Merged

fix(metric_alerts): Fix column type of TimeSeriesSnapshot.value to be an array of floats#19502
wedamija merged 1 commit into
masterfrom
danf/timeseriessnapshot_fix_value_type

Conversation

@wedamija

@wedamija wedamija commented Jun 23, 2020

Copy link
Copy Markdown
Member

Initially we implemented this table when we were using south, and it correctly set the column type
to int[]. When we switched over from South it looks like we broke the type detection of our
custom array column, and so going forward new array columns became text[]. This means they worked
correctly, and we were behaving differently to prod.

This means that when we changed this column to float locally it worked fine, since it just stored
the floats as text and interpreted them. However in production, postgres is silently converting
these values to int because it still has an int[] column type.

Adding a quick fix here to correct the column type of this table. There are low thousands of rows
here, so it should be fine to convert this over in production. I'll create a task to fix the custom
Array column itself later on.

…be an array of floats

Initially we implemented this table when we were using south, and it correctly set the column type
to `int[]`. When we switched over from South it looks like we broke the type detection of our
custom array column, and so going forward new array columns became `text[]`. This means they worked
correctly, and we were behaving differently to prod.

This means that when we changed this column to float locally it worked fine, since it just stored
the floats as text and interpreted them. However in production, postgres is silently converting
these values to `int` because it still has an int column type.

Adding a quick fix here to correct the column type of this table. There are low thousands of rows
here, so it should be fine to convert this over in production. I'll create a task to fix the custom
Array column itself later on.
@wedamija wedamija requested a review from a team June 23, 2020 00:37
@wedamija wedamija requested a review from a team as a code owner June 23, 2020 00:37
@github-actions

github-actions Bot commented Jun 23, 2020

Copy link
Copy Markdown
Contributor

This PR has a migration; here is the generated SQL

BEGIN;
--
-- Raw SQL operation
--
ALTER TABLE sentry_timeseriessnapshot ALTER COLUMN values SET DATA TYPE float[] USING values::float[];
COMMIT;

@evanpurkhiser

Copy link
Copy Markdown
Member

If I'm understanding this right..

and so going forward new array columns became text[].

How many new array columns did we make? Are there others in production that we should be concerned about?

This means that when we changed this column to float locally it worked fine

When did we change it to float?

@wedamija

wedamija commented Jun 23, 2020

Copy link
Copy Markdown
Member Author

If I'm understanding this right..

and so going forward new array columns became text[].

How many new array columns did we make? Are there others in production that we should be concerned about?

None, fortunately. We don't use this column much, and actually all other uses of it are just defaulting to text. So no real cleanup to do 😅

This means that when we changed this column to float locally it worked fine

When did we change it to float?

A week or two ago. This doesn't affect anything other than prod, since no one else can use this yet.

@wedamija wedamija merged commit 5e1c25f into master Jun 23, 2020
@wedamija wedamija deleted the danf/timeseriessnapshot_fix_value_type branch June 23, 2020 17:34
priscilawebdev pushed a commit that referenced this pull request Jun 25, 2020
…be an array of floats (#19502)

Initially we implemented this table when we were using south, and it correctly set the column type
to `int[]`. When we switched over from South it looks like we broke the type detection of our
custom array column, and so going forward new array columns became `text[]`. This means they worked
correctly, and we were behaving differently to prod.

This means that when we changed this column to float locally it worked fine, since it just stored
the floats as text and interpreted them. However in production, postgres is silently converting
these values to `int` because it still has an int column type.

Adding a quick fix here to correct the column type of this table. There are low thousands of rows
here, so it should be fine to convert this over in production. I'll create a task to fix the custom
Array column itself later on.
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants