JordanMarr / SqlHydra

SqlHydra is a suite of NuGet packages for working with databases in F# including code generation tools and query expressions.
MIT License
212 stars 21 forks source link

Add provider specific column types #10

Closed MargaretKrutikova closed 2 years ago

MargaretKrutikova commented 2 years ago

This PR attempts to fix #7 by adding a new attribute for fields that require db provider-specific type to be set on the corresponding command parameter. E.g. postgres requires NpgsqlDbType.Json and NpgsqlDbType.Jsonb to be set on the command parameter for columns of types json and jsonb correspondingly when using Npgsql data provider.

I am planning to add tests for the functionality after the initial review, and when I figure out how to add a new json column to the existing database used in tests 😄

MargaretKrutikova commented 2 years ago

I would like to add a field of type json and jsonb to the postgres test database so that I can test inserts and updates for those fields in QueryIntegrationTests, what is the right way to do it?

JordanMarr commented 2 years ago

I would like to add a field of type json and jsonb to the postgres test database so that I can test inserts and updates for those fields in QueryIntegrationTests, what is the right way to do it?

You can just add a test that drops and then creates a new table as is done in Dapper.FSharp.

MargaretKrutikova commented 2 years ago

I have added some tests but stumbled into a couple of problems:

  1. It doesn't seem like a good solution to generate a table inside a test since other tests rely on it. I need to regenerate AdventureWorks to be able to test the functionality I added so the new table has to permanently stay in the database and other test rely on the fact that there are certain number of tables present. See comment.
  2. I can't run any mssql integration tests with the error
    A network-related or instance-specific error occurred while establishing a connection to SQL Server. 
    The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is 
    configured to allow remote connections. (provider: TCP Provider, error: 35 - An internal exception was caught)

    even though it is running in docker. Could it be smth OS-specific? I am running mac, but I assume you are developing on windows (from the bat scripts and windows-like paths)?

JordanMarr commented 2 years ago

I have added some tests but stumbled into a couple of problems:

  1. It doesn't seem like a good solution to generate a table inside a test since other tests rely on it. I need to regenerate AdventureWorks to be able to test the functionality I added so the new table has to permanently stay in the database and other test rely on the fact that there are certain number of tables present. See comment.

See response above.

  1. I can't run any mssql integration tests with the error
 A network-related or instance-specific error occurred while establishing a connection to SQL Server. 
The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is 
configured to allow remote connections. (provider: TCP Provider, error: 35 - An internal exception was caught)

even though it is running in docker. Could it be smth OS-specific? I am running mac, but I assume you are developing on windows (from the bat scripts and windows-like paths)?

Assuming the mssql instance is running properly, it should run on standard 1433 port if you are running vs code with the remote-containers extension. But if you are manually spinning up from docker-compose then it's on 12019. I created a "DebugLocal" config that switches the test connection strings which changes the connection string to allow me to run tests from my localhost using Visual Studio (since I do not personally use vs-code).

(Maybe I should add this detail to the Contributing section.)

JordanMarr commented 2 years ago

How's it going? Is this ready to merge or still 🚧🚧? No rush, of course; just wanted to make sure you weren't waiting on me.

MargaretKrutikova commented 2 years ago

Was away for a couple of days 🙂 Just ran the tests and all of them passed. Will you be able to review the code one last final time? I saw there was another PR fixing the order of the columns, but I think it is only for SqlServer, and I only touched Npgsql.

JordanMarr commented 2 years ago

For some reason it is not adding the new table in postgres. I think it may have something to do with with the files on the local volume not be removed, so I tried docker-compose down -v to remove them but no luck. Do you have any ideas?

MargaretKrutikova commented 2 years ago

I think I had to remove and re-create postgres service from docker. You can also try and run docker-compose up --build --force-recreate --no-deps -d postgresql to just demolish the service all together and rebuild it 🙂

JordanMarr commented 2 years ago

Fantastic work, Margarita. Thank you! 🎉

MargaretKrutikova commented 2 years ago

Thanks a lot for your help! 😄