PHP Notices when table has no primary key

[TheSavior]

If you have a table with no primary keys and run a query on it, you get a few notices saying:

Notice: Undefined index: ENTITY_NAME in ...Spot/Entity/Manager.php on line 204

[vlucas]

You seriously have tables with NO primary key? :P

Now I'm wondering if this should be a feature of Spot.. supporting tables without PKs. Because as of now, Spot assumes it does in many places, like when using $mapper->find(1). Should that throw an exception when there is no PK? Should Entities without PKs even be allowed?

[TheSavior]

I'm totally fine with Spot not allowing tables without primary keys. There is no reason to not have a primary key (if it is missing, it is usually by accident). A notice though is totally not okay, the question is when SHOULD the system notify? I definitely think it should be an exception, but ideally it should be thrown if there is an entity without a key, regardless of whether Spot is trying to use that primary key. This would inform the developer immediately during testing to fix it.

I can't imagine a valid way to make that work though, so perhaps the primaryKey method should just throw the exception and we should be okay with that.

[vlucas]

I can throw an exception from Spot\Entity\Manager when it reads the Entity definitions, but that's always going to be on a query of some sort, because the Entities are only loaded when needed. So the trick will be to just check for the 'primary' designation on any field, and if one is not found an Exception will be thrown.

[pnomolos]

I'm going to add an additional thought - what about tables with a composite primary key (such as a has many through join table, where the PK could conceivably be the two foreign keys)? I don't know if this is something that should be added or not, though.

[TheSavior]

@pnomolos https://github.com/actridge/Spot/issues/4

One of the issues that didn't get transferred from the old repo. Supporting composite keys would be quite nice.

[brandonlamb]

Were there any initial thoughts/ideas on handling this yet? I unfortunately have some legacy tables with composite keys and need to add this support.

My first rough thought was setting the pk for an entity to either string or an array, so while looping through the fields if multiple fields define primary = true, it would make an array out of these. Or it would do some kind of implode(',', $pks) and have a parsable string, so in the mapper we would have something like if strpos(',', $pk)

Thats about as far into the thinking as I got. Another would be maybe some helper method like if isset($entity->primaryKeys()) or something. Sorry I know this is kind of babbling.

[vlucas]

@brandonlamb That's actually exactly how indexes and unique keys work. If you use a name after 'index' (instead of boolean true) it will use that name. If multiple fields have the same index name, it will make a compound index out of them in the auto-migrations. I use this all the time. I have no problem with primary keys working the same way.

As for changes, another thing to consider is the get method. It currently takes only one argument (or none for an empty object), so that would have to be altered as well as everywhere primaryKey is used to set the update/delete conditions for an entity.

[brandonlamb]

Ahh I see the checks for the index are in the mysql adapter which I have not been using. I will look into this and see what needs to be pulled over into other adapter(s). In mapper, the save() calls adapter->update() and passes a where of pk => value, I was figuring that part needed to be changed.