On Tue, Sep 19, 2023 at 01:23:54PM +0300, Aleksander Alekseev wrote: > You are right, I missed it. Your patch is correct while the original > one is not quite so.
Actually there was a bit more to it in the presence of \e, that could also get some unpredictible behaviors if some errors happen while editing a query, which is something unlikely, still leads to strange behaviors on failure injections. I was considering first to move the reset in do_edit(), but also we have the case of \e[v|f] where the buffer has no edits so it felt a bit more natural to do that in the upper layer like in this patch. Another aspect of all these code paths is the line number that can be optionally number after an object name for \e[v|f] or a file name for \e (in the latter case it is possible to have a line number without a file name, as well). Anyway, we only fill the query buffer after validating all the options at hand. So, while the status is set to PSQL_CMD_ERROR, we'd still do a reset of the query buffer but nothing got added to it yet. I've also considered a backpatch for this change, but at the end discarded this option, at least for now. I don't think that someone is relying on the existing behavior of having the query buffer still around on failure if \ev or \ef fail their object lookup as the contents are undefined, because that's not unintuitive, but this change is not critical enough to make it backpatchable if somebody's been actually relying on the previous behavior. I'm OK to revisit this choice later on depending on the feedback, though. -- Michael
signature.asc
Description: PGP signature
