Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken - Mailing list pgsql-hackers
From | Bruce Momjian |
---|---|
Subject | Re: [HACKERS] TCL_ARRAYS code in libpgtcl is pretty seriously broken |
Date | |
Msg-id | 199810050032.UAA29120@candle.pha.pa.us Whole thread Raw |
In response to | TCL_ARRAYS code in libpgtcl is pretty seriously broken (Tom Lane <tgl@sss.pgh.pa.us>) |
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. 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
pgsql-hackers by date: