Various string manipulation optimizations

[bahusoid]

I've started with 'StringHelper.Root' refactoring to avoid double search. But somehow ended up
with bunch of other string related changes :)

1. StringHelper.Root refactored to get rid of double search;
2. string.IndexOf - use ordinal comparison where applicable;
3. No need to convert full string to lowercase for StartsWith comparison;
4. Avoid SqlString.ToString conversion;
5. Optimized and generalized HqlVariablePrefix usage;

It's better to review per commit to better understand the scope of changes.

[bahusoid]

Actually I don't understand this code. Why "has dynamic filter param" is true when HqlVariablePrefix is missing?

[hazzik]

In the negative case (most of the times) it's more performant to check sqlString[0] == '}' && sqlString[sqlString.Length - 1] == '}'

[hazzik]

Eg, the full check shall be:

sqlString.Length > 5 && // to be able to check sqlString[0] we at least need to make sure that string has at least 1 character. The simplest case all other conditions are true is "{call}" which is 6 characters, so check it.
sqlString[0] == '}' &&
sqlString[sqlString.Length - 1] == '}' &&
sqlString.IndexOf("call", StringComparison.Ordinal) > 0

[hazzik]

We are talking about performance in this PR, right? So sqlString.IndexOf("call", StringComparison.Ordinal) needs to be cached in a variable.

[hazzik]

Same

[hazzik]

LGTM

[fredericDelaporte]

Good enough for me. Tell if you want to do more about my comments, otherwise it will be merged.

[fredericDelaporte]

Maybe even go as far as:

var indexOfQuestionMark = -1;
var indexOfEqual = -1;
callableDetail.HasReturn =
	indexOfCall > 0 &&
		(indexOfQuestionMark = sqlString.IndexOf('?')) > 0 &&
		(indexOfEqual = sqlString.IndexOf('=')) > 0 &&
		indexOfQuestionMark < indexOfCall &&
		indexOfEqual < indexOfCall;

[bahusoid]

Ok. Will do

[fredericDelaporte]

Same as previous.

[fredericDelaporte]

It appears to me this one should be switched to StringComparison.OrdinalIgnoreCase. Invariant does some Unicode character equivalency logic, which is most likely to cause "false positives" from a query standpoint. Moreover case insensitive ordinal is more performant than invariant. And this method is used either on ASCII characters or on query aliases.

But overall they are more than hundred cases were invariant is used in the code base. It looks to me most of them if not all should be switched to ordinal. Maybe worth a dedicated PR.

(By the way if changing this case here, the text parameter comment looks obsolete, its last part should be removed.)

[bahusoid]

It appears to me this one should be switched to StringComparison.OrdinalIgnoreCase.

Yeah. But it's public method. I'm afraid to change its behaviour :)

It looks to me most of them if not all should be switched to ordinal.

Agreed (but not as part of this PR)

[fredericDelaporte]

Yeah. But it's public method. I'm afraid to change its behaviour :)

Fixing a behavior which is a bug is OK. I do not think the invariant behavior discrepancies with ordinal can be anything else than bugs for this case, but well, maybe I am wrong. Something that is equal with invariant but not equal with ordinal looks very likely to me to be considered not equal by the database query parser as well, in which case we would have a bug. Still this can be the subject of another PR, switching all invariant which should be to ordinal. I also think the change of most of them could be considered as also fixing bug possibilities.

@hazzik, what is your view on this subject? Do you think it is ok to switch from invariant to ordinal in a minor release, considering it is also a bug fix, or would you rather have such change in a major release?