Ticket #1170 (closed defect: wontfix)

Opened 12 months ago

Last modified 12 months ago

Pager count query doesn't trigger dql callbacks

Reported by: francois Owned by: guilhermeblanco
Priority: major Milestone: 0.11.0
Component: Pager Version: 0.11.0
Severity: Keywords:
Cc: Has Test: no
Status: Pending Core Response Has Patch: no

Description

Take a table (or a template) which initializes dql callbacks in the setTableDefinition() method. This is the case of Doctrine_Template_SoftDelete for instance.

    /**
     * Set table definition for SoftDelete behavior
     *
     * @return void
     */
    public function setTableDefinition()
    {
        $this->hasColumn($this->_options['name'], $this->_options['type'], $this->_options['length'], $this->_options['options']);

        $this->addListener(new Doctrine_Template_Listener_SoftDelete($this->_options));
    }

Now create a pager on that table, with whatever query you want. The pager will issue two database queries: one for the count, and one for the page. The problem is that the count query doesn't execute the dql callbacks (e.g. preDqlSelect()) registered on the table. This results in wrong counts on pagers on tables with DQL behaviors.

The reason seems to be that the count query doesn't execute the setTableDefinition(). If I write

 Doctrine_Manager::getInstance()->setAttribute('use_dql_callbacks', true);

in a place where I'm sure it is executed before the pager, the count query problem disappears. Another workaround is to call Doctrine::getTable('MyTable'); before executing the pager (this one will execute setTableDefinition()).

Hope the problem is clear...

Change History

Changed 12 months ago by guilhermeblanco

  • priority changed from minor to major
  • status changed from new to assigned
  • milestone changed from 0.11.3 to 0.11.1

The problem is not related to Pager. As always, the problem comes from another place but Pager is the centralizer of complex things and they always explode there.

Let me explain: The issue is related to Doctrine_Query::count() method. This one is already on our schedule to be rewritten for 0.11.1, since we could not completely fix the ticket #1133. The problem seems different, I know, but the explosion happens at the same place.

I'll move your ticket to be fixed in 0.11.1, since we plan to fix the other issue in this release too.

Thanks for reporting us! =)

Changed 12 months ago by guilhermeblanco

  • milestone changed from 0.11.1 to 0.11.0

Changed 12 months ago by jwage

  • status changed from assigned to closed
  • resolution set to wontfix

The issue here is that DQL callbacks are off by default. The reason is because of the extra processing required in order to determine what models are involved in a query so that we can invoke the hooks on those models to alter the query. I think we just need to document that in order to use use the dql callbacks or behaviors that use them, you have to explicitly enable them. In your case the model isn't instantiated yet, so dql call backs aren't enabled when the query is executed.

Like you said the fix is to enable the use_dql_callbacks attribute on the manager.

Changed 12 months ago by jwage

Added coverage in r4561

Changed 12 months ago by francois

er... It's the SoftDelete? template itself that enables dql callbacks. So if I understand you correctly, the setting of the dql_callbacks should be removed from the template's setTableDefinition() and added to the documentation? In all behaviors?

Changed 12 months ago by jwage

Yes, we'll need to remove it from the constructor of the SoftDelete? listener since it is pretty much useless. It is not enabled until it is instantiated, and the query is executed before the SoftDelete? listener is instantiated and attached.

I have updated the documentation of the SoftDelete? behavior to mention this. It is also mentioned in the DQL Listeners:  http://www.phpdoctrine.org/documentation/manual/0_11/?one-page#event-listeners:dql-query-listeners

I tried to come up with a way to automatically turn on dql listeners if you use a behavior that makes use of them, but I couldn't come up with a way to do it.

Note: See TracTickets for help on using tickets.