You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2023/08/08 15:01:21 UTC
[GitHub] [iceberg] advancedxy opened a new pull request, #8259: core: initial support of multi-arg bucket
advancedxy opened a new pull request, #8259:
URL: https://github.com/apache/iceberg/pull/8259
This commit makes one simple E2E of multi-arg bucket transform works on spark 3.3 and spark 3.4
--
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
[GitHub] [iceberg] advancedxy commented on pull request #8259: core: initial support of multi-arg bucket
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1741674765
> @advancedxy, thanks for your patience on this. I'm going to be looking at it more once the 1.4 release is out.
That sounds reasonable and great to me.
> I have a couple of things that I'm thinking through on this:
>
> * If we're updating the function, should we use a faster algorithm like XXH?
If we are going to add `bucketV2` as the new bucket function for both single-arg and multi-arg parameters. We can certainly use a faster/new hash algorithm.
> * Should we make hash(str) and hash(int) the same to enable type promotion?
Currently, I don't think we can promote int to str due to the truncate/bucket(original) transform since we cannot produce same partition ing value. Unless we add more checks and restrictions about type promotion.
I will also put more thinkings in this, and let's discuss after 1.4 is released .
--
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
[GitHub] [iceberg] rdblue commented on pull request #8259: core: initial support of multi-arg bucket
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1742169815
Good point about the truncate transform not matching. We'll have to check when promoting types that there aren't incompatible transforms either way so we shouldn't over-complicate the bucket function to accomplish 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.
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] core: initial support of multi-arg bucket [iceberg]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-3077451420
@advancedxy Hi, thanks for the create work. Seems the pr has been closed by github actions. Is it possible to push this pr forward? We do need it while integrating [Fluss](https://github.com/apache/fluss) with Iceberg.
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-2347443939
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 dev@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-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
[GitHub] [iceberg] advancedxy commented on a diff in pull request #8259: core: initial support of multi-arg bucket
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#discussion_r1296730292
##########
api/src/main/java/org/apache/iceberg/transforms/Bucket.java:
##########
@@ -265,4 +285,58 @@ protected int hash(BigDecimal value) {
return BucketUtil.hash(value);
}
}
+
+ private static class BucketStructLike extends Bucket<StructLike>
+ implements SerializableFunction<StructLike, Integer> {
+
+ private final Types.StructType type;
+
+ private BucketStructLike(int numBuckets, Types.StructType type) {
+ super(numBuckets);
+ this.type = type;
+ }
+
+ @Override
+ protected int hash(StructLike value) {
+ Hasher hasher = BucketUtil.hasher();
+ boolean isNull = true;
+ for (int i = 0; i < value.size(); i += 1) {
+ Type fieldType = type.fields().get(i).type();
+ Object val = value.get(i, fieldType.typeId().javaClass());
+ if (val == null) {
+ continue;
+ } else {
+ isNull = false;
+ }
+ switch (fieldType.typeId()) {
+ case INTEGER:
+ case DATE:
+ hasher.putLong((long) ((int) val));
+ break;
+ case LONG:
+ case TIME:
+ case TIMESTAMP:
+ hasher.putLong((long) val);
+ break;
+ case DECIMAL:
+ hasher.putBytes(((BigDecimal) val).unscaledValue().toByteArray());
+ break;
+ case STRING:
+ hasher.putString((CharSequence) val, StandardCharsets.UTF_8);
+ break;
+ case FIXED:
+ case BINARY:
+ hasher.putBytes((ByteBuffer) val);
+ break;
+ case UUID:
+ UUID uuid = (UUID) val;
+ hasher.putLong(Long.reverseBytes(uuid.getMostSignificantBits()));
+ hasher.putLong(Long.reverseBytes(uuid.getLeastSignificantBits()));
+ break;
+ }
+ }
+ Preconditions.checkArgument(!isNull, "All fields are null");
Review Comment:
just check here for now, if all fields are null, this bucket function should produce null instead.
--
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
[GitHub] [iceberg] rdblue commented on pull request #8259: core: initial support of multi-arg bucket
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1740103438
@advancedxy, thanks for your patience on this. I'm going to be looking at it more once the 1.4 release is out.
I have a couple of things that I'm thinking through on this:
* If we're updating the function, should we use a faster algorithm like XXH?
* Should we use a different approach to get to a bucket number rather than modulus?
* Should we make hash(str) and hash(int) the same to enable type promotion?
--
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
[GitHub] [iceberg] advancedxy commented on pull request #8259: core: initial support of multi-arg bucket
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1680045177
> This looks interesting, looks like we are discussing first in: https://docs.google.com/document/d/1aDoZqRgvDOOUVAGhvKZbp5vFstjsAMY4EFCyjlxpaaw/edit
Yeah, let's discuss this in the google doc first. I probably would raise a spec PR in this or next week. Once we have discussed this in the next community sync, I will refine and divide the PR into multiple parts. This PR is for reference only to get what the whole picture looks like for Spark(at least) engine: like roughly how many lines of code, which classes would be modified, how the interface would be changed/replaced etc.
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-2365374106
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #8259: core: initial support of multi-arg bucket
URL: https://github.com/apache/iceberg/pull/8259
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1908721572
@advancedxy do you mind taking the generic changes here (not including bucketv2) and splitting into another pr? (Just like we did for the doc changes). I feel we can parallelize more this way.
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1910770730
@advancedxy feel free to work on bucketv2, based on the ideas of @rdblue and @aokolnychyi . We were interested to work on geo partitioning multi-arg partition functions, like z ordering
--
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] core: initial support of multi-arg bucket [iceberg]
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1909241699
> @advancedxy do you mind taking the generic changes here (not including bucketv2) and splitting into another pr? (Just like we did for the doc changes). I feel we can parallelize more this way. Thanks for the great work, by the way.
Of course, I will split the API/Core parts into another prs, it may consist of multiple PRs to be reviewed. As for `bucketV2`, I do think it's important and the motivation of multi-arg transform is to support multi-arg bucket initially. Do you or @aokolnychyi have interest to work on `bucketV2`?
--
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
[GitHub] [iceberg] szehon-ho commented on pull request #8259: core: initial support of multi-arg bucket
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #8259:
URL: https://github.com/apache/iceberg/pull/8259#issuecomment-1679679995
This looks interesting, looks like we are discussing first in: https://docs.google.com/document/d/1aDoZqRgvDOOUVAGhvKZbp5vFstjsAMY4EFCyjlxpaaw/edit
--
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