This issue is part of #2454513: [meta] Make Drupal 8 work with SQLite.

Drupal\system\Tests\Entity\EntityQueryTest is currently failing on SQLite due to case sensitivity differences with MySQL.

SQLite does not have a native ILIKE operator (which could be used as a base for case-insensitive LIKE queries), so we need to overload the non-standard GLOB operator for case-sensitive matching. Another option would have been to overload another non-standard operator, MATCH, but that one does not support the NOT keyword prefix.

CommentFileSizeAuthor
#16 interdiff.txt840 bytesamateescu
#16 2475187-16.patch2.55 KBamateescu
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,667 pass(es). View
#8 interdiff.txt1.86 KBamateescu
#8 2475187-8.patch2.37 KBamateescu
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,362 pass(es). View
#3 2475187-3.patch3.11 KBamateescu
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,691 pass(es). View
#1 2475187.patch3.09 KBamateescu
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2475187.patch. Unable to apply patch. See the log in the details link for more information. View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

amateescu’s picture

Status: Active » Needs review
FileSize
3.09 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2475187.patch. Unable to apply patch. See the log in the details link for more information. View

This was not trivial to figure out, basically we have to implement SQL LIKE in regex in order to make everything work properly :)

Without the patch:

Test summary
------------

Drupal\system\Tests\Entity\EntityQueryTest                   131 passes             1 exceptions             

Test run duration: 12 sec

...

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY000]: General
error: 1 near ":db_condition_placeholder_0": syntax error: SELECT
base_table.revision_id AS revision_id, base_table.id AS id
FROM 
{entity_test_mulrev} base_table
INNER JOIN {entity_test_mulrev__field_cs} entity_test_mulrev__field_cs ON
entity_test_mulrev__field_cs.entity_id = base_table.id
WHERE  (entity_test_mulrev__field_cs.field_cs_value LIKE BINARY
:db_condition_placeholder_0) ; Array
(
    [:db_condition_placeholder_0] => o4wtt5ha%
)

With the patch applied:

Test summary
------------

Drupal\system\Tests\Entity\EntityQueryTest                   141 passes                                      

Test run duration: 12 sec

Status: Needs review » Needs work

The last submitted patch, 1: 2475187.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review
FileSize
3.11 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,691 pass(es). View

Rerolled on latest HEAD.

Damien Tournoud’s picture

Unfortunately, this is going to force SQLite to skip the indexes... Do we have a lot of prefix-based LIKE queries?

amateescu’s picture

You mean NOT LIKE queries? Probably not so many. But the LIKE overloading from the patch is more about having proper case-insensitivity handling for non-ASCII characters.

If we prefer to not care about that, we can just keep the GLOB overloading for case-sensitive LIKE BINARY queries.

Damien Tournoud’s picture

No, I mean prefix selectors like field LIKE "prefix%", those can be satisfied by a range on an index starting with field. Having a custom implementation will prevent SQLite from doing so.

amateescu’s picture

Ah, got it. Any entity query that uses the STARTS_WITH condition will be affected: http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Entity/Query...

Also views queries that are using the string filter: http://cgit.drupalcode.org/drupal/tree/core/modules/views/src/Plugin/vie...

amateescu’s picture

Title: Add user-space implementations for LIKE and LIKE BINARY in SQLite » Add a user-space implementation for LIKE BINARY in SQLite
FileSize
2.37 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,362 pass(es). View
1.86 KB

Ok, so let's add just the LIKE BINARY implementation, which is needed to support case-sensitive conditions. Anyone who uses SQLite should know from the start that they won't have proper UTF-8 support :/

amateescu’s picture

Assigned: Unassigned » Damien Tournoud

@DamZ, are you ok with the latest patch that doesn't change anything for LIKE?

catch’s picture

Status: Needs review » Reviewed & tested by the community

This looks great to me, and I think it resolves DamZ's concerns.

Also if we find a better solution later on, that's a great thing to do with a 100% sqlite pass rate.

We only have this problem at all because we rely on LIKE being case-insensitive on MySQL to avoid LOWER() (or extra lowercased columns in the schema), tbh even if that meant not using indexes on sqlite I'd have been OK with it, but great that abusing glob gets around this.

Moving to RTBC but leaving assigned to Damien and I won't personally commit this for a few days - someone else can though.

amateescu’s picture

Issue summary: View changes

Updated the issue summary since we're not overloading LIKE anymore.

Fabianx’s picture

RTBC + 1 - This is the last remaining SQlite issue, so would be great to get in.

dawehner’s picture

+++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php
@@ -257,6 +263,21 @@ public static function sqlFunctionRegexp($pattern, $subject) {
+    $pattern = str_replace(array('%', '_'), array('.*?', '.'), preg_quote($pattern, '/'));
+    return preg_match('/^' . $pattern . '$/', $subject);

Can we explain a bit how / why this implementation works?

webchick’s picture

I pinged Damien via e-mail to raise this issue to his attention.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

And #13 sounds like a needs work for documentation.

amateescu’s picture

Priority: Normal » Major
Status: Needs work » Needs review
FileSize
2.55 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,667 pass(es). View
840 bytes

Can we explain a bit how / why this implementation works?

I'm not sure I understand the question, do you have any concern that it will not work? Would something like this be enough for documentation?

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Looks great to me!

  • catch committed fe2eeca on
    Issue #2475187 by amateescu: Add a user-space implementation for LIKE...
catch’s picture

Assigned: Damien Tournoud » Unassigned
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

@Damien if you have comments on this, please open a follow-up and link here, but to me this is a good approach.

webchick’s picture

Yeah and worst-case it's only affecting SQLite, not any of the main DB drivers. Hooray!

Side-note: perhaps folks working on these issues would care to nominate themselves for additional PostgreSQL and/or SQLite maintainers? :D

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.