Skip to content

Conversation

@greg0ire
Copy link
Member

Its purpose is unclear since there is FieldMapping::$options['default'] already.

Its purpose is unclear since there is FieldMapping::$options['default']
already.
@stof
Copy link
Member

stof commented Nov 14, 2025

I don't think supporting $options['default'] in the FieldMapping is an expected API today (if you go that way, almost all fields of the FieldMapping end up being used to configure the DBAL column options, so you could argue that they could all be deprecated in favor of $options, but this leaks the DBAL API in the public API of the ORM.

I don't think this deprecation makes sense.

I would rather deprecate passing options for option names that are handled by SchemaTool based on the field mapping.

@greg0ire
Copy link
Member Author

greg0ire commented Nov 14, 2025

I don't think supporting $options['default'] in the FieldMapping is an expected API today

Maybe not today, but in the past, that seemed to be the only way to do it: https://stackoverflow.com/a/53224518/353612

Also, notice how the Column mapping attribute does not have a default property:

final class Column implements MappingAttribute
{
/**
* @param int|null $precision The precision for a decimal (exact numeric) column (Applies only for decimal column).
* @param int|null $scale The scale for a decimal (exact numeric) column (Applies only for decimal column).
* @param class-string<BackedEnum>|null $enumType
* @param array<string,mixed> $options
* @phpstan-param 'NEVER'|'INSERT'|'ALWAYS'|null $generated
*/
public function __construct(
public readonly string|null $name = null,
public readonly string|null $type = null,
public readonly int|null $length = null,
public readonly int|null $precision = null,
public readonly int|null $scale = null,
public readonly bool $unique = false,
public readonly bool $nullable = false,
public readonly bool $insertable = true,
public readonly bool $updatable = true,
public readonly string|null $enumType = null,
public readonly array $options = [],
public readonly string|null $columnDefinition = null,
public readonly string|null $generated = null,
public readonly bool $index = false,
) {
}
}

this leaks the DBAL API in the public API of the ORM.

Are you suggesting that people should not refer to the new DBAL value objects in their mappings?

I would rather deprecate passing options for option names that are handled by SchemaTool based on the field mapping.

I don't really understand what that means… can you please elaborate? Are you maybe talking about these:

private const KNOWN_COLUMN_OPTIONS = ['comment', 'unsigned', 'fixed', 'default', 'values'];
? And if yes, are you saying all of these should be exposed in the mappings?

In any case, I could not find documentation on this feature. I think that usually default values are derived from the default value of the property, which might explain the lack of docs. However for DateTime field, I don't think it would be possible to have some kind of magic value that we would translate later on into DBAL value objects, since you cannot use new to initialize the default value of a property

@stof
Copy link
Member

stof commented Nov 17, 2025

oh, I was not aware that FieldMapping had this default property but not Column. Then, I'm fine with that deprecation (the main public API is clearly Column there)

@greg0ire greg0ire added this to the 3.6.0 milestone Nov 19, 2025
@greg0ire greg0ire merged commit f0562f4 into doctrine:3.6.x Nov 19, 2025
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants