Fails to insert jsonb values in Postgres

[MargaretKrutikova]

We have a table with a field of type jsonb, which becomes string in the generated type, but it doesn't seem to work when inserting a string with the json format, the exception I get is:

Npgsql.PostgresException (0x80004005): 42804: column "test" is of type jsonb but expression is of type text
Exception data:
    Severity: ERROR
    SqlState: 42804
    MessageText: column "test" is of type jsonb but expression is of type text
    Hint: You will need to rewrite or cast the expression.
    Position: 97
    File: parse_target.c
    Line: 588
    Routine: transformAssignedExpr

when trying to insert """{"publisher": "test"}""". Would you be able to help with this?

[Mousaka]

Looks like it's not actually implemented to me. There is a "jsonb" but it looks like it's treating it like a string? -> https://github.com/JordanMarr/SqlHydra/blob/main/src/SqlHydra.Npgsql/NpgsqlDataTypes.fs#L24

This implementation looks like it's depending on System.Data.DbType which do not have any DbType.json orDbType.jsonb`. This might be related https://github.com/dotnet/runtime/issues/30677

[JordanMarr]

What should happen during the insert for the jsonb field? Does the string need to be transformed before being inserted?

[MargaretKrutikova]

No, I think the exception comes from the native Npgsql driver which needs NpgsqlDbType.Jsonb for jsonb fields, but it is defined as DbType.String in https://github.com/JordanMarr/SqlHydra/blob/main/src/SqlHydra.Npgsql/NpgsqlDataTypes.fs#L24. Is it possible to have type mappings specific for postgres? Then we can use NpgsqlDbType.Jsonb from Npgsql directily, https://github.com/npgsql/npgsql/blob/661a54ff6cf4fa030705a3b0c0e82b01ba51bb6b/src/Npgsql/TypeMapping/GlobalTypeMapper.cs#L326.

[JordanMarr]

The DbType mapping is only used for inspection purposes when creating the generated code, so that won't make a difference here.

[MargaretKrutikova]

How is DbType.String used then? You would only need string for the generated code, right?

[JordanMarr]

There seem to be a few related issues on the SqlKata repo: https://github.com/sqlkata/querybuilder/issues/356

Maybe @toburger has some insights into a workaround?

[MargaretKrutikova]

Oh right, it might be a problem in SqlKata since it is what actually performs the insert-commands.

[JordanMarr]

Right -- I don't actually send the DbType.String to the insert.

[JordanMarr]

However, I do build the command, so perhaps there is a fix that can be implemented at this point.

[MargaretKrutikova]

Yeah, if I use that code for building the command and set the correct type of the parameter manually, like this

 p.NpgsqlDbType <- NpgsqlDbType.Jsonb

it works. Do you think it is possible to make it work in a generic case? Maybe allow having specific type mappings for different database drivers and set the type depending on the driver?

[JordanMarr]

There are two ways I can think of to do this.

Option 1 would involve creating many provider specific versions of SqlHydra.Query. I would prefer to avoid this option.

Option 2 would involve using reflection to set provider specific type mapping enum:

• Generated table record properties could be decorated with a metadata attribute that preserves their mapped type as a string: Ex: `[<Parameter("NpgsqlDbType.Jsonb")>].
• BuildCommand would use some reflection to check for existence of attribute, and if found, use reflection to assign parameter type.

[MargaretKrutikova]

The second option sounds great, we could map certain postgres column types to their corresponding NpgsqlDbType where it is needed, like jsonb and json and generated the attributes.

How do you run the tests? I have started the dev container and tried following the "Contributing"-section of the readme, To initialize SQL Server after the mssql container spins up, open the CLI and run bash install.sh which failed for both mssql and postgres with a bunch of No such file or directory. What's the proper local setup?

[JordanMarr]

You should only have to run the install.sh script for SQL Server:

[MargaretKrutikova]

How do I run the tests for postgres? Specifically Tests/Npgsql.

[JordanMarr]

The most simple way if you are using vscode is to dotnet run the Tests project which will run all tests. If you want to only run the postgres tests then you will need to change the [<Tests>] attribute to [<FTests>] for the files that you want to focus. You can also change test to ftest if you to target specific tests.

(I personally use VS2019 which allows me to specify test(s) via Test Explorer.)

[MargaretKrutikova]

I started looking into this issue, and while I am able to generate the attribute as you suggested, I am not sure I understand how to access this value inside BuildCommand and parse it into the actual value e.g. NpgsqlDbType.Jsonb since we are using string inside the attribute. Can we have some kind of DU as the attribute value? Like

type DbColumnType =
    | Npgsql of NpgsqlDbType
    | Mssql of ....

Although this would have to be inside Domain.fs as part of the Column, and I assume we don't want to introduce any dependencies on the database type inside the domain.

[JordanMarr]

The technique would be the same one used by SQLProvider to do load provider-specific objects via reflection.

• first they create a new parameter object. (We already have one, so can probably skip this step)
• But we would need the dbTypeSetter to set the NpgsqlDbType.
• it will probably be necessary to reflect the NpgsqlDbType type as well to parse the enum string before setting it

[MargaretKrutikova]

Ok, I got the idea, and all the reflection business seems to work. The problem I am having now is how to match the field attributes to the corresponding parameter in compiledQuery.NamedBindings inside BuildCommand, because all the information about the record fields inside Query is lost and all the parameters are called @p0 etc. Do you have any on how to do it?

[JordanMarr]

The plot thickens!

It seems that we may need to check for the existence of the column metadata attribute in the InsertExpressionBuilder and UpdateExpressionBuilder when record properties are being added.

The nice thing is that the LinqExpressionVisitors.visitPropertySelector returns MemberInfo that should give us the attribute metadata for free. So if any metadata exists, it can be added to the InsertQuerySpec or UpdateQuerySpec in the order at which the columns were added.

Then the QueryContext insert and update methods should have enough info to create a Map of parameter type info to the BuildCommand methods.

[MargaretKrutikova]

I am not sure we can rely on the order of the parameters since NamedBindings in the compiled query is of type Dictionary where the order is not guaranteed, right? It would be great if instead of @p0 we could get the actual column names from SqlKata, but it doesn't seem like it is going to be implemented, though the issue for named parameters exists and is closed.

[JordanMarr]

Good point. It does not look like the SqlKata maintainer is going to implement the change.

But I think I have a workaround:

• Create a new record type in Kata.fs that we can use as a trojan horse to carry the parameter type payload:

  type QueryParameter = 
  {
      Value: obj
      Type: string // Ex: "NpgsqlDbType.jsonb"
  }

• In KataUtils fromUpdate and fromInsert, right before calling .AsUpdate and .AsInsert, transform each value value to a QueryParameter, and then box it back to an obj.
• Finally in BuildCommand, dismantle the ParameterValue to get the value and the parameter type to build up the params.

Getting the attributes in the builders as I suggested earlier probably isn't possible since entities are sometimes passed instead of individual values. This means that it would need to be done in BuildCommand (and preferably cached, although caching can be added after the fact).

This sounds like it has become a fairly involved change, so I'd be glad to help with any of this if you want to submit a pull request with what you have. Of course if you are enjoying the process, then by all means please keep slogging through!

[MargaretKrutikova]

Oh, that's genius 😄 I almost made it work with this new suggestion ... will need to clean up the mess and open a PR in the beginning of the week. Yeah, I am enjoying the process, thanks for the help!