jonathangeiger / kohana-jelly

See the link below for the most up-to-date code
https://github.com/creatoro/jelly
MIT License
146 stars 34 forks source link

Feature Request: Calculated/Function fields #118

Closed mintbridge closed 14 years ago

mintbridge commented 14 years ago

There is already the option to set a column to not be in the database, can we extend it so that a function can be in the select to populate the field value for example if you wanted to CONCAT() two other fields so CONCAT(first_name, ' ', last_name) AS full_name

I have a working version that only required a couple of small changes, setting a "expr" key the field array 'full_name' => new Field_String(array( 'in_db' => FALSE, 'expr' => DB::expr("CONCAT(first_name, ' ', last_name)"), )),

and then a small change to line 276 of jelly / builder / core.php

elseif(!empty($field->expr))
{
     $add_columns[] = array($field->expr, $field->name);
}

It would mean that the developer would have to know that the columns in the expression were correct, but i think its a neat way of getting calculated fields without having to create a model function to do it

jonathangeiger commented 14 years ago

I think this might be better suited to simply writing or overriding a field class that does this in an abstracted way. I know it's a bit more code, but it seems like too specific of a use case to code in for all fields.

Any other opinions?

banks commented 14 years ago

If this can be done currently by creating a custom field then I don't think there is much argument for core support. Surely you can define a field where the column is a database expression?

mintbridge - can you confirm that this technique works?

banks commented 14 years ago

I've actually come up against this issue directly in trying to implement polymorphism.

I thought it would have been possible to just set the column property to be a database expression object and have this work. This doesn't work because Jelly stores it's reverse column lookup in the meta object as $column => array('fields',...) and an object can't be the key of an array in PHP.

Jon, is the reverse column lookup used anymore? I can't see where if it is? If it isn't used, mintbridge's suggestion could be simplified to this code in builder line 276 (in select_array())

elseif($field->column instanceof Database_Expression)
{
    $add_columns[] = array($field->column, $field->name);
}

And no extra property would need to be added. This seems like a relatively neat solution to allow some useful if not-that-common cases.

It would really simplify the handling of polymorphic model type fields since you can ensure that however type is encoded in the database row, you can return it from the query as a full model type string ready to be passed to factory().

The PHP alternative would involve having mechanisms to specify which fields need to be returned (possibly they aren't fields you need otherwise in the model) and then passing the whole row to an abstracted function in the meta object on every iteration to determine the class to instantiate.

What do you think Jon? if reverse column look up isn't used, I'd say this is a good addition.

jonathangeiger commented 14 years ago

Go for it. Reverse column lookup isn't used anywhere anymore.

jonathangeiger commented 14 years ago

Closed by 4b0b06a6c0c9db9566e3d66f8558283d7e46eddc. Thanks mintbridge and banks!

banks commented 14 years ago

Great stuff. Thanks Jon

mintbridge commented 14 years ago

Awesome, thats a much neater way of doing it :) Thanks