public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. 11+ messages / 10 participants [nested] [flat]
* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. @ 2021-02-04 19:36 Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw) If a multi-byte character is escaped with a backslash in TEXT mode input, we didn't always treat the escape correctly. If: - a multi-byte character is escaped with a backslash, and - the second byte of the character is 0x5C, i.e. the ASCII code of a backslash (\), and - the next character is a dot (.), then CopyReadLineText function would incorrectly interpret the sequence as an end-of-copy marker (\.). This can only happen in encodings that can "embed" ascii characters as the second byte. One example of such sequence is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and load it with COPY FROM, you'd incorrectly get an "end-of-copy marker corrupt" error. Backpatch to all supported versions. Reviewed-by: John Naylor, Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi --- src/backend/commands/copyfromparse.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c index b843d315b1..315b16fd7a 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate) break; } else if (!cstate->opts.csv_mode) - + { /* * If we are here, it means we found a backslash followed by * something other than a period. In non-CSV mode, anything @@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate) * backslashes are not special, so we want to process the * character after the backslash just like a normal character, * so we don't increment in those cases. + * + * Set 'c' to skip whole character correctly in multi-byte + * encodings. If we don't have the whole character in the + * buffer yet, we might loop back to process it, after all, + * but that's OK because multi-byte characters cannot have any + * special meaning. */ raw_buf_ptr++; + c = c2; + } } /* -- 2.30.0 --------------CFC80B40DFDD66DF067F288D-- ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-19 19:18 Tom Lane <[email protected]> 0 siblings, 2 replies; 11+ messages in thread From: Tom Lane @ 2024-05-19 19:18 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers Dmitry Dolgov <[email protected]> writes: > There are lots of good takes on this in the thread. It also makes clear what's > at stake -- as Melanie pointed out with the patch about EXPLAIN for parallel > bitmap heap scan, we're loosing potential contributors for no reasons. But I'm > a bit concerned about what are the next steps: if memory serves, every couple > of years there is a discussion about everything what goes wrong with the review > process, commitfests, etc. Yet to my (admittedly limited) insight into the > community, not many things have changed due to those discussions. How do we > make sure this time it will be different? Things *have* changed, if you take the long view. We didn't have commitfests at all until around 2007, and we've changed the ground rules for them a couple of times since then. We didn't have the CF app at all until, well, I don't recall when, but the first few CFs were managed by keeping patch lists on a wiki page. It's not that people are unwilling to change this stuff, but that it's hard to identify what will make things better. IMV one really fundamental problem is the same as it's been for a couple of decades: too many patch submissions, too few committers. We can't fix that by just appointing a ton more committers, at least not if we want to keep the project's quality up. We have to grow qualified committers. IIRC, one of the main reasons for instituting the commitfests at all was the hope that if we got more people to spend time reading the code and reviewing patches, some of them would learn enough to become good committers. I think that's worked, again taking a long view. I just did some quick statistics on the commit history, and I see that we were hovering at somewhere around ten active committers from 1999 to 2012, but since then it's slowly crept up to about two dozen today. (I'm counting "active" as "at least 10 commits per year", which is an arbitrary cutoff --- feel free to slice the data for yourself.) Meanwhile the number of submissions has also grown, so I'm not sure how much better the load ratio is. My point here is not that things are great, but just that we are indeed improving, and I hope we can continue to. Let's not be defeatist about it. I think this thread has already identified a few things we have consensus to improve in the CF app, and I hope somebody goes off and makes those happen (I lack the web skillz to help myself). However, the app itself is just a tool; what counts more is our process around it. I have a couple of thoughts about that: * Patches that sit in the queue a long time tend to be ones that lack consensus, either about the goal or the details of how to achieve it. Sometimes "lacks consensus" really means "nobody but the author thinks this is a good idea, but we haven't mustered the will to say no". But I think it's more usually the case that there are plausible competing opinions about what the patch should do or how it should do it. How can we resolve such differences and get something done? * Another reason for things sitting a long time is that they're too big to review without an unreasonable amount of effort. We should encourage authors to break large patches into smaller stepwise refinements. It may seem like that will result in taking forever to reach the end goal, but dropping a huge patchset on the community isn't going to give speedy results either. * Before starting this thread, Robert did a lot of very valuable review of some individual patches. I think what prompted him to start the thread was the realization that he'd only made a small dent in the problem. Maybe we could divide and conquer: get a dozen-or-so senior contributors to split up the list of pending patches and then look at each one with an eye to what needs to happen to move it along (*not* to commit it right away, although in some cases maybe that's the thing to do). It'd be great if that could happen just before each commitfest, but that's probably not practical with the current patch volume. What I'm thinking for the moment is to try to make that happen once a year or so. > What I think wasn't discussed yet in details is the question of motivation. > Surely, it would be great to have a process that will introduce as less burden > as possible. But giving more motivation to follow the process / use the tool is > as important. What motivates folks to review patches, figure out status of a > complicated patch thread, maintain a list of open items, etc? Yeah, all this stuff ultimately gets done "for the good of the project", which isn't the most reliable motivation perhaps. I don't see a better one... regards, tom lane ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 00:20 Bruce Momjian <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 1 reply; 11+ messages in thread From: Bruce Momjian @ 2024-05-20 00:20 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers On Sun, May 19, 2024 at 03:18:11PM -0400, Tom Lane wrote: > * Another reason for things sitting a long time is that they're too > big to review without an unreasonable amount of effort. We should > encourage authors to break large patches into smaller stepwise > refinements. It may seem like that will result in taking forever > to reach the end goal, but dropping a huge patchset on the community > isn't going to give speedy results either. I think it is sometimes hard to incrementally apply patches if the long-term goal isn't agreed or know to be achievable. > * Before starting this thread, Robert did a lot of very valuable > review of some individual patches. I think what prompted him to > start the thread was the realization that he'd only made a small > dent in the problem. Maybe we could divide and conquer: get a > dozen-or-so senior contributors to split up the list of pending > patches and then look at each one with an eye to what needs to > happen to move it along (*not* to commit it right away, although > in some cases maybe that's the thing to do). It'd be great if > that could happen just before each commitfest, but that's probably > not practical with the current patch volume. What I'm thinking > for the moment is to try to make that happen once a year or so. For me, if someone already knows what the blocker is, it saves me a lot of time if they can state that somewhere. -- Bruce Momjian <[email protected]> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 01:09 Tom Lane <[email protected]> parent: Bruce Momjian <[email protected]> 0 siblings, 3 replies; 11+ messages in thread From: Tom Lane @ 2024-05-20 01:09 UTC (permalink / raw) To: Bruce Momjian <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers Bruce Momjian <[email protected]> writes: > On Sun, May 19, 2024 at 03:18:11PM -0400, Tom Lane wrote: >> * Another reason for things sitting a long time is that they're too >> big to review without an unreasonable amount of effort. We should >> encourage authors to break large patches into smaller stepwise >> refinements. It may seem like that will result in taking forever >> to reach the end goal, but dropping a huge patchset on the community >> isn't going to give speedy results either. > I think it is sometimes hard to incrementally apply patches if the > long-term goal isn't agreed or know to be achievable. True. The value of the earlier patches in the series can be unclear if you don't understand what the end goal is. But I think people could post a "road map" of how they intend a patch series to go. Another way of looking at this is that sometimes people do post large chunks of work in long many-patch sets, but we tend to look at the whole series as something to review and commit as one (or I do, at least). We should be more willing to bite off and push the earlier patches in such a series even when the later ones aren't entirely done. (The cfbot tends to discourage this, since as soon as one of the patches is committed it no longer knows how to apply the rest. Can we improve on that tooling somehow?) regards, tom lane ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 05:49 Thomas Munro <[email protected]> parent: Tom Lane <[email protected]> 2 siblings, 2 replies; 11+ messages in thread From: Thomas Munro @ 2024-05-20 05:49 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers On Mon, May 20, 2024 at 1:09 PM Tom Lane <[email protected]> wrote: > (The cfbot tends to discourage this, since as soon as one of the > patches is committed it no longer knows how to apply the rest. > Can we improve on that tooling somehow?) Cfbot currently applies patches with (GNU) patch --no-backup-if-mismatch -p1 -V none -f. The -f means that any questions of the form "Reversed (or previously applied) patch detected! Assume -R? [y]" is answered with "yes", and the operation fails. I wondered if --forward would be better. It's the right idea, but it seems to be useless in practice for this purpose because the command still fails: tmunro@phonebox postgresql % patch --forward -p1 < x.patch || echo XXX failed $? patching file 'src/backend/postmaster/postmaster.c' Ignoring previously applied (or reversed) patch. 1 out of 1 hunks ignored--saving rejects to 'src/backend/postmaster/postmaster.c.rej' XXX failed 1 I wondered if it might be distinguishable from other kinds of failure that should stop progress, but nope: patch's exit status is 0 if all hunks are applied successfully, 1 if some hunks cannot be applied or there were merge conflicts, and 2 if there is more serious trouble. When applying a set of patches in a loop it behooves you to check this exit status so you don't apply a later patch to a partially patched file. I guess I could parse stdout or whatever that is and detect all-hunks-ignored condition, but that doesn't sound like fun... Perhaps cfbot should test explicitly for patches that have already been applied with something like "git apply --reverse --check", and skip them. That would work for exact patches, but of course it would be confused by any tweaks made before committing. If going that way, it might make sense to switch to git apply/am (instead of GNU patch), to avoid contradictory conclusions. The reason I was using GNU patch in the first place is that it is/was a little more tolerant of some of the patches people used to send a few years back, but now I expect everyone uses git format-patch and would be prepared to change their ways if not. In the past we had a couple of cases of the reverse, that is, GNU patch couldn't apply something that format-patch produced (some edge case of renaming, IIRC) and I'm sorry that I never got around to changing that. Sometimes I question the sanity of the whole thing. Considering cfbot's original "zero-effort CI" goal (or I guess "zero-extra-effort" would be better), I was curious about what other projects had the same idea, or whether we're really just starting at the "wrong end", and came up with: https://github.com/getpatchwork/patchwork http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf <-- example user https://github.com/patchew-project/patchew Actually cfbot requires more effort than those, because it's driven first by Commitfest app registration. Those projects are extremists IIUC: just write to a mailing list, no other bureaucracy at all (at least for most participants, presumably administrators can adjust the status in some database when things go wrong?). We're actually halfway to Gitlab et al already, with a web account and interaction required to start the process of submitting a patch for consideration. What I'm less clear on is who else has come up with the "bitrot" test idea, either at the mailing list or web extremist ends of the scale. Those are also generic tools, and cfbot obviously knows lots of things about PostgreSQL, like the "highlights" and probably more things I'm forgetting. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 08:16 Daniel Gustafsson <[email protected]> parent: Thomas Munro <[email protected]> 1 sibling, 0 replies; 11+ messages in thread From: Daniel Gustafsson @ 2024-05-20 08:16 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers > On 20 May 2024, at 07:49, Thomas Munro <[email protected]> wrote: > We're actually > halfway to Gitlab et al already, with a web account and interaction > required to start the process of submitting a patch for consideration. Another Web<->Mailinglist extreme is Git themselves who have a Github bridge for integration with their usual patch-on-mailinglist workflow. https://github.com/gitgitgadget/gitgitgadget > What I'm less clear on is who else has come up with the "bitrot" test > idea, either at the mailing list or web extremist ends of the scale. Most web based platforms like Github register the patch against the tree at the time of submitting, and won't refresh unless the user does so. Github does detect bitrot and show a "this cannot be merged" error message, but it doesn't alter any state on the PR etc. -- Daniel Gustafsson ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 09:20 Mark Cave-Ayland <[email protected]> parent: Tom Lane <[email protected]> 2 siblings, 0 replies; 11+ messages in thread From: Mark Cave-Ayland @ 2024-05-20 09:20 UTC (permalink / raw) To: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers On 20/05/2024 06:56, Mark Cave-Ayland wrote: > In general you find that a series consists of 2 parts: 1) a set of refactorings to > enable a new feature and 2) the new feature itself. Even if the details of 2) are > still under discussion, often it is possible to merge 1) fairly quickly which also > has the knock-on effect of reducing the size of later iterations of the series. This > also helps with new contributors since having parts of the series merged sooner helps > them feel valued and helps to provide immediate feedback. [resend due to DKIM header failure] Something else I also notice is that PostgreSQL doesn't have a MAINTAINERS or equivalent file, so when submitting patches it's difficult to know who is expected to review and/or commit changes to a particular part of the codebase (this is true both with and without the CF system). ATB, Mark. ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 11:41 Alvaro Herrera <[email protected]> parent: Tom Lane <[email protected]> 2 siblings, 0 replies; 11+ messages in thread From: Alvaro Herrera @ 2024-05-20 11:41 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers On 2024-May-19, Tom Lane wrote: > (The cfbot tends to discourage this, since as soon as one of the > patches is committed it no longer knows how to apply the rest. > Can we improve on that tooling somehow?) I think a necessary next step to further improve the cfbot is to get it integrated in pginfra. Right now it runs somewhere in Thomas's servers or something, and there's no real integration with the commitfest proper except by scraping. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "La libertad es como el dinero; el que no la sabe emplear la pierde" (Alvarez) ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 14:16 Robert Haas <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 1 reply; 11+ messages in thread From: Robert Haas @ 2024-05-20 14:16 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; pgsql-hackers On Sun, May 19, 2024 at 3:18 PM Tom Lane <[email protected]> wrote: > Dmitry Dolgov <[email protected]> writes: > > How do we make sure this time it will be different? > > Things *have* changed, if you take the long view. That's true, but I think Dmitry's point is well-taken all the same: we haven't really made a significant process improvement in many years, and in some ways, I think things have been slowly degrading. I don't believe it's necessary or possible to solve all of the accumulated problems overnight, but we need to get serious about admitting that there is a problem which, in my opinion, is an existential threat to the project. > IMV one really fundamental problem is the same as it's been for a > couple of decades: too many patch submissions, too few committers. > We can't fix that by just appointing a ton more committers, at least > not if we want to keep the project's quality up. We have to grow > qualified committers. IIRC, one of the main reasons for instituting > the commitfests at all was the hope that if we got more people to > spend time reading the code and reviewing patches, some of them would > learn enough to become good committers. I think that's worked, again > taking a long view. I just did some quick statistics on the commit > history, and I see that we were hovering at somewhere around ten > active committers from 1999 to 2012, but since then it's slowly crept > up to about two dozen today. (I'm counting "active" as "at least 10 > commits per year", which is an arbitrary cutoff --- feel free to slice > the data for yourself.) Meanwhile the number of submissions has also > grown, so I'm not sure how much better the load ratio is. That's an interesting statistic. I had not realized that the numbers had actually grown significantly. However, I think that the new-patch-submitter experience has not improved; if anything, I think it may have gotten worse. It's hard to compare the subjective experience between 2008 when I first got involved and now, especially since at that time I was a rank newcomer experiencing things as a newcomer, and now I'm a long-time committer trying to judge the newcomer experience. But it seems to me that when I started, there was more of a "middle tier" of people who were not committers but could do meaningful review of patches and help you push them in the direction of being something that a committer might not loathe. Now, I feel like there are a lot of non-committer reviews that aren't actually adding a lot of value: people come along and say the patch doesn't apply, or a word is spelled wrong, and we don't get meaningful review of whether the design makes sense, or if we do, it's wrong. Perhaps this is just viewing the past with rose-colored glasses: I wasn't in as good a position to judge the value of reviews that I gave and received at that point as I am now. But what I do think is happening today is that a lot of committer energy gets focused on the promising non-committers who someone thinks might be able to become committers, and other non-committers struggle to make any headway. On the plus side, I think we make more of an effort not to be a jerk to newcomers than we used to. I also have a strong hunch that it may not be as good as it needs to be. > * Patches that sit in the queue a long time tend to be ones that lack > consensus, either about the goal or the details of how to achieve it. > Sometimes "lacks consensus" really means "nobody but the author thinks > this is a good idea, but we haven't mustered the will to say no". > But I think it's more usually the case that there are plausible > competing opinions about what the patch should do or how it should > do it. How can we resolve such differences and get something done? This is a great question. We need to do better with that. Also, it would be helpful to have better ways of handling it when the author has gotten to a certain point with it but doesn't necessarily have the time/skills/whatever to drive it forward. Such patches are quite often a good idea, but it's not clear what we can do with them procedurally other than hit the reject button, which doesn't feel great. > * Another reason for things sitting a long time is that they're too > big to review without an unreasonable amount of effort. We should > encourage authors to break large patches into smaller stepwise > refinements. It may seem like that will result in taking forever > to reach the end goal, but dropping a huge patchset on the community > isn't going to give speedy results either. Especially if it has a high rate of subtle defects, which is common. > * Before starting this thread, Robert did a lot of very valuable > review of some individual patches. I think what prompted him to > start the thread was the realization that he'd only made a small > dent in the problem. Maybe we could divide and conquer: get a > dozen-or-so senior contributors to split up the list of pending > patches and then look at each one with an eye to what needs to > happen to move it along (*not* to commit it right away, although > in some cases maybe that's the thing to do). It'd be great if > that could happen just before each commitfest, but that's probably > not practical with the current patch volume. What I'm thinking > for the moment is to try to make that happen once a year or so. I like this idea. > Yeah, all this stuff ultimately gets done "for the good of the > project", which isn't the most reliable motivation perhaps. > I don't see a better one... This is the really hard part. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-20 21:33 Maciek Sakrejda <[email protected]> parent: Thomas Munro <[email protected]> 1 sibling, 0 replies; 11+ messages in thread From: Maciek Sakrejda @ 2024-05-20 21:33 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Tom Lane <[email protected]>; Bruce Momjian <[email protected]>; Dmitry Dolgov <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers On Sun, May 19, 2024 at 10:50 PM Thomas Munro <[email protected]> wrote: > Sometimes I question the sanity of the whole thing. Considering > cfbot's original "zero-effort CI" goal (or I guess "zero-extra-effort" > would be better), I was curious about what other projects had the same > idea, or whether we're really just starting at the "wrong end", and > came up with: > > https://github.com/getpatchwork/patchwork > http://vger.kernel.org/bpfconf2022_material/lsfmmbpf2022-bpf-ci.pdf > <-- example user > https://github.com/patchew-project/patchew > > Actually cfbot requires more effort than those, because it's driven > first by Commitfest app registration. Those projects are extremists > IIUC: just write to a mailing list, no other bureaucracy at all (at > least for most participants, presumably administrators can adjust the > status in some database when things go wrong?). We're actually > halfway to Gitlab et al already, with a web account and interaction > required to start the process of submitting a patch for consideration. > What I'm less clear on is who else has come up with the "bitrot" test > idea, either at the mailing list or web extremist ends of the scale. > Those are also generic tools, and cfbot obviously knows lots of things > about PostgreSQL, like the "highlights" and probably more things I'm > forgetting. For what it's worth, a few years before cfbot, I had privately attempted a similar idea for Postgres [1]. The project here is basically a very simple API and infrastructure for running builds and make check. A previous version [2] subscribed to the mailing lists and used Travis CI (and accidentally spammed some Postgres committers [3]). The project petered out as my work responsibilities shifted (and to be honest, after I felt sheepish about the spamming). I think cfbot is way, way ahead of where my project got at this point. But since you asked about other similar projects, I'm happy to discuss further if it's helpful to bounce ideas off someone who's thought about the same problem (though not for a while now, I admit). Thanks, Maciek [1]: https://github.com/msakrejda/pg-quilter [2]: https://github.com/msakrejda/pg-quilter/blob/2038d9493f9aa7d43d3eb0aec1d299b94624602e/lib/pg-quilter... [3]: https://www.postgresql.org/message-id/flat/CAM3SWZQboGoVYAJNoPMx%3DuDLE%2BZh5k2MQa4dWk91YPGDxuY-gQ%4... ^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: commitfest.postgresql.org is no longer fit for purpose @ 2024-05-24 15:40 Tomas Vondra <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 0 replies; 11+ messages in thread From: Tomas Vondra @ 2024-05-24 15:40 UTC (permalink / raw) To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Dmitry Dolgov <[email protected]>; pgsql-hackers On 5/20/24 16:16, Robert Haas wrote: > On Sun, May 19, 2024 at 3:18 PM Tom Lane <[email protected]> wrote: > >... > >> * Another reason for things sitting a long time is that they're too >> big to review without an unreasonable amount of effort. We should >> encourage authors to break large patches into smaller stepwise >> refinements. It may seem like that will result in taking forever >> to reach the end goal, but dropping a huge patchset on the community >> isn't going to give speedy results either. > > Especially if it has a high rate of subtle defects, which is common. > I think breaking large patches into reasonably small parts is a very important thing. Not only is it really difficult (or perhaps even practically impossible) to review patches over a certain size, because you have to grok and review everything at once. But it's also not great from the cost/benefit POV - the improvement may be very beneficial, but if it's one huge lump of code that never gets committable as a whole, there's no benefit in practice. Which makes it less likely I'll even start looking at the patch very closely, because there's a risk it'd be just a waste of time in the end. So I think this is another important reason to advise people to split patches into smaller parts - not only it makes it easier to review, it makes it possible to review and commit the parts incrementally, getting at least some benefits early. >> * Before starting this thread, Robert did a lot of very valuable >> review of some individual patches. I think what prompted him to >> start the thread was the realization that he'd only made a small >> dent in the problem. Maybe we could divide and conquer: get a >> dozen-or-so senior contributors to split up the list of pending >> patches and then look at each one with an eye to what needs to >> happen to move it along (*not* to commit it right away, although >> in some cases maybe that's the thing to do). It'd be great if >> that could happen just before each commitfest, but that's probably >> not practical with the current patch volume. What I'm thinking >> for the moment is to try to make that happen once a year or so. > > I like this idea. > Me too. Do you think it'd happen throughout the whole year, or at some particular moment? We used to do a "triage" at the FOSDEM PGDay meeting, but that used to be more of an ad hoc thing to use the remaining time, with only a small subset of people. But that seems pretty late in the dev cycle, I guess we'd want it to happen early, perhaps during the first CF? FWIW this reminds me the "CAN reports" tracking the current "conditions, actions and needs" of a ticket. I do that for internal stuff, and I find that quite helpful (as long as it's kept up to date). >> Yeah, all this stuff ultimately gets done "for the good of the >> project", which isn't the most reliable motivation perhaps. >> I don't see a better one... > > This is the really hard part. > I think we have plenty of motivated people with good intentions. Some are motivated by personal interest, some by trying to achieve stuff related to their work/research/... I don't think the exact reasons matter too much, and it's often a combination. IMHO we should look at this from the other end - people are motivated to get a patch reviewed & committed, and if we introduce a process that's more likely to lead to that result, people will mostly adopt that. And if we could make that process more convenient by improving the CF app to support it, that'd be even better ... I'm mostly used to the mailing list idea, but with the volume it's a constant struggle to keep track of new patch versions that I wanted/promised to review, etc. The CF app helps with that a little bit, because I can "become a reviewer" but I still don't get notifications or anything like that :-( regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2024-05-24 15:40 UTC | newest] Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]> 2024-05-19 19:18 Re: commitfest.postgresql.org is no longer fit for purpose Tom Lane <[email protected]> 2024-05-20 00:20 ` Re: commitfest.postgresql.org is no longer fit for purpose Bruce Momjian <[email protected]> 2024-05-20 01:09 ` Re: commitfest.postgresql.org is no longer fit for purpose Tom Lane <[email protected]> 2024-05-20 05:49 ` Re: commitfest.postgresql.org is no longer fit for purpose Thomas Munro <[email protected]> 2024-05-20 08:16 ` Re: commitfest.postgresql.org is no longer fit for purpose Daniel Gustafsson <[email protected]> 2024-05-20 21:33 ` Re: commitfest.postgresql.org is no longer fit for purpose Maciek Sakrejda <[email protected]> 2024-05-20 09:20 ` Re: commitfest.postgresql.org is no longer fit for purpose Mark Cave-Ayland <[email protected]> 2024-05-20 11:41 ` Re: commitfest.postgresql.org is no longer fit for purpose Alvaro Herrera <[email protected]> 2024-05-20 14:16 ` Re: commitfest.postgresql.org is no longer fit for purpose Robert Haas <[email protected]> 2024-05-24 15:40 ` Re: commitfest.postgresql.org is no longer fit for purpose Tomas Vondra <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox