[dns] initial tree functions #4563
Draft
Conversation
This file contains hidden or 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
This is a pure refactor CL, no behaviour change. Mainly this is taking steps to decouple interpreter and dataframe: 1) datafarame namespace -> core::dataframe *but* keeping dataframe:: as a ergonommic alias to avoid typing too much. We want to the dataframe:: namespace because all callers need to do core::dataframe::X which is just too much. *but* we also want to be in core:: because otherwise we have to do core:: everywhere inside dataframe. Declaring the alias solves all problems 2) Change common -> util which is a better reflection of that and add new common/ for pure specs which are shared amongst the interpreter, dataframe and, in the future. tree code. 3) Explicitly define the things in dataframe/ that interpreter dpeends on. In follow up CLs will make this nothing as we reduce dependencies.
Split out these headers to reduce compile time: any template magic can be moved to the cc files as much as possible or headers which are not included in lots of places.
This CL finally achieves to goal over the last few CLs of making the interpreter decoupled from the dataframe. This is necessary to allow for graphs/trees support to be added to the interpreter.
There are two current patterns which are not great: 1) Calling PlanQuery/Execute with no filters or sorts etc. This ships multiple full copy of the interpreter for zero reson 2) Calling GetCellLegacy In both cases, we can migrate to a new GetCell method which allows for type-safe callback style method for extracting data from a dataframe. We're also going to use this in upcoming code.
LalitMaganti
force-pushed
the
dev/lalitm/interpreter-4
branch
2 times, most recently
from
40061ab to
605b194
Compare
🎨 Perfetto UI Build✅ UI build is ready: https://storage.googleapis.com/perfetto-ci-artifacts/gh-21359611790-1-ui/ui/index.html |
LalitMaganti
force-pushed
the
dev/lalitm/interpreter-4
branch
from
12fd58a to
d9725ec
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.