You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "a-agmon (via GitHub)" <gi...@apache.org> on 2024/09/27 03:42:27 UTC
[PR] add test for table provider creation [iceberg-rust]
a-agmon opened a new pull request, #651:
URL: https://github.com/apache/iceberg-rust/pull/651
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "a-agmon (via GitHub)" <gi...@apache.org>.
a-agmon commented on code in PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651#discussion_r1778719810
##########
crates/integrations/datafusion/src/table.rs:
##########
@@ -110,3 +110,43 @@ impl TableProvider for IcebergTableProvider {
Ok(filter_support)
}
}
+
+#[cfg(test)]
+mod tests {
+ use iceberg::io::FileIO;
+ use iceberg::table::{StaticTable, Table};
+ use iceberg::TableIdent;
+
+ use super::*;
+
+ async fn get_test_table_from_metadata_file() -> Table {
+ let metadata_file_name = "TableMetadataV2Valid.json";
+ let metadata_file_path = format!(
+ "{}/tests/test_data/{}",
+ env!("CARGO_MANIFEST_DIR"),
+ metadata_file_name
+ );
+ let file_io = FileIO::from_path(&metadata_file_path)
+ .unwrap()
+ .build()
+ .unwrap();
+ let static_identifier = TableIdent::from_strs(["static_ns", "static_table"]).unwrap();
+ let static_table =
+ StaticTable::from_metadata_file(&metadata_file_path, static_identifier, file_io)
+ .await
+ .unwrap();
+ static_table.into_table()
+ }
+
+ #[tokio::test]
+ async fn test_try_new_from_table() {
Review Comment:
Sure. Done.
Thanks @FANNG1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651#issuecomment-2380512330
Thank you @a-agmon to build this and @FANNG1 for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "FANNG1 (via GitHub)" <gi...@apache.org>.
FANNG1 commented on code in PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651#discussion_r1778674865
##########
crates/integrations/datafusion/src/table.rs:
##########
@@ -110,3 +110,43 @@ impl TableProvider for IcebergTableProvider {
Ok(filter_support)
}
}
+
+#[cfg(test)]
+mod tests {
+ use iceberg::io::FileIO;
+ use iceberg::table::{StaticTable, Table};
+ use iceberg::TableIdent;
+
+ use super::*;
+
+ async fn get_test_table_from_metadata_file() -> Table {
+ let metadata_file_name = "TableMetadataV2Valid.json";
+ let metadata_file_path = format!(
+ "{}/tests/test_data/{}",
+ env!("CARGO_MANIFEST_DIR"),
+ metadata_file_name
+ );
+ let file_io = FileIO::from_path(&metadata_file_path)
+ .unwrap()
+ .build()
+ .unwrap();
+ let static_identifier = TableIdent::from_strs(["static_ns", "static_table"]).unwrap();
+ let static_table =
+ StaticTable::from_metadata_file(&metadata_file_path, static_identifier, file_io)
+ .await
+ .unwrap();
+ static_table.into_table()
+ }
+
+ #[tokio::test]
+ async fn test_try_new_from_table() {
Review Comment:
could you use datafusion to test the Iceberg table schema?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "a-agmon (via GitHub)" <gi...@apache.org>.
a-agmon commented on code in PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651#discussion_r1778693746
##########
crates/integrations/datafusion/src/table.rs:
##########
@@ -110,3 +110,43 @@ impl TableProvider for IcebergTableProvider {
Ok(filter_support)
}
}
+
+#[cfg(test)]
+mod tests {
+ use iceberg::io::FileIO;
+ use iceberg::table::{StaticTable, Table};
+ use iceberg::TableIdent;
+
+ use super::*;
+
+ async fn get_test_table_from_metadata_file() -> Table {
+ let metadata_file_name = "TableMetadataV2Valid.json";
+ let metadata_file_path = format!(
+ "{}/tests/test_data/{}",
+ env!("CARGO_MANIFEST_DIR"),
+ metadata_file_name
+ );
+ let file_io = FileIO::from_path(&metadata_file_path)
+ .unwrap()
+ .build()
+ .unwrap();
+ let static_identifier = TableIdent::from_strs(["static_ns", "static_table"]).unwrap();
+ let static_table =
+ StaticTable::from_metadata_file(&metadata_file_path, static_identifier, file_io)
+ .await
+ .unwrap();
+ static_table.into_table()
+ }
+
+ #[tokio::test]
+ async fn test_try_new_from_table() {
Review Comment:
you mean adding something like this:
```rust
let ctx = SessionContext::new();
ctx.register_table("mytable", Arc::new(table_provider))
.unwrap();
let df = ctx.sql("SELECT * FROM mytable").await.unwrap();
```
or running an actual query using it? that would require a real table (the dummy metadata I used is not really pointing at a real location)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test (datafusion): add test for table provider creation [iceberg-rust]
Posted by "a-agmon (via GitHub)" <gi...@apache.org>.
a-agmon commented on code in PR #651:
URL: https://github.com/apache/iceberg-rust/pull/651#discussion_r1778693746
##########
crates/integrations/datafusion/src/table.rs:
##########
@@ -110,3 +110,43 @@ impl TableProvider for IcebergTableProvider {
Ok(filter_support)
}
}
+
+#[cfg(test)]
+mod tests {
+ use iceberg::io::FileIO;
+ use iceberg::table::{StaticTable, Table};
+ use iceberg::TableIdent;
+
+ use super::*;
+
+ async fn get_test_table_from_metadata_file() -> Table {
+ let metadata_file_name = "TableMetadataV2Valid.json";
+ let metadata_file_path = format!(
+ "{}/tests/test_data/{}",
+ env!("CARGO_MANIFEST_DIR"),
+ metadata_file_name
+ );
+ let file_io = FileIO::from_path(&metadata_file_path)
+ .unwrap()
+ .build()
+ .unwrap();
+ let static_identifier = TableIdent::from_strs(["static_ns", "static_table"]).unwrap();
+ let static_table =
+ StaticTable::from_metadata_file(&metadata_file_path, static_identifier, file_io)
+ .await
+ .unwrap();
+ static_table.into_table()
+ }
+
+ #[tokio::test]
+ async fn test_try_new_from_table() {
Review Comment:
you mean adding something like this:
```rust
let ctx = SessionContext::new();
ctx.register_table("mytable", Arc::new(table_provider))
.unwrap();
let df = ctx.sql("SELECT * FROM mytable").await.unwrap();
```
or running an actual query using it? that would require a real table (the dummy metadata I used is not really pointing at a real location)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org