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

> 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.

I have just started to learn TCL, and have Practical Tcl and TK by Brent
Welch on my desk.

Sounds like our interface needs fixing.  I am sure we currently do not
handle full protocol correctly.

--
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026