Issue: peewee internal error on updating class with generated columns
Author: ghost
State: closed
Created: 2019-08-09 17:26:45

Dear colleagues,
I have an issue with the updating class with generated column with MariaDB 10.4 and peewee 3.10.0

This is SQL definition:
```
CREATE TABLE IF NOT EXISTS `mw`.`project` (
  `uuid` VARCHAR(36) NULL,
  `qid` VARCHAR(36) GENERATED ALWAYS AS (uuid),
  `utime` DATETIME NULL,
[ ... ]
```
this is how pwiz generates model:
```
class Project(BaseModel):
    qid = CharField(null=True)
    utime = DateTimeField(null=True)
    uuid = CharField(null=True, unique=True)
[ ... ]
```

this is code which generate error (please, note that I don't modify 'qid') :
```
pj = Project.get(Project.id == 25)
pj.utime = datetime.now()
pj.save()
```

error is:
```
Traceback (most recent call last):
  File "/project/venv/lib/python3.7/site-packages/peewee.py", line 2981, in execute_sql
    cursor.execute(sql, params or ())
  File "/project/venv/lib/python3.7/site-packages/pymysql/cursors.py", line 170, in execute
    result = self._query(query)
  File "/project/venv/lib/python3.7/site-packages/pymysql/cursors.py", line 328, in _query
    conn.query(q)
  File "/project/venv/lib/python3.7/site-packages/pymysql/connections.py", line 517, in query
    self._affected_rows = self._read_query_result(unbuffered=unbuffered)
  File "/project/venv/lib/python3.7/site-packages/pymysql/connections.py", line 732, in _read_query_result
    result.read()
  File "/project/venv/lib/python3.7/site-packages/pymysql/connections.py", line 1075, in read
    first_packet = self.connection._read_packet()
  File "/project/venv/lib/python3.7/site-packages/pymysql/connections.py", line 684, in _read_packet
    packet.check_error()
  File "/project/venv/lib/python3.7/site-packages/pymysql/protocol.py", line 220, in check_error
    err.raise_mysql_exception(self._data)
  File "/project/venv/lib/python3.7/site-packages/pymysql/err.py", line 109, in raise_mysql_exception
    raise errorclass(errno, errval)
pymysql.err.InternalError: (1906, "The value specified for generated column 'qid' in table 'project' has been ignored")
```
Are there ways to declare 'qid' in peewee model as generated and never update it?

Thank you.

> Are there ways to declare 'qid' in peewee model as generated and never update it?

You're best off using `INSERT` or `UPDATE` directly in that case, so you can be more specific about which columns are being written to.

Peewee typically handles server-side defaults just fine, as typically they are provided by the `DEFAULT` SQL clause, so NULLs are handled correctly. Apparently this particular example is a bit different.

Another option is to specify a list of fields to update when calling `save()`, e.g. `.save(only=[Project.utime])`.

Hi Charles, thanks for the suggestion. An additional note on this - this issue exists on Mariadb 10.2.4+ and MySQL 5.7+ (when teams changed  default for strict_mode). Rationale behind this can be found here - https://blog.sqlyog.com/beware-virtual-columns-may-render-backups-unusable/ at 4th comment.

I showed an example - of course, in the example I can use INSERT/UPDATE directly, but you've invented Peewee to make life easier :-) and in real life I update lot of fields of an instance depending on lot of conditions and even not always know what exactly was updated, and, e.g. I just save the instance if "dirty_fields" is.

Of course, it's the way to use smth like .save(except=[Project.qid]) (is it possible BTW?) but in fact it's the way to errors (if it was forgotten, for example) and, IMHO, it much more easier and safer is to define attribute 'computed' (well, 'readonly') and never include it in modify operations over DB (regardless of DB type). From the link above - "This is a new feature, and with every new feature that affects DDL statements, the tools must evolve to deal with that. It has happened for triggers, stored routines, events, partitioning , and it must happen for virtual columns."

Thank you.

It seems problematic and difficult to try and determine how to handle something like "read-only" fields. Calls to save() delegate to `.insert()` or `.update()`, so we would need to exclude read-only fields there. But should one prevent explicitly attempting to store data via calls directly to `.insert()` / `.update()` / `.insert_many()`? Validation would possibly need to be added in several places. Additionally, it's not clear how setting "read-only" would affect DDL ... do we prevent creating tables with read-only fields? Do we extend the field constructor to require specification of the expression/generator?

As long as you're not modifying a field's value, presumably it wouldn't be included in the list of dirty fields, so that should allow you to avoid this issue when using `.save()`.

I'd suggest that you look at wrapping things in helper methods that do exactly what you want, and then just calling those. Trying to handle this correctly seems a bit fraught.

Just look at this list of gotcha's a mile long. Fuck no:

https://mariadb.com/kb/en/library/documentation/sql-statements-structure/sql-statements/data-definition/create/generated-columns/

:-D Well, thank you!

Hello, Charles, I fixed the problem by disabling strict_mode in DB, so it's not a problem for me anymore. But just one more comment to the statement "Peewee typically handles server-side defaults just fine". With new defaults it doesn't, actually.

There are two types of DB updates:
- .update(), .insert() and others with explicit fields specification, where writer exactly know what he is doing and fully responsible for this. Issues with these operation can be easily tracked and fixed.
- kinds of .save() with implicit fields specification, where Peewee takes responsibility of what exactly will be saved to DB.

I think, it will be reasonable compromise to take care only about "implicit" calls like .save() where writer can't influence selection of fields to be saved. In this case field attribute like 'readonly' will allow to specify fields which is prohibited to update in DB.

Thank you.

> I think, it will be reasonable compromise to take care only about "implicit" calls like .save() where writer can't influence selection of fields to be saved. In this case field attribute like 'readonly' will allow to specify fields which is prohibited to update in DB.

I agree that this is probably the place to put such logic.

Might be time to add it, because I recall someone asking for a readonly-type feature a while ago.

For the issue of DDL, I think this will need to be handled by the user.

I pushed a branch... I'm not in love with the way it's shaping out, but it provides a starting place.

Will close this issue for now and put this on the back-burner to think about ways to integrate this stuff.

Thanks, Charles! You're doing a great job. Thank you again.