Dibi Fluent: cast objects to string in translation phase, and not immediately

[czukowski]

• bug report? no
• feature request? yes

Description

The proposed change is to remove casting to string here Fluent.php#L192 and only do it during the arguments translation phase, where it's being done anyway.

Rationale

A simple rationale is that there's no need for a cast to occur here (IMO), other than adding parentheses around the argument, which could be easily achieved by other means.

The real-world example is that this would give a chance to the users to add subselects to their queries and still be able to edit the subselects down the road before the query is executed.

For example, let's say we need to optimize a slow query like this one:

$db->select()
    ->from('some_table')
    ->join('a_really_huge_table')->as('main')->on(...)
    ->join('another_table')->on(...)
    ->join('yet_another_table')->on(...)
    ->join('a_few_more_tables')->on(...)
    ->where('main.indexed_condition = 1')
    ->groupBy('main.id')
    ->orderBy('main.%n', $orderColumn)
    ->limit($limit)
    ->offset($offset)

It could be rewritten to a faster performing version (while making certain assumptions that are beyond this scope):

$db->select()
    ->from('some_table')
    ->join(
        $db->select()
            ->from('a_really_huge_table')->as('main')
            ->where('main.indexed_condition = 1')
            ->orderBy('main.%n', $orderColumn)
            ->limit($limit)
            ->offset($offset)
    )->as('main')->on(...)
    ->join('another_table')->on(...)
    ->join('yet_another_table')->on(...)
    ->join('a_few_more_tables')->on(...)
    ->groupBy('main.id')
    ->orderBy('main.%n', $orderColumn)

So far so good, but let's suppose the whole process of query building is abstracted away in a dynamic query builder which was necessary because the joins, wheres, etc needed to be variable, based on the parameters passed to it. This suddenly means that the such a relatively small change in the resulting query requires major changes to the query builder interface to be able to cope with it. I would totally agree to an argument that it would be a limitation of some 3rd party code that has nothing to do with Dibi and its authors should have done it 'better' in the first place. On the other hand, if it was possible to pass a subselect to the Dibi Fluent interface and apply ordering and limiting to it later, a real-world situation like this would have been a non-issue and the changes to the query builder could be relatively simple.

[dg]

I'm just afraid it might be a big BC break. Because of the ability to change objects later.

[czukowski]

I see. Does a Literal class have to cast a constructor argument to string? A similar effect could be achieved by removing the cast from there, then it would be possible to do something like this without breaking backwards compatibility:

   ->join(new Literal($subselect))

From what I can tell, the Literal class is only used in Translator. Alternatively, it could be subclassed or had interface extracted.

[czukowski]

I've been able to achieve the effect by subclassing a Literal class with implementation like this:

    public function __construct(Dibi\Fluent $query)
    {
        $this->query = $query;
    }

    public function __toString()
    {
        return '(' . $this->query->__toString() . ')';
    }

No changes to Dibi were necessary after all. Feel free to close this issue, unless you would consider eg BC breaking changes for major releases in the future.