AbstractLengthColumn subclasses throw ValueError when rendered without a length

[simon-mundy]

Package Version

0.5.0 (also 0.6.x-dev @ e037464a and the 0.6.x head)

Php Version

8.4

Database Engine

MySQL

Basic Information

Every AbstractLengthColumn subclass accepts ?int $length = null, but render one without a length and vsprintf() throws a ValueError.

Affected: Varchar, Char, Binary, Varbinary, Floating, Double, Decimal — every AbstractLengthColumn/AbstractPrecisionColumn subclass except Text/Blob, which only escape because they override $specification to %s %s and drop the length entirely.

Cause: AbstractLengthColumn.php#L14 fixes the spec to '%s %s(%s)', while #L53-L55 only splices the length Literal in when the length expression is non-empty and not '0'. Placeholder count and value count disagree whenever the length is null or 0.

test/unit/Sql/Ddl/Column/VarcharTest.php::testGetExpressionDataWithNullLength currently pins exactly this mismatch (three placeholders, two values), and its own comment says "need to verify actual behavior". No column unit test anywhere calls getSqlString(), which is why the suite has never noticed.

Steps to Reproduce

1. Build a table with a length-less column:

   $table = new CreateTable('x');
   $table->addColumn(new Column\Varchar('v'));

2. $table->getSqlString($platform);

Expected Behavior

A rendered column or a typed exception, decided per type. Proposed: build the spec at render time (%s %s(%s) with Literal((string) $length) at index 2 when a length is set, %s %s otherwise), then:

• Varchar, Varbinary: MySQL requires a length (VARCHAR NOT NULL is 1064). Throw InvalidArgumentException at render, or make $length required in those two constructors — a BC break, but one worth taking before 1.0.
• Char, Binary: MySQL defaults to length 1; render without parentheses (CHAR NOT NULL executes).
• Floating, Double, Decimal: precision is optional; render without parentheses (DECIMAL NOT NULL executes).

That policy is MySQL-driven (verified on 8.4.10, reproduced on 8.0.46). PostgreSQL accepts a bare VARCHAR, so the Varchar/Varbinary rule is really a MySQL constraint enforced in core — happy to leave that to the platform if preferred.

Test plan:

• new Char('c') renders `c` CHAR NOT NULL; new Decimal('d') renders `d` DECIMAL NOT NULL; new Floating('f', 10) still renders FLOAT(10); new Decimal('d', 10, 2) still renders DECIMAL(10,2).
• new Varchar('v') and new Varbinary('v') fail with a typed exception naming the column, or can't be constructed without a length.
• VarcharTest::testGetExpressionDataWithNullLength asserts the fixed behaviour; each affected class gets a "no length" test that calls getSqlString(), not just getExpressionData().
• AbstractPrecisionColumn::getLengthExpression() returns '' when both digits and decimal are null. The decimal-only case currently renders ,2; cover that too.

Actual behavior?

ValueError: The arguments array must contain 3 items, 2 given
  AbstractSql.php:164  vsprintf('%s %s(%s) NOT NULL', [Identifier('v'), Literal('VARCHAR')])

Additional Info

Reproduced on 0.5.x, e037464a and the 0.6.x head. From an internal DDL audit (not published — the finding is reproduced above). The Integer display-width position bug (#178) and the Check constraint bug (#177) are filed separately.