Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken

From: Massimo Dal Zotto <dz(at)cs(dot)unitn(dot)it>
To: hackers(at)postgreSQL(dot)org (PostgreSQL Hackers)
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us (Tom Lane)
Subject: Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken
Date: 1998-10-05 19:59:33
Message-ID: 199810051959.VAA10430@tango.cs.unitn.it
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-interfaces

>
> 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(at)cs(dot)unitn(dot)it |
| Via Marconi, 141 phone: ++39-461-534251 |
| 38057 Pergine Valsugana (TN) www: http://www.cs.unitn.it/~dz/ |
| Italy pgp: finger dz(at)tango(dot)cs(dot)unitn(dot)it |
+----------------------------------------------------------------------+

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 1998-10-05 23:39:39 Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken
Previous Message Marc G. Fournier 1998-10-05 18:29:41 Re: [HACKERS] Open 6.4 items

Browse pgsql-interfaces by date

  From Date Subject
Next Message Gerald Gryschuk 1998-10-05 20:36:34 Re: [INTERFACES] Re: Just some unfinished stuff.
Previous Message brian 1998-10-05 19:32:17 Libpq question