developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 13 years ago

Closed 12 years ago

#572 closed bug (fixed)

Instructions in/not in dans jDaoConditions

Reported by: yferp Owned by: Julien
Priority: normal Milestone: Jelix 1.0.5
Component: jelix:dao Version: 1.0.3
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Il y a un bug lors de la génération de requêtes utilisant des "in" et "not in" SQL : la requête générée est syntaxiquement incorrecte

Attachments (2)

572-jDaoConditions-in-notin.diff (1.3 KB) - added by Julien 13 years ago.
572-jDaoConditions-in-notin.2.diff (1.2 KB) - added by Julien 12 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 13 years ago by yferp

  • Milestone changed from jTpl 1.0 to Jelix 1.0.4

Changed 13 years ago by Julien

comment:2 Changed 13 years ago by Julien

  • Owner set to Julien

Je crois que ce patch fixe le problème.

code test :

$dao = jDao::get('news');
$conditions = jDao::createConditions();
$conditions->addCondition('id_news','IN',array(1,2));
$news = $dao->findBy($conditions);
echo $news->rowCount();

NB : on peut aussi écrire :

$conditions->addCondition('id_news','IN','1,2');

NB2 : Quand le 3è paramètre est un tableau PHP, chaque élément du tableau est converti en fonction du type du champ dans la base (comme pour les in/notin générés dans les méthodes XML du DAO) :

$conditions->addCondition('id_news','IN',array(1,'toto',2));

'toto' sera converti en 0, comme id_news est de type int dans le DAO

NB3 : au passage, je ne suis pas persuadé que cette conversion soit une bonne chose.

On devrait sans doute plutôt faire quelque chose comme :

foreach($cond['value'] as $value){
    $prepared_value = $this->_prepareValue($value,$prop['datatype']);
    if($value === $prepared_value)
        $values[] = $value;

car au final, une condition "IN (1,0,2)" et "IN (1,2)", ce n'est pas du tout la même chose. Je dois vérifier ce que j'avance dans le cadre des méthodes XML avec IN/NOT IN (pour l'instant j'ai juste lu rapidement pour écrire ce patch), mais je crois bien que le problème se pose.

comment:3 Changed 13 years ago by Julien

  • Status changed from new to assigned

comment:4 Changed 13 years ago by laurentj

  • review set to review?

comment:5 Changed 13 years ago by laurentj

  • Component changed from jelix to jelix:dao

comment:6 Changed 13 years ago by laurentj

  • review changed from review? to review+

r+, mais j'aurais bien aimé avoir des tests unitaires...

comment:7 Changed 13 years ago by Julien

Laurent,

je n'avais pas pas mis review? car je voulais encore vérifier cette histoire de vérification de type (voir fin du comment 2)

dans lib/jelix/dao/jDaoFactoryBase.class.php

...
final protected function _prepareValue($value, $fieldType){
        switch(strtolower($fieldType)){
            case 'int':
            case 'integer':
            case 'autoincrement':
                $value = $value === null ? 'NULL' : intval($value);
                break;
...

va faire que si le développeur demande IN (ou NOT IN) (1,'toto',3) sur un champ de type "int", la condition générée va être "IN (1,0,3)" et non "IN (1,3)", ce qui n'est pas du tout la même chose.

NB : selon moi le problème se pose à plein d'autres endroits, mais c'est sans doute un peu chaud de le fixer pour 1.0.4 niveau timing.

Par exemple :

select * from pages where parent = $var

parent est de type "int", si on passe $var = 'toto', ce n'est pas pareil que $var = 0 (intval('toto') = 0). On devrait effectivement avoir un resultset vide, mais pas le resultset des pages de premier niveau.

bref, l'idée (à développer dans un autre ticket sans doute, puisque ça ne concerne pas que IN/NOT IN), c'est de dire que si on a quelque chose du type :

select * from table where field = $value

et que value n'est pas du type de $field, alors la requête devrait retourner un resultset vide.

dans le cas de in/not in, on peut envisager de simplement ignorer la valeur qui pose problème, en faisant attention à ne pas avoir in IN/NOT IN vide.

Autre solution, plus radicale, émettre des erreurs/warnings si les types ne sont pas appropriés aux champs, et retourner des résultats vides.

Comme dit, je n'ai rien testé en ce sens, mais d'après une lecture rapide du code, j'ai bien l'impression que c'est comme je l'imagine. Ou bien j'ai picolé ?

Je vais quand même faire 2-3 testcases après manger.

comment:8 Changed 13 years ago by Julien

Hello,

bug confirmé, ça agit bien comme je le pensais.

Une condition IN/NOT IN avec une valeur array(1,'toto',2) donne une requête avec IN/NOT IN (1,0,2), ce qui n'est pas bon...

du coup ça doit aussi faire les choses que je décris dans le commentaire précédent... pas cool.

Sinon, j'ai aussi trouvé un autre bug IN/NOT IN spécifique aux méthodes XML DAO... Je vais ouvrir un autre ticket...

Je vais améliorer le patch du présent ticket pour fixer le problème décrits dans les commentaires + hauts, mais il faudra sans doute revoir tout ça + en profondeur :(

Je vais ouvrir des tickets pour tous les bugs que je vois en rapport avec ça...

comment:9 Changed 13 years ago by Julien

  • review review+ deleted

en attendant un meilleur patch, j'enlève r+

Changed 12 years ago by Julien

comment:10 Changed 12 years ago by Julien

  • review set to review?

Bon, là le patch (le même que le précédent, un tout petit peu amélioré) règle le problème évoqué sur la construction syntaxique de la requête.

Il y a d'autres problèmes, mais je vais faire d'autres tickets.

comment:11 Changed 12 years ago by laurentj

  • review changed from review? to review+

C'est ok. Mais il n'y a toujours pas de tests ! :-p

comment:12 Changed 12 years ago by laurentj

  • Resolution set to fixed
  • Status changed from assigned to closed

intégré dans le trunk et la branche 1.0.x

Note: See TracTickets for help on using tickets.