Add Yardstick adapter format with SQL parity support#103
Add Yardstick adapter format with SQL parity support#103
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e893ce9085
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self._looks_like_yardstick_query(sql): | ||
| return self._rewrite_yardstick_query(sql) |
There was a problem hiding this comment.
strict=False before Yardstick rewrite dispatch
rewrite() promises that non-strict mode passes through SQL it cannot safely rewrite, but this early Yardstick dispatch runs before any of the non-strict guards and always takes the Yardstick error path. In contexts that call rewrite(..., strict=False) (for example the server passthrough path), queries containing AGGREGATE(...) can now raise ValueError (e.g. the SEMANTIC-prefix check) instead of being forwarded unchanged, which breaks previously-supported passthrough behavior.
Useful? React with 👍 / 👎.
| # If the query references a single model, unqualified columns map to that model. | ||
| default_alias = next(iter(source_models)) if len(source_models) == 1 else None |
There was a problem hiding this comment.
This treats len(source_models) == 1 as “single-model query”, but source_models only includes semantic models and ignores non-semantic tables in the same FROM/JOIN scope. As a result, a query with one semantic model plus other joined tables will still force unqualified columns to the semantic alias, rewriting valid references from non-semantic tables to the wrong table and causing wrong results or missing-column errors.
Useful? React with 👍 / 👎.
1 participant
Summary
AS MEASURESQL into models/metrics.AGGREGATE(...)and AT modifiers (ALL,ALL EXCEPT,WHERE,SET,VISIBLE) in the SQL rewriter/generator.Notes
maininto this branch to resolve conflicts.