You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/03/03 03:16:42 UTC
[GitHub] [orc] wgtmac opened a new pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
wgtmac opened a new pull request #648:
URL: https://github.com/apache/orc/pull/648
1. Use RowReaderOptions to pass SearchArgument to enable PPD.
2. Modify RowReaderImpl::startNextStripe to seek to next matched row group based on sarg evaluation.
3. RowReaderImpl::next seeks to next matched row group based on sarg evaluation.
4. RowReaderImpl::seekToRow also jumps to the 1st matched row group after specified row number.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r586336023
##########
File path: c++/include/orc/Reader.hh
##########
@@ -538,6 +549,12 @@ namespace orc {
*/
virtual void seekToRow(uint64_t rowNumber) = 0;
+ /**
+ * If PPD is enabled, returns true and store number of selected RGs and
+ * number of evaluated RGs into the stats pair; otherwise returns false.
+ */
+ virtual bool getPPDStats(std::pair<uint64_t, uint64_t>& stats) const = 0;
Review comment:
I like this -- just wondering if we should do it in a follow up and create a Java equivalent?
Idealy one would like a complete set of stats like: total vs read Stripes, total vs read Row groups and total vs read Rows/bytes.
##########
File path: c++/src/Reader.cc
##########
@@ -68,6 +68,12 @@ namespace orc {
return columnPath.substr(0, columnPath.length() - 1);
}
+ WriterVersion getWriterVersionImpl(const FileContents * contents) {
+ if (!contents->postscript->has_writerversion()) {
+ return WriterVersion_ORIGINAL;
Review comment:
Just a reminder to check why this is not FUTURE as in Java equivalent
##########
File path: c++/src/Reader.cc
##########
@@ -293,33 +308,43 @@ namespace orc {
previousRow = rowNumber;
startNextStripe();
- uint64_t rowsToSkip = currentRowInStripe;
+ // when predicate push down is enabled, above call to startNextStripe()
+ // will move current row to 1st matching row group; here we only need
+ // to deal with the case when PPD is not enabled.
+ if (!sargsApplier) {
Review comment:
Maybe move this part on the startNextStripe() method?
##########
File path: c++/src/Reader.cc
##########
@@ -892,29 +939,67 @@ namespace orc {
void RowReaderImpl::startNextStripe() {
reader.reset(); // ColumnReaders use lots of memory; free old memory first
- currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
- uint64_t fileLength = contents->stream->getLength();
- if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
+ rowIndexes.clear();
+ bloomFilterIndex.clear();
+
+ do {
+ currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
+ uint64_t fileLength = contents->stream->getLength();
+ if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
currentStripeInfo.datalength() + currentStripeInfo.footerlength() >= fileLength) {
- std::stringstream msg;
- msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
- << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
- << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
- << ", footerLength=" << currentStripeInfo.footerlength() << ")";
- throw ParseError(msg.str());
+ std::stringstream msg;
+ msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
+ << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
+ << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
+ << ", footerLength=" << currentStripeInfo.footerlength() << ")";
+ throw ParseError(msg.str());
+ }
+ currentStripeFooter = getStripeFooter(currentStripeInfo, *contents.get());
+ rowsInCurrentStripe = currentStripeInfo.numberofrows();
+
+ if (sargsApplier) {
+ // read row group statistics and bloom filters of current stripe
+ loadStripeIndex();
+
+ // select row groups to read in the current stripe
+ sargsApplier->pickRowGroups(rowsInCurrentStripe,
+ rowIndexes,
+ bloomFilterIndex);
+ if (sargsApplier->hasSelectedFrom(currentRowInStripe)) {
+ // current stripe has at least one row group matching the predicate
+ break;
+ } else {
+ // advance to next stripe when current stripe has no matching rows
+ currentStripe += 1;
+ currentRowInStripe = 0;
+ }
+ }
+ } while (sargsApplier && currentStripe < lastStripe);
+
+ if (currentStripe < lastStripe) {
+ const Timezone& writerTimezone =
Review comment:
Lets add a but more doc here
##########
File path: c++/src/Reader.cc
##########
@@ -944,13 +1042,67 @@ namespace orc {
// update row number
previousRow = firstRowOfStripe[currentStripe] + currentRowInStripe;
currentRowInStripe += rowsToRead;
+
+ // check if we need to advance to next selected row group
+ if (sargsApplier) {
+ uint64_t nextRowToRead = advanceToNextRowGroup(currentRowInStripe,
+ rowsInCurrentStripe,
+ footer->rowindexstride(),
+ sargsApplier->getRowGroups());
+ if (currentRowInStripe != nextRowToRead) {
+ // it is guaranteed to be at start of a row group
+ currentRowInStripe = nextRowToRead;
+ if (currentRowInStripe < rowsInCurrentStripe) {
+ seekToRowGroup(static_cast<uint32_t>(currentRowInStripe / footer->rowindexstride()));
+ }
+ }
+ }
+
if (currentRowInStripe >= rowsInCurrentStripe) {
currentStripe += 1;
currentRowInStripe = 0;
}
return rowsToRead != 0;
}
+ uint64_t RowReaderImpl::computeBatchSize(uint64_t requestedSize,
+ uint64_t currentRowInStripe,
+ uint64_t rowsInCurrentStripe,
+ uint64_t rowIndexStride,
+ const std::vector<bool>& includedRowGroups) {
+ uint64_t endRowInStripe = rowsInCurrentStripe;
Review comment:
I would definitely copy the marker explanation comment here with JIRA from the Java side -- would also try to follow a similar logic: if (ncludedRowGroups.empty()) batchsize = else ...
Adding a Debug log would also make sense
##########
File path: c++/src/sargs/SargsApplier.cc
##########
@@ -25,7 +26,13 @@ namespace orc {
const std::string& colName) {
for (uint64_t i = 0; i != type.getSubtypeCount(); ++i) {
if (type.getFieldName(i) == colName) {
- return type.getSubtype(i)->getColumnId();
+ if (type.getKind() == CHAR || type.getKind() == VARCHAR) {
Review comment:
Shall we leave this to the [consumer](https://github.com/apache/orc/blob/295ea5be6c2ae643bb3c3ca6db7b018aeb51bebc/java/core/src/java/org/apache/orc/impl/SchemaEvolution.java#L378)? We dont have anything similar for the Java side.
We probably should address this as part of ORC-612
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r586348582
##########
File path: c++/src/Reader.cc
##########
@@ -892,29 +939,67 @@ namespace orc {
void RowReaderImpl::startNextStripe() {
reader.reset(); // ColumnReaders use lots of memory; free old memory first
- currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
- uint64_t fileLength = contents->stream->getLength();
- if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
+ rowIndexes.clear();
+ bloomFilterIndex.clear();
+
+ do {
+ currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
+ uint64_t fileLength = contents->stream->getLength();
+ if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
currentStripeInfo.datalength() + currentStripeInfo.footerlength() >= fileLength) {
- std::stringstream msg;
- msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
- << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
- << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
- << ", footerLength=" << currentStripeInfo.footerlength() << ")";
- throw ParseError(msg.str());
+ std::stringstream msg;
+ msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
+ << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
+ << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
+ << ", footerLength=" << currentStripeInfo.footerlength() << ")";
+ throw ParseError(msg.str());
+ }
+ currentStripeFooter = getStripeFooter(currentStripeInfo, *contents.get());
+ rowsInCurrentStripe = currentStripeInfo.numberofrows();
+
+ if (sargsApplier) {
+ // read row group statistics and bloom filters of current stripe
+ loadStripeIndex();
+
+ // select row groups to read in the current stripe
+ sargsApplier->pickRowGroups(rowsInCurrentStripe,
+ rowIndexes,
+ bloomFilterIndex);
+ if (sargsApplier->hasSelectedFrom(currentRowInStripe)) {
+ // current stripe has at least one row group matching the predicate
+ break;
+ } else {
+ // advance to next stripe when current stripe has no matching rows
+ currentStripe += 1;
+ currentRowInStripe = 0;
+ }
+ }
+ } while (sargsApplier && currentStripe < lastStripe);
+
+ if (currentStripe < lastStripe) {
+ const Timezone& writerTimezone =
Review comment:
Lets add some doc here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] wgtmac commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
wgtmac commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r589472878
##########
File path: c++/include/orc/Reader.hh
##########
@@ -538,6 +549,12 @@ namespace orc {
*/
virtual void seekToRow(uint64_t rowNumber) = 0;
+ /**
+ * If PPD is enabled, returns true and store number of selected RGs and
+ * number of evaluated RGs into the stats pair; otherwise returns false.
+ */
+ virtual bool getPPDStats(std::pair<uint64_t, uint64_t>& stats) const = 0;
Review comment:
That sounds good. We should think carefully what stats are required and what kind of API to provide.
##########
File path: c++/src/Reader.cc
##########
@@ -68,6 +68,12 @@ namespace orc {
return columnPath.substr(0, columnPath.length() - 1);
}
+ WriterVersion getWriterVersionImpl(const FileContents * contents) {
+ if (!contents->postscript->has_writerversion()) {
+ return WriterVersion_ORIGINAL;
Review comment:
Sure.
##########
File path: c++/src/Reader.cc
##########
@@ -944,13 +1042,67 @@ namespace orc {
// update row number
previousRow = firstRowOfStripe[currentStripe] + currentRowInStripe;
currentRowInStripe += rowsToRead;
+
+ // check if we need to advance to next selected row group
+ if (sargsApplier) {
+ uint64_t nextRowToRead = advanceToNextRowGroup(currentRowInStripe,
+ rowsInCurrentStripe,
+ footer->rowindexstride(),
+ sargsApplier->getRowGroups());
+ if (currentRowInStripe != nextRowToRead) {
+ // it is guaranteed to be at start of a row group
+ currentRowInStripe = nextRowToRead;
+ if (currentRowInStripe < rowsInCurrentStripe) {
+ seekToRowGroup(static_cast<uint32_t>(currentRowInStripe / footer->rowindexstride()));
+ }
+ }
+ }
+
if (currentRowInStripe >= rowsInCurrentStripe) {
currentStripe += 1;
currentRowInStripe = 0;
}
return rowsToRead != 0;
}
+ uint64_t RowReaderImpl::computeBatchSize(uint64_t requestedSize,
+ uint64_t currentRowInStripe,
+ uint64_t rowsInCurrentStripe,
+ uint64_t rowIndexStride,
+ const std::vector<bool>& includedRowGroups) {
+ uint64_t endRowInStripe = rowsInCurrentStripe;
Review comment:
Added a comment here. The logic is slightily different with the java side so I'd prefer to keep the logic here.
##########
File path: c++/src/sargs/SargsApplier.cc
##########
@@ -25,7 +26,13 @@ namespace orc {
const std::string& colName) {
for (uint64_t i = 0; i != type.getSubtypeCount(); ++i) {
if (type.getFieldName(i) == colName) {
- return type.getSubtype(i)->getColumnId();
+ if (type.getKind() == CHAR || type.getKind() == VARCHAR) {
Review comment:
Sure
##########
File path: c++/src/Reader.cc
##########
@@ -293,33 +308,43 @@ namespace orc {
previousRow = rowNumber;
startNextStripe();
- uint64_t rowsToSkip = currentRowInStripe;
+ // when predicate push down is enabled, above call to startNextStripe()
+ // will move current row to 1st matching row group; here we only need
+ // to deal with the case when PPD is not enabled.
+ if (!sargsApplier) {
Review comment:
startNextStripe() simply does its job to start a next stripe. this logic better stays here.
##########
File path: c++/src/Reader.cc
##########
@@ -892,29 +939,67 @@ namespace orc {
void RowReaderImpl::startNextStripe() {
reader.reset(); // ColumnReaders use lots of memory; free old memory first
- currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
- uint64_t fileLength = contents->stream->getLength();
- if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
+ rowIndexes.clear();
+ bloomFilterIndex.clear();
+
+ do {
+ currentStripeInfo = footer->stripes(static_cast<int>(currentStripe));
+ uint64_t fileLength = contents->stream->getLength();
+ if (currentStripeInfo.offset() + currentStripeInfo.indexlength() +
currentStripeInfo.datalength() + currentStripeInfo.footerlength() >= fileLength) {
- std::stringstream msg;
- msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
- << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
- << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
- << ", footerLength=" << currentStripeInfo.footerlength() << ")";
- throw ParseError(msg.str());
+ std::stringstream msg;
+ msg << "Malformed StripeInformation at stripe index " << currentStripe << ": fileLength="
+ << fileLength << ", StripeInfo=(offset=" << currentStripeInfo.offset() << ", indexLength="
+ << currentStripeInfo.indexlength() << ", dataLength=" << currentStripeInfo.datalength()
+ << ", footerLength=" << currentStripeInfo.footerlength() << ")";
+ throw ParseError(msg.str());
+ }
+ currentStripeFooter = getStripeFooter(currentStripeInfo, *contents.get());
+ rowsInCurrentStripe = currentStripeInfo.numberofrows();
+
+ if (sargsApplier) {
+ // read row group statistics and bloom filters of current stripe
+ loadStripeIndex();
+
+ // select row groups to read in the current stripe
+ sargsApplier->pickRowGroups(rowsInCurrentStripe,
+ rowIndexes,
+ bloomFilterIndex);
+ if (sargsApplier->hasSelectedFrom(currentRowInStripe)) {
+ // current stripe has at least one row group matching the predicate
+ break;
+ } else {
+ // advance to next stripe when current stripe has no matching rows
+ currentStripe += 1;
+ currentRowInStripe = 0;
+ }
+ }
+ } while (sargsApplier && currentStripe < lastStripe);
+
+ if (currentStripe < lastStripe) {
+ const Timezone& writerTimezone =
Review comment:
Done
##########
File path: c++/src/Reader.hh
##########
@@ -142,6 +143,30 @@ namespace orc {
// row index of current stripe with column id as the key
std::unordered_map<uint64_t, proto::RowIndex> rowIndexes;
+ std::map<uint32_t, BloomFilterIndex> bloomFilterIndex;
Review comment:
it is used to hold bloom filter index in Reader.cc (e.g. line 378)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r586361659
##########
File path: c++/src/sargs/SargsApplier.cc
##########
@@ -25,7 +26,13 @@ namespace orc {
const std::string& colName) {
for (uint64_t i = 0; i != type.getSubtypeCount(); ++i) {
if (type.getFieldName(i) == colName) {
- return type.getSubtype(i)->getColumnId();
+ if (type.getKind() == CHAR || type.getKind() == VARCHAR) {
Review comment:
Shall we leave this to the [consumer](https://github.com/apache/orc/blob/295ea5be6c2ae643bb3c3ca6db7b018aeb51bebc/java/core/src/java/org/apache/orc/impl/SchemaEvolution.java#L378)? We dont have anything similar on the Java side.
We probably should address this as part of ORC-612
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] wgtmac commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
wgtmac commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-789396385
@dongjoon-hyun I have closed the old PR and moved it here. Please review it again. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] wgtmac commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
wgtmac commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-793602786
> > Thank you for updating, @wgtmac .
> > Do you have other comments, @pgaref ?
>
> Hey @dongjoon-hyun PR looks good to me as well, we just need to remove the newly added getPPDStats() API before merging.
It has been removed now. Thanks for reviewing the PR! @pgaref @dongjoon-hyun
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r589630797
##########
File path: c++/include/orc/Reader.hh
##########
@@ -538,6 +549,12 @@ namespace orc {
*/
virtual void seekToRow(uint64_t rowNumber) = 0;
+ /**
+ * If PPD is enabled, returns true and store number of selected RGs and
+ * number of evaluated RGs into the stats pair; otherwise returns false.
+ */
+ virtual bool getPPDStats(std::pair<uint64_t, uint64_t>& stats) const = 0;
Review comment:
Agreed, lets remove this for now and use a separate ticket to track.
I believe the remaining changes are ready to be merged after that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-793659019
> > > Thank you for updating, @wgtmac .
> > > Do you have other comments, @pgaref ?
> >
> >
> > Hey @dongjoon-hyun PR looks good to me as well, we just need to remove the newly added getPPDStats() API before merging.
>
> It has been removed now. Thanks for reviewing the PR! @pgaref @dongjoon-hyun
Thanks for updating @wgtmac ! seems like mStats can also be removed as its not longer used but its less important.
+1 from my side
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun merged pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun merged pull request #648:
URL: https://github.com/apache/orc/pull/648
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #648:
URL: https://github.com/apache/orc/pull/648#discussion_r588047504
##########
File path: c++/src/Reader.cc
##########
@@ -68,6 +68,12 @@ namespace orc {
return columnPath.substr(0, columnPath.length() - 1);
}
+ WriterVersion getWriterVersionImpl(const FileContents * contents) {
+ if (!contents->postscript->has_writerversion()) {
+ return WriterVersion_ORIGINAL;
Review comment:
+1 for @pgaref 's comment.
##########
File path: c++/src/Reader.hh
##########
@@ -142,6 +143,30 @@ namespace orc {
// row index of current stripe with column id as the key
std::unordered_map<uint64_t, proto::RowIndex> rowIndexes;
+ std::map<uint32_t, BloomFilterIndex> bloomFilterIndex;
Review comment:
Shall we remove this in this PR because this seems to be unused in this PR?
##########
File path: c++/src/sargs/SargsApplier.cc
##########
@@ -25,7 +26,13 @@ namespace orc {
const std::string& colName) {
for (uint64_t i = 0; i != type.getSubtypeCount(); ++i) {
if (type.getFieldName(i) == colName) {
- return type.getSubtype(i)->getColumnId();
+ if (type.getKind() == CHAR || type.getKind() == VARCHAR) {
+ ///FIXME: disable PPD for char and varchar types as their rules
+ // vary among different engines.
+ return INVALID_COLUMN_ID;
Review comment:
Do we have a test coverage for this code path?
If not, I'm +1 for @pgaref 's opinion. We can do this later.
##########
File path: c++/include/orc/Reader.hh
##########
@@ -538,6 +549,12 @@ namespace orc {
*/
virtual void seekToRow(uint64_t rowNumber) = 0;
+ /**
+ * If PPD is enabled, returns true and store number of selected RGs and
+ * number of evaluated RGs into the stats pair; otherwise returns false.
+ */
+ virtual bool getPPDStats(std::pair<uint64_t, uint64_t>& stats) const = 0;
Review comment:
+1 for @pgaref 's comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-794232710
Thank you, @wgtmac and @pgaref . Merged to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-793148903
Thank you for updating, @wgtmac .
Do you have other comments, @pgaref ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] pgaref commented on pull request #648: ORC-751: [C++] Implement Predicate Pushdown for C++ Reader
Posted by GitBox <gi...@apache.org>.
pgaref commented on pull request #648:
URL: https://github.com/apache/orc/pull/648#issuecomment-793151175
> Thank you for updating, @wgtmac .
> Do you have other comments, @pgaref ?
Hey @dongjoon-hyun PR looks good to me as well, we just need to remove the newly added getPPDStats() API before merging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org