liurenjie1024 commented on code in PR #262:
URL: https://github.com/apache/iceberg-rust/pull/262#discussion_r1524080929
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
Review Comment:
This should be updated by current ts.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: 0,
Review Comment:
This should be schema's largest field id.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: 0,
+ schemas: HashMap::from([(0, Arc::new(schema))]),
+ current_schema_id: 0,
+ partition_specs: partition_spec
Review Comment:
This is incorrect, we should bind partition spec to schema.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: 0,
+ schemas: HashMap::from([(0, Arc::new(schema))]),
+ current_schema_id: 0,
+ partition_specs: partition_spec
+ .map(|x| {
+ let partition_spec = PartitionSpecRef::new(x.create_new());
+ HashMap::from([(partition_spec.spec_id, partition_spec)])
+ })
+ .unwrap_or_default(),
+ default_spec_id: 0,
+ last_partition_id: 0,
+ properties,
+ current_snapshot_id: None,
+ snapshots: Default::default(),
+ snapshot_log: vec![],
+ metadata_log: vec![],
+ sort_orders: sort_order
Review Comment:
We should bind to schema.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: 0,
+ schemas: HashMap::from([(0, Arc::new(schema))]),
+ current_schema_id: 0,
Review Comment:
Ditto, this should be schema's id.
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -275,6 +277,78 @@ impl TableMetadata {
}
}
+/// Manipulating table metadata.
+pub struct TableMetadataBuilder(TableMetadata);
+
+impl TableMetadataBuilder {
+ /// Creates a new table metadata builder from the given table metadata.
+ pub fn new(origin: TableMetadata) -> Self {
+ Self(origin)
+ }
+
+ /// Creates a new table metadata builder from the given table creation.
+ pub fn from_table_creation(table_creation: TableCreation) -> Result<Self> {
+ let TableCreation {
+ name: _,
+ location,
+ schema,
+ partition_spec,
+ sort_order,
+ properties,
+ } = table_creation;
+
+ let table_metadata = TableMetadata {
+ format_version: FormatVersion::V2,
+ table_uuid: Uuid::new_v4(),
+ location: location.ok_or_else(|| {
+ Error::new(
+ ErrorKind::DataInvalid,
+ "Can't create table without location",
+ )
+ })?,
+ last_sequence_number: 0,
+ last_updated_ms: 0,
+ last_column_id: 0,
+ schemas: HashMap::from([(0, Arc::new(schema))]),
Review Comment:
The key should be schema id.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
