Skip to content

feat(dataframe): improve DataFrame::from_columns input types (#24630) #24633

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.

Sign up for GitHub

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
kosiew merged 6 commits into from
Sep 3, 2026
Merged

feat(dataframe): improve DataFrame::from_columns input types (#24630) #24633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
271bd05
feat(dataframe): improve DataFrame::from_columns input types (#24630)
cj-zhukov Aug 24, 2026
ec0137a
fix cargo clippy in test_dataframe_from_columns
cj-zhukov Aug 24, 2026
1ce3db7
update from_columns types and add tests
cj-zhukov Aug 28, 2026
a01cbbb
fix clippy and test_dataframe_from_columns_empty
cj-zhukov Aug 28, 2026
51453fd
add upgrading guide
cj-zhukov Aug 30, 2026
1a5e7c2
test_dataframe_from_columns_empty is non async
cj-zhukov Sep 1, 2026
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
23 changes: 11 additions & 12 deletions datafusion/core/src/dataframe/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2614,7 +2614,7 @@ impl DataFrame {
/// # async fn main() -> Result<()> {
/// let id: ArrayRef = Arc::new(Int32Array::from(vec![1, 2, 3]));
/// let name: ArrayRef = Arc::new(StringArray::from(vec!["foo", "bar", "baz"]));
/// let df = DataFrame::from_columns(vec![("id", id), ("name", name)])?;
/// let df = DataFrame::from_columns([("id", id), ("name", name)])?;
/// let expected = vec![
/// "+----+------+",
/// "| id | name |",
Expand All @@ -2628,17 +2628,16 @@ impl DataFrame {
/// # Ok(())
/// # }
/// ```
pub fn from_columns(columns: Vec<(&str, ArrayRef)>) -> Result<Self> {
let fields = columns
.iter()
.map(|(name, array)| Field::new(*name, array.data_type().clone(), true))
.collect::<Vec<_>>();

let arrays = columns
pub fn from_columns<'a, I>(columns: I) -> Result<Self>

@kosiew kosiew Aug 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this. One API compatibility concern still remains here.

This changes the released non-generic from_columns(Vec<(&str, ArrayRef)>) signature to from_columns<'a, I>, so the SemVer/API-health issue is still present. cargo-semver-checks will report method_requires_different_generic_type_params, and downstream code that uses this method as a non-generic function item can break.

Using parameter-position impl IntoIterator<Item = (&str, ArrayRef)> may avoid that specific cargo-semver-checks diagnostic and would preserve normal Vec call syntax, but it is still an implicit generic parameter, so it would not be a strict compatibility fix either.

To preserve the existing public API, I think the safest option is to keep from_columns(Vec<...>) and add the iterator or array-taking behavior under a new method name. The old API could then be deprecated later according to policy if desired.

If this signature change is intentional instead, it should be treated and documented as a breaking API change under the API-health policy, including the api-change label and upgrade guidance.

@cj-zhukov cj-zhukov Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kosiew Thanks for the clarification. I understand the remaining compatibility concern now.

My goal is to allow from_columns to accept arrays and other IntoIterator inputs while keeping the existing Vec usage working. I’d prefer to keep this behavior under from_columns rather than introduce a second method.

I agree that this is a breaking change under the API-health policy. I’ll update the PR accordingly, including adding the api-change label, updating the description, and providing upgrade guidance where appropriate.

Once the changes are ready, I’ll let you know and ask for another review.

where
I: IntoIterator<Item = (&'a str, ArrayRef)>,
{
let (fields, arrays): (Vec<_>, Vec<_>) = columns
.into_iter()
.map(|(_, array)| array)
.collect::<Vec<_>>();

.map(|(name, array)| {
(Field::new(name, array.data_type().clone(), true), array)
})
.unzip();
let schema = Arc::new(Schema::new(fields));
let batch = RecordBatch::try_new(schema, arrays)?;
let ctx = SessionContext::new();
Expand Down Expand Up @@ -2695,7 +2694,7 @@ macro_rules! dataframe {
use datafusion::prelude::DataFrame;
use datafusion::common::test_util::IntoArrayRef;

let columns = vec![
let columns = [
$(
($name, $data.into_array_ref()),
)+
Expand Down
91 changes: 83 additions & 8 deletions datafusion/core/tests/dataframe/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6968,7 +6968,7 @@ async fn test_dataframe_from_columns() -> Result<()> {
let strings: ArrayRef =
Arc::new(StringArray::from(vec![Some("foo"), Some("bar"), None]));

let df = DataFrame::from_columns(vec![
let columns = [
("bool", bools),
("i8", i8s),
("i16", i16s),
Expand All @@ -6982,10 +6982,10 @@ async fn test_dataframe_from_columns() -> Result<()> {
("f32", f32s),
("f64", f64s),
("str", strings),
])?;
];

assert_eq!(df.schema().fields().len(), 13);
assert_eq!(df.clone().count().await?, 3);
let df1 = DataFrame::from_columns(columns.clone())?;

@kosiew kosiew Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the final API keeps S: AsRef<str> as an intentional feature, could we add a small test using String column names and a non-collection iterator, such as .into_iter().map(...)?

The current tests cover arrays and Vecs with &str names, but they do not compile-cover the broader string-like name and iterator contract.

@cj-zhukov cj-zhukov Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it makes sense to extend the tests to cover the IntoIterator API. Since I'm keeping the existing (&str, ArrayRef) item type for compatibility, I'll add a test with a non-collection iterator using .into_iter().map(...).

let df2 = DataFrame::from_columns(columns.to_vec())?;

let expected_types = [
("bool", DataType::Boolean),
Expand All @@ -7003,14 +7003,89 @@ async fn test_dataframe_from_columns() -> Result<()> {
("str", DataType::Utf8),
];

let schema = df.schema();
for df in [df1, df2] {
assert_eq!(df.schema().fields().len(), expected_types.len());
assert_eq!(df.clone().count().await?, 3);

for (name, data_type) in expected_types {
assert_eq!(schema.field_with_name(None, name)?.data_type(), &data_type);
let schema = df.schema();

for (name, data_type) in &expected_types {
assert_eq!(schema.field_with_name(None, name)?.data_type(), data_type);
}

let rows = df.sort(vec![col("i32").sort(true, true)])?;

assert_batches_eq!(
&[
"+-------+----+-----+-----+-----+----+-----+-----+-----+-----+-----+-----+-----+",
"| bool | i8 | i16 | i32 | i64 | u8 | u16 | u32 | u64 | f16 | f32 | f64 | str |",
"+-------+----+-----+-----+-----+----+-----+-----+-----+-----+-----+-----+-----+",
"| true | -1 | -1 | -1 | -1 | 0 | 0 | 0 | 0 | 1 | 1.0 | 1.0 | foo |",
"| false | 0 | 0 | 0 | 0 | 1 | 1 | 1 | 1 | 2 | 2.0 | 2.0 | bar |",
"| true | 1 | 1 | 1 | 1 | 2 | 2 | 2 | 2 | 3 | 3.0 | 3.0 | |",
"+-------+----+-----+-----+-----+----+-----+-----+-----+-----+-----+-----+-----+",
],
&rows.collect().await?
);
}

let rows = df.sort(vec![col("i32").sort(true, true)])?;
Ok(())
}

#[test]
fn test_dataframe_from_columns_empty() {
let result = DataFrame::from_columns(vec![]);
assert!(result.is_err());

let result = DataFrame::from_columns([]);
assert!(result.is_err());
}

#[tokio::test]
async fn test_dataframe_from_columns_with_iterator() -> Result<()> {
let bools: ArrayRef = Arc::new(BooleanArray::from(vec![true, false, true]));
let i8s: ArrayRef = Arc::new(Int8Array::from(vec![-1, 0, 1]));
let i16s: ArrayRef = Arc::new(Int16Array::from(vec![-1, 0, 1]));
let i32s: ArrayRef = Arc::new(Int32Array::from(vec![-1, 0, 1]));
let i64s: ArrayRef = Arc::new(Int64Array::from(vec![-1, 0, 1]));

let u8s: ArrayRef = Arc::new(UInt8Array::from(vec![0, 1, 2]));
let u16s: ArrayRef = Arc::new(UInt16Array::from(vec![0, 1, 2]));
let u32s: ArrayRef = Arc::new(UInt32Array::from(vec![0, 1, 2]));
let u64s: ArrayRef = Arc::new(UInt64Array::from(vec![0, 1, 2]));

let f16s: ArrayRef = Arc::new(Float16Array::from(vec![
half::f16::from_f64(1.0),
half::f16::from_f64(2.0),
half::f16::from_f64(3.0),
]));
let f32s: ArrayRef = Arc::new(Float32Array::from(vec![1.0, 2.0, 3.0]));
let f64s: ArrayRef = Arc::new(Float64Array::from(vec![1.0, 2.0, 3.0]));

let strings: ArrayRef =
Arc::new(StringArray::from(vec![Some("foo"), Some("bar"), None]));

let columns = [
("bool", bools),
("i8", i8s),
("i16", i16s),
("i32", i32s),
("i64", i64s),
("u8", u8s),
("u16", u16s),
("u32", u32s),
("u64", u64s),
("f16", f16s),
("f32", f32s),
("f64", f64s),
("str", strings),
];

let df = DataFrame::from_columns(columns.into_iter())?;

assert_eq!(df.schema().fields().len(), 13);
assert_eq!(df.clone().count().await?, 3);
let rows = df.sort(vec![col("i32").sort(true, true)])?;
assert_batches_eq!(
&[
"+-------+----+-----+-----+-----+----+-----+-----+-----+-----+-----+-----+-----+",
Expand Down
23 changes: 22 additions & 1 deletion docs/source/library-user-guide/upgrading/56.0.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,25 @@ The output type of the `floor` and `ceil` UDFs has been changed from the exact i

Change the expected type or wrap the expression in `CAST`. It's recommended to avoid relying on decimal's exact precision and scale.

[#24703]: https://github.com/apache/datafusion/pull/24703
### `DataFrame::from_columns` accepts `IntoIterator`

`DataFrame::from_columns` now accepts any `IntoIterator<Item = (&str, ArrayRef)>`
instead of specifically accepting a `Vec<(&str, ArrayRef)>`.

```rust,ignore
// Existing Vec usage continues to work
let df = DataFrame::from_columns(vec![
("id", id),
("name", name),
])?;

// Arrays can now be used directly
let df = DataFrame::from_columns([
("id", id),
("name", name),
])?;
```

Most existing call sites using `Vec` require no changes. Code that relies on
the exact non-generic function signature of `DataFrame::from_columns` may need
to be updated to account for the new generic API.