This is a discussion on Re: Concurrent psql patch within the Pgsql Patches forums, part of the PostgreSQL category; --> On Mon, May 14, 2007 at 12:51:39PM +0100, Gregory Stark wrote: > "Tom Lane" <tgl@sss.pgh.pa.us> writes: > > > ...
| |||||||
| Register | FAQ | Members List | Calendar | Search | Today's Posts | Mark Forums Read |
| ||||
| On Mon, May 14, 2007 at 12:51:39PM +0100, Gregory Stark wrote: > "Tom Lane" <tgl@sss.pgh.pa.us> writes: > > > Gregory Stark <stark@enterprisedb.com> writes: > >> So would you prefer \g& as Jim Nasby suggested? I hadn't even considered that > >> previously since I'm not accustomed to using \g but it does seem kind of > >> pretty. I normally use ; but I suppose there's nothing wrong with just > >> declaring that asynchronous commands must be issued using \g& rather than use > >> the semicolon to fire them off. > > > > It makes sense to me... but what is the state of the session afterward? > > Should this be combined with switching to another connection? > > It's an interesting idea since you'll inevitably have to switch connections. > If you issue a second query it'll forces the session to wait for the results. > (It doesn't seem like there's any point in keeping a queue of pending queries > per session.) > > However we do still need a command to switch back anyways so there doesn't > seem to be any advantage in combining the two. I'd thought about this, and the question I came up with was: what connection should we switch to? First thought was to switch back to whatever connection we'd been using before this one, but then you'd quickly have 2 connections tied up... then what? If someone could come up with a logical session to connect to automatically that'd be great. In the meantime, what about allowing \g& accept a connection number to switch to? Also, I'd really love it if we could also do ';&'... I didn't mention it before because I'm assuming it's essentially not possible, but I'd like to be wrong... -- Jim Nasby decibel@decibel.org EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq |
| |||
| On Mon, May 14, 2007 at 11:03:52AM -0400, Tom Lane wrote: > Gregory Stark <stark@enterprisedb.com> writes: > > But these kinds of inconsistent behaviours can be traps for users. It means > > "\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres" > > do the same thing. And it means "\c1" might connect to a database named "1" > > today but switch sessions tomorrow. > > The real problem here is trying to overload an existing command name > with too many different meanings. You need to pick some other name > besides \c. > > If you were willing to think of it as "switch session" instead of "connect", > then \S is available ... Since this command will be getting used very frequently by anyone using concurrent connections interactively, it'd be nice if it was lower-case. It looks like that limits us to j, k, m, n, v, and y. In unix this idea is about jobs, what about using \j? -- Jim Nasby decibel@decibel.org EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org |
| |||
| "Jim C. Nasby" <decibel@decibel.org> writes: > Since this command will be getting used very frequently by anyone using > concurrent connections interactively, it'd be nice if it was lower-case. > It looks like that limits us to j, k, m, n, v, and y. In unix this idea > is about jobs, what about using \j? Well currently it's not really terribly interesting to use interactively since you could always just start a second shell and run a second instance of psql. I really only have regression tests in mind for it. That's why I don't find it a problem at all to only extend \g and not semicolon handling. That said, I think a next step for this for interactive use would be to handle C-z to "background" the currently running query. So perhaps it does make sense to keep use cases like that when deciding on command names now. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq |
| |||
| On Mon, May 14, 2007 at 06:26:42PM +0100, Gregory Stark wrote: > > "Jim C. Nasby" <decibel@decibel.org> writes: > > > Since this command will be getting used very frequently by anyone using > > concurrent connections interactively, it'd be nice if it was lower-case. > > It looks like that limits us to j, k, m, n, v, and y. In unix this idea > > is about jobs, what about using \j? > > Well currently it's not really terribly interesting to use interactively since > you could always just start a second shell and run a second instance of psql. > I really only have regression tests in mind for it. That's why I don't find it > a problem at all to only extend \g and not semicolon handling. > > That said, I think a next step for this for interactive use would be to handle > C-z to "background" the currently running query. So perhaps it does make sense > to keep use cases like that when deciding on command names now. Yeah, I think having the ability to open up another connection within psql will turn out to be very useful from an interactive standpoint; \c& (or whatever command we use to duplicate the current connection) is going to be a lot easier to enter than actually starting up a new psql in many production environments. -- Jim Nasby decibel@decibel.org EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---------------------------(end of broadcast)--------------------------- TIP 5: don't forget to increase your free space map settings |
| |||
| On Mon, May 14, 2007 at 11:55:07AM -0500, Jim C. Nasby wrote: > On Mon, May 14, 2007 at 11:03:52AM -0400, Tom Lane wrote: > > Gregory Stark <stark@enterprisedb.com> writes: > > > But these kinds of inconsistent behaviours can be traps for users. It means > > > "\c1" and "\c 1" do different things even though "\cpostgres" and \c postgres" > > > do the same thing. And it means "\c1" might connect to a database named "1" > > > today but switch sessions tomorrow. > > > > The real problem here is trying to overload an existing command name > > with too many different meanings. You need to pick some other name > > besides \c. > > > > If you were willing to think of it as "switch session" instead of "connect", > > then \S is available ... > > Since this command will be getting used very frequently by anyone using > concurrent connections interactively, it'd be nice if it was lower-case. > It looks like that limits us to j, k, m, n, v, and y. In unix this idea > is about jobs, what about using \j? I suppose there is some reason the bash/csh job control characters: %- %+ %1 won't work? -dg -- David Gould daveg@sonic.net If simplicity worked, the world would be overrun with insects. ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org |
| |||
| Gregory Stark wrote: > Attached is an updated patch. > This patch appears to add a nonexistent test to the regression schedules. cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match |
| |||
| Am Montag, 21. Mai 2007 15:21 schrieb Gregory Stark: > Also, I forgot to mention previously there is an unrelated trivial hunk in > here. I noticed we free the password early, presumably for security > reasons, No, to save memory. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---------------------------(end of broadcast)--------------------------- TIP 6: explain analyze is your friend |
| |||
| Gregory Stark wrote: > "Andrew Dunstan" <andrew@dunslane.net> writes: > > >> Gregory Stark wrote: >> >>> Attached is an updated patch. >>> >>> >> This patch appears to add a nonexistent test to the regression schedules. >> > > I must have forgotten to cvs add it. Sorry. > > Also, I forgot to mention previously there is an unrelated trivial hunk in > here. I noticed we free the password early, presumably for security reasons, > but don't actually overwrite the memory at that point. I added a memset in > there, otherwise the free seems kind of pointless. I normally don't bother > with "security" features like that since they don't really add any security > but as long as it's there it may as well do something vaguely useful. > > Greg, In general this looks quite reasonable. It's a very large patch for a feature of this size, but luckily it's mostly changes due to the new pset structure rather than new code. There are some places that it doesn't use PG style (e.g. no newline before brace after if) and some comments that need to be fixed (e.g. /* XXX get result */ ) If you can fix that I'll apply the patch - I especially want to get it tested widely via the buildfarm. I am leaving town for about 5 or 6 days on Friday morning, and my availability will be somewhat restricted during that time, so if this isn't fixed pronto it will possibly have to wait till my return or till another reviewer takes pity on you :-) cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq |
| |||
| Andrew Dunstan <andrew@dunslane.net> writes: > There are some places that it doesn't use PG style (e.g. no newline > before brace after if) and some comments that need to be fixed (e.g. /* > XXX get result */ ) Of course you both realize pgindent will take care of the former ;-). But yes, bogus comments are worth the trouble to fix now. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 5: don't forget to increase your free space map settings |
| ||||
| "Andrew Dunstan" <andrew@dunslane.net> writes: > Greg, > > In general this looks quite reasonable. It's a very large patch for a feature > of this size, but luckily it's mostly changes due to the new pset structure > rather than new code. Really? I expected there would be a few issues raised. For one about the need to use PG_ASYNC from libpq-int.h. Perhaps what I should do it split it into two patches, one which just adds \connect& and \S (switch connection) which will be the larger patch because it has to break out the connection structure like you mention. And a second which does the asynchronous response handling which is where there may be some questions about how to handle things. > There are some places that it doesn't use PG style (e.g. no newline before > brace after if) and some comments that need to be fixed (e.g. /* XXX get result > */ ) Ah, but it's not just the comment, if I put an XXX in it's definitely because there's something I'm not certain about. In this case now that I look at it again I think it's usually ok to ignore the result but in a session with ON_ERROR_STOP set (such as one running a script) we would want to exit I think. Another XXX is next to the include of libpq-int.h and a third one is where I have the pg_sleep loop instead of a select/poll loop. It occurs to me now that that loop should check cancel_pressed too. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---------------------------(end of broadcast)--------------------------- TIP 6: explain analyze is your friend |