Re: [HACKERS] Lessons from commit fest
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Wed, Apr 16, 2008 at 2:17 AM, Joshua D. Drake wrote: On Tue, 15 Apr 2008 12:12:24 -0400 Alvaro Herrera wrote: Tom Lane wrote: I tend to just fix this stuff while committing, rather than lecture the submitters about it, but it undoubtedly is a time sink. Lesson learned: a useful task for another reviewer to do is to grab the patch, fix the style issues, and post the fixed version. That way, the higher level reviewer does not have to waste time on a task that, really, anybody can do. The idea that we fix stylistic issues on the fly is not sustainable. We should offer help and mentorship to new patch submitters in all areas (including stylistic) but they should do the work. It is the only way we will mold them to submit patches in the proper way. I agree. As a submitter I would much rather get an email saying e.g. Hey, your patch is nice but the code style sticks out like a sore thumb. Please adopt surrounding naming convention and fix your indentation per the rules at [link]. than have it fixed silently on its way to being committed. With the former I learn something and get to improve my own work. With the latter, my next patch is probably going to have the exact same problem, which is in the long term just making extra work for the reviewers. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBZZ95YBsbHkuyV0RAjviAJ9pBG6I3DAySIx6ARp2cLnYe+bAdACg/VAR Xgvpg4iyWsbNk4QKGI//diw= =yDoO -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
On Apr 14, 2008, at 4:15 PM, Bruce Momjian wrote: If you don't want an issue to get forgotten, then make a TODO entry for it. But the purpose of commit fest is to make sure we deal with things that can be dealt with in a timely fashion. It's not going to cause solutions to unsolved problems to appear from nowhere. I need is to know if they are ideas worthy of TODO. This is something I think we could really improve upon... For one, I think the impression (right or wrong) is that Bruce is the person that gets say on what goes on the TODO. We also don't have any good mechanism for general users to provide feedback on what things are important to them. Of course it's ultimately about a developer scratching an itch, but I think it would be useful to have some idea of how popular TODO items were to help guide development efforts, as well as possibly target items that should just be removed. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] Lessons from commit fest
Hi, The idea that we fix stylistic issues on the fly is not sustainable. We should offer help and mentorship to new patch submitters in all areas (including stylistic) but they should do the work. It is the only way we will mold them to submit patches in the proper way. I agree. As a submitter I would much rather get an email saying e.g. Hey, your patch is nice but the code style sticks out like a sore thumb. Please adopt surrounding naming convention and fix your indentation per the rules at [link]. than have it fixed silently on its way to being committed. With the former I learn something and get to improve my own work. With the latter, my next patch is probably going to have the exact same problem, which is in the long term just making extra work for the reviewers. I think, us patch-submitters should be asked to do a run of pg_indent on the files that we have modified. That should take care of atleast the indentation related issues. I looked at the README of src/tools/pgindent, and it should be easy to run enough (or is it not?). Only one thing that caught my eye was: 1) Build the source tree with _debug_ symbols and all possible configure options Can the above point be elaborated further? What all typical and possible configure options should be used to get a clean and complete pg_indent run? And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] pulling libpqtypes from queue
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Tue, Apr 15, 2008 at 10:57:37AM -0400, Merlin Moncure wrote: On Tue, Apr 15, 2008 at 10:48 AM, [EMAIL PROTECTED] wrote: On Tue, Apr 15, 2008 at 09:48:37AM -0400, Merlin Moncure wrote: On Tue, Apr 15, 2008 at 9:36 AM, Alvaro Herrera [EMAIL PROTECTED] wrote: I expect you intend to get at least the hooks in, right? [...] libpqtypes was designed to handle this with our without hooking. (the 'hooking' debate was mainly about exactly how libpq and libpqtypes was going to be separated). libpqtypes had a superclassing concept (not much discussed on the lists) where you could introduce new type handlers that wrapped existing ones and was desgined exactly for things like this. That sounds cool. So in a way you do have the hooks. That would be the libpqtypes without the types, no? keep an eye on our upcoming pgfoundry project. I sure will. Thanks - -- tomás -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFIBaFWBcgs9XrR2kYRAgHNAJ9L59j9eHw0EkZ5Imzg5txAX1lF4ACcCbgS 6bgw0sejoFlCbHXvV55kimo= =QuvI -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCHES] [HACKERS] Text - C string
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 - -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sat, Mar 29, 2008 at 5:40 AM, Brendan Jurd wrote: On 29/03/2008, Tom Lane wrote: I intentionally didn't touch xml.c, nor anyplace that is not dealing in text, even if it happens to be binary-compatible with text. Hmm, okay. My original submission did include a few such changes; for example, in xml_in and xml_out_internal I saw that the conversion between xmltype and cstring was identical to the text conversion, so I went ahead and used the text functions. Feedback upthread suggested that it was okay to go ahead with casting in identical cases. [1] I saw that these changes made it into the commit, so I assumed that it was the right call. If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? Cheers, BJ - -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBaFy5YBsbHkuyV0RAsMmAKDHaEb7aMudKJgVxcf5RRcOtAJ+bwCgivLI 5B3xJze46NGWjEyOpq/TSaY= =BObd - -END PGP SIGNATURE- -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBaIl5YBsbHkuyV0RArDDAJ0QThLXAhzy+NX+2YsF+q4z/sIy1QCeJPiW s/rVns3FFQVP5g9DTOshDfQ= =4Tdh -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
NikhilS wrote: Hi, The idea that we fix stylistic issues on the fly is not sustainable. We should offer help and mentorship to new patch submitters in all areas (including stylistic) but they should do the work. It is the only way we will mold them to submit patches in the proper way. I agree. As a submitter I would much rather get an email saying e.g. Hey, your patch is nice but the code style sticks out like a sore thumb. Please adopt surrounding naming convention and fix your indentation per the rules at [link]. than have it fixed silently on its way to being committed. With the former I learn something and get to improve my own work. With the latter, my next patch is probably going to have the exact same problem, which is in the long term just making extra work for the reviewers. I think, us patch-submitters should be asked to do a run of pg_indent on the files that we have modified. That should take care of atleast the indentation related issues. I looked at the README of src/tools/pgindent, and it should be easy to run enough (or is it not?). Only one thing that caught my eye was: 1) Build the source tree with _debug_ symbols and all possible configure options Can the above point be elaborated further? What all typical and possible configure options should be used to get a clean and complete pg_indent run? And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Pg_upgrade - page layout converter (PLC) hook
Heikki Linnakangas napsal(a): Zdenek Kotala wrote: Heikki Linnakangas napsal(a): I would suggest starting with putting some serious effort into pg_migrator. This page layout conversion is actually pretty trivial compared to all that, and even more so if you can get the page layout conversion working in pg_migrator first. By my opinion pg_migrator is good workaround but it is not suitable for real production. For example TOAST relid protection is dirty hack and you have problem with tablespaces and so on... Sure, it's not perfect, that's exactly why I suggested working on it! If you have something else that works better, that's fine, but you will need to release it under the BSD license if you want help with it. Sure, my upgrade script is in opensolaris and it is under CDDL license. By my meaning it is only workaround until PostgreSQL 8.x will not have upgrade integrated inside. After that this script will be removed. Of course all work will be under BSD. Which versions do you plan to support, initially? Currently I'm able to upgrade from 8.1, 8.2 to 8.3, 8.4. It means conversion between layout version 3 and 4. I'm verifying PLC now and when this part will be ready it is very easy to implement others as well. Hmm. I don't see any of that code in the directory you posted. ? PLC is not there yet, because it is not reviewed, tested and verified. And I tested it only on SPARC where varlen works without any modification, but on x86 it needs extra work yet. b) log_newpage is used for new page logging, but I use it for storing converted page. It seems to me that it safe and heap_xlog_newpage correctly works for new and converted page. I have only doubt about assert macro mdextend/mdwrite which checks extend vs.write. That should be fine. In WAL replay, we don't distinguish between write and extend. But in this case the asserts macros in mdexted/mdwrite are odd and the should be removed. Those asserts are enforced during normal operation. It's just in WAL replay that we extend the relation automatically when full pages are restored. See XLogReadBuffer(). OK + /* Hook for page layout convertor plugin */ + typedef void (*plc_hook_type)(char *buffer); + extern PGDLLIMPORT plc_hook_type plc_hook; That's not flexible enough. We've changed the internal representations of data types in the past, and will likely do that in the future. The hook therefore needs to have at least the tuple descriptor, to know how to convert the tuples. I would suggest passing Relation, we have that available at the call site, and that should contain all the necessary information. Good point, I thought about it. I currently don't use it but in the future it could be useful. I will extend it. Surely you need it already to do the packed varlen conversion? It works on SPARC without any varlen modification :-). I originally planed to implement varlen modification in another way but it seems to be better to do it in this place as well. Thanks for your comment -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] count(*) performance improvement ideas
Tom Lane wrote: Stephen Denne [EMAIL PROTECTED] writes: From: Tom Lane [mailto:[EMAIL PROTECTED] As for 2) and 3), can't you look into the pg_settings view? pg_settings view doesn't contain custom variables created on the fly, Really? [ pokes around ... ] Hm, you're right, because add_placeholder_variable() sets the GUC_NO_SHOW_ALL flag, and in this usage it'll never be cleared. I wonder if we should change that. The whole thing is a bit of an abuse of what the mechanism was intended for, and so I'm not sure we should rejigger GUC's behavior to make it more pleasant, but on the other hand if we're not ready to provide a better substitute ... While I agree with that part, is there any actual *reason* why we shouldn't have the custom variables included in pg_settings? ISTM that it would be usable in cases that aren't an abuse as well... //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I think if we want pg_terminate_backend, we have to do it the way that was originally discussed: have it issue SIGTERM and fix whatever needs to be fixed in the SIGTERM code path. Well, with no movement on this TODO item since it was removed in 8.0, I am willing to give users something that works most of the time. If the users want it so bad, why has no one stepped up to do the testing? Good question. Tom and I talked about this on the phone today. I think the problem is testing to try to prove the lack of a bug. How long does someone test to know they have reasonably proven a bug doesn't exist? Right. I think we have *a lot* of users that regularly use SIGTERM to kill the backends, becuase they have no idea it's dangerous. They just use ps to find the backend and kill to see it go away. If we had a *big* problem with it, we'd hear about it. So I doubt we have. But it's quite possible that we have a narrow problem that can cause problems in some issues - but I expect most people who run this stuff without knowing it's dangerous aren't running the worlds largest and most loaded databases... //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane wrote: I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4 if there is some reasonable amount of testing done during this development cycle to try to expose any problems. If no one is willing to do any such testing, then as far as I'm concerned there is no market for the feature and we should drop it from TODO. If someone can come up with an automated script to do this kind of testing, I can commit a VM or three to running this 24/7 for a month, easily... But I don't trust myself in coming up with a test-case that's good enough :-P //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem with site doc search
Notice that : http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=r and http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=d do not provide same result (3 results by date, 1 by rank) even if only the sorting is changed. -- Cédric Villemain Administrateur de Base de Données Cel: +33 (0)6 74 15 56 53 http://dalibo.com - http://dalibo.org signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] DROP DATABASE vs patch to not remove files right away
Tom Lane wrote: ISTM that we must fix the bgwriter so that ForgetDatabaseFsyncRequests causes PendingUnlinkEntrys for the doomed DB to be thrown away too. This should prevent the unlink-live-data scenario, I think. Even then, concurrent deletion attempts are probably possible (since ForgetDatabaseFsyncRequests is asynchronous) and rmtree() is being far too fragile about dealing with them. I think that it should be coded to ignore ENOENT the same as the bgwriter does, and that it should press on and keep trying to delete things even if it gets a failure. Yep. I can write a patch for that, unless you're onto it already? However, this makes me reconsider Florian's suggestion to just make relfilenode larger and avoid reusing them altogether. It would simplify the code quite a bit, and make it more robust. That is good because even if we fix these problems per your suggestion, I'm left wondering if we've missed some even weirder corner-cases. Florian suggested a scheme where the xid and epoch is embedded in the filename, but that's unnecessarily complex. We could just make relfilenode a 64-bit integer. 2^64 should be enough for everyone. You listed these problems with Florian's suggestion back then: 1. Zero chance of ever backpatching. (I know I said I wasn't excited about that, but it's still a strike against a proposed fix.) Still true. We would need to do what you suggested for 8.3, but simplifying the code would be good thing in the long run. 2. Adds new fields to RelFileNode, which will be a major code change, and possibly a noticeable performance hit (bigger hashtable keys). We talked about this wrt. map forks, and concluded that it's not an issue. If we add the map forks as well, BufferTag struct would grow from 16 bytes to 24 bytes. It's worth doing some more micro-benchmarking with that, but it's probably acceptable. Or we could allocate a few bits of the 64-bit relfilenode field in RelFileNode to indicate the map fork. 3. Adds new columns to pg_class, which is a real PITA ... We would only have to change relfilenode from oid to int64. 4. Breaks oid2name and all similar code that knows about relfilenode. True, but they're not hard to fix. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem with site doc search
Oleg Bartunov wrote: On Tue, 15 Apr 2008, Magnus Hagander wrote: I didn't do anything, but possibly it got fixed by a different upgrade at some point, and the recrawling of the sites. Magnus, we have parser for indexing pgdocs, do you need it ? Yes, please! //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem with site doc search
Cédric Villemain wrote: Notice that : http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=r and http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=d do not provide same result (3 results by date, 1 by rank) even if only the sorting is changed. Actually, I get 5 and 7, in the other order. The reason for this is that Tom Lane is way too active. It's gin_fuzzy_search_limit that's doing the restriction first, and the date restriction comes in later. //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Problem with site doc search
Le Wednesday 16 April 2008, Magnus Hagander a écrit : Cédric Villemain wrote: Notice that : http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=r and http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=d do not provide same result (3 results by date, 1 by rank) even if only the sorting is changed. Actually, I get 5 and 7, in the other order. The reason for this is that Tom Lane is way too active. It's gin_fuzzy_search_limit that's doing the restriction first, and the date restriction comes in later. Yes, you are perfectly right. Can I suggest to deactivate gin_fuzzy_search_limit (or increase the value) when one condition (and use the condition earlier) can considerably reduce the number of results (like the 'post date' here) ? //Magnus -- Cédric Villemain Administrateur de Base de Données Cel: +33 (0)6 74 15 56 53 http://dalibo.com - http://dalibo.org signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Problem with site doc search
Cédric Villemain wrote: Le Wednesday 16 April 2008, Magnus Hagander a écrit : Cédric Villemain wrote: Notice that : http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=r and http://search.postgresql.org/search?q=tom+lanem=1l=d=1s=d do not provide same result (3 results by date, 1 by rank) even if only the sorting is changed. Actually, I get 5 and 7, in the other order. The reason for this is that Tom Lane is way too active. It's gin_fuzzy_search_limit that's doing the restriction first, and the date restriction comes in later. Yes, you are perfectly right. Can I suggest to deactivate gin_fuzzy_search_limit (or increase the value) when one condition (and use the condition earlier) can considerably reduce the number of results (like the 'post date' here) ? You'd have to convince the planner to actually not use an indexscan at all on the tsvector. Normally it'll choose an index scan on each and then a bitmap join, and we don't want to bring back so many rows... //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SET TRANSACTION not compliant with SQL:2003
On Tue, 2008-04-08 at 20:41 -0400, Tom Lane wrote: So I'm of the opinion that there's no good reason to change either our code or our docs. The standard-incompatibility is with BEGIN, not SET TRANSACTION, and it's already documented. That's a good case. Patch withdrawn. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Improve shutdown during online backup
On Tue, 2008-04-08 at 09:16 +0200, Albe Laurenz wrote: Heikki Linnakangas wrote: Albe Laurenz wrote: Moreover, if Shutdown == SmartShutdown, new connections won't be accepted, and nobody can connect and call pg_stop_backup(). So even if I'd add a check for (pmState == PM_WAIT_BACKENDS) !BackupInProgress() somewhere in the ServerLoop(), it wouldn't do much good, because the only way for somebody to cancel online backup mode would be to manually remove the file. Good point. So the only reasonable thing to do on smart shutdown during an online backup is to have the shutdown request fail, right? The only alternative being that a smart shutdown request should interrupt online backup mode. Or we can add another state, PM_WAIT_BACKUP, before PM_WAIT_BACKENDS, that allows new connections, and waits until the backup ends. That's an option. Maybe it is possible to restrict connections to superusers (who are the only ones who can call pg_stop_backup() anyway). Or, we could allow superuser connections in state PM_WAIT_BACKENDS... That sounds right. Completely unrelated to backups, if you issue a smart shutdown and it doesn't, you probably would like to connect and see what is happening and why. The reason may not be a backup-in-progress. Personally, I think smart shutdown could be even smarter. It should kick off unwanted sessions, such as an idle pgAdmin session - maybe a rule like anything that has been idle for 30 seconds. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving planner variable handling
I wonder if this would help to clean up the equivalence class hacks in Greg's ordered append patch? Tom Lane wrote: I've been thinking about how to improve the planner's poor handling of variables in outer-join situations. Here are some past examples for motivation: http://archives.postgresql.org/pgsql-hackers/2006-02/msg00154.php http://archives.postgresql.org/pgsql-general/2008-03/msg01440.php The reason why the planner acts so stupidly in these examples is that we're still using a kluge solution for this old bug: http://archives.postgresql.org/pgsql-bugs/2001-04/msg00223.php The root of the problem is that the planner does not worry about computing output expressions until the top of a plan tree. All lower-level join nodes are made to output simple lists of Vars referencing columns of the base relations of the join. We handle outer-join cases by forcing the values of the Vars of the nullable side to null at the level of the join, whenever there's no matching row in the nullable side. If one of the base relations of the join is a sub-SELECT whose output list includes expressions that don't certainly go to null when the input variables are forced to null, then we can't flatten that sub-SELECT, because flattening the sub-SELECT means that the expression evaluations bubble to the top of the plan tree and can produce non-null results when they shouldn't (as happened in the above bug, before we realized that we had to prevent flattening in this case). Another problem with this approach is that depending on what level of the plan tree you are thinking about, a Var nominally referencing tab.col might really mean the value of tab.col, or it might mean either tab.col or NULL depending on what happened at some lower level of outer join. Since we can't readily tell the difference, we have estimation errors arising from failure to expect some NULLs (there have been recent complaints about this), and we need some pretty ugly kluges in places like EquivalenceClass processing to handle the risk that apparently identical expressions might not really be equal. I think the basic solution for this is that upper levels of the plan tree should refer to the nullable output columns of an outer join using alias Vars that name the join rel, not the underlying base relation, even if there is a simple base-relation Var that the alias is tracking. In the case involving a sub-SELECT, the alias Var would stand for whatever output expression appears in the sub-SELECT. We already have the concept of these alias Vars, in fact --- that's exactly the representation emitted by the parser. But historically the planner has smashed aliases down to their base Vars as early as possible (see flatten_join_alias_vars). That has some advantages but I'm thinking it's outweighed by the disadvantages. I'd like to try leaving alias Vars as aliases all the way through the planner, in any case where they might be semantically different from their referent (ie, whenever there's a possible force-to-null involved). To make this work, we'd need to have the constructed plan tree compute the alias Var from its referent expression at the lowest outer-join that can null the alias Var. The trick for the executor is to know when to force the value to null instead of computing the expression. I first thought about marking entries of the join node's targetlist as to be forced to null if left or right input row is null. However, that fails if we want the join node to compute some projection expressions on top of the raw join output (as would certainly happen if it were the top node of the tree, for example). That could be handled by inserting another level of plan node (ie, a Result) to do the projection, but that seems a pretty ugly and inefficient solution. What I have in mind instead is to insert a new kind of expression node ForceToNull atop the referent expression, with this node able to look at the EState (in the same way a regular Var node would) to see if it should return a null instead of computing its child expression. Then expansion of an alias Var into a ForceToNull and the underlying expression would work. I'm envisioning keeping track of active alias Vars and their expansions in a new list attached to the PlannerInfo root node. This would provide a place to record important information like which level of the join tree such a Var needs to be evaluated at. This is all pretty handwavy yet, but I don't think I'll be able to fill in many more details until I try to code it. I thought I'd put up this summary to see if anyone can shoot holes in it at this level of detail ... Comments? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane [EMAIL PROTECTED] writes:
ISTM that there will be more cases like this in future, so we need a
general solution anyway. I propose the following sort of code structure
for these situations:
on_shmem_exit(cleanup_routine, arg);
PG_TRY();
{
... do something ...
cancel_shmem_exit(cleanup_routine, arg);
}
PG_CATCH();
{
cancel_shmem_exit(cleanup_routine, arg);
cleanup_routine(arg);
PG_RE_THROW();
}
PG_END_TRY();
[We would also have to block SIGTERM around the second cancel_shmem_exit and
cleanup_routine, no? Or if it's idempotent (actually, wouldn't it have to be?)
run them in the reverse order.]
This seems exceptionally fragile. Someone's bound to add another case without
realizing it. I thought about doing something like having a flag
in_unprotected_pg_try which PG_TRY would set to true, and on_shmem_exit would
clear. Then any LWLock or other shmem operation could assert it's cleared.
But that doesn't work for nested PG_TRY blocks. Possibly we could get it to
work by keeping a pair of counters but I'm not sure that's sufficient.
Are all the known cases LWLocks? Is there a reason we couldn't just have a
generic cleanup routine which just releases all LWLocks we hold before
exiting?
Or weakly -- an assert in CHECK_FOR_INTERRUPTS that there are no LWLocks held
or if there are that there's a cleanup routine in place. We wouldn't know for
sure that it's the *right* cleanup routine or enough cleanup routines but
hopefully we would catch the error often enough to notice.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pulling libpqtypes from queue
[EMAIL PROTECTED] wrote: I expect you intend to get at least the hooks in, right? [...] libpqtypes was designed to handle this with our without hooking. (the 'hooking' debate was mainly about exactly how libpq and libpqtypes was going to be separated). libpqtypes had a superclassing concept (not much discussed on the lists) where you could introduce new type handlers that wrapped existing ones and was desgined exactly for things like this. That sounds cool. So in a way you do have the hooks. A patch has been submited for supporting libpq object hooks, which allows one to associate private storage with a PGconn or PGresult object. The hook is called when a conn or result is created or destroyed (PQreset for conn as well). http://archives.postgresql.org/pgsql-patches/2008-04/msg00309.php For libpqtypes, this means it can operate outside of libpq. libpqtypes was initially designed as a direct extension to libpq (internal code), but the community prefered using a generic hook interface that allowed libpqtypes to work externally. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Gregory Stark [EMAIL PROTECTED] writes: Or weakly -- an assert in CHECK_FOR_INTERRUPTS oops, that's irrelevant of course. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgwin32_safestat weirdness
Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Shouldn't be too hard to do, but I keep thinking it'd be cleaner to just not do the redefine when building libpq. It means we'd add a define like BUILDING_LIBPQ or something to the libpq Makefile, and exclude the redefine if set. +1 for that general approach, but let's call the macro something like UNSAFE_STAT_OKAY. If the day ever comes that we need safestat inside libpq, or more likely that we want to exclude it from some other piece of code, it'll be clearer what to do. Hmm. I thought BUILDING_LIBPQ would be the more generic one, since we might want to control other stuff from it. I recall wanting that define at some point in the past, but I can't recall why... :-) But - I'll do it with UNSAFE_STAT_OK if that's what ppl want. And then a simple ifeq() section in libpq Makefile, right? Or we could have libpq define the BUILDING_LIBPQ, and have a header say #ifdef BUILDING_LIBPQ / #define UNSAFE_STAT_OK / #endif That would certainly be the most flexible, but maybe not the prettiest solution until such time as we actually need it. I think a simple approach is all we need for now - not even sure we need an ifeq() section in the makefile. Here's a patch, which I'll apply unless there's an objection. cheers andrew Index: src/include/port.h === RCS file: /cvsroot/pgsql/src/include/port.h,v retrieving revision 1.120 diff -c -r1.120 port.h *** src/include/port.h 11 Apr 2008 23:53:00 - 1.120 --- src/include/port.h 16 Apr 2008 12:16:01 - *** *** 287,294 * * We must pull in sys/stat.h here so the system header definition * goes in first, and we redefine that, and not the other way around. */ ! #if defined(WIN32) !defined(__CYGWIN__) #include sys/stat.h extern int pgwin32_safestat(const char *path, struct stat *buf); #define stat(a,b) pgwin32_safestat(a,b) --- 287,297 * * We must pull in sys/stat.h here so the system header definition * goes in first, and we redefine that, and not the other way around. + * + * Some frontends don't need the size from stat, so if UNSAFE_STAT_OK + * is defined we don't bother with this. */ ! #if defined(WIN32) !defined(__CYGWIN__) !defined(UNSAFE_STAT_OK) #include sys/stat.h extern int pgwin32_safestat(const char *path, struct stat *buf); #define stat(a,b) pgwin32_safestat(a,b) Index: src/interfaces/libpq/Makefile === RCS file: /cvsroot/pgsql/src/interfaces/libpq/Makefile,v retrieving revision 1.165 diff -c -r1.165 Makefile *** src/interfaces/libpq/Makefile 7 Apr 2008 14:15:58 - 1.165 --- src/interfaces/libpq/Makefile 16 Apr 2008 12:16:01 - *** *** 19,25 SO_MAJOR_VERSION= 5 SO_MINOR_VERSION= 2 ! override CPPFLAGS := -DFRONTEND -I$(srcdir) $(CPPFLAGS) -I$(top_builddir)/src/port ifneq ($(PORTNAME), win32) override CFLAGS += $(PTHREAD_CFLAGS) endif --- 19,25 SO_MAJOR_VERSION= 5 SO_MINOR_VERSION= 2 ! override CPPFLAGS := -DFRONTEND -DUNSAFE_STAT_OK -I$(srcdir) $(CPPFLAGS) -I$(top_builddir)/src/port ifneq ($(PORTNAME), win32) override CFLAGS += $(PTHREAD_CFLAGS) endif -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Collation at database level
Hi, I'm working on the bachelor thesis. The goal of the work will be to implement collation at database level based on POSIX locales and make foundations for further national language support development. User will be able to set collation when creating database or change collation of existing one. Particulary commands CREATE DATABASE... COLLATE … and ALTER DATABASE … COLLATE … regarding ANSI standard. Work will also implement possibility of creating users's own collation collection – commands CREATE COLLATION … FROM … USING and DROP COLLATION regaring ANSI standard. Additional features like ascending, descending ordering and key sensitivity will be included (these are not in ANSI standard). The initial part of my work has been completed and submitted as part of a patch contributed by Alexey Slynko (http://www.activebait.net/msg00019.html). I'm now in stage of adding collation catalogs, that will be important for further multi language support. The problem with POSIX locales is that you never know what locales user have got installed. I've discovered that some linux distros don't even have other than UTF-8 based locales. Because of ANSI defines collations deffined by ISO-8859-1 and UTF-* we need to somehow implement these collations. From my point of view, to create a catalog will be extremely slow so I'm thinking of writing two function for collation that will use both system locales as well as some hard-coded collations. Any sugestions? Radek Strnad -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Collation at database level
Radek Strnad [EMAIL PROTECTED] writes: The problem with POSIX locales is that you never know what locales user have got installed. I've discovered that some linux distros don't even have other than UTF-8 based locales. On Debian you're even deeper in it. The user can configure which locales he's actually interested in having on a machine. They're listed in /etc/locale.gen but I wouldn't suggest looking there. I think you have to try switching locales and see if setlocale returns NULL. Because of ANSI defines collations deffined by ISO-8859-1 and UTF-* we need to somehow implement these collations. These are encodings. What ANSI spec are you referring to, SQL? What does it actually say? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve shutdown during online backup
Simon Riggs [EMAIL PROTECTED] writes: Personally, I think smart shutdown could be even smarter. It should kick off unwanted sessions, such as an idle pgAdmin session - maybe a rule like anything that has been idle for 30 seconds. That's not a bad idea in itself but I don't think it's something the server should be in the business of doing. One big reason is that the server shouldn't be imposing arbitrary policy. That should be something the person running the shutdown is in control over. What you could do is have a separate program (I would write a client but a server-side function would work too) to kick off users based on various criteria you can specify. Then you can put in your backup scripts two commands, one to kick off idle users and then do a smart shutdown. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgwin32_safestat weirdness
Andrew Dunstan wrote: Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Shouldn't be too hard to do, but I keep thinking it'd be cleaner to just not do the redefine when building libpq. It means we'd add a define like BUILDING_LIBPQ or something to the libpq Makefile, and exclude the redefine if set. +1 for that general approach, but let's call the macro something like UNSAFE_STAT_OKAY. If the day ever comes that we need safestat inside libpq, or more likely that we want to exclude it from some other piece of code, it'll be clearer what to do. Hmm. I thought BUILDING_LIBPQ would be the more generic one, since we might want to control other stuff from it. I recall wanting that define at some point in the past, but I can't recall why... :-) But - I'll do it with UNSAFE_STAT_OK if that's what ppl want. And then a simple ifeq() section in libpq Makefile, right? Or we could have libpq define the BUILDING_LIBPQ, and have a header say #ifdef BUILDING_LIBPQ / #define UNSAFE_STAT_OK / #endif That would certainly be the most flexible, but maybe not the prettiest solution until such time as we actually need it. I think a simple approach is all we need for now - not even sure we need an ifeq() section in the makefile. Here's a patch, which I'll apply unless there's an objection. Seems a reasonable step for now, yeah - we can add BUILDING_LIBPQ sometime in the future if we need it. However, you patch needs to set the define in the MSVC build as well, to make sure that the produced libpq.dll is equivalent in functionality. /Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Magnus Hagander [EMAIL PROTECTED] writes: I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Yeah, I've had no luck with it either. Every so often there are discussions of going over to GNU indent instead. Presumably that would solve the portability problem. The last time we tried it (which was a long time ago) it seemed to have too many bugs and idiosyncrasies of its own, but it would be worth a fresh round of experimenting IMHO. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
On Wed, Apr 16, 2008 at 1:07 PM, Magnus Hagander [EMAIL PROTECTED] wrote: I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. Yeah, I recall trying it a while ago and not having happy memories. It should be possible (and indeed, mandatory) for patch submitters to run it over their code before submitting it - having core guys whose time is so valuable dealing with indentation etc seems an incredible waste. It won't fix everything, as Tom points out, but it's a better starting point. Cheers Tom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgwin32_safestat weirdness
Magnus Hagander wrote: Here's a patch, which I'll apply unless there's an objection. Seems a reasonable step for now, yeah - we can add BUILDING_LIBPQ sometime in the future if we need it. However, you patch needs to set the define in the MSVC build as well, to make sure that the produced libpq.dll is equivalent in functionality. Applied with this addition. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] count(*) performance improvement ideas
Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: Really? [ pokes around ... ] Hm, you're right, because add_placeholder_variable() sets the GUC_NO_SHOW_ALL flag, and in this usage it'll never be cleared. I wonder if we should change that. The whole thing is a bit of an abuse of what the mechanism was intended for, and so I'm not sure we should rejigger GUC's behavior to make it more pleasant, but on the other hand if we're not ready to provide a better substitute ... While I agree with that part, is there any actual *reason* why we shouldn't have the custom variables included in pg_settings? IIRC, the motivation for doing that was to not expose a completely bogus set of attributes for a variable whose defining C-module hadn't been loaded yet. I thought about this in the shower just now, and ISTM that if we want to turn this into an actual feature rather than a kluge, there needs to be some sort of define variable command that sets up a custom variable and specifies its type (and whatever other properties seem worth setting). IOW expose the DefineCustomFooVariable functions to SQL users. I'd be a bit inclined to restrict the namespace that can be set up that way, eg allow only local. or session. as the prefix. Maybe that's just being too anal, but we could guarantee not to introduce colliding built-in GUCs in future releases, whereas people trying to define variables with any random name would definitely be at risk. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DROP DATABASE vs patch to not remove files right away
Heikki Linnakangas [EMAIL PROTECTED] writes: Florian suggested a scheme where the xid and epoch is embedded in the filename, but that's unnecessarily complex. We could just make relfilenode a 64-bit integer. 2^64 should be enough for everyone. Doesn't fix the problem unless DB and TS OIDs become int64 too; in fact, given that we generate relfilenodes off the OID counter, it's difficult to see how you do this without making *all* OIDs 64-bit. Plus you're assuming that the machine has working 64-bit ints. There's a large difference in my mind between saying bigint doesn't work right if you don't have working int64 and we don't guarantee the safety of your data if you don't have int64. I'm not prepared to rip out the non-collision code until such time as we irrevocably refuse to build on machines without int64. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: ISTM that there will be more cases like this in future, so we need a general solution anyway. I propose the following sort of code structure for these situations: [We would also have to block SIGTERM around the second cancel_shmem_exit and cleanup_routine, no? Or if it's idempotent (actually, wouldn't it have to be?) run them in the reverse order.] No, we wouldn't, because a SIGTERM can only actually fire at a CHECK_FOR_INTERRUPTS() call. You'd just need to be sure there wasn't one in the cleanup code. Are all the known cases LWLocks? *None* of the known cases are LWLocks, nor any other resource that we have generic cleanup code for. The problem cases are one-off resources that it seemed we could avoid having a real cleanup mechanism for. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] count(*) performance improvement ideas
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: Really? [ pokes around ... ] Hm, you're right, because add_placeholder_variable() sets the GUC_NO_SHOW_ALL flag, and in this usage it'll never be cleared. I wonder if we should change that. The whole thing is a bit of an abuse of what the mechanism was intended for, and so I'm not sure we should rejigger GUC's behavior to make it more pleasant, but on the other hand if we're not ready to provide a better substitute ... While I agree with that part, is there any actual *reason* why we shouldn't have the custom variables included in pg_settings? I've needed it myself before -- I think it is a good idea. IIRC, the motivation for doing that was to not expose a completely bogus set of attributes for a variable whose defining C-module hadn't been loaded yet. I thought about this in the shower just now, and ISTM that if we want to turn this into an actual feature rather than a kluge, there needs to be some sort of define variable command that sets up a custom variable and specifies its type (and whatever other properties seem worth setting). IOW expose the DefineCustomFooVariable functions to SQL users. I'd be a bit inclined to restrict the namespace that can be set up that way, eg allow only local. or session. as the prefix. Maybe that's just being too anal, but we could guarantee not to introduce colliding built-in GUCs in future releases, whereas people trying to define variables with any random name would definitely be at risk. Comments? Would it make sense to have built-in GUCs belong to pg_catalog. and user defined GUCs default to public.? Joe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane [EMAIL PROTECTED] writes: [We would also have to block SIGTERM around the second cancel_shmem_exit and cleanup_routine, no? Or if it's idempotent (actually, wouldn't it have to be?) run them in the reverse order.] No, we wouldn't, because a SIGTERM can only actually fire at a CHECK_FOR_INTERRUPTS() call. You'd just need to be sure there wasn't one in the cleanup code. Wait, huh? In that case I don't see what advantage any of this has over Bruce's patch. And his approach seemed a lot more robust. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgwin32_safestat weirdness
Magnus Hagander [EMAIL PROTECTED] writes: Andrew Dunstan wrote: I think a simple approach is all we need for now - not even sure we need an ifeq() section in the makefile. Here's a patch, which I'll apply unless there's an objection. Seems a reasonable step for now, yeah - we can add BUILDING_LIBPQ sometime in the future if we need it. +1 from here too, modulo this: However, you patch needs to set the define in the MSVC build as well, to make sure that the produced libpq.dll is equivalent in functionality. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DROP DATABASE vs patch to not remove files right away
Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: Florian suggested a scheme where the xid and epoch is embedded in the filename, but that's unnecessarily complex. We could just make relfilenode a 64-bit integer. 2^64 should be enough for everyone. Doesn't fix the problem unless DB and TS OIDs become int64 too; Remember what the original scenario was: 1. DROP TABLE foo; 2. BEGIN; CREATE TABLE bar; COPY TO bar ...; COMMIT -- bar gets the same relfilenode as foo, by chance 3. crash 4. WAL replay of DROP TABLE foo deletes the data inserted at step 2. Because the table was created in the same transaction, we skipped WAL logging the inserts, and the data is lost. Even if we can get a collision in DB or tablespace OIDs, we can't get a collision at step 2 if we don't reuse relfilenodes. in fact, given that we generate relfilenodes off the OID counter, it's difficult to see how you do this without making *all* OIDs 64-bit. By generating them off a new 64-bit counter instead of the OID counter. Or by extending the OID counter to 64-bits, but only using the lower 64 bits for OIDs. Plus you're assuming that the machine has working 64-bit ints. There's a large difference in my mind between saying bigint doesn't work right if you don't have working int64 and we don't guarantee the safety of your data if you don't have int64. I'm not prepared to rip out the non-collision code until such time as we irrevocably refuse to build on machines without int64. Hmm. We could make it a struct of two int4s if we have to, couldn't we? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving planner variable handling
Heikki Linnakangas [EMAIL PROTECTED] writes: I wonder if this would help to clean up the equivalence class hacks in Greg's ordered append patch? Pretty hard to say at this early stage, but the more I think about it the more I like the idea of never using the same Var to represent two values that aren't guaranteed equal. The maybe-its-null business has been a thorn in the side of any sort of close semantic analysis for a long time, but somehow I never perceived what a bogus idea that was. I'm almost more excited about that than about fixing the original problem ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane [EMAIL PROTECTED] writes: Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: No, we wouldn't, because a SIGTERM can only actually fire at a CHECK_FOR_INTERRUPTS() call. You'd just need to be sure there wasn't one in the cleanup code. Wait, huh? In that case I don't see what advantage any of this has over Bruce's patch. And his approach seemed a lot more robust. Maybe I missed something, but I thought he was just proposing some macro syntactic sugar over the same code that I described. No, I meant the earlier patch which you rejected with the flag in MyProc. I realize there were other issues but the initial concern was that it wouldn't respond promptly because it would wait for CHECK_FOR_INTERRUPTS. But if sigterm was never handled except at a CHECK_FOR_INTERRUPTS then that was never a factor. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Gregory Stark [EMAIL PROTECTED] writes:
Tom Lane [EMAIL PROTECTED] writes:
No, we wouldn't, because a SIGTERM can only actually fire at a
CHECK_FOR_INTERRUPTS() call. You'd just need to be sure there wasn't
one in the cleanup code.
Wait, huh? In that case I don't see what advantage any of this has over
Bruce's patch. And his approach seemed a lot more robust.
Maybe I missed something, but I thought he was just proposing some
macro syntactic sugar over the same code that I described.
I'm not against syntactic sugar, but it doesn't seem to be totally
clean; I think you would need two repetitions of the function and
arg anyway:
PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg);
{
... risky code here ...
}
PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg);
because both the before and after parts need to know the function
name and the arg to pass it. The only way to avoid duplication would be
if the whole controlled code block was actually an argument to the
macro:
PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg,
... risky code here ...
);
but there are very good reasons not to do that when the controlled
code can be large; for instance that #if can't be used portably
in a macro argument.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCHES] [HACKERS] Text - C string
Brendan Jurd [EMAIL PROTECTED] writes: If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? I dunno. I know there was previously some handwaving about representing XML values in some more intelligent fashion than a plain text string, but I have no idea if anyone is likely to do anything about it in the foreseeable future. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCHES] [HACKERS] Text - C string
Tom Lane wrote: Brendan Jurd [EMAIL PROTECTED] writes: If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? I dunno. I know there was previously some handwaving about representing XML values in some more intelligent fashion than a plain text string, but I have no idea if anyone is likely to do anything about it in the foreseeable future. It seems very unlikely to me. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Gregory Stark [EMAIL PROTECTED] writes: No, I meant the earlier patch which you rejected with the flag in MyProc. I realize there were other issues but the initial concern was that it wouldn't respond promptly because it would wait for CHECK_FOR_INTERRUPTS. No, that's not the concern in the least. The concern is that something could trap the attempted throwing of the error *after* CHECK_FOR_INTERRUPTS has noticed the signal. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improving planner variable handling
Tom Lane [EMAIL PROTECTED] writes: Heikki Linnakangas [EMAIL PROTECTED] writes: I wonder if this would help to clean up the equivalence class hacks in Greg's ordered append patch? Pretty hard to say at this early stage, I've started to have some doubts myself about stuffing all these vars into the same equivalence class. My concern here is actually performance. I'm concerned that if you use a partitioned table within a larger query once we've generated the paths for the partitioned table we don't want to then carry that baggage of hundreds or possibly thousands of variables for planning the whole rest of the query above that rel. They'll never be relevant again after that. The more these ideas start fitting together in my head the more I think Tom's original instinct would perform better. I was concerned that pulling up and pushing down pathkey lists sounded like a lot of extra work. But at least you would only have to pay that for that one level, not carry it along and pay for it for the rest of the planning. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] /etc/init.d/postgresql status error
I have just installed the rpm found at:
http://www.postgresql.org/ftp/binary/v8.2.7/linux/rpms/redhat/rhel-4-i386/
and the status option generates an error:
$ /etc/init.d/postgresql status
pidof: invalid options on command line!
pidof: invalid options on command line!
-p is stopped
I was able to fix it in the following mode:
status)
# status -p /var/run/postmaster.${PGPORT}.pid
status postmaster
script_result=$?
;;
Commented the original line and replaced with the one just below.
Regards
Gaetano Mendola
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I am starting to think we need to analyze the source code rather than testing, because of what we are testing for. I was thinking about that a bit more, in connection with trying to verbalize why I don't like your patch ;-) The fundamental assumption here is that we think that proc_exit() from the main loop of PostgresMain() should be safe, because that's exercised anytime a client quits or dies unexpectedly, and so it should be a well-tested path. What is lacking such test coverage is the many code paths that start proc_exit() from any random CHECK_FOR_INTERRUPTS() point in the backend. Some of those points might be (in fact are known to be, as of today) holding locks or otherwise in a state that prevents a clean proc_exit(). Your patch proposes to replace that code path by one that fakes a QueryCancel error and then does proc_exit() once control reaches the outer level. While that certainly *should* work (ignoring the question of whether control gets to the outer level in any reasonable amount of time), it's a bit of a stretch to claim that it's a well-tested case. At the moment it would only arise if a QueryCancel interrupt arrived at approximately the same time that the client died or sent a disconnect message, which is surely far less probable than client death by itself. So I don't buy the argument that this is a better-tested path, and I definitely don't want to introduce new complexity in order to make it happen like that. I would like my use of SIGINT to be like someone pressing ^C and then \q in psql, but I can see that the SIGINT might be delivered in places where libpq would not deliver it, like during shutdown. Hopefully that could be addressed, but I agree using SIGTERM is better and more useful. Now, as to whether a SIGTERM-style exit is safe. With the exception of the PG_CATCH issues that we already know about, any other case seems like a clear coding error: you'd have to be doing something that you know could throw an error, without having made provision to clean up whatever unclean state you are in. It'd be nice to think that we haven't been that silly anywhere. Experience however teaches that such an assumption is hubris. I think that the way forward is to fix the PG_CATCH issues (which I will try to get done later this week) and then get someone to mount a serious effort to break it, as outlined in previous discussions: http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php I was thinking of running two parallel regression tests and killing one at random and see if the others complete. One problem is that the regression tests issue a lot of server error messages so somehow I would need to filter out the normal errors, which I think I could do. Magnus has offered to test too. I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4 if there is some reasonable amount of testing done during this development cycle to try to expose any problems. If no one is willing to do any such testing, then as far as I'm concerned there is no market for the feature and we should drop it from TODO. Agreed, but the feature is going to be asked for again soon so we would probably have to put it on the features we don't want, and that is going to look pretty odd. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Brendan Jurd wrote: The idea that we fix stylistic issues on the fly is not sustainable. We should offer help and mentorship to new patch submitters in all areas (including stylistic) but they should do the work. It is the only way we will mold them to submit patches in the proper way. I agree. As a submitter I would much rather get an email saying e.g. Hey, your patch is nice but the code style sticks out like a sore thumb. Please adopt surrounding naming convention and fix your indentation per the rules at [link]. than have it fixed silently on its way to being committed. With the former I learn something and get to improve my own work. With the latter, my next patch is probably going to have the exact same problem, which is in the long term just making extra work for the reviewers. If the patch submitters hasn't read the developers' FAQ, we assume they are not interested in improving the style of their patch. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Magnus Hagander wrote: And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. Agreed, pgindent is too cumbersome to require patch submitters to use. One idea would be to allow C files to be emailed and the indented version automatically returned via email. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4 if there is some reasonable amount of testing done during this development cycle to try to expose any problems. If someone can come up with an automated script to do this kind of testing, I can commit a VM or three to running this 24/7 for a month, easily... But I don't trust myself in coming up with a test-case that's good enough :-P The closest thing I can think of to an automated test is to run repeated sets of the parallel regression tests, and each time SIGTERM a randomly chosen backend at a randomly chosen time. Then see if anything funny happens. The hard part here is distinguishing expected from unexpected regression outputs, especially in view of the fact that some of the tests depend on database contents set up by earlier tests. I'm thinking that you could automatically discard the regression diff for the specific test that got SIGTERM'd, as long as it looked like the normal output up to the point where the terminated by administrator error appears. Then what you'd have is the potential for downstream failures due to things not being created, which *should* fall into a fairly stylized set of possible diffs. So get the script to throw away any diffs that exactly match ones seen previously. Run it for awhile, and then hand-validate the set of diffs that it's saved ... or if any of 'em look funny, report. One gotcha I can think of is that killing the prepared_xacts test can leave you with open 2PC transactions, which will interfere with starting the next cycle of the tests (you have to kill them before you can dropdb). But you could add a rollback prepared to the driver script to clean out any uncommitted prepared xact. Whether this is workable or not depends on the size of the set of expected downstream-failure diffs. My gut feeling from many years of watching regression test crashes is that it'd be large but not completely impractical to look through by hand. I haven't time to write something like that myself, but offhand it seems like it could be done without more than a day or so's work, especially if you start from the buildfarm infrastructure. BTW, don't forget to include autovac workers in the set of SIGTERM target candidates. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
[EMAIL PROTECTED] (Tom Lane) writes: Magnus Hagander [EMAIL PROTECTED] writes: I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Yeah, I've had no luck with it either. Every so often there are discussions of going over to GNU indent instead. Presumably that would solve the portability problem. The last time we tried it (which was a long time ago) it seemed to have too many bugs and idiosyncrasies of its own, but it would be worth a fresh round of experimenting IMHO. Well, GNU indent is now on version 2.2.9, and has evidently addressed *some* problems with it. Unfortunately, the pgindent README does not actually specify what any of the actual problems with GNU indent are, thus making it pretty much impossible to evaluate whether or not any of the subsequent releases might have addressed any of those problems. I doubt that the pgindent issues have been addressed. -- output = reverse(ofni.sesabatadxunil @ enworbbc) http://linuxfinances.info/info/sgml.html In elementary school, in case of fire you have to line up quietly in a single file line from smallest to tallest. What is the logic? Do tall people burn slower? -- Warren Hutcherson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Tom Lane wrote: I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4 if there is some reasonable amount of testing done during this development cycle to try to expose any problems. If someone can come up with an automated script to do this kind of testing, I can commit a VM or three to running this 24/7 for a month, easily... But I don't trust myself in coming up with a test-case that's good enough :-P The closest thing I can think of to an automated test is to run repeated sets of the parallel regression tests, and each time SIGTERM a randomly chosen backend at a randomly chosen time. Then see if anything funny Yep, that was my plan, plus running the parallel regression tests you get the possibility of 2 backends. happens. The hard part here is distinguishing expected from unexpected regression outputs, especially in view of the fact that some of the tests depend on database contents set up by earlier tests. Oh, that is a problem. I was going to get the typical errors, sort them and put them in a file. Then when I run the test, I sort the log again and use diff | grep '' to see an differences. If there are cases that generate new errors on a previous failure, I will have to add that output too. I'm thinking that you could automatically discard the regression diff for the specific test that got SIGTERM'd, as long as it looked like the normal output up to the point where the terminated by administrator error appears. Then what you'd have is the potential for downstream failures due to things not being created, which *should* fall into a fairly stylized set of possible diffs. So get the script to throw away any diffs that exactly match ones seen previously. Run it for awhile, and then hand-validate the set of diffs that it's saved ... or if any of 'em look funny, report. Yep, see above. One gotcha I can think of is that killing the prepared_xacts test can leave you with open 2PC transactions, which will interfere with starting the next cycle of the tests (you have to kill them before you can dropdb). But you could add a rollback prepared to the driver script to clean out any uncommitted prepared xact. Good idea. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: The closest thing I can think of to an automated test is to run repeated sets of the parallel regression tests, and each time SIGTERM a randomly chosen backend at a randomly chosen time. Then see if anything funny Yep, that was my plan, plus running the parallel regression tests you get the possibility of 2 backends. I was intentionally suggesting only one kill per test cycle. Multiple kills will probably create an O(N^2) explosion in the set of possible downstream-failure deltas. I doubt you'd really get any improvement in testing coverage to justify the much larger amount of hand validation needed. It also strikes me that you could make some simple alterations to the regression tests to reduce the set of observable downstream deltas. For example, anyplace where a test loads a table with successive INSERTs and that table is used by later tests, wrap the INSERT sequence with BEGIN/END. Then there is only one possible downstream delta (empty table) and not N different possibilities for an N-row table. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Chris Browne [EMAIL PROTECTED] writes: [EMAIL PROTECTED] (Tom Lane) writes: Every so often there are discussions of going over to GNU indent instead. Presumably that would solve the portability problem. The last time we tried it (which was a long time ago) it seemed to have too many bugs and idiosyncrasies of its own, but it would be worth a fresh round of experimenting IMHO. Well, GNU indent is now on version 2.2.9, and has evidently addressed *some* problems with it. Unfortunately, the pgindent README does not actually specify what any of the actual problems with GNU indent are, thus making it pretty much impossible to evaluate whether or not any of the subsequent releases might have addressed any of those problems. This wouldn't be hard for someone to investigate. Check out our CVS from immediately after the last pgindent run. Run GNU indent on it. See what options result in the smallest diff, and what the diff looks like in terms of making the code look nicer or less nice. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
On Wed, 16 Apr 2008 12:36:50 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: If the patch submitters hasn't read the developers' FAQ, we assume they are not interested in improving the style of their patch. I think that point is fairly flawed in consideration. I know for a fact that I have had to repeat even to long time contributors source references for information even though they new about it previously. Yourself included. People get in a hurry, they should be reminded with reference. JD, thanks for the patch but you missed foo.. please check 1.5 of the Developers FAQ. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_terminate_backend() issues
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: The closest thing I can think of to an automated test is to run repeated sets of the parallel regression tests, and each time SIGTERM a randomly chosen backend at a randomly chosen time. Then see if anything funny Yep, that was my plan, plus running the parallel regression tests you get the possibility of 2 backends. I was intentionally suggesting only one kill per test cycle. Multiple kills will probably create an O(N^2) explosion in the set of possible downstream-failure deltas. I doubt you'd really get any improvement in testing coverage to justify the much larger amount of hand validation needed. No, the point is that you have 2 backends to choose from to kill. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
[EMAIL PROTECTED] (Bruce Momjian) writes: Magnus Hagander wrote: And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. Agreed, pgindent is too cumbersome to require patch submitters to use. One idea would be to allow C files to be emailed and the indented version automatically returned via email. Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent - Build it _in place_ inside the code tree (e.g. - don't assume it will get installed in /usr/local/bin) - Thus have the ability to run it in place? -- let name=cbbrowne and tld=linuxfinances.info in name ^ @ ^ tld;; http://cbbrowne.com/info/lisp.html It worked about as well as sticking a blender in the middle of a lime plantation and hoping they'll make margaritas out of themselves. -- Frederick J. Polsky v1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] Avahi support for Postgresql
Added to TODO: * Implement the non-threaded Avahi service discovery protocol http://archives.postgresql.org/pgsql-hackers/2008-02/msg00939.php http://archives.postgresql.org/pgsql-patches/2008-02/msg00097.php http://archives.postgresql.org/pgsql-hackers/2008-03/msg01211.php http://archives.postgresql.org/pgsql-patches/2008-04/msg1.php --- Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Mathias Hasselmann wrote: The patches were in my initial mail, but now I've also uploaded them to my personal site for convenience: http://taschenorakel.de/files/pgsql-avahi-support/ Hmm, a quick look at the third patch reveals that it is using the threaded Avahi client. That's a showstopper. Please consider using some other approach -- writing our own handlers for AvahiPoll would seem apropos. Based on the threading usage in this patch, I assume it is rejected, and that we don't want a TODO item for this. This particular patch should be rejected, but we certainly do need a TODO item IMHO. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Chris Browne wrote: [EMAIL PROTECTED] (Bruce Momjian) writes: Magnus Hagander wrote: And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. Agreed, pgindent is too cumbersome to require patch submitters to use. One idea would be to allow C files to be emailed and the indented version automatically returned via email. Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent - Build it _in place_ inside the code tree (e.g. - don't assume it will get installed in /usr/local/bin) - Thus have the ability to run it in place? Yes, but it bloats our code and people still need to generate the typedefs and follow the instructions. The other problem is if they run it on a file they have modified, it is going to adjust places they didn't touch, thereby making the patch harder to review. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Chris Browne wrote: [EMAIL PROTECTED] (Tom Lane) writes: Magnus Hagander [EMAIL PROTECTED] writes: I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Yeah, I've had no luck with it either. Every so often there are discussions of going over to GNU indent instead. Presumably that would solve the portability problem. The last time we tried it (which was a long time ago) it seemed to have too many bugs and idiosyncrasies of its own, but it would be worth a fresh round of experimenting IMHO. Well, GNU indent is now on version 2.2.9, and has evidently addressed *some* problems with it. Unfortunately, the pgindent README does not actually specify what any of the actual problems with GNU indent are, thus making it pretty much impossible to evaluate whether or not any of the subsequent releases might have addressed any of those problems. I doubt that the pgindent issues have been addressed. What I did was to run pgindent on the source tree, make a copy, then run GNU indent on it and diff -r. I can try that test again in the next few months. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Tue, 11 Mar 2008 11:07:27 -0700 Joshua D. Drake [EMAIL PROTECTED] wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Tue, 11 Mar 2008 10:46:23 -0700 Joshua D. Drake [EMAIL PROTECTED] wrote: And the -c version :) (thanks bruce) Joshua D. Drake What is the feedback on this patch? Is there anything I need to do to get it into the next commit fest? Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Joshua D. Drake wrote: What is the feedback on this patch? Is there anything I need to do to get it into the next commit fest? Please add it to the commitfest wiki page. http://wiki.postgresql.org/wiki/CommitFest:May -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Wed, 16 Apr 2008 13:36:50 -0400 Alvaro Herrera [EMAIL PROTECTED] wrote: Joshua D. Drake wrote: What is the feedback on this patch? Is there anything I need to do to get it into the next commit fest? Please add it to the commitfest wiki page. http://wiki.postgresql.org/wiki/CommitFest:May Done: http://wiki.postgresql.org/wiki/CommitFest:May#Pending_patches Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] How to submit a patch
Hello, Could someone break out exactly what the process is now for submitting a patch? Last month I sent a patch for pg_dump which never got feedback (at least on thread). I just asked and alvaro asked me to add it to the commitfest page. Which I have done but I think we need to known all the steps and get it documented. I have: post to pgsql-hackers with idea, take feedback, code to consensus post to pgsql-patches, await feedback ... ? Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Bruce Momjian wrote: Chris Browne wrote: Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent - Build it _in place_ inside the code tree (e.g. - don't assume it will get installed in /usr/local/bin) - Thus have the ability to run it in place? Yes, but it bloats our code and people still need to generate the typedefs and follow the instructions. I suggested to you awhile back that we could keep a typedef file on the repository, saving one step. I don't see the problem with keeping the BSD indent for now, until we figure out the set of options GNU indent needs to behave properly. It's not all that much code, after all. The other problem is if they run it on a file they have modified, it is going to adjust places they didn't touch, thereby making the patch harder to review. That should be rare if pgindent is run frequently, I think. (And anyway, there are tools to remove unwanted hunks from patches if the author so desires.) I guess the point here is that pgindent is a tool that should _help_ developers. Making it harder to run is not helping anyone. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Joshua D. Drake wrote: Hello, Could someone break out exactly what the process is now for submitting a patch? Last month I sent a patch for pg_dump which never got feedback (at least on thread). I just asked and alvaro asked me to add it to the commitfest page. Which I have done but I think we need to known all the steps and get it documented. I have: post to pgsql-hackers with idea, take feedback, code to consensus post to pgsql-patches, await feedback ... ? If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. As for the rest, we are still feeling our way a bit, as should have been apparent from the list emails, so formal documentation is probably premature. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) As for the rest, we are still feeling our way a bit, as should have been apparent from the list emails, so formal documentation is probably premature. I assumed the docs would be subject to change but something that gives one off and not often patch submitters a clue is probably useful. It also allows us to actually discuss a pattern of behavior we are starting versus bouncing through 50 threads trying to figure out what's next. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Joshua D. Drake wrote: What is the feedback on this patch? Is there anything I need to do to get it into the next commit fest? Yes, go add it to the wiki page ;-): http://wiki.postgresql.org/wiki/CommitFest:May I agree that we should do that, but the thread on -hackers (Autovacuum vs statement_timeout) wasn't totally conclusive. Greg Sabine Mullane and Peter Eisentraut argued that we shouldn't, but neither provided a plausible use case where a statement_timeout on restoring a dump would be useful. I'm thinking we should apply the patch unless someone comes up with one. To quote Tom: I think we need to be careful to distinguish three situations: * statement_timeout during pg_dump * statement_timeout during pg_restore * statement_timeout during psql reading a pg_dump script file This patch addresses the third situation, but leaves open the 1st and the 2nd. IMO, we should set statement_timeout = 0 in them as well, unless someone comes up with plausible use case for using a non-zero statement_timeout. Ps. If you want to save the committer a couple of minutes of valuable time, you can fix the indentation to use tabs instead of spaces, and remove the spurious whitespace change on the empty line. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Wed, 16 Apr 2008 21:04:17 +0300 Heikki Linnakangas [EMAIL PROTECTED] wrote: To quote Tom: I think we need to be careful to distinguish three situations: * statement_timeout during pg_dump * statement_timeout during pg_restore * statement_timeout during psql reading a pg_dump script file This patch addresses the third situation, but leaves open the 1st and the 2nd. IMO, we should set statement_timeout = 0 in them as well, unless someone comes up with plausible use case for using a non-zero statement_timeout. My patch addresses all three, unless I am misunderstanding your meaning. The patch does the following: After connection with pg_dump it executes set statement_timeout = 0; This fixed the pg_dump timeout issue. It also writes set statement_timeout = 0 into the archive file, which fixed pg_restore and psql. Ps. If you want to save the committer a couple of minutes of valuable time, you can fix the indentation to use tabs instead of spaces, and remove the spurious whitespace change on the empty line. I can do that. Thanks for the feedback. Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Joshua D. Drake wrote: On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Wed, 16 Apr 2008 14:10:30 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: Joshua D. Drake wrote: On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Bruce Momjian wrote: Joshua D. Drake wrote: On Wed, 16 Apr 2008 14:10:30 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: Joshua D. Drake wrote: On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Apply to CVS. Bruce, I know you don't mean this, but it reads like you are undertaking to review and apply all patches. BTW, I don't see why the wiki can't pick up patches that were submitted before it started. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
Joshua D. Drake wrote: On Wed, 16 Apr 2008 21:04:17 +0300 Heikki Linnakangas [EMAIL PROTECTED] wrote: To quote Tom: I think we need to be careful to distinguish three situations: * statement_timeout during pg_dump * statement_timeout during pg_restore * statement_timeout during psql reading a pg_dump script file This patch addresses the third situation, but leaves open the 1st and the 2nd. IMO, we should set statement_timeout = 0 in them as well, unless someone comes up with plausible use case for using a non-zero statement_timeout. My patch addresses all three, unless I am misunderstanding your meaning. The patch does the following: After connection with pg_dump it executes set statement_timeout = 0; This fixed the pg_dump timeout issue. It also writes set statement_timeout = 0 into the archive file, which fixed pg_restore and psql. Oh, ok, I misread the patch. Sorry for the noise. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Wed, 16 Apr 2008 14:13:53 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Apply to CVS. O.k. so patches may be applied through the development cycle but no patches will be accepted after -X- date for a current commit fest. So my question is: If you are going to review the patch and apply or reply to the author, at one point is it supposed to be on the wiki? Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout
On Wed, 16 Apr 2008 21:20:17 +0300 Heikki Linnakangas [EMAIL PROTECTED] wrote: My patch addresses all three, unless I am misunderstanding your meaning. The patch does the following: After connection with pg_dump it executes set statement_timeout = 0; This fixed the pg_dump timeout issue. It also writes set statement_timeout = 0 into the archive file, which fixed pg_restore and psql. Oh, ok, I misread the patch. Sorry for the noise. :) Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Andrew Dunstan wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Apply to CVS. Bruce, I know you don't mean this, but it reads like you are undertaking to review and apply all patches. Didn't you read review it and apply or reply to the author. BTW, I don't see why the wiki can't pick up patches that were submitted before it started. True. Frankly, I am getting pretty tired of people complaining about what I am doing. Perhaps I should just stop and let everyone else deal with things. I have lots of things I would rather be doing. This is one of the reasons I didn't want to add wiki maintenance to my already full workload. Instead I am having to field complaints. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Joshua D. Drake wrote: On Wed, 16 Apr 2008 14:10:30 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: Joshua D. Drake wrote: On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Apply to CVS. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Joshua D. Drake wrote: On Wed, 16 Apr 2008 14:13:53 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: I review it and apply or reply to the author. The wiki had started being updated after your submission so this is a transitionary phase. Wait... apply where? The wiki? or to the tree? Apply to CVS. O.k. so patches may be applied through the development cycle but no patches will be accepted after -X- date for a current commit fest. So my question is: If you are going to review the patch and apply or reply to the author, at one point is it supposed to be on the wiki? I have no idea. If not dealt with, it will be on my web page once the next commit fest starts. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Bruce Momjian wrote: Frankly, I am getting pretty tired of people complaining about what I am doing. Perhaps I should just stop and let everyone else deal with things. I have lots of things I would rather be doing. This is one of the reasons I didn't want to add wiki maintenance to my already full workload. Instead I am having to field complaints. I didn't mean to complain about anything. Personally, I'm in favor of reducing your workload. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Race conditions in relcache load (again)
I wrote: I realized that we failed to plug all the gaps of this type, because relcache.c contains *internal* cache load/reload operations that aren't protected. In particular the LOAD_CRIT_INDEX macro calls invoke relcache load on indexes that aren't locked. So they'd be at risk from a concurrent REINDEX or similar on those system indexes. RelationReloadIndexInfo seems at risk as well. On closer analysis, LOAD_CRIT_INDEX is clearly broken here, but the other places I was worried about seem safe, because they are all used to rebuild relcache entries that are being held open by higher-level code; and the contract is that there should be a lock protecting any open relcache entry. I've committed a fix that adds locking to LOAD_CRIT_INDEX and adds some comments about the other cases. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Joshua D. Drake wrote: On Wed, 16 Apr 2008 13:59:50 -0400 Andrew Dunstan [EMAIL PROTECTED] wrote: If you posted it last month then it was too late for the commit-fest that started on March 1, IIRC, so the fact that you didn't get feedback is hardly surprising - a commit-fest is like a mini-feature-freeze. O.k. then what happens at that point? It wasn't in the queue for May (I had to add it) There is different viewpoints on how it should happen. Hopefully the picture will be clearer after one or two more commit fests. Based on my observations, there's basically three different workflows a patch can follow (assuming the patch gets committed in the end): Workflow A: 1. You post patch to pgsql-patches 2. a committer picks it up immediately, and commits it. Workflow B: 1. You post a patch to pgsql-patches 2. You add a link to the wiki page of the next commit fest 3. A committer picks up the patch from the wiki page, and commits it Workflow C: 1. You post a patch to pgsql-patches 2. Bruce adds the patch to the unapplied patches queue after a while 3. At the beginning of the next commit fest, Alvaro (with the help from others, I hope) goes through the patches queue, and puts a link to the wiki page of the next commit fest 4. A committer picks up the patch from the wiki page, and commits it -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Wed, 16 Apr 2008 14:24:34 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: If you are going to review the patch and apply or reply to the author, at one point is it supposed to be on the wiki? I have no idea. If not dealt with, it will be on my web page once the next commit fest starts. This is the exact reason I posted the question :). Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Andrew Dunstan wrote: BTW, I don't see why the wiki can't pick up patches that were submitted before it started. Of course it can. This one wasn't added initially because I didn't see it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] MERGE SQL Statement
I've analysed various flavours of MERGE command to understand and
propose what we should use for PostgreSQL.
The results aren't what you'd expect from a quick flick through the
standard, so lets look at my main concerns:
1. The simplest syntax is for SQL:2003. The syntax for DB2, SQL Server
and Oracle is more complex, with SQL:2008(final draft) being very
similar to DB2 and SQL Server, so unlikely to be a point of contention
in the standard. I suggest we go with the latter, but yes, its still in
draft (yawn).
2. MySQL and Teradata have their own syntax for the row-oriented Upsert
operation. Both of those are more useful (IMHO) than MERGE for OLTP
apps, while MERGE is very useful for bulk data loads. I'm open to the
idea that we do something like this in addition to MERGE.
3. The way MERGE works is to define a left outer join between source and
target, then specify a series of WHEN clauses that may or may not apply.
It **isn't** just a simple Update/Insert and so much of what we have
discussed previously goes straight in the trash. AFAICS the way it is
specified to work it would be fairly straightforward to cause race
conditions and failures when using multiple concurrent MERGE statements.
General Example of the recommended syntax for PostgreSQL
MERGE INTO Stock S /* target */
USING DailySales DS /* source table */
ON S.Item = DS.Item /* left outer join source to target */
WHEN MATCHED AND (QtyOnHand - QtySold = 0) THEN
/* delete item if no stock remaining */
DELETE
WHEN MATCHED THEN /* No AND clause, so drop thru */
/* update value if some remains */
UPDATE SET QtyOnHand = QtyOnHand - QtySold
WHEN NOT MATCHED THEN
/* insert a row if the stock is new */
INSERT VALUES (Item, QtySold)
;
So lets look at the syntaxes and then review how it might work.
SYNTAX
==
SQL:2003
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON search-condition
[WHEN MATCHED THEN MergeUpdate]
[WHEN NOT MATCHED THEN MergeInsert]
Oracle 11g
--
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON search-condition
[WHEN MATCHED THEN MergeUpdate
WHERE where-clause DELETE WHERE where-clause]
[WHEN NOT MATCHED THEN MergeInsert
WHERE where-clause]
Differences from SQL:2003 are
* Update and Insert have WHERE clauses on them
* Oracle allows multiple WHEN ... WHERE clauses
* Oracle allows an error logging clause also
* optional DELETE statement as part of the UPDATE, so you can only
DELETE what you update (yeh, really)
* WHEN MATCHED/WHEN NOT MATCHED must be in fixed order, only
IBM DB2
---
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON search-condition
[WHEN MATCHED [AND where-clause] THEN MergeUpdate | MergeDelete]
[WHEN NOT MATCHED [AND where-clause] THEN MergeInsert | SignalClause]
[ELSE IGNORE]
Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* DB2 allows multiple WHEN ... AND clauses
* DELETE is also a full-strength option, not part of the MergeUpdate
clause as it is in Oracle
* DB2 allows a SIGNAL statement, similar to RAISE
* ELSE IGNORE is an optional syntax, which does nothing
SQL Server 2008
---
MERGE [INTO] target [AS correlation-name]
USING [table-ref | subquery]
ON search-condition
[WHEN MATCHED [AND where-clause] THEN MergeUpdate | MergeDelete]
[WHEN NOT MATCHED [AND where-clause] THEN MergeInsert]
Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* DB2 allows multiple WHEN ... AND clauses
* DELETE is also a full-strength option, not part of the MergeUpdate
clause as it is in Oracle
SQL:2008
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON search-condition
[WHEN MATCHED [AND where-clause] THEN MergeUpdate]
[WHEN NOT MATCHED [AND where-clause] THEN MergeInsert]
Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* Allows multiple WHEN ... AND clauses
Alternate Syntax
MySQL supports
* REPLACE INTO
* INSERT ... ON DUPLICATE KEY UPDATE ...
Teradata supports
* UPDATE ... ELSE INSERT ...
* MERGE with an additional error logging clause
The Teradata and Oracle error logging clauses are very cute and I
suggest we do something similar for COPY, at least.
Proposed Syntax for PostgreSQL
==
MERGE INTO table [[AS] alias]
USING [table-ref | query]
ON join-condition
[WHEN MATCHED [AND condition] THEN MergeUpdate | DELETE]
[WHEN NOT MATCHED [AND condition] THEN MergeInsert]
MergeUpdate is
UPDATE SET { column = { expression | DEFAULT } |
( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) }
[, ...]
(yes, there is no WHERE clause here)
MergeInsert is
INSERT [ ( column [, ...] ) ]
{ DEFAULT VALUES | VALUES ( { expression | DEFAULT } [, ...] )
[, ...]}
Re: [HACKERS] Commit fest queue
Peter Eisentraut wrote: Am Dienstag, 15. April 2008 schrieb Zdenek Kotala: JavaDB and Firebird community use Jira Jira had already been rejected many years ago because it is not open source. Jira is also tremendously slow. Not a good system when an individual has to move quickly through a lot of reports. -Bryce -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Andrew Dunstan wrote: Bruce Momjian wrote: Frankly, I am getting pretty tired of people complaining about what I am doing. Perhaps I should just stop and let everyone else deal with things. I have lots of things I would rather be doing. This is one of the reasons I didn't want to add wiki maintenance to my already full workload. Instead I am having to field complaints. I didn't mean to complain about anything. Personally, I'm in favor of reducing your workload. OK. FYI, what would be really nice would be for someone to review and apply the patch or give the author feedback so we could avoid adding it to the wiki at all. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
[EMAIL PROTECTED] (Bruce Momjian) writes: Chris Browne wrote: [EMAIL PROTECTED] (Bruce Momjian) writes: Magnus Hagander wrote: And I think adopting surrounding naming, commeting, coding conventions should come naturally as it can aide in copy-pasting too :) I think pg_indent has to be made a lot more portable and easy to use before that can happen :-) I've run it once or twice on linux machines, and it comes out with huge changes compared to what Bruce gets on his machine. Other times, it doesn't :-) So yeah, it could be that it just needs to be made easier to use, because I may certainly have done something wrong. Agreed, pgindent is too cumbersome to require patch submitters to use. One idea would be to allow C files to be emailed and the indented version automatically returned via email. Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent - Build it _in place_ inside the code tree (e.g. - don't assume it will get installed in /usr/local/bin) - Thus have the ability to run it in place? Yes, but it bloats our code and people still need to generate the typedefs and follow the instructions. The other problem is if they run it on a file they have modified, it is going to adjust places they didn't touch, thereby making the patch harder to review. The bloat is 154K, on a project with something around 260MB of code. I don't think this is a particlarly material degree of bloat. If it is included in src/tools/pgindent, you can add in a Makefile such that it is automatically built, so the cost of running it goes way down, so that it could be run all the time rather than once in a great long while. If it was being run *all* the time, would we not expect to find that we would be seeing relatively smaller sets of changes coming out of it? We are presently at the extreme position where pgindent is run once in a very long time (~ once a year), at pretty considerable cost, and with the associated cost that a whole lot of indentation problems are managed by hand. If we ran pgindent really frequently, there would admittedly be the difference that there would be a lot of little cases of changes-from-pgindent being committed along the way, but [1] might it not be cheaper to accept that cost, with the concomittant benefit that you could tell patchers Hey, run pgindent before submitting that patch, and that'll clean up a number of of the issues. Yes, it doesn't address code changes like typedef generation, but that never was an argument against running pgindent... [1] In cases where the differences primarily fall from differences in indentation, cvs diff -u can drop out those differences when reading the effects of a patch... -- let name=cbbrowne and tld=cbbrowne.com in name ^ @ ^ tld;; http://linuxdatabases.info/info/sap.html A study in the Washington Post says that women have better verbal skills than men. I just want to say to the authors of that study: Duh. -- Conan O' Brien -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 5:17 AM, Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: This is one of the reasons I didn't want to add wiki maintenance to my already full workload. Instead I am having to field complaints. I didn't mean to complain about anything. Personally, I'm in favor of reducing your workload. OK. FYI, what would be really nice would be for someone to review and apply the patch or give the author feedback so we could avoid adding it to the wiki at all. Bruce, Yes, that would be nice! But not likely in practice, unless your patch happens to immediately catch the interest of a suitably qualified person with commit privileges. However, I don't know of any way the maintenance of the wiki is an addition to your workload. I feel that the onus of adding the patch to the wiki should be on the submitter, and we've already had some success getting submitters to add their own patches. And Alvaro has already offered to pick up the slack in cases where the submitter fails to add their patch to the queue. Every patch that somebody else adds to the wiki is another patch you don't have to add to your queue, so how can this be anything but a plus for you? Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBlfb5YBsbHkuyV0RAs6tAKDNR3CbqFC4YwROaoiwNMXSWKXWTgCbBaz1 pO90zcotv+/UMJrotfDTg/M= =IY5R -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DROP DATABASE vs patch to not remove files right away
Tom Lane wrote: Actually ... what if the same DB OID and relfilenode get re-made before the checkpoint? Then we'd be unlinking live data. This is improbable but hardly less so than the scenario the PendingUnlinkEntry code was put in to prevent. ISTM that we must fix the bgwriter so that ForgetDatabaseFsyncRequests causes PendingUnlinkEntrys for the doomed DB to be thrown away too. Because of the asynchronous nature of ForgetDatabaseFsyncRequests, this still isn't enough, I'm afraid. 1. DROP TABLE footable; 2. Checkpoint begins. 3. DROP DATABASE foodb; 4. CREATE DATABASE bardb; -- bardb gets same OID as foodb, and a table copied from template database, let's call it bartable, gets same OID as footable 5. Checkpoint processes pending unlink for footable, but removes bartable instead Needs more thought, after some sleep... -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Wed, Apr 16, 2008 at 3:47 PM, Brendan Jurd [EMAIL PROTECTED] wrote: On Thu, Apr 17, 2008 at 5:17 AM, Bruce Momjian Andrew Dunstan wrote: Bruce Momjian wrote: This is one of the reasons I didn't want to add wiki maintenance to my already full workload. Instead I am having to field complaints. I didn't mean to complain about anything. Personally, I'm in favor of reducing your workload. OK. FYI, what would be really nice would be for someone to review and apply the patch or give the author feedback so we could avoid adding it to the wiki at all. Yes, that would be nice! But not likely in practice, unless your patch happens to immediately catch the interest of a suitably qualified person with commit privileges. However, I don't know of any way the maintenance of the wiki is an addition to your workload. I feel that the onus of adding the patch to the wiki should be on the submitter, and we've already had some success getting submitters to add their own patches. And Alvaro has already offered to pick up the slack in cases where the submitter fails to add their patch to the queue. One small point here. I've been mostly following this discussion on this particular topic but have absolutely no idea what, if anything, to do on the wiki in terms of submitting patch. There was a spot related to the commit fest where we kept an to date version of our patch which I can't find any more (might be lousy search skills on my end). There seems to be information scattered all over the place with various overlapping lists whose function and location are changing constantly. This is not a gripe by any meansit just that you might get a little more help from the grass roots on the wiki as the process settles down and there are examples of what to do. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Chris Browne wrote: Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent - Build it _in place_ inside the code tree (e.g. - don't assume it will get installed in /usr/local/bin) - Thus have the ability to run it in place? Yes, but it bloats our code and people still need to generate the typedefs and follow the instructions. The other problem is if they run it on a file they have modified, it is going to adjust places they didn't touch, thereby making the patch harder to review. The bloat is 154K, on a project with something around 260MB of code. I don't think this is a particlarly material degree of bloat. You mean 37kb vs 13MB, right? That is the tarball sizes I see. If we ran pgindent really frequently, there would admittedly be the difference that there would be a lot of little cases of changes-from-pgindent being committed along the way, but [1] might it not be cheaper to accept that cost, with the concomittant benefit that you could tell patchers Hey, run pgindent before submitting that patch, and that'll clean up a number of of the issues. Yes, it doesn't address code changes like typedef generation, but that never was an argument against running pgindent... That is much a more radical use of pgindent than it has had in the past but it is certainly possible. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Brendan Jurd wrote: OK. FYI, what would be really nice would be for someone to review and apply the patch or give the author feedback so we could avoid adding it to the wiki at all. Bruce, Yes, that would be nice! But not likely in practice, unless your patch happens to immediately catch the interest of a suitably qualified person with commit privileges. However, I don't know of any way the maintenance of the wiki is an addition to your workload. I feel that the onus of adding the patch to the wiki should be on the submitter, and we've already had some success getting submitters to add their own patches. And Alvaro has already offered to pick up the slack in cases where the submitter fails to add their patch to the queue. Every patch that somebody else adds to the wiki is another patch you don't have to add to your queue, so how can this be anything but a plus for you? Yes, but unless people actually applies patches from the wiki, it doesn't help me --- adding patches to my queue is zero cost for me. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 6:03 AM, Merlin Moncure wrote: One small point here. I've been mostly following this discussion on this particular topic but have absolutely no idea what, if anything, to do on the wiki in terms of submitting patch. There was a spot related to the commit fest where we kept an to date version of our patch which I can't find any more (might be lousy search skills on my end). Fair enough. The current commitfest page is at http://wiki.postgresql.org/wiki/CommitFest:May I agree that it's not well linked from the front page of the wiki (you have to follow Development information - Project management documentation - Patches for May Commitfest). I've added a redirect at http://wiki.postgresql.org/wiki/CommitFest which currently points to May, but should be updated whenever we close a commitfest against new submissions. That way submitters will always have one URL to visit to add their stuff. There seems to be information scattered all over the place with various overlapping lists whose function and location are changing constantly. This is not a gripe by any meansit just that you might get a little more help from the grass roots on the wiki as the process settles down and there are examples of what to do. I agree, and in fact I've just recently added some documentation about how to add your patch to the wiki up the top of the CommitFest:May page; I invite you to take a look and post back if you have any comments. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBl9f5YBsbHkuyV0RAoP8AKDAOaXzhsC7ap0/AVf0F6SqxHOZwACfQ4LR JJQcrRKbxoIzQHRbja6gqBw= =tLDr -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
On Thu, 17 Apr 2008 06:19:49 +1000 Brendan Jurd [EMAIL PROTECTED] wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 6:03 AM, Merlin Moncure wrote: One small point here. I've been mostly following this discussion on this particular topic but have absolutely no idea what, if anything, to do on the wiki in terms of submitting patch. There was a spot related to the commit fest where we kept an to date version of our patch which I can't find any more (might be lousy search skills on my end). Fair enough. The current commitfest page is at http://wiki.postgresql.org/wiki/CommitFest:May I agree that it's not well linked from the front page of the wiki (you have to follow Development information - Project management documentation - Patches for May Commitfest). I've added a redirect at http://wiki.postgresql.org/wiki/CommitFest which currently points to May, but should be updated whenever we close a commitfest against new submissions. That way submitters will always have one URL to visit to add their stuff. We should also update the FAQ. Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to submit a patch
Merlin Moncure wrote: I've been mostly following this discussion on this particular topic but have absolutely no idea what, if anything, to do on the wiki in terms of submitting patch. I think the short answer right now to this and to Joshua's original question is: to submit a patch do what has always been done, i.e. send the patch to the mailing list. You can also add the patch to the list on the wiki, but if you don't, it won't be forgotten. All the rest is for reviewers/committers. I've been wondering idly and probably pointlessly if we should try adopting something like the methodology of the Usenet Oracle, which, if you ask it a question will usually send you one to answer in return. Obviously we can't always do that, but maybe we should be asking people who are obviously competent enough (judging by the quality and complexity of the patches they submit) to step up to the plate more on reviewing patches. No amount of process will substitute for more reviewers. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Timely reporting of COPY errors
Hi, I notice that while doing bulk-loads that any errors detected by the backend arn't noticed by libpq until right at the end. Is this intentional? Looking at the code we have this comment in putCopyData: /* * Process any NOTICE or NOTIFY messages that might be pending in the * input buffer. Since the server might generate many notices during the * COPY, we want to clean those out reasonably promptly to prevent * indefinite expansion of the input buffer. (Note: the actual read of * input data into the input buffer happens down inside pqSendSome, but * it's not authorized to get rid of the data again.) */ Except that pqSendSome won't try reading anything until it has a problem writing. Since the backend will consume copy data indefinitly, the error message sits in the kernel buffers until the end. Is there anything that can be done? I've tried putting in PQconsumeInput in places but it doesn't appear to help. Any ideas? -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Please line up in a tree and maintain the heap invariant while boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] Timely reporting of COPY errors
Martijn, * Martijn van Oosterhout ([EMAIL PROTECTED]) wrote: Is there anything that can be done? I've tried putting in PQconsumeInput in places but it doesn't appear to help. I certainly hope something can be done, I've noticed this exact same issue myself and it's very annoying. I've resorted to watching 'top' on the server and hitting ctrl-c when it goes 'idle' but my psql hasn't returned yet. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Lessons from commit fest
Chris Browne [EMAIL PROTECTED] writes: Would it be a terrible idea to... - Draw the indent code from NetBSD into src/tools/pgindent I am not real eager to become maintainers of our own indent fork, which is what you propose. (Just for starters, what will we have to do to make it run on non-BSD systems?) We are presently at the extreme position where pgindent is run once in a very long time (~ once a year), at pretty considerable cost, and with the associated cost that a whole lot of indentation problems are managed by hand. Yeah. One reason for that is that the typedef problem makes it a pretty manual process. The main problem I see with pgindent early and often is that it only works well if everyone is using exactly the same pgindent code (and exactly the same typedef list). Otherwise you just get buried in useless whitespace diffs. It's bad enough that Bruce whacks around his copy from time to time :-(. I would say that the single greatest annoyance for maintaining our back branches is that patches tend to not back-patch cleanly, and well over half the time it's because of random reformattings done by pgindent to code that hadn't changed at all, but it had formatted differently the prior year. For the same reason, my take on your random whitespace changes are acceptable theory is not no but hell no. It's gonna cost us, permanently, in manual patch adjustments if we allow the repository to get cluttered with content-free diffs. I guess an advantage of maintaining our own fork is that there'd be Only One True pgindent, thereby alleviating that problem; but I'm still not eager to go there. If we were going to do it, I'd really wish that we could standardize on a version that didn't need a typedef list. The random whitespace changes you get after changing the typedef list are another thorn in my side. But in any case, this is all focusing on trivialities. The stuff pgindent can fix is, by definition, stuff that committers don't really have to worry about during patch review. The stuff I'm worried about is at a higher level than that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCHES] [HACKERS] Text - C string
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Apr 17, 2008 at 1:16 AM, Tom Lane wrote: Brendan Jurd writes: If we don't want to meddle with xmltype/bytea/VarChar at all, we'll have to revert those changes, and I'll have to seriously scale back the cleanup patch I was about to post. Still not sure where we stand on the above. To cast, or not to cast? I dunno. I know there was previously some handwaving about representing XML values in some more intelligent fashion than a plain text string, but I have no idea if anyone is likely to do anything about it in the foreseeable future. Well ... if somebody does want to change the representation of xml down the road, he's going to have to touch all the sites where the code converts to and from cstring anyway, right? So the only real difference this will make is that, instead of having to replace four lines of VARDATA/memcpy per site, he'll have to replace a single function call. That doesn't seem like a negative. With that in mind, please find attached my followup patch. It cleans up another 21 sites manually copying between cstring and varlena, for a net reduction of 115 lines of code. I didn't attempt to work through every reference to VARDATA, but I did look at every hit from a `grep -Rn VARDATA . | grep memcpy`. All regression tests passed (contrib tests included) on gentoo. Patch added to commitfest queue. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBm105YBsbHkuyV0RAmqfAKCfyNyFdciqX4QV81sG9MhPt+KXuACfe694 3d/ICZF6yqV6K20X3TVX+So= =CvRM -END PGP SIGNATURE- text-cstring-followup.diff.bz2 Description: BZip2 compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Timely reporting of COPY errors
Martijn van Oosterhout [EMAIL PROTECTED] writes: I notice that while doing bulk-loads that any errors detected by the backend arn't noticed by libpq until right at the end. Is this intentional? I dunno about intentional, but the API exposed by libpq for COPY doesn't really permit any other behavior: you push all the data and then look to see if it worked or not. Even if we had some way of letting the application notice that the copy had already failed, I don't see that psql could do very much with it, at least not for COPY FROM STDIN. It's got to read through the source data anyway or it'll be out of sync with the script file. We could possibly fix libpq to start dropping the data on the floor if it sees an error reply already pending, but that's only going to be an incremental change. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Lessons from commit fest
Tom Lane wrote:
The main problem I see with pgindent early and often is that it only
works well if everyone is using exactly the same pgindent code (and
exactly the same typedef list). Otherwise you just get buried in
useless whitespace diffs.
It's bad enough that Bruce whacks around his copy from time to time :-(.
I would say that the single greatest annoyance for maintaining our back
branches is that patches tend to not back-patch cleanly, and well over
half the time it's because of random reformatting done by pgindent
to code that hadn't changed at all, but it had formatted differently
the prior year.
I hate to say it but pgindent formatting changes are usually made in
cases where you or the community want pgindent to improve its indenting. :-)
So we improve pgindent but pay for it in backpatching difficulties. :-(
I guess an advantage of maintaining our own fork is that there'd be Only
One True pgindent, thereby alleviating that problem; but I'm still not
eager to go there. If we were going to do it, I'd really wish that we
could standardize on a version that didn't need a typedef list. The
I don't think that is possible. GNU indent 2.2.9 requires the same -T
option:
You must use the -T option to tell indent the name of all the
type names in your program that are defined by typedef. -T can be
specified more than once, and all names specified are used. For
example, if your program contains
typedef unsigned long CODE_ADDR;
typedef enum {red, blue, green} COLOR;
you would use the options -T CODE_ADDR -T COLOR
astyle doesn't seem to require it but perhaps it just mishandles them.
As I remember there isn't anything about the C grammar that can tell
identify a typedef when it needs to.
But in any case, this is all focusing on trivialities. The stuff
pgindent can fix is, by definition, stuff that committers don't really
have to worry about during patch review. The stuff I'm worried about
is at a higher level than that.
Agreed. Reformatting is small compared to understanding the patch,
adjusting in the comments, testing, documentation, etc.
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
