-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
✨ add type and sort columns to variables table #3362
Conversation
@danyx23 we've agreed on using
What do you think? |
There's another #2039 we had open back then. We should either merge them or simply kill the old one. |
I copy-pasted the code that I needed from #2039 into this PR, so I'm happy for it to be killed |
I think the current approach is good @Marigold. We'll probably query the type field much more often than the sort field in the DB, so having the type field as a column makes sense to me. |
Thanks! @sophiamersmann I removed |
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.
Nice!
I didn't realize that this would be so easy, and that we can just write a drop-in replacement of sortedUniqNonEmptyStringVals
.
Also, 👑 to Mojmir for setting up the staging server with the staging indicator, that was super helpful to see this in action.
20a8611
to
b191d4e
Compare
Thanks for the review! I pushed one more commit that updates Grapher's db types and asked Daniel to sign off on it. This is good to merge now, but I'll wait for the holidays to be over :) |
Grapher part of adding indicator
type
to MySQL and adding ordinal type. Here's ETL pull request.Fixes #1690