From 03170042fc5af5c64e1f665a675a181e3a8f81a0 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Sun, 31 Dec 2023 00:57:18 +0800 Subject: [PATCH 1/8] add ability to create union type --- src/vtab/logical_type.rs | 57 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 00845260..312dde47 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -217,6 +217,23 @@ impl LogicalType { } } + /// Make a `LogicalType` for `union` + pub fn union_type(fields: &[(&str, LogicalType)]) -> Self { + let keys: Vec = fields.iter().map(|f| CString::new(f.0).unwrap()).collect(); + let values: Vec = fields.iter().map(|it| it.1.ptr).collect(); + let name_ptrs = keys.iter().map(|it| it.as_ptr()).collect::>(); + + unsafe { + Self { + ptr: duckdb_create_union_type( + *values.as_slice().as_ptr(), + name_ptrs.as_slice().as_ptr().cast_mut(), + fields.len() as idx_t, + ), + } + } + } + /// Logical type ID pub fn id(&self) -> LogicalTypeId { let duckdb_type_id = unsafe { duckdb_get_type_id(self.ptr) }; @@ -226,15 +243,19 @@ impl LogicalType { /// Logical type children num pub fn num_children(&self) -> usize { match self.id() { - LogicalTypeId::Struct => unsafe { duckdb_struct_type_child_count(self.ptr) as usize }, + LogicalTypeId::Union | LogicalTypeId::Struct => unsafe { + duckdb_struct_type_child_count(self.ptr) as usize + }, LogicalTypeId::List => 1, _ => 0, } } /// Logical type child name by idx + /// + /// Panics if the logical type is not a struct or union pub fn child_name(&self, idx: usize) -> String { - assert_eq!(self.id(), LogicalTypeId::Struct); + assert!(self.id() == LogicalTypeId::Struct || self.id() == LogicalTypeId::Union); unsafe { let child_name_ptr = duckdb_struct_type_child_name(self.ptr, idx as u64); let c_str = CString::from_raw(child_name_ptr); @@ -280,4 +301,36 @@ mod test { assert_eq!(typ.decimal_width(), 0); assert_eq!(typ.decimal_scale(), 0); } + + #[test] + fn test_union_type() { + assert_eq!( + LogicalType::new(crate::vtab::LogicalTypeId::Integer).id(), + crate::vtab::LogicalTypeId::Integer + ); + + let fields = &[ + ("hello", LogicalType::new(crate::vtab::LogicalTypeId::Boolean)), + ("world", LogicalType::new(crate::vtab::LogicalTypeId::Integer)), + ]; + let typ = LogicalType::union_type(fields); + + assert_eq!(typ.num_children(), 2); + + // first child is the union tag + // TODO: should we hide this? + assert_eq!(typ.child_name(0), ""); + assert_eq!(typ.child(0).id(), crate::vtab::LogicalTypeId::UTinyint); + + assert_eq!(typ.child_name(1), "hello"); + assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Boolean); + + for i in 0..typ.num_children() { + println!("{}: {:?}", i, typ.child(i)); + } + + assert_eq!(typ.child_name(2), "world"); + return; // FIXME: this is broken + assert_eq!(typ.child(2).id(), crate::vtab::LogicalTypeId::Integer); + } } From 4aeca695a5223d2c376eace982b5b05e52883a79 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Sun, 31 Dec 2023 18:56:39 +0800 Subject: [PATCH 2/8] rework to use proper union functions --- src/vtab/logical_type.rs | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 312dde47..671aab68 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -243,9 +243,8 @@ impl LogicalType { /// Logical type children num pub fn num_children(&self) -> usize { match self.id() { - LogicalTypeId::Union | LogicalTypeId::Struct => unsafe { - duckdb_struct_type_child_count(self.ptr) as usize - }, + LogicalTypeId::Struct => unsafe { duckdb_struct_type_child_count(self.ptr) as usize }, + LogicalTypeId::Union => unsafe { duckdb_union_type_member_count(self.ptr) as usize }, LogicalTypeId::List => 1, _ => 0, } @@ -255,9 +254,12 @@ impl LogicalType { /// /// Panics if the logical type is not a struct or union pub fn child_name(&self, idx: usize) -> String { - assert!(self.id() == LogicalTypeId::Struct || self.id() == LogicalTypeId::Union); unsafe { - let child_name_ptr = duckdb_struct_type_child_name(self.ptr, idx as u64); + let child_name_ptr = match self.id() { + LogicalTypeId::Struct => duckdb_struct_type_child_name(self.ptr, idx as u64), + LogicalTypeId::Union => duckdb_union_type_member_name(self.ptr, idx as u64), + _ => panic!("not a struct or union"), + }; let c_str = CString::from_raw(child_name_ptr); let name = c_str.to_str().unwrap(); name.to_string() @@ -266,7 +268,13 @@ impl LogicalType { /// Logical type child by idx pub fn child(&self, idx: usize) -> Self { - let c_logical_type = unsafe { duckdb_struct_type_child_type(self.ptr, idx as u64) }; + let c_logical_type = unsafe { + match self.id() { + LogicalTypeId::Struct => duckdb_struct_type_child_type(self.ptr, idx as u64), + LogicalTypeId::Union => duckdb_union_type_member_type(self.ptr, idx as u64), + _ => panic!("not a struct or union"), + } + }; Self::from(c_logical_type) } } @@ -317,20 +325,10 @@ mod test { assert_eq!(typ.num_children(), 2); - // first child is the union tag - // TODO: should we hide this? - assert_eq!(typ.child_name(0), ""); - assert_eq!(typ.child(0).id(), crate::vtab::LogicalTypeId::UTinyint); - - assert_eq!(typ.child_name(1), "hello"); - assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Boolean); - - for i in 0..typ.num_children() { - println!("{}: {:?}", i, typ.child(i)); - } + assert_eq!(typ.child_name(0), "hello"); + assert_eq!(typ.child(0).id(), crate::vtab::LogicalTypeId::Boolean); - assert_eq!(typ.child_name(2), "world"); - return; // FIXME: this is broken - assert_eq!(typ.child(2).id(), crate::vtab::LogicalTypeId::Integer); + assert_eq!(typ.child_name(1), "world"); + assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Integer); } } From d0310c8ecc29989cdf7b054b95180f8451cc23e8 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Wed, 3 Jan 2024 13:40:35 +0800 Subject: [PATCH 3/8] check for panic --- src/vtab/logical_type.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 671aab68..cd1a6976 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -329,6 +329,11 @@ mod test { assert_eq!(typ.child(0).id(), crate::vtab::LogicalTypeId::Boolean); assert_eq!(typ.child_name(1), "world"); - assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Integer); + + let panicked = std::panic::catch_unwind(|| { + assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Integer); + }); + // this bug was fixed in https://github.com/duckdb/duckdb/pull/10097, can remove catch_unwind then + assert!(panicked.is_err()); } } From aac85998da4842cfc47cd136ee94423000d7119a Mon Sep 17 00:00:00 2001 From: Elliana May Date: Wed, 3 Jan 2024 14:34:07 +0800 Subject: [PATCH 4/8] swap for if --- src/vtab/logical_type.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index cd1a6976..69dd95ea 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -330,10 +330,10 @@ mod test { assert_eq!(typ.child_name(1), "world"); - let panicked = std::panic::catch_unwind(|| { + // this bug was fixed in https://github.com/duckdb/duckdb/pull/10097, can unwrap if then + if std::env::var("CARGO_PKG_VERSION").unwrap().starts_with("0.10.0") { assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Integer); - }); - // this bug was fixed in https://github.com/duckdb/duckdb/pull/10097, can remove catch_unwind then - assert!(panicked.is_err()); + panic!("FIXME: unwrap this if after duckdb 0.10.0 is released"); + } } } From c158cc0441dbff9a0ec597f926652d704c4a9e35 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Wed, 3 Jan 2024 14:44:25 +0800 Subject: [PATCH 5/8] tweak import --- src/vtab/logical_type.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 69dd95ea..915dcd3e 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -281,7 +281,7 @@ impl LogicalType { #[cfg(test)] mod test { - use crate::vtab::LogicalType; + use super::{LogicalType, LogicalTypeId}; #[test] fn test_struct() { @@ -312,27 +312,22 @@ mod test { #[test] fn test_union_type() { - assert_eq!( - LogicalType::new(crate::vtab::LogicalTypeId::Integer).id(), - crate::vtab::LogicalTypeId::Integer - ); - let fields = &[ - ("hello", LogicalType::new(crate::vtab::LogicalTypeId::Boolean)), - ("world", LogicalType::new(crate::vtab::LogicalTypeId::Integer)), + ("hello", LogicalType::new(LogicalTypeId::Boolean)), + ("world", LogicalType::new(LogicalTypeId::Integer)), ]; let typ = LogicalType::union_type(fields); assert_eq!(typ.num_children(), 2); assert_eq!(typ.child_name(0), "hello"); - assert_eq!(typ.child(0).id(), crate::vtab::LogicalTypeId::Boolean); + assert_eq!(typ.child(0).id(), LogicalTypeId::Boolean); assert_eq!(typ.child_name(1), "world"); // this bug was fixed in https://github.com/duckdb/duckdb/pull/10097, can unwrap if then if std::env::var("CARGO_PKG_VERSION").unwrap().starts_with("0.10.0") { - assert_eq!(typ.child(1).id(), crate::vtab::LogicalTypeId::Integer); + assert_eq!(typ.child(1).id(), LogicalTypeId::Integer); panic!("FIXME: unwrap this if after duckdb 0.10.0 is released"); } } From 93a0a1b8cb83488100a9c98a514c8dfcfdf45a33 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Tue, 20 Feb 2024 23:39:39 +0800 Subject: [PATCH 6/8] remove workaround --- src/vtab/logical_type.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 915dcd3e..76485319 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -324,11 +324,6 @@ mod test { assert_eq!(typ.child(0).id(), LogicalTypeId::Boolean); assert_eq!(typ.child_name(1), "world"); - - // this bug was fixed in https://github.com/duckdb/duckdb/pull/10097, can unwrap if then - if std::env::var("CARGO_PKG_VERSION").unwrap().starts_with("0.10.0") { - assert_eq!(typ.child(1).id(), LogicalTypeId::Integer); - panic!("FIXME: unwrap this if after duckdb 0.10.0 is released"); - } + assert_eq!(typ.child(1).id(), LogicalTypeId::Integer); } } From 1ba45113cc7766c2db2b933e18e0a0dea6dc35a9 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Tue, 20 Feb 2024 23:46:08 +0800 Subject: [PATCH 7/8] remove deref --- src/vtab/logical_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vtab/logical_type.rs b/src/vtab/logical_type.rs index 76485319..7fe0a45d 100644 --- a/src/vtab/logical_type.rs +++ b/src/vtab/logical_type.rs @@ -226,7 +226,7 @@ impl LogicalType { unsafe { Self { ptr: duckdb_create_union_type( - *values.as_slice().as_ptr(), + values.as_slice().as_ptr().cast_mut(), name_ptrs.as_slice().as_ptr().cast_mut(), fields.len() as idx_t, ), From 4d6522a938f418b936c247e4ee48eeee1a40d621 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Wed, 21 Feb 2024 01:49:52 +0800 Subject: [PATCH 8/8] Remove duplicate use --- src/r2d2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/r2d2.rs b/src/r2d2.rs index b05225e5..75203671 100644 --- a/src/r2d2.rs +++ b/src/r2d2.rs @@ -102,7 +102,7 @@ impl r2d2::ManageConnection for DuckdbConnectionManager { mod test { extern crate r2d2; use super::*; - use crate::{types::Value, Result}; + use crate::types::Value; use std::{sync::mpsc, thread}; use tempdir::TempDir;