Skip to content
This repository was archived by the owner on Jun 7, 2021. It is now read-only.

[TRAFODION-2791] 'Not casespecific' column comparison returns wrong #1307

Open
duoluodexiaokeke wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[TRAFODION-2791] 'Not casespecific' column comparison returns wrong #1307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions core/sql/optimizer/ItemColRef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,14 @@ class ConstValue : public ItemExpr
NAMemory * outHeap = CmpCommon::statementHeap()
);

ConstValue(const NAString& strval,
NABoolean isCaseInSensitive,
enum CharInfo::CharSet charSet=CharInfo::DefaultCharSet,
enum CharInfo::Collation collation=CharInfo::DefaultCollation,
enum CharInfo::Coercibility coercibility=CharInfo::COERCIBLE,
NAMemory * outHeap = CmpCommon::statementHeap()
);
Copy link
Contributor

@zellerh zellerh Nov 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding another constructor, I would suggest to add an optional argument isCaseInSensitive to the previous constructor.

Copy link
Author

@duoluodexiaokeke duoluodexiaokeke Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@zellerh zellerh Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you need to add it as the last argument of the existing constructors.


// constructor for a wide (unicode) string constant
ConstValue(const NAWString& strval,
enum CharInfo::CharSet charSet=CharInfo::UNICODE,
Expand All @@ -359,6 +367,16 @@ class ConstValue : public ItemExpr
enum CharInfo::CharSet strLitPrefixCharSet=CharInfo::UnknownCharSet
);

ConstValue(const NAWString& strval,
Copy link
Contributor

@zellerh zellerh Nov 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here.

NABoolean isCaseInSensitive,
enum CharInfo::CharSet charSet=CharInfo::UNICODE,
enum CharInfo::Collation collation=CharInfo::DefaultCollation,
enum CharInfo::Coercibility coercibility=CharInfo::COERCIBLE,
NAMemory * outHeap = CmpCommon::statementHeap(),
enum CharInfo::CharSet strLitPrefixCharSet=CharInfo::UnknownCharSet
);


// constructor for a string constant with unknown charset (both the
// single-byte and double-byte string values are known)
ConstValue(
Expand Down
40 changes: 40 additions & 0 deletions core/sql/optimizer/ItemExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9971,6 +9971,22 @@ ConstValue::ConstValue(const NAString & strval,
((char*)strval.data(), strval.length(), outHeap);
}

ConstValue::ConstValue(const NAString & strval,
NABoolean isCaseInSensitive,
enum CharInfo::CharSet charSet,
enum CharInfo::Collation collation,
enum CharInfo::Coercibility coercibility,
NAMemory * outHeap)
: ItemExpr(ITM_CONSTANT), isNull_(IS_NOT_NULL),
textIsValidatedSQLLiteralInUTF8_(FALSE), isStrLitWithCharSetPrefix_(FALSE),
isSystemSupplied_(FALSE), locale_wstrval(0), rebindNeeded_(FALSE)
{
initCharConstValue(strval, charSet, collation, coercibility, isCaseInSensitive, outHeap);
locale_strval = new (outHeap) NAString
((char*)strval.data(), strval.length(), outHeap);
}


void ConstValue::initCharConstValue
(
const NAString & strval,
Expand Down Expand Up @@ -10070,6 +10086,30 @@ ConstValue::ConstValue(const NAWString& wstrval,
(wstrval.data(), wstrval.length(), CmpCommon::statementHeap());
}

ConstValue::ConstValue(const NAWString& wstrval,
NABoolean isCaseInSensitive,
enum CharInfo::CharSet charSet,
enum CharInfo::Collation collation,
enum CharInfo::Coercibility coercibility,
NAMemory * outHeap,
enum CharInfo::CharSet strLitPrefixCharSet
)
: ItemExpr(ITM_CONSTANT), isNull_(IS_NOT_NULL),
isSystemSupplied_(FALSE),
value_(0),
text_(0),
locale_strval(0),
locale_wstrval(0),
isStrLitWithCharSetPrefix_(FALSE),
rebindNeeded_(FALSE)
{
initCharConstValue(wstrval, charSet, collation, coercibility, isCaseInSensitive, outHeap,
strLitPrefixCharSet);

locale_wstrval = new (CmpCommon::statementHeap()) NAWString
(wstrval.data(), wstrval.length(), CmpCommon::statementHeap());
}

void ConstValue::initCharConstValue(const NAWString& strval,
enum CharInfo::CharSet charSet,
enum CharInfo::Collation collation,
Expand Down
22 changes: 18 additions & 4 deletions core/sql/optimizer/OptRange.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
{
Copy link
Contributor

@zellerh zellerh Nov 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@duoluodexiaokeke duoluodexiaokeke Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@zellerh zellerh Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

/* add constvalue ‘not casespecific’ to type_*/
((CharType*)getType())->setCaseinsensitive(
((CharType *)(((ConstValue*)(right))->getType()))->isCaseinsensitive());

return static_cast<ConstValue*>(right);
}
else
return NULL;
} // getConstOperand()
Expand Down Expand Up @@ -1712,10 +1718,13 @@ ItemExpr* OptRangeSpec::makeSubrangeItemExpr(SubrangeBase* subrange,
(Subrange<RangeWString>*)subrange;
if (parentOfStart)
parentOfStart->child(1) =
new(mvqrHeap_) ConstValue(wcharSubrange->start);
new(mvqrHeap_) ConstValue(wcharSubrange->start,
((CharType *)type)->isCaseinsensitive());

if (parentOfEnd)
parentOfEnd->child(1) =
new(mvqrHeap_) ConstValue(wcharSubrange->end);
new(mvqrHeap_) ConstValue(wcharSubrange->end,
((CharType *)type)->isCaseinsensitive());
}
else
{
Expand All @@ -1724,10 +1733,15 @@ ItemExpr* OptRangeSpec::makeSubrangeItemExpr(SubrangeBase* subrange,
= (Subrange<RangeString>*)subrange;
if (parentOfStart)
parentOfStart->child(1) =
new(mvqrHeap_) ConstValue(charSubrange->start, ((CharType *)type)->getCharSet() );
new(mvqrHeap_) ConstValue(charSubrange->start,
((CharType *)type)->isCaseinsensitive(),
((CharType *)type)->getCharSet() );

if (parentOfEnd)
parentOfEnd->child(1) =
new(mvqrHeap_) ConstValue(charSubrange->end, ((CharType *)type)->getCharSet() );
new(mvqrHeap_) ConstValue(charSubrange->end,
((CharType *)type)->isCaseinsensitive(),
((CharType *)type)->getCharSet() );
}
}
break;
Expand Down