chore(hogql): Use the C++ parser in local dev #18112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mariusandra
approved these changes
Oct 20, 2023
There was a problem hiding this comment.
Agreed in general. However will it be a lot of trouble to move the flag to change the parser into the HogQL modifiers system? See modifiers.py and HogQLDebug.tsx? This keeps all things that can affect a query neatly packed away... and we could use it to try faster queries in production as well. If too much hassle, then 👍 just for this regardless.
|
Only a bit more trouble – I think that makes sense, I will do that as the next step of rollout (where the default value of the modifier would depend on |
daibhin
pushed a commit
that referenced
this pull request
Oct 23, 2023
* chore(hogql): Use the C++ parser in local dev * Update all the parsing functions, not just `parse_select`
Justicea83
pushed a commit
to Justicea83/posthog
that referenced
this pull request
Oct 25, 2023
* chore(hogql): Use the C++ parser in local dev * Update all the parsing functions, not just `parse_select`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
It's time to actually start using the C++ parser.
Changes
Before we do that in production, I would prefer to add more NULL guards to
parser.cpp(we don't want to run into the billion-dollar mistake), just so we're sure the production processes can't segfault (this should never happen in normal operation, but e.g. if a pod runs out of memory, there is currently a risk that the parser can't allocate a Python object, and currently the whole Python process crashes with no recourse, which would be very annoying).But we were just hacking with @thmsobrmlr when suddenly we had to wait TWENTY seconds for Python to parse a statement (the actual query took 0.2s). Let's start using the new parser in local dev and we might also uncover some issue (or find that it's perfect).