Normalize optimization.Table DB storage

[glatterf42]

This PR is a follow-up to #122: moving to the next optimization item, it redesigns the data model to normalize it and hopefully also make it more performant.

For Tables, this is a bit more tricky than for Indexsets. Tables have multiple columns, each of which needs to be linked to an Indexset but can have a name other than the Indexset's name. Each Table also needs to store data in table form, where different Tables will have a different number of columns.
Previously, we solved this by dumping the data into a JSON field, but that did not achieve the desired performance, nor did it follow DB model normalization rules.

In preparation of this PR, I first thought that our ideal model would look like this:

• Table and Indexset should get a many-to-many relationship. Sqlalchemy's association proxy extension is an elegant solution for this, as it also offers to save optional fields (like distinct dimension names) along the relationship.
• Table.data only consists of values that are stored inside Indexset.data (that the Table is linked to), so ideally, we save just references to these values in a new table TableData and have a one-to-many relationship between Table and that. A relationship between that and IndexSetData seems like the ideal object for this.

Unfortunately, only the first point worked out. Sqlalchemy can't handle a relationship as part of a UniqueConstraint, preventing the INSERT workflow that this whole refactoring is aimed at achieving. Instead, the TableData is now storing the actual str form of the values, increasing storage requirements (compared to the ideal solution). This might be a source of future improvement.
To tackle the problem of varying numbers of columns, TableData is now hardcoded to store up to 15 values per row, most of which will always be NULL. This requires some clean up before returning the actual DB object like dropping NULL columns and renaming columns (to the name from either their indexset or specific column_name), which is likely the messiest part of this PR. Please let me know how that could be improved.

Locally, the tests run just as long as they do on main, but this is not proper benchmarking, which would be the actual test for this PR: it should handle a million values faster than main (and hopefully old ixmp). I previously tested this for Parameters (which utilize upserting rather than just inserting), though, so I might have to draft another PR very similar to this one for them and run the manual benchmark again. A proper benchmark setup for optimization items (to run automatically) would be great (depending on how long we need to reach ixmp's performance).

[codecov]

Codecov Report

Attention: Patch coverage is 97.27273% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.5%. Comparing base (1885f0c) to head (4af4ffb).

Files with missing lines	Patch %	Lines
ixmp4/data/db/optimization/table/repository.py	85.7%	3 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                       @@
##           enh/remove-superfluous-ellipses    #143   +/-   ##
===============================================================
  Coverage                             88.4%   88.5%           
===============================================================
  Files                                  231     231           
  Lines                                 8093    8155   +62     
===============================================================
+ Hits                                  7160    7219   +59     
- Misses                                 933     936    +3     

Files with missing lines	Coverage Δ
ixmp4/core/optimization/table.py	92.4% <100.0%> (-0.2%)	⬇️
ixmp4/data/abstract/optimization/table.py	96.9% <100.0%> (+<0.1%)	⬆️
ixmp4/data/api/optimization/table.py	94.0% <100.0%> (ø)
ixmp4/data/db/base.py	92.3% <100.0%> (+<0.1%)	⬆️
ixmp4/data/db/optimization/__init__.py	100.0% <100.0%> (ø)
ixmp4/data/db/optimization/base.py	100.0% <100.0%> (ø)
ixmp4/data/db/optimization/indexset/__init__.py	100.0% <100.0%> (ø)
ixmp4/data/db/optimization/indexset/repository.py	98.1% <100.0%> (ø)
ixmp4/data/db/optimization/table/model.py	100.0% <100.0%> (ø)
ixmp4/data/db/optimization/utils.py	100.0% <100.0%> (ø)
... and 4 more

[meksor]

Unfortunately, only the first point worked out. Sqlalchemy can't handle a relationship as part of a UniqueConstraint, preventing the INSERT workflow that this whole refactoring is aimed at achieving. Instead, the TableData is now storing the actual str form of the values, increasing storage requirements (compared to the ideal solution). This might be a source of future improvement.

Yes the relationship is a ORM concept the database does not know about it. Your current UniqueConstraint should not work either though, unless all columns are not NULL. From the postgres docs:

In general, a unique constraint is violated if there is more than one row in the table where the values of all of the columns included in the constraint are equal. By default, two null values are not considered equal in this comparison. That means even in the presence of a unique constraint it is possible to store duplicate rows that contain a null value in at least one of the constrained columns. This behavior can be changed by adding the clause NULLS NOT DISTINCT [...]

https://www.postgresql.org/docs/15/ddl-constraints.html#DDL-CONSTRAINTS-UNIQUE-CONSTRAINTS

You could add NULLS NOT DISTINCT, but i think that will break compat. with sqlite.

Am I wrong or are the tests spotty? Or does it not matter if that constraint works at all?

Im having a hard time gauging where potential performance bottlenecks are without any benchmarking and profiling tests and since we dont have any data on both approaches (is that right?) I feel like im choosing between:

a. massive hack, potentially better performance but who knows?
b. less massive hack, potentially worse performance but also -- who knows?

both without any explanation as to why they would be performing better or worse...

I know this is becoming an annoyingly common occurrence but im leaning towards a "No" on this one again without any further info...

[glatterf42]

Just summarizing the status here: I still think we would benefit from normalizing the data-storage-part of the optimization tables, possibly using the approach contained here.
However, especially after #219, this branch will not be mergeable without much refactoring, so if the approach seems useful, it might be best to start a new branch and cherry-pick (via git or manually) the changes to that one.
In order to fully understand if this is a good idea, we'd need the spare time to come up with performance tests, though. Ideally testing:

• tables.add_data()
• tables.remove_data()
• and both options in ixmp/on JDBC for comparison

Then we could compare the JSON and normalized DB models, and also provide numbers to the MESSAGE team whether ixmp4 improves performance in this regard (noting that Parameter.remove_data() on JDBC is a very common pain point).