fix: order_by for compile_sql now works as expected

[serramatutu]

Summary

This PR changes the internal representation of QueryParameters to add stricter validation and fix the ambiguity of order_by parameters when sent via GraphQL. It also exposes new OrderByMetric and OrderByGroupBy top-level objects so that users can specify their clauses more precisely if strings don't suffice.

I recommend you review this commit by commit :)

Rationale of the changes

Improving the internal representation of query parameters

To facilitate typing across different classes, we created a QueryParameters class that is a TypedDict. While this is convenient for static typing of kwargs, TypedDicts don't do any validation. To fix that, I created two dataclasses: AdhocQueryParametersStrict and SavedQueryQueryParametersStrict, which represent "strictly" validated parameters that are ready to be sent to the API. Then, I improved the validate_query_parameters method so that it returns one of these, and raises errors in case there was any validation error of the parameters dict.

Remove ambiguity of order_by

The GraphQL API forces us to specify whether an order_by clause is a metric or a dimension. To remove ambiguity from the representation, the "strict" parameter dataclasses only support OrderByMetric and OrderByGroupBy, and it is the job of the validator to convert strings like -my_metric into an object like OrderByMetric(name="my_metric", descending=True). Now, the protocol implementations don't need to worry about what is what since that's solved in the validation layer, before calling the ADBC or GraphQL protocols.

Making protocols use the new representation

Finally, I changed the GraphQL and ADBC protocols to use the return value of validate_query_parameters when building a query request.

Breaking changes

Since we don't have the list of known metrics/dimensions at query time when querying through a saved query, you now have to specifically provide a OrderByMetric or OrderByGroupBy in that case, otherwise the SDK will raise an error. For adhoc queries, you can still provide a string and the SDK will parse the spec objects from that.

[WilliamDee]

may need to update this (along with whereever else uses TimeGranularity), since I believe with custom granularity, this can be an arbitrary string. cc: @courtneyholcomb to confirm

[courtneyholcomb]

that's correct!

[serramatutu]

yep! I wanted to do that in a followup PR to avoid mixing things but I'm aware :)

In the folowup PR, I'll do something along the lines of

Grain = Union[TimeGranularity, str]

Then update all the call sites that take TimeGranularity and make them use Grain instead.

[WilliamDee]

Do we have any tests against object syntax through adbc? Something like this should work

{{ semantic_layer.query(metrics=[Metric("orders")]) }}

[serramatutu]

Oh the SDK only generates object syntax for order by. For metrics and group by we just use regular string arrays.

Is there a reason we would need that?

[serramatutu]

For order by there are tests tho

[serramatutu]

For extra context: there's no way for a user to submit a raw string to be sent via ADBC. They have to do like:

with client.session():
    table = client.query(metrics=["my_metric"], group_by=["my_dim"])
    print(table)

Under the hood, the SDK will generate a SQL statement like the following and send it via ADBC.

SELECT * FROM {{ semantic_layer.query(metrics=["my_metric"], group_by=["my_dim"]) }}