PostgreSQL fixes

(note: edited from my original for clarity/readability)

Thank you for your patch.

----

On the second fix, shouldn't an error be coming back if content_id is empty? Or, to ask another way, when is it the case that it is valid for content_id to be empty? Maybe my memory is rusty on the code.

----

On the first fix, you point out a SQL statement that may have seriously needed some looking at. Thank you.
I think, however, your fix doesn't work -- it gets rid of the error, but it breaks the purpose of the line:

sql = "SELECT * FROM (SELECT DISTINCT x.hostname, x.timestamp, COUNT(x.hostname) AS count FROM (SELECT timestamp, hostname FROM {spam_tracker} WHERE score > %d ORDER BY timestamp DESC) AS x GROUP BY x.hostname, x.timestamp) AS y WHERE y.count >= %d";

will, since the chance is nearly 0% that anything will have an identical timestamp to something else, make everything have count = 1 and thereby effectively turn off the blacklist (while slowing everything down with an extra GROUP BY that matches everything individually).

The original:

sql = "SELECT * FROM (SELECT DISTINCT x.hostname, x.timestamp, COUNT(x.hostname) AS count FROM (SELECT timestamp, hostname FROM {spam_tracker} WHERE score > %d ORDER BY timestamp DESC) AS x GROUP BY x.hostname) AS y WHERE y.count >= %d";

is better fixed by adding a MAX aggregate function to the timestamp (and removing the now-redundant ORDER BY and probably never actually necessary DISTINCT clauses), fixing your problem while returning the same thing as the MySQL would (which I believe your code would not):

sql = "SELECT * FROM (SELECT x.hostname, MAX(x.timestamp), COUNT(x.hostname) AS count FROM (SELECT timestamp, hostname FROM {spam_tracker} WHERE score > %d) AS x GROUP BY x.hostname) AS y WHERE y.count >= %d";

Then the inner SQL simplifies to one SELECT clause, so the final statement would be:

sql = "SELECT * FROM (SELECT hostname, MAX(timestamp), COUNT(hostname) AS count FROM {spam_tracker} WHERE score > %d GROUP BY hostname) AS y WHERE y.count >= %d";

That final SQL string should work in both MySQL and Postgres, and additionally execute a good deal faster than the first version.

Wooooah. Bigger problem with the content_id string vs. integer issue.

Recall that my first question was, "why would the content_id ever be blank?" What I was getting at was -- the problem isn't that it's typed as a string. SQL will know to clobber it to an integer anyway. The problem is that there are cases where it would be blank, and it should *never* be blank -- and that's the problem that needs fixing.

Well, look at the line itself:

- db_query("UPDATE {spam_tracker} SET score = %d, hostname = %d, timestamp = %d WHERE content_type = '%s' AND content_id = '%s'", $score, time(), $type, $id);
+ db_query("UPDATE {spam_tracker} SET score = %d, hostname = %d, timestamp = %d WHERE content_type = '%s' AND content_id = %d", $score, time(), $type, $id);


I pasted both, because both the original version and your version have the same problem.

score = %d
hostname = %d
timestamp = %d
content_type = %s
content_id = %s or %d

That's five replacement variables.

And yet the replacements are: $score, time(), $type, $id.

content_id is blank because it's *not getting a value*. Ever.

Looking at it that way, it's pretty obvious that between $score and time(), you need the hostname passed. (That, and the hostname should be a string replacement, not an integer.) That'll fix your original problem.