[TRAFODION-2791] 'Not casespecific' column comparison returns wrong #1307
Conversation
…esults. now like and t.c1=t.c2 not support
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
jenkins, ok to test |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2193/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2193/ |
| enum CharInfo::Collation collation=CharInfo::DefaultCollation, | ||
| enum CharInfo::Coercibility coercibility=CharInfo::COERCIBLE, | ||
| NAMemory * outHeap = CmpCommon::statementHeap() | ||
| ); |
There was a problem hiding this comment.
Instead of adding another constructor, I would suggest to add an optional argument isCaseInSensitive to the previous constructor.
There was a problem hiding this comment.
first, I think isCaseInSensitive is a import attribute,
second, i don't think add this argument to the end of constructor is a good way.
if add this argument in the second position, it will change some code where call it.
so i add a new constructor.
There was a problem hiding this comment.
Yes, you need to add it as the last argument of the existing constructors.
| @@ -359,6 +367,16 @@ class ConstValue : public ItemExpr | |||
| enum CharInfo::CharSet strLitPrefixCharSet=CharInfo::UnknownCharSet | |||
| ); | |||
|
|
|||
| ConstValue(const NAWString& strval, | |||
| @@ -503,7 +503,13 @@ ConstValue* OptRangeSpec::getConstOperand(ItemExpr* predExpr, Lng32 constInx) | |||
| // currently support. Predicates involving types not yet supported will be | |||
| // treated as residual predicates. | |||
| if (QRDescGenerator::typeSupported(static_cast<ConstValue*>(right)->getType())) | |||
| { | |||
There was a problem hiding this comment.
This adds a case-insensitive flag to the type in the RangeSpec, but I don't think RangeSpecs are written to handle case-insensitive comparisons. Take a look at the methods that deal with comparisons when building RangeSpecs, in file Range.cpp. So, I think you would have to do one of two things: a) disable the RangeSpec transformation for case-insensitive comparison operators (the easy way) or b) change the RangeSpec methods to handle case-insensitive comparisons.
There was a problem hiding this comment.
I do those operators in order to add case-insensitive to new constvalue.
i don't understand the a
for b, case-insensitive only use in
if (parentOfStart)
parentOfStart->child(1) =
new(mvqrHeap_) ConstValue(charSubrange->start,
if add a RangeSpec methods about case-insensitive , we need add this attribute to parentOfStart->child(1) after operating above.
so i don't think this is a good way
There was a problem hiding this comment.
RangeSpecs keep a list of points and intervals and these intervals need to be ordered, otherwise RangeSpec doesn't work. If we have case-insensitive comparison, that ordering needs to be different, but as far as I could tell, there is no code at the moment that would do a case-insensitive ordering of the values.
|
-1 |
|
Hi again, here is a suggestion: Change method OptRangeSpec::buildRange() and add a check whether a comparison operation (ITM_EQUAL, ITM_LESS, etc.) involves a case-inspecific comparison. If so, then set the local variable isRange to FALSE. This should avoid getting into this problem, as there will be no more RangeSpecs for case-inspecific comparisons. |
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2289/ |
|
Can one of the admins verify this patch? |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2289/ |
|
core/TEST032 and core/TEST038 are failing with compiler assertion errors. Those need to be fixed before we can merge this change. |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2890/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2890/ |
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
'Not casespecific' column comparison returns wrongresult, now like and t.c1=t.c2 not support