Allow 64bit ints for TTL in client.

cpp-redis / cpp_redis

C++11 Lightweight Redis client: async, thread-safe, no dependency, pipelining, multi-platform

MIT License

720 stars 199 forks source link

Allow 64bit ints for TTL in client. #13

Closed dgpetrie closed 5 years ago

dgpetrie commented 5 years ago

Redis server supports 64 bit ints for TTL. Actually up to 0x7FFFFFFFFFFFFFFFLL milliseconds. Or 0x7FFFFFFFFFFFFFFFLL/1000 seconds.

appkins commented 5 years ago

This commit stumps me. I'd like to support 64 bit ints, but don't want to create compatibility issues. I'm tempted to create an alias with pre-compiler support for int/int64 with a 64 bit default, but don't want to hinder cross compatibility with other libraries by introducing an int_t type.

We could namespace the int_t, but it is common for developers to include the cpp_redis namespace which could complicate implementation.

dgpetrie commented 5 years ago

Hi: Thank you for looking at this!

Help me understand the compablity issue. Won't most compliers allow a 32 bit int to be passed where the argument type is a 64 bit int? I think the worst case is a warning isn't it? Or is your concern that there are C++ platforms that don't support 64 bit ints?

We could create a typedef for TTL.

Cheers, Dan

appkins commented 5 years ago

I've looked into this further and believe it will be fine. Worst case scenario is it will negatively impact performance on 32 bit systems if they do not first cast to 32 bit. I would prefer to avoid unnecessary typedefs if they can be avoided, so for now I'll just go ahead and merge the changes.

Thanks for the contribution! If you plan on contributing further, I'm happy to add you to the team for the project. I'd also like to get a contributor list going in the README