418 - Add support for Duration datatype #330
Conversation
* Added cass_statement_bind_duration() function to bind a duration parameter (in the form of months, days, nanos) to a statement. * Added cass_value_get_duration() function to get a duration out of a CassValue (in the form of months, days, nanos). * Added example program to show usage.
* Refactored duration encoding logic to a new encode.cpp and created CassDuration type. * Replaced use of flsll with compiler intrinsics to (hopefully) fix the broken build on Linux (where flsll is not defined).
* Fix still broken Linux build.
* Fix still broken Linux build.
* Fix broken Windows build. * Fix minor bug in duration.c example that cause warning in Windows build.
* Added integration tests * Fixed some defects flagged by tests. * Added "invalid value" unit tests for cass_value_get* functions.
* Cleaned up duration example. * Added unit test for decode_zig_zag edge case.
* Moved CASS_VALUE_TYPE_DURATION to end of enum with value 0xff to not conflict with "real" C* types.
* Fixed compile errors seen in Travis (Linux).
* Changed CASS_VALUE_TYPE_DURATION to have value 0xfffd (since 0xffff is UNKNOWN and CASS_VALUE_TYPE_LAST_ENTRY needs a unique value (e.g. 0xfffe)).
* Speculative fix for Windows build failure; added 32-bit Windows impl for num_leading_zeros.
* Move CASS_VALUE_TYPE_DURATION up in the CassValueType enum (just below UNKNOWN) and have CASS_VALUE_TYPE_LAST_ENTRY * Change the data-type-cache in DataTypeDecoder from an array to a dense hash-map.
* Speculative fix for gcc build failure of the cpp dse driver (for Travis).
src/encode.cpp
Outdated
| uint64_t zigzag_values[3]; | ||
|
|
||
| // We need varint sizes for each attribute. | ||
| size_t varint_sizes[3]; |
There was a problem hiding this comment.
What do you think about changing the name of these to "vint" instead of "varint"? "varint" is already a Cassandra type.
There was a problem hiding this comment.
Sounds good; will do.
| inline size_t varint_size(cass_int64_t value) { | ||
| // | with 1 to ensure magnitude <= 63, so (63 - 1) / 7 <= 8 | ||
| size_t magnitude = num_leading_zeros(value | 1); | ||
| return magnitude ? (9 - ((magnitude - 1) / 7)) : 9; |
There was a problem hiding this comment.
This doesn't need a branch: (0 - 1) / 7 == 0
There was a problem hiding this comment.
magnitude is an unsigned type, so subtracting 1 and then dividing by 7 causes it to be junk. We could cast magnitude to an int when doing the calculation instead of the branch...
There was a problem hiding this comment.
Ah, that makes sense. Thanks for the explanation.
src/encode.cpp
Outdated
| // Write the bytes of zigzag value to the data array with most significant byte | ||
| // in first byte of buffer (cur_byte), and so on. | ||
| for (int j = varint_sizes[i] - 1 ; j >= 0 ; --j) { | ||
| *(cur_byte + j) = zigzag_values[i] & 0xff; |
There was a problem hiding this comment.
Should this be logic be encapsulated in an char* encode_vint(char* output, int64_t value, size_t) function?
There was a problem hiding this comment.
Ok, sounds good.
src/utils.hpp
Outdated
| #endif | ||
| } | ||
|
|
||
| inline cass_int64_t decode_zig_zag(cass_uint64_t n) { |
There was a problem hiding this comment.
decode_zig_zag() an encode_zig_zag() might be a better fit in serialization.hpp?
There was a problem hiding this comment.
ahh, didn't realize that was there. Will move it.
src/value.cpp
Outdated
| // will be set. Since that's not the case, this is a one-byte value. | ||
| *outs[ctr] = first_byte; | ||
| } else { | ||
| // The number of consecutive most significant bits of the first-byte tell us how |
There was a problem hiding this comment.
Should this logic also be encapsulated in a function char * decode_vint(char* input, int64_t* out)?
There was a problem hiding this comment.
Sounds good, will do.
|
+1 w/ comments above and the custom type change we talked about. Great work. |
* Miscellaneous refinements based on comments in PR.
* Removed special-casing logic in integration tests, which was mapping CASS_VALUE_TYPE_DURATION to CASS_VALUE_TYPE_CUSTOM. * Added logic in DataTypeDecoder to decode duration's that come in as custom types to the DataType for CASS_VALUE_TYPE_DURATION.
* Implemented mapping custom type to Duration type better.
| private: | ||
| char* buffer_; | ||
| DataType::Ptr data_type_cache_[CASS_VALUE_TYPE_LAST_ENTRY]; | ||
| sparsehash::dense_hash_map<uint16_t, DataType::Ptr> data_type_cache_; |
There was a problem hiding this comment.
I'd like to move this back to using an array now that the potential values are more dense.
CPP-854 - Removing Simulacron in favor of unit tests
2 participants
No description provided.