Bugfix for multivalue filters

Hey,

I've been fiddling around with filters a bit and think I've found a bug. In the PntSqlJoinFilter class, a join is made between two tables. The following function finds one of those two table names.

   function getTableName()
        {
                if ($this->tableName)
                        return $this->tableName;

                $clsDes =& PntClassDescriptor::getInstance($this->itemType);
                $idProp =& $this->getIdPropertyDescriptor();
                if ($this->idColumnIsOnAlias) {
                        return $this->tableName = $clsDes->getTableName();
                } else {
                        $prop =& $clsDes->getPropertyDescriptor($this->key);
                        return $this->tableName = $prop->getTableName();
                }
        }

The idColumnIsOnAlias value is used to mirror the behaviour for joining on multivalue properties, so this code looks ok at first sight. Although I do not yet fully understand how this is supposed to work, the above code conflicts with the way idColumnIsOnAlias is set:

    function &getIdPropertyDescriptor()
        {
                $clsDes =& PntClassDescriptor::getInstance($this->itemType);
                $prop =& $clsDes->getPropertyDescriptor($this->key);
                $idProp =& $prop->getIdPropertyDescriptor();
                if (!$idProp) trigger_error("no id property found for ". $prop->getLabel(), E_USER_ERROR);
                
                $this->idColumnIsOnAlias = !$prop->isMultiValue(); // was: $idProp->ownerName == $this->itemType; maar dat is niet ok als recursieve relatie
                return $idProp;
        }

AFAICS, idColumnIsOnAlias can be false only if isMultiValue() is true. If so, $prop is of class PntMultiValuePropertyDescriptor ($prop is the same in both functions). Yet, PntMultiValuePropertyDescriptor does not have a getTableName() method, which is called in the last line of the first function. AFICS, this means this code will always crash on multi value properties. I've "solved" this by removing the last if in the first function and leaving just
                        return $this->tableName = $clsDes->getTableName();
there. How is this supposed to work?

P.S Is there anyway to properly mark the code in this post so it will end up between <pre> tags?



Post Edited (11-10-06 11:29)

Hi Matthijs,

You are right that version 1.2.0 is not doing joins over multi value properties. 1.3.beta1 is supposed to search over multi value properties, and even another step to search over m to n properties. I remember this took some refactoring of the generation of JOIN conditions, including naming the aliases differently in the SQL. I consider the query model as one of the most complex parts of phpPeanuts, i hope you excuse me for not going into any detail right now about this refactoring. More details on 1.3.beta1 are in [url]http://www.phppeanuts.org/examples/changes.txt[/url]. I will finisch the packaging of the beta tomorrow, i guess you can download it before 18:00 CET.

To my regret the forum does not support <pre> tags. It's code is a nigtmare for an OO developer like me, i quess it needs to be replaced entirely, but due to growth of business i currently have very little time left for things like packaging a new version, making the new examples that shoud accompany it, upgrading the website and promoting phpPeanuts. Luckily there is a positive side to this business too: much of it is development in php. The XP priciples of 'once and only once' and 'refactor reletlessly' allways drive part of the functionality i develop into phpPeanuts, so new features are inevitable.

Greetings,

Henk.