Re: CURRENT OF causes an error when IndexOnlyScan is used - Mailing list pgsql-hackers

I wrote:
> In the meantime, we could fix execCurrent.c so that it throws
> FEATURE_NOT_SUPPORTED rather than the current failure if the slot it's
> looking at doesn't contain a physical tuple.

Concretely, I think we should do the attached, so as to cover any other
situations where the scan type doesn't return a physical tuple.  I kept
it separate from your patch so it's easy to test (the original case you
gave now throws the appropriate error).

The existing error when execCurrentOf can't figure out what to do with
the plan is ERRCODE_INVALID_CURSOR_STATE with message
"cursor \"%s\" is not a simply updatable scan of table \"%s\""
so for this draft patch I just used that.  I'm not sure if it would be
a better idea to throw a different SQLSTATE or different error text
for this case.  Any thoughts on that?

            regards, tom lane

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index a2f67f2..c45a488 100644
*** a/src/backend/access/common/heaptuple.c
--- b/src/backend/access/common/heaptuple.c
*************** slot_attisnull(TupleTableSlot *slot, int
*** 1367,1372 ****
--- 1367,1398 ----
  }

  /*
+  * slot_getsysattr
+  *        This function fetches a system attribute of the slot's current tuple.
+  *        Unlike slot_getattr, if the slot does not contain system attributes,
+  *        this will return false (with a NULL attribute value) instead of
+  *        throwing an error.
+  */
+ bool
+ slot_getsysattr(TupleTableSlot *slot, int attnum,
+                 Datum *value, bool *isnull)
+ {
+     HeapTuple    tuple = slot->tts_tuple;
+
+     Assert(attnum < 0);            /* else caller error */
+     if (tuple == NULL ||
+         tuple == &(slot->tts_minhdr))
+     {
+         /* No physical tuple, or minimal tuple, so fail */
+         *value = (Datum) 0;
+         *isnull = true;
+         return false;
+     }
+     *value = heap_getsysattr(tuple, attnum, slot->tts_tupleDescriptor, isnull);
+     return true;
+ }
+
+ /*
   * heap_freetuple
   */
  void
diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c
index ce7d4ac..2296c9b 100644
*** a/src/backend/executor/execCurrent.c
--- b/src/backend/executor/execCurrent.c
*************** execCurrentOf(CurrentOfExpr *cexpr,
*** 150,157 ****
      else
      {
          ScanState  *scanstate;
          bool        lisnull;
-         Oid            tuple_tableoid PG_USED_FOR_ASSERTS_ONLY;
          ItemPointer tuple_tid;

          /*
--- 150,157 ----
      else
      {
          ScanState  *scanstate;
+         Datum        ldatum;
          bool        lisnull;
          ItemPointer tuple_tid;

          /*
*************** execCurrentOf(CurrentOfExpr *cexpr,
*** 183,201 ****
          if (TupIsNull(scanstate->ss_ScanTupleSlot))
              return false;

!         /* Use slot_getattr to catch any possible mistakes */
!         tuple_tableoid =
!             DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
!                                           TableOidAttributeNumber,
!                                           &lisnull));
!         Assert(!lisnull);
!         tuple_tid = (ItemPointer)
!             DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
!                                          SelfItemPointerAttributeNumber,
!                                          &lisnull));
          Assert(!lisnull);

!         Assert(tuple_tableoid == table_oid);

          *current_tid = *tuple_tid;

--- 183,213 ----
          if (TupIsNull(scanstate->ss_ScanTupleSlot))
              return false;

!         /*
!          * Try to fetch tableoid and CTID from the scan node's current tuple.
!          * If the scan type hasn't provided a physical tuple, we have to fail.
!          */
!         if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
!                              TableOidAttributeNumber,
!                              &ldatum,
!                              &lisnull))
!             ereport(ERROR,
!                     (errcode(ERRCODE_INVALID_CURSOR_STATE),
!                      errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
!                             cursor_name, table_name)));
          Assert(!lisnull);
+         Assert(DatumGetObjectId(ldatum) == table_oid);

!         if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
!                              SelfItemPointerAttributeNumber,
!                              &ldatum,
!                              &lisnull))
!             ereport(ERROR,
!                     (errcode(ERRCODE_INVALID_CURSOR_STATE),
!                      errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
!                             cursor_name, table_name)));
!         Assert(!lisnull);
!         tuple_tid = (ItemPointer) DatumGetPointer(ldatum);

          *current_tid = *tuple_tid;

diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index 0642a3a..a5779b1 100644
*** a/src/include/executor/tuptable.h
--- b/src/include/executor/tuptable.h
*************** extern Datum slot_getattr(TupleTableSlot
*** 170,174 ****
--- 170,176 ----
  extern void slot_getallattrs(TupleTableSlot *slot);
  extern void slot_getsomeattrs(TupleTableSlot *slot, int attnum);
  extern bool slot_attisnull(TupleTableSlot *slot, int attnum);
+ extern bool slot_getsysattr(TupleTableSlot *slot, int attnum,
+                 Datum *value, bool *isnull);

  #endif                            /* TUPTABLE_H */