Skip to content

Commit 565839e

Browse files
authored
fix(views): make timestamp take &self & fix set_current_version_id (#1101)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #1099 - Closes #1100 ## What changes are included in this PR? 1. fixes set current view version 2. makes ViewVersionLog::timestamp not consume `self` ## Are these changes tested? 1. Added a test which sets current view version to -1 2. doesn't require a test, it's a quality of life improvement <!-- Specify what test covers (unit test, integration test, etc.). If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? -->
1 parent 299d05e commit 565839e

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

crates/iceberg/src/spec/view_metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ impl ViewVersionLog {
226226
}
227227

228228
/// Returns the last updated timestamp as a DateTime<Utc> with millisecond precision.
229-
pub fn timestamp(self) -> Result<DateTime<Utc>> {
229+
pub fn timestamp(&self) -> Result<DateTime<Utc>> {
230230
timestamp_ms_to_utc(self.timestamp_ms)
231231
}
232232
}

crates/iceberg/src/spec/view_metadata_builder.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,16 @@ impl ViewMetadataBuilder {
181181
/// - The specified `version_id` does not exist.
182182
/// - The specified `version_id` is `-1` but no version has been added.
183183
pub fn set_current_version_id(mut self, mut version_id: i32) -> Result<Self> {
184-
if version_id == Self::LAST_ADDED && self.last_added_version_id.is_none() {
185-
version_id = self.last_added_version_id.ok_or_else(|| {
186-
Error::new(
184+
if version_id == Self::LAST_ADDED {
185+
let Some(last_added_id) = self.last_added_version_id else {
186+
return Err(Error::new(
187187
ErrorKind::DataInvalid,
188188
"Cannot set current version id to last added version: no version has been added.",
189-
)
190-
})?;
189+
));
190+
};
191+
version_id = last_added_id;
191192
}
193+
192194
let version_id = version_id; // make immutable
193195

194196
if version_id == self.metadata.current_version_id {
@@ -1196,6 +1198,24 @@ mod test {
11961198
.contains("Cannot set current version to unknown version with id: 10"));
11971199
}
11981200

1201+
#[test]
1202+
fn test_set_current_version_to_last_added() {
1203+
let builder = builder_without_changes();
1204+
let v1 = new_view_version(2, 1, "select * from ns.tbl");
1205+
let v2 = new_view_version(3, 1, "select a,b from ns.tbl");
1206+
let meta = builder
1207+
.clone()
1208+
.add_version(v1)
1209+
.unwrap()
1210+
.add_version(v2)
1211+
.unwrap()
1212+
.set_current_version_id(-1)
1213+
.unwrap()
1214+
.build()
1215+
.unwrap();
1216+
assert_eq!(meta.metadata.current_version_id, 3);
1217+
}
1218+
11991219
#[test]
12001220
fn test_error_when_setting_negative_version_history_size() {
12011221
let builder = builder_without_changes();

0 commit comments

Comments
 (0)