Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composed or nested queries returning tuples can result in client-side filtering #40

Closed
KevinRansom opened this issue Jan 18, 2015 · 2 comments
Labels
Area-Queries Query expressions and library implementation Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@KevinRansom
Copy link
Member

Opened at codeplex by: latkin

When F# queries which return tuples are composed or nested, the queries are not translated efficiently, resulting in client-side filtering where server-side filtering should have been possible.

This becomes very problematic in big-data scenarios: you test with a small data set and results look correct, but when you move to big data set you are unintentionally sucking down tons of data to the client when you thought the query resulted in server-side filtering.

Issue is related to the gymnastics done to swap in mutable tuples required during query translation, then switching back to immutable tuples on the output end.

Repro:

#r "FSharp.Data.TypeProviders"
#r "System.Data.Entity"
#r "System.Data.Services.Client"

open Microsoft.FSharp.Data.TypeProviders

type Northwind = ODataService< "http://services.odata.org/V2/Northwind/Northwind.svc/" >
let client = Northwind.GetDataContext()

client.DataContext.SendingRequest.Add (fun x -> printfn "requesting %A" x.Request.RequestUri)

Proper server-side filtering done here:
requesting http://services.odata.org/V2/Northwind/Northwind.svc/Customers()?$filter=substringof('S',ContactName)& $top=10 &$select=*
'''fsharp
let r =
query { for g in client.Customers do
where (g.ContactName.Contains "S")
take 10
select (g, 1) }

for (g, _) in r do
printfn "Name %s" g.ContactName

Improper client-side filtering done here when query is composed: 
requesting http://services.odata.org/V2/Northwind/Northwind.svc/Customers()?$filter=substringof('S',ContactName)&$select=*
```fsharp
let r1 = 
    let a = 
        query 
            { for g in client.Customers do  
              where (g.ContactName.Contains "S")            
              select (g, 1) }
    let b = 
        query { for g in a do take 10}
    b
for (g, _) in r1 do
    printfn "Name %s" g.ContactName

Similarly with nested queries:

let r2 = 
    let b = 
        query {
          for g in
            (query {
              for g in client.Customers do  
              where (g.ContactName.Contains "S")            
              select (g, 1) }
              ) do  
              take 10 }
    b

for (g, _) in r2 do
    printfn "Name %s" g.ContactName
@latkin latkin added the Bug label Jan 19, 2015
@latkin latkin closed this as completed in b748533 Apr 8, 2015
@dsyme
Copy link
Contributor

dsyme commented Apr 9, 2015

@latkin - did you close the right bug? The commit seems unrelated?

@latkin
Copy link
Contributor

latkin commented Apr 9, 2015

Oops, typo. I managed to close the right one, but forgot to re-open this one.

@latkin latkin reopened this Apr 9, 2015
@dsyme dsyme added Area-Library Issues for FSharp.Core not covered elsewhere Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. labels Jan 8, 2016
ncave added a commit to ncave/fsharp that referenced this issue Jan 20, 2018
* Add option to toggle unused declarations analyzer (dotnet#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert dotnet#1650 (and dotnet#3366) (dotnet#4173)

* Fix error logging in brace matching code (dotnet#4140)

* Remove error logger pushing code

* Update service.fs

* Fix dotnet#4200: Vsix: fix empty "New file" window for web projects (dotnet#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (dotnet#4194)

* Fixed FCS netcore tests (dotnet#4180)

* Remove ambiguous resolution error FS0332 (dotnet#4170)

* Add IsInteractive to parsing options for script load closures (dotnet#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (dotnet#4211)

* Minor fix (dotnet#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (dotnet#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (dotnet#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests
ncave added a commit to ncave/fsharp that referenced this issue Jan 24, 2018
* Add option to toggle unused declarations analyzer (dotnet#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert dotnet#1650 (and dotnet#3366) (dotnet#4173)

* Fix error logging in brace matching code (dotnet#4140)

* Remove error logger pushing code

* Update service.fs

* Fix dotnet#4200: Vsix: fix empty "New file" window for web projects (dotnet#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (dotnet#4194)

* Fixed FCS netcore tests (dotnet#4180)

* Remove ambiguous resolution error FS0332 (dotnet#4170)

* Add IsInteractive to parsing options for script load closures (dotnet#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (dotnet#4211)

* Minor fix (dotnet#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (dotnet#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (dotnet#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests
ncave added a commit to ncave/fsharp that referenced this issue Jan 24, 2018
* Add option to toggle unused declarations analyzer (dotnet#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert dotnet#1650 (and dotnet#3366) (dotnet#4173)

* Fix error logging in brace matching code (dotnet#4140)

* Remove error logger pushing code

* Update service.fs

* Fix dotnet#4200: Vsix: fix empty "New file" window for web projects (dotnet#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (dotnet#4194)

* Fixed FCS netcore tests (dotnet#4180)

* Remove ambiguous resolution error FS0332 (dotnet#4170)

* Add IsInteractive to parsing options for script load closures (dotnet#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (dotnet#4211)

* Minor fix (dotnet#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (dotnet#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (dotnet#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests
KevinRansom pushed a commit that referenced this issue Feb 17, 2018
* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests (#40)

* Add option to toggle unused declarations analyzer (#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert #1650 (and #3366) (#4173)

* Fix error logging in brace matching code (#4140)

* Remove error logger pushing code

* Update service.fs

* Fix #4200: Vsix: fix empty "New file" window for web projects (#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (#4194)

* Fixed FCS netcore tests (#4180)

* Remove ambiguous resolution error FS0332 (#4170)

* Add IsInteractive to parsing options for script load closures (#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (#4211)

* Minor fix (#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests

* test conditions update

* test update

* test condition update

* test update

* review update

* added checked operators

* fixed dual conversions

* review fixes

* more targeted replacements

* adapt to latest

* added more tests

* added more tests

* review fixes

* fixed warnings
@cartermp cartermp added this to the Unknown milestone Aug 25, 2018
@cartermp cartermp modified the milestones: Unknown / not bug, Backlog May 23, 2019
@dsyme dsyme added Area-Queries Query expressions and library implementation and removed Area-Library Issues for FSharp.Core not covered elsewhere labels Mar 30, 2022
@vzarytovskii vzarytovskii moved this to Not Planned in F# Compiler and Tooling Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Queries Query expressions and library implementation Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Archived in project
Development

No branches or pull requests

4 participants