dannycjones commented on issue #3258:
URL: https://github.com/apache/iceberg-rust/issues/3258#issuecomment-5767573394
To provide an idea of what I'm considering, here's the two gates and
associated doc comment I'm drafting.
```rust
/// Whether this library implements reading a column of this type.
///
/// Reads and writes are answered by separate methods, and neither infers
the other, so that
/// support can land one direction at a time.
///
/// **Correctness risk**: This method must only return `true` when the
read-path has been fully reviewed and tested.
/// It is OK if only some parts of implemented, so long as they gracefully
fall back.
/// For example, pruning data files based on field bounds can be safely
omitted as that is only a read optimization.
/// If the interpretation of those bounds is wrong, that's a correctness
issue.
///
/// Read support includes the following areas, but may not be exhaustive:
/// - Interpretation of data file bounds for pruning.
/// - Interpretation of `initial_default` values.
/// - Implementation of correct comparison between [`Datum`] values.
/// - Allowing safe type promotion from the underlying data file's type.
///
/// [`Datum`]: crate::spec::Datum
pub(crate) fn supports_reads(&self) -> bool {
// Matched exhaustively, including on PrimitiveType itself,
// to force newly introduced types to properly consider if it is safe to
allow reads.
match self {
Type::Primitive(
PrimitiveType::Binary
| PrimitiveType::Boolean
| PrimitiveType::Date
| PrimitiveType::Decimal { .. }
| PrimitiveType::Double
| PrimitiveType::Fixed(_)
| PrimitiveType::Float
| PrimitiveType::Int
| PrimitiveType::Long
| PrimitiveType::String
| PrimitiveType::Time
| PrimitiveType::Timestamp
| PrimitiveType::Timestamptz
| PrimitiveType::TimestampNs
| PrimitiveType::TimestamptzNs
| PrimitiveType::Uuid,
) => true,
Type::Primitive(PrimitiveType::Geography(_) |
PrimitiveType::Geometry(_)) => false,
Type::List(_) | Type::Map(_) | Type::Struct(_) => true,
Type::Variant(_) => false,
}
}
/// Whether this library implements writing a column of this type.
///
/// Reads and writes are answered by separate methods, and neither infers
the other, so that
/// support can land one direction at a time.
///
/// **Correctness risk**: This method must only return `true` when the
write-path has been fully reviewed and tested.
/// It is OK if only some parts of implemented, so long as they gracefully
fall back.
/// For example, writing manifest entry bounds for the column can be safely
omitted as these are optional in the spec.
/// However, if the bounds are calculated and written incorrectly, that's a
correctness issue.
///
/// Write support includes the following areas, but may not be exhaustive:
/// - Calculation and writing of data file bounds in manifest.
/// - Interpretation of `write_default` values.
/// - Tranformation from Iceberg schema to Arrow schema, attaching any
relevant Arrow extension type.
/// - Validating if the type is permitted in the current partition spec and
sort order.
pub(crate) fn supports_writes(&self) -> bool {
// Matched exhaustively, including on PrimitiveType itself,
// to force newly introduced types to properly consider if it is safe to
allow writes.
match self {
Type::Primitive(
PrimitiveType::Binary
| PrimitiveType::Boolean
| PrimitiveType::Date
| PrimitiveType::Decimal { .. }
| PrimitiveType::Double
| PrimitiveType::Fixed(_)
| PrimitiveType::Float
| PrimitiveType::Int
| PrimitiveType::Long
| PrimitiveType::String
| PrimitiveType::Time
| PrimitiveType::Timestamp
| PrimitiveType::Timestamptz
| PrimitiveType::TimestampNs
| PrimitiveType::TimestamptzNs
| PrimitiveType::Uuid,
) => true,
Type::Primitive(PrimitiveType::Geography(_) |
PrimitiveType::Geometry(_)) => false,
Type::List(_) | Type::Map(_) | Type::Struct(_) => true,
Type::Variant(_) => false,
}
}
```
--
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]
