Water chemistry is still broken

[mikfire]

[15:34:01.109] (1dor1tsx4w) ERROR : int ObjectStore::impl::insertObjectInDb(QSqlDatabase&, const QObject&, bool) 
Error executing database query  "INSERT INTO water (name, display, deleted, folder, notes, amount, calcium, bicarbonate, sulfate, sodium, chloride, magnesium, ph, alkalinity, wtype, mash_ro, sparge_ro, as_hco3) 
VALUES (:name, :display, :deleted, :folder, :notes, :amount, :calcium, :bicarbonate, :sulfate, :sodium, :chloride, :magnesium, :ph, :alkalinity, :wtype, :mash_ro, :sparge_ro, :as_hco3);" :  
"ERROR:  column \"target\" does not exist\nLINE 1: ...forward profile', 0, 75, 0, 150, 10, 50, 5, 7, 0, TARGET, 0,...\n

At a raw guess, it was missed that I don't store strings in the database for the water tables. I store the integers, mostly because I really do not like using strings in the database like that.

[matty0ung]

Ah, hmm, OK, will have a look. There is actually even a comment in the code saying this is stored in the DB as a number!

[matty0ung]

Whilst I'm looking at this, there's a slightly related question I keep meaning to ask. On Water, there is a property called alkalinity, which is either a measure of carbonate (CO3) or of bicarbonate (HCO3) depending on the value of alkalinityAsHCO3. Is the alkalinity value always measured in parts-per-million (like magnesium_ppm, sodium_ppm, etc? Or is it something else? (I was trying to deduce the answer by looking at the calculations in WaterDialog.cpp but it was making my head hurt!)

[mikfire]

Writing that code made my head hurt. A lot.

The alkalinity is measured in ppm. I probably had a reason for breaking the naming convention, but that reason escapes me right now.

[matty0ung]

Writing that code made my head hurt. A lot.

Glad it's not just me! :smiley:

The alkalinity is measured in ppm. I probably had a reason for breaking the naming convention, but that reason escapes me right now.

Thanks. I'm editing the file anyway, so I'll bring the naming into line with the other ppm fields.

[matty0ung]

By the way, this is turning out to be a very good bug to investigate. The fact that you run PostgreSQL picks up all sorts of things that SQLite will just let slide (eg storing a double in an int field etc). I'm adding some extra checks in the ObjectStore code to sanity-check check these things.

Longer-term, I wonder if there might be a better alternative to SQLite for our native DB. It feels a bit wrong to do everything strongly-typed in C++ and then have SQLite say "Yeah, whatever you told me this column is, I'll just ignore that and let you store anything you want in there. Just like JavaScript." :smile_cat:

EDIT: Maybe this https://www.sqlite.org/stricttables.html is what we want in the meantime...

[matty0ung]

Eg https://github.com/Brewtarget/brewtarget/blob/ef14cdc504e37e7b3f2c06ce714c45f506917cbf/src/model/NamedEntityWithInventory.h#L52 should read QPROPERTY( int ....

[matty0ung]

OK, I worked out what's going on here. When you store an enum in a QVariant, that QVariant container is too helpful. It gives you the name of the enum value when you call .toString() and the numeric value of the enum value when you call toInt(). In the database layer, when we ask a Water object for its type property, we get such a two-faced QVariant, and when we give it as a bound value to a QSqlQuery SQL statement, the inner workings of Qt call .toString(), which is not what we want on the water table.

(On other tables, where we are already mapping enum values to the same string representations that we use for BeerXML serialisation, we don't hit this problem.)

The fix is to force the QVariant to be the same type as we've specified in the DB mapping. PR coming soon.

[matty0ung]

Should be fixed in https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.8, but please shout if not!