Conversation
|
|
Hydrocharged
left a comment
There was a problem hiding this comment.
Really not a fan of moving InterpreterOperation to another package in core, since it's very specific to plpgsql (we may not reuse it for other interpreter backends).
I'm also not a fan of putting the pgproto3.Backend in contextValues. Although we're not necessarily doing it now, contextValues is moreso for temporary usage, and it's fine to completely rebuild it between statements, in the middle of statements, etc. This backend is being set once per query, which puts an unintended requirement on contextValues.
I'd much rather have a separate, more permanent construct where we store the backend, but my true preference would be for the context to hold any notices until the query completes, fails, etc. The logic in /server that sends messages back to the client would read any notices and send those first. I think this also takes care of the import cycle, and it keeps the client-communicating logic all in the same package.
Would like to know your thoughts on this!
|
Thanks for taking a look. Yeah, I hear ya – I wasn't crazy about pulling |
90c0ce4 to
980cf3c
Compare
Hydrocharged
left a comment
There was a problem hiding this comment.
Much better! Fantastic work!
Adds initial support for
RAISEstatements in PL/pgSQL functions. There are still a few edge case TODOs (e.g. using theclient_min_messagesconfig param to determine what level of notices to send to clients), and this initial pass does not include any exception handling.Notice messages are queued in the current session, then sent to the client from the connection handler, right before results are sent to the client. Support for setting notices in the DoltSession is added in dolthub/dolt#8974.
PostgreSQL Docs