From 372cc43311f912780d9816c8eccd212597e0b579 Mon Sep 17 00:00:00 2001
From: Rui Mo <rui.mo@intel.com>
Date: Tue, 23 Aug 2022 15:39:26 +0800
Subject: [PATCH] Support selecting subfield from struct (#38)

Fix bigint range (#40)
---
 velox/substrait/SubstraitToVeloxExpr.cpp | 33 +++++++++++++++++++++---
 velox/type/Filter.cpp                    |  1 -
 velox/type/Filter.h                      |  4 +--
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/velox/substrait/SubstraitToVeloxExpr.cpp b/velox/substrait/SubstraitToVeloxExpr.cpp
index 7a2dc93a4fa1..563443de41f4 100644
--- a/velox/substrait/SubstraitToVeloxExpr.cpp
+++ b/velox/substrait/SubstraitToVeloxExpr.cpp
@@ -159,19 +159,43 @@ SubstraitVeloxExprConverter::toVeloxExpr(
     case ::substrait::Expression::FieldReference::ReferenceTypeCase::
         kDirectReference: {
       const auto& dRef = substraitField.direct_reference();
+      VELOX_CHECK(dRef.has_struct_field(), "Struct field expected.");
       int32_t colIdx = subParser_->parseReferenceSegment(dRef);
+      std::optional<int32_t> childIdx;
+      if (dRef.struct_field().has_child()) {
+        childIdx =
+            subParser_->parseReferenceSegment(dRef.struct_field().child());
+      }
+
+      const auto& inputTypes = inputType->children();
       const auto& inputNames = inputType->names();
       const int64_t inputSize = inputNames.size();
-      if (colIdx <= inputSize) {
-        const auto& inputTypes = inputType->children();
-        // Convert type to row.
+
+      if (colIdx >= inputSize) {
+        VELOX_FAIL("Missing the column with id '{}' .", colIdx);
+      }
+
+      if (!childIdx.has_value()) {
         return std::make_shared<core::FieldAccessTypedExpr>(
             inputTypes[colIdx],
             std::make_shared<core::InputTypedExpr>(inputTypes[colIdx]),
             inputNames[colIdx]);
       } else {
-        VELOX_FAIL("Missing the column with id '{}' .", colIdx);
+        // Select a subfield in a struct by name.
+        if (auto inputColumnType = asRowType(inputTypes[colIdx])) {
+          if (childIdx.value() >= inputColumnType->size()) {
+            VELOX_FAIL("Missing the subfield with id '{}' .", childIdx.value());
+          }
+          return std::make_shared<core::FieldAccessTypedExpr>(
+              inputColumnType->childAt(childIdx.value()),
+              std::make_shared<core::FieldAccessTypedExpr>(
+                  inputTypes[colIdx], inputNames[colIdx]),
+              inputColumnType->nameOf(childIdx.value()));
+        } else {
+          VELOX_FAIL("RowType expected.");
+        }
       }
+      break;
     }
     default:
       VELOX_NYI(
@@ -250,6 +274,7 @@ SubstraitVeloxExprConverter::toVeloxExpr(
   if (veloxFunction == "alias") {
     return toAliasExpr(std::move(params));
   }
+
   if (veloxFunction == "is_not_null") {
     return toIsNotNullExpr(std::move(params), toVeloxType(typeName));
   }
diff --git a/velox/type/Filter.cpp b/velox/type/Filter.cpp
index 83c57b64075e..b348fea5b9d6 100644
--- a/velox/type/Filter.cpp
+++ b/velox/type/Filter.cpp
@@ -576,7 +576,6 @@ std::unique_ptr<Filter> createBigintValuesFilter(
       min, max, values, nullAllowed);
 }
 
-
 std::unique_ptr<Filter> createBigintValues(
     const std::vector<int64_t>& values,
     bool nullAllowed) {
diff --git a/velox/type/Filter.h b/velox/type/Filter.h
index 4b7adf3f5bea..c76f1f18b3c4 100644
--- a/velox/type/Filter.h
+++ b/velox/type/Filter.h
@@ -564,8 +564,8 @@ class BigintRange final : public Filter {
       bool upperExclusive,
       bool nullAllowed)
       : Filter(true, nullAllowed, FilterKind::kBigintRange),
-        lower_(lowerExclusive ? lower - 1 : lower),
-        upper_(upperExclusive ? upper + 1 : upper),
+        lower_(lowerExclusive ? lower + 1 : lower),
+        upper_(upperExclusive ? upper - 1 : upper),
         lower32_(std::max<int64_t>(lower, std::numeric_limits<int32_t>::min())),
         upper32_(std::min<int64_t>(upper, std::numeric_limits<int32_t>::max())),
         lower16_(std::max<int64_t>(lower, std::numeric_limits<int16_t>::min())),