fix: EXPOSED-669 transform() broken for entities instantiated via wrapRow through an alias

[obabichevjb]

Description

ResultRow could be converted into entity via EntityClass::wrapRow() method in the following way:

val tableAlias = TransformTable.alias("alias")
val entity = tableAlias.selectAll().map { TransformEntity.wrapRow(it, tableAlias) }.first()

But if the table has transformed columns it could fail, because internally wrapRow uses function ResultRow::createAndFillValues() that expects unwrapped values, but wrapRow calls it with wrapped values.

It happens because wrapRow firstly reads the values from ResultRow and received wrapped values (ResultRow does not allow to receive unwrapped values).

The solution is to add one more method ResultRow::createAndFillFromWrappedValues() that would allow to create ResultRow from wrapped values.

---

Type of Change

Please mark the relevant options with an "X":

• Bug fix

Affected databases:

• All

---

Related Issues

EXPOSED-669 transform() broken for entities instantiated via wrapRow through an alias

[bog-walk]

In my opinion, adding a new ResultRow function to the public API isn't needed and adds potential noise & not much value.
If this is added, then the original createAndFillValues() will only be used in 1 location in the code base, and its argument would actually be provided by unwrapColumnValues() first anyway (inside InsertStatement).

I believe that users rely directly on these ResultRow rarely, if at all.
So the way I look at it is, how would I explain to someone the difference between createAndFillValues and createAndFillFromWrappedValues?
Ok, 1 unwraps any wrapped values first before doing the exact same thing.
But if they then ask me why createAndFillValues is used nowhere, then I don't have an answer. Because the usage in InsertStatement.processResults() would be the same as calling this new function, right?

That makes me ask whether we maybe need to be calling unwrapColumnValues(data) inside createAndFillValues() instead of introducing this.
But if you think that's potentially a bad idea, then I think it would make more sense to duplicate unwrapColumnValues() in exposed-dao and use it there instead.
In my opinion, this is a case where duplicating 4 lines of internal logic is cleaner than adding this new member to the public API.

Alternatively, maybe there could be more value to making unwrapColumnValues() public, so it could be used across both modules. Could it maybe be worthwhile for users to access this if they want to perform custom logic on their statement arguments, for example?

What do you think about any of the 3 alternatives?

[obabichevjb]

Thank you for the comment,

I also started from duplication of unwrapColumnValues insed DAO, and also was not sure if I should add new public function I added.

I remember that it was not good idea to update existing createAndFillValues because it's used inside InsertStatement with the result of returnedValues() method, that returns unwrapped values. If createAndFillValues also unwraps values it would be necessary to make extra wrap/unwrap cycle for these values.

So I'll return to the variant of copying the unwrapColumnValues method into DAO module. It would keep the API cleaner.