You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/10/07 12:24:15 UTC
[GitHub] [arrow-datafusion] Jimexist opened a new pull request #1081: [crypto] add blake2b and blake2s functions
Jimexist opened a new pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081
# Which issue does this PR close?
add blake2b and blake2s functions
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Jimexist commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
Jimexist commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-937813833
so strange that i cannot reproduce the errors in CI
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a change in pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#discussion_r725169079
##########
File path: datafusion/Cargo.toml
##########
@@ -39,7 +39,7 @@ path = "src/lib.rs"
[features]
default = ["crypto_expressions", "regex_expressions", "unicode_expressions"]
simd = ["arrow/simd"]
-crypto_expressions = ["md-5", "sha2"]
+crypto_expressions = ["md-5", "sha2", "blake2"]
Review comment:
💯 for making these functions optional
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] houqp commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-939231672
> so strange that i cannot reproduce the errors in CI
@Jimexist are you using the same rustc toolchain that's being used in our CI?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] houqp edited a comment on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
houqp edited a comment on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-939231672
> so strange that i cannot reproduce the errors in CI
@Jimexist are you using a rust toolchain with the same version that's being used in our CI?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
alamb merged pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] xudong963 commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
xudong963 commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-937847916
> so strange that i cannot reproduce the errors in CI
Maybe CI doesn't run with the newest code
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Jimexist commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
Jimexist commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-937813833
so strange that i cannot reproduce the errors in CI
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-939460625
Thanks @Jimexist
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Jimexist commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
Jimexist commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-939303227
> 🤔 there seems to be a build failures but otherwise this looks good to me
>
> https://github.com/apache/arrow-datafusion/pull/1081/checks?check_run_id=3840276390
>
> ```
> error[E0004]: non-exhaustive patterns: `&Blake2b` and `&Blake2s` not covered
> --> /home/runner/work/arrow-datafusion/arrow-datafusion/datafusion/src/physical_plan/functions.rs:319:15
> |
> 174 | / pub enum BuiltinScalarFunction {
> 175 | | // math functions
> 176 | | /// abs
> 177 | | Abs,
> ... |
> 275 | | Blake2b,
> | | ------- not covered
> 276 | | /// Blake2s
> 277 | | Blake2s,
> | | ------- not covered
> ... |
> 305 | | RegexpMatch,
> 306 | | }
> | |_- `BuiltinScalarFunction` defined here
> ...
> 319 | match self {
> | ^^^^ patterns `&Blake2b` and `&Blake2s` not covered
> |
> = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
> = note: the matched value is of type `&BuiltinScalarFunction`
> ```
>
> FWIW I looked at postgres and what it does is rather than having individual functions for different hashing algorithms, it appears to have a single `digest` function that takes the requested algorithm as a string parameter
>
> ```
> F.26.1.1. Digest()
> digest(data text, type text) returns bytea
> digest(data bytea, type text) returns bytea
> ```
>
> https://www.postgresql.org/docs/current/pgcrypto.html
let's address this in #1090 so merge that first
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] xudong963 commented on pull request #1081: [crypto] add blake2b and blake2s functions
Posted by GitBox <gi...@apache.org>.
xudong963 commented on pull request #1081:
URL: https://github.com/apache/arrow-datafusion/pull/1081#issuecomment-937847916
> so strange that i cannot reproduce the errors in CI
Maybe CI doesn't run with the newest code
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org