Merge branch 'main' into sts/remove_table_properties

risingwavelabs · Feb 4, 2024 · b213bc5 · b213bc5
2 parents f5aa9af + afc3028
commit b213bc5
Show file tree

Hide file tree

Showing 305 changed files with 1,777 additions and 1,204 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lints/Cargo.lock b/lints/Cargo.lock
diff --git a/lints/Cargo.toml b/lints/Cargo.toml
@@ -22,6 +22,7 @@ itertools = "0.12"
 dylint_testing = "2.6.0"
 
 # UI test dependencies
+anyhow = "1"
 tracing = "0.1"
 
 [package.metadata.rust-analyzer]

diff --git a/lints/src/format_error.rs b/lints/src/format_error.rs
@@ -62,7 +62,8 @@ impl_lint_pass!(FormatError => [FORMAT_ERROR]);
 
 const TRACING_FIELD_DEBUG: [&str; 3] = ["tracing_core", "field", "debug"];
 const TRACING_FIELD_DISPLAY: [&str; 3] = ["tracing_core", "field", "display"];
-const TRACING_MACRO_EVENT: [&str; 3] = ["tracing", "macros", "event"];
+const TRACING_MACROS_EVENT: [&str; 3] = ["tracing", "macros", "event"];
+const ANYHOW_MACROS_ANYHOW: [&str; 3] = ["anyhow", "macros", "anyhow"];
 
 impl<'tcx> LateLintPass<'tcx> for FormatError {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@@ -76,12 +77,14 @@ impl<'tcx> LateLintPass<'tcx> for FormatError {
             .or_else(|| match_function_call(cx, expr, &TRACING_FIELD_DISPLAY))
             && let [arg_expr, ..] = args
         {
-            check_fmt_arg(cx, arg_expr);
+            check_fmt_arg_in_tracing_event(cx, arg_expr);
         }
 
-        // `{}`, `{:?}` in format macros.
-        let in_tracing_macro_event = macro_backtrace(expr.span)
-            .any(|macro_call| match_def_path(cx, macro_call.def_id, &TRACING_MACRO_EVENT));
+        // Indirect `{}`, `{:?}` from other macros.
+        let in_tracing_event_macro = macro_backtrace(expr.span)
+            .any(|macro_call| match_def_path(cx, macro_call.def_id, &TRACING_MACROS_EVENT));
+        let in_anyhow_macro = macro_backtrace(expr.span)
+            .any(|macro_call| match_def_path(cx, macro_call.def_id, &ANYHOW_MACROS_ANYHOW));
 
         for macro_call in macro_backtrace(expr.span) {
             if is_format_macro(cx, macro_call.def_id)
@@ -93,8 +96,14 @@ impl<'tcx> LateLintPass<'tcx> for FormatError {
                         && let Some(arg) = format_args.arguments.all_args().get(index)
                         && let Ok(arg_expr) = find_format_arg_expr(expr, arg)
                     {
-                        if in_tracing_macro_event {
+                        if in_tracing_event_macro {
                             check_fmt_arg_in_tracing_event(cx, arg_expr);
+                        } else if in_anyhow_macro {
+                            if format_args.template.len() == 1 {
+                                check_fmt_arg_in_anyhow_error(cx, arg_expr);
+                            } else {
+                                check_fmt_arg_in_anyhow_context(cx, arg_expr);
+                            }
                         } else {
                             check_fmt_arg(cx, arg_expr);
                         }
@@ -113,25 +122,44 @@ impl<'tcx> LateLintPass<'tcx> for FormatError {
 }
 
 fn check_fmt_arg(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
-    check_arg(
+    check_fmt_arg_with_help(
         cx,
         arg_expr,
-        arg_expr.span,
         "consider importing `thiserror_ext::AsReport` and using `.as_report()` instead",
-    );
+    )
 }
 
 fn check_fmt_arg_in_tracing_event(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
     // TODO: replace `<error>` with the actual code snippet.
-    check_arg(
+    check_fmt_arg_with_help(
+        cx,
+        arg_expr,
+        "consider importing `thiserror_ext::AsReport` and recording the error as a field \
+        with `error = %<error>.as_report()` instead",
+    );
+}
+
+fn check_fmt_arg_in_anyhow_error(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
+    check_fmt_arg_with_help(
         cx,
         arg_expr,
-        arg_expr.span,
-        "consider importing `thiserror_ext::AsReport` and recording the error as a field
-with `error = %<error>.as_report()` instead",
+        "consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting it",
     );
 }
 
+fn check_fmt_arg_in_anyhow_context(cx: &LateContext<'_>, arg_expr: &Expr<'_>) {
+    check_fmt_arg_with_help(
+        cx,
+        arg_expr,
+        "consider using `anyhow::Error::context`, `anyhow::Context::(with_)context` to \
+        attach additional message to the error and make it an error source instead",
+    );
+}
+
+fn check_fmt_arg_with_help(cx: &LateContext<'_>, arg_expr: &Expr<'_>, help: &str) {
+    check_arg(cx, arg_expr, arg_expr.span, help);
+}
+
 fn check_to_string_call(cx: &LateContext<'_>, receiver: &Expr<'_>, to_string_span: Span) {
     check_arg(
         cx,

diff --git a/lints/ui/format_error.rs b/lints/ui/format_error.rs
@@ -48,4 +48,11 @@ fn main() {
     let _ = (err.clone()).to_string();
     let _ = err.to_string().to_string();
     let _ = (&&err).to_string();
+
+    use anyhow::anyhow;
+
+    let _ = anyhow!("{}", err);
+    let _ = anyhow!("{:?}", err);
+    let _ = anyhow!("some error occurred: {}", err);
+    let _ = anyhow!("some error occurred: {:?}", err);
 }
diff --git a/lints/ui/format_error.stderr b/lints/ui/format_error.stderr
@@ -118,74 +118,71 @@ error: should not format error directly
 LL |     info!("{}", err);
    |                 ^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field
-           with `error = %<error>.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:32:20
    |
 LL |     my_info!("{}", err);
    |                    ^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field
-           with `error = %<error>.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:34:29
    |
 LL |     tracing::field::display(&err);
    |                             ^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:35:27
    |
 LL |     tracing::field::debug(err.clone());
    |                           ^^^^^^^^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:37:5
    |
 LL |     info!(%err, "233");
    |     ^^^^^^^^^^^^^^^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:38:5
    |
 LL |     info!(?err, "233");
    |     ^^^^^^^^^^^^^^^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:39:23
    |
 LL |     info!(%err, "{}", err);
    |                       ^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field
-           with `error = %<error>.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:39:5
    |
 LL |     info!(%err, "{}", err);
    |     ^^^^^^^^^^^^^^^^^^^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:40:13
    |
 LL |     let _ = info_span!("span", %err);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = help: consider importing `thiserror_ext::AsReport` and using `.as_report()` instead
+   = help: consider importing `thiserror_ext::AsReport` and recording the error as a field with `error = %<error>.as_report()` instead
 
 error: should not format error directly
   --> $DIR/format_error.rs:44:9
@@ -243,5 +240,37 @@ LL |     let _ = (&&err).to_string();
    |
    = help: consider importing `thiserror_ext::AsReport` and using `.to_report_string()` instead
 
-error: aborting due to 30 previous errors
+error: should not format error directly
+  --> $DIR/format_error.rs:54:27
+   |
+LL |     let _ = anyhow!("{}", err);
+   |                           ^^^
+   |
+   = help: consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting it
+
+error: should not format error directly
+  --> $DIR/format_error.rs:55:29
+   |
+LL |     let _ = anyhow!("{:?}", err);
+   |                             ^^^
+   |
+   = help: consider directly wrapping the error with `anyhow::anyhow!(..)` instead of formatting it
+
+error: should not format error directly
+  --> $DIR/format_error.rs:56:48
+   |
+LL |     let _ = anyhow!("some error occurred: {}", err);
+   |                                                ^^^
+   |
+   = help: consider using `anyhow::Error::context`, `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead
+
+error: should not format error directly
+  --> $DIR/format_error.rs:57:50
+   |
+LL |     let _ = anyhow!("some error occurred: {:?}", err);
+   |                                                  ^^^
+   |
+   = help: consider using `anyhow::Error::context`, `anyhow::Context::(with_)context` to attach additional message to the error and make it an error source instead
+
+error: aborting due to 34 previous errors
 
diff --git a/proto/plan_common.proto b/proto/plan_common.proto
@@ -61,7 +61,7 @@ message ColumnDesc {
 
   ColumnDescVersion version = 10;
 
-  AdditionalColumn additional_columns = 11;
+  AdditionalColumn additional_column = 11;
 }
 
 message ColumnCatalog {

diff --git a/proto/telemetry.proto b/proto/telemetry.proto
@@ -2,6 +2,8 @@ syntax = "proto3";
 
 package telemetry;
 
+option go_package = "risingwavelabs.com/risingwave/proto/telemetry";
+
 enum MetaBackend {
   META_BACKEND_UNSPECIFIED = 0;
   META_BACKEND_MEMORY = 1;

diff --git a/src/batch/clippy.toml b/src/batch/clippy.toml
@@ -1,11 +1,6 @@
-disallowed-methods = [
-]
+disallowed-methods = []
 
-disallowed-types = [
-    { path = "risingwave_common::error::ErrorCode", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::RwError", reason = "Please use per-crate error type instead." },
-    { path = "risingwave_common::error::Result", reason = "Please use per-crate error type instead." },
-]
+disallowed-types = []
 
 doc-valid-idents = [
     "RisingWave",
@@ -16,7 +11,7 @@ doc-valid-idents = [
     "PostgreSQL",
     "MySQL",
     "TopN",
-    "VNode"
+    "VNode",
 ]
 avoid-breaking-exported-api = false
 upper-case-acronyms-aggressive = true

diff --git a/src/batch/src/error.rs b/src/batch/src/error.rs
@@ -18,7 +18,7 @@ use std::sync::Arc;
 
 pub use anyhow::anyhow;
 use risingwave_common::array::ArrayError;
-use risingwave_common::error::{BoxedError, ErrorCode, RwError};
+use risingwave_common::error::BoxedError;
 use risingwave_common::util::value_encoding::error::ValueEncodingError;
 use risingwave_dml::error::DmlError;
 use risingwave_expr::ExprError;
@@ -145,19 +145,6 @@ impl From<tonic::Status> for BatchError {
     }
 }
 
-impl From<BatchError> for RwError {
-    fn from(s: BatchError) -> Self {
-        ErrorCode::BatchError(Box::new(s)).into()
-    }
-}
-
-// TODO(error-handling): remove after eliminating RwError from connector.
-impl From<RwError> for BatchError {
-    fn from(s: RwError) -> Self {
-        Self::Internal(anyhow!(s))
-    }
-}
-
 impl<'a> From<&'a BatchError> for Status {
     fn from(err: &'a BatchError) -> Self {
         err.to_status(tonic::Code::Internal, "batch")

diff --git a/src/bench/Cargo.toml b/src/bench/Cargo.toml
@@ -8,6 +8,7 @@ license = { workspace = true }
 repository = { workspace = true }
 
 [dependencies]
+anyhow = "1"
 async-trait = "0.1"
 aws-config = { workspace = true }
 aws-sdk-s3 = { workspace = true }

diff --git a/src/bench/s3_bench/main.rs b/src/bench/s3_bench/main.rs
@@ -28,7 +28,6 @@ use futures::stream::{self, StreamExt};
 use futures::{future, Future, FutureExt};
 use itertools::Itertools;
 use rand::{Rng, SeedableRng};
-use risingwave_common::error::RwError;
 use tokio::join;
 use tokio::sync::RwLock;
 use tracing::debug;
@@ -233,7 +232,7 @@ async fn multi_part_upload(
             let part_t = Instant::now();
             let result = a.send().await.unwrap();
             let part_ttl = part_t.elapsed();
-            Ok::<_, RwError>((result, part_ttl))
+            Ok::<_, anyhow::Error>((result, part_ttl))
         })
         .collect_vec();
     let ttfb = t.elapsed();
@@ -318,7 +317,7 @@ async fn multi_part_get(
         .into_iter()
         .map(create_part_get)
         .map(|resp| async move {
-            let result: Result<(usize, Duration), RwError> = Ok((
+            let result: anyhow::Result<(usize, Duration)> = Ok((
                 resp.await
                     .unwrap()
                     .body
@@ -381,7 +380,7 @@ async fn run_case(
     cfg: Arc<Config>,
     client: Arc<Client>,
     objs: Arc<RwLock<ObjPool>>,
-) -> Result<(), RwError> {
+) -> anyhow::Result<()> {
     let (name, analysis) = match case.clone() {
         Case::Put {
             name,