flux-pr-1839
sqlparser-rs (Rust) · W2 · GPT-5.4
Tests passed. 1/1 commands passed. Strength: strong.
env PATH=/root/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin cargo test --all-featuresPartial score: 1/1
Trajectory
codex · partial order onlyprovider-native trajectory captured; validation and decision steps are appended with coarse ordering only
Quality
Equivalence Reasoning
behavioral
The patch covers most MSSQL function additions (inline TVF `RETURN SELECT`, optional `AS`, multi-statement TVF with `RETURNS @name TABLE(...)`, and column constraints), but it changes `CreateFunction` shape globally (`return_table_name`) and does not implement the anonymous-vs-defined table return type model from intent (`RETURNS TABLE` vs `RETURNS TABLE(...)`) in the AST/type system. It also models inline return as `AsReturnQuery(Box<Query>)`, which is broader/different than intended bare-`SELECT` support and may accept forms outside the requested MSSQL semantics. Most importantly, it appears to reject `RETURN` with no expression/query in `BEGIN...END` bodies for MSSQL functions (expects `RETURN <query>` in non-BEGIN branch and no explicit handling for empty RETURN in inline branch), risking incompatibility with required multi-statement TVF behavior.
Code Review
The patch appears to implement part of MSSQL CREATE FUNCTION TVF support, but it likely does not fully satisfy the intended change due to modeling and parsing mismatches that can produce invalid states and brittle TABLE semantics.
The patch introduces `return_table_name` on `CreateFunction` and parses it independently of `return_type`, allowing invalid or inconsistent states (e.g., named return variable with non-table type) rather than modeling it as a typed table return variant.
Inline TVF RETURN body is parsed via `parse_query()` (optionally parenthesized), which accepts broader query constructs than the requested SELECT/subquery forms and may accept unsupported function bodies.
New MSSQL test asserts `DataType::Table(columns)` with `columns.is_empty()` for `RETURNS TABLE`, indicating sentinel encoding instead of an explicit anonymous-table variant; this is brittle for formatting/round-tripping and does not cleanly distinguish `TABLE` vs `TABLE(...)`.
Adding `return_table_name` required touching many dialect tests/constructors that are unrelated to MSSQL behavior, increasing maintenance burden versus containing MSSQL semantics within type variants.