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

Fix iterating using only a timestamp column #5

Merged

Conversation

scottmatthewman
Copy link
Contributor

When the cursor is created with only one time column, decoding it would crash, reporting that the columns and values have different sizes.

This is because Array(values) attempts to call #to_ary and #to_a on its arguments before manually wrapping it in an array. This means that Time#to_a is called, which returns an array of 10 elements.

The remedy is to do that manual wrapping ourselves, so that we can ensure that the values are always wrapped in an array, and then call flatten so that if an array was passed in, it is unwrapped back into a single level array.

Fixes #4

When the cursor is created with only one time column, decoding it would
crash, reporting that the columns and values have different sizes.

This is because `Array(values)` attempts to call `#to_ary` and `#to_a`
on its arguments before manually wrapping it in an array. This means
that `Time#to_a` is called, which returns an array of 10 elements.
@fatkodima fatkodima force-pushed the allow-single-time-column-cursors branch from 503ce51 to 9a4699d Compare May 13, 2024 10:34
@fatkodima fatkodima changed the title Fix cursor decoding with only one time column Fix iterating using only a timestamp column May 13, 2024
@@ -48,8 +48,8 @@ def deserialize_time_if_needed(value)
attr_reader :columns, :values

def initialize(columns:, values:)
@columns = Array(columns)
@values = Array(values)
@columns = Array.wrap(columns)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's even neater! Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix! ❤️ I will merge this and release a new version later today.

@jjb jjb merged commit 76c4bf3 into healthie:master May 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Deserialising cursor fails when based solely on a timestamp column
3 participants