Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix filter crash #5720

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Fix filter crash #5720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41e1e22
Fix filter crash
JorisGoosen Oct 24, 2024
052a80f
if one wants to replace both the orignal value and display of a label…
JorisGoosen Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 77 additions & 17 deletions CommonData/column.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void Column::dbLoad(int id, bool getValues)

_emptyValues->fromJson(emptyVals);

labelsTempReset();
_resetLabelValueMap();
db().labelsLoad(this);

if(getValues)
Expand Down Expand Up @@ -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);
Expand All @@ -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<int> valuesToRemove, bool updateOrder)
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -1350,7 +1385,7 @@ bool Column::replaceDoubleLabelFromRowWithDouble(size_t row, double dbl)
return true;
}

void Column::labelValueChanged(Label *label, double aDouble, const Json::Value & previousOriginal)
void Column::labelValueChanged(Label *label, const Json::Value & previousOriginal)
{
auto oldValDis = std::make_pair(Label::originalValueAsString(this, previousOriginal), label->labelDisplay());
bool merged = _labelByValDis.count(label->origValDisplay()) != 0;
Expand All @@ -1363,17 +1398,11 @@ void Column::labelValueChanged(Label *label, double aDouble, const Json::Value &
//And that its new location is free:
assert(_labelByValDis.count(label->origValDisplay()) == 0 || _labelByValDis.at(label->origValDisplay()) == label);

//Lets assume that all occurences of a label in _dbls are the same.
//So when we encounter one that is the same as what is passed here we can return immediately
double theDouble = label->originalValue().isDouble() ? label->originalValue().asDouble() : EmptyValues::missingValueDouble;

for(size_t r=0; r<_dbls.size(); r++)
if(_ints[r] == label->intsId())
{
if(Utils::isEqual(_dbls[r], aDouble))
return;

_dbls[r] = aDouble;
}

_dbls[r] = theDouble;

_labelByValDis.erase(oldValDis);
_labelByValDis[label->origValDisplay()] = label;
Expand Down Expand Up @@ -1422,6 +1451,37 @@ void Column::labelDisplayChanged(Label *label, const std::string & previousDispl
_labelsTempRevision++;
}

void Column::labelValDisplayChanged(Label *label, const std::string &previousDisplay, const Json::Value &previousOriginal)
{
auto oldValDis = std::make_pair(Label::originalValueAsString(this, previousOriginal), previousDisplay),
newValDis = std::make_pair(label->originalValueAsString(), label->labelDisplay());
bool merged = _labelByValDis.count(label->origValDisplay()) != 0;

if(merged)
labelsMergeDuplicateInto(label);

//Make sure it was registered before:
assert(_labelByValDis[oldValDis] == label);
//And that its new location is free:
assert(_labelByValDis.count(label->origValDisplay()) == 0 || _labelByValDis.at(label->origValDisplay()) == label);

double newOrigValDbl = label->originalValue().isDouble() ? label->originalValue().asDouble() : EmptyValues::missingValueDouble;

//Lets assume that all occurences of a label in _dbls are the same.
//So when we encounter one that is the same as what is passed here we can return immediately
for(size_t r=0; r<_dbls.size(); r++)
if(_ints[r] == label->intsId())
_dbls[r] = newOrigValDbl;

_labelByValDis.erase(oldValDis);
_labelByValDis[label->origValDisplay()] = label;

if(merged)
_dbUpdateLabelOrder();

dbUpdateValues();
}

Label * Column::labelByRow(int row) const
{
if (row < rowCount() && _type != columnType::scale && _ints[row] != EmptyValues::missingValueInteger)
Expand Down
9 changes: 6 additions & 3 deletions CommonData/column.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -153,9 +154,9 @@ class Column : public DataSetBaseNode
Label * replaceDoublesTillLabelsRowWithLabels(size_t row, double returnForDbl = NAN);
bool replaceDoubleLabelFromRowWithDouble(size_t row, double dbl); ///< Returns true if succes

void labelValueChanged(Label * label, double aDouble, const Json::Value & previousOriginal); ///< Pass NaN for non-convertible values
void labelValueChanged(Label * label, int anInteger, const Json::Value & previousOriginal) { labelValueChanged(label, double(anInteger), previousOriginal); }
void labelDisplayChanged(Label * label, const std::string & previousDisplay);
void labelValueChanged( Label * label, const Json::Value & previousOriginal); ///< Pass NaN for non-convertible values
void labelDisplayChanged( Label * label, const std::string & previousDisplay);
void labelValDisplayChanged( Label * label, const std::string & previousDisplay, const Json::Value & previousOriginal);

bool setStringValue( size_t row, const std::string & value, const std::string & label = "", bool writeToDB = true); ///< Does two things, if label=="" it will handle user input, as value or label depending on columnType. Otherwise it will simply try to use userEntered as a value. But this will trigger the setting of type
bool setValue( size_t row, const std::string & value, const std::string & label, bool writeToDB = true);
Expand Down Expand Up @@ -244,6 +245,7 @@ class Column : public DataSetBaseNode
void _resetLabelValueMap();
doublevec valuesNumericOrdered();
std::map<Label*,size_t> valuesAlphabeticalOffsets();
int _labelMapIt(Label *label);

private:
DataSet * const _data;
Expand Down Expand Up @@ -282,6 +284,7 @@ class Column : public DataSetBaseNode
static bool _autoSortByValuesByDefault;



};

typedef std::vector<Column*> Columns;
Expand Down
6 changes: 2 additions & 4 deletions CommonData/databaseinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
41 changes: 33 additions & 8 deletions CommonData/label.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -146,23 +147,47 @@ bool Label::setLabel(const std::string & label)
return false;
}

bool Label::setOriginalValue(const Json::Value & originalLabel)
bool Label::setOriginalValue(const Json::Value & originalValue)
{
if(_originalValue != originalLabel)
if(_originalValue != originalValue)
{
Json::Value previous = _originalValue;
_originalValue = originalLabel;
Json::Value previous = _originalValue;
_originalValue = originalValue;
dbUpdate();

if(_originalValue.isDouble()) _column->labelValueChanged(this, _originalValue.asDouble(), previous);
else if(_originalValue.isInt()) _column->labelValueChanged(this, _originalValue.asInt(), previous);
else _column->labelValueChanged(this, EmptyValues::missingValueDouble, previous);
_column->labelValueChanged(this, previous);

return true;
}
return false;
}

bool Label::setOrigValLabel(const Json::Value &originalValue)
{
std::string oldLabel = _label,
newLabel = !originalValue.isDouble() ? originalValue.asString() : _column->doubleToDisplayString(originalValue.asDouble(), false, false);
Json::Value previous = _originalValue;
bool labelChange = _label != newLabel,
valChange = _originalValue != originalValue,
aChange = labelChange || valChange;

if(labelChange)
_label = newLabel;

if(valChange)
_originalValue = originalValue;


if(aChange)
{
dbUpdate();

_column->labelValDisplayChanged(this, oldLabel, previous);
return true;
}
return false;
}

bool Label::setDescription(const std::string &description)
{
if(_description != description)
Expand Down
1 change: 1 addition & 0 deletions CommonData/label.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Label : public DataSetBaseNode
void setDbId( int id) { _dbId = id; }
bool setLabel( const std::string & label);
bool setOriginalValue( const Json::Value & originalValue);
bool setOrigValLabel( const Json::Value & originalValue);
bool setDescription( const std::string & description);
bool setFilterAllows( bool allowFilter);
void setInformation(Column * column, int id, int order, const std::string &label, int value, bool filterAllows, const std::string & description, const Json::Value & originalValue);
Expand Down
26 changes: 15 additions & 11 deletions Desktop/data/datasetpackage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
Expand Down Expand Up @@ -951,20 +952,23 @@ bool DataSetPackage::setLabelValue(const QModelIndex &index, const QString &newL
aChange = true;
}

//If the user is changing the value of a column to a string we want the display/label to also change if its the same
//to a double/int however might mean recoding so it would be a bit impractical to have the label dissappear
if( label->originalValueAsString(false) == label->labelDisplay())
{
if(!originalValue.isDouble())
aChange = label->setLabel(originalValue.asString()) || aChange;
// Here we will overwrite the original value with the new origval.
// but if the label is the same as the original value we want to make the users life easier and replace it as well.
// this makes sense if the user is changing a string or number. But if the user is recoding, so turning values from str => dbl
// then we dont want to do this, because then the label should be different afterwards.

else if(label->originalValue().isDouble())
label->setLabel(column->doubleToDisplayString(originalValue.asDouble(), false));
//summarized:
// if orgval == label then:
// if (oldorigval == dbl && newOrigVal == dbl) || (olorigval != dbl && newOrigVal != dbl) then replace both
// if neworigval == dbl and oldorigval != dbl then replace only value

if( label->originalValueAsString(false) != label->labelDisplay() || (originalValue.isDouble() && !label->originalValue().isDouble()))
aChange = label->setOriginalValue(originalValue) || aChange;
else
aChange = label->setOrigValLabel(originalValue) || aChange;
}

aChange = label->setOriginalValue(originalValue) || aChange;

column->labelsHandleAutoSort();

if(aChange)
Expand Down Expand Up @@ -2195,8 +2199,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);
Expand Down
Loading