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

Nullables Still Acting Up #22

Closed
say25 opened this issue Jun 20, 2018 · 17 comments
Closed

Nullables Still Acting Up #22

say25 opened this issue Jun 20, 2018 · 17 comments
Assignees
Labels

Comments

@say25
Copy link
Member

say25 commented Jun 20, 2018

Try to run copyHelper.SaveAll(connection, entities);

Where entities is a list of Student Objects with a string Name field and nullable int field for age.

If the first student in the entities list has a null age and the second has an age defined, it freaks out and fails.

@bytefish bytefish self-assigned this Jun 20, 2018
@bytefish bytefish added the bug label Jun 20, 2018
@bytefish
Copy link
Member

bytefish commented Jun 20, 2018

Weird. 🤔 I will look into it, but it will take some time. Would you mind writing a small Unit Test to reproduce the problem?

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

Working on that now

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

Without exposing my code too much (working in an Enterprise project), the test should look like this:

        [Test]
        public void Test_NullableSmallInt_FirstNull()
        {

            var entity0 = new TestEntity()
            {
                SmallInt = null
            };

            var entity1 = new TestEntity()
            {
                SmallInt = Int16.MaxValue
            };

            subject.SaveAll(connection, new[] { entity0, entity1 });

            var result = GetAll();

            // Check if we have the amount of rows:
            Assert.AreEqual(2, result.Count);

            Assert.IsNotNull(result[0][0]);
            Assert.IsNotNull(result[1][0]);

            Assert.AreEqual(Int16.MinValue, (Int16)result[0][0]);
            Assert.AreEqual(Int16.MaxValue, (Int16)result[1][0]);
        }

Unfortunately running the test against my RDS actually fails :(

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

This is obviously an example but should apply to all types, Integer, Numeric, etc.

@bytefish
Copy link
Member

Thanks! I will investigate it. Honestly I think about rolling everything back to Npgsql 3.2.7. 😥

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

Got it working. I needed to simply run create schema sample; first. but yea these all fail. Let me put in a PR for the tests.

@bytefish
Copy link
Member

bytefish commented Jun 20, 2018

Looking at the test it has to fail, because of this line:

 Assert.AreEqual(Int16.MinValue, (Int16)result[0][0]);

The result should be null. I am not near a computer, so I cannot look into it. 😌

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

Fails before it gets to that line unfortunately. But yea I will patch that in the PR.

   at Npgsql.NpgsqlParameter`1.ValidateAndGetLength() in C:\projects\npgsql\src\Npgsql\NpgsqlParameter`.cs:line 90
   at Npgsql.NpgsqlBinaryImporter.Write[T](T value, NpgsqlParameter param) in C:\projects\npgsql\src\Npgsql\NpgsqlBinaryImporter.cs:line 244
   at PostgreSQLCopyHelper.PostgreSQLCopyHelper`1.<>c__DisplayClass12_0`1.<MapNullable>b__0(NpgsqlBinaryImporter writer, TEntity entity) in D:\work\projects\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper.cs:line 61
   at PostgreSQLCopyHelper.PostgreSQLCopyHelper`1.WriteToStream(NpgsqlBinaryImporter writer, IEnumerable`1 entities) in D:\work\projects\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper.cs:line 74
   at PostgreSQLCopyHelper.PostgreSQLCopyHelper`1.SaveAll(NpgsqlConnection connection, IEnumerable`1 entities) in D:\work\projects\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper.cs:line 39
   at PostgreSQLCopyHelper.Test.NullableBasicDataTypesTest.Test_NullableSmallInt_NullFirst() in D:\work\projects\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper\PostgreSQLCopyHelper.Test\NullableBasicDataTypesTest.cs:line 76

Here is the stack FYI

@bytefish
Copy link
Member

Meanwhile, please stay on Version 1.3.0 until there is a fix in place.

@bytefish
Copy link
Member

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

I looked there first. It should basically use the writer.Write(DBNull.Value, type); the first time and then writer.Write(value, type); the second time. Looks like it should work but... :/

@bytefish
Copy link
Member

The Npgsql Binary writer has a WriteNull() method. Could you try if replacing this with my current DBNull.Value writing fixes the issue?

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

That does fix the issue 😎

@bytefish
Copy link
Member

💪💪💪

I will fix it and release a new version to NuGet. Thanks for your help!

@say25
Copy link
Member Author

say25 commented Jun 20, 2018

No problem thanks for your responsiveness :D

bytefish added a commit that referenced this issue Jun 20, 2018
@bytefish
Copy link
Member

I have released a new Version 2.2.0 to NuGet, which should fix the issue. If the problem still persists, please reopen the ticket. And thanks again! 👍

@say25 say25 mentioned this issue Jun 20, 2018
@bytefish
Copy link
Member

Sorry I was too quick to close! I will add your tests later today in a separate merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants