From 41e1e22d6f9b0d08c9ce2e34597f70d10a1f301f Mon Sep 17 00:00:00 2001 From: Joris Goosen Date: Thu, 24 Oct 2024 14:10:13 +0200 Subject: [PATCH] Fix filter crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bug happened where if you entered in new data: (one by one) column1, "c", "1" column2, "2" the engine would crash because of some bad housekeeping of data... this is because labelsAdd was used which checks the valDisMap and then doesnt really add anything. The db was iterating over rows though... But ive now replaced it with column->labelsSet. It takes the honor of either adding a label or overwriting one on a particular place. And adds them to the also freshly cleaned valDisMap and intsIdMap. Also, try not to resize the labels to be bigger then it needs to be to avoid all those 0pointer exceptions 😿 --- CommonData/column.cpp | 49 +++++++++++++++++++++++++++----- CommonData/column.h | 3 ++ CommonData/databaseinterface.cpp | 6 ++-- CommonData/label.cpp | 3 +- Desktop/data/datasetpackage.cpp | 5 ++-- 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/CommonData/column.cpp b/CommonData/column.cpp index 89feab2959..b0dd91343d 100644 --- a/CommonData/column.cpp +++ b/CommonData/column.cpp @@ -62,7 +62,7 @@ void Column::dbLoad(int id, bool getValues) _emptyValues->fromJson(emptyVals); - labelsTempReset(); + _resetLabelValueMap(); db().labelsLoad(this); if(getValues) @@ -721,6 +721,17 @@ int Column::labelsAdd(const std::string & display, const std::string & descripti return labelsAdd(++_highestIntsId, display, true, description, originalValue); } +int Column::_labelMapIt(Label * label) +{ + _labelByIntsIdMap[label->intsId()] = label; + _labelByValDis[label->origValDisplay()] = label; + + _highestIntsId = std::max(_highestIntsId, label->intsId()); + + _dbUpdateLabelOrder(true); + return label->intsId(); +} + int Column::labelsAdd(int value, const std::string & display, bool filterAllows, const std::string & description, const Json::Value & originalValue, int order, int id) { JASPTIMER_SCOPE(Column::labelsAdd lotsa arg); @@ -733,13 +744,36 @@ int Column::labelsAdd(int value, const std::string & display, bool filterAllows, Label * label = new Label(this, display, value, filterAllows, description, originalValue, order, id); _labels.push_back(label); - _labelByIntsIdMap[label->intsId()] = label; - _labelByValDis[valDisplay] = label; + return _labelMapIt(label); +} - _highestIntsId = std::max(_highestIntsId, label->intsId()); +int Column::labelsSet(int labelIndex, int value, const std::string &display, bool filterAllows, const std::string &description, const Json::Value &originalValue, int order, int id) +{ + JASPTIMER_SCOPE(Column::labelsSet); - _dbUpdateLabelOrder(true); - return label->intsId(); + Label * label = nullptr; + if(_labels.size() > labelIndex) + { + label = _labels[labelIndex]; + label->setInformation(this, id, order, display, value, filterAllows, description, originalValue); + } + else + { + if(_labels.size() != labelIndex) + Log::log() << "Labels are not being set in a sensible order it seems." << std::endl; + + if(id == -1) + Log::log() << "This functions expects a label to be set from the DB, so why is dbId -1?" << std::endl; + + if(_labels.size() < labelIndex+1) + _labels . resize(labelIndex+1); + _labels[labelIndex] = new Label(this, display, value, filterAllows, description, originalValue, order, id); + label = _labels[labelIndex]; + } + + assert(label); + + return _labelMapIt(label); } void Column::labelsRemoveByIntsId(std::set valuesToRemove, bool updateOrder) @@ -809,7 +843,8 @@ void Column::labelsRemoveBeyond(size_t desiredLabelsSize) for(size_t i=desiredLabelsSize; i<_labels.size(); i++) delete _labels[i]; - _labels.resize(desiredLabelsSize); + if(desiredLabelsSize < _labels.size()) + _labels.resize(desiredLabelsSize); _resetLabelValueMap(); } diff --git a/CommonData/column.h b/CommonData/column.h index 253121808b..37684f94d0 100644 --- a/CommonData/column.h +++ b/CommonData/column.h @@ -114,6 +114,7 @@ class Column : public DataSetBaseNode int labelsAdd( const std::string & display); int labelsAdd( const std::string & display, const std::string & description, const Json::Value & originalValue); int labelsAdd( int value, const std::string & display, bool filterAllows, const std::string & description, const Json::Value & originalValue, int order=-1, int id=-1); + int labelsSet(int lbId, int value, const std::string & display, bool filterAllows, const std::string & description, const Json::Value & originalValue, int order=-1, int id=-1); void labelsRemoveByIntsId( intset valuesToRemove, bool updateOrder = true); strintmap labelsResetValues( int & maxValue); void labelsRemoveBeyond( size_t indexToStartRemoving); @@ -244,6 +245,7 @@ class Column : public DataSetBaseNode void _resetLabelValueMap(); doublevec valuesNumericOrdered(); std::map valuesAlphabeticalOffsets(); + int _labelMapIt(Label *label); private: DataSet * const _data; @@ -282,6 +284,7 @@ class Column : public DataSetBaseNode static bool _autoSortByValuesByDefault; + }; typedef std::vector Columns; diff --git a/CommonData/databaseinterface.cpp b/CommonData/databaseinterface.cpp index 8849c650b9..76bf67be1e 100644 --- a/CommonData/databaseinterface.cpp +++ b/CommonData/databaseinterface.cpp @@ -1290,14 +1290,12 @@ void DatabaseInterface::labelsLoad(Column * column) if (originalValueJson.isNull() && !originalValueJsonStr.empty()) originalValueJson = originalValueJsonStr; // For backward compatibility: in some JASP files the originalValueJson is not a json string but just the original string. - - if(column->labels().size() == row) column->labelsAdd(value, label, filterAllows, description, originalValueJson, order, id); - else column->labels()[row]->setInformation(column, id, order, label, value, filterAllows, description, originalValueJson); + column->labelsSet(row, value, label, filterAllows, description, originalValueJson, order, id); }; runStatements("SELECT id, value, label, ordering, filterAllows, description, originalValueJson FROM Labels WHERE columnId = ? ORDER BY ordering;", prepare, processRow); - + column->labelsRemoveBeyond(labelsSize); column->endBatchedLabelsDB(false); diff --git a/CommonData/label.cpp b/CommonData/label.cpp index 0f264a69d4..142b2c6254 100644 --- a/CommonData/label.cpp +++ b/CommonData/label.cpp @@ -92,7 +92,8 @@ void Label::dbUpdate() void Label::setInformation(Column * column, int id, int order, const std::string &label, int value, bool filterAllows, const std::string & description, const Json::Value & originalValue) { - _dbId = id; + assert(_column == column); + _dbId = id; _order = order; _label = label; _intsId = value; diff --git a/Desktop/data/datasetpackage.cpp b/Desktop/data/datasetpackage.cpp index a4b89b1772..737aa2001f 100644 --- a/Desktop/data/datasetpackage.cpp +++ b/Desktop/data/datasetpackage.cpp @@ -719,6 +719,7 @@ bool DataSetPackage::setData(const QModelIndex &index, const QVariant &value, in setManualEdits(true); //Don't synch with external file after editing column->labelsRemoveOrphans(); + column->labelsTempReset(); column->labelsHandleAutoSort(); stringvec changedCols = {column->name()}; @@ -2195,8 +2196,8 @@ bool DataSetPackage::removeRows(int row, int count, const QModelIndex & aparent) { changed.push_back(column->name()); - if(row+count > column->rowCount()) - Log::log() << "???" << std::endl; + //if(row+count > column->rowCount()) + // Log::log() << "???" << std::endl; for(int r=row+count; r>row; r--) column->rowDelete(r-1);