Bug #7604
Switch does not honor case sensitivity of character
100%
History
#1 Updated by Eduard Soltan 12 months ago
case substring("IKI", 1, 2, "character":U): when 'ik':U then message "Hello1". when 'vk':U then message "Hello2". when 'ih':U then message "Hello3". when 'vh':U then message "Hello4". end case.
Code in OE get converted in;
switch (StringHelper.safeTrimTrailing((substring("IKI", 1, 2, "character")).toStringMessage())) { case "ik": { message("Hello1"); } break; case "vk": { message("Hello2"); } break; case "ih": { message("Hello3"); } break; case "vh": { message("Hello4"); } break; }
The following code in OE displays Hello1
on the screen, but in FWD it does not enter any switch cases.
#2 Updated by Eduard Soltan 12 months ago
Tested switch
with following cases:
- character variable case-insensitive
- character variable case-sensitive
- entry function on character variable case-insensitive
- entry function on character variable case-sensitive
- trim function on character variable case-insensitive
- trim function on character variable case-sensitive
- substring function on character variable case-sensitive
In all cases fwd
works as expected. The only problem appears at using substring function with character variable with case-insensitive.
#4 Updated by Alexandru Lungu 12 months ago
- Status changed from New to WIP
- Assignee set to Eduard Soltan
#5 Updated by Eduard Soltan 12 months ago
- File 7604_switch_substring.diff added
case FUNC_CHAR: case PLUS: for (int k = 0; k < ast.getNumImmediateChildren(); k++) { if (!isCaseInsensitive(ast.getChildAt(k))) { // if a child is found to be case-sensitive then the result is case-sensitive return false; } }
offset
and length
paramentes from substring
function, isCaseInsensitive
method for ExpressionConversionWorker.java
class returns false, because they are of integer type.
#6 Updated by Alexandru Lungu 12 months ago
- % Done changed from 0 to 50
offset and length paramentes from substring function, isCaseInsensitive method for ExpressionConversionWorker.java class returns false, because they are of integer type.
This is quite "cryptic" :) Eduard, please make some second/third rechecks before Redmine posts. Also, please explain the relevance of the snippet in regard to the changes. As far as I could "decrypt": SUBSTRING
has 3 parameters, from which the second (offset
) and the third (length
) are integer. Therefore, the for-loop in the snippet is returning false
for sub-string, because integer data-type causes !isCaseInsensitive(ast.getChildAt(k))
to evaluate to true, so the switch is case-sensitive. This is not right for SUBSTRING
, as the case-sensitivity depends only on the first character parameter.
Is the diff
working for your case? It is quite weird that you use SUBSTRING
, but made changes to an IF with ENTRY
. I expected to more like:
+ if (ast.getText().equalsIgnoreCase(/*"ENTRY"*/ "SUBSTRING") && isCaseInsensitive(ast.getChildAt(0)))
+ {
+ return true;
+ }
Also, can you make a special case for "SUBSTRING" entirely:
+ if (ast.getText().equalsIgnoreCase("SUBSTRING"))
+ {
+ return isCaseInsensitive(ast.getChildAt(0));
+ }
Is this correct? Please make some tests similar to #7604-2 and ensure nothing is broken + the issue is fixed.
#7 Updated by Eduard Soltan 12 months ago
- File 7604_substring_parameter_check.diff added
Alexandru Lungu wrote:
This is quite "cryptic" :) Eduard, please make some second/third rechecks before Redmine posts. Also, please explain the relevance of the snippet in regard to the changes. As far as I could "decrypt":
SUBSTRING
has 3 parameters, from which the second (offset
) and the third (length
) are integer. Therefore, the for-loop in the snippet is returningfalse
for sub-string, because integer data-type causes!isCaseInsensitive(ast.getChildAt(k))
to evaluate to true, so the switch is case-sensitive. This is not right forSUBSTRING
, as the case-sensitivity depends only on the first character parameter.Is the
diff
working for your case? It is quite weird that you useSUBSTRING
, but made changes to an IF withENTRY
. I expected to more like:+ if (ast.getText().equalsIgnoreCase(/*"ENTRY"*/ "SUBSTRING") && isCaseInsensitive(ast.getChildAt(0))) + { + return true; + }
I am so sorry, I was in a rush when I wrote the previous post. Yeah you got exactly right, substring
function in 4gl has 4 parameters (source
, position
, length
and type
). source
and type
are of type character, position
and length
are of type integer. When for
snippet from #7604-5 is being executed, !isCaseInsensitive(ast.getChildAt(k))
for position
and length
return true
, causing isCaseInsensitive(substring(source, position, length, type))
to return false
, for substring
expression.
Also, I had some small irrelevant changes on the trunk, this is why I had to bzr revert
to create a patch, and on rewriting changes I had to changes in ExpressionConvertorWorker.java
, I made a silly mistake by typing entry
instead of substring
.
Finally tested with the change on all cases from #7604-2, works as expected. Also works with substring
function on case-insensitive.
#11 Updated by Eduard Soltan 12 months ago
Committed on 7464a revision 14664.
#12 Updated by Alexandru Lungu 11 months ago
- Status changed from WIP to Review
- % Done changed from 50 to 100
Merged 7464a in trunk as rev. 14673. Archived 7464a.
#14 Updated by Alexandru Lungu 10 months ago
Eduard, now that the change reached customer applications (newer binaries), please check that conversion is OK, so we can close this.
#15 Updated by Eduard Soltan 10 months ago
Yes, conversion seems good.
#16 Updated by Alexandru Lungu 10 months ago
Greg, we can close this.