search

python-gino / gino

GINO Is Not ORM - a Python asyncio ORM on SQLAlchemy core.
https://python-gino.org/
Other
2.67k stars 150 forks source link

Add extra_returning_fields in apply #767

Open kigawas opened 3 years ago

kigawas commented 3 years ago

Closes #730

xnuinside commented 3 years ago

@kigawas, hi! Good PR & additional feature.

@wwwjfy, @fantix, what do you think?

wwwjfy commented 3 years ago

Sorry for the late.

I'm thinking of a parameter to return/reload every column instead of specified ones only. It saves a get call, which is probably being used in such case now.

Another nitpicky point is the usage of tuple in this PR. As I understand, list is more suitable here, semantically. Quoting Python doc: Tuples are immutable sequences, typically used to store collections of heterogeneous data

kigawas commented 3 years ago

Yes. Tuple should be replaced by Sequence in Python, which sugguests an immutable sequence although you can still pass list.

I'm thinking of a parameter to return/reload every column instead of specified ones only. It saves a get call, which is probably being used in such case now.

It's certainly better to get all columns by default, but for now a transitional approach would be more realistic to avoid API change

wwwjfy commented 3 years ago

I don't mean that big a change, but just an additional parameter like update_all (need a good name) to return all columns and update the instance. Similar to the one in the PR, but no need to list every single one.