Missing unit tests for the MySQLSimpleDMLQueryBuilder and the DatabaseDMLQueryBuilder.
Overall this looks quite good though. Some minor issues that need fixing and then this should be good to go :)

This is potentially fine, but I doubt it qualifies as an external interaction. Either put it below the top section combined with all the other project and backup files, or give it a separate new section and explain wtf this is for.
Either way, it's an unrelated change and needs to go into a separate commit/review

This is a bit lacking. Also, why "$condition"?

is there a possibility of this ever being something else than "USING"?

this will break many things. Be careful with adding spurious empty lines

it would probably be nice for the simple DML query builder to allow passing a column list here, just like select() allows.

why "$name" and "expression"? expression of what?

spurious empty line

2016-2016? ;)

spurious empty line

spurious empty line

There is one other area of concern. USING is incompatible with ON, yet in the current implementation, it's still allowed to use both, thus potentially constructing invalid queries.

The query builder should take care to never produce invalid queries. Generally in case of conflicts we use a "first come, first served" principle. So whichever function is called first is being used.
In summary:
$builder->join()->using()->join()->on()

$builder->join()->on()->join()->using()
should be working, but
$builder->join()->using()->on()

$builder->join()->on()->using()
should not

how does the using part end up in the query if you remove it here?

why is this suddenly unindented?

indentation
also, why would using call table()?

This is weird. Your documentation states $using is always a string, so why would you test whether it's an array? Why would it be an array in the first place?

ditto

property unhandled in constructor/destructor

Use $column_list instead of $name

misleading explanation. JOIN and USING must be used together. It's ON and USING that shouldn't be mixed.

you're not finishing the join

You don't need both isset() and empty(). empty() checks for a superset of possible values.
Also, you're making this more complicated than it needs to be:
if (empty($join) || $this->is_unfinished_join)

$column_list

why do you escape a column as table?

No array here

@covers should only note the one specific function you are testing

$column_list

missing tests for multiple columns, and finished/unfinished joins.

misalignment of "="

Turns out, storing using() separately from join doesn't work. It breaks when you join multiple times, in which case you might also have multiple different using() clauses.
Example:

JOIN foo USING(bar) JOIN foobar USING(baz)


would translate to

$builder->join('foo')
        ->using('bar')
        ->join('foobar')
        ->using('baz');


The current code however would create:

JOIN foo JOIN foobar USING(bar, baz)

These tests are either wrongly classified, are in the wrong file, or for the wrong class

Something messed with the coding standard. Please don't mix style and functional changes.

Boolean variables should be clearly understandable.
$join_type = TRUE // USING

$join_type = FALSE // ON
Is not clearly understandable. There's no way to make this connection naturally

that's not a boolean

Why not reset this also for natural joins?

why keep it unfinished?

this breaks where/having

this breaks where/having

this breaks where/having

style: TRUE

misleading test name

if you're not invoking using(), the test is in the wrong file

no mixing of camel and snake case

that's a rather hacky approach to things and would need to be done in many places. I would prefer a solutions that wouldn't have that.

eh?

I marked a couple missing tests, but not all of them. This should give you a better idea though

odd

Not sure what you use for checking that, but Jenkins doesn't have an issue for this line listed.
Either way, if this is changed, it should use [] instead of array()

there's no test for this

There is a test for join_type = 'on' and join_type = '', but not join_type = 'using'

we generally don't do "huge" overarching if statements, but rather early exits.
So, rather than

if (TRUE)
{
  ... long block...
}


we'd have

if (FALSE)
{
  return
}

... long block...

missing test(s) for is_unfinished_join = FALSE

yeah, no. Please leave as is

missing tests for join_type = '' / is_unfinished_join = TRUE

missing test that this is set correctly

hm?

That seems wrong. What good would

$join_type = 'where';


do?

don't fix this pls

or if you do, do it right (equal signs should align)

probably better to do assertSame() here

redundant

Looking quite good now, but you should take some time to fix the test names :)

missing remaining tests for ON

there are tests missing to verify that where/having do not set a join_type

Mostly looks fine, some small issues. Forward to the next class ;)

shouldn't be necessary

assertPropertyEquals

I tried to merge this but there are too many conflicts.

Please rebase against master and squash all commits into one.

Ship It!