osscm commented on code in PR #11041: URL: https://github.com/apache/iceberg/pull/11041#discussion_r4126794820
########## format/view-spec.md: ########## @@ -63,11 +75,13 @@ The view version metadata file has the following fields: | _required_ | `versions` | A list of known [versions](#versions) of the view [1] | | _required_ | `version-log` | A list of [version log](#version-log) entries with the timestamp and `version-id` for every change to `current-version-id` | | _optional_ | `properties` | A string to string map of view properties [2] | +| _optional_ | `max-staleness-ms` | The maximum time interval in milliseconds during which changed source table snapshots are considered fresh enough to skip refreshing [3] | Review Comment: Curious if this survived into the current draft — I don't see a `max-staleness-ms` field (or any staleness-window config) anywhere in the current spec text, and the "Recency policy" bullet later in the doc still says "falls within a staleness window" without anything defining what that window actually is. Not sure if that's leftover wording from a since-superseded revision, or an intentional decision to leave the staleness bound out of the interchange metadata entirely and let each engine define its own out-of-band policy. On the Option 1 vs. Option 2 debate (refresh-start-timestamp vs. earliest-changed-source-snapshot-timestamp): for what it's worth, StarRocks' `mv_rewrite_staleness_second` property already ships something closer to Option 2/delayed-view semantics — it computes staleness as the gap between the base tables' current data timestamp and the point at which the MV's freshness was last *confirmed* by a complete refresh, and allows query rewrite to use the MV as long as that gap stays within the configured bound, rather than gating purely on "how long ago did the last refresh start." One detail from that implementation that might be worth folding in if this comes back into the spec: it has an explicit guard for when a base table's timestamp *regresses* (e.g. after `rollback_to_snapshot`, or dropped newest partitions) — the staleness window is deliberately not trusted in that case, since a naive time-based check could otherwise serve rows that no longer exist upstream. -- 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]
