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

Convert data type of column json_feature (attributes) from json to SUPER for Redshift #77

Merged
merged 3 commits into from
Jan 2, 2025

Conversation

matthieu-carmeille
Copy link
Contributor

Super is more performant than json and easier to query.

Output of this code: type is super and the payload is valid:

Screenshot 2024-12-18 at 21 18 16

@matthieu-carmeille
Copy link
Contributor Author

Happy new year @bcodell ! When can this PR be reviewed please?

@bcodell
Copy link
Owner

bcodell commented Jan 2, 2025

Hey @matthieu-carmeille happy new year! I left a comment on the PR about handling the data type casting for the case when the json object is null. Did you see that comment?

@bcodell
Copy link
Owner

bcodell commented Jan 2, 2025

@matthieu-carmeille My previous comment is here for reference: #77 (comment)

@matthieu-carmeille
Copy link
Contributor Author

@matthieu-carmeille My previous comment is here for reference: #77 (comment)

sorry missed the comment and did not receive any notification. Still can not see the comment. When i click on the link above, it stays on the same page and no comment.

@bcodell
Copy link
Owner

bcodell commented Jan 2, 2025

@matthieu-carmeille My previous comment is here for reference: #77 (comment)

sorry missed the comment and did not receive any notification. Still can not see the comment. When i click on the link above, it stays on the same page and no comment.

Ah weird - sorry about that. Screenshot of comment is below. Let me know if it makes sense or if you have any pushback!

image

cast null as super
@matthieu-carmeille
Copy link
Contributor Author

good point. amended. Thanks @bcodell .

Copy link
Owner

@bcodell bcodell left a comment

Choose a reason for hiding this comment

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

looks good! thanks @matthieu-carmeille

macros/activity_schema/activity/build_json.sql Outdated Show resolved Hide resolved
@bcodell bcodell merged commit 3cc7834 into bcodell:main Jan 2, 2025
1 check failed
@bcodell
Copy link
Owner

bcodell commented Jan 2, 2025

@matthieu-carmeille thanks for this! I'm going to cut a new minor version for this in a moment. Not sure if you've used the package to query activity streams for building datasets yet, but a nice-to-have improvement there would be to use dot notation when interacting with SUPER columns, instead of the default json_extract_path_text. Let me know if you want to take a stab at this!

@matthieu-carmeille
Copy link
Contributor Author

@matthieu-carmeille thanks for this! I'm going to cut a new minor version for this in a moment. Not sure if you've used the package to query activity streams for building datasets yet, but a nice-to-have improvement there would be to use dot notation when interacting with SUPER columns, instead of the default json_extract_path_text. Let me know if you want to take a stab at this!

done in this PR

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.

2 participants