Hi Tanmay and Andrei (cc: Prashant and Szehon)

Thanks very much for the detailed feedback, and I'm so sorry for delaying
my response. I agree that the read cost, failure handling, and impact on
server-side planning should be clear. I updated the PR based on these
points and the subsequent reviews:

   - Removed the new `table_properties_log` table and added an optional
   properties column to the existing metadata_log_entries table
   - Historical metadata files are read only when properties is selected or
   used in a filter. The read cost therefore still scales with the number of
   retained metadata files for these queries. Queries that do not reference
   properties avoid the additional reads, and the current metadata is reused.
   - A missing historical metadata file returns null for properties with a
   warning. This also enables the scan to continue. Other read failures still
   surface so that corruption or permission issues remain visible.
   - Each row represents a retained metadata version, so consecutive rows
   may contain the same properties.

Regarding the option of materializing property history in the catalog which
Andrei mentioned, I kept the implementation in Iceberg core to preserve
portability across catalogs and avoid a spec or write-path change at this
time.
This alternative could reduce planning-time I/O, especially with
server-side planning. I’ll explore its feasibility and, if appropriate,
open a separate PR.

I’m also adding Prashant and Szehon to this thread. Thank you both for the
detailed PR reviews.
I’d appreciate any further feedback on the updated implementation.

Best regards,
Tom

On Fri, Jul 3, 2026 at 9:56 PM Andrei Tserakhau via dev <
[email protected]> wrote:

