Other functions migrate to Function2 trait #4036

junli1026 · 2022-01-31T05:30:48Z

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Migrate other functions to Function2

Changelog

Improvement

Related Issues

Fixes #3974

Test Plan

Unit Tests

Stateless Tests

databend-bot · 2022-01-31T05:30:51Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

databend-bot · 2022-01-31T05:30:51Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

vercel · 2022-01-31T05:30:52Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/databend/databend/253HGRXVucQphNZMBWXGfKWq6tYh
✅ Preview: Canceled

[Deployment for 21965c4 canceled]

codecov-commenter · 2022-01-31T06:05:00Z

Codecov Report

Merging #4036 (21965c4) into datavalues-dev (622025e) will increase coverage by 0%.
The diff coverage is 72%.

@@               Coverage Diff               @@
##           datavalues-dev   #4036    +/-   ##
===============================================
  Coverage              56%     56%            
===============================================
  Files                 841     841            
  Lines               45049   45155   +106     
===============================================
+ Hits                25605   25677    +72     
- Misses              19444   19478    +34

Impacted Files	Coverage Δ
common/functions/src/scalars/function_factory.rs	`95% <ø> (-1%)`	⬇️
common/functions/src/scalars/others/ignore.rs	`18% <15%> (-3%)`	⬇️
.../src/scalars/others/running_difference_function.rs	`56% <60%> (-14%)`	⬇️
common/datavalues2/src/columns/nullable/mod.rs	`60% <66%> (-1%)`	⬇️
common/functions/src/scalars/others/inet_aton.rs	`68% <72%> (+<1%)`	⬆️
common/functions/src/scalars/function2_adapter.rs	`80% <88%> (+<1%)`	⬆️
common/functions/src/scalars/others/inet_ntoa.rs	`84% <94%> (+16%)`	⬆️
common/functions/src/scalars/function2_factory.rs	`96% <100%> (+<1%)`	⬆️
common/functions/src/scalars/others/other.rs	`92% <100%> (+3%)`	⬆️
common/datavalues/src/arrays/string/builder.rs	`75% <0%> (-13%)`	⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 622025e...21965c4. Read the comment docs.

sundy-li · 2022-01-31T12:32:58Z

tests/suites/0_stateless/02_function/02_0002_function_others.sql

+SELECT '=== INET_ATON ===';
+SELECT INET_ATON('10.0.5.9');
+SELECT INET_ATON(NULL);
+SELECT INET_ATON('hello');


Should we throw the exceptions if parsing failed?
We can have TRY_INET_ATON later to enable the catch of the error to be null.

sundy-li · 2022-02-02T01:45:34Z

common/functions/src/scalars/function2_adapter.rs

@@ -120,7 +133,12 @@ impl Function2 for Function2Adapter {
                    }
                };

-                let col = NullableColumn::new(col, validity);
+                let col = if col.is_nullable() {
+                    let nullable_column: &NullableColumn = col.as_any().downcast_ref().unwrap();


Series::static_cast

sundy-li · 2022-02-02T01:46:36Z

common/functions/src/scalars/others/ignore.rs

-        Ok(DataColumn::Constant(DataValue::UInt8(Some(0)), input_rows))
+    fn eval(&self, _columns: &ColumnsWithField, input_rows: usize) -> Result<ColumnRef> {
+        let return_type = type_primitive::UInt8Type::arc();
+        let return_value = DataValue::try_from(0_u8)?;


I just thought that we can return Boolean Column instead of U8 Column.

sundy-li · 2022-02-02T01:47:08Z

common/functions/src/scalars/others/inet_aton.rs

-        // For invalid input, the function returns null, so the nullable is true.
-        Ok(DataTypeAndNullable::create(&data_type, true))
+
+        if SUPPRESS_PARSE_ERROR {


sundy-li · 2022-02-02T01:48:45Z

common/functions/src/scalars/others/inet_aton.rs

+            return Ok(builder.build(input_rows));
+        }
+
+        if columns[0].column().data_type_id() == TypeID::Null {


It looks we already did it in the adaptor.

I have passthrough_null is false for this case ...... 😢

sundy-li · 2022-02-02T04:51:33Z

LGTM

junli1026 requested a review from BohuTANG as a code owner January 31, 2022 05:30

databend-bot added pr-improvement labels Jan 31, 2022

databend-bot added the need-review label Jan 31, 2022

vercel bot temporarily deployed to Preview January 31, 2022 05:30 Inactive

junli1026 changed the base branch from main to datavalues-dev January 31, 2022 05:31

junli1026 force-pushed the datavalues/jun/dev branch from 89b59a6 to 60744da Compare January 31, 2022 05:33

vercel bot temporarily deployed to Preview January 31, 2022 05:33 Inactive

BohuTANG requested review from sundy-li and zhyass January 31, 2022 10:56

sundy-li reviewed Jan 31, 2022

View reviewed changes

vercel bot temporarily deployed to Preview February 2, 2022 01:36 Inactive

junli1026 force-pushed the datavalues/jun/dev branch from 4697288 to a3d8a0f Compare February 2, 2022 01:39

vercel bot temporarily deployed to Preview February 2, 2022 01:39 Inactive

sundy-li reviewed Feb 2, 2022

View reviewed changes

junli1026 added 5 commits February 1, 2022 18:16

Apply function2 for ignore

e131698

inet_aton to function2

b7693ae

inet_ntoa to function2

77dbc90

runningDifference to function2

caf7496

Address feedback and add 'try_xxxxxxx' functions

21965c4

junli1026 force-pushed the datavalues/jun/dev branch from a3d8a0f to 21965c4 Compare February 2, 2022 02:50

vercel bot temporarily deployed to Preview February 2, 2022 02:50 Inactive

sundy-li merged commit 86b6037 into datafuselabs:datavalues-dev Feb 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Other functions migrate to Function2 trait #4036

Other functions migrate to Function2 trait #4036

junli1026 commented Jan 31, 2022 •

edited

databend-bot commented Jan 31, 2022

databend-bot commented Jan 31, 2022

vercel bot commented Jan 31, 2022 •

edited

codecov-commenter commented Jan 31, 2022 •

edited

sundy-li Jan 31, 2022

sundy-li Feb 2, 2022

sundy-li Feb 2, 2022

sundy-li Feb 2, 2022

sundy-li Feb 2, 2022

junli1026 Feb 2, 2022 •

edited

sundy-li commented Feb 2, 2022

Other functions migrate to Function2 trait #4036

Other functions migrate to Function2 trait #4036

Conversation

junli1026 commented Jan 31, 2022 • edited

Summary

Changelog

Related Issues

Test Plan

databend-bot commented Jan 31, 2022

databend-bot commented Jan 31, 2022

vercel bot commented Jan 31, 2022 • edited

codecov-commenter commented Jan 31, 2022 • edited

Codecov Report

sundy-li Jan 31, 2022

Choose a reason for hiding this comment

sundy-li Feb 2, 2022

Choose a reason for hiding this comment

sundy-li Feb 2, 2022

Choose a reason for hiding this comment

sundy-li Feb 2, 2022

Choose a reason for hiding this comment

sundy-li Feb 2, 2022

Choose a reason for hiding this comment

junli1026 Feb 2, 2022 • edited

Choose a reason for hiding this comment

sundy-li commented Feb 2, 2022

junli1026 commented Jan 31, 2022 •

edited

vercel bot commented Jan 31, 2022 •

edited

codecov-commenter commented Jan 31, 2022 •

edited

junli1026 Feb 2, 2022 •

edited