From e4c2590a54c216a100afc2385e66fce27f552e6a Mon Sep 17 00:00:00 2001 From: comphead Date: Mon, 11 Sep 2023 16:34:13 -0700 Subject: [PATCH 1/6] Make backtrace as a cargo feature --- datafusion/common/src/error.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs index 3b17c467ad1d..9e1bfd3a9625 100644 --- a/datafusion/common/src/error.rs +++ b/datafusion/common/src/error.rs @@ -419,13 +419,21 @@ impl DataFusionError { .to_string() } + #[cfg(backtrace)] + #[inline(always)] pub fn get_back_trace() -> String { let back_trace = Backtrace::capture(); if back_trace.status() == BacktraceStatus::Captured { return format!("\n\nbacktrace: {}", back_trace); } - "".to_string() + "".to_owned() + } + + #[cfg(not(backtrace))] + #[inline(always)] + pub fn get_back_trace() -> String { + "".to_owned() } } From 176698d82ef6c3bb98f13ea3e42b550866994783 Mon Sep 17 00:00:00 2001 From: comphead Date: Mon, 11 Sep 2023 17:00:44 -0700 Subject: [PATCH 2/6] clippy --- datafusion/common/src/error.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs index 9e1bfd3a9625..812aff8653e7 100644 --- a/datafusion/common/src/error.rs +++ b/datafusion/common/src/error.rs @@ -16,8 +16,9 @@ // under the License. //! DataFusion error types - +#[cfg(backtrace)] use std::backtrace::{Backtrace, BacktraceStatus}; + use std::error::Error; use std::fmt::{Display, Formatter}; use std::io; From bedd5a461e973ac8759cebddae58106fcec471ed Mon Sep 17 00:00:00 2001 From: comphead Date: Tue, 12 Sep 2023 17:20:07 -0700 Subject: [PATCH 3/6] docs --- datafusion/common/Cargo.toml | 1 + datafusion/common/src/error.rs | 65 +++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/datafusion/common/Cargo.toml b/datafusion/common/Cargo.toml index 782801a7a9f4..4638bdaf4447 100644 --- a/datafusion/common/Cargo.toml +++ b/datafusion/common/Cargo.toml @@ -37,6 +37,7 @@ avro = ["apache-avro"] compression = ["xz2", "bzip2", "flate2", "zstd", "async-compression"] default = ["compression", "parquet"] pyarrow = ["pyo3", "arrow/pyarrow"] +backtrace = [] [dependencies] apache-avro = { version = "0.15", default-features = false, features = ["snappy"], optional = true } diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs index 812aff8653e7..0c117cef55bb 100644 --- a/datafusion/common/src/error.rs +++ b/datafusion/common/src/error.rs @@ -16,7 +16,7 @@ // under the License. //! DataFusion error types -#[cfg(backtrace)] +#[cfg(feature = "backtrace")] use std::backtrace::{Backtrace, BacktraceStatus}; use std::error::Error; @@ -369,6 +369,8 @@ impl From for io::Error { } impl DataFusionError { + const BACK_TRACE_SEP: &str = "\n\nbacktrace: "; + /// Get deepest underlying [`DataFusionError`] /// /// [`DataFusionError`]s sometimes form a chain, such as `DataFusionError::ArrowError()` in order to conform @@ -413,27 +415,33 @@ impl DataFusionError { pub fn strip_backtrace(&self) -> String { self.to_string() - .split("\n\nbacktrace: ") + .split(Self::BACK_TRACE_SEP) .collect::>() .first() .unwrap_or(&"") .to_string() } - #[cfg(backtrace)] + /// To enable optional rust backtrace in DataFusion: + /// - [`Setup Env Variables`]https://doc.rust-lang.org/std/backtrace/index.html#environment-variables + /// - Enable `backtrace` cargo feature + /// + /// Example: + /// cargo build --features 'backtrace' + /// RUST_BACKTRACE=1 ./app #[inline(always)] pub fn get_back_trace() -> String { - let back_trace = Backtrace::capture(); - if back_trace.status() == BacktraceStatus::Captured { - return format!("\n\nbacktrace: {}", back_trace); - } + #[cfg(feature = "backtrace")] + { + let back_trace = Backtrace::capture(); + if back_trace.status() == BacktraceStatus::Captured { + return format!("{}{}", Self::BACK_TRACE_SEP, back_trace); + } - "".to_owned() - } + "".to_owned() + } - #[cfg(not(backtrace))] - #[inline(always)] - pub fn get_back_trace() -> String { + #[cfg(not(feature = "backtrace"))] "".to_owned() } } @@ -531,6 +539,39 @@ mod test { assert_eq!(res.strip_backtrace(), "Arrow error: Schema error: bar"); } + // RUST_BACKTRACE=1 cargo test --features backtrace --package datafusion-common --lib -- error::test::test_backtrace + #[cfg(feature = "backtrace")] + #[test] + fn test_enabled_backtrace() { + let res: Result<(), DataFusionError> = plan_err!("Err"); + let err = res.unwrap_err().to_string(); + assert!(err.contains(DataFusionError::BACK_TRACE_SEP)); + assert_eq!( + err.split(DataFusionError::BACK_TRACE_SEP) + .collect::>() + .get(0) + .unwrap(), + &"Error during planning: Err" + ); + assert!( + err.split(DataFusionError::BACK_TRACE_SEP) + .collect::>() + .get(1) + .unwrap() + .len() + > 0 + ); + } + + #[cfg(not(feature = "backtrace"))] + #[test] + fn test_disabled_backtrace() { + let res: Result<(), DataFusionError> = plan_err!("Err"); + let err = res.unwrap_err().to_string(); + assert!(!err.contains(DataFusionError::BACK_TRACE_SEP)); + assert_eq!(err, "Error during planning: Err"); + } + #[test] fn test_find_root_error() { do_root_test( From 5d2ad657a3a5b573ca662255172cc0d978ba4030 Mon Sep 17 00:00:00 2001 From: comphead Date: Wed, 13 Sep 2023 07:48:43 -0700 Subject: [PATCH 4/6] fmt --- datafusion/common/Cargo.toml | 2 +- datafusion/common/src/error.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/datafusion/common/Cargo.toml b/datafusion/common/Cargo.toml index 4638bdaf4447..4cc2dfd28153 100644 --- a/datafusion/common/Cargo.toml +++ b/datafusion/common/Cargo.toml @@ -34,10 +34,10 @@ path = "src/lib.rs" [features] avro = ["apache-avro"] +backtrace = [] compression = ["xz2", "bzip2", "flate2", "zstd", "async-compression"] default = ["compression", "parquet"] pyarrow = ["pyo3", "arrow/pyarrow"] -backtrace = [] [dependencies] apache-avro = { version = "0.15", default-features = false, features = ["snappy"], optional = true } diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs index 0c117cef55bb..d7a0e1b59dba 100644 --- a/datafusion/common/src/error.rs +++ b/datafusion/common/src/error.rs @@ -423,7 +423,7 @@ impl DataFusionError { } /// To enable optional rust backtrace in DataFusion: - /// - [`Setup Env Variables`]https://doc.rust-lang.org/std/backtrace/index.html#environment-variables + /// - [`Setup Env Variables`] /// - Enable `backtrace` cargo feature /// /// Example: @@ -542,6 +542,7 @@ mod test { // RUST_BACKTRACE=1 cargo test --features backtrace --package datafusion-common --lib -- error::test::test_backtrace #[cfg(feature = "backtrace")] #[test] + #[allow(clippy::unnecessary_literal_unwrap)] fn test_enabled_backtrace() { let res: Result<(), DataFusionError> = plan_err!("Err"); let err = res.unwrap_err().to_string(); @@ -565,11 +566,12 @@ mod test { #[cfg(not(feature = "backtrace"))] #[test] + #[allow(clippy::unnecessary_literal_unwrap)] fn test_disabled_backtrace() { let res: Result<(), DataFusionError> = plan_err!("Err"); - let err = res.unwrap_err().to_string(); - assert!(!err.contains(DataFusionError::BACK_TRACE_SEP)); - assert_eq!(err, "Error during planning: Err"); + let res = res.unwrap_err().to_string(); + assert!(!res.contains(DataFusionError::BACK_TRACE_SEP)); + assert_eq!(res, "Error during planning: Err"); } #[test] From dea5c6d6a15142374e847b8c90ec14411a9a7bc6 Mon Sep 17 00:00:00 2001 From: comphead Date: Wed, 13 Sep 2023 16:40:25 -0700 Subject: [PATCH 5/6] yml --- .github/workflows/rust.yml | 6 +++--- datafusion/core/Cargo.toml | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d2fb9b23bdf2..eae5b2d594b1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -96,7 +96,7 @@ jobs: with: rust-version: stable - name: Run tests (excluding doctests) - run: cargo test --lib --tests --bins --features avro,json,dictionary_expressions + run: cargo test --lib --tests --bins --features avro,json,dictionary_expressions,backtrace - name: Verify Working Directory Clean run: git diff --exit-code @@ -302,7 +302,7 @@ jobs: shell: bash run: | export PATH=$PATH:$HOME/d/protoc/bin - cargo test --lib --tests --bins --features avro,json,dictionary_expressions + cargo test --lib --tests --bins --features avro,json,dictionary_expressions,backtrace cd datafusion-cli cargo test --lib --tests --bins --all-features env: @@ -337,7 +337,7 @@ jobs: - name: Run tests (excluding doctests) shell: bash run: | - cargo test --lib --tests --bins --features avro,json,dictionary_expressions + cargo test --lib --tests --bins --features avro,json,dictionary_expressions,backtrace cd datafusion-cli cargo test --lib --tests --bins --all-features env: diff --git a/datafusion/core/Cargo.toml b/datafusion/core/Cargo.toml index d5738cb1fe13..269d329de413 100644 --- a/datafusion/core/Cargo.toml +++ b/datafusion/core/Cargo.toml @@ -36,6 +36,7 @@ path = "src/lib.rs" [features] # Used to enable the avro format avro = ["apache-avro", "num-traits", "datafusion-common/avro"] +backtrace = ["datafusion-common/backtrace"] compression = ["xz2", "bzip2", "flate2", "zstd", "async-compression"] crypto_expressions = ["datafusion-physical-expr/crypto_expressions", "datafusion-optimizer/crypto_expressions"] default = ["crypto_expressions", "encoding__expressions", "regex_expressions", "unicode_expressions", "compression"] From f55c2bc9bcafdbf69d4010ca902e8c80b7933057 Mon Sep 17 00:00:00 2001 From: comphead Date: Thu, 14 Sep 2023 08:25:36 -0700 Subject: [PATCH 6/6] yml --- .github/workflows/rust.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index eae5b2d594b1..995916ef0d4e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -308,6 +308,7 @@ jobs: env: # do not produce debug symbols to keep memory usage down RUSTFLAGS: "-C debuginfo=0" + RUST_BACKTRACE: "1" macos: name: cargo test (mac) @@ -343,6 +344,7 @@ jobs: env: # do not produce debug symbols to keep memory usage down RUSTFLAGS: "-C debuginfo=0" + RUST_BACKTRACE: "1" test-datafusion-pyarrow: name: cargo test pyarrow (amd64)