Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use TypeSignature::Coercible for crypto functions #14826

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Chen-Yuan-Lai
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the common Related to common crate label Feb 23, 2025
@Chen-Yuan-Lai Chen-Yuan-Lai force-pushed the use_coercible_for_crypto branch 3 times, most recently from 3cb1d36 to 2dcee88 Compare February 23, 2025 16:25
@github-actions github-actions bot removed the common Related to common crate label Feb 23, 2025
@Chen-Yuan-Lai
Copy link
Contributor Author

Hi @jayzhan211, I encountered some CI failures in the digest function. It seems the Exact(vec![Utf8View, Utf8View]) case can't be replaced with TypeSignature::Coercible because TypeSignature::Coercible processes each parameter independently, without considering the types of other parameters

[SQL] EXPLAIN SELECT
  digest(column1_utf8view, 'md5') as c
FROM test;
[Diff] (-expected|+actual)
    logical_plan
-   01)Projection: digest(test.column1_utf8view, Utf8View("md5")) AS c
+   01)Projection: digest(test.column1_utf8view, Utf8("md5")) AS c
    02)--TableScan: test projection=[column1_utf8view]

Should we:

  1. Implement a new TypeSignature::Coercible method to support this feature?
  2. Mixing the usage TypeSignature::Coercible and Exact(vec![Utf8View, Utf8View]) in digest function?

I'm not sure which one is better, could you give me some suggestions or other best practices? thanks very much!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeSignature::Coercible for crypto functions
1 participant