Bug in field length handling for CONNECT engine

In November 2020, github user Buggynours introduced an if statement that did not previously exist, and branches on field_length >= 0.

https://github.com/MariaDB/server/commit/28af4212b6e2afe1d42729c9c36215ed8a8d38cb#diff-df4382972e2df64f7429b4c4b2037ddeff0c1d6970a5e13b4f98f7995c928cf0

	if (fp->field_length >= 0) {

		pcf->Length = fp->field_length;

		// length is bytes for Connect, not characters

		if (!strnicmp(chset, "utf8", 4))

			pcf->Length /= 3;

	} else {

		pcf->Length= 256;            // BLOB?

However, field_length is defined as uint32_t, and has been for at least 21 years.

Since this variable is unsigned, the if statement will always take the true branch. It will never take the false branch.

• What is the right behaviour here?
• Is the current version correct? ("Always take the true branch" → we should delete the nonsense false branch)
• Or is there some real case where the false branch should be taken but which is currently being ignored? (The false branch comment "BLOB?" strongly suggests yes)
  • If so, this is a bug, and it has been merged into 10.2.37+, 10.3.28+, 10.4.18+, 10.5.9+, 10.6., 10.7., etc.
• Or something else?

This was caught by building with -Wtype-limits.