chore: add tracking issue to NotImplementedError (#1639)

* chore: add tracking issue to NotImplemented Signed-off-by: TennyZhuang <[email protected]> * fix planner test Signed-off-by: TennyZhuang <[email protected]>
risingwavelabs · Apr 7, 2022 · 48d4f94 · 48d4f94
1 parent f066f84
commit 48d4f94
Show file tree

Hide file tree

Showing 27 changed files with 171 additions and 104 deletions.
diff --git a/src/common/src/error.rs b/src/common/src/error.rs
@@ -34,6 +34,44 @@ pub const RW_ERROR_GRPC_HEADER: &str = "risingwave-error-bin";
 
 pub type BoxedError = Box<dyn Error + Send + Sync>;
 
+#[derive(Debug, Clone, Copy)]
+pub struct TrackingIssue(Option<u32>);
+
+impl TrackingIssue {
+    pub fn new(id: u32) -> Self {
+        TrackingIssue(Some(id))
+    }
+
+    pub fn none() -> Self {
+        TrackingIssue(None)
+    }
+}
+
+impl From<u32> for TrackingIssue {
+    fn from(id: u32) -> Self {
+        TrackingIssue(Some(id))
+    }
+}
+
+impl From<Option<u32>> for TrackingIssue {
+    fn from(id: Option<u32>) -> Self {
+        TrackingIssue(id)
+    }
+}
+
+impl Display for TrackingIssue {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self.0 {
+            Some(id) => write!(
+                f,
+                "Tracking issue: https://github.com/singularity-data/risingwave/issues/{}",
+                id
+            ),
+            None => write!(f, "No tracking issue"),
+        }
+    }
+}
+
 #[derive(Error, Debug)]
 pub enum ErrorCode {
     #[error("ok")]
@@ -44,8 +82,8 @@ pub enum ErrorCode {
     InternalError(String),
     #[error(transparent)]
     ProstError(prost::DecodeError),
-    #[error("Feature is not yet implemented: {0}")]
-    NotImplementedError(String),
+    #[error("Feature is not yet implemented: {0}, {1}")]
+    NotImplemented(String, TrackingIssue),
     #[error(transparent)]
     IoError(IoError),
     #[error("Storage error: {0}")]
@@ -211,7 +249,7 @@ impl ErrorCode {
             ErrorCode::OK => 0,
             ErrorCode::InternalError(_) => 1,
             ErrorCode::MemoryError { .. } => 2,
-            ErrorCode::NotImplementedError(_) => 4,
+            ErrorCode::NotImplemented(..) => 4,
             ErrorCode::IoError(_) => 5,
             ErrorCode::StorageError(_) => 6,
             ErrorCode::ParseError(_) => 7,
@@ -510,7 +548,7 @@ mod tests {
         check_grpc_error(ErrorCode::TaskNotFound, Code::Internal);
         check_grpc_error(ErrorCode::InternalError(String::new()), Code::Internal);
         check_grpc_error(
-            ErrorCode::NotImplementedError(String::new()),
+            ErrorCode::NotImplemented(String::new(), None.into()),
             Code::Internal,
         );
     }

diff --git a/src/expr/src/expr/build_expr_from_prost.rs b/src/expr/src/expr/build_expr_from_prost.rs
@@ -35,7 +35,7 @@ fn get_return_type_and_children(prost: &ExprNode) -> Result<(Vec<ExprNode>, Data
     if let RexNode::FuncCall(func_call) = prost.get_rex_node()? {
         Ok((func_call.get_children().to_vec(), ret_type))
     } else {
-        Err(RwError::from(ErrorCode::NotImplementedError(
+        Err(RwError::from(ErrorCode::InternalError(
             "expects a function call".to_string(),
         )))
     }

diff --git a/src/expr/src/expr/expr_input_ref.rs b/src/expr/src/expr/expr_input_ref.rs
@@ -63,7 +63,7 @@ impl<'a> TryFrom<&'a ExprNode> for InputRefExpression {
                 idx: input_ref_node.column_idx as usize,
             })
         } else {
-            Err(RwError::from(ErrorCode::NotImplementedError(
+            Err(RwError::from(ErrorCode::InternalError(
                 "expects a input ref node".to_string(),
             )))
         }

diff --git a/src/expr/src/expr/expr_unary.rs b/src/expr/src/expr/expr_unary.rs
@@ -60,10 +60,10 @@ macro_rules! gen_cast_impl {
                 ),
             )*
             _ => {
-                return Err(ErrorCode::NotImplementedError(format!(
+                return Err(ErrorCode::NotImplemented(format!(
                     "CAST({:?} AS {:?}) not supported yet!",
                     $child.return_type(), $ret
-                ))
+                ), 1632.into())
                 .into());
             }
         }
@@ -159,10 +159,10 @@ macro_rules! gen_neg_impl {
                 ),
             )*
             _ => {
-                return Err(ErrorCode::NotImplementedError(format!(
+                return Err(ErrorCode::NotImplemented(format!(
                     "Neg is not supported on {:?}",
                     $child.return_type()
-                ))
+                ), 112.into())
                 .into());
             }
         }
@@ -251,10 +251,10 @@ pub fn new_unary_expr(
         (ProstType::PgSleep, _, DataType::Decimal) => Box::new(PgSleepExpression::new(child_expr)),
 
         (expr, ret, child) => {
-            return Err(ErrorCode::NotImplementedError(format!(
+            return Err(ErrorCode::NotImplemented(format!(
                 "The expression {:?}({:?}) ->{:?} using vectorized expression framework is not supported yet.",
                 expr, child, ret
-            ))
+            ), 112.into())
             .into());
         }
     };

diff --git a/src/frontend/src/binder/expr/binary_op.rs b/src/frontend/src/binder/expr/binary_op.rs
@@ -43,7 +43,7 @@ impl Binder {
             BinaryOperator::Or => ExprType::Or,
             BinaryOperator::Like => ExprType::Like,
             BinaryOperator::NotLike => return self.bind_not_like(bound_left, bound_right),
-            _ => return Err(ErrorCode::NotImplementedError(format!("{:?}", op)).into()),
+            _ => return Err(ErrorCode::NotImplemented(format!("{:?}", op), 112.into()).into()),
         };
         FunctionCall::new_or_else(func_type, vec![bound_left, bound_right], |inputs| {
             Self::err_unsupported_binary_op(op, inputs)
@@ -73,6 +73,6 @@ impl Binder {
             op,
             bound_right.return_type(),
         );
-        ErrorCode::NotImplementedError(desc).into()
+        ErrorCode::NotImplemented(desc, 112.into()).into()
     }
 }
diff --git a/src/frontend/src/binder/expr/function.rs b/src/frontend/src/binder/expr/function.rs
@@ -69,10 +69,10 @@ impl Binder {
                     ExprType::RoundDigit
                 }
                 _ => {
-                    return Err(ErrorCode::NotImplementedError(format!(
-                        "unsupported function: {:?}",
-                        function_name
-                    ))
+                    return Err(ErrorCode::NotImplemented(
+                        format!("unsupported function: {:?}", function_name),
+                        112.into(),
+                    )
                     .into())
                 }
             };
@@ -81,10 +81,11 @@ impl Binder {
             })?
             .into())
         } else {
-            Err(
-                ErrorCode::NotImplementedError(format!("unsupported function: {:?}", f.name))
-                    .into(),
+            Err(ErrorCode::NotImplemented(
+                format!("unsupported function: {:?}", f.name),
+                112.into(),
             )
+            .into())
         }
     }
 
@@ -93,10 +94,10 @@ impl Binder {
             .iter()
             .map(|i| format!("{:?}", i.return_type()))
             .join(",");
-        ErrorCode::NotImplementedError(format!(
-            "function {}({}) doesn't exist",
-            function_name, args
-        ))
+        ErrorCode::NotImplemented(
+            format!("function {}({}) doesn't exist", function_name, args),
+            112.into(),
+        )
         .into()
     }
 

diff --git a/src/frontend/src/binder/expr/mod.rs b/src/frontend/src/binder/expr/mod.rs
@@ -93,9 +93,11 @@ impl Binder {
                 self.bind_between(*expr, negated, *low, *high)?,
             ))),
             Expr::Extract { field, expr } => self.bind_extract(field, *expr),
-            _ => Err(
-                ErrorCode::NotImplementedError(format!("unsupported expression {:?}", expr)).into(),
-            ),
+            _ => Err(ErrorCode::NotImplemented(
+                format!("unsupported expression {:?}", expr),
+                112.into(),
+            )
+            .into()),
         }
     }
 
