Skip to content

Typecasting data when inserting into Redis #22

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

Merged
merged 11 commits into from
Sep 6, 2021
Merged

Conversation

lsylvester
Copy link
Contributor

Currently when inserting a value into redis using a typed data structure - the value is serialized to redis based on its type, and not the type of the data structure.

As a result - it is possble to insert data that can not be read out again. For example, following the example in the readme:

set = Kredis.set "myset", typed: :datetime
set.add(DateTime.tomorrow, DateTime.yesterday)           # => SADD myset "2021-02-03 00:00:00 +0100" "2021-02-01 00:00:00 +0100"
set << DateTime.tomorrow                                 # => SADD myset "2021-02-03 00:00:00 +0100"
2 == set.size                                            # => SCARD myset
[ DateTime.tomorrow, DateTime.yesterday ] == set.members # => SMEMBERS myset

I get ArgumentError (invalid xmlschema format: "2021-03-02") when trying to read the members out of Redis.

In addition to this - if you inserted values into a unique list/set that didn't match the correct type of the unique set - then they are stored in redis as different values but then converted to be the same value when retrieved.

For example:

set = Kredis.set "iset", typed: :integer
set.add(["1-a", 1, 2])
set.members #=> [1, 1, 2]

which is an unexpected result.

This PR uses ActiveModel::Types to convert perform serialization to redis and casting from the redis stored values. Some of these needed to be slightly different from the existing types as the storage (Redis) doesn't know how to store the same types of data as the relational databases.

An alternative to this approach would be to raise an error if the data passed in did not match the type on the data structure - but I think that casting similar to Active Record would provide a nicer experience.

@dhh
Copy link
Member

dhh commented Sep 3, 2021

This is great. There are a few failures against master now. Would you mind having a look? I'll get this merged then.

@lsylvester
Copy link
Contributor Author

@dhh I have updated the tests on this PR to pass.

The behavior of the decimal test changed as a result of the value being stored in redis being cast to a decimal - so I have added kredis_float to Attributes to allow for the behavior that was previously displayed in this test.

@dhh dhh merged commit f85af53 into rails:main Sep 6, 2021
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.

2 participants