Skip to content

Fix clippy warnings + symbolize ASAN stack traces #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .github/workflows/rust.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,15 @@ jobs:
with:
toolchain: nightly
components: 'rust-src'

# Install LLVM tools
- name: Install LLVM
run: |
sudo apt-get install -y llvm
- name: Tests with asan
env:
RUSTFLAGS: -Zsanitizer=address -C debuginfo=0
RUSTDOCFLAGS: -Zsanitizer=address
ASAN_OPTIONS: "detect_stack_use_after_return=1:detect_leaks=1"
ASAN_OPTIONS: "detect_stack_use_after_return=1:detect_leaks=1:symbolize=1"
# Work around https://github.com/rust-lang/rust/issues/59125 by
# disabling backtraces. In an ideal world we'd probably suppress the
# leak sanitization, but we don't care about backtraces here, so long
Expand All @@ -142,6 +145,8 @@ jobs:
# We cannot run "modern-full" with asan as the chrono feature will auto-load relase binaries of
# the ICU-extension, which are not built with ASAN and will cause a crash.
run: |
export ASAN_SYMBOLIZER_PATH=$(which llvm-symbolizer)
echo $ASAN_SYMBOLIZER_PATH
cargo -Z build-std test --features "serde_json url r2d2 uuid polars extensions-full" --target x86_64-unknown-linux-gnu --package duckdb
- name: publish crates --dry-run
uses: katyo/publish-crates@v2
Expand Down
18 changes: 9 additions & 9 deletions crates/duckdb/src/appender_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ use sealed::Sealed;
/// - a reference to an array of references, as in `thing.query(&["foo",
/// "bar", "baz"])` or `thing.query(&[&1i32, &2, &3])`.
///
/// (Note: in this case we don't implement this for slices for coherence
/// reasons, so it really is only for the "reference to array" types —
/// hence why the number of parameters must be <= 32 or you need to
/// reach for `duckdb::params!`)
///
/// Unfortunately, in the current design it's not possible to allow this for
/// references to arrays of non-references (e.g. `&[1i32, 2, 3]`). Code like
/// this should instead either use `params!`, an array literal, a `&[&dyn
/// ToSql]` or if none of those work, [`ParamsFromIter`].
/// (Note: in this case we don't implement this for slices for coherence
/// reasons, so it really is only for the "reference to array" types —
/// hence why the number of parameters must be <= 32 or you need to
/// reach for `duckdb::params!`)
///
/// Unfortunately, in the current design it's not possible to allow this for
/// references to arrays of non-references (e.g. `&[1i32, 2, 3]`). Code like
/// this should instead either use `params!`, an array literal, a `&[&dyn
/// ToSql]` or if none of those work, [`ParamsFromIter`].
///
/// - As a slice of `ToSql` trait object references, e.g. `&[&dyn ToSql]`. This
/// is mostly useful for passing parameter lists around as arguments without
Expand Down
18 changes: 9 additions & 9 deletions crates/duckdb/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ use sealed::Sealed;
/// - a reference to an array of references, as in `thing.query(&["foo",
/// "bar", "baz"])` or `thing.query(&[&1i32, &2, &3])`.
///
/// (Note: in this case we don't implement this for slices for coherence
/// reasons, so it really is only for the "reference to array" types —
/// hence why the number of parameters must be <= 32 or you need to
/// reach for `duckdb::params!`)
///
/// Unfortunately, in the current design it's not possible to allow this for
/// references to arrays of non-references (e.g. `&[1i32, 2, 3]`). Code like
/// this should instead either use `params!`, an array literal, a `&[&dyn
/// ToSql]` or if none of those work, [`ParamsFromIter`].
/// (Note: in this case we don't implement this for slices for coherence
/// reasons, so it really is only for the "reference to array" types —
/// hence why the number of parameters must be <= 32 or you need to
/// reach for `duckdb::params!`)
///
/// Unfortunately, in the current design it's not possible to allow this for
/// references to arrays of non-references (e.g. `&[1i32, 2, 3]`). Code like
/// this should instead either use `params!`, an array literal, a `&[&dyn
/// ToSql]` or if none of those work, [`ParamsFromIter`].
///
/// - As a slice of `ToSql` trait object references, e.g. `&[&dyn ToSql]`. This
/// is mostly useful for passing parameter lists around as arguments without
Expand Down
21 changes: 8 additions & 13 deletions crates/duckdb/src/vtab/arrow.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
use super::{BindInfo, DataChunkHandle, InitInfo, LogicalTypeHandle, TableFunctionInfo, VTab};
use std::sync::Arc;
use std::sync::{atomic::AtomicBool, Mutex};
use std::sync::{atomic::AtomicBool, Arc, Mutex};

use crate::{
core::{ArrayVector, FlatVector, Inserter, ListVector, LogicalTypeId, StructVector, Vector},
types::DuckString,
};

use arrow::array::as_map_array;
use arrow::{
array::{
as_boolean_array, as_generic_binary_array, as_large_list_array, as_list_array, as_primitive_array,
as_string_array, as_struct_array, Array, ArrayData, AsArray, BinaryArray, BinaryViewArray, BooleanArray,
Date32Array, Decimal128Array, FixedSizeBinaryArray, FixedSizeListArray, GenericBinaryBuilder, GenericListArray,
GenericStringArray, IntervalMonthDayNanoArray, LargeBinaryArray, LargeStringArray, OffsetSizeTrait,
PrimitiveArray, StringArray, StringViewArray, StructArray, TimestampMicrosecondArray, TimestampNanosecondArray,
as_boolean_array, as_generic_binary_array, as_large_list_array, as_list_array, as_map_array,
as_primitive_array, as_string_array, as_struct_array, Array, ArrayData, AsArray, BinaryArray, BinaryViewArray,
BooleanArray, Date32Array, Decimal128Array, FixedSizeBinaryArray, FixedSizeListArray, GenericBinaryBuilder,
GenericListArray, GenericStringArray, IntervalMonthDayNanoArray, LargeBinaryArray, LargeStringArray,
OffsetSizeTrait, PrimitiveArray, StringArray, StringViewArray, StructArray, TimestampMicrosecondArray,
TimestampNanosecondArray,
},
buffer::{BooleanBuffer, NullBuffer},
compute::cast,
Expand Down Expand Up @@ -1714,11 +1713,7 @@ mod test {

let mut stmt = db.prepare("SELECT * FROM arrow(?, ?)").unwrap();

let res = match stmt.execute(arrow_recordbatch_to_query_params(batch)) {
Ok(..) => None,
Err(e) => Some(e),
}
.unwrap();
let res = stmt.execute(arrow_recordbatch_to_query_params(batch)).err().unwrap();

assert_eq!(
res,
Expand Down
Loading