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.

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

Aggregation bucket align (for RedisTimeSeries >= 1.6.0) #30

Merged
merged 1 commit into from
Dec 19, 2021

Conversation

xdev-developer
Copy link
Contributor

Aggregation align support.

ALIGN - Time bucket alignment control for AGGREGATION. This will control the time bucket timestamps by changing the reference timestamp on which a bucket is defined. Possible values:

start or - : The reference timestamp will be the query start interval time ( fromTimestamp ).
end or + : The reference timestamp will be the query end interval time ( toTimestamp ).
A specific timestamp: align the reference timestamp to a specific time.
Note: when not provided alignment is set to 0 .

@codecov
Copy link

codecov bot commented Dec 18, 2021

Codecov Report

Merging #30 (9a1910d) into master (4ebd561) will increase coverage by 0.11%.
The diff coverage is 7.14%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #30      +/-   ##
============================================
+ Coverage     65.56%   65.68%   +0.11%     
- Complexity      580      585       +5     
============================================
  Files           104      105       +1     
  Lines          2053     2066      +13     
  Branches        172      173       +1     
============================================
+ Hits           1346     1357      +11     
- Misses          606      611       +5     
+ Partials        101       98       -3     
Impacted Files Coverage Δ
.../dengliming/redismodule/redistimeseries/Align.java 0.00% <0.00%> (ø)
...ming/redismodule/redistimeseries/RangeOptions.java 72.00% <0.00%> (+22.00%) ⬆️
...redismodule/redistimeseries/protocol/Keywords.java 100.00% <100.00%> (ø)
...g/redismodule/redistimeseries/RedisTimeSeries.java 90.42% <0.00%> (+2.12%) ⬆️

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 4ebd561...9a1910d. Read the comment docs.

@xdev-developer xdev-developer changed the title Aggregation bucket align (for RedisTimeSeries >= 1.6.0) [WIP] Aggregation bucket align (for RedisTimeSeries >= 1.6.0) Dec 18, 2021
@dengliming
Copy link
Owner

@xdev-developer Thanks for your contribution. Is this PR ready to merge?

@xdev-developer
Copy link
Contributor Author

I'm trying add more understandable test.

@dengliming dengliming added enhancement New feature or request RedisTimeSeries labels Dec 18, 2021
@xdev-developer
Copy link
Contributor Author

@dengliming should I ignore this test by default? (it will fail with redis ts < 1.6.0)

@dengliming
Copy link
Owner

@xdev-developer Looks good. Can you check the ci? There are some check style errors. : )

@dengliming
Copy link
Owner

dengliming commented Dec 18, 2021

@dengliming should I ignore this test by default? (it will fail with redis ts < 1.6.0)

Agree. Let's ignore this test by default. We can test it with different versions in the future. e.g. we can introduce custom annotation for test.

@EnabledOnVersion("1.6.0")
public void testAggregationsAlign() {}

@xdev-developer
Copy link
Contributor Author

Yep, will fix soon

@xdev-developer xdev-developer changed the title [WIP] Aggregation bucket align (for RedisTimeSeries >= 1.6.0) Aggregation bucket align (for RedisTimeSeries >= 1.6.0) Dec 18, 2021
@xdev-developer xdev-developer force-pushed the aggregation-align branch 3 times, most recently from b3887f4 to 5eaba48 Compare December 19, 2021 01:53
Signed-off-by: Den Kovalevskyi <xdev.developer@gmail.com>
@dengliming dengliming merged commit 5f68f19 into dengliming:master Dec 19, 2021
@dengliming
Copy link
Owner

@xdev-developer Thanks. That's merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request RedisTimeSeries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants