Primary Column Can't Be String

[spencerfinnell]

When setting a primary column if the type of column is not an int, tinyint, etc, queries will not work. get_item_ids() always uses wp_parse_id_list which forces an integer value. This will destroy non-integer results.

Use case: Stripe object IDs are strings and it would be ideal to use those as the primary column when tracking the objects internally.

[spencerfinnell]

This also happens again in set_items() and shape_item_id().

[JJJ]

Most of Berlin is designed with the idea that the primary column is always numeric and auto-incrementing.

Stripe object IDs are strings and it would be ideal to use those as the primary column when tracking the objects internally.

Why is that ideal? I think I would still want/suggest a traditional numeric auto-incrementing id column as the primary key, and then add the Stripe object ID as the next indexed string column. What do you think?

Using auto-incrementing IDs in SQL brings some benefits:

• Retrieves data faster/easier because each primary key is a simple, sequential integer (rather than a complex mix of other string values).
• Avoid breaking stuff if other values change, as would happen if your primary key was a compound of other column values.
• Ensure the uniqueness of each key without locking or race conditions, as auto-incremented IDs automatically avoid repetition and duplication.
• Improve system performance by being a compact data type, allowing for optimized code paths that avoid skipping between columns to create keys.
• Relationship tables (like meta data, etc...) make it easier to identify row connections with an int than a complex string.

An obvious point against numeric/autoincrement is what happened at Mt Gox back in the day.

[spencerfinnell]

@JJJ That all seems reasonable. I guess my main thinking was that I will always know the Stripe object ID so I can use it immediately on update and delete calls without first finding the relevant row via the Stripe object ID so I can then use auto incremented primary key.

i.e

$coupon = get_coupon_by( 'object_id', $object_id );
$db->delete_item( $coupon->id );

vs.

$db->delete_item( $object_id );

Is there enough caching in play where that doesn't matter? Or are the performance implications of a string based primary key worse?

[alexstandiford]

You can also set the stripe ID as a key in your table schema, so you'd get similar speed benefits if it were the primary key.

If you want to be able to delete a record using the Stripe object ID, I would consider extending the delete method to support the stripe ID, or create a a different method specifically for that.

I know it seems wasteful to have to fetch the record from the DB first, but that happens inside Berlin regardless, and it's caching mechanism is smart enough to only query once. It shouldn't have any significant impact on performance, but you still get the benefits mentioned by @JJJ

[ashleyfae]

You could just use wpdb directly to delete in that instance. Their delete method supports a where clause.

$wpdb->delete( $table_name, array( 'object_id' => $object_id ), array( '%s' ) );

Translates into:

DELETE FROM my_table WHERE object_id = 'sub_123';

[alexstandiford]

What @ashleyfae is suggesting might be better, but it doesn't clear the item's cache. Make sure you take a look at the delete item method to see how it handles updating the cache for the record otherwise you'll end up with unexpected results should you make another query for that record in the same request.

[spencerfinnell]

Thanks for the tips/info @ashleyfae and @alexstandiford. As with most things, after rethinking things a bit, I think I can get by with a "normal" primary key without issue. It would be nice to document this expected behavior or throw an error when things are misconfigured, similar to some of the object properties. I had to dig down a bit to see where/why things were failing.

[JJJ]

It would be nice to document this expected behavior or throw an error when things are misconfigured

Noted. Great feedback. 👍

[JJJ]

PR #140 merged a few enhancements for 2.1.0:

• Primary column no longer forced to be an int
• Primary column of ID no longer forced to id (yay wp_users:ID)
• Column names no longer forced to be lowercase (yay wp_comments:comment_author_IP)
• Table names no longer forced to be lowercase (eh, fine)

See #130 for discussion and work towards improving delete_item() 👍