Add DAX parser and TMDL import#144
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: 534c07ec96
ℹ️ 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".
| metric = Metric( | ||
| name=node.name or "", | ||
| agg=agg, | ||
| sql=sql or expression if not agg else sql, |
There was a problem hiding this comment.
When a TMDL measure is not reducible to a simple aggregate, such as CALCULATE([Total Sales], SAMEPERIODLASTYEAR(...)), this stores the raw DAX expression in Metric.sql while also marking it as DAX. The SQL generator does not consult expression_language and treats Metric.sql as SQL for derived metrics, so sidemantic query/layer.query will emit DAX into the SQL sent to DuckDB/Postgres/etc. rather than failing clearly or using only translated SQL. Please keep untranslated DAX in dax/metadata only, or exclude/guard these metrics from SQL generation until they are translated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c0c96d371
ℹ️ 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".
| source_column = _string_prop(props.get("sourcecolumn")) | ||
| sql = source_column or expression |
There was a problem hiding this comment.
For TMDL calculated columns with no sourceColumn, this assigns the DAX expression to Dimension.sql; later query generation projects dimension.window_sql_expr directly into the SQL CTE, so a CLI query that groups by a calculated column such as CALCULATE(...) or RELATED(...) will send DAX to DuckDB/Postgres instead of failing clearly. Please keep untranslated DAX only in dax/metadata (or add a guard like the metric guard) unless a SQL translation is actually available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3466fa4015
ℹ️ 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 func == "countrows": | ||
| return agg, None |
There was a problem hiding this comment.
When a TMDL measure uses COUNTROWS with anything other than the current table, such as COUNTROWS(Customer) or COUNTROWS(FILTER(Sales, ...)), this branch still returns a simple count with sql=None. The SQL generator then counts rows from the containing model, so these valid DAX measures silently produce wrong results instead of being left as untranslated DAX unless the argument is proven equivalent to the current table.
Useful? React with 👍 / 👎.
| table=model_table, | ||
| sql=model_sql, | ||
| dax=model_dax, |
There was a problem hiding this comment.
For calculatedTable imports, the branch above sets model_table and model_sql to None while preserving only model_dax; validation accepts that, but the SQL generator builds CTEs from model.sql or model.table and has no model-level untranslated-DAX guard. Any CLI query against dimensions or metrics from such a calculated table will therefore generate an invalid FROM None/missing source instead of failing clearly or skipping the model until DAX table lowering exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05f6a95e74
ℹ️ 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".
| measure_dax = _dax_text(measure) | ||
| if measure_dax: | ||
| measure_def["dax"] = measure_dax | ||
| measure_def["expression_language"] = "dax" | ||
| elif measure.sql: |
There was a problem hiding this comment.
When a TMDL measure is reduced to a normal SQL aggregate, the metric carries both the original dax and the translated sql (for example SUM(Sales[Amount]) becomes agg: sum, sql: Amount, dax: ...). This export branch writes only the DAX whenever it exists, dropping the SQL translation; after a native Sidemantic YAML round-trip the metric still has agg so it is not guarded as untranslated DAX, but SQL generation falls back to summing the metric name/raw column instead of Amount. Please serialize the translated sql alongside dax when it is available.
Useful? React with 👍 / 👎.
| if table and table.lower() == table_name.lower(): | ||
| return agg, column | ||
| if table: | ||
| return agg, f"{table}.{column}" | ||
| return agg, column |
There was a problem hiding this comment.
When a DAX measure on one table aggregates a column from a different table, this path still treats it as a simple translated metric and stores SQL like Products.Price. The model CTE for that metric is built only from the containing table, so querying a valid Power BI measure such as SUM(Products[Price]) on Sales will emit Products.Price in the Sales CTE instead of joining or failing clearly. Please only translate this form when the referenced table is the current table, or keep it as untranslated DAX.
Useful? React with 👍 / 👎.
1 participant
Adds the Rust-backed DAX parser package and optional
sidemantic[dax]extra.Adds Power BI TMDL parser/import/export support, loader discovery, source metadata preservation, and permissively licensed Power BI fixtures.
Keeps this PR to parser/import scope. DAX lowering, DAX query execution, and Sidequery-facing contracts are follow-up PRs.