V5 behavior change how it handles creating relationship entity

[mskocik]

First of all, I am not raising this as an issue, because I am not sure it's an issue. Depending on point of view I guess 🤷.

There is an function HasOne::createEntity():

protected function createEntity($entity, bool $allowNull): ?IEntity
{
    // previous uninteresting code...
    } elseif (is_scalar($entity)) {
        // this has changed! From getById() to getByIdChecked()
        return $this->getTargetRepository()->getByIdChecked($entity);

    }
}

I have entity like this. It's very important to note, that column CAN BE nullable. For non-nullable columns the new (v5) behavior is desired.

/**
 * @property ?Target $target {m:1 Trend, oneSided=true}
 */
class Trend extends Entity {}

This throws an exception if expect entity cannot be fetched from DB. BUT I have use case when I create entities in collection from data fetched from clickhouse and sometimes data for this nullable column contains 0. Which is perfectly valid for my use case and expected. I show "N/A" instead of Target detail.

I ended up patching it with this:

} elseif (is_scalar($entity)) {
    if ($this->metadata->isNullable) {  // only if column is nullable.
	$entity = $this->getTargetRepository()->getById($entity);
	$entity === null && $this->setRawValue(null);	// to prevent inifinite loop
	return $entity;	
    }
			
    return $this->getTargetRepository()->getByIdChecked($entity);

}

I think it is useful to have it "not-so-strict" to provide way how to map custom DB data to "virtual" entities, but still having the benefits of auto-fetching related models. What do you think?

[hrach]

If I understand your use case correctly, you have a column that maps a relationship.
Possible values:

• null
• 0
• actual id

And you're telling me that previously the 0 behaved the same as null, right?

Imo that goes against consistency and the way db works - you should have set up a foreign key and that would prevent that as well, wouldn't it?

[mskocik]

I never had null in this case. Under normal circumstances this goes agains consistency. By normal circumstances I mean mapping DB tables to entities. But I am talking about specific dataset from custom complex queries mapped to entities for easier "read-only" usage.

[hrach]

TBH I do not understand you much.

But let me comment on this:

Having this functionality may prevent race conditions (read of row with a foreign key, but reading relationship later won't give you that that), but such issues should be solved by setting up transaction isolation level. Not by a custom single-time non-strictness.

Next, your case is somehow special (your word), so I guess it is not a question of whether this should be built-in, right?

[mskocik]

I try to describe it more for you to understand better.

We have model entity.

/**
 * @property ?PubEntity     $pub     {m:1 PubEntity, oneSided=true}
 * @property ?DriveEntity   $drv     {m:1 DriveEntity, oneSided=true}
 */
class Trend extends Entity {}

Imagine you have some query, in which you can group dynamically by some model entities (pub, drv in this case)

query example

WITH temp AS (
  SELECT 
    sum(`mileage`) AS mileage, 
    sum(`mpg`) AS mpg, 
    sum(`spend`) AS spend, 
    `pub`, 
    `drv` 
  FROM 
    (
      SELECT 
        soid AS pub, 
        aid AS drv, 
        count() AS mileage, 
        ifNull(
          sumIf(`cmsn`, `type` = 'sold'), 
          0
        ) AS mpg, 
        ifNull(
          sumIf(`price`, `type` = 'sold'), 
          0
        ) AS spend 
      FROM 
        `logs` 
        LEFT JOIN (
          SELECT 
            `clk_id` AS p_clk_id, 
            sum(`spent`) AS p_spent, 
            sum(`total`) AS p_total
          FROM 
            `table_1` 
          WHERE 
            (
              (
                `status` = 'VALID' 
                AND `model_type` IN ('aaa', 'bbb', 'ccc')
              ) 
              AND (
                toDateTime(`click_date_time`) BETWEEN '2025-02-20 04:49:35' 
                AND '2025-02-20 09:49:01'
              )
            ) 
            AND (
              isNotNull(`trx_id`)
            ) 
          GROUP BY 
            p_clk_id
        ) p ON (`p_clk_id` = `clk_id`) 
      WHERE 
        date_time >= NOW() - INTERVAL 300 MINUTE 
      GROUP BY 
        `pub`, 
        `drv`
    ) 
  GROUP BY 
    GROUPING SETS(
      (pub, drv), 
      (pub)
    )
) 
SELECT 
  `pub`, 
  `drv`, 
  `mileage`, 
  `mpg`, 
  `spend`, 
  `d1_mileage`, 
  `d1_mpg`, 
  `d1_spend` 
FROM 
  temp 
  LEFT JOIN (
    SELECT 
      `pub` AS d1_pub, 
      `mileage` AS d1_mileage, 
      `mpg` AS d1_mpg, 
      `spend` AS d1_spend 
    FROM 
      temp 
    WHERE 
      isNotNull(`pub`) 
      AND isNull(`drv`)
  ) d1 ON (`d1_pub` = `pub`) 
WHERE 
  isNotNull(`pub`) 
  AND isNotNull(`drv`) 
ORDER BY 
  d1_mileage DESC, 
  pub ASC, 
  mileage DESC, 
  drv ASC 
LIMIT 
  30 OFFSET 0
  

Then you use this dataset to create entities in extended DbalCollection. Standard stuff...

And some arrays can look like this:

$arrayData = [
    'mileage' => 123,
    'mpg' => 1234,
    'spend' => 12345,
    'pub' => 0,   // 0 which should map to null for PubEntity
    'drv' => 100
];

When showing entity listing in grid, ORM correctly fetches all records from PubEntity table, performing query SELECT * FROM pub_table WHERE id IN (1,2,3,4,5,0). Notice 0 is here. Returns only 5 records, because there is no record with id 0.

Then when given row (with pub=0) is being rendered, the exception is being thrown. Because entity with given (invalid) id is not present in identity map from previous request, it tries to fetch it again with repo->getByIdChecked(0) which fails as expected. Worked in v4.

Next, your case is somehow special (your word), so I guess it is not a question of whether this should be built-in, right?

Honestly I don't know. That's why I opened this as a discussion and not an issue. This case shouldn't happen with standard ORM usage.

But we sometimes work against real clickhouse DB for reading with conjuction with writable local dev mysql DB. And this can/is happening too - FK mapping to records which are not in dev DB, because dev DB isn't updated all the time. This was working for us in v4.

I am just sharing my quite edge use case. But (as I am writing this now) probably it could be solved by some custom modifier extending default ManyHasOne, but I haven't though of that before.