Issue: feature request: Introspector.make_column_name should have different logic for foreign key fields
Author: cognifloyd
State: closed
Created: 2018-11-30 20:36:34

I'm using peewee to access another application's database. To avoid writing the models for the db, I'm using `playhouse.reflection.Introspector.generate_models()`. In this database, some of the tables use the suffix `_id` on the primary key column. Then, other tables that have a foreign key reference to that table use the same column name (with the `_id` suffix) to reference the other table.

As is, peewee is stripping the `_id` suffix whether the column has a foreign key constraint or not. I would like the ability to tell the `Introspector` to only strip the `_id` suffix on foreign key fields.

Here's a fun example:

A database named `dr_seuss.db` has several tables such as
- a table `things` with primary key `thing_id`, and
- a table `cats_things_map` with a column `thing_id`
`cats_things_map.thing_id` has a foreign key constraint on `things.thing_id`.

(there would of course be two entries in a `cats_things_map` where ["The Cat in the Hat"](https://en.wikipedia.org/wiki/The_Cat_in_the_Hat) would have a one to many relation with "Thing One" and "Thing Two")

In this case, I would like the `playhouse.reflection.Introspector` to give me models like this:
- `cats_things_map` should have two fields `thing` and `thing_id`, where `thing` is the orm injected object from the `things` model, and `thing_id` is the actual id of that object. _peewee already does this_
- `things` should have a `thing_id` field (NOT `thing`) that maps to the `thing_id` column contents (the primary key). _peewee does not currently let me skip stripping `_id` on columns that are not foreign-key-constrained_

Is there a way that either `make_column_name` can differentiate columns with foreign key constraints?
https://github.com/coleifer/peewee/blob/d31e7bf972e69e227c10694fac71b966959e715f/playhouse/reflection.py#L485-L492
It looks like foreign keys should be available in `foreign_keys[table]` where this is used in `Introspector.introspect()` here:
https://github.com/coleifer/peewee/blob/d31e7bf972e69e227c10694fac71b966959e715f/playhouse/reflection.py#L545-L549
And I use `Introspect.introspect()` indirectly via `Introspect.generate_models()` here. So I would need a flag called something like `literal_column_names_unless_foreign_key` (an awful name - but similar to `literal_column_names`) that gets passed from `generate_models()` to `introspect()`:
https://github.com/coleifer/peewee/blob/d31e7bf972e69e227c10694fac71b966959e715f/playhouse/reflection.py#L615-L619

I thought about using `literal_column_names` but then the foreign key fields end up as `thing_id` and `thing_id_id` which is also not as clean an interface for me.

context: I'm actually doing config management on that application and the config has to be in the db.
I avoid dipping into another application's database as much as possible, but peewee makes it easier to dip in when no other mechanism exists in the app to access or manage the info I need.
The Introspector lets me gloss over most of the schema changes (such as adding a column in version X.Y of the app) and just access the tables and columns I need to. Introspector saves me the maintenance overhead of writing models for multiple versions of the schema. Thanks so much for the Introspector as it allows me to have a much looser coupling on that other app's database.

In some databases people don't use actual foreign-key constraints, especially SQLite and MySQL. Additionally, introspecting a column named "foo_id", regardless of whether or not its a foreign-key, I think it's a reasonable assumption that we assume the Pythonic name should be "foo".

That being said, there's also the case of an externally-managed ID. Say you're caching some response data from a 3rd-party API and want to store a user_id that has no reference to anything else in your DB. In that case it seems reasonable to retain the "user_id" column name and that calling it "user" would actually be somewhat misleading.

After giving it some thought, I think I agree with you that this logic should only apply in the case of foreign-keys.

Merged a patch that is similar to #1800 but with the following differences:

* is_foreign_key=False by default (yours has =True)
* has tests 
* not O(n) on each column (!!)