Skip to content

Fix the extract values function sigature when used with filter process in selective reader #10956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xiaoxmeng wants to merge 1 commit into from
Closed

Fix the extract values function sigature when used with filter process in selective reader #10956

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
4 changes: 2 additions & 2 deletions velox/dwio/common/ColumnVisitors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class ColumnVisitor {
TFilter& filter,
SelectiveColumnReader* reader,
const RowSet& rows,
const ExtractValues& values)
ExtractValues values)
: filter_(filter),
reader_(reader),
allowNulls_(!TFilter::deterministic || filter.testNull()),
Expand Down Expand Up @@ -719,7 +719,7 @@ class DictionaryColumnVisitor
TFilter& filter,
SelectiveColumnReader* reader,
const RowSet& rows,
const ExtractValues& values)
ExtractValues values)
: ColumnVisitor<T, TFilter, ExtractValues, isDense>(
filter,
reader,
Expand Down
4 changes: 2 additions & 2 deletions velox/dwio/common/SelectiveByteRleColumnReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class SelectiveByteRleColumnReader : public SelectiveColumnReader {
void readHelper(
velox::common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues);
ExtractValues extractValues);

template <typename Reader, bool kEncodingHasNulls>
void readCommon(
Expand All @@ -77,7 +77,7 @@ template <
void SelectiveByteRleColumnReader::readHelper(
velox::common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues) {
ExtractValues extractValues) {
reinterpret_cast<Reader*>(this)->readWithVisitor(
rows,
ColumnVisitor<int8_t, TFilter, ExtractValues, isDense>(
Expand Down
16 changes: 9 additions & 7 deletions velox/dwio/common/SelectiveFloatingPointColumnReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ class SelectiveFloatingPointColumnReader : public SelectiveColumnReader {
typename TFilter,
bool isDense,
typename ExtractValues>
void
readHelper(velox::common::Filter* filter, RowSet rows, ExtractValues values);
void readHelper(
velox::common::Filter* filter,
const RowSet& rows,
ExtractValues values);

template <
typename Reader,
Expand All @@ -66,11 +68,11 @@ class SelectiveFloatingPointColumnReader : public SelectiveColumnReader {
typename ExtractValues>
void processFilter(
velox::common::Filter* filter,
RowSet rows,
const RowSet& rows,
ExtractValues extractValues);

template <typename Reader, bool isDense>
void processValueHook(RowSet rows, ValueHook* hook);
void processValueHook(const RowSet& rows, ValueHook* hook);
};

template <typename TData, typename TRequested>
Expand All @@ -81,7 +83,7 @@ template <
typename ExtractValues>
void SelectiveFloatingPointColumnReader<TData, TRequested>::readHelper(
velox::common::Filter* filter,
RowSet rows,
const RowSet& rows,
ExtractValues extractValues) {
reinterpret_cast<Reader*>(this)->readWithVisitor(
rows,
Expand All @@ -97,7 +99,7 @@ template <
typename ExtractValues>
void SelectiveFloatingPointColumnReader<TData, TRequested>::processFilter(
velox::common::Filter* filter,
RowSet rows,
const RowSet& rows,
ExtractValues extractValues) {
if (filter == nullptr) {
readHelper<Reader, velox::common::AlwaysTrue, isDense>(
Expand Down Expand Up @@ -144,7 +146,7 @@ void SelectiveFloatingPointColumnReader<TData, TRequested>::processFilter(
template <typename TData, typename TRequested>
template <typename Reader, bool isDense>
void SelectiveFloatingPointColumnReader<TData, TRequested>::processValueHook(
RowSet rows,
const RowSet& rows,
ValueHook* hook) {
switch (hook->kind()) {
case aggregate::AggregationHook::kDoubleSum:
Expand Down
8 changes: 4 additions & 4 deletions velox/dwio/common/SelectiveIntegerColumnReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class SelectiveIntegerColumnReader : public SelectiveColumnReader {
typename ExtractValues>
void processFilter(
velox::common::Filter* filter,
const ExtractValues& extractValues,
ExtractValues extractValues,
const RowSet& rows);

// Switches based on the type of ValueHook between different readWithVisitor
Expand All @@ -65,7 +65,7 @@ class SelectiveIntegerColumnReader : public SelectiveColumnReader {
void readHelper(
velox::common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues);
ExtractValues extractValues);

// The common part of integer reading. calls the appropriate
// instantiation of processValueHook or processFilter based on
Expand All @@ -82,7 +82,7 @@ template <
void SelectiveIntegerColumnReader::readHelper(
velox::common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues) {
ExtractValues extractValues) {
switch (valueSize_) {
case 2:
reinterpret_cast<Reader*>(this)->Reader::readWithVisitor(
Expand Down Expand Up @@ -124,7 +124,7 @@ template <
typename ExtractValues>
void SelectiveIntegerColumnReader::processFilter(
velox::common::Filter* filter,
const ExtractValues& extractValues,
ExtractValues extractValues,
const RowSet& rows) {
if (filter == nullptr) {
readHelper<Reader, velox::common::AlwaysTrue, isDense>(
Expand Down
12 changes: 5 additions & 7 deletions velox/dwio/dwrf/reader/SelectiveStringDictionaryColumnReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,14 @@ class SelectiveStringDictionaryColumnReader
void readWithVisitor(const RowSet& rows, TVisitor visitor);

template <typename TFilter, bool isDense, typename ExtractValues>
void readHelper(
common::Filter* filter,
const RowSet& rows,
const ExtractValues& values);
void
readHelper(common::Filter* filter, const RowSet& rows, ExtractValues values);

template <bool isDense, typename ExtractValues>
void processFilter(
common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues);
ExtractValues extractValues);

// Fills 'values' from 'data' and 'lengthDecoder'. The count of
// values is in 'values.numValues'.
Expand Down Expand Up @@ -129,7 +127,7 @@ template <typename TFilter, bool isDense, typename ExtractValues>
void SelectiveStringDictionaryColumnReader::readHelper(
common::Filter* filter,
const RowSet& rows,
const ExtractValues& values) {
ExtractValues values) {
readWithVisitor(
rows,
dwio::common::
Expand All @@ -141,7 +139,7 @@ template <bool isDense, typename ExtractValues>
void SelectiveStringDictionaryColumnReader::processFilter(
common::Filter* filter,
const RowSet& rows,
const ExtractValues& extractValues) {
ExtractValues extractValues) {
if (filter == nullptr) {
readHelper<common::AlwaysTrue, isDense>(
&dwio::common::alwaysTrue(), rows, extractValues);
Expand Down
43 changes: 43 additions & 0 deletions velox/exec/tests/TableScanTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3153,6 +3153,49 @@ TEST_F(TableScanTest, mapIsNullFilter) {
"SELECT * FROM tmp WHERE c0 is null");
}

TEST_F(TableScanTest, stringIsNullFilter) {
constexpr int kSize = 1000;
const char* baseStrings[] = {
"qwertyuiopasdfghjklzxcvbnm",
"qazwsxedcrfvtgbyhnujmikolp",
};
auto indices = allocateIndices(kSize, pool_.get());
for (int i = 0; i < kSize; ++i) {
indices->asMutable<vector_size_t>()[i] = i % 2;
}
auto nullBuffer =
AlignedBuffer::allocate<bool>(kSize, pool_.get(), bits::kNotNull);
auto* rawNullBuffer = nullBuffer->asMutable<uint64_t>();
for (int i = 0; i < kSize; i += 100) {
bits::setNull(rawNullBuffer, i);
}
auto dict = BaseVector::wrapInDictionary(
nullBuffer,
indices,
kSize,
makeFlatVector<std::string>({baseStrings[0], baseStrings[1]}));

auto rows = makeRowVector({"c0", "c1"}, {dict, dict});
auto tableType = asRowType(rows->type());
auto file = TempFilePath::create();
writeToFile(file->getPath(), {rows});
createDuckDbTable({rows});

const auto outputType = ROW({"c1"}, {VARCHAR()});
auto makePlan = [&](const std::vector<std::string>& filters) {
return PlanBuilder()
.tableScan(outputType, filters, "", tableType)
.planNode();
};

assertQuery(
makePlan({"c0 is not null"}),
{file},
"SELECT c1 FROM tmp WHERE c0 is not null");
assertQuery(
makePlan({"c0 is null"}), {file}, "SELECT c1 FROM tmp WHERE c0 is null");
}

TEST_F(TableScanTest, compactComplexNulls) {
constexpr int kSize = 10;
auto iota = makeFlatVector<int64_t>(kSize, folly::identity);
Expand Down