diff --git a/bindings/builtin/reindexer_cgo.h b/bindings/builtin/reindexer_cgo.h index a94b0be42..bdca6187c 100644 --- a/bindings/builtin/reindexer_cgo.h +++ b/bindings/builtin/reindexer_cgo.h @@ -4,8 +4,8 @@ extern "C" { #endif -void reindexer_enable_go_logger(); -void reindexer_disable_go_logger(); +void reindexer_enable_go_logger(void); +void reindexer_disable_go_logger(void); #ifdef __cplusplus } diff --git a/bindings/builtin/symbolizer.go b/bindings/builtin/symbolizer.go index 1b9a0f4be..7b80a84b2 100644 --- a/bindings/builtin/symbolizer.go +++ b/bindings/builtin/symbolizer.go @@ -2,7 +2,7 @@ package builtin // extern void cgoTraceback(void*); // extern void cgoSymbolizer(void*); -// extern void cgoSignalsInit(); +// extern void cgoSignalsInit(void); import "C" import ( "os" diff --git a/bindings/consts.go b/bindings/consts.go index 5a959d560..534e59cad 100644 --- a/bindings/consts.go +++ b/bindings/consts.go @@ -2,7 +2,7 @@ package bindings const CInt32Max = int(^uint32(0) >> 1) -const ReindexerVersion = "v4.17.1" +const ReindexerVersion = "v4.17.2" // public go consts from type_consts.h and reindexer_ctypes.h const ( diff --git a/changelog.md b/changelog.md index 2c483d7d4..9b507e5a7 100644 --- a/changelog.md +++ b/changelog.md @@ -1,3 +1,20 @@ +# Version 4.17.2 *beta* (19.09.2024) +## Core +- [fix] Fixed data race in cached comparators (`joins cache` may cause incorrect comparators deletion) +- [fix] Changed `select fields filters` behavior: incorrect fields in the filter will be ignored (instead of full filter reset) + +## Go connector +- [fix] Unexported fields and fields, marked with `"json":-`, will not create indexes anymore (including nested ones). Check updated example [here](readme.md#nested-structs) +- [fix] Unexported fields, makred with `joined` now produce explicit error (previously such fields silently did not work) + +## Deploy +- [fea] Added `RedOS 8` prebuilt packages + +## Face +- [fea] Changed column tooltips on `Statistics->Queries` page +- [fea] Grouped slow log settings on `Database config` page +- [fix] Fixed the `JOIN` links in `Explain result` table + # Version 4.17.1 *beta* (05.09.2024) ## Core - [fix] Fixed pagination for `MERGE` fulltext queries. Previously those queries could return duplicates on different pages due to missing ordering guarantees diff --git a/cpp_src/CMakeLists.txt b/cpp_src/CMakeLists.txt index 0377cae6d..1e111cebf 100644 --- a/cpp_src/CMakeLists.txt +++ b/cpp_src/CMakeLists.txt @@ -45,7 +45,7 @@ else() option(LINK_RESOURCES "Link web resources as binary data" ON) endif() -set (REINDEXER_VERSION_DEFAULT "4.17.1") +set (REINDEXER_VERSION_DEFAULT "4.17.2") if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "RelWithDebInfo") diff --git a/cpp_src/cluster/config.cc b/cpp_src/cluster/config.cc index 2354a8002..6ce5ebd6b 100644 --- a/cpp_src/cluster/config.cc +++ b/cpp_src/cluster/config.cc @@ -28,7 +28,8 @@ static void ValidateDSN(std::string_view dsn) { Error NodeData::FromJSON(span json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "NodeData: %s", ex.what()); } catch (const Error& err) { @@ -63,7 +64,8 @@ void NodeData::GetJSON(WrSerializer& ser) const { Error RaftInfo::FromJSON(span json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "RaftInfo: %s", ex.what()); } catch (const Error& err) { @@ -293,7 +295,8 @@ Error AsyncReplConfigData::FromYAML(const std::string& yaml) { Error AsyncReplConfigData::FromJSON(std::string_view json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const Error& err) { return err; } catch (const gason::Exception& ex) { @@ -906,7 +909,8 @@ Error ShardingConfig::FromYAML(const std::string& yaml) { Error ShardingConfig::FromJSON(span json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "ShardingConfig: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/cluster/stats/replicationstats.cc b/cpp_src/cluster/stats/replicationstats.cc index c0a291a28..68ce66270 100644 --- a/cpp_src/cluster/stats/replicationstats.cc +++ b/cpp_src/cluster/stats/replicationstats.cc @@ -137,7 +137,8 @@ void NodeStats::GetJSON(JsonBuilder& builder) const { Error ReplicationStats::FromJSON(span json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "RaftInfo: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/cmake/modules/RxPrepareInstallFiles.cmake b/cpp_src/cmake/modules/RxPrepareInstallFiles.cmake index a9365af19..df12f1c91 100644 --- a/cpp_src/cmake/modules/RxPrepareInstallFiles.cmake +++ b/cpp_src/cmake/modules/RxPrepareInstallFiles.cmake @@ -14,6 +14,13 @@ endif() function(generate_libs_list INPUT_LIBS OUTPUT_LIST_NAME) set (flibs ${${OUTPUT_LIST_NAME}}) foreach(lib ${INPUT_LIBS}) + get_filename_component(lib_dir ${lib} DIRECTORY) + if (NOT ${lib_dir} STREQUAL "") + set(lib_dir " -L${lib_dir}") + if (NOT (${lib_dir} IN_LIST flibs)) + list(APPEND flibs ${lib_dir}) + endif() + endif() if (${lib} MATCHES "jemalloc" OR ${lib} MATCHES "tcmalloc") elseif(${lib} STREQUAL "-pthread") list(APPEND flibs " -lpthread") diff --git a/cpp_src/cmd/reindexer_tool/commandsprocessor.cc b/cpp_src/cmd/reindexer_tool/commandsprocessor.cc index 078452693..5f6004c2c 100644 --- a/cpp_src/cmd/reindexer_tool/commandsprocessor.cc +++ b/cpp_src/cmd/reindexer_tool/commandsprocessor.cc @@ -30,7 +30,7 @@ Error CommandsProcessor::process(const std::string& command) { template template void CommandsProcessor::setCompletionCallback(T& rx, void (T::*set_completion_callback)(const new_v_callback_t&)) { - (rx.*set_completion_callback)([this](const std::string& input, int) -> replxx::Replxx::completions_t { + (rx.*set_completion_callback)([this](const std::string& input, int) { std::vector completions; const auto err = executor_.GetSuggestions(input, completions); replxx::Replxx::completions_t result; @@ -47,13 +47,16 @@ template template void CommandsProcessor::setCompletionCallback(T& rx, void (T::*set_completion_callback)(const old_v_callback_t&, void*)) { (rx.*set_completion_callback)( - [this](const std::string& input, int, void*) -> replxx::Replxx::completions_t { + [this](const std::string& input, int, void*) { std::vector completions; const auto err = executor_.GetSuggestions(input, completions); - if (!err.ok()) { - return {}; + replxx::Replxx::completions_t result; + if (err.ok()) { + for (const std::string& suggestion : completions) { + result.emplace_back(suggestion); + } } - return completions; + return result; }, nullptr); } diff --git a/cpp_src/core/cbinding/reindexer_c.h b/cpp_src/core/cbinding/reindexer_c.h index 56b8e003d..6823c886c 100644 --- a/cpp_src/core/cbinding/reindexer_c.h +++ b/cpp_src/core/cbinding/reindexer_c.h @@ -8,7 +8,7 @@ extern "C" { #include "core/type_consts.h" #include "reindexer_ctypes.h" -uintptr_t init_reindexer(); +uintptr_t init_reindexer(void); uintptr_t init_reindexer_with_config(reindexer_config config); void destroy_reindexer(uintptr_t rx); @@ -70,9 +70,9 @@ reindexer_error reindexer_erase_events(uintptr_t rx, uint32_t events_count); reindexer_error reindexer_cancel_context(reindexer_ctx_info ctx_info, ctx_cancel_type how); void reindexer_enable_logger(void (*logWriter)(int level, char* msg)); -void reindexer_disable_logger(); +void reindexer_disable_logger(void); -void reindexer_init_locale(); +void reindexer_init_locale(void); #ifdef __cplusplus } diff --git a/cpp_src/core/dbconfig.cc b/cpp_src/core/dbconfig.cc index 36d7f8854..7c08f6204 100644 --- a/cpp_src/core/dbconfig.cc +++ b/cpp_src/core/dbconfig.cc @@ -297,7 +297,8 @@ Error ReplicationConfigData::FromYAML(const std::string& yaml) { Error ReplicationConfigData::FromJSON(std::string_view json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const Error& err) { return err; } catch (const gason::Exception& ex) { diff --git a/cpp_src/core/indexdef.cc b/cpp_src/core/indexdef.cc index be03453d6..3033add3b 100644 --- a/cpp_src/core/indexdef.cc +++ b/cpp_src/core/indexdef.cc @@ -155,7 +155,8 @@ bool isStore(IndexType type) noexcept { Error IndexDef::FromJSON(span json) { try { - IndexDef::FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + IndexDef::FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "IndexDef: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/core/namespace/namespacestat.cc b/cpp_src/core/namespace/namespacestat.cc index dcd8de8e3..6f1581508 100644 --- a/cpp_src/core/namespace/namespacestat.cc +++ b/cpp_src/core/namespace/namespacestat.cc @@ -274,7 +274,8 @@ void ClusterizationStatus::GetJSON(JsonBuilder& builder) const { Error ClusterizationStatus::FromJSON(span json) { try { - FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "ClusterizationStatus: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/core/namespace/snapshot/snapshotrecord.cc b/cpp_src/core/namespace/snapshot/snapshotrecord.cc index a0969cb25..95ee80b9b 100644 --- a/cpp_src/core/namespace/snapshot/snapshotrecord.cc +++ b/cpp_src/core/namespace/snapshot/snapshotrecord.cc @@ -52,7 +52,8 @@ Error SnapshotOpts::FromJSON(const gason::JsonNode& root) { Error SnapshotOpts::FromJSON(span json) { try { - return FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + return FromJSON(parser.Parse(json)); } catch (const Error& err) { return err; } catch (const gason::Exception& ex) { diff --git a/cpp_src/core/namespacedef.cc b/cpp_src/core/namespacedef.cc index c43161175..f97393f39 100644 --- a/cpp_src/core/namespacedef.cc +++ b/cpp_src/core/namespacedef.cc @@ -10,7 +10,8 @@ using namespace std::string_view_literals; Error NamespaceDef::FromJSON(span json) { try { - FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "NamespaceDef: %s", ex.what()); } catch (const Error& err) { @@ -57,7 +58,8 @@ bool EnumNamespacesOpts::MatchFilter(std::string_view nsName, const std::shared_ Error NsReplicationOpts::FromJSON(span json) { try { - FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "NamespaceDef: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/core/nsselecter/comparator/comparator_indexed.cc b/cpp_src/core/nsselecter/comparator/comparator_indexed.cc index 513c81ac8..3adebc35c 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_indexed.cc +++ b/cpp_src/core/nsselecter/comparator/comparator_indexed.cc @@ -85,26 +85,31 @@ typename reindexer::comparators::ValuesHolder::Type } } -template -void initComparator(const reindexer::VariantArray& from, typename reindexer::comparators::ValuesHolder::Type& to) { - if constexpr (Cond == CondSet) { - for (const reindexer::Variant& v : from) { - to.insert(reindexer::comparators::GetValue(v)); - } - } else if constexpr (Cond == CondAllSet) { - int i = 0; - for (const reindexer::Variant& v : from) { - to.values_.emplace(reindexer::comparators::GetValue(v), i); - ++i; - } +template +void initComparatorSet(const reindexer::VariantArray& from, reindexer::comparators::DataHolder& to, SetT&& set) { + for (const reindexer::Variant& v : from) { + set.insert(reindexer::comparators::GetValue(v)); + } + using SetWrpType = typename reindexer::comparators::DataHolder::SetWrpType; + to.setPtr_ = reindexer::make_intrusive(std::forward(set)); +} + +template +void initComparatorAllSet(const reindexer::VariantArray& from, reindexer::comparators::DataHolder& to, SetT&& set) { + int i = 0; + for (const reindexer::Variant& v : from) { + set.values_.emplace(reindexer::comparators::GetValue(v), i); + ++i; } + using AllSetType = typename reindexer::comparators::DataHolder::AllSetType; + to.allSetPtr_ = std::make_unique(std::forward(set)); } template void initComparator(CondType cond, const reindexer::VariantArray& from, reindexer::comparators::DataHolder& to) { using namespace reindexer::comparators; - using SetWrpType = typename DataHolder::SetWrpType; - using AllSetWrpType = typename DataHolder::AllSetWrpType; + using SetType = typename DataHolder::SetType; + using AllSetType = typename DataHolder::AllSetType; switch (cond) { case CondEq: case CondLt: @@ -118,12 +123,10 @@ void initComparator(CondType cond, const reindexer::VariantArray& from, reindexe to.value2_ = GetValue(cond, from, 1); break; case CondSet: - to.setPtr_ = make_intrusive(); - initComparator(from, *to.setPtr_); + initComparatorSet(from, to, SetType{}); break; case CondAllSet: - to.allSetPtr_ = make_intrusive(); - initComparator(from, *to.allSetPtr_); + initComparatorAllSet(from, to, AllSetType{}); break; case CondAny: case CondEmpty: @@ -139,8 +142,6 @@ void initStringComparator(CondType cond, const reindexer::VariantArray& from, re using namespace reindexer::comparators; using SetType = DataHolder::SetType; using AllSetType = DataHolder::AllSetType; - using SetWrpType = DataHolder::SetWrpType; - using AllSetWrpType = DataHolder::AllSetWrpType; switch (cond) { case CondEq: case CondLt: @@ -155,12 +156,10 @@ void initStringComparator(CondType cond, const reindexer::VariantArray& from, re to.value2_ = GetValue(cond, from, 1); break; case CondSet: - to.setPtr_ = make_intrusive(SetType{collate}); - initComparator(from, *to.setPtr_); + initComparatorSet(from, to, SetType{collate}); break; case CondAllSet: - to.allSetPtr_ = make_intrusive(AllSetType{collate, {}}); - initComparator(from, *to.allSetPtr_); + initComparatorAllSet(from, to, AllSetType{collate, {}}); break; case CondAny: case CondEmpty: @@ -499,13 +498,12 @@ ComparatorIndexedComposite::ComparatorIndexedComposite(const VariantArray& value value2_ = GetValue(cond, values, 1); break; case CondSet: - setPtr_ = make_intrusive(SetType{values.size(), reindexer::hash_composite_ref{payloadType, fields}, - reindexer::equal_composite_ref{payloadType, fields}}); - initComparator(values, *setPtr_); + initComparatorSet(values, *this, + SetType{values.size(), reindexer::hash_composite_ref{payloadType, fields}, + reindexer::equal_composite_ref{payloadType, fields}}); break; case CondAllSet: - allSetPtr_ = make_intrusive(AllSetType{{reindexer::PayloadType{payloadType}, reindexer::FieldsSet{fields}}, {}}); - initComparator(values, *allSetPtr_); + initComparatorAllSet(values, *this, AllSetType{{reindexer::PayloadType{payloadType}, reindexer::FieldsSet{fields}}, {}}); break; case CondAny: case CondEmpty: diff --git a/cpp_src/core/nsselecter/comparator/comparator_indexed.h b/cpp_src/core/nsselecter/comparator/comparator_indexed.h index 254dbf02e..d50a39604 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_indexed.h +++ b/cpp_src/core/nsselecter/comparator/comparator_indexed.h @@ -98,17 +98,24 @@ template struct DataHolder { using SingleType = typename ValuesHolder::Type; using SetType = typename ValuesHolder::Type; - using SetWrpType = intrusive_rc_wrapper; + using SetWrpType = const intrusive_atomic_rc_wrapper; // must be const for safe intrusive copying using SetPtrType = intrusive_ptr; using AllSetType = typename ValuesHolder::Type; - using AllSetWrpType = intrusive_rc_wrapper; - using AllSetPtrType = intrusive_ptr; + using AllSetPtrType = std::unique_ptr; DataHolder() noexcept : cond_{CondEq} {} DataHolder(DataHolder&& other) noexcept = default; - DataHolder(const DataHolder& other) = default; + DataHolder(const DataHolder& o) + : cond_{o.cond_}, + value_{o.value_}, + value2_{o.value2_}, + setPtr_{o.setPtr_}, + allSetPtr_{o.allSetPtr_ ? std::make_unique(*o.allSetPtr_) : nullptr} { + // allSetPtr's data are modified during comparison, so we have to make a real copy + } DataHolder& operator=(DataHolder&& other) noexcept = default; - DataHolder& operator=(const DataHolder& other) = default; + DataHolder& operator=(const DataHolder& o) = delete; + CondType cond_; SingleType value_{}; // Either single value or right range boundry SingleType value2_{}; // Left range boundry @@ -837,7 +844,7 @@ class ComparatorIndexedOffsetArrayString : private DataHolder { [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { if (cond_ == CondAllSet) { assertrx_dbg(this->allSetPtr_); - allSetPtr_->allSetValues_.clear(); + this->allSetPtr_->allSetValues_.clear(); } const PayloadFieldValue::Array& arr = *reinterpret_cast(item.Ptr() + offset_); const p_string* ptr = reinterpret_cast(item.Ptr() + arr.offset); @@ -921,7 +928,7 @@ class ComparatorIndexedOffsetArrayStringDistinct : private DataHolderallSetPtr_); - allSetPtr_->allSetValues_.clear(); + this->allSetPtr_->allSetValues_.clear(); } const PayloadFieldValue::Array& arr = *reinterpret_cast(item.Ptr() + offset_); const p_string* ptr = reinterpret_cast(item.Ptr() + arr.offset); @@ -1017,7 +1024,7 @@ class ComparatorIndexedJsonPathString : private DataHolder { [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { if (cond_ == CondAllSet) { assertrx_dbg(this->allSetPtr_); - allSetPtr_->allSetValues_.clear(); + this->allSetPtr_->allSetValues_.clear(); } buffer_.clear(); ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer_, KeyValueType::String{}); @@ -1105,7 +1112,7 @@ class ComparatorIndexedJsonPathStringDistinct : private DataHolder { [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { if (cond_ == CondAllSet) { assertrx_dbg(this->allSetPtr_); - allSetPtr_->allSetValues_.clear(); + this->allSetPtr_->allSetValues_.clear(); } buffer_.clear(); ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer_, KeyValueType::String{}); @@ -1242,7 +1249,7 @@ class ComparatorIndexedComposite : private DataHolder { private: // Using pointer for cheap copying and ExpressionTree size reduction - using FieldsSetWrp = intrusive_rc_wrapper; + using FieldsSetWrp = const intrusive_atomic_rc_wrapper; // must be const for safe intrusive copying const CollateOpts* collateOpts_; intrusive_ptr fields_; @@ -1305,13 +1312,13 @@ class ComparatorIndexedOffsetArrayDWithinDistinct { class ComparatorIndexedJsonPathDWithin { public: ComparatorIndexedJsonPathDWithin(const FieldsSet& fields, const PayloadType& payloadType, const VariantArray&); - [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { - buffer_.clear(); - ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer_, KeyValueType::Double{}); - if rx_unlikely (buffer_.size() != 2) { + [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) const { + VariantArray buffer; + ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer, KeyValueType::Double{}); + if rx_unlikely (buffer.size() != 2) { throw Error(errQueryExec, "DWithin with not point data"); } - return DWithin(Point{buffer_[0].As(), buffer_[1].As()}, point_, distance_); + return DWithin(Point{buffer[0].As(), buffer[1].As()}, point_, distance_); } [[nodiscard]] std::string ConditionStr() const; [[nodiscard]] bool IsDistinct() const noexcept { return false; } @@ -1321,7 +1328,6 @@ class ComparatorIndexedJsonPathDWithin { private: PayloadType payloadType_; TagsPath tagsPath_; - VariantArray buffer_; Point point_; double distance_; }; @@ -1329,24 +1335,24 @@ class ComparatorIndexedJsonPathDWithin { class ComparatorIndexedJsonPathDWithinDistinct { public: ComparatorIndexedJsonPathDWithinDistinct(const FieldsSet& fields, const PayloadType& payloadType, const VariantArray&); - [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { - buffer_.clear(); - ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer_, KeyValueType::Double{}); - if rx_unlikely (buffer_.size() != 2) { + [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) const { + VariantArray buffer; + ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer, KeyValueType::Double{}); + if rx_unlikely (buffer.size() != 2) { throw Error(errQueryExec, "DWithin with not point data"); } - const Point p{buffer_[0].As(), buffer_[1].As()}; + const Point p{buffer[0].As(), buffer[1].As()}; return DWithin(p, point_, distance_) && distinct_.Compare(p); } [[nodiscard]] std::string ConditionStr() const; [[nodiscard]] bool IsDistinct() const noexcept { return true; } void ExcludeDistinctValues(const PayloadValue& item, IdType /*rowId*/) { - buffer_.clear(); - ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer_, KeyValueType::Double{}); - if rx_unlikely (buffer_.size() != 2) { + VariantArray buffer; + ConstPayload(payloadType_, item).GetByJsonPath(tagsPath_, buffer, KeyValueType::Double{}); + if rx_unlikely (buffer.size() != 2) { return; } - distinct_.ExcludeValues(Point{buffer_[0].As(), buffer_[1].As()}); + distinct_.ExcludeValues(Point{buffer[0].As(), buffer[1].As()}); } void ClearDistinctValues() noexcept { distinct_.ClearValues(); } @@ -1354,7 +1360,6 @@ class ComparatorIndexedJsonPathDWithinDistinct { ComparatorIndexedDistinct> distinct_; PayloadType payloadType_; TagsPath tagsPath_; - VariantArray buffer_; Point point_; double distance_; }; @@ -1456,10 +1461,10 @@ class ComparatorIndexedOffsetArrayAnyDistinct { void ClearDistinctValues() noexcept { distinct_.ClearValues(); } private: - using ComparatoristincType = std::conditional_t, ComparatorIndexedDistinct>, - ComparatorIndexedDistinct>; + using ComparatorDistinctType = std::conditional_t, ComparatorIndexedDistinct>, + ComparatorIndexedDistinct>; - ComparatoristincType distinct_; + ComparatorDistinctType distinct_; size_t offset_; }; diff --git a/cpp_src/core/nsselecter/comparator/comparator_indexed_distinct.h b/cpp_src/core/nsselecter/comparator/comparator_indexed_distinct.h index 5a0fd6ae9..e43fa7c8b 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_indexed_distinct.h +++ b/cpp_src/core/nsselecter/comparator/comparator_indexed_distinct.h @@ -12,7 +12,12 @@ namespace reindexer { template > class ComparatorIndexedDistinct { public: - ComparatorIndexedDistinct() : values_{make_intrusive()} {} + ComparatorIndexedDistinct() : values_{std::make_unique()} {} + ComparatorIndexedDistinct(ComparatorIndexedDistinct&&) = default; + ComparatorIndexedDistinct& operator=(ComparatorIndexedDistinct&&) = default; + ComparatorIndexedDistinct(const ComparatorIndexedDistinct& o) + : values_{o.values_ ? std::make_unique(*o.values_) : std::make_unique()} {} + [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const T& v) const noexcept { assertrx_dbg(values_); return values_->find(v) == values_->cend(); @@ -27,15 +32,19 @@ class ComparatorIndexedDistinct { } private: - using SetWrpType = intrusive_rc_wrapper; - using SetPtrType = intrusive_ptr; + using SetPtrType = std::unique_ptr; SetPtrType values_; }; class ComparatorIndexedDistinctString { public: - ComparatorIndexedDistinctString(const CollateOpts& collate) : values_{make_intrusive(collate)} {} + ComparatorIndexedDistinctString(const CollateOpts& collate) : values_{std::make_unique(collate)}, collateOpts_{&collate} {} + ComparatorIndexedDistinctString(ComparatorIndexedDistinctString&&) = default; + ComparatorIndexedDistinctString& operator=(ComparatorIndexedDistinctString&&) = default; + ComparatorIndexedDistinctString(const ComparatorIndexedDistinctString& o) + : values_{o.values_ ? std::make_unique(*o.values_) : std::make_unique(*o.collateOpts_)} {} + [[nodiscard]] RX_ALWAYS_INLINE bool Compare(std::string_view str) const noexcept { assertrx_dbg(values_); return values_->find(str) == values_->cend(); @@ -86,10 +95,10 @@ class ComparatorIndexedDistinctString { }; using SetType = string_view_set; - using SetWrpType = intrusive_rc_wrapper; - using SetPtrType = intrusive_ptr; + using SetPtrType = std::unique_ptr; SetPtrType values_; + const CollateOpts* collateOpts_; }; } // namespace reindexer diff --git a/cpp_src/core/nsselecter/comparator/comparator_not_indexed.h b/cpp_src/core/nsselecter/comparator/comparator_not_indexed.h index 097dff18f..e74ee8843 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_not_indexed.h +++ b/cpp_src/core/nsselecter/comparator/comparator_not_indexed.h @@ -18,7 +18,7 @@ template class ComparatorNotIndexedImplBase { protected: ComparatorNotIndexedImplBase(const VariantArray&); - ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = delete; + ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = default; ComparatorNotIndexedImplBase& operator=(const ComparatorNotIndexedImplBase&) = delete; ComparatorNotIndexedImplBase(ComparatorNotIndexedImplBase&&) = default; ComparatorNotIndexedImplBase& operator=(ComparatorNotIndexedImplBase&&) = default; @@ -48,7 +48,7 @@ template <> class ComparatorNotIndexedImplBase { protected: ComparatorNotIndexedImplBase(const VariantArray&); - ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = delete; + ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = default; ComparatorNotIndexedImplBase& operator=(const ComparatorNotIndexedImplBase&) = delete; ComparatorNotIndexedImplBase(ComparatorNotIndexedImplBase&&) = default; ComparatorNotIndexedImplBase& operator=(ComparatorNotIndexedImplBase&&) = default; @@ -71,7 +71,7 @@ class ComparatorNotIndexedImplBase { values_.insert(v); } } - ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = delete; + ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = default; ComparatorNotIndexedImplBase& operator=(const ComparatorNotIndexedImplBase&) = delete; ComparatorNotIndexedImplBase(ComparatorNotIndexedImplBase&&) = default; ComparatorNotIndexedImplBase& operator=(ComparatorNotIndexedImplBase&&) = default; @@ -89,7 +89,7 @@ template <> class ComparatorNotIndexedImplBase { protected: ComparatorNotIndexedImplBase(const VariantArray&); - ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = delete; + ComparatorNotIndexedImplBase(const ComparatorNotIndexedImplBase&) = default; ComparatorNotIndexedImplBase& operator=(const ComparatorNotIndexedImplBase&) = delete; ComparatorNotIndexedImplBase(ComparatorNotIndexedImplBase&&) = default; ComparatorNotIndexedImplBase& operator=(ComparatorNotIndexedImplBase&&) = default; @@ -117,7 +117,7 @@ class ComparatorNotIndexedImpl : private ComparatorNotIndexedImplBa public: ComparatorNotIndexedImpl(const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath) : Base{values}, payloadType_{payloadType}, fieldPath_{fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -149,7 +149,7 @@ class ComparatorNotIndexedImpl : private ComparatorNotIndexedImplBas public: ComparatorNotIndexedImpl(const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath) : Base{values}, distinct_{}, payloadType_{payloadType}, fieldPath_{fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -185,7 +185,7 @@ class ComparatorNotIndexedImpl { public: ComparatorNotIndexedImpl(const PayloadType& payloadType, const TagsPath& fieldPath) : payloadType_{payloadType}, fieldPath_{fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -215,7 +215,7 @@ class ComparatorNotIndexedImpl { public: ComparatorNotIndexedImpl(const PayloadType& payloadType, const TagsPath& fieldPath) : payloadType_{payloadType}, fieldPath_{fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -251,7 +251,7 @@ class ComparatorNotIndexedImpl { public: ComparatorNotIndexedImpl(const PayloadType& payloadType, const TagsPath& fieldPath) : payloadType_{payloadType}, fieldPath_{fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -282,7 +282,7 @@ class ComparatorNotIndexedImpl : private ComparatorNotIndexedIm public: ComparatorNotIndexedImpl(const PayloadType& payloadType, const TagsPath& fieldPath) : Base{payloadType, fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -298,18 +298,19 @@ template <> class ComparatorNotIndexedImpl { public: ComparatorNotIndexedImpl(const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath); - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; [[nodiscard]] std::string ConditionStr() const; [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { - ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer_, KeyValueType::Undefined{}); - if (buffer_.size() < 2 || !buffer_[0].Type().IsNumeric() || !buffer_[1].Type().IsNumeric()) { + VariantArray buffer; + ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer, KeyValueType::Undefined{}); + if (buffer.size() < 2 || !buffer[0].Type().IsNumeric() || !buffer[1].Type().IsNumeric()) { return false; } - return DWithin(Point{buffer_[0].As(), buffer_[1].As()}, point_, distance_); + return DWithin(Point{buffer[0].As(), buffer[1].As()}, point_, distance_); } [[nodiscard]] bool IsDistinct() const noexcept { return false; } void ClearDistinctValues() const noexcept {} @@ -318,7 +319,6 @@ class ComparatorNotIndexedImpl { protected: PayloadType payloadType_; TagsPath fieldPath_; - VariantArray buffer_; Point point_; double distance_; }; @@ -330,27 +330,29 @@ class ComparatorNotIndexedImpl : private ComparatorNotIndexed public: ComparatorNotIndexedImpl(const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath) : Base{values, payloadType, fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; [[nodiscard]] RX_ALWAYS_INLINE bool Compare(const PayloadValue& item, IdType /*rowId*/) { - ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer_, KeyValueType::Undefined{}); - if (buffer_.size() != 2 || !buffer_[0].Type().Is() || !buffer_[0].Type().Is()) { + VariantArray buffer; + ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer, KeyValueType::Undefined{}); + if (buffer.size() != 2 || !buffer[0].Type().Is() || !buffer[0].Type().Is()) { return false; } - const Point p{buffer_[0].As(), buffer_[1].As()}; + const Point p{buffer[0].As(), buffer[1].As()}; return DWithin(p, point_, distance_) && distinct_.Compare(Variant{p}); } [[nodiscard]] bool IsDistinct() const noexcept { return true; } void ClearDistinctValues() noexcept { distinct_.ClearValues(); } void ExcludeDistinctValues(const PayloadValue& item, IdType /*rowId*/) { - ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer_, KeyValueType::Undefined{}); - if (buffer_.size() != 2 || !buffer_[0].Type().Is() || !buffer_[0].Type().Is()) { + VariantArray buffer; + ConstPayload{payloadType_, item}.GetByJsonPath(fieldPath_, buffer, KeyValueType::Undefined{}); + if (buffer.size() != 2 || !buffer[0].Type().Is() || !buffer[0].Type().Is()) { return; } - const Point p{buffer_[0].As(), buffer_[1].As()}; + const Point p{buffer[0].As(), buffer[1].As()}; distinct_.ExcludeValues(Variant{p}); } using Base::ConditionStr; @@ -370,7 +372,7 @@ class ComparatorNotIndexedImpl { ++i; } } - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -411,7 +413,7 @@ class ComparatorNotIndexedImpl : private ComparatorNotIndexedI public: ComparatorNotIndexedImpl(const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath) : Base{values, payloadType, fieldPath} {} - ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = delete; + ComparatorNotIndexedImpl(const ComparatorNotIndexedImpl&) = default; ComparatorNotIndexedImpl& operator=(const ComparatorNotIndexedImpl&) = delete; ComparatorNotIndexedImpl(ComparatorNotIndexedImpl&&) = default; ComparatorNotIndexedImpl& operator=(ComparatorNotIndexedImpl&&) = default; @@ -454,16 +456,24 @@ using ComparatorNotIndexedVariant = std::variant< ComparatorNotIndexedImpl, ComparatorNotIndexedImpl, ComparatorNotIndexedImpl, ComparatorNotIndexedImpl, ComparatorNotIndexedImpl, ComparatorNotIndexedImpl>; -using ComparatorNotIndexedVariantWrp = intrusive_rc_wrapper; - } // namespace comparators class ComparatorNotIndexed { public: ComparatorNotIndexed(std::string_view fieldName, CondType cond, const VariantArray& values, const PayloadType& payloadType, const TagsPath& fieldPath, bool distinct) - : impl_{make_intrusive(createImpl(cond, values, payloadType, fieldPath, distinct))}, + : impl_{std::make_unique(createImpl(cond, values, payloadType, fieldPath, distinct))}, fieldName_{fieldName} {} + ComparatorNotIndexed(ComparatorNotIndexed&&) = default; + ComparatorNotIndexed& operator=(ComparatorNotIndexed&&) = default; + ComparatorNotIndexed(const ComparatorNotIndexed& o) + : impl_{o.impl_ ? std::make_unique(*o.impl_) : nullptr}, + matchedCount_{o.matchedCount_}, + isNotOperation_{o.isNotOperation_}, + fieldName_{o.fieldName_} { + assertrx_throw(o.impl_); + } + [[nodiscard]] const std::string& Name() const& noexcept { return fieldName_; } auto Name() const&& = delete; [[nodiscard]] std::string ConditionStr() const; @@ -676,7 +686,7 @@ class ComparatorNotIndexed { using ImplVariantType = comparators::ComparatorNotIndexedVariant; static ImplVariantType createImpl(CondType, const VariantArray& values, const PayloadType&, const TagsPath&, bool distinct); // Using pointer to reduce ExpressionTree Node size - intrusive_ptr impl_; + std::unique_ptr impl_; int matchedCount_{0}; bool isNotOperation_{false}; std::string fieldName_; diff --git a/cpp_src/core/nsselecter/comparator/comparator_not_indexed_distinct.h b/cpp_src/core/nsselecter/comparator/comparator_not_indexed_distinct.h index fa00b3095..f7e9fc5ab 100644 --- a/cpp_src/core/nsselecter/comparator/comparator_not_indexed_distinct.h +++ b/cpp_src/core/nsselecter/comparator/comparator_not_indexed_distinct.h @@ -8,7 +8,7 @@ namespace reindexer { class ComparatorNotIndexedDistinct { public: ComparatorNotIndexedDistinct() = default; - ComparatorNotIndexedDistinct(const ComparatorNotIndexedDistinct&) = delete; + ComparatorNotIndexedDistinct(const ComparatorNotIndexedDistinct&) = default; ComparatorNotIndexedDistinct& operator=(const ComparatorNotIndexedDistinct&) = delete; ComparatorNotIndexedDistinct(ComparatorNotIndexedDistinct&&) = default; ComparatorNotIndexedDistinct& operator=(ComparatorNotIndexedDistinct&&) = default; diff --git a/cpp_src/core/nsselecter/comparator/equalposition_comparator.cc b/cpp_src/core/nsselecter/comparator/equalposition_comparator.cc index 4cd3e775e..2594a4131 100644 --- a/cpp_src/core/nsselecter/comparator/equalposition_comparator.cc +++ b/cpp_src/core/nsselecter/comparator/equalposition_comparator.cc @@ -6,19 +6,18 @@ namespace reindexer { -void EqualPositionComparatorImpl::BindField(const std::string& name, int field, const VariantArray& values, CondType cond, - const CollateOpts& collate) { +void EqualPositionComparator::BindField(const std::string& name, int field, const VariantArray& values, CondType cond, + const CollateOpts& collate) { bindField(name, field, values, cond, collate); } -void EqualPositionComparatorImpl::BindField(const std::string& name, const FieldsPath& fieldPath, const VariantArray& values, - CondType cond) { +void EqualPositionComparator::BindField(const std::string& name, const FieldsPath& fieldPath, const VariantArray& values, CondType cond) { bindField(name, fieldPath, values, cond, CollateOpts{}); } template -void EqualPositionComparatorImpl::bindField(const std::string& name, F field, const VariantArray& values, CondType cond, - const CollateOpts& collate) { +void EqualPositionComparator::bindField(const std::string& name, F field, const VariantArray& values, CondType cond, + const CollateOpts& collate) { fields_.push_back(field); Context& ctx = ctx_.emplace_back(collate); @@ -34,7 +33,7 @@ void EqualPositionComparatorImpl::bindField(const std::string& name, F field, co name_ += ' ' + name; } -bool EqualPositionComparatorImpl::Compare(const PayloadValue& pv, IdType /*rowId*/) { +bool EqualPositionComparator::Compare(const PayloadValue& pv, IdType /*rowId*/) { ConstPayload pl(payloadType_, pv); size_t len = INT_MAX; @@ -66,14 +65,13 @@ bool EqualPositionComparatorImpl::Compare(const PayloadValue& pv, IdType /*rowId } if (cmpRes) { - ++matchedCount_; return true; } } return false; } -bool EqualPositionComparatorImpl::compareField(size_t field, const Variant& v) { +bool EqualPositionComparator::compareField(size_t field, const Variant& v) { return v.Type().EvaluateOneOf( [&](KeyValueType::Bool) { return ctx_[field].cmpBool.Compare(ctx_[field].cond, static_cast(v)); }, [&](KeyValueType::Int) { return ctx_[field].cmpInt.Compare(ctx_[field].cond, static_cast(v)); }, diff --git a/cpp_src/core/nsselecter/comparator/equalposition_comparator.h b/cpp_src/core/nsselecter/comparator/equalposition_comparator.h index f5942efcc..c06fd3744 100644 --- a/cpp_src/core/nsselecter/comparator/equalposition_comparator.h +++ b/cpp_src/core/nsselecter/comparator/equalposition_comparator.h @@ -7,13 +7,13 @@ namespace reindexer { -class EqualPositionComparatorImpl : public intrusive_rc_base { +class EqualPositionComparator { public: - EqualPositionComparatorImpl(const PayloadType& payloadType) : payloadType_{payloadType}, name_{"EqualPositions"} {} - EqualPositionComparatorImpl(const EqualPositionComparatorImpl&) = delete; - EqualPositionComparatorImpl(EqualPositionComparatorImpl&&) = delete; - EqualPositionComparatorImpl& operator=(const EqualPositionComparatorImpl&) = delete; - EqualPositionComparatorImpl& operator=(EqualPositionComparatorImpl&&) = delete; + EqualPositionComparator(const PayloadType& payloadType) : payloadType_{payloadType}, name_{"EqualPositions"} {} + EqualPositionComparator(const EqualPositionComparator&) = default; + EqualPositionComparator(EqualPositionComparator&&) = default; + EqualPositionComparator& operator=(const EqualPositionComparator&) = delete; + EqualPositionComparator& operator=(EqualPositionComparator&&) = default; void BindField(const std::string& name, int field, const VariantArray&, CondType, const CollateOpts&); void BindField(const std::string& name, const FieldsPath&, const VariantArray&, CondType); @@ -57,30 +57,4 @@ class EqualPositionComparatorImpl : public intrusive_rc_base { int matchedCount_{0}; }; -class EqualPositionComparator { -public: - EqualPositionComparator(const PayloadType& payloadType) : impl_{make_intrusive(payloadType)} {} - - void BindField(const std::string& name, int field, const VariantArray& values, CondType cond, const CollateOpts& opts) { - return impl_->BindField(name, field, values, cond, opts); - } - void BindField(const std::string& name, const FieldsPath& fields, const VariantArray& values, CondType cond) { - return impl_->BindField(name, fields, values, cond); - } - bool Compare(const PayloadValue& pv, IdType id) { return impl_->Compare(pv, id); } - bool IsBinded() noexcept { return impl_->IsBinded(); } - [[nodiscard]] int GetMatchedCount() const noexcept { return impl_->GetMatchedCount(); } - [[nodiscard]] int FieldsCount() const noexcept { return impl_->FieldsCount(); } - [[nodiscard]] const std::string& Name() const& noexcept { return impl_->Name(); } - [[nodiscard]] const std::string& Dump() const& noexcept { return impl_->Name(); } - [[nodiscard]] double Cost(int expectedIterations) const noexcept { return impl_->Cost(expectedIterations); } - - auto Name() const&& = delete; - auto Dump() const&& = delete; - -private: - // Using pointer to reduce ExpressionTree Node size - intrusive_ptr impl_; -}; - } // namespace reindexer diff --git a/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h b/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h index 0eb935e2f..c3bed1e8a 100644 --- a/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h +++ b/cpp_src/core/nsselecter/comparator/equalposition_comparator_impl.h @@ -1,6 +1,5 @@ #pragma once -#include #include "core/index/string_map.h" #include "core/keyvalue/geometry.h" #include "core/keyvalue/p_string.h" @@ -17,19 +16,9 @@ class EqualPositionComparatorTypeImpl { using AllSetValuesSet = fast_hash_set; public: - EqualPositionComparatorTypeImpl() noexcept = default; - EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl(EqualPositionComparatorTypeImpl&&) = default; - EqualPositionComparatorTypeImpl& operator=(EqualPositionComparatorTypeImpl&&) = default; - void SetValues(CondType cond, const VariantArray& values) { - if (cond == CondSet) { - valuesS_ = std::make_unique(); - } else if (cond == CondAllSet) { - valuesS_ = std::make_unique(); - allSetValuesS_ = std::make_unique(); - } + assertrx_throw(valuesS_.empty()); + assertrx_throw(allSetValuesS_.empty()); for (Variant key : values) { key.Type().EvaluateOneOf([](OneOf) {}, @@ -62,14 +51,14 @@ class EqualPositionComparatorTypeImpl { assertrx_throw(values_.size() == 2); return lhs >= values_[0] && lhs <= values_[1]; case CondSet: - return valuesS_->find(lhs) != valuesS_->end(); + return valuesS_.find(lhs) != valuesS_.end(); case CondAllSet: { - const auto it = valuesS_->find(lhs); - if (it == valuesS_->end()) { + const auto it = valuesS_.find(lhs); + if (it == valuesS_.end()) { return false; } - allSetValuesS_->insert(&*it); - return allSetValuesS_->size() == valuesS_->size(); + allSetValuesS_.insert(&*it); + return allSetValuesS_.size() == valuesS_.size(); } case CondAny: return true; @@ -82,14 +71,11 @@ class EqualPositionComparatorTypeImpl { std::abort(); } - void ClearAllSetValues() { - assertrx_throw(allSetValuesS_); - allSetValuesS_->clear(); - } + void ClearAllSetValues() { allSetValuesS_.clear(); } h_vector values_; - std::unique_ptr valuesS_; - std::unique_ptr allSetValuesS_; + ValuesSet valuesS_; + AllSetValuesSet allSetValuesS_; private: KeyValueType type() { @@ -110,7 +96,7 @@ class EqualPositionComparatorTypeImpl { void addValue(CondType cond, T value) { if (cond == CondSet || cond == CondAllSet) { - valuesS_->emplace(value); + valuesS_.emplace(value); } else { values_.emplace_back(value); } @@ -123,19 +109,9 @@ class EqualPositionComparatorTypeImpl { using AllSetValuesSet = fast_hash_set; public: - EqualPositionComparatorTypeImpl() noexcept = default; - EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl(EqualPositionComparatorTypeImpl&&) = default; - EqualPositionComparatorTypeImpl& operator=(EqualPositionComparatorTypeImpl&&) = default; - void SetValues(CondType cond, const VariantArray& values) { - if (cond == CondSet) { - valuesS_ = std::make_unique(); - } else if (cond == CondAllSet) { - valuesS_ = std::make_unique(); - allSetValuesS_ = std::make_unique(); - } + assertrx_throw(valuesS_.empty()); + assertrx_throw(allSetValuesS_.empty()); for (const Variant& key : values) { key.Type().EvaluateOneOf( @@ -172,14 +148,14 @@ class EqualPositionComparatorTypeImpl { assertrx_throw(values_.size() >= 2); return lhs >= values_[0] && lhs <= values_[1]; case CondSet: - return valuesS_->find(lhs) != valuesS_->end(); + return valuesS_.find(lhs) != valuesS_.end(); case CondAllSet: { - const auto it = valuesS_->find(lhs); - if (it == valuesS_->end()) { + const auto it = valuesS_.find(lhs); + if (it == valuesS_.end()) { return false; } - allSetValuesS_->insert(&*it); - return allSetValuesS_->size() == valuesS_->size(); + allSetValuesS_.insert(&*it); + return allSetValuesS_.size() == valuesS_.size(); } case CondAny: return true; @@ -191,19 +167,16 @@ class EqualPositionComparatorTypeImpl { } std::abort(); } - void ClearAllSetValues() { - assertrx_throw(allSetValuesS_); - allSetValuesS_->clear(); - } + void ClearAllSetValues() { allSetValuesS_.clear(); } h_vector values_; - std::unique_ptr valuesS_; - std::unique_ptr allSetValuesS_; + ValuesSet valuesS_; + AllSetValuesSet allSetValuesS_; private: void addValue(CondType cond, Uuid value) { if (cond == CondSet || cond == CondAllSet) { - valuesS_->emplace(value); + valuesS_.emplace(value); } else { values_.emplace_back(value); } @@ -213,19 +186,15 @@ class EqualPositionComparatorTypeImpl { template <> class EqualPositionComparatorTypeImpl { public: - EqualPositionComparatorTypeImpl(const CollateOpts& collate) : collate_{collate} {} - EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = delete; + EqualPositionComparatorTypeImpl(const CollateOpts& collate) : valuesS_(collate), collate_{collate} {} + EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = default; + EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = default; EqualPositionComparatorTypeImpl(EqualPositionComparatorTypeImpl&&) = default; EqualPositionComparatorTypeImpl& operator=(EqualPositionComparatorTypeImpl&&) = default; void SetValues(CondType cond, const VariantArray& values) { - if (cond == CondSet) { - valuesS_ = std::make_unique(collate_); - } else if (cond == CondAllSet) { - valuesS_ = std::make_unique(collate_); - allSetValuesS_ = std::make_unique>(); - } + assertrx_throw(valuesS_.empty()); + assertrx_throw(allSetValuesS_.empty()); for (Variant key : values) { key.convert(KeyValueType::String{}); @@ -250,17 +219,14 @@ class EqualPositionComparatorTypeImpl { return (collateCompare(std::string_view(lhs), rhs, collate_) & ComparationResult::Ge) && (collateCompare(std::string_view(lhs), std::string_view(*values_[1]), collate_) & ComparationResult::Le); case CondSet: - assertrx_dbg(valuesS_); - return valuesS_->find(std::string_view(lhs)) != valuesS_->end(); + return valuesS_.find(std::string_view(lhs)) != valuesS_.end(); case CondAllSet: { - assertrx_dbg(valuesS_); - assertrx_dbg(allSetValuesS_); - auto it = valuesS_->find(lhs); - if (it == valuesS_->end()) { + auto it = valuesS_.find(lhs); + if (it == valuesS_.end()) { return false; } - allSetValuesS_->insert(&*it); - return allSetValuesS_->size() == valuesS_->size(); + allSetValuesS_.insert(&*it); + return allSetValuesS_.size() == valuesS_.size(); } case CondAny: return true; @@ -274,10 +240,7 @@ class EqualPositionComparatorTypeImpl { } std::abort(); } - void ClearAllSetValues() { - assertrx_dbg(allSetValuesS_); - allSetValuesS_->clear(); - } + void ClearAllSetValues() { allSetValuesS_.clear(); } h_vector values_; std::string_view cachedValueSV_; @@ -290,13 +253,13 @@ class EqualPositionComparatorTypeImpl { less_key_string(opts)) {} }; - std::unique_ptr valuesS_; - std::unique_ptr> allSetValuesS_; + key_string_set valuesS_; + fast_hash_set allSetValuesS_; private: void addValue(CondType cond, const key_string& value) { if (cond == CondSet || cond == CondAllSet) { - valuesS_->emplace(value); + valuesS_.emplace(value); } else { values_.emplace_back(value); if (values_.size() == 1) { @@ -311,21 +274,11 @@ template <> class EqualPositionComparatorTypeImpl { public: EqualPositionComparatorTypeImpl() = delete; - EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl(EqualPositionComparatorTypeImpl&&) = default; - EqualPositionComparatorTypeImpl& operator=(EqualPositionComparatorTypeImpl&&) = default; }; template <> class EqualPositionComparatorTypeImpl { public: - EqualPositionComparatorTypeImpl() noexcept = default; - EqualPositionComparatorTypeImpl(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl& operator=(const EqualPositionComparatorTypeImpl&) = delete; - EqualPositionComparatorTypeImpl(EqualPositionComparatorTypeImpl&&) = default; - EqualPositionComparatorTypeImpl& operator=(EqualPositionComparatorTypeImpl&&) = default; - void SetValues(const VariantArray& values) { if (values.size() != 2) { throw Error(errQueryExec, "CondDWithin expects two arguments"); diff --git a/cpp_src/core/nsselecter/comparator/fieldscomparator.cc b/cpp_src/core/nsselecter/comparator/fieldscomparator.cc index 54edeba82..8f257c4b0 100644 --- a/cpp_src/core/nsselecter/comparator/fieldscomparator.cc +++ b/cpp_src/core/nsselecter/comparator/fieldscomparator.cc @@ -68,7 +68,7 @@ class ArrayAdapter { namespace reindexer { -FieldsComparatorImpl::FieldsComparatorImpl(std::string_view lField, CondType cond, std::string_view rField, PayloadType plType) +FieldsComparator::FieldsComparator(std::string_view lField, CondType cond, std::string_view rField, PayloadType plType) : condition_{cond}, payloadType_{std::move(plType)} { switch (condition_) { case CondEq: @@ -86,14 +86,17 @@ FieldsComparatorImpl::FieldsComparatorImpl(std::string_view lField, CondType con case CondDWithin: throw Error{errQueryExec, "Condition %s is not supported for two field comparing", CondTypeToStr(condition_)}; } + ctx_.resize(1); std::stringstream nameStream; nameStream << lField << ' ' << condition_ << ' ' << rField; name_ = nameStream.str(); } template -bool FieldsComparatorImpl::compare(const LArr& lhs, const RArr& rhs) { +bool FieldsComparator::compare(const LArr& lhs, const RArr& rhs) const { static constexpr bool needCompareTypes{std::is_same_v || std::is_same_v}; + const static CollateOpts kDefaultCollateOpts; + const CollateOpts& collateOpts = collateOpts_ ? *collateOpts_ : kDefaultCollateOpts; switch (condition_) { case CondRange: if (rhs.size() < 2 || rhs[0].Type().template Is() || rhs[1].Type().template Is()) { @@ -105,8 +108,8 @@ bool FieldsComparatorImpl::compare(const LArr& lhs, const RArr& rhs) { continue; } } - if ((v.RelaxCompare(rhs[0], collateOpts_) & ComparationResult::Ge) && - (v.RelaxCompare(rhs[1], collateOpts_) & ComparationResult::Le)) { + if ((v.RelaxCompare(rhs[0], collateOpts) & ComparationResult::Ge) && + (v.RelaxCompare(rhs[1], collateOpts) & ComparationResult::Le)) { return true; } } @@ -138,7 +141,7 @@ bool FieldsComparatorImpl::compare(const LArr& lhs, const RArr& rhs) { continue; } } - if (lv.RelaxCompare(rv, collateOpts_) == ComparationResult::Eq) { + if (lv.RelaxCompare(rv, collateOpts) == ComparationResult::Eq) { found = true; break; } @@ -171,7 +174,7 @@ bool FieldsComparatorImpl::compare(const LArr& lhs, const RArr& rhs) { continue; } } - const auto compRes = lv.RelaxCompare(rv, collateOpts_); + const auto compRes = lv.RelaxCompare(rv, collateOpts); switch (condition_) { case CondEq: case CondSet: @@ -214,7 +217,7 @@ bool FieldsComparatorImpl::compare(const LArr& lhs, const RArr& rhs) { } } -bool FieldsComparatorImpl::compare(const PayloadValue& item, const Context& ctx) { +bool FieldsComparator::compare(const PayloadValue& item, const Context& ctx) const { bool result; if (ctx.lCtx_.fields_.getTagsPathsLength() > 0) { VariantArray lhs; @@ -256,13 +259,10 @@ bool FieldsComparatorImpl::compare(const PayloadValue& item, const Context& ctx) result = compare(ArrayAdapter(item.Ptr() + ctx.lCtx_.offset_, 1, ctx.lCtx_.sizeof_, ctx.lCtx_.type_), ArrayAdapter(item.Ptr() + ctx.rCtx_.offset_, 1, ctx.rCtx_.sizeof_, ctx.rCtx_.type_)); } - if (result) { - ++matchedCount_; - } return result; } -void FieldsComparatorImpl::validateTypes(KeyValueType lType, KeyValueType rType) const { +void FieldsComparator::validateTypes(KeyValueType lType, KeyValueType rType) const { if (lType.IsSame(rType) || lType.Is() || rType.Is()) { return; } diff --git a/cpp_src/core/nsselecter/comparator/fieldscomparator.h b/cpp_src/core/nsselecter/comparator/fieldscomparator.h index d785b48c9..43456120e 100644 --- a/cpp_src/core/nsselecter/comparator/fieldscomparator.h +++ b/cpp_src/core/nsselecter/comparator/fieldscomparator.h @@ -10,24 +10,27 @@ namespace reindexer { -class FieldsComparatorImpl : public intrusive_rc_base { +class FieldsComparator { public: - FieldsComparatorImpl(std::string_view lField, CondType cond, std::string_view rField, PayloadType plType); - FieldsComparatorImpl(const FieldsComparatorImpl&) = delete; - FieldsComparatorImpl(FieldsComparatorImpl&&) = delete; - FieldsComparatorImpl& operator=(const FieldsComparatorImpl&) = delete; - FieldsComparatorImpl& operator=(FieldsComparatorImpl&&) = delete; + FieldsComparator(std::string_view lField, CondType cond, std::string_view rField, PayloadType plType); + FieldsComparator(const FieldsComparator&) = default; + FieldsComparator(FieldsComparator&&) = default; + FieldsComparator& operator=(const FieldsComparator&) = delete; + FieldsComparator& operator=(FieldsComparator&&) = default; bool Compare(const PayloadValue& item, IdType /*rowId*/) { - if (ctx_.size() > 1) { - for (const auto& c : ctx_) { - if (!compare(item, c)) { - return false; - } + if (ctx_.size() == 1) { + const bool res = compare(item, ctx_[0]); + matchedCount_ += int(res); + return res; + } + for (const auto& c : ctx_) { + if (!compare(item, c)) { + return false; } - return true; } - return compare(item, ctx_[0]); + ++matchedCount_; + return true; } double Cost(int expectedIterations) const noexcept { double cost = 1.0; @@ -48,16 +51,18 @@ class FieldsComparatorImpl : public intrusive_rc_base { const std::string& Name() const&& = delete; const std::string& Dump() const& noexcept { return Name(); } const std::string& Dump() const&& = delete; - int GetMatchedCount() const noexcept { return matchedCount_; } void SetLeftField(const FieldsSet& fields) { + assertrx_throw(!leftFieldSet_); setField(fields, ctx_[0].lCtx_); - leftFieldSet = true; + leftFieldSet_ = true; } void SetRightField(const FieldsSet& fields) { - assertrx_dbg(leftFieldSet); + assertrx_throw(leftFieldSet_); setField(fields, ctx_[0].rCtx_); } - void SetLeftField(const FieldsSet& fset, KeyValueType type, bool isArray) { + void SetLeftField(const FieldsSet& fset, KeyValueType type, bool isArray, const CollateOpts& cOpts) { + assertrx_throw(!leftFieldSet_); + collateOpts_ = &cOpts; if (type.Is()) { ctx_.clear(); ctx_.resize(fset.size()); @@ -65,10 +70,10 @@ class FieldsComparatorImpl : public intrusive_rc_base { } else { setField(ctx_[0].lCtx_, fset, type, isArray); } - leftFieldSet = true; + leftFieldSet_ = true; } void SetRightField(const FieldsSet& fset, KeyValueType type, bool isArray) { - assertrx_dbg(leftFieldSet); + assertrx_throw(leftFieldSet_); if ((ctx_.size() > 1) != type.Is()) { throw Error{errQueryExec, "A composite index cannot be compared with a non-composite one: %s", name_}; } @@ -82,7 +87,7 @@ class FieldsComparatorImpl : public intrusive_rc_base { setField(ctx_[0].rCtx_, fset, type, isArray); } } - void SetCollateOpts(const CollateOpts& cOpts) { collateOpts_ = cOpts; } + int GetMatchedCount() const noexcept { return matchedCount_; } private: struct FieldContext { @@ -133,8 +138,8 @@ class FieldsComparatorImpl : public intrusive_rc_base { } } template - bool compare(const LArr& lhs, const RArr& rhs); - bool compare(const PayloadValue& item, const Context&); + bool compare(const LArr& lhs, const RArr& rhs) const; + bool compare(const PayloadValue& item, const Context&) const; void validateTypes(KeyValueType lType, KeyValueType rType) const; inline static bool compareTypes(KeyValueType lType, KeyValueType rType) noexcept { if (lType.IsSame(rType)) { @@ -153,33 +158,11 @@ class FieldsComparatorImpl : public intrusive_rc_base { std::string name_; CondType condition_; + int matchedCount_{0}; PayloadType payloadType_; - CollateOpts collateOpts_; - h_vector ctx_{Context{}}; - int matchedCount_ = 0; - bool leftFieldSet = false; -}; - -class FieldsComparator { -public: - FieldsComparator(std::string_view lField, CondType cond, std::string_view rField, PayloadType plType) - : impl_{make_intrusive(lField, cond, rField, plType)} {} - bool Compare(const PayloadValue& item, IdType rowId) { return impl_->Compare(item, rowId); } - double Cost(int expectedIterations) const noexcept { return impl_->Cost(expectedIterations); } - const std::string& Name() const& noexcept { return impl_->Name(); } - const std::string& Name() const&& = delete; - const std::string& Dump() const& noexcept { return impl_->Dump(); } - const std::string& Dump() const&& = delete; - int GetMatchedCount() const noexcept { return impl_->GetMatchedCount(); } - void SetLeftField(const FieldsSet& fields) { return impl_->SetLeftField(fields); } - void SetRightField(const FieldsSet& fields) { return impl_->SetRightField(fields); } - void SetLeftField(const FieldsSet& fset, KeyValueType type, bool isArray) { return impl_->SetLeftField(fset, type, isArray); } - void SetRightField(const FieldsSet& fset, KeyValueType type, bool isArray) { return impl_->SetRightField(fset, type, isArray); } - void SetCollateOpts(const CollateOpts& cOpts) { impl_->SetCollateOpts(cOpts); } - -private: - // Using pointer to reduce ExpressionTree Node size - intrusive_ptr impl_; + const CollateOpts* collateOpts_{nullptr}; + std::vector ctx_; + bool leftFieldSet_{false}; }; } // namespace reindexer diff --git a/cpp_src/core/nsselecter/selectiteratorcontainer.cc b/cpp_src/core/nsselecter/selectiteratorcontainer.cc index d7ffa1d64..836af43e6 100644 --- a/cpp_src/core/nsselecter/selectiteratorcontainer.cc +++ b/cpp_src/core/nsselecter/selectiteratorcontainer.cc @@ -261,8 +261,7 @@ void SelectIteratorContainer::processField(FieldsComparator& fc, const QueryFiel if (field.IsFieldIndexed()) { auto& index = ns.indexes_[field.IndexNo()]; if constexpr (left) { - fc.SetCollateOpts(index->Opts().collateOpts_); - fc.SetLeftField(field.Fields(), field.FieldType(), index->Opts().IsArray()); + fc.SetLeftField(field.Fields(), field.FieldType(), index->Opts().IsArray(), index->Opts().collateOpts_); } else { fc.SetRightField(field.Fields(), field.FieldType(), index->Opts().IsArray()); } diff --git a/cpp_src/core/payload/fieldsset.cc b/cpp_src/core/payload/fieldsset.cc index 3a2dfeee7..19b34fd69 100644 --- a/cpp_src/core/payload/fieldsset.cc +++ b/cpp_src/core/payload/fieldsset.cc @@ -9,7 +9,9 @@ namespace reindexer { FieldsSet::FieldsSet(const TagsMatcher& tagsMatcher, const h_vector& fields) : mask_(0) { for (const std::string& str : fields) { - tagsPaths_.emplace_back(tagsMatcher.path2tag(str)); + if (auto tagsPath = tagsMatcher.path2tag(str); !tagsPath.empty()) { + tagsPaths_.emplace_back(std::move(tagsPath)); + } } } diff --git a/cpp_src/core/query/query.h b/cpp_src/core/query/query.h index 63fbb9cb3..50bbce6b3 100644 --- a/cpp_src/core/query/query.h +++ b/cpp_src/core/query/query.h @@ -673,6 +673,9 @@ class Query { } /// Sets list of columns in this namespace to be finally selected. + /// The columns should be specified in the same case as the jsonpaths corresponding to them. + /// Non-existent fields and fields in the wrong case are ignored. + /// If there are no fields in this list that meet these conditions, then the filter works as "*". /// @param l - list of columns to be selected. template >* = nullptr> Query& Select(std::initializer_list l) & { @@ -692,7 +695,7 @@ class Query { selectFilter_.insert(selectFilter_.begin(), l.begin(), l.end()); selectFilter_.erase( std::remove_if(selectFilter_.begin(), selectFilter_.end(), [](const auto& s) { return s == "*"sv || s.empty(); }), - selectFilter_.end()); + selectFilter_.end()); return *this; } template diff --git a/cpp_src/estl/intrusive_ptr.h b/cpp_src/estl/intrusive_ptr.h index ea19a4a46..b45474137 100644 --- a/cpp_src/estl/intrusive_ptr.h +++ b/cpp_src/estl/intrusive_ptr.h @@ -164,21 +164,21 @@ template class intrusive_atomic_rc_wrapper; template -inline void intrusive_ptr_add_ref(intrusive_atomic_rc_wrapper* x) noexcept { +inline void intrusive_ptr_add_ref(const intrusive_atomic_rc_wrapper* x) noexcept { if (x) { x->refcount.fetch_add(1, std::memory_order_relaxed); } } template -inline void intrusive_ptr_release(intrusive_atomic_rc_wrapper* x) noexcept { +inline void intrusive_ptr_release(const intrusive_atomic_rc_wrapper* x) noexcept { if (x && x->refcount.fetch_sub(1, std::memory_order_acq_rel) == 1) { delete x; } } template -inline bool intrusive_ptr_is_unique(intrusive_atomic_rc_wrapper* x) noexcept { +inline bool intrusive_ptr_is_unique(const intrusive_atomic_rc_wrapper* x) noexcept { // std::memory_order_acquire - is essential for COW constructions based on intrusive_ptr return !x || (x->refcount.load(std::memory_order_acquire) == 1); } @@ -190,12 +190,12 @@ class intrusive_atomic_rc_wrapper : public T { intrusive_atomic_rc_wrapper(Args&&... args) : T(std::forward(args)...) {} intrusive_atomic_rc_wrapper& operator=(const intrusive_atomic_rc_wrapper&) = delete; -protected: - std::atomic refcount{0}; +private: + mutable std::atomic refcount{0}; - friend void intrusive_ptr_add_ref<>(intrusive_atomic_rc_wrapper* x) noexcept; - friend void intrusive_ptr_release<>(intrusive_atomic_rc_wrapper* x) noexcept; - friend bool intrusive_ptr_is_unique<>(intrusive_atomic_rc_wrapper* x) noexcept; + friend void intrusive_ptr_add_ref<>(const intrusive_atomic_rc_wrapper* x) noexcept; + friend void intrusive_ptr_release<>(const intrusive_atomic_rc_wrapper* x) noexcept; + friend bool intrusive_ptr_is_unique<>(const intrusive_atomic_rc_wrapper* x) noexcept; }; template @@ -227,7 +227,7 @@ class intrusive_rc_wrapper : public T { intrusive_rc_wrapper(Args&&... args) : T(std::forward(args)...) {} intrusive_rc_wrapper& operator=(const intrusive_rc_wrapper&) = delete; -protected: +private: int refcount{0}; friend void intrusive_ptr_add_ref<>(intrusive_rc_wrapper* x) noexcept; @@ -240,27 +240,27 @@ class intrusive_atomic_rc_base { intrusive_atomic_rc_base& operator=(const intrusive_atomic_rc_base&) = delete; virtual ~intrusive_atomic_rc_base() = default; -protected: - std::atomic refcount{0}; +private: + mutable std::atomic refcount{0}; - friend void intrusive_ptr_add_ref(intrusive_atomic_rc_base* x) noexcept; - friend void intrusive_ptr_release(intrusive_atomic_rc_base* x) noexcept; - friend bool intrusive_ptr_is_unique(intrusive_atomic_rc_base* x) noexcept; + friend void intrusive_ptr_add_ref(const intrusive_atomic_rc_base* x) noexcept; + friend void intrusive_ptr_release(const intrusive_atomic_rc_base* x) noexcept; + friend bool intrusive_ptr_is_unique(const intrusive_atomic_rc_base* x) noexcept; }; -inline void intrusive_ptr_add_ref(intrusive_atomic_rc_base* x) noexcept { +inline void intrusive_ptr_add_ref(const intrusive_atomic_rc_base* x) noexcept { if (x) { x->refcount.fetch_add(1, std::memory_order_relaxed); } } -inline void intrusive_ptr_release(intrusive_atomic_rc_base* x) noexcept { +inline void intrusive_ptr_release(const intrusive_atomic_rc_base* x) noexcept { if (x && x->refcount.fetch_sub(1, std::memory_order_acq_rel) == 1) { delete x; } } -inline bool intrusive_ptr_is_unique(intrusive_atomic_rc_base* x) noexcept { +inline bool intrusive_ptr_is_unique(const intrusive_atomic_rc_base* x) noexcept { // std::memory_order_acquire - is essential for COW constructions based on intrusive_ptr return !x || (x->refcount.load(std::memory_order_acquire) == 1); } @@ -270,7 +270,7 @@ class intrusive_rc_base { intrusive_rc_base& operator=(const intrusive_rc_base&) = delete; virtual ~intrusive_rc_base() = default; -protected: +private: int refcount{0}; friend void intrusive_ptr_add_ref(intrusive_rc_base* x) noexcept; diff --git a/cpp_src/events/listener.cc b/cpp_src/events/listener.cc index 6dabf1110..50ac564d2 100644 --- a/cpp_src/events/listener.cc +++ b/cpp_src/events/listener.cc @@ -7,8 +7,11 @@ constexpr double kUpdatesHandlingPeriod = 0.05; EventsListener::EventsListener(const std::string& dbName, size_t maxUpdatesQueueSize) : updatesQueue_(maxUpdatesQueueSize), dbName_(dbName) { - using VecT = std::vector; - updatesQueue_.Init(std::optional().emplace(), nullptr); // TODO: Add some logger #1724 + { + using VecT = std::vector; + VecT container; + updatesQueue_.Init(std::move(container), nullptr); // TODO: Add some logger #1724 + } terminateAsync_.set(loop_); terminateAsync_.set([](net::ev::async& a) noexcept { a.loop.break_loop(); }); terminateAsync_.start(); diff --git a/cpp_src/events/observer.cc b/cpp_src/events/observer.cc index e3df35387..cae139d9e 100644 --- a/cpp_src/events/observer.cc +++ b/cpp_src/events/observer.cc @@ -69,7 +69,8 @@ bool UpdatesFilters::Check(std::string_view ns) const noexcept { Error UpdatesFilters::FromJSON(span json) { try { - FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "UpdatesFilter: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/events/subscriber_config.cc b/cpp_src/events/subscriber_config.cc index 8c271b26e..ac2220d6b 100644 --- a/cpp_src/events/subscriber_config.cc +++ b/cpp_src/events/subscriber_config.cc @@ -8,7 +8,8 @@ using namespace std::string_view_literals; Error EventSubscriberConfig::FromJSON(span json) noexcept { try { - FromJSON(gason::JsonParser().Parse(json)); + gason::JsonParser parser; + FromJSON(parser.Parse(json)); } catch (const gason::Exception& ex) { return Error(errParseJson, "UpdatesFilter: %s", ex.what()); } catch (const Error& err) { diff --git a/cpp_src/gtests/tests/fixtures/item_move_semantics_api.h b/cpp_src/gtests/tests/fixtures/item_move_semantics_api.h index 09cd0f0fe..c8f241f3d 100644 --- a/cpp_src/gtests/tests/fixtures/item_move_semantics_api.h +++ b/cpp_src/gtests/tests/fixtures/item_move_semantics_api.h @@ -49,7 +49,8 @@ class ItemMoveSemanticsApi : public ReindexerApi { auto&& item = pair.second; Error err = rt.reindexer->Upsert(default_namespace, item); ASSERT_TRUE(err.ok()) << err.what(); - ASSERT_NO_THROW(gason::JsonParser().Parse(item.GetJSON())); + gason::JsonParser parser; + ASSERT_NO_THROW(parser.Parse(item.GetJSON())); } } diff --git a/cpp_src/gtests/tests/fixtures/join_selects_api.h b/cpp_src/gtests/tests/fixtures/join_selects_api.h index 4097e3e38..870781838 100644 --- a/cpp_src/gtests/tests/fixtures/join_selects_api.h +++ b/cpp_src/gtests/tests/fixtures/join_selects_api.h @@ -212,7 +212,8 @@ class JoinSelectsApi : public ReindexerApi { if (!err.ok()) { break; } - gason::JsonParser().Parse(reindexer::giftStr(wrSer.Slice())); + gason::JsonParser parser; + parser.Parse(reindexer::giftStr(wrSer.Slice())); } } catch (const gason::Exception& ex) { return Error(errParseJson, "VerifyResJSON: %s", ex.what()); diff --git a/cpp_src/gtests/tests/fixtures/servercontrol.cc b/cpp_src/gtests/tests/fixtures/servercontrol.cc index 44553a3d2..2bc2fb195 100644 --- a/cpp_src/gtests/tests/fixtures/servercontrol.cc +++ b/cpp_src/gtests/tests/fixtures/servercontrol.cc @@ -366,7 +366,8 @@ void ServerControl::Interface::AddFollower(const std::string& dsn, std::optional err = qr.begin().GetJSON(ser, false); ASSERT_TRUE(err.ok()) << err.what(); curConf = cluster::AsyncReplConfigData(); - err = curConf.FromJSON(gason::JsonParser().Parse(ser.Slice())["async_replication"]); + gason::JsonParser parser; + err = curConf.FromJSON(parser.Parse(ser.Slice())["async_replication"]); ASSERT_TRUE(err.ok()) << err.what(); }; cluster::AsyncReplConfigData curConf; diff --git a/cpp_src/gtests/tests/unit/csv2jsonconverter.cc b/cpp_src/gtests/tests/unit/csv2jsonconverter.cc index ae407e276..03f34b327 100644 --- a/cpp_src/gtests/tests/unit/csv2jsonconverter.cc +++ b/cpp_src/gtests/tests/unit/csv2jsonconverter.cc @@ -70,7 +70,8 @@ std::string csv2json(std::string_view row, const std::vector& schem for (size_t i = 0; i < fields.size(); ++i) { if (!fields[i].empty()) { try { - gason::JsonParser().Parse(std::string_view{fields[i]}); + gason::JsonParser parser; + parser.Parse(std::string_view{fields[i]}); builder.Raw(schema[i], fields[i]); } catch (const gason::Exception&) { builder.Raw(schema[i], '"' + fields[i] + '"'); diff --git a/cpp_src/server/CMakeLists.txt b/cpp_src/server/CMakeLists.txt index e48132a5b..080e3380b 100644 --- a/cpp_src/server/CMakeLists.txt +++ b/cpp_src/server/CMakeLists.txt @@ -4,7 +4,7 @@ cmake_minimum_required(VERSION 3.10) project(reindexer_server_library) set (SWAGGER_VERSION "2.x") -set (GH_FACE_VERSION "4.17.1") +set (GH_FACE_VERSION "4.17.2") set (GH_FACE_TAG "v${GH_FACE_VERSION}") set (TARGET reindexer_server_library) set (SERVER_LIB_DIR ${PROJECT_BINARY_DIR} PARENT_SCOPE) diff --git a/cpp_src/server/cbinding/server_c.h b/cpp_src/server/cbinding/server_c.h index ef4959fcf..058262126 100644 --- a/cpp_src/server/cbinding/server_c.h +++ b/cpp_src/server/cbinding/server_c.h @@ -7,7 +7,7 @@ extern "C" { #include "core/cbinding/reindexer_ctypes.h" -uintptr_t init_reindexer_server(); +uintptr_t init_reindexer_server(void); void destroy_reindexer_server(uintptr_t psvc); reindexer_error start_reindexer_server(uintptr_t psvc, reindexer_string config); reindexer_error stop_reindexer_server(uintptr_t psvc); diff --git a/cpp_src/server/contrib/server.md b/cpp_src/server/contrib/server.md index 57bdc4424..0aab58f38 100644 --- a/cpp_src/server/contrib/server.md +++ b/cpp_src/server/contrib/server.md @@ -137,7 +137,7 @@ Reindexer is compact, fast and it does not have heavy dependencies. ### Version information -*Version* : 4.17.1 +*Version* : 4.17.2 ### License information diff --git a/cpp_src/server/contrib/server.yml b/cpp_src/server/contrib/server.yml index affaff3b4..1cb0fcc1d 100644 --- a/cpp_src/server/contrib/server.yml +++ b/cpp_src/server/contrib/server.yml @@ -4,7 +4,7 @@ info: **Reindexer** is an embeddable, in-memory, document-oriented database with a high-level Query builder interface. Reindexer's goal is to provide fast search with complex queries. Reindexer is compact, fast and it does not have heavy dependencies. - version: "4.17.1" + version: "4.17.2" title: "Reindexer REST API" license: name: "Apache 2.0" diff --git a/cpp_src/server/rpcserver.cc b/cpp_src/server/rpcserver.cc index bfee99573..5243a64a5 100644 --- a/cpp_src/server/rpcserver.cc +++ b/cpp_src/server/rpcserver.cc @@ -1028,7 +1028,7 @@ Error RPCServer::Select(cproto::Context& ctx, p_string queryBin, int flags, int } RPCQrWatcher::Ref qres; - RPCQrId id{-1, data->caps.HasResultsWithShardIDs() ? RPCQrWatcher::kUninitialized : RPCQrWatcher::kDisabled}; + RPCQrId id{-1, data->caps.HasQrIdleTimeouts() ? RPCQrWatcher::kUninitialized : RPCQrWatcher::kDisabled}; try { qres = createQueryResults(ctx, id, flags); } catch (Error& e) { @@ -1052,7 +1052,7 @@ Error RPCServer::Select(cproto::Context& ctx, p_string queryBin, int flags, int Error RPCServer::SelectSQL(cproto::Context& ctx, p_string querySql, int flags, int limit, p_string ptVersionsPck) { const auto data = getClientDataSafe(ctx); - RPCQrId id{-1, data->caps.HasResultsWithShardIDs() ? RPCQrWatcher::kUninitialized : RPCQrWatcher::kDisabled}; + RPCQrId id{-1, data->caps.HasQrIdleTimeouts() ? RPCQrWatcher::kUninitialized : RPCQrWatcher::kDisabled}; RPCQrWatcher::Ref qres; try { qres = createQueryResults(ctx, id, flags); diff --git a/cpp_src/tools/jsontools.cc b/cpp_src/tools/jsontools.cc index a2e2b528f..c34e5cbb5 100644 --- a/cpp_src/tools/jsontools.cc +++ b/cpp_src/tools/jsontools.cc @@ -88,7 +88,8 @@ void jsonValueToString(gason::JsonValue o, WrSerializer& ser, int shift, int ind } void prettyPrintJSON(span json, WrSerializer& ser, int shift) { - jsonValueToString(gason::JsonParser().Parse(json).value, ser, shift, 0); + gason::JsonParser parser; + jsonValueToString(parser.Parse(json).value, ser, shift, 0); } std::string stringifyJson(const gason::JsonNode& elem, bool escapeStrings) { diff --git a/cpp_src/vendor/gason/gason.cc b/cpp_src/vendor/gason/gason.cc index 5c4a4141e..9828bdd2b 100644 --- a/cpp_src/vendor/gason/gason.cc +++ b/cpp_src/vendor/gason/gason.cc @@ -407,7 +407,7 @@ const JsonNode& JsonNode::operator[](std::string_view key) const { // TODO: Remove NOLINT after pyreindexer update. Issue #1736 JsonNode JsonNode::EmptyNode() noexcept { return {{JsonTag(JSON_EMPTY)}, nullptr, {}}; } // NOLINT(*EnumCastOutOfRange) -JsonNode JsonParser::Parse(span str, size_t* length) { +JsonNode JsonParser::Parse(span str, size_t* length) & { largeStrings_->clear(); char* endp = nullptr; JsonNode val{{}, nullptr, {}}; @@ -423,7 +423,7 @@ JsonNode JsonParser::Parse(span str, size_t* length) { return val; } -JsonNode JsonParser::Parse(std::string_view str, size_t* length) { +JsonNode JsonParser::Parse(std::string_view str, size_t* length) & { tmp_.reserve(str.size()); tmp_.assign(str.begin(), str.end()); return Parse(span(&tmp_[0], tmp_.size()), length); diff --git a/cpp_src/vendor/gason/gason.h b/cpp_src/vendor/gason/gason.h index cf9b5edd5..b3c846e20 100644 --- a/cpp_src/vendor/gason/gason.h +++ b/cpp_src/vendor/gason/gason.h @@ -270,9 +270,9 @@ class JsonParser { public: JsonParser(LargeStringStorageT* strings = nullptr) : largeStrings_(strings ? strings : &internalLargeStrings_) {} // Inplace parse. Buffer pointed by str will be changed - JsonNode Parse(span str, size_t* length = nullptr); + JsonNode Parse(span str, size_t* length = nullptr) &; // Copy str. Buffer pointed by str will be copied - JsonNode Parse(std::string_view str, size_t* length = nullptr); + JsonNode Parse(std::string_view str, size_t* length = nullptr) &; private: JsonAllocator alloc_; diff --git a/go.mod b/go.mod index c36621b23..9e111b33c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/restream/reindexer/v4 -go 1.15 +go 1.17 require ( github.com/golang/snappy v0.0.4 @@ -13,3 +13,20 @@ require ( gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 ) + +require ( + github.com/beorn7/perks v1.0.1 // indirect + github.com/cespare/xxhash/v2 v2.1.2 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/go-logr/logr v1.2.3 // indirect + github.com/go-logr/stdr v1.2.2 // indirect + github.com/golang/protobuf v1.5.2 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/prometheus/client_model v0.2.0 // indirect + github.com/prometheus/common v0.32.1 // indirect + github.com/prometheus/procfs v0.7.3 // indirect + golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect + google.golang.org/protobuf v1.26.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/query.go b/query.go index 068ec27c3..815481f23 100644 --- a/query.go +++ b/query.go @@ -1188,6 +1188,9 @@ func (q *Query) On(index string, condition int, joinIndex string) *Query { } // Select add filter to fields of result's objects +// The `fields` should be specified in the same case as the jsonpaths corresponding to them. +// Non-existent `fields` and `fields` in the wrong case are ignored. +// If there are no `fields` in this list that meet these conditions, then the filter works as "*". func (q *Query) Select(fields ...string) *Query { for _, field := range fields { q.ser.PutVarCUInt(querySelectFilter).PutVString(field) diff --git a/readme.md b/readme.md index 45f8042ba..6ac4fbbb0 100644 --- a/readme.md +++ b/readme.md @@ -183,7 +183,7 @@ import ( ) -// Define struct with reindex tags +// Define struct with reindex tags. Fields must be exported - private fields can not be written into reindexer type Item struct { ID int64 `reindex:"id,,pk"` // 'id' is primary key Name string `reindex:"name"` // add index by 'name' field @@ -469,10 +469,12 @@ Fields with regular indexes are not nullable. Condition `is NULL` is supported o ### Nested Structs By default, Reindexer scans all nested structs and adds their fields to the namespace (as well as indexes specified). +During indexes scan private (unexported fields), fields tagged with `reindex:"-"` and fields tagged with `json:"-"` will be skipped. ```go type Actor struct { Name string `reindex:"actor_name"` + Age int `reindex:"age"` } type BaseItem struct { @@ -481,13 +483,16 @@ type BaseItem struct { } type ComplexItem struct { - BaseItem // Index fields of BaseItem will be added to reindex - Actor []Actor // Index fields of Actor will be added to reindex as arrays - Name string `reindex:"name"` // Hash-index for "name" - Year int `reindex:"year,tree"` // Tree-index for "year" - Value int `reindex:"value,-"` // Store(column)-index for "value" - Metainfo int `json:"-"` // Field "MetaInfo" will not be stored in reindexer - Parent *Item `reindex:"-"` // Index fields of parent will NOT be added to reindex + BaseItem // Index fields of BaseItem will be added to reindex + Actor []Actor // Index fields ("name" and "age") of Actor will be added to reindex as array-indexes + Name string `reindex:"name"` // Hash-index for "name" + Year int `reindex:"year,tree"` // Tree-index for "year" + Value int `reindex:"value,-"` // Store(column)-index for "value" + Metainfo int `json:"-"` // Field "MetaInfo" will not be stored in reindexer + Parent *Item `reindex:"-"` // Index fields of "Parent" will NOT be added to reindex and all of the "Parent" exported content will be stored as non-indexed data + ParentHidden *Item `json:"-"` // Indexes and fields of "ParentHidden" will NOT be added to reindexer + privateParent *Item // Indexes and fields of "ParentHidden" will NOT be added to reindexer (same as with `json:"-"`) + AnotherActor Actor `reindex:"actor"` // Index fields of "AnotherActor" will be added to reindexer with prefix "actor." (in this example two indexes will be created: "actor.actor_name" and "actor.age") } ``` @@ -943,6 +948,7 @@ type Actor struct { IsVisible bool `reindex:"is_visible"` } +// Fields, marked with 'joined' must also be exported - otherwise reindexer's binding will not be able to put data in those fields type ItemWithJoin struct { ID int `reindex:"id"` Name string `reindex:"name"` @@ -1382,7 +1388,7 @@ In case of requirement to serialize results of Query in JSON format, then it is ```go ... iterator := db.Query("items"). - Select ("id","name"). // Filter output JSON: Select only "id" and "name" fields of items, another fields will be omitted + Select ("id","name"). // Filter output JSON: Select only "id" and "name" fields of items, another fields will be omitted. This fields should be specified in the same case as the jsonpaths corresponding to them. Limit (1). ExecToJson ("root_object") // Name of root object of output JSON @@ -1457,7 +1463,7 @@ WARNING: when used `AllowUnsafe(true)` queries returns shared pointers to struct #### Limit size of object cache -By default, maximum size of object cache is 256000 items for each namespace. To change maximum size use `ObjCacheSize` method of `NameapaceOptions`, passed +By default, maximum size of object cache is 256000 items for each namespace. To change maximum size use `ObjCacheSize` method of `NamespaceOptions`, passed to OpenNamespace. e.g. ```go diff --git a/reflect.go b/reflect.go index a1d9bcefb..10471c80d 100644 --- a/reflect.go +++ b/reflect.go @@ -35,6 +35,8 @@ type indexOptions struct { isSparse bool rtreeType string isUuid bool + isJoined bool + isComposite bool } func parseRxTags(field reflect.StructField) (idxName string, idxType string, expireAfter string, idxSettings []string) { @@ -56,7 +58,7 @@ func parseRxTags(field reflect.StructField) (idxName string, idxType string, exp return } -func parseIndexes(namespace string, st reflect.Type, joined *map[string][]int) (indexDefs []bindings.IndexDef, err error) { +func parseIndexes(st reflect.Type, joined *map[string][]int) (indexDefs []bindings.IndexDef, err error) { if err = parseIndexesImpl(&indexDefs, st, false, "", "", joined, nil); err != nil { return nil, err } @@ -64,7 +66,7 @@ func parseIndexes(namespace string, st reflect.Type, joined *map[string][]int) ( return indexDefs, nil } -func parseSchema(namespace string, st reflect.Type) *bindings.SchemaDef { +func parseSchema(st reflect.Type) *bindings.SchemaDef { reflector := &jsonschema.Reflector{} reflector.FieldIsInScheme = func(f reflect.StructField) bool { _, _, _, idxSettings := parseRxTags(f) @@ -102,19 +104,20 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } for i := 0; i < st.NumField(); i++ { - t := st.Field(i).Type + field := st.Field(i) + t := field.Type if t.Kind() == reflect.Ptr { t = t.Elem() } // Get and parse tags - jsonPath := strings.Split(st.Field(i).Tag.Get("json"), ",")[0] + jsonTag := strings.Split(field.Tag.Get("json"), ",")[0] - if len(jsonPath) == 0 && !st.Field(i).Anonymous { - jsonPath = st.Field(i).Name + if len(jsonTag) == 0 && !field.Anonymous { + jsonTag = field.Name } - jsonPath = jsonBasePath + jsonPath + jsonPath := jsonBasePath + jsonTag - idxName, idxType, expireAfter, idxSettings := parseRxTags(st.Field(i)) + idxName, idxType, expireAfter, idxSettings := parseRxTags(field) if idxName == "-" { continue } @@ -126,17 +129,33 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } if opts.isPk && strings.TrimSpace(idxName) == "" { - return fmt.Errorf("No index name is specified for primary key in field %s", st.Field(i).Name) + return fmt.Errorf("no index name is specified for primary key in field '%s'; jsonpath: '%s'", field.Name, jsonPath) } if idxType == "rtree" { if t.Kind() != reflect.Array || t.Len() != 2 || t.Elem().Kind() != reflect.Float64 { - return fmt.Errorf("'rtree' index allowed only for [2]float64 or reindexer.Point field type") + return fmt.Errorf("'rtree' index allowed only for [2]float64 or reindexer.Point field type (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) } } - if parseByKeyWord(&idxSettings, "composite") { + + if jsonTag == "-" && !opts.isComposite && !opts.isJoined { + if reindexTag := field.Tag.Get("reindex"); reindexTag != "" { + return fmt.Errorf("non-composite/non-joined field ('%s'), marked with `json:-` can not have explicit reindex tags, but it does ('%s')", field.Name, reindexTag) + } + continue + } + if !opts.isComposite && !field.IsExported() { + if reindexTag := field.Tag.Get("reindex"); reindexTag != "" { + return fmt.Errorf("unexported non-composite field ('%s') can not have reindex tags, but it does ('%s')", field.Name, reindexTag) + } + continue + } + + if opts.isComposite { if t.Kind() != reflect.Struct || t.NumField() != 0 { - return fmt.Errorf("'composite' tag allowed only on empty on structs: Invalid tags %v on field %s", strings.SplitN(st.Field(i).Tag.Get("reindex"), ",", 3), st.Field(i).Name) + return fmt.Errorf("'composite' tag allowed only on empty on structs: Invalid tags '%v' on field '%s'", + strings.SplitN(field.Tag.Get("reindex"), ",", 3), field.Name) } indexDef := makeIndexDef(parseCompositeName(reindexPath), parseCompositeJsonPaths(reindexPath), idxType, "composite", opts, CollateNone, "", parseExpireAfter(expireAfter)) @@ -144,13 +163,17 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray return err } } else if t.Kind() == reflect.Struct { + if opts.isJoined { + return fmt.Errorf("joined index must be a slice of structs/pointers, but it is a single struct (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) + } if err := parseIndexesImpl(indexDefs, t, subArray, reindexPath, jsonPath, joined, parsed); err != nil { return err } } else if (t.Kind() == reflect.Slice || t.Kind() == reflect.Array) && (t.Elem().Kind() == reflect.Struct || (t.Elem().Kind() == reflect.Ptr && t.Elem().Elem().Kind() == reflect.Struct)) { // Check if field nested slice of struct - if parseByKeyWord(&idxSettings, "joined") && len(idxName) > 0 { + if opts.isJoined && len(idxName) > 0 { (*joined)[idxName] = st.Field(i).Index } else if err := parseIndexesImpl(indexDefs, t.Elem(), true, reindexPath, jsonPath, joined, parsed); err != nil { return err @@ -165,17 +188,22 @@ func parseIndexesImpl(indexDefs *[]bindings.IndexDef, st reflect.Type, subArray } if opts.isUuid { if fieldType != "string" { - return fmt.Errorf("UUID index is not applicable with '%v' field, only with 'string'", fieldType) + return fmt.Errorf("UUID index is not applicable with '%v' field, only with 'string' (index name: '%s', field name: '%s', jsonpath: '%s')", + fieldType, reindexPath, field.Name, jsonPath) } fieldType = "uuid" } + if opts.isJoined { + return fmt.Errorf("joined index must be a slice of objects/pointers, but it is a scalar value (index name: '%s', field name: '%s', jsonpath: '%s')", + reindexPath, field.Name, jsonPath) + } indexDef := makeIndexDef(reindexPath, []string{jsonPath}, idxType, fieldType, opts, collateMode, sortOrderLetters, parseExpireAfter(expireAfter)) if err := indexDefAppend(indexDefs, indexDef, opts.isAppenable); err != nil { return err } } if len(idxSettings) > 0 { - return fmt.Errorf("Unknown index settings are found: %v", idxSettings) + return fmt.Errorf("unknown index settings are found: '%v'", idxSettings) } } @@ -202,6 +230,10 @@ func parseOpts(idxSettingsBuf *[]string) indexOptions { opts.rtreeType = idxSetting case "uuid": opts.isUuid = true + case "joined": + opts.isJoined = true + case "composite": + opts.isComposite = true default: newIdxSettingsBuf = append(newIdxSettingsBuf, idxSetting) } @@ -238,7 +270,7 @@ func parseCollate(idxSettingsBuf *[]string) (int, string) { if newCollateMode, ok := collateModes[kvIdxSettings[0]]; ok { if collateMode != CollateNone { - panic(fmt.Errorf("Collate mode is already set to %d. Misunderstanding %s", collateMode, idxSetting)) + panic(fmt.Errorf("collate mode is already set to '%d'. Misunderstanding '%s'", collateMode, idxSetting)) } collateMode = newCollateMode @@ -324,14 +356,6 @@ func getJoinedField(val reflect.Value, joined map[string][]int, name string) (re return ret } -func makeFieldDef(JSONPath string, fieldType string, isArray bool) bindings.FieldDef { - return bindings.FieldDef{ - JSONPath: JSONPath, - Type: fieldType, - IsArray: isArray, - } -} - func makeIndexDef(index string, jsonPaths []string, indexType, fieldType string, opts indexOptions, collateMode int, sortOrder string, expireAfter int) bindings.IndexDef { cm := "" switch collateMode { @@ -373,19 +397,17 @@ func indexDefAppend(indexDefs *[]bindings.IndexDef, indexDef bindings.IndexDef, indexDefExists = true foundIndexDef = indexDef foundIndexPos = pos - break } } if !indexDefExists { *indexDefs = append(*indexDefs, indexDef) - return nil } if indexDef.IndexType != foundIndexDef.IndexType { - return fmt.Errorf("Index %s has another type: %+v", name, indexDef) + return fmt.Errorf("index '%s' has another type: found index def is '%+v' and new index def is '%+v'", name, foundIndexDef, indexDef) } if len(indexDef.JSONPaths) > 0 && indexDef.IndexType != "composite" { @@ -400,9 +422,8 @@ func indexDefAppend(indexDefs *[]bindings.IndexDef, indexDef bindings.IndexDef, if !isPresented { if !isAppendable { - return fmt.Errorf("Index %s is not appendable", name) + return fmt.Errorf("index '%s' is not appendable. Attempt to create array index with multiple JSON-paths: %v", name, indexDef.JSONPaths) } - foundIndexDef.JSONPaths = append(foundIndexDef.JSONPaths, indexDef.JSONPaths[0]) } diff --git a/reindexer_impl.go b/reindexer_impl.go index e41084abe..36615f53d 100644 --- a/reindexer_impl.go +++ b/reindexer_impl.go @@ -451,6 +451,7 @@ func (db *reindexerImpl) openNamespace(ctx context.Context, namespace string, op db.binding.DropNamespace(ctx, namespace) continue } + db.unregisterNamespaceImpl(namespace) db.binding.CloseNamespace(ctx, namespace) break } @@ -478,6 +479,12 @@ func (db *reindexerImpl) registerNamespace(ctx context.Context, namespace string return db.registerNamespaceImpl(namespace, opts, s) } +func (db *reindexerImpl) unregisterNamespaceImpl(namespace string) { + db.lock.Lock() + defer db.lock.Unlock() + delete(db.ns, namespace) +} + // registerNamespace Register go type against namespace. There are no data and indexes changes will be performed func (db *reindexerImpl) registerNamespaceImpl(namespace string, opts *NamespaceOptions, s interface{}) (err error) { t := reflect.TypeOf(s) @@ -532,10 +539,10 @@ func (db *reindexerImpl) registerNamespaceImpl(namespace string, opts *Namespace if err = validator.Validate(s); err != nil { return err } - if ns.indexes, err = parseIndexes(namespace, ns.rtype, &ns.joined); err != nil { + if ns.indexes, err = parseIndexes(ns.rtype, &ns.joined); err != nil { return err } - if schema := parseSchema(namespace, ns.rtype); schema != nil { + if schema := parseSchema(ns.rtype); schema != nil { ns.schema = *schema } @@ -558,10 +565,7 @@ func (db *reindexerImpl) dropNamespace(ctx context.Context, namespace string) er db.nsModifySlowLock.Lock() defer db.nsModifySlowLock.Unlock() - db.lock.Lock() - delete(db.ns, namespace) - db.lock.Unlock() - + db.unregisterNamespaceImpl(namespace) return db.binding.DropNamespace(ctx, namespace) } @@ -625,10 +629,7 @@ func (db *reindexerImpl) closeNamespace(ctx context.Context, namespace string) e db.nsModifySlowLock.Lock() defer db.nsModifySlowLock.Unlock() - db.lock.Lock() - delete(db.ns, namespace) - db.lock.Unlock() - + db.unregisterNamespaceImpl(namespace) return db.binding.CloseNamespace(ctx, namespace) } diff --git a/test/index_struct_test.go b/test/index_struct_test.go new file mode 100644 index 000000000..70bd89be8 --- /dev/null +++ b/test/index_struct_test.go @@ -0,0 +1,145 @@ +package reindexer + +import ( + "testing" + + "github.com/restream/reindexer/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type Account struct { + Id int64 `json:"id" reindex:"id,,pk"` + SAN string `json:"san" reindex:"san,hash"` + Count string `json:"-" reindex:"count,hash"` +} + +type Orders struct { + OrderId int64 `json:"-"` + OrderOwnerFName string + OrderTime uint32 `reindex:"ordertime"` + OrderCity string `reindex:"ordercity,tree"` +} + +type TestOpenNamespace struct { + Id int64 `json:"id" reindex:"id,,pk"` + First int `reindex:"first,-" json:"first"` + Age1 int64 `json:"-"` + Age2 int64 `reindex:"-"` + Field int64 + Parent *Orders `reindex:"-"` + + privateSimple string `json:"-"` + privateSlice string `json:"-"` + privateStruct struct{} `json:"-"` + + Accounts1 []Account `json:"-" reindex:"accounts,,joined"` + Accounts2 []Account `json:"-"` + Accounts3 []Account `reindex:"accounts,,joined"` + + Obj []Orders `json:"Ord1,omitempty"` + privateObj []Orders `json:"Ord2,omitempty"` + + _ struct{} `json:"-" reindex:"id+first,,composite"` + privateComposite1 struct{} `reindex:"first+id,,composite"` +} + +type FailSimple struct { + Id int64 `json:"id" reindex:"id,,pk"` + Age string `json:"-" reindex:"age,hash"` +} + +type FailPrivate struct { + Id int64 `json:"id" reindex:"id,,pk"` + private string `json:"private" reindex:"private,hash"` +} + +type FailPrivateJoin struct { + Id int64 `json:"id" reindex:"id,,pk"` + privateAccounts []Account `json:"-" reindex:"accounts,,joined"` +} + +type FailJoinScalar struct { + Id int64 `json:"id" reindex:"id,,pk"` + Accounts string `json:"-" reindex:"accounts,,joined"` +} + +type FailJoinSingleStruct struct { + Id int64 `json:"id" reindex:"id,,pk"` + Accounts Account `json:"-" reindex:"accounts,,joined"` +} + +type FailComposite struct { + Id int64 `json:"id" reindex:"id,,pk"` + Composite []Orders `json:"-" reindex:"ordertime+ordercity,,composite"` +} + +type ExpectedIndexDef struct { + Name string + JSONPaths []string + IndexType string + FieldType string +} + +func TestOpenNs(t *testing.T) { + t.Parallel() + + t.Run("open namespase: check indexes creation", func(t *testing.T) { + if len(DB.slaveList) > 0 { + t.Skip() // This test contains ns open/close and won't work with our replication testing logic + } + + const ns = "test_namespace_open" + err := DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), TestOpenNamespace{}) + defer DB.CloseNamespace(ns) + require.NoError(t, err) + desc, err := DB.DescribeNamespace(ns) + require.NoError(t, err) + actual := make([]ExpectedIndexDef, 0) + for _, idx := range desc.Indexes { + actual = append(actual, ExpectedIndexDef{ + idx.IndexDef.Name, + idx.IndexDef.JSONPaths, + idx.IndexDef.IndexType, + idx.IndexDef.FieldType, + }) + } + expected := []ExpectedIndexDef{ + {Name: "id", JSONPaths: []string{"id"}, IndexType: "hash", FieldType: "int64"}, + {Name: "first", JSONPaths: []string{"first"}, IndexType: "-", FieldType: "int64"}, + {Name: "ordertime", JSONPaths: []string{"Ord1.OrderTime"}, IndexType: "hash", FieldType: "int"}, + {Name: "ordercity", JSONPaths: []string{"Ord1.OrderCity"}, IndexType: "tree", FieldType: "string"}, + {Name: "id+first", JSONPaths: []string{"id", "first"}, IndexType: "hash", FieldType: "composite"}, + {Name: "first+id", JSONPaths: []string{"first", "id"}, IndexType: "hash", FieldType: "composite"}, + } + assert.Equal(t, expected, actual) + }) + + t.Run("no open namespace: check indexes are not created", func(t *testing.T) { + const ns = "test_no_namespace_open" + + err := DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailSimple{}) + assert.ErrorContains(t, err, + "non-composite/non-joined field ('Age'), marked with `json:-` can not have explicit reindex tags, but it does ('age,hash')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailPrivate{}) + assert.ErrorContains(t, err, + "unexported non-composite field ('private') can not have reindex tags, but it does ('private,hash')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailPrivateJoin{}) + assert.ErrorContains(t, err, + "unexported non-composite field ('privateAccounts') can not have reindex tags, but it does ('accounts,,joined')") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailJoinScalar{}) + assert.ErrorContains(t, err, + "joined index must be a slice of objects/pointers, but it is a scalar value") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailJoinSingleStruct{}) + assert.ErrorContains(t, err, + "joined index must be a slice of structs/pointers, but it is a single struct") + + err = DB.OpenNamespace(ns, reindexer.DefaultNamespaceOptions(), FailComposite{}) + assert.ErrorContains(t, err, + "'composite' tag allowed only on empty on structs: Invalid tags '[ordertime+ordercity composite]' on field 'Composite'") + }) +} diff --git a/test/uuid_test.go b/test/uuid_test.go index 6a859b933..774e3c844 100644 --- a/test/uuid_test.go +++ b/test/uuid_test.go @@ -220,25 +220,25 @@ func TestNotStringItemsWithUuidTag(t *testing.T) { nsOpts := reindexer.DefaultNamespaceOptions() t.Run("cant open ns when int field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int64' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int64' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestIntItemWithUuidTagNs, nsOpts, TestIntItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when int64 field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int64' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int64' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestInt64ItemWithUuidTagNs, nsOpts, TestInt64ItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when float field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'double' field, only with 'string'" + errExpr := "UUID index is not applicable with 'double' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestFloatItemWithUuidTagNs, nsOpts, TestFloatItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when double field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'double' field, only with 'string'" + errExpr := "UUID index is not applicable with 'double' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestDoubleItemWithUuidTagNs, nsOpts, TestDoubleItemWithUuidTagStruct{}), errExpr) }) @@ -250,13 +250,13 @@ func TestNotStringItemsWithUuidTag(t *testing.T) { }) t.Run("cant open ns when byte field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'int' field, only with 'string'" + errExpr := "UUID index is not applicable with 'int' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestByteItemWithUuidTagNs, nsOpts, TestByteItemWithUuidTagStruct{}), errExpr) }) t.Run("cant open ns when bool field with uuid tag", func(t *testing.T) { - errExpr := "UUID index is not applicable with 'bool' field, only with 'string'" + errExpr := "UUID index is not applicable with 'bool' field, only with 'string' (index name: 'uuid', field name: 'Uuid', jsonpath: 'uuid')" assert.EqualError(t, DB.OpenNamespace(TestBoolItemWithUuidTagNs, nsOpts, TestBoolItemWithUuidTagStruct{}), errExpr) })