> +1 to Tanmay on Q4, though I don't think the real question is this table.
> It's whether we want a metadata table whose read cost and failure surface
> scale with retention config rather than with the snapshot being scanned. We
> haven't had to draw that line before, and a couple more tables are behind
> it with the same shape (view history #9844, as_of columns on the all_*
> tables #8856), so it's worth deciding the pattern now rather than one PR at
> a time.
>
> The clearest comparison is metadata_log_entries, since this table is
> modeled on it. Both walk the retained previous-metadata list, but
> metadata_log_entries builds rows from TableMetadata already in memory, so
> it does no extra I/O and has no file-open failure mode.
> table_properties_log turns that same list into up to previous-versions-max
> reads of metadata.json at plan time. That gives us both the cost and the
> failure case: TableMetadataParser.read throws, so one
> expired-but-not-yet-deleted file fails the whole scan, and that file set is
> exactly what background maintenance races with.
>
> Before deciding whether file-backed reads are acceptable for a metadata
> table, I'd also ask why the engine has to reconstruct this on demand.
> Property history is derived data. A catalog could materialize it in the
> background as a normal Iceberg table, one row per metadata version, and
> expose it as a system table. Then scans have no plan-time reads, no
> retention-scaled cost, and no per-row failure handling to design.
>
> This matters more with server-side scan planning. Since #14881, a REST
> catalog can plan these scans, and because this table materializes rows at
> plan time, the up-to-100 metadata.json reads land on the catalog as
> synchronous planning work. A materialized table avoids that and plans like
> any other table.
>
> So the fork I'd want the thread to settle is: property history as an
> engine-core metadata table that reads files on demand, or as a real table
> the catalog materializes. The first is portable and works with any catalog,
> but pays at read time and inherits the failure mode. The second is cheaper
> and more robust to scan, but catalog-side, eventually consistent, and not
> portable today.
>
> For this PR, if the consensus is engine-core file reading, per-row
> degradation (an error/null column instead of a failed scan) feels close to
> required. If metadata tables should stay on in-memory state and
> manifests-of-a-snapshot, then the snapshot-summary route in Alternatives
> fits the existing model better, spec cost and all.
>
> Andrei
>
> On Wed, Jul 1, 2026 at 7:23 AM Tanmay Rauth <[email protected]> wrote:
>
>>
>> Thanks Tom for putting this together, reconstructing when a property
>> changed by hand-diffing metadata.json files is genuinely painful today, so
>> the underlying need resonates.
>>
>> The question I'd focus the discussion on is your #4, since it's what
>> makes this table structurally different from the rest. The metadata-log
>> entries retain only a timestamp and a file path
>> (TableMetadata.MetadataLogEntry), not the property map, so surfacing
>> historical properties necessarily means opening and parsing each previous
>> metadata.json. No existing metadata table re-reads historical root metadata
>> that way, the ones that do I/O (files, manifests, entries) read manifests /
>> manifest lists for a given snapshot, not previous metadata.json files. So
>> this would set a new precedent: a metadata table whose scan cost scales
>> with the number of retained metadata versions.
>>
>> *Two properties* of that approach I'd want pinned down before the schema
>> or name, because they're independent of both:
>>
>>  - Read cost. In the current PR the rows are materialized when the scan
>> is planned, reading up to write.metadata.previous-versions-max
>> (default 100) files sequentially, so a single query expands into that many
>> object-store reads before it returns any row.
>>  - Failure mode. TableMetadataParser.read throws if a referenced file
>> can't be read, and there's no per-row handling, so one absent or unreadable
>> metadata file fails the whole scan. metadata_log_entries never has this
>> failure mode because it builds its rows from in-memory metadata and does no
>> such reads.
>>
>> If the community decides property history deserves first-class support,
>> it may be worth revisiting the "Alternatives Considered": carrying the
>> values in the metadata-log entry or a dedicated structure keeps the read
>> path  I/O-free, at the cost of the write/spec change you noted. That's the
>> trade-off I'd most like to hear others weigh in on.
>>
>> On naming and schema I don't have strong objections. One thing worth
>> making explicit in the docs is that the table emits one row per retained
>> metadata version, not per property change, so consecutive rows are
>> frequently identical.
>>
>> Overall I think it's worth pursuing if the read-cost and failure-handling
>> story is nailed down.
>>
>> Regards,
>> Tanmay Rauth
>>
>> On Tue, Jun 30, 2026 at 12:39 AM Tomohiro Tanaka <[email protected]>
>> wrote:
>>
>>> Hello everyone,
>>>
>>> I’d like to ask for feedback on whether adding a `table_properties_log`
>>> metadata table is a direction worth pursuing.
>>>
>>> PR: https://github.com/apache/iceberg/pull/16859
>>>
>>> This PR adds a read-only metadata table that exposes the history of
>>> table properties from retained Iceberg metadata files.
>>> In the current version of Apache Iceberg, if users want to understand
>>> when a table property changed, they need to follow the metadata
>>> log/previous metadata files and inspect `metadata.json` files manually.
>>> The PR enables to retain table properties for each snapshot version
>>> through the existing metadata table mechanism.
>>>
>>> The proposed table returns one row per retained metadata version with:
>>> `timestamp`, `file`, `latest_snapshot_id` and `properties`.
>>>
>>> *Example use cases*:
>>>
>>>    - Audit/RCA: check whether properties like `gc.enabled` or metadata
>>>    cleanup settings were enabled before a maintenance operation.
>>>    - Debugging regressions: correlate behavior changes with updates to
>>>    properties like `write.update|delete|merge.mode`,
>>>    `write.target-file-size-bytes` or `write.distribution-mode`.
>>>
>>>
>>> Note that the PR does NOT change the table spec or write path. It only
>>> exposes information that is already retained in metadata files, and makes
>>> it available through Spark/Flink metadata table syntax.
>>>
>>> *The primary questions* I’d like feedback on are below, but any other
>>> feedback or concerns are also welcome:
>>>
>>> 1. Is this metadata table useful enough to add?
>>> 2. Is `table_properties_log` the right user-facing name?
>>> 3. Is the proposed schema reasonable?
>>> 4. Is reading retained previous metadata files acceptable for this
>>> read-only metadata table?
>>>
>>> If this direction makes sense, I’d also appreciate review on the PR. If
>>> the community thinks this is too narrow or not worth adding, I’m happy to
>>> close it or rework the proposal.
>>>
>>> Best regards,
>>> Tom
>>>
>>

Reply via email to