Conversation
There was a problem hiding this comment.
Pull request overview
Adds SQL Common Table Expression (CTE) handling to the TS generator so that CTE output columns can be referenced for type inference (including wildcard selects) and demos/snapshots validate the behavior.
Changes:
- Add recursive CTE processing in
translate_queryand register CTE output columns as “virtual tables”. - Extend wildcard (
SELECT *) and parameter type inference to consult registered virtual-table columns. - Add demo CTE queries plus generated TS query typings/snapshots.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/demo/cte/cte.ts | Adds demo SQL queries covering simple, multiple, windowed, and wildcard CTE patterns |
| tests/demo/cte/cte.snapshot.ts | Adds snapshot typings for newly introduced CTE demo queries |
| tests/demo/cte/cte.queries.ts | Adds generated query typings for newly introduced CTE demo queries |
| src/ts_generator/sql_parser/translate_query.rs | Recursively translates CTEs and registers their output columns for downstream inference |
| src/ts_generator/sql_parser/expressions/translate_wildcard_expr.rs | Expands wildcard translation to use registered virtual-table columns (CTE/TVF) |
| src/ts_generator/sql_parser/expressions/translate_expr.rs | Uses virtual-table columns for placeholder parameter type inference in expressions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for table_name in &table_names { | ||
| if let Some(tvf_columns) = ts_query.table_valued_function_columns.get(table_name).cloned() { | ||
| for (col_name, ts_type) in tvf_columns { | ||
| ts_query.result.insert(col_name, vec![ts_type]); | ||
| } | ||
| return Ok(()); | ||
| } | ||
| } | ||
|
|
||
| if table_names.len() > 1 { | ||
| warning!("Impossible to calculate appropriate field names of a wildcard query with multiple tables. Please use explicit field names instead. Query: {}", select.to_string()); | ||
| } |
There was a problem hiding this comment.
This returns early as soon as any table name matches a registered virtual table, which drops columns from other tables in FROM (CTE + real table, or multiple CTEs). A safer behavior is: only short-circuit when table_names.len() == 1, otherwise merge virtual-table columns into the result and continue with the existing multi-table handling (including schema fetch + warning).
| single_table_name: &Option<&str>, | ||
| table_with_joins: &Option<Vec<TableWithJoins>>, | ||
| db_conn: &DBConn, | ||
| cte_columns: &std::collections::HashMap<String, std::collections::HashMap<String, TsFieldType>>, |
There was a problem hiding this comment.
This parameter is named cte_columns, but call sites pass &ts_query.table_valued_function_columns, and translate_query also stores CTE columns in table_valued_function_columns. This naming mismatch makes the API harder to understand and maintain. Consider renaming the parameter (and related comments/locals) to something like virtual_table_columns (or splitting CTE vs TVF maps if they’re intended to be distinct concepts).
1 participant
No description provided.