Skip to content

Conversation

vustef
Copy link
Collaborator

@vustef vustef commented Sep 7, 2025

Adds support for the async interface provided by iceberg_rust_ffi in RelationalAI/iceberg_rust_ffi#3

Copy link
Collaborator Author

@vustef vustef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


[deps]
Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45"
BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we switch our example to a test, and make this a test dependency.

free_table(state.table)
catch e
# Log but don't throw in finalizer
# TODO: should we keep this log or throw?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's figure this out. Will it show with safelogging?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure tbh. The problem is I guess we cannot catch that exception in raicode no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure what ahppens with exceptions in finalizer, probably nothing, or might crash the process? But also not sure if we can safe log them, given that we don't have access to safe logging. Can we ask somebody what's the best practice here?

@vustef
Copy link
Collaborator Author

vustef commented Sep 24, 2025

Let's just also add a short description here, maybe link iceberg_rust_ffi PR too

@vustef vustef marked this pull request as ready for review September 24, 2025 10:34
@vustef
Copy link
Collaborator Author

vustef commented Sep 24, 2025

The tests are failing, not sure if this would block the merge, but we should fix them

Copy link
Collaborator Author

@vustef vustef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to say that I approve, thanks!
The only dilemma is how to safe-log from the finalizer...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants