From 148a8184ac936382a3ed96987833886e37ab6577 Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Mon, 2 Nov 2026 18:13:10 +0000 Subject: [PATCH 1/4] Fix for #746 --- docs/RELEASE_NOTES.md | 1 + docs/content/core/general.fsx | 1 - docs/content/core/mssql.fsx | 3 - docs/content/index.fsx | 2 - src/SQLProvider.Common/SqlRuntime.Common.fs | 21 ++++++- src/SQLProvider.Common/SqlRuntime.Linq.fs | 3 +- .../SqlRuntime.QueryExpression.fs | 24 +++++-- src/SQLProvider.Runtime/Providers.DuckDb.fs | 10 +-- src/SQLProvider.Runtime/Providers.Firebird.fs | 10 +-- src/SQLProvider.Runtime/Providers.MSAccess.fs | 10 +-- .../Providers.MsSqlServer.Dynamic.fs | 10 +-- .../Providers.MsSqlServer.Ssdt.fs | 10 +-- .../Providers.MsSqlServer.fs | 10 +-- src/SQLProvider.Runtime/Providers.MySql.fs | 10 +-- src/SQLProvider.Runtime/Providers.Odbc.fs | 10 +-- src/SQLProvider.Runtime/Providers.Oracle.fs | 10 +-- .../Providers.Postgresql.fs | 10 +-- src/SQLProvider.Runtime/Providers.SQLite.fs | 10 +-- .../MsSqlSsdt.Tests/MsSqlSsdt.Tests.fsproj | 2 +- .../MySql/SqlProvider.Core.Tests.fsproj | 2 +- .../more/SubqueryCompositionTests.fs | 62 +++++++++++++++++++ 21 files changed, 160 insertions(+), 71 deletions(-) diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index 7b02d927..e24b581a 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -1,6 +1,7 @@ ### 1.5.29 - 02.11.2026 - Fix: parameters of a sub-query nested in another sub-query are bound (IN, EXISTS, UNION and the like) - Fix: groupBy keys with option or value-option columns, and with x.IsSome / x.IsNone in a tuple key +- Fix: a grouped sub-query of IN / NOT IN that selects an aggregate selects it without the group keys, #746 - PostgreSQL: identity columns count as generated, so the typed ``Create(...)`` no longer takes them. Breaking for code that passed a value to a BY DEFAULT identity column: use Create() and set it, or Create(data). A saved ContextSchemaPath cache keeps the old signature until regenerated. ### 1.5.28 - 17.10.2026 diff --git a/docs/content/core/general.fsx b/docs/content/core/general.fsx index 23ce12b7..9d37d46e 100644 --- a/docs/content/core/general.fsx +++ b/docs/content/core/general.fsx @@ -6,7 +6,6 @@ let [] resolutionPath = __SOURCE_DIRECTORY__ + @"/../../files/sqlite" (*** hide ***) let [] connectionString = "Data Source=" + __SOURCE_DIRECTORY__ + @"\..\northwindEF.db;Version=3;Read Only=false;FailIfMissing=True;" -(*** hide ***) (** # SQL Provider Basics diff --git a/docs/content/core/mssql.fsx b/docs/content/core/mssql.fsx index f227b1f9..f3472ed5 100644 --- a/docs/content/core/mssql.fsx +++ b/docs/content/core/mssql.fsx @@ -1,7 +1,4 @@ (*** hide ***) - -(*** hide ***) -#I @"../../../bin/mssql/lib/net8.0/" #I @"../../../bin/mssql/lib/net8.0/" #r "FSharp.Data.SqlProvider.Common.dll" #r "FSharp.Data.SqlProvider.MsSql.dll" diff --git a/docs/content/index.fsx b/docs/content/index.fsx index 5b7f38c9..ca77c11a 100644 --- a/docs/content/index.fsx +++ b/docs/content/index.fsx @@ -1,5 +1,3 @@ -(*** hide ***) - (** SQLProvider =========== diff --git a/src/SQLProvider.Common/SqlRuntime.Common.fs b/src/SQLProvider.Common/SqlRuntime.Common.fs index a5368d1b..0978b744 100644 --- a/src/SQLProvider.Common/SqlRuntime.Common.fs +++ b/src/SQLProvider.Common/SqlRuntime.Common.fs @@ -708,6 +708,14 @@ type SqlExp = | AggregateOp(_,_,rest) -> isSortBy rest isSortBy this +/// One groupBy of a query: its key columns and the aggregates over each group. +type GroupingColumns = + { Keys : (alias * SqlColumnType) list + Aggregates : (alias * SqlColumnType) list + /// The key columns are selected besides the aggregates. False only for a sub-query of IN / NOT IN that selects + /// aggregates: it may return one column, so it selects the aggregates alone. + SelectKeys : bool } + type SqlQuery = { Filters : Condition list HavingFilters : Condition list @@ -716,13 +724,13 @@ type SqlQuery = Aliases : Map Ordering : (alias * SqlColumnType * bool) list Projection : Expression list - Grouping : (list * list) list //key columns, aggregate columns + Grouping : GroupingColumns list Distinct : bool UltimateChild : (string * Table) option Skip : int voption Take : int voption Union : (UnionType*string*seq) option - Count : bool + Count : bool AggregateOp : (alias * SqlColumnType) list } with static member Empty = { Filters = []; Links = []; Grouping = []; Aliases = Map.empty; Ordering = []; Count = false; AggregateOp = []; CrossJoins = [] @@ -760,7 +768,7 @@ type SqlQuery = let baseAlias:alias = grp.PrimaryTable.Name let f = grp.KeyColumns |> List.map (fun (al,k) -> legaliseName (if al<>"" then al else baseAlias), k) let s = grp.AggregateColumns |> List.map (fun (al,opKey) -> legaliseName (if al<>"" then al else baseAlias), opKey) - (f,s)::q.Grouping + { Keys = f; Aggregates = s; SelectKeys = true }::q.Grouping Projection = match grp.Projection with Some p -> p::q.Projection | None -> q.Projection } rest | FilterClause(c,rest) -> convert { q with Filters = (c)::q.Filters } rest | HavingClause(c,rest) -> convert { q with HavingFilters = (c)::q.HavingFilters } rest @@ -1036,6 +1044,13 @@ module CommonTasks = sprintf "%s as %s" notation alias | _ -> plain + /// The select list of a grouped query: the key columns (`keys`, in the order of the groupings' Keys) of each + /// grouping that selects its keys, then the aggregates. + let groupSelectColumns (grouping:GroupingColumns list) (keys:string list) (aggregates:string seq) = + let selected = grouping |> List.collect (fun grp -> grp.Keys |> List.map (fun _ -> grp.SelectKeys)) + let keys = List.zip selected keys |> List.filter fst |> List.map snd + String.Join(", ", keys @ List.ofSeq aggregates) + let parseHaving fieldNotation (keys:(alias*SqlColumnType) list) (conditionList : Condition list) = if keys.Length <> 1 then failwithf "Unsupported having. Expected 1 key column, found: %i" keys.Length diff --git a/src/SQLProvider.Common/SqlRuntime.Linq.fs b/src/SQLProvider.Common/SqlRuntime.Linq.fs index 13b64577..31d6cf94 100644 --- a/src/SQLProvider.Common/SqlRuntime.Linq.fs +++ b/src/SQLProvider.Common/SqlRuntime.Linq.fs @@ -671,7 +671,8 @@ module internal QueryImplementation = let svc = (qry :?> IWithSqlService) use con = svc.Provider.CreateConnection svc.DataContext.ConnectionString - let (query,parameters,projector,baseTable) = QueryExpressionTransformer.convertExpression svc.SqlExpression svc.TupleIndex con svc.Provider false true + // IN / NOT IN takes one column: a grouped sub-query selects its aggregate without the keys + let (query,parameters,projector,baseTable) = QueryExpressionTransformer.convertExpressionWith false svc.SqlExpression svc.TupleIndex con svc.Provider false true let ``nested param names`` = $"@param{abs(query.GetHashCode())}{nestCount}nested" nestCount <- nestCount + 1 diff --git a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs index bf35a591..6a66e03b 100644 --- a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs +++ b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs @@ -632,7 +632,9 @@ module internal QueryExpressionTransformer = newProjection, projectionMap, groupProjectionMap - let convertExpression exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = + /// As convertExpression; `selectGroupKeys` false leaves the group keys out of the select of a grouped query that + /// selects aggregates, for a sub-query of IN / NOT IN, which may return one column. + let convertExpressionWith (selectGroupKeys: bool) exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = // first convert the abstract query tree into a more useful format let legaliseName (alias:alias) = if alias.StartsWith "_" then alias.TrimStart [|'_'|] else alias @@ -832,7 +834,8 @@ module internal QueryExpressionTransformer = | c -> c let gatheredAggregations = - sqlQuery.Grouping |> List.map(fun (group,x) -> + sqlQuery.Grouping |> List.map(fun grp -> + let group, x = grp.Keys, grp.Aggregates // GroupBy: collect aggreagte operations // Should gather the column names what we want to aggregate, not just operations. let aggregations:(alias * SqlColumnType) list = @@ -909,7 +912,11 @@ module internal QueryExpressionTransformer = match lst |> List.filter(fun (keys,agg) -> not agg.IsEmpty) with | [] -> lst | filtered -> filtered - { sqlQuery with Grouping = cleaned |> Seq.distinct |> Seq.toList } + { sqlQuery with + Grouping = + cleaned + |> List.distinct + |> List.map (fun (keys, aggs) -> { Keys = keys; Aggregates = aggs; SelectKeys = true }) } else sqlQuery // a special case here to handle queries that start from the relationship of an individual @@ -1050,7 +1057,7 @@ module internal QueryExpressionTransformer = | x -> (x,resolveC b,c) | a,c,b -> resolve a, resolveC c,b) sqlQuery.Ordering // Resolve other canonical function columns also: - Grouping = sqlQuery.Grouping |> List.map(fun (g,a) -> g|>List.map(fun (ga, gk) -> resolve ga, resolveC gk), a|>List.map(fun(ag,aa)->resolve ag,resolveC aa)) + Grouping = sqlQuery.Grouping |> List.map(fun grp -> { grp with Keys = grp.Keys |> List.map(fun (ga, gk) -> resolve ga, resolveC gk); Aggregates = grp.Aggregates |> List.map(fun(ag,aa)->resolve ag,resolveC aa) }) AggregateOp = sqlQuery.AggregateOp |> List.map(fun (a,c) -> (resolve a, resolveC c)) } @@ -1121,6 +1128,15 @@ module internal QueryExpressionTransformer = let myLock = provider.GetLockObject() lock myLock (fun () -> provider.GetColumns (con,table) |> ignore )) + // A grouping without aggregates selects its keys: they are its only columns + let sqlQuery = + if selectGroupKeys then sqlQuery + else { sqlQuery with Grouping = sqlQuery.Grouping |> List.map (fun grp -> { grp with SelectKeys = grp.Aggregates.IsEmpty }) } + let (sql,parameters) = provider.GenerateQueryText(sqlQuery,baseAlias,baseTable,projectionColumns,isDeleteScript,con) (sql,parameters,projectionDelegate,baseTable) + + /// The query of an expression as the SQL text, its parameters, the projection and the base table. + let convertExpression exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = + convertExpressionWith true exp entityIndex con provider isDeleteScript useCanonicalsOnSelect diff --git a/src/SQLProvider.Runtime/Providers.DuckDb.fs b/src/SQLProvider.Runtime/Providers.DuckDb.fs index aeb3948f..4a4e76fb 100644 --- a/src/SQLProvider.Runtime/Providers.DuckDb.fs +++ b/src/SQLProvider.Runtime/Providers.DuckDb.fs @@ -916,16 +916,16 @@ type internal DuckDbProvider(resolutionPath, contextSchemaPath, owner:string, re match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation DuckDb.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation DuckDb.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1002,13 +1002,13 @@ type internal DuckDbProvider(resolutionPath, contextSchemaPath, owner:string, re // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.Firebird.fs b/src/SQLProvider.Runtime/Providers.Firebird.fs index ec4df856..b70be8a6 100644 --- a/src/SQLProvider.Runtime/Providers.Firebird.fs +++ b/src/SQLProvider.Runtime/Providers.Firebird.fs @@ -1010,16 +1010,16 @@ type internal FirebirdProvider(resolutionPath, contextSchemaPath, owner, referen match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation Firebird.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as [%s{fn}]" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation Firebird.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1096,13 +1096,13 @@ type internal FirebirdProvider(resolutionPath, contextSchemaPath, owner, referen // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.MSAccess.fs b/src/SQLProvider.Runtime/Providers.MSAccess.fs index 80f994bd..66bc16bc 100644 --- a/src/SQLProvider.Runtime/Providers.MSAccess.fs +++ b/src/SQLProvider.Runtime/Providers.MSAccess.fs @@ -549,16 +549,16 @@ type internal MSAccessProvider(contextSchemaPath) = match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as [%s{fn}]" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] when String.IsNullOrEmpty(selectcolumns) -> "*" | [] -> selectcolumns @@ -632,13 +632,13 @@ type internal MSAccessProvider(contextSchemaPath) = // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.MsSqlServer.Dynamic.fs b/src/SQLProvider.Runtime/Providers.MsSqlServer.Dynamic.fs index 1e012c9b..74063f0c 100644 --- a/src/SQLProvider.Runtime/Providers.MsSqlServer.Dynamic.fs +++ b/src/SQLProvider.Runtime/Providers.MsSqlServer.Dynamic.fs @@ -1045,16 +1045,16 @@ type internal MSSqlServerDynamicProvider(resolutionPath, contextSchemaPath, refe match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1128,13 +1128,13 @@ type internal MSSqlServerDynamicProvider(resolutionPath, contextSchemaPath, refe // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.MsSqlServer.Ssdt.fs b/src/SQLProvider.Runtime/Providers.MsSqlServer.Ssdt.fs index 50a8f09e..ca68785a 100644 --- a/src/SQLProvider.Runtime/Providers.MsSqlServer.Ssdt.fs +++ b/src/SQLProvider.Runtime/Providers.MsSqlServer.Ssdt.fs @@ -704,16 +704,16 @@ type internal MSSqlServerProviderSsdt(tableNames: string, ssdtPath: string) = match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation MSSqlServer.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation MSSqlServer.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -791,13 +791,13 @@ type internal MSSqlServerProviderSsdt(tableNames: string, ssdtPath: string) = // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.MsSqlServer.fs b/src/SQLProvider.Runtime/Providers.MsSqlServer.fs index 8025740d..ddd0672b 100644 --- a/src/SQLProvider.Runtime/Providers.MsSqlServer.fs +++ b/src/SQLProvider.Runtime/Providers.MsSqlServer.fs @@ -965,16 +965,16 @@ type internal MSSqlServerProvider(contextSchemaPath, tableNames:string) = match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation MSSqlServer.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation MSSqlServer.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1052,13 +1052,13 @@ type internal MSSqlServerProvider(contextSchemaPath, tableNames:string) = // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.MySql.fs b/src/SQLProvider.Runtime/Providers.MySql.fs index f25747b8..a245addc 100644 --- a/src/SQLProvider.Runtime/Providers.MySql.fs +++ b/src/SQLProvider.Runtime/Providers.MySql.fs @@ -989,16 +989,16 @@ type internal MySqlProvider(resolutionPath, contextSchemaPath, owner:string, ref match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation MySql.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation MySql.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1075,13 +1075,13 @@ type internal MySqlProvider(resolutionPath, contextSchemaPath, owner:string, ref // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.Odbc.fs b/src/SQLProvider.Runtime/Providers.Odbc.fs index c3579eca..87e71191 100644 --- a/src/SQLProvider.Runtime/Providers.Odbc.fs +++ b/src/SQLProvider.Runtime/Providers.Odbc.fs @@ -588,16 +588,16 @@ type internal OdbcProvider(contextSchemaPath, quotechar : OdbcQuoteCharacter) = match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -678,13 +678,13 @@ type internal OdbcProvider(contextSchemaPath, quotechar : OdbcQuoteCharacter) = // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.Oracle.fs b/src/SQLProvider.Runtime/Providers.Oracle.fs index d294e4ca..a06d2d3d 100644 --- a/src/SQLProvider.Runtime/Providers.Oracle.fs +++ b/src/SQLProvider.Runtime/Providers.Oracle.fs @@ -1037,16 +1037,16 @@ type internal OracleProvider(resolutionPath, contextSchemaPath, owner, reference match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation Oracle.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as \"%s{fn}\"" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation Oracle.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1112,13 +1112,13 @@ type internal OracleProvider(resolutionPath, contextSchemaPath, owner, reference // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.Postgresql.fs b/src/SQLProvider.Runtime/Providers.Postgresql.fs index 72820e1d..40d5bf52 100644 --- a/src/SQLProvider.Runtime/Providers.Postgresql.fs +++ b/src/SQLProvider.Runtime/Providers.Postgresql.fs @@ -1211,16 +1211,16 @@ type internal PostgresqlProvider(resolutionPath, contextSchemaPath, owner, refer match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation PostgreSQL.fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else sprintf "%s as \"%s\"" fn (fn.Replace("\"", "")) CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation PostgreSQL.fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -1287,13 +1287,13 @@ type internal PostgresqlProvider(resolutionPath, contextSchemaPath, owner, refer // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/src/SQLProvider.Runtime/Providers.SQLite.fs b/src/SQLProvider.Runtime/Providers.SQLite.fs index 3fa8f298..00eeaba1 100644 --- a/src/SQLProvider.Runtime/Providers.SQLite.fs +++ b/src/SQLProvider.Runtime/Providers.SQLite.fs @@ -898,16 +898,16 @@ type internal SQLiteProvider(resolutionPath, contextSchemaPath, referencedAssemb match sqlQuery.Grouping with | [] -> Utilities.parseAggregates fieldNotation fieldNotationAlias sqlQuery.AggregateOp | g -> - let keyColumns = g |> List.collect fst + let keyColumns = g |> List.collect (fun grp -> grp.Keys) let keys = keyColumns |> List.mapi(fun i (a,c) -> let fn = fieldNotation a c if not (tmpGrpParams.ContainsKey (a,c)) then tmpGrpParams.Add((a,c), fn) let plain = if sqlQuery.Aliases.Count < 2 then fn else $"%s{fn} as '%s{fn}'" CommonTasks.groupKeyColumn keyColumns i c fn plain) - let aggs = g |> List.collect snd + let aggs = g |> List.collect (fun grp -> grp.Aggregates) let res2 = Utilities.parseAggregates fieldNotation fieldNotationAlias aggs |> List.toSeq - [String.Join(", ", keys) + (if List.isEmpty aggs || List.isEmpty keys then "" else ", ") + String.Join(", ", res2)] + [CommonTasks.groupSelectColumns g keys res2] match extracolumns with | [] -> selectcolumns | h::t -> h @@ -973,13 +973,13 @@ type internal SQLiteProvider(resolutionPath, contextSchemaPath, referencedAssemb // GROUP BY if sqlQuery.Grouping.Length > 0 then - let groupkeys = sqlQuery.Grouping |> List.collect fst + let groupkeys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) if groupkeys.Length > 0 then ~~" GROUP BY " groupByBuilder groupkeys if sqlQuery.HavingFilters.Length > 0 then - let keys = sqlQuery.Grouping |> List.collect fst + let keys = sqlQuery.Grouping |> List.collect (fun grp -> grp.Keys) let f = [And([],Some (sqlQuery.HavingFilters |> CommonTasks.parseHaving fieldNotation keys))] ~~" HAVING " diff --git a/tests/SqlProvider.Core.Tests/MsSqlSsdt/MsSqlSsdt.Tests/MsSqlSsdt.Tests.fsproj b/tests/SqlProvider.Core.Tests/MsSqlSsdt/MsSqlSsdt.Tests/MsSqlSsdt.Tests.fsproj index 88ded177..c5b4b0bb 100644 --- a/tests/SqlProvider.Core.Tests/MsSqlSsdt/MsSqlSsdt.Tests/MsSqlSsdt.Tests.fsproj +++ b/tests/SqlProvider.Core.Tests/MsSqlSsdt/MsSqlSsdt.Tests/MsSqlSsdt.Tests.fsproj @@ -19,7 +19,7 @@ - + diff --git a/tests/SqlProvider.Core.Tests/MySql/SqlProvider.Core.Tests.fsproj b/tests/SqlProvider.Core.Tests/MySql/SqlProvider.Core.Tests.fsproj index 589d1069..3c853ff0 100644 --- a/tests/SqlProvider.Core.Tests/MySql/SqlProvider.Core.Tests.fsproj +++ b/tests/SqlProvider.Core.Tests/MySql/SqlProvider.Core.Tests.fsproj @@ -42,7 +42,7 @@ - + diff --git a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs index 2ee133cd..ff8e2c4c 100644 --- a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs +++ b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs @@ -433,3 +433,65 @@ let ``parameters of three nested subquery levels are bound``() = Assert.IsNotEmpty expected Assert.AreEqual(expected, result) + +[] +let ``a grouped sub-query selecting only an aggregate gives one column to IN``() = + let latestOrderIds = + query { + for order in dc.Main.Orders do + groupBy order.CustomerId into g + select (g.Max(fun order -> order.OrderId)) + } + + let result = + query { + for order in dc.Main.Orders do + where (latestOrderIds.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> Array.sort + + let expected = + query { for o in dc.Main.Orders do select (o.CustomerId, o.OrderId) } + |> Seq.groupBy fst |> Seq.map (fun (_, rows) -> rows |> Seq.map snd |> Seq.max) |> Seq.toArray |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) + +[] +let ``a grouped sub-query selecting only an aggregate gives one column to NOT IN``() = + let latestOrderIds = + query { + for order in dc.Main.Orders do + groupBy order.CustomerId into g + select (g.Max(fun order -> order.OrderId)) + } + + let result = + query { + for order in dc.Main.Orders do + where (not (latestOrderIds.Contains order.OrderId)) + select order.OrderId + } |> Seq.toArray |> Array.sort + + let latest = + query { for o in dc.Main.Orders do select (o.CustomerId, o.OrderId) } + |> Seq.groupBy fst |> Seq.map (fun (_, rows) -> rows |> Seq.map snd |> Seq.max) |> Set.ofSeq + + let expected = + query { for o in dc.Main.Orders do select o.OrderId } + |> Seq.filter (latest.Contains >> not) |> Seq.toArray |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) + +[] +let ``a grouped query keeps its key in the select outside IN sub-queries``() = + let latestOrderIds = + query { + for order in dc.Main.Orders do + groupBy order.CustomerId into g + select (g.Max(fun order -> order.OrderId)) + } + + let sql, _ = FSharp.Data.Sql.Runtime.QueryInspection.toQueryString latestOrderIds + StringAssert.Contains("[CustomerID]", sql.Substring(0, sql.IndexOf "FROM")) From 5ebcc74dc7b7049db9e1b0dd5c56e660b478b48c Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Tue, 3 Nov 2026 07:30:23 +0000 Subject: [PATCH 2/4] Union in subquery fix --- .config/dotnet-tools.json | 2 +- docs/RELEASE_NOTES.md | 4 +- docs/content/core/async.fsx | 14 +-- docs/content/core/composable.fsx | 36 +++---- docs/content/core/crud.fsx | 28 +++--- docs/content/core/mappers.fsx | 3 +- docs/content/core/mysql.fsx | 4 +- docs/content/core/odbc.fsx | 2 +- docs/content/core/querying.fsx | 10 +- src/SQLProvider.Common/SqlRuntime.Common.fs | 20 ++-- src/SQLProvider.Common/SqlRuntime.Linq.fs | 24 +++-- .../SqlRuntime.QueryExpression.fs | 30 ++++-- src/SQLProvider.DesignTime/SqlDesignTime.fs | 20 ++-- .../more/SubqueryCompositionTests.fs | 95 +++++++++++++++++++ 14 files changed, 214 insertions(+), 78 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index bf2c65d0..19874704 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -45,7 +45,7 @@ "rollForward": false }, "fsdocs-tool": { - "version": "20.0.0", + "version": "22.2.0", "commands": [ "fsdocs" ], diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index e24b581a..ebc59899 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -1,7 +1,9 @@ ### 1.5.29 - 02.11.2026 - Fix: parameters of a sub-query nested in another sub-query are bound (IN, EXISTS, UNION and the like) - Fix: groupBy keys with option or value-option columns, and with x.IsSome / x.IsNone in a tuple key -- Fix: a grouped sub-query of IN / NOT IN that selects an aggregate selects it without the group keys, #746 +- Fix: a grouped sub-query of IN / NOT IN that selects an aggregate selects it without the group keys, on both sides of a union too, #746 +- IN / NOT IN sub-queries with a projection computed in .NET (e.g. `g.Max(...) + 1L`) fail with an error instead of comparing against the uncomputed column +- Design time: a provider instance that goes away closes its connection instead of disposing it, as the cached types of other instances still use it ("Cannot access a disposed object") - PostgreSQL: identity columns count as generated, so the typed ``Create(...)`` no longer takes them. Breaking for code that passed a value to a BY DEFAULT identity column: use Create() and set it, or Create(data). A saved ContextSchemaPath cache keeps the old signature until regenerated. ### 1.5.28 - 17.10.2026 diff --git a/docs/content/core/async.fsx b/docs/content/core/async.fsx index d739676e..db59506d 100644 --- a/docs/content/core/async.fsx +++ b/docs/content/core/async.fsx @@ -57,7 +57,7 @@ open System.Threading.Tasks open FSharp.Data.Sql type MyWebServer() = - member __.``Execute Business Logics`` (id : Guid) : Task<_> = + member __.``Execute Business Logics`` (orderId : int64) : Task<_> = task { use transaction = new System.Transactions.TransactionScope( @@ -67,14 +67,14 @@ type MyWebServer() = let context = TypeProviderConnection.GetDataContext cstr let! fetched = query { - for t2 in context.MyDataBase.MyTable2 do - join t1 in context.MyDataBase.MyTable1 on (t2.ForeignId = t1.Id) - where (t2.Id = id) - select t1 + for order in context.Main.Orders do + join customer in context.Main.Customers on (order.CustomerId.Value = customer.CustomerId) + where (order.OrderId = orderId) + select order } |> Seq.executeQueryAsync - fetched |> Seq.iter (fun entity -> - entity.SetColumn("Updated", DateTime.UtcNow |> box) + fetched |> Seq.iter (fun order -> + order.ShippedDate <- Some DateTime.UtcNow ) do! context.SubmitUpdatesAsync() diff --git a/docs/content/core/composable.fsx b/docs/content/core/composable.fsx index 896474d2..11943b3e 100644 --- a/docs/content/core/composable.fsx +++ b/docs/content/core/composable.fsx @@ -19,7 +19,7 @@ type sql = SqlDataProvider< (*** hide ***) let ctx = sql.GetDataContext() (*** hide ***) -let xs = [].AsQueryable() +let xs = ([] : int list).AsQueryable() (** # Composable Queries @@ -82,10 +82,10 @@ In other words, SQLProvider queries typically follow this pattern: *) -let (qry:IQueryable<'T>) = +let pattern (xs:IQueryable<'T>) : IQueryable<'T> = query { //for is like C# foreach - for x in (xs:IQueryable<'T>) do + for x in xs do select x } @@ -125,7 +125,7 @@ let companyNameFilter inUse = Then, you can create the main query *) -let query1 = +let mainQuery = query { for customers in ctx.Main.Customers do where (customers.ContactTitle = "USA") @@ -136,7 +136,7 @@ let query1 = and now call you are able to call the second query like this *) -let res = companyNameFilter true query1 |> Seq.toArray +let res = companyNameFilter true mainQuery |> Seq.toArray (** ### Generate composable queries by using FSharp.Linq.ComposableQuery @@ -150,18 +150,18 @@ Example for using FSharpComposableQuery *) let qry1 = - query { for u in dbContext.Users do - select (u.Id, u.Name, u.Email) + query { for c in ctx.Main.Customers do + select (c.CustomerId, c.CompanyName, c.City) } let qry2 = - query { for c in dbContext.Cars do - select (c.UserId, c.Brand, c.Year) + query { for o in ctx.Main.Orders do + select (o.CustomerId, o.OrderId, o.Freight) } query { for (i,n,e) in qry1 do join (u,b,y) in qry2 on (i = u) - where (y > 2015) + where (y > 100m) select (i,n,e,u,b,y) } |> Seq.toArray @@ -180,29 +180,31 @@ open System.Linq let nestedQueryTest = let qry1 = query { - for emp in ctx.Hr.Employees do + for emp in ctx.Main.Employees do where (emp.FirstName.StartsWith("S")) select emp.FirstName } query { - for emp in ctx.Hr.Employees do + for emp in ctx.Main.Employees do where (qry1.Contains(emp.FirstName)) select (emp.FirstName, emp.LastName) } |> Seq.toArray (** -### Using non-strongly-typed __.GetColumn "name" +### Using non-strongly-typed GetColumn "name" -All the entities inherit from SqlEntity which has GetColumn-method. -So you can use non-strongly-typed columns like this: +All the entities are SqlEntity objects, which implement `IColumnHolder` with a GetColumn method. +So you can use non-strongly-typed columns, by the database's column name, like this: *) +open FSharp.Data.Sql.Common + let qry = query { for x in query1 do - where ((x.GetColumn "CustomerId") = "ALFKI") - select (x.GetColumn "CustomerId") + where (((x :> IColumnHolder).GetColumn "CustomerID") = "ALFKI") + select ((x :> IColumnHolder).GetColumn "CustomerID") } |> Seq.head (** diff --git a/docs/content/core/crud.fsx b/docs/content/core/crud.fsx index 15d2711c..6b77a2d4 100644 --- a/docs/content/core/crud.fsx +++ b/docs/content/core/crud.fsx @@ -143,15 +143,15 @@ ctx.SubmitUpdates() *) type Employee2 = { - Id:int + Id:int64 FirstName:string LastName:string } let updateEmployee (employee: Employee2) = let foundEmployeeMaybe = query { - for p in ctx.Public.Employee2 do - where (p.Id = employee.Id) + for p in ctx.Main.Employees do + where (p.EmployeeId = employee.Id) select (Some p) exactlyOneOrDefault } @@ -164,8 +164,8 @@ let updateEmployee (employee: Employee2) = let updateEmployee' (employee: Employee2) = query { - for p in ctx.Public.Employee2 do - where (p.Id = employee.Id) + for p in ctx.Main.Employees do + where (p.EmployeeId = employee.Id) } |> Seq.iter( fun e -> e.FirstName <- employee.FirstName @@ -174,7 +174,7 @@ let updateEmployee' (employee: Employee2) = ctx.SubmitUpdates() let john = { - Id = 1 + Id = 1L FirstName = "John" LastName = "Doe" } @@ -188,7 +188,7 @@ output of .ColumnValues: employees |> Seq.map (fun x -> - employee.Create(x.ColumnValues)) // create twins + ctx.Main.Employees.Create(x.ColumnValues)) // create twins |> Seq.toList /// Retrieve the FSharp.Data.Sql.Common.SqlEntity objects @@ -284,14 +284,12 @@ You can leverage this feature by setting the `OnConflict` property on a row obje Sql Server has a similar feature in the form of the MERGE statement. This is not yet supported. *) -let ctx = sql.GetDataContext() - -let emp = ctx.Main.Employees.Create() -emp.Id <- 1 -emp.FirstName <- "Jane" -emp.LastName <- "Doe" +let jane = ctx.Main.Employees.Create() +jane.EmployeeId <- 1L +jane.FirstName <- "Jane" +jane.LastName <- "Doe" -emp.OnConflict <- FSharp.Data.Sql.Common.OnConflict.Update +jane.OnConflict <- FSharp.Data.Sql.Common.OnConflict.Update ctx.SubmitUpdates() @@ -321,7 +319,7 @@ There are 3 overrides of create. *) -let emp = ctx.Main.Employees.``Create(FirstName, LastName)``("Don", "Syme") +let don = ctx.Main.Employees.``Create(FirstName, LastName)``("Don", "Syme") (** diff --git a/docs/content/core/mappers.fsx b/docs/content/core/mappers.fsx index bcfde729..916ccc6a 100644 --- a/docs/content/core/mappers.fsx +++ b/docs/content/core/mappers.fsx @@ -102,7 +102,8 @@ If you want to use SQLProvider as code-generator and copy and paste the tables a *) -sql.dataContext.DesignTimeCommands.TemplateAsRecord.``main.OrdersTemplate`` +// The template is a nested type: type its path and a dot in the editor +// sql.dataContext.DesignTimeCommands.TemplateAsRecord.``main.OrdersTemplate``. // intellisense will generate you code that you can copy and paste as template to create your own type: // ``type MainOrders = { CustomerId : String voption; EmployeeId : Int64 voption; Freight : Decimal voption; OrderDate : DateTime voption; OrderId : Int64; RequiredDate : DateTime voption; ShipAddress : String voption; ShipCity : String voption; ShipCountry : String voption; ShipName : String voption; ShipPostalCode : String voption; ShipRegion : String voption; ShippedDate : DateTime voption }`` diff --git a/docs/content/core/mysql.fsx b/docs/content/core/mysql.fsx index 41cc2f35..525c5e1b 100644 --- a/docs/content/core/mysql.fsx +++ b/docs/content/core/mysql.fsx @@ -197,7 +197,7 @@ If you want to use the drivers in parallel, you need two resolution paths: *) type HRFast = SqlDataProvider -type HRProcs = SqlDataProvider +type HRProcs = SqlDataProvider (** @@ -206,7 +206,7 @@ type HRProcs = SqlDataProvider@"Driver={Microsoft Access Driver (*.mdb, *.accdb)};Dbq=C:\Northwind.mdb" ### Quote Character diff --git a/docs/content/core/querying.fsx b/docs/content/core/querying.fsx index efc29ebf..464dd0da 100644 --- a/docs/content/core/querying.fsx +++ b/docs/content/core/querying.fsx @@ -103,6 +103,12 @@ let xs2 = (** F#'s own `Seq.contains` is not translated to SQL. + + The sub-query selects one column that the database computes: a column, an aggregate of a groupBy + (`select (g.Max(fun o -> o.OrderId))`, without the key), or arithmetic the provider translates. A projection + computed in .NET, such as `g.Max(...) + 1L` or a call to your own F# function, fails with an error; select the + column itself, or read the values first with `Seq.toArray`. A union (`xs.Union(ys).Contains x`) follows the same + rule on both sides. - **One query for every case of a filter.** Compute flags outside the query and test them inside it, so the same query can serve "all" and "one": @@ -385,7 +391,7 @@ Math.Min(x,y) | SELECT(MIN) | LEAST | LEAST | LEAST | MIN | iif(x Math.Pow(x,y) | POWER(x,y) | POWER(x,y) | POWER(x,y) | POWER(x,y) | | x^y | POWER(x,y) | | (+) | + | + | + | + | + | + | + | | (-) | - | - | - | - | - | - | - | | -(*) | * | * | * | * | * | * | * | | +(*) | * | * | * | * | * | * | * | | (/) | / | / | / | / | / | / | / | | (%) | % | % | % | % | % | % | % | | @@ -525,7 +531,7 @@ Notes and limitations: * `groupJoin ... into g` followed by `for x in g do` is supported and behaves as an inner join. Aggregating the group itself (e.g. `select (c, g.Count())`) is not supported - use `groupBy` for aggregates. -* A `groupBy` after a `leftOuterJoin` follows SQL semantics: `COUNT(*)` counts the unmatched +* A `groupBy` after a `leftOuterJoin` follows SQL semantics: COUNT(*) counts the unmatched NULL row, so a customer with no orders gets count 1, not 0. * `DefaultIfEmpty(customDefaultValue)` is not supported; only the parameterless form is. * A filtered sub-query is not supported as a left-join destination (the filter would belong in diff --git a/src/SQLProvider.Common/SqlRuntime.Common.fs b/src/SQLProvider.Common/SqlRuntime.Common.fs index 0978b744..6446a655 100644 --- a/src/SQLProvider.Common/SqlRuntime.Common.fs +++ b/src/SQLProvider.Common/SqlRuntime.Common.fs @@ -654,8 +654,9 @@ type SqlExp = | Distinct of SqlExp /// alias and column name, bool indicates ascending sort | OrderBy of alias * SqlColumnType * bool * SqlExp - /// union type and subquery - | Union of UnionType * string * seq * SqlExp + /// union type and the subquery as SQL text with its parameters, rendered as the outer query is (true for a + /// sub-query of IN / NOT IN), so that both sides select the same columns + | Union of UnionType * (bool -> string * seq) * SqlExp | Skip of int * SqlExp | Take of int * SqlExp | Count of SqlExp @@ -672,7 +673,7 @@ type SqlExp = | OrderBy(_,_,_,rest) | Skip(_,rest) | Take(_,rest) - | Union(_,_,_,rest) + | Union(_,_,rest) | Count(rest) | AggregateOp(_,_,rest) -> aux rest aux this @@ -688,7 +689,7 @@ type SqlExp = | OrderBy(_,_,_,rest) | Skip(_,rest) | Take(_,rest) - | Union(_,_,_,rest) + | Union(_,_,rest) | Count(rest) | AggregateOp(_,_,rest) -> isGroupBy rest isGroupBy this @@ -703,7 +704,7 @@ type SqlExp = | Distinct rest | Skip(_,rest) | Take(_,rest) - | Union(_,_,_,rest) + | Union(_,_,rest) | Count(rest) | AggregateOp(_,_,rest) -> isSortBy rest isSortBy this @@ -736,7 +737,8 @@ type SqlQuery = static member Empty = { Filters = []; Links = []; Grouping = []; Aliases = Map.empty; Ordering = []; Count = false; AggregateOp = []; CrossJoins = [] Projection = []; Distinct = false; UltimateChild = None; Skip = ValueNone; Take = ValueNone; Union = None; HavingFilters = [] } - static member ofSqlExp(exp,entityIndex: string ResizeArray) = + /// The query of `exp`; `membershipSubquery` is true for a sub-query of IN / NOT IN, see SqlExp.Union. + static member ofSqlExp(exp,entityIndex: string ResizeArray, membershipSubquery: bool) = let legaliseName (alias:alias) = if alias.StartsWith "_" then alias.TrimStart [|'_'|] else alias @@ -792,10 +794,12 @@ type SqlQuery = | Count(rest) -> if q.Count then failwith "count may only be specified once" else convert { q with Count = true } rest - | Union(all,subquery, pars, rest) -> + | Union(all, render, rest) -> if q.Union.IsSome then failwith "Union may only be specified once. However you can try: xs.Union(ys.Union(zs))" elif q.Take.IsSome then failwith "Union and take-limit is not yet supported as SQL-syntax varies." - else convert { q with Union = Some(all,subquery,pars) } rest + else + let subquery, pars = render membershipSubquery + convert { q with Union = Some(all,subquery,pars) } rest | AggregateOp(alias, operationWithKey, rest) -> convert { q with AggregateOp = (alias, operationWithKey)::q.AggregateOp } rest let sq = convert SqlQuery.Empty exp diff --git a/src/SQLProvider.Common/SqlRuntime.Linq.fs b/src/SQLProvider.Common/SqlRuntime.Linq.fs index 31d6cf94..a0d69e4f 100644 --- a/src/SQLProvider.Common/SqlRuntime.Linq.fs +++ b/src/SQLProvider.Common/SqlRuntime.Linq.fs @@ -671,8 +671,8 @@ module internal QueryImplementation = let svc = (qry :?> IWithSqlService) use con = svc.Provider.CreateConnection svc.DataContext.ConnectionString - // IN / NOT IN takes one column: a grouped sub-query selects its aggregate without the keys - let (query,parameters,projector,baseTable) = QueryExpressionTransformer.convertExpressionWith false svc.SqlExpression svc.TupleIndex con svc.Provider false true + // IN / NOT IN takes one column, as the database computes it + let (query,parameters,projector,baseTable) = QueryExpressionTransformer.convertExpressionWith true svc.SqlExpression svc.TupleIndex con svc.Provider false true let ``nested param names`` = $"@param{abs(query.GetHashCode())}{nestCount}nested" nestCount <- nestCount + 1 @@ -704,7 +704,7 @@ module internal QueryImplementation = | SelectMany(a,b,_,rest) -> a :: b :: collect rest | FilterClause(_,rest) | HavingClause(_,rest) | Projection(_,rest) | Distinct rest | OrderBy(_,_,_,rest) | Skip(_,rest) | Take(_,rest) - | Union(_,_,_,rest) | Count rest | AggregateOp(_,_,rest) -> collect rest + | Union(_,_,rest) | Count rest | AggregateOp(_,_,rest) -> collect rest // a blank inner base-table alias will later be named after the predicate's parameter innerParam.Name :: collect innersrc.SqlExpression |> List.filter ((<>) "") let checkCollision (alias:string) = @@ -1417,12 +1417,20 @@ module internal QueryImplementation = | MethodCall(None,(MethodWithName("Union") | MethodWithName("Concat") | MethodWithName("Intersect") | MethodWithName("Except") as meth), [SourceWithQueryData source; SeqValuesQueryable values]) when (values :? IWithSqlService) -> let subquery = values :?> IWithSqlService - use con = subquery.Provider.CreateConnection source.DataContext.ConnectionString - let (query,parameters,projector,baseTable) = QueryExpressionTransformer.convertExpression subquery.SqlExpression subquery.TupleIndex con subquery.Provider false (source.DataContext.SqlOperationsInSelect=SelectOperations.DatabaseSide) + let dataContext = source.DataContext - let ``nested param names`` = $"@param{abs(query.GetHashCode())}nested" + // Rendered as the outer query is: a sub-query of IN / NOT IN computes its column in the database + let renderAs membershipSubquery = + use con = subquery.Provider.CreateConnection dataContext.ConnectionString + let useCanonicals = membershipSubquery || dataContext.SqlOperationsInSelect = SelectOperations.DatabaseSide + let (query,parameters,_,_) = QueryExpressionTransformer.convertExpressionWith membershipSubquery subquery.SqlExpression subquery.TupleIndex con subquery.Provider false useCanonicals + let ``nested param names`` = $"@param{abs(query.GetHashCode())}nested" + let rendered, modified = renameNestedParameters ``nested param names`` query parameters + rendered, (modified :> seq) - let subquery, modified = renameNestedParameters ``nested param names`` query parameters + // The usual rendering is made here, so a sub-query that cannot be translated fails where it is composed + let usual = renderAs false + let render membershipSubquery = if membershipSubquery then renderAs true else usual //let ty = typedefof>.MakeGenericType(meth.GetGenericArguments().[0]) let utyp = @@ -1435,7 +1443,7 @@ module internal QueryImplementation = let ty = typeof> - Activator.CreateInstance(ty, [| source.DataContext |> box; source.Provider; Union(utyp,subquery,modified,source.SqlExpression); source.TupleIndex |]) :?> IQueryable<'T> + Activator.CreateInstance(ty, [| source.DataContext |> box; source.Provider; Union(utyp,render,source.SqlExpression); source.TupleIndex |]) :?> IQueryable<'T> | x -> failwith ("unrecognised method call " + x.ToString()) diff --git a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs index 6a66e03b..e14700fd 100644 --- a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs +++ b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs @@ -632,16 +632,28 @@ module internal QueryExpressionTransformer = newProjection, projectionMap, groupProjectionMap - /// As convertExpression; `selectGroupKeys` false leaves the group keys out of the select of a grouped query that - /// selects aggregates, for a sub-query of IN / NOT IN, which may return one column. - let convertExpressionWith (selectGroupKeys: bool) exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = + /// A projection that reads one selected column as it is: a column, an aggregate or a group key, through casts and + /// an option's Value. Anything else is computed in .NET after the rows are read. + let rec private isColumnRead (e:Expression) = + match e with + | :? UnaryExpression as u when u.NodeType = ExpressionType.Convert || u.NodeType = ExpressionType.ConvertChecked || u.NodeType = ExpressionType.TypeAs -> + isColumnRead u.Operand + | :? MemberExpression as m when m.Member.Name = "Value" && not (isNull m.Expression) -> isColumnRead m.Expression + | :? MemberExpression as m when m.Member.Name = "Key" -> true + | :? MethodCallExpression as c -> c.Method.Name.StartsWith "GetColumn" || c.Method.Name.StartsWith "Aggregate" + | _ -> false + + /// As convertExpression; `membershipSubquery` true is for a sub-query of IN / NOT IN, which may return one column + /// that the database computes: a grouped query that selects aggregates leaves its keys out of the select, a set + /// operation renders its operands the same way, and a projection computed in .NET is refused. + let convertExpressionWith (membershipSubquery: bool) exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = // first convert the abstract query tree into a more useful format let legaliseName (alias:alias) = if alias.StartsWith "_" then alias.TrimStart [|'_'|] else alias let entityIndex = ResizeArray<_>(entityIndex |> Seq.map legaliseName) - let sqlQuery = SqlQuery.ofSqlExp(exp,entityIndex) + let sqlQuery = SqlQuery.ofSqlExp(exp,entityIndex,membershipSubquery) let groupgin = ResizeArray<_>() // note : the baseAlias here will always be "" when no criteria has been applied, because the LINQ tree never needed to refer to it @@ -681,7 +693,7 @@ module internal QueryExpressionTransformer = | SelectMany(_,_,_,rest) | FilterClause(_,rest) | HavingClause(_,rest) | Projection(_,rest) | Distinct rest | OrderBy(_,_,_,rest) | Skip(_,rest) | Take(_,rest) - | Union(_,_,_,rest) | Count rest | AggregateOp(_,_,rest) -> find rest + | Union(_,_,rest) | Count rest | AggregateOp(_,_,rest) -> find rest match find exp with | Some p when obj.ReferenceEquals(p, projs.Head) -> Some p | _ -> None @@ -903,6 +915,8 @@ module internal QueryExpressionTransformer = let generatedMegaLambda, finalParams = composeProjections projs (Unchecked.defaultof) (Dictionary>()) QueryEvents.PublishExpression generatedMegaLambda + if membershipSubquery && not (isNull generatedMegaLambda) && not (isColumnRead generatedMegaLambda.Body) then + failwithf "IN / NOT IN needs a sub-query that selects one column as the database computes it, but this projection is computed in .NET after the rows are read: %O. Select the column or aggregate itself, or read the values first with Seq.toArray." generatedMegaLambda (generatedMegaLambda.Compile(),finalParams) let sqlQuery = @@ -1130,8 +1144,8 @@ module internal QueryExpressionTransformer = // A grouping without aggregates selects its keys: they are its only columns let sqlQuery = - if selectGroupKeys then sqlQuery - else { sqlQuery with Grouping = sqlQuery.Grouping |> List.map (fun grp -> { grp with SelectKeys = grp.Aggregates.IsEmpty }) } + if membershipSubquery then { sqlQuery with Grouping = sqlQuery.Grouping |> List.map (fun grp -> { grp with SelectKeys = grp.Aggregates.IsEmpty }) } + else sqlQuery let (sql,parameters) = provider.GenerateQueryText(sqlQuery,baseAlias,baseTable,projectionColumns,isDeleteScript,con) @@ -1139,4 +1153,4 @@ module internal QueryExpressionTransformer = /// The query of an expression as the SQL text, its parameters, the projection and the base table. let convertExpression exp (entityIndex:string ResizeArray) con (provider:ISqlProvider) isDeleteScript useCanonicalsOnSelect = - convertExpressionWith true exp entityIndex con provider isDeleteScript useCanonicalsOnSelect + convertExpressionWith false exp entityIndex con provider isDeleteScript useCanonicalsOnSelect diff --git a/src/SQLProvider.DesignTime/SqlDesignTime.fs b/src/SQLProvider.DesignTime/SqlDesignTime.fs index 50e9f382..b0765b0c 100644 --- a/src/SQLProvider.DesignTime/SqlDesignTime.fs +++ b/src/SQLProvider.DesignTime/SqlDesignTime.fs @@ -542,7 +542,7 @@ module DesignTimeUtils = designTimeCommandsContainer.AddMember m designTimeCommandsContainer, saveResponse, mOld, designTime, None - let rec createTypes (rootType:ProvidedTypeDefinition) (serviceType:ProvidedTypeDefinition) (readServiceType:ProvidedTypeDefinition) (config:TypeProviderConfig) (sqlRuntimeInfo:_) invalidate registerDispose args = + let rec createTypes (rootType:ProvidedTypeDefinition) (serviceType:ProvidedTypeDefinition) (readServiceType:ProvidedTypeDefinition) (config:TypeProviderConfig) (sqlRuntimeInfo:_) invalidate registerClose args = let struct(connectionString, conStringName,dbVendor,resolutionPath,individualsAmount,useOptionTypes,owner,caseSensitivity, tableNames, contextSchemaPath, odbcquote, sqliteLibrary, ssdtPath, rootTypeName) = args let resolutionPath = if String.IsNullOrWhiteSpace resolutionPath @@ -575,7 +575,7 @@ module DesignTimeUtils = match prov.GetSchemaCache().IsOffline with | false -> let con = prov.CreateConnection conString - registerDispose (con, dbVendor) + registerClose (con, dbVendor) try con.Open() with @@ -1045,7 +1045,7 @@ module DesignTimeUtils = <@@ ((%%a0 : obj) :?> ISqlDataContext).CreateConnection() @@>) :> MemberInfo if not isReadonly then - let recreate = fun () -> createTypes rootType serviceType readServiceType config sqlRuntimeInfo invalidate registerDispose (connectionString, conStringName,dbVendor,resolutionPath,individualsAmount,useOptionTypes,owner,caseSensitivity, tableNames, contextSchemaPath, odbcquote, sqliteLibrary, ssdtPath, rootTypeName) + let recreate = fun () -> createTypes rootType serviceType readServiceType config sqlRuntimeInfo invalidate registerClose (connectionString, conStringName,dbVendor,resolutionPath,individualsAmount,useOptionTypes,owner,caseSensitivity, tableNames, contextSchemaPath, odbcquote, sqliteLibrary, ssdtPath, rootTypeName) let designTimeCommandsContainer, saveResponse, mOld, designTime, invalidateActionResponse = createDesignTimeCommands prov contextSchemaPath recreate invalidate @@ -1471,10 +1471,16 @@ type public SqlTypeProvider(config: TypeProviderConfig) as this = let empty = fun (_:Expr list) -> <@@ () @@> - let registerDispose = fun (con, dbVendor) -> + // The provided types are kept in DesignTimeCache and shared by every provider instance with the same static + // parameters, and their delayed members use the connection. So an instance that goes away closes it, which + // releases the database (e.g. an SQLite file), and never disposes it: the providers reopen a closed connection. + let registerClose = fun (con, dbVendor) -> this.Disposing.Add(fun _ -> - if con <> Unchecked.defaultof && dbVendor <> DatabaseProviderTypes.MSACCESS - then con.Dispose()) + if con <> Unchecked.defaultof && dbVendor <> DatabaseProviderTypes.MSACCESS then + try + if con.State <> ConnectionState.Closed then con.Close() + with + | :? ObjectDisposedException -> ()) let invalidate = fun _ -> this.Invalidate() @@ -1546,7 +1552,7 @@ type public SqlTypeProvider(config: TypeProviderConfig) as this = let serviceType = ProvidedTypeDefinition( "dataContext", Some typeof, isErased=true) let readServiceType = ProvidedTypeDefinition( "readDataContext", Some typeof, isErased=true) - createTypes rootType serviceType readServiceType config sqlRuntimeInfo invalidate registerDispose args + createTypes rootType serviceType readServiceType config sqlRuntimeInfo invalidate registerClose args createConstructors config (rootType, serviceType, readServiceType, args) rootType diff --git a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs index ff8e2c4c..087d97e9 100644 --- a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs +++ b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs @@ -495,3 +495,98 @@ let ``a grouped query keeps its key in the select outside IN sub-queries``() = let sql, _ = FSharp.Data.Sql.Runtime.QueryInspection.toQueryString latestOrderIds StringAssert.Contains("[CustomerID]", sql.Substring(0, sql.IndexOf "FROM")) + +[] +let ``a union of grouped sub-queries gives one column to IN on both sides``() = + let latestByCustomer = + query { + for order in dc.Main.Orders do + groupBy order.CustomerId into g + select (g.Max(fun order -> order.OrderId)) + } + + let latestByEmployee = + query { + for order in dc.Main.Orders do + groupBy order.EmployeeId into g + select (g.Max(fun order -> order.OrderId)) + } + + let result = + query { + for order in dc.Main.Orders do + where (latestByCustomer.Union(latestByEmployee).Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> Array.sort + + let latestBy key = + query { for o in dc.Main.Orders do select (o.CustomerId, o.EmployeeId, o.OrderId) } + |> Seq.groupBy key |> Seq.map (fun (_, rows) -> rows |> Seq.map (fun (_, _, id) -> id) |> Seq.max) + + let expected = + Seq.append (latestBy (fun (customer, _, _) -> box customer)) (latestBy (fun (_, employee, _) -> box employee)) + |> Seq.distinct |> Seq.toArray |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) + +[] +let ``an IN sub-query computing on an aggregate in .NET is refused``() = + let shifted = + query { + for order in dc.Main.Orders do + groupBy order.CustomerId into g + select (g.Max(fun order -> order.OrderId) + 1L) + } + + let error = + Assert.Throws(fun () -> + query { + for order in dc.Main.Orders do + where (shifted.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> ignore) + + StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) + +let private doubled (id: int64) = id * 2L + +[] +let ``an IN sub-query calling an F# function the database cannot run is refused``() = + let doubledIds = + query { + for order in dc.Main.Orders do + select (doubled order.OrderId) + } + + let error = + Assert.Throws(fun () -> + query { + for order in dc.Main.Orders do + where (not (doubledIds.Contains order.OrderId)) + select order.OrderId + } |> Seq.toArray |> ignore) + + StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) + +[] +let ``an IN sub-query with arithmetic the database computes still runs``() = + let nextIds = + query { + for order in dc.Main.Orders do + select (order.OrderId + 1L) + } + + let result = + query { + for order in dc.Main.Orders do + where (nextIds.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> Array.sort + + let ids = query { for o in dc.Main.Orders do select o.OrderId } |> Seq.toArray + let next = ids |> Array.map (fun id -> id + 1L) |> Set.ofArray + let expected = ids |> Array.filter next.Contains |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) From 4c457ace406e5b5eb3d3d7c570271bc239c6e0df Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Tue, 3 Nov 2026 07:49:54 +0000 Subject: [PATCH 3/4] xmldoc comment fixes --- docs/content/_template.html | 1 + docs/content/content/fsdocs-custom.css | 30 +++++++++- docs/content/content/fsdocs-xmldoc-tips.js | 69 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 docs/content/content/fsdocs-xmldoc-tips.js diff --git a/docs/content/_template.html b/docs/content/_template.html index 800c14dd..8b3abfad 100644 --- a/docs/content/_template.html +++ b/docs/content/_template.html @@ -110,6 +110,7 @@