@@ -107,11 +109,14 @@ impl Binder {
                 self.bind_expr(expr)?,
             ],
             |inputs| {
-                ErrorCode::NotImplementedError(format!(
-                    "function extract({} from {:?}) doesn't exist",
-                    field,
-                    inputs[1].return_type()
-                ))
+                ErrorCode::NotImplemented(
+                    format!(
+                        "function extract({} from {:?}) doesn't exist",
+                        field,
+                        inputs[1].return_type()
+                    ),
+                    112.into(),
+                )
                 .into()
             },
         )?
@@ -126,21 +131,21 @@ impl Binder {
                 return self.rewrite_positive(expr);
             }
             _ => {
-                return Err(ErrorCode::NotImplementedError(format!(
-                    "unsupported unary expression: {:?}",
-                    op
-                ))
+                return Err(ErrorCode::NotImplemented(
+                    format!("unsupported unary expression: {:?}", op),
+                    112.into(),
+                )
                 .into())
             }
         };
         let expr = self.bind_expr(expr)?;
         let return_type = expr.return_type();
         FunctionCall::new(func_type, vec![expr])
             .ok_or_else(|| {
-                ErrorCode::NotImplementedError(format!(
-                    "unsupported unary expression {:?} {:?}",
-                    op, return_type
-                ))
+                ErrorCode::NotImplemented(
+                    format!("unsupported unary expression {:?} {:?}", op, return_type),
+                    112.into(),
+                )
                 .into()
             })
             .map(|f| f.into())
