Move VectorReader test to its own file, fixed some typos (#1330)

velox/experimental/codegen/CMakeLists.txt

@@ -26,6 +26,9 @@ add_subdirectory(code_generator)
 add_subdirectory(ast)
 add_subdirectory(udf_manager)
 add_subdirectory(utils)
+add_subdirectory(benchmark)
+add_subdirectory(functions)
+add_subdirectory(vector_function)
 
 add_library(velox_experimental_codegen Codegen.cpp CodegenStubs.cpp
                                        CodegenLogger.cpp)

velox/experimental/codegen/benchmark/CMakeLists.txt

@@ -13,11 +13,12 @@
 add_executable(
   velox_codegen_benchmark_single_output
   SingleOutputDefaultNullsBenchmark.cpp
-  SingleOutputNotDefaultNullsBenchmark.cpp BooleanBenchmark.cpp)
+  SingleOutputNotDefaultNullsBenchmark.cpp BooleanBenchmarks.cpp)
 
 target_link_libraries(
   velox_codegen_benchmark_single_output
   velox_exec_test_lib
+  velox_codegen_utils_resource_path
   velox_codegen_code_generator
   ${FOLLY_BENCHMARK}
   ${FOLLY_WITH_DEPENDENCIES}
@@ -26,7 +27,6 @@ target_link_libraries(
   ${GFLAGS_LIBRARIES}
   ${FMT}
   velox_core
-  koski_parser
   velox_exec_test_util
   velox_functions_lib
   velox_functions_common
@@ -46,8 +46,7 @@ add_compile_definitions(velox_codegen_test
 add_compile_definitions(
   velox_codegen_test
   CODEGEN="$<TARGET_LINKER_FILE:velox_codegen_code_generator>")
-add_compile_definitions(velox_codegen_test
-                        KOSKI_PARSER="$<TARGET_LINKER_FILE:koski_parser>")
+
 add_compile_definitions(
   velox_codegen_test
   VELOX_FUNCTIONS_LIB="$<TARGET_LINKER_FILE:velox_functions_lib>")
@@ -89,7 +88,7 @@ target_link_libraries(
   ${GLOG}
   ${FMT}
   velox_core
-  koski_parser
+  velox_codegen_utils_resource_path
   velox_exec_test_util
   velox_functions_lib
   velox_functions_common

velox/experimental/codegen/code_generator/tests/ArithmeticFunctionsTest.cpp

@@ -466,7 +466,11 @@ TEST_F(ArithmeticFunctionsTest, DISABLED_testRound) {
       "round(C0)", {{32}, {13}, {-13}, {1}, {-1}});
 }
 
-TEST_F(ArithmeticFunctionsTest, testHash) {
+// FIXME: This test errors on macs:
+// velox/velox/functions/common/HashImpl.h:22:31: error: implicit instantiation
+// of undefined template 'folly::hasher<std::__1::basic_string_view<char, \
+// std::__1::char_traits<char> >, void>'
+TEST_F(ArithmeticFunctionsTest, DISABLED_testHash) {
   StringView input("hi welcome");
   evaluateAndCompare<
       RowTypeTrait<TypeKind::VARCHAR>,

velox/experimental/codegen/functions/CMakeLists.txt

@@ -0,0 +1,14 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+add_library(velox_codegen_functions INTERFACE)
+target_link_libraries(velox_codegen_functions INTERFACE ${FMT})

velox/experimental/codegen/tests/CMakeLists.txt

@@ -15,12 +15,12 @@ add_dependencies(velox_experimental_codegen_test velox_codegen_expression_test)
 target_link_libraries(
   velox_experimental_codegen_test
   ${GTEST_BOTH_LIBRARIES}
-  velox_codegen_utils_resource_path
   velox_core
   velox_exec
   velox_experimental_codegen
   velox_exec_test_lib
   velox_exec_test_util
-  velox_functions_common)
+  velox_functions_common
+  velox_codegen_utils_resource_path)
 
 add_test(velox_experimental_codegen_test velox_experimental_codegen_test)

velox/experimental/codegen/vector_function/VectorReader-inl.h

@@ -33,7 +33,7 @@ namespace codegen {
 //   // true means set to null, false means not null
 //   static constexpr bool intializedWithNullSet_
 //
-//   // when true, the reader will never reveive a null value to write
+//   // when true, the reader will never receive a null value to write
 //   static constexpr bool mayWriteNull_
 //
 //
@@ -78,17 +78,13 @@ struct VectorReader {
 
     if constexpr (Config::isWriter_) {
       mutableRawNulls_ = flatVector->mutableRawNulls();
+      mutableRawValues_ = flatVector->mutableRawValues();
     } else {
       if constexpr (Config::mayReadNull_) {
         // TODO when read only vector does not have nulls we dont need to
         // allocate nulls
         mutableRawNulls_ = flatVector->mutableRawNulls();
       }
-    }
-
-    if constexpr (Config::isWriter_) {
-      mutableRawValues_ = flatVector->mutableRawValues();
-    } else {
       mutableRawValues_ = const_cast<NativeType*>(flatVector->rawValues());
     }
   }
@@ -183,17 +179,13 @@ struct VectorReader<
 
     if constexpr (Config::isWriter_) {
       mutableRawNulls_ = flatVector->mutableRawNulls();
+      mutableRawValues_ = flatVector->template mutableRawValues<uint64_t>();
     } else {
       // TODO when read only vector does not have nulls we dont need to allocate
       // nulls
       if constexpr (Config::mayReadNull_) {
         mutableRawNulls_ = flatVector->mutableRawNulls();
       }
-    }
-
-    if constexpr (Config::isWriter_) {
-      mutableRawValues_ = flatVector->template mutableRawValues<uint64_t>();
-    } else {
       mutableRawValues_ =
           const_cast<uint64_t*>(flatVector->template rawValues<uint64_t>());
     }
@@ -311,17 +303,13 @@ struct VectorReader<
 
     if constexpr (Config::isWriter_) {
       mutableRawNulls_ = flatVector->mutableRawNulls();
+      mutableRawValues_ = flatVector->template mutableRawValues<StringView>();
     } else {
       // TODO when read only vector does not have nulls we dont need to allocate
       // nulls
       if constexpr (Config::mayReadNull_) {
         mutableRawNulls_ = flatVector->mutableRawNulls();
       }
-    }
-
-    if constexpr (Config::isWriter_) {
-      mutableRawValues_ = flatVector->template mutableRawValues<StringView>();
-    } else {
       mutableRawValues_ =
           const_cast<StringView*>(flatVector->template rawValues<StringView>());
     }

velox/experimental/codegen/vector_function/tests/CMakeLists.txt

@@ -10,8 +10,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-add_executable(velox_codegen_vector_function_test
-               veloxCodegenVectorFunctionTest.cpp, TempStringTest.cpp)
+add_executable(
+  velox_codegen_vector_function_test CodegenVectorFunctionTest.cpp
+                                     TempStringTest.cpp VectorReaderTest.cpp)
 add_test(velox_codegen_vector_function_test velox_codegen_vector_function_test)
 
 target_link_libraries(
@@ -37,7 +38,7 @@ target_link_libraries(
   velox_dwio_type_fbhive
   velox_dwrf_test_utils
   velox_presto_serializer
-  velox_transform
+  velox_transform_utils
   ${ANTLR4_RUNTIME}
   ${Boost_ATOMIC_LIBRARIES}
   ${Boost_CONTEXT_LIBRARIES}

velox/experimental/codegen/vector_function/tests/CodegenVectorFunctionTest.cpp

@@ -76,38 +76,6 @@ TEST(TestConcat, BasicConcatRow) {
   EXPECT_EQ(std::get<1>(output), *std::get<0>(args) - *std::get<1>(args));
 }
 
-TEST(VectorReader, ReadDoublesVectors) {
-  const size_t vectorSize = 1000;
-  auto inRowType = ROW({"columnA", "columnB"}, {DOUBLE(), DOUBLE()});
-  auto outRowType = ROW({"expr1", "expr2"}, {DOUBLE(), DOUBLE()});
-
-  auto pool_ = memory::getDefaultScopedMemoryPool();
-  auto pool = pool_.get();
-  auto inRowVector = BaseVector::create(inRowType, vectorSize, pool);
-  auto outRowVector = BaseVector::create(outRowType, vectorSize, pool);
-
-  VectorPtr& in1 = inRowVector->as<RowVector>()->childAt(0);
-
-  SelectivityVector selectivityVector(vectorSize);
-  selectivityVector.setAll();
-  in1->resize(vectorSize);
-  in1->addNulls(nullptr, selectivityVector);
-  VectorReader<DoubleType, OutputReaderConfig<false, false>> writer(in1);
-  VectorReader<DoubleType, InputReaderConfig<false, false>> reader(in1);
-
-  for (size_t row = 0; row < vectorSize; row++) {
-    writer[row] = (double)row;
-  }
-
-  for (size_t row = 0; row < vectorSize; row++) {
-    ASSERT_DOUBLE_EQ((double)row, *reader[row]);
-  }
-
-  for (size_t row = 0; row < vectorSize; row++) {
-    ASSERT_DOUBLE_EQ(*reader[row], in1->asFlatVector<double>()->valueAt(row));
-  }
-}
-
 TEST(TestConcat, EvalConcatFunction) {
   const size_t rowLength = 1000;
   SelectivityVector rows(rowLength);
@@ -222,48 +190,6 @@ struct GeneratedVectorFunctionConfigBool {
   static constexpr bool isDefaultNullStrict = false;
 };
 
-TEST(VectorReader, ReadBoolVectors) {
-  // TODO: Move those to test class
-  auto pool_ = memory::getDefaultScopedMemoryPool();
-  auto pool = pool_.get();
-  const size_t vectorSize = 1000;
-
-  auto inRowType = ROW({"columnA", "columnB"}, {BOOLEAN(), BOOLEAN()});
-  auto outRowType = ROW({"expr1", "expr2"}, {BOOLEAN(), BOOLEAN()});
-
-  auto inRowVector = BaseVector::create(inRowType, vectorSize, pool);
-  auto outRowVector = BaseVector::create(outRowType, vectorSize, pool);
-
-  VectorPtr& inputVector = inRowVector->as<RowVector>()->childAt(0);
-  inputVector->resize(vectorSize);
-  VectorReader<BooleanType, InputReaderConfig<false, false>> reader(
-      inputVector);
-  VectorReader<BooleanType, OutputReaderConfig<false, false>> writer(
-      inputVector);
-
-  for (size_t row = 0; row < vectorSize; row++) {
-    writer[row] = row % 2 == 0;
-  }
-
-  // Check that writing of values to the reader was success
-  for (size_t row = 0; row < vectorSize; row++) {
-    ASSERT_DOUBLE_EQ((row % 2 == 0), *reader[row]);
-    ASSERT_DOUBLE_EQ(
-        (row % 2 == 0), inputVector->asFlatVector<bool>()->valueAt(row));
-  }
-
-  // Write a null at even indices
-  for (size_t row = 0; row < vectorSize; row++) {
-    if (row % 2) {
-      writer[row] = std::nullopt;
-    }
-  }
-
-  for (size_t row = 0; row < vectorSize; row++) {
-    ASSERT_EQ(inputVector->asFlatVector<bool>()->isNullAt(row), row % 2);
-  }
-}
-
 TEST(TestBooEvalVectorFunction, EvalBoolExpression) {
   // TODO: Move those to test class
   auto pool_ = memory::getDefaultScopedMemoryPool();

velox/experimental/codegen/vector_function/tests/VectorReaderTest.cpp

@@ -0,0 +1,95 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/Random.h>
+#include <glog/logging.h>
+#include <gtest/gtest.h>
+#include "velox/experimental/codegen/vector_function/GeneratedVectorFunction-inl.h" // NOLINT (CLANGTIDY  )
+#include "velox/experimental/codegen/vector_function/StringTypes.h"
+
+namespace facebook::velox::codegen {
+TEST(VectorReader, ReadDoublesVectors) {
+  const size_t vectorSize = 1000;
+  auto inRowType = ROW({"columnA", "columnB"}, {DOUBLE(), DOUBLE()});
+  auto outRowType = ROW({"expr1", "expr2"}, {DOUBLE(), DOUBLE()});
+
+  auto pool_ = memory::getDefaultScopedMemoryPool();
+  auto pool = pool_.get();
+  auto inRowVector = BaseVector::create(inRowType, vectorSize, pool);
+  auto outRowVector = BaseVector::create(outRowType, vectorSize, pool);
+
+  VectorPtr& in1 = inRowVector->as<RowVector>()->childAt(0);
+
+  SelectivityVector selectivityVector(vectorSize);
+  selectivityVector.setAll();
+  in1->resize(vectorSize);
+  in1->addNulls(nullptr, selectivityVector);
+  VectorReader<DoubleType, OutputReaderConfig<false, false>> writer(in1);
+  VectorReader<DoubleType, InputReaderConfig<false, false>> reader(in1);
+
+  for (size_t row = 0; row < vectorSize; row++) {
+    writer[row] = (double)row;
+  }
+
+  for (size_t row = 0; row < vectorSize; row++) {
+    ASSERT_DOUBLE_EQ((double)row, *reader[row]);
+  }
+
+  for (size_t row = 0; row < vectorSize; row++) {
+    ASSERT_DOUBLE_EQ(*reader[row], in1->asFlatVector<double>()->valueAt(row));
+  }
+}
+
+TEST(VectorReader, ReadBoolVectors) {
+  // TODO: Move those to test class
+  auto pool_ = memory::getDefaultScopedMemoryPool();
+  auto pool = pool_.get();
+  const size_t vectorSize = 1000;
+
+  auto inRowType = ROW({"columnA", "columnB"}, {BOOLEAN(), BOOLEAN()});
+  auto outRowType = ROW({"expr1", "expr2"}, {BOOLEAN(), BOOLEAN()});
+
+  auto inRowVector = BaseVector::create(inRowType, vectorSize, pool);
+  auto outRowVector = BaseVector::create(outRowType, vectorSize, pool);
+
+  VectorPtr& inputVector = inRowVector->as<RowVector>()->childAt(0);
+  inputVector->resize(vectorSize);
+  VectorReader<BooleanType, InputReaderConfig<false, false>> reader(
+      inputVector);
+  VectorReader<BooleanType, OutputReaderConfig<false, false>> writer(
+      inputVector);
+
+  for (size_t row = 0; row < vectorSize; row++) {
+    writer[row] = row % 2 == 0;
+  }
+
+  // Check that writing of values to the reader was success
+  for (size_t row = 0; row < vectorSize; row++) {
+    ASSERT_DOUBLE_EQ((row % 2 == 0), *reader[row]);
+    ASSERT_DOUBLE_EQ(
+        (row % 2 == 0), inputVector->asFlatVector<bool>()->valueAt(row));
+  }
+
+  // Write a null at even indices
+  for (size_t row = 0; row < vectorSize; row++) {
+    if (row % 2) {
+      writer[row] = std::nullopt;
+    }
+  }
+
+  for (size_t row = 0; row < vectorSize; row++) {
+    ASSERT_EQ(inputVector->asFlatVector<bool>()->isNullAt(row), row % 2);
+  }
+}
+} // namespace facebook::velox::codegen