Merge branch '1490_fix_join_fields_access' into 'develop'

Fix join entry fields access - [#1490]

See merge request itv-backend/reindexer!1372

cjson/decoder.go

@@ -711,7 +711,7 @@ func (dec *Decoder) Decode(cjson []byte, dest interface{}) (err error) {
 		}
 	}()
 
-	fieldsoutcnt := make([]int, 64, 64)
+	fieldsoutcnt := make([]int, MaxIndexes)
 	ctagsPath := make([]int, 0, 8)
 
 	dec.decodeValue(nil, ser, reflect.ValueOf(dest), fieldsoutcnt, ctagsPath)

cpp_src/CMakeLists.txt

@@ -89,6 +89,10 @@ endif ()
 
 set (EXTRA_FLAGS "")
 
+if (WITH_ASAN AND WITH_TSAN)
+  message(FATAL_ERROR "You cannot use the ASAN and TSAN options at the same time, CMake will exit.")
+endif()
+
 if (WITH_ASAN)
   set (EXTRA_FLAGS "-fsanitize=address")
   add_definitions(-DREINDEX_WITH_ASAN)

cpp_src/cmd/reindexer_server/test/test_storage_compatibility.sh

@@ -0,0 +1,194 @@
+#!/bin/bash
+# Task: https://github.com/restream/reindexer/-/issues/1188
+set -e
+
+function KillAndRemoveServer {
+	local pid=$1
+	kill $pid
+	wait $pid
+	yum remove -y 'reindexer*' > /dev/null
+}
+
+function WaitForDB {
+	# wait until DB is loaded
+	set +e # disable "exit on error" so the script won't stop when DB's not loaded yet
+	is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list');
+	while [[ $is_connected != "test" ]]
+		do
+			sleep 2
+			is_connected=$(reindexer_tool --dsn $ADDRESS --command '\databases list');
+		done
+	set -e
+}
+
+function CompareNamespacesLists {
+	local ns_list_actual=$1
+	local ns_list_expected=$2
+	local pid=$3
+
+	diff=$(echo ${ns_list_actual[@]} ${ns_list_expected[@]} | tr ' ' '\n' | sort | uniq -u) # compare in any order
+	if [ "$diff" == "" ]; then
+	    echo "## PASS: namespaces list not changed"
+	else
+	    echo "##### FAIL: namespaces list was changed"
+	    echo "expected: $ns_list_expected"
+	    echo "actual: $ns_list_actual"
+		KillAndRemoveServer $pid;
+	    exit 1
+    fi
+}
+
+function CompareMemstats {
+	local actual=$1
+	local expected=$2
+	local pid=$3
+	diff=$(echo ${actual[@]} ${expected[@]} | tr ' ' '\n' | sed 's/\(.*\),$/\1/' | sort | uniq -u) # compare in any order
+	if [ "$diff" == "" ]; then
+	    echo "## PASS: memstats not changed"
+	else
+	    echo "##### FAIL: memstats was changed"
+	    echo "expected: $expected"
+	    echo "actual: $actual"
+		KillAndRemoveServer $pid;
+	    exit 1
+    fi
+}
+
+
+RX_SERVER_CURRENT_VERSION_RPM="$(basename build/reindexer-*server*.rpm)"
+VERSION_FROM_RPM=$(echo "$RX_SERVER_CURRENT_VERSION_RPM" | grep -o '.*server-..')
+VERSION=$(echo ${VERSION_FROM_RPM: -2:1})   # one-digit version
+
+if [ $VERSION == 3 ]; then
+  LATEST_RELEASE=$(curl -s http://repo.restream.ru/itv-api-ng/7/x86_64/ | grep -o '>reindexer-server-.*.rpm' | tail -n1 | cut -c 2-)
+  namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg'
+elif [ $VERSION == 4 ]; then
+  LATEST_RELEASE=$(curl -s http://repo.restream.ru/itv-api-ng/7/x86_64/ | grep -o '>reindexer-4-server-.*.rpm' | tail -n1 | cut -c 2-)
+  # replicationstats ns added for v4
+  namespaces_list_expected=$'purchase_options_ext_dict\nchild_account_recommendations\n#config\n#activitystats\n#replicationstats\nradio_channels\ncollections\n#namespaces\nwp_imports_tasks\nepg_genres\nrecom_media_items_personal\nrecom_epg_archive_default\n#perfstats\nrecom_epg_live_default\nmedia_view_templates\nasset_video_servers\nwp_tasks_schedule\nadmin_roles\n#clientsstats\nrecom_epg_archive_personal\nrecom_media_items_similars\nmenu_items\naccount_recommendations\nkaraoke_items\nmedia_items\nbanners\n#queriesperfstats\nrecom_media_items_default\nrecom_epg_live_personal\nservices\n#memstats\nchannels\nmedia_item_recommendations\nwp_tasks_tasks\nepg'
+else
+  echo "Unknown version"
+  exit 1
+fi
+
+echo "## downloading latest release rpm file: $LATEST_RELEASE"
+curl "http://repo.itv.restr.im/itv-api-ng/7/x86_64/$LATEST_RELEASE" --output $LATEST_RELEASE;
+echo "## downloading example DB"
+curl "https://git.restream.ru/MaksimKravchuk/reindexer_testdata/-/raw/master/big.zip" --output big.zip;
+unzip -o big.zip # unzips into mydb_big.rxdump;
+
+ADDRESS="cproto://127.0.0.1:6534/"
+DB_NAME="test"
+
+memstats_expected=$'[
+{"replication":{"data_hash":24651210926,"data_count":3}},
+{"replication":{"data_hash":6252344969,"data_count":1}},
+{"replication":{"data_hash":37734732881,"data_count":28}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":1024095024522,"data_count":1145}},
+{"replication":{"data_hash":8373644068,"data_count":1315}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":7404222244,"data_count":97}},
+{"replication":{"data_hash":94132837196,"data_count":4}},
+{"replication":{"data_hash":1896088071,"data_count":2}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":-672103903,"data_count":33538}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":6833710705,"data_count":1}},
+{"replication":{"data_hash":5858155773472,"data_count":4500}},
+{"replication":{"data_hash":-473221280268823592,"data_count":65448}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":8288213744,"data_count":3}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":0,"data_count":0}},
+{"replication":{"data_hash":354171024786967,"data_count":3941}},
+{"replication":{"data_hash":-6520334670,"data_count":35886}},
+{"replication":{"data_hash":112772074632,"data_count":281}},
+{"replication":{"data_hash":-12679568198538,"data_count":1623116}}
+]
+Returned 27 rows'
+
+echo "##### Forward compatibility test #####"
+
+DB_PATH=$(pwd)"/rx_db"
+
+echo "Database: "$DB_PATH
+
+echo "## installing latest release: $LATEST_RELEASE"
+yum install -y $LATEST_RELEASE > /dev/null;
+# run RX server with disabled logging
+reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH &
+server_pid=$!
+sleep 2;
+
+reindexer_tool --dsn $ADDRESS$DB_NAME -f mydb_big.rxdump --createdb;
+sleep 1;
+
+namespaces_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list');
+echo $namespaces_1;
+CompareNamespacesLists "${namespaces_1[@]}" "${namespaces_list_expected[@]}" $server_pid;
+
+memstats_1=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats');
+CompareMemstats "${memstats_1[@]}" "${memstats_expected[@]}" $server_pid;
+
+KillAndRemoveServer $server_pid;
+
+echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM"
+yum install -y build/*.rpm > /dev/null;
+reindexer_server -l0  --corelog=none --httplog=none --rpclog=none --db $DB_PATH &
+server_pid=$!
+sleep 2;
+
+WaitForDB
+
+namespaces_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list');
+echo $namespaces_2;
+CompareNamespacesLists "${namespaces_2[@]}" "${namespaces_1[@]}" $server_pid;
+
+memstats_2=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats');
+CompareMemstats "${memstats_2[@]}" "${memstats_1[@]}" $server_pid;
+
+KillAndRemoveServer $server_pid;
+rm -rf $DB_PATH;
+sleep 1;
+
+echo "##### Backward compatibility test #####"
+
+echo "## installing current version: $RX_SERVER_CURRENT_VERSION_RPM"
+yum install -y build/*.rpm > /dev/null;
+reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH &
+server_pid=$!
+sleep 2;
+
+reindexer_tool --dsn $ADDRESS$DB_NAME -f mydb_big.rxdump --createdb;
+sleep 1;
+
+namespaces_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list');
+echo $namespaces_3;
+CompareNamespacesLists "${namespaces_3[@]}" "${namespaces_list_expected[@]}" $server_pid;
+
+memstats_3=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats');
+CompareMemstats "${memstats_3[@]}" "${memstats_expected[@]}" $server_pid;
+
+KillAndRemoveServer $server_pid;
+
+echo "## installing latest release: $LATEST_RELEASE"
+yum install -y $LATEST_RELEASE > /dev/null;
+reindexer_server -l warning --httplog=none --rpclog=none --db $DB_PATH &
+server_pid=$!
+sleep 2;
+
+WaitForDB
+
+namespaces_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command '\namespaces list');
+echo $namespaces_4;
+CompareNamespacesLists "${namespaces_4[@]}" "${namespaces_3[@]}" $server_pid;
+
+memstats_4=$(reindexer_tool --dsn $ADDRESS$DB_NAME --command 'select replication.data_hash, replication.data_count from #memstats');
+CompareMemstats "${memstats_4[@]}" "${memstats_3[@]}" $server_pid;
+
+KillAndRemoveServer $server_pid;
+rm -rf $DB_PATH;

cpp_src/cmd/reindexer_tool/commandsexecutor.cc

@@ -917,8 +917,8 @@ Error CommandsExecutor<DBInterface>::commandBench(const std::string& command) {
 	LineParser parser(command);
 	parser.NextToken();
 
-	int benchTime = reindexer::stoi(parser.NextToken());
-	if (benchTime == 0) benchTime = kBenchDefaultTime;
+	const std::string_view benchTimeToken = parser.NextToken();
+	const int benchTime = benchTimeToken.empty() ? kBenchDefaultTime : reindexer::stoi(benchTimeToken);
 
 	db().DropNamespace(kBenchNamespace);
 

cpp_src/core/cjson/fieldextractor.h

@@ -16,7 +16,7 @@ class FieldsExtractor {
 	};
 
 	FieldsExtractor() = default;
-	FieldsExtractor(VariantArray *va, KeyValueType expectedType, int expectedPathDepth, FieldsSet *filter = nullptr,
+	FieldsExtractor(VariantArray *va, KeyValueType expectedType, int expectedPathDepth, const FieldsSet *filter,
 					FieldParams *params = nullptr) noexcept
 		: values_(va), expectedType_(expectedType), expectedPathDepth_(expectedPathDepth), filter_(filter), params_(params) {}
 	FieldsExtractor(FieldsExtractor &&other) = default;
@@ -176,7 +176,7 @@ class FieldsExtractor {
 	VariantArray *values_ = nullptr;
 	KeyValueType expectedType_{KeyValueType::Undefined{}};
 	int expectedPathDepth_ = 0;
-	FieldsSet *filter_;
+	const FieldsSet *filter_;
 	FieldParams *params_;
 };
 

cpp_src/core/cjson/tagsmatcherimpl.h

@@ -4,7 +4,6 @@
 #include <sstream>
 #include <string>
 
-#include "core/keyvalue/key_string.h"
 #include "core/payload/payloadtype.h"
 #include "core/payload/payloadtypeimpl.h"
 #include "ctag.h"
@@ -75,25 +74,29 @@ class TagsMatcherImpl {
 					if (content == "*"sv) {
 						node.MarkAllItems(true);
 					} else {
-						int index = stoi(content);
-						if (index == 0 && content != "0"sv && ev) {
-							VariantArray values = ev(content);
-							if (values.size() != 1) {
-								throw Error(errParams, "Index expression_ has wrong syntax: '%s'", content);
+						auto index = try_stoi(content);
+						if (!index) {
+							if (ev) {
+								VariantArray values = ev(content);
+								if (values.size() != 1) {
+									throw Error(errParams, "Index expression_ has wrong syntax: '%s'", content);
+								}
+								values.front().Type().EvaluateOneOf(
+									[](OneOf<KeyValueType::Double, KeyValueType::Int, KeyValueType::Int64>) noexcept {},
+									[&](OneOf<KeyValueType::Bool, KeyValueType::String, KeyValueType::Tuple, KeyValueType::Composite,
+											  KeyValueType::Null, KeyValueType::Undefined, KeyValueType::Uuid>) {
+										throw Error(errParams, "Wrong type of index: '%s'", content);
+									});
+								node.SetExpression(content);
+								index = values.front().As<int>();
+							} else {
+								throw Error(errParams, "Can't convert '%s' to number", content);
 							}
-							values.front().Type().EvaluateOneOf(
-								[](OneOf<KeyValueType::Double, KeyValueType::Int, KeyValueType::Int64>) noexcept {},
-								[&](OneOf<KeyValueType::Bool, KeyValueType::String, KeyValueType::Tuple, KeyValueType::Composite,
-										  KeyValueType::Null, KeyValueType::Undefined, KeyValueType::Uuid>) {
-									throw Error(errParams, "Wrong type of index: '%s'", content);
-								});
-							node.SetExpression(content);
-							index = values.front().As<int>();
 						}
 						if (index < 0) {
 							throw Error(errLogic, "Array index value cannot be negative");
 						}
-						node.SetIndex(index);
+						node.SetIndex(*index);
 					}
 					field = field.substr(0, openBracketPos);
 				}

cpp_src/core/comparator.cc

@@ -191,7 +191,7 @@ void Comparator::ExcludeDistinct(const PayloadValue &data, int rowId) {
 	} else {
 		// Exclude field from payload by offset (fast path)
 
-		assertrx(!type_.Is<KeyValueType::Composite>());
+		assertrx_throw(!type_.Is<KeyValueType::Composite>());
 
 		// Check if we have column (rawData_), then go to fastest path with column
 		if (rawData_) return excludeDistinct(rawData_ + rowId * sizeof_);

cpp_src/core/itemmodifier.cc

@@ -1,9 +1,9 @@
 #include "itemmodifier.h"
+#include "core/itemimpl.h"
 #include "core/namespace/namespaceimpl.h"
 #include "core/query/expressionevaluator.h"
 #include "core/selectfunc/functionexecutor.h"
 #include "index/index.h"
-#include "tools/logger.h"
 
 namespace reindexer {
 

cpp_src/core/keyvalue/variant.h

@@ -212,6 +212,8 @@ template <>
 std::string Variant::As<std::string>() const;
 
 class VariantArray : public h_vector<Variant, 2> {
+	using Base = h_vector<Variant, 2>;
+
 public:
 	VariantArray() noexcept = default;
 	explicit VariantArray(Point) noexcept;
@@ -225,9 +227,14 @@ class VariantArray : public h_vector<Variant, 2> {
 		return std::move(*this);
 	}
 	void MarkObject() noexcept { isObjectValue = true; }
-	using h_vector<Variant, 2>::h_vector;
-	using h_vector<Variant, 2>::operator==;
-	using h_vector<Variant, 2>::operator!=;
+	using Base::Base;
+	using Base::operator==;
+	using Base::operator!=;
+	template <bool FreeHeapMemory = true>
+	void clear() noexcept {
+		isArrayValue = isObjectValue = false;
+		Base::clear<FreeHeapMemory>();
+	}
 	size_t Hash() const noexcept {
 		size_t ret = this->size();
 		for (size_t i = 0; i < this->size(); ++i) ret = (ret * 127) ^ this->at(i).Hash();

cpp_src/core/namespace/itemsloader.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <condition_variable>
+#include "core/itemimpl.h"
 #include "namespaceimpl.h"
 
 namespace reindexer {

cpp_src/core/namespace/namespaceimpl.h

@@ -20,11 +20,10 @@
 #include "core/schema.h"
 #include "core/storage/idatastorage.h"
 #include "core/storage/storagetype.h"
-#include "core/transactionimpl.h"
+#include "core/transaction.h"
 #include "estl/contexted_locks.h"
 #include "estl/fast_hash_map.h"
 #include "estl/shared_mutex.h"
-#include "estl/smart_lock.h"
 #include "estl/syncpool.h"
 #include "replicator/updatesobserver.h"
 #include "replicator/waltracker.h"

cpp_src/core/nsselecter/aggregator.cc

@@ -26,8 +26,13 @@ static void copy(It begin, It end, std::vector<FacetResult> &facets, const Field
 			ConstPayload pl(payloadType, begin->first);
 			VariantArray va;
 			if (fields[i] == IndexValueType::SetByJsonPath) {
-				const TagsPath &tagsPath = fields.getTagsPath(tagPathIdx++);
-				pl.GetByJsonPath(tagsPath, va, KeyValueType::Undefined{});
+				if (fields.isTagsPathIndexed(tagPathIdx)) {
+					const IndexedTagsPath &tagsPath = fields.getIndexedTagsPath(tagPathIdx++);
+					pl.GetByJsonPath(tagsPath, va, KeyValueType::Undefined{});
+				} else {
+					const TagsPath &tagsPath = fields.getTagsPath(tagPathIdx++);
+					pl.GetByJsonPath(tagsPath, va, KeyValueType::Undefined{});
+				}
 				if (va.IsObjectValue()) {
 					throw Error(errQueryExec, "Cannot aggregate object field");
 				}