[C#] Module bindings for Typed Query Builder #4159
Conversation
…p-module-query-builder
…p-module-query-builder
joshua-spacetime
left a comment
joshua-spacetime
left a comment
There was a problem hiding this comment.
Is it possible to share the core query builder types between the sdk and module bindings? I also think we should have end-to-end tests similar to what we have for the client builder.
Requested changes made, merging both the common Module Bindings and Client SDK sides of Module Builder into the |
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| } | ||
| */ | ||
| Message: The call is ambiguous between the following methods or properties: 'TestDuplicateTableNameCols.TestDuplicateTableNameCols(string)' and 'TestDuplicateTableNameCols.TestDuplicateTableNameCols(string)', |
There was a problem hiding this comment.
Which tests generate these errors?
There was a problem hiding this comment.
Running dotnet test crates/bindings-csharp/Codegen.Tests executes TestDiagnostics, which compiles the diag fixture and captures compiler errors for validating the error output.
There was a problem hiding this comment.
But do these errors make sense? Are these from new tests you added?
There was a problem hiding this comment.
Sorry, not exactly a test I've added, but is a result of the code changes.
diag already does a test of duplicate table names (see TestDuplicateTableName) in it's Lib.cs. With QueryBuilder's additional generated helpers are emitted for both of these tables with a duplicate name, the Roslyn generator sees the duplicate type/constructor symbols and generates the error.
The error's text comes strait from the C# compiler, not something we authored.
Co-authored-by: joshua-spacetime <josh@clockworklabs.io> Signed-off-by: Ryan <r.ekhoff@clockworklabs.io>
2 participants
Description of Changes
This is the implementation of the Typed Query Builder for C# modules outlined in #3750.
This requires the changes from #4146 to be in place before it can properly function.
This aligns the C# typed query builder SQL formatter with the Rust implementation:
And/Orwith same grouping styles as Rust.Eq/Neq/Lt/…) wrap expressions in parentheses so chained predicates produce byte-for-byte identical SQL.API and ABI breaking changes
Not breaking. Existing modules keep producing the same SQL semantics; only redundant parentheses were added to match the Rust formatter.
Expected complexity level and risk
2 — localized string-format updates plus parity harness tweaks. Low functional risk; largest concern is ensuring every comparison path gained parentheses.
Testing