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

Bug: null reference during fetch #748

Closed
kbilsted opened this issue Jan 19, 2021 · 21 comments · Fixed by #749
Closed

Bug: null reference during fetch #748

kbilsted opened this issue Jan 19, 2021 · 21 comments · Fixed by #749
Assignees
Labels
bug Something isn't working working-as-expected The reported issue is working as expected.

Comments

@kbilsted
Copy link

Bug Description

When fetching I get the exception

I cannot reproduce in a small demo application. I need a few hints on what field e.g. it is failing on.

Exception Message:

System.NullReferenceException : Object reference not set to an instance of an object.
   at RepoDb.Reflection.Compiler.<>c__DisplayClass62_0`1.<GetClassPropertyParameterInfos>b__1(ParameterInfo parameterInfo)
   at System.Collections.Generic.List`1.ForEach(Action`1 action)
   at RepoDb.Reflection.Compiler.GetClassPropertyParameterInfos[TResult](IEnumerable`1 readerFieldsName, IDbSetting dbSetting)
   at RepoDb.Reflection.Compiler.GetMemberBindingsForDataEntity[TResult](ParameterExpression readerParameterExpression, IEnumerable`1 readerFields, IDbSetting dbSetting)
   at RepoDb.Reflection.Compiler.CompileDataReaderToDataEntity[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.Compiler.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.FunctionFactory.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.FunctionCache.DataReaderToTypeCache`1.Get(DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.FunctionCache.GetDataReaderToTypeCompiledFunction[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.DataReader.ToEnumerable[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)+MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at RepoDb.Extensions.EnumerableExtension.AsList[T](IEnumerable`1 value)
   at RepoDb.DbConnectionExtension.ExecuteQueryInternalForType[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck)
   at RepoDb.DbConnectionExtension.ExecuteQueryInternal[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck)
   at RepoDb.DbConnectionExtension.ExecuteQuery[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache)

Schema and Model:

Please share to us the schema of the table (not actual) that could help us replicate the issue if necessary.

  var sql = @"SELECT *
                        FROM [dbo].[xx]
                        WHERE Id = @id";

            return tx.Connection.ExecuteQuery<YYYY>(sql, new { id }, transaction: tx)
                .FirstOrDefault();
        }

And also the model that corresponds the schema.

Your class model here.

Library Version:

Example: RepoDb v1.12.6 and RepoDb.SqlServer v1.1.2

@kbilsted kbilsted added the bug Something isn't working label Jan 19, 2021
@SimonCropp
Copy link
Contributor

@kbilsted i think this is a potential fix #749 do u have the capacity to verify it locally?

@kbilsted
Copy link
Author

where can i download a DLL to test? https://ci.appveyor.com/project/mikependon/repodb-pqvj7/builds/37333306/artifacts has nothing

@mikependon mikependon pinned this issue Jan 19, 2021
@mikependon
Copy link
Owner

@kbilsted - we will setup a nightly build soon. For now, if you can build it locally, then that would be great. Thanks @SimonCropp for the PR.

@kbilsted
Copy link
Author

I have recompiled and now I get a just a puzzleing error. Do note that this code is working in Dapper, so I don't think it is a column name mismatch

System.InvalidOperationException : Compiler: Failed to initialize the member properties or the constructor arguments.
  ----> System.ArgumentException : Incorrect number of arguments for constructor
   at RepoDb.Reflection.Compiler.CompileDataReaderToDataEntity[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\Compiler\DataReaderToType.cs:line 116
   at RepoDb.Reflection.Compiler.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\Compiler\DataReaderToType.cs:line 30
   at RepoDb.Reflection.FunctionFactory.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\FunctionFactory.cs:line 27
   at RepoDb.FunctionCache.DataReaderToTypeCache`1.Get(DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\FunctionCache.cs:line 81
   at RepoDb.FunctionCache.GetDataReaderToTypeCompiledFunction[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\FunctionCache.cs:line 55
   at RepoDb.Reflection.DataReader.ToEnumerable[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)+MoveNext() in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\DataReader.cs:line 30
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at RepoDb.Extensions.EnumerableExtension.AsList[T](IEnumerable`1 value) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Extensions\EnumerableExtension.cs:line 74
   at RepoDb.DbConnectionExtension.ExecuteQueryInternalForType[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Extensions\DbConnectionExtension.cs:line 597
   at RepoDb.DbConnectionExtension.ExecuteQueryInternal[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Extensions\DbConnectionExtension.cs:line 461
   at RepoDb.DbConnectionExtension.ExecuteQuery[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Extensions\DbConnectionExtension.cs:line 390
   at XxXxXx.xService.Database.V1.Repository.GetById(DbTransaction tx, Guid id) in C:\xxxxcs:line 20
...
--ArgumentException
   at System.Dynamic.Utils.ExpressionUtils.ValidateArgumentCount(MethodBase method, ExpressionType nodeKind, Int32 count, ParameterInfo[] pis)
   at System.Dynamic.Utils.ExpressionUtils.ValidateArgumentTypes(MethodBase method, ExpressionType nodeKind, ReadOnlyCollection`1& arguments, String methodParamName)
   at System.Linq.Expressions.Expression.New(ConstructorInfo constructor, IEnumerable`1 arguments)
   at RepoDb.Reflection.Compiler.CompileDataReaderToDataEntity[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting) in C:\temp\RepoDB-potentialFixFor748\RepoDb.Core\RepoDb\Reflection\Compiler\DataReaderToType.cs:line 107

at least the error reporting should report the type it is operating on

catch (Exception ex)
{
throw new InvalidOperationException($"Compiler: Failed to initialize the member properties or the constructor arguments.", ex);
}
doesn't say much

@mikependon
Copy link
Owner

@kbilsted - I assumed that the null check introduced earlier eliminates the fields necessary for the constructor. It is good if we can revisit it. Would you be able to share the model you are using? Good if you can also share your schema.

@mikependon mikependon reopened this Jan 19, 2021
@kbilsted
Copy link
Author

@kbilsted - I assumed that the null check introduced earlier eliminates the fields necessary for the constructor. It is good if we can revisit it. Would you be able to share the model you are using? Good if you can also share your schema.

I am wondering how such a change could not break any existing test?

@mikependon
Copy link
Owner

Sorry to hear that. We need to investigate if your scenario is really covered. If not, we will guard it as always as we found the issues by writing a new Unit or Integration tests. I assumed, your issue is related to immutability which was recently been supported at v1.12.x. Maybe, our test coverage is doesn't covered much as we expected.

@kbilsted
Copy link
Author

no it is straight forward POCO with string, int, enums and a 1-* relation. On the tables there are triggers perhaps you are parsing the DB schema on startup with the SqlServerBootstrap.Initialize(); ?

@kbilsted
Copy link
Author

How about reporting more type information in the error messages so that we can narrow down the problem?

@mikependon
Copy link
Owner

Can you revisits whether ever argument in your POCO's constructor is corresponding to the fields returned by your query?

@kbilsted
Copy link
Author

Can you revisits whether ever argument in your POCO's constructor is corresponding to the fields returned by your query?

Hi well I'm not sure what type is the problem. You are not reporting the type in the error message '-)

. I have a candidate that has 3 ctor one with 0 arg and two that do not match the SQL query. The class only holds public properties.

Perhaps dapper is always choosing the zero arg ctor since it works fine there

@mikependon
Copy link
Owner

The logic that has been embedded in RepoDB since the start of support to the Immutable Classes is to get the most-argument(ed) ctor and process the resultset of the DbDataReader towards that ctor (Type and Name equality). This means that, you can select more fields from the database, but you need to ensure that all the ctor arguments are present on the resultset fields.

If any of the ctor argument is not present on the resultset's fields, then an exception will be thrown.

@mikependon
Copy link
Owner

@kbilsted
Copy link
Author

Explained on this blog: https://repodb.net/blogs/repodb/2020/09/24/repodb-version-release-fsharp-classhandler/#immutable-classes

Ok changing the code, rewriting the constructors to factory methods resolved the issue

2 suggestions

  • much better error message around this error. This has taken too long to figure out taken the problem into consideration
  • given that the parameters do not match, and the exist a zero-arg ctor, and all properties are mutable - then use the zero-arg ctor rather than fail

@mikependon
Copy link
Owner

That's a good suggestion. I will try to see how to optimize the mapping of the ctor, probably inspecting the properties, or even pass the default values to the non-matched ctor argument (null for nullables).

@mikependon mikependon added wontfix This will not be worked on working-as-expected The reported issue is working as expected. and removed wontfix This will not be worked on labels Jan 21, 2021
@mikependon
Copy link
Owner

We will tag this as working as expected, but we will improve the logic here. We will soon create a dedicated story for this.

@kbilsted
Copy link
Author

It only works as expected with the PR from @SimonCropp , right ? :-)

@mikependon
Copy link
Owner

Yeah, you are correct. Btw, do you still encountered the problem on the latest version given with your refactoring on your model? I thought you optimize the way you did the ctor(s)?

@kbilsted
Copy link
Author

Yeah, you are correct. Btw, do you still encountered the problem on the latest version given with your refactoring on your model? I thought you optimize the way you did the ctor(s)?

I have not properly checked, since my test suite continued a bit further but only to fail on another problem that I just reported.

@mikependon
Copy link
Owner

Thanks. Please do let us know if you encountered it on the latest version. In anyway, the fix of @SimonCropp will still be a part of the next release, so either verify wait for it or just verify it there (and revert if it is not a problem there).

@mikependon mikependon unpinned this issue Jan 23, 2021
@mikependon
Copy link
Owner

Closing this one as I had created a separate ticket for the enhancement, and beside, the issue is fixed on the next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working working-as-expected The reported issue is working as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants