Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken - Mailing list pgsql-hackers
From | Massimo Dal Zotto |
---|---|
Subject | Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken |
Date | |
Msg-id | 199810051959.VAA10430@tango.cs.unitn.it Whole thread Raw |
In response to | TCL_ARRAYS code in libpgtcl is pretty seriously broken (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken
|
List | pgsql-hackers |
> > There is some code in libpgtcl that purports to convert Postgres array > data values into Tcl lists. Is anyone prepared to argue that that code > does something useful in its present state? I can name half a dozen > bugs in it without breathing hard: > > 1. Blithely assumes that any data value beginning with '{' and ending > with '}' must represent an array value. Should have some more robust > way of discovering whether a column is array type. (In fairness, this > might require a FE/BE protocol change, unless arrayness can be > determined from the tuple descriptors provided by the backend, ie, > field type OID, size, and attmod. Anybody know a way to do that?) > > 2. Applies a translation that converts all backslash escape sequences > defined for C string constants into their equivalent single characters. > Since neither the backend nor Tcl generate anything close to C-string > escapes, the point of this is difficult to determine. It does however > result in unexpected output, eg disappearing backslashes. > > 3. Applies said translation even when processing a non-array data value. > > 4. Doesn't actually manage to produce a valid Tcl list, if the data > contains anything Tcl considers a special character. What it *should* > be doing is quoting, not de-quoting. > > 5. Fails to modify \\, \{, and \} (thus quite unintentionally doing > almost the right thing...) when these sequences appear inside an array > value, because "they will be unescaped by Tcl in Tcl_AppendElement". > But in fact Tcl_AppendElement is not invoked on the results of this > code. > > 6. Modifies the string returned by libpq *in place*. This would be a > const-ness violation if we had been more careful about declaring things > const. More importantly, it means that re-examining the same tuple of > the PGresult will yield a different result. Not cool. > > 7. The TCL_ARRAYS code is only invoked in the "-assign" variant of > the pgtcl pg_result statement, not in any of the other paths that allow > tuple values to be examined. This is presumably an oversight, not > the intended behavior. > > 8. Does not cope with MULTIBYTE strings. (But I don't think Tcl does > either, so it's not clear that this can be called a bug.) > > > I am strongly inclined to rip this code out, because it is responsible > for several behaviors that were correctly called bugs when backslash- > handling was discussed on pgsql-interfaces back in August. If we don't > rip it out, it needs a complete rewrite. > > Unless there is a bulletproof solution to problem #1 (how to tell > whether a field's data type is array), I do not think it is appropriate > for the basic pg_result code to be applying any such transform. Perhaps > it would be reasonable to invent a separate string-formatting function, > say "pg_arraytolist", that would perform the conversion. It would then > be the application writer's responsibility to know which fields were > arrays and apply the conversion if he wanted it. > > Comments? > > regards, tom lane I wrote this code and used it for two years without any problem. All the bugs you mentioned disappear if you use the proper string output functions which C-like escapes (code in contrib/string-io). This makes also possible to distinguish between array and normal attibutes. The code works fine in this case. I did a lot of testing at the time. However it is ok to move it into a separate tcl command. -- Massimo Dal Zotto +----------------------------------------------------------------------+ | Massimo Dal Zotto email: dz@cs.unitn.it | | Via Marconi, 141 phone: ++39-461-534251 | | 38057 Pergine Valsugana (TN) www: http://www.cs.unitn.it/~dz/ | | Italy pgp: finger dz@tango.cs.unitn.it | +----------------------------------------------------------------------+
pgsql-hackers by date: