-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Add Sqllite json_valid #4465
Conversation
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.
Thanks for opening this PR and sorry for taking some days to get back with hints what's wrong here.
The implementation is already on a good way, there are only minor nits that needs to be fixed to run the tests + make this good to merge.
ecfff1b
to
b4f640f
Compare
Adding json_valid helper for sqllite.
b4f640f
to
128ffba
Compare
I'm sorry that it looks like #4463 was not answered. From my point of view the issues outlined there were addressed with the earlier set of comments here. There was a new set of errors in the relevant doc test, which was essentially caused by the set of comments above being not 100% complete in what needed to be changed. I've now pushed 6399f89 to fix these issues, as they were really only minimal syntax errors. I've also rebased the PR and pushed 128ffba to pull in recent changes from the master branch. Thanks again for contributing ❤️ |
e6cdc69
to
d1f64cb
Compare
Related to #4366
also I couldn't test the functionality fully, and I've asked a question related to testing in #4463