Reduce ORM over-hydration in dimensions graph traversal - #2436
Draft
shangyian wants to merge 2 commits into
Draft
Conversation
…rsal - get_dimension_nodes now uses a slim option set (only what get_dimension_attributes actually reads) instead of the full _node_output_options() chain, dropping unused parents/dimension_links/ tags/owners/catalog/Column.dimension/partition eager loads. - Batch-load the dimension nodes referenced via col.dimension_id across all candidate columns in a single query, instead of issuing session.refresh(col, ["dimension"]) plus three more refreshes per column inside build_reference_link (N+1 across every column of every discovered dimension node). build_reference_link now accepts an optional pre-loaded dimension_node to skip those refreshes entirely. - get_metric_parents_map's Node selects now use load_only(id, name, type, current_version) plus noload(created_by)/noload(tags), since only those four fields are read off the parent nodes by this function and its callers.
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
…mension_attributes Adds a test where a source column has a valid dimension_id/dimension_column pair but the target column name no longer exists on the referenced dimension node's current columns, so build_reference_link returns None and the candidate is skipped. This exercises the 481->480 loop-continuation branch in get_dimension_attributes that no test previously reached.
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
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.
Summary
get_dimension_nodesnow uses a trimmed down set of load options (only whatget_dimension_attributesactually needs) instead of the full_node_output_options()chain.col.dimension_idacross all candidate columns in a single query, instead of issuing refreshes on each column insidebuild_reference_link(this is N+1 across every column of every discovered dimension node).build_reference_linknow accepts an optional pre-loadeddimension_nodeto skip those refreshes entirely.get_metric_parents_map's Node selects are trimmed down to only load the fields that are used.Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan