Extracts clause creation into clause methods.

[alexstandiford]

Part of #18 Originally, this was a work-in progress implementation to add support for update_many method.

While working on this, I also made a significant set of changes to the get_results method. Originally, this method had several sanitization steps for each clause baked directly into the method. This update has broken each clause into their own method. This makes it easier to build secure queries that don't fall in-line with the get_results method.

I anticipate that these methods will prove to be useful when building custom queries outside of the what get_results can't do on its own.

These methods will also make it easier to build in support for JOIN statements in the future.

See https://github.com/berlindb/core/pull/20/commits/4f6fa92448eb3f9908bc4384180e404947b807ee

[JJJ]

Again, thinking about this quite a bit over the past few weeks.

Is it possible that a Query_Clause should be its own class, and the contents and functionalities of each inherit something from Base while also having their own properties?

BerlinDB did not originally set out to replace something like Doctrine, where the MySQL syntax can be narrowly abstracted into precisely called PHP methods to avoid writing MySQL. This PR starts down that road, so I'd like to discuss (here is OK!) what we think is at the end of that road before we drive down it too far, if that's OK @alexstandiford ?

[alexstandiford]

@JJJ Absolutely.

To me, a key reason to want to use BerlinDB is to have an easy way to build a database schema, and read the data from that table.

In many cases, query handles this adequately, however, in the time I've spent with the plugin I have inevitably ran into circumstances where I'm better-off writing a custom query.

In cases like this, my process to identify the intended query looks like this:

1. Open Sequel Pro, and write the query until I get the desired results
2. Copy/Paste that query into my code
3. Modify the query so that it can be safely ran with the necessary variables.

I think it would be nice to be able to write out something like:

$query = new Table_Query();

$query = "";

$query .= $query->select_clause(['field_name' => 'field_name_as', 'sum' => ['sum_field_name']]); // Sanitizes and validates viable database fields.
$query .= $query->from_clause() // Automatically generates from clause from table schema
$query .= $query->where_clause();
$query .= $query->group_by_clause();

Some benefits I've noticed:

1. I get a much clearer idea on where my generated query messed up, because the error is better-localized inside a method.
2. I find it to be a lot easier to sanitize the table data, because the sanitization needs happen on a one-by-one scale, instead of in one monolithic black-box sanitization method.
3. Currently, there isn't a great way to handle permissions on this data if you work outside of the fundamental methods that ship with Berlin. I don't use these features in my build, but I would be concerned about using them if I had custom queries that could totally bypass that.

If it is determined that this does belong in BerlinDB, I love the idea of breaking this out as its own class. It would be useful to be able to extend this if you had home-made clauses that you need to re-use throughout.

[DrewAPicture]

Feel free to crib any or all or none of https://github.com/sandhillsdevelopment/claws if you proceed with this line of thinking.

[alexstandiford]

Oh I like that idea. Make the claws class the BerlinDB class we're discussing makes perfect sense. It's cleaner than what I've proposed so-far, and fits with @JJJ's suggestion of moving this functionality into a separate class.

I know at least for AffiliateWP, we would really benefit from something like this, especially if we can get sum(), count(), and join() added into this class sometime in the future.

But even for the short-term, what @DrewAPicture has built in Claws already makes this possible, you just to manually do some parts of the sql when claws can't do it.

Example:

function get_total_amount( $user_id ){
    global $wpdb;
    $clause = claws();

    $clause->add_clause_sql( 'SELECT user_id, SUM(amount) as amount_sum FROM table_name' );
    $clause->where( 'user_id' )->equals( $user_id, 'int' )
    $clause->add_clause_sql( 'GROUP BY user_id' );

    return $wpdb->get_results( $clause->get_sql() );
}

A good reason to include this in the library instead of keeping them separate is all of the benefits Claws could get if had access to BerlinDB's schema and table classes. A simple example of this benefit can be seen if I re-write the above example, assuming that Claws had a method, from(), and put it in an extended Query class.

class User_Query extends Query {
...

    public function claws(){
        return new Claws( $this );
    }

    public function get_total_amount( $user_id ){
        $clause = $this->claws();
        $clause->add_clause_sql( 'SELECT user_id, SUM(amount) as amount_sum' );
        $clause->from()->where( 'user_id' )->equals( $user_id )
        $clause->add_clause_sql( 'GROUP BY user_id' );

        $results = $this->get_db()->get_results( $clause->get_sql() );

        return $results;
}
...
}

The above is a plausible example of how this could work. $this->claws() would have the context of the query class made available to it. This means nothing needs specified in the from() method above. We also don't need to specify the second argument in equals because in this context, we know the database schema and already know that user_id is an int.

Other possible benefits:

1. Access to cache methods.
2. Access to item shaping (see issue #19 )
3. Access to the table schema

[alexstandiford]

If we decided to implement something like this, it may be useful to also make it somehow support a way to shape the query results https://github.com/berlindb/core/issues/14

[alexstandiford]

Like Issue #14, It seems like this is going to the wayside in-favor of #55.

I'm under the impression that if a custom query needs to be ran, the BerlinDB methodology would be to extend the Query class instead of running direct mySQL queries. I'm not saying that we wouldn't benefit from something to help build clauses, but I don't think it would look like the proposed PR. We can probably close this.

[JJJ]

I think I agree. The work here may not get merged as is, but it wasn't a waste. I'd like to see some of this happen.

I like the direction you were going @alexstandiford with 4f6fa92448eb3f9908bc4384180e404947b807ee, as well as, like @DrewAPicture said, taking some inspiration from Claws (specifically the way each comparison has its own convenience method).