feat(expr): introduce deprecated to #[function] macro

[wangrunji0408]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR introduces deprecated attribute to #[function] macro. Deprecated functions should not be used in the frontend, but for backward compatibility, they are still in the signature map and are available in the backend.

Current deprecated functions:

• to_timestamp1(varchar, varchar) -> timestamp
• array_length(list) -> int64
• cardinality(list) -> int64
• strpos(varchar, varchar) -> int32

With this feature, can we remove these cases from infer_type_for_special? @xiangjinwu

Checklist

• I have written necessary rustdoc comments
• I have added necessary unit tests and integration tests
• I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
• My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
• All checks passed in ./risedev check (or alias, ./risedev c)
• My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

• My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

• My PR contains user-facing changes.

[codecov]

Codecov Report

Merging #11189 (39bb9dd) into main (cafbd0c) will decrease coverage by 0.01%.
The diff coverage is 69.44%.

@@            Coverage Diff             @@
##             main   #11189      +/-   ##
==========================================
- Coverage   69.73%   69.72%   -0.01%     
==========================================
  Files        1313     1313              
  Lines      224159   224165       +6     
==========================================
- Hits       156307   156296      -11     
- Misses      67852    67869      +17     

Flag	Coverage Δ
rust	69.72% <69.44%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/expr/macro/src/lib.rs	94.73% <ø> (ø)
src/expr/src/agg/mod.rs	74.41% <0.00%> (-1.78%)	⬇️
src/expr/src/expr/build.rs	80.13% <0.00%> (-0.40%)	⬇️
src/expr/src/sig/agg.rs	68.18% <0.00%> (-1.59%)	⬇️
src/expr/src/sig/mod.rs	0.00% <0.00%> (ø)
src/expr/src/sig/table_function.rs	52.38% <0.00%> (-1.28%)	⬇️
src/expr/src/table_function/mod.rs	70.87% <0.00%> (-0.70%)	⬇️
src/expr/src/window_function/state/mod.rs	51.02% <0.00%> (-1.07%)	⬇️
src/expr/src/sig/func.rs	88.59% <87.50%> (-0.79%)	⬇️
src/expr/macro/src/gen.rs	95.52% <100.00%> (+0.01%)	⬆️
... and 6 more

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[xiangjinwu]

Thank you for the support! I was also thinking about updating the macro to mark a signature as "skip during frontend matching, or sqlsmith generation"

Just some related thoughts,

• There are certain signatures that are always rewritten. We may need to similarly mark a signature as frontend-only, or backend-only-but-not-deprecated. (Another solution is to avoid such rewriting and pass timezone in context, of cource.)
  • For example, to_timestamp1(varchar, varchar) -> timestamptz should only be used in frontend, while to_timestamp1(varchar, varchar, varchar) -> timestamptz should only exists in backend
  • But at the same time, date_trunc(varchar, timestamptz) -> timestamptz should only be used in frontend, while date_trunc(varchar, timestamptz, varchar) -> timestamptz works both in frontend and backend
• We may want to add metrics on usage (just a counter) of deprecated expressions / code paths, so that we can confidently clean then up in the future when usage drops.

[xiangjinwu]

Yes we should be able to remove cardinality and array_length from infer_type_for_special.

As mentioned above, deprecated signature should also be excluded from sqlsmith as in #11055

Rest LGTM. Thank you.

[xiangjinwu]

It seems a typo would make it ineffective silently #[xxx(.., nam)]. How can we improve on this? Maybe warn on all unrecognized names?

[wangrunji0408]

I see that we use bae::FromAttributes in the proc-macros of the common crate. Maybe we can borrow it from there.

[xiangjinwu]

CI failure is due to slight change of error message.

[wangrunji0408]

Should I manage to keep the error message as before or modify the test script? Looks like type unknown is better than the new message.

[xiangjinwu]

Should I manage to keep the error message as before or modify the test script? Looks like type unknown is better than the new message.

I do not have a strong opinion on this. One is more general and one is more specific, and it is also more accurate to call it type unknown rather than ::Varchar.

risingwave/src/frontend/src/expr/mod.rs

Line 257 in 43b7273

"could not determine polymorphic type because input has type unknown".into(),

risingwave/src/frontend/src/expr/type_inference/func.rs

Line 53 in 43b7273

"Cannot implicitly cast '{:?}' to polymorphic type {:?}",

[wangrunji0408]

I don't know how to change the code properly. So I just updated test script to let it pass. Maybe refine it later.