Allow measures that join to time spine to use agg_time_dim

[courtneyholcomb]

Resolves #1009

Description

Some measures specify that they should be joined to time spine in the YAML. We only execute this join if metric time is queried, since that's the only time joining to time spine is allowed. Since we're relaxing that restriction, we now can execute the join if agg_time_dimension is queried.

[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.

[tlento]

Checking in before approving - are there any cases where this might result in an ambiguous plan?

I'm thinking of a situation where someone has multiple metrics, or some combination nested metric, and they group by this agg_time_dimension and also metric_time. I'm not even sure if that's a valid query ordinarily, honestly.

[tlento]

Yeah if we have some kind of helper I think it'll clean up a lot of these conditionals as well, because it'll hide the wonky logic inside of the helper and this can become something like:

if query_contains_agg_time_dimension(queried_linkable_specs, measure_spec):

[courtneyholcomb]

Updated!

[tlento]

Ok, so previously this would have just run the query but skipped the join, correct? That seems strange and incorrect to me.

[courtneyholcomb]

The logic there was that if there's no time dimension to join to, we can't join to time spine anyway.
And then, if there is a time dimension that isn't agg time, should we time spine join to that dimension? Seems inconsistent if we're treating it like any other categorical dimension in all other ways.

[courtneyholcomb]

Checking in before approving - are there any cases where this might result in an ambiguous plan?

I'm thinking of a situation where someone has multiple metrics, or some combination nested metric, and they group by this agg_time_dimension and also metric_time. I'm not even sure if that's a valid query ordinarily, honestly.

@tlento This clause should be able to execute for as many measures as are requested, each join will happen in its own subquery. We're checking for the agg_time_dimension specific to the measure here. So we would join to time spine only for measures whose agg_time_dimension was queried (or if metric_time was queried).
Does that answer the question?

[tlento]

This clause should be able to execute for as many measures as are requested, each join will happen in its own subquery.

Duh, of course it does, thank you.

Also, in re-reading that comment, I think the right answer for people duplicating their requested time dimensions across metrics is "don't do that" - I'm sure I'm missing some super critical use case where that's a totally reasonable thing to do, but it doesn't sound reasonable when I read it out loud.