Proposed protocols & implementations for query parameters

[courtneyholcomb]

Completes SL-993

Description

Proposed updates to the protocols and implementations used for queries.

IMO, we are using the same protocols for too many things. For instance, we had a QueryParameterDimension as the type for the group_by param. QueryParameterDimension was required to have grain, and date_part attributes. That means that if the client wants to pass an entity or a categorical dimension into the group_by param, they still need to use an object with grain and date_part attributes even though those are not valid attributes for entities or categorical dimensions. This leaves us with 2 options:

1. Force the client to always include those attributes, even when they aren't valid. This simplifies type checking a lot in MF, but it seems pretty unreasonable on the client side.
2. Separate the group by options into two different protocols, one of which includes those attributes (TimeDimensionQueryParameter). This creates some awkwardness because we have to accept a Union group_by parameter and figure out which type the user passed.

In this PR I implement option 2, because to me it seems much cleaner to handle that logic in one place (here in MF) than to put the onus on all clients to handle the awkwardness of passing objects around with invalid attributes.

Second, we were using DimensionQueryParameter as the type for both group_by and order_by inputs. This forces us to add a descending parameter to DimensionQueryParameter, even though the descending param is not valid as a group_by input. Instead, I added a new OrderByQueryParameter protocol that includes a descending attribute and a nested order_by item, which can be a MetricQueryParameter, GroupByQueryParameter, or TimeDimensionQueryParameter.

One other note about the group_by param - I changed this param to expect a Tuple, since that's the only type hint that allows you to have multiple datatypes in one iterable. (This is just an annoying mypy limitation.)

Let me know if y'all agree with these changes or if you have other ideas!

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[linear]

SL-993 Support object params in MF

[tlento]

I made a comment on @DevonFulcher's PR over in dbt-semantic-interfaces that this kind of protocol separation might be good, but in the context of that change it seemed fine to keep on with the current layout.

My default is to prefer strongly typed things to weakly typed things, and having optional parameters to satisfy the interface is, from a design perspective, unsatisfying. That said, I'm not at all sure what the right balance is here because I'm having a super hard time keeping track of all of the interfaces and where these things will be used.

Can we set up a 30 minute chat where we just list out the different query param interfaces, where they're defined, where they get called/how they get invoked, who is accessing them, and how they interact with types (like where filter users get no benefit from typing because they give us a string and we resolve it, but GraphQL callers should get support from the API)? I think if we have that list it'll be more obvious how to lay things out.

[tlento]

Ugh, this function is terrible. Sorry. :(

[courtneyholcomb]

Can we set up a 30 minute chat where we just list out the different query param interfaces, where they're defined, where they get called/how they get invoked, who is accessing them, and how they interact with types (like where filter users get no benefit from typing because they give us a string and we resolve it, but GraphQL callers should get support from the API)? I think if we have that list it'll be more obvious how to lay things out.

Sounds good - I set up 30 min for the three of us on Monday!

[courtneyholcomb]

Updated based on our convo today!

[DevonFulcher]

Non-blocking: should these both have the Input prefix? Or neither?

[courtneyholcomb]

Basically I just wanted to differentiate that InputOrderByParameter is a nested input to the OrderByQueryParameter. So I followed the naming convention we use for InputMetrics on derived metrics, if that makes sense! But I don't think that applies to the GroupByParameter since there's no nesting.

[DevonFulcher]

Got it, thanks!

[DevonFulcher]

Looks great!