Re: [HACKERS] Support UTF-8 files with BOM in COPY FROM
On Sep 25, 2011, at 9:58 PM, Itagaki Takahiro wrote: > I'd like to support UTF-8 text or csv files that has BOM (byte order mark) > in COPY FROM command. BOM will be automatically detected and ignored > if the file encoding is UTF-8. WIP patch attached. By my reading of http://unicode.org/faq/utf_bom.html#bom5, I'd say +1 So I think what you propose makes sense. > I'm thinking about only COPY FROM for reads, but if someone wants to add > BOM in COPY TO, we might also support COPY TO WITH BOM for writes. I think it would have to be optional, since "some recipients of UTF-8 encoded data do not expect a BOM." Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Support UTF-8 files with BOM in COPY FROM
Hi, I'd like to support UTF-8 text or csv files that has BOM (byte order mark) in COPY FROM command. BOM will be automatically detected and ignored if the file encoding is UTF-8. WIP patch attached. I'm thinking about only COPY FROM for reads, but if someone wants to add BOM in COPY TO, we might also support COPY TO WITH BOM for writes. Comments welcome. -- Itagaki Takahiro copy_from_bom.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] contrib/sepgsql regression tests are a no-go
On Mon, Sep 26, 2011 at 12:06 AM, Tom Lane wrote: >> Then we >> could get rid of the associated configure option, which no longer >> serves any other purpose, and just say that if you want to build (or >> regression-test) sepgsql, well, you gotta go to that directory and do >> it by hand. > > The trouble with getting rid of configure support is that we lose the > opportunity to verify that you have a new-enough version of libselinux. > That's maybe not a big deal, but it's a point worth noting. I suppose the main downside there is that you will get a more cryptic error message if it isn't new enough. On the other hand, even if you don't use --with-selinux, you can still do "make -C contrib/sepgsql" and you'll have the same problem, so I'm not sure that we're really heavily shielded against a failure of this type as it is. Perhaps we could remove --with-selinux, but *always* test whether we have a new enough version of selinux. If we don't, we set it up so that configure succeeds anyway, but any attempt to build sepgsql fails? (Not sure if there's a clean way to do that.) -- 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] contrib/sepgsql regression tests are a no-go
Robert Haas writes: > On Sun, Sep 25, 2011 at 9:07 PM, Tom Lane wrote: >> As a stopgap, what about removing sepgsql from the list of contrib >> modules tested by "make -C contrib check"? > +1. > In fact, I've been wondering if we ought to go a step further and not > recurse into the sepgsql directory for *any* of the targets. Hmm. That would be a lot cleaner than the alternatives I've thought of so far for recursing for only some targets. > Then we > could get rid of the associated configure option, which no longer > serves any other purpose, and just say that if you want to build (or > regression-test) sepgsql, well, you gotta go to that directory and do > it by hand. The trouble with getting rid of configure support is that we lose the opportunity to verify that you have a new-enough version of libselinux. That's maybe not a big deal, but it's a point worth noting. 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] contrib/sepgsql regression tests are a no-go
On Sun, Sep 25, 2011 at 9:07 PM, Tom Lane wrote: > As a stopgap, what about removing sepgsql from the list of contrib > modules tested by "make -C contrib check"? +1. In fact, I've been wondering if we ought to go a step further and not recurse into the sepgsql directory for *any* of the targets. Then we could get rid of the associated configure option, which no longer serves any other purpose, and just say that if you want to build (or regression-test) sepgsql, well, you gotta go to that directory and do it by hand. -- 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] Inlining comparators as a performance optimisation
On Sun, Sep 25, 2011 at 10:12 PM, Peter Geoghegan wrote: > [ new results ] Nice results. I think these are far more convincing than the last set, because (1) the gains are bigger and (2) they survive -O2 and (3) you tested an actual query, not just qsort() itself. I don't want to take time to review this in detail right now, because I don't think it would be fair to put this ahead of things that were submitted for the current CommitFest, but I'm impressed. > On the subject of highly ambitious optimisations to sorting, one > possibility I consider much more practicable than GPU-accelerated > sorting is simple threading; quicksort can be parallelised very > effectively, due to its divide-and-conquer nature. If we could agree > on a threading abstraction more sophisticated than forking, it's > something I'd be interested in looking at. To do so would obviously > entail lots of discussion about how that relates to whatever way we > eventually decide on implementing parallel query, and that's obviously > a difficult discussion. I have the same feeling about about this that I do about almost every executor optimization that anyone proposes: the whole project would be entirely simple and relatively painless if it weren't for the need to make planner changes. I mean, deciding on a threading interface is likely to be a somewhat contentious discussion, with differences of opinion on whether we should do it and what the API should look like. But at the end of the day it's not rocket science, and I expect that we would end up with something reasonable. What seems much harder is figuring out how to decide when to perform quicksort in parallel vs. single-threaded, and how much parallelism would be appropriate. I haven't seen anyone propose even a shadow of an idea about how to make such decisions intelligently, either in general or in specific cases. The other issue is that, while threading would be possibly suitable for this particular case, at least for built-in datatypes with comparison operations that basically reduce to single machine-language comparison instructions, it's hard to see how we could take it much further. It would be unsafe for these multiple threads of execution to do anything that could possibly throw an error or anything that touches a lightweight lock or, really, just about anything at all. Trying to make the entire backend thread-safe - or even any significant portion of it - seems like a colossal effort that will most likely fail, but maybe not without eating an enormous amount of developer time first. And without doing that, I don't think we could even extend this as far as, say, numeric, whose functions do things like palloc() and ereport() internally. So I feel like this whole approach might be a dead-end - there's a narrow range of cases where it could be made to work, I think, but after that I think you hit a titanium wall. -- 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] [v9.2] Fix Leaky View Problem
On Sun, Sep 25, 2011 at 10:38 PM, Noah Misch wrote: > On Sun, Sep 25, 2011 at 11:22:03AM -0500, Kevin Grittner wrote: >> Robert Haas 09/25/11 10:58 AM >>> >> >> > I'm not sure we've been 100% consistent about that, since we >> > previously made CREATE OR REPLACE LANGUAGE not replace the owner >> > with the current user. >> >> I think we've been consistent in *not* changing security on an >> object when it is replaced. > >> [CREATE OR REPLACE FUNCTION does not change proowner or proacl] > > Good point. C-O-R VIEW also preserves column default values. I believe we > are > consistent to the extent that everything possible to specify in each C-O-R > statement gets replaced outright. The preserved characteristics *require* > commands like GRANT, COMMENT and ALTER VIEW to set in the first place. > > The analogue I had in mind is SECURITY DEFINER, which C-O-R FUNCTION reverts > to > SECURITY INVOKER if it's not specified each time. That default is safe, > though, > while the proposed default of security_barrier=false is unsafe. Even though I normally take the opposite position, I still like the idea of dedicated syntax for this feature. Not knowing what view options we might end up with in the future, I hate having to decide on what the general behavior ought to be. But it would be easy to decide that CREATE SECURITY VIEW followed by CREATE OR REPLACE VIEW leaves the security flag set; it would be consistent with what we're doing with owner and acl information and wouldn't back us into any unpleasant decisions down the road. -- 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] [v9.2] Fix Leaky View Problem
On Sun, Sep 25, 2011 at 11:22:03AM -0500, Kevin Grittner wrote: > Robert Haas 09/25/11 10:58 AM >>> > > > I'm not sure we've been 100% consistent about that, since we > > previously made CREATE OR REPLACE LANGUAGE not replace the owner > > with the current user. > > I think we've been consistent in *not* changing security on an > object when it is replaced. > [CREATE OR REPLACE FUNCTION does not change proowner or proacl] Good point. C-O-R VIEW also preserves column default values. I believe we are consistent to the extent that everything possible to specify in each C-O-R statement gets replaced outright. The preserved characteristics *require* commands like GRANT, COMMENT and ALTER VIEW to set in the first place. The analogue I had in mind is SECURITY DEFINER, which C-O-R FUNCTION reverts to SECURITY INVOKER if it's not specified each time. That default is safe, though, while the proposed default of security_barrier=false is unsafe. Thanks, nm -- 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] Online base backup from the hot-standby
On 11-09-22 09:24 AM, Fujii Masao wrote: On Wed, Sep 21, 2011 at 11:50 AM, Fujii Masao wrote: 2011/9/13 Jun Ishiduka: Update patch. Changes: * set 'on' full_page_writes by user (in document) * read "FROM: XX" in backup_label (in xlog.c) * check status when pg_stop_backup is executed (in xlog.c) Thanks for updating the patch. Before reviewing the patch, to encourage people to comment and review the patch, I explain what this patch provides: Attached is the updated version of the patch. I refactored the code, fixed some bugs, added lots of source code comments, improved the document, but didn't change the basic design. Please check this patch, and let's use this patch as the base if you agree with that. I have looked at both Jun's patch from Sept 13 and Fujii's updates to the patch. I agree that Fujii's updated version should be used as the basis for changes going forward. My comments below refer to that version (unless otherwise noted). In backup.sgml the new section titled "Making a Base Backup during Recovery" I would prefer to see some mention in the title that this procedure is for standby servers ie "Making a Base Backup from a Standby Database". Users who have setup a hot-standby database should be familiar with the 'standby' terminology. I agree that the "during recovery" description is technically correct but I'm not sure someone who is looking through the manual for instructions on making a base backup from here standby will realize this is the section they should read. Around line 969 where you give an example of copying the control file I would be a bit clearer that this is an example command. Ie (Copy the pg_control file from the cluster directory to the global sub-directory of the backup. For example "cp $PGDATA/global/pg_control /mnt/server/backupdir/global") Testing Notes - I created a standby server from a base backup of another standby server. On this new standby server I then 1. Ran pg_start_backup('3'); and left the psql connection open 2. touch /tmp/3 -- my trigger_file ssinger@ssinger-laptop:/usr/local/pgsql92git/bin$ LOG: trigger file found: /tmp/3 FATAL: terminating walreceiver process due to administrator command LOG: restored log file "00010006" from archive LOG: record with zero length at 0/60002F0 LOG: restored log file "00010006" from archive LOG: redo done at 0/6000298 LOG: restored log file "00010006" from archive PANIC: record with zero length at 0/6000298 LOG: startup process (PID 19011) was terminated by signal 6: Aborted LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. The new postmaster (the one trying to be promoted) dies. This is somewhat repeatable. If a base backup is in progress on a recovery database and that recovery database is promoted to master, following the promotion (if you don't restart the postmaster). I see select pg_stop_backup(); ERROR: database system status mismatches between pg_start_backup() and pg_stop_backup() If you restart the postmaster this goes away. When the postmaster leaves recovery mode I think it should abort an existing base backup so pg_stop_backup() will say no backup in progress, or give an error message on pg_stop_backup() saying that the base backup won't be usable. The above error doesn't really tell the user why there is a mismatch. - In my testing a few times I got into a situation where a standby server coming from a recovery target took a while to finish recovery (this is on a database with no activity). Then when i tried promoting that server to master I got LOG: trigger file found: /tmp/3 FATAL: terminating walreceiver process due to administrator command LOG: restored log file "00010009" from archive LOG: restored log file "00010009" from archive LOG: redo done at 0/9E8 LOG: restored log file "00010009" from archive PANIC: unexpected pageaddr 0/600 in log file 0, segment 9, offset 0 LOG: startup process (PID 1804) was terminated by signal 6: Aborted LOG: terminating any other active server processes It is *possible* I mixed up the order of a step somewhere since my testing isn't script based. A standby server that 'looks' okay but can't actually be promoted is dangerous. This version of the patch (I was testing the Sept 22nd version) seems less stable than how I remember the version from the July CF. Maybe I'm just testing it harder or maybe something has been broken. In the current patch, there is no safegua
Re: [HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
> Tom Lane wrote: > "Kevin Grittner" writes: >> This is a review of the patch at this CF location: >> https://commitfest.postgresql.org/action/patch_view?id=598 >> as posted here: >> http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com > > Hmm, why is that patch the one posted for review, when several > better versions were already discussed? See thread starting here: > http://archives.postgresql.org/pgsql-hackers/2011-07/msg00028.php The patch I reviewed was added to the CF app before the post you cite was written. I didn't see it in following the links (probably because it crossed a month boundary). Thanks for pointing that out; I'll update the CF app and review the later versions. -Kevin -- 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] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
"Kevin Grittner" writes: > This is a review of the patch at this CF location: > https://commitfest.postgresql.org/action/patch_view?id=598 > as posted here: > http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com Hmm, why is that patch the one posted for review, when several better versions were already discussed? See thread starting here: http://archives.postgresql.org/pgsql-hackers/2011-07/msg00028.php 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
[HACKERS] contrib/sepgsql regression tests are a no-go
So I thought it would be a good idea to enable contrib/sepgsql in the Fedora build of 9.1. This soon crashed and burned, though, because (1) if you build sepgsql, there is no way to omit the sepgsql regression tests, other than by not regression-testing contrib at all. I didn't see that as a step forward. (2) the sepgsql regression tests cannot be run without having done assorted root-privilege-required tweaks to the SELinux configuration. RPMs are not customarily built with root privilege, and even if they were, having one fool with the SELinux configuration like that would set off alarm bells for a lot of people. In particular, point (2) means that "make check" is a joke. You can't do it without serious side-effects on the state of the host system, which is contrary to the entire purpose of that make target. I am of the opinion that these regression tests need to be reworked so that they do not require a nonstandard SELinux environment. I realize however that that's likely to take nontrivial work. As a stopgap, what about removing sepgsql from the list of contrib modules tested by "make -C contrib check"? (I haven't looked at exactly how ugly it might be to do that, nor whether we'd have to also disable installcheck from recursing to sepgsql.) 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] [v9.2] Fix Leaky View Problem
On Sun, Sep 25, 2011 at 3:25 PM, Kohei KaiGai wrote: >> I'm a bit nervous about storing security_barrier in the RTE. What >> happens to stored rules if the security_barrier option gets change >> later? >> > The rte->security_barrier is evaluated when a query referencing security > views get expanded. So, rte->security_barrier is not stored to catalog. I think it is. If you create a view that involves an RTE, the node tree is going to get stored in pg_rewrite.ev_action. And it's going to include the security_barrier attribute, because you added outfuncs support for it... No? -- 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] index-only scans
On Sun, Sep 25, 2011 at 2:43 PM, Marti Raudsepp wrote: > On Sun, Aug 14, 2011 at 00:31, Heikki Linnakangas > wrote: >> That is somewhat compensated by the fact that tuples that are accessed more >> often are also more likely to be in cache. Fetching the heap tuple to check >> visibility is very cheap when the tuple is in cache. >> >> I'm not sure how far that compensates it, though. I'm sure there's typically >> nevertheless a fairly wide range of pages that have been modified since the >> last vacuum, but not in cache anymore. > > Would it make sense to re-evaluate the visibility bit just before a > page gets flushed out from shared buffers? On a system with no long > transactions, it seems likely that a dirty page is already all-visible > by the time bgwriter (or shared buffers memory pressure) gets around > to writing it out. That way we don't have to wait for vacuum to do it > and would make your observation hold more often. This has been suggested before, and, sure, there might be cases where it helps. But you need to choose your test case fairly carefully. For example, if you're doing a large sequential scan on a table, the ring-buffer logic causes processes to evict their own pages, and so the background writer doesn't get a chance to touch any of those pages. You need some kind of a workload where pages are being evicted from shared buffers slowly enough that it ends up being the background writer, rather than the individual backends, that do the work. But if you have that kind of workload, then we can infer that most of your working set fits into shared buffers. And in that case you don't really need index-only scans in the first place. The main point of index only scans is to optimize the case where you have a gigantic table and you're trying to avoid swamping the system with random I/O. I'm not saying that such a change would be a particularly bad idea, but I'm not personally planning to work on it any time soon because I can't convince myself that it really helps all that much. I think the real solution to getting visibility map bits set is to vacuum more frequently, but have it be cheaper each time. Our default autovacuum settings vacuum the table when the number of updates and deletes reaches 20% of the table size. But those settings were put in place under the assumption that we'll have to scan the entire heap, dirtying every page that contains dead tuples, scan all the indexes and remove the associated index pointers, and then scan and dirty the heap pages that contain now-dead line pointers a second time to remove those. The visibility map has eroded those assumptions to some degree, because now we probably won't have to scan the entire heap every time we vacuum; and I'm hoping we're going to see some further erosion. Pavan has a pending patch which, if we can work out the details, will eliminate the second heap scan; and we've also talked about doing the index scan only when there are enough dead line pointers to justify the effort. That, it seems to me, would open the door to lowering the scale factor, maybe by quite a bit - which, in turn, would help us control bloat better and get visibility map bits set sooner. -- 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
[HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
This is a review of the patch at this CF location: https://commitfest.postgresql.org/action/patch_view?id=598 as posted here: http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com This patch applied cleanly and compiled without warning. It performed correctly. Since the patch modifies one function which has one input and one output (through a pointer parameter), with no other side effects, it has low risk of surprising problems. It is strictly a performance patch, and accomplishes its goals. I ran the entire dictionary through the patched and unpatched versions five times each (using the supplied debugging patch) and saw no changes to the behavior of the function -- identical results every time. I ran performance tests with building and reindexing the sorted dictionary, a randomly ordered dictionary, a randomly ordered dictionary with the entries randomly truncated to 0 to 3 characters, and a large README file -- five times each with and without the patch. The patch was a clear winner with all of those except the truncated dictionary, where the difference we well within the noise (0.05% difference summing five runs when individual runs could vary by up to about 4%). While there was no reason to believe it could affect search performance, I tested that anyway, and found no difference. Once this message shows up on the list (so I can retrieve the message ID) I will mark this "Ready for Committer". -Kevin -- 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] fix for pg_upgrade
panam wrote: > OK, i started once again: > > > I hope the following is the correct way of querying the table corresponding > to a relid: > > > > > > > > > > -- > View this message in context: > http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html Yes, that is very close to what I needed. Ideally you would have included the oid from pg_class: select oid, * from pg_class where oid = 465783 or oid = 16505 Can you supply that? Also can you email me privately the following output from the old database? It should only be the schema and not your data: pg_dumpall --schema-only --binary-upgrade I am looking for something like this in the file: -- For binary upgrade, must preserve pg_class oids SELECT binary_upgrade.set_next_heap_pg_class_oid('16385'::pg_catalog.oid); CREATE TABLE test ( x integer ); but for your case it would be the 'accounts' file. You can email just those lines if you want, and that you can probably email to hackers. Thanks. -- 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] [v9.2] DROP statement reworks
Hi, Kohei KaiGai writes: > 2011/8/15 Kohei KaiGai : >> The attached three patches try to consolidate code path of DROP >> statement on various kind of object classes. > > These are rebased to the latest tree, and the part-3 portion also consolidates > DROP OPERATOR FAMILY/CLASS routines that I forgot to rework in the > previous patch. I've been reviewing those patches, that are all about code refactoring. I like what it's doing, generalizing ad-hoc code by adding some more knowledge about the source tree into some data structures. Typically, what catcache to use for a given object's class-id. The patches are not in git am format nor in patch format, so I could only read them, I didn't install them nor compiled the code, didn't run the regression tests. 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] unite recovery.conf and postgresql.conf
Joshua Berkus writes: > include_if_exists certainly solves the recovery.conf/recovery.done problem. > We can even phase it out, like this: > 9.2: include_if_exists = 'recovery.conf' in the default postgresql.conf file. > 9.3: include_if_exists = 'recovery.conf' commented out by default > 9.4: renaming recovery.conf to recovery.done by core PG code removed. > This gives users/vendors 3 years to update their scripts to remove dependence > on recovery.conf. I'm afraid that I agree with Simon that there's already a > whole buncha 3rd-party code out there to support the current system. If there is indeed code out there that depends on the current system, why do you think that waiting several releases to change it will make things better? I think it's more likely that we'd just have *more* code that needs to be changed, and no reduction in the pain level when the transition does finally happen. In any case, I thought we'd agreed that the use of that file as a flag should go away now, so I quite fail to understand your comment about 9.4. 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] unite recovery.conf and postgresql.conf
> I rather like Tom's suggestion of include_if_exists. include_if_exists certainly solves the recovery.conf/recovery.done problem. We can even phase it out, like this: 9.2: include_if_exists = 'recovery.conf' in the default postgresql.conf file. 9.3: include_if_exists = 'recovery.conf' commented out by default 9.4: renaming recovery.conf to recovery.done by core PG code removed. This gives users/vendors 3 years to update their scripts to remove dependence on recovery.conf. I'm afraid that I agree with Simon that there's already a whole buncha 3rd-party code out there to support the current system. --Josh Berkus -- 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] [v9.2] "database" object class of contrib/sepgsql
2011/9/23 Robert Haas : > On Mon, Sep 12, 2011 at 5:45 AM, Kohei KaiGai wrote: >> The attached patch is a portion that we splitted off when we added >> pg_shseclabel system catalog. >> >> It enables the control/sepgsql to assign security label on pg_database >> objects that are utilized as a basis to compute a default security >> label of schema object. > > Committed, although the fact that it didn't compile until I made > schema.c include pg_database.h makes me wonder how thoroughly you > tested this. > Hmm.. As I did usually, I might build the module and run installation script and regression test when I submitted this patch. However, it was fact I submitted a patch with an obvious miss. Sorry, I'll be careful to check the code being tested. -- KaiGai Kohei -- 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] [v9.2] Fix Leaky View Problem
2011/9/24 Robert Haas : > On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai wrote: >> I updated the patches of fix-leaky-view problem, according to the >> previous discussion. >> The "NOLEAKY" option was replaced by "LEAKPROOF" option, and several >> regression >> test cases were added. Rest of stuffs are unchanged. > > You have a leftover reference to NOLEAKY. > Oops, I'll fix it. >> For convenience of reviewer, below is summary of these patches: >> >> The Part-1 implements corresponding SQL syntax stuffs which are >> "security_barrier" >> reloption of views, and "LEAKPROOF" option on creation of functions to be >> stored >> new pg_proc.proleakproof field. > > The way you have this implemented, we just blow away all view options > whenever we do CREATE OR REPLACE VIEW. Is that the behavior we want? > If a security_barrier view gets accidentally turned into a > non-security_barrier view, doesn't that create a security_hole? > > I'm also wondering if the way you're using ResetViewOptions() is the > right way to handle this anyhow. Isn't that going to update pg_class > twice? I guess that's probably harmless from a performance > standpoint, but wouldn't it be better not to? I guess we could define > something like AT_ReplaceRelOptions to handle this case. > IIRC, we had a discussion that the behavior should follow the case when a view is newly defined, even if it would be replaced actually. However, it seems to me consistent way to keep existing setting as long as user does not provide new option explicitly. If so, I think AT_ReplaceRelOptions enables to simplify the code to implement such a behavior. > The documentation in general is not nearly adequate, at least IMHO. > Do you think the description is poor to introduce the behavior changes corresponding to security_barrier option? OK, I'll try to update the documentation. > I'm a bit nervous about storing security_barrier in the RTE. What > happens to stored rules if the security_barrier option gets change > later? > The rte->security_barrier is evaluated when a query referencing security views get expanded. So, rte->security_barrier is not stored to catalog. Even if security_barrier option was changed after PREPARE statement with references to security view, our existing mechanism re-evaluate the query on EXECUTE, thus, it shall be executed as we expected. (As an aside, I didn't know this mechanism and surprised at EXECUTE works correctly, even if VIEW definition was changed after PREPARE.) Thanks, -- KaiGai Kohei -- 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: [PATCH] Caching for stable expressions with constant arguments v3
All, I'd love to see someone evaluate the impact of Marti's patch on JDBC applications which use named prepared statements. Anyone have a benchmark handy? --Josh -- 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] Range Types - symmetric
> > Reminder: BETWEEEN supports the SYMMETRIC keyword, so there is > > a precedent for this. > > And I don't see it as valuable enough to justify changing the > grammar. I agree that we should leave symmetry until 9.3. --Josh Berkus -- 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] Range Types - symmetric
On Sat, 2011-09-24 at 10:49 -0400, Bruce Momjian wrote: > > I'll add that it would also cause a little confusion with inclusivity. > > What if you do: '[5,2)'::int4range? Is that really '[2,5)' or '(2,5]'? > > Reminder: BETWEEEN supports the SYMMETRIC keyword, so there is > a precedent for this. And I don't see it as valuable enough to justify changing the grammar. Also, that still leaves confusion about inclusivity when applied to range types. Regards, Jeff Davis -- 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] index-only scans
On Sun, Aug 14, 2011 at 00:31, Heikki Linnakangas wrote: > That is somewhat compensated by the fact that tuples that are accessed more > often are also more likely to be in cache. Fetching the heap tuple to check > visibility is very cheap when the tuple is in cache. > > I'm not sure how far that compensates it, though. I'm sure there's typically > nevertheless a fairly wide range of pages that have been modified since the > last vacuum, but not in cache anymore. Would it make sense to re-evaluate the visibility bit just before a page gets flushed out from shared buffers? On a system with no long transactions, it seems likely that a dirty page is already all-visible by the time bgwriter (or shared buffers memory pressure) gets around to writing it out. That way we don't have to wait for vacuum to do it and would make your observation hold more often. Regards, Marti -- 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] unite recovery.conf and postgresql.conf
> There might be a use case for a separate directive include_if_exists, > or some such name. But I think the user should have to tell us very > clearly that it's okay for the file to not be found. Better to go back to include_directory, then. --Josh Berkus -- 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] unite recovery.conf and postgresql.conf
Joshua Berkus writes: > What happens currently if we have an \include in postgresql.conf for a file > which doesn't exist? Is it ignored, or do we error out? It's an error, and I think changing that would be a really bad idea. > If it could just be ignored, maybe with a note in the logs, then we could be > a lot more flexible. There might be a use case for a separate directive include_if_exists, or some such name. But I think the user should have to tell us very clearly that it's okay for the file to not be found. 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] unite recovery.conf and postgresql.conf
Folks, What happens currently if we have an \include in postgresql.conf for a file which doesn't exist? Is it ignored, or do we error out? If it could just be ignored, maybe with a note in the logs, then we could be a lot more flexible. --Josh Berkus -- 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] unite recovery.conf and postgresql.conf
Robert Haas writes: > On Sep 24, 2011, at 1:04 PM, Tom Lane wrote: >> I don't exactly buy this argument. If postgresql.conf is hard to >> machine-edit, why is recovery.conf any easier? > Because you generally just write a brand-new file, without worrying > about preserving existing settings. You aren't really editing at all, > just writing. If that's all the requirement is, it's trivial to implement. 1. Write your-random-configuration-settings into recovery.conf (or any other file name you choose ... something named after your tool would be a better idea). 2. Temporarily append "include recovery.conf" to the end of postgresql.conf. Restart server. 3. When done, remove "include recovery.conf" from the end of postgresql.conf. The hard cases involve merging user-supplied and tool-supplied settings, but "let's overwrite recovery.conf in toto" never would have been able to handle such cases either. 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] unite recovery.conf and postgresql.conf
Simon Riggs writes: > On Sat, Sep 24, 2011 at 6:01 PM, Tom Lane wrote: >> Okay, so you do agree that eventually we want to be rid of >> recovery.conf? I think everyone else agrees on that. But if we are >> going to remove recovery.conf eventually, what is the benefit of >> postponing doing so? > My joyous rush into agreeing to removal has since been replaced with > the cold reality that we must support backwards compatibility. > Emphasise "must". [ shrug... ] I do not agree with your conclusion. We have to break some eggs to make this omelet. The reason why we have a mess here is that the recovery.conf mechanism, which was designed with only the one-shot archive-recovery case in mind, has been abused beyond its capacity. If we don't break with past practice we are not going to be able to fix it. And it's not like we don't break configuration file contents in most releases anyway, so I really fail to see why this one has suddenly become sacrosanct. 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] Adding CORRESPONDING to Set Operations
Kerem Kat writes: > On Sat, Sep 24, 2011 at 19:51, Tom Lane wrote: >> Why? CORRESPONDING at a given set-operation level doesn't affect either >> sub-query, so I don't see why you'd need a different representation for >> the sub-queries. > In the planner to construct a subquery out of SetOperationStmt or > RangeTblRef, a new RangeTblRef is needed. > To create a RangeTableRef, parser state is needed and planner assumes > root->parse->rtable be not modified > after generating simple_rte_array. Actually, after looking at the code again, I don't think you need any of that, since there's already a SubqueryScan node being inserted into the plan. You just need to improve generate_setop_tlist so that it can deal with cases where the mapping from subplan targetlist to the setop output columns isn't one-to-one. 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] Inserting heap tuples in bulk in COPY
> Kohei KaiGai wrote: > I'm not clear the reason why the argument of > CheckForSerializableConflictIn() was changed from the one in > heap_insert(). The code was probably just based on heap_insert() before this recent commit: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9d306c66e63eb7f45eab9475b3f96c3134bacac6 > Is it feasible that newly inserted tuples conflict with existing > tuple locks when we expand insert to support multiple tuples at > once? No. -Kevin -- 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] [v9.2] Fix Leaky View Problem
Robert Haas 09/25/11 10:58 AM >>> > I'm not sure we've been 100% consistent about that, since we > previously made CREATE OR REPLACE LANGUAGE not replace the owner > with the current user. I think we've been consistent in *not* changing security on an object when it is replaced. test=# create user someoneelse; CREATE ROLE test=# create user yetanother; CREATE ROLE test=# create function one() returns int language sql as 'select 1;'; CREATE FUNCTION test=# alter function one() owner to someoneelse; ALTER FUNCTION test=# revoke execute on function one() from public; REVOKE test=# create or replace function one() returns int language plpgsql as $$begin return 1; end;$$; CREATE FUNCTION test=# \df+ one() List of functions Schema | Name | Result data type | Argument data types | Type | Volatility |Owner| Language | Source code | Description +--+--+-+++-+--+--+- public | one | integer | | normal | volatile | someoneelse | plpgsql | begin return 1; end; | (1 row) test=# set role yetanother; SET test=> select one(); ERROR: permission denied for function one -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] RES: [GENERAL] Alter column...using failure under 9.0.4
That's it: a check constraint I was not aware of... Thanks! Edson -Mensagem original- De: pgsql-general-ow...@postgresql.org [mailto:pgsql-general-ow...@postgresql.org] Em nome de Tom Lane Enviada em: domingo, 25 de setembro de 2011 13:04 Para: pgsql-gene...@postgresql.org; pgsql-hackers@postgresql.org Assunto: Re: [GENERAL] Alter column...using failure under 9.0.4 =?iso-8859-1?Q?Bj=F6rn_H=E4user?= writes: > Am 25.09.2011 um 17:17 schrieb Edson Carlos Ericksson Richter: >> alter table usuario alter column ativo type smallint using (case when >> ativo then 1 else 0 end); >> ERROR: argument of IS FALSE must be type boolean, not type smallint > you could check for indices or something like that. Yeah, looks like expression index or CHECK constraint or something similar that includes "ativo IS FALSE". Note to hackers: I wonder whether we could make this a bit more user-friendly by providing a CONTEXT line that shows which table property we're trying to convert. regards, tom lane -- Sent via pgsql-general mailing list (pgsql-gene...@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-general -- 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] Alter column...using failure under 9.0.4
=?iso-8859-1?Q?Bj=F6rn_H=E4user?= writes: > Am 25.09.2011 um 17:17 schrieb Edson Carlos Ericksson Richter: >> alter table usuario alter column ativo type smallint using (case when ativo >> then 1 else 0 end); >> ERROR: argument of IS FALSE must be type boolean, not type smallint > you could check for indices or something like that. Yeah, looks like expression index or CHECK constraint or something similar that includes "ativo IS FALSE". Note to hackers: I wonder whether we could make this a bit more user-friendly by providing a CONTEXT line that shows which table property we're trying to convert. 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] Inserting heap tuples in bulk in COPY
On Wed, Sep 14, 2011 at 6:52 AM, Heikki Linnakangas wrote: >> Why do you need new WAL replay routines? Can't you just use the existing >> XLOG_HEAP_NEWPAGE support? >> >> By any large, I think we should be avoiding special-purpose WAL entries >> as much as possible. > > I tried that, but most of the reduction in WAL-size melts away with that. > And if the page you're copying to is not empty, logging the whole page is > even more expensive. You'd need to fall back to retail inserts in that case > which complicates the logic. Where does it go? I understand why it'd be a problem for partially filled pages, but it seems like it ought to be efficient for pages that are initially empty. -- 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] unite recovery.conf and postgresql.conf
On Sat, Sep 24, 2011 at 3:49 PM, Joshua Berkus wrote: >> Since we haven't yet come up with a reasonable way of machine-editing >> postgresql.conf, this seems like a fairly serious objection to >> getting >> rid of recovery.conf. I wonder if there's a way we can work around >> that... > > Well, we *did* actually come up with a reasonable way, but it died under an > avalanche of bikeshedding and > "we-must-do-everything-the-way-we-always-have-done". I refer, of course, to > the "configuration directory" patch, which was a fine solution, and would > indeed take care of the recovery.conf issues as well had we implemented it. > We can *still* implement it, for 9.2. Well, I find that a fairly ugly solution to the problem, but I agree that it is solvable, if we could get a critical mass on any some particular solution. -- 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] [v9.2] Fix Leaky View Problem
On Sat, Sep 24, 2011 at 5:37 PM, Noah Misch wrote: > On Fri, Sep 23, 2011 at 06:25:01PM -0400, Robert Haas wrote: >> On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai wrote: >> > The Part-1 implements corresponding SQL syntax stuffs which are >> > "security_barrier" >> > reloption of views, and "LEAKPROOF" option on creation of functions to be >> > stored >> > new pg_proc.proleakproof field. >> >> The way you have this implemented, we just blow away all view options >> whenever we do CREATE OR REPLACE VIEW. Is that the behavior we want? >> If a security_barrier view gets accidentally turned into a >> non-security_barrier view, doesn't that create a security_hole? > > I think CREATE OR REPLACE needs to keep meaning just that, never becoming > "replace some characteristics, merge others". The consequence is less than > delightful here, but I don't have an idea that avoids this problem without > running afoul of some previously-raised design constraint. Hmm, you might be right, although I'm not sure we've been 100% consistent about that, since we previously made CREATE OR REPLACE LANGUAGE not replace the owner with the current user. -- 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] posix_fadvsise in base backups
Hi Greg, On Sunday, September 25, 2011 03:25:50 AM Greg Stark wrote: > On Sat, Sep 24, 2011 at 4:16 PM, Magnus Hagander wrote: > > I was assuming the kernel was smart enough to read this as "*this* > > process is not going to be using this file anymore", not "nobody in > > the whole machine is going to use this file anymore". And the process > > running the base backup is certainly not going to read it again. > > > > But that's a good point - do you know if that is the case, or does it > > mandate more testing? > It's not the case on Linux. I used to use DONTNEED to flush pages from > cache before running a benchmark. I verified with mincore that the > pages were actually getting removed from cache. Sometimes there was > the occasional straggler but nearly all got flushed and after a second > or third pass the stragglers were gone too. Not sure what exactly is "not the case on Linux". Your answer could be read in a way that the fadvise/DONTNEED adheres to some sort of refcounting scheme (which it afaik does not) or that it doesn't. > In case you're wondering, this was because using /proc/.../drop_caches > caused flaky benchmarks. My theory was that it was causing pages of > the executable to trigger page faults in the middle of the benchmark. That should be easily possible to rule out by preloading the applications+libraries? I think there were plans to teach the dynamic linker to enforce doing so, but I am not sure they were ever folloowed through. Andres -- 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] TABLE tab completion
On 24 September 2011 11:59, Magnus Hagander wrote: > TABLE tab completion in psql only completes to tables, not views. but > the TABLE command works fine for both tables and views (and also > sequences). > > Seems we should just complete it to relations and not tables - or can > anyone see a particular reason why we shouldn't? > Doesn't that mean that "DROP TABLE " would offer up views as well as tables, which would be incorrect? 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
Re: [HACKERS] fix for pg_upgrade
OK, i started once again: I hope the following is the correct way of querying the table corresponding to a relid: -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Inserting heap tuples in bulk in COPY
On 25 September 2011 09:43, Kohei KaiGai wrote: > Hi Heikki, > > I checked your patch, then I have a comment and two questions here. > > 2011/9/14 Heikki Linnakangas : >> >> Attached is a new version of the patch. It is now complete, including WAL >> replay code. Hi, I had a quick look at the patch as well and spotted an oversight: the multi-insert code branch fails to invoke AFTER ROW triggers. 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
Re: [HACKERS] Inserting heap tuples in bulk in COPY
Hi Heikki, I checked your patch, then I have a comment and two questions here. The heap_prepare_insert() seems a duplication of code with earlier half of existing heap_insert(). I think it is a good question to consolidate these portion of the code. I'm not clear the reason why the argument of CheckForSerializableConflictIn() was changed from the one in heap_insert(). Its source code comment describes as: : * For a heap insert, we only need to check for table-level SSI locks. * Our new tuple can't possibly conflict with existing tuple locks, and * heap page locks are only consolidated versions of tuple locks; they do * not lock "gaps" as index page locks do. So we don't need to identify * a buffer before making the call. */ Is it feasible that newly inserted tuples conflict with existing tuple locks when we expand insert to support multiple tuples at once? It is a bit confusing for me. This patch disallow multiple-insertion not only when before-row trigger is configured, but volatile functions are used to compute a default value also. Is it a reasonable condition to avoid multiple-insertion? All the datums to be delivered to heap_form_tuple() is calculated in NextCopyFrom, and default values are also constructed here; sequentially. So, it seems to me we have no worry about volatile functions are not invoked toward each tuples. Or, am I missing something? Thanks, 2011/9/14 Heikki Linnakangas : > On 13.08.2011 17:33, Tom Lane wrote: >> >> Heikki Linnakangas writes: >>> >>> The patch is WIP, mainly because I didn't write the WAL replay routines >>> yet, but please let me know if you see any issues. >> >> Why do you need new WAL replay routines? Can't you just use the existing >> XLOG_HEAP_NEWPAGE support? >> >> By any large, I think we should be avoiding special-purpose WAL entries >> as much as possible. > > I tried that, but most of the reduction in WAL-size melts away with that. > And if the page you're copying to is not empty, logging the whole page is > even more expensive. You'd need to fall back to retail inserts in that case > which complicates the logic. > >> Also, I strongly object to turning regular heap_insert into a wrapper >> around some other more complicated operation. That will likely have >> bad consequences for the performance of every other operation. > > Ok. I doubt it makes any noticeable difference for performance, but having > done that, it's more readable, too, to duplicate the code. > > Attached is a new version of the patch. It is now complete, including WAL > replay code. > > -- > 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 > > -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers