Issue: Hybrid properties and insert_many
Author: qacollective
State: closed
Created: 2020-09-01 13:25:09

Hello,

Peewee **rocks**.  I use it everywhere I go!

Really hoping this isn't a facepalm moment for me but here goes ...

I have a model with a hybrid property:

```
class Order(BaseModel):
    id = IntegerField(primary_key = True)
    _time_setup_msc = DateTimeField()

    @hybrid.hybrid_property
    def time_setup_msc(self):
        return self._time_setup_msc

    @time_setup_msc.setter
    def set_time_setup_msc(self, time_setup_msc):
        self._time_setup_msc = datetime.utcfromtimestamp(time_setup_msc/1000)
```

With data stored in a list of dicts in `orders`, I then attempt to do:

```
Order.insert_many(orders).on_conflict(conflict_target=[Order.id], update=Order._meta.fields).execute()
```

Peewee returns an error from the postgres database:
```
peewee.IntegrityError: null value in column "_time_setup_msc" violates not-null constraint
```
So it appears that `insert_many` is not invoking the hybrid property setter to generate each insertion?  I was kinda expecting this to be done for me in this situation, but perhaps its up to me to use `dict_to_model`?  Will that properly invoke the setter method?  

Perhaps I've missed or stuffed something?

:worried: 

Can you produce a test-case that illustrates what you believe to be the problem? Without seeing what the order dicts look like it's hard to say.

I'm a little confused as to how to reproduce this bug or what exactly your intent is. The whole reason to use a hybrid property is to express a computation (or scalar function) that would be useful both on a model instance and when querying against a model class. A setter is only assignable on an instance, not on a class. So what you really want is just a regular property, if I understand correctly. A reason this seems to work on `create()` but not `insert_many()` (I'm inferring this is why you're confused) is because when a model is instantiated the keyword args are applied to the instance, which has the effect (in your example) of setting the value.

Since you are storing your timestamp in the database as a date-time field, then it seems to me that you are going through all this trouble just to set the field to a datetime during creation. This isn't at all the purpose of hybrid properties. See the [documentation](http://docs.peewee-orm.com/en/latest/peewee/playhouse.html#hybrid-attributes) for examples.

Apologies, I posted this and went to bed, which is why I didn't respond until many hours later.  Clearly you run a very tight ship here! :blush: 

> So what you really want is just a regular property, if I understand correctly.

Yes, I think you have understood correctly and I may have misunderstood the purpose of a **hybrid** property needing to be useful both on a model instance and class.

> A reason this seems to work on `create()` but not `insert_many()` (I'm inferring this is why you're confused) is because when a model is instantiated the keyword args are applied to the instance, which has the effect (in your example) of setting the value.

Correct again.  So if I define a regular property, will `insert_many()` invoke it when attempting to insert each row?

My intent here is hopefully clear by now: to house simple conversion logic in the peewee model, as it is clearly the best place to implement an abstraction layer between code using millisecond epoch integers and a database requiring timestamps.  Obviously this is possible with model by model instantiation, but I'm wondering if the **bulk** operations in peewee would see it work the same way?

The reasoning behind this is that my data processing code doesn't deal with the specifics of any model - its purpose is largely to receive data from one source and put it in the database - so it is written generically to handle multiple arbitrary source/model pairs.  I planned to put the small conversion logic needed between source and database format in the peewee models as properties.  I receive data in blocks of 100,000 rows so figured it was most efficient to use `insert_many` under `with db.atomic():`.  Now I realise that `insert_many` won't invoke the property logic?

Should it?

In short, I am trying to combine the convenience of 'upsert' type functionality with properties and the speed improvements suggested in the documentation at https://docs.peewee-orm.com/en/3.4.0/peewee/querying.html#bulk-inserts ... perhaps that's not possible?

It seems like the most important thing you want to achieve is simply converting a ms timestamp to a datetime. This can be done by:

1. subclassing a field and implementing a `db_value()` method that converts the timestamp to a datetime
2. pre-processing your data

Since it's specific to your data-source, it seems to me that you're better of pre-processing these timestamps - but that's up to you. See how `TimestampField` is implemented if you're curious.

Of course you are right in this scenario, however the example I gave was just a small one.  

It reality, the task is far greater - for example a lot of the input data needs simple changes (like this one) or code lookups.  

I can appreciate your thinking "why don't you simply pre-process your data?!", but for me it is about design elegance and common sense co-location of code: if I already have to enumerate a data model for Peewee, the most sensible place to perform minor field specific tranformations is in that same space - as opposed to having a code block someplace else that picks through fields and does transformations before data gets to peewee.

In my view of things, if peewee related code can do 'all things data', code in my other files can simply focus on using peewee at the model level, without bloating over field manipulations.

Just my view.

Thanks for your help!  I may end up subclassing fields as you say, and detecting the field name and selectively transforming it based on model name and field name outsourced to a YAML config file or similar ... sounds more complicated / less elegant than what I was originally thinking but ...  

Tightly coupling my bulk-loading code (a 1-time operation) with my model code is exactly the kind of thing I would avoid. I'd write a function to process the data.

Oh I see, you see your bulk loading code as a 1 time operation, which is likely true for a lot of cases.  In my case, its happening every couple of minutes...which is why my original code had an `on_conflict` part.  So for me that processing has to happen somewhere, it's just a matter of where. 

I see your point of view-I may just end up writing a simple configurable transformation engine then.

It's a write-once operation.