[PATCHES] Walker/mutator prototype.
This patch adds proper prototypes to the walkers and mutators.
I changed the patch as Greg Stark suggested.
Kurt
Index: src/backend/catalog/dependency.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/catalog/dependency.c,v
retrieving revision 1.34
diff -u -b -r1.34 dependency.c
--- src/backend/catalog/dependency.c29 Nov 2003 19:51:42 - 1.34
+++ src/backend/catalog/dependency.c14 Dec 2003 15:56:35 -
@@ -108,8 +108,7 @@
ObjectAddresses *oktodelete,
Relation depRel);
static void doDeletion(const ObjectAddress *object);
-static bool find_expr_references_walker(Node *node,
- find_expr_references_context
*context);
+static bool find_expr_references_walker(Node *node, void *context);
static void eliminate_duplicate_dependencies(ObjectAddresses *addrs);
static int object_address_comparator(const void *a, const void *b);
static void init_object_addresses(ObjectAddresses *addrs);
@@ -973,9 +972,10 @@
* of the alias list when we find a reference to it.
*/
static bool
-find_expr_references_walker(Node *node,
- find_expr_references_context
*context)
+find_expr_references_walker(Node *node, void *vcontext)
{
+ find_expr_references_context *context = vcontext;
+
if (node == NULL)
return false;
if (IsA(node, Var))
Index: src/backend/commands/tablecmds.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/commands/tablecmds.c,v
retrieving revision 1.94
diff -u -b -r1.94 tablecmds.c
--- src/backend/commands/tablecmds.c29 Nov 2003 19:51:47 - 1.94
+++ src/backend/commands/tablecmds.c14 Dec 2003 15:56:37 -
@@ -77,7 +77,7 @@
static List *MergeAttributes(List *schema, List *supers, bool istemp,
List **supOids, List **supconstr, bool *supHasOids);
-static bool change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
+static bool change_varattnos_of_a_node(Node *node, AttrNumber *newattno);
static void StoreCatalogInheritance(Oid relationId, List *supers);
static int findAttrByName(const char *attributeName, List *schema);
static void setRelhassubclassInRelation(Oid relationId, bool relhassubclass);
@@ -833,8 +833,10 @@
* Note that the passed node tree is modified in place!
*/
static bool
-change_varattnos_walker(Node *node, const AttrNumber *newattno)
+change_varattnos_walker(Node *node, void *context)
{
+ const AttrNumber *newattno = context;
+
if (node == NULL)
return false;
if (IsA(node, Var))
@@ -855,11 +857,11 @@
return false;
}
return expression_tree_walker(node, change_varattnos_walker,
- (void *) newattno);
+ context);
}
static bool
-change_varattnos_of_a_node(Node *node, const AttrNumber *newattno)
+change_varattnos_of_a_node(Node *node, AttrNumber *newattno)
{
return change_varattnos_walker(node, newattno);
}
Index: src/backend/optimizer/path/costsize.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/optimizer/path/costsize.c,v
retrieving revision 1.117
diff -u -b -r1.117 costsize.c
--- src/backend/optimizer/path/costsize.c 3 Dec 2003 17:45:07 - 1.117
+++ src/backend/optimizer/path/costsize.c 14 Dec 2003 15:56:38 -
@@ -104,7 +104,7 @@
static Selectivity estimate_hash_bucketsize(Query *root, Var *var,
int nbuckets);
-static bool cost_qual_eval_walker(Node *node, QualCost *total);
+static bool cost_qual_eval_walker(Node *node, void *total);
static Selectivity approx_selectivity(Query *root, List *quals,
JoinType jointype);
static void set_rel_width(Query *root, RelOptInfo *rel);
@@ -1503,8 +1503,10 @@
}
static bool
-cost_qual_eval_walker(Node *node, QualCost *total)
+cost_qual_eval_walker(Node *node, void *context)
{
+ QualCost *total = context;
+
if (node == NULL)
return false;
Index: src/backend/optimizer/plan/setrefs.c
===
RCS file: /projects/cvsroot/pgsql-server/src/backend/optimizer/plan/setrefs.c,v
retrieving revision 1.99
diff -u -b -r1.99 setrefs.c
--- src/backend/optimizer/plan/setrefs.c29 Nov 2003 19:51:50 - 1.99
+++ src/backend/optimizer/plan/setrefs.c14 Dec 2003 15:56:39 -
@@ -51,14 +51,12 @@
List *inner_tlist,
Index acceptable_rel,
bool
Re: [PATCHES] fork/exec patch
Bruce Momjian [EMAIL PROTECTED] writes: Let me add that Claudio is doing a fantastic job on this. The changes are minimal and clean. I think the writing of a per-backend temp file has allowed this patch to be smaller than it might have been. Did we REALLY conclude that the best way to work around the lack of fork() on Win32 is by writing variables out to disk and reading them back in? Frankly, that seems like an enormous kludge. For example, couldn't we write this data into a particular location in shared memory, and then pass that location to the child? That is still ugly, slow, and prone to failure (shmem being statically sized), but ISTM that the proposed implementation already possesses those attributes :-) (/me goes off to re-read the archives on this issue...) -Neil ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch
Bruce Momjian [EMAIL PROTECTED] writes: I don't think we ever discussed it, but it seemed logical and a minimal change to the code. We already have a GUC write of non-default values for exec and no one had issues with that. For the record, I think that is ugly as well :-) Anyway, I'm not necessarily arguing that using shmem is the right way to go here -- that was merely an off-the-cuff suggestion. I'm just saying that whatever solution we end up with, ISTM we can do better than writing out + reading in a file for /every/ new connection. -Neil ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] fork/exec patch
On Sun, 14 Dec 2003, Bruce Momjian wrote: change to the code. We already have a GUC write of non-default values for exec and no one had issues with that. Of course, this one is per-backend. Yea, we could use shared memory for this too, but I don't see a problem with using the file system. Why not use an anonymous pipe to send data from the parent to the child process? That is a common way to handle this problem in win32 (and in unix by the way). The parent sets up the pipe and the child process inherits the handle, and after that the child and parent can excange information in private. -- /Dennis ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] fork/exec patch
Dennis Bjorklund wrote: On Sun, 14 Dec 2003, Bruce Momjian wrote: change to the code. We already have a GUC write of non-default values for exec and no one had issues with that. Of course, this one is per-backend. Yea, we could use shared memory for this too, but I don't see a problem with using the file system. Why not use an anonymous pipe to send data from the parent to the child process? That is a common way to handle this problem in win32 (and in unix by the way). The parent sets up the pipe and the child process inherits the handle, and after that the child and parent can excange information in private. Doesn't that require the postmaster to stay around to feed that information into the pipe or can the postmaster just shove the data and continue on, and how do the old pipes get cleaned up? Seems messy. Also has to work on Unix too for testing. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch
On Sun, 14 Dec 2003, Bruce Momjian wrote: Why not use an anonymous pipe to send data from the parent to the child process? Doesn't that require the postmaster to stay around to feed that information into the pipe or can the postmaster just shove the data and continue on, and how do the old pipes get cleaned up? I think that one can just output the data and close that end of the pipe. But i've not looked at win32 the last 5 years or so, I could be wrong. Seems messy. Maybe, but to me the solution where you write to files are much more ugly. If one does not like pipes, there are other ipc mechanisms that does not involve creating, reading and deleting a file on each connect. Does windows have a temp filesystem where the temp files are not actually written out on disk? It's still ugly but better then hitting a disk all the time. Also has to work on Unix too for testing. Everything can not work in unix, CreateProcess() and fork() are different. However, the pipe solution can be mimiced in unix, but it will not be the same code since the api's are different. So that does not give much. -- /Dennis ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] fork/exec patch
Hi all, Dennis Bjorklund wrote: Also has to work on Unix too for testing. Everything can not work in unix, CreateProcess() and fork() are different. True (but CreateProcess and fork followed by exec are pretty close). I think what Bruce is implying is that, ideally, we'd like to keep things as close as possible between Unix fork/exec and Windows code bases, so that: * it remains possible to advance the Windows port under a *nix dev environment and * should (when!) issues arise in the Windows implementation, it will be easier to identify and debug Neil Conway wrote: For example, couldn't we write this data into a particular location in shared memory, and then pass that location to the child? That is still ugly, slow, and prone to failure (shmem being statically sized), but ISTM that the proposed implementation already possesses those attributes :-) I agree that this is a better implementation. Bruce, do we implement this now, or just hold it as something to revisit down the track? I'm all for leaving it as is. Moreover, in general, how do we handle things like this? IMHO, I'd rather live with a few kludges (that don't impact the *nix code) until the Windows port is actually a reality, and then reiterate (having the discussions as we go along, however, is necessary). Perhaps adding a TO_REVISIT section to your Win32 Status Report page? Or do people have strong leanings towards fix as you go along? Just feels like that way could see us getting bogged down making things perfect instead of advancing the port... Comments? Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see a href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em ailpolicy.html/a ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] fork/exec patch
On Mon, 15 Dec 2003, Claudio Natoli wrote: Moreover, in general, how do we handle things like this? IMHO, I'd rather live with a few kludges (that don't impact the *nix code) until the Windows port is actually a reality As long as it does not hurt the unix code it's not a big problem as I see it. The usual open source solution is that since no one else writes the code, you can do it the way you think works the best. To change this in the future does not mean that everything else has to be rewritten which is good. It does also not mean that one can not discuss the implementation. A fair amount of discussion is always good. -- /Dennis ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] fork/exec patch
Bruce Momjian [EMAIL PROTECTED] writes: I don't think we ever discussed it, but it seemed logical and a minimal change to the code. We already have a GUC write of non-default values for exec and no one had issues with that. You can hardly claim that no one had issues with that. I complained about it and I think other people did too. It's a messy, ugly approach; moreover we have no field experience that says it's reliable. It may be the least messy, ugly approach available, but I concur with Neil's wish to at least look for other answers. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] fork/exec patch
Alvaro Herrera [EMAIL PROTECTED] writes: On Sun, Dec 14, 2003 at 06:53:22PM -0500, Tom Lane wrote: You can hardly claim that no one had issues with that. Don't the FSM and the system catalog cache use a similar mechanism? FSM uses a backing file to hold information over a database shutdown (write once during shutdown, read once during startup). That's a little different from once per backend fork. Also, there are no race conditions to worry about, and finally the system does not *require* the backing file to be either present or correct. The catalog cache uses a file that typically gets updated once per VACUUM. Again, the file does not have to be present, nor correct. There are mechanisms in place to deal with the cases (including race conditions) where it's broken or obsolete. I have not looked at the proposed fork/exec code in any detail, but IIUC it will be *necessary* that the intermediate file be present, and correct. I think a minimum requirement for accepting this solution is a sketch of how race conditions will be dealt with (ie, postmaster changes its own value of some variable immediately after making the temp file). I don't necessarily say that the first-cut patch has to get it right, but we'd better understand how we will get to where it is right. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] minor SGML improvements
This patch makes some SGML markup more consistent and makes a small improvement to the SSL auth docs. Patch applied to HEAD. -Neil You're a committer now, Neil? Way to go! :) Chris ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch
Claudio Natoli wrote: For example, couldn't we write this data into a particular location in shared memory, and then pass that location to the child? That is still ugly, slow, and prone to failure (shmem being statically sized), but ISTM that the proposed implementation already possesses those attributes :-) I agree that this is a better implementation. Bruce, do we implement this now, or just hold it as something to revisit down the track? I'm all for leaving it as is. Moreover, in general, how do we handle things like this? IMHO, I'd rather live with a few kludges (that don't impact the *nix code) until the Windows port is actually a reality, and then reiterate (having the discussions as we go along, however, is necessary). Perhaps adding a TO_REVISIT section to your Win32 Status Report page? Or do people have strong leanings towards fix as you go along? Just feels like that way could see us getting bogged down making things perfect instead of advancing the port... Let's get it working first. I have added an item to the Win32 status page so we will not forget it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] fork/exec patch
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I don't think we ever discussed it, but it seemed logical and a minimal change to the code. We already have a GUC write of non-default values for exec and no one had issues with that. You can hardly claim that no one had issues with that. I complained about it and I think other people did too. It's a messy, ugly approach; moreover we have no field experience that says it's reliable. It may be the least messy, ugly approach available, but I concur with Neil's wish to at least look for other answers. Absolutely. I am not happy with the GUC file either, but can't see a better way right now. I have already documented your concern about the GUC race condition issue on the Win32 status page so we will not forget about it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch
Bruce Momjian [EMAIL PROTECTED] writes: Agreed, added to the Win32 status page: * remove per-backend parameter file and move into shared memory [itch] I'm not sure that's an answer either; see my comments about how the postmaster shouldn't depend on the contents of shared memory being valid. We could get away with the postmaster having a write-only relationship to shared memory (put value of variable X into predetermined location Y), but I don't think that helps. It doesn't work for variable-size values --- we certainly don't want the postmaster dependent on memory allocation structures being valid within shared memory --- and what about locks? Do you want the postmaster writing shared values without taking a lock, or relying on shared-memory lock structures to be valid enough to not lock it up or crash it? My answer to either of those is no way, Jose ... Writing temp files may actually be a cleaner solution than writing shared memory, once we take these considerations into account. My gripe about race conditions was I want to see how you solve this, and wasn't intended to mean I don't think that is soluble. regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Agreed, added to the Win32 status page: * remove per-backend parameter file and move into shared memory [itch] I'm not sure that's an answer either; see my comments about how the postmaster shouldn't depend on the contents of shared memory being valid. We could get away with the postmaster having a write-only relationship to shared memory (put value of variable X into predetermined location Y), but I don't think that helps. It doesn't work for variable-size values --- we certainly don't want the postmaster dependent on memory allocation structures being valid within shared memory --- and what about locks? Do you want the postmaster writing shared values without taking a lock, or relying on shared-memory lock structures to be valid enough to not lock it up or crash it? My answer to either of those is no way, Jose ... Writing temp files may actually be a cleaner solution than writing shared memory, once we take these considerations into account. My gripe about race conditions was I want to see how you solve this, and wasn't intended to mean I don't think that is soluble. Read my idea that shared memory for signals might be required, and a separate shared memory segment might be used for parameter passing too. I added a question mark to the win32 TODO item, so we can keep as an open item. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
