You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "qian0817 (via GitHub)" <gi...@apache.org> on 2025/02/06 07:22:27 UTC
[PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
qian0817 opened a new pull request, #3147:
URL: https://github.com/apache/parquet-java/pull/3147
### Rationale for this change
#3146
### What changes are included in this PR?
Optimize the binaryToDecimal function
### Are these changes tested?
pass unit test.
### Are there any user-facing changes?
No
Closes #3146
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "qian0817 (via GitHub)" <gi...@apache.org>.
qian0817 commented on code in PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#discussion_r1946032903
##########
parquet-pig/src/test/java/org/apache/parquet/pig/TestDecimalUtils.java:
##########
@@ -60,12 +60,12 @@ public void testBinaryToDecimal() throws Exception {
// Test LONG
testDecimalConversion(Long.MAX_VALUE, 19, 0, "9223372036854775807");
testDecimalConversion(Long.MIN_VALUE, 19, 0, "-9223372036854775808");
- testDecimalConversion(0L, 0, 0, "0.0");
+ testDecimalConversion(0L, 0, 0, "0");
Review Comment:
For this use case, 0 is the correct value. Using double to construct BigDecimal previously may lead to potential incorrect behavior.
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "qian0817 (via GitHub)" <gi...@apache.org>.
qian0817 commented on PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#issuecomment-2642067708
> parquet-pig has been discussed to be removed: https://lists.apache.org/thread/vh1twzdbvm4fr4sl2wt8swqgq92k8369
>
> Is it actually used in your case @qian0817?
>
> cc @Fokko
I did not directly use the parquet-pig module; while writing my own parquet converter, I referenced some code from parquet-pig and then discovered the optimization points here.
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#issuecomment-2662594842
@qian0817 I see, thanks for the added context.
Since Pig is deprecated, I'm going to close this PR. That said, I really appreciate taking the time for creating this PR, and hope we'll see more of these in the future 👍
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko closed pull request #3147: [GH-3146] Optimize the binaryToDecimal function
URL: https://github.com/apache/parquet-java/pull/3147
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "qian0817 (via GitHub)" <gi...@apache.org>.
qian0817 commented on PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#issuecomment-2661812252
> Thanks for pinging me here @wgtmac. I've just raised a PR to remove Pig: #3153
>
> @qian0817 Can I ask why you went through the trouble of writing your own converter?
@Fokko I need to read the Parquet file and convert it to our internal system's special format.
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#discussion_r1945998580
##########
parquet-pig/src/test/java/org/apache/parquet/pig/TestDecimalUtils.java:
##########
@@ -60,12 +60,12 @@ public void testBinaryToDecimal() throws Exception {
// Test LONG
testDecimalConversion(Long.MAX_VALUE, 19, 0, "9223372036854775807");
testDecimalConversion(Long.MIN_VALUE, 19, 0, "-9223372036854775808");
- testDecimalConversion(0L, 0, 0, "0.0");
+ testDecimalConversion(0L, 0, 0, "0");
Review Comment:
Why do these two lines need change?
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "qian0817 (via GitHub)" <gi...@apache.org>.
qian0817 commented on code in PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#discussion_r1946032903
##########
parquet-pig/src/test/java/org/apache/parquet/pig/TestDecimalUtils.java:
##########
@@ -60,12 +60,12 @@ public void testBinaryToDecimal() throws Exception {
// Test LONG
testDecimalConversion(Long.MAX_VALUE, 19, 0, "9223372036854775807");
testDecimalConversion(Long.MIN_VALUE, 19, 0, "-9223372036854775808");
- testDecimalConversion(0L, 0, 0, "0.0");
+ testDecimalConversion(0L, 0, 0, "0");
Review Comment:
For this use case, 0 is the correct value. Using double to construct BigDecimal previously may lead to potential errors.
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [GH-3146] Optimize the binaryToDecimal function [parquet-java]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #3147:
URL: https://github.com/apache/parquet-java/pull/3147#issuecomment-2657515753
Thanks for pinging me here @wgtmac. I've just raised a PR to remove Pig: https://github.com/apache/parquet-java/pull/3153
@qian0817 Can I ask why you went through the trouble of writing your own converter?
--
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@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org