{{fsdocs-collection-name}}

+ diff --git a/docs/content/content/fsdocs-custom.css b/docs/content/content/fsdocs-custom.css index 67026c11..884494f2 100644 --- a/docs/content/content/fsdocs-custom.css +++ b/docs/content/content/fsdocs-custom.css @@ -345,7 +345,7 @@ blockquote { #content pre.fssnip code { font: 0.86rem 'Fira Code Fixed', monospace; - font-weight: 600; + font-weight: 500; } #content table.pre { @@ -553,7 +553,7 @@ blockquote { } /* keywords */ #content span.k { - color: #1f4ee9; + color: #4878f0; } /* comment */ #content span.c { @@ -597,7 +597,6 @@ div.fsdocs-tip { border-radius: 4px; font: 0.85rem 'Fira Code Fixed', monospace; padding: 6px 8px 6px 8px; - display: none; color: #d1d1d1; pointer-events: none; } @@ -607,6 +606,31 @@ div.fsdocs-tip { font: 0.85rem 'Fira Code Fixed', monospace; } +/* XML doc in a tool tip, rendered by fsdocs-xmldoc-tips.js */ +div.fsdocs-tip-doc { + margin-top: 6px; + padding-top: 6px; + border-top: 1px solid #5f7478; + white-space: normal; +} + +div.fsdocs-tip-doc-summary { + color: #f0d98c; +} + +div.fsdocs-tip-doc-remarks, +div.fsdocs-tip-doc-param { + margin-top: 4px; +} + +span.fsdocs-tip-doc-name { + color: #9cdcfe; +} + +div.fsdocs-tip-doc code { + color: #ffffff; +} + #content code { color: #8c938c } diff --git a/docs/content/content/fsdocs-xmldoc-tips.js b/docs/content/content/fsdocs-xmldoc-tips.js new file mode 100644 index 00000000..b1d2fe9a --- /dev/null +++ b/docs/content/content/fsdocs-xmldoc-tips.js @@ -0,0 +1,69 @@ +// fsdocs puts a member's XML doc into its tooltip as raw text, tags included. +// This renders the tags: the summary in its own colour, one line per parameter, the return value, and +// / as the name they refer to. + +const docTag = /<\/?(summary|param|returns|remarks|exception|typeparam|see|paramref|typeparamref|para|c|code)\b/; + +function refName(el) { + const name = el.getAttribute("cref") || el.getAttribute("name") || el.getAttribute("langword") || el.textContent; + return name.replace(/^[A-Z]:/, ""); +} + +function inline(node, target) { + for (const child of node.childNodes) { + if (child.nodeType === Node.TEXT_NODE) { + target.append(child.textContent.replace(/\s+/g, " ")); + } else if (child.nodeType === Node.ELEMENT_NODE) { + const tag = child.localName; + if (tag === "see" || tag === "seealso" || tag === "paramref" || tag === "typeparamref" || tag === "c" || tag === "code") { + const code = document.createElement("code"); + code.textContent = child.textContent.trim() || refName(child); + target.append(code); + } else { + inline(child, target); + } + } + } +} + +function line(cls, label, el) { + const body = document.createDocumentFragment(); + inline(el, body); + body.normalize(); + if (body.firstChild?.nodeType === Node.TEXT_NODE) body.firstChild.textContent = body.firstChild.textContent.trimStart(); + if (body.lastChild?.nodeType === Node.TEXT_NODE) body.lastChild.textContent = body.lastChild.textContent.trimEnd(); + const div = document.createElement("div"); + div.className = cls; + if (label) { + const name = document.createElement("span"); + name.className = "fsdocs-tip-doc-name"; + name.textContent = label; + div.append(name, " "); + } + div.append(body); + return div; +} + +function render(text) { + const xml = new DOMParser().parseFromString(`${text}`, "application/xml"); + if (xml.getElementsByTagName("parsererror").length > 0) return null; + const doc = document.createElement("div"); + doc.className = "fsdocs-tip-doc"; + for (const el of xml.documentElement.children) { + switch (el.localName) { + case "summary": doc.append(line("fsdocs-tip-doc-summary", null, el)); break; + case "remarks": doc.append(line("fsdocs-tip-doc-remarks", null, el)); break; + case "param": + case "typeparam": doc.append(line("fsdocs-tip-doc-param", el.getAttribute("name") + ":", el)); break; + case "returns": doc.append(line("fsdocs-tip-doc-param", "Returns:", el)); break; + case "exception": doc.append(line("fsdocs-tip-doc-param", "Throws " + refName(el) + ":", el)); break; + } + } + return doc.childNodes.length > 0 ? doc : null; +} + +for (const em of document.querySelectorAll("div.fsdocs-tip > em")) { + if (!docTag.test(em.textContent)) continue; + const doc = render(em.textContent); + if (doc) em.replaceWith(doc); +} From b7ec7d01f339a3419f263b9fcfa6a7e54abf5e5d Mon Sep 17 00:00:00 2001 From: Tuomas Hietanen Date: Tue, 3 Nov 2026 08:19:55 +0000 Subject: [PATCH 4/4] copilot review addressed --- docs/RELEASE_NOTES.md | 2 +- .../SqlRuntime.QueryExpression.fs | 64 +++++++++- .../more/SubqueryCompositionTests.fs | 109 ++++++++++++++++++ 3 files changed, 168 insertions(+), 7 deletions(-) diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index ebc59899..2d1e7a6c 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -2,7 +2,7 @@ - Fix: parameters of a sub-query nested in another sub-query are bound (IN, EXISTS, UNION and the like) - Fix: groupBy keys with option or value-option columns, and with x.IsSome / x.IsNone in a tuple key - Fix: a grouped sub-query of IN / NOT IN that selects an aggregate selects it without the group keys, on both sides of a union too, #746 -- IN / NOT IN sub-queries with a projection computed in .NET (e.g. `g.Max(...) + 1L`) fail with an error instead of comparing against the uncomputed column +- IN / NOT IN sub-queries with a projection computed in .NET (e.g. `g.Max(...) + 1L`, a narrowing conversion, a user function) fail with an error instead of comparing against the uncomputed column - Design time: a provider instance that goes away closes its connection instead of disposing it, as the cached types of other instances still use it ("Cannot access a disposed object") - PostgreSQL: identity columns count as generated, so the typed ``Create(...)`` no longer takes them. Breaking for code that passed a value to a BY DEFAULT identity column: use Create() and set it, or Create(data). A saved ContextSchemaPath cache keeps the old signature until regenerated. diff --git a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs index e14700fd..e3ea7f37 100644 --- a/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs +++ b/src/SQLProvider.Common/SqlRuntime.QueryExpression.fs @@ -632,15 +632,67 @@ module internal QueryExpressionTransformer = newProjection, projectionMap, groupProjectionMap - /// A projection that reads one selected column as it is: a column, an aggregate or a group key, through casts and - /// an option's Value. Anything else is computed in .NET after the rows are read. + /// Numeric conversions that keep every value of the source type. + let private wideningConversions = + let integralTo (from:Type) (targets:Type list) = targets |> List.map (fun t -> from, t) + [ yield! integralTo typeof [typeof; typeof; typeof; typeof; typeof; typeof] + yield! integralTo typeof [typeof; typeof; typeof; typeof; typeof; typeof; typeof; typeof; typeof] + yield! integralTo typeof [typeof; typeof; typeof; typeof; typeof] + yield! integralTo typeof [typeof; typeof; typeof; typeof; typeof; typeof; typeof] + yield! integralTo typeof [typeof; typeof; typeof] + yield! integralTo typeof [typeof; typeof; typeof; typeof] + yield! integralTo typeof [typeof] + yield! integralTo typeof [typeof] + typeof, typeof ] + + /// A conversion that keeps the value: identity, Nullable wrapping, a reference cast, boxing or unboxing, an enum and + /// its underlying type, or a numeric widening. A user-defined conversion operator runs .NET code, so it doesn't. + let private isValuePreserving (u:UnaryExpression) = + let underlying (t:Type) = match Nullable.GetUnderlyingType t with null -> t | n -> n + let source, target = underlying u.Operand.Type, underlying u.Type + isNull u.Method + && (Type.(=)(source, target) + || not source.IsValueType || not target.IsValueType + || (source.IsEnum && Type.(=)(Enum.GetUnderlyingType source, target)) + || (target.IsEnum && Type.(=)(Enum.GetUnderlyingType target, source)) + || wideningConversions |> List.exists (fun (s, t) -> Type.(=)(s, source) && Type.(=)(t, target))) + + /// The row a projection reads: its parameter, a tuple or anonymous-object item of it, or a joined table's row of + /// it, through casts. + let rec private isRowReference (e:Expression) = + match e with + | :? ParameterExpression -> true + | :? MethodCallExpression as c when Type.(=)(c.Method.DeclaringType, typeof) && (c.Method.Name = "GetSubTable" || c.Method.Name = "GetSubTableNullable") -> + not (isNull c.Object) && c.Arguments |> Seq.forall (fun a -> a :? ConstantExpression) && isRowReference c.Object + | :? UnaryExpression as u when (u.NodeType = ExpressionType.Convert || u.NodeType = ExpressionType.TypeAs) && isNull u.Method -> + isRowReference u.Operand + | :? MemberExpression as m when not (isNull m.Expression) -> + let owner = m.Member.DeclaringType + (Microsoft.FSharp.Reflection.FSharpType.IsTuple owner || owner.Name.StartsWith "AnonymousObject" || owner.Name.Contains "AnonymousType") + && isRowReference m.Expression + | _ -> false + + /// A projection that reads one selected column as it is: a column, an aggregate or a group key, through + /// value-preserving casts and an option's Value. Anything else is computed in .NET after the rows are read. let rec private isColumnRead (e:Expression) = match e with | :? UnaryExpression as u when u.NodeType = ExpressionType.Convert || u.NodeType = ExpressionType.ConvertChecked || u.NodeType = ExpressionType.TypeAs -> - isColumnRead u.Operand - | :? MemberExpression as m when m.Member.Name = "Value" && not (isNull m.Expression) -> isColumnRead m.Expression - | :? MemberExpression as m when m.Member.Name = "Key" -> true - | :? MethodCallExpression as c -> c.Method.Name.StartsWith "GetColumn" || c.Method.Name.StartsWith "Aggregate" + isValuePreserving u && isColumnRead u.Operand + | :? MemberExpression as m when m.Member.Name = "Value" && not (isNull m.Expression) -> + (Utilities.isCOpt m.Member.DeclaringType || Utilities.isVOpt m.Member.DeclaringType || not (isNull (Nullable.GetUnderlyingType m.Expression.Type))) + && isColumnRead m.Expression + | :? MemberExpression as m when m.Member.Name = "Key" && not (isNull m.Expression) -> + let owner = m.Member.DeclaringType + (Utilities.isGrp owner || (owner.IsGenericType && Type.(=)(owner.GetGenericTypeDefinition(), typedefof>))) + && isRowReference m.Expression + | :? MethodCallExpression as c when not (isNull c.Object) && c.Arguments |> Seq.forall (fun a -> a :? ConstantExpression) -> + let owner = c.Method.DeclaringType + if Type.(=)(owner, typeof) then + (c.Method.Name = "GetColumn" || c.Method.Name = "GetColumnOption" || c.Method.Name = "GetColumnValueOption") + && isRowReference c.Object + else + owner.IsGenericType && Type.(=)(owner.GetGenericTypeDefinition(), typedefof>) + && c.Method.Name.StartsWith "Aggregate" && isRowReference c.Object | _ -> false /// As convertExpression; `membershipSubquery` true is for a sub-query of IN / NOT IN, which may return one column diff --git a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs index 087d97e9..b6933a8b 100644 --- a/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs +++ b/tests/SqlProvider.Tests/more/SubqueryCompositionTests.fs @@ -569,6 +569,115 @@ let ``an IN sub-query calling an F# function the database cannot run is refused` StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) +[] +let ``an IN sub-query converting an aggregate to a narrower number in .NET is refused``() = + let roundedAverages = + query { + for detail in dc.Main.OrderDetails do + groupBy detail.ProductId into g + select (int64 (g.Average(fun detail -> detail.UnitPrice))) + } + + let error = + Assert.Throws(fun () -> + query { + for product in dc.Main.Products do + where (roundedAverages.Contains product.ProductId) + select product.ProductId + } |> Seq.toArray |> ignore) + + StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) + +let private GetColumnDoubled (id: int64) = id * 2L + +[] +let ``an IN sub-query calling a function named like a column read is refused``() = + let doubledIds = + query { + for order in dc.Main.Orders do + select (GetColumnDoubled order.OrderId) + } + + let error = + Assert.Throws(fun () -> + query { + for order in dc.Main.Orders do + where (doubledIds.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> ignore) + + StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) + +type private DoubledId(id: int64) = + member _.Key = id * 2L + +[] +let ``an IN sub-query reading a Key that is not a group key is refused``() = + let doubledIds = + query { + for order in dc.Main.Orders do + select (DoubledId(order.OrderId).Key) + } + + let error = + Assert.Throws(fun () -> + query { + for order in dc.Main.Orders do + where (doubledIds.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> ignore) + + StringAssert.StartsWith("IN / NOT IN needs a sub-query that selects one column", error.Message) + +[] +let ``an IN sub-query selecting a group key still runs``() = + let popular = + query { + for detail in dc.Main.OrderDetails do + groupBy detail.ProductId into g + where (g.Count() > 40) + select g.Key + } + + let result = + query { + for product in dc.Main.Products do + where (popular.Contains product.ProductId) + select product.ProductId + } |> Seq.toArray |> Array.sort + + let expected = + query { for d in dc.Main.OrderDetails do select d.ProductId } + |> Seq.countBy id |> Seq.filter (fun (_, n) -> n > 40) |> Seq.map fst |> Seq.toArray |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) + +[] +let ``a grouped IN sub-query with a having filter selects only its aggregate``() = + let latestOfPopular = + query { + for detail in dc.Main.OrderDetails do + groupBy detail.ProductId into g + where (g.Count() > 40) + select (g.Max(fun detail -> detail.OrderId)) + } + + let result = + query { + for order in dc.Main.Orders do + where (latestOfPopular.Contains order.OrderId) + select order.OrderId + } |> Seq.toArray |> Array.sort + + let expected = + query { for d in dc.Main.OrderDetails do select (d.ProductId, d.OrderId) } + |> Seq.groupBy fst |> Seq.filter (fun (_, rows) -> Seq.length rows > 40) + |> Seq.map (fun (_, rows) -> rows |> Seq.map snd |> Seq.max) |> Seq.distinct |> Seq.toArray |> Array.sort + + Assert.IsNotEmpty expected + Assert.AreEqual(expected, result) + [] let ``an IN sub-query with arithmetic the database computes still runs``() = let nextIds =