Add SavedQuery nodes #8798
Add SavedQuery nodes #8798
Conversation
In 0.3.x of `dbt-semantic-intefaces` the location of the WhereFilterParser moved to be grouped in with a bunch of new adjacent code. As such, we needed to correct our import path of it.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8798 +/- ##
==========================================
- Coverage 86.54% 84.65% -1.89%
==========================================
Files 176 177 +1
Lines 25856 26354 +498
==========================================
- Hits 22376 22311 -65
- Misses 3480 4043 +563
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Unfortunately things are a bit too intertwined currently for SavedQuery to be in it's own file. We need to add the SavedQuery node to the GraphMemberNode, unfortunately with SavedQuery in it's own file, importing it would have caused a circular dependency. We'll need to separately come in and split things up as a cleanup portion of work.
QMalcolm
force-pushed
the
qmalcolm--8594-add-saved-query-nodes
branch
from
396ea4f to
88e46cd
Compare
Our grouping logic seems to be in a weird spot. It seems liek we're moving to setting the `group` for a node in the node's `config` however, all of the logic around grouping is still focused on the top level `group` property on a nodes. To get group stuff plumbed I've thus added `group` as a top level property of the `SavedQuery` node, and populated it from the config group value.
I don't like making scatter shot commits like this. However, a lot of this commit was written ~4am, soooo yea. Things were broken, I wanted things to be unbroken. I mostly searched for `semantic_models` and added the equivalent necessary `saved_queries`. Some stuff is in support of writing out the manifest, some stuff helps with node selection, it's a lot of miscelaneous stuff that I don't fully understand.
QMalcolm
force-pushed
the
qmalcolm--8594-add-saved-query-nodes
branch
from
88e46cd to
0e15768
Compare
We don't actually append anything to `refs` for SavedQuery nodes currently. I'm not sure if anything needs to be appended to them. Regardless, we access the `refs` property throughout the codebase while iterating over nodes. It seems wise to support this attribute as to not accidently blow something up with it not existing.
QMalcolm
force-pushed
the
qmalcolm--8594-add-saved-query-nodes
branch
from
982fd98 to
df343cf
Compare
We're gonna release DSI 0.3.0, and if this PR automatically pulls that in things will break. But the things that need fixing should be handled separately from this PR. After releasing DSI 0.3.0 I'm going to create a branch off/ontop of this one, and open a stacked PR with the associated changes.
…tr and list of strs
| if isinstance(node, SeedNode): | ||
| return | ||
| elif isinstance(node, SavedQuery): | ||
| metrics = [[metric] for metric in node.metrics] |
There was a problem hiding this comment.
for my own understanding - why is this necessary?
There was a problem hiding this comment.
This necessary because the for loop that begins on 1620 expects each "metric" in the list of "metrics" to be a list of either 1 or 2 strings (i.e. List[List[str]]. Where the first string is the target_metric_name and the second (if it exists) is the target_metric_package. Saved query node metrics don't support package targeting. and it's metrics is just a list of strings (i.e. List[str]), with each string being a target_metric_name. Thus instead of handling them differently within the for loop that resolves the metrics, it was easier to just massage the saved query metrics to have the same shape.
…ueries I missed this when I was copy and pasting 🤦
|
@QMalcolm I haven't looked into the intended semantics of this new node type, but one file you might have missed is compilation.py. I also missed it in my initial implementation of |
[Preview](https://docs-getdbt-com-git-dbeatty10-patch-1-dbt-labs.vercel.app/reference/node-selection/methods#the-saved_query-method) ## What are you changing in this pull request and why? The "saved_query" selector method was [added in v1.7](https://github.com/dbt-labs/dbt-core/blob/1.7.latest/CHANGELOG.md#dbt-core-170---november-02-2023) by dbt-labs/dbt-core#8594 / dbt-labs/dbt-core#8798. ## Checklist - [x] Review the [Content style guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md) so my content adheres to these guidelines. - [x] I have verified all new code examples work as described
resolves #8594
Problem
We need to support a new node type
SavedQueryfor 1.7 of core. This is work is to support MetricFlow and the Semantic Layer.Solution
We
docsupport for SavedQuery node decsriptionsChecklist