nextras / orm

Orm with clean object design, smart relationship loading and powerful collections.
https://nextras.org/orm
MIT License
310 stars 57 forks source link

Allow persistance without unnecessary columns #195

Closed dasim closed 7 years ago

dasim commented 7 years ago

Hi, I’m new to this library and have two questions. Couldn’t find answer in docs, issues or Nette forum so hopefully I’m not wasting your time.

Let’s say I have these two entities with relation 1:M (one user can have multiple phones, one phone has only one owner).

<?php

namespace App\Model;

/**
 * @property int         $id         {primary}
 * @property User        $owner      {m:1 User::$phones}
 * @property string      $name
 * @property DateTime    $createdWhen
 * @property DateTime    $lastUpdatedWhen
 */
class Phone extends \Nextras\Orm\Entity\Entity
{
}

/**
 * @property int         $id      {primary}
 * @property string      $email
 *
 * @property OneHasMany|Phone[]  $phones            {1:m Phone::$owner}
 */
class User extends \Nextras\Orm\Entity\Entity
{
}

I have a phone table defined like this, note the datetime columns default values:

CREATE TABLE `phone` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `owner_id` int(10) unsigned NOT NULL,
  `name` varchar(250) COLLATE utf8_czech_ci,
  `created_when` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `last_updated_when` datetime DEFAULT NULL ON UPDATE CURRENT_TIMESTAMP,
  PRIMARY KEY (`id`),
  KEY `owner_id` (`owner_id`),
  CONSTRAINT `phone_ibfk_owner` FOREIGN KEY (`owner_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_czech_ci;

When inserting a new Phone, is there a way to do it similar to this?

$phone = new Phone();
$phone->name = $name;
$phone->description = $description;
$phone->owner = $ownerId;
$orm->phones->persistAndFlush($phone);

Because I have two problems that I’m searching solution for:

  1. I have to either set default values for created_when and last_updated_when here or in entity definition, although if they would be omitted completely, MySQL would handle the values just fine. I get Nextras\Orm\InvalidStateException: Property App\Model\Phone::$createdWhen is not set.. It can be fixed with @property DateTime $createdWhen {default now} but that seems rather conter-productive since MySQL would handle it on its own.

  2. The owner property doesn’t like to be given ID but expects this: $phone->owner = $orm->users->getById($ownerId);. I get it but it leads to one unnecessary query (from user table) and I can’t find a way to make it work without it. When I try something like $map->owner = $ownerId; I get Nextras\Orm\InvalidStateException: Entity is not attached to repository.

Thanks for any advice or clarification.

hrach commented 7 years ago
  1. ... seems rather conter-productive since MySQL would handle it on its own.

You are right, you have set the default in PHP. Basically, the ORM doesn't force you to use MySQL or other SQL storage and it is independent from it. That's why some functionality is duplicated on PHP side. (default values, type validation, cascade remove). I consider such behavior as a correct one.

  1. ... it leads to one unnecessary query ...

You are correct again and this is actually expected behavior. Orm does not have some semi-loaded entities as Doctrine. (EntityManager::getReference()). This may change in future, but for now I see no major trouble with this. Do you have some real impact on performance because of this?

dasim commented 7 years ago

Basically, the ORM doesn't force you to use MySQL or other SQL storage and it is independent from it. ... I consider such behavior as a correct one.

Me too and I understand why it makes sense as a default behaviour. I’d just like to have an option whether they are required for persistance operations (insert/update) or not. Since I believe majority of use cases are connected with MySQL/Postgres/relational DBs, it would make sense to me to make it easier to use their advantages.

Orm does not have some semi-loaded entities as Doctrine.

I am honestly no expert in ORMs. What I see as a developer is that Relationships make it easy to fetch related records but it don’t respect the fact that those relationships are (IMO in majority of use cases) defined as a link by a single key. That’s why I’d expect to be able to supply the key during persistance operations, not the whole fetched record.

Since there is already a “magic” behind loading the related records through @property definitions, it’d be nice to see it work the other way as well for more effective persistance.

Do you have some real impact on performance because of this?

No, I can live with this, obviously it’s not some major bottleneck. For now. Because that’s what I thought during my last huge project in the beginning when it was impossible to foresee all the effects of a technology choices 3 years later :) But I asked mostly for the purpose of learning. And providing feedback.

Thanks!

JanTvrdik commented 7 years ago

Hi @dasim, The issue with using default values from database is that e.g. (new Phone)->createdWhen will not work (unless we somehow fetch those default values from MySQL schema when building the entity metadata).

I think that better solution is to have a tool which would sync database schema with entity definitions.

hrach commented 7 years ago

To the second poit, you may do something like

$phone = new Phone();
$this->phones->attach($phone);
$phone->name = $name;
$phone->description = $description;
$phone->owner = $ownerId;
$orm->phones->persistAndFlush($phone);

It will also make a query into db, but you don't have to do it manually.

hrach commented 7 years ago

Closing. Both cases are currently considered as valid bahavior. Thanks for the feedback!