lecousin / lc-spring-data-r2dbc

An extension of spring-data-r2dbc to provide features such as relationships, joins, cascading save/delete, lazy loading, sequence, schema generation, composite id
Apache License 2.0
48 stars 12 forks source link

Exception when lazy loading #8

Closed chrisvettese closed 2 years ago

chrisvettese commented 2 years ago

I am getting an exception when trying to lazy load a joined table. I'm using postgres. The app is written in kotlin but I haven't seen any indication yet that the error is kotlin-related.

This entity is successfully loaded from the repository: image

Joined entity: image

When calling lazyGetShipment:

java.util.NoSuchElementException: null
    at kotlinx.coroutines.reactor.MonoKt.awaitSingle(Mono.kt:79) ~[kotlinx-coroutines-reactor-1.5.2.jar:na]
    at com.shipi.shipi_shipment.service.ShipmentService.getStoreShipments$suspendImpl(ShipmentService.kt:77) ~[main/:na]
    at com.shipi.shipi_shipment.service.ShipmentService$getStoreShipments$1.invokeSuspend(ShipmentService.kt) ~[main/:na]
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) ~[kotlin-stdlib-1.6.10.jar:1.6.10-release-923(1.6.10)]
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]

When calling loadEntity() in ShipmentHistoryEntity:

2022-03-29 02:55:56.189  WARN 8712 --- [actor-tcp-nio-1] i.r.p.client.ReactorNettyClient          : Error: SEVERITY_LOCALIZED=ERROR, SEVERITY_NON_LOCALIZED=ERROR, CODE=42703, MESSAGE=column entity.shipment does not exist, HINT=Perhaps you meant to reference the column "entity.shipment_id"., POSITION=80, FILE=parse_relation.c, LINE=3359, ROUTINE=errorMissingColumn
2022-03-29 02:55:56.199  WARN 8712 --- [atcher-worker-1] n.g.e.SimpleDataFetcherExceptionHandler  : Exception while fetching data (/storeShipments) : executeMany; bad SQL grammar [SELECT entity.id AS f0000, entity.status AS f0001, entity.updated_at AS f0002, entity.shipment AS f0003 FROM shipment_history entity WHERE entity.id = $1 LIMIT 1 OFFSET 0]; nested exception is io.r2dbc.postgresql.ExceptionFactory$PostgresqlBadGrammarException: [42703] column entity.shipment does not exist

org.springframework.r2dbc.BadSqlGrammarException: executeMany; bad SQL grammar [SELECT entity.id AS f0000, entity.status AS f0001, entity.updated_at AS f0002, entity.shipment AS f0003 FROM shipment_history entity WHERE entity.id = $1 LIMIT 1 OFFSET 0]; nested exception is io.r2dbc.postgresql.ExceptionFactory$PostgresqlBadGrammarException: [42703] column entity.shipment does not exist
    at org.springframework.r2dbc.connection.ConnectionFactoryUtils.convertR2dbcException(ConnectionFactoryUtils.java:235) ~[spring-r2dbc-5.3.14.jar:5.3.14]
    Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
    *__checkpoint ⇢ SELECT FROM ShipmentHistoryEntity AS entity WHERE entity.id EQUALS 0 LIMIT 0,1
Original Stack Trace:
        at org.springframework.r2dbc.connection.ConnectionFactoryUtils.convertR2dbcException(ConnectionFactoryUtils.java:235) ~[spring-r2dbc-5.3.14.jar:5.3.14]

The second exception is interesting to me. It indicates that it is attempting to fetch shipment from the shipment_history table, even though shipment is annotated as a foreign key to the shipment table. Is there anything I'm doing wrong?

lecousin commented 2 years ago

For the second exception, it is normal that it is attempting to retrieve the "shipment" column (but not fetching the linked entity, only the foreign id). When a field is declared as a @ForeignKey, it expects to have a column in the table representing this foreign key and making the link between the two tables.

Do you have a column "shipment" in your table "shipment_history" ? Or may be you give it another name ?

chrisvettese commented 2 years ago

So the column is shipment_id, which is a references foreign key id in the shipment table. image

I have tried renaming shipment to shipment_id as you said (pardon the bad naming practices): image

However, I am still getting exceptions. For lazyGetShipment_id:

2022-03-29 03:52:21.147  WARN 6600 --- [atcher-worker-1] n.g.e.SimpleDataFetcherExceptionHandler  : Exception while fetching data (/storeShipments) : Cannot invoke "net.lecousin.reactive.data.relational.enhance.EntityState.load(Object)" because "com.shipi.shipi_shipment.entity.ShipmentHistoryEntity.getShipment_id()._lcState" is null

java.lang.NullPointerException: Cannot invoke "net.lecousin.reactive.data.relational.enhance.EntityState.load(Object)" because "com.shipi.shipi_shipment.entity.ShipmentHistoryEntity.getShipment_id()._lcState" is null
    at com.shipi.shipi_shipment.entity.ShipmentHistoryEntity.lazyGetShipment_id(ShipmentHistoryEntity.kt) ~[main/:na]
    at com.shipi.shipi_shipment.service.ShipmentService.getStoreShipments$suspendImpl(ShipmentService.kt:77) ~[main/:na]
    at com.shipi.shipi_shipment.service.ShipmentService$getStoreShipments$1.invokeSuspend(ShipmentService.kt) ~[main/:na]
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) ~[kotlin-stdlib-1.6.10.jar:1.6.10-release-923(1.6.10)]
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665) ~[kotlinx-coroutines-core-jvm-1.5.2.jar:na]

For loadEntity:

2022-03-29 03:49:23.588  WARN 19272 --- [atcher-worker-1] n.g.e.SimpleDataFetcherExceptionHandler  : Exception while fetching data (/storeShipments) : Cannot invoke "Object.getClass()" because "entity" is null

java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "entity" is null
    at net.lecousin.reactive.data.relational.enhance.EntityState.updatePersistedValues(EntityState.java:135) ~[core-0.8.1.jar:na]
    at net.lecousin.reactive.data.relational.enhance.EntityState.loaded(EntityState.java:129) ~[core-0.8.1.jar:na]
    at net.lecousin.reactive.data.relational.enhance.EntityState.lambda$loading$0(EntityState.java:115) ~[core-0.8.1.jar:na]
    at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onComplete(MonoPeekTerminal.java:289) ~[reactor-core-3.4.13.jar:3.4.13]
lecousin commented 2 years ago

Thanks for your feedback. Regarding the naming, you can keep both your column name "shipment_id", and attribute name "shipment". For that just specify the name using the Column annotation, like:

@Column("shipment_id")
@ForeignKey
var shipment: ShipmentEntity? = null

For the exception, I tried to reproduce on my side with a simple example (I just put a text field in the entities, and test in blocking mode just to reproduce the case), and it worked:

        Shipment shipment = new Shipment();
        shipment.setText("hello");
        shipmentRepo.save(shipment).block(); // create a Shipment in database

        ShipmentHistory history = new ShipmentHistory();
        history.setShipment(shipment);
        history.setHistoryText("my history entity");
        historyRepo.save(history).block(); // create a ShipmentHistory in database

        history = historyRepo.findAll().blockFirst(); // get the ShipmentHistory back from database so the link is not yet fetch and we can test lazyGetShipment
        System.out.println("History loaded = " + history);
        System.out.println("Text before lazy loading = " + history.getShipment().getText());
        System.out.println("Text with lazy loading = " + history.lazyGetShipment().block().getText());

I guess the problem comes from the way you fetch the ShipmentHistoryEntity from the database before calling lazyGetShipment. Can you show the code how you get your entity before calling lazyGetShipment ?

chrisvettese commented 2 years ago

For sure, here is the repository.

import com.shipi.shipi_shipment.entity.ShipmentHistoryEntity
import net.lecousin.reactive.data.relational.repository.LcR2dbcRepository
import org.springframework.data.r2dbc.repository.Query
import org.springframework.stereotype.Repository
import reactor.core.publisher.Flux

@Repository
interface ShipmentHistoryRepository : LcR2dbcRepository<ShipmentHistoryEntity, Long> {
    @Query(
        "select * from " +
            "(select distinct on (h.shipment_id) h.id as h_id, h.status as h_status, h.updated_at as h_updated_at from shipment_history h " +
            "join shipment s on h.shipment_id = s.id and s.store_id = :storeId " +
            "order by h.shipment_id, h.updated_at desc) " +
            "as a where a.h_status\\:\\:varchar in (:statuses) limit :limit offset :offset"
    )
    fun findByStoreIdAndLatestShipmentStatusIn(
        statuses: List<String>,
        storeId: Long,
        limit: Int,
        offset: Int
    ): Flux<ShipmentHistoryEntity>
}

I then call the method like this:

        val shipments = shipmentHistoryRepository.findByStoreIdAndLatestShipmentStatusIn(
            req.statuses.map { it.toString() },
            storeId.value.toLong(),
            req.limit ?: 100,
            req.offset ?: 0
        ).collectList().awaitSingle()
        /*val ids = shipments.map {
            it.shipment.id.toString()
        }*/

        val test0 = shipments[0].entityLoaded()
        val test = shipments[0].shipment_id
        val test2 = shipments[0].lazyGetShipment_id().awaitSingle()
        println(test2)
        val test1 = shipments[0].loadEntity().awaitSingle()
        println(test1)

awaitSingle is a Kotlin method for getting the value of a Mono without blocking the thread. Not sure if this would cause the issue. The errors were on shipments[0].lazyGetShipment_id().awaitSingle() and shipments[0].loadEntity().awaitSingle().

Also thought I'd attach what the shipments list looks like after the repository method is called. image

The ShipmentHistoryEntities load correctly. The ShipmentEntities contain default values which is expected because they're supposed to lazy load.

chrisvettese commented 2 years ago

Also I'm not sure if this would count as a bug, or just a feature that hasn't been implemented yet, but I tried using a SelectQuery to achieve something similar and got this exception

org.springframework.r2dbc.BadSqlGrammarException: executeMany; bad SQL grammar [SELECT shipment.id FROM shipment shipment LEFT OUTER JOIN shipment_history history ON history.shipment_id = shipment.id WHERE shipment.store_id = $1 AND history.shipment_id = shipment.id AND history.status IN ($2, $3) GROUP BY shipment.id ORDER BY MAX(history.status) DESC LIMIT 100 OFFSET 0]; nested exception is io.r2dbc.postgresql.ExceptionFactory$PostgresqlBadGrammarException: [42883] operator does not exist: delivery_status = character varying

I have seen this error before, it happens when checking if a column of type enum is in a list. The easiest fix I know is to cast the enum column to be a varchar, something like history.status::varchar IN ($2, $3). Not sure if there's a way to do this with SelectQuery but I think it could be a useful feature. The exception was from this line Criteria.property("history", "status").in(statuses), with status being a Postgres enum and Java/kotlin enum.

Update - with the original method of lazy loading, I was able to get it to work when using the default findById method. This leads me to believe the issue may be related to how my query used the Query annotation.

lecousin commented 2 years ago

Ok now I understand the issue. In your query you are using aliases, so the database returns columns with your aliases (h_id, h_status, ...). When mapping those columns to the entity, none matches so your entity ShipmentHistoryEntity contains only default values.

For the mapping to work correctly, the name of columns returned by the database must match with the entity's field. In your query we may do it by replacing the "select * from" by renaming the aliases to the entity's fields, and make sure that the inner select returns all the columns so we can map all fields. Can you try something like this:

    @Query(
        "select h_id as id, h_status as status, h_updated_at as updated_at, h_shipment_id as shipment_id from " +
            "(select distinct on (h.shipment_id) h.id as h_id, h.status as h_status, h.updated_at as h_updated_at, h.shipment_id as h_shipment_id from shipment_history h " +
            "join shipment s on h.shipment_id = s.id and s.store_id = :storeId " +
            "order by h.shipment_id, h.updated_at desc) " +
            "as a where a.h_status\\:\\:varchar in (:statuses) limit :limit offset :offset"
    )

Note that I added the alias h_shipment_id in the inner select so we can return it in the root select

chrisvettese commented 2 years ago

Good catch, that seems to have fixed it. Thank you for the help, this library has saved me a lot of time.

lecousin commented 2 years ago

You're welcome. Still your issue raise 2 problems for me that I will have a look at: