Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove JSON column from ClickHouse backend #482

Closed
bmtcril opened this issue Oct 25, 2023 · 1 comment · Fixed by #493
Closed

Remove JSON column from ClickHouse backend #482

bmtcril opened this issue Oct 25, 2023 · 1 comment · Fixed by #493
Assignees
Milestone

Comments

@bmtcril
Copy link
Contributor

bmtcril commented Oct 25, 2023

Feature Request

When I implemented the ClickHouse backend I used the experimental JSON column type based on feedback from ClickHouse that it would be promoted out of experimental status "soon". We discovered that it had some serious missing functionality and added in the String column version of the JSON data next to it as a workaround. I recently reconnected with ClickHouse on the topic and it seems like they are not prioritizing getting the JSON field fully functional any time soon, so I would like to remove it!

Describe the solution you'd like

I'd like to remove the JSON column and change the existing handful of places in the code that use it to use the String column with JSON string function calls. They are faster, fully functional, and it will reduce the storage for the xAPI statements significantly. We entirely use the String column in Aspects.

Describe alternatives you've considered

Waiting for CH to finish their feature and dropping the String column instead. 😄 Last I heard from them "no one is working on that".

Discovery, Documentation, Adoption, Migration Strategy

This would be a breaking change for any downstream consumers of the Ralph database directly, but since I think everyone using this would already have all of their data in the String column I think the only necessary migration internal to Ralph would be to drop the column. Since Ralph doesn't even manage the schema I hope that would be a small issue.

Do you want to work on it through a Pull Request?

I'd be happy to do this work, I just want to make sure it would be a welcome change and answer any questions you may have before doing the work. I also know there was work happening on the abstracting the backends that may need to land first.

@wilbrdt
Copy link
Contributor

wilbrdt commented Oct 26, 2023

Hi @bmtcril ,
As you mentioned, I don't think of any barrier for this change and it would be welcomed! Happy to assist you with this task, and we can aim for a release in version 4.0.

@wilbrdt wilbrdt added this to the 4.0 milestone Oct 26, 2023
bmtcril added a commit to bmtcril/ralph that referenced this issue Nov 2, 2023
Initial implementation of the ClickHouse backend used the experimental JSON column type, with the string of the JSON as a backup. The JSON type has never graduated out of experimental status and has some serious issues that make it undesirable to support.

This commit removes all references to the column in favor of using the JSON ClickHouse functions, which are generally faster and better supported.
bmtcril added a commit to bmtcril/ralph that referenced this issue Nov 2, 2023
Initial implementation of the ClickHouse backend used the experimental JSON column type, with the string of the JSON as a backup. The JSON type has never graduated out of experimental status and has some serious issues that make it undesirable to support.

This commit removes all references to the column in favor of using the JSON ClickHouse functions, which are generally faster and better supported.
bmtcril added a commit to bmtcril/ralph that referenced this issue Nov 2, 2023
Initial implementation of the ClickHouse backend used the experimental JSON column type, with the string of the JSON as a backup. The JSON type has never graduated out of experimental status and has some serious issues that make it undesirable to support.

This commit removes all references to the column in favor of using the JSON ClickHouse functions, which are generally faster and better supported.
bmtcril added a commit to bmtcril/ralph that referenced this issue Nov 2, 2023
Initial implementation of the ClickHouse backend used the experimental JSON
column type, with the string of the JSON as a backup. The JSON type has never
graduated out of experimental status and has some serious issues that make it
undesirable to support.

This commit removes all references to the column in favor of using the JSON
ClickHouse functions, which are generally faster and better supported.
@wilbrdt wilbrdt linked a pull request Nov 3, 2023 that will close this issue
bmtcril added a commit to bmtcril/ralph that referenced this issue Nov 3, 2023
Initial implementation of the ClickHouse backend used the experimental JSON
column type, with the string of the JSON as a backup. The JSON type has never
graduated out of experimental status and has some serious issues that make it
undesirable to support.

This commit removes all references to the column in favor of using the JSON
ClickHouse functions, which are generally faster and better supported.
wilbrdt pushed a commit that referenced this issue Nov 3, 2023
Initial implementation of the ClickHouse backend used the experimental JSON
column type, with the string of the JSON as a backup. The JSON type has never
graduated out of experimental status and has some serious issues that make it
undesirable to support.

This commit removes all references to the column in favor of using the JSON
ClickHouse functions, which are generally faster and better supported.
@github-project-automation github-project-automation bot moved this from Todo to Closed in Ralph roadmap Nov 3, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

2 participants