Skip to content

fix substring function#636

Merged
databend-bot merged 2 commits into
databendlabs:masterfrom
b41sh:fix-substring
May 27, 2021
Merged

fix substring function#636
databend-bot merged 2 commits into
databendlabs:masterfrom
b41sh:fix-substring

Conversation

@b41sh

@b41sh b41sh commented May 26, 2021

Copy link
Copy Markdown
Member

Summary

add DataArrayArithmetic::data_array_unary_arithmetic_op to support unary operation.
add some test case from substring

Changelog

  • Bug Fix
  • Improvement

Related Issues

Fixes #573

Test Plan

Unit Tests
Stateless Tests

@databend-bot databend-bot added pr-bugfix this PR patches a bug in codebase pr-improvement labels May 26, 2021
@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your title and description.

Please review the labels and make any necessary changes.

@codecov-commenter

codecov-commenter commented May 26, 2021

Copy link
Copy Markdown

Codecov Report

Merging #636 (bbbb858) into master (8ed7986) will increase coverage by 0%.
The diff coverage is 81%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #636   +/-   ##
======================================
  Coverage      80%     80%           
======================================
  Files         286     284    -2     
  Lines       13782   13848   +66     
======================================
+ Hits        11094   11150   +56     
- Misses       2688    2698   +10     
Impacted Files Coverage Δ
common/datavalues/src/macros.rs 86% <ø> (ø)
common/functions/src/arithmetics/arithmetic.rs 70% <50%> (-11%) ⬇️
common/datavalues/src/data_array_arithmetic.rs 81% <83%> (+1%) ⬆️
...ommon/datavalues/src/data_array_arithmetic_test.rs 93% <91%> (-1%) ⬇️
common/datavalues/src/data_type.rs 73% <100%> (+2%) ⬆️
common/planners/src/plan_expression_test.rs 97% <0%> (-3%) ⬇️
common/datablocks/src/data_block_kernel.rs 91% <0%> (-2%) ⬇️
common/planners/src/plan_expression.rs 88% <0%> (-1%) ⬇️
common/planners/src/plan_rewriter.rs 51% <0%> (ø)
... and 17 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 8ed7986...bbbb858. Read the comment docs.

Comment thread common/datavalues/src/data_type.rs Outdated
}

#[inline]
pub fn numerical_singed_coercion(val_type: &DataType) -> Result<DataType> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, PTAL @sundy-li

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

numerical_signed_coercion convert unsigned data type to signed

@sundy-li

Copy link
Copy Markdown
Member

Stateless tests are not passed. Please take look at: https://github.com/datafuselabs/datafuse/blob/master/scripts/ci/ci-stateless-tests-standalone.sh

arrow_primitive_array_negate!(&value_array, &coercion_type)
}
// @todo support other unary operation
_ => Result::Err(ErrorCodes::BadArguments("Unsupported unary operation"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to hint what's the op is in the error:
ErrorCodes::BadArguments(format!( "Unsupported unary operation: {:?} as argument", op))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modfied, PTAL @bohutang

@b41sh

b41sh commented May 27, 2021

Copy link
Copy Markdown
Member Author

Stateless tests are not passed. Please take look at: https://github.com/datafuselabs/datafuse/blob/master/scripts/ci/ci-stateless-tests-standalone.sh

I have fixed the test. PTAL @sundy-li

@sundy-li sundy-li left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot
databend-bot merged commit c278fc1 into databendlabs:master May 27, 2021
@bohutang bohutang mentioned this pull request May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix this PR patches a bug in codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SUBSTRING: Unsupported expression: - 3

5 participants