Use SQLite's query SQL generator for window functions - #36
Open
virzak wants to merge 2 commits into
Open
Conversation
The build failed with 19 analyzer diagnostics promoted to errors by TreatWarningsAsErrors. All are style/quality rules; no behavior changes. - IDE0046: fold the DeclaringType guard into the switch expression in BinaryTranslator and WindowFunctionsTranslator. - IDE0028: back ChainedSqlExpression.List with a List<T> field instead of casting the IReadOnlyList<T> back to List<T> on every Add. - CA1860: use Count > 0 instead of Any() on Partitions/Orderings. - IDE0031: use null-conditional assignment (x?[i] = v) in WindowFunctionsSqlNullabilityProcessorHelper. - IDE0370: drop null-forgiving operators that are no longer needed on WindowFrame.ToString() and TypeBuilder.CreateType(). - IDE0305: use a collection expression instead of ToList(). - IDE0290: suppress on constructors inside #if branches, which cannot be primary constructors, matching the existing suppressions in those files. - IDE0120: use Sum(g => g.Count()) instead of Select(...).Sum(). Build is clean (0 warnings, 0 errors). The three SQLite-backed test projects pass; the SQL Server and Npgsql ones need live databases and were not run.
UseWindowFunctions() registered the provider agnostic WindowQuerySqlGeneratorFactory for SQLite, so queries compiled with the generic relational generator instead of SqliteQuerySqlGenerator. That produced ANSI OFFSET/FETCH paging, which SQLite rejects with 'near "FETCH": syntax error', breaking First/Single/Take/Skip on any query once window functions were enabled - even queries containing no window function at all. Add WindowFunctionsSqliteQuerySqlGenerator deriving from SqliteQuerySqlGenerator, matching what the SQL Server and Npgsql packages already do, and register its factory instead. Add PagingTests to the shared testing project so First/Take/Skip are covered on all three providers, plus a SQLite specific assertion that the generated SQL uses LIMIT and not FETCH. SubQueryTests.RowNumberWithSingle was skipped on SQLite with a "Fixme: investigate why this fails" note. It failed for this same reason and now passes, so the skip is removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #23.
UseWindowFunctions() registered the provider agnostic WindowQuerySqlGeneratorFactory for SQLite, so queries compiled with the generic relational generator instead of SqliteQuerySqlGenerator. Anything the SQLite generator handles specially then failed: OFFSET/FETCH paging (
near "FETCH": syntax error) and JSON scalar access (This node should be handled by provider-specific SQL generator), even on queries with no window function in them.Adds WindowFunctionsSqliteQuerySqlGenerator deriving from SqliteQuerySqlGenerator, matching the SQL Server and Npgsql packages, and registers its factory. PagingTests cover First/Take/Skip on all three providers.
Stacked on fix/analyzer-build-errors so the build passes; rebase once that lands.
Generated with Claude Code