Initial PostgreSQL support #1933

Wieter · 2020-10-02T09:42:07Z

I've been working to get bolt compatible with PostgreSQL.
From my end everything seems to be working fine, good enough to be used for the purpose at hand (running at FreeBSD 12.1 \w PostgreSQL v11.6).
A quick check from my side on usage of MySQL and SQLite did not show serious problems after fixing a CAST specifically for MySQL (2707e71).

There might be some minor bugs still around (hence the WIP), that need user testing. For example something strange with the template used (bypassed in c68e6d5) where a selectfield does not have an .selectedIds (integer). Somehow sqlite & mysql have no problems with that, but postgres is more strict. There may arise more of those issues.

…ion: 7 ERROR: function lower(jsonb) does not exist

…: syntax error at or near user - user is a protected name and need quotation!

…MySQL

bobdenotter · 2020-10-02T11:18:41Z

@Wieter Awesome, thank you for your work! 👍

I'll keep a close eye on this, and as soon as the [WIP] is removed, we'll merge it in!

Wieter · 2020-10-02T12:51:52Z

@bobdenotter Sure no problem!
I'm just trying to improve the PostgreSQL (or better: database agnostic) compatibility while not breaking other things :)
A new (postgres-only), more tricky, issue popped up at the admin search /bolt/?filter=css
SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: jsonb ~~ unknown
LINE 1: ...t_type = $6) AND (b0_.status <> $7 AND (b2_.value LIKE $8 AN...
seems like b2_.value needs also to be casted to TEXT for non-mysql.
I didn't find out yet where the WHERE clause of this query gets assembled, might take some time.
Meanwhile I'm looking into the doctrine docs if this can be done automatically through the model annotation, which would be a silver bullet to cover for any remaining issues of the field_translation.value json-->jsonb change.

bobdenotter · 2020-10-03T11:19:28Z

Two quick pointers:

Some of the "Json or not" stuff is handled in wrapJsonFunction() in src/Doctrine/JsonHelper.php. Perhaps the hasJson() method in src/Doctrine/Version.php needs to be adjusted, too.
The heavy lifting of the queries generated from {% setcontent %} tags is done here: src/Storage/Handler/SelectQueryHandler.php

bobdenotter

Shaping up nicely! 👍

I've left some comments/remarks in your changes.

bobdenotter · 2020-10-04T07:30:36Z

.env.dist

@@ -24,7 +24,7 @@ DATABASE_URL=sqlite:///%kernel.project_dir%/var/data/bolt.sqlite
 #DATABASE_URL=mysql://db_user:"db_password"@localhost:3306/db_name

 # Postgres
-#DATABASE_URL=postgresql://db_user:"db_password"@localhost:5432/db_name?serverVersion=11"
+#DATABASE_URL=postgresql://db_user:"db_password"@localhost:5432/db_name?serverVersion=11&charset=utf8"


Is utf8 not the default? Do we need to explicitly set it here?

I think it is better to explicitly set here, to force users to think about the charset.
Additionally, it is hardcoded in config/packages/doctrine.yaml as charset: utf8mb4, which is fine for MySQL but unsupported in PostgreSQL (for as far as I could find). Setting it in the DATABASE_URL ensures an override to UTF8.

Ok, fair enough! Does Postgres still handle emoji properly, if it doesn't have utf8mb4?

Emoji should work as they are UTF-8, I'll have to test it to know for sure.
From what I understand, MySQL utf-8 is not really full utf-8, it uses 3-bytes per character instead the regular 4byte. I do not know why it was decided, but here lies the origin: https://github.com/mysql/mysql-server/commit/43a506c0ced0e6ea101d3ab8b4b423ce3fa327d0
That's probably why for MySQL utf8mb4 is required. For postgres. utf8 is full utf8, so 4 byte.
something to keep in mind in case of problems -> https://stackoverflow.com/questions/36198847/what-type-should-i-use-to-store-emoji-in-postgresql

bobdenotter · 2020-10-04T07:31:02Z

config/extensions/acmecorp-referenceextension.yaml

@@ -0,0 +1,3 @@
+# Reference extension configuration file


This file doesn't have to be committed.

bobdenotter · 2020-10-04T07:31:54Z

config/packages/doctrine.yaml

@@ -31,8 +31,11 @@ doctrine:
                alias: App
        dql:
            string_functions:
-                JSON_EXTRACT: Bolt\Doctrine\Functions\JsonExtract
-                CAST: DoctrineExtensions\Query\Mysql\Cast
+                JSON_EXTRACT: Bolt\Doctrine\Functions\JsonExtract # Why have a duplicate of the Scienta version?


I know that it used to be different. I'll check if that's still the case, or if we can fall back to the original. I'll get back to you on that.

OK, looked it up: The Scienta versions (like src/Query/AST/Functions/Mysql/JsonExtract.php) are specific for SQLite, Mysql or Postgres. Ours extends AbstractJsonFunctionNode instead, and can be used on either.

config/packages/doctrine.yaml

bobdenotter · 2020-10-04T07:33:10Z

src/.preload.php

@@ -0,0 +1,5 @@
+<?php


This file is not needed. I don't know where it came from, but I occasionally see it popping up on my end as well. Regardless, we can remove it here.

bobdenotter · 2020-10-04T07:33:25Z

src/Doctrine/JsonHelper.php

@@ -23,9 +23,19 @@ class JsonHelper
    public static function wrapJsonFunction(?string $where = null, ?string $slug = null, Connection $connection)
    {
        $version = new Version($connection);
+        //print_r($version->getPlatform()['driver_name']); # pgsql
+        //exit(print($where)); // translations_anyField.value


Let's remove commented-out code

Oh, sidenote: instead of print_r, use dump(). It's so much nicer. ;-)

https://symfony.com/doc/current/components/var_dumper.html#dump-examples-and-output

src/Doctrine/JsonHelper.php

bobdenotter · 2020-10-04T07:34:38Z

templates/helpers/_field_blocks.twig

@@ -43,11 +43,14 @@
 {% block extended_fields %}

    {# Special case for 'select' fields: if it's a multiple select, the field is an array. #}
+    {# I dont know what is going on here, but field.selectedIds for pgsql showcase/mr-jean-feeney (and others) #}
+    {# yield field values, NOT integer IDs.. Also, selectedIds does not exists (try dump(field) ) #}
+    {# TODO: FIX IT, bypassing for now by setting field.id #}	


This is weird. Might be an unrelated bug, that's coming to light here.. I'll check this one out as well.

Ok, took a look at this. Not sure why it was failing on your end.. {{ field.id }} is not a good fix, because it's an entirely different ID than the one(s) we need.

It uses method getSelectedIds from src/Entity/Field/SelectField.php, so it really should exist, if the field is a SelectField.

One thing you might try is to explicitly set it to return an array instead of one item:

{% setcontent selected = field.contentType where {'id': field.selectedIds} returnmultiple %}

Should be fixed now. selectedIds function returned full (string) values, instead of (integer)IDs. So, why not select on field.value: .. instead of field.id: .. :)
Works nicely in postgres, I still have to run a test for the others.

vendor/vaimo/webdriver-binary-downloader/src/Analysers/PlatformAnalyser.php

vendor/vaimo/webdriver-binary-downloader/src/Interfaces/PlatformAnalyserInterface.php

…PHP supports SQLite since version 5.3.0

bobdenotter · 2020-10-05T10:02:20Z

src/Doctrine/Version.php

+    // JSON supported since SQLite version 3.9.0
+    public const SQLITE_WITH_JSON = '3.9.0';
+    // PHP supports SQLite since version 5.3.0
+    public const PHP_WITH_SQLITE = '5.3.0';


This is not correct. If we merge this in, Bolt will absolutely assume it has JSON support on installations where it doesn't have it.

Maybe we should change the name of the constant, because it's more "PHP_WITH_SQLITE_THAT_HAS_JSON_SUPPORT_BUNDLED". I've definitely seen this break on PHP 7.2 setups. (Ubuntu 2018.04, if i recall correctly). I'm aware that some installations on lower versions have this bundled, but we can't rely on it. Two examples are listed right above, as "Not OK".

Ideally, we could properly test if we have it, but I have not found a proper way to do that.. If you have a good suggestion, I'm all for it!

I've written a small test and implemented in a new version (e827a79). Meanwhile I'll leave the (restored) old code commented out until we've fully tested everything. Cleanup is the last step.

Wieter · 2020-10-05T13:18:13Z

@bobdenotter somehow the test fails, while if I manually perform the failed action in the test myself (by going to ../bolt/edit/44 and embedding the video) the correct output is given ("Matched embed" --> Silversun Pickups - Nightlight (Official Video), and Silversun Pickups)

For now, I'll leave it like this and work on something differently.

sidenote: it would be nice if a test can be run for database-specific implementation, i.e. postgres vs mysql.

bobdenotter · 2020-10-05T14:41:22Z

@Wieter Travis failing is a known issue. It's a bit annoying, but i've restarted the build.

We have an issue in the works, where we'll improve it: #1918

bobdenotter · 2020-10-06T14:31:14Z

Travis is extra special stubborn on this one! I'll fix the conflict and Travis checks on this PR, when i have a bit of time.. (i expect tomorrow!)

… ERROR: invalid input syntax for integer: "a voluptas possimus aut libero doloremque."") in "helpers/_field_blocks.twig".

Wieter · 2020-10-06T20:32:57Z

It seems that for PostgreSQL on my side everything is working nicely, but other users should help test the full functionality as well, as bugs might be hidden.
Once the Travis tests pass I think we can remove [WIP] and implement the merge. Perhaps mark as "experimental PostgreSQL support"?

bobdenotter · 2020-10-07T10:37:04Z

It looks like some of the tests are failing, because of the JSON check now. See: https://travis-ci.com/github/bolt/core/jobs/396066538

I'll see if I can reproduce that on my end!

bobdenotter · 2020-10-07T12:47:55Z

@Wieter Tests are passing now!

Once the Travis tests pass I think we can remove [WIP] and implement the merge. Perhaps mark as "experimental PostgreSQL support"?

That sounds like the prudent thing to do! Let's roll with that.

Once again, thank you for your contribution on this feature!

Wieter · 2020-10-07T15:31:20Z

@bobdenotter You're welcome!
I see that while we are struggling with a new Ziggo modem (working now all day from a mobile hotspot..), you managed to fix the last bugs! Great!

Spomsoree · 2023-02-08T09:24:14Z

src/Entity/FieldTranslation.php

@@ -21,7 +21,7 @@ class FieldTranslation implements TranslationInterface
     */
    private $id;

-    /** @ORM\Column(type="json") */
+    /** @ORM\Column(type="json", options={"jsonb": true}) */


@Wieter do you remember if this is part of the fix, or a improvement made on the fly?

Because this is causing trouble for me with mysql: #3281

WellinqScience1337 added 11 commits September 29, 2020 13:54

Freebsd-Installable

6b700c7

Fixtures can be loaded (fix of bolt#1905)

16ef756

Bypassing lower-case search bug: BUG SQLSTATE[42883]: Undefined funct…

22dafba

…ion: 7 ERROR: function lower(jsonb) does not exist

Missing assets due to wrong order in README

b9934e6

Bugfix for admin login to log. SQLSTATE[42601]: Syntax error: 7 ERROR…

b070b4a

…: syntax error at or near user - user is a protected name and need quotation!

Fixed RandomSelect for records in PostgreSQL

ed7396d

Ran Code Style checking / Static Analysis

ad555b2

Fixed search function for PostgreSQL and jsonb

f85fd41

Fixed the search/CAST for MySQL - JSON is already considered TEXT in …

2707e71

…MySQL

found strange bug, partial fix

c68e6d5

Minor cleanup and fix

4729e76

Wieter changed the title ~~WIP: Development postgres~~ [WIP] Development postgres Oct 2, 2020

Lower is breaking more than gaining

4ac9bb8

Fully postgres/mysql compatible

d9c2312

bobdenotter requested changes Oct 4, 2020

View reviewed changes

WellinqScience1337 added 4 commits October 4, 2020 22:29

Some cleanup

d3b2e2b

Further cleanup & fix of showcase editability in postgres

fea65d4

style fixes

cc659b2

Fix search bug SQLite that appeared since inclusion in JSONwrapper - …

f873aef

…PHP supports SQLite since version 5.3.0

bobdenotter reviewed Oct 5, 2020

View reviewed changes

WellinqScience1337 added 2 commits October 5, 2020 12:21

Trying to get test to pass for SQLite in Docker

d7ab4a0

improved sqlite json checking

e827a79

bobdenotter and others added 2 commits October 6, 2020 20:31

Merge branch 'master' into development-postgres

3bbb957

cleanup

39490e4

Fix for returned bug: SQLSTATE[22P02]: Invalid text representation: 7…

89fa4b7

… ERROR: invalid input syntax for integer: "a voluptas possimus aut libero doloremque."") in "helpers/_field_blocks.twig".

bobdenotter added 2 commits October 7, 2020 14:05

Don't overwrite $searchTerm

0ec56d4

Don't use Bolt\Doctrine\Version

0d0f50d

bobdenotter added the new feature label Oct 7, 2020

bobdenotter changed the title ~~[WIP] Development postgres~~ Initial PostgreSQL support Oct 7, 2020

bobdenotter approved these changes Oct 7, 2020

View reviewed changes

bobdenotter merged commit b6ba7a1 into bolt:master Oct 7, 2020

bobdenotter mentioned this pull request Oct 7, 2020

Postgresql json operators #1460

Closed

andysh-uk mentioned this pull request Oct 8, 2020

Setcontent with order by causes SQL exception in 4.1 #1971

Open

bobdenotter mentioned this pull request Oct 9, 2020

Setup not working with postgres #1905

Closed

Wieter mentioned this pull request Oct 28, 2020

Ordering with PostgreSQL backend #2069

Open

Spomsoree reviewed Feb 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial PostgreSQL support #1933

Initial PostgreSQL support #1933

Wieter commented Oct 2, 2020

bobdenotter commented Oct 2, 2020

Wieter commented Oct 2, 2020

bobdenotter commented Oct 3, 2020

bobdenotter left a comment

bobdenotter Oct 4, 2020

Wieter Oct 4, 2020

bobdenotter Oct 5, 2020

Wieter Oct 5, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

bobdenotter Oct 4, 2020

Wieter Oct 5, 2020

bobdenotter Oct 5, 2020 •

edited

Loading

Wieter Oct 5, 2020

Wieter commented Oct 5, 2020

bobdenotter commented Oct 5, 2020

bobdenotter commented Oct 6, 2020

Wieter commented Oct 6, 2020

bobdenotter commented Oct 7, 2020

bobdenotter commented Oct 7, 2020

Wieter commented Oct 7, 2020

Spomsoree Feb 8, 2023

Initial PostgreSQL support #1933

Initial PostgreSQL support #1933

Conversation

Wieter commented Oct 2, 2020

bobdenotter commented Oct 2, 2020

Wieter commented Oct 2, 2020

bobdenotter commented Oct 3, 2020

bobdenotter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobdenotter Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wieter commented Oct 5, 2020

bobdenotter commented Oct 5, 2020

bobdenotter commented Oct 6, 2020

Wieter commented Oct 6, 2020

bobdenotter commented Oct 7, 2020

bobdenotter commented Oct 7, 2020

Wieter commented Oct 7, 2020

Choose a reason for hiding this comment

bobdenotter Oct 5, 2020 •

edited

Loading