Add toYYYYMM function #1726

ygf11 · 2021-09-04T13:45:37Z

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

Summary

impl date16/date32/dateTime32 to YYYYMM UInt32 number.

Changelog

New Feature

Related Issues

Related #853

Test Plan

Unit Tests

Stateless Tests

databend-bot · 2021-09-04T13:45:40Z

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.

PsiACE · 2021-09-04T13:49:22Z

/review @sundy-li

databend-bot · 2021-09-04T13:49:24Z

Take the reviewer to sundy-li

codecov-commenter · 2021-09-04T14:07:46Z

Codecov Report

Merging #1726 (922c86a) into master (0d10503) will decrease coverage by 0%.
The diff coverage is 1%.

@@          Coverage Diff           @@
##           master   #1726   +/-   ##
======================================
- Coverage      72%     72%   -1%     
======================================
  Files         573     574    +1     
  Lines       33938   34002   +64     
======================================
+ Hits        24642   24644    +2     
- Misses       9296    9358   +62

Impacted Files	Coverage Δ
...mon/functions/src/scalars/dates/number_function.rs	`0% <0%> (ø)`
common/functions/src/scalars/dates/date.rs	`90% <100%> (+1%)`	⬆️
store/src/meta_service/meta_service_impl.rs	`76% <0%> (+1%)`	⬆️

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 0d10503...922c86a. Read the comment docs.

sundy-li · 2021-09-04T14:28:51Z

tests/suites/0_stateless/02_0012_function_datetimes.sql

@@ -16,3 +16,9 @@ select today() - 1 = yesterday();
 select today() - yesterday()  = 1;
 select today() + 1 = tomorrow();
 select tomorrow() - today() = 1;
+
+select toYYYYMM(toDateTime(1630320462));
+select toYYYYMM(today() - 30) = toYYYYMM(today()) - 1;


What if today is the 2021-10-31?

sundy-li · 2021-09-04T14:31:31Z

common/functions/src/scalars/dates/number_function.rs

+    t: PhantomData<T>,
+}
+
+pub trait NumberResultFunction {


Great, we can even make it more generic using NumberResultFunction<T> , this can be addressed in another pr to support toYYYYMMDDhhmmss, which returns UInt64.

I will address this in next pr.

BohuTANG · 2021-09-04T15:05:49Z

common/functions/src/scalars/dates/number_function.rs

+        Ok(false)
+    }
+
+    fn eval(&self, _columns: &DataColumnsWithField, _input_rows: usize) -> Result<DataColumn> {


_columns -> columns

sundy-li · 2021-09-05T03:56:01Z

tests/suites/0_stateless/02_0012_function_datetimes.sql

-select toYYYYMM(today() + 30) = toYYYYMM(today()) + 1;
-select toYYYYMM(today()) - toYYYYMM(today() - 30) = 1;
-select toYYYYMM(today() + 30) - toYYYYMM(today()) = 1;
+select toYYYYMM(today() - 31) = toYYYYMM(today()) - 1;


That's not correct too. What if today is 03-01, toYYYYMM(today() - 31) will be toYYYYMM(today()) - 2

@sundy-li Yeah, you are right.
For the case of 03-01, all the four eq expression may return false.
As datafuse does not support inserting date16/date32/datetime32 sql like following now.

mysql> CREATE TABLE test(a UInt8, b date) engine = Memory; Query OK, 0 rows affected (0.01 sec) mysql> desc test; +-------+--------+------+ | Field | Type | Null | +-------+--------+------+ | a | UInt8 | NO | | b | Date16 | NO | +-------+--------+------+ 2 rows in set (0.00 sec) mysql> insert into test(a, b) values(1, toDate(1630320462)); ERROR 1105 (HY000): Code: 10, displayText = create_deserializer does not support type 'Date16'. mysql> insert into test(a, b) values(1, '2021-08-10'); ERROR 1105 (HY000): Code: 10, displayText = create_deserializer does not support type 'Date16'. ``` How can I add the stateless test? Please give some idea.

As datafuse does not support inserting date16/date32/datetime32 sql like following now.

Yes, we will add this in another pr.
Currently

select toUInt32(now()) returns `1630833797` select toUInt16(today()) returns `18875`

How about

select toYYYYMM(toDateTime(1630833797) ) = 202109; select toYYYYMM(toDate(18875) ) = 202109;

sundy-li · 2021-09-05T09:37:59Z

common/functions/src/scalars/dates/number_function.rs

+            DataType::Date16 | DataType::Date32 => {
+                let number_vec_result: Vec<Option<u32>> = columns[0]
+                    .column()
+                    .to_values()?


to_values has poor performance than native array iter.

How about using let col = columns[0].u16()? and let col = columns[0].u32()?.
Then we can have col.iter().map(...)

Is the u16() method in SeriesTrait?

sundy-li · 2021-09-05T14:14:09Z

common/functions/src/scalars/dates/number_function.rs

+    }
+
+    fn eval(&self, columns: &DataColumnsWithField, _input_rows: usize) -> Result<DataColumn> {
+        if columns.is_empty() {


No need to check, already checked in num_arguments, but it's trival.

sundy-li · 2021-09-05T14:17:31Z

common/functions/src/scalars/dates/number_function.rs

+        let data_type = columns[0].data_type();
+        let number_vec: Vec<Option<u32>> = match data_type {
+            DataType::Date16 => {
+                let number_vec_result = columns[0].column().to_array()?.u16()?.iter().map(|value|{


Great, but there are also much more spaces to improve this function.

Constant path to check, which may help in select toYYYYMM( 123455 ) from number(xxx)

Put the whole eval function into execute function, this can be done in toYYYYMMDDhhmmss, but it's return value is u64.

Constant path to check, which may help in select toYYYYMM( 123455 ) from number(xxx)

It is nice to do this, but there are somethings I am not sure:

If I understand correctly , toYYYYMM(123455) is the same as select toYYYYMM(toDate(123455)) or toYYYYMM(toDateTime(123455)), so which one to choose? toDateTime?

Constant may be negative value , what should return for this situation?

After this, select toYYYYMM(number) from number(xxx) will also work. Is it ok?

is the same as select toYYYYMM(toDate(123455)) or toYYYYMM(toDateTime(123455))

It's related to DataType::Date16 or DataType::DateTime ..

Constant may be negative value

Actually, if its logical type is Date16/DateTime32, which physical type is u16 and u32, the constant value will never be negative.

toYYYYMM(number) will never work, we just need to make toYYYYMM(Date/DateTime) work, the inner argument is physically constant.

Let's use

let c = columns[0].column().to_array()?.u16()?; c.apply_cast_numeric(..)

instead, it takes consideration of the null values.

Let's use
....
instead, it takes consideration of the null values.

Got it.

Constant path to check, which may help in select toYYYYMM( 123455 ) from number(xxx)

As DataColumn.to_array() has coverage the Constant case as Series, what did you mean Constant path to check?

Constant path can have better performance. You are right, you can simply use to_array().
But it will build an array with the same number which has poor performance because you need to calculate each row.

Thanks, got it.

sundy-li

LGTM, thanks @ygf11

databend-bot · 2021-09-05T14:18:19Z

CI Passed
Reviewer Approved
Let's Merge

mergify · 2022-03-16T10:20: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 added the pr-feature this PR introduces a new feature to the codebase label Sep 4, 2021

databend-bot requested a review from sundy-li September 4, 2021 13:49

sundy-li reviewed Sep 4, 2021

View reviewed changes

BohuTANG reviewed Sep 4, 2021

View reviewed changes

ygf11 force-pushed the feat_toYYYYMM_function branch from d329a59 to 00c6eea Compare September 5, 2021 03:38

sundy-li reviewed Sep 5, 2021

View reviewed changes

ygf11 added 4 commits September 5, 2021 20:21

feat: impl toYYYYMM function

0668a32

fix: remove unused

cca122e

fix: toYYYYMM stateless_test

2bc4a8f

fix: using native array iter

922c86a

ygf11 force-pushed the feat_toYYYYMM_function branch from 00c6eea to 922c86a Compare September 5, 2021 12:26

sundy-li reviewed Sep 5, 2021

View reviewed changes

sundy-li approved these changes Sep 5, 2021

View reviewed changes

databend-bot merged commit dc0df57 into databendlabs:master Sep 5, 2021

BohuTANG added the pr-need-doc label Sep 5, 2021

This was referenced Sep 7, 2021

DOC-1706: add toYYYYMM date function #1751

Merged

[functions] Functions Working with Date/DateTime #853

Closed

BohuTANG removed the pr-need-doc label Mar 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add toYYYYMM function #1726

Add toYYYYMM function #1726

ygf11 commented Sep 4, 2021 •

edited by sundy-li

Loading

databend-bot commented Sep 4, 2021

PsiACE commented Sep 4, 2021

databend-bot commented Sep 4, 2021

codecov-commenter commented Sep 4, 2021 •

edited

Loading

sundy-li Sep 4, 2021

sundy-li Sep 4, 2021

ygf11 Sep 5, 2021

BohuTANG Sep 4, 2021

sundy-li Sep 5, 2021

ygf11 Sep 5, 2021 •

edited

Loading

sundy-li Sep 5, 2021 •

edited

Loading

ygf11 Sep 5, 2021

sundy-li Sep 5, 2021

ygf11 Sep 5, 2021

sundy-li Sep 5, 2021

sundy-li Sep 5, 2021

sundy-li Sep 5, 2021

ygf11 Sep 6, 2021

sundy-li Sep 6, 2021 •

edited

Loading

sundy-li Sep 6, 2021 •

edited

Loading

ygf11 Sep 6, 2021

sundy-li Sep 6, 2021 •

edited

Loading

ygf11 Sep 6, 2021

sundy-li left a comment

databend-bot commented Sep 5, 2021

mergify bot commented Mar 16, 2022

Add toYYYYMM function #1726

Add toYYYYMM function #1726

Conversation

ygf11 commented Sep 4, 2021 • edited by sundy-li Loading

Summary

Changelog

Related Issues

Test Plan

databend-bot commented Sep 4, 2021

PsiACE commented Sep 4, 2021

databend-bot commented Sep 4, 2021

codecov-commenter commented Sep 4, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ygf11 Sep 5, 2021 • edited Loading

Choose a reason for hiding this comment

sundy-li Sep 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundy-li Sep 6, 2021 • edited Loading

Choose a reason for hiding this comment

sundy-li Sep 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundy-li Sep 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sundy-li left a comment

Choose a reason for hiding this comment

databend-bot commented Sep 5, 2021

mergify bot commented Mar 16, 2022

ygf11 commented Sep 4, 2021 •

edited by sundy-li

Loading

codecov-commenter commented Sep 4, 2021 •

edited

Loading

ygf11 Sep 5, 2021 •

edited

Loading

sundy-li Sep 5, 2021 •

edited

Loading

sundy-li Sep 6, 2021 •

edited

Loading

sundy-li Sep 6, 2021 •

edited

Loading

sundy-li Sep 6, 2021 •

edited

Loading