Unknown computed column type

[boutinb]

The computedColumnType 'unknown' does not exist anymore, but might be stored in the database. So we need to take care of this.
Fixes jasp-stats/jasp-issues#2517

[JorisGoosen]

You dont need to make changes to column.cpp I think?

Just the change to DatabaseInterface::columnGetComputedInfo should be enough

[boutinb]

I was not quite completely sure that this error could not arise here. For the case, it does not hurt I fimd.

[JorisGoosen]

Yeah but if the changes are not necessary at all why make them?

[JorisGoosen]

Ah while testing I found that loading the jasp file from the issue with the current PR gets:

So it is "not computed" yet it is computed according to the columnheader.

So this is not really a fix yet.

[JorisGoosen]

the question is more, how can unknown even be written there? Or was that simply the default when we didnt set it before?
How is it possible that a column was stored with unknown as computed column type at all?

This has a rootcause and we need to figure it out how it happened.

Also, to expand this workaround it would perhaps be good to check in DatabaseInterface::columnGetComputedInfo whether the computedColumnType isnt notComputed before returning true (because that says: isComputed)

[boutinb]

the question is more, how can unknown even be written there? Or was that simply the default when we didnt set it before? How is it possible that a column was stored with unknown as computed column type at all?

This has a rootcause and we need to figure it out how it happened.

When I open the file in 0.18.1, then I see also a not computed column with an icon telling this is a computed column. So the situation was buggy in 0.18.1. The rcode of this column is also empty.
I have tried to reproduce this in 0.18.1 by undo/redo, copy/paste, but could not find a way to get such a situation.
But the code should be better in 0.18.2! So we can expect that this is not reproducible anyway.

The isComputed column in the database is not needed anymore. So I would just remove it.

[JorisGoosen]

The isComputed column in the database is not needed anymore. So I would just remove it.

Ok so remove it in void DatabaseInterface::upgradeDBFromVersion(Version originalVersion)
By adding something to it like:

	   if(originalVersion < "0.18.2")
			   runStatements("ALTER TABLE DataSets ADD COLUMN description     TEXT;"                 "\n");

	   if(originalVersion < "0.18.3")
			   runStatements("ALTER TABLE Columns DROP COLUMN isComputed;"                 "\n");

And then remove it from all the functions that expect it...

You want this to be a hotfix or a potential place to create more bugs though? ;)

[boutinb]

Hmm by removing isColumn in the database, we have then a downgrade issue: JASP file made in 0.18.3 won't be readable in 0.18.2 or lower... This is not expected for a minor release. So I will let the isColumn in the database. We can remove it in 0.19

[JorisGoosen]

Exactly, however, you still need to fix the output of that function then. The best thing would be to make sure that if the computedColumnType is "notComputed" the isComputed thing is set FALSE

[boutinb]

I have removed the _isColumn property of the Column class, as it can be deduced by the codeType property
However for downgrade purpose, the isColumn column in the database is kept so that JASP file made in 0.18.3 can be still read in 0.18.2 or 0.18.1.