@@ -287,8 +292,9 @@ impl Binder {
         expr: Expr,
     ) -> Result<FunctionCall> {
         let expr = self.bind_expr(expr)?;
-        FunctionCall::new(func_type, vec![expr])
-            .ok_or_else(|| ErrorCode::NotImplementedError(format!("{:?}", &func_type)).into())
+        FunctionCall::new(func_type, vec![expr]).ok_or_else(|| {
+            ErrorCode::NotImplemented(format!("{:?}", &func_type), None.into()).into()
+        })
     }
 
     pub(super) fn bind_cast(&mut self, expr: Expr, data_type: AstDataType) -> Result<FunctionCall> {
@@ -319,10 +325,10 @@ pub fn bind_data_type(data_type: &AstDataType) -> Result<DataType> {
             datatype: Box::new(bind_data_type(datatype)?),
         },
         _ => {
-            return Err(ErrorCode::NotImplementedError(format!(
-                "unsupported data type: {:?}",
-                data_type
-            ))
+            return Err(ErrorCode::NotImplemented(
+                format!("unsupported data type: {:?}", data_type),
+                None.into(),
+            )
             .into())
         }
     };

diff --git a/src/frontend/src/binder/expr/subquery.rs b/src/frontend/src/binder/expr/subquery.rs
@@ -22,6 +22,6 @@ impl Binder {
             return Ok(Subquery::new(query, kind).into());
         }
 
-        Err(ErrorCode::NotImplementedError("correlated subquery".to_string()).into())
+        Err(ErrorCode::NotImplemented("correlated subquery".to_string(), 1343.into()).into())
     }
 }
diff --git a/src/frontend/src/binder/expr/value.rs b/src/frontend/src/binder/expr/value.rs
@@ -36,7 +36,7 @@ impl Binder {
                 last_field: None,
                 fractional_seconds_precision: None,
             } => self.bind_interval(value, leading_field),
-            _ => Err(ErrorCode::NotImplementedError(format!("{:?}", value)).into()),
+            _ => Err(ErrorCode::NotImplemented(format!("{:?}", value), None.into()).into()),
         }
     }
 

