[3.0] Replace count_posts (where 0 = true) with posts_count (where 1 = true)

[Sesquipedalian]

Fixes #9415

As explained here, the count_posts column of the boards table uses 0 for true and 1 for false, and it has been doing this since way back in SMF 1.0.

This is insane.

Code that was newly written for SMF 3.0 has been built on the reasonable assumption that 1 means true and 0 means false, whereas old code that was ported over from SMF 2.1 without significant changes still assumes that count_posts uses 0 for true and 1 for false. This inconsistency is what ultimately caused #9415, because it created inconsistent handling of member's post counts.

Rather than patching the new code in order to perpetuate the madness, I have instead decided to fix all the old code in order to end this lunacy once and for all.

In order to avoid confusion in the mind of any developer looking at this code in the future, I have decided to completely replace the old count_posts column with a new posts_count column that uses sane logic. This change means that all existing installs of SMF 3.0 will need be upgraded by running the upgrader after this PR has been merged.

In order to maintain backward compatibility with any old mods that expected the old count_posts column and its inverted logic, I have also added some code to the database API classes that transparently replaces any references to the old count_posts column in query strings with references to the new posts_count column.

[albertlast]

I have tested this on both engines, and the diagnosis and the fix are right.

I reproduced #9415 on release-3.0 first, so I had something to compare against. It is exactly the inversion you describe — in a board that counts posts, starting a topic and replying both add nothing, deleting the reply is a 500 with id_post_group_1, and in a board that does not count posts the count goes up instead:

== a board that counts posts ==                          release-3.0
  FAIL  starting a topic added one post   -- was 0, now 0
  FAIL  replying added one post           -- was 0, now 0
  FAIL  deleting the reply did not error  -- 500 …action=deletemsg…
  FAIL  nothing was logged -- The database value you're trying to insert
                              does not exist: id_post_group_1
== a board that does not count posts ==
  FAIL  starting a topic added nothing    -- was 0, now 1

On this branch the same driver is 16/16 on MySQL and on PostgreSQL, both from a fresh install and after the upgrader.

I then swept the areas around it over real HTTP rather than trusting the grep: the admin board form round trip, creating a board, the board report, movetopic2 in both directions, quick-moderation moves, recounting member posts, approving and unapproving with post moderation on, recycle and restore, and the profile stats panel. 44 of 46 checks pass on each engine, and both failures are pre-existing — they behave the same on release-3.0 — so they are filed separately as #9518 and #9520.

On whether you got all of them: yes, for SMF's own code. Nothing outside the deliberate compatibility code and Db/Schema/v2_1/ still reads the old column, and the 'count_posts' => '!posts_count' alias returns the inverted value correctly both as a property and as an array key. The MySQL migration is right too — I ran it and the values inverted as intended.

Four things I do not think hold up, though.

---

1. backcompatFixes() rewrites comparisons into invalid SQL

$new_col is never imported into the closure (Sources/Db/APIs/MySQL.php:2966, and the same line at PostgreSQL.php:2840):

'/\b' . $old_col . '\s*(!=|<(?:=|>)?|=|>=?)\s*([01])\b/' => function ($m) {   // no use ($new_col)
    ...
    return $new_col . ' ' . $m[1] . ' ' . ((int) !$m[2]);
},

So with $backward_compatibility = 1, every comparison an old mod writes comes back as a syntax error plus a warning:

in:  SELECT id_board FROM {db_prefix}boards WHERE count_posts = 0
out: SELECT id_board FROM smf_boards WHERE = 1
     Warning: Undefined variable $new_col in .../MySQL.php on line 2966

The second pattern only works because it is an arrow function, which captures by value automatically.

2. <= is not inverted

The match has '>=' => '<=' twice and no '<=' arm, so the one case that needs flipping most obviously falls through to default:

count_posts <= 0   ->   posts_count <= 1      (always true; should be >= 1)

The other four operators are correct.

3. The shim never fires at all under SSI on MySQL

MySQL::initiate() rewrites the prefix to `smf`.smf_ when SMF == 'SSI', and the pattern's leading \b cannot match at a backtick preceded by a space, so preg_match() fails and nothing is rewritten:

Db::$db->prefix under SSI: '`smf`.smf_'
  FAIL  count_posts is still rewritten
        got: SELECT id_board FROM `smf`.smf_boards WHERE count_posts = 0

4. Db::$db->insert() is not covered

insert() calls quote() on the VALUES fragment only, which has no table name in it for the pattern to find, and then passes the assembled statement to query() with security_override, which is exactly the condition that skips quote(). So an old mod inserting a board gets:

Database Error: Unknown column 'count_posts' in 'field list'

---

Two smaller notes on the same shim: a rewritten SELECT list loses the old name entirely (SELECT count_posts becomes SELECT posts_count with no AS count_posts), so $row['count_posts'] is simply gone for the mod reading it — and if it were aliased it would still be carrying the flipped value. And an unqualified column beside an aliased table (SELECT id_board FROM {db_prefix}boards AS b WHERE count_posts = 0, which is legal SQL) is left alone, because $old_col has become b.count_posts.

---

5. The migration cannot run on PostgreSQL

addColumn() there emits a bare ADD COLUMN, then leaves not_null and default to change_column() — and in PostgreSQL SET DEFAULT does not backfill existing rows, so the constraint arrives while every row is still NULL:

ERROR:  column "posts_count" of relation "smf_boards" contains null values
STATEMENT:  ALTER TABLE smf_boards
                ALTER COLUMN posts_count SET NOT NULL

It leaves the table with both columns and a NULL posts_count, and it is not re-runnable — a second attempt fails at the same statement, so the forum is stuck half-migrated and unusable, since the new code reads a column that is NULL for every board. Doing the UPDATE before the column is made NOT NULL would sidestep it. The root cause is in the schema layer rather than in your migration (the recurring-events migration dies the same way), so I have filed it as #9519 — but as it stands this PR cannot be upgraded onto on PostgreSQL. Once I finished the migration by hand, everything else on PostgreSQL was green.

---

Two last things, both minor: in Board::modify() the round trip recomputes $params['posts_count'] from count_posts unconditionally, so a modern mod that sets posts_count in integrate_modify_board has its change silently discarded; and the matching round trip in Msg::remove() is dead code, because that hook takes $row by value.

php-cs-fixer is clean on all 17 files.

[jdarwood007]

Any reason we shouldn't include a property hook for legacy calls?

	public bool $count_posts{
		get {
			return !$this->posts_count;
		}
	}
}

[albertlast]

Retested at a3a0fa2, on both engines, and also against a real 2.1 forum this time — the committed SMF 2.1.7 baseline from #9330, 403 members and 6 000 messages across 24 boards, restored and then upgraded.

Four of the five things I raised are fixed. There is one new problem that stops the branch running at all, and one regression in the alias path.

---

The branch does not boot: backcompatInsertFixes() returns the wrong type

It is declared : string and returns an array on both paths, including the early one (MySQL.php:2992, PostgreSQL.php:2866). Since insert() calls it unconditionally, every insert throws — with $backward_compatibility = 0 just as much as with it on:

front page: 500

SMF\Db\APIs\MySQL::backcompatInsertFixes(): Return value must be of type string,
array returned in /var/www/html/Sources/Db/APIs/MySQL.php:2995
  #0 backcompatInsertFixes('smf_background_...', Array, Array, Array)

Logging in is enough to hit it. : array is all it wants.

The insert fix can never match its table

if ($table === $this->prefix . '_boards') {

$table has already had {db_prefix} replaced at that point, so this compares smf_boards against smf__boards. With the underscore removed and the return type corrected, Db::insert() with count_posts works and stores the inverted value, which is what it was for.

Aliased comparisons are now rewritten into nonsense

Making the alias prefix optional turned it into a capturing group:

$old_col = (!empty($matches[1]) ? '(' . $matches[1] . '\.)?' : '') . 'count_posts';

so in the aliased case $m[1] is the alias, the operator has moved to $m[2] and the value to $m[3] — while the callback still reads $m[1] as the operator and $m[2] as the value:

b.count_posts  = 0   ->   b.posts_count b. 0
b.count_posts != 0   ->   b.posts_count b. 0
b.count_posts  > 0   ->   b.posts_count b. 0
b.count_posts <= 0   ->   b.posts_count b. 0
   count_posts  = 0   ->   b.posts_count 0        (operator dropped)

Every operator, and the unqualified form beside an aliased table as well. The unaliased case still works, because then no group is built and the numbering is unchanged — which is why the first batch of checks passes. Aliasing the boards table as b is what SMF's own queries have always done, so it is the form old mods copied. (?:…)? restores the numbering.

The same optional group has a side effect worth a thought: a mod's own table is now caught whenever a boards table appears anywhere in the query, since the alias is optional but $new_col is not.

SELECT m.count_posts FROM {db_prefix}my_mod_table AS m
    INNER JOIN {db_prefix}boards AS b ON (b.id_board = m.id_board)

->  SELECT m.b.posts_count FROM smf_my_mod_table AS m INNER JOIN smf_boards AS b ...

---

What is fixed

The closure captures $new_col, the <= arm is right, and moving to Config::$db_prefix makes the pattern match under SSI, where the prefix is database-qualified. All eight operators come out correct in the unaliased form, through a literal and through {int:…} alike, and a mod's own count_posts in a query with no boards table is still left alone.

With the two blockers patched locally

• MySQL and PostgreSQL, fresh installs: 16/16 on the [3.0] Deleting a reply returns HTTP 500: id_post_group_1 is missing from the query parameters #9415 reproduction, 44/46 across the admin board form, board creation, the board report, movetopic2 both ways, quick-moderation moves, recounting member posts, approve/unapprove under post moderation, recycle and restore, and the profile stats panel. The two failures are the pre-existing ones in [3.0]: Member post counts drift in four places outside the create and delete path #9518 and [3.0]: Two fatals: the board index with unapproved posts, and the CLI installer #9520.
• The migration is right on real 2.1 data. Restoring the 2.1.7 baseline, setting boards 2 and 5 to count_posts = 1 and upgrading gives posts_count = 0 for exactly those two and 1 for the other 22, NOT NULL DEFAULT 1, with messages and per-member post counts untouched (6 000 and 5 999, identical member by member). The drivers then pass on the upgraded forum too.
• The same holds on PostgreSQL once the schema layer is fixed — isCandidate() goes false afterwards and the values invert correctly.

One thing to know about that last point: on PostgreSQL the upgrader cannot currently reach this migration at all. PostgreSQL::change_column() applies SET NOT NULL whenever the key is set even when its value is false, and SET DEFAULT does not backfill the rows that are already there, so posts_count fails its own constraint and the table is left holding both columns with the new one null. Details and the other blockers on that path are in #9519; the engine-independent ones are #9521. None of that is yours to fix here, but it does mean this migration cannot land on PostgreSQL until the schema layer is sorted.