refactor: remove stale AggKind::RowCount (#3774)

* refactor: remove stale AggKind::RowCount * update doc Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
risingwavelabs · Jul 11, 2022 · f93d8e3 · f93d8e3
1 parent 2ee98b5
commit f93d8e3
Show file tree

Hide file tree

Showing 10 changed files with 10 additions and 40 deletions.
diff --git a/src/expr/src/expr/agg.rs b/src/expr/src/expr/agg.rs
@@ -24,7 +24,6 @@ pub enum AggKind {
     Max,
     Sum,
     Count,
-    RowCount,
     Avg,
     StringAgg,
     SingleValue,
@@ -38,7 +37,6 @@ impl std::fmt::Display for AggKind {
             AggKind::Max => write!(f, "max"),
             AggKind::Sum => write!(f, "sum"),
             AggKind::Count => write!(f, "count"),
-            AggKind::RowCount => write!(f, "row_count"),
             AggKind::Avg => write!(f, "avg"),
             AggKind::StringAgg => write!(f, "string_agg"),
             AggKind::SingleValue => write!(f, "single_value"),
@@ -75,9 +73,6 @@ impl AggKind {
             Self::Count => Type::Count,
             Self::StringAgg => Type::StringAgg,
             Self::SingleValue => Type::SingleValue,
-            Self::RowCount => {
-                panic!("cannot convert RowCount to prost, TODO: remove RowCount from AggKind")
-            }
             Self::ApproxCountDistinct => Type::ApproxCountDistinct,
         }
     }

diff --git a/src/frontend/src/expr/agg_call.rs b/src/frontend/src/expr/agg_call.rs
@@ -52,13 +52,6 @@ impl AggCall {
     /// Infer the return type for the given agg call.
     /// Returns error if not supported or the arguments are invalid.
     pub fn infer_return_type(agg_kind: &AggKind, inputs: &[DataType]) -> Result<DataType> {
-        let unsupported = || {
-            let args = inputs.iter().map(|t| format!("{:?}", t)).join(", ");
-            Err(RwError::from(ErrorCode::NotImplemented(
-                format!("Unsupported aggregation: {}({})", agg_kind, args),
-                112.into(),
-            )))
-        };
         let invalid = || {
             let args = inputs.iter().map(|t| format!("{:?}", t)).join(", ");
             Err(RwError::from(ErrorCode::InvalidInputSyntax(format!(
@@ -107,9 +100,6 @@ impl AggCall {
             // SingleValue
             (AggKind::SingleValue, [input]) => input.clone(),
             (AggKind::SingleValue, _) => return invalid(),
-
-            // Others
-            _ => return unsupported(),
         };
 
         Ok(return_type)

diff --git a/src/frontend/src/optimizer/plan_node/logical_agg.rs b/src/frontend/src/optimizer/plan_node/logical_agg.rs
@@ -103,9 +103,7 @@ impl PlanAggCall {
             | AggKind::StringAgg
             | AggKind::SingleValue => self.agg_kind.clone(),
 
-            AggKind::Count | AggKind::RowCount | AggKind::Sum | AggKind::ApproxCountDistinct => {
-                AggKind::Sum
-            }
+            AggKind::Count | AggKind::Sum | AggKind::ApproxCountDistinct => AggKind::Sum,
         };
         PlanAggCall {
             agg_kind: total_agg_kind,
@@ -184,7 +182,6 @@ impl LogicalAgg {
                 }
                 AggKind::Sum
                 | AggKind::Count
-                | AggKind::RowCount
                 | AggKind::Avg
                 | AggKind::SingleValue
                 | AggKind::ApproxCountDistinct => {

diff --git a/src/stream/src/executor/aggregation/agg_call.rs b/src/stream/src/executor/aggregation/agg_call.rs
@@ -20,7 +20,7 @@ use risingwave_expr::expr::AggKind;
 /// An aggregation function may accept 0, 1 or 2 arguments.
 #[derive(Clone, Debug)]
 pub enum AggArgs {
-    /// `None` is used for aggregation function accepts 0 arguments, such as [`AggKind::RowCount`].
+    /// `None` is used for aggregation function accepts 0 arguments, such as `count(*)`.
     None,
     /// `Unary` is used for aggregation function accepts 1 argument, such as [`AggKind::Sum`].
     Unary(DataType, usize),

diff --git a/src/stream/src/executor/aggregation/mod.rs b/src/stream/src/executor/aggregation/mod.rs
@@ -269,12 +269,6 @@ pub fn create_streaming_agg_state(
         }
         [] => {
             match (agg_type, return_type, datum) {
-                // `AggKind::Count` for partial/local Count(*) == RowCount while `AggKind::Sum` for
-                // final/global Count(*)
-                (AggKind::RowCount, DataType::Int64, Some(datum)) => {
-                    Box::new(StreamingRowCountAgg::with_row_cnt(datum))
-                }
-                (AggKind::RowCount, DataType::Int64, None) => Box::new(StreamingRowCountAgg::new()),
                 // According to the function header comments and the link, Count(*) == RowCount
                 // `StreamingCountAgg` does not count `NULL`, so we use `StreamingRowCountAgg` here.
                 (AggKind::Count, DataType::Int64, Some(datum)) => {

diff --git a/src/stream/src/executor/global_simple_agg.rs b/src/stream/src/executor/global_simple_agg.rs
@@ -321,7 +321,7 @@ mod tests {
         let append_only = false;
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only,

diff --git a/src/stream/src/executor/hash_agg.rs b/src/stream/src/executor/hash_agg.rs
@@ -573,7 +573,7 @@ mod tests {
         let append_only = false;
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only,
@@ -660,7 +660,7 @@ mod tests {
         let append_only = false;
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only,
@@ -755,7 +755,7 @@ mod tests {
         let keys = vec![0];
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only: false,
@@ -844,7 +844,7 @@ mod tests {
         let append_only = true;
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only,

diff --git a/src/stream/src/executor/integration_tests.rs b/src/stream/src/executor/integration_tests.rs
@@ -60,7 +60,7 @@ async fn test_merger_sum_aggr() {
             input.boxed(),
             vec![
                 AggCall {
-                    kind: AggKind::RowCount,
+                    kind: AggKind::Count,
                     args: AggArgs::None,
                     return_type: DataType::Int64,
                     append_only,

diff --git a/src/stream/src/executor/local_simple_agg.rs b/src/stream/src/executor/local_simple_agg.rs
@@ -183,7 +183,7 @@ mod tests {
         tx.push_barrier(3, false);
 
         let agg_calls = vec![AggCall {
-            kind: AggKind::RowCount,
+            kind: AggKind::Count,
             args: AggArgs::None,
             return_type: DataType::Int64,
             append_only: false,
@@ -237,7 +237,7 @@ mod tests {
         // This is local simple aggregation, so we add another row count state
         let agg_calls = vec![
             AggCall {
-                kind: AggKind::RowCount,
+                kind: AggKind::Count,
                 args: AggArgs::None,
                 return_type: DataType::Int64,
                 append_only: false,

diff --git a/src/stream/src/executor/managed_state/aggregation/mod.rs b/src/stream/src/executor/managed_state/aggregation/mod.rs
@@ -164,12 +164,6 @@ impl<S: StateStore> ManagedStateImpl<S> {
                     ManagedValueState::new(agg_call, row_count, pk, state_table).await?,
                 ))
             }
-            AggKind::RowCount => {
-                assert!(is_row_count);
-                Ok(Self::Value(
-                    ManagedValueState::new(agg_call, row_count, pk, state_table).await?,
-                ))
-            }
             AggKind::SingleValue => Ok(Self::Value(
                 ManagedValueState::new(agg_call, row_count, pk, state_table).await?,
             )),