Re: pltcl - "Cache lookup for attribute" error - version 2 - Mailing list pgsql-patches

I wrote:
> Patrick Samson <p_samson@yahoo.com> writes:
>> - Add the same filter in the build of TG_relatts.
>> This will prevent a tcl script which loops on
>> TG_relattrs to fail in trying to use a dropped
>> column.

> This is deliberately *not* done in 7.4, because it would break the
> documented behavior of TG_relatts:

I thought of a good compromise solution: instead of putting the dropped
columns' names into TG_relatts, we can put empty-string list elements.
This preserves the attnum correspondence, and anything that iterates
over TG_relatts should be coded to ignore empty elements anyway, no?

I've applied the attached patch to 7.4 and HEAD branches.  (The other
places Patrick identified are already fixed in 7.4.)

            regards, tom lane


*** doc/src/sgml/pltcl.sgml.orig    Sat Nov 29 14:51:37 2003
--- doc/src/sgml/pltcl.sgml    Sat Jan 24 17:58:35 2004
***************
*** 516,522 ****
           element. So looking up a column name in the list with <application>Tcl</>'s
           <function>lsearch</> command returns the element's number starting
       with 1 for the first column, the same way the columns are customarily
!      numbered in <productname>PostgreSQL</productname>.
      </para>
         </listitem>
        </varlistentry>
--- 516,525 ----
           element. So looking up a column name in the list with <application>Tcl</>'s
           <function>lsearch</> command returns the element's number starting
       with 1 for the first column, the same way the columns are customarily
!      numbered in <productname>PostgreSQL</productname>.  (Empty list
!      elements also appear in the positions of columns that have been
!      dropped, so that the attribute numbering is correct for columns
!      to their right.)
      </para>
         </listitem>
        </varlistentry>
*** src/pl/tcl/pltcl.c.orig    Tue Jan  6 18:55:19 2004
--- src/pl/tcl/pltcl.c    Sat Jan 24 17:58:41 2004
***************
*** 695,705 ****
      pfree(stroid);

      /* A list of attribute names for argument TG_relatts */
-     /* note: we deliberately include dropped atts here */
      Tcl_DStringAppendElement(&tcl_trigtup, "");
      for (i = 0; i < tupdesc->natts; i++)
!         Tcl_DStringAppendElement(&tcl_trigtup,
!                                  NameStr(tupdesc->attrs[i]->attname));
      Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
      Tcl_DStringFree(&tcl_trigtup);
      Tcl_DStringInit(&tcl_trigtup);
--- 695,709 ----
      pfree(stroid);

      /* A list of attribute names for argument TG_relatts */
      Tcl_DStringAppendElement(&tcl_trigtup, "");
      for (i = 0; i < tupdesc->natts; i++)
!     {
!         if (tupdesc->attrs[i]->attisdropped)
!             Tcl_DStringAppendElement(&tcl_trigtup, "");
!         else
!             Tcl_DStringAppendElement(&tcl_trigtup,
!                                      NameStr(tupdesc->attrs[i]->attname));
!     }
      Tcl_DStringAppendElement(&tcl_cmd, Tcl_DStringValue(&tcl_trigtup));
      Tcl_DStringFree(&tcl_trigtup);
      Tcl_DStringInit(&tcl_trigtup);
***************
*** 881,889 ****
          siglongjmp(Warn_restart, 1);
      }

!     i = 0;
!     while (i < ret_numvals)
      {
          int            attnum;
          HeapTuple    typeTup;
          Oid            typinput;
--- 885,894 ----
          siglongjmp(Warn_restart, 1);
      }

!     for (i = 0; i < ret_numvals; i += 2)
      {
+         CONST84 char *ret_name = ret_values[i];
+         CONST84 char *ret_value = ret_values[i + 1];
          int            attnum;
          HeapTuple    typeTup;
          Oid            typinput;
***************
*** 891,914 ****
          FmgrInfo    finfo;

          /************************************************************
!          * Ignore pseudo elements with a dot name
           ************************************************************/
!         if (*(ret_values[i]) == '.')
!         {
!             i += 2;
              continue;
-         }

          /************************************************************
           * Get the attribute number
           ************************************************************/
!         attnum = SPI_fnumber(tupdesc, ret_values[i++]);
          if (attnum == SPI_ERROR_NOATTRIBUTE)
!             elog(ERROR, "invalid attribute \"%s\"",
!                  ret_values[--i]);
          if (attnum <= 0)
!             elog(ERROR, "cannot set system attribute \"%s\"",
!                  ret_values[--i]);

          /************************************************************
           * Lookup the attribute type in the syscache
--- 896,920 ----
          FmgrInfo    finfo;

          /************************************************************
!          * Ignore ".tupno" pseudo elements (see pltcl_set_tuple_values)
           ************************************************************/
!         if (strcmp(ret_name, ".tupno") == 0)
              continue;

          /************************************************************
           * Get the attribute number
           ************************************************************/
!         attnum = SPI_fnumber(tupdesc, ret_name);
          if (attnum == SPI_ERROR_NOATTRIBUTE)
!             elog(ERROR, "invalid attribute \"%s\"", ret_name);
          if (attnum <= 0)
!             elog(ERROR, "cannot set system attribute \"%s\"", ret_name);
!
!         /************************************************************
!          * Ignore dropped columns
!          ************************************************************/
!         if (tupdesc->attrs[attnum - 1]->attisdropped)
!             continue;

          /************************************************************
           * Lookup the attribute type in the syscache
***************
*** 932,938 ****
          UTF_BEGIN;
          modvalues[attnum - 1] =
              FunctionCall3(&finfo,
!                           CStringGetDatum(UTF_U2E(ret_values[i++])),
                            ObjectIdGetDatum(typelem),
                     Int32GetDatum(tupdesc->attrs[attnum - 1]->atttypmod));
          UTF_END;
--- 938,944 ----
          UTF_BEGIN;
          modvalues[attnum - 1] =
              FunctionCall3(&finfo,
!                           CStringGetDatum(UTF_U2E(ret_value)),
                            ObjectIdGetDatum(typelem),
                     Int32GetDatum(tupdesc->attrs[attnum - 1]->atttypmod));
          UTF_END;