feat: add UUIDs to columns#2598
Conversation
Would not implement it now as it is not required currently. If necessary for an auto-update of a table without specifying local ids, this can be always added alter on easily. |
|
I guess that uuid should be unique across table (we need add unique id for table_id+uuid). Here use-cases that we need to support: Table update between instance
Table duplocation a single NC instanceA second scenario is especially important for end users who do not have separate environments:
|
Yupp, this scenario is exactly what are having in mind.
This would also be covered, as importing always requires a target table. So having columns with the same UUID across different tables is intentional. Hence importing, so the plan, would only affect the columns on the targeted table. |
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
| private function assignUuid(): void { | ||
| if ($this->uuid !== null) { | ||
| throw new \RuntimeException('This column already has a UUID, they are immutable'); | ||
| } | ||
| $this->uuid = Uuid::v7()->toRfc4122(); | ||
| } | ||
|
|
||
| protected function setUuid(string $uuid): void { | ||
| if ($this->uuid !== null) { | ||
| throw new \RuntimeException('This column already has a UUID, they are immutable'); | ||
| } | ||
| $this->uuid = $uuid; | ||
| } | ||
|
|
There was a problem hiding this comment.
QBMapper::insert() only writes fields returned by getUpdatedFields(). So we need to call markFieldUpdated('uuid') ?
| $this->uuid = Uuid::v7()->toRfc4122(); | ||
| } | ||
|
|
||
| protected function setUuid(string $uuid): void { |
There was a problem hiding this comment.
do we ever call this? and is it being protected useful. Cuz when the mapper reads a row, Entity::fromRow() runs (Entity.php:50):
phpforeach ($row as $key => $value) {
$prop = $instance->columnToProperty($key);
$instance->setter($prop, [$value]); // <-- calls the generic setter()
}It calls the generic setter() method directly with the property name 'uuid'. i don't think it calls setUuid()? So the custom method and its RuntimeException are skipped?
| * | ||
| * @psalm-import-type TablesColumn from ResponseDefinitions | ||
| * | ||
| * @method string getUuid() |
There was a problem hiding this comment.
Since the property is ?string $uuid
| * @method string getUuid() | |
| @method ?string getUuid() |
| public function jsonSerialize(): array { | ||
| return [ | ||
| 'id' => $this->id, | ||
| 'uuid' => $this->uuid, |
There was a problem hiding this comment.
do we need to update ResponseDefinitions.php?
There was a problem hiding this comment.
In ColumnService.php, we call new Column(). Do we have to do assignUuid() there?
| 'notnull' => false, | ||
| 'default' => null, | ||
| 'length' => 36, | ||
| 'comment' => 'UUIDv7 identifier to support structural updates across instances', |
There was a problem hiding this comment.
so it will be nice to have a a UNIQUE index?
Base for structural updates, cf. my comment in #2504 (comment)