Skip to content

Initial PostgreSQL support #1933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6b700c7
Freebsd-Installable
WellinqScience1337 Sep 29, 2020
16ef756
Fixtures can be loaded (fix of #1905)
WellinqScience1337 Sep 29, 2020
22dafba
Bypassing lower-case search bug: BUG SQLSTATE[42883]: Undefined funct…
WellinqScience1337 Sep 29, 2020
b9934e6
Missing assets due to wrong order in README
WellinqScience1337 Sep 29, 2020
b070b4a
Bugfix for admin login to log. SQLSTATE[42601]: Syntax error: 7 ERROR…
WellinqScience1337 Sep 29, 2020
ed7396d
Fixed RandomSelect for records in PostgreSQL
WellinqScience1337 Sep 29, 2020
ad555b2
Ran Code Style checking / Static Analysis
WellinqScience1337 Sep 29, 2020
f85fd41
Fixed search function for PostgreSQL and jsonb
WellinqScience1337 Sep 29, 2020
2707e71
Fixed the search/CAST for MySQL - JSON is already considered TEXT in …
WellinqScience1337 Sep 30, 2020
c68e6d5
found strange bug, partial fix
WellinqScience1337 Sep 30, 2020
4729e76
Minor cleanup and fix
WellinqScience1337 Oct 2, 2020
4ac9bb8
Lower is breaking more than gaining
WellinqScience1337 Oct 2, 2020
d9c2312
Fully postgres/mysql compatible
WellinqScience1337 Oct 3, 2020
d3b2e2b
Some cleanup
WellinqScience1337 Oct 4, 2020
fea65d4
Further cleanup & fix of showcase editability in postgres
WellinqScience1337 Oct 5, 2020
cc659b2
style fixes
WellinqScience1337 Oct 5, 2020
f873aef
Fix search bug SQLite that appeared since inclusion in JSONwrapper - …
WellinqScience1337 Oct 5, 2020
d7ab4a0
Trying to get test to pass for SQLite in Docker
WellinqScience1337 Oct 5, 2020
e827a79
improved sqlite json checking
WellinqScience1337 Oct 5, 2020
3bbb957
Merge branch 'master' into development-postgres
bobdenotter Oct 6, 2020
39490e4
cleanup
WellinqScience1337 Oct 6, 2020
89fa4b7
Fix for returned bug: SQLSTATE[22P02]: Invalid text representation: 7…
WellinqScience1337 Oct 6, 2020
0ec56d4
Don't overwrite `$searchTerm`
bobdenotter Oct 7, 2020
0d0f50d
Don't use `Bolt\Doctrine\Version`
bobdenotter Oct 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


# MYSQL / MariaDB (additional settings, needed for Docker)
#DATABASE_USER=db_user
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,7 @@ appveyor.yml
###> phpunit/phpunit ###
/phpunit.xml
###< phpunit/phpunit ###

###> .preload dev ###
/src/.preload.php
###< .preload dev ###
25 changes: 13 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,19 @@ bin/console doctrine:fixtures:load -n

Alternatively, run `make db-reset`, on a UNIX-like system.

4 Run the prototype
4 How to build assets
-------------------

To set up initially, run `npm install` to get the required dependencies /
`node_modules`. Alternatively give the path to the python executable (`npm install --python="/usr/local/bin/python3.7"`) Then:
- Prepare directory structure `mkdir -p node_modules/node-sass/vendor`
- Rebuild npm environment for current OS `npm rebuild node-sass`
- Run `npm run start` (alternatively `npm run start --python="/usr/local/bin/python3.7"`)

See the other options by running `npm run`.
(Note: as I'm testing this as well remotely, I copied all assets from the released composer installation by `cp -r ../www_backup/public/assets/* public/assets/`)

5 Run the prototype
-------------------

- Using the Symfony CLI tool, just run `symfony server:start`.
Expand All @@ -134,17 +146,6 @@ You can log on, using the default user & pass:
- pass: `admin%1`


How to build assets
-------------------

To set up initially, run `npm install` to get the required dependencies /
`node_modules`. Then:

- Run `npm run start`

See the other options by running `npm run`.


Code Style checking / Static Analysis
----------------------------

Expand Down
3 changes: 2 additions & 1 deletion config/packages/doctrine.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ doctrine:
dql:
string_functions:
JSON_EXTRACT: Bolt\Doctrine\Functions\JsonExtract
CAST: DoctrineExtensions\Query\Mysql\Cast
JSON_GET_TEXT: Scienta\DoctrineJsonFunctions\Query\AST\Functions\Postgresql\JsonGetText
CAST: Bolt\Doctrine\Query\Cast
numeric_functions:
RAND: Bolt\Doctrine\Functions\Rand

4 changes: 4 additions & 0 deletions src/Doctrine/Functions/Rand.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public function getSql(\Doctrine\ORM\Query\SqlWalker $sqlWalker)
if (property_exists($this->expression, 'value') && $this->expression->value === '1') {
return 'random()';
}
// value is two if PostgreSQL. See Bolt\Storage\Directive\RandomDirectiveHandler
if (property_exists($this->expression, 'value') && $this->expression->value === '2') {
return 'RANDOM()';
}

return 'RAND()';
}
Expand Down
9 changes: 8 additions & 1 deletion src/Doctrine/JsonHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ public static function wrapJsonFunction(?string $where = null, ?string $slug = n
$version = new Version($connection);

if ($version->hasJson()) {
$resultWhere = 'JSON_EXTRACT(' . $where . ", '$[0]')";
//PostgreSQL handles JSON differently than MySQL
if ($version->getPlatform()['driver_name'] === 'pgsql') {
// PostgreSQL
$resultWhere = 'JSON_GET_TEXT(' . $where . ', 0)';
} else {
// MySQL and SQLite
$resultWhere = 'JSON_EXTRACT(' . $where . ", '$[0]')";
}
$resultSlug = $slug;
} else {
$resultWhere = $where;
Expand Down
54 changes: 54 additions & 0 deletions src/Doctrine/Query/Cast.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

declare(strict_types=1);

namespace Bolt\Doctrine\Query;

use Doctrine\ORM\Query\AST\Functions\FunctionNode;
use Doctrine\ORM\Query\Lexer;
use Doctrine\ORM\Query\Parser;
use Doctrine\ORM\Query\SqlWalker;

class Cast extends FunctionNode
{
/** @var \Doctrine\ORM\Query\AST\PathExpression */
protected $first;
/** @var string */
protected $second;
/** @var string */
protected $backend_driver;

public function getSql(SqlWalker $sqlWalker): string
{
$backend_driver = $sqlWalker->getConnection()->getDriver()->getName();

// test if we are using MySQL
if (mb_strpos($backend_driver, 'mysql') !== false) {
// YES we are using MySQL
// how do we know what type $this->first is? For now hardcoding
// type(t.value) = JSON for MySQL. JSONB for others.
// alternatively, test if true: $this->first->dispatch($sqlWalker)==='b2_.value',
// b4_.value for /bolt/new/showcases
if ($this->first->dispatch($sqlWalker) === 'b2_.value' ||
$this->first->dispatch($sqlWalker) === 'b4_.value') {
return $this->first->dispatch($sqlWalker);
}
}

return sprintf('CAST(%s AS %s)',
$this->first->dispatch($sqlWalker),
$this->second
);
}

public function parse(Parser $parser): void
{
$parser->match(Lexer::T_IDENTIFIER);
$parser->match(Lexer::T_OPEN_PARENTHESIS);
$this->first = $parser->ArithmeticPrimary();
$parser->match(Lexer::T_AS);
$parser->match(Lexer::T_IDENTIFIER);
$this->second = $parser->getLexer()->token['value'];
$parser->match(Lexer::T_CLOSE_PARENTHESIS);
}
}
61 changes: 50 additions & 11 deletions src/Doctrine/Version.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,31 @@
use Doctrine\DBAL\Driver\PDOConnection;
use Doctrine\DBAL\Platforms\MariaDb1027Platform;
use Doctrine\DBAL\Platforms\MySQL57Platform;
use Doctrine\DBAL\Platforms\PostgreSQL92Platform;
use Doctrine\DBAL\Platforms\SqlitePlatform;

class Version
{
/**
* We're g̶u̶e̶s̶s̶i̶n̶g̶ doing empirical research on which versions of SQLite
* support JSON. So far, tests indicate:
* https://www.sqlite.org/json1.html --> JSON since SQLite version 3.9.0 (2015-10-14)
* Docker uses an outdated version of SQLite/PHP combi with wrong implementation of JSON1 extensions:
* https://www.talvbansal.me/blog/unit-tests-with-json-columns-in-sqlite/
* This explains why BOLT is working in stand-alone production/dev envs. but not in unit tests using Docker
* We need to replace this with a proper function test, instead of a guestimate.
* - 3.32.2 - OK (Wytse's FBSD 12.1 \w PHP 7.2)
* - 3.20.1 - Not OK (Travis PHP 7.2)
* - 3.27.2 - OK (Bob's Raspberry Pi, running PHP 7.3.11 on Raspbian)
* - 3.28.0 - OK (Travis PHP 7.3)
* - 3.28.0 - Not OK (Bob's PHP 7.2, installed with Brew)
* - 3.29.0 - OK (MacOS Mojave)
* - 3.30.1 - OK (MacOS Catalina)
*/
// JSON supported since SQLite version 3.9.0
public const SQLITE_WITH_JSON = '3.27.0';
public const PHP_WITH_SQLITE = '7.3.0';
// PHP supports SQLite since version 5.3.0, but not always bundled with JSON support!
public const PHP_WITH_SQLITE_JSON_SUPPORT = '7.3.0';

/** @var Connection */
private $connection;
Expand Down Expand Up @@ -79,14 +88,23 @@ public function hasJson(): bool
$platform = $this->connection->getDatabasePlatform();

if ($platform instanceof SqlitePlatform) {
return $this->checkSqliteVersion();
// new method to test for JSON support
return $this->hasSQLiteJSONSupport();

// temporarily leave this in, until above method is fully tested
// return $this->checkSqliteVersion();
}

// MySQL80Platform is implicitly included with MySQL57Platform
if ($platform instanceof MySQL57Platform || $platform instanceof MariaDb1027Platform) {
return true;
}

// PostgreSQL supports JSON from v9.2 and above, later versions are implicitly included
if ($platform instanceof PostgreSQL92Platform) {
return true;
}

return false;
}

Expand All @@ -104,19 +122,40 @@ public function hasCast(): bool
return true;
}

private function checkSqliteVersion(): bool
/* leave until alternative method fully tested */
// private function checkSqliteVersion(): bool
// {
// /** @var PDOConnection */
// $wrapped = $this->connection->getWrappedConnection();

// // If the wrapper doesn't have `getAttribute`, we bail…
// if (! method_exists($wrapped, 'getAttribute')) {
// return false;
// }

// [$client_version] = explode(' - ', $wrapped->getAttribute(\PDO::ATTR_CLIENT_VERSION));

// return (version_compare($client_version, self::SQLITE_WITH_JSON) > 0) &&
// (version_compare(PHP_VERSION, self::PHP_WITH_SQLITE_JSON_SUPPORT) > 0);
//}

private function hasSQLiteJSONSupport(): bool
{
/** @var PDOConnection $wrapped */
$wrapped = $this->connection->getWrappedConnection();
// Here we can test SQLite for JSON support
// This should also work for MySQL
// For PostgreSQL a different query is required, but this should be easy to expand (check for driver + adjust query)
// Query = "SELECT JSON_EXTRACT('{"jsonfunctionalitytest":["succes"]}', '$.jsonfunctionalitytest') as value";
// Should return value ["succes"], and throw an error if JSON unsupported.

// If the wrapper doesn't have `getAttribute`, we bail…
if (! method_exists($wrapped, 'getAttribute')) {
try {
$query = $this->connection->createQueryBuilder();
$query
->select('JSON_EXTRACT(\'{"jsonfunctionalitytest":["succes"]}\', \'$.jsonfunctionalitytest\') as value');
$query->execute();
} catch (\Throwable $e) {
return false;
}

[$client_version] = explode(' - ', $wrapped->getAttribute(\PDO::ATTR_CLIENT_VERSION));

return (version_compare($client_version, self::SQLITE_WITH_JSON) > 0) &&
(version_compare(PHP_VERSION, self::PHP_WITH_SQLITE) > 0);
return true;
}
}
2 changes: 1 addition & 1 deletion src/Entity/FieldTranslation.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FieldTranslation implements TranslationInterface
*/
private $id;

/** @ORM\Column(type="json") */
/** @ORM\Column(type="json", options={"jsonb": true}) */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

protected $value = [];

public function getId(): ?int
Expand Down
2 changes: 1 addition & 1 deletion src/Entity/Log.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Log
/** @ORM\Column(name="extra", type="array", nullable=true) */
private $extra;

/** @ORM\Column(name="user", type="array", nullable=true) */
/** @ORM\Column(name="`user`", type="array", nullable=true) */
private $user;

/** @ORM\Column(type="content", type="integer", nullable=true) */
Expand Down
6 changes: 5 additions & 1 deletion src/Repository/ContentRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,14 @@ public function searchNaive(string $searchTerm, int $page, int $amountPerPage, C
$qb = $this->getQueryBuilder()
->select('partial content.{id}');

// proper JSON wrapping solves a lot of problems (added PostgreSQL compatibility)
$connection = $qb->getEntityManager()->getConnection();
[$where] = JsonHelper::wrapJsonFunction('t.value', $searchTerm, $connection);

$qb->addSelect('f')
->innerJoin('content.fields', 'f')
->innerJoin('f.translations', 't')
->andWhere($qb->expr()->like('t.value', ':search'))
->andWhere($qb->expr()->like($where, ':search'))
->setParameter('search', '%' . $searchTerm . '%');

// These are the ID's of content we need.
Expand Down
3 changes: 2 additions & 1 deletion src/Storage/Directive/OrderDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ private function setOrderBy(QueryInterface $query, string $order, string $direct
} else {
// Note the `lower()` in the `addOrderBy()`. It is essential to sorting the
// results correctly. See also https://github.com/bolt/core/issues/1190
// again: lower breaks postgresql jsonb compatibility, first cast as txt
$query
->getQueryBuilder()
->addOrderBy('lower(' . $translationsAlias . '.value)', $direction);
->addOrderBy('lower(CAST(' . $translationsAlias . '.value as TEXT))', $direction);
}
$query->incrementIndex();
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/Storage/Directive/RandomDirectiveHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public function __invoke(QueryInterface $query, $value, &$directives): void

return;
}
if ($this->version->getPlatform()['driver_name'] === 'pgsql') {
$query->getQueryBuilder()->addSelect('RAND(2) as HIDDEN rand')->orderBy('rand');

return;
}

$query->getQueryBuilder()->addSelect('RAND(0) as HIDDEN rand')->orderBy('rand');
}
Expand Down
4 changes: 3 additions & 1 deletion src/Storage/SelectQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,9 @@ private function getRegularFieldExpression(Filter $filter, EntityManagerInterfac

$originalLeftExpression = 'content.' . $filter->getKey();
// LOWER() added to query to enable case insensitive search of JSON values. Used in conjunction with converting $params of setParameter() to lowercase.
$newLeftExpression = JsonHelper::wrapJsonFunction('LOWER(' . $valueAlias . ')', null, $em->getConnection());
// BUG SQLSTATE[42883]: Undefined function: 7 ERROR: function lower(jsonb) does not exist
// We want to be able to search case-insensitive, database-agnostic, have to think of a good way..
$newLeftExpression = JsonHelper::wrapJsonFunction($valueAlias, null, $em->getConnection());
$valueWhere = $filter->getExpression();
$valueWhere = str_replace($originalLeftExpression, $newLeftExpression, $valueWhere);
$expr->add($valueWhere);
Expand Down
5 changes: 4 additions & 1 deletion templates/helpers/_field_blocks.twig
Original file line number Diff line number Diff line change
Expand Up @@ -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 (strings containing full name!), NOT integer IDs.. #}
{# TODO: FIX IT, bypassing for now by selecting on 'value' (string) and not 'id' (int) #}
{% if type == "select" and field is not empty %}
<p><strong>{{ field|label }}: </strong></p>
<ul>
{% if field.contentSelect %}
{% setcontent selected = field.contentType where {'id': field.selectedIds} returnmultiple %}
{% setcontent selected = field.contentType where {'value': field.selectedIds} returnmultiple %}
{% for record in selected %}
<li><a href="{{ record|link }}">{{ record|title }}</a></li>
{% endfor %}
Expand Down