delta-io / delta-rs

A native Rust library for Delta Lake, with bindings into Python
https://delta-io.github.io/delta-rs/
Apache License 2.0
2.16k stars 387 forks source link

Extend CommitInfo with version when retrieving the history of a delta table #2191

Open Nordalf opened 7 months ago

Nordalf commented 7 months ago

Description

When retrieving the history of a DeltaTable using Rust, the version of each commit is not included. This is a proposal with code included to extend the CommitInfo struct.

The problem today

let history = match delta_table.history(limit).await {
        Ok(vci) => {
            vci
        },
        Err(_) => Vec::new() // empty
    };

    for h in history {
      // Not able to get the version here
    }

Proposal This proposal makes a minor change to the CommitInfo-struct and how the function commit_infos(...) operates when mapping actions.

In core/src/kernel/models/action.rs extend the CommitInfo with:

    /// The version of the commit - Note: Never included in the json
    #[serde(skip_serializing_if = "Option::is_none")]
    pub version: Option<i64>,

And in core/src/kernel/snapshot/mod.rs expand line 253:255 from:

if let Action::CommitInfo(commit_info) = action {
  return Ok::<_, DeltaTableError>(Some(commit_info));
}

To:

if let Action::CommitInfo(mut commit_info) = action {
  commit_info.version = meta.location.commit_version();
  return Ok::<_, DeltaTableError>(Some(commit_info));
}

_Note: If you are using the SQL-extended command DESCRIBE HISTORY table_name, the version is included._

Use Case

With this "minor" change, we are now able to include the version of each commit:

let history = match delta_table.history(limit).await {
  Ok(vci) => {
    vci
  },
  Err(_) => Vec::new() // empty
};

for h in history {
  println("{}", h.version());
}

Related Issue(s) None

Nordalf commented 7 months ago

I am aware of the issue that this breaks some kind of responsibility of CommitInfo. It is no longer completely matching the JSON-response from the file itself.

Nordalf commented 7 months ago

@rtyler - What do you think of this?