So, we want to have PostgreSQL tested on Travis CI?! Nice try…
Part I - Flow
First of all, the compilation phase when “booting” with PostgreSQL already dies with an error. Not an easy ont to debug (partly because the DB is cleaned up right after the error happens). Anyway, a rather stupid one, fixed with (merged):
Now the tests actually start to run. The next one reminded me of a bug I had back in 2004 when working on DBAL, that also appeared as soon as I started to use PostgreSQL. I guess you do know that MySQL silently truncates data that won’t fit into a field? No? Now you do. And here is the modern incarnation of that bug, only affecting a test (that uses invalid data to begin with, merged):
Ok, next up is an error that is known behavior of Doctine ORM. The
commonObjectIsPersistedAndIsReconstituted() tests the behavior of the vanilla types Doctrine ORM supports, among them
object. The object we assign in the test is
Persistence\Fixtures\CommonObject, containing a protected property. Doctrine stores
serialize()d PHP data in a text column. Which doesn’t work on PostgreSQL, since the string is truncated at the first
null byte, used in the serialised data to mark the protected property. The official fix is to use a custom datatype if you need it, for the test I decided to skip it if on PostgreSQL (merged):
With this, the functional tests for the Flow framework pass on PostgreSQL. Getting the Neos functional tests to run is a different story. To be continued here… But first please give those PRs above a look.
Part II - Neos
Because it fits:
First of all, to (easily) support PostgreSQL, we’d need to raise the required version to 9.4, as in:
Then Neos needs to be tweaked as well:
These are marked as WIP right now, since they need migrations, didn’t do those yet.
Thanks for taking care! I definitely think it’s needed.
I have been continuing this the last days…
The pull request
is now done IMHO, I added PostgreSQL for functional and behavioural tests on Travis CI in that PR as well. Passes just fine.
This needs to be merged so the PRs for Neos can be finalized. That means, I’d love to have feedback on the branch this should go in: 3.0 or master.
The thing is, when this is merged:
- the minimum supported version of PostgreSQL is raised to 9.4 (from 9.3)
- projects using the
flow_json_array type on PostgreSQL will need to create database migrations
So technically speaking this is a breaking change, but…
Anyway, I do not want to decide this alone.
Fine for me here, it’s such an edge case and not released for so long I doubt it will be a big problem.
What would be the drawback of having it only in master?
Mostly that Neos 2.0 will not work on PostgreSQL.
And, strictly speaking, Neos 2.x, because if we consider this a “real” breaking change, it must only go into Flow 4.0… and that would be Neos 3.0.
Sounds reasonable to put it in next minor. If implementation was broken before anyway, you theoretically can’t break any existing project, except for the ones where people manually solved the issues, in which case I guess they are fit to properly take care of correctly upgrading.
The culprit with the
json column type is, that it’s not sortable and thus cannot be used in a
DISTINCT query. I did more tests and started to wonder where I observed this…
So, in 2.0 only the site export uses
DISTINCT and is broken because of the
json column type (and I mainly realized it’s broken because of functional tests).
That means, if we decide to leave site export broken for Neos 2.1, we do not need this particlar change in Flow 3.0 (but should still fix it in Flow 3.1 for Neos 2.1).
As just explained in Event logging fixes (not only) for PostgreSQL there is another PR related to this topic:
All related changes have been merged by now and PostgreSQL is being tested on Travis CI along with MySQL.