diff --git a/src/frontend/src/binder/insert.rs b/src/frontend/src/binder/insert.rs
@@ -58,7 +58,11 @@ impl Binder {
             // TODO: insert type cast for select exprs
             SetExpr::Select(_) => self.bind_query(source)?,
 
-            _ => return Err(ErrorCode::NotImplementedError(format!("{:?}", source.body)).into()),
+            _ => {
+                return Err(
+                    ErrorCode::NotImplemented(format!("{:?}", source.body), None.into()).into(),
+                )
+            }
         };
 
         let insert = BoundInsert {

diff --git a/src/frontend/src/binder/query.rs b/src/frontend/src/binder/query.rs
@@ -100,7 +100,9 @@ impl Binder {
         let name = match order_by_expr.expr {
             Expr::Identifier(name) => name.value,
             expr => {
-                return Err(ErrorCode::NotImplementedError(format!("ORDER BY {:?}", expr)).into())
+                return Err(
+                    ErrorCode::NotImplemented(format!("ORDER BY {:?}", expr), 1635.into()).into(),
+                )
             }
         };
         let index = *name_to_index

diff --git a/src/frontend/src/binder/relation.rs b/src/frontend/src/binder/relation.rs
@@ -144,7 +144,7 @@ impl Binder {
         Ok(match constraint {
             JoinConstraint::None => ExprImpl::literal_bool(true),
             JoinConstraint::Natural => {
-                return Err(ErrorCode::NotImplementedError("Natural join".into()).into())
+                return Err(ErrorCode::NotImplemented("Natural join".into(), 1633.into()).into())
             }
             JoinConstraint::On(expr) => {
                 let bound_expr = self.bind_expr(expr)?;
@@ -158,7 +158,7 @@ impl Binder {
                 bound_expr
             }
             JoinConstraint::Using(_columns) => {
-                return Err(ErrorCode::NotImplementedError("USING".into()).into())
+                return Err(ErrorCode::NotImplemented("USING".into(), 1636.into()).into())
             }
         })
     }
@@ -172,10 +172,10 @@ impl Binder {
                 } else {
                     let kind =
                         WindowTableFunctionKind::from_str(&name.0[0].value).map_err(|_| {
-                            ErrorCode::NotImplementedError(format!(
-                                "unknown window function kind: {}",
-                                name.0[0].value
-                            ))
+                            ErrorCode::NotImplemented(
+                                format!("unknown window function kind: {}", name.0[0].value),
+                                1191.into(),
+                            )
                         })?;
                     Ok(Relation::WindowTableFunction(Box::new(
                         self.bind_window_table_function(kind, args)?,
@@ -188,17 +188,17 @@ impl Binder {
                 alias,
             } => {
                 if lateral {
-                    Err(ErrorCode::NotImplementedError("unsupported lateral".into()).into())
+                    Err(ErrorCode::NotImplemented("unsupported lateral".into(), None.into()).into())
                 } else {
                     Ok(Relation::Subquery(Box::new(
                         self.bind_subquery_relation(*subquery, alias)?,
                     )))
                 }
             }
-            _ => Err(ErrorCode::NotImplementedError(format!(
-                "unsupported table factor {:?}",
-                table_factor
-            ))
+            _ => Err(ErrorCode::NotImplemented(
+                format!("unsupported table factor {:?}", table_factor),
+                None.into(),
+            )
             .into()),
         }
     }

diff --git a/src/frontend/src/binder/set_expr.rs b/src/frontend/src/binder/set_expr.rs
@@ -61,7 +61,7 @@ impl Binder {
         match set_expr {
             SetExpr::Select(s) => Ok(BoundSetExpr::Select(Box::new(self.bind_select(*s)?))),
             SetExpr::Values(v) => Ok(BoundSetExpr::Values(Box::new(self.bind_values(v, None)?))),
-            _ => Err(ErrorCode::NotImplementedError(format!("{:?}", set_expr)).into()),
+            _ => Err(ErrorCode::NotImplemented(format!("{:?}", set_expr), None.into()).into()),
         }
     }
 }