Fix multiple values for a single label data in R

CommonData/column.cpp

@@ -1221,7 +1221,7 @@ stringvec Column::displaysAsStrings() const
 	return returnMe;
 }
 
-stringvec Column::dataAsRLevels(intvec & values, const boolvec & filter, bool useLabels )
+stringvec Column::dataAsRLevels(intvec & values, const boolvec & filter)
 {
 	JASPTIMER_SCOPE(Column::dataAsRLevels);
 
@@ -1231,7 +1231,8 @@ stringvec Column::dataAsRLevels(intvec & values, const boolvec & filter, bool us
 	const bool 	useFilter 	= filter.size() == rowCount();
 	int  		valuesSize	= 0;
 
-	intset ids, usedIds;
+	intset		ids;
+	stringset	usedLevels; //https://github.com/jasp-stats/jasp-issues/issues/3721 points out rightly that if you map multiple values to the same label, they should then be 1 label in ordinal/nominal!
 
 	for(size_t row=0; row<rowCount(); row++)
 		if(!useFilter || filter[row])
@@ -1244,21 +1245,21 @@ stringvec Column::dataAsRLevels(intvec & values, const boolvec & filter, bool us
 		if(id != Label::NO_LABEL && id != EmptyValues::missingValueInteger)
 		{
 			Label * label = labelByIntsId(id);
-			
+
 			if(label && !label->isEmptyValue())
-				usedIds.insert(id);
+				usedLevels.insert(label->label());
 		}
 
 	stringvec levels;
-	levels.reserve(usedIds.size());
+	levels.reserve(usedLevels.size());
 
-	intintmap idToLevel;
+	strintmap labelToLevel;
 
 	for(Label * label : _labels)
-		if(usedIds.count(label->intsId()) || !shouldDropLevels())
+		if(!labelToLevel.count(label->label()) && (usedLevels.count(label->label()) || !shouldDropLevels()))
 		{
-			levels.push_back(useLabels ? label->label() : label->originalValueAsString(false));
-			idToLevel[label->intsId()] = levels.size();
+			levels.push_back(label->label());
+			labelToLevel[label->label()] = levels.size();
 		}
 
 	values.resize(valuesSize);
@@ -1268,8 +1269,13 @@ stringvec Column::dataAsRLevels(intvec & values, const boolvec & filter, bool us
 		{
 			values[valueRow] = EmptyValues::missingValueInteger;
 
-			if(_ints[row] != Label::NO_LABEL && _ints[row] != EmptyValues::missingValueInteger && idToLevel.count(_ints[row]))
-				values[valueRow] = idToLevel.at(_ints[row]);
+			if(_ints[row] != Label::NO_LABEL && _ints[row] != EmptyValues::missingValueInteger)
+			{
+				Label * label = labelByIntsId(_ints[row]);
+
+				if(label && labelToLevel.count(label->label()))
+					values[valueRow] = labelToLevel.at(label->label());
+			}
 
 			valueRow++;
 		}

CommonData/column.h

@@ -147,7 +147,7 @@ class Column : public DataSetBaseNode
 			stringvec				labelsAsStrings()																						const;
 			stringvec				nonEmptyLevelsStrings()																					const;
 			stringvec				displaysAsStrings()																						const;
-			stringvec				dataAsRLevels(intvec & values, const boolvec & filter, bool useLabels = true)			; ///< values is output! If filter is of different length than the data an error is thrown, if length is zero it is ignored. useLabels indicates whether the levels will be based on the label or on the value as specified in the label editor.
+			stringvec				dataAsRLevels(intvec & values, const boolvec & filter)			; ///< values is output! If filter is of different length than the data an error is thrown, if length is zero it is ignored. useLabels indicates whether the levels will be based on the label or on the value as specified in the label editor.
 			doublevec				dataAsRDoubles(const boolvec & filter)													const; ///< If filter is of different length than the data an error is thrown, if length is zero it is ignored
 
 			void					labelValueChanged(		Label * label,	const Json::Value & previousOriginal); ///< Pass NaN for non-convertible values

CommonData/databaseinterface.cpp

@@ -760,7 +760,7 @@ void DatabaseInterface::dataSetBatchedValuesLoad(DataSet *data, std::function<vo
 			cols.push_back(data->column(curCol));
 
 
-        threads.push_back(std::thread([cols, group, &loadBatchOfColumns,this]()
+		threads.push_back(std::thread([cols, group, &loadBatchOfColumns,this]()
 		{
 			preloadInterfaceForThread();
 			loadBatchOfColumns(cols, group);
@@ -2054,10 +2054,18 @@ sqlite3 * DatabaseInterface::_db()
 	if(_dbCreated && _dbCreator == id)
 		return _dbCreated;
 
-	if(!_dbs.count(id))
+	_dbCheckMutex.lock();
+	bool itsNotThereYet = !_dbs.count(id); //tip toe around the map
+	_dbCheckMutex.unlock();
+
+	if(itsNotThereYet)
 		load();
 
-	return _dbs.at(id);
+	_dbCheckMutex.lock();
+	sqlite3 * dbFound = _dbs.at(id);
+	_dbCheckMutex.unlock();
+
+	return dbFound;
 }
 
 void DatabaseInterface::create()
@@ -2071,7 +2079,7 @@ void DatabaseInterface::create()
 		std::filesystem::remove(dbFile());
 	}
 
-    int ret = sqlite3_open_v2(dbFile().c_str(), &_dbCreated, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX, NULL);
+	int ret = sqlite3_open_v2(dbFile().c_str(), &_dbCreated, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX, NULL);
 
 	if(ret != SQLITE_OK)
 	{
@@ -2132,29 +2140,29 @@ void DatabaseInterface::doWalCheckPoint()
 
 void DatabaseInterface::preloadInterfaceForThread()
 {
-    //Load the interface by asking for it
-    _db();
+	//Load the interface by asking for it
+	_db();
 }
 
 void DatabaseInterface::load()
 {
-    JASPTIMER_SCOPE(DatabaseInterface::load);
-    assert(!_dbCreated || std::this_thread::get_id() != _dbCreator);
+	JASPTIMER_SCOPE(DatabaseInterface::load);
+	assert(!_dbCreated || std::this_thread::get_id() != _dbCreator);
 
-    _loadMutex.lock();
-    if(_dbs.count(std::this_thread::get_id()))
-    {
-        sqlite3* connection = _dbs.at(std::this_thread::get_id());
-        _loadMutex.unlock();
+	_loadMutex.lock();
+	if(_dbs.count(std::this_thread::get_id()))
+	{
+		sqlite3* connection = _dbs.at(std::this_thread::get_id());
+		_loadMutex.unlock();
 		return;
-    }
+	}
 
-    if(!std::filesystem::exists(dbFile()))
-        throw std::runtime_error("Trying to load '" + dbFile() + "' but it doesn't exist!");
+	if(!std::filesystem::exists(dbFile()))
+		throw std::runtime_error("Trying to load '" + dbFile() + "' but it doesn't exist!");
 
-    bool        loadingWorked	= false;
-    size_t      loadingAttempt	= 0;
-    sqlite3 *   db              = nullptr;
+	bool        loadingWorked	= false;
+	size_t      loadingAttempt	= 0;
+	sqlite3 *   db              = nullptr;
 
 	int ret = sqlite3_open_v2(dbFile().c_str(), &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_NOMUTEX, NULL);
 
@@ -2166,38 +2174,40 @@ void DatabaseInterface::load()
 	else
 		Log::log() << "Opened internal sqlite database for loading at '" << dbFile() << "'. This is for thread " << std::this_thread::get_id() << std::endl;
 
+	_dbCheckMutex.lock();
 	_dbs[std::this_thread::get_id()] = db;
+	_dbCheckMutex.unlock();
 	_loadMutex.unlock();
 
-    for(bool loadingWorked = false; !loadingWorked; )
-    {
+	for(bool loadingWorked = false; !loadingWorked; )
+	{
 		sqlite3_busy_timeout(db, 100);
 
-        try
-        {
-            int tableCount = runStatementsId("SELECT COUNT(*) FROM sqlite_schema WHERE type ='table' AND name NOT LIKE 'sqlite_%';");
-            if(tableCount < 0)
-                throw dbMalformedException();
-
-            Log::log() << "Loaded a database with #" << tableCount << " tables." << std::endl;
-            loadingWorked = true;
-        }
-        catch(dbMalformedException & e)
-        {
-            //Unfortunate, but perhaps we were too quick?
-            loadingWorked = false;
-            loadingAttempt++;
-        }
-
-        if(!loadingWorked)
-        {
+		try
+		{
+			int tableCount = runStatementsId("SELECT COUNT(*) FROM sqlite_schema WHERE type ='table' AND name NOT LIKE 'sqlite_%';");
+			if(tableCount < 0)
+				throw dbMalformedException();
+
+			Log::log() << "Loaded a database with #" << tableCount << " tables." << std::endl;
+			loadingWorked = true;
+		}
+		catch(dbMalformedException & e)
+		{
+			//Unfortunate, but perhaps we were too quick?
+			loadingWorked = false;
+			loadingAttempt++;
+		}
+
+		if(!loadingWorked)
+		{
 			if(loadingAttempt > 10 * 60) //Timeout is 0.1 sec, so this lets the db try for 1 minute to connect...
-                throw dbMalformedException();
+				throw dbMalformedException();
 
-            Log::log() << "There was a problem loading the database, retrying for the #" << loadingAttempt << " time" << std::endl;
+			Log::log() << "There was a problem loading the database, retrying for the #" << loadingAttempt << " time" << std::endl;
 			std::this_thread::sleep_for(std::chrono::nanoseconds(100000000));
-        }
-    }
+		}
+	}
 
 	return;
 }
@@ -2208,6 +2218,8 @@ void DatabaseInterface::close()
 
 	std::set<sqlite3*> waitingFor;
 
+	_dbCheckMutex.lock();
+
 	for(auto & idDb : _dbs)
 		waitingFor.insert(idDb.second);
 
@@ -2228,6 +2240,8 @@ void DatabaseInterface::close()
 	while(waitingFor.size() > 0);
 
 	_dbs.clear();
+
+	_dbCheckMutex.unlock();
 
 	while(sqlite3_close(_dbCreated) != SQLITE_OK)
 	{

CommonData/databaseinterface.h

@@ -203,7 +203,8 @@ class DatabaseInterface
 	std::thread::id							_dbCreator;
 	sqlite3*								_dbCreated = nullptr;
 	bool									_inMemory;
-    std::mutex                              _loadMutex;
+	std::mutex                              _loadMutex,
+											_dbCheckMutex;
 
 	static			std::string _wrap_sqlite3_column_text(sqlite3_stmt * stmt, int iCol);
 	static const	std::string _dbConstructionSql;

CommonData/rbridge.cpp

@@ -459,7 +459,7 @@ extern "C" RBridgeColumn* STDCALL rbridge_readDataSet(RBridgeColumnType* colHead
 			if(obeyFilter)
 				filterToUse = rbridge_dataSet->filter()->filtered();
 
-			stringvec levels = column->dataAsRLevels(vals, filterToUse, true);
+			stringvec levels = column->dataAsRLevels(vals, filterToUse);
 
 			memcpy(resultCol.ints, vals.data(), vals.size() * sizeof(int));