Re: [PATCH] Patch to fix a crash of psql

Tatsuo Ishii <ishii@postgresql.org> writes:
>> TBH I think that a message here is unnecessary; it's sufficient to
>> ensure psql doesn't crash.  The backend will produce a better message
>> than this anyway once the data gets there, and that way we don't have to
>> invent a new error recovery path inside psql.  As-is, the patch just
>> creates the question of what to do after issuing the error.

> Problem is, backend does not always detect errors.

The reason it doesn't produce an error in Jiang Guiqing's example is
that the encoding error is in a comment and thus is never transmitted
to the backend at all.  I don't see a big problem with that.  If we
did have a problem with it, I think the better fix would be to stop
having psql strip comments from what it sends.  (I've considered
proposing such a change anyway, in order that logged statements look
more like what the user typed.)

> IMO it is a benefit for users to detect such errors earlier.

It is not a benefit for users to get randomly different (and less
informative) messages and different error behaviors for the same
problem.

I think we should just do this and call it good:

diff --git a/src/bin/psql/psqlscan.l b/src/bin/psql/psqlscan.l
index d32a12c63c856625aa42c708b24d4b58e3cdd677..6c1429815f2eec597f735d18ea86d9c8c9f1f3a2 100644
*** a/src/bin/psql/psqlscan.l
--- b/src/bin/psql/psqlscan.l
*************** prepare_buffer(const char *txt, int len,
*** 1807,1813 ****             /* first byte should always be okay... */             newtxt[i] = txt[i];
i++;
!             while (--thislen > 0)                 newtxt[i++] = (char) 0xFF;         }     }
--- 1807,1813 ----             /* first byte should always be okay... */             newtxt[i] = txt[i];
i++;
!             while (--thislen > 0 && i < len)                 newtxt[i++] = (char) 0xFF;         }     }

        regards, tom lane