fsprojects / SQLProvider

A general F# SQL database erasing type provider, supporting LINQ queries, schema exploration, individuals, CRUD operations and much more besides.
https://fsprojects.github.io/SQLProvider
Other
580 stars 146 forks source link

OnConflict.Update ignores None/ValueNone values when using DatabaseProviderTypes.POSTGRESQL #814

Open mishun opened 10 months ago

mishun commented 10 months ago

Hi!

Describe the bug If I create following simple key-value pairs Postgresql database:

begin;

create table "Test" (
    "ID"    int4 generated by default as identity primary key,
    "Value" int4 null
);

insert into "Test" ("ID", "Value") values
    (0, 1),
    (1, 1);

commit;

and run following code:

module Main

open System.Threading.Tasks
open FSharp.Data.Sql
open FSharp.Data.Sql.Common

[<Literal>]
let private resolutionPath = __SOURCE_DIRECTORY__ + "/obj/db-libs/"

[<Literal>]
let private connectionString = "Host=localhost;Port=5433;Database=test;User ID=test;Password=test"

type postgres =
    SqlDataProvider<
        DatabaseVendor   = DatabaseProviderTypes.POSTGRESQL,
        ResolutionPath   = resolutionPath,
        ConnectionString = connectionString,
        UseOptionTypes   = NullableColumnType.OPTION
    >

let update (key : int) (value : int option) = task {
    let ctx = postgres.GetDataContext connectionString
    let row = ctx.Public.Test.Create()
    row.Id <- key
    row.Value <- value
    row.OnConflict <- OnConflict.Update
    do! ctx.SubmitUpdatesAsync()
}

[<EntryPoint>]
let main args =
    Task.WaitAll(task {
        try
            do! update 0 None
        with ex ->
            printfn "Exception: %A" ex
    })
    0

I'd expect Value in the first row to be changed to NULL, but in fact nothing happens. Same goes for NullableColumnType.VALUE_OPTION and ValueNone. Something like update 0 (Some 69) and update 2 None work as expected.

Tested with SQLProvider 1.3.23 from nuget package.

Additional context fsproj and docker-compose.yaml for reproduction:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="Program.fs" />
  </ItemGroup>

  <ItemGroup>
    <PackageReference Update="FSharp.Core" Version="7.0.400" />
    <PackageReference Include="Npgsql" Version="7.0.6" />
    <PackageReference Include="SQLProvider" Version="1.3.23" />
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.3" GeneratePathProperty="true" />
  </ItemGroup>

  <Target Name="CopyDBLibs" BeforeTargets="BeforeBuild">
    <Copy SourceFiles="$(PkgMicrosoft_Extensions_Logging_Abstractions)/lib/net6.0/Microsoft.Extensions.Logging.Abstractions.dll"
          DestinationFolder="$(MSBuildThisFileDirectory)/obj/db-libs/"
          UseSymbolicLinksIfPossible="True" />
  </Target>
</Project>
version: "3.8"

services:
  geometra-db:
    image: postgres:14
    container_name: test-db
    restart: no
    ports:
      - 5433:5432
    volumes:
      - ./init.sql:/docker-entrypoint-initdb.d/00-init.sql:ro
    environment:
      POSTGRES_USER: test
      POSTGRES_PASSWORD: test

Complete example can be found here.

jimfoye commented 3 months ago

That's a great bug report. I'd love to take a stab at fixing this, but I can't currently build SQLProvider on my installation of Fedora. I have SDK version 6.0.132 installed, I don't think I can install 6.0.403 through the Fedora package manager, and I don't see that version on the MS downloads page. All attempts to target a different version of the SDK than 6.0.132 failed. @Thorium if you have any ideas how I can get this to build, please share.

Thorium commented 3 months ago

For the original issue:

  1. Typically SQL provider works in a way that you fetch the full entity, then you update the columns you want. And then you submit updates. This is safest way, to not conflict with column values: If your code would work, it would be unclear which of the partially created entity update nulls are intentional and which are just not set properties. So exactly the typical issue with NULL breaking single responsibility principle.

  2. I recommend using transactions instead of onconflict. However I expect this issue is nothing to do with onconflict.

  3. Yes there is the hack to do partial updates that does about this, but I don't recommend doing that: https://github.com/fsprojects/SQLProvider/issues/561#issuecomment-404956768

What comes to .Net version, your best shot is to modify globals.json content. I've sent F# FAKE a few PRs which I help with these in the future, but those are not merged and released yet. When they are, I will update FAKE build, so there is a hope for a better future.