Skip to content

Commit a3c52a5

Browse files
peterenescumeta-codesync[bot]
authored andcommitted
fix(nimble): Preserve MAP spec in flat map column readers (facebookincubator#15803)
Summary: X-link: facebookincubator/nimble#369 Pull Request resolved: facebookincubator#15803 A production issue S602695 VeloxRuntimeError blocking feature injection pipeline exposed an issue in the Nimble flat map reader. For context, earlier this half ROO feature injection was enabled in production and supported by both Velox map and flat map vector encodings. A particular pipeline was blocked this past weekend due to flat map reader crashing. A temporary mitigation was to use regular map instead of the flat map encoding as it appeared flat map reader was attempting to set values that were uninitialized by the reader. The culprit was how scan spec is constructed for flat map. Rather than appending all features to keys and values children, we should follow map scan spec construction by preserving the keys and values and create child columns using the preserved values child spec. Reviewed By: Yuhta Differential Revision: D89326920 fbshipit-source-id: e4b70115c8642b2cd6f4e8ee6bcf3e0ebae44d2a
1 parent 7b10ae2 commit a3c52a5

File tree

1 file changed

+10
-36
lines changed

1 file changed

+10
-36
lines changed

velox/dwio/common/SelectiveFlatMapColumnReader.cpp

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -57,42 +57,16 @@ void SelectiveFlatMapColumnReader::getValues(
5757
auto* resultFlatMap = prepareResult(*result, keysVector_, rows.size());
5858
setComplexNulls(rows, *result);
5959

60-
for (const auto& childSpec : scanSpec_->children()) {
61-
VELOX_TRACE_HISTORY_PUSH("getValues %s", childSpec->fieldName().c_str());
62-
if (!childSpec->keepValues()) {
63-
continue;
64-
}
65-
66-
VELOX_CHECK(
67-
childSpec->readFromFile(),
68-
"Flatmap children must always be read from file.");
69-
70-
if (childSpec->subscript() == kConstantChildSpecSubscript) {
71-
continue;
72-
}
73-
74-
const auto channel = childSpec->channel();
75-
const auto index = childSpec->subscript();
76-
auto& childResult = resultFlatMap->mapValuesAt(channel);
77-
78-
VELOX_CHECK(
79-
!childSpec->deltaUpdate(),
80-
"Delta update not supported in flat map yet");
81-
VELOX_CHECK(
82-
!childSpec->isConstant(),
83-
"Flat map values cannot be constant in scanSpec.");
84-
VELOX_CHECK_EQ(
85-
childSpec->columnType(),
86-
velox::common::ScanSpec::ColumnType::kRegular,
87-
"Flat map only supports regular column types in scan spec.");
88-
89-
children_[index]->getValues(rows, &childResult);
90-
91-
for (size_t i = 0; i < children_.size(); ++i) {
92-
const auto& inMap = inMapBuffer(i);
93-
if (inMap) {
94-
resultFlatMap->inMapsAt(i, true) = inMap;
95-
}
60+
// Loop over column readers
61+
for (int i = 0; i < children_.size(); ++i) {
62+
auto& child = children_[i];
63+
VectorPtr values;
64+
child->getValues(rows, &values);
65+
resultFlatMap->mapValuesAt(i) = values;
66+
67+
const auto& inMap = inMapBuffer(i);
68+
if (inMap) {
69+
resultFlatMap->inMapsAt(i, true) = inMap;
9670
}
9771
}
9872
}

0 commit comments

Comments
 (0)