Re: BUG #17893: Assert failed in heap_update()/_delete() when FK modiified by RI trigger in non-read-committed xact

Hello Heikki,

27.11.2023 22:23, Heikki Linnakangas wrote:
> I propose to attached slight variation, which moves the assertions before the crosscheck snapshot check. The 
> assertions are correct as they are, if you don't need to take the crosscheck into account.
>

Thank you for paying attention to this!
I agree with your variation, it's more accurate.

> I'm a little worried if the callers of tuple_update() might also get confused when tuple_update() returns TM_Updated

> but xmax is invalid. As you said:
> ...
> I think you only get that error with REPEATABLE READ or SERIALIZABLE isolation level. I don't quite understand how 
> this works. In READ COMMITTED level, ISTM that ExecUpdate() or ExecDelete() will proceed with EvalPlanQualSlot() and

> locking the row with table_tuple_lock(). Or do we never use the cross-check snapshot in READ COMMITTED mode?
>

I think so, as I see the following in ri_PerformCheck():
     /*
      * In READ COMMITTED mode, we just need to use an up-to-date regular
      * snapshot, and we will see all rows that could be interesting. But in
      * transaction-snapshot mode, we can't change the transaction snapshot. If
      * the caller passes detectNewRows == false then it's okay to do the query
      * with the transaction snapshot; otherwise we use a current snapshot, and
      * tell the executor to error out if it finds any rows under the current
      * snapshot that wouldn't be visible per the transaction snapshot.  Note
      * that SPI_execute_snapshot will register the snapshots, so we don't need
      * to bother here.
      */
     if (IsolationUsesXactSnapshot() && detectNewRows)
     {
         CommandCounterIncrement();  /* be sure all my own work is visible */
         test_snapshot = GetLatestSnapshot();
         crosscheck_snapshot = GetTransactionSnapshot();
     }

IIUC, that's the only place where crosscheck_snapshot (and then
es_crosscheck_snapshot) may get a non-invalid value.

Best regards,
Alexander