You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by nonstop-qfchen <gi...@git.apache.org> on 2015/10/19 05:47:29 UTC
[GitHub] incubator-trafodion pull request: fix JIRA461
GitHub user nonstop-qfchen opened a pull request:
https://github.com/apache/incubator-trafodion/pull/132
fix JIRA461
This set contains the fix to JIRA461, which addresses the problem as follows.
1. For range partition function only, the the begin B and end E partition is identified using the begin key and end contained in the SearchKey generated for the FileScan object.
2. The number of active partitions is the number of partitions contained within B and E.
3. The work is done for both Trafodion and Native Hbase table, where key comparison is different: binary key comparison for encoded key in a Traf table, and ASCII key comparison for a Native Hbase table.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nonstop-qfchen/incubator-trafodion jira461final
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/132.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #132
----
commit 6b06768a7048c94db822d51284f642ae22e60f88
Author: Qifan Chen <qf...@dev02.trafodion.org>
Date: 2015-10-19T03:39:22Z
fix JIRA461
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42408115
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
+{
+ Int32 cmpLow = memcmp(low, key, keyLen);
+ Int32 cmpHigh = memcmp(key, high, keyLen);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+NABoolean
+compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
+{
+ Int32 cmpLow = strverscmp(low, key);
+ Int32 cmpHigh = strverscmp(key, high);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+
+// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
+// index of the boundary low. Return -1 otherwise, or the key lengths are different.
+Int32 RangePartitionBoundaries::findBeginBoundary(char* encodedKey, Int32 keyLen,
+ compFuncPtrT compFunc) const
+{
+ // boundaries are stored in entries in the range [0, partitionCount_]
+ for (Lng32 i=partitionCount_-1; i>= 0; i--) {
+
+ const char* low = getBinaryBoundaryValue(i);
+ const char* high = getBinaryBoundaryValue(i+1);
+
+ // test if encodedKey is in [low, high)
+ if ( (*compFunc)(low, encodedKey, high, keyLen, i==partitionCount_-1) )
+ return i;
+ }
+
+ return -1;
+}
+
+// find a boundary pair [low, high) with the largest low value in which keys fall, and return the
+// index of the boundary low. Return -1 otherwise, or the key lengths are different.
+Int32 RangePartitionBoundaries::findEndBoundary(char* encodedKey, Int32 keyLen,
+ compFuncPtrT compFunc) const
+{
+ // boundaries are stored in entries in the range [0, partitionCount_]
+ for (Lng32 i=0; i<partitionCount_-1; i++ ) {
+
+ const char* low = getBinaryBoundaryValue(i);
+ const char* high = getBinaryBoundaryValue(i+1);
+
+ // test if encodedKey is in [low, high)
+ if ( (*compFunc)(low, encodedKey, high, keyLen, i==partitionCount_-1) )
+ return i;
+ }
+
+ return -1;
+}
+
+Int32
+RangePartitioningFunction::computeNumOfActivePartitions(SearchKey* skey, const TableDesc* tDesc) const
+{
+ const RangePartitionBoundaries* boundaries = getRangePartitionBoundaries();
+
+ Int32 origPartitions = getCountOfPartitions();
+ Int32 partitions = origPartitions;
+ Int32 bIndex = 0;
+
+ const NATable* naTable = tDesc->getNATable();
+ NABoolean isNativeHbase = (naTable->isHbaseCellTable() || naTable->isHbaseRowTable());
+ compFuncPtrT compFuncPtr = ( isNativeHbase ) ? compareAsciiKey: compareEncodedKey;
--- End diff --
Do we care about Hive tables in this code path?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42538971
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
What's the reason for not calling the method that already does this, NAType::getEncodedKeyLength()?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42519468
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
--- End diff --
Yes, it looks like multiplying by 8 (or 6 after change) is generic. I think
it is safe to multiplying by 6 because the max # of bytes in UTF8 is 6
which can encode code points up to 7FFFFFFF.
On Tue, Oct 20, 2015 at 10:41 AM, Hans Zeller <no...@github.com>
wrote:
> In core/sql/optimizer/ItemExpr.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42512787>
> :
>
> > +ConstValue* ItemExpr::compute(CollHeap* heap)
> > +{
> > + ValueIdList exprs;
> > + exprs.insert(getValueId());
> > +
> > + const NAType& dataType = getValueId().getType();
> > +
> > + Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
> > +
> > + char staticDecodeBuf[200];
> > + Lng32 staticDecodeBufLen = 200;
> > +
> > + char* decodeBuf = staticDecodeBuf;
> > + Lng32 decodeBufLen = staticDecodeBufLen;
> > +
> > + // For character types, multiplying by 8 to deal with conversions between
>
> There is actually a method to compute this length,
> CharInfo::getMaxConvertedLenInBytes() (file common/charinfo.h) but it may
> be inconvenient and overkill in this situation.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42512787>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42408687
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
+{
+ Int32 cmpLow = memcmp(low, key, keyLen);
+ Int32 cmpHigh = memcmp(key, high, keyLen);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+NABoolean
+compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
+{
+ Int32 cmpLow = strverscmp(low, key);
+ Int32 cmpHigh = strverscmp(key, high);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+
+// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
+// index of the boundary low. Return -1 otherwise, or the key lengths are different.
+Int32 RangePartitionBoundaries::findBeginBoundary(char* encodedKey, Int32 keyLen,
+ compFuncPtrT compFunc) const
+{
+ // boundaries are stored in entries in the range [0, partitionCount_]
+ for (Lng32 i=partitionCount_-1; i>= 0; i--) {
+
+ const char* low = getBinaryBoundaryValue(i);
+ const char* high = getBinaryBoundaryValue(i+1);
+
+ // test if encodedKey is in [low, high)
+ if ( (*compFunc)(low, encodedKey, high, keyLen, i==partitionCount_-1) )
+ return i;
+ }
+
+ return -1;
+}
+
+// find a boundary pair [low, high) with the largest low value in which keys fall, and return the
+// index of the boundary low. Return -1 otherwise, or the key lengths are different.
+Int32 RangePartitionBoundaries::findEndBoundary(char* encodedKey, Int32 keyLen,
+ compFuncPtrT compFunc) const
+{
+ // boundaries are stored in entries in the range [0, partitionCount_]
+ for (Lng32 i=0; i<partitionCount_-1; i++ ) {
+
+ const char* low = getBinaryBoundaryValue(i);
+ const char* high = getBinaryBoundaryValue(i+1);
+
+ // test if encodedKey is in [low, high)
+ if ( (*compFunc)(low, encodedKey, high, keyLen, i==partitionCount_-1) )
+ return i;
+ }
+
+ return -1;
+}
+
+Int32
+RangePartitioningFunction::computeNumOfActivePartitions(SearchKey* skey, const TableDesc* tDesc) const
+{
+ const RangePartitionBoundaries* boundaries = getRangePartitionBoundaries();
+
+ Int32 origPartitions = getCountOfPartitions();
+ Int32 partitions = origPartitions;
+ Int32 bIndex = 0;
+
+ const NATable* naTable = tDesc->getNATable();
+ NABoolean isNativeHbase = (naTable->isHbaseCellTable() || naTable->isHbaseRowTable());
+ compFuncPtrT compFuncPtr = ( isNativeHbase ) ? compareAsciiKey: compareEncodedKey;
--- End diff --
Good point. I'll add a check for hive.
On Mon, Oct 19, 2015 at 1:30 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/PartFunc.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42408115>
> :
>
> > +
> > + return -1;
> > +}
> > +
> > +Int32
> > +RangePartitioningFunction::computeNumOfActivePartitions(SearchKey* skey, const TableDesc* tDesc) const
> > +{
> > + const RangePartitionBoundaries* boundaries = getRangePartitionBoundaries();
> > +
> > + Int32 origPartitions = getCountOfPartitions();
> > + Int32 partitions = origPartitions;
> > + Int32 bIndex = 0;
> > +
> > + const NATable* naTable = tDesc->getNATable();
> > + NABoolean isNativeHbase = (naTable->isHbaseCellTable() || naTable->isHbaseRowTable());
> > + compFuncPtrT compFuncPtr = ( isNativeHbase ) ? compareAsciiKey: compareEncodedKey;
>
> Do we care about Hive tables in this code path?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42408115>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42407164
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
--- End diff --
Yes it is true for all intervals except the last one, which is an
all-closed interval.
On Mon, Oct 19, 2015 at 1:18 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/PartFunc.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42406620>
> :
>
> > @@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
> > partitionBoundaries_->print(ofd, indent, title);
> >
> > } // RangePartitioningFunction::print()
> > +
> > +
> > +NABoolean
> > +compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
>
> So, this routine checks to see if the key is in the half-open interval
> [low,hi), correct?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42406620>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42522891
--- Diff: core/sql/optimizer/ValueDesc.cpp ---
@@ -6397,3 +6400,111 @@ Lng32 ValueIdList::findPrefixLength(const ValueIdSet& x) const
return ct;
}
+// -----------------------------------------------------------------------
+// replace any ColReference (of the given column name) in of this value
+// expression with the given expression.
+// used in ValueId::computeEncodedKey() to assign key values into the
+// salt/DivisionByto expression.
+// ----------------------------------------------------------------------
+void ValueId::replaceColReferenceWithExpr(const NAString& colName,
+ const ValueId & vid)
+{
+ ItemExpr* thisItemExpr = getItemExpr();
+ for( Lng32 i = 0; i < thisItemExpr->getArity(); i++ )
+ {
+ ValueId childValueId = thisItemExpr->child(i).getValueId();
+ ItemExpr* childItemExpr = childValueId.getItemExpr();
+
+ if( childItemExpr->getOperatorType() == ITM_REFERENCE)
+ {
+ if( ((ColReference*)childItemExpr)->getColRefNameObj().getColName() == colName )
+ thisItemExpr->setChild( i, vid.getItemExpr() );
+ }
+ childValueId.replaceColReferenceWithExpr( colName, vid );
+ }
+}
+
+
+char*
+ValueIdList::computeEncodedKey(const TableDesc* tDesc, NABoolean isMaxKey,
+ char*& encodedKeyBuffer, Int32& keyBufLen) const
+{
+ const NATable* naTable = tDesc->getNATable();
+
+
+ CollIndex count = entries();
+ NAString** inputStrings = new (STMTHEAP) NAStringPtr[count];
+
+ for (Int32 j=0; j<count; j++ )
+ inputStrings[j] = NULL;
+
+ for (Int32 j=0; j<count; j++ ) {
+
+ ValueId vid = (*this)[j];
+ ItemExpr* ie = vid.getItemExpr();
+
+ if ( ie->getOperatorType() != ITM_CONSTANT ) {
+ ConstValue* value = ie->compute(STMTHEAP);
--- End diff --
fixed.
On Tue, Oct 20, 2015 at 11:30 AM, Hans Zeller <no...@github.com>
wrote:
> In core/sql/optimizer/ValueDesc.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42519204>
> :
>
> > + const NATable* naTable = tDesc->getNATable();
> > +
> > +
> > + CollIndex count = entries();
> > + NAString** inputStrings = new (STMTHEAP) NAStringPtr[count];
> > +
> > + for (Int32 j=0; j<count; j++ )
> > + inputStrings[j] = NULL;
> > +
> > + for (Int32 j=0; j<count; j++ ) {
> > +
> > + ValueId vid = (*this)[j];
> > + ItemExpr* ie = vid.getItemExpr();
> > +
> > + if ( ie->getOperatorType() != ITM_CONSTANT ) {
> > + ConstValue* value = ie->compute(STMTHEAP);
>
> Shouldn't we call ItemExpr::doesExprEvaluateToConstant(TRUE, TRUE) here
> before calling compute()? What happens when a non-constant like a VEGRef or
> a parameter is passed to compute()?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42519204>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42545760
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
Hi,
I just submitted the rework. Please review at your earliest convenience.
Thanks a lot!
On Tue, Oct 20, 2015 at 2:50 PM, Qifan Chen <qi...@esgyn.com> wrote:
> Thanks!
>
> On Tue, Oct 20, 2015 at 2:02 PM, Hans Zeller <no...@github.com>
> wrote:
>
>> In core/sql/optimizer/NAFileSet.cpp
>> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42538971>
>> :
>>
>> > @@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
>> > return keyLength_;
>> > }
>> >
>> > +// returns the length of the encoded key in bytes for this index
>> > +Lng32 NAFileSet::getEncodedKeyLength()
>> > +{
>> > + if(encodedKeyLength_ >0) return encodedKeyLength_;
>> > +
>> > + for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
>> > + {
>> > + encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
>>
>> What's the reason for not calling the method that already does this,
>> NAType::getEncodedKeyLength()?
>>
>> —
>> Reply to this email directly or view it on GitHub
>> <https://github.com/apache/incubator-trafodion/pull/132/files#r42538971>.
>>
>
>
>
> --
> Regards, --Qifan
>
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42446838
--- Diff: core/sql/optimizer/RelExpr.cpp ---
@@ -9515,7 +9515,8 @@ FileScan::FileScan(const CorrName& tableName,
estRowsAccessed_ (0),
mdamFlag_(UNDECIDED),
skipRowsToPreventHalloween_(FALSE),
- doUseSearchKey_(TRUE)
+ doUseSearchKey_(TRUE),
+ computedNumOfActivePartiions_(-1)
--- End diff --
Partitions is misspelled 8 times in this file, might make sense to fix the spelling for all of those.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404401
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
--- End diff --
I'm slightly confused about what this function is for. The comments and the name ("compute") suggest that it is for evaluation of arbitrary constant expressions, but the comments and variable names inside the function make reference to decoding and keys. But I'm guessing there is nothing in this function that is decoding-specific or key-specific. Is that correct?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42406933
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
--- End diff --
Maybe rename it as evaluateToConsValue()?
On Mon, Oct 19, 2015 at 1:01 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/ItemExpr.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404401>
> :
>
> > @@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
> > return FALSE;
> > }
> >
> > +// Compute the exprssion at compile time. Assume all operands are constants.
> > +// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
> > +ConstValue* ItemExpr::compute(CollHeap* heap)
>
> I'm slightly confused about what this function is for. The comments and
> the name ("compute") suggest that it is for evaluation of arbitrary
> constant expressions, but the comments and variable names inside the
> function make reference to decoding and keys. But I'm guessing there is
> nothing in this function that is decoding-specific or key-specific. Is that
> correct?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42404401>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/132
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42520005
--- Diff: core/sql/optimizer/ValueDesc.cpp ---
@@ -6397,3 +6400,111 @@ Lng32 ValueIdList::findPrefixLength(const ValueIdSet& x) const
return ct;
}
+// -----------------------------------------------------------------------
+// replace any ColReference (of the given column name) in of this value
+// expression with the given expression.
+// used in ValueId::computeEncodedKey() to assign key values into the
+// salt/DivisionByto expression.
+// ----------------------------------------------------------------------
+void ValueId::replaceColReferenceWithExpr(const NAString& colName,
+ const ValueId & vid)
+{
+ ItemExpr* thisItemExpr = getItemExpr();
+ for( Lng32 i = 0; i < thisItemExpr->getArity(); i++ )
+ {
+ ValueId childValueId = thisItemExpr->child(i).getValueId();
+ ItemExpr* childItemExpr = childValueId.getItemExpr();
+
+ if( childItemExpr->getOperatorType() == ITM_REFERENCE)
+ {
+ if( ((ColReference*)childItemExpr)->getColRefNameObj().getColName() == colName )
+ thisItemExpr->setChild( i, vid.getItemExpr() );
+ }
+ childValueId.replaceColReferenceWithExpr( colName, vid );
+ }
+}
+
+
+char*
+ValueIdList::computeEncodedKey(const TableDesc* tDesc, NABoolean isMaxKey,
+ char*& encodedKeyBuffer, Int32& keyBufLen) const
+{
+ const NATable* naTable = tDesc->getNATable();
+
+
+ CollIndex count = entries();
+ NAString** inputStrings = new (STMTHEAP) NAStringPtr[count];
+
+ for (Int32 j=0; j<count; j++ )
+ inputStrings[j] = NULL;
+
+ for (Int32 j=0; j<count; j++ ) {
+
+ ValueId vid = (*this)[j];
+ ItemExpr* ie = vid.getItemExpr();
+
+ if ( ie->getOperatorType() != ITM_CONSTANT ) {
+ ConstValue* value = ie->compute(STMTHEAP);
+ if ( !value )
+ return NULL;
+
+ inputStrings[j] = new (STMTHEAP) NAString(value->getConstStr(FALSE));
--- End diff --
As so often in the compiler we go through complicated paths to get to our goal, here we already have the binary value, then make it into a string, then make it back into a binary in encodeKeyValues below. Just an observation, I don't have a better solution. I wonder whether we should use this compute() method and ItemExpr::doesExprEvaluateToConstant() in ItemExpr::foldConstants() to make that method better. Again, just a thought, not a suggestion to change anything right now, but if we did that, we could just use constantFold() in this method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42447036
--- Diff: core/sql/optimizer/RelExpr.cpp ---
@@ -9515,7 +9515,8 @@ FileScan::FileScan(const CorrName& tableName,
estRowsAccessed_ (0),
mdamFlag_(UNDECIDED),
skipRowsToPreventHalloween_(FALSE),
- doUseSearchKey_(TRUE)
+ doUseSearchKey_(TRUE),
+ computedNumOfActivePartiions_(-1)
--- End diff --
Will do. Sorry about it.
On Mon, Oct 19, 2015 at 8:29 PM, Hans Zeller <no...@github.com>
wrote:
> In core/sql/optimizer/RelExpr.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42446838>
> :
>
> > @@ -9515,7 +9515,8 @@ FileScan::FileScan(const CorrName& tableName,
> > estRowsAccessed_ (0),
> > mdamFlag_(UNDECIDED),
> > skipRowsToPreventHalloween_(FALSE),
> > - doUseSearchKey_(TRUE)
> > + doUseSearchKey_(TRUE),
> > + computedNumOfActivePartiions_(-1)
>
> Partitions is misspelled 8 times in this file, might make sense to fix the
> spelling for all of those.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42446838>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42522238
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
What I have found is that the encoded keys for a salted table do not
contain length field for VARCHAR key column. Hence this new method.
Let me double check.
On Tue, Oct 20, 2015 at 11:07 AM, Hans Zeller <no...@github.com>
wrote:
> In core/sql/optimizer/NAFileSet.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42516287>
> :
>
> > @@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
> > return keyLength_;
> > }
> >
> > +// returns the length of the encoded key in bytes for this index
> > +Lng32 NAFileSet::getEncodedKeyLength()
> > +{
> > + if(encodedKeyLength_ >0) return encodedKeyLength_;
> > +
> > + for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
> > + {
> > + encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
>
> This should also add the null indicator length for nullable types.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42516287>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42519204
--- Diff: core/sql/optimizer/ValueDesc.cpp ---
@@ -6397,3 +6400,111 @@ Lng32 ValueIdList::findPrefixLength(const ValueIdSet& x) const
return ct;
}
+// -----------------------------------------------------------------------
+// replace any ColReference (of the given column name) in of this value
+// expression with the given expression.
+// used in ValueId::computeEncodedKey() to assign key values into the
+// salt/DivisionByto expression.
+// ----------------------------------------------------------------------
+void ValueId::replaceColReferenceWithExpr(const NAString& colName,
+ const ValueId & vid)
+{
+ ItemExpr* thisItemExpr = getItemExpr();
+ for( Lng32 i = 0; i < thisItemExpr->getArity(); i++ )
+ {
+ ValueId childValueId = thisItemExpr->child(i).getValueId();
+ ItemExpr* childItemExpr = childValueId.getItemExpr();
+
+ if( childItemExpr->getOperatorType() == ITM_REFERENCE)
+ {
+ if( ((ColReference*)childItemExpr)->getColRefNameObj().getColName() == colName )
+ thisItemExpr->setChild( i, vid.getItemExpr() );
+ }
+ childValueId.replaceColReferenceWithExpr( colName, vid );
+ }
+}
+
+
+char*
+ValueIdList::computeEncodedKey(const TableDesc* tDesc, NABoolean isMaxKey,
+ char*& encodedKeyBuffer, Int32& keyBufLen) const
+{
+ const NATable* naTable = tDesc->getNATable();
+
+
+ CollIndex count = entries();
+ NAString** inputStrings = new (STMTHEAP) NAStringPtr[count];
+
+ for (Int32 j=0; j<count; j++ )
+ inputStrings[j] = NULL;
+
+ for (Int32 j=0; j<count; j++ ) {
+
+ ValueId vid = (*this)[j];
+ ItemExpr* ie = vid.getItemExpr();
+
+ if ( ie->getOperatorType() != ITM_CONSTANT ) {
+ ConstValue* value = ie->compute(STMTHEAP);
--- End diff --
Shouldn't we call ItemExpr::doesExprEvaluateToConstant(TRUE, TRUE) here before calling compute()? What happens when a non-constant like a VEGRef or a parameter is passed to compute()?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42407695
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
+{
+ Int32 cmpLow = memcmp(low, key, keyLen);
+ Int32 cmpHigh = memcmp(key, high, keyLen);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+NABoolean
+compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
+{
+ Int32 cmpLow = strverscmp(low, key);
+ Int32 cmpHigh = strverscmp(key, high);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+
+// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
--- End diff --
findBeginBoundary starts its search at the end of the partition array, while findEndBoundary starts its search at the beginning. I would have thought we would have done the opposite. A full scan would cause us to search the entire array twice. Though I suppose once we get to 100's of partitions, we'll want to change both of these to binary searches.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42406620
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
--- End diff --
So, this routine checks to see if the key is in the half-open interval [low,hi), correct?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42534514
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
add the following line of code for null indicator.
Lng32 NAFileSet::getEncodedKeyLength()
{
if(encodedKeyLength_ >0) return encodedKeyLength_;
for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
{
encodedKeyLength_ +=
indexKeyColumns_[i]->getType()->getNominalSize();
encodedKeyLength_ +=
indexKeyColumns_[i]->getType()->getSQLnullHdrSize();
}
return encodedKeyLength_;
}
On Tue, Oct 20, 2015 at 11:55 AM, Qifan Chen <qi...@esgyn.com> wrote:
> What I have found is that the encoded keys for a salted table do not
> contain length field for VARCHAR key column. Hence this new method.
>
> Let me double check.
>
> On Tue, Oct 20, 2015 at 11:07 AM, Hans Zeller <no...@github.com>
> wrote:
>
>> In core/sql/optimizer/NAFileSet.cpp
>> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42516287>
>> :
>>
>> > @@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
>> > return keyLength_;
>> > }
>> >
>> > +// returns the length of the encoded key in bytes for this index
>> > +Lng32 NAFileSet::getEncodedKeyLength()
>> > +{
>> > + if(encodedKeyLength_ >0) return encodedKeyLength_;
>> > +
>> > + for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
>> > + {
>> > + encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
>>
>> This should also add the null indicator length for nullable types.
>>
>> —
>> Reply to this email directly or view it on GitHub
>> <https://github.com/apache/incubator-trafodion/pull/132/files#r42516287>.
>>
>
>
>
> --
> Regards, --Qifan
>
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42516287
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
This should also add the null indicator length for nullable types.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42405678
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
+ // any two known character sets supported.
+ Lng32 factor = (DFS2REC::isAnyCharacter(dataType.getFSDatatype())) ? 8 : 1;
+
+ if ( staticDecodeBufLen < decodedValueLen * factor) {
+ decodeBufLen = decodedValueLen * factor;
+ decodeBuf = new (STMTHEAP) char[decodeBufLen];
--- End diff --
No. null will not be needed since a binary value will be the result.
On Mon, Oct 19, 2015 at 1:04 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/ItemExpr.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404746>
> :
>
> > +
> > + Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
> > +
> > + char staticDecodeBuf[200];
> > + Lng32 staticDecodeBufLen = 200;
> > +
> > + char* decodeBuf = staticDecodeBuf;
> > + Lng32 decodeBufLen = staticDecodeBufLen;
> > +
> > + // For character types, multiplying by 8 to deal with conversions between
> > + // any two known character sets supported.
> > + Lng32 factor = (DFS2REC::isAnyCharacter(dataType.getFSDatatype())) ? 8 : 1;
> > +
> > + if ( staticDecodeBufLen < decodedValueLen * factor) {
> > + decodeBufLen = decodedValueLen * factor;
> > + decodeBuf = new (STMTHEAP) char[decodeBufLen];
>
> We don't care about trailing nulls, right? (Hence, no +1 after
> decodeBufLen)
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42404746>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42544428
--- Diff: core/sql/optimizer/NAFileSet.cpp ---
@@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
return keyLength_;
}
+// returns the length of the encoded key in bytes for this index
+Lng32 NAFileSet::getEncodedKeyLength()
+{
+ if(encodedKeyLength_ >0) return encodedKeyLength_;
+
+ for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
+ {
+ encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
--- End diff --
Thanks!
On Tue, Oct 20, 2015 at 2:02 PM, Hans Zeller <no...@github.com>
wrote:
> In core/sql/optimizer/NAFileSet.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42538971>
> :
>
> > @@ -161,6 +162,18 @@ Lng32 NAFileSet::getKeyLength()
> > return keyLength_;
> > }
> >
> > +// returns the length of the encoded key in bytes for this index
> > +Lng32 NAFileSet::getEncodedKeyLength()
> > +{
> > + if(encodedKeyLength_ >0) return encodedKeyLength_;
> > +
> > + for(CollIndex i=0;i<indexKeyColumns_.entries();i++)
> > + {
> > + encodedKeyLength_ += indexKeyColumns_[i]->getType()->getNominalSize();
>
> What's the reason for not calling the method that already does this,
> NAType::getEncodedKeyLength()?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42538971>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42512787
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
--- End diff --
There is actually a method to compute this length, CharInfo::getMaxConvertedLenInBytes() (file common/charinfo.h) but it may be inconvenient and overkill in this situation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42517288
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
+{
+ Int32 cmpLow = memcmp(low, key, keyLen);
+ Int32 cmpHigh = memcmp(key, high, keyLen);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+NABoolean
+compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
+{
+ Int32 cmpLow = strverscmp(low, key);
+ Int32 cmpHigh = strverscmp(key, high);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+
+// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
--- End diff --
Wonder whether it would be worth doing a binary search.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42406579
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
--- End diff --
Good point. I'll fix it.
On Mon, Oct 19, 2015 at 1:02 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/ItemExpr.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404547>
> :
>
> > +ConstValue* ItemExpr::compute(CollHeap* heap)
> > +{
> > + ValueIdList exprs;
> > + exprs.insert(getValueId());
> > +
> > + const NAType& dataType = getValueId().getType();
> > +
> > + Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
> > +
> > + char staticDecodeBuf[200];
> > + Lng32 staticDecodeBufLen = 200;
> > +
> > + char* decodeBuf = staticDecodeBuf;
> > + Lng32 decodeBufLen = staticDecodeBufLen;
> > +
> > + // For character types, multiplying by 8 to deal with conversions between
>
> UTF-8 characters are a max of 6 bytes. Do we know of any that are longer?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42404547>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42405465
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
+ // any two known character sets supported.
+ Lng32 factor = (DFS2REC::isAnyCharacter(dataType.getFSDatatype())) ? 8 : 1;
+
+ if ( staticDecodeBufLen < decodedValueLen * factor) {
+ decodeBufLen = decodedValueLen * factor;
+ decodeBuf = new (STMTHEAP) char[decodeBufLen];
+ }
+
+ Lng32 resultLength = 0;
+ Lng32 resultOffset = 0;
+
+ // Produce the decoded key. Refer to
+ // ex_function_encode::decodeKeyValue() for the
+ // implementation of the decoding logic.
+ ex_expr::exp_return_type rc = exprs.evalAtCompileTime
+ (0, ExpTupleDesc::SQLARK_EXPLODED_FORMAT, decodeBuf, decodeBufLen,
+ &resultLength, &resultOffset, CmpCommon::diags()
+ );
+
+
+ ConstValue* result = NULL;
+
+ if ( rc == ex_expr::EXPR_OK ) {
+ CMPASSERT(resultOffset == dataType.getPrefixSizeWithAlignment());
+ // expect the decodeBuf to have this layout
+ // | null ind. | varchar length ind. | alignment | result |
+ // |<---getPrefixSizeWithAlignment-------------->|
+ // |<----getPrefixSize-------------->|
+
+ // The method getPrefixSizeWithAlignment(), the diagram above,
+ // and this code block assumes that varchar length ind. is
+ // 2 bytes if present. If it is 4 bytes we should fail the
+ // previous assert
+
+ // Next we get rid of alignment bytes by prepending the prefix
+ // (null ind. + varlen ind.) to the result. ConstValue constr.
+ // will process prefix + result. The assert above ensures that
+ // there are no alignment fillers at the beginning of the
+ // buffer. Given the previous assumption about size
+ // of varchar length indicator, alignment bytes will be used by
+ // expression evaluator only if column is of nullable type.
+ // For a description of how alignment is computed, please see
+ // ExpTupleDesc::sqlarkExplodedOffsets() in exp/exp_tuple_desc.cpp
+
+ if (dataType.getSQLnullHdrSize() > 0)
+ memmove(&decodeBuf[resultOffset - dataType.getPrefixSize()],
--- End diff --
The source and target fields seem to overlap? How long is the null indicator? Two bytes? Does memmove move a byte at a time?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by nonstop-qfchen <gi...@git.apache.org>.
Github user nonstop-qfchen commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42409339
--- Diff: core/sql/optimizer/PartFunc.cpp ---
@@ -5009,6 +5020,118 @@ void RangePartitioningFunction::print(FILE* ofd, const char* indent,
partitionBoundaries_->print(ofd, indent, title);
} // RangePartitioningFunction::print()
+
+
+NABoolean
+compareEncodedKey(const char* low, const char* key, const char* high, Int32 keyLen, NABoolean checkLast)
+{
+ Int32 cmpLow = memcmp(low, key, keyLen);
+ Int32 cmpHigh = memcmp(key, high, keyLen);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+NABoolean
+compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
+{
+ Int32 cmpLow = strverscmp(low, key);
+ Int32 cmpHigh = strverscmp(key, high);
+
+ if ( cmpLow <= 0 && cmpHigh < 0 )
+ return TRUE;
+
+ return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
+}
+
+
+// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
--- End diff --
I'll address it.
On Mon, Oct 19, 2015 at 1:26 PM, DaveBirdsall <no...@github.com>
wrote:
> In core/sql/optimizer/PartFunc.cpp
> <https://github.com/apache/incubator-trafodion/pull/132#discussion_r42407695>
> :
>
> > +}
> > +
> > +NABoolean
> > +compareAsciiKey(const char* low, const char* key, const char* high, Int32, NABoolean checkLast)
> > +{
> > + Int32 cmpLow = strverscmp(low, key);
> > + Int32 cmpHigh = strverscmp(key, high);
> > +
> > + if ( cmpLow <= 0 && cmpHigh < 0 )
> > + return TRUE;
> > +
> > + return (checkLast && cmpLow <= 0 && cmpHigh <= 0);
> > +}
> > +
> > +
> > +// find a boundary pair [low, high) with smallest low value in which keys fall, and return the
>
> findBeginBoundary starts its search at the end of the partition array,
> while findEndBoundary starts its search at the beginning. I would have
> thought we would have done the opposite. A full scan would cause us to
> search the entire array twice. Though I suppose once we get to 100's of
> partitions, we'll want to change both of these to binary searches.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-trafodion/pull/132/files#r42407695>.
>
--
Regards, --Qifan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42409193
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
--- End diff --
If it were a general routine, though, instead of "decodedKeyBuf", maybe "resultBuf". But it is OK as is I think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404746
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
+ // any two known character sets supported.
+ Lng32 factor = (DFS2REC::isAnyCharacter(dataType.getFSDatatype())) ? 8 : 1;
+
+ if ( staticDecodeBufLen < decodedValueLen * factor) {
+ decodeBufLen = decodedValueLen * factor;
+ decodeBuf = new (STMTHEAP) char[decodeBufLen];
--- End diff --
We don't care about trailing nulls, right? (Hence, no +1 after decodeBufLen)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: fix JIRA461
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/132#discussion_r42404547
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -14906,3 +14906,80 @@ NABoolean LOBoper::isCovered
return FALSE;
}
+// Compute the exprssion at compile time. Assume all operands are constants.
+// Return NULL if the computation fails and CmpCommon::diags() may be side-affected.
+ConstValue* ItemExpr::compute(CollHeap* heap)
+{
+ ValueIdList exprs;
+ exprs.insert(getValueId());
+
+ const NAType& dataType = getValueId().getType();
+
+ Lng32 decodedValueLen = dataType.getNominalSize() + dataType.getSQLnullHdrSize();
+
+ char staticDecodeBuf[200];
+ Lng32 staticDecodeBufLen = 200;
+
+ char* decodeBuf = staticDecodeBuf;
+ Lng32 decodeBufLen = staticDecodeBufLen;
+
+ // For character types, multiplying by 8 to deal with conversions between
--- End diff --
UTF-8 characters are a max of 6 bytes. Do we know of any that are longer?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---