From 5f75bdcc37b1b805012282ab96b12da2bf851568 Mon Sep 17 00:00:00 2001 From: Lloyd-Pottiger <60744015+Lloyd-Pottiger@users.noreply.github.com> Date: Fri, 3 Nov 2023 10:32:09 +0800 Subject: [PATCH] minmax: fix copy index data in a wrong way (#8302) close pingcap/tiflash#8301 --- .../Storages/DeltaMerge/Index/MinMaxIndex.cpp | 48 ++++++++++++++----- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/dbms/src/Storages/DeltaMerge/Index/MinMaxIndex.cpp b/dbms/src/Storages/DeltaMerge/Index/MinMaxIndex.cpp index 8e1922e6a17..38a6f35eb0a 100644 --- a/dbms/src/Storages/DeltaMerge/Index/MinMaxIndex.cpp +++ b/dbms/src/Storages/DeltaMerge/Index/MinMaxIndex.cpp @@ -305,10 +305,13 @@ RSResults MinMaxIndex::checkNullableEqual( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkEqual(value, type, min, max); } return results; @@ -404,10 +407,13 @@ RSResults MinMaxIndex::checkNullableIn( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkIn(values, type, min, max); } return results; @@ -495,10 +501,13 @@ RSResults MinMaxIndex::checkEqual(size_t start_pack, size_t pack_count, const Fi size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkEqual(value, type, min, max); } return results; @@ -588,10 +597,13 @@ RSResults MinMaxIndex::checkIn( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkIn(values, type, min, max); } return results; @@ -682,10 +694,13 @@ RSResults MinMaxIndex::checkNullableGreater( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkGreater(value, type, min, max); } return results; @@ -778,10 +793,13 @@ RSResults MinMaxIndex::checkGreater( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String - auto min = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` + auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; - auto max = String(chars[prev_offset], offsets[pos] - prev_offset - 1); + auto max = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); results[i - start_pack] = RoughCheck::checkGreater(value, type, min, max); } return results; @@ -873,6 +891,9 @@ RSResults MinMaxIndex::checkNullableGreaterEqual( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1]; @@ -970,6 +991,9 @@ RSResults MinMaxIndex::checkGreaterEqual( size_t pos = i * 2; size_t prev_offset = pos == 0 ? 0 : offsets[pos - 1]; // todo use StringRef instead of String + // When using String, we should use reinterpret_cast(&chars[prev_offset]) instead of chars[prev_offset] + // so that it will call constructor `constexpr basic_string( const CharT* s, const Allocator& alloc = Allocator())` + // rather than `constexpr basic_string( size_type count, CharT ch, const Allocator& alloc = Allocator() );` auto min = String(reinterpret_cast(&chars[prev_offset]), offsets[pos] - prev_offset - 1); pos = i * 2 + 1; prev_offset = offsets[pos - 1];