Search ranking for number of views fails in PostgreSQL

Right. And I think this is the part of the query that's generated inside of the SearchQuery class, not the parts specifically generated in NodeSearch from the hook_rankings() stuff from Statistics and other modules.

I think we're going to have to revisit the really convoluted code in SearchQuery that adds the scale to the rankings stuff. We should also look at
#2194403: Remove unnecessary cast to float from comment_ranking() and statistics_ranking()
in PostgreSQL and see if we can clean all of that code up, plus making sure it works in PostgreSQL. It's really... weird queries and weird code. To say the least.

I am working on a patch for this issue.

OK, I don't get it.

What's happening here is that statistics_ranking() is setting a "score" expression of:

'2.0 - 2.0 / (1.0 + node_counter.totalcount * :statistics_scale)'

and then it is adding an argument:

       'arguments' => array(':statistics_scale' => (float) \Drupal::state()->get('statistics.node_counter_scale') ?: 0),

However, when PostgreSQL substitutes that argument into the query, for some reason it is going in as '0.2' instead of the number 0.2.

I do not understand why that would be. Any ideas?

I did some further debugging.

In my test site with manual testing, I made some content and set Search to use Number of Views.

I'm getting pretty much the same error that comes up in the tests, with a slightly different number for the scale coming through:

PDOException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "0.5" LINE 1: ...SCE((2.0 - 2.0 / (1.0 + node_counter.totalcount * '0.5')), 0... ^ in PDOStatement->execute() (line 61 of core/lib/Drupal/Core/Database/Statement.php).

The full query in the error is this, as reported above:

SELECT i.langcode AS langcode, i.type AS type, i.sid AS sid, SUM(:multiply_0 * COALESCE((2.0 - 2.0 / (1.0 + node_counter.totalcount * :statistics_scale)), 0) / :total_0) AS calculated_score
FROM 
{search_index} i
INNER JOIN {node_field_data} n ON n.nid = i.sid
INNER JOIN {search_total} t ON i.word = t.word
LEFT JOIN {node_counter} node_counter ON node_counter.nid = i.sid
INNER JOIN {search_dataset} d ON i.sid = d.sid AND i.type = d.type
WHERE  (n.status = :db_condition_placeholder_0) AND( (i.word = :db_condition_placeholder_1) )AND (i.type = :db_condition_placeholder_2) AND( (d.data ILIKE :db_condition_placeholder_3) )
GROUP BY i.type, i.sid, i.langcode
HAVING  (COUNT(*) >= :matches) 
ORDER BY calculated_score DESC
LIMIT 10 OFFSET 0

So in core/lib/Drupal/Core/Database/Query::arguments(), I put in some debugging and I verified that in the arguments array that this function returns, the argument called ':statistics_scale' is definitely a float value (not a string).

I also looked into the database code, and it looks to me like once arguments() is called to get the substitution list, it is just passed into the PDO execute() method for final substitution.

So I still don't get it. Why would PDO decide that this particular argument should be substituted in as a string?

It looks like if you embed a numeric placeholder deep inside a query expression like this, the PostgreSQL PDO driver simply gets confused and it might sometimes put in a string, and although PostgreSQL can sometimes do automatic casts, it doesn't always work out.

So the only solution appears to be to put a CAST into the SQL, which will negate any possible problems with this.

Here's a patch. I'm going to run all the Search tests with this patch installed and see if they work, while I set the bot loose on the MySQL tests.

Oops, I missed a ) in comment_ranking() in the score expression. This should fix it. I had tested manually on PostgreSQL with the statistics ranking turned on, but not comment ranking, which I broke with the patch.

So, with this new patch, all of the Search module tests pass in PostgreSQL except for one that is a separate and unrelated issue already.

And by the way, this patch also gets rid of the whole idea of the kludgy code that had in the past caused problems with international users not being able to use search, if they had "," as their decimal point. This was fixed in 8 but it was still a kludge; it's slightly less so now.

Great!

Just a note for whoever commits this: It will conflict with #2101183: Move {comment_entity_statistics} to proper service, but this issue has better code in it that will work with PostgreSQL. We need to make sure the better code here eventually gets into the new class being created for comment statistics on that other issue.

OK.

OK, here's a new patch. The only difference is that I expanded the code comments in comment.module and statistics.module. Interdiff also attached.

Good point.

Unassigning because I know some potential committers ignore issues in the RTBC queue assigned to me.