berlindb / core Goto Github PK

It would be really useful if it was possible to register different query processors into the Query class, similar to the Date class that exists now. This would allow plugin developers to seamlessly inject their own query extensions into the query class, and provide Query with the instructions necessary to run the query.

It would also give us a clear path on how to improve, and expand on BerlinDB's query functionality in the future. I'm thinking about issues like #50.

Right now, items like Date have to be hard-coded directly into the class to work, which obviously isn't ideal.

Another thing that would be nice about this, is when contributing to BerlinDB, pre-made libraries can be added, and even pitched to be put into core. This gives everyone a relatively easy way to test, and try out a new feature without needing to completely replace their installation of BerlinDB just to make it work.

I tried to use a date query and Berlin crashed. This was because it was unable to extend the base class. Date is set to extend Base in the same namespace as Date, however it is in a different namespace.

This is causing issues when trying to run a date query.

Needs confirmation.

This line specifically appears (visually, from my eyes) to not be correct:

And probably should be something like:

    delete_metadata_by_mid( $this->apply_prefix( $this->item_name ), $mid );

This is because the metadata_by_mid() functions in WordPress call _get_meta_table() internally, so we are unable to use Query::get_meta_table_name() because it would double-suffix the table name resulting in something like sc_eventmetameta .

Sorry for the not-so-good format of this issue. Juggling a few things right now and wanted to jot this down before I forgot about it.

Unprepared basic WHERE clause: https://github.com/berlindb/core/blob/master/query.php#L2828

In my install of Berlin, delete_items appears to delete an item as-expected, however, if that same item is fetched in the same request after it is deleted, the record can still be acquired.

$query = new Query();

$id = $query->add_item();

$event = $query->get_item( $id );

$deleted = $query->delete_item( $id );

$event_after_deletion = $query->get_item( $id );

var_dump( compact( 'event', 'event_after_deletion' ) ); // both event and event_after_deletion return the same value.

In the above example, I would expect $event_after_deletion to be false since the record no-longer exists, however, it returns the value. I'm pretty sure this is because the item is cached, and for some reason the system mistakenly thinks that record is still valid.

If you try to drop a table using Table::drop inside of a WordPress unit test case, you will receive an error something like this:

It's entirely possible this is just a bug in what I've built, but wanted to see if others are experiencing this.

When I add IF EXISTS to the end of the drop statement, these errors go away. I'm not certain this is the best way to handle this, however, since in my context I usually drop a table to reset the table back to the default settings. So I suppose my questions are:

1. Is there a more-appropriate way to reset a database table?
2. Is there any reason why we should not add IF EXISTS to the drop table syntax?
3. Is there some logic we should introduce that works around MySQL transactions?

I did a small amount of digging, and found this:
https://wordpress.stackexchange.com/a/220308/105777

You've just discovered an important feature of the core test suite: it forces any tables created during the test to be temporary tables.

If you look in the WP_UnitTestCase::setUp() method you'll see that it calls a method called start_transaction(). That start_transaction() method starts a MySQL database transaction:

    function start_transaction() {
            global $wpdb;
            $wpdb->query( 'SET autocommit = 0;' );
            $wpdb->query( 'START TRANSACTION;' );
            add_filter( 'query', array( $this, '_create_temporary_tables' ) );
            add_filter( 'query', array( $this, '_drop_temporary_tables' ) );
    }

It does this so that any changes that your test makes in the database can just be rolled back afterward in the tearDown() method. This means that each test starts with a clean WordPress database, untainted by the prior tests.

However, you'll note that start_transaction() also hooks two methods to the 'query' filter: _create_temporary_tables and _drop_temporary_tables. If you look at the source of these methods, you'll see that they cause any CREATE or DROP table queries to be for temporary tables instead:

    function _create_temporary_tables( $query ) {
            if ( 'CREATE TABLE' === substr( trim( $query ), 0, 12 ) )
                    return substr_replace( trim( $query ), 'CREATE TEMPORARY TABLE', 0, 12 );
            return $query;
    }

    function _drop_temporary_tables( $query ) {
            if ( 'DROP TABLE' === substr( trim( $query ), 0, 10 ) )
                    return substr_replace( trim( $query ), 'DROP TEMPORARY TABLE', 0, 10 );
            return $query;
    }

The 'query' filter is applied to all database queries passed through $wpdb->query(), which dbDelta() uses. So that means when your tables are created, they are created as temporary tables.

So in order to list those tables, I think you'd have to show temporary tables instead: $sql = "SHOW TEMPORARY TABLES LIKE '%'";

Update: MySQL doesn't allow you to list the temporary tables like you can with the regular tables. You will have to use another method of checking if the table exists, like attempting to insert a row.

But why does the unit test case require temporary tables to be created in the first place? Remember that we are using MySQL transactions so that the database stays clean. We don't want to ever commit the transactions though, we want to always roll them back at the end of the test. But there are some MySQL statements that will cause an implicit commit. Among these are, you guessed it, CREATE TABLE and DROP TABLE. However, per the MySQL docs:

CREATE TABLE and DROP TABLE statements do not commit a transaction if the TEMPORARY keyword is used.

So to avoid an implicit commit, the test case forces any tables created or dropped to be temporary tables.

This is not a well documented feature, but once you understand what is going on it should be pretty easy to work with.

Right now, parse_query has a pattern that makes it possible to manipulate the class using a WordPress action. This works, but it creates an awkward pattern when this action is referenced directly in the child class that uses it.

I think it would be better to change parse_query so that a protected method runs after it's done to serve the same fundamental purpose as the action that runs at the bottom of the method. This would allow extended classes to modify behavior in a cleaner manner.

It seems like it's only possible to filter by a single date column. If I'm correct about that, it would be nice if Query could accept an array of date queries instead of a single query, or something like that.

Imagine you have a table, books with these columns:

• date_published
• date_created
• author
• title
• publisher

There's currently no way to query against both date_created and date_published in BerlinDB.

I would expect that it would be possible to-do something like:

// Select books that were published after last Tuesday, and also only select the fields that were created since yesterday.
$query = new Book_Query( array(
    'date_query' => array(
        'relation' => 'AND',
        array(
            'field' => 'date_created',
           array(
                'after' => 'yesterday'
            )
        ),
        array(
            'field' => 'date_published',
           array(
                'after' => 'last tuesday'
            )
        )
    )
) );

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.

Right now if you are using BerlinDB via composer you would have to set $db_global in every class that extends Base. This feels mighty redundant.

It would be nice to be able to configure the variables in Base (db_global, prefix) from some outside method in the plugin.

I hope we go only with PHP 7.
Please add declare(strict_types = 1); to each file.

SlevomatCodingStandard.TypeHints.DeclareStrictTypes does it for you.

See #75.

Getting ready to try using this in a plugin, and realized that there isn't a composer.json set-up. Since this is generally going to be used as a dependency in a plugin, it would be useful if it had one.

Is it possible to add a WHERE <field> IN ( <int>, <int> ... ) clause?
So quotes will not be inserted while escaping, unlike this: WHERE post_id IN ("1", "2", "3")

See: awesomemotive/easy-digital-downloads#8136

In 87fa6d5, Berlin brought 2 WordPress core functions into Queries\Date public methods.

Now, inside of build_mysql_week(), there is a get_option( 'start_of_week' ) call that Sugar Calendar would benefit from being able to more easily override.

The purpose of this get_option() call is to ensure that WEEK() MySQL statements on specific columns use the correct the "offset" or INTERVAL and DAY as needed. Sugar Calendar uses its own sc_start_of_week setting, which does fallback to start_of_week already, so it's not possible to filter it the other direction without creating an infinite loop situation.

It would be nice if, after is_success() ran, if an action would fire. This would allow plugins to capture database events, and potentially log when something goes wrong. It would also make it possible to take a corrective action, if possible.

This seems to be happening because of a vague check in Query::parse_where.

...
if (  ! empty( $this->query_vars[ $column->name ] ) ) {
...

Both 0, and false both cause empty to return true. This causes parse_where to skip parsing a filter by a value set to 0 or false.

Originally, part of what I'd wanted Berlin to be able to do, was allow for object variables to be retrieved from methods inside the calling class, like this very bad example below:

Say you want $this->thing but you always want it formatted a certain way. In theory, you could just add this method and it would always get returned formatted as you wanted it:

public function get_thing() {
    return sanitize_key( $this->thing );
}

Unfortunately, the magic getter isn't actually working because all of the class definitions currently require - as a best practice - that their default object variables be defined. This causes PHP to always skip the magic getter and use the property that is defined.

The bug here, is that you should be able to override properties simply but you can't, even though there is code inside of Base::__get() that tries to support it.

You also cannot work around it by omitting object properties, because they are required as part of the way that Berlin expects for classes to be written.

Berlin has always depended on WP_Meta_Query. This was simply due to lack of time when pushing to projects that depended on Berlin for their database table management.

We can incrementally begin this task by creating our own Queries\Meta class that extends WP_Meta_Query with no additional changes.

That will ensure that all future versions of Berlin will already be using the new class path, namespace, and are including the eventual meta.php file.

If I run a query without a date query, everything works as-expected. However, if I include a date query, get_db() returns false.

This appears to be because the date query object is a separate type of query, and it expects that it should also have a $db_global variable set.

As a hotfix, I currently have hard-coded this in my Date_Query object, and it appears to be fixing the problem:

	protected $db_global = 'wpdb';

Obviously this isn't ideal, but I'm really not sure how else this can be resolved. Shouldn't the date query object inherit the db global from the query it's running within?

Here the key is set to compare_query https://github.com/berlindb/core/blob/master/query.php#L445

But later it's referenced as compare : https://github.com/berlindb/core/blob/master/query.php#L1281-L1293

I think it used to be compare but later got changed and not all instances were updated.

Currently, Berlin does not automatically unserialize/serialize array data when it goes in, or comes out of the database. This surprised me as this is a common scenario.

In fact, it doesn't seem to do any typing when the item is shaped at all.

Is this a design decision that I'm missing, or something?

In the get_results() method of the query class, the logic that validates the $where_cols is not working correctly. Currently it is:

foreach ( $columns as $column ) {
	if ( ! array_key_exists( $column, $column_names ) ) {
		unset( $where_cols[ $column ] );
	}
}

This always results in the $where_cols getting set to an empty array.

To fix it, we need to unset using the column index, like so:

foreach ( $columns as $index => $column ) {
	if ( ! array_key_exists( $column, $column_names ) ) {
		unset( $where_cols[ $index ] );
	}
}

Discovered via Sugar Calendar https://github.com/sugarcalendar/standard/issues/352

Hi all

I've been chatting to @JJJ via email regarding a baked-in REST / GraphQL API for BerlinDB and I figure it makes sense to open an issue for it. I'm hoping to use BerlinDB as the db engine for a headless project. Here are 2 features I'd love to see in an implementation:

1. Built in caching using the Transients API, or at least native ways to enable caching and selectively flush endpoint caches. At the moment I'm using custom REST endpoints plus https://wordpress.org/plugins/wp-rest-cache/ and it makes a huge difference for performance. They also uses the Transients API which, meaning we can leverage Redis for API endpoints.

2. This might not be possible, but is there a way of querying data without having to load the theme, all plugins etc first? My understanding is that both WPGraphQL and the REST API do this and that it really impacts performance. Since BerlinDB is designed to be a performance-oriented DB framework, some kind of bespoke API that is just as performance-oriented would be a good fit for the the project.

Is something like this planned and what would be an approximate timeframe for a dev version?

Cheers

Similar to the column_exists() method, some database table upgrade routines would benefit from an easy & reliable way to check whether a database table column already has a specific index on it or not.

Easy Digital Downloads 3.0 specifically, has at least one upgrade method where a method (such as the one I'm proposing here) would be useful to avoid attempting adding an index where it already exists.

I've witnessed a single case of an upgrade loop occurring because this type of ALTER query fails, which does not bump the table version, etc...

As I can see there was a high momentum after WCEU (I was there at the talk) and a lot of people was interested on getting this library.

Right now this project is not very healthy also if there is an organization, there are no maintainers so doesn't have much sense to have an organization.

Also, there is no documentation but it is suggested to reverse engineering other plugins that use it (#7).
There are also different pull requests that are stalling since months and this is not inviting getting new contributors (why I have to contribute if no one cares of what I do).

Doesn't include also a comparison with others ORM or WordPress Orm like to just understand if it is helpful.

I am just asking here to move on the project in a solid way and not just publish the code as it is that is not very helpful to change the WordPress ecosystem (instead a lot of people was thinking that this project can do that).

Berlin expects to be working with datetime's in UTC.

WordPress sets the timezone to UTC using date_default_timezone_set( 'UTC' ).

This isn't really a problem, but it could be if date_default_timezone_set() was ever called using anything but UTC.

It would be cool to support rollbacks, kind of like Laravel does. For each table upgrade, you'd also write a rollback so you can go forward and go backwards to revert it.

It seems like the upgrader never runs database upgrades. It looks like the array_filter that filters out the upgrades that have already ran does not filter by the version variable, but by the callback method variable.

As WordPress continues to be the primary focus of this project, many of the classes currently use WordPress specific function calls.

One of the longterm goals of this project, is for it to be platform and environment agnostic. For this to happen, anything specific to WordPress needs to be abstracted into some type of middleware.

Without looking, wp_parse_args and sanitize_key() come to mind. There are probably at least a dozen functions being used.

AFAIK if you return an object (an instance of a class) you get a clone, not the object itself. We had a problem with an OOP ACF plugin that does that as we use Redis caching thus the database is usually not used.

Please see this kind of OOP access of $wpdb https://github.com/szepeviktor/debian-server-tools/blob/master/webserver/wordpress/_core-wpdb.php

This might be off the mark with BerlinDB direction, but It would be useful to have the ability to "SUM" specific columns in the database tables without having to write custom queries as a fallback. Having the ability to use AVG/MAX/MIN would also be beneficial in a lot of use cases.

I <3 command-line generators. They allow us developers to debug our own code quickly, create better unit tests, and create quick on-the fly scripts to help illustrate problems to other people.

Unfortunately, a data generator is almost never created before a plugin has been completely built, simply because it takes a significant amount of time to build, even though it would be extremely helpful if one was prepared early-on.

I think Berlin could solve this problem.

Ideally, Berlin would have a way to generate data with a PHP method, and possibly a WP_CLI command. It's possible the CLI would need to be in a separate library, since BerlinDB isn't necessarily specific to WordPress.

But how would we associate data with other tables in a generator? Data is usually interconnected in some manner. A good example of this is in AffiliateWP:

A referred transaction needs a referral.
A referral needs an affiliate, a customer, and an order.
An order needs a customer, and products.
An affiliate needs a user ID
A customer needs a user ID

On a basic level, you could create generator methods that would generate affiliates, customers, orders, users, and products individually. This would require 6 separate commands:

wp berlindb generate users --number=10
wp berlindb generate customers --number=1 --users="1,2,3,4,5,6,7,8,9,10"
wp berlindb generate affiliates --number=1 --users="1,2,3,4,5,6,7,8,9,10"
wp berlindb generate products --number=10
wp berlindb generate orders --number=10 --products="1,2,3,3,4,5,6,7,8,9,10"
wp berlindb generate referrals --number=10 --orders="1,2,3,4,5,6,7,8,9,10" --affiliates=1 -- customers=1

Even if you chain these commands together, that's an awful lot of work to build out a test database. It works, it's just not as simple as it could be.

Ideally, it should be possible to configure, and do something more like this:

wp berlindb generate orders --number=10

The simplest solution I can think of is to create a method in the schema, generate that can be overridden by child classes. This would provide a fairly straightforward way to override how generation functionality would work for individual classes.

If an unexpected type of value for $column_value is passed into get_item_by() (like an array or an object) it subsequently being passed into the Cache API will result in PHP Errors.

Hey JJJ,

My developer was wondering if you have any documentation to help implement this in our own plugins?

Cheers

While doing some tests, I noticed that the metadata associated with a table was not getting deleted. It appears to be related to how delete_metadata_by_mid converts table names, and how we're passing that table name in.

If we truly want to make BerlinDB compatible with systems outside of WordPress, it may be prudent to make a change to how Base::get_db() works. Not all systems utilize a DB global in the way WordPress does, and few classes actually have the exact same methods as $wpdb.

It may be best to consider creating an interface that allows BerlinDB to interact with other database systems dynamically. This could implement the methods that Berlin calls directly from $wpdb. This would allow people to translate their system into Berlin's system.

Instead of having BerlinDB look for a global, perhaps it would fetch the specified instance, check if it implements, and use it.

This would require an extra file to make WPDB work with it, which is kind-of a bummer, but I think that's a small price to-pay. Since 2.0 is compatible with Composer, this interface and implementation could be somehow added as a composer package. This would make installing a WP-compatible version of Berlin as simple as running composer require berlindb/wordpress, or something similar.

Further, I can see this pattern being used in other places, such as an object cache implementation.

$column_names returns an array, where the column name is the key.

https://github.com/berlindb/core/blob/master/query.php#L2795

But then the column validation checks in_array(), when it should actually be checking array_key_exists()

https://github.com/berlindb/core/blob/master/query.php#L2797-L2802

There are many situations where it's practical to create, or update many records at one time, using the same values. Currently, the only way to-do this with Berlin is by querying the records, looping through each one, and updating the record one at a time.

While working with Mongoose, I found that they have three methods, updateMany, addMany, and deleteMany, which would make the database update using a single query instead of x+1 queries as mentioned in the above paragraph.

Query::validate_item() runs stripslahes() on all values. This immediately converts null to an empty string and thus makes it impossible to intentionally set a null value.

The Problem

When someone calls
$query->delete_item( $id );
it deletes the item and then deletes it from the cache. This is done calling
$this->clean_item_cache( $item ); (where $item is an object)

The problem is the clean_item_cache() method accepts an array of object items. There is a cast done here but it actually casts the object passed to an array, where each object attribute becomes an index (instead of ending up with an array of objects). That said, the if ( ! is_object( $item ) ) { test is always true (we are iterating over the attributes, not over objects as intended).

Possible Solutions

The possible solutions seem to be simple:

1. Changing
   $this->clean_item_cache( $item ); to
   $this->clean_item_cache( array( $item ) );
   fixes the problem.

2. Changing
   $items = (array) $items; to
   if ( ! is_array( $items ) ) { $items = array( $items ); }

Please, let me know if this all makes sense and I'll be more than happy to open a PR. Thanks!

Because of the way the Date Query class currently works (it being inherited from WordPress) the resulting MySQL string needs to be stashed temporarily in a private variable, so that it can be referred to later in a different method.

This is not ideal for a few reasons:

• It does not match the way Meta & Compare queries work
• It is not obvious that it works this way
• It no longer needs to work this way, now that we've brought our own implementation of the Date Query in-house

It would be much better for it to work more like Meta & Compare, and simply return everything it needs all at once, and provide a subsequent get_sql() method to retrieve the query string part as needed.

Just as the title says, the Table class currently lacks any method(s) related to taking all of the data in the current table and copying it (temporarily or permanently) to another table/location.

Related: #22

The decimal and numeric validations do not allow for saving null values.

validate_decimal converts it to 0 https://github.com/berlindb/core/blob/master/column.php#L784-L786

and the default validation for integer types is intval, which would also convert it to 0 https://github.com/berlindb/core/blob/master/column.php#L727

Ideally both would allow saving null values if the column allows it.

One of the more confusing things about BerlinDB is the inability to create queries that create JOINs in any meaningful way. This is mostly due to how the caching works around shape_items.

From how I understand it, BerlinDB's Query class fundamentally does this:

1. Parse the query
2. Fetch the IDs from the database
3. Loop through each of these items, and query for the records. This is also known as "shaping" in BerlinDB.
4. Cache each record as it is looped, so in the future it can be fetched faster.

The reasoning for this is because it makes it possible to cache individual records, and provide a lot of flexibility in getting as many records as possible with less effort.

The problem with this methodology is that it makes it impossible to really manipulate the query, which I think is a design decision, and probably a good thing.

But what happens when I want to make a query that JOINs two tables? There really isn't a way to-do that in BerlinDB.

It would be really nice if there were some way to utilize the cache functionality baked into BerlinDB in the context of a JOINED row, as well.

Please consider moving class files to src/
and naming class files after class names, e.g. Schema.php

Thank you for considering!

Right now is required to study the code of the ohter plugins that use it.
It will be helpful a bit of examples or docs to evaluate it in future projects :-)

I really wish it were possible to do asynchronous actions in BerlinDB. There are numerous instances in-which a process does not need to wait for a database change to happen to continue.

A simple example is deleting related data from other related tables, such as metadata.

There are also less-common scenarios where the system needs to save data, but does not need to use that record right away.

I recognize that PHP doesn't have any true async capabilities like Javascript, but perhaps we could set something up that runs a set of operations in a separate request.

I'm writing this with the intent to discuss different ways to approach this, identify pitfalls, and even identify if this sort of thing should be in BerlinDB at all.

Currently, get_results requires an array of columns to fetch, however, this is a little impractical when you just want to get all of the columns in the table. It would be nice if you could pass * as the cols argument instead of an array.