You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/10/28 20:14:07 UTC
[GitHub] [iceberg] RussellSpitzer opened a new pull request #1673: Entries Tables Working with Delete Manifests
RussellSpitzer opened a new pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673
Previously the entries metadatat tables would throw an exception if a table
had been modified with a rowDelta and had a delete manifest. To fix this we
switch the reader based on the type of manifest that is being read by the
metadata table so that both type of manifest can be displayed.
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] RussellSpitzer commented on pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673#issuecomment-718182356
@aokolnychyi + @rdblue - As I noted on slack, once merged I'll rebase the distributed job pr onto this.
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] rdblue commented on pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673#issuecomment-718188878
+1 when tests are passing.
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] aokolnychyi merged pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
aokolnychyi merged pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] rdblue commented on a change in pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673#discussion_r513738098
##########
File path: core/src/main/java/org/apache/iceberg/ManifestEntriesTable.java
##########
@@ -112,28 +113,36 @@ protected long targetSplitSize(TableOperations ops) {
ResidualEvaluator residuals = ResidualEvaluator.unpartitioned(filter);
return CloseableIterable.transform(manifests, manifest ->
- new ManifestReadTask(ops.io(), manifest, fileSchema, schemaString, specString, residuals));
+ new ManifestReadTask(ops.io(), manifest, fileSchema, schemaString, specString, residuals, ops.current()
+ .specsById()));
Review comment:
Nit: I think it's usually better to wrap a line at the start of an argument instead of at a method call in an argument, so `\nops.current().specsById()` instead of `ops.current()\n.specsById()`. Otherwise, it looks like you may be calling `specsById` on `ManifestReadTask`.
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673#discussion_r513743779
##########
File path: core/src/main/java/org/apache/iceberg/ManifestEntriesTable.java
##########
@@ -112,28 +113,36 @@ protected long targetSplitSize(TableOperations ops) {
ResidualEvaluator residuals = ResidualEvaluator.unpartitioned(filter);
return CloseableIterable.transform(manifests, manifest ->
- new ManifestReadTask(ops.io(), manifest, fileSchema, schemaString, specString, residuals));
+ new ManifestReadTask(ops.io(), manifest, fileSchema, schemaString, specString, residuals, ops.current()
+ .specsById()));
Review comment:
Sounds good to me, My preferred is new lines for every arg every time :) But I think this is reasonable too
----------------------------------------------------------------
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.
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
[GitHub] [iceberg] RussellSpitzer commented on pull request #1673: Entries Tables Working with Delete Manifests
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1673:
URL: https://github.com/apache/iceberg/pull/1673#issuecomment-718193819
Forced pushed because I had the wrong author, also fixed the style nit
----------------------------------------------------------------
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.
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