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

Error creating Null string #339

Closed
rockResolve opened this issue Jan 13, 2020 · 16 comments
Closed

Error creating Null string #339

rockResolve opened this issue Jan 13, 2020 · 16 comments
Assignees
Labels

Comments

@rockResolve
Copy link
Contributor

When using "select string(null)" I get the "Ambiguous invocation of 'String' constructor" error.
I was able to use this select in the old kahanu/System.Linq.Dynamic.

I need the String cast because I Concat this with a query with another and the Concat insists on matching types.

How can I get a Select to return a null string from a literal null?

@StefH
Copy link
Collaborator

StefH commented Jan 13, 2020

See also this one:
#284
Doe this solve your issue?

@rockResolve
Copy link
Contributor Author

No. Constants.IsNull sees the value as "null" and returns false

@rockResolve
Copy link
Contributor Author

When I use select string(null) in the old kahanu/System.Linq.Dynamic it works.

Diagnosis: The difference is in MethodFinder.cs IsBetterThan.

The string constructor has 8 methods, FindBestMethod finds 3 methods (Char*, SByte* & Char[]) initially applicable.
kahanu IsBetterThan returns false when CompareConversion finds the parms are equal
So FindBestMethod returns final applicable.Length = 0 and the calling ParseTypeAccess falls back to GenerateConversion which succeeds.

However System.Linq.Dynamic.Core IsBetterThan returns true when the parms are equal.
So FindBestMethod returns final applicable.Length = 3 and the calling ParseTypeAccess throws the exception.

Not sure how you would want to fix this.

@StefH StefH added the bug label Jan 14, 2020
@StefH StefH self-assigned this Jan 14, 2020
@StefH
Copy link
Collaborator

StefH commented Jan 14, 2020

@rockResolve
Can you try MyGet version System.Linq.Dynamic.Core.1.0.20-ci-12452.nupkg ?

@StefH
Copy link
Collaborator

StefH commented Jan 14, 2020

Actually using code like string(null) should not call the new operator, it's just a cast.
Also when casting to nullable int, you would use : int?(5).

@rockResolve
Copy link
Contributor Author

Thanks for the quick reply. The package didn't quite work for me.
I am using the results with a concat, so need string(null), not just null. 

So I downloaded the branch to look closer. After I removed the "If only 1 argument, and the arg is ConstantExpression, just return the ConstantExpression" code, your pointer constructor removal worked well and built my string(null).
Now all my tests pass, which include testing nullable bools, ints, dates.

Are you able to remove the "If only 1 argument, and the arg is ConstantExpression, just return the ConstantExpression" code or is it there for another reason?

@StefH
Copy link
Collaborator

StefH commented Jan 15, 2020

When you remove that line, the dynamic code behaves like this normal c# code:
new string((char[]) null); and will create an empty string, which differs from a null string!

If you need concat, can't you just use string(\"\") ?

@rockResolve
Copy link
Contributor Author

rockResolve commented Jan 16, 2020

Oops your right my test was falsely passing with an empty string.

However I still do really need a string(null).
I am using linq.concat (like tsql union) to join several sets, but concat insists on matching types.
I am using string(null) to ensure this set is sorted first/last

@StefH
Copy link
Collaborator

StefH commented Jan 16, 2020

Can you provide an simple console-app?

@rockResolve
Copy link
Contributor Author

Finally got a simple console-app made.
I had to use z.entityframework to get my group by working correctly in core.
What is the best way to share - a PR in this repo?

@StefH
Copy link
Collaborator

StefH commented Jan 30, 2020

Just create a new project on github.

If needed I can copy it and debug in my solution.

@rockResolve
Copy link
Contributor Author

Added console-app at https://github.com/rockResolve/dlinq-demo-string-null.
Currently using System.Linq.Dynamic.Core 1.0.20-ci-12519, so string(null) raises pointer constructor error.

@rockResolve
Copy link
Contributor Author

Also added possible System.Linq.Dynamic.Core fix with added/updated tests at:
https://github.com/rockResolve/System.Linq.Dynamic.Core/tree/Better_Error-null-string

@StefH
Copy link
Collaborator

StefH commented Feb 24, 2020

Can you create a PR, I'll have to check your changes.

@rockResolve
Copy link
Contributor Author

PR #354
I based it on the latest master, to pick up your latest changes

@StefH
Copy link
Collaborator

StefH commented Mar 18, 2020

PR is merged ; closing this issue.

@StefH StefH closed this as completed Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants