Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
arkadius commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2503682790 > @arkadius: Could you please backport the change to the relevant older Flink branches? Sure, here it is: #11665 -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
pvary commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2497876484 @arkadius: Could you please backport the change to the relevant older Flink branches? -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
pvary commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2497874666 Thanks for the PR @arkadius and @rodmeneses for the review! -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
pvary merged PR #11244: URL: https://github.com/apache/iceberg/pull/11244 -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
arkadius commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2491435175 > @arkadius: Could you please make sure the tests are run? Just change the commit message of the last commit and force push the branch, or something? Sure, I rebased the branch to main to make sure that all tests will be run on the state that will be present after merge -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
pvary commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2491241065 We definitely need this. @arkadius: Could you please make sure the tests are run? Just change the commit message of the last commit and force push the branch, or something? -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
arkadius commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2490187518 I think that this option would benefit this project in a good transitional migration to the new interface in Table API / Flink SQL. What is the alternative plan for the migration between the old implementation and the new one? -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
rodmeneses commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2489891920 Hi @arkadius is this still needed/relevant ? please advise, as it will be closed due to inactivity soon cc @pvary -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
github-actions[bot] commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2489805105 This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the d...@iceberg.apache.org list. Thank you for your contributions. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
arkadius commented on PR #11244: URL: https://github.com/apache/iceberg/pull/11244#issuecomment-2425998578 @rodmeneses @pvary I've done all fixes that were discussed in comments. Can we move forward with this change? Do we need more eyes to take a look at the docs part? @stevenzwu can you take a look? -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
Re: [PR] Flink: Add table.exec.iceberg.use-v2-sink option [iceberg]
arkadius commented on code in PR #11244:
URL: https://github.com/apache/iceberg/pull/11244#discussion_r1795763231
##
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/TestFlinkTableSinkExtended.java:
##
@@ -165,18 +210,25 @@ public void testWriteParallelism() {
"INSERT INTO %s /*+ OPTIONS('write-parallelism'='1') */ SELECT *
FROM %s",
TABLE, SOURCE_TABLE);
ModifyOperation operation = (ModifyOperation)
planner.getParser().parse(insertSQL).get(0);
-Transformation dummySink =
planner.translate(Collections.singletonList(operation)).get(0);
-Transformation committer = dummySink.getInputs().get(0);
-Transformation writer = committer.getInputs().get(0);
-
-assertThat(writer.getParallelism()).as("Should have the expected 1
parallelism.").isEqualTo(1);
-writer
-.getInputs()
-.forEach(
-input ->
-assertThat(input.getParallelism())
-.as("Should have the expected parallelism.")
-.isEqualTo(isStreamingJob ? 2 : 4));
+Transformation sink =
planner.translate(Collections.singletonList(operation)).get(0);
Review Comment:
I don't think that it is possible as we don't have builders here. Instead,
we have already built a chain of operators. This test works on the table
connectors level - builders are hidden behind the table connector API as an
implementation detail.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
