Thread: pltcl - "Cache lookup for attribute" error - version 2
After a deeper analysis, this post supersedes my previous one with the same subject. I got the error message: ERROR: pltcl: Cache lookup for attribute '........pg.droppped.7........' type 0 failed when a pltcl trigger handler is fired. Attribute names beginning with a dot are filtered just in one place, in pltcl_trigger_handler(). (version 7.3.5) Attached is a patch to : - Add a filter in two other places, in relation with the mentioned error message: pltcl_set_tuple_values() pltcl_build_tuple_argument() - 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. Note 1: the filter method is changed from testing if attname begins with a dot to testing the setting of attisdropped. I presume (to be confirmed or contradicted by a more authoritative developer than me) that the original dot filtering intent was to filter dropped columns. In other words, there is no case where a column name begins with '.' except dropped columns. If this is right, testing attisdropped is an adequate and more readable replacement. Note 2: with the addition of the filter in pltcl_build_tuple_argument(), filtering in pltcl_trigger_handler() is no more needed, so I put these lines in comments (may be removed in a final release). __________________________________ Do you Yahoo!? Yahoo! SiteBuilder - Free web site building tool. Try it! http://webhosting.yahoo.com/ps/sb/--- pltcl.orig 2003-10-30 03:00:44.000000000 +0100 +++ pltclDotFilter2.c 2004-01-23 09:28:33.359375000 +0100 @@ -678,8 +678,9 @@ /* A list of attribute names for argument TG_relatts */ Tcl_DStringAppendElement(&tcl_trigtup, ""); for (i = 0; i < tupdesc->natts; i++) - Tcl_DStringAppendElement(&tcl_trigtup, - NameStr(tupdesc->attrs[i]->attname)); + if (!tupdesc->attrs[i]->attisdropped) + 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); @@ -863,11 +864,11 @@ /************************************************************ * Ignore pseudo elements with a dot name ************************************************************/ - if (*(ret_values[i]) == '.') - { - i += 2; - continue; - } + //if (*(ret_values[i]) == '.') + //{ + // i += 2; + // continue; + //} /************************************************************ * Get the attribute number @@ -2352,6 +2353,12 @@ for (i = 0; i < tupdesc->natts; i++) { /************************************************************ + * Ignore dropped columns (attname begins with a dot, + * such as "........pg.dropped.7........") + ************************************************************/ + if (tupdesc->attrs[i]->attisdropped) continue; + + /************************************************************ * Get the attribute name ************************************************************/ attname = NameStr(tupdesc->attrs[i]->attname); @@ -2424,6 +2431,12 @@ for (i = 0; i < tupdesc->natts; i++) { /************************************************************ + * Ignore dropped columns (attname begins with a dot, + * such as "........pg.dropped.7........") + ************************************************************/ + if (tupdesc->attrs[i]->attisdropped) continue; + + /************************************************************ * Get the attribute name ************************************************************/ attname = NameStr(tupdesc->attrs[i]->attname);
Patrick Samson <p_samson@yahoo.com> writes:
> Attribute names beginning with a dot are filtered
> just in one place, in pltcl_trigger_handler().
> (version 7.3.5)
I am not sure why that code is there. It is *not* there to prevent the
loop from touching dropped attributes, because the same code is in the
original 1.1 version of pltcl.c, long before we could drop attributes.
Jan, do you remember why you put this into pltcl_trigger_handler()?
/************************************************************
* Ignore pseudo elements with a dot name
************************************************************/
if (*(ret_values[i]) == '.') {
i += 2;
continue;
}
It's not documented behavior that I can see, and it doesn't seem to have
any use other than making pltcl triggers fail if a user chooses a field
name starting with a dot :-(
> Attached is a patch to :
> - Add a filter in two other places, in relation
> with the mentioned error message:
> pltcl_set_tuple_values()
> pltcl_build_tuple_argument()
This is already done in 7.4, although for some reason
pltcl_trigger_handler got overlooked - I will fix that.
> - 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:
$TG_relatts
A Tcl list of the table column names, prefixed with an empty list
element. So looking up a column name in the list with
Tcl's lsearch command returns the element's number starting with 1
for the first column, the same way the
columns are customarily numbered in PostgreSQL.
I think we need to preserve the relationship to column numbers. People
who just want a list of the live columns can get it from the OLD or NEW
arrays.
regards, tom lane
Tom Lane wrote:
> Patrick Samson <p_samson@yahoo.com> writes:
>> Attribute names beginning with a dot are filtered
>> just in one place, in pltcl_trigger_handler().
>> (version 7.3.5)
>
> I am not sure why that code is there. It is *not* there to prevent the
> loop from touching dropped attributes, because the same code is in the
> original 1.1 version of pltcl.c, long before we could drop attributes.
> Jan, do you remember why you put this into pltcl_trigger_handler()?
>
> /************************************************************
> * Ignore pseudo elements with a dot name
> ************************************************************/
> if (*(ret_values[i]) == '.') {
> i += 2;
> continue;
> }
>
> It's not documented behavior that I can see, and it doesn't seem to have
> any use other than making pltcl triggers fail if a user chooses a field
> name starting with a dot :-(
right, this is documented nowhere :-(
When assigning a tuple to an array, PL/Tcl creates one extra array
element .tupno telling the SPI_tuptable index of the result tuple. I
think I originally planned to have more of these critters ... but
probably never really needed them. It is in there since 6.3!
Bottom line is, if one has a trigger, and inside the trigger he does an
SPI_exec, fetches a tuple into an array and then returns [array get x]
instead of new or old ... so from the back through the right chest into
the left eye ... then it will fail if the .tupno isn't filtered out.
Jan
>
>> Attached is a patch to :
>> - Add a filter in two other places, in relation
>> with the mentioned error message:
>> pltcl_set_tuple_values()
>> pltcl_build_tuple_argument()
>
> This is already done in 7.4, although for some reason
> pltcl_trigger_handler got overlooked - I will fix that.
>
>> - 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:
>
> $TG_relatts
>
> A Tcl list of the table column names, prefixed with an empty list
> element. So looking up a column name in the list with
> Tcl's lsearch command returns the element's number starting with 1
> for the first column, the same way the
> columns are customarily numbered in PostgreSQL.
>
> I think we need to preserve the relationship to column numbers. People
> who just want a list of the live columns can get it from the OLD or NEW
> arrays.
>
> regards, tom lane
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes:
> When assigning a tuple to an array, PL/Tcl creates one extra array
> element .tupno telling the SPI_tuptable index of the result tuple. I
> think I originally planned to have more of these critters ... but
> probably never really needed them. It is in there since 6.3!
> Bottom line is, if one has a trigger, and inside the trigger he does an
> SPI_exec, fetches a tuple into an array and then returns [array get x]
> instead of new or old ... so from the back through the right chest into
> the left eye ... then it will fail if the .tupno isn't filtered out.
Hm. Perhaps we should tighten the test to reject only ".tupno", rather
than any name starting with dot?
regards, tom lane
Tom Lane wrote:
> Jan Wieck <JanWieck@Yahoo.com> writes:
>> When assigning a tuple to an array, PL/Tcl creates one extra array
>> element .tupno telling the SPI_tuptable index of the result tuple. I
>> think I originally planned to have more of these critters ... but
>> probably never really needed them. It is in there since 6.3!
>
>> Bottom line is, if one has a trigger, and inside the trigger he does an
>> SPI_exec, fetches a tuple into an array and then returns [array get x]
>> instead of new or old ... so from the back through the right chest into
>> the left eye ... then it will fail if the .tupno isn't filtered out.
>
> Hm. Perhaps we should tighten the test to reject only ".tupno", rather
> than any name starting with dot?
Man you have worries ... aren't people who use identifiers with a
leading dot supposed to have problems? What about changing it to ..
instead? I mean, how does such a thing look like?
SELECT ".. some column .." FROM ".. the schema ..".".. a table .."
WHERE ".. the schema ..".".. a table ..".".. some column .."
IN ('.oh.', '.give.', '.me.', '.a.', '.break!');
If you like to, tighten it.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #
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;
Jan Wieck wrote:
> Tom Lane wrote:
>
> > Jan Wieck <JanWieck@Yahoo.com> writes:
> >> When assigning a tuple to an array, PL/Tcl creates one extra array
> >> element .tupno telling the SPI_tuptable index of the result tuple. I
> >> think I originally planned to have more of these critters ... but
> >> probably never really needed them. It is in there since 6.3!
> >
> >> Bottom line is, if one has a trigger, and inside the trigger he does an
> >> SPI_exec, fetches a tuple into an array and then returns [array get x]
> >> instead of new or old ... so from the back through the right chest into
> >> the left eye ... then it will fail if the .tupno isn't filtered out.
> >
> > Hm. Perhaps we should tighten the test to reject only ".tupno", rather
> > than any name starting with dot?
>
> Man you have worries ... aren't people who use identifiers with a
> leading dot supposed to have problems? What about changing it to ..
> instead? I mean, how does such a thing look like?
>
> SELECT ".. some column .." FROM ".. the schema ..".".. a table .."
> WHERE ".. the schema ..".".. a table ..".".. some column .."
> IN ('.oh.', '.give.', '.me.', '.a.', '.break!');
>
> If you like to, tighten it.
Jan, if I understand correctly, I agree with Tom. It seems strange to
add a restriction on indentifiers in pl/tcl that doesn't exist in any of
the other interfaces.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Jan, if I understand correctly, I agree with Tom. It seems strange to
> add a restriction on indentifiers in pl/tcl that doesn't exist in any of
> the other interfaces.
It's already done --- CVS tip checks specifically for ".tupno" and not
for anything else.
regards, tom lane
--- Tom Lane wrote: > I wrote: > > Patrick Samson 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? > OK, I didn't pay attention to the numbering concern. Your compromise suits me. I prefer to filter on empty strings than on something with some dots in it. Or to run an spi_exec to test the attisdropped. Tom, your suggestion to look in OLD or NEW for live columns may be cover all situations. I use a code (not mine, but I try to make it run on Cygwin) that use a NULL value if [info exists NEW($field)] is false. Many thanks to all. Patrick __________________________________ Do you Yahoo!? Yahoo! SiteBuilder - Free web site building tool. Try it! http://webhosting.yahoo.com/ps/sb/