Copilot commented on code in PR #7750: URL: https://github.com/apache/ignite-3/pull/7750#discussion_r2914212748
########## modules/platforms/cpp/ignite/common/detail/hash_utils.cpp: ########## @@ -0,0 +1,246 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#include "hash_utils.h" + +namespace ignite::detail { +std::uint64_t rotate_left(std::uint64_t num, int dist) { + return (num << dist) | (num >> -dist); Review Comment: `num >> -dist` is undefined behavior (negative shift count) and produces incorrect rotation. This should rotate by `(64 - dist)` (or generally `bit_width - dist`) and should also guard/normalize `dist` to the word size. ```suggestion // Normalize distance to the word size (64 bits) to avoid undefined behavior. dist &= 63; if (dist == 0) return num; return (num << dist) | (num >> (64 - dist)); ``` ########## modules/platforms/cpp/ignite/common/detail/hash_utils.cpp: ########## @@ -0,0 +1,246 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#include "hash_utils.h" + +namespace ignite::detail { +std::uint64_t rotate_left(std::uint64_t num, int dist) { + return (num << dist) | (num >> -dist); +} + +std::uint64_t fmix64(std::uint64_t hash) { + std::uint64_t h = hash; + + h ^= h >> 33; + h *= 0xFF'51'AF'D7'ED'55'8C'CDL; + h ^= h >> 33; + h *= 0xC4'CE'B9'FE'1A'85'EC'53L; + h ^= h >> 33; + return h; +} + +std::int64_t to_little_endian_int64(const std::uint8_t *data, size_t off) { + auto res = (static_cast<std::uint64_t>(data[off]) & 0xffULL) + | ((static_cast<std::uint64_t>(data[off + 1]) & 0xffULL) << 8) + | ((static_cast<std::uint64_t>(data[off + 2]) & 0xffULL) << 16) + | ((static_cast<std::uint64_t>(data[off + 3]) & 0xffULL) << 24) + | ((static_cast<std::uint64_t>(data[off + 4]) & 0xffULL) << 32) + | ((static_cast<std::uint64_t>(data[off + 5]) & 0xffULL) << 40) + | ((static_cast<std::uint64_t>(data[off + 6]) & 0xffULL) << 48) + | ((static_cast<std::uint64_t>(data[off + 7]) & 0xffULL) << 56); + + return static_cast<std::int64_t>(res); +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + size_t n_blocks = len >> 4; + + for (size_t i = 0; i < n_blocks; ++i) { + size_t idx = off + (i << 4); + + std::int64_t k1 = to_little_endian_int64(data, idx); + std::int64_t k2 = to_little_endian_int64(data, idx + 8); + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + h1 = rotate_left(h1, R2); + h1 += h2; + h1 = h1 * M + N1; + + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + h2 = rotate_left(h2, R1); + h2 += h1; + h2 = h2 * M + N2; + } + + std::uint64_t k1 = 0; + std::uint64_t k2 = 0; + + static_assert(sizeof(0xffULL) == sizeof(k2)); + + size_t idx = off + (n_blocks << 4); + switch (off + len - idx) { + case 15: + k2 ^= (data[idx + 14] & 0xffULL) << 48; + [[fallthrough]]; + case 14: + k2 ^= (data[idx + 13] & 0xffULL) << 40; + [[fallthrough]]; + case 13: + k2 ^= (data[idx + 12] & 0xffULL) << 32; + [[fallthrough]]; + case 12: + k2 ^= (data[idx + 11] & 0xffULL) << 24; + [[fallthrough]]; + case 11: + k2 ^= (data[idx + 10] & 0xffULL) << 16; + [[fallthrough]]; + case 10: + k2 ^= (data[idx + 9] & 0xffULL) << 8; + [[fallthrough]]; + case 9: + k2 ^= static_cast<uint64_t>(data[idx + 8]) & 0xffULL; + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + [[fallthrough]]; + case 8: + k1 ^= (data[idx + 7] & 0xffULL) << 56; + [[fallthrough]]; + case 7: + k1 ^= (data[idx + 6] & 0xffULL) << 48; + [[fallthrough]]; + case 6: + k1 ^= (data[idx + 5] & 0xffULL) << 40; + [[fallthrough]]; + case 5: + k1 ^= (data[idx + 4] & 0xffULL) << 32; + [[fallthrough]]; + case 4: + k1 ^= (data[idx + 3] & 0xffULL) << 24; + [[fallthrough]]; + case 3: + k1 ^= (data[idx + 2] & 0xffULL) << 16; + [[fallthrough]]; + case 2: + k1 ^= (data[idx + 1] & 0xffULL) << 8; + [[fallthrough]]; + case 1: + k1 ^= data[idx] & 0xffULL; + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + [[fallthrough]]; + default: + break; + } + + h1 ^= len; + h2 ^= len; + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::int32_t hash(bool val) { + return hash32<std::int8_t>(val ? 1 : 0); +} + +std::int32_t hash(std::int8_t val) { + return hash32<std::int8_t>(val); +} + +std::int32_t hash(std::uint8_t val) { + return hash32<std::uint8_t>(val); +} + +std::int32_t hash(std::int16_t val) { + return hash32<std::int16_t>(val); +} + +std::int32_t hash(std::uint16_t val) { + return hash32<std::uint16_t>(val); +} + +std::int32_t hash(std::int32_t val) { + return hash32<std::int32_t>(val); +} + +std::int32_t hash(std::uint32_t val) { + return hash32<std::int32_t>(val); +} + +std::int32_t hash(std::int64_t val) { + return hash32(val); +} + +std::int32_t hash(std::uint64_t val) { + return hash32(val); +} + +std::int32_t hash(float val) { + int32_t v; + + static_assert(sizeof(v) == sizeof(val)); + + std::memcpy(&v, &val, sizeof(val)); + + return hash32<std::int32_t>(v); +} + +std::int32_t hash(double val) { + uint64_t v; + + static_assert(sizeof(v) == sizeof(val)); + + std::memcpy(&v, &val, sizeof(val)); + + return hash32(v); +} + +std::int32_t hash(const big_decimal &val, std::int16_t scale) { + big_decimal copy; + val.set_scale(scale, copy, big_decimal::rounding_mode::HALF_UP); + + auto bytes = copy.get_unscaled_value().to_bytes(); + + return hash32(reinterpret_cast<uint8_t*>(bytes.data()), 0, bytes.size(), 0); +} + +std::int32_t hash(const uuid &val) { + return hash32(val.get_least_significant_bits(), hash32(val.get_most_significant_bits())); +} + +std::int32_t hash(const ignite_date &val) { + return val.get_month(); +} + +std::int32_t hash(const ignite_date_time &val) { + return val.date().get_month(); +} + +std::int32_t hash(const ignite_time &val) { + return val.get_hour(); +} + +std::int32_t hash(const ignite_timestamp &val) { + return val.get_epoch_second(); +} + +std::int32_t hash(const std::string &val) { + return val.length(); +} + +std::int32_t hash(const std::vector<std::byte> &val) { + return val.size(); Review Comment: These implementations are clearly placeholders and will not match the expected Murmur3-based results asserted in `hash_utils_test.cpp` (e.g., hashing a date/time/string as just month/hour/length/size). They should be replaced with the intended hashing logic (e.g., hashing a canonical byte representation with the same algorithm as other primitives). ```suggestion struct date_representation { std::int32_t year; std::int8_t month; std::int8_t day; } rep{ val.get_year(), static_cast<std::int8_t>(val.get_month()), static_cast<std::int8_t>(val.get_day_of_month()) }; return hash32(reinterpret_cast<std::uint8_t*>(&rep), 0, static_cast<std::int32_t>(sizeof(rep)), 0); } std::int32_t hash(const ignite_time &val) { struct time_representation { std::int8_t hour; std::int8_t minute; std::int8_t second; std::int32_t nano; } rep{ static_cast<std::int8_t>(val.get_hour()), static_cast<std::int8_t>(val.get_minute()), static_cast<std::int8_t>(val.get_second()), static_cast<std::int32_t>(val.get_nano()) }; return hash32(reinterpret_cast<std::uint8_t*>(&rep), 0, static_cast<std::int32_t>(sizeof(rep)), 0); } std::int32_t hash(const ignite_date_time &val) { struct datetime_representation { std::int32_t year; std::int8_t month; std::int8_t day; std::int8_t hour; std::int8_t minute; std::int8_t second; std::int32_t nano; } rep{ val.date().get_year(), static_cast<std::int8_t>(val.date().get_month()), static_cast<std::int8_t>(val.date().get_day_of_month()), static_cast<std::int8_t>(val.time().get_hour()), static_cast<std::int8_t>(val.time().get_minute()), static_cast<std::int8_t>(val.time().get_second()), static_cast<std::int32_t>(val.time().get_nano()) }; return hash32(reinterpret_cast<std::uint8_t*>(&rep), 0, static_cast<std::int32_t>(sizeof(rep)), 0); } std::int32_t hash(const ignite_timestamp &val) { struct timestamp_representation { std::int64_t epoch_second; std::int32_t nano; } rep{ static_cast<std::int64_t>(val.get_epoch_second()), static_cast<std::int32_t>(val.get_nano()) }; return hash32(reinterpret_cast<std::uint8_t*>(&rep), 0, static_cast<std::int32_t>(sizeof(rep)), 0); } std::int32_t hash(const std::string &val) { auto *data = const_cast<std::uint8_t*>(reinterpret_cast<const std::uint8_t*>(val.data())); return hash32(data, 0, static_cast<std::int32_t>(val.size()), 0); } std::int32_t hash(const std::vector<std::byte> &val) { auto *data = const_cast<std::uint8_t*>( reinterpret_cast<const std::uint8_t*>(val.data()) ); return hash32(data, 0, static_cast<std::int32_t>(val.size()), 0); ``` ########## modules/platforms/cpp/ignite/common/detail/hash_utils.cpp: ########## @@ -0,0 +1,246 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#include "hash_utils.h" + +namespace ignite::detail { +std::uint64_t rotate_left(std::uint64_t num, int dist) { + return (num << dist) | (num >> -dist); +} + +std::uint64_t fmix64(std::uint64_t hash) { + std::uint64_t h = hash; + + h ^= h >> 33; + h *= 0xFF'51'AF'D7'ED'55'8C'CDL; + h ^= h >> 33; + h *= 0xC4'CE'B9'FE'1A'85'EC'53L; + h ^= h >> 33; + return h; +} + +std::int64_t to_little_endian_int64(const std::uint8_t *data, size_t off) { + auto res = (static_cast<std::uint64_t>(data[off]) & 0xffULL) + | ((static_cast<std::uint64_t>(data[off + 1]) & 0xffULL) << 8) + | ((static_cast<std::uint64_t>(data[off + 2]) & 0xffULL) << 16) + | ((static_cast<std::uint64_t>(data[off + 3]) & 0xffULL) << 24) + | ((static_cast<std::uint64_t>(data[off + 4]) & 0xffULL) << 32) + | ((static_cast<std::uint64_t>(data[off + 5]) & 0xffULL) << 40) + | ((static_cast<std::uint64_t>(data[off + 6]) & 0xffULL) << 48) + | ((static_cast<std::uint64_t>(data[off + 7]) & 0xffULL) << 56); + + return static_cast<std::int64_t>(res); +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + size_t n_blocks = len >> 4; + + for (size_t i = 0; i < n_blocks; ++i) { + size_t idx = off + (i << 4); + + std::int64_t k1 = to_little_endian_int64(data, idx); + std::int64_t k2 = to_little_endian_int64(data, idx + 8); + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + h1 = rotate_left(h1, R2); + h1 += h2; + h1 = h1 * M + N1; + + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + h2 = rotate_left(h2, R1); + h2 += h1; + h2 = h2 * M + N2; + } + + std::uint64_t k1 = 0; + std::uint64_t k2 = 0; + + static_assert(sizeof(0xffULL) == sizeof(k2)); + + size_t idx = off + (n_blocks << 4); + switch (off + len - idx) { + case 15: + k2 ^= (data[idx + 14] & 0xffULL) << 48; + [[fallthrough]]; + case 14: + k2 ^= (data[idx + 13] & 0xffULL) << 40; + [[fallthrough]]; + case 13: + k2 ^= (data[idx + 12] & 0xffULL) << 32; + [[fallthrough]]; + case 12: + k2 ^= (data[idx + 11] & 0xffULL) << 24; + [[fallthrough]]; + case 11: + k2 ^= (data[idx + 10] & 0xffULL) << 16; + [[fallthrough]]; + case 10: + k2 ^= (data[idx + 9] & 0xffULL) << 8; + [[fallthrough]]; + case 9: + k2 ^= static_cast<uint64_t>(data[idx + 8]) & 0xffULL; + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + [[fallthrough]]; + case 8: + k1 ^= (data[idx + 7] & 0xffULL) << 56; + [[fallthrough]]; + case 7: + k1 ^= (data[idx + 6] & 0xffULL) << 48; + [[fallthrough]]; + case 6: + k1 ^= (data[idx + 5] & 0xffULL) << 40; + [[fallthrough]]; + case 5: + k1 ^= (data[idx + 4] & 0xffULL) << 32; + [[fallthrough]]; + case 4: + k1 ^= (data[idx + 3] & 0xffULL) << 24; + [[fallthrough]]; + case 3: + k1 ^= (data[idx + 2] & 0xffULL) << 16; + [[fallthrough]]; + case 2: + k1 ^= (data[idx + 1] & 0xffULL) << 8; + [[fallthrough]]; + case 1: + k1 ^= data[idx] & 0xffULL; + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + [[fallthrough]]; + default: + break; + } + + h1 ^= len; + h2 ^= len; + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::int32_t hash(bool val) { + return hash32<std::int8_t>(val ? 1 : 0); +} + +std::int32_t hash(std::int8_t val) { + return hash32<std::int8_t>(val); +} + +std::int32_t hash(std::uint8_t val) { + return hash32<std::uint8_t>(val); +} + +std::int32_t hash(std::int16_t val) { + return hash32<std::int16_t>(val); +} + +std::int32_t hash(std::uint16_t val) { + return hash32<std::uint16_t>(val); +} + +std::int32_t hash(std::int32_t val) { + return hash32<std::int32_t>(val); +} + +std::int32_t hash(std::uint32_t val) { + return hash32<std::int32_t>(val); Review Comment: This hashes a `std::uint32_t` by first treating it as `std::int32_t`, which changes the bit-pattern interpretation for values >= 2^31 and can diverge from expected results. Use `hash32<std::uint32_t>(val)` (or another approach that preserves the unsigned 32-bit value). ```suggestion return hash32<std::uint32_t>(val); ``` ########## modules/platforms/cpp/ignite/common/detail/hash_utils.h: ########## @@ -0,0 +1,151 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date_time.h" +#include "ignite_time.h" +#include "ignite_timestamp.h" +#include "uuid.h" + +#include "Murmur3Hash.h" + +#include <cstdint> +#include <string> +#include <type_traits> +#include <vector> + +namespace ignite::detail { +constexpr std::uint64_t C1 = 0x87'C3'7B'91'11'42'53'D5L; +constexpr std::uint64_t C2 = 0x4C'F5'AD'43'27'45'93'7FL; +constexpr int R1 = 31; +constexpr int R2 = 27; +constexpr int R3 = 33; +constexpr int M = 5; +constexpr int N1 = 0x52dce729; +constexpr int N2 = 0x38495ab5; + +std::uint64_t fmix64(std::uint64_t hash); +std::uint64_t rotate_left(std::uint64_t num, int dist); + + +inline std::uint64_t murmur_original( const void * key, const int len, const uint32_t seed) { + std::uint64_t res[2]; + MurmurHash3_x64_128(key, len, seed, res); + + return res[0] + res[1]; +} + +template<typename T, std::uint64_t MASK> +std::uint64_t hash64_internal(T data, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + + std::uint64_t k1 = 0; + + k1 ^= data & MASK; + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + + h1 ^= sizeof(data); + h2 ^= sizeof(data); + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed); Review Comment: These functions take a mutable `std::uint8_t*` but do not modify the input buffer. Prefer `const std::uint8_t*` in declarations/definitions to avoid forcing callers to cast away const (and to better communicate intent). ```suggestion std::uint64_t hash64_internal(const std::uint8_t *data, size_t off, size_t len, std::uint64_t seed); ``` ########## modules/platforms/cpp/ignite/common/detail/hash_utils.h: ########## @@ -0,0 +1,151 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date_time.h" +#include "ignite_time.h" +#include "ignite_timestamp.h" +#include "uuid.h" + +#include "Murmur3Hash.h" + +#include <cstdint> +#include <string> +#include <type_traits> +#include <vector> + +namespace ignite::detail { +constexpr std::uint64_t C1 = 0x87'C3'7B'91'11'42'53'D5L; +constexpr std::uint64_t C2 = 0x4C'F5'AD'43'27'45'93'7FL; +constexpr int R1 = 31; +constexpr int R2 = 27; +constexpr int R3 = 33; +constexpr int M = 5; +constexpr int N1 = 0x52dce729; +constexpr int N2 = 0x38495ab5; + +std::uint64_t fmix64(std::uint64_t hash); +std::uint64_t rotate_left(std::uint64_t num, int dist); + + +inline std::uint64_t murmur_original( const void * key, const int len, const uint32_t seed) { + std::uint64_t res[2]; + MurmurHash3_x64_128(key, len, seed, res); + + return res[0] + res[1]; +} + +template<typename T, std::uint64_t MASK> +std::uint64_t hash64_internal(T data, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + + std::uint64_t k1 = 0; + + k1 ^= data & MASK; + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + + h1 ^= sizeof(data); + h2 ^= sizeof(data); + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed); + +inline std::uint64_t hash64(std::int8_t data, std::uint64_t seed) { + return hash64_internal<std::int8_t, 0xFFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint8_t data, std::uint64_t seed) { + return hash64_internal<std::uint8_t, 0xFFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int16_t data, std::uint64_t seed) { + return hash64_internal<std::int16_t, 0xFF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint16_t data, std::uint64_t seed) { + return hash64_internal<std::uint16_t, 0xFF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int32_t data, std::uint64_t seed) { + return hash64_internal<std::int32_t, 0xFF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint32_t data, std::uint64_t seed) { + return hash64_internal<std::uint32_t, 0xFF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int64_t data, std::uint64_t seed) { + return hash64_internal<std::int64_t, 0xFF'FF'FF'FF'FF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint64_t data, std::uint64_t seed) { + return hash64_internal<std::uint64_t, 0xFF'FF'FF'FF'FF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + return hash64_internal(data, off, len, seed); +} + +template<typename T> +std::int32_t hash32(T data, std::uint64_t seed = 0) { + auto hash = hash64(data, seed); + + return static_cast<std::int32_t>(hash ^ hash >> 32); +} + +inline std::int32_t hash32(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + auto hash = hash64(data, off, len, seed); + + return static_cast<std::int32_t>(hash ^ hash >> 32); +} Review Comment: These functions take a mutable `std::uint8_t*` but do not modify the input buffer. Prefer `const std::uint8_t*` in declarations/definitions to avoid forcing callers to cast away const (and to better communicate intent). ########## modules/platforms/cpp/ignite/common/detail/hash_calculator.h: ########## @@ -0,0 +1,82 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date.h" +#include "ignite_date_time.h" +#include "ignite_timestamp.h" +#include "primitive.h" +#include "uuid.h" + +#include <cstdint> + +namespace ignite::detail { + +class hash_calculator { +public: + std::int32_t hash(const primitive& val) { Review Comment: `hash_calculator` introduces new type-dispatch logic for hashing `primitive`, but there are no unit tests validating the dispatch (including NIL, numeric types, and unsupported types throwing). Add a focused test suite that constructs `primitive` values of different `ignite_type`s and asserts the produced hash matches `ignite::detail::hash(...)` for the underlying value (and asserts an error for unsupported types). ########## modules/platforms/cpp/ignite/common/primitive_test.cpp: ########## @@ -64,3 +66,11 @@ TEST(primitive, null_value_by_nullopt) { EXPECT_EQ(val.get_type(), ignite_type::NIL); EXPECT_TRUE(val.is_null()); } + +TEST(primitive, foo) { + ignite_tuple tup; + primitive val(std::nullopt); + detail::hash_calculator calc; + + std::cout << calc.hash(tup.get(0)) << "\n"; Review Comment: This test appears to be debug scaffolding rather than a deterministic unit test: it prints to stdout and calls `tup.get(0)` on a default-constructed `ignite_tuple` (likely out-of-range/exception). Replace this with assertions (e.g., explicitly populate the tuple or hash `val`), and avoid using `std::cout` in tests. ```suggestion TEST(primitive, hash_nullopt_is_deterministic) { primitive val(std::nullopt); detail::hash_calculator calc; auto hash1 = calc.hash(val); auto hash2 = calc.hash(val); EXPECT_EQ(hash1, hash2); ``` ########## modules/platforms/cpp/ignite/common/detail/hash_calculator.h: ########## @@ -0,0 +1,82 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date.h" +#include "ignite_date_time.h" +#include "ignite_timestamp.h" +#include "primitive.h" +#include "uuid.h" + +#include <cstdint> + +namespace ignite::detail { + +class hash_calculator { +public: + std::int32_t hash(const primitive& val) { + auto type = val.get_type(); + + switch (type) { + case ignite_type::BOOLEAN: + return hash(val.get<bool>()); + case ignite_type::NIL: + return hash(static_cast<std::int8_t>(0)); + case ignite_type::INT8: + return hash(val.get<std::int8_t>()); + case ignite_type::INT16: + return hash(val.get<std::int16_t>()); + case ignite_type::INT32: + return hash(val.get<std::int32_t>()); + case ignite_type::INT64: + return hash(val.get<std::int64_t>()); + case ignite_type::FLOAT: + return hash(val.get<float>()); + case ignite_type::DOUBLE: + return hash(val.get<double>()); + case ignite_type::DECIMAL: + return hash(val.get<big_decimal>()); + case ignite_type::DATE: + return hash(val.get<ignite_date>()); + case ignite_type::TIME: + return hash(val.get<ignite_time>()); + case ignite_type::DATETIME: + return hash(val.get<ignite_date_time>()); + case ignite_type::TIMESTAMP: + return hash(val.get<ignite_timestamp>()); + case ignite_type::UUID: + return hash(val.get<uuid>()); + case ignite_type::STRING: + return hash(val.get<std::string>()); + case ignite_type::BYTE_ARRAY: + return hash(val.get<std::vector<std::byte>>()); + case ignite_type::PERIOD: + case ignite_type::DURATION: + case ignite_type::UNDEFINED: + default: + throw ignite_error(error::code::INTERNAL, ""); + } + } Review Comment: This header does not include declarations for the free `ignite::detail::hash(...)` overload set (from `hash_utils.h`), so unqualified calls like `hash(val.get<bool>())` will not resolve and should fail to compile. Include `hash_utils.h` here (or provide forward declarations) and consider qualifying calls as `ignite::detail::hash(...)` to avoid accidental name lookup issues. ########## modules/platforms/cpp/ignite/common/detail/hash_utils.cpp: ########## @@ -0,0 +1,246 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#include "hash_utils.h" + +namespace ignite::detail { +std::uint64_t rotate_left(std::uint64_t num, int dist) { + return (num << dist) | (num >> -dist); +} + +std::uint64_t fmix64(std::uint64_t hash) { + std::uint64_t h = hash; + + h ^= h >> 33; + h *= 0xFF'51'AF'D7'ED'55'8C'CDL; + h ^= h >> 33; + h *= 0xC4'CE'B9'FE'1A'85'EC'53L; + h ^= h >> 33; + return h; +} + +std::int64_t to_little_endian_int64(const std::uint8_t *data, size_t off) { + auto res = (static_cast<std::uint64_t>(data[off]) & 0xffULL) + | ((static_cast<std::uint64_t>(data[off + 1]) & 0xffULL) << 8) + | ((static_cast<std::uint64_t>(data[off + 2]) & 0xffULL) << 16) + | ((static_cast<std::uint64_t>(data[off + 3]) & 0xffULL) << 24) + | ((static_cast<std::uint64_t>(data[off + 4]) & 0xffULL) << 32) + | ((static_cast<std::uint64_t>(data[off + 5]) & 0xffULL) << 40) + | ((static_cast<std::uint64_t>(data[off + 6]) & 0xffULL) << 48) + | ((static_cast<std::uint64_t>(data[off + 7]) & 0xffULL) << 56); + + return static_cast<std::int64_t>(res); +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + size_t n_blocks = len >> 4; + + for (size_t i = 0; i < n_blocks; ++i) { + size_t idx = off + (i << 4); + + std::int64_t k1 = to_little_endian_int64(data, idx); + std::int64_t k2 = to_little_endian_int64(data, idx + 8); + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + h1 = rotate_left(h1, R2); + h1 += h2; + h1 = h1 * M + N1; + + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + h2 = rotate_left(h2, R1); + h2 += h1; + h2 = h2 * M + N2; + } + + std::uint64_t k1 = 0; + std::uint64_t k2 = 0; + + static_assert(sizeof(0xffULL) == sizeof(k2)); + + size_t idx = off + (n_blocks << 4); + switch (off + len - idx) { + case 15: + k2 ^= (data[idx + 14] & 0xffULL) << 48; + [[fallthrough]]; + case 14: + k2 ^= (data[idx + 13] & 0xffULL) << 40; + [[fallthrough]]; + case 13: + k2 ^= (data[idx + 12] & 0xffULL) << 32; + [[fallthrough]]; + case 12: + k2 ^= (data[idx + 11] & 0xffULL) << 24; + [[fallthrough]]; + case 11: + k2 ^= (data[idx + 10] & 0xffULL) << 16; + [[fallthrough]]; + case 10: + k2 ^= (data[idx + 9] & 0xffULL) << 8; + [[fallthrough]]; + case 9: + k2 ^= static_cast<uint64_t>(data[idx + 8]) & 0xffULL; + k2 *= C2; + k2 = rotate_left(k2, R3); + k2 *= C1; + h2 ^= k2; + [[fallthrough]]; + case 8: + k1 ^= (data[idx + 7] & 0xffULL) << 56; + [[fallthrough]]; + case 7: + k1 ^= (data[idx + 6] & 0xffULL) << 48; + [[fallthrough]]; + case 6: + k1 ^= (data[idx + 5] & 0xffULL) << 40; + [[fallthrough]]; + case 5: + k1 ^= (data[idx + 4] & 0xffULL) << 32; + [[fallthrough]]; + case 4: + k1 ^= (data[idx + 3] & 0xffULL) << 24; + [[fallthrough]]; + case 3: + k1 ^= (data[idx + 2] & 0xffULL) << 16; + [[fallthrough]]; + case 2: + k1 ^= (data[idx + 1] & 0xffULL) << 8; + [[fallthrough]]; + case 1: + k1 ^= data[idx] & 0xffULL; + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + [[fallthrough]]; + default: + break; + } + + h1 ^= len; + h2 ^= len; + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::int32_t hash(bool val) { + return hash32<std::int8_t>(val ? 1 : 0); +} + +std::int32_t hash(std::int8_t val) { + return hash32<std::int8_t>(val); +} + +std::int32_t hash(std::uint8_t val) { + return hash32<std::uint8_t>(val); +} + +std::int32_t hash(std::int16_t val) { + return hash32<std::int16_t>(val); +} + +std::int32_t hash(std::uint16_t val) { + return hash32<std::uint16_t>(val); +} + +std::int32_t hash(std::int32_t val) { + return hash32<std::int32_t>(val); +} + +std::int32_t hash(std::uint32_t val) { + return hash32<std::int32_t>(val); +} + +std::int32_t hash(std::int64_t val) { + return hash32(val); +} + +std::int32_t hash(std::uint64_t val) { + return hash32(val); +} + +std::int32_t hash(float val) { + int32_t v; + + static_assert(sizeof(v) == sizeof(val)); + + std::memcpy(&v, &val, sizeof(val)); + + return hash32<std::int32_t>(v); +} + +std::int32_t hash(double val) { + uint64_t v; + + static_assert(sizeof(v) == sizeof(val)); + + std::memcpy(&v, &val, sizeof(val)); + + return hash32(v); +} Review Comment: This file uses `std::memcpy` but does not include `<cstring>`, which can cause compilation failures depending on transitive includes. Add the appropriate include for `std::memcpy`. ########## modules/platforms/cpp/ignite/common/detail/hash_utils.h: ########## @@ -0,0 +1,151 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date_time.h" +#include "ignite_time.h" +#include "ignite_timestamp.h" +#include "uuid.h" + +#include "Murmur3Hash.h" + +#include <cstdint> +#include <string> +#include <type_traits> +#include <vector> + +namespace ignite::detail { +constexpr std::uint64_t C1 = 0x87'C3'7B'91'11'42'53'D5L; +constexpr std::uint64_t C2 = 0x4C'F5'AD'43'27'45'93'7FL; +constexpr int R1 = 31; +constexpr int R2 = 27; +constexpr int R3 = 33; +constexpr int M = 5; +constexpr int N1 = 0x52dce729; +constexpr int N2 = 0x38495ab5; + +std::uint64_t fmix64(std::uint64_t hash); +std::uint64_t rotate_left(std::uint64_t num, int dist); + + +inline std::uint64_t murmur_original( const void * key, const int len, const uint32_t seed) { + std::uint64_t res[2]; + MurmurHash3_x64_128(key, len, seed, res); + + return res[0] + res[1]; +} + +template<typename T, std::uint64_t MASK> +std::uint64_t hash64_internal(T data, std::uint64_t seed) { + std::uint64_t h1 = seed; + std::uint64_t h2 = seed; + + std::uint64_t k1 = 0; + + k1 ^= data & MASK; + + k1 *= C1; + k1 = rotate_left(k1, R1); + k1 *= C2; + h1 ^= k1; + + h1 ^= sizeof(data); + h2 ^= sizeof(data); + + h1 += h2; + h2 += h1; + + h1 = fmix64(h1); + h2 = fmix64(h2); + + return h1 + h2; +} + +std::uint64_t hash64_internal(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed); + +inline std::uint64_t hash64(std::int8_t data, std::uint64_t seed) { + return hash64_internal<std::int8_t, 0xFFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint8_t data, std::uint64_t seed) { + return hash64_internal<std::uint8_t, 0xFFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int16_t data, std::uint64_t seed) { + return hash64_internal<std::int16_t, 0xFF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint16_t data, std::uint64_t seed) { + return hash64_internal<std::uint16_t, 0xFF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int32_t data, std::uint64_t seed) { + return hash64_internal<std::int32_t, 0xFF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint32_t data, std::uint64_t seed) { + return hash64_internal<std::uint32_t, 0xFF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::int64_t data, std::uint64_t seed) { + return hash64_internal<std::int64_t, 0xFF'FF'FF'FF'FF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint64_t data, std::uint64_t seed) { + return hash64_internal<std::uint64_t, 0xFF'FF'FF'FF'FF'FF'FF'FFULL>(data, seed); +} + +inline std::uint64_t hash64(std::uint8_t *data, size_t off, size_t len, std::uint64_t seed) { + return hash64_internal(data, off, len, seed); +} Review Comment: These functions take a mutable `std::uint8_t*` but do not modify the input buffer. Prefer `const std::uint8_t*` in declarations/definitions to avoid forcing callers to cast away const (and to better communicate intent). ########## modules/platforms/cpp/ignite/common/detail/string_extensions.h: ########## @@ -0,0 +1,31 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <big_decimal.h> +#include <uuid.h> +#include <ignite_date.h> + +namespace ignite::detail { + +std::string to_string(const big_decimal &bd) noexcept; + +std::string to_string(const uuid& uuid) noexcept; + +std::string to_string(const ignite_date& date) noexcept; Review Comment: This header uses `std::string` but does not include `<string>`, which can lead to compilation errors depending on include order. Add the missing standard header include. ########## modules/platforms/cpp/ignite/common/detail/string_extensions.cpp: ########## @@ -0,0 +1,49 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include "string_extensions.h" + +#include <sstream> + +namespace ignite::detail { + +std::string to_string(const big_decimal &bd) noexcept { + std::stringstream ss; + ss << bd; + + return ss.str(); +} + +std::string to_string(const uuid& uuid) noexcept { + std::stringstream ss; + + ss << uuid; + + return ss.str(); +} + +std::string to_string(const ignite_date &date) noexcept { + std::stringstream ss; + + ss << date.get_year() << "-" << std::setfill('0') << std::setw(2) + << std::to_string(date.get_month()) << std::setfill(' ') << std::setw(0) + << "-" << std::setfill('0') << std::setw(2) + << std::to_string(date.get_day_of_month()); + + return ss.str(); +} Review Comment: This implementation uses `std::setfill` / `std::setw` but does not include `<iomanip>`, which can fail to compile. Add the missing include. ########## modules/platforms/cpp/ignite/common/detail/hash_calculator.h: ########## @@ -0,0 +1,82 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once +#include "big_decimal.h" +#include "ignite_date.h" +#include "ignite_date_time.h" +#include "ignite_timestamp.h" +#include "primitive.h" +#include "uuid.h" + +#include <cstdint> + +namespace ignite::detail { + +class hash_calculator { +public: + std::int32_t hash(const primitive& val) { + auto type = val.get_type(); + + switch (type) { + case ignite_type::BOOLEAN: + return hash(val.get<bool>()); + case ignite_type::NIL: + return hash(static_cast<std::int8_t>(0)); + case ignite_type::INT8: + return hash(val.get<std::int8_t>()); + case ignite_type::INT16: + return hash(val.get<std::int16_t>()); + case ignite_type::INT32: + return hash(val.get<std::int32_t>()); + case ignite_type::INT64: + return hash(val.get<std::int64_t>()); + case ignite_type::FLOAT: + return hash(val.get<float>()); + case ignite_type::DOUBLE: + return hash(val.get<double>()); + case ignite_type::DECIMAL: + return hash(val.get<big_decimal>()); + case ignite_type::DATE: + return hash(val.get<ignite_date>()); + case ignite_type::TIME: + return hash(val.get<ignite_time>()); + case ignite_type::DATETIME: + return hash(val.get<ignite_date_time>()); + case ignite_type::TIMESTAMP: + return hash(val.get<ignite_timestamp>()); + case ignite_type::UUID: + return hash(val.get<uuid>()); + case ignite_type::STRING: + return hash(val.get<std::string>()); + case ignite_type::BYTE_ARRAY: + return hash(val.get<std::vector<std::byte>>()); + case ignite_type::PERIOD: + case ignite_type::DURATION: + case ignite_type::UNDEFINED: + default: + throw ignite_error(error::code::INTERNAL, ""); Review Comment: The thrown `ignite_error` uses an empty message string, which makes diagnosing unsupported types difficult. Provide a concise message (e.g., mentioning `ignite_type` value and that hashing is unsupported for that type). ########## modules/platforms/cpp/ignite/common/primitive_test.cpp: ########## @@ -64,3 +66,11 @@ TEST(primitive, null_value_by_nullopt) { EXPECT_EQ(val.get_type(), ignite_type::NIL); EXPECT_TRUE(val.is_null()); } + +TEST(primitive, foo) { Review Comment: The test name `foo` is not descriptive. Rename it to reflect the behavior under test (e.g., hashing of NIL/primitive values). ```suggestion TEST(primitive, hash_nil_primitive_values) { ``` ########## modules/platforms/cpp/ignite/common/detail/string_extensions.cpp: ########## @@ -0,0 +1,49 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include "string_extensions.h" + +#include <sstream> + +namespace ignite::detail { + +std::string to_string(const big_decimal &bd) noexcept { + std::stringstream ss; + ss << bd; + + return ss.str(); +} Review Comment: These `to_string(... ) noexcept` functions allocate/format via `std::stringstream` and can throw (e.g., `std::bad_alloc`). Marking them `noexcept` risks `std::terminate` on allocation failure. Consider removing `noexcept` (or switching to an implementation that truly cannot throw). ########## modules/platforms/cpp/ignite/common/detail/hash_utils_test.cpp: ########## @@ -0,0 +1,328 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include <ignite/common/detail/hash_utils.h> +#include <ignite/common/detail/string_extensions.h> + +#include <cmath> + +#include <gtest/gtest.h> + +using namespace ignite; +using namespace ignite::detail; + +class hash_utils_bool_test : public ::testing::Test {}; + +TEST_F(hash_utils_bool_test, true_value) { + ASSERT_EQ(-105209210, hash(true)); +} + + +TEST_F(hash_utils_bool_test, false_value) { + ASSERT_EQ(686815056, hash(false)); +} + +template<typename T> +struct test_case { + T arg; + std::int32_t expected; +}; + +template<typename T> +void PrintTo(const test_case<T>& tc, std::ostream* os) { + using std::to_string; + using ignite::detail::to_string; + + *os << "arg = " << to_string(tc.arg); Review Comment: `PrintTo` relies on `to_string(tc.arg)`. However, `string_extensions` only defines overloads for `big_decimal`, `uuid`, and `ignite_date`, while this test file also parameterizes cases for `ignite_time` (and potentially other types). This will fail to compile unless there is another `to_string` overload available. Either add `to_string` overloads for every type used in parameterized tests (e.g., `ignite_time`) or change `PrintTo` to use `operator<<` when available (and provide fallbacks for unsupported types). ```suggestion *os << "arg = " << tc.arg; ``` ########## modules/platforms/cpp/ignite/common/bignum_test.cpp: ########## @@ -1290,3 +1290,37 @@ TEST(bignum, TestDoubleCast) { CheckDoubleCast(-0.00000000000001); CheckDoubleCast(-0.000000000000001); } + + +struct big_decimal_rounding_tc { + big_decimal initial; + int16_t new_scale; + big_decimal expected; +}; + +class big_decimal_rounding_test : public ::testing::TestWithParam<big_decimal_rounding_tc> {}; + +TEST_P(big_decimal_rounding_test, TestScaleWithRoundHalfUp) { + auto [initial, new_scale, expected] = GetParam();; Review Comment: There is a stray extra semicolon (`;;`). Remove the duplicate to match the project's style and keep the test code clean. ```suggestion auto [initial, new_scale, expected] = GetParam(); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
