From eb3a1dd951a201b0a8ad6839947766c6fd6a936b Mon Sep 17 00:00:00 2001
From: Jimmy Lu <jimmylu@meta.com>
Date: Tue, 20 Aug 2024 10:07:38 -0700
Subject: [PATCH] Schema evolution support for reader value hooks (#72)

Summary:
Pull Request resolved: https://github.com/facebookincubator/nimble/pull/72

X-link: https://github.com/facebookincubator/velox/pull/10755

Currently reader value hook is not considering schema evolution at all, this change fix that.

Reviewed By: kevinwilfong

Differential Revision: D61229494

fbshipit-source-id: 729bb90611fb3164282b524376eda20985a30194
---
 dwio/nimble/encodings/DictionaryEncoding.h     | 13 +++++--------
 dwio/nimble/encodings/MainlyConstantEncoding.h |  3 +--
 dwio/nimble/encodings/RleEncoding.h            |  3 +--
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/dwio/nimble/encodings/DictionaryEncoding.h b/dwio/nimble/encodings/DictionaryEncoding.h
index 0aa6b18..42568d7 100644
--- a/dwio/nimble/encodings/DictionaryEncoding.h
+++ b/dwio/nimble/encodings/DictionaryEncoding.h
@@ -137,19 +137,16 @@ class DictionaryIndicesHook : public velox::ValueHook {
     return false;
   }
 
-  void addValue(vector_size_t i, const void* value) final {
-    indices_[i - offset_] = *reinterpret_cast<const uint32_t*>(value);
+  void addValue(vector_size_t i, int64_t value) final {
+    indices_[i - offset_] = value;
   }
 
   void addValues(
       const vector_size_t* rows,
-      const void* values,
-      vector_size_t size,
-      uint8_t valueWidth) final {
-    NIMBLE_DASSERT(valueWidth == sizeof(uint32_t), "");
-    auto* indices = reinterpret_cast<const uint32_t*>(values);
+      const int32_t* values,
+      vector_size_t size) final {
     for (vector_size_t i = 0; i < size; ++i) {
-      indices_[rows[i] - offset_] = indices[i];
+      indices_[rows[i] - offset_] = values[i];
     }
   }
 
diff --git a/dwio/nimble/encodings/MainlyConstantEncoding.h b/dwio/nimble/encodings/MainlyConstantEncoding.h
index 5bbef06..b17ea99 100644
--- a/dwio/nimble/encodings/MainlyConstantEncoding.h
+++ b/dwio/nimble/encodings/MainlyConstantEncoding.h
@@ -301,8 +301,7 @@ void MainlyConstantEncoding<T>::bulkScan(
   visitor.setRowIndex(visitor.numRows());
   if constexpr (V::kHasHook) {
     NIMBLE_DASSERT(numValues == numNonNulls, "");
-    visitor.hook().addValues(
-        scatterRows, values, numNonNulls, sizeof(ValueType));
+    visitor.hook().addValues(scatterRows, values, numNonNulls);
   } else {
     visitor.addNumValues(V::kFilterOnly ? numHits : numValues);
   }
diff --git a/dwio/nimble/encodings/RleEncoding.h b/dwio/nimble/encodings/RleEncoding.h
index 38fcdcd..614c297 100644
--- a/dwio/nimble/encodings/RleEncoding.h
+++ b/dwio/nimble/encodings/RleEncoding.h
@@ -377,8 +377,7 @@ void RLEEncoding<T>::bulkScan(
   }
   if constexpr (V::kHasHook) {
     NIMBLE_DASSERT(numValues == numNonNulls, "");
-    visitor.hook().addValues(
-        scatterRows, values, numNonNulls, sizeof(ValueType));
+    visitor.hook().addValues(scatterRows, values, numNonNulls);
   } else {
     visitor.addNumValues(V::kFilterOnly ? numHits : numValues);
   }