Skip to content

Commit

Permalink
Fix calling PrestoSerializer::flush after PrestoSerializer::maxSerial…
Browse files Browse the repository at this point in the history
…izedSize has wrong result (6973)
  • Loading branch information
rui-mo committed Oct 10, 2023
1 parent 07afb52 commit 8324109
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
10 changes: 7 additions & 3 deletions velox/serializers/PrestoSerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,10 +919,11 @@ class VectorStream {
child->flush(out);
}
writeInt32(out, nullCount_ + nonNullCount_);
if (nullCount_ + nonNullCount_ == 0) {
if (!counted_ && nullCount_ + nonNullCount_ == 0) {
// If nothing was added, there is still one offset in the wire
// format.
lengths_.appendOne<int32_t>(0);
counted_ = true;
}
lengths_.flush(out);
flushNulls(out);
Expand All @@ -931,10 +932,11 @@ class VectorStream {
case TypeKind::ARRAY:
children_[0]->flush(out);
writeInt32(out, nullCount_ + nonNullCount_);
if (nullCount_ + nonNullCount_ == 0) {
if (!counted_ && nullCount_ + nonNullCount_ == 0) {
// If nothing was added, there is still one offset in the wire
// format.
lengths_.appendOne<int32_t>(0);
counted_ = true;
}
lengths_.flush(out);
flushNulls(out);
Expand All @@ -946,10 +948,11 @@ class VectorStream {
// hash table size. -1 means not included in serialization.
writeInt32(out, -1);
writeInt32(out, nullCount_ + nonNullCount_);
if (nullCount_ + nonNullCount_ == 0) {
if (!counted_ && nullCount_ + nonNullCount_ == 0) {
// If nothing was added, there is still one offset in the wire
// format.
lengths_.appendOne<int32_t>(0);
counted_ = true;
}

lengths_.flush(out);
Expand Down Expand Up @@ -995,6 +998,7 @@ class VectorStream {
int32_t nullCount_{0};
int32_t totalLength_{0};
bool hasLengths_{false};
bool counted_{false};
ByteRange header_;
ByteStream nulls_;
ByteStream lengths_;
Expand Down
16 changes: 16 additions & 0 deletions velox/serializers/tests/PrestoSerializerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,22 @@ TEST_P(PrestoSerializerTest, ioBufRoundTrip) {
}
}

TEST_P(PrestoSerializerTest, roundTrip) {
VectorFuzzer::Options opts;
opts.timestampPrecision =
VectorFuzzer::Options::TimestampPrecision::kMilliSeconds;
opts.nullRatio = 0.1;
VectorFuzzer fuzzer(opts, pool_.get());

const size_t numRounds = 20;

for (size_t i = 0; i < numRounds; ++i) {
auto rowType = fuzzer.randRowType();
auto inputRowVector = fuzzer.fuzzInputRow(rowType);
testRoundTrip(inputRowVector);
}
}

INSTANTIATE_TEST_SUITE_P(
PrestoSerializerTest,
PrestoSerializerTest,
Expand Down

0 comments on commit 8324109

Please sign in to comment.