Added `OCT`, `REPEAT` scalar functions. #3092

sunli829 · 2021-11-25T07:16:44Z

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

Summary

Added OCT, REPEAT scalar functions.
Added helper methods:

DataType::is_integer DataType::is_signed_integer DataType::is_unsigned_integer

DataValue::is_integer DataValue::is_signed_integer DataValue::is_unsigned_integer

Changelog

New Feature

Related Issues

fixes #3074, fixes #3068, fixes #3055

Test Plan

Unit Tests

Stateless Tests

databend-bot · 2021-11-25T07:16:47Z

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 · 2021-11-25T07:16:47Z

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.

codecov-commenter · 2021-11-25T07:36:53Z

Codecov Report

Merging #3092 (f5fda2e) into main (507c834) will decrease coverage by 0%.
The diff coverage is 7%.

@@          Coverage Diff           @@
##            main   #3092    +/-   ##
======================================
- Coverage     68%     68%    -1%     
======================================
  Files        652     658     +6     
  Lines      34329   34519   +190     
======================================
+ Hits       23488   23549    +61     
- Misses     10841   10970   +129

Impacted Files	Coverage Δ
common/datavalues/src/data_value.rs	`44% <0%> (-2%)`	⬇️
common/datavalues/src/types/data_type.rs	`62% <0%> (-6%)`	⬇️
common/functions/src/scalars/strings/repeat.rs	`5% <5%> (ø)`
common/functions/src/scalars/strings/oct.rs	`6% <6%> (ø)`
common/functions/src/scalars/strings/string.rs	`83% <100%> (+8%)`	⬆️
common/streams/src/stream_source.rs	`0% <0%> (-100%)`	⬇️
query/src/interpreters/interpreter_insert_into.rs	`78% <0%> (-8%)`	⬇️
...ry/src/datasources/table/fuse/io/block_appender.rs	`92% <0%> (-3%)`	⬇️
query/src/interpreters/plan_scheduler.rs	`32% <0%> (-2%)`	⬇️
query/src/datasources/table/fuse/table.rs	`88% <0%> (-2%)`	⬇️
... and 46 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 507c834...f5fda2e. Read the comment docs.

sundy-li · 2021-11-25T11:12:06Z

common/functions/src/scalars/strings/oct.rs

+    }
+
+    fn return_type(&self, args: &[DataType]) -> Result<DataType> {
+        if !args[0].is_integer() {


args[0] can be DataType::null.

if args[0] is String, we can cast it into Int64 using cast_with_type

MySQL support explicit cast:

select oct('56');
select oct('56a');
select oct(null);

sundy-li · 2021-11-25T11:35:45Z

common/functions/src/scalars/strings/repeat.rs

+            )));
+        }
+
+        if !args[1].is_integer() {


let's just accept args[1] as unsigned integer | String | Null.

Then we can cast column[1] to UInt64Column to reduce the matches.

Cast is not expensive because it's vectorized.

Then we can cast column[1] to UInt64Column to reduce the matches.

Will this cause unnecessary memory allocation?

Yes, but I think it's ok. The old column memory will be dropped immediately if no reference.

In most case, the second column is a DataColumn::Constant, so we can use let times = columns[1].to_minimal_array().cast_with_type(&DataType::UInt64).

Since we already wrote the match macro, it's ok to not change that :)

Let's add String|Null support!

sundy-li · 2021-11-25T11:36:00Z

common/functions/src/scalars/strings/repeat.rs

+    }
+
+    fn return_type(&self, args: &[DataType]) -> Result<DataType> {
+        if !matches!(args[0], DataType::String) {


DataType::Null can be supported.

sundy-li · 2021-11-25T11:37:00Z

tests/suites/0_stateless/02_0018_function_strings_repeat.sql

+select repeat(s, 0) from strings_repeat_sample_3;
+select repeat(s, -5) from strings_repeat_sample_3;
+
+drop table strings_repeat_sample_3;


Newline needed.

sundy-li · 2021-11-25T13:14:23Z

common/functions/src/scalars/strings/repeat.rs

+        for n in $times {
+            let value = n.and_then(|n| {
+                if *n >= 0 {
+                    Some($input_string.repeat(*n as usize))


How about adding max_repeat_times check. I don't want this to be an oom killer.

https://github.com/ClickHouse/ClickHouse/blob/master/src/Functions/repeat.cpp#L26-L32

Bumps [crc32fast](https://github.com/srijs/rust-crc32fast) from 1.2.1 to 1.2.2. - [Release notes](https://github.com/srijs/rust-crc32fast/releases) - [Commits](srijs/rust-crc32fast@v1.2.1...v1.2.2) --- updated-dependencies: - dependency-name: crc32fast dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

Add helper methods `DataType::is_integer`, `DataType::is_signed_integer`, `DataType::is_unsigned_integer`, `DataValue::is_integer`, `DataValue::is_signed_integer` and `DataValue::is_unsigned_integer`.

sunli829 · 2021-11-26T01:51:41Z

done. 🙂

sundy-li

/LGTM

databend-bot · 2021-11-26T02:33:23Z

Wait for another reviewer approval

sunli829 requested a review from BohuTANG as a code owner November 25, 2021 07:16

databend-bot added the pr-feature this PR introduces a new feature to the codebase label Nov 25, 2021

databend-bot added the need-review label Nov 25, 2021

sunli829 marked this pull request as draft November 25, 2021 07:21

sunli829 marked this pull request as ready for review November 25, 2021 07:22

BohuTANG requested a review from sundy-li November 25, 2021 07:26

sundy-li reviewed Nov 25, 2021

View reviewed changes

dependabot bot and others added 2 commits November 26, 2021 09:50

Added OCT, REPEAT scalar functions.

88bffea

Add helper methods `DataType::is_integer`, `DataType::is_signed_integer`, `DataType::is_unsigned_integer`, `DataValue::is_integer`, `DataValue::is_signed_integer` and `DataValue::is_unsigned_integer`.

sunli829 force-pushed the main branch from 2cfc07b to 88bffea Compare November 26, 2021 01:50

Update repeat.rs

f5fda2e

sundy-li approved these changes Nov 26, 2021

View reviewed changes

sundy-li merged commit 71fe902 into databendlabs:main Nov 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `OCT`, `REPEAT` scalar functions. #3092

Added `OCT`, `REPEAT` scalar functions. #3092

sunli829 commented Nov 25, 2021 •

edited by zhang2014

Loading

databend-bot commented Nov 25, 2021

databend-bot commented Nov 25, 2021

codecov-commenter commented Nov 25, 2021 •

edited

Loading

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sunli829 Nov 25, 2021

sundy-li Nov 25, 2021 •

edited

Loading

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sundy-li Nov 25, 2021

sunli829 commented Nov 26, 2021

sundy-li left a comment

databend-bot commented Nov 26, 2021

Added OCT, REPEAT scalar functions. #3092

Added OCT, REPEAT scalar functions. #3092

Conversation

sunli829 commented Nov 25, 2021 • edited by zhang2014 Loading

Summary

Changelog

Related Issues

Test Plan

databend-bot commented Nov 25, 2021

databend-bot commented Nov 25, 2021

codecov-commenter commented Nov 25, 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

sundy-li Nov 25, 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

sunli829 commented Nov 26, 2021

sundy-li left a comment

Choose a reason for hiding this comment

databend-bot commented Nov 26, 2021

Added `OCT`, `REPEAT` scalar functions. #3092

Added `OCT`, `REPEAT` scalar functions. #3092

sunli829 commented Nov 25, 2021 •

edited by zhang2014

Loading

codecov-commenter commented Nov 25, 2021 •

edited

Loading

sundy-li Nov 25, 2021 •

edited

Loading