liurenjie1024 commented on code in PR #89:
URL: https://github.com/apache/iceberg-rust/pull/89#discussion_r1378337379
##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -109,50 +110,139 @@ pub struct TableMetadata {
}
impl TableMetadata {
+ /// Returns format version of this metadata.
+ #[inline]
+ pub fn format_version(&self) -> FormatVersion {
+ self.format_version.clone()
+ }
+
+ /// Returns uuid of current table.
+ #[inline]
+ pub fn uuid(&self) -> &Uuid {
+ &self.table_uuid
+ }
+
+ /// Returns table location.
+ #[inline]
+ pub fn location(&self) -> &str {
+ self.location.as_str()
+ }
+
+ /// Returns last sequence number.
+ #[inline]
+ pub fn last_sequence_number(&self) -> i64 {
+ self.last_sequence_number
+ }
+
+ /// Returns last updated time.
+ #[inline]
+ pub fn last_updated_ms(&self) -> i64 {
+ self.last_updated_ms
+ }
+
+ /// Returns schemas
+ #[inline]
+ pub fn schemas(&self) -> impl Iterator<Item = &SchemaRef> {
+ self.schemas.values()
+ }
+
+ /// Lookup schema by id.
+ #[inline]
+ pub fn schema_by_id(&self, schema_id: i32) -> Option<&SchemaRef> {
+ self.schemas.get(&schema_id)
+ }
+
/// Get current schema
#[inline]
- pub fn current_schema(&self) -> Result<Arc<Schema>, Error> {
- self.schemas
- .get(&self.current_schema_id)
- .ok_or_else(|| {
- Error::new(
- ErrorKind::DataInvalid,
- format!("Schema id {} not found!", self.current_schema_id),
- )
- })
- .cloned()
+ pub fn current_schema(&self) -> &SchemaRef {
+ self.schema_by_id(self.current_schema_id)
+ .expect("Current schema id set, but not found in table metadata")
+ }
+
+ /// Returns all partition specs.
+ #[inline]
+ pub fn partition_specs(&self) -> impl Iterator<Item = &PartitionSpecRef> {
+ self.partition_specs.values()
}
+
+ /// Lookup partition spec by id.
+ #[inline]
+ pub fn partition_spec_by_id(&self, spec_id: i32) ->
Option<&PartitionSpecRef> {
+ self.partition_specs.get(&spec_id)
+ }
+
/// Get default partition spec
#[inline]
- pub fn default_partition_spec(&self) -> Result<&PartitionSpec, Error> {
- self.partition_specs
- .get(&self.default_spec_id)
- .ok_or_else(|| {
- Error::new(
- ErrorKind::DataInvalid,
- format!("Partition spec id {} not found!",
self.default_spec_id),
- )
- })
+ pub fn default_partition_spec(&self) -> Option<&PartitionSpecRef> {
+ if self.default_spec_id == DEFAULT_SPEC_ID {
+ self.partition_spec_by_id(DEFAULT_SPEC_ID)
+ } else {
+ Some(
+ self.partition_spec_by_id(DEFAULT_SPEC_ID)
+ .expect("Default partition spec id set, but not found in
table metadata"),
+ )
+ }
+ }
+
+ /// Returns all snapshots
+ #[inline]
+ pub fn snapshots(&self) -> impl Iterator<Item = &SnapshotRef> {
Review Comment:
Collect to `Vec` involves memory allocation, and it's not difficult to do it
in rust, just one more line of code. I can thing of some use case where `Vec`
is unnecessary:
1. User just wants to print snapshot for debugging.
2. User just wants to search a snapshot later than some creation time in
time travel use case.
I think renaming it to `snapshots_iter` is a good suggestion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
