Re: [HACKERS] Problem with pg_upgrade's directory write check on Windows
Robert Haas wrote: > On Sat, Jul 23, 2011 at 9:14 AM, Bruce Momjian wrote: > > Andrew Dunstan wrote: > >> > We do use access() in a few other places in our code, but not for > >> > directory permission checks. > >> > > >> > Any ideas on a solution? ?Will checking stat() work? ?Do I have to try > >> > creating a dummy file and delete it? > >> > >> That looks like the obvious solution - it's what came to my mind even > >> before reading this sentence. > > > > Well, the easy fix is to see if ALL_DUMP_FILE > > ("pg_upgrade_dump_all.sql") exists, and if so remove it, and if not, > > create it and remove it. > > > > Should I fix this in pg_upgrade 9.1 for Windows or just in 9.2? ?The > > check works fine on non-Windows. > > Seems worth back-patching to me. Attached patch applied and backpatched to 9.1. I was able to test both code paths on my BSD machine by modifying the ifdefs. I will have EnterpriseDB do further testing. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c new file mode 100644 index a76c06e..3493696 *** a/contrib/pg_upgrade/exec.c --- b/contrib/pg_upgrade/exec.c *** *** 16,21 --- 16,24 static void check_data_dir(const char *pg_data); static void check_bin_dir(ClusterInfo *cluster); static void validate_exec(const char *dir, const char *cmdName); + #ifdef WIN32 + static int win32_check_directory_write_permissions(void); + #endif /* *** verify_directories(void) *** 97,113 prep_status("Checking current, bin, and data directories"); - if (access(".", R_OK | W_OK #ifndef WIN32 ! ! /* ! * Do a directory execute check only on Unix because execute permission on ! * NTFS means "can execute scripts", which we don't care about. Also, X_OK ! * is not defined in the Windows API. ! */ ! | X_OK #endif - ) != 0) pg_log(PG_FATAL, "You must have read and write access in the current directory.\n"); --- 100,110 prep_status("Checking current, bin, and data directories"); #ifndef WIN32 ! if (access(".", R_OK | W_OK | X_OK) != 0) ! #else ! if (win32_check_directory_write_permissions() != 0) #endif pg_log(PG_FATAL, "You must have read and write access in the current directory.\n"); *** verify_directories(void) *** 119,124 --- 116,147 } + #ifdef WIN32 + /* + * win32_check_directory_write_permissions() + * + * access() on WIN32 can't check directory permissions, so we have to + * optionally create, then delete a file to check. + * http://msdn.microsoft.com/en-us/library/1w06ktdy%28v=vs.80%29.aspx + */ + static int + win32_check_directory_write_permissions(void) + { + int fd; + + /* + * We open a file we would normally create anyway. We do this even in + * 'check' mode, which isn't ideal, but this is the best we can do. + */ + if ((fd = open(GLOBALS_DUMP_FILE, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) < 0) + return -1; + close(fd); + + return unlink(GLOBALS_DUMP_FILE); + } + #endif + + /* * check_data_dir() * -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
On Fri, Jul 22, 2011 at 10:36 PM, Joey Adams wrote: > Interesting. This leads to a couple more questions: > > * Should the JSON data type (eventually) have an equality operator? +1. > * Should the JSON input function alphabetize object members by key? I think it would probably be better if it didn't. I'm wary of overcanonicalization. It can be useful to have things come back out in more or less the format you put them in. > If we canonicalize strings and numbers and alphabetize object members, > then our equality function is just texteq. The only stumbling block > is canonicalizing numbers. Fortunately, JSON's definition of a > "number" is its decimal syntax, so the algorithm is child's play: > > * Figure out the digits and exponent. > * If the exponent is greater than 20 or less than 6 (arbitrary), use > exponential notation. > > The problem is: 2.718282e-1000 won't equal 0 as may be expected. I > doubt this matters much. I don't think that 2.718282e-100 SHOULD equal 0. > If, in the future, we add the ability to manipulate large JSON trees > efficiently (e.g. by using an auxiliary table like TOAST does), we'll > probably want unique members, so enforcing them now may be prudent. I doubt you're going to want to reinvent TOAST, but I do think there are many advantages to forbidding duplicate keys. ISTM the question is whether to throw an error or just silently discard one of the k/v pairs. Keeping both should not be on the table, IMHO. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Also, should I forbid the escape \u (in all database encodings)? Pros: * If \u is forbidden, and the server encoding is UTF-8, then every JSON-wrapped string will be convertible to TEXT. * It will be consistent with the way PostgreSQL already handles text, and with the decision to use database-encoded JSON strings. * Some applications choke on strings with null characters. For example, in some web browsers but not others, if you pass "Hello\uworld" to document.write() or assign it to a DOM object's innerHTML, it will be truncated to "Hello". By banning \u, users can catch such rogue strings early. * It's a little easier to represent internally. Cons: * Means JSON type will accept a subset of the JSON described in RFC4627. However, the RFC does say "An implementation may set limits on the length and character contents of strings", so we can arguably get away with banning \u while being law-abiding citizens. * Being able to store U+–U+00FF means users can use JSON strings to hold binary data: by treating it as Latin-1. - Joey -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgbench cpu overhead (was Re: [HACKERS] lazy vxid locks, v1)
On Mon, Jun 13, 2011 at 7:03 AM, Stefan Kaltenbrunner wrote: > On 06/13/2011 01:55 PM, Stefan Kaltenbrunner wrote: > > [...] > >> all those tests are done with pgbench running on the same box - which >> has a noticable impact on the results because pgbench is using ~1 core >> per 8 cores of the backend tested in cpu resoures - though I don't think >> it causes any changes in the results that would show the performance >> behaviour in a different light. > > actuall testing against sysbench with the very same workload shows the > following performance behaviour: > > with 40 threads(aka the peak performance point): > > pgbench: 223308 tps > sysbench: 311584 tps > > with 160 threads (backend contention dominated): > > pgbench: 57075 > sysbench: 43437 > > > so it seems that sysbench is actually significantly less overhead than > pgbench and the lower throughput at the higher conncurency seems to be > cause by sysbench being able to stress the backend even more than > pgbench can. > > > for those curious - the profile for pgbench looks like: > > samples % symbol name > 29378 41.9087 doCustom > 17502 24.9672 threadRun > 7629 10.8830 pg_strcasecmp > 5871 8.3752 compareVariables > 2568 3.6633 getVariable > 2167 3.0913 putVariable > 2065 2.9458 replaceVariable > 1971 2.8117 parseVariable > 534 0.7618 xstrdup > 278 0.3966 xrealloc > 137 0.1954 xmalloc Hi Stefan, How was this profile generated? I get a similar profile using --enable-profiling and gprof, but I find it not believable. The complete absence of any calls to libpq is not credible. I don't know about your profiler, but with gprof they should be listed in the call graph even if they take a negligible amount of time. So I think pgbench is linking to libpq libraries that do not themselves support profiling (I have no idea how that could happen though). If the calls graphs are not getting recorded correctly, surely the timing can't be reliable either. (I also tried profiling pgbench with "perf", but in that case I get nothing other than kernel and libc calls showing up. I don't know what that means) To support this, I've dummied up doCustom so that does all the work of deciding what needs to happen, executing the metacommands, interpolating the variables into the SQL string, but then simply refrains from calling the PQ functions to send and receive the query and response. (I had to make a few changes around the select loop in threadRun to support this). The result is that the dummy pgbench is charged with only 57% more CPU time than the stock one, but it gets over 10 times as many "transactions" done. I think this supports the notion that the CPU bottleneck is not in pgbench.c, but somewhere in the libpq or the kernel. Cheers, Jeff -- 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] XPATH vs. server_encoding != UTF-8
On Jul23, 2011, at 22:49 , Peter Eisentraut wrote: > On lör, 2011-07-23 at 17:49 +0200, Florian Pflug wrote: >> The current thread about JSON and the ensuing discussion about the >> XML types' behaviour in non-UTF8 databases made me try out how well >> XPATH() copes with that situation. The code, at least, looks >> suspicious - XPATH neither verifies that the server encoding is UTF-8, >> not does it pass the server encoding on to libxml's xpath functions. > > This issue is on the Todo list, and there are some archive links there. Thanks for the pointer, but I think the discussion there doesn't really apply here. First, I didn't suggest (or implement) full support for XPATH() together with server encodings other than UTF-8. My suggested patch simply closes a hole in the implementation of the current behaviour. Instead of relying on libxml to be able to detect that the encoding isn't UTF-8, it relies on it only to detect that the encoding isn't ASCII. Since supported server encodings are supersets of ASCII, the latter is trivial. xml.c also seems to have changed quite a bite since this was last discussed. Tom Lane argued against the proposed patch on the grounds that there are many more places in xml.c which pass strings to libxml without charset conversion. However, looking at it now, it seems that all XML validation goes through xml_parse(), which actually converts the XML to UTF-8. Only XPATH contains a separate code path, and chooses to ignore encoding issues all together. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Looks like we can't declare getpeereid on Windows anyway.
On 06/02/2011 05:27 PM, Tom Lane wrote: Looks like we can't declare getpeereid on Windows anyway. ... for lack of the uid_t and gid_t typedefs. Per buildfarm. This has given rise to a bunch of warnings on Windows. I'm confused about how we can compile with a signature that includes uid_t and gid_t but not have a prototype because of those. In any case, why are we even compiling it on Windows since it's only called when HAVE_UNIX_SOCKETS is true? 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] XPATH vs. server_encoding != UTF-8
On lör, 2011-07-23 at 17:49 +0200, Florian Pflug wrote: > The current thread about JSON and the ensuing discussion about the > XML types' behaviour in non-UTF8 databases made me try out how well > XPATH() copes with that situation. The code, at least, looks > suspicious - XPATH neither verifies that the server encoding is UTF-8, > not does it pass the server encoding on to libxml's xpath functions. This issue is on the Todo list, and there are some archive links there. -- 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] Policy on pulling in code from other projects?
On 07/23/2011 03:39 AM, Andrew Dunstan wrote: 1. I think the proposed use is of very marginal value at best, and certainly not worth importing an external library for. 2. Even if we have the feature, we do not need to parse URIs generally. A small amount of hand written C code should suffice. If it doesn't that would itself be an argument against it. Well the one area that I think this might be useful in the future is the pg_hba.conf but that is for a whole other thread. JD cheers andrew -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development The PostgreSQL Conference - http://www.postgresqlconference.org/ @cmdpromptinc - @postgresconf - 509-416-6579 -- 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] Policy on pulling in code from other projects?
On 07/22/2011 05:00 PM, Josh Berkus wrote: Arguments in favor of coding from scratch: 1) Does not introduce new dependencies into postgresql-client packages. (note how much of a problem Readline has been) Readline has license issues, this doesn't. 2) keeps psql as lightweight as possible Agreed on that one. 3) We don't need to be able to parse any potential URI, just the ones we accept. True. Arguments against: a) waste of time coding a parser b) eventual feature creep as people ask us for more and more syntax support Yep. Overall, though, I'd say that argument (1) against is pretty powerful. Count the number of complaints and build bug reports about readline& libedit on the lists. That is a good argument. -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development The PostgreSQL Conference - http://www.postgresqlconference.org/ @cmdpromptinc - @postgresconf - 509-416-6579 -- 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] [GENERAL] Dropping extensions
On Sat, 2011-07-23 at 11:08 -0400, Tom Lane wrote: > > If I drop the extension veil_demo, I am left with the veil_demo version > > of veil_init(). > > > Is this a feature or a bug? Is there a work-around? > > Hmm. I don't think we have any code in there to prohibit the same > object from being made a member of two different extensions ... but this > example suggests that maybe we had better check that. > > In general, though, it is not intended that extension creation scripts > use CREATE OR REPLACE, which I gather you must be doing. That's right. Ultimately I'd like to be able to create a number of extensions, all further extending the base functionality of veil, with each one further extending veil_init(). I could consider a more generalised callback mechanism but that adds more complexity and overhead without really buying me anything functionally. I will look into it though. While it would be great to be able to return functions to their previous definition automatically, other simpler mechanisms might suffice. For me, being able to run a post-drop script would probably be adequate. For now, I will just add some notes to the documentation. Thanks for the response. __ Marc signature.asc Description: This is a digitally signed message part
Re: [HACKERS] XPATH vs. server_encoding != UTF-8
[Resent with pgsql-hackers re-added to the recipient list. I presume you didn't remove it on purpose] On Jul23, 2011, at 18:11 , Joey Adams wrote: > On Sat, Jul 23, 2011 at 11:49 AM, Florian Pflug wrote: >> So what I think we should do is tell libxml that the encoding is ASCII >> if the server encoding isn't UTF-8. With that change, the query above >> produces > > I haven't had time to digest this situation, but there is a function > called pg_encoding_to_char for getting a string representation of the > encoding. However, it might not produce a string that libxml > understands in all cases. > > Would it be better to tell libxml the server encoding, whatever it may be? Ultimately, yes. However, I figured if it was as easy as translating our encoding names to those of libxml, the current code would probably do that instead of converting the XML to UTF-8 before validating it. (Validation and XPATH processing use a different code path there!) I'm also not aware of any actual complaints about XPATH's restriction to UTF-8, and it's not a case that I personally care for, so I'm a bit hesitant to put in the time and energy required to extend it to other encodings. But once I had stumbled over this, I didn't want to ignore it all together, so looked for simple way to make the current behaviour more bullet-proof. The patch accomplishes that, I think, and without any major change in behaviour. You only observe the difference if you indeed have non-UTF-8 XMLs which look like valid UTF-8. > In the JSON encoding discussion, the last idea (the one I was planning > to go with) was to allow non-ASCII characters in any server encoding > (like ä in ISO-8859-1), but not allow non-ASCII escapes (like \u00E4) > unless the server encoding is UTF-8. Yeah, that's how I understood your proposal, and it seems sensible. > I think your patch would more > closely match the opposite: allow any escapes, but only allow ASCII > text if the server encoding is not UTF-8. Yeah, but only for XPATH(). XML input validation uses a different code path, and seems to convert the XML to UTF-8 before verifying it's well-formedness with libxml (as you already discovered previously). The difference between JSON and XML here is that the XML types has to live with libxml's idiosyncrasies and restrictions. If we could make libxml use our encoding and text handling infrastructure, the UTF-8 restrictions would probably not exist. But as it stands, libxml has it's own machinery for dealing with encodings... I wonder, BTW, what happens if you attempt to store an XML containing a character not representable in UNICODE. If the conversion to UTF-8 simply replaces it with a placeholder, we'd be fine, since just a replacement cannot affect the well-formedness of an XML. If OTOH it raised an error, that'd be a bit unfortunate... best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] XPATH vs. server_encoding != UTF-8
Hi The current thread about JSON and the ensuing discussion about the XML types' behaviour in non-UTF8 databases made me try out how well XPATH() copes with that situation. The code, at least, looks suspicious - XPATH neither verifies that the server encoding is UTF-8, not does it pass the server encoding on to libxml's xpath functions. So I created a database with encoding ISO-8859-1 (LATIN1), and did (which aclient encoding matching my terminal's settings) CREATE TABLE X (d XML); INSERT INTO X VALUES (''); i.e, I inserted the XML document , but without using an entity reference for the german Umlaut-A. Then I attempted to extract the length of r's attribute "a" with the XPATH /r/@a, both with the XPath function string-length (which works now! yay!) and with postgres' LENGTH() function. SELECT (XPATH('string-length(/r/@a)', d))[1] AS xpath_length, LENGTH((XPATH('/r/@a', d))[1]::text) AS pg_length FROM X; The XPATH() function itself doesn't complain, but libxml does - it expects UTF-8 encoded data, and screams bloody murder when it encounters the ISO-8859-1-encoded Umlaut-A ERROR: could not parse XML document DETAIL: line 1: Input is not proper UTF-8, indicate encoding ! Bytes: 0xE4 0x22 0x2F 0x3E That might seem fine on the surface - we did, after all, error out instead of producing potentially non-sensical results. However, libxml's ability to detect this error relies on it's ability to distinguish between UTF-8 and non-UTF-8 encoded strings. Which, of course, doesn't work in the general case. So for my next try, I deliberately set client_encoding to ISO-8859-1, even though my terminal uses UTF-8, removed all data from table X, and did INSERT INTO X VALUES (''); again. The effect is that is that X now contains ISO-8859-1 encoded data which *happens* to look like valid UTF-8. After changing the client_encoding back to UTF-8, the value we just inserted looks like that Now I invoked the XPATH query from above again. SELECT (XPATH('string-length(/r/@a)', d))[1] AS xpath_length, LENGTH((XPATH('/r/@a', d))[1]::text) AS pg_length FROM X; As predicted, it doesn't raise an error this time, since libxml is unable to distinguish the ISO-8859-1 string ''); SELECT (XPATH('string-length(/r/@a)', d))[1] AS xpath_length, LENGTH((XPATH('/r/@a', d))[1]::text) AS pg_length FROM X; gives xpath_length | pg_length --+--- 1| 1 Proof-of-concept patch attached, but doesn't yet include documentation updates. Comments? Thoughts? Suggestions? best regards, Florian Pflug xpath_nonutf8.patch Description: Binary 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] proposal: a validator for configuration files
Florian Pflug writes: > A variant of this would be to allow extensions (in the CREATE EXTENSION > sense) to declare custom GUCs in their control file. Then we'd only > need to load those files, which seems better than loading a shared > library. The original patch for the extensions had that feature. It had to be removed, though. The control file value has to be stored in the catalogs and now only backends can look that up once connected to a database. This part worked well. http://git.postgresql.org/gitweb/?p=postgresql-extension.git;a=commit;h=480fa10f4ec7b495cf4f434e6f44a50b94487326 http://git.postgresql.org/gitweb/?p=postgresql-extension.git;a=commit;h=98d802aa1ee12c77c5270c7bc9ed7479360f35b9 IIRC the problem is that now, the postmaster is not seeing cvc at all, and you can have there some custom parameters to set shared memory segments, which only postmaster will take care of. An example of that is the pg_stat_statements contrib. I would love that we find a way around that, btw. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] [GENERAL] Dropping extensions
Marc Munro writes: > In postgres 9.1 I have created 2 extensions, veil and veil_demo. When I > install veil, it creates a default (not very useful) version of a > function: veil_init(). > When I create veil_demo, it replaces this version of the function with > it's own (useful) version. > If I drop the extension veil_demo, I am left with the veil_demo version > of veil_init(). > Is this a feature or a bug? Is there a work-around? Hmm. I don't think we have any code in there to prohibit the same object from being made a member of two different extensions ... but this example suggests that maybe we had better check that. In general, though, it is not intended that extension creation scripts use CREATE OR REPLACE, which I gather you must be doing. 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] Questions and experiences writing a Foreign Data Wrapper
Andrew Dunstan writes: > In that case I think I'm in favor of the suggestion of an implied empty > user mapping for PUBLIC, as long as it can be overridden. But how would you do that (override it)? All you can do is create an explicit mapping, and then you still have a mapping that allows access to PUBLIC. (Or not, but if an empty one does, you're stuck.) 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] Questions and experiences writing a Foreign Data Wrapper
"Albe Laurenz" writes: > I don't like to think of a user mapping as a means to restrict access > to the foreign data source, because in effect that is the same as > restricting access to the foreign table, which is the ACL's job. No, the standard is quite clear that those are distinct things. See the NOTE I quoted upthread. 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] Questions and experiences writing a Foreign Data Wrapper
On 07/23/2011 10:42 AM, Tom Lane wrote: Andrew Dunstan writes: What does the standard say? Well, there is not a statement in so many words that you have to have a relevant USER MAPPING to use a foreign table. But the spec does specify that an FDW's ConnectServer function takes a UserHandle as one input parameter and should throw an exception if that handle isn't valid. And as far as I can tell a UserHandle can only be created from a relevant USER MAPPING entry. So I think the behavior I'm arguing for would emerge from an FDW that was built using the spec-defined API. We only have an opportunity to argue about it because we chose to invent our own FDW API. In that case I think I'm in favor of the suggestion of an implied empty user mapping for PUBLIC, as long as it can be overridden. It does seem to be very late in the day to be arguing about such details, though, unless we're talking about changing it in the 9.2 cycle. 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] Questions and experiences writing a Foreign Data Wrapper
Andrew Dunstan writes: > What does the standard say? Well, there is not a statement in so many words that you have to have a relevant USER MAPPING to use a foreign table. But the spec does specify that an FDW's ConnectServer function takes a UserHandle as one input parameter and should throw an exception if that handle isn't valid. And as far as I can tell a UserHandle can only be created from a relevant USER MAPPING entry. So I think the behavior I'm arguing for would emerge from an FDW that was built using the spec-defined API. We only have an opportunity to argue about it because we chose to invent our own FDW API. 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] Problem with pg_upgrade's directory write check on Windows
On Sat, Jul 23, 2011 at 9:14 AM, Bruce Momjian wrote: > Andrew Dunstan wrote: >> > We do use access() in a few other places in our code, but not for >> > directory permission checks. >> > >> > Any ideas on a solution? Will checking stat() work? Do I have to try >> > creating a dummy file and delete it? >> >> That looks like the obvious solution - it's what came to my mind even >> before reading this sentence. > > Well, the easy fix is to see if ALL_DUMP_FILE > ("pg_upgrade_dump_all.sql") exists, and if so remove it, and if not, > create it and remove it. > > Should I fix this in pg_upgrade 9.1 for Windows or just in 9.2? The > check works fine on non-Windows. Seems worth back-patching to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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 pg_upgrade's directory write check on Windows
Andrew Dunstan wrote: > > We do use access() in a few other places in our code, but not for > > directory permission checks. > > > > Any ideas on a solution? Will checking stat() work? Do I have to try > > creating a dummy file and delete it? > > That looks like the obvious solution - it's what came to my mind even > before reading this sentence. Well, the easy fix is to see if ALL_DUMP_FILE ("pg_upgrade_dump_all.sql") exists, and if so remove it, and if not, create it and remove it. Should I fix this in pg_upgrade 9.1 for Windows or just in 9.2? The check works fine on non-Windows. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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 pg_upgrade's directory write check on Windows
On 07/23/2011 08:45 AM, Bruce Momjian wrote: Pg_upgrade writes temporary files (e.g. _dumpall output) into the current directory, rather than a temporary directory or the user's home directory. (This was decided by community discussion.) I have a check in pg_upgrade 9.1 to make sure pg_upgrade has write permission in the current directory: if (access(".", R_OK | W_OK #ifndef WIN32 /* * Do a directory execute check only on Unix because execute permission on * NTFS means "can execute scripts", which we don't care about. Also, X_OK * is not defined in the Windows API. */ | X_OK #endif ) != 0) pg_log(PG_FATAL, "You must have read and write access in the current directory.\n"); Unfortunately, I have received a bug report from EnterpriseDB testing that this does not trigger the FATAL exit on Windows even if the user does not have write permission in the current directory, e.g. C:\. I think I see the cause of the problem. access() on Windows is described here: http://msdn.microsoft.com/en-us/library/1w06ktdy%28v=vs.80%29.aspx It specifically says: When used with directories, _access determines only whether the specified directory exists; in Windows NT 4.0 and Windows 2000, all directories have read and write access. ... This function only checks whether the file and directory are read-only or not, it does not check the filesystem security settings. For that you need an access token. We do use access() in a few other places in our code, but not for directory permission checks. Any ideas on a solution? Will checking stat() work? Do I have to try creating a dummy file and delete it? That looks like the obvious solution - it's what came to my mind even before reading this sentence. 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] Problem with pg_upgrade's directory write check on Windows
Pg_upgrade writes temporary files (e.g. _dumpall output) into the current directory, rather than a temporary directory or the user's home directory. (This was decided by community discussion.) I have a check in pg_upgrade 9.1 to make sure pg_upgrade has write permission in the current directory: if (access(".", R_OK | W_OK #ifndef WIN32 /* * Do a directory execute check only on Unix because execute permission on * NTFS means "can execute scripts", which we don't care about. Also, X_OK * is not defined in the Windows API. */ | X_OK #endif ) != 0) pg_log(PG_FATAL, "You must have read and write access in the current directory.\n"); Unfortunately, I have received a bug report from EnterpriseDB testing that this does not trigger the FATAL exit on Windows even if the user does not have write permission in the current directory, e.g. C:\. I think I see the cause of the problem. access() on Windows is described here: http://msdn.microsoft.com/en-us/library/1w06ktdy%28v=vs.80%29.aspx It specifically says: When used with directories, _access determines only whether the specified directory exists; in Windows NT 4.0 and Windows 2000, all directories have read and write access. ... This function only checks whether the file and directory are read-only or not, it does not check the filesystem security settings. For that you need an access token. We do use access() in a few other places in our code, but not for directory permission checks. Any ideas on a solution? Will checking stat() work? Do I have to try creating a dummy file and delete it? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] cataloguing NOT NULL constraints
On Sat, Jul 23, 2011 at 4:37 AM, Dean Rasheed wrote: > That looks wrong to me, because a NOT NULL constraint is a column > constraint not a table constraint. The CREATE TABLE syntax explicitly > distinguishes these 2 cases, and only allows NOT NULLs in column > constraints. So from a consistency point-of-view, I think that ALTER > TABLE should follow suit. > > So the new syntax could be: > > ALTER TABLE table_name ALTER [COLUMN] col_name ADD column_constraint > > where column_constraint is the same as in CREATE TABLE (i.e., allowing > all the other constraint types at the same time). > > It looks like that approach would probably lend itself to more > code-reusability too, especially once we start adding options to the > constraint. So you'd end up with something like this? ALTER TABLE foo ALTER COLUMN bar ADD CONSTRAINT somename NOT NULL That works for me. I think sticking the name of the constraint in there at the end of the line as Alvaro proposed would be terrible for future syntax extensibility - we'll be much less likely to paint ourselves into a corner with something like this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Questions and experiences writing a Foreign Data Wrapper
On 07/22/2011 11:34 AM, Robert Haas wrote: No, you can specify connection details at per-server and per-foreign-table level too. The FDW implementation is free to accept or reject options where-ever it wants. Well, if we are going to take that viewpoint, then not having a user mapping *shouldn't* be an error, for any use-case. What would be an error would be not having the foreign-user-name-or-equivalent specified anywhere in the applicable options, but it's up to the FDW to notice and complain about that. +1. What does the standard say? You can get around most of the inconvenience with an empty PUBLIC user mapping, although it's mildly annoying if you've forgotten to make one. 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] Policy on pulling in code from other projects?
On 07/22/2011 10:51 AM, Joshua D. Drake wrote: On 07/21/2011 11:13 AM, Tom Lane wrote: "Joshua D. Drake" writes: So I am looking intently on what it is going to take to get the URI patch done for psql [1] and was digging around the web and have a URI parser library. It is under the New BSD license and is strictly RFC RFC 3986 [2] compliant . Surely we do not need a whole library to parse URIs. regards, tom lane Any other comments? I don't want to do a bunch of work just to have it tossesd on a technicality. 1. I think the proposed use is of very marginal value at best, and certainly not worth importing an external library for. 2. Even if we have the feature, we do not need to parse URIs generally. A small amount of hand written C code should suffice. If it doesn't that would itself be an argument against it. 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] cataloguing NOT NULL constraints
On 22 July 2011 22:28, Robert Haas wrote: > On Fri, Jul 22, 2011 at 4:39 PM, Alvaro Herrera > wrote: >> Excerpts from Robert Haas's message of vie jul 22 12:14:30 -0400 2011: >>> On Thu, Jul 21, 2011 at 7:51 PM, Alvaro Herrera >>> wrote: >>> >> I think that there probably ought to be a way to display the NOT NULL >>> >> constraint names (perhaps through \d+). For example, if you're >>> >> planning to support NOT VALID on top of this in the future, then there >>> >> needs to be a way to get the constraint's name to validate it. >>> > >>> > Absolutely true. Another thing that needs to be done here is to let the >>> > ALTER TABLE and ALTER DOMAIN commands use the constraint names; right >>> > now, they simply let you add the constraint but not specify the name. >>> > That should probably be revisited. >>> >>> That, at least, seems like something that should be fixed before commit. >> >> Hmm, which point, Dean's or mine? Dean was saying that the name should >> be displayed by some flavor of \d; > > That might not be 100% necessary for the initial commit, but seems > easy to fix, so why not? > Agreed. >> mine was that we need a command such >> as >> >> ALTER TABLE foo ALTER COLUMN bar SET NOT NULL name_of_notnull_constr >> >> where the last bit is what's new. > > Well, if you don't have that, I don't see how you have any chance of > pg_dump working correctly. Ah yes, pg_dump's dumpConstraint() needs a clause to alter a table adding a named NOT NULL constraint (and the DOMAIN case should be preserving the constraint's name too). So it looks like some new syntax for ALTER TABLE to add named NOT NULL constraints is probably needed before this can be committed. > Though I think it should use the table > constraint syntax: > > CONSTRAINT name_of_notnull_constr constraint_definition > > I'm not exactly sure what to propose for the constraint_definition. > Perhaps just: > > CONSTRAINT name_of_notnull_constr NOT NULL column_name > That looks wrong to me, because a NOT NULL constraint is a column constraint not a table constraint. The CREATE TABLE syntax explicitly distinguishes these 2 cases, and only allows NOT NULLs in column constraints. So from a consistency point-of-view, I think that ALTER TABLE should follow suit. So the new syntax could be: ALTER TABLE table_name ALTER [COLUMN] col_name ADD column_constraint where column_constraint is the same as in CREATE TABLE (i.e., allowing all the other constraint types at the same time). It looks like that approach would probably lend itself to more code-reusability too, especially once we start adding options to the constraint. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers