Fix Ossie logical expressions and aggregate grains in Python - #402
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
nicosuave
changed the title
![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: a6083fd170
ℹ️ 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".
| def extract(value: exp.Expression, unit: str) -> exp.Expression: | ||
| return exp.Extract(this=exp.Var(this=unit), expression=value.copy()) | ||
|
|
||
| if part in {"YEAR", "MONTH", "QUARTER"}: |
There was a problem hiding this comment.
Implement the boundary-counting rewrite for units beyond year/month/quarter. For example, DATEDIFF(hour, TIMESTAMP '2024-01-01 10:59:00', TIMESTAMP '2024-01-01 11:01:00') must return 1 under the portable Snowflake-style contract, but the unhandled PostgreSQL path falls through to SQLGlot's elapsed-interval translation and returns 0. This silently corrupts PostgreSQL results for hour/minute/second differences that cross a calendar boundary without spanning a full unit.
Useful? React with 👍 / 👎.
| if target not in {"duckdb", "postgres", "snowflake", "bigquery", "databricks"}: | ||
| raise ValueError(f"Unsupported OSSIE_SQL_2026 target {target_dialect!r}") |
There was a problem hiding this comment.
Include Spark in portable lowering or fall back to a provided ANSI variant. spark is already accepted by the Ossie lowering parser and is a supported Sidemantic database dialect, but any field containing an OSSIE_SQL_2026 variant is selected first and then rejected by this allowlist; even a document that also supplies a valid ANSI_SQL expression becomes non-executable for Spark.
Useful? React with 👍 / 👎.
a6083fd to
c48fb07
Compare
1 participant
Ossie metrics could bind to physical columns instead of declared logical fields, lose computed join keys, or change existing totals when another metric was selected. This fixes the Python execution path and adds
OSSIE_SQL_2026parsing and target lowering.Logical names now bind before physical-column fallback, with exact quoted identities, normalized regular identifiers, and validated metric dependencies. Aggregate leaves retain independent entity or joined-row populations; computed relationship keys and imported window fields keep their intended semantics. The portable-expression corpus supplies independently expected results for required function families.
Local focused Python execution/export validation: 315 passed. Ruff, formatting, and whitespace checks passed. Python-specific tests select that engine explicitly; the later shared corpus exercises every case through Python, public Rust without fallback, and native Rust. Exact BigQuery percentiles and unsupported target combinations return explicit diagnostics.