fix: Revert PR 1137 (facebookincubator#11743)

Summary:
Pull Request resolved: facebookincubator#11743

This diff reverts facebookincubator#11737 which broke dozens of tests.

Reviewed By: Yuhta

Differential Revision: D66773506

fbshipit-source-id: 8f0ac9ec66d1b05bee5ebd45e5e94fee6cc80120

velox/common/base/tests/IndexedPriorityQueueTest.cpp

@@ -232,6 +232,7 @@ TEST_F(IndexedPriorityQueueTest, remove) {
 }
 
 TEST_F(IndexedPriorityQueueTest, fuzz) {
+  const int numIterations{1000};
   std::mt19937 rng{100};
   IndexedPriorityQueue<uint32_t, true> maxQueue;
   fuzz<uint32_t, true>(maxQueue, 1'000, rng);

velox/common/base/tests/ScratchTest.cpp

@@ -35,6 +35,7 @@ TEST(ScratchTest, basic) {
   {
     ScratchPtr<int32_t> ints(scratch);
     ScratchPtr<int64_t> longs(scratch);
+    auto tempLongs = longs.get(2000);
     auto tempInts = ints.get(1000);
     std::fill(tempInts, tempInts + 1000, -1);
     std::fill(tempInts, tempInts + 2000, -1);

velox/common/base/tests/SimdUtilTest.cpp

@@ -171,6 +171,7 @@ TEST_F(SimdUtilTest, gather32) {
 
 TEST_F(SimdUtilTest, gather64) {
   int32_t indices4[4] = {3, 2, 1, 0};
+  int32_t indices3[4] = {3, 2, 1, 1 << 31};
   int64_t data[4] = {44, 55, 66, 77};
   constexpr int kBatchSize = xsimd::batch<int64_t>::size;
   const int32_t* indices = indices4 + (4 - kBatchSize);

velox/common/memory/tests/AllocationPoolTest.cpp

@@ -53,6 +53,7 @@ TEST_F(AllocationPoolTest, hugePages) {
   allocationPool->setHugePageThreshold(128 << 10);
   int32_t counter = 0;
   for (;;) {
+    int32_t usedKB = 0;
     allocationPool->newRun(32 << 10);
     // Initial allocations round up to 64K
     EXPECT_EQ(1, allocationPool->numRanges());

velox/common/memory/tests/ArbitrationParticipantTest.cpp

@@ -1370,6 +1370,7 @@ DEBUG_ONLY_TEST_F(ArbitrationParticipantTest, reclaimLock) {
   auto task = createTask(kMemoryCapacity);
   const auto config = arbitrationConfig();
   auto participant = ArbitrationParticipant::create(10, task->pool(), &config);
+  const uint64_t allocatedBytes = 32 * MB;
   for (int i = 0; i < 32; ++i) {
     task->allocate(MB);
   }

velox/common/memory/tests/MemoryAllocatorTest.cpp

@@ -637,6 +637,7 @@ TEST_P(MemoryAllocatorTest, allocationClass2) {
 
 TEST_P(MemoryAllocatorTest, stats) {
   const std::vector<MachinePageCount>& sizes = instance_->sizeClasses();
+  MachinePageCount capacity = kCapacityPages;
   for (auto i = 0; i < sizes.size(); ++i) {
     std::unique_ptr<Allocation> allocation = std::make_unique<Allocation>();
     auto size = sizes[i];
@@ -962,6 +963,7 @@ TEST_P(MemoryAllocatorTest, allocContiguous) {
   for (const auto& testData : testSettings) {
     SCOPED_TRACE(fmt::format("{} useMmap{}", testData.debugString(), useMmap_));
     setupAllocator();
+    const MachinePageCount nonContiguousPages = 100;
     Allocation allocation;
     if (testData.nonContiguousPages != 0) {
       instance_->allocateNonContiguous(testData.nonContiguousPages, allocation);
@@ -1063,6 +1065,7 @@ TEST_P(MemoryAllocatorTest, allocContiguousFail) {
     SCOPED_TRACE(
         fmt::format("{} useMmap {}", testData.debugString(), useMmap_));
     setupAllocator();
+    const MachinePageCount nonContiguousPages = 100;
     Allocation allocation;
     if (testData.nonContiguousPages != 0) {
       instance_->allocateNonContiguous(testData.nonContiguousPages, allocation);
@@ -1373,6 +1376,7 @@ TEST_P(MemoryAllocatorTest, StlMemoryAllocator) {
     // Allocation from classes and ContiguousAllocation outside size
     // classes.
     constexpr int32_t kNumDoubles = 256 * 1024;
+    size_t capacity = 0;
     for (auto i = 0; i < kNumDoubles; i++) {
       data.push_back(i);
     }

velox/common/memory/tests/MemoryManagerTest.cpp

@@ -554,6 +554,7 @@ TEST_F(MemoryManagerTest, concurrentPoolAccess) {
   std::atomic<bool> stopCheck{false};
   std::thread checkThread([&]() {
     while (!stopCheck) {
+      const int numPools = manager.numPools();
       std::this_thread::sleep_for(std::chrono::microseconds(1));
     }
   });

velox/common/memory/tests/MemoryPoolTest.cpp

@@ -144,6 +144,7 @@ class MemoryPoolTest : public testing::TestWithParam<TestParam> {
 };
 
 TEST_P(MemoryPoolTest, ctor) {
+  constexpr uint16_t kAlignment = 64;
   setupMemory({.alignment = 64, .allocatorCapacity = kDefaultCapacity});
   MemoryManager& manager = *getMemoryManager();
   const int64_t capacity = 4 * GB;
@@ -689,6 +690,7 @@ TEST_P(MemoryPoolTest, alignmentCheck) {
     ASSERT_EQ(
         pool->alignment(),
         alignment == 0 ? MemoryAllocator::kMinAlignment : alignment);
+    const int32_t kTestIterations = 10;
     for (int32_t i = 0; i < 10; ++i) {
       const int64_t bytesToAlloc = 1 + folly::Random::rand32() % (1 * MB);
       void* ptr = pool->allocate(bytesToAlloc);
@@ -1026,6 +1028,11 @@ TEST_P(MemoryPoolTest, contiguousAllocate) {
       ASSERT_GE(numAllocatedPages, 0);
       allocations.erase(allocations.begin() + freeAllocationIdx);
     }
+    const MachinePageCount minSizeClass = folly::Random().oneIn(4)
+        ? 0
+        : std::min(
+              manager->allocator()->largestSizeClass(),
+              folly::Random().rand32() % kMaxAllocationPages);
     pool->allocateContiguous(pagesToAllocate, allocation);
     numAllocatedPages += allocation.numPages();
     for (int32_t j = 0; j < allocation.size(); ++j) {
@@ -1075,6 +1082,7 @@ TEST_P(MemoryPoolTest, contiguousAllocateExceedLimit) {
 TEST_P(MemoryPoolTest, badContiguousAllocation) {
   auto manager = getMemoryManager();
   auto pool = manager->addLeafPool("badContiguousAllocation");
+  constexpr MachinePageCount kAllocSize = 8;
   ContiguousAllocation allocation;
   ASSERT_THROW(pool->allocateContiguous(0, allocation), VeloxRuntimeError);
 }
@@ -2492,6 +2500,7 @@ TEST_P(MemoryPoolTest, concurrentUpdateToSharedPools) {
 TEST_P(MemoryPoolTest, concurrentPoolStructureAccess) {
   folly::Random::DefaultGenerator rng;
   rng.seed(1234);
+  constexpr int64_t kMaxMemory = 8 * GB;
   MemoryManager& manager = *getMemoryManager();
   auto root = manager.addRootPool();
   std::atomic<int64_t> poolId{0};
@@ -2656,6 +2665,7 @@ TEST(MemoryPoolTest, debugMode) {
 
 TEST(MemoryPoolTest, debugModeWithFilter) {
   constexpr int64_t kMaxMemory = 10 * GB;
+  constexpr int64_t kNumIterations = 100;
   const std::vector<int64_t> kAllocSizes = {128, 8 * KB, 2 * MB};
   const std::vector<bool> debugEnabledSet{true, false};
   for (const auto& debugEnabled : debugEnabledSet) {

velox/connectors/hive/tests/HiveDataSinkTest.cpp

@@ -976,6 +976,7 @@ DEBUG_ONLY_TEST_F(HiveDataSinkTest, sortWriterAbortDuringFinish) {
     dataSink->appendData(vector);
   }
 
+  std::atomic_int injectCount{0};
   SCOPED_TESTVALUE_SET(
       "facebook::velox::dwrf::Writer::write",
       std::function<void(dwrf::Writer*)>([&](dwrf::Writer* /*unused*/) {

velox/dwio/common/tests/BitPackDecoderBenchmark.cpp

@@ -542,6 +542,7 @@ void naiveDecodeBitsLE(
     }
     return;
   }
+  auto lastSafe = bufferEnd - sizeof(uint64_t);
   int32_t numSafeRows = numRows;
   bool anyUnsafe = false;
   if (bufferEnd) {

velox/dwio/common/tests/IntDecoderBenchmark.cpp

@@ -910,6 +910,7 @@ BENCHMARK_RELATIVE(decodeNew_16) {
 BENCHMARK(decodeOld_32) {
   size_t currentLen = len_u32;
   const size_t startingLen = len_u32;
+  int32_t i = 0;
   while (currentLen != 0) {
     auto result =
         readVuLong(buffer_u32.data() + (startingLen - currentLen), currentLen);

velox/dwio/common/tests/utils/DataSetBuilder.cpp

@@ -254,6 +254,7 @@ DataSetBuilder& DataSetBuilder::makeMapStringValues(
     auto keyKind = map->type()->childAt(0)->kind();
     auto valueKind = map->type()->childAt(1)->kind();
     auto offsets = map->rawOffsets();
+    auto sizes = map->rawSizes();
     int32_t offsetIndex = 0;
     auto mapSize = map->size();
     auto getNextOffset = [&]() {

velox/dwio/dwrf/test/CacheInputTest.cpp

@@ -476,6 +476,8 @@ TEST_F(CacheTest, window) {
   auto cacheInput = dynamic_cast<CacheInputStream*>(stream.get());
   EXPECT_TRUE(cacheInput != nullptr);
   ASSERT_EQ(cacheInput->getName(), "CacheInputStream 0 of 13631488");
+  auto maxSize =
+      allocator_->sizeClasses().back() * memory::AllocationTraits::kPageSize;
   const void* buffer;
   int32_t size;
   int32_t numRead = 0;

velox/dwio/dwrf/test/ColumnWriterTest.cpp

@@ -1873,6 +1873,7 @@ int64_t generateRangeWithCustomLimits(
   // Generate the range such that we have similar amounts of values generated
   // for each exponent.
   double center = size % 2 ? -0.5 : interval / 2 - 0.5;
+  double value = center + (i - size / 2) * interval;
   // Return a guard-railed value with the numeric limits.
   // NOTE: There can be a more compact way to write this if we cast i and size
   // to signed types, but it's not worth the effort enforcing the assumptions.

velox/dwio/dwrf/test/DecryptionTests.cpp

@@ -43,7 +43,7 @@ TEST(Decryption, NoKeyProvider) {
   auto type = parser.parse("struct<a:int>");
   proto::Footer footer;
   ProtoUtils::writeType(*type, footer);
-  footer.mutable_encryption();
+  auto enc = footer.mutable_encryption();
   TestDecrypterFactory factory;
   ASSERT_THROW(
       DecryptionHandler::create(footer, &factory), exception::LoggedException);

velox/dwio/dwrf/test/E2EReaderTest.cpp

@@ -231,6 +231,7 @@ TEST_P(E2EReaderTest, SharedDictionaryFlatmapReadAsStruct) {
         ASSERT_TRUE(resultTypeRow.childAt(col)->isMap());
         ASSERT_EQ(batchRow->childAt(col)->typeKind(), TypeKind::MAP);
         auto& resultTypeChild = resultTypeRow.childAt(col)->as<TypeKind::MAP>();
+        auto* batchRowChild = batchRow->childAt(col)->as<MapVector>();
         ASSERT_EQ(
             resultTypeChild.keyType()->kind(), schemaChild.keyType()->kind());
         ASSERT_EQ(

velox/dwio/dwrf/test/E2EWriterTest.cpp

@@ -1960,6 +1960,7 @@ TEST_F(E2EWriterTest, memoryReclaimAfterClose) {
     VELOX_ASSERT_THROW(writer->flush(), "Writer is not running");
 
     memory::MemoryReclaimer::Stats stats;
+    const auto oldCapacity = writerPool->capacity();
     writerPool->reclaim(1L << 30, 0, stats);
     if (testData.abort || !testData.canReclaim) {
       ASSERT_EQ(stats.numNonReclaimableAttempts, 0);

velox/dwio/parquet/reader/IntegerColumnReader.h

@@ -68,6 +68,7 @@ class IntegerColumnReader : public dwio::common::SelectiveIntegerColumnReader {
       int64_t offset,
       const RowSet& rows,
       const uint64_t* /*incomingNulls*/) override {
+    auto& data = formatData_->as<ParquetData>();
     VELOX_WIDTH_DISPATCH(
         parquetSizeOfIntKind(fileType_->type()->kind()),
         prepareRead,

velox/dwio/parquet/reader/TimestampColumnReader.h

@@ -133,6 +133,7 @@ class TimestampColumnReader : public IntegerColumnReader {
       int64_t offset,
       const RowSet& rows,
       const uint64_t* /*incomingNulls*/) override {
+    auto& data = formatData_->as<ParquetData>();
     // Use int128_t as a workaround. Timestamp in Velox is of 16-byte length.
     prepareRead<int128_t>(offset, rows, nullptr);
     readCommon<TimestampColumnReader, true>(rows);

velox/dwio/parquet/tests/reader/ParquetReaderTest.cpp

@@ -1365,6 +1365,7 @@ TEST_F(ParquetReaderTest, arrayOfMapOfIntKeyArrayValue) {
   facebook::velox::dwio::common::ReaderOptions readerOptions{leafPool_.get()};
   auto reader = createReader(sample, readerOptions);
   EXPECT_EQ(reader->rowType()->toString(), expectedVeloxType);
+  auto numRows = reader->numberOfRows();
   auto type = reader->typeWithId();
   RowReaderOptions rowReaderOpts;
   auto rowType = ROW({"test"}, {ARRAY(MAP(VARCHAR(), ARRAY(INTEGER())))});
@@ -1398,6 +1399,7 @@ TEST_F(ParquetReaderTest, arrayOfMapOfIntKeyStructValue) {
   facebook::velox::dwio::common::ReaderOptions readerOptions{leafPool_.get()};
   auto reader = createReader(sample, readerOptions);
   EXPECT_EQ(reader->rowType()->toString(), expectedVeloxType);
+  auto numRows = reader->numberOfRows();
   auto type = reader->typeWithId();
   RowReaderOptions rowReaderOpts;
   auto rowType = reader->rowType();
@@ -1431,6 +1433,7 @@ TEST_F(ParquetReaderTest, struct_of_array_of_array) {
       getExampleFilePath("struct_of_array_of_array.parquet"));
   facebook::velox::dwio::common::ReaderOptions readerOptions{leafPool_.get()};
   auto reader = createReader(sample, readerOptions);
+  auto numRows = reader->numberOfRows();
   auto type = reader->typeWithId();
   EXPECT_EQ(type->size(), 1ULL);
   EXPECT_EQ(reader->rowType()->toString(), expectedVeloxType);
@@ -1504,6 +1507,7 @@ TEST_F(ParquetReaderTest, testLzoDataPage) {
   rowReaderOpts.setScanSpec(makeScanSpec(outputRowType));
   auto rowReader = reader->createRowReader(rowReaderOpts);
 
+  uint64_t total = 0;
   VectorPtr result = BaseVector::create(outputRowType, 0, &*leafPool_);
   rowReader->next(23'547ULL, result);
   EXPECT_EQ(23'547ULL, result->size());

velox/exec/HashJoinBridge.cpp

@@ -256,7 +256,7 @@ void HashJoinBridge::appendSpilledHashTablePartitionsLocked(
   }
   auto spillPartitionIdSet = toSpillPartitionIdSet(spillPartitionSet);
   if (restoringSpillPartitionId_.has_value()) {
-    for ([[maybe_unused]] const auto& id : spillPartitionIdSet) {
+    for (const auto& id : spillPartitionIdSet) {
       VELOX_DCHECK_LT(
           restoringSpillPartitionId_->partitionBitOffset(),
           id.partitionBitOffset());

velox/exec/fuzzer/DuckQueryRunner.cpp

@@ -423,6 +423,18 @@ std::optional<std::string> DuckQueryRunner::toSql(
 
 std::optional<std::string> DuckQueryRunner::toSql(
     const std::shared_ptr<const core::NestedLoopJoinNode>& joinNode) {
+  const auto& joinKeysToSql = [](auto keys) {
+    std::stringstream out;
+    for (auto i = 0; i < keys.size(); ++i) {
+      if (i > 0) {
+        out << ", ";
+      }
+      out << keys[i]->name();
+    }
+    return out.str();
+  };
+
+  const auto& outputNames = joinNode->outputType()->names();
   std::stringstream sql;
 
   // Nested loop join without filter.

velox/exec/fuzzer/PrestoQueryRunner.cpp

@@ -638,6 +638,18 @@ std::optional<std::string> PrestoQueryRunner::toSql(
 
 std::optional<std::string> PrestoQueryRunner::toSql(
     const std::shared_ptr<const core::NestedLoopJoinNode>& joinNode) {
+  const auto& joinKeysToSql = [](auto keys) {
+    std::stringstream out;
+    for (auto i = 0; i < keys.size(); ++i) {
+      if (i > 0) {
+        out << ", ";
+      }
+      out << keys[i]->name();
+    }
+    return out.str();
+  };
+
+  const auto& outputNames = joinNode->outputType()->names();
   std::stringstream sql;
 
   // Nested loop join without filter.

velox/exec/tests/AggregationTest.cpp

@@ -1012,6 +1012,7 @@ TEST_F(AggregationTest, largeValueRangeArray) {
 
 TEST_F(AggregationTest, partialAggregationMemoryLimitIncrease) {
   constexpr int64_t kGB = 1 << 30;
+  constexpr int64_t kB = 1 << 10;
   auto vectors = {
       makeRowVector({makeFlatVector<int32_t>(
           100, [](auto row) { return row; }, nullEvery(5))}),
@@ -1103,6 +1104,7 @@ TEST_F(AggregationTest, partialAggregationMaybeReservationReleaseCheck) {
 
   constexpr int64_t kGB = 1 << 30;
   const int64_t kMaxPartialMemoryUsage = 1 * kGB;
+  const int64_t kMaxUserMemoryUsage = 2 * kMaxPartialMemoryUsage;
   // Make sure partial aggregation runs out of memory after first batch.
   CursorParameters params;
   params.queryCtx = core::QueryCtx::create(executor_.get());

velox/exec/tests/HashJoinTest.cpp

@@ -6816,6 +6816,7 @@ TEST_F(HashJoinTest, leftJoinPreserveProbeOrder) {
 }
 
 DEBUG_ONLY_TEST_F(HashJoinTest, minSpillableMemoryReservation) {
+  constexpr int64_t kMaxBytes = 1LL << 30; // 1GB
   VectorFuzzer fuzzer({.vectorSize = 1000}, pool());
   const int32_t numBuildVectors = 10;
   std::vector<RowVectorPtr> buildVectors;
@@ -7365,6 +7366,7 @@ DEBUG_ONLY_TEST_F(HashJoinTest, exceptionDuringFinishJoinBuild) {
 
 DEBUG_ONLY_TEST_F(HashJoinTest, arbitrationTriggeredDuringParallelJoinBuild) {
   std::unique_ptr<memory::MemoryManager> memoryManager = createMemoryManager();
+  const auto& arbitrator = memoryManager->arbitrator();
   const uint64_t numDrivers = 2;
 
   // Large build side key product to bump hash mode to kHash instead of kArray

velox/exec/tests/OutputBufferManagerTest.cpp

@@ -329,6 +329,7 @@ class OutputBufferManagerTest : public testing::Test {
         return;
       }
       const int64_t maxBytes = folly::Random().oneIn(4, rng) ? 32'000'000 : 1;
+      int64_t receivedSequence;
       bool atEnd{false};
       folly::EventCount dataWait;
       auto dataWaitKey = dataWait.prepareWait();

velox/exec/tests/PrefixSortTest.cpp

@@ -151,6 +151,7 @@ const RowVectorPtr PrefixSortTest::generateExpectedResult(
 }
 
 TEST_F(PrefixSortTest, singleKey) {
+  const int numRows = 5;
   const int columnsSize = 7;
 
   // Vectors without nulls.
@@ -182,6 +183,7 @@ TEST_F(PrefixSortTest, singleKey) {
 }
 
 TEST_F(PrefixSortTest, singleKeyWithNulls) {
+  const int numRows = 5;
   const int columnsSize = 7;
 
   Timestamp ts = {5, 5};

velox/exec/tests/RowContainerTest.cpp

@@ -2479,7 +2479,7 @@ TEST_F(RowContainerTest, invalidatedColumnStats) {
     EXPECT_EQ(data->columnStats(4)->numCells(), 0);
 
     for (int i = 0; i < kNumRows; ++i) {
-      data->newRow();
+      auto row = data->newRow();
     }
     EXPECT_EQ(kNumRows, data->numRows());
     RowContainerIterator iter;

velox/exec/tests/SortBufferTest.cpp

@@ -503,6 +503,7 @@ DEBUG_ONLY_TEST_P(SortBufferTest, spillDuringInput) {
   const std::shared_ptr<memory::MemoryPool> fuzzerPool =
       memory::memoryManager()->addLeafPool("spillDuringInput");
   VectorFuzzer fuzzer({.vectorSize = 1024}, fuzzerPool.get());
+  uint64_t totalNumInput{0};
 
   ASSERT_EQ(memory::spillMemoryPool()->stats().usedBytes, 0);
   const auto peakSpillMemoryUsage =
@@ -554,6 +555,7 @@ DEBUG_ONLY_TEST_P(SortBufferTest, spillDuringOutput) {
   const std::shared_ptr<memory::MemoryPool> fuzzerPool =
       memory::memoryManager()->addLeafPool("spillDuringOutput");
   VectorFuzzer fuzzer({.vectorSize = 1024}, fuzzerPool.get());
+  uint64_t totalNumInput{0};
 
   ASSERT_EQ(memory::spillMemoryPool()->stats().usedBytes, 0);
   const auto peakSpillMemoryUsage =