Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
rdblue merged PR #8834: URL: https://github.com/apache/iceberg/pull/8834 -- 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.apac

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
rdblue commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1360880736 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +460,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1360817555 ## core/src/test/java/org/apache/iceberg/TestManifestReader.java: ## @@ -61,16 +70,14 @@ public void testReaderWithFilterWithoutSelect() throws IOException { Mani

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
advancedxy commented on PR #8834: URL: https://github.com/apache/iceberg/pull/8834#issuecomment-1764363047 > > Other than to revert the optimize in #8336, is it better to invalidate the cached `splitOffsetList`? The proposed change is in the `org.apache.iceberg.BaseFile#put` function: >

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
nastra commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1360559226 ## core/src/test/java/org/apache/iceberg/TestManifestReader.java: ## @@ -61,16 +70,14 @@ public void testReaderWithFilterWithoutSelect() throws IOException { Manif

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-16 Thread via GitHub
bryanck commented on PR #8834: URL: https://github.com/apache/iceberg/pull/8834#issuecomment-1764216831 > Other than to revert the optimize in #8336, is it better to invalidate the cached `splitOffsetList`? The proposed change is in the `org.apache.iceberg.BaseFile#put` function: > `

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-15 Thread via GitHub
advancedxy commented on PR #8834: URL: https://github.com/apache/iceberg/pull/8834#issuecomment-1763676271 Other than to revert the optimize in https://github.com/apache/iceberg/pull/8336, is it better to invalidate the cached `splitOffsetList`? The proposed change is in the `org.apache.ic

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-15 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359928626 ## core/src/test/java/org/apache/iceberg/TestManifestReader.java: ## @@ -41,6 +44,12 @@ public static Object[] parameters() { return new Object[] {1, 2}; } +

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-15 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359928395 ## core/src/test/java/org/apache/iceberg/TableTestBase.java: ## @@ -127,6 +129,7 @@ public class TableTestBase { .withFileSizeInBytes(10) .withPar

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-15 Thread via GitHub
RussellSpitzer commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359918684 ## core/src/test/java/org/apache/iceberg/TestManifestReader.java: ## @@ -41,6 +44,12 @@ public static Object[] parameters() { return new Object[] {1, 2};

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359633367 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -476,7 +472,7 @@ long[] splitOffsetArray() { @Override public List equalityFieldIds() { -retur

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359632733 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359633367 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -476,7 +472,7 @@ long[] splitOffsetArray() { @Override public List equalityFieldIds() { -retur

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359632733 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359633367 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -476,7 +472,7 @@ long[] splitOffsetArray() { @Override public List equalityFieldIds() { -retur

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on PR #8834: URL: https://github.com/apache/iceberg/pull/8834#issuecomment-1763214878 > I'm working on adding a test, but wanted to post this ASAP. I updated a test to cover this case. -- This is an automated message from the Apache Git Service. To respond to the m

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359639050 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
danielcweeks commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359638251 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
danielcweeks commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359638251 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on PR #8834: URL: https://github.com/apache/iceberg/pull/8834#issuecomment-1763167751 I'm working on adding a test, but wanted to post this ASAP. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the UR

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359633367 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -476,7 +472,7 @@ long[] splitOffsetArray() { @Override public List equalityFieldIds() { -retur

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359632733 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359632733 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

Re: [PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck commented on code in PR #8834: URL: https://github.com/apache/iceberg/pull/8834#discussion_r1359632733 ## core/src/main/java/org/apache/iceberg/BaseFile.java: ## @@ -463,11 +463,7 @@ public ByteBuffer keyMetadata() { @Override public List splitOffsets() { -if

[PR] Core: fix reading of split offsets in manifests [iceberg]

2023-10-14 Thread via GitHub
bryanck opened a new pull request, #8834: URL: https://github.com/apache/iceberg/pull/8834 This PR fixes a critical bug in reading split offsets from manifests. A change in https://github.com/apache/iceberg/pull/8336 added caching of the offsets collection in `BaseFile` to avoid reallocatio