Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: 1295 udaf default value for min_where and max_where #1403

Merged
merged 7 commits into from
Mar 10, 2022
Merged

fix: 1295 udaf default value for min_where and max_where #1403

merged 7 commits into from
Mar 10, 2022

Conversation

aceforeverd
Copy link
Collaborator

close #1295

rules:

  1. if no cols matched, return NULL
  2. if all cols matched is NULL, return NULL

@aceforeverd
test: add missing unit test for max, min & count
025ac50
@aceforeverd
fix(4paradigm#1295): fix max_where
c7474f8 
return NULL if no col matches or all col matches is NULL
@aceforeverd
fix(4paradigm#1295): fix min_where
801cb15 
return NULL if no col matches or all col matches is NULL
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2022 •
edited
Loading

Linux Test Report

     112 files       238 suites   44m 8s ⏱️
  8 620 tests   8 617 ✔️ 3 💤 0 ❌
12 752 runs  12 749 ✔️ 3 💤 0 ❌

Results for commit 761f896.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2022 •
edited
Loading

HybridSE Linux Test Report

       65 files       231 suites   5m 45s ⏱️
19 160 tests 19 160 ✔️ 0 💤 0 ❌

Results for commit 761f896.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2022 •
edited
Loading

HybridSE Mac Test Report

       65 files       231 suites   6m 54s ⏱️
19 160 tests 19 160 ✔️ 0 💤 0 ❌

Results for commit 761f896.

♻️ This comment has been updated with latest results.

@aceforeverd
Copy link
Collaborator Author

bugfix-liguo

@codecov
Copy link

codecov bot commented Mar 9, 2022 •
edited
Loading

Codecov Report

Merging #1403 (761f896) into main (62b5157) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1403      +/-   ##
============================================
+ Coverage     65.48%   65.50%   +0.01%     
  Complexity      236      236              
============================================
  Files           571      571              
  Lines        106939   107029      +90     
  Branches        859      859              
============================================
+ Hits          70034    70113      +79     
- Misses        36753    36764      +11     
  Partials        152      152
Impacted Files Coverage Δ
hybridse/src/udf/default_udf_library.cc 96.31% <100.00%> (+0.07%) ⬆️
hybridse/src/udf/udaf_test.cc 100.00% <100.00%> (ø)
src/catalog/sdk_catalog.cc 73.40% <0.00%> (-1.07%) ⬇️
src/replica/log_replicator.cc 77.19% <0.00%> (-0.66%) ⬇️
src/sdk/db_sdk.cc 62.42% <0.00%> (-0.61%) ⬇️
src/sdk/sql_cluster_router.cc 38.12% <0.00%> (-0.36%) ⬇️
src/zk/zk_client.cc 82.92% <0.00%> (-0.28%) ⬇️
src/tablet/tablet_impl.cc 37.81% <0.00%> (-0.03%) ⬇️
src/catalog/tablet_catalog.cc 69.81% <0.00%> (ø)
hybridse/src/udf/literal_traits.h 98.70% <0.00%> (+<0.01%) ⬆️
... and 4 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 62b5157...761f896. Read the comment docs.

@aceforeverd aceforeverd requested a review from tobegit3hub March 9, 2022 16:20
@aceforeverd aceforeverd added this to the v0.5 milestone Mar 9, 2022
@aceforeverd aceforeverd self-assigned this Mar 9, 2022
@aceforeverd aceforeverd added the bug Something isn't working label Mar 9, 2022
@zhanghaohit zhanghaohit self-requested a review March 10, 2022 03:11
zhanghaohit
zhanghaohit approved these changes Mar 10, 2022
View reviewed changes
Copy link
Collaborator

@zhanghaohit zhanghaohit left a comment

Choose a reason for hiding this comment

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

LGTM

@vagetablechicken vagetablechicken enabled auto-merge (squash) March 10, 2022 10:49
@vagetablechicken vagetablechicken merged commit f7e8258 into 4paradigm:main Mar 10, 2022
dl239 pushed a commit to dl239/OpenMLDB that referenced this pull request Mar 11, 2022
@aceforeverd @dl239
fix: 1295 udaf default value for min_where and max_where (4paradi…
47c1dc2 
…gm#1403)

close 4paradigm#1295 

rules: 
1. if no cols matched, return `NULL`
2. if all cols matched is `NULL`, return `NULL`
@lumianph lumianph mentioned this pull request Mar 28, 2022
Closed
@aceforeverd aceforeverd deleted the fix/1295-udaf-default-value branch May 8, 2022 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vagetablechicken vagetablechicken vagetablechicken approved these changes

@zhanghaohit zhanghaohit zhanghaohit approved these changes

@jingchen2222 jingchen2222 Awaiting requested review from jingchen2222

@tobegit3hub tobegit3hub Awaiting requested review from tobegit3hub

Assignees

@aceforeverd aceforeverd

Labels
bug Something isn't working
Projects
None yet
Milestone
v0.5
3 participants
@aceforeverd @zhanghaohit @vagetablechicken