You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ZENOTME (via GitHub)" <gi...@apache.org> on 2025/02/07 11:03:43 UTC
[PR] feat: add apply in transaction to support stack action [iceberg-rust]
ZENOTME opened a new pull request, #949:
URL: https://github.com/apache/iceberg-rust/pull/949
This PR resolve: #596. I refer the implementation from pyiceberg: https://github.com/apache/iceberg-python/blob/4d648bb834963efba886d62e79fa3e1e26288dd0/pyiceberg/table/__init__.py#L258
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2655981130
Hi @liurenjie1024, I think we should resolve this PR first before working on #964. It will affect the interface in transactions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2642680924
cc @Fokko @liurenjie1024 @Xuanwo @sdd
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 merged PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#discussion_r1990812405
##########
crates/iceberg/src/transaction.rs:
##########
@@ -261,7 +286,7 @@ impl<'a> FastAppendAction<'a> {
let mut manifest_stream = self
.snapshot_produce_action
.tx
- .table
+ .current_table
Review Comment:
I realized that there are some functions based on the table. And storing only current metadata may cause inconsistencies easily in the future. So change to store current table instead of metadata
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2713273786
I think this PR is ready to review. cc @Fokko @liurenjie1024 @Xuanwo @sdd
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "jonathanc-n (via GitHub)" <gi...@apache.org>.
jonathanc-n commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2791012883
I think we should try to move this along. cc @liurenjie1024 @Fokko @sdd
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2796252142
> I think we should try to move this along. cc @liurenjie1024 @Fokko @sdd
Thanks, I'll take a look recently.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#discussion_r2053503940
##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -37,7 +38,8 @@ use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdat
/// Table transaction.
pub struct Transaction<'a> {
- table: &'a Table,
+ base_table: &'a Table,
+ current_table: Table,
Review Comment:
Sounds reasonable to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2820395208
cc @ZENOTME Would you help to resolve the conflicts?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#issuecomment-2821393190
> cc @ZENOTME Would you help to resolve the conflicts?
Sure.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "ZENOTME (via GitHub)" <gi...@apache.org>.
ZENOTME commented on code in PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#discussion_r2052452699
##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -37,7 +38,8 @@ use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdat
/// Table transaction.
pub struct Transaction<'a> {
- table: &'a Table,
+ base_table: &'a Table,
+ current_table: Table,
Review Comment:
Some info is getting using the table directly, e.g. https://github.com/apache/iceberg-rust/blob/e264f948a083fc080026285658a6a8cce6b4e781/crates/iceberg/src/transaction/append.rs#L121
So we have to save as the Table. But looks like MetadataTable only use TableMetadata in Table. I think we can refactor it and save TableMetadata after refactor. Seems this will not change the interface, we can do it after this PR
as a refactor?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: add apply in transaction to support stack action [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on code in PR #949:
URL: https://github.com/apache/iceberg-rust/pull/949#discussion_r2052342540
##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -37,7 +38,8 @@ use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdat
/// Table transaction.
pub struct Transaction<'a> {
- table: &'a Table,
+ base_table: &'a Table,
+ current_table: Table,
Review Comment:
How about just save current `TableMetadata`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org