Skip to content

Commit c1f1370

Browse files
authored
chore: remove DataPtr trait since Arc::ptr_eq ignores pointer metadata (#10378)
* chore: remove DataPtr trait since Arc::ptr_eq ignores pointer metadata * fix: remove DataPtr unit tests
1 parent b412dba commit c1f1370

File tree

4 files changed

+4
-55
lines changed

4 files changed

+4
-55
lines changed

datafusion/common/src/utils.rs

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -525,34 +525,6 @@ pub fn list_ndims(data_type: &DataType) -> u64 {
525525
}
526526
}
527527

528-
/// An extension trait for smart pointers. Provides an interface to get a
529-
/// raw pointer to the data (with metadata stripped away).
530-
///
531-
/// This is useful to see if two smart pointers point to the same allocation.
532-
pub trait DataPtr {
533-
/// Returns a raw pointer to the data, stripping away all metadata.
534-
fn data_ptr(this: &Self) -> *const ();
535-
536-
/// Check if two pointers point to the same data.
537-
fn data_ptr_eq(this: &Self, other: &Self) -> bool {
538-
// Discard pointer metadata (including the v-table).
539-
let this = Self::data_ptr(this);
540-
let other = Self::data_ptr(other);
541-
542-
std::ptr::eq(this, other)
543-
}
544-
}
545-
546-
// Currently, it's brittle to compare `Arc`s of dyn traits with `Arc::ptr_eq`
547-
// due to this check including v-table equality. It may be possible to use
548-
// `Arc::ptr_eq` directly if a fix to https://github.com/rust-lang/rust/issues/103763
549-
// is stabilized.
550-
impl<T: ?Sized> DataPtr for Arc<T> {
551-
fn data_ptr(this: &Self) -> *const () {
552-
Arc::as_ptr(this) as *const ()
553-
}
554-
}
555-
556528
/// Adopted from strsim-rs for string similarity metrics
557529
pub mod datafusion_strsim {
558530
// Source: https://github.com/dguo/strsim-rs/blob/master/src/lib.rs
@@ -974,26 +946,6 @@ mod tests {
974946
assert_eq!(longest_consecutive_prefix([1, 2, 3, 4]), 0);
975947
}
976948

977-
#[test]
978-
fn arc_data_ptr_eq() {
979-
let x = Arc::new(());
980-
let y = Arc::new(());
981-
let y_clone = Arc::clone(&y);
982-
983-
assert!(
984-
Arc::data_ptr_eq(&x, &x),
985-
"same `Arc`s should point to same data"
986-
);
987-
assert!(
988-
!Arc::data_ptr_eq(&x, &y),
989-
"different `Arc`s should point to different data"
990-
);
991-
assert!(
992-
Arc::data_ptr_eq(&y, &y_clone),
993-
"cloned `Arc` should point to same data as the original"
994-
);
995-
}
996-
997949
#[test]
998950
fn test_merge_and_order_indices() {
999951
assert_eq!(

datafusion/core/src/physical_optimizer/join_selection.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,7 +1544,6 @@ mod hash_join_tests {
15441544

15451545
use arrow::datatypes::{DataType, Field};
15461546
use arrow::record_batch::RecordBatch;
1547-
use datafusion_common::utils::DataPtr;
15481547

15491548
struct TestCase {
15501549
case: String,
@@ -1937,8 +1936,8 @@ mod hash_join_tests {
19371936
..
19381937
}) = plan.as_any().downcast_ref::<HashJoinExec>()
19391938
{
1940-
let left_changed = Arc::data_ptr_eq(left, &right_exec);
1941-
let right_changed = Arc::data_ptr_eq(right, &left_exec);
1939+
let left_changed = Arc::ptr_eq(left, &right_exec);
1940+
let right_changed = Arc::ptr_eq(right, &left_exec);
19421941
// If this is not equal, we have a bigger problem.
19431942
assert_eq!(left_changed, right_changed);
19441943
assert_eq!(

datafusion/physical-expr-common/src/physical_expr.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ use arrow::array::BooleanArray;
2424
use arrow::compute::filter_record_batch;
2525
use arrow::datatypes::{DataType, Schema};
2626
use arrow::record_batch::RecordBatch;
27-
use datafusion_common::utils::DataPtr;
2827
use datafusion_common::{internal_err, not_impl_err, Result};
2928
use datafusion_expr::interval_arithmetic::Interval;
3029
use datafusion_expr::ColumnarValue;
@@ -188,7 +187,7 @@ pub fn with_new_children_if_necessary(
188187
|| children
189188
.iter()
190189
.zip(old_children.iter())
191-
.any(|(c1, c2)| !Arc::data_ptr_eq(c1, c2))
190+
.any(|(c1, c2)| !Arc::ptr_eq(c1, c2))
192191
{
193192
Ok(expr.with_new_children(children)?)
194193
} else {

datafusion/physical-plan/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ use crate::sorts::sort_preserving_merge::SortPreservingMergeExec;
3030
use arrow::datatypes::SchemaRef;
3131
use arrow::record_batch::RecordBatch;
3232
use datafusion_common::config::ConfigOptions;
33-
use datafusion_common::utils::DataPtr;
3433
use datafusion_common::Result;
3534
use datafusion_execution::TaskContext;
3635
use datafusion_physical_expr::{
@@ -684,7 +683,7 @@ pub fn with_new_children_if_necessary(
684683
|| children
685684
.iter()
686685
.zip(old_children.iter())
687-
.any(|(c1, c2)| !Arc::data_ptr_eq(c1, c2))
686+
.any(|(c1, c2)| !Arc::ptr_eq(c1, c2))
688687
{
689688
plan.with_new_children(children)
690689
} else {

0 commit comments

Comments
 (0)