Skip to content

Add string field for date columns and ColumnCodeProducer cleanup#122

Open
mringler wants to merge 3 commits intoperplorm:mainfrom
mringler:temporal_columns_as_string
Open

Add string field for date columns and ColumnCodeProducer cleanup#122
mringler wants to merge 3 commits intoperplorm:mainfrom
mringler:temporal_columns_as_string

Conversation

@mringler
Copy link
Copy Markdown
Collaborator

@mringler mringler commented Mar 16, 2026

Currently, temporal columns are stored as DateTimeInterface objects, which are created when the model object is hydrated.

This PR stores the value as string, the DateTimeInterface object is only created on access and stored in a second object property. Using the date string as default means fewer object have to be created (as long as the date field is not accessed), and it provides a value for binding to prepared statements without having to call DateTimeInterface::format().

Splitting database fields into a raw value and a deserialized value attribute is used for all columns with serialization (like types OBJECT, BINARY_SET, ARRAY), and I used the occasion to unify that code into a new AbstractDeserializableColumnCodeProducer, which all those CodeProducers extend now.

Changing the way time columns work affected code in ObjectBuilder, which I split up and moved into the corresponding CodeProducers (like ObjectBuilder::addClear(), ObjectBuilder::addHydrateBody()).

Also, the ColumnCodeProducers now use a method that creates the whole variable access string ('$this->my_field'), which makes template code much more readable:

        $stringAttribute = $this->getAttributeName();
        $arrayAttribute = $this->getDeserializedAttributeName();

        $script .= "
        if ($arrayAttribute === null) {
            $arrayAttribute = !$stringAttribute ? [] : static::unserializeArray($stringAttribute);
        }";

instead of

        $clo = $this->column->getLowercasedName();
        $cloUnserialized = $clo . '_unserialized';

        $script .= "
        if (!\$this->$cloUnserialized && \$this->$clo !== null) {
            \$this->$cloUnserialized = !\$this->$clo ? [] : static::unserializeArray(\$this->$clo);
        }";

I disabled a broken sniffer rule from Spryker, it does not recognize that a param string|null $param is nullable (only ?string $param is considered nullable). This is an issue in spryker/code-sniffer/Spryker/Sniffs/Commenting/DocBlockParamAllowDefaultValueSniff.php.


Psalm currently has a bug where it complains about the #[\Override] attribute on functions (see bug report), causing CI to fail.

@mringler
Copy link
Copy Markdown
Collaborator Author

This breaks BC when a new DateTimeInterface object is set and then its value is changed:

$date = new DateTime(...);
$row->setDate($date);
$date->setTime(...); // <----- will not change actual value in $row, but did before
$row->save();

Note that neither version stores such a change when the object from the row is used:

$row = RowQuery::create()->findOne();
$row->getDate()->setTime(...);
$row->save(); // field is not marked as changed, so nothing happens 

@mringler mringler changed the base branch from develop to main March 17, 2026 17:45
@mringler mringler force-pushed the temporal_columns_as_string branch from af93a42 to b3ae361 Compare March 30, 2026 18:45
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant