Love Fuel?    Donate

FuelPHP Forums

Ask your question about FuelPHP in the appropriate forum, or help others by answering their questions.
Unexpected behavior using find_by_pk and find_by_*
  • I was just trying to use the above mentioned methods on an ORM-Model, but the first method (find_by_pk) dumps an error like
    Fuel\Core\Database_Exception [ Error ]: SQLSTATE[42S22]: Column not
    found: 1054 Unknown column '' in 'where clause' with query
    when used via \Profiles\Model\Field::find_by_pk($pk). (I just found out, that find_by_pk is actually only defined on Model\Crud, but why isn't it on Orm\Model?).
    The second method doesn't return a row when used like \Profiles\Model\Field::find_by_slug($slug). I found, that the primary-key(s) are added to the query, which really surprised me

    Besides that, if I changed find_by_slug($slug) to 'find_by('slug', $slug) I get an error like
    Fuel\Core\PhpErrorException [ Warning ]: array_key_exists() expects parameter 2 to be array, string given
    I did some var_dump/print_r-ing on the Orm\Model::__callStatic() method and found an error for the lastly mentioned problem, however, I'm not sure if I'm just not doing it right.
    Maybe somebody can help me? Since there are definitely some people out there successfully using those methods ;)
  • Okay, I solved some problems myself, but I'm still expericencing the issue with find_by('slug', $slug) and find_by_slug($slug) where the latter method dumps the aformentioned
    Fuel\Core\PhpErrorException [ Warning ]: array_key_exists() expects parameter 2 to be array, string given error.
    Is it an ID10T/PEBKAC-error or an ORM-related error? Any help appreciated!
  • HarroHarro
    Accepted Answer
    Orm\Model and Model_Crud are from two different authors. We've been busy over the last releases to unify the API, but obviously there are still differences. Never knew "find_by_pk" existed...

    I'm pretty sure OrmModel doesn't support find_by() as well, only find_by_fieldname().
  • I guess you're pretty right. The code of Orm\Model doesn't seem to show a 'find_by' option. However, if you're trying to unify the APIs, how far is your progress? I wouldn't mind to work on this topic and to make the 'find_by*' APIs unified (as well as add 'min_*' and 'max_*' to Orm\Model, which are mentioned in a comment at the end of __callStatic ;) )
  • You can't really make them 100% compatible without breaking stuff, and we don't break BC in minor releases.

    Model_Crud's methods take different arguments. In Orm, find_by defaults to 'first', returning only one result, in Model_Crud to 'all', returning a collection. And there are probably more significant differences.

    So there's a limit to what can be unified. I don't really know why Model_Crud had to exist (I was not in favour of it), as it doesn't really have a lot of benefit compared to ORM...
  • By now, after extensively using the ORM, I don't wanna miss its features any more. But before that, I, too, would have said "You need a Model_Crud". Everybody's got their favorites ;)

    Anyway, I'm still stuck with find_by_*, in particular in my case: find_by_slug_or_major_id($slug-from-uri, $slug-from-uri). This does not return the expected result as the query I get is a simple find('first'):

    SELECT `t0`.`name` AS `t0_c0`, `t0`.`slug` AS `t0_c1`, `t0`.`created_at`
    AS `t0_c2`, `t0`.`updated_at` AS `t0_c3`, `t0`.`major_id` AS `t0_c4`
    FROM `majors` AS `t0` ORDER BY `t0`.`major_id` ASC LIMIT 1

    Looking at the source code of Orm\Model::__callStatic I see an option called 'or_where' created and passed to Orm\Model::find('first', $options). However, in Orm\Query::__construct() nothing is taking care of the 'or_where' option. Dirty hacking a

    case 'or_where':
    $this->_parse_where_array($val, '', true);

    into Orm\Query::__construct() didn't really solve the problem at all (there is still now WHERE ( `slug` = 'slug-from-uri' OR `major_id` = 'slug-from-uri') in the generated query.)

    Now, if you take a look at the first query posted, you will see that there is absolutely no where on either 'slug' or 'major_id'. If I change the method call to 'find_by_slug_and_major_id' (just for illustration), I get the following query

    SELECT `t0`.`name` AS `t0_c0`, `t0`.`slug` AS `t0_c1`, `t0`.`created_at`
    AS `t0_c2`, `t0`.`updated_at` AS `t0_c3`, `t0`.`major_id` AS `t0_c4`
    FROM `majors` AS `t0` WHERE (`t0`.`slug` = 'slug_from_uri') AND
    (`t0`.`major_id` = 'slug-from-uri') ORDER BY `t0`.`major_id` ASC LIMIT 1

    So this works, at least. Yet, if I'm not mistaken, then this looks weird anyway. Shouldn't the WHERE * ANDs be inside brackets?

    Adding the ''case: 'or_where' '' mentioned above to Orm\Query::__construct() at least generates an imho properly bracketed query:

    SELECT `t0`.`name` AS `t0_c0`, `t0`.`slug` AS `t0_c1`, `t0`.`created_at`
    AS `t0_c2`, `t0`.`updated_at` AS `t0_c3`, `t0`.`major_id` AS `t0_c4`
    FROM `majors` AS `t0` WHERE ((`t0`.`slug` = 'slug-from-uri') AND
    (`t0`.`major_id` = 'slug-from-uri')) ORDER BY `t0`.`major_id` ASC LIMIT 1

    But I'm anyway not getting the results I want. Am I working to illiberally, or is there are bug somewhere in the ORM package?
  • I would not call it a bug.

    Model_Crud and ORM are made by two different people. In case of Model_Crud, a find_by simply returns all results, in ORM a find_by always implies a get_one().

    It's debatable whether or not that is a good thing (I don't know the reasoning behind it), but since we can't change it without breaking backward compatibility big-time, it's not really relevant at the moment. We can only think about this for 2.0.

    As to the Query object, ideally it's constructor should simply support all methods it has, so I would consider that a bug. I don't immediately see why these where clauses are in brackets, it's not needed. That needs to be looked at too.
  • Looked into it a bit further.

    I think not all methods defined in the Query class are available as options in the constructor, because they don't really make sense.

    The or_where is a perfect example, a combination of where clauses with AND or OR (and possibly brackets) only work if it is possible to specify the correct order in which they are processed. Which is not a problem when you chain methods, but impossible to do when you process an array like it is done in the Query constructor.

    Your dirty hack fixes it for your particular use case, but it will leave users with the impression that you can now use 'or_where' in the options array, which is only true up to a point.

    And even then, the constructor loops over the options in the order in which they are defined.

    _parse_where_array() supports nested arrays, so perhaps it would be better to inject:

    'or' => array(...)

    into the where array and have _parse_where_array() deal with it.
  • p.s. You can use "find_all_by_*" to run an "all" query, instead of a "first" query...
  • SELECT `t0`.`name` AS `t0_c0`, `t0`.`slug` AS `t0_c1`, `t0`.`created_at`
    AS `t0_c2`, `t0`.`updated_at` AS `t0_c3`, `t0`.`major_id` AS `t0_c4`
    FROM `majors` AS `t0` WHERE ((`t0`.`slug` = 'slug-from-uri') AND
    (`t0`.`major_id` = 'slug-from-uri')) ORDER BY `t0`.`major_id` ASC LIMIT 1
    This still adds an AND in between the two WHERE clauses, which is why you don't get the results you want.

    I checked a git blame, and it looks like the person initially responsible for Model_Crud (and not particularly an ORM lover) added the "find_by" functionality to the Model, and from the looks of it didn't check at all if the query object supported it. So this has never worked since it was introduces early 2011... :-(
  • HarroHarro
    Accepted Answer
    Think i fixed it. A query like this:

    \Model_Test::find_by_id_and_username_or_email_and_group_id(10, 'test', 'test', 999);
    Now comes out like this:
    SELECT `t0`.`username` AS `t0_c0`, `t0`.`email` AS `t0_c1`, 
    `t0`.`group_id` AS `t0_c2`, `t0`.`id` AS `t0_c3`
    FROM `users` AS `t0` WHERE `t0`.`id` = 10 AND `t0`.`group_id` =
    999 AND (`t0`.`username` = 'test' OR `t0`.`email` = 'test') ORDER BY
    `t0`.`id` ASC LIMIT 1
  • That's awesome. I just checked out orm/1.7/develop and running find_by_slug_or_id works just the way I wanted it to work :)

    But Sorry to bug you again, but somehow your update made some part of the code work but broke others i.e., running find_by_slug('slug-from-uri') now runs the following query:
    SELECT `t0`.`name` AS `t0_c0`, `t0`.`slug` AS `t0_c1`, `t0`.`created_at` AS `t0_c2`,
    `t0`.`updated_at` AS `t0_c3`, `t0`.`major_id` AS `t0_c4`
    FROM `majors` AS `t0`
    WHERE `t0`.`slug` = 'slug-from-uri'
    AND ()
    ORDER BY `t0`.`major_id` ASC LIMIT 1
    which returns a MySQL 1064 error (as there is the AND() inside).
  • Hmm... I'll have a look.
  • Fixed!
  • Yap. Perfect. Awesome! Thanks a lot for your patience ;)

    I owe you at least one soft drink or alcoholic beverage. Whichever you prefer ;)
  • I'm not often in your neck of the woods, but I keep it in mind... :-)

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

In this Discussion