databendlabs / databend

๐——๐—ฎ๐˜๐—ฎ, ๐—”๐—ป๐—ฎ๐—น๐˜†๐˜๐—ถ๐—ฐ๐˜€ & ๐—”๐—œ. Modern alternative to Snowflake. Cost-effective and simple for massive-scale analytics. https://databend.com
https://docs.databend.com
Other
7.88k stars 751 forks source link

feat: add aws glue as an iceberg connection type #16824

Closed Rowlandev closed 6 days ago

Rowlandev commented 1 week ago

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Tests

Type of change


This change isโ€‚Reviewable

BohuTANG commented 1 week ago

For the check CI failed, we should keep the Cargo.toml crate is in order, the fix is(taplo fmt -f):

diff --git a/Cargo.toml b/Cargo.toml
index 71b5ec05d2..e82e52ac7e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -308,9 +308,9 @@ humantime = "2.1.0"
 hyper = "1"
 hyper-util = { version = "0.1.9", features = ["client", "client-legacy", "tokio", "service"] }
 iceberg = { version = "0.3.0", git = "https://github.com/Xuanwo/iceberg-rust/", rev = "fe5df3f" }
+iceberg-catalog-glue = { version = "0.3.0", git = "https://github.com/Xuanwo/iceberg-rust/", rev = "fe5df3f" }
 iceberg-catalog-hms = { version = "0.3.0", git = "https://github.com/Xuanwo/iceberg-rust/", rev = "fe5df3f" }
 iceberg-catalog-rest = { version = "0.3.0", git = "https://github.com/Xuanwo/iceberg-rust/", rev = "fe5df3f" }
-iceberg-catalog-glue = { version = "0.3.0", git = "https://github.com/Xuanwo/iceberg-rust/", rev = "fe5df3f" }
 indexmap = "2.0.0"
 indicatif = "0.17.5"
 itertools = "0.13.0"
diff --git a/src/query/storages/iceberg/Cargo.toml b/src/query/storages/iceberg/Cargo.toml
index fc336ee6a0..add1c755fa 100644
--- a/src/query/storages/iceberg/Cargo.toml
+++ b/src/query/storages/iceberg/Cargo.toml
@@ -25,9 +25,9 @@ databend-storages-common-table-meta = { workspace = true }
 fastrace = { workspace = true }
 futures = { workspace = true }
 iceberg = { workspace = true }
+iceberg-catalog-glue = { workspace = true }
 iceberg-catalog-hms = { workspace = true }
 iceberg-catalog-rest = { workspace = true }
-iceberg-catalog-glue = { workspace = true }
 match-template = { workspace = true }
 ordered-float = { workspace = true }
 serde = { workspace = true }
Xuanwo commented 1 week ago

Hi, @Rowlandev, thank you so much for creating thisโ€”it's greatly appreciated! Most of the changes look good to me. The only missing part is integrating with our proto tests to ensure compatibility with our meta services.

https://github.com/databendlabs/databend/blob/7abfa9616e84d047bfcb72821c15df46acf06a48/src/meta/proto-conv/src/util.rs#L142-L147

You can refer to

https://github.com/databendlabs/databend/blob/7abfa9616e84d047bfcb72821c15df46acf06a48/src/meta/proto-conv/tests/it/v098_catalog_option.rs#L34-L59

But changed with glue catalog.

Rowlandev commented 1 week ago

@Xuanwo, @BohuTANG & @drmingdrmer - thanks for the help! Let me know what else I can do to fix up this pull request.

drmingdrmer commented 1 week ago

@Xuanwo, @BohuTANG & @drmingdrmer - thanks for the help! Let me know what else I can do to fix up this pull request.

Thank you. Most to this PR looks good enough to me. As I mentioned in a previous comment, A new version and a corresponding test case should be added:

In this comment: https://github.com/databendlabs/databend/pull/16824#pullrequestreview-2431392697

Rowlandev commented 1 week ago

@drmingdrmer My apologies, using git on this repo with two different machines, thought I pushed up.

drmingdrmer commented 1 week ago

@drmingdrmer My apologies, using git on this repo with two different machines, thought I pushed up.

Does not matter :D

I'll receive a notification once there is a new push.

Rowlandev commented 1 week ago

Is it party time? @drmingdrmer

drmingdrmer commented 1 week ago

Is it party time? @drmingdrmer

Not quite yet - still have some work to wrap up, though I'm flexible with the schedule:)

drmingdrmer commented 6 days ago

It looks like the source code is not well formatted, which caused the check fails: https://github.com/databendlabs/databend/actions/runs/11857098771/job/33044752102?pr=16824

You can run cargo fmt locally to fix this issue.

drmingdrmer commented 6 days ago

Nice job! Thank you! Let's merge!

BohuTANG commented 4 days ago

@soyeric128 Please update the documentation. Thanks.