Version 3: Update interfaces and nothing else

[pmjones]

There have been several questions lately regarding the fact that the interfaces do not match the classes. The disconnect is because the classes have had features added, but adding the relevant methods to the interface would be a BC break.

Perhaps the time has come to update the interfaces, and in so doing move to version 3, with no other functionality changes.

Thoughts?

[cdekok]

If you plan to change it you might want to consider changing the split with SubSelectInterface and SelectInterface it's a bit annoying you cannot get any typehint as you can only typehint on one of interfaces please consider extending the interface for ease of use with an IDE.

[pavarnos]

Yes please!
Also fix col() which is yellow in phpStorm with 2 arguments.

I have ended up creating a small wrapper class that fixes some annoyances and adds extra utility functions. For example, adding the ability to include columns from the join table that get automatically prefixed by the table name, a joinLeft() / joinInner(), and extra methods to ease working with subselects (not shown below).

class WrappedSelect {
   /** @var Aura\SqlQuery\Common\SelectInterface */
   private $select;

    /**
     * Adds a JOIN table and columns to the query.
     * @param string       $joinType  The join type: inner, left, natural, etc.
     * @param string       $spec      The table specification; "real_table" or "real_table AS alias" 
     * @param string       $condition Join on this condition.
     * @param array        $columns   columns as passed to cols()
     * @return $this
     */
    public function join($joinType, $spec, $condition = null, $columns = null)
    {
        $this->select->join($joinType, $spec, $condition);
        $this->addColumns($columns, $spec);
        return $this;
    }

    /**
     * @param string $spec      The table specification; "foo" or "foo AS bar".
     * @param string $condition Join on this condition.
     * @param array  $columns   columns as passed to cols()
     * @return $this
     */
    public function joinLeft($spec, $condition = null, $columns = [])
    {
        return $this->join('left', $spec, $condition, $columns);
    }

    /**
     * Adds a FROM element to the query; quotes the table name automatically.
     * @param string $table The table specification; "foo" or "foo AS bar".
     * @param array  $columns
     * @return $this
     */
    public function from($table, $columns = [])
    {
        $this->select->from($table);
        $this->addColumns($columns, $table);
        return $this;
    }

    /**
     * @param array  $columns
     * @param string $tableName table name
     */
    private function addColumns($columns, $tableName)
    {
        if (empty($columns)) {
            return;
        }
        if (strpos($tableName, ' ') !== false) {
            // if the table spec is a subselect or function or anything funky then just add the raw columns
            $this->cols($columns);
            return;
        }
        $resolved = [];
        foreach ($columns as $col => $alias) {
            if (is_string($col)) {
                $col = $tableName . '.' . $col;
            } else if (!preg_match('|[.("]|', $alias)) {
                // no function call or other weirdness
                $alias = $tableName . '.' . $alias;
            }
            $resolved[$col] = $alias;
        }
        $this->cols($resolved);
    }

// ... etc

[pavarnos]

Can we also make several protected methods public and available via the interface? eg reset() and the build* functions, or provide getters for flags (for DISTINCT), groupBy, orderBy, having.

I recently built a paginator class that builds a PageInformation data transfer object for a sliding window of pages. Its based on ZF1, ZF2, PagerFanta and jasongrimes/php-paginator. But none of the above support Aura, and seem quite complex for what is actually needed. With the above changes, i can make the paginator even cleaner / clearer and offer it to the Aura project if you want it.

[pavarnos]

can you create a v3 branch we can make pull requests against?

[harikt]

@pavarnos have created a 3.x branch.

[pavarnos]

a suggested "ship list" for v3?

• add missing public methods to SelectInterface and related. #112 add missing public methods to select interface (and related eg ValuesInterface, Limit*Interface). Add missing public methods to UpdateInterface and WhereInterface (for addRow missing on the interface #107)
• merge SubSelectInterface into SelectInterface? (see Version 3: Update interfaces and nothing else #108 (comment): can not do this. Needed for https://github.com/auraphp/Aura.SqlQuery/blob/3.x/src/AbstractQuery.php#L403)
• use variable argument lists (...$value) to make function arguments explicit (insert->col() and update->col() use variable argument lists #111 for col(), make $bind parameters explicit in WhereInterface and in having() clauses #114 for having(), where(), orWhere() etc)
• REPLACE sql command #100 add useReplace() sql command for MySQL (see add Insert::orReplace #115)
• fix as many Scrutinizer warnings as possible (some are pretty easy fixes) (fix scrutinizer messages #117)
• add documentation for additional public methods (eg leftJoin() which I didn't realise existed until digging in: see my earlier comments on this issue)

I'm OK with doing all this: i have time this week. @harikt has been great at responding quickly.

[harikt]

@pavarnos I would suggest you to create separate PR's . Small PR's are easy to review and merge. Changes to interfaces in PR may take some time for review and getting merged.

I will try my best to respond / merge with my knowledge.

Thank you.

[harikt]

By the way the docs structure will change. I will make the changes soon.

[syrm]

Don't forget to add hasCols and getCols in interface ;)

[pavarnos]

@OXman see #112: just waiting on approval / merge. Its a big PR though...

[pmjones]

@pavarnos et al: I've just merged a bunch of 3.x-related stuff; let me know how much that moves the checklist forward.

[pavarnos]

thanks for all the merges. updated the checklist. i'm short of time to contribute much for the next wee while...

[pmjones]

@pavarnos No hurry on my part. Thanks to you and @harikt for all your work here!

[pmjones]

My apologies, folks -- my recent PRs to the 3.x branch have been over-and-above this plan. After the Quoter PR I will cease, and move on to documentation.

Further, I think the Subselect interface can be merged to SelectInterface, so long as we typehint that particular bit of logic to SelectInterface instead of Subselect.

Thoughts?

[pavarnos]

Sounds good

[harikt]

UPDATE : I have made the 3.x as default branch.

[pavarnos]

all the things in this issue have now been resolved? can it be closed?

[pmjones]

Looks like it.