berlindb / core

All of the required core code
MIT License
253 stars 27 forks source link

Update shape items to be public #14

Closed alexstandiford closed 3 years ago

alexstandiford commented 4 years ago

There are situations where it would be nice to have direct access to shape_items. Sometimes it is best to just write out a custom query instead of using Berlin's API. In such cases, it would be helpful to be able to use shape_items so custom queries can behave in the same way as queries used within Berlin's system.

JJJ commented 4 years ago

Sometimes it is best to just write out a custom query instead of using Berlin's API

In these cases, we should identify why Berlin can’t be used, and try to patch that case.

Once shape_items goes public, it can’t ever go back, so we need to make sure this is what is best.

My fear of making it public, is that anything could be passed into it and arbitrarily shaped, without any other kind of check or balance. Is that what we want?

alexstandiford commented 4 years ago

Perhaps instead of making it public, we go with protected? This would at least force everyone to put these custom queries inside of the extended query class, where I can access shape_items without making it public.

I have this function that exists in a controller outside of my query class:

    /**
     * Runs a fancy query
     *
     * @since 1.0.0
     *
     * @return array list of results from fancy query.
     */
    function fancy_query() {
        global $wpdb;

        // Run a fancy query
        $query = $wpdb->prepare( "..." );

        $fancy_query = new Fancy_Query();

        // Return the items shaped by the query
        return $fancy_query->shape_items( $wpdb->get_results( $query ) );
    }

If I just refactor this so that this is inside Fancy_Query, I have access to shape items:


class Fancy_Query extends Query {
...
    /**
     * Runs a fancy query
     *
     * @since 1.0.0
     *
     * @return array list of results from fancy query.
     */
    public function fancy_query() {
        // Run a fancy query
        $query = $this->get_db()->prepare( "..." );

        return $this->shape_items( $this->get_db()->get_results( $query ) );
    }
...
}

If we still want to keep shape_items private, then we'll need to be able to accommodate for more complex queries. I'm using this in a circumstance where I need to get a collection of database objects with a JOIN, and I expect it to return an array of rows of that database object:

    /**
     * Gets the ads for the current ad unit
     *
     * @since 1.0.0
     *
     * @return array list of ads, if any.
     */
    public function ads() {
        global $wpdb;
        $query = $wpdb->prepare( "
        SELECT * FROM {$wpdb->prefix}aam_ads ad
        INNER JOIN {$wpdb->prefix}aam_ad_correlations correlation
        ON ad.ad_id=correlation.ad_id
        WHERE correlation.ad_unit_id=%d
        ORDER BY correlation.priority", $this->ad_unit_id );

        $ad_query = new Ad_Query();

        return $ad_query->shape_items( $wpdb->get_results( $query ) );
    }

Maybe https://github.com/berlindb/core/pull/20 is somehow related to this?

alexstandiford commented 3 years ago

I think we should close this in-favor of issue #55.

Custom join queries, and advanced cases like this should be possible in BerlinDB, but I don't think writing a custom query and using shape_items is appropriate. Instead, I think there should be some kind-of query processor class that can be implemented to manipulate how Query fundamentally works.