- Notifications
You must be signed in to change notification settings - Fork5
TestRunnerStatement with dynamic Parameter-list#96
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Responsibility to build SQL fragment now also lies in the DynamicParameter
Needed some ugly mocking, probably something to refactor in future
…sionas being equal to anything in comparing version
| if (i1 == null && i2 == null) { | ||
| return 0; | ||
| } else if (i1 == null) { | ||
| return -1; | ||
| returnnullMeansEqual ? 0 :-1; | ||
| } else if (i2 == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Don't we need to checknullMeansEqual here so that comparetoWithNulls is symmetric?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry, I don't get it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shouldn't i2 be treated same way as i1 so that if i2 is null you check nullMeansEqual
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah.
I thought about this a lot yesterday. The thing is - we are not implementing the normal compareTo method, but we implement something which is only used byisGreaterOrEqualTo andisLessOrEqualTo.
For these 2 cases I think the comparison order is important, because the lazier "equal" depends on the base version to have a null.
Example:
Technically, when comparing 3.1.7 with 3.1.7.3085, 3.1.7 is less and not equal. That's how the comparison was before (null being treated as 0 basically).
But that's not what I want when usingisGreaterOrEqualTo - I want this function to be forgiving if I only provide 3.1.7 as base, so any number in the 4th level of the comparison should be treated as being equal.
On the other hand, when I provide a specific base (e.g. 3.1.7.3085), I really expect it to be precise, so I can't treat i2 the same way in that case.
Maybe I should not use parameters here and instead write theisGreaterOrEqualTo function separately so it's more clear we're not implementing a strictcompareTo here.
Uh oh!
There was an error while loading.Please reload this page.
This removes the necessity for several different TestRunnerStatements, adding all functionality into one Statement with a dynamic Parameterlist.
It's now very clear which features are supported by which utPLSQL version.
Also, NULL-parameters are now handled much better, allowing the core to deal with defaults.
Fixes#92