berlindb / core

All of the required core code
MIT License
253 stars 27 forks source link

Add support for automated data typing #13

Open alexstandiford opened 4 years ago

alexstandiford commented 4 years ago

Currently, Berlin does not automatically unserialize/serialize array data when it goes in, or comes out of the database. This surprised me as this is a common scenario.

In fact, it doesn't seem to do any typing when the item is shaped at all.

Is this a design decision that I'm missing, or something?

alexstandiford commented 4 years ago

Currently working around this by manually typing these values after the constructor method runs in my row classes, like so:

/**
 * @property string|array  array_column
 * @property int           int_column
 * @property float         float_column
 * @property string        string_column
 * @property bool          boolean_column
 */
class Ad extends Row {
    private $behavior = false;
    public function __construct( $item = null ) {
        parent::__construct( $item );
        $this->array_column     = maybe_unserialize( $this->array_column );
        $this->int_column       = (int) $this->int_column;
        $this->string_column    = (int) $this->string_column;
        $this->float_column     = (float) $this->float_column;
        $this->boolean_column   = (bool) $this->boolean_column;
    }
...
}
JJJ commented 4 years ago

This was a design decision, at least in the Query class and Shape methods.

This should be up to the Row object to decide what the values look like when they are retrieved from the database and ultimately converted into PHP objects.

We could use the Column definitions to infer what a default type casting could be.

Un/serializing wasn’t a requirement, so it never got built. Performing these actions automatically is largely considered a security concern, as objects can be unexpectedly awakened and executed.

I think one of the luxuries of defining your own database table schema is no longer needing to store serialized arrays, as you can now invent a better and more suitable one with relative ease.

alexstandiford commented 4 years ago

Alright. Honestly, after doing it this way a few times, I have come to appreciate it. I've had situations where the variable needed more manipulation than an array could easily express, and typing the items really isn't that big of a deal. I just initially expected this to be in the schema, not the row.

JJJ commented 4 years ago

I just initially expected this to be in the schema, not the row.

I can see why you’d think that. The original intention of the Schema and Column attributes, was mostly for the database write side. Everything for shaping items on the way out came later. I’m super open to finding a more elegant/obvious way to define that ahead of time.

alexstandiford commented 3 years ago

I think creating an instance of Row and setting the type as described above makes sense in many cases, but I can see simpler tables that would benefit from simply setting a type in the schema, that would automatically get set using settype or something when the row is instantiated.

I'm seeing more and more opportunities to specify how a table behaves using the Schema class (see https://github.com/berlindb/core/issues/26) and I think this could benefit from it all the same.

I don't think this is an either/or thing, either. The Schema could set the initial type from the database, and then the Row could override that using the method above if-necessary. This allows us to set a basic type in Schema and then do something a little more advanced in Row. Between these two things, we would have everything we need to set up our rows.

ashleyfae commented 3 years ago

I'm a fan of how Laravel does this: https://laravel.com/docs/8.x/eloquent-mutators#attribute-casting