Skip to content

fix(#3023): query expr at(list, numeric) as non-window function#3298

Merged
aceforeverd merged 3 commits into4paradigm:mainfrom
aceforeverd:feat-3023-at
Jun 16, 2023
Merged

fix(#3023): query expr at(list, numeric) as non-window function#3298
aceforeverd merged 3 commits into4paradigm:mainfrom
aceforeverd:feat-3023-at

Conversation

@aceforeverd
Copy link
Copy Markdown
Collaborator

@aceforeverd aceforeverd commented May 30, 2023

FIX support of at as non-window function.

distinct at & lag. Key difference:

  • lag & lead is window functions, and should only used with a window
  • at can be used else where too, like at(split_by_key(col, ",", ":"), 1), a udf function

The rules is not enforced in this PR, should be backward-compatible. So it may work like lag(split(col, ','), 1), but is generally not recommended. Breaking changes may introduce later, see #3023 (comment).

@github-actions github-actions bot added the execute-engine hybridse sql engine label May 30, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2023

Codecov Report

Patch coverage: 76.00% and project coverage change: +0.01 🎉

Comparison is base (d2a8300) 75.58% compared to head (afa83ab) 75.60%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3298      +/-   ##
============================================
+ Coverage     75.58%   75.60%   +0.01%     
  Complexity      393      393              
============================================
  Files           681      686       +5     
  Lines        125697   125852     +155     
  Branches       1186     1186              
============================================
+ Hits          95014    95153     +139     
- Misses        30446    30462      +16     
  Partials        237      237              
Impacted Files Coverage Δ
hybridse/include/codec/list_iterator_codec.h 79.09% <0.00%> (-1.47%) ⬇️
hybridse/src/node/sql_node.cc 76.60% <ø> (ø)
hybridse/include/codec/row_list.h 65.21% <90.00%> (+15.21%) ⬆️
hybridse/src/plan/planner.cc 88.15% <100.00%> (+0.01%) ⬆️
hybridse/src/udf/default_defs/feature_zero_def.cc 83.80% <100.00%> (+0.42%) ⬆️
...ridse/src/udf/default_defs/window_functions_def.cc 84.90% <100.00%> (+2.04%) ⬆️

... and 16 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@aceforeverd aceforeverd changed the title fix(#3023): query expr at(list, numric) fix(#3023): query expr at(list, numeric) as non-window function Jun 1, 2023
@aceforeverd aceforeverd requested review from dl239 and lqy222 June 15, 2023 02:22
@aceforeverd aceforeverd merged commit 8882a0a into 4paradigm:main Jun 16, 2023
@aceforeverd aceforeverd deleted the feat-3023-at branch June 16, 2023 04:46
dl239 pushed a commit that referenced this pull request Jun 29, 2023
)

FIX support of `at` as non-window function, partially resolve #3023 

distinct `at` & `lag`. Key difference:
- `lag` & `lead` is window functions, and should only used with a window
- `at` can be used else where too, like `at(split_by_key(col, ",", ":"), 1)`, a udf function

The rules is not enforced in this PR, should be backward-compatible. So it may work like `lag(split(col, ','), 1)`, but is generally not recommended.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

execute-engine hybridse sql engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants