You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "liurenjie1024 (via GitHub)" <gi...@apache.org> on 2023/12/13 07:22:08 UTC
[PR] test: Remove binary manifest list avro file [iceberg-rust]
liurenjie1024 opened a new pull request, #118:
URL: https://github.com/apache/iceberg-rust/pull/118
Close #70
We generate manifest list avro file on the fly.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426370018
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
I have the same concern as well. Would be great to have some additional tests around Iceberg-specific things, for example, if the field-id resolution works correctly.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426257091
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
Yes, keeping unit tests simple and easy to maintain. We can leave parts difficult to setup to integration tests.๐
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426215180
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
> Interesting, it seems that it requires java to run.
Yes. Another way is to run it in docker https://hub.docker.com/r/coderfi/avro-tools. ๐
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426207729
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
Interesting, it seems that it requires java to run.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426434871
##########
crates/iceberg/src/spec/manifest.rs:
##########
@@ -1311,511 +1311,452 @@ mod tests {
use crate::spec::Type;
use std::sync::Arc;
- #[test]
- fn test_parse_manifest_v2_unpartition() {
- let path = format!(
- "{}/testdata/unpartition_manifest_v2.avro",
- env!("CARGO_MANIFEST_DIR")
- );
- let bs = fs::read(path).expect("read_file must succeed");
- let manifest = Manifest::parse_avro(bs.as_slice()).unwrap();
- // test metadata
- assert!(manifest.metadata.schema_id == 0);
- assert_eq!(manifest.metadata.schema, {
- let fields = vec![
- // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
- Arc::new(NestedField::optional(
- 1,
- "id",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 2,
- "v_int",
- Type::Primitive(PrimitiveType::Int),
- )),
- Arc::new(NestedField::optional(
- 3,
- "v_long",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 4,
- "v_float",
- Type::Primitive(PrimitiveType::Float),
- )),
- Arc::new(NestedField::optional(
- 5,
- "v_double",
- Type::Primitive(PrimitiveType::Double),
- )),
- Arc::new(NestedField::optional(
- 6,
- "v_varchar",
- Type::Primitive(PrimitiveType::String),
- )),
- Arc::new(NestedField::optional(
- 7,
- "v_bool",
- Type::Primitive(PrimitiveType::Boolean),
- )),
- Arc::new(NestedField::optional(
- 8,
- "v_date",
- Type::Primitive(PrimitiveType::Date),
- )),
- Arc::new(NestedField::optional(
- 9,
- "v_timestamp",
- Type::Primitive(PrimitiveType::Timestamptz),
- )),
- Arc::new(NestedField::optional(
- 10,
- "v_decimal",
- Type::Primitive(PrimitiveType::Decimal {
- precision: 36,
- scale: 10,
- }),
- )),
- Arc::new(NestedField::optional(
- 11,
- "v_ts_ntz",
- Type::Primitive(PrimitiveType::Timestamp),
- )),
- ];
- Schema::builder().with_fields(fields).build().unwrap()
- });
- assert!(manifest.metadata.partition_spec.fields.is_empty());
- assert!(manifest.metadata.content == ManifestContentType::Data);
- assert!(manifest.metadata.format_version == FormatVersion::V2);
- // test entries
- assert!(manifest.entries.len() == 1);
- let entry = &manifest.entries[0];
- assert!(entry.status == ManifestStatus::Added);
- assert!(entry.snapshot_id == Some(0));
- assert!(entry.sequence_number == Some(1));
- assert!(entry.file_sequence_number == Some(1));
- assert_eq!(
- entry.data_file,
- DataFile {
- content: DataContentType::Data,
- file_path: "s3a://icebergdata/demo/s1/t1/data/00000-0-ba56fbfa-f2ff-40c9-bb27-565ad6dc2be8-00000.parquet".to_string(),
- file_format: DataFileFormat::Parquet,
- partition: Struct::empty(),
- record_count: 1,
- file_size_in_bytes: 5442,
- column_sizes: HashMap::from([(0,73),(6,34),(2,73),(7,61),(3,61),(5,62),(9,79),(10,73),(1,61),(4,73),(8,73)]),
- value_counts: HashMap::from([(4,1),(5,1),(2,1),(0,1),(3,1),(6,1),(8,1),(1,1),(10,1),(7,1),(9,1)]),
- null_value_counts: HashMap::from([(1,0),(6,0),(2,0),(8,0),(0,0),(3,0),(5,0),(9,0),(7,0),(4,0),(10,0)]),
- nan_value_counts: HashMap::new(),
- lower_bounds: HashMap::new(),
- upper_bounds: HashMap::new(),
- key_metadata: Vec::new(),
- split_offsets: vec![4],
- equality_ids: Vec::new(),
- sort_order_id: None,
- }
- );
+ #[tokio::test]
+ async fn test_parse_manifest_v2_unpartition() {
+ let manifest = Manifest {
+ metadata: ManifestMetadata {
+ schema_id: 0,
+ schema: Schema::builder()
+ .with_fields(vec![
+ // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
+ Arc::new(NestedField::optional(
+ 1,
+ "id",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 2,
+ "v_int",
+ Type::Primitive(PrimitiveType::Int),
+ )),
+ Arc::new(NestedField::optional(
+ 3,
+ "v_long",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 4,
+ "v_float",
+ Type::Primitive(PrimitiveType::Float),
+ )),
+ Arc::new(NestedField::optional(
+ 5,
+ "v_double",
+ Type::Primitive(PrimitiveType::Double),
+ )),
+ Arc::new(NestedField::optional(
+ 6,
+ "v_varchar",
+ Type::Primitive(PrimitiveType::String),
+ )),
+ Arc::new(NestedField::optional(
+ 7,
+ "v_bool",
+ Type::Primitive(PrimitiveType::Boolean),
+ )),
+ Arc::new(NestedField::optional(
+ 8,
+ "v_date",
+ Type::Primitive(PrimitiveType::Date),
+ )),
+ Arc::new(NestedField::optional(
+ 9,
+ "v_timestamp",
+ Type::Primitive(PrimitiveType::Timestamptz),
+ )),
+ Arc::new(NestedField::optional(
+ 10,
+ "v_decimal",
+ Type::Primitive(PrimitiveType::Decimal {
+ precision: 36,
+ scale: 10,
+ }),
+ )),
+ Arc::new(NestedField::optional(
+ 11,
+ "v_ts_ntz",
+ Type::Primitive(PrimitiveType::Timestamp),
+ )),
+ ])
+ .build()
+ .unwrap(),
+ partition_spec: PartitionSpec {
+ spec_id: 0,
+ fields: vec![],
+ },
+ content: ManifestContentType::Data,
+ format_version: FormatVersion::V2,
+ },
+ entries: vec![
+ ManifestEntry {
+ status: ManifestStatus::Added,
+ snapshot_id: Some(0),
+ sequence_number: Some(1),
+ file_sequence_number: Some(1),
Review Comment:
Thanks for check. In v2 by default it's `None`, and in v1 it should be `0`.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1425089356
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
Yes, that's a problem. But I don't see other ways to do it. I think such kinds of bugs can only be avoid during integrations tests against other systems such as spark.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#issuecomment-1855466917
Thanks again @liurenjie1024 for working on this. @Xuanwo and @ZENOTME thanks 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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426434871
##########
crates/iceberg/src/spec/manifest.rs:
##########
@@ -1311,511 +1311,452 @@ mod tests {
use crate::spec::Type;
use std::sync::Arc;
- #[test]
- fn test_parse_manifest_v2_unpartition() {
- let path = format!(
- "{}/testdata/unpartition_manifest_v2.avro",
- env!("CARGO_MANIFEST_DIR")
- );
- let bs = fs::read(path).expect("read_file must succeed");
- let manifest = Manifest::parse_avro(bs.as_slice()).unwrap();
- // test metadata
- assert!(manifest.metadata.schema_id == 0);
- assert_eq!(manifest.metadata.schema, {
- let fields = vec![
- // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
- Arc::new(NestedField::optional(
- 1,
- "id",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 2,
- "v_int",
- Type::Primitive(PrimitiveType::Int),
- )),
- Arc::new(NestedField::optional(
- 3,
- "v_long",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 4,
- "v_float",
- Type::Primitive(PrimitiveType::Float),
- )),
- Arc::new(NestedField::optional(
- 5,
- "v_double",
- Type::Primitive(PrimitiveType::Double),
- )),
- Arc::new(NestedField::optional(
- 6,
- "v_varchar",
- Type::Primitive(PrimitiveType::String),
- )),
- Arc::new(NestedField::optional(
- 7,
- "v_bool",
- Type::Primitive(PrimitiveType::Boolean),
- )),
- Arc::new(NestedField::optional(
- 8,
- "v_date",
- Type::Primitive(PrimitiveType::Date),
- )),
- Arc::new(NestedField::optional(
- 9,
- "v_timestamp",
- Type::Primitive(PrimitiveType::Timestamptz),
- )),
- Arc::new(NestedField::optional(
- 10,
- "v_decimal",
- Type::Primitive(PrimitiveType::Decimal {
- precision: 36,
- scale: 10,
- }),
- )),
- Arc::new(NestedField::optional(
- 11,
- "v_ts_ntz",
- Type::Primitive(PrimitiveType::Timestamp),
- )),
- ];
- Schema::builder().with_fields(fields).build().unwrap()
- });
- assert!(manifest.metadata.partition_spec.fields.is_empty());
- assert!(manifest.metadata.content == ManifestContentType::Data);
- assert!(manifest.metadata.format_version == FormatVersion::V2);
- // test entries
- assert!(manifest.entries.len() == 1);
- let entry = &manifest.entries[0];
- assert!(entry.status == ManifestStatus::Added);
- assert!(entry.snapshot_id == Some(0));
- assert!(entry.sequence_number == Some(1));
- assert!(entry.file_sequence_number == Some(1));
- assert_eq!(
- entry.data_file,
- DataFile {
- content: DataContentType::Data,
- file_path: "s3a://icebergdata/demo/s1/t1/data/00000-0-ba56fbfa-f2ff-40c9-bb27-565ad6dc2be8-00000.parquet".to_string(),
- file_format: DataFileFormat::Parquet,
- partition: Struct::empty(),
- record_count: 1,
- file_size_in_bytes: 5442,
- column_sizes: HashMap::from([(0,73),(6,34),(2,73),(7,61),(3,61),(5,62),(9,79),(10,73),(1,61),(4,73),(8,73)]),
- value_counts: HashMap::from([(4,1),(5,1),(2,1),(0,1),(3,1),(6,1),(8,1),(1,1),(10,1),(7,1),(9,1)]),
- null_value_counts: HashMap::from([(1,0),(6,0),(2,0),(8,0),(0,0),(3,0),(5,0),(9,0),(7,0),(4,0),(10,0)]),
- nan_value_counts: HashMap::new(),
- lower_bounds: HashMap::new(),
- upper_bounds: HashMap::new(),
- key_metadata: Vec::new(),
- split_offsets: vec![4],
- equality_ids: Vec::new(),
- sort_order_id: None,
- }
- );
+ #[tokio::test]
+ async fn test_parse_manifest_v2_unpartition() {
+ let manifest = Manifest {
+ metadata: ManifestMetadata {
+ schema_id: 0,
+ schema: Schema::builder()
+ .with_fields(vec![
+ // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
+ Arc::new(NestedField::optional(
+ 1,
+ "id",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 2,
+ "v_int",
+ Type::Primitive(PrimitiveType::Int),
+ )),
+ Arc::new(NestedField::optional(
+ 3,
+ "v_long",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 4,
+ "v_float",
+ Type::Primitive(PrimitiveType::Float),
+ )),
+ Arc::new(NestedField::optional(
+ 5,
+ "v_double",
+ Type::Primitive(PrimitiveType::Double),
+ )),
+ Arc::new(NestedField::optional(
+ 6,
+ "v_varchar",
+ Type::Primitive(PrimitiveType::String),
+ )),
+ Arc::new(NestedField::optional(
+ 7,
+ "v_bool",
+ Type::Primitive(PrimitiveType::Boolean),
+ )),
+ Arc::new(NestedField::optional(
+ 8,
+ "v_date",
+ Type::Primitive(PrimitiveType::Date),
+ )),
+ Arc::new(NestedField::optional(
+ 9,
+ "v_timestamp",
+ Type::Primitive(PrimitiveType::Timestamptz),
+ )),
+ Arc::new(NestedField::optional(
+ 10,
+ "v_decimal",
+ Type::Primitive(PrimitiveType::Decimal {
+ precision: 36,
+ scale: 10,
+ }),
+ )),
+ Arc::new(NestedField::optional(
+ 11,
+ "v_ts_ntz",
+ Type::Primitive(PrimitiveType::Timestamp),
+ )),
+ ])
+ .build()
+ .unwrap(),
+ partition_spec: PartitionSpec {
+ spec_id: 0,
+ fields: vec![],
+ },
+ content: ManifestContentType::Data,
+ format_version: FormatVersion::V2,
+ },
+ entries: vec![
+ ManifestEntry {
+ status: ManifestStatus::Added,
+ snapshot_id: Some(0),
+ sequence_number: Some(1),
+ file_sequence_number: Some(1),
Review Comment:
Fixed. In v2 by default it's `None`, and in v1 it should be `0`.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426255077
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
> I did some tests and found that the json file doesn't contains all information, for example meta is missing. The docker image is not well maintained, for example last update time is 8 years ago. Given the complexity introduced, I think using integration tests with other systems maybe better?
Thanks for your test! So seems there is no better way. Using integration tests with other systems also looks good to me. The unit test can be used to test that the reader and writer can process each other.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426141277
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
It's possible. But I took a look at python implementation, it seems that they implemented the encoding/decoding of avro in pyiceberg, that's why they need cross validation of another library. This is not the case for rust. I think if we want to verify correctness, maybe setting up integration tests against systems like spark would be more appropriate.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1425080204
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
I'm bit worried about this. What will happen if we have bugs in both `ManifestListWriter` and `ManifestListReader`?
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426184104
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
I find that avro-tool can generate an avro file by schema file and data JSON file. https://www.michael-noll.com/blog/2013/03/17/reading-and-writing-avro-files-from-the-command-line/
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426215180
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
> Interesting, it seems that it requires java to run.
Yes. Another way is to run it in docker https://hub.docker.com/r/coderfi/avro-tools.๐
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
> Interesting, it seems that it requires java to run.
Yes. Another way is to run it in docker https://hub.docker.com/r/coderfi/avro-tools.๐
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426250459
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
I did some tests and found that the json file doesn't contains all information, for example meta is missing. The docker image is not well maintained, for example last update time is 8 years ago. Given the complexity introduced, I think using integration tests with other systems maybe better?
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426386121
##########
Makefile:
##########
@@ -34,6 +34,9 @@ cargo-sort:
check: check-fmt check-clippy cargo-sort
+unit-test:
Review Comment:
Are these ran in the CI?
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426366787
##########
crates/iceberg/src/spec/manifest.rs:
##########
@@ -1311,511 +1311,452 @@ mod tests {
use crate::spec::Type;
use std::sync::Arc;
- #[test]
- fn test_parse_manifest_v2_unpartition() {
- let path = format!(
- "{}/testdata/unpartition_manifest_v2.avro",
- env!("CARGO_MANIFEST_DIR")
- );
- let bs = fs::read(path).expect("read_file must succeed");
- let manifest = Manifest::parse_avro(bs.as_slice()).unwrap();
- // test metadata
- assert!(manifest.metadata.schema_id == 0);
- assert_eq!(manifest.metadata.schema, {
- let fields = vec![
- // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
- Arc::new(NestedField::optional(
- 1,
- "id",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 2,
- "v_int",
- Type::Primitive(PrimitiveType::Int),
- )),
- Arc::new(NestedField::optional(
- 3,
- "v_long",
- Type::Primitive(PrimitiveType::Long),
- )),
- Arc::new(NestedField::optional(
- 4,
- "v_float",
- Type::Primitive(PrimitiveType::Float),
- )),
- Arc::new(NestedField::optional(
- 5,
- "v_double",
- Type::Primitive(PrimitiveType::Double),
- )),
- Arc::new(NestedField::optional(
- 6,
- "v_varchar",
- Type::Primitive(PrimitiveType::String),
- )),
- Arc::new(NestedField::optional(
- 7,
- "v_bool",
- Type::Primitive(PrimitiveType::Boolean),
- )),
- Arc::new(NestedField::optional(
- 8,
- "v_date",
- Type::Primitive(PrimitiveType::Date),
- )),
- Arc::new(NestedField::optional(
- 9,
- "v_timestamp",
- Type::Primitive(PrimitiveType::Timestamptz),
- )),
- Arc::new(NestedField::optional(
- 10,
- "v_decimal",
- Type::Primitive(PrimitiveType::Decimal {
- precision: 36,
- scale: 10,
- }),
- )),
- Arc::new(NestedField::optional(
- 11,
- "v_ts_ntz",
- Type::Primitive(PrimitiveType::Timestamp),
- )),
- ];
- Schema::builder().with_fields(fields).build().unwrap()
- });
- assert!(manifest.metadata.partition_spec.fields.is_empty());
- assert!(manifest.metadata.content == ManifestContentType::Data);
- assert!(manifest.metadata.format_version == FormatVersion::V2);
- // test entries
- assert!(manifest.entries.len() == 1);
- let entry = &manifest.entries[0];
- assert!(entry.status == ManifestStatus::Added);
- assert!(entry.snapshot_id == Some(0));
- assert!(entry.sequence_number == Some(1));
- assert!(entry.file_sequence_number == Some(1));
- assert_eq!(
- entry.data_file,
- DataFile {
- content: DataContentType::Data,
- file_path: "s3a://icebergdata/demo/s1/t1/data/00000-0-ba56fbfa-f2ff-40c9-bb27-565ad6dc2be8-00000.parquet".to_string(),
- file_format: DataFileFormat::Parquet,
- partition: Struct::empty(),
- record_count: 1,
- file_size_in_bytes: 5442,
- column_sizes: HashMap::from([(0,73),(6,34),(2,73),(7,61),(3,61),(5,62),(9,79),(10,73),(1,61),(4,73),(8,73)]),
- value_counts: HashMap::from([(4,1),(5,1),(2,1),(0,1),(3,1),(6,1),(8,1),(1,1),(10,1),(7,1),(9,1)]),
- null_value_counts: HashMap::from([(1,0),(6,0),(2,0),(8,0),(0,0),(3,0),(5,0),(9,0),(7,0),(4,0),(10,0)]),
- nan_value_counts: HashMap::new(),
- lower_bounds: HashMap::new(),
- upper_bounds: HashMap::new(),
- key_metadata: Vec::new(),
- split_offsets: vec![4],
- equality_ids: Vec::new(),
- sort_order_id: None,
- }
- );
+ #[tokio::test]
+ async fn test_parse_manifest_v2_unpartition() {
+ let manifest = Manifest {
+ metadata: ManifestMetadata {
+ schema_id: 0,
+ schema: Schema::builder()
+ .with_fields(vec![
+ // id v_int v_long v_float v_double v_varchar v_bool v_date v_timestamp v_decimal v_ts_ntz
+ Arc::new(NestedField::optional(
+ 1,
+ "id",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 2,
+ "v_int",
+ Type::Primitive(PrimitiveType::Int),
+ )),
+ Arc::new(NestedField::optional(
+ 3,
+ "v_long",
+ Type::Primitive(PrimitiveType::Long),
+ )),
+ Arc::new(NestedField::optional(
+ 4,
+ "v_float",
+ Type::Primitive(PrimitiveType::Float),
+ )),
+ Arc::new(NestedField::optional(
+ 5,
+ "v_double",
+ Type::Primitive(PrimitiveType::Double),
+ )),
+ Arc::new(NestedField::optional(
+ 6,
+ "v_varchar",
+ Type::Primitive(PrimitiveType::String),
+ )),
+ Arc::new(NestedField::optional(
+ 7,
+ "v_bool",
+ Type::Primitive(PrimitiveType::Boolean),
+ )),
+ Arc::new(NestedField::optional(
+ 8,
+ "v_date",
+ Type::Primitive(PrimitiveType::Date),
+ )),
+ Arc::new(NestedField::optional(
+ 9,
+ "v_timestamp",
+ Type::Primitive(PrimitiveType::Timestamptz),
+ )),
+ Arc::new(NestedField::optional(
+ 10,
+ "v_decimal",
+ Type::Primitive(PrimitiveType::Decimal {
+ precision: 36,
+ scale: 10,
+ }),
+ )),
+ Arc::new(NestedField::optional(
+ 11,
+ "v_ts_ntz",
+ Type::Primitive(PrimitiveType::Timestamp),
+ )),
+ ])
+ .build()
+ .unwrap(),
+ partition_spec: PartitionSpec {
+ spec_id: 0,
+ fields: vec![],
+ },
+ content: ManifestContentType::Data,
+ format_version: FormatVersion::V2,
+ },
+ entries: vec![
+ ManifestEntry {
+ status: ManifestStatus::Added,
+ snapshot_id: Some(0),
+ sequence_number: Some(1),
+ file_sequence_number: Some(1),
Review Comment:
Typically when a new manifest-entry is being added, the snapshot-id and sequence-numbers are not set and inherited from the snapshot itself.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426414222
##########
Makefile:
##########
@@ -34,6 +34,9 @@ cargo-sort:
check: check-fmt check-clippy cargo-sort
+unit-test:
Review Comment:
The ci will run all tests, using the `make test` command. This command just makes running unit test convenient.
--
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: Remove binary manifest list avro file [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #118:
URL: https://github.com/apache/iceberg-rust/pull/118#discussion_r1426135812
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
Can we call FastAvro(python) in rust to generate the file for testing๐ค?
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -931,55 +931,104 @@ mod test {
use super::_serde::ManifestListV2;
- #[test]
- fn test_parse_manifest_list_v1() {
- let path = format!(
- "{}/testdata/simple_manifest_list_v1.avro",
- env!("CARGO_MANIFEST_DIR")
+ #[tokio::test]
+ async fn test_parse_manifest_list_v1() {
+ let manifest_list = ManifestList {
+ entries: vec![
+ ManifestListEntry {
+ manifest_path: "/opt/bitnami/spark/warehouse/db/table/metadata/10d28031-9739-484c-92db-cdf2975cead4-m0.avro".to_string(),
+ manifest_length: 5806,
+ partition_spec_id: 0,
+ content: ManifestContentType::Data,
+ sequence_number: 0,
+ min_sequence_number: 0,
+ added_snapshot_id: 1646658105718557341,
+ added_data_files_count: Some(3),
+ existing_data_files_count: Some(0),
+ deleted_data_files_count: Some(0),
+ added_rows_count: Some(3),
+ existing_rows_count: Some(0),
+ deleted_rows_count: Some(0),
+ partitions: vec![],
+ key_metadata: vec![],
+ }
+ ]
+ };
+
+ let file_io = FileIOBuilder::new_fs_io().build().unwrap();
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_name = "simple_manifest_list_v1.avro";
+ let full_path = format!("{}/{}", tmp_dir.path().to_str().unwrap(), file_name);
+
+ let mut writer = ManifestListWriter::v1(
Review Comment:
Can we call FastAvro(python) in rust to generate the file for testing๐ค?
--
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