Remove implicit fall through in Spark SumAggregates (facebookincubato…

…r#9081) Summary: An implicit fall through was detected in SumAggregates.cpp ; Implicit fallthroughs have a very high bug rate, so they are being made a compiler error by default. This change removes the implicit fall through by adding a VELOX_CHECK and removing the if. Pull Request resolved: facebookincubator#9081 Reviewed By: mbasmanova, r-barnes Differential Revision: D54904041 Pulled By: kgpai fbshipit-source-id: c01013cfca30c11dec9e08747a83636f35ed9ce4
karteekmurthys · Mar 14, 2024 · 704113a · 704113a
1 parent 177ff56
commit 704113a
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 6 deletions.
diff --git a/velox/experimental/wave/exec/WaveOperator.cpp b/velox/experimental/wave/exec/WaveOperator.cpp
@@ -45,6 +45,7 @@ void WaveOperator::definesSubfields(
         defines_[Value(field)] = operand;
       }
     }
+      [[fallthrough]];
       // TODO:Add cases for nested types.
     default: {
       return;

diff --git a/velox/functions/sparksql/aggregates/AverageAggregate.cpp b/velox/functions/sparksql/aggregates/AverageAggregate.cpp
@@ -183,6 +183,7 @@ exec::AggregateRegistrationResult registerAverage(
                 return std::make_unique<DecimalAverageAggregateBase<int64_t>>(
                     resultType);
               }
+              [[fallthrough]];
             default:
               VELOX_FAIL(
                   "Unsupported result type for final aggregation: {}",

diff --git a/velox/functions/sparksql/aggregates/SumAggregate.cpp b/velox/functions/sparksql/aggregates/SumAggregate.cpp
@@ -113,12 +113,11 @@ exec::AggregateRegistrationResult registerSum(
                 BIGINT());
           }
           case TypeKind::HUGEINT: {
-            if (inputType->isLongDecimal()) {
-              // If inputType is long decimal,
-              // its output type is always long decimal.
-              return std::make_unique<exec::SimpleAggregateAdapter<
-                  DecimalSumAggregate<int128_t, int128_t>>>(resultType);
-            }
+            VELOX_CHECK(inputType->isLongDecimal());
+            // If inputType is long decimal,
+            // its output type is always long decimal.
+            return std::make_unique<exec::SimpleAggregateAdapter<
+                DecimalSumAggregate<int128_t, int128_t>>>(resultType);
           }
           case TypeKind::REAL:
             if (resultType->kind() == TypeKind::REAL) {
@@ -147,6 +146,7 @@ exec::AggregateRegistrationResult registerSum(
                   DecimalSumAggregate<int128_t, int128_t>>>(resultType);
             }
           }
+            [[fallthrough]];
           default:
             VELOX_CHECK(
                 false,

diff --git a/velox/vector/tests/VectorSaverTest.cpp b/velox/vector/tests/VectorSaverTest.cpp
@@ -59,6 +59,7 @@ class VectorSaverTest : public testing::Test, public VectorTestBase {
           // different values in its flat children of size 1.
           break;
         }
+        [[fallthrough]];
       case VectorEncoding::Simple::DICTIONARY:
         if (expected->valueVector()) {
           ASSERT_TRUE(actual->valueVector() != nullptr);