Fix PostgreSQL operator in views

Replacing occurrences of db_like with Database::getConnection()->escapeLike($string) in Views module.

Submitting new patch

Patch #6 is applying successfully.

Added my new patch

+++ b/core/modules/views/src/Plugin/views/display/EntityReference.php
@@ -122,7 +124,7 @@ public function query() {
+      $value = Database::getConnection()->escapeLike($options['match']) . '%';

Wouldn't it be better to inject the database service into the plugin via ContainerFactoryPluginInterface?

Likewise for other instances. I'm not familiar with enough with views plugins to know if there is a significant performance impact to use the injection interface instead of tightly-coupling the Database class.

re-roll

addresses #13 except in EntityReference class because of special handling in DisplayPluginBase: parent::__construct(array(), $plugin_id, $plugin_definition)

#21.1 done
#21.2 addWhere uses Connection::mapConditionOperator during \Drupal\Core\Database\Query\Condition->compile() and addWhereExpression doesn't.

Perhaps pointing out to use addWhereExpression with Connection::mapConditionOperator would be helpful for contrib.

Thanks slasher13 and chanderbhushan. I've re-rolled patch 22 for 8.3.x.

As patch was not applying so i rerolled it for 8.4.x.

Thank you for attempting a re-roll, @Meenakshi Gupta. The patch in #28 seems to not have all the changes in #26. Did you run into conflicts applying all of the changes in #26? A helpful way to determine what changes occurred between patches is to create an "interdiff" between 2 patches. You can learn more about how to create an interdiff at https://www.drupal.org/documentation/git/interdiff.

For a re-roll, an interdiff probably won't show any changes, but if you do see changes, then it's possible your new patch is missing changes from the old patch. It is not necessary to upload the interdiff file. However if we need to add/remove or make other changes in our new patch, then an interdiff should be posted in order to help patch reviewers.

I'm at DC Vienna and having a look at this.

Patch re-rolled for 8.5-x

Thank you for the patch, @miiimooo.

I found a couple of minor code standard issues with the patch.

Also, it seems like code in patch from #26 and #28 is not in the patch in #35, and that code still applies. You should try to apply the changes from #28 as well.

@@ -23,6 +24,43 @@ class StringFilter extends FilterPluginBase {
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
+      $container->get('database')
+      );

The ending parentheses should be indented at 4 spaces instead of 6 spaces.

@@ -236,8 +274,27 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
     }
   }
 
+  /**
+   * Overwrite equal operator for complex expressions.
+   *
+   * @return string
+   *   Overwritten, database driver specific operator.
+   */

I think the convention here is to use {@inheritdoc} for methods that are extended.

Thanks for the review @mradcliffe

I had more time now and managed to incorporate all the above patches.

Fix tests

Patch for version 8.6.x,8.5.x and 8.4.x, hope it shows green :)

What is Vienna2017 and CSKyiv18 to which this issue is tagged in ?

I think we can remove those tags as corresponding events are already past(obvious for DrupalCon, proof for CSKiyv18 https://groups.drupal.org/node/517964).

We should keep issue tags that are related to sprints because it is useful for sprint organizers to see a list of issues that were worked on during a sprint.

Related #2850037: Replace all calls to db_like(), which is deprecated

Patch #46 looks good, thanks for the patch. Marking RTBC.

There should be a failing test at least on Postgres as this change claims to be a bugfix for something in Postgres. Also the issue summary should be updated to reflect what is being fixed.

What's also odd is that we're mixing a task - replacing db_like() with a bug fix. I think this issue should only do the necessary to "Fix PostgreSQL operator"... the "and replace db_like wherever possible in views" should be done by #2850037: Replace all calls to db_like(), which is deprecated

This should solve the issue.

Reroll #56 patch for 8.7.x

+++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
@@ -2,8 +2,10 @@
+use Drupal\Core\Database\Connection;
...
+use Symfony\Component\DependencyInjection\ContainerInterface;

unused use - just remove it

Reroll #56 patch for 8.7.x and added changes with Dependency Injection from #46 patch.

I'm adding the Needs tests tag based on @alexpott in comment #55. I think that we should add a kernel test to assert the failure.

The issue title and summary mentions replacing db_like() and like @alexpott mentioned this should not be done in this issue. I added the Needs issue summary update tag because we should clarify the proposed resolution and remaining tasks. Not everyone is knowledgeable about PostgreSQL so it would be great to add an explanation of the bug. This would probably require some digging and background research.

Updated IS. Reverted changes that are out of scope.
Still requires IS update to clarify the proposed resolution and remaining tasks.

Forgot about the title update.

https://www.drupal.org/pift-ci-job/1066793
11 coding standards messages

I can't open it, but use of short array syntax might be one of them.

addressed #70

Last patch was commited as part of db_like() conversion so only extra test coverage here makes sense

See https://cgit.drupalcode.org/drupal/commit/?id=a0608f0

In this patch mostly changed constructions like

db_like($this->value)
on:
$this->connection->escapeLike($this->value)

Looks like the #74 patch can no longer be applied.
There's a new one.

The patch looks good. Just a couple of minor points:

1. +++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
   @@ -300,8 +300,24 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   +  /**
   +   * {@inheritdoc}
   +   */
      public function operator() {
   

   Where does the inherit comes from? I cannot find it.

2. In the method Combine::opContainsWord() is the method mapConditionOperator() called. Can we there use the new method getConditionOperator().

3. +++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
   @@ -300,8 +300,24 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   +  public function getConditionOperator($operator) {
   

   Does this "helper" method need to be public. Can we change it to a protected method?

4. +++ b/core/modules/views/src/Plugin/views/filter/Combine.php
   @@ -133,13 +133,14 @@ public function validate() {
   -    $this->query->addWhereExpression($this->options['group'], "$expression $operator $placeholder", [$placeholder => $this->value]);
   ...
   +    $this->query->addWhereExpression($this->options['group'], "$expression $operator $placeholder", [$placeholder => '%' . $this->connection->escapeLike($this->value) . '%']);
   

   Adding the $this->connection->escapeLike() should not be part of this patch. It is out of scope for this issue.

Addressed #82.1,#82.3,#82.4
For #82.2, In #80, It is already using the new method getConditionOperator.

Working on the tests.

Added tests and also fixed a number of "LIKE"'s in StringFilter.php

Updated the IS.

Updated the patch for the comment #87 from @lendude.
Also added a tests only patch.

I reviewed the latest patch. The usage of the getConditionOperator() method is clean and shouldn't pose any risk.

I found one nitpick code standard issue in that patch.

+++ b/core/modules/views/src/Plugin/views/filter/StringFilter.php
@@ -299,8 +299,28 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
+   *   Condition operator.
+   * @return string

Nit: There should be a separate line between @param and @return annotations

[x] Separate the @param and @return sections by a blank line

Fixed the coding standard violation.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!