fix review

Author: jinchengchenghh

velox/substrait/SubstraitToVeloxPlan.cpp

@@ -1763,66 +1763,34 @@ core::TypedExprPtr SubstraitVeloxPlanConverter::connectWithAnd(
   auto inputType = ROW(std::move(inputNameList), std::move(inputTypeList));
 
   // Filter for scalar functions.
-  std::shared_ptr<const core::ITypedExpr> scalarFilter = nullptr;
-  if (scalarFunctions.size() > 0) {
-    scalarFilter = exprConverter_->toVeloxExpr(scalarFunctions[0], inputType);
-    // Will connect multiple functions with AND.
-    uint32_t idx = 1;
-    while (idx < scalarFunctions.size()) {
-      scalarFilter = connectWithAnd(
-          scalarFilter,
-          exprConverter_->toVeloxExpr(scalarFunctions[idx], inputType));
-      idx += 1;
+  std::vector<std::shared_ptr<const core::ITypedExpr>> allFilters;
+  ;
+  for (auto scalar : scalarFunctions) {
+    auto filter = exprConverter_->toVeloxExpr(scalar, inputType);
+    if (filter != nullptr) {
+      allFilters.emplace_back(filter);
     }
   }
 
-  // Filter for OrList.
-  std::shared_ptr<const core::ITypedExpr> orListFilter = nullptr;
-  if (singularOrLists.size() > 0) {
-    orListFilter = exprConverter_->toVeloxExpr(singularOrLists[0], inputType);
-    uint32_t idx = 1;
-    while (idx < singularOrLists.size()) {
-      orListFilter = connectWithAnd(
-          orListFilter,
-          exprConverter_->toVeloxExpr(singularOrLists[idx], inputType));
-      idx += 1;
+  for (auto orList : singularOrLists) {
+    auto filter = exprConverter_->toVeloxExpr(orList, inputType);
+    if (filter != nullptr) {
+      allFilters.emplace_back(filter);
     }
   }
 
-  std::shared_ptr<const core::ITypedExpr> ifThenFilter = nullptr;
-  if (ifThens.size() > 0) {
-    ifThenFilter = exprConverter_->toVeloxExpr(ifThens[0], inputType);
-    uint32_t idx = 1;
-    while (idx < ifThens.size()) {
-      ifThenFilter = connectWithAnd(
-          ifThenFilter, exprConverter_->toVeloxExpr(ifThens[idx], inputType));
-      idx += 1;
+  for (auto ifThen : ifThens) {
+    auto filter = exprConverter_->toVeloxExpr(ifThen, inputType);
+    if (filter != nullptr) {
+      allFilters.emplace_back(filter);
     }
   }
-
-  VELOX_CHECK(
-      scalarFilter != nullptr || orListFilter != nullptr ||
-          ifThenFilter != nullptr,
-      "One filter should be valid.");
-  if (scalarFilter != nullptr && orListFilter != nullptr) {
-    auto filter = connectWithAnd(scalarFilter, orListFilter);
-    if (ifThenFilter != nullptr) {
-      return connectWithAnd(filter, ifThenFilter);
-    }
-    return filter;
-  } else if (scalarFilter != nullptr) {
-    if (ifThenFilter != nullptr) {
-      return connectWithAnd(scalarFilter, ifThenFilter);
-    }
-    return scalarFilter;
-  } else if (orListFilter != nullptr) {
-    if (ifThenFilter != nullptr) {
-      return connectWithAnd(orListFilter, ifThenFilter);
-    }
-    return orListFilter;
-  } else {
-    return ifThenFilter;
+  VELOX_CHECK_GT(allFilters.size(), 0, "One filter should be valid.")
+  std::shared_ptr<const core::ITypedExpr> andFilter = allFilters[0];
+  for (auto i = 1; i < allFilters.size(); i++) {
+    andFilter = connectWithAnd(andFilter, allFilters[i]);
   }
+  return andFilter;
 }
 
 core::TypedExprPtr SubstraitVeloxPlanConverter::connectWithAnd(