CommitFest 2009-09, two weeks on
in [PgSql]
|
From: Joe Conway on 30 Sep 2009 13:33 Robert Haas wrote: > On Wed, Sep 30, 2009 at 12:27 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> Joe Conway <mail(a)joeconway.com> writes: >>> Robert Haas wrote: >>>> - There is one dblink pach left over from last CommitFest. Joe Conway >>>> was going to review it the weekend of July 18th-19th, but that didn't >>>> end up happening and so that patch is still waiting. We might be able >>>> to find someone else to review it, but I'm not sure whether that will >>>> help unless there is a committer other than Joe with bandwidth to do >>>> the final review and commit. >>> I will get to it before the end of this commitfest, but I have to admit >>> I'm not all that excited about this patch in the first place. I don't >>> know that I agree with the need. >> Well, you're the dblink expert. If you think it should be rejected >> I doubt many of us will argue with you. > > Yep. CommitFest doesn't mean "commit it"; it means "decide whether to > commit it". Things being rejected or returned with feedback for > further improvement is fine; we're just trying to avoid long periods > with no response at all. The issue is not so much technical as it is philosophical. The patch basically forces all use of libpq by dblink to be asynchronous (internally) so that a cancel can be sensed and passed down to the remote side and everything cleaned up. Possibly the right thing to do, but dblink already allows the use of async queries, and the current synchronous method uses standard libpq calls. If all of this is really necessary, doesn't every libpq client have the same issue? If so why have the synchronous libpq functions at all? So while I can vet the patch technically, and spend more time understanding the use case, and maybe explaining it better, I think other people should weigh in on the change as it is significant and points to other potential issues. Joe
From: Magnus Hagander on 30 Sep 2009 14:08 On Wed, Sep 30, 2009 at 18:34, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas(a)gmail.com> writes: >> ... (and many of the more >> significant remaining patches look like they are right up Tom's alley >> anyway). > > FWIW, if left to my own devices I will eventually get to everything > except the dblink, ecpg, and encoding/win32 patches. I don't intend > to touch any of those because there are other committers better > qualified to review them. (I don't actually think we have anybody > except Michael who's really familiar with ecpg.) I can certainly review the win32 encoding patch, but I was rather hoping for some comments from others on if we're interested in a win32 only solution, or if we want something more generic. Should we just go with the win32-only one for now? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Alvaro Herrera on 30 Sep 2009 14:11 Magnus Hagander escribi�: > On Wed, Sep 30, 2009 at 18:34, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > > Robert Haas <robertmhaas(a)gmail.com> writes: > >> ... (and many of the more > >> significant remaining patches look like they are right up Tom's alley > >> anyway). > > > > FWIW, if left to my own devices I will eventually get to everything > > except the dblink, ecpg, and encoding/win32 patches. �I don't intend > > to touch any of those because there are other committers better > > qualified to review them. �(I don't actually think we have anybody > > except Michael who's really familiar with ecpg.) > > I can certainly review the win32 encoding patch, but I was rather > hoping for some comments from others on if we're interested in a win32 > only solution, or if we want something more generic. Should we just go > with the win32-only one for now? Just a couple of days ago a question came on the spanish list because someone was getting mixed UTF8 and Latin1 output in a log file. This was in Fedora IIRC, so maybe we do want something more general. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 30 Sep 2009 15:38 Magnus Hagander <magnus(a)hagander.net> writes: > I can certainly review the win32 encoding patch, but I was rather > hoping for some comments from others on if we're interested in a win32 > only solution, or if we want something more generic. Should we just go > with the win32-only one for now? That was actually the only substantive comment I had about it. I don't see why it's a win32-only problem or why a win32-only solution is a good approach. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Magnus Hagander on 30 Sep 2009 15:45
On Wed, Sep 30, 2009 at 21:38, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Magnus Hagander <magnus(a)hagander.net> writes: >> I can certainly review the win32 encoding patch, but I was rather >> hoping for some comments from others on if we're interested in a win32 >> only solution, or if we want something more generic. Should we just go >> with the win32-only one for now? > > That was actually the only substantive comment I had about it. I don't > see why it's a win32-only problem or why a win32-only solution is a good > approach. Yeah, that's my thought as well. If we want a complete one, we should reject this patch and ask for one that does that. If we are fine with a win32 only one, I can review this one and get it in. I'm leaning towards us wanting a general one, but I'm unsure how much work that will take. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |