Skip to content
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

fix partition values #45

Merged
merged 1 commit into from
Oct 23, 2024
Merged

fix partition values #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 5 additions & 11 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

68 changes: 4 additions & 64 deletions datafusion_iceberg/src/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,20 +596,14 @@ impl DataSink for IcebergDataSink {
#[cfg(test)]
mod tests {

use datafusion::{
arrow::{
array::{Float32Array, Int64Array},
record_batch::RecordBatch,
},
prelude::SessionContext,
};
use datafusion::{arrow::array::Int64Array, prelude::SessionContext};
use iceberg_rust::spec::{
partition::{PartitionField, Transform},
schema::Schema,
types::{PrimitiveType, StructField, StructType, Type},
};
use iceberg_rust::{
catalog::{identifier::Identifier, tabular::Tabular, Catalog},
catalog::Catalog,
spec::{
partition::PartitionSpec,
view_metadata::{Version, ViewRepresentation},
Expand All @@ -618,64 +612,10 @@ mod tests {
view::View,
};
use iceberg_sql_catalog::SqlCatalog;
use object_store::{local::LocalFileSystem, memory::InMemory, ObjectStore};
use object_store::{memory::InMemory, ObjectStore};
use std::sync::Arc;

use crate::{catalog::catalog::IcebergCatalog, error::Error, DataFusionTable};

#[tokio::test]
pub async fn test_datafusion_table_scan() {
let object_store: Arc<dyn ObjectStore> =
Arc::new(LocalFileSystem::new_with_prefix("../iceberg-tests/nyc_taxis").unwrap());

let catalog: Arc<dyn Catalog> = Arc::new(
SqlCatalog::new("sqlite://", "test", object_store.clone())
.await
.unwrap(),
);
let identifier = Identifier::parse("test.table1", None).unwrap();

catalog.clone().register_table(identifier.clone(), "/home/iceberg/warehouse/nyc/taxis/metadata/fb072c92-a02b-11e9-ae9c-1bb7bc9eca94.metadata.json").await.expect("Failed to register table.");

let table = if let Tabular::Table(table) = catalog
.load_tabular(&identifier)
.await
.expect("Failed to load table")
{
Ok(Arc::new(DataFusionTable::from(table)))
} else {
Err(Error::InvalidFormat(
"Entity returned from catalog".to_string(),
))
}
.unwrap();

let ctx = SessionContext::new();

ctx.register_table("nyc_taxis", table).unwrap();

let df = ctx
.sql("SELECT vendor_id, MIN(trip_distance) FROM nyc_taxis GROUP BY vendor_id")
.await
.unwrap();

// execute the plan
let results: Vec<RecordBatch> = df.collect().await.expect("Failed to execute query plan.");

let batch = results
.into_iter()
.find(|batch| batch.num_rows() > 0)
.expect("All record batches are empty");

let values = batch
.column(1)
.as_any()
.downcast_ref::<Float32Array>()
.expect("Failed to get values from batch.");

// Value can either be 0.9 or 1.8
assert!(((1.35 - values.value(0)).abs() - 0.45).abs() < 0.001)
}
use crate::{catalog::catalog::IcebergCatalog, DataFusionTable};

#[tokio::test]
pub async fn test_datafusion_table_insert() {
Expand Down
48 changes: 28 additions & 20 deletions iceberg-rust-spec/src/spec/manifest_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use std::{
io::Read,
iter::{repeat, Map, Repeat, Zip},
sync::OnceLock,
};

use apache_avro::{types::Value as AvroValue, Reader as AvroReader, Schema as AvroSchema};
Expand Down Expand Up @@ -42,12 +43,13 @@ impl<'a, 'metadata, R: Read> Iterator for ManifestListReader<'a, 'metadata, R> {

impl<'a, 'metadata, R: Read> ManifestListReader<'a, 'metadata, R> {
/// Create a new ManifestFile reader
pub fn new(
reader: R,
table_metadata: &'metadata TableMetadata,
) -> Result<Self, apache_avro::Error> {
pub fn new(reader: R, table_metadata: &'metadata TableMetadata) -> Result<Self, Error> {
let schema: &AvroSchema = match table_metadata.format_version {
FormatVersion::V1 => manifest_list_schema_v1(),
FormatVersion::V2 => manifest_list_schema_v2(),
};
Ok(Self {
reader: AvroReader::new(reader)?
reader: AvroReader::with_schema(&schema, reader)?
.zip(repeat(table_metadata))
.map(avro_value_to_manifest_file),
})
Expand Down Expand Up @@ -404,11 +406,11 @@ impl FieldSummary {
}
}

impl ManifestListEntry {
/// Get schema of the manifest list
pub fn schema(format_version: &FormatVersion) -> Result<AvroSchema, Error> {
let schema = match format_version {
FormatVersion::V1 => r#"
pub fn manifest_list_schema_v1() -> &'static AvroSchema {
static MANIFEST_LIST_SCHEMA_V1: OnceLock<AvroSchema> = OnceLock::new();
MANIFEST_LIST_SCHEMA_V1.get_or_init(|| {
AvroSchema::parse_str(
r#"
{
"type": "record",
"name": "manifest_file",
Expand Down Expand Up @@ -544,9 +546,16 @@ impl ManifestListEntry {
}
]
}
"#
.to_owned(),
&FormatVersion::V2 => r#"
"#,
)
.unwrap()
})
}
pub fn manifest_list_schema_v2() -> &'static AvroSchema {
static MANIFEST_LIST_SCHEMA_V2: OnceLock<AvroSchema> = OnceLock::new();
MANIFEST_LIST_SCHEMA_V2.get_or_init(|| {
AvroSchema::parse_str(
r#"
{
"type": "record",
"name": "manifest_file",
Expand Down Expand Up @@ -673,11 +682,10 @@ impl ManifestListEntry {
}
]
}
"#
.to_owned(),
};
AvroSchema::parse_str(&schema).map_err(Into::into)
}
"#,
)
.unwrap()
})
}

/// Convert an avro value to a [ManifestFile] according to the provided format version
Expand Down Expand Up @@ -771,7 +779,7 @@ mod tests {
key_metadata: None,
};

let schema = ManifestListEntry::schema(&FormatVersion::V2).unwrap();
let schema = manifest_list_schema_v2();

let mut writer = apache_avro::Writer::new(&schema, Vec::new());

Expand Down Expand Up @@ -851,7 +859,7 @@ mod tests {
key_metadata: None,
};

let schema = ManifestListEntry::schema(&FormatVersion::V1).unwrap();
let schema = manifest_list_schema_v1();

let mut writer = apache_avro::Writer::new(&schema, Vec::new());

Expand Down
62 changes: 32 additions & 30 deletions iceberg-rust/src/table/transaction/operation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,23 @@ use std::{
sync::Arc,
};

use apache_avro::from_value;
use futures::{lock::Mutex, stream, StreamExt, TryStreamExt};
use iceberg_rust_spec::spec::{
manifest::{partition_value_schema, DataFile, ManifestEntry, ManifestWriter, Status},
manifest_list::{Content, FieldSummary, ManifestListEntry, ManifestListEntryEnum},
partition::PartitionField,
schema::Schema,
snapshot::{
generate_snapshot_id, SnapshotBuilder, SnapshotReference, SnapshotRetention, Summary,
use iceberg_rust_spec::{error::Error as SpecError, spec::table_metadata::TableMetadata};
use iceberg_rust_spec::{manifest_list::ManifestListReader, util::strip_prefix};
use iceberg_rust_spec::{
manifest_list::{manifest_list_schema_v1, manifest_list_schema_v2},
spec::{
manifest::{partition_value_schema, DataFile, ManifestEntry, ManifestWriter, Status},
manifest_list::{Content, FieldSummary, ManifestListEntry},
partition::PartitionField,
schema::Schema,
snapshot::{
generate_snapshot_id, SnapshotBuilder, SnapshotReference, SnapshotRetention, Summary,
},
values::{Struct, Value},
},
values::{Struct, Value},
table_metadata::FormatVersion,
};
use iceberg_rust_spec::util::strip_prefix;
use iceberg_rust_spec::{error::Error as SpecError, spec::table_metadata::TableMetadata};
use object_store::ObjectStore;

use crate::{
Expand Down Expand Up @@ -102,8 +105,10 @@ impl Operation {
},
)?);

let manifest_list_schema =
ManifestListEntry::schema(&table_metadata.format_version)?;
let manifest_list_schema = match table_metadata.format_version {
FormatVersion::V1 => manifest_list_schema_v1(),
FormatVersion::V2 => manifest_list_schema_v2(),
};

let manifest_list_writer = Arc::new(Mutex::new(apache_avro::Writer::new(
&manifest_list_schema,
Expand All @@ -128,23 +133,14 @@ impl Operation {
let existing_manifest_iter = if let Some(manifest_list_bytes) = &manifest_list_bytes
{
let manifest_list_reader =
apache_avro::Reader::new(manifest_list_bytes.as_ref())?;
ManifestListReader::new(manifest_list_bytes.as_ref(), &table_metadata)?;

Some(stream::iter(manifest_list_reader).filter_map(|manifest| {
let datafiles = datafiles.clone();
let existing_partitions = existing_partitions.clone();
let partition_spec = partition_spec.clone();
async move {
let manifest = manifest
.map_err(Into::into)
.and_then(|value| {
ManifestListEntry::try_from_enum(
from_value::<ManifestListEntryEnum>(&value)?,
table_metadata,
)
})
.unwrap();

let manifest = manifest.ok()?;
if let Some(summary) = &manifest.partitions {
let partition_values = partition_values_in_bounds(
summary,
Expand Down Expand Up @@ -384,8 +380,10 @@ impl Operation {
(ManifestStatus::New(manifest), vec![partition_value.clone()])
});

let manifest_list_schema =
ManifestListEntry::schema(&table_metadata.format_version)?;
let manifest_list_schema = match table_metadata.format_version {
FormatVersion::V1 => manifest_list_schema_v1(),
FormatVersion::V2 => manifest_list_schema_v2(),
};

let manifest_list_writer = Arc::new(Mutex::new(apache_avro::Writer::new(
&manifest_list_schema,
Expand Down Expand Up @@ -624,10 +622,12 @@ fn update_partitions(
partition_values: &Struct,
partition_columns: &[PartitionField],
) -> Result<(), Error> {
for (field, summary) in partition_columns.iter().zip(partitions.iter_mut()) {
let value = &partition_values.get(field.name()).and_then(|x| x.as_ref());
for (field, summary) in partition_columns.into_iter().zip(partitions.iter_mut()) {
let value = partition_values.get(field.name()).and_then(|x| x.as_ref());
if let Some(value) = value {
if let Some(lower_bound) = &mut summary.lower_bound {
if summary.lower_bound.is_none() {
summary.lower_bound = Some(value.clone());
} else if let Some(lower_bound) = &mut summary.lower_bound {
match (value, lower_bound) {
(Value::Int(val), Value::Int(current)) => {
if *current > *val {
Expand Down Expand Up @@ -672,7 +672,9 @@ fn update_partitions(
_ => {}
}
}
if let Some(upper_bound) = &mut summary.upper_bound {
if summary.upper_bound.is_none() {
summary.upper_bound = Some(value.clone());
} else if let Some(upper_bound) = &mut summary.upper_bound {
match (value, upper_bound) {
(Value::Int(val), Value::Int(current)) => {
if *current < *val {
Expand Down
Binary file removed ...use/nyc/taxis/data/vendor_id=1/00000-0-03c9b632-a796-4f56-97b0-a638a6f6d6f4-00001.parquet
View file
Open in desktop
Binary file not shown.
Binary file removed ...use/nyc/taxis/data/vendor_id=1/00003-3-ae86257a-5d0b-4c42-9782-f08ec510637e-00001.parquet
View file
Open in desktop
Binary file not shown.
Binary file removed ...use/nyc/taxis/data/vendor_id=2/00001-1-2a1bfa65-21d8-4302-ad47-85c00b092e8b-00001.parquet
View file
Open in desktop
Binary file not shown.
Binary file removed ...use/nyc/taxis/data/vendor_id=2/00002-2-22cded08-1e3c-4905-a73c-5e0ea8ed268f-00001.parquet
View file
Open in desktop
Binary file not shown.
1 change: 0 additions & 1 deletion ...iceberg/warehouse/nyc/taxis/metadata/0-0b944f80-1326-447a-97fd-f4827081b3c5.metadata.json
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...iceberg/warehouse/nyc/taxis/metadata/0-1a907b10-fcab-4be8-903c-25bc0974156b.metadata.json
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...iceberg/warehouse/nyc/taxis/metadata/0-1c785339-c427-4232-ab45-704a663ff72b.metadata.json
View file
Open in desktop

This file was deleted.

Loading
Loading