nreco / lambdaparser

Runtime parser for string expressions (formulas, method calls). Builds dynamic LINQ expression tree and compiles it to lambda delegate.
http://www.nrecosite.com/
MIT License
307 stars 55 forks source link

Fixed a issue when comparing null values with <, <=, >, >= operators #9

Closed OPunktSchmidt closed 7 years ago

OPunktSchmidt commented 7 years ago

This operators should return false if one or both operands are null

Example:

The previous version would return 'true' for expression: (new System.TimeSpan(15,0,0) > null) Now it returns false. That is now the exactly result which original c# code would return.

VitaliyMF commented 7 years ago

Topic about comparison with nulls can be rather discussive - especially in case what should be returned if both operands are null. Currently if variable "a"=null expression "a==null" returns true and this is how comparison works in C#.

I can explain why "new System.TimeSpan(15,0,0) > null" returns true; this happens because ValueComparer treats "null" as "MinValue" for ">" and "<" comparisons.

I think that there are no one "only-true" answer how comparison with null should work for ALL possible usage scenarios and currently NReco.LambdaParser has comparison logic "as is" and it should be preserved for backward compatibility.

This means that I cannot accept your pull request; instead of that I've created new issue #10 that should solves this problem in a more generic way.

OPunktSchmidt commented 7 years ago

Yes your are absoluty right. This problem should be solved in a more generic way. Thanks for creating an issue. Until then I will continue using my fork :)

VitaliyMF commented 7 years ago

@OPunktSchmidt Oliver I've just pushed issue #10 changes. Now you cna force SQL-like comparison for nulls in the following way:

var lambdaParser = new LambdaParser(
  new ValueComparer() { NullComparison = ValueComparer.NullComparisonMode.Sql }); 

Could you build NReco.LambdaParser from 'master' and try to use it in your project? If everything is ok just let me know and I'll publish it on nuget.

OPunktSchmidt commented 7 years ago

It seems to work everything I need. Looking forward for a nuget package. Thank you.

VitaliyMF commented 7 years ago

Version 1.0.6 published on nuget