mringler
commented
2 years ago Hey wundii,
that is an interesting issue. I think you ran into a structural problem in Propel here.
In general, it is desired behavior that Propel gives you the same object for the same row. You do not want different objects representing the same row in the database, because then a change to the first object is not reflected in the other object, meaning that if you change the first object and save it, then save object 2, the changes you made through object 1 will be overridden (it is a bit more complicated in Propel due to change detection, but you get the idea). This is not desirable. Disabling instance pooling or cloning objects is almost guaranteed to give you the weirdest anomalies somewhere down the line.
I think the "correctish" behavior of withColumn() would be to return an array in your case. Seems to me the result of these statements should be the same:
$authorCollection1 = AuthorQuery::create()
->filterById(2949)
->joinWithBook() // <-------- join whole table
->find();
var_export($authorCollection1->count()); // 1
var_export($authorCollection1[0]->getBooks()->getColumnValues('Id')); // [3554, 3555]
$authorCollection2 = AuthorQuery::create()
->filterById(2949)
->joinBook()->withColumn('Book.Id', 'BookIdsOnly') // <------------- join column of table
->find();
var_export($authorCollection2->count()); // 2 <------ debatable
var_export($authorCollection2[0]->getVirtualColumn('BookIdsOnly')); // 3555 <----------- not 3554, issue of ticket, but I think should be [3554, 3555],
var_export($authorCollection2[1]->getVirtualColumn('BookIdsOnly')); // 3555Throwing an exception that withColumn() does not work with a one-to-many relation would also be a valuable change.
On a side note, this here makes sense considering above, but without context, it is somewhat confusing:
$authorCollection3 = AuthorQuery::create()
->useBookQuery()
->filterByAuthorId(2949)
->endUse()
->find();
var_export($authorCollection3->count()); // 2
var_export($authorCollection3[0]->getBooks()->getColumnValues('Id')); // [3554, 3555],
var_export($authorCollection3[1]->getBooks()->getColumnValues('Id')); // [3554, 3555],Not sure if it makes sens that a Query object can return more than one object for a row in the table. Results with duplicates are usually not usable.
tl;dr I think cloning the result is not a good idea, changing withColumn() to return an array is better, but just throwing an exception would also be a valuable change. What do you think?
codecov-commenter
wundii
dereuromark
At this point, problems arise if there is a one to n relationship to its table (tableOne -> tableN). If you build a query from tableOne and now add a withColumn(virtualColumn), all the different ones at tableN will get the same content because the same ObjectId is always fetched by the InstancePoolTrait. Since in my view the ObjectFormatter->format is only used for the find methods, a clone would be a good bug fix for one to n relationships.
user1 (baseball & football) user2 (baseball)
UserOneQuery::create()->useHobbyManyQuery()->endUse()->withColumn(HobbyManyTableMap::COL_HOBBY)->find(); result: -user1 | football -user1 | football -user2 | baseball
** Update I have found an alternative to the clone, Propel::disableInstancePooling(). I am now a little unsure which is the better approach for the problem described.