Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_app_transaction_version() returns wrong result #2340

Closed
qinix opened this issue Mar 25, 2024 · 6 comments
Closed

get_app_transaction_version() returns wrong result #2340

qinix opened this issue Mar 25, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@qinix
Copy link
Contributor

qinix commented Mar 25, 2024

Environment

Delta-rs version: current main(abafd2d)

Binding: rust


Bug

What happened: get_app_transaction_version() returns empty result

What you expected to happen: get_app_transaction_version() returns previously written transaction versions

How to reproduce it:

#[tokio::main]
async fn main() {
    let dir = tempfile::TempDir::new().unwrap();
    let location = dir.path().to_str().unwrap();
    deltalake::operations::create::CreateBuilder::default()
        .with_location(location.to_string())
        .with_column(
            "col1",
            deltalake::kernel::DataType::Primitive(deltalake::kernel::PrimitiveType::Long),
            false,
            None,
        )
        .await
        .unwrap();

    let table = deltalake::open_table(location).await.unwrap();
    let version = deltalake::operations::transaction::CommitBuilder::default()
        .with_actions(vec![deltalake::kernel::Txn {
            app_id: "myapp".to_string(),
            version: 42,
            last_updated: None,
        }
        .into()])
        .build(
            table
                .state
                .as_ref()
                .map(|f| f as &dyn deltalake::operations::transaction::TableReference),
            table.log_store().clone(),
            deltalake::protocol::DeltaOperation::Write {
                mode: deltalake::protocol::SaveMode::Append,
                partition_by: None,
                predicate: None,
            },
        )
        .unwrap()
        .await
        .unwrap()
        .version();
    assert_eq!(version, 1);
    assert_eq!(table.get_app_transaction_version().get("myapp"), Some(&42));

    let table = deltalake::open_table(location).await.unwrap();
    assert_eq!(table.version(), 1);
    assert_eq!(table.get_app_transaction_version().get("myapp"), Some(&42));
}

More details:

After searching the entire code base, I found the DeltaTableState.app_transaction_version was constructed as Default and never mutated. Seems #2037 is correlated to this issue.

It seems like checkpointing is affected as well.

This function was working on previous version, which was largely used by delta-io/kafka-delta-ingest.

@qinix qinix added the bug Something isn't working label Mar 25, 2024
@vegarsti
Copy link
Contributor

vegarsti commented Jun 3, 2024

This is still the case.

@vegarsti
Copy link
Contributor

vegarsti commented Jun 3, 2024

@qinix Do you recall where in the code you found this?

After searching the entire code base, I found the DeltaTableState.app_transaction_version was constructed as Default and never mutated.

@vegarsti
Copy link
Contributor

vegarsti commented Jun 3, 2024

@rtyler @ion-elgreco Happy to help contribute to this if needed. I'm not yet sure where in the code the fix is needed, though.

@ion-elgreco
Copy link
Collaborator

@rtyler @ion-elgreco Happy to help contribute to this if needed. I'm not yet sure where in the code the fix is needed, though.

Any help is welcome 🤗

Have you checked if this is still the case against main? There was recently a merged PR regarding txs

@vegarsti
Copy link
Contributor

vegarsti commented Jun 3, 2024

@rtyler @ion-elgreco Happy to help contribute to this if needed. I'm not yet sure where in the code the fix is needed, though.

Any help is welcome 🤗

Have you checked if this is still the case against main? There was recently a merged PR regarding txs

Hey, you're right! This is fixed now! I hadn't updated to a recent enough one. Might have been fixed by this one? 88ea110

@vegarsti
Copy link
Contributor

vegarsti commented Jun 3, 2024

(I think this can be closed now!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants