Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Removed order implementation for days_ms / Interval(DayTime) #285

Merged
merged 1 commit into from Aug 13, 2021

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Aug 13, 2021

The days_ms type cannot be ordered as there is a number of ms m over which (1,m) > (2,0), and thus ordering by days and then by milliseconds is not a valid order. In general such intervals have no order relation.

Thanks a lot to @emkornfield for pointing this here.

All operations relying on order are no longer supported for the logical type Interval(DayTime) (ord, comparison, sort).

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Aug 13, 2021
@jorgecarleitao jorgecarleitao changed the title Removed order from days_ms. Removed order implementation for days_ms. Aug 13, 2021
@jorgecarleitao jorgecarleitao changed the title Removed order implementation for days_ms. Removed order implementation for days_ms / Interval(DayTime) Aug 13, 2021
@codecov
Copy link

codecov bot commented Aug 13, 2021

Codecov Report

Merging #285 (6645a02) into main (86edfd6) will increase coverage by 0.53%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #285      +/-   ##
==========================================
+ Coverage   77.33%   77.86%   +0.53%     
==========================================
  Files         254      255       +1     
  Lines       20694    21235     +541     
==========================================
+ Hits        16003    16535     +532     
- Misses       4691     4700       +9     
Impacted Files Coverage Δ
src/array/ord.rs 77.85% <ø> (+0.51%) ⬆️
src/compute/comparison/mod.rs 93.61% <ø> (+2.89%) ⬆️
src/compute/sort/mod.rs 54.00% <ø> (+0.45%) ⬆️
src/types/mod.rs 56.52% <ø> (+4.52%) ⬆️
src/buffer/immutable.rs 97.12% <0.00%> (-0.14%) ⬇️
src/bitmap/utils/mod.rs 100.00% <0.00%> (ø)
src/bitmap/utils/fmt.rs 96.87% <0.00%> (ø)
src/bitmap/mutable.rs 93.33% <0.00%> (+1.16%) ⬆️
src/buffer/mutable.rs 92.93% <0.00%> (+1.17%) ⬆️
src/io/csv/write/mod.rs 88.88% <0.00%> (+1.38%) ⬆️
... and 1 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 86edfd6...6645a02. Read the comment docs.

@emkornfield
Copy link

I think we landed on officially day milliseconds being unorderable. There was a comment in the java code at some point that MS component was meant to be less than a day (but I don't think it is enforced any place)

@jorgecarleitao jorgecarleitao merged commit 8a5df12 into main Aug 13, 2021
@jorgecarleitao jorgecarleitao deleted the ord_interval branch August 13, 2021 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants