Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ nupkg
**/.idea/**/tasks.xml
**/.idea/shelf/*
**/.idea/dictionaries
.idea

# Sensitive or high-churn files
**/.idea/**/dataSources/
Expand Down
9 changes: 8 additions & 1 deletion src/SqlHydra.Npgsql/NpgsqlSchemaProvider.fs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module SqlHydra.Npgsql.NpgsqlSchemaProvider

open System.Data
open Npgsql
open NpgsqlTypes
open SqlHydra.Domain

let getSchema (cfg: Config) : Schema =
Expand Down Expand Up @@ -59,6 +59,12 @@ let getSchema (cfg: Config) : Schema =
|}
)

let getDbColumnType =
function
| "json" -> { TypeName = nameof NpgsqlDbType; TypeValue = nameof NpgsqlDbType.Json } |> Some
| "jsonb" -> { TypeName = nameof NpgsqlDbType; TypeValue = nameof NpgsqlDbType.Jsonb } |> Some
| _ -> None

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than defining a lookup function, I think that this mapping should be added as a string option field on the supportedTypeMappings for each of the providers (NpgsqlDataTypes.fs, SqliteDataTypes.fs, and SqlServerDataTypes.fs. Most of them can default to None.

This value can then be moved from the Column record to to the TypeMapping record.
The TypeMapping record already has a DbType property that holds the System.Data.DbType enum.
Since each provider has its own more specific enumeration (i.e. NpgsqlDbType for Postgres, SqlDbType for SQL Server -- and I don't think SQLite has a more specific enum), I suppose the new property should be ProviderDbType.

let tables =
sTables.Rows
|> Seq.cast<DataRow>
Expand Down Expand Up @@ -87,6 +93,7 @@ let getSchema (cfg: Config) : Schema =
|> Option.map (fun typeMapping ->
{
Column.Name = col.ColumnName
Column.DbColumnType = getDbColumnType col.ProviderTypeName
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated above, move this property to TypeMapping and rename to ProviderDbType.

Column.IsNullable = col.IsNullable
Column.TypeMapping = typeMapping
Column.IsPK = pks.Contains(col.TableSchema, col.TableName, col.ColumnName)
Expand Down
25 changes: 20 additions & 5 deletions src/SqlHydra.Query/Kata.fs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
namespace SqlHydra.Query

open System.Reflection
open SqlHydra.DbColumnTypeAttribute
open SqlHydra.Domain
open SqlKata
open System.Collections.Generic
open System
Expand All @@ -25,6 +28,12 @@ module FQ =
| Some schema -> $"{schema}.{tbl.Name}"
| None -> tbl.Name

type QueryParameter =
{
Value: obj
Type: DbColumnType option
}

type InsertQuerySpec<'T, 'Identity> =
{
Table: string
Expand Down Expand Up @@ -80,28 +89,34 @@ module private KataUtils =
| null -> box System.DBNull.Value
| o -> o

let getDbColumnType (p: PropertyInfo) =
let attrs = p.GetCustomAttributes(true)
(attrs
|> Seq.choose (function | :? DbColumnTypeAttribute as attr -> Some attr.ColumnType | _ -> None))
|> Seq.tryHead

let fromUpdate (spec: UpdateQuerySpec<'T>) =
let kvps =
match spec.Entity, spec.SetValues with
| Some entity, [] ->
match spec.Fields with
| [] ->
FSharp.Reflection.FSharpType.GetRecordFields(typeof<'T>)
|> Array.map (fun p -> p.Name, p.GetValue(entity))
|> Array.map (fun p -> p.Name, { Value = p.GetValue(entity) |> boxValueOrOption; Type = getDbColumnType p } :> obj)

| fields ->
let included = fields |> Set.ofList
FSharp.Reflection.FSharpType.GetRecordFields(typeof<'T>)
|> Array.filter (fun p -> included.Contains(p.Name))
|> Array.map (fun p -> p.Name, p.GetValue(entity))
|> Array.map (fun p -> p.Name, { Value = p.GetValue(entity) |> boxValueOrOption; Type = getDbColumnType p } :> obj)

| Some _, _ -> failwith "Cannot have both `entity` and `set` operations in an `update` expression."
| None, [] -> failwith "Either an `entity` or `set` operations must be present in an `update` expression."
| None, setValues -> setValues |> List.toArray

let preparedKvps =
kvps
|> Seq.map (fun (key,value) -> key, boxValueOrOption value)
|> Seq.map (fun (key,value) -> key, value)
|> dict
|> Seq.map id

Expand Down Expand Up @@ -129,7 +144,7 @@ module private KataUtils =
| [ entity ] ->
let keyValuePairs =
includedProperties
|> Array.map (fun p -> KeyValuePair(p.Name, p.GetValue(entity) |> boxValueOrOption))
|> Array.map (fun p -> KeyValuePair(p.Name, { Value = p.GetValue(entity) |> boxValueOrOption; Type = getDbColumnType p } :> obj))
|> Array.toList
Query(spec.Table).AsInsert(keyValuePairs, returnId = spec.IdentityField.IsSome)

Expand All @@ -141,7 +156,7 @@ module private KataUtils =
entities
|> List.map (fun entity ->
includedProperties
|> Array.map (fun p -> p.GetValue(entity) |> boxValueOrOption)
|> Array.map (fun p -> { Value = p.GetValue(entity) |> boxValueOrOption; Type = getDbColumnType p } :> obj)
|> Array.toSeq
)
Query(spec.Table).AsInsert(columns, rowsValues)
Expand Down
19 changes: 17 additions & 2 deletions src/SqlHydra.Query/QueryContext.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@ open SqlKata

/// Contains methods that compile and read a query.
type QueryContext(conn: DbConnection, compiler: SqlKata.Compilers.Compiler) =

let setParameterDbType (param: DbParameter) (qp: QueryParameter) =
match qp.Type, compiler with
| Some type', :? SqlKata.Compilers.PostgresCompiler when type'.TypeName = "NpgsqlDbType" ->
let property = param.GetType().GetProperty("NpgsqlDbType")
let dbTypeSetter = property.GetSetMethod()

let value = System.Enum.Parse(property.PropertyType, type'.TypeValue)
dbTypeSetter.Invoke(param, [|value|]) |> ignore
| _ -> ()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a handler here for SQL Server as well:

| Some type', :? SqlKata.Compilers.SqlServerCompiler when type'.TypeName = "SqlDbType" ->
            let property = param.GetType().GetProperty("SqlDbType")
 ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had TypeName and TypeValue in the attribute before, but I figured you meant to just have a string there so I stored only the enum value of the provider type without the type name (like NpgsqlDbType.Json became just Json). So now I can't really do the check on type'.TypeName = "SqlDbType". What are you thoughts on this? Do we need to store the full type like NpgsqlDbType.Json or SqlDbType.Whatever?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you have in NpgsqlDataTypes.fs and the attribute looks perfect. I think that it will be safe enough to assume that if they are using a given SqlKata compiler that they should be passing in types with the appropriate attributes.

So this should work:

    let setParameterDbType (param: DbParameter) (qp: QueryParameter) =
      match qp.ProviderDbType, compiler with
      | Some dbType, :? SqlKata.Compilers.PostgresCompiler ->
          let property = param.GetType().GetProperty("NpgsqlDbType")
          let dbTypeSetter = property.GetSetMethod()            
          let value = System.Enum.Parse(property.PropertyType, dbType)
          dbTypeSetter.Invoke(param, [|value|]) |> ignore

      | Some dbType, :? SqlKata.Compilers.SqlServerCompiler ->
          let property = param.GetType().GetProperty("SqlDbType")
          let dbTypeSetter = property.GetSetMethod()            
          let value = System.Enum.Parse(property.PropertyType, dbType)
          dbTypeSetter.Invoke(param, [|value|]) |> ignore
      | _ -> ()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the code now looks almost as yours, just with a helper function to hide the reflection ugliness 🙂

interface System.IDisposable with
member this.Dispose() =
conn.Dispose()
Expand Down Expand Up @@ -44,7 +53,13 @@ type QueryContext(conn: DbConnection, compiler: SqlKata.Compilers.Compiler) =
for kvp in compiledQuery.NamedBindings do
let p = cmd.CreateParameter()
p.ParameterName <- kvp.Key
p.Value <- kvp.Value

match kvp.Value with
| :? QueryParameter as qp ->
do setParameterDbType p qp
p.Value <- qp.Value
| _ ->
p.Value <- kvp.Value
cmd.Parameters.Add(p) |> ignore
cmd

Expand Down
6 changes: 5 additions & 1 deletion src/SqlHydra.Query/SqlHydra.Query.fsproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<TargetFramework>net5.0</TargetFramework>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<WarnOn>3390;$(WarnOn)</WarnOn>
<Version>0.530.0</Version>
Expand Down Expand Up @@ -32,4 +32,8 @@
<PackageReference Include="SqlKata" Version="2.3.7" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\SqlHydra\SqlHydra.fsproj" />
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a wrong thing to do, I needed to reference the attribute I created inside SqlHydra. Does it mean the nuget package will get a dependency to SqlHydra?
Also had to change the target framework to be compatible with that project. Is it on purpose that this project is targeting netstandard2.0 to be compatible with the older versions of dotnet? Does it make sense to create a shared project that will be referenced from both projects instead?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, SqlHydra.Query is netstandard2.0 to make it compatible with more project types. Since SqlHydra.Query doesn't use any new features, it would be nice to allow greater compatibility for a little while longer if possible.

I think you should be able to:

  • Change SqlHydra project to use netstandard2.0 so that it can be used in SqlHydra.Query
  • Add the following line into the SqlHydra.Query project so that it will copy the SqlHydra dll to nuget (this has already been done in the provider projects that reference SqlHydra.):
<TargetsForTfmSpecificBuildOutput>$(TargetsForTfmSpecificBuildOutput);CopyProjectReferencesToPackage</TargetsForTfmSpecificBuildOutput>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like SqlHydra.fsproj references package Microsoft.Build version 16.11.0 which gives warning that it may not be fully compatible with netstandard 2.0. Not sure whether this can seriously break things somewhere.

Copy link
Owner

@JordanMarr JordanMarr Oct 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlHydra.Query really doesn't need to reference Microsoft.Build or anything like that since most of that stuff is only used by the generator projects.

So we will have to do the following:

  • Create a new netstandard2.0 project, SqlHydra.Domain.
  • Move Domain.fs and the new attribute to the new project.
  • In Domain.fs, open GlobExpressions and applyFilters function need to be moved back to the SqlHydra project; they can be added to a new file Filters.fs (since this is only used by the generators).

</ItemGroup>

</Project>
3 changes: 2 additions & 1 deletion src/SqlHydra.SqlServer/SqlServerSchemaProvider.fs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ let getSchema (cfg: Config) : Schema =
|> Seq.choose (fun col ->
SqlServerDataTypes.tryFindTypeMapping(col.ProviderTypeName)
|> Option.map (fun typeMapping ->
{
{
Column.Name = col.ColumnName
Column.DbColumnType = None
Column.IsNullable = col.IsNullable
Column.TypeMapping = typeMapping
Column.IsPK = pks.Contains(col.TableSchema, col.TableName, col.ColumnName)
Expand Down
1 change: 1 addition & 0 deletions src/SqlHydra.Sqlite/SqliteSchemaProvider.fs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ let getSchema (cfg: Config) : Schema =
|> Option.map (fun typeMapping ->
{
Column.Name = col.ColumnName
Column.DbColumnType = None
Column.IsNullable = col.IsNullable
Column.TypeMapping = typeMapping
Column.IsPK = col.IsPK
Expand Down
13 changes: 13 additions & 0 deletions src/SqlHydra/DbColumnTypeAttribute.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module SqlHydra.DbColumnTypeAttribute

open System
open SqlHydra.Domain

[<AttributeUsage(AttributeTargets.Property
||| AttributeTargets.Field)>]
type DbColumnTypeAttribute(columnTypeName: string, columnTypeValue: string) =
inherit Attribute()

member this.ColumnType: DbColumnType =
{ TypeName = columnTypeName
TypeValue = columnTypeValue }
7 changes: 7 additions & 0 deletions src/SqlHydra/Domain.fs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,16 @@ type TypeMapping =
ReaderMethod: string
}

type DbColumnType =
{
TypeName: string
TypeValue: string
}

type Column =
{
Name: string
DbColumnType: DbColumnType option
TypeMapping: TypeMapping
IsNullable: bool
IsPK: bool
Expand Down
48 changes: 40 additions & 8 deletions src/SqlHydra/SchemaGenerator.fs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
module SqlHydra.SchemaGenerator
open FSharp.Compiler.SyntaxTree
open FSharp.Compiler
open FSharp.Compiler.XmlDoc
open FsAst
open Fantomas
open Domain
open System.Data
open SqlHydra.DbColumnTypeAttribute

let range0 = Range.range.Zero

Expand All @@ -23,26 +25,53 @@ let cliMutableAttribute =
let atts = [ SynAttributeList.Create(attr) ]
SynModuleDecl.CreateAttributes(atts)

let createDbColumnTypeAttributes (column: Column) =
column.DbColumnType
|> Option.map (fun type' ->
let attr =
{ TypeName = LongIdentWithDots.CreateString (nameof DbColumnTypeAttribute)
; ArgExpr = SynExpr.CreateParenedTuple [ SynExpr.CreateConst (SynConst.String(type'.TypeName, range0))
SynExpr.CreateConst (SynConst.String(type'.TypeValue, range0)) ]
; Target = None
; AppliesToGetterAndSetter = false
; Range = range0 } : SynAttribute

SynAttributes.Cons (SynAttributeList.Create attr, SynAttributes.Empty)
) |> Option.defaultValue SynAttributes.Empty

/// Creates a record definition named after a table.
let createTableRecord (tbl: Table) =
let myRecordId = LongIdentWithDots.CreateString tbl.Name
let recordCmpInfo = SynComponentInfoRcd.Create(myRecordId.Lid)

let recordDef =
tbl.Columns
|> List.map (fun col ->
|> List.map (fun col ->
let field =
if col.TypeMapping.ClrType = "byte[]" then
let b = SynType.Create("byte")
SynType.Array(0, b, range0)
else
SynType.Create(col.TypeMapping.ClrType)

if col.IsNullable then
let opt = SynType.Option(field)
SynFieldRcd.Create(Ident.Create(col.Name), opt)
else
SynFieldRcd.Create(Ident.Create(col.Name), field)

let attributes = createDbColumnTypeAttributes col

let type' =
if col.IsNullable then
SynType.Option(field)
else
field

{
Attributes = attributes
IsStatic = false
Id = Some (Ident.Create(col.Name))
Type = type'
IsMutable = false
XmlDoc = PreXmlDoc.Empty
Access = None
Range = range0
}
)
|> SynTypeDefnSimpleReprRecordRcd.Create
|> SynTypeDefnSimpleReprRcd.Record
Expand Down Expand Up @@ -631,7 +660,10 @@ let substitutions =
[
/// Reader classes at top of namespace
"open Substitute.Extensions",
"""type Column(reader: System.Data.IDataReader, getOrdinal: string -> int, column) =
"""
open SqlHydra.DbColumnTypeAttribute

type Column(reader: System.Data.IDataReader, getOrdinal: string -> int, column) =
member __.Name = column
member __.IsNull() = getOrdinal column |> reader.IsDBNull
override __.ToString() = __.Name
Expand Down
1 change: 1 addition & 0 deletions src/SqlHydra/SqlHydra.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

<ItemGroup>
<Compile Include="Domain.fs" />
<Compile Include="DbColumnTypeAttribute.fs" />
<Compile Include="SchemaGenerator.fs" />
<Compile Include="TomlConfigParser.fs" />
<Compile Include="Fsproj.fs" />
Expand Down