[Bug]: SchemaValidationException message lists valid columns and omits the nullable marker on the expected type

[bendavies]

What happened?

DataFrame::match() (with the default StrictValidator) produces a misleading SchemaValidationException message. When validation fails because of one genuinely-incompatible column, the message has two separate defects:

1. It lists columns the validator actually accepts. Flow\ETL\Schema\Validator\StrictValidator::isValid() has a special-case that accepts a FROM_NULL string column (e.g. one produced by withEntry('x', lit(null))) against a nullable expected column:

if (
    $expectedDefinition->isNullable()
    && $givenDefinition->metadata()->has(Metadata::FROM_NULL)
    && type_equals($givenDefinition->type(), type_string())
) {
    continue;
}

But Flow\ETL\Exception\SchemaValidationException rebuilds its "Mismatched Definitions" list using Definition::isCompatible() alone, which does not replicate that skip. So a column the validator accepted is still reported as mismatched.

2. It drops the nullable ? marker on the expected side. In the mismatched-definitions entry, the expected type is printed without the ($expectedDefinition->isNullable() ? '?' : '') prefix, whereas the given side and the missing-definitions branch both include it. A column declared datetime_schema('deleted_at', nullable: true) therefore renders as expected: deleted_at<datetime> instead of expected: deleted_at<?datetime>.

The two defects compound. The reader sees:

|-- expected: deleted_at<datetime>, given: deleted_at<?string>

and reasonably concludes there is both a type and a nullability problem - when in reality the expected definition is ?datetime and the validator already forgives the ?string via the FROM_NULL rule. deleted_at is not the real failure at all; it is noise that points debugging at the wrong column.

Expected

The message should only list columns that actually failed validation, and should render the expected nullability consistently with the given side.

Suggested fix

• Build the message from the validator's actual findings instead of recomputing with isCompatible() (or apply the same FROM_NULL skip when formatting the mismatched list).
• Add the missing ($expectedDefinition->isNullable() ? '?' : '') prefix to the expected type in the mismatched branch, matching the given side and the missing-definitions branch.

How to reproduce?

composer.json:

{
    "require": {
        "flow-php/etl": "1.x-dev"
    },
    "minimum-stability": "dev",
    "prefer-stable": true
}

repro.php (then composer install && php repro.php):

<?php

declare(strict_types=1);

require __DIR__ . '/vendor/autoload.php';

use Flow\ETL\Exception\SchemaValidationException;

use function Flow\ETL\DSL\data_frame;
use function Flow\ETL\DSL\datetime_schema;
use function Flow\ETL\DSL\from_array;
use function Flow\ETL\DSL\int_schema;
use function Flow\ETL\DSL\lit;
use function Flow\ETL\DSL\schema;
use function Flow\ETL\DSL\string_schema;

// Given data is IDENTICAL in both scenarios:
//   id          -> always a non-null integer
//   deleted_at  -> always null via lit(null) => inferred as ?string carrying FROM_NULL metadata
$read = static fn() => data_frame()
    ->read(from_array([['id' => 1], ['id' => 2]]))
    ->withEntry('deleted_at', lit(null));

// Scenario A: deleted_at (FROM_NULL) vs nullable datetime, id matches -> validator ACCEPTS it.
echo "Scenario A: deleted_at (lit(null)) vs nullable datetime, id matches\n";

try {
    $read()
        ->match(schema(
            int_schema('id'),
            datetime_schema('deleted_at', nullable: true),
        ))
        ->run();

    echo "  => match() SUCCEEDED: the validator accepts `deleted_at`.\n\n";
} catch (SchemaValidationException $e) {
    echo "  => UNEXPECTED exception:\n" . $e->getMessage() . "\n\n";
}

// Scenario B: identical data + identical deleted_at expectation; only `id` is now expected
// as string (a genuine mismatch that forces the exception).
echo "Scenario B: same data + same deleted_at expectation, but id is now expected as string\n";

try {
    $read()
        ->match(schema(
            string_schema('id'),                           // genuine mismatch: given is integer
            datetime_schema('deleted_at', nullable: true), // SAME as Scenario A -> should still be accepted
        ))
        ->run();

    echo "  => match() unexpectedly succeeded\n";
} catch (SchemaValidationException $e) {
    $message = $e->getMessage();
    echo $message . "\n";
    echo 'BUG 1 - message lists `deleted_at` even though the validator accepts it (Scenario A): '
        . (str_contains($message, 'deleted_at') ? 'YES' : 'no') . "\n";
    echo 'BUG 2 - expected side drops the nullable `?` (shows `<datetime>`, not `<?datetime>`): '
        . (str_contains($message, 'expected: deleted_at<datetime>') ? 'YES' : 'no') . "\n";
}

Data required to reproduce bug locally

Inline in the snippet: from_array([['id' => 1], ['id' => 2]]) plus withEntry('deleted_at', lit(null)). No external data needed.

Version

1.x-dev (reproduced on flow-php/etl @ 1e66019d9e74 and @ 03c69e40dfc8)

Relevant error output

Scenario A: deleted_at (lit(null)) vs nullable datetime, id matches
  => match() SUCCEEDED: the validator accepts `deleted_at`.

Scenario B: same data + same deleted_at expectation, but id is now expected as string
Schema validation failed:
  Mismatched Definitions:
    |-- expected: id<string>, given: id<integer>
    |-- expected: deleted_at<datetime>, given: deleted_at<?string>

BUG 1 - message lists `deleted_at` even though the validator accepts it (Scenario A): YES
BUG 2 - expected side drops the nullable `?` (shows `<datetime>`, not `<?datetime>`): YES

[norberttech]

I'm going to tackle this once I'm done with telemetry, thanks for detailed report, will help to reproduce it in test! 🤝