feat: clean up dependencies and feature flags #1014

roeap · 2022-12-13T01:33:01Z

Description

This PR updates datafusion and related dependencies to their latest versions. Since datafusion now has improved support for loading partition columns with non string types, we update our scan methods to take advantage of that.

While working on dependencies, I took the opportunity to do some housekeeping.

do not use chrono with default features
make aws-profile from object_store optional. The upstream create explicitly discourages its usage, and it brings quite a few new dependencies, as it pulls in some aws sdk.
rename datafusion-ext feature to datafusion. The ext suffix is still from a time where there were less options to define features. I kept the ols feature around as an alias.

Related Issue(s)

closes #914

Documentation

rtyler

I re-ran the failed CI job, which was an unrelated cloud timeout 🤷

rtyler · 2022-12-13T16:08:57Z

rust/Cargo.toml

+# AWS_PROFILE is experimental in object_store, and comes with a hefty load of dependencies.
+# must be used in conjunction with s3 or s3-rustls feature
+aws-profile = ["object_store/aws_profile"]


I don't mind putting this functionality behind a feature flag, but this sort of change would be easier for us to discuss as a community if it were submitted separately.

Since @fvaleye just landed this functionality, I would like him to take a look at this pull request before merging too.

Agreed, this would have been better in a separate PR - here I was working in WSL and struggling with some memory issues, which is why this change made my live a bit easier there and then 😆.

Just realized, that the current setup is likely slightly broken anyhow. IF we include the feature in the object-store crate by default, this will enable aws / s3 support in the object store crate as well, however we are not enabling either s3 or s3-rustls by default on our end, in which case delta.-rs will not be able to make use of these features.

Following the previous comment, we could set this aws-profile feature with S3 features since it's only related to the AWS S3 backend storage.

rtyler · 2022-12-13T16:09:08Z

rust/README.md


 - `s3` - enable the S3 storage backend to work with Delta Tables in AWS S3.
 - `s3-rustls` - enable the S3 storage backend but rely on [rustls](https://github.com/ctz/rustls) rather than OpenSSL (`native-tls`).
 - `glue` - enable the Glue data catalog to work with Delta Tables with AWS Glue.
 - `azure` - enable the Azure storage backend to work with Delta Tables in Azure Data Lake Storage Gen2 accounts.
 - `gcs` - enable the Google storage backend to work with Delta Tables in Google Cloud Storage.
- `datafusion-ext` - enable the `datafusion::datasource::TableProvider` trait implementation for Delta Tables, allowing them to be queried using [DataFusion](https://github.com/apache/arrow-datafusion).
+- `datafusion` - enable the `datafusion::datasource::TableProvider` trait implementation for Delta Tables, allowing them to be queried using [DataFusion](https://github.com/apache/arrow-datafusion).


rust/README.md

rust/src/lib.rs

rtyler · 2022-12-13T16:21:55Z

rust/src/lib.rs

+))]
+compile_error!("Feature aws-profile must be used together with s3 or s3-rustls feature");
+
+#[cfg(all(feature = "s3", feature = "s3-rustls"))]


rtyler · 2022-12-13T16:25:15Z

rust/tests/datafusion_test.rs

+    let expected = vec![
+        "+----+----+----+",
+        "| c3 | c1 | c2 |",
+        "+----+----+----+",
+        "| 5  | 4  | c  |",
+        "| 6  | 5  | b  |",
+        "| 4  | 6  | a  |",
+        "+----+----+----+",
+    ];
+
+    assert_batches_sorted_eq!(&expected, &batches);


I was trying to understand this code, this macro in datafusion is a very surprising test helper to me!

Personally, I like these helpers quite a bit, especially once we dive into more complex operations, I believe this makes it quite straight forward to reason about the tests.

Co-authored-by: R. Tyler Croy <rtyler@brokenco.de>

roeap · 2022-12-16T17:44:18Z

@rtyler @fvaleye - after #1020 has been merged, this PR now only clean up the dependencies and does some grooming on the feature flags.Could you take another look?

houqp · 2022-12-17T03:05:48Z

rust/Cargo.toml

@@ -77,14 +77,15 @@ glibc_version = { path = "../glibc_version", version = "0.1" }

 [features]
 default = ["arrow", "parquet"]


should we also add aws-profile in the default feature list? profile based auth is a very common practice in production environment.

maybe add it as default to s3* features, since those are the only ones where it is useful?

wjones127

One other cleanup piece we might want: we don't have a CI job that runs cargo check --no-default-features and on the main branch it doesn't succeed. Do you want to look at addressing that in this PR? Or should we create a separate issue for that?

roeap · 2022-12-30T21:45:28Z

Good point - happy to add this in this PR.

wjones127

Great cleanup, thank you!

# Description ~~This PR updates datafusion and related dependencies to their latest versions. Since datafusion now has improved support for loading partition columns with non string types, we update our scan methods to take advantage of that.~~ While working on dependencies, I took the opportunity to do some housekeeping. - do not use chrono with default features - make `aws-profile` from object_store optional. The upstream create explicitly discourages its usage, and it brings quite a few new dependencies, as it pulls in some aws sdk. - rename `datafusion-ext` feature to `datafusion`. The ext suffix is still from a time where there were less options to define features. I kept the ols feature around as an alias. # Related Issue(s) closes delta-io#914 # Documentation  Co-authored-by: R. Tyler Croy <rtyler@brokenco.de>

roeap added 3 commits December 13, 2022 01:18

chore: update datafusion dependencies

09a45a3

test: add test for reading non string partitions

ecb1c87

chore: use chrono without default features

3b9d027

roeap requested review from wjones127, fvaleye, rtyler, houqp, xianwill and mosyp as code owners December 13, 2022 01:33

roeap changed the title ~~feat: load tables with non-string patition types~~ feat: load tables with non-string partition types Dec 13, 2022

fix: move profile code behind feature flag

b1b566e

rtyler reviewed Dec 13, 2022

View reviewed changes

Apply suggestions from code review

c55878f

Co-authored-by: R. Tyler Croy <rtyler@brokenco.de>

roeap mentioned this pull request Dec 15, 2022

Support DataFusion 15 #1021

Merged

roeap added 2 commits December 16, 2022 18:29

chore: merge main

d47dcfd

chore: remove test added elsewhere

9b858f5

roeap changed the title ~~feat: load tables with non-string partition types~~ feat: clean up dependencies and feature flags Dec 16, 2022

andrei-ionescu previously approved these changes Dec 16, 2022

View reviewed changes

houqp reviewed Dec 17, 2022

View reviewed changes

roeap added 2 commits December 30, 2022 22:18

Merge branch 'main' into dependencies

b0a13e3

refactor: enable aws-profile for s3 features

363b6d3

roeap dismissed andrei-ionescu’s stale review via 363b6d3 December 30, 2022 21:28

github-actions bot added delta-rs-crate python rust labels Dec 30, 2022

roeap added 2 commits December 30, 2022 22:37

fix: update missed feature flag

f294b20

docs: remove outdated feature docs

90b9979

wjones127 reviewed Dec 30, 2022

View reviewed changes

roeap added 3 commits December 30, 2022 23:35

build: allow building with no-default-features

a6277a9

fix: fmt

3eaa2bd

fix: remove outdated feature flags

26c1153

wjones127 approved these changes Dec 30, 2022

View reviewed changes

roeap merged commit d444cdf into delta-io:main Dec 30, 2022

roeap deleted the dependencies branch December 30, 2022 23:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: clean up dependencies and feature flags #1014

feat: clean up dependencies and feature flags #1014

roeap commented Dec 13, 2022 •

edited

Loading

rtyler left a comment •

edited

Loading

rtyler Dec 13, 2022

roeap Dec 13, 2022

roeap Dec 15, 2022

fvaleye Dec 21, 2022

rtyler Dec 13, 2022

rtyler Dec 13, 2022

rtyler Dec 13, 2022

roeap Dec 13, 2022

roeap commented Dec 16, 2022

houqp Dec 17, 2022

roeap Dec 17, 2022

fvaleye Dec 21, 2022

wjones127 left a comment

roeap commented Dec 30, 2022

wjones127 left a comment

		@@ -77,14 +77,15 @@ glibc_version = { path = "../glibc_version", version = "0.1" }

		[features]
		default = ["arrow", "parquet"]

feat: clean up dependencies and feature flags #1014

feat: clean up dependencies and feature flags #1014

Conversation

roeap commented Dec 13, 2022 • edited Loading

Description

Related Issue(s)

Documentation

rtyler left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roeap commented Dec 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjones127 left a comment

Choose a reason for hiding this comment

roeap commented Dec 30, 2022

wjones127 left a comment

Choose a reason for hiding this comment

roeap commented Dec 13, 2022 •

edited

Loading

rtyler left a comment •

edited

Loading