Re: [HACKERS] GSoC project : K-medoids clustering in Madlib
I'm a bit confused as to why this is being proposed as a Postgres-related project. I don't even know what MADlib is, but I'm pretty darn sure that no part of Postgres uses it. KNNGist certainly doesn't. It's a reasonably well established extension for Postgres for statistical and machine learning methods. Rather neat, but as you indicate, it's not part of Postgres proper. http://madlib.net/ https://github.com/madlib/madlib/ It is the extension that is normally referred to when we talk about data analytics in Postgres. As you said, it is not part of postgres proper,but IMO, if we want to extend the data analytics functionalities of postgres, we need to work on MADlib. Regards, Atri -- Regards, Atri l'apprenant -- 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] [COMMITTERS] pgsql: Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.
On 26.03.2013 22:14, Kevin Grittner wrote: Alvaro Herreraalvhe...@2ndquadrant.com wrote: Not happy with misc.c as a filename. We already have two misc.c files: src/backend/utils/adt/misc.c src/interfaces/ecpg/ecpglib/misc.c I much prefer not to repeat the same filename in different directories if we can avoid it. How about pg_dump_utils.c or pg_dump_misc.c? Those seem reasonable to me. pg_dump_utils.c could be confused with dumputils.c. And I'd rather not have pg_dump in the filename, because the file is for functions that are used in both pg_dump and pg_restore. To add to the confusion, there's also common.c, which is only used by pg_dump (historical reasons). There's a bunch of files called pg_backup_*.c that are also shared between pg_dump and pg_restore, so perhaps pg_backup_utils.c? - Heikki -- 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] GSoC project : K-medoids clustering in Madlib
On 27.03.2013 08:51, Atri Sharma wrote: I'm a bit confused as to why this is being proposed as a Postgres-related project. I don't even know what MADlib is, but I'm pretty darn sure that no part of Postgres uses it. KNNGist certainly doesn't. It's a reasonably well established extension for Postgres for statistical and machine learning methods. Rather neat, but as you indicate, it's not part of Postgres proper. http://madlib.net/ https://github.com/madlib/madlib/ It is the extension that is normally referred to when we talk about data analytics in Postgres. As you said, it is not part of postgres proper,but IMO, if we want to extend the data analytics functionalities of postgres, we need to work on MADlib. Perhaps we could do this under the PostgreSQL organization, but we'd definitely need to get someone from the MADLib project to mentor it. But it would be even better if MADLib would apply to GSoC as an independent organization. The deadline for organization applications is on March 29th, so if the MADLIb people are interested in that, they need to hurry and send the application right now. - Heikki -- 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] Ideas for improving Concurrency Tests
On Tuesday, March 26, 2013 9:49 PM Greg Stark wrote: On Tue, Mar 26, 2013 at 7:31 AM, Amit Kapila amit.kap...@huawei.com wrote: Above ideas could be useful to improve concurrency testing and can also be helpful to generate test cases for some of the complicated bugs for which there is no direct test. I wonder how much explicit sync points would help with testing though. It seems like they suffer from the problem that you'll only put sync points where you actually expect problems and not where you don't expect them -- which is exactly where problems are likely to occur. We can do it for different kind of operations. For example: 1. All the operations which are done in Phase: a. Create Index Concurrently - Some time back, I was going through the design of Create Index Concurrently and I found a problem which I reported in mail below: http://www.postgresql.org/message-id/006801cdb72e$96b62330$c4226990$@kapila@ huawei.com It occurs because we change design/implementation for RelationGetIndexList() to address Drop Index Concurrently. Such issues are sometimes difficult to catch through normal tests. However if we have defined sync points for each phase and its dependent operations, it would be comparatively easier to catch if any change occurs. It could have been caught if we could define sync points for step-3 and step-4 as mentioned in mail. b. Alter Table - In this also we do the operation in 3 phases, so we can define sync points between each phase and its dependent ops. 2. Some time back, one defect is fixed for concurrency between insert cleaning the btree page and vacuum, Commit log: http://www.postgresql.org/message-id/e1rzvx1-0005nb...@gemulon.postgresql.or g Even if such synchronization points are difficult to think ahead, we can protect their breakage later on by some other change by having test case for them. Such tests would also need sync points. Wouldn't it be more useful to implicitly create sync points whenever synchronization events like spinlocks being taken occur? It will be really useful, but how in such cases will we make sure from test case that what action (WAIT, SIGNAL or IGNORE) to take on sync point. For example S-1 Insert into tbl values(1); S-2 Select * from tbl; If both S-1,S-2 run parallel, it could be difficult to say weather '1' will be visible to S-2. However if S-2 waits for signal in GetSnapshotData() before taking ProcArrayLock, and S-1 sets the signal after release of ProcArrayLock in function ProcArrayEndTransaction, S-2 can expect to see value '1'. For above test, how will we make sure that only S-2 should wait in GetSnapshotData not S-2? Could you elaborate bit more, may be I am not getting your point completely? And likewise explicitly listing the timing sequences to test seems unconvincing. If we could arrange for two threads to execute every possible interleaving of code by exhaustively trying every combination that would be far more convincing. I think for this part, the main point is how from test, we can synchronize each interleaving part of code. Any ideas how this can be realized? Most bugs are likely to hang out in combinations we don't see in practice -- for instance having a tuple deleted and a new one inserted in the same slot in the time a different transaction was context switched out. With Regards, Amit Kapila. -- 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] GSoC project : K-medoids clustering in Madlib
But it would be even better if MADLib would apply to GSoC as an independent organization. The deadline for organization applications is on March 29th, so if the MADLIb people are interested in that, they need to hurry and send the application right now. Agreed. Is there any way we could add in house support for basic data analytics,maybe as a proper postgres extension? Regards, Atri -- Regards, Atri l'apprenant -- 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 support for zero-attribute unique/etc keys
Darren Duncan wrote: Consider the context however. We're talking about a UNIQUE constraint and so what we want to do is prevent the existence of multiple tuples in a relation that are the same for some defined subset of their attributes. I would argue that logically, and commonsensically, two tuples with no attributes are the same, and hence a set of distinct tuples having zero attributes could have no more than one member, and so a UNIQUE constraint over zero attributes would say the relation can't have more than one tuple. So unless someone wants to argue that two tuples with no attributes are not the same, my interpretation makes more sense and is clearly the one to follow. -- Darren Duncan What you propose is not an interpretation, but an extension of the standard. I'm not certain about that clearly either; intuition is a questionable guideline when it comes to the standard: SELECT 'Things that feel equal are ' || CASE WHEN NULL = NULL THEN '' ELSE 'not always ' END || 'equal'; ?column? - Things that feel equal are not always equal (1 row) Yours, Laurenz Albe -- 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] GSoC project : K-medoids clustering in Madlib
On 27 March 2013 08:12, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 08:51, Atri Sharma wrote: I'm a bit confused as to why this is being proposed as a Postgres-related project. I don't even know what MADlib is, but I'm pretty darn sure that no part of Postgres uses it. KNNGist certainly doesn't. It's a reasonably well established extension for Postgres for statistical and machine learning methods. Rather neat, but as you indicate, it's not part of Postgres proper. http://madlib.net/ https://github.com/madlib/madlib/ It is the extension that is normally referred to when we talk about data analytics in Postgres. As you said, it is not part of postgres proper,but IMO, if we want to extend the data analytics functionalities of postgres, we need to work on MADlib. Perhaps we could do this under the PostgreSQL organization, but we'd definitely need to get someone from the MADLib project to mentor it. But it would be even better if MADLib would apply to GSoC as an independent organization. The deadline for organization applications is on March 29th, so if the MADLIb people are interested in that, they need to hurry and send the application right now. It would also help if they were able to get in contact so that I could add them as a project we'd vouch for as part of our own application. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Proof of concept: using genetic algorithm to come up with most optimal PostgreSQL.conf
(Resending, I think google mail failed delivering it first time). Hi folks, I've always been fascinated with genetic algorithms. Having had a chance to implement it once before, to solve real life issue - I knew they can be brilliant at searching for right solutions in multi dimensional space. Thinking about just the postgresql.conf and number of possible options there to satisfy performance needs - I thought, this does sound like a good example of problem that can be solved using genetic algorithm. So I sat down after work for few days, and came up with a simple proof of concept. It generates random population of postgresql configuration files, and runs simple pgbench test on each one of them. It takes the average TPS for 3 consecutive runs as the score that then is applied to each 'guy'. Then I run a typical - I suppose - cross over operation and slight mutation of each new chromosome - to come up with new population, and so on and so forth. Running this for 2 days - I came up to conclusion that it does indeed seem to work, although default pgbench 'test cases' are not really stressing the database enough for it to generate diverse enough populations each time. Also, ideally this sort of thing should be run on two or more different hosts. One (master) that just generates new configurations, saves, restores, manages the whole operation - and 'slave' host(s) that run the actual tests. One benefit of that would be the fact that genetic algorithms are highly parallelizable. I did reboot my machines after tests couple times, to test configuration files and to see if the results were in fact repeatable (as much as they can be) - and I have to say, to my surprise - they were. I.e. the configuration files with poor results were still obviously slower then the best ones. I did include my sample results for everyone to see, including nice spreadsheet with graphs (everyone loves graphs) showing the scores across all populations. The tests were ran on my mac laptops (don’t have access to bunch of servers that I can test things like that on for couple days, sorry). The project, including readme file is available to look at:https://github.com/waniek/genpostgresql.conf Things I know so far: * I probably need to take into account more configuration options; * pgbench with its default test case is not the right characterization suite for this exercise, I need something more life like. I suppose we all have some sort of a characterization suite that could be used here; * Code needs a lot work on it, if this was to be used professionally; * Just restarting postgresql with different configuration file doesn't really constitute fully proper way to test new configuration files, but it seem to work; I don't expect much out of this - after all this is just a proof of concept. But if there are people out there thinking this can be in any way useful - please give us a shout. Also, if you know something more about genetic algorithms then I do - and can suggest improvement - let me know. Lastly, I'm looking for some more sophisticated pgbench test cases that I could throw in at it. I think in general pgbench as a project could use some more sophisticated benchmarks that should be included with the project, for everyone to see. Perhaps even to run some automated regression tests against git head. -- 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] Review of Row Level Security
2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] Review of Row Level Security
I moved Row Level Security patch to the 2013-Next commitfest. 2013/3/27 Kohei KaiGai kai...@kaigai.gr.jp: 2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] Review of Row Level Security
On 27 March 2013 10:57, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/3/25 Simon Riggs si...@2ndquadrant.com: On 19 March 2013 15:08, Kohei KaiGai kai...@kaigai.gr.jp wrote: It is much helpful if someone give me comments around these arguable portions from the standpoint with fresh eyes. My feeling at this point is that I don't think I should commit this and related patches without more work. I certainly have time and willingness to do that in the next release cycle, but I feel like we need substantially longer to confirm that this is as rock solid as it needs to be. With everybody's permission, I'd like to move this to the next CF, with apologies to KaiGai. I have to admit it will become time to move v9.4 development cycle soon, and row-level security patch still needs some more works to merge into the core. At least, it does not stand on 40km point at marathon. One thing we need to consider is, the current version of RLS patch has cut-down functionality about writer side on INSERT / UPDATE commands. So, we anyway needed to work on this feature on v9.4 development cycle. So, I can agree to move this patch to the v9.4 development cycle. Also, I'd like to have discussion for this feature in earlier half of v9.4 to keep time for the remaining features, such as check on writer-side, integration with selinux, and so on. Thanks for your hard work, and understanding. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Feature Request: pg_replication_master()
On 24 December 2012 17:15, Andres Freund and...@2ndquadrant.com wrote: On Mon, Dec 24, 2012 at 03:13:59PM +, Simon Riggs wrote: From all of the above, I think its worth doing this * allowing recovery.conf to be in a different directory * make recovery config parameters into GUCs * no other changes to way things currently work I happen to like this proposal, +1 from me. It also seems to be a good building block to do work on the other things asked for in this and related threads. As discussed previously, I've committed a patch for * allowing recovery.conf to be in a different directory and am now starting work on * make recovery config parameters into GUCs using Fujii's patch as a start These are separate thoughts and hence separate patches. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? - Heikki -- 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.3] OAT_POST_ALTER object access hooks
On Tue, Mar 19, 2013 at 9:28 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: But I've left it the way you have it for now. Part of me thinks that what you really want here is a pre-alter hook rather than a post-alter hook... Yes. It is the reason why I wanted to put a pre-alter hook for this code path exceptionally. The attached patch is rebased version of contrib/sepgsql patch. Even though I added nothing from the previous one, it might make sense to add checks for the cases when user altered external properties of tables (such as triggers, attribute-default, ...). How about your opinion? It is middle of March now. It might make sense to rework this further in a future release, but I don't think we can do that now. Anyhow, I've committed the rest of this patch. Sorry that took so long. -- 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] [sepgsql 1/3] add name qualified creation label
On Fri, Jan 25, 2013 at 10:29 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I asked folks of Debian-JP how and when does package maintainer pushes new versions. Usually, new versions shall be pushed to unstable branch, then testing and stable. But it is now feature freeze period thus it is prohibited to push new features to unstable. Thus, newer libselinux (2.1.12) is now in experimental branch, but not in unstable branch. He also said, the newer libselinux will likely moved to unstable when feature freeze is unlocked soon. The pgsql-v9.3 shall be released several months later, so it also shall be pushed to unstable branch several months later at least. It does not make problems. Due to same reason, RHEL7 does not make a problem even if it ships with pgsql-9.3, because the latest libselinux already support 2.1.10 feature. Thus, required libselinux version should be sufficient when pgsql-9.3 become available on Fedora. Based on KaiGai's analysis, it seems to me that there is no serious problem here in terms of versioning, and as this patch represents a small but useful step forward in our support for SELinux integration, I'd like to go ahead and push it. Are there serious objections to that course of action? -- 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] [PATCH] avoid buffer underflow in errfinish()
On Sat, Mar 23, 2013 at 6:38 PM, Xi Wang xi.w...@gmail.com wrote: CHECK_STACK_DEPTH checks if errordata_stack_depth is negative. Move the dereference of errordata[errordata_stack_depth] after the check to avoid out-of-bounds read. This seems sensible and I'm inclined to commit it. It's unlikely to matter very much in practice, since the only point of checking the stack depth in the first place is to catch a seemingly-unlikely coding error; and it's unlikely that referencing beyond the stack bounds would do anything too horrible, either. But we may as well do it right. -- 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] [PATCH] avoid buffer underflow in errfinish()
On Sat, Mar 23, 2013 at 6:45 PM, Xi Wang xi.w...@gmail.com wrote:
A side question: at src/backend/storage/lmgr/proc.c:1150, is there a
null pointer deference for `autovac'?
Not really. If the deadlock_state is DS_BLOCKED_BY_AUTOVACUUM, there
has to be a blocking autovacuum proc. As in the other case that you
found, though, some code rearrangement would likely make the intent of
the code more clear and avoid future mistakes.
Perhaps something like:
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel
(autovac = GetBlockingAutoVacuumPgproc()) != NULL)
{
PGXACT *autovac_pgxact =
ProcGlobal-allPgXact[autovac-pgprocno];
...
--
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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On Wed, Mar 27, 2013 at 8:44 AM, Thom Brown t...@linux.com wrote: On 27 March 2013 12:33, Robert Haas rh...@postgresql.org wrote: sepgsql: Support for new post-ALTER access hook. I notice that due to commit bc5334d8 I can't actually build the docs at the moment. But I think the language here definitely needs improving: On CREATE FUNCTION, install permission will be checked if leakproof attribute was given, not only create on the new function. This permission will be also checked when user tries to turn on leakproof attribute using ALTER FUNCTION command, with setattr permission on the function being altered. What do you suggest? I thought about changing the wording but the new wording is parallel to what's already in that paragraph, so likely the whole thing needs to be rewritten if we change any of it. That seemed beyond the scope of this commit, but I'm happy to have us do it. And are the literals there capitalised when rendered? If not, could I suggest they be capitalised in the SGML? AFAIK, there's nothing that would change capitalization automatically in our doc toolchain. Possibly LEAKPROOF should be capitalized but the rest look right. setattr, etc. should not be capitalized, at least according to my limited understanding of how SELinux capitalization conventions work. s/for each object types/for each object type/ In the interest of avoiding a proliferation of tiny commits, I'll hold off on fixing this until we figure out what to do about the rest of it. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 9:12 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? At first glance, I am not sure this goes in the right direction. Why is it necessary to add a GUC parameter for that? In the patch I sent based on Masao's first version, if we merge of postgresql.conf and recovery.conf, users will be encouraged to migrate to the new system by including recovery.conf or a file containing recovery parameters using include_path or include_if_exists, so you shouldn't need a new parameter to include recovery.conf. I have the feeling that this complicates even more the settings. Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. -- Michael
Re: [HACKERS] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 9:59 PM, Michael Paquier michael.paqu...@gmail.comwrote: On Wed, Mar 27, 2013 at 9:12 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? At first glance, I am not sure this goes in the right direction. Why is it necessary to add a GUC parameter for that? In the patch I sent based on Masao's first version, if we merge of postgresql.conf and recovery.conf, users will be encouraged to migrate to the new system by including recovery.conf or a file containing recovery parameters using include_path or include_if_exists, so you shouldn't need a new parameter to include recovery.conf. I have the feeling that this complicates even more the settings. Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. Please note also that based on the documentation include* params can used absolute paths: http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES -- Michael
Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()
On 27.03.2013 14:50, Robert Haas wrote:
On Sat, Mar 23, 2013 at 6:45 PM, Xi Wangxi.w...@gmail.com wrote:
A side question: at src/backend/storage/lmgr/proc.c:1150, is there a
null pointer deference for `autovac'?
I think you mean on line 1142:
PGPROC *autovac = GetBlockingAutoVacuumPgproc();
*HERE* PGXACT *autovac_pgxact = ProcGlobal-allPgXact[autovac-pgprocno];
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
/*
* Only do it if the worker is not working to protect against
Xid
* wraparound.
*/
if ((autovac != NULL)
(autovac_pgxact-vacuumFlags PROC_IS_AUTOVACUUM)
!(autovac_pgxact-vacuumFlags
PROC_VACUUM_FOR_WRAPAROUND))
Not really. If the deadlock_state is DS_BLOCKED_BY_AUTOVACUUM, there
has to be a blocking autovacuum proc. As in the other case that you
found, though, some code rearrangement would likely make the intent of
the code more clear and avoid future mistakes.
Perhaps something like:
if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM
allow_autovacuum_cancel
(autovac = GetBlockingAutoVacuumPgproc()) != NULL)
{
PGXACT *autovac_pgxact =
ProcGlobal-allPgXact[autovac-pgprocno];
...
Writing it like that suggests that autovac might sometimes be NULL, even
if deadlock_state == DS_BLOCKED_BY_AUTOVACUUM. From your explanation
above, I gather that's not possible (and I think you're right), so the
NULL check is unnecessary. If we think it might be NULL after all, the
above makes sense.
- Heikki
--
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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 12:12, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? Why so? I made a clear proposal on how to proceed and this was the first part of it. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On 27 March 2013 12:58, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 27, 2013 at 8:44 AM, Thom Brown t...@linux.com wrote: On 27 March 2013 12:33, Robert Haas rh...@postgresql.org wrote: sepgsql: Support for new post-ALTER access hook. I notice that due to commit bc5334d8 I can't actually build the docs at the moment. But I think the language here definitely needs improving: On CREATE FUNCTION, install permission will be checked if leakproof attribute was given, not only create on the new function. This permission will be also checked when user tries to turn on leakproof attribute using ALTER FUNCTION command, with setattr permission on the function being altered. What do you suggest? I thought about changing the wording but the new wording is parallel to what's already in that paragraph, so likely the whole thing needs to be rewritten if we change any of it. That seemed beyond the scope of this commit, but I'm happy to have us do it. Perhaps something along the lines of: When a CREATE FUNCTION command is executed, the install permission will be checked to determine whether the LEAKPROOF attribute was present. This permission will also be checked when the user tries to apply the LEAKPROOF attribute using the ALTER FUNCTION command. I'm not sure what the last part is actually describing (with setattr permission on the function being altered.), so I'm not sure how that should be read. It doesn't help that I'm not familiar with SELinux terms. And are the literals there capitalised when rendered? If not, could I suggest they be capitalised in the SGML? AFAIK, there's nothing that would change capitalization automatically in our doc toolchain. Possibly LEAKPROOF should be capitalized but the rest look right. setattr, etc. should not be capitalized, at least according to my limited understanding of how SELinux capitalization conventions work. I was really just thinking of CREATE and LEAKPROOF, but I'm not sure CREATE should be in there anyway. -- Thom -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 12:59, Michael Paquier michael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. We can have the whole debate again, if you wish. There is no reason to break backwards compatibility to get what we want. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Default connection parameters for postgres_fdw and dblink
On Mon, Mar 25, 2013 at 4:38 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: I agree that this is an unhappy situation. If possible, I would suggest the following defaults (that's what I as a user would expect without thinking too hard): 1) Default the user to the current effective database user. 2) Default the port to 5432. 3) Default the database name to the current database name. +1. Speaking about exposing the server environment, I have been bitten before by the fact that the default client encoding is taken from the server's PGCLIENTENCODING at postmaster start time, but that's slightly off topic. Yeah. I really hate environment variables as a way of setting defaults for things, because they tend to start creeping into unfortunate places. It's not so bad to have one or two, but when you start to have PGDATA, PGPORT, PGUSER, PGSERVICE, PGSERVICEFILE, PGSSLMODE, PGCONNECT_TIMEOUT, PGHOST, PGHOSTADDR, PGREALM, PGOPTIONS, PGAPPNAME, PGREQUIRESSL, PGSSLCOMPRESSION, PGSYSCONFDIR, PGLOCALEDIR, PGSSLROOTCERT, PGSSLCERT, PGGEQO, PGTZ, PGDATESTYLE, PGCLIENTENCODING, PGKRBSRVNAME, PGGSSLIB, PGPASSFILE, OLDDATADIR, NEWDATADIR, OLDBINDIR, NEWBINDIR, and probably a few others I'm missing, it becomes very difficult to sanitize an environment (such as the postmaster) against undesirable intrusions. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 12:59, Michael Paquier michael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. I assume that you are referring to this message: http://www.postgresql.org/message-id/CA+U5nMK8n=sq-xpvbvtics3nbvobjuvm5xbr+faeqn-rjjg...@mail.gmail.com I don't see a followup from anyone clearly agreeing that this was a useful thing to do. There is a lot of support for turning recovery.conf parameters into GUCs. But I don't remember anyone supporting this idea, and like Heikki and Michael, I don't understand how it moves the ball forward. Considering there's been no discussion of this particular change in three months, and not a whole lot back then, I think it would have been polite to post the patch and ask for comments before committing it. -- 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: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 26 March 2013 20:44, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
Well, you could easily change array_ndims() to error out if ARR_NDIM()
is negative or more than MAXDIM and return NULL only if it's exactly
0. That wouldn't break backward compatibility because it would throw
an error only if fed a value that shouldn't ever exist in the first
place, short of a corrupted database. I imagine the other functions
are amenable to similar treatment.
I haven't looked at the patch in detail, but I thought one of the key
changes was that '{}' would now be interpreted as a zero-length 1-D
array rather than a zero-D array. If we do that it seems a bit moot
to argue about whether we should exactly preserve backwards-compatible
behavior in array_ndims(), because the input it's looking at won't be
the same anymore anyway.
The patch is also allowing '{{},{},{}}' which is described up-thread
as a 2-D empty array. That's pretty misleading, since it has length 3
(in the first dimension). '{{},{}}' and '{{}}' are both more empty,
but neither is completely empty. It feels as though, if you were going
down that road, then for completeness you'd need a new syntax for a
truly empty 2-D array, e.g., '{}^2', but I can't really think of a
use-case for that, or for any of the others for that matter.
You'd be making it possible to have multiple different 2-D arrays, all
empty in the sense of having no elements, but which would compare
differently. Pretty much all you would be able to do with such arrays
would be to append additional empty arrays.
In any case, the entire point of this proposal is that the current
behavior around zero-D arrays is *broken* and we don't want to be
backwards-compatible with it anymore. So if you wish to argue against
that opinion, do so; but it seems a bit beside the point to simply
complain that backwards compatibility is being lost.
I'm not saying that the current situation is not broken. I'm just
questioning whether the fix is actually any less confusing than what
we have now.
+1 for adding an array_is_empty() function though
--- I think there is enough evidence just in this thread that the
current API is confusing. We don't currently provide a definitive way
to test for empty arrays, so people inevitably invent their own (all
subtly different) solutions.
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] [COMMITTERS] pgsql: Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.
Heikki Linnakangas hlinnakan...@vmware.com writes: Alvaro Herreraalvhe...@2ndquadrant.com wrote: Not happy with misc.c as a filename. There's a bunch of files called pg_backup_*.c that are also shared between pg_dump and pg_restore, so perhaps pg_backup_utils.c? Works for me. There's inherently not going to be a lot of content in this filename, so we aren't going to get anything particularly memorable (though I agree with the duplicativeness argument against misc.c). 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 13:21, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 12:59, Michael Paquier michael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. I assume that you are referring to this message: http://www.postgresql.org/message-id/CA+U5nMK8n=sq-xpvbvtics3nbvobjuvm5xbr+faeqn-rjjg...@mail.gmail.com I don't see a followup from anyone clearly agreeing that this was a useful thing to do. Please look again. There is a lot of support for turning recovery.conf parameters into GUCs. Who is against it? I am not. Even, I am working on it now, as already said in at least 3 different places. But I don't remember anyone supporting this idea, and like Heikki and Michael, I don't understand how it moves the ball forward. Considering there's been no discussion of this particular change in three months, and not a whole lot back then, I think it would have been polite to post the patch and ask for comments before committing it. Given various confusions and multiple patches, posting another wouldn't help much. In terms of politeness, I certainly mean no rudeness, only to move forward as agreed. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 15:11, Simon Riggs wrote: On 27 March 2013 12:59, Michael Paquiermichael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. Well, there wasn't much discussion about it back then. The way I read the thread is that people agreed with the general approach, as now implemented in Michael's patch, based on Fujii's earlier patch. This might be a good idea or not, but it's a new and separate feature, not related to whatever else we might do with recovery.conf. If we are to discuss the merits of this patch now, a few thoughts: 1. This is going to make life more complicated for tools that want to mess with recovery.conf, as it's no longer guaranteed to be in $PGDATA. 2. An admin can no longer tell if a server is in standby or PITR mode just by checking for $PGDATA/recovery.conf 3. Would it make sense to make the option recovery_config_file, pointing to the file, instead of just the directory? 4. Could you achieve the same with a symlink in $PGDATA? We can have the whole debate again, if you wish. There is no reason to break backwards compatibility to get what we want. AFAICS this is completely orthogonal to backwards-compatibility and other aspects of the upcoming patch to merge recovery.conf and postgresql.conf. - Heikki -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 9:40 AM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 13:21, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 12:59, Michael Paquier michael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. I assume that you are referring to this message: http://www.postgresql.org/message-id/CA+U5nMK8n=sq-xpvbvtics3nbvobjuvm5xbr+faeqn-rjjg...@mail.gmail.com I don't see a followup from anyone clearly agreeing that this was a useful thing to do. Please look again. I have a better idea: how about if you tell me where you see any such message? Because I think the reason I don't see it is because it doesn't exist. It's not my job to go back and scour the archives for evidence that there is some consensus around this commit. It's your job to provide some evidence that such a consensus exists. Or else revert the commit, because so far no one but you seems to believe that this was a good idea. The fact that nobody specifically objected to one line in an email message you posted three months ago does not constitute grounds to go change it without so much as posting the patch. Or at least I can't imagine that any other committer would take it that way. Even Tom posts his patches before committing them, unless there's been specific and recent discussion of the topic. What gives you the right to do otherwise? -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 13:53, Robert Haas robertmh...@gmail.com wrote: Please look again. I have a better idea: how about if you tell me where you see any such message? Because I think the reason I don't see it is because it doesn't exist. http://www.postgresql.org/message-id/20130109204225.gb21...@momjian.us http://www.postgresql.org/message-id/20121224171529.gb19...@alap2.fritz.box -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Enabling Checksums
On Mon, Mar 18, 2013 at 4:31 PM, Greg Smith g...@2ndquadrant.com wrote: to get them going again. If the install had checksums, I could have figured out which blocks were damaged and manually fixed them, basically go on a hunt for torn pages and the last known good copy via full-page write. Wow. How would you extract such a block image from WAL? That would be a great tool to have, but I didn't know there was any practical way of doing it today. -- 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] Enabling Checksums
On 2013-03-27 10:06:19 -0400, Robert Haas wrote: On Mon, Mar 18, 2013 at 4:31 PM, Greg Smith g...@2ndquadrant.com wrote: to get them going again. If the install had checksums, I could have figured out which blocks were damaged and manually fixed them, basically go on a hunt for torn pages and the last known good copy via full-page write. Wow. How would you extract such a block image from WAL? That would be a great tool to have, but I didn't know there was any practical way of doing it today. Given pg_xlogdump that should be doable with 5min of hacking in 9.3. Just add some hunk to write out the page to the if (config-bkp_details) hunk in pg_xlogdump.c:XLogDumpDisplayRecord. I have done that for some debugging already. If somebody comes up with a sensible simple UI for this I am willing to propose a patch adding it to pg_xlogdump. One would have to specify the rel/file/node, the offset, and the target file. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 10:06 AM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 13:53, Robert Haas robertmh...@gmail.com wrote: Please look again. I have a better idea: how about if you tell me where you see any such message? Because I think the reason I don't see it is because it doesn't exist. http://www.postgresql.org/message-id/20130109204225.gb21...@momjian.us http://www.postgresql.org/message-id/20121224171529.gb19...@alap2.fritz.box I don't think the first one can be read as a message of support for this change, but I agree that the second one can. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 10:11 PM, Simon Riggs si...@2ndquadrant.com wrote: On 27 March 2013 12:59, Michael Paquier michael.paqu...@gmail.com wrote: Also, based on Greg's spec (that Robert and I basically agreed on), if recovery.conf is found at the root of data folder an error is returned to user, recommending him to migrate correctly by referring to dedicated documentation. I'm following what was agreed on 24 December. We can have the whole debate again, if you wish. There is no reason to break backwards compatibility to get what we want. OK here is an idea I just got: why not replacing the possibility to define a custom repository for recovery.conf by the possibility to define a custom *file*? Here would be the plan: 1) we move all the recovery parameters to GUC as proposed Masao's patch (and somewhat my patch now). 2) as default, the custom file that is used to trigger recovery is called recovery.conf and needs to be located in data folder. This could be the default value used by this feature. 3) When migrating to the new system, we recommend users to include recovery.conf with include_if_exists. Even better, we add by default an include_if_exists during initdb in postgresql.conf to include recovery.conf. If we do that users don't even need to perform any migration operation. You don't even need to maintain two parsing interfaces for recovery parameters, and you don't even need to think about things like which file has the priority on the other. So happy end. -- Michael
Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)
On 15.03.2013 23:00, Alvaro Herrera wrote:
Dimitri Fontaine wrote:
Please find attached v3 of the Extension Templates patch, with full
pg_dump support thanks to having merged default_full_version, appended
with some regression tests now that it's possible.
Here's a rebased version; there were some merge conflicts with master.
I also fixed some compiler warnings.
I'm quite worried about the security ramifications of this patch. Today,
if you're not sure if a system has e.g sslinfo installed, you can safely
just run CREATE EXTENSION sslinfo. With this patch, that's no longer
true, because foo might not be the extension you're looking for.
Mallory might've done this:
create template for extension sslinfo version '1.0' with (schema public)
as $$ DO EVIL STUFF $$;
Now if you run CREATE EXTENSION sslinfo as superuser, you've been
compromised. This is not only a problem when sitting at a psql console,
it also just became really dangerous to run pg_dump backups without
ensuring that all the extensions are installed beforehand. That's
exactly the situation we wanted to avoid when extensions were introduced
in the first place.
Things get even more complicated if there's version 1.0 of sslinfo
already installed, and you create an extension template for sslinfo
version 1.1. Is that possible? How does it behave?
Below are some random bugs that I bumped into while testing. These could
be fixed, but frankly I think this should be rejected for security reasons.
Documentation doesn't build, multiple errors. In addition to the
reference pages, there should be a section in the main docs about these
templates.
postgres=# create template for extension myextension version '1.0' with () as
'foobar';
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension;
ERROR: syntax error at or near foobar
LINE 1: create extension myextension;
^
Confusing error message.
postgres=# create template for extension myextension version '1.0' with () as
$$create table foobar(i int4) $$;
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension;
CREATE EXTENSION
postgres=# select * from foobar;
ERROR: relation foobar does not exist
LINE 1: select * from foobar;
^
Where did that table go?
postgres=# create template for extension myextension version '1.0' with () as
$$ create function myfunc() returns int4 AS $f$ select 123; $f$ language sql;
$$;
CREATE TEMPLATE FOR EXTENSION
postgres=# create extension myextension version '1.0';
CREATE EXTENSION
postgres=# select * from pg_namespace; nspname | nspowner |
nspacl
+--+---
pg_toast | 10 |
pg_temp_1 | 10 |
pg_toast_temp_1| 10 |
pg_catalog | 10 | {heikki=UC/heikki,=U/heikki}
public | 10 | {heikki=UC/heikki,=UC/heikki}
information_schema | 10 | {heikki=UC/heikki,=U/heikki}
1.0| 10 |
(7 rows)
Ah, here... Where did that 1.0 schema come from?
postgres= create template for extension myextension version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
CREATE TEMPLATE FOR EXTENSION
postgres= create extension myextension version '1.0';ERROR: permission denied
for language internal
postgres= drop template for extension myextension version '1.0';
ERROR: extension with OID 16440 does not exist
Something wrong with catalog caching.
$ make -s install
/usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory
make: *** [install] Error 1
Installing hstore fails.
postgres= create template for extension sslinfo version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
ERROR: extension sslinfo is already available
postgres= create template for extension sslinfo2 version '1.0' with (schema
public) as $$ create function evilfunc() returns int4 AS 'evilfunc' language
internal; $$;
CREATE TEMPLATE FOR EXTENSION
postgres= alter template for extension sslinfo2 rename to sslinfo;
ALTER TEMPLATE FOR EXTENSION
If we check for an existing extension at CREATE, should also check for
that in ALTER ... RENAME TO.
- Heikki
--
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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 10:43 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: 3. Would it make sense to make the option recovery_config_file, pointing to the file, instead of just the directory? +1 on that. I just sent the same suggestion. -- Michael
Re: [HACKERS] Default connection parameters for postgres_fdw and dblink
Robert Haas robertmh...@gmail.com writes: On Mon, Mar 25, 2013 at 4:38 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: If possible, I would suggest the following defaults (that's what I as a user would expect without thinking too hard): 1) Default the user to the current effective database user. 2) Default the port to 5432. 3) Default the database name to the current database name. +1. I think (2) should be default to whatever the configure-selected default port is --- not hard-wired to 5432. I'm also a bit unclear on what the argument is for (3), as opposed to following the default that we use in every other context, namely set dbname equal to username. Yeah. I really hate environment variables as a way of setting defaults for things, because they tend to start creeping into unfortunate places. It's not so bad to have one or two, but when you start to have PGDATA, PGPORT, PGUSER, PGSERVICE, PGSERVICEFILE, PGSSLMODE, PGCONNECT_TIMEOUT, PGHOST, PGHOSTADDR, PGREALM, PGOPTIONS, PGAPPNAME, PGREQUIRESSL, PGSSLCOMPRESSION, PGSYSCONFDIR, PGLOCALEDIR, PGSSLROOTCERT, PGSSLCERT, PGGEQO, PGTZ, PGDATESTYLE, PGCLIENTENCODING, PGKRBSRVNAME, PGGSSLIB, PGPASSFILE, OLDDATADIR, NEWDATADIR, OLDBINDIR, NEWBINDIR, and probably a few others I'm missing, it becomes very difficult to sanitize an environment (such as the postmaster) against undesirable intrusions. It's arguable that we should unsetenv all of these inside the postmaster (once it's absorbed the values from the ones it historically pays attention to), so that the postmaster environment does not impinge on the behavior of libpq inside a server process. This would cause a non-backwards-compatible change in the behavior of dblink, though. Are we okay with that? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 15:09, Simon Riggs wrote: On 27 March 2013 12:12, Heikki Linnakangashlinnakan...@vmware.com wrote: On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? Why so? I made a clear proposal on how to proceed and this was the first part of it. You didn't answer the question. Does this get us any closer to merging postgresql.conf and recovery.conf? Why is this bundled in with that? ISTM the quickest way forward is to revert this, and proceed with the rest of the plan: get Michael/Fujii's patch into shape, and commit it. If we still think this additional GUC is a good idea after that, we can add it afterwards just as well. - Heikki -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 10:15 AM, Michael Paquier michael.paqu...@gmail.com wrote: Here would be the plan: 1) we move all the recovery parameters to GUC as proposed Masao's patch (and somewhat my patch now). 2) as default, the custom file that is used to trigger recovery is called recovery.conf and needs to be located in data folder. This could be the default value used by this feature. 3) When migrating to the new system, we recommend users to include recovery.conf with include_if_exists. Even better, we add by default an include_if_exists during initdb in postgresql.conf to include recovery.conf. I don't think it's a good to call the trigger file recovery.conf. That's just plain confusing. There are also weird edge cases here when the server is promoted. The recovery.conf file won't exist any more, but the GUC settings changes it contains will live on until the next SIGHUP. The proposal Greg Smith floated previously, which I supported and I believe others did as well, was to convert the parameters to GUCs, and error out if recovery.conf existed. That way, anyone who is doing it wrong (for the new release), gets a clear error message. If we were doing that (and I had somehow thought THAT was the consensus), then this patch is moot, because you can't set a location for recovery.conf if that concept doesn't exist any more. You might need a parameter to set the location for the trigger file, perhaps. -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On Wed, Mar 27, 2013 at 10:16 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I'm quite worried about the security ramifications of this patch. Today, if you're not sure if a system has e.g sslinfo installed, you can safely just run CREATE EXTENSION sslinfo. With this patch, that's no longer true, because foo might not be the extension you're looking for. Mallory might've done this: create template for extension sslinfo version '1.0' with (schema public) as $$ DO EVIL STUFF $$; Surely creating an extension template must be a superuser-only operation, in which case this is an issue because Mallory could also have just blown up the world directly if he's already a superuser anyway. If the current patch isn't enforcing that, it's 100% broken. -- 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] in-catalog Extension Scripts and Control parameters (templates?)
On 27.03.2013 16:16, Heikki Linnakangas wrote: Below are some random bugs that I bumped into while testing. These could be fixed, but frankly I think this should be rejected for security reasons. Also: pg_dump does not dump the owner of an extension template correctly. - Heikki -- 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] in-catalog Extension Scripts and Control parameters (templates?)
Robert Haas escribió: On Wed, Mar 27, 2013 at 10:16 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I'm quite worried about the security ramifications of this patch. Today, if you're not sure if a system has e.g sslinfo installed, you can safely just run CREATE EXTENSION sslinfo. With this patch, that's no longer true, because foo might not be the extension you're looking for. Mallory might've done this: create template for extension sslinfo version '1.0' with (schema public) as $$ DO EVIL STUFF $$; Surely creating an extension template must be a superuser-only operation, in which case this is an issue because Mallory could also have just blown up the world directly if he's already a superuser anyway. Yeah .. (except this is NOT an issue) If the current patch isn't enforcing that, it's 100% broken. Even if it's not enforcing that, it's not 100% broken, it only contains one more bug we need to fix. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Default connection parameters for postgres_fdw and dblink
On Wed, Mar 27, 2013 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Mar 25, 2013 at 4:38 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: If possible, I would suggest the following defaults (that's what I as a user would expect without thinking too hard): 1) Default the user to the current effective database user. 2) Default the port to 5432. 3) Default the database name to the current database name. +1. I think (2) should be default to whatever the configure-selected default port is --- not hard-wired to 5432. I'm also a bit unclear on what the argument is for (3), as opposed to following the default that we use in every other context, namely set dbname equal to username. Well, those both seem reasonable alternative proposals. No argument here. Yeah. I really hate environment variables as a way of setting defaults for things, because they tend to start creeping into unfortunate places. It's not so bad to have one or two, but when you start to have PGDATA, PGPORT, PGUSER, PGSERVICE, PGSERVICEFILE, PGSSLMODE, PGCONNECT_TIMEOUT, PGHOST, PGHOSTADDR, PGREALM, PGOPTIONS, PGAPPNAME, PGREQUIRESSL, PGSSLCOMPRESSION, PGSYSCONFDIR, PGLOCALEDIR, PGSSLROOTCERT, PGSSLCERT, PGGEQO, PGTZ, PGDATESTYLE, PGCLIENTENCODING, PGKRBSRVNAME, PGGSSLIB, PGPASSFILE, OLDDATADIR, NEWDATADIR, OLDBINDIR, NEWBINDIR, and probably a few others I'm missing, it becomes very difficult to sanitize an environment (such as the postmaster) against undesirable intrusions. It's arguable that we should unsetenv all of these inside the postmaster (once it's absorbed the values from the ones it historically pays attention to), so that the postmaster environment does not impinge on the behavior of libpq inside a server process. This would cause a non-backwards-compatible change in the behavior of dblink, though. Are we okay with that? I feel like unsetting all of these (or whatever the canonical list is) in the postmaster is a bit like trying to bail out the ocean with a bucket, but since a bucket appears to be the only instrument at hand, sure, why not? As far as breaking backward incompatibility goes, it doesn't strike me as likely that anyone is relying on the current behavior, but on the off chance that they are, do we have some other way for them to set defaults? What I'm worried about with the current behavior is that people will accidentally absorb behavior changes they don't want (or that are insecure). But if there's no other way to set defaults explicitly then you could we'd be ripping out a feature without providing a replacement, something I am loathe to do. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Wed, Mar 27, 2013 at 11:26 PM, Robert Haas robertmh...@gmail.com wrote: There are also weird edge cases here when the server is promoted. The recovery.conf file won't exist any more, but the GUC settings changes it contains will live on until the next SIGHUP. Yes indeed, I forgot that. The proposal Greg Smith floated previously, which I supported and I believe others did as well, was to convert the parameters to GUCs, and error out if recovery.conf existed. That way, anyone who is doing it wrong (for the new release), gets a clear error message. If we were doing that (and I had somehow thought THAT was the consensus), then this patch is moot, because you can't set a location for recovery.conf if that concept doesn't exist any more. You might need a parameter to set the location for the trigger file, perhaps. I am perfectly fine with this plan, especially the part where server errors out if recovery.conf is found. It makes clear to the user that it is not supported anymore. I just tried to find some new fresh ideas that would satisfy everybody... So let's keep up with the plan that Greg proposed and forget what I wrote previously. -- Michael
Re: [HACKERS] Default connection parameters for postgres_fdw and dblink
Robert Haas robertmh...@gmail.com writes: On Wed, Mar 27, 2013 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote: It's arguable that we should unsetenv all of these inside the postmaster (once it's absorbed the values from the ones it historically pays attention to), so that the postmaster environment does not impinge on the behavior of libpq inside a server process. This would cause a non-backwards-compatible change in the behavior of dblink, though. Are we okay with that? I feel like unsetting all of these (or whatever the canonical list is) in the postmaster is a bit like trying to bail out the ocean with a bucket, but since a bucket appears to be the only instrument at hand, sure, why not? As far as breaking backward incompatibility goes, it doesn't strike me as likely that anyone is relying on the current behavior, but on the off chance that they are, do we have some other way for them to set defaults? What I'm worried about with the current behavior is that people will accidentally absorb behavior changes they don't want (or that are insecure). But if there's no other way to set defaults explicitly then you could we'd be ripping out a feature without providing a replacement, something I am loathe to do. Use a service file maybe? But you can't have it both ways: either we like the behavior of libpq absorbing defaults from the postmaster environment, or we don't. You were just arguing this was a bug, and now you're calling it a feature. (I guess we could have a switch to control whether the environment gets cleared, so that anybody who really needs the old behavior could still get it. But ugh.) 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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On Wed, Mar 27, 2013 at 9:09 AM, Thom Brown t...@linux.com wrote:
Perhaps something along the lines of:
When a CREATE FUNCTION command is executed, the install permission
will be checked to determine whether the LEAKPROOF attribute was
present. This permission will also be checked when the user tries to
apply the LEAKPROOF attribute using the ALTER FUNCTION command.
I'm not sure what the last part is actually describing (with setattr
permission on the function being altered.), so I'm not sure how that
should be read. It doesn't help that I'm not familiar with SELinux
terms.
Right, so what it's trying to say is: whenever you modify an object,
we check whether you've got {setattr} permission for that object and
disallow the operation if not. However, for some operations on some
object types, {setattr} is necessary but not sufficient. The
paragraph is recapping, for various cases, which operations require
additional permissions, and what those additional things are.
I was really just thinking of CREATE and LEAKPROOF, but I'm not sure
CREATE should be in there anyway.
create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.
--
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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On 27 March 2013 14:50, Robert Haas robertmh...@gmail.com wrote:
On Wed, Mar 27, 2013 at 9:09 AM, Thom Brown t...@linux.com wrote:
Perhaps something along the lines of:
When a CREATE FUNCTION command is executed, the install permission
will be checked to determine whether the LEAKPROOF attribute was
present. This permission will also be checked when the user tries to
apply the LEAKPROOF attribute using the ALTER FUNCTION command.
I'm not sure what the last part is actually describing (with setattr
permission on the function being altered.), so I'm not sure how that
should be read. It doesn't help that I'm not familiar with SELinux
terms.
Right, so what it's trying to say is: whenever you modify an object,
we check whether you've got {setattr} permission for that object and
disallow the operation if not. However, for some operations on some
object types, {setattr} is necessary but not sufficient. The
paragraph is recapping, for various cases, which operations require
additional permissions, and what those additional things are.
I was really just thinking of CREATE and LEAKPROOF, but I'm not sure
CREATE should be in there anyway.
create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.
My bad.
--
Thom
--
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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On Wed, Mar 27, 2013 at 10:51 AM, Thom Brown t...@linux.com wrote: create here is referring to the sepgsql permission, not the SQL command, so it's correct as-is. My bad. Here's an attempt at reworking the whole section to be more understandable. I took the liberty of rearranging the text into bulleted lists, which I hope is more clear. Comments welcome. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company sepgsql-ddl-doc-fix.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] Default connection parameters for postgres_fdw and dblink
On Wed, Mar 27, 2013 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Mar 27, 2013 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote: It's arguable that we should unsetenv all of these inside the postmaster (once it's absorbed the values from the ones it historically pays attention to), so that the postmaster environment does not impinge on the behavior of libpq inside a server process. This would cause a non-backwards-compatible change in the behavior of dblink, though. Are we okay with that? I feel like unsetting all of these (or whatever the canonical list is) in the postmaster is a bit like trying to bail out the ocean with a bucket, but since a bucket appears to be the only instrument at hand, sure, why not? As far as breaking backward incompatibility goes, it doesn't strike me as likely that anyone is relying on the current behavior, but on the off chance that they are, do we have some other way for them to set defaults? What I'm worried about with the current behavior is that people will accidentally absorb behavior changes they don't want (or that are insecure). But if there's no other way to set defaults explicitly then you could we'd be ripping out a feature without providing a replacement, something I am loathe to do. Use a service file maybe? But you can't have it both ways: either we like the behavior of libpq absorbing defaults from the postmaster environment, or we don't. You were just arguing this was a bug, and now you're calling it a feature. Maybe we could compromise and call it a beature. Or a fug. In all seriousness, it's in that grey area where most people would probably consider this a surprising and unwanted behavior, but there might be a few out there who had a problem and discovered that they could use this trick to solve it. In terms of a different solution, what about a GUC that can contain a connection string which is used to set connection defaults, but which can still be overriden? So it would mimic the semantics of setting an environment variable, but it would be better, because you could do all of the usual GUC things with it instead of having to hack on the postmaster startup environment. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 14:20, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 15:09, Simon Riggs wrote: On 27 March 2013 12:12, Heikki Linnakangashlinnakan...@vmware.com wrote: On 27.03.2013 13:47, Simon Riggs wrote: Allow external recovery_config_directory If required, recovery.conf can now be located outside of the data directory. Server needs read/write permissions on this directory. This was a surprising commit. Does this get us any closer to merging postgresql.conf and recovery.conf? Why so? I made a clear proposal on how to proceed and this was the first part of it. You didn't answer the question. Does this get us any closer to merging postgresql.conf and recovery.conf? Why is this bundled in with that? Why do you think these points are bundled? It clearly isn't and I've not claimed it gets us any closer to that goal. But it is the first part of two agreed changes. And I am now working on the second, which is the recovery.conf GUCs. ISTM the quickest way forward is to revert this, and proceed with the rest of the plan: get Michael/Fujii's patch into shape, and commit it. If we still think this additional GUC is a good idea after that, we can add it afterwards just as well. My review of that patch is on file and my rejection of it clear for all to see. I have proposed a way forwards, which achieved agreement then and I have made it clear all the way that I would work on that, and am now doing so. None of that is a surprise. And Fujii will receive credit for his contribution, which is the bit where we make recovery parms into GUCs. The quickest way forward is to proceed with The Plan as agreed on Dec 24 - Jan 9. Restarting a discussion and formulating an alternative plan is just deliberate interference with no justification, especially not when we pretend that the later plan somehow supercedes the earlier agreed one, and certainly not just because a few months went by in between. In summary, we have clear agreement that a file needs to trigger recovery. We have no reason to believe that renaming the trigger file to something else is a good thing, hence recovery.conf should remain and its contents be treated as GUCs. And recovery.conf now has the option of living in a different directory, which needs to be writable. So we have the new features desired, plus backwards compatibility. And off I go to code now. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.
On 27 March 2013 15:19, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 27, 2013 at 10:51 AM, Thom Brown t...@linux.com wrote: create here is referring to the sepgsql permission, not the SQL command, so it's correct as-is. My bad. Here's an attempt at reworking the whole section to be more understandable. I took the liberty of rearranging the text into bulleted lists, which I hope is more clear. Comments welcome. This looks a lot better, apart from: s/permssions/permissions/ +1 from me. -- Thom -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 17:23, Simon Riggs wrote: On 27 March 2013 14:20, Heikki Linnakangashlinnakan...@vmware.com wrote: You didn't answer the question. Does this get us any closer to merging postgresql.conf and recovery.conf? Why is this bundled in with that? Why do you think these points are bundled? Because you say that this controversial commit is the 1st step before the 2nd step, which is to actually merge postgresql.conf and recovery.conf. It clearly isn't and I've not claimed it gets us any closer to that goal. Ok, cool. Can you please revert this commit so that we can move on, then? But it is the first part of two agreed changes. And I am now working on the second, which is the recovery.conf GUCs. Thanks! ISTM the quickest way forward is to revert this, and proceed with the rest of the plan: get Michael/Fujii's patch into shape, and commit it. If we still think this additional GUC is a good idea after that, we can add it afterwards just as well. My review of that patch is on file and my rejection of it clear for all to see. I have proposed a way forwards, which achieved agreement then and I have made it clear all the way that I would work on that, and am now doing so. None of that is a surprise. And Fujii will receive credit for his contribution, which is the bit where we make recovery parms into GUCs. Oh, ok. I thought the patch in the commitfest implemented what was agreed on, but I admit I haven't looked at it closely. In summary, we have clear agreement that a file needs to trigger recovery. We have no reason to believe that renaming the trigger file to something else is a good thing, hence recovery.conf should remain and its contents be treated as GUCs. I'm fine with that. But at least Robert just said he thinks the trigger file should not be called recovery.conf, based on Greg Smith's earlier proposal. I'm starting to feel that when we seemed to have a consensus around Christmas, some people thought we agreed on one thing, and others thought we agreed on something else. For the record, I'm happy with calling the file recovery.conf, so that it's backwards-compatible. I'm also happy with renaming it, per Greg Smith's/Robert Haas' proposal. And recovery.conf now has the option of living in a different directory, which needs to be writable. So we have the new features desired, plus backwards compatibility. And off I go to code now. Yeah, we need to see the actual patch, so that everyone knows what exactly is being proposed. In any case, it's independent of this commit. - Heikki -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 15:35, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 17:23, Simon Riggs wrote: On 27 March 2013 14:20, Heikki Linnakangashlinnakan...@vmware.com wrote: You didn't answer the question. Does this get us any closer to merging postgresql.conf and recovery.conf? Why is this bundled in with that? Why do you think these points are bundled? Because you say that this controversial commit is the 1st step before the 2nd step, which is to actually merge postgresql.conf and recovery.conf. At no point have I said this commit has anything to do with making recovery.conf into GUCs, in fact, I said exactly that they were separate things, though discussed on the same thread. Requesting a revoke because they are *not* connected doesn't make sense. It clearly isn't and I've not claimed it gets us any closer to that goal. Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. This commit was an agreed upon, uncontroversial feature. What changed? I'm fine with that. But at least Robert just said he thinks the trigger file should not be called recovery.conf, based on Greg Smith's earlier proposal. I'm starting to feel that when we seemed to have a consensus around Christmas, some people thought we agreed on one thing, and others thought we agreed on something else. And recovery.conf now has the option of living in a different directory, which needs to be writable. So we have the new features desired, plus backwards compatibility. And off I go to code now. Yeah, we need to see the actual patch, so that everyone knows what exactly is being proposed. In any case, it's independent of this commit. In the absence of reasons for change we leave things as they are. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] money with 4 digits after dot
I'm trying to insert data into Postgres using COPY command. The data originates from AdventureWorksDW. However, it fails with: ERROR: invalid input syntax for type money: 2171.2942 Is it possible to import money into Postgres with 4 digits after the dot?
Re: [HACKERS] [COMMITTERS] pgsql: Add PF_PRINTF_ATTRIBUTE to on_exit_msg_fmt.
On 27.03.2013 15:28, Tom Lane wrote: Heikki Linnakangashlinnakan...@vmware.com writes: Alvaro Herreraalvhe...@2ndquadrant.com wrote: Not happy with misc.c as a filename. There's a bunch of files called pg_backup_*.c that are also shared between pg_dump and pg_restore, so perhaps pg_backup_utils.c? Works for me. Ok, sold. - Heikki -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 18:10, Simon Riggs wrote: On 27 March 2013 15:35, Heikki Linnakangashlinnakan...@vmware.com wrote: Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. If an admin can't trust that the file is placed in $PGDATA, it's harder to determine if a server is a master or a standby. It makes tools that try to promote / demote a server more complicated, because they need to take this setting into account. Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. From a process standpoint, this is a new feature that should've been submitted before the commitfest deadline. I'm sure we'll make exceptions to that every now and then, but by default new features should be bumped to the next release at this point. - Heikki -- 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] money with 4 digits after dot
Konstantin Izmailov pgf...@gmail.com writes: Is it possible to import money into Postgres with 4 digits after the dot? You would need to set lc_monetary to a locale that permits that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 00:21, Dean Rasheed dean.a.rash...@gmail.com wrote:
The patch is also allowing '{{},{},{}}' which is described up-thread
as a 2-D empty array. That's pretty misleading, since it has length 3
(in the first dimension). '{{},{}}' and '{{}}' are both more empty,
but neither is completely empty.
I'm not saying that the current situation is not broken. I'm just
questioning whether the fix is actually any less confusing than what
we have now.
Well the fix is primarily about 1-D empty arrays, and in that respect
it is much less confusing than what we have now. As for multidim
arrays, I don't use them in the field, so I don't have a strong
opinion about how (or even whether) we should support empty multidim.
I included the '{{}}' syntax following comments from Tom that we
should consider supporting that if we were to get rid of zero-D, but I
don't really have any skin in that game.
Since we require multidim arrays to be regular, perhaps they would
need extents in all dimensions to be practically useful ... if you
wanted to define a blank tic-tac-toe board using a 2-D array, for
example, you'd probably define it as 3x3 with NULL values in each
position, rather than trying to make it empty.
Cheers,
BJ
--
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] [COMMITTERS] pgsql: Allow external recovery_config_directory
Heikki Linnakangas hlinnakan...@vmware.com writes: On 27.03.2013 18:10, Simon Riggs wrote: On 27 March 2013 15:35, Heikki Linnakangashlinnakan...@vmware.com wrote: Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. If an admin can't trust that the file is placed in $PGDATA, it's harder to determine if a server is a master or a standby. It makes tools that try to promote / demote a server more complicated, because they need to take this setting into account. Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. FWIW, I agree that this is a bad idea and should be reverted. Simon is claiming that because he described this idea in one sentence (out of a larger post) three months ago, everyone agreed to the idea and there is no longer any room for discussion. In reality I suspect nobody really thought about the implications at the time. In any case, the arguments that have been made today seem to me to be sufficient reasons why we *don't* want to put recovery.conf in random places outside the data directory. 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 16:24, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 18:10, Simon Riggs wrote: On 27 March 2013 15:35, Heikki Linnakangashlinnakan...@vmware.com wrote: Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. If an admin can't trust that the file is placed in $PGDATA, it's harder to determine if a server is a master or a standby. It makes tools that try to promote / demote a server more complicated, because they need to take this setting into account. Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. AFAIK pg_basebackup doesn't backup files not in the data directory and tablespace dirs. This doesn't change that. If it does, and I am mistaken, then it will be an easy fix. From a process standpoint, this is a new feature that should've been submitted before the commitfest deadline. I'm sure we'll make exceptions to that every now and then, but by default new features should be bumped to the next release at this point. I'm adding it by popular request, agreement and an explicit timing plan, all of which I followed. I didn't add this feature because I want it, and honestly, I don't really care about it myself, which is why I left it to last thing on my work schedule. But I do listen to requests from others, which is why I've spent close to 2 days of my time on it as part of my contribution to the team. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 19:34, Simon Riggs wrote: On 27 March 2013 16:24, Heikki Linnakangashlinnakan...@vmware.com wrote: Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. AFAIK pg_basebackup doesn't backup files not in the data directory and tablespace dirs. Imagine that you have set recovery_config_directory='/foo/' in the master server. Now you want to set up a standby, so you do:' pg_basebackup -D data-standby -R With the -R option, pg_basebackup creates data-standby/recovery.conf to point to the master, with standby_mode='on'. But if you start the server, it will start up as a master, not as a standby, because recovery_config_directory points elsewhere. - Heikki -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 17:23, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: On 27.03.2013 18:10, Simon Riggs wrote: On 27 March 2013 15:35, Heikki Linnakangashlinnakan...@vmware.com wrote: Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. If an admin can't trust that the file is placed in $PGDATA, it's harder to determine if a server is a master or a standby. It makes tools that try to promote / demote a server more complicated, because they need to take this setting into account. Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. FWIW, I agree that this is a bad idea and should be reverted. Simon is claiming that because he described this idea in one sentence (out of a larger post) three months ago, everyone agreed to the idea and there is no longer any room for discussion. In reality I suspect nobody really thought about the implications at the time. In any case, the arguments that have been made today seem to me to be sufficient reasons why we *don't* want to put recovery.conf in random places outside the data directory. If anybody thought one sentence wasn't descriptive enough, they could have said. They didn't because its a trivial patch with very little room for alternative interpretations. Arguments against? I have seen only one, Heikki's above, and its not a good one, given related similar issues. It's an option, you don't have to put recovery.conf anywhere else, unless you wish to. Anyway, as I said, I didn't do this because I want it. I did it because it's been agreed. Without some reasonable objection, I see no reason to revoke. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 17:50, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 19:34, Simon Riggs wrote: On 27 March 2013 16:24, Heikki Linnakangashlinnakan...@vmware.com wrote: Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. AFAIK pg_basebackup doesn't backup files not in the data directory and tablespace dirs. Imagine that you have set recovery_config_directory='/foo/' in the master server. Now you want to set up a standby, so you do:' pg_basebackup -D data-standby -R With the -R option, pg_basebackup creates data-standby/recovery.conf to point to the master, with standby_mode='on'. But if you start the server, it will start up as a master, not as a standby, because recovery_config_directory points elsewhere. Yeh, I get it. Same argument applies to all conf files, not just recovery.conf. Sounds like the patch to add -R to pg_basebackup should be revoked as being not well thought out. Or it should be fixed, in which case this works the same. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Support for REINDEX CONCURRENTLY
On Wed, Mar 27, 2013 at 8:26 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
On Wed, Mar 27, 2013 at 3:05 AM, Fujii Masao masao.fu...@gmail.com wrote:
ISTM you failed to make the patches from your repository.
20130323_1_toastindex_v7.patch contains all the changes of
20130323_2_reindex_concurrently_v25.patch
Oops, sorry I haven't noticed.
Please find correct versions attached (realigned with latest head at the
same time).
Thanks!
- reltoastidxid = rel-rd_rel-reltoastidxid;
+ /* Fetch the list of indexes on toast relation if necessary */
+ if (OidIsValid(reltoastrelid))
+ {
+ Relation toastRel = relation_open(reltoastrelid, lockmode);
+ RelationGetIndexList(toastRel);
+ reltoastidxids = list_copy(toastRel-rd_indexlist);
+ relation_close(toastRel, NoLock);
list_copy() seems not to be required here. We can just set reltoastidxids to
the return list of RelationGetIndexList().
Since we call relation_open() with lockmode, ISTM that we should also call
relation_close() with the same lockmode instead of NoLock. No?
- if (OidIsValid(reltoastidxid))
- ATExecSetTableSpace(reltoastidxid, newTableSpace, lockmode);
+ foreach(lc, reltoastidxids)
+ {
+ Oid idxid = lfirst_oid(lc);
+ if (OidIsValid(idxid))
+ ATExecSetTableSpace(idxid, newTableSpace, lockmode);
Since idxid is the pg_index.indexrelid, ISTM it should never be invalid.
If this is true, the check of OidIsValid(idxid) is not required.
Regards,
--
Fujii Masao
--
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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Thu, Mar 28, 2013 at 1:24 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 27.03.2013 18:10, Simon Riggs wrote: On 27 March 2013 15:35, Heikki Linnakangashlinnakan...@vmware.com wrote: Ok, cool. Can you please revert this commit so that we can move on, then? Please explain why you want this reverted, without mentioning the other task we agree is required. If an admin can't trust that the file is placed in $PGDATA, it's harder to determine if a server is a master or a standby. It makes tools that try to promote / demote a server more complicated, because they need to take this setting into account. Lastly, it breaks the new pg_basebackup -R functionality; pg_basebackup will create the recovery.conf file, but it won't take effect. This patch breaks also pg_ctl promote because it always checks the existence of $PGDATA/recovery.conf. Regards, -- Fujii Masao -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Thu, Mar 28, 2013 at 2:58 AM, Simon Riggs si...@2ndquadrant.com wrote: Arguments against? Also the fact that many discussions have been done on recovery.conf between the time this feature has been decided and actually committed (perhaps too promptly just by looking at how this thread is becoming long)? -- Michael
Re: [HACKERS] spoonbill vs. -HEAD
On 03/26/2013 11:30 PM, Tom Lane wrote: Stefan Kaltenbrunner ste...@kaltenbrunner.cc writes: hmm - will look into that in a bit - but I also just noticed that on the same day spoonbill broke there was also a commit to that file immediately before that code adding the fflush() calls. It's hard to see how those would be related to this symptom. My bet is that the new fk-deadlock test exposed some pre-existing issue in isolationtester. Not quite clear what yet, though. yeah removing them does not seem to change the behaviour at all A different line of thought is that the cancel was received by the backend but didn't succeed in cancelling the query for some reason. I added the pgcancel failed codepath you suggested but it does not seem to get triggered at all so the above might actually be what is happening... Stefan -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 18:15, Fujii Masao masao.fu...@gmail.com wrote: This patch breaks also pg_ctl promote because it always checks the existence of $PGDATA/recovery.conf. You're right. It does. Good catch. That also suggest a solution: create a status file called $PGDATA/recovery_in_progress which would then allow pg_ctl and everybody else to be able to see that the server is currently in recovery. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] spoonbill vs. -HEAD
On 03/26/2013 10:18 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: There is some timeout code already in the buildfarm client. It was originally put there to help us when we got CVS hangs, a not infrequent occurrence in the early days, so it's currently only used if configured for the checkout phase, but it could easily be used to create a build timeout which would kill the whole process group if the timeout expired. It wouldn't work on Windows, and of course it won't solve whatever problem caused the hang in the first place, but it still might be worth doing. +1 --- at least then we'd get reports of failures, rather than the current behavior where the animal just stops reporting. yeah I have had multiple buildfarm members running into similiar issues (like the still-unexplained issues on spoonbill from a year back: http://www.postgresql.org/message-id/4fe4b674.3020...@kaltenbrunner.cc) so I would really like to see an option for a global timeout. Stefan -- 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] spoonbill vs. -HEAD
On 03/27/2013 03:49 PM, Stefan Kaltenbrunner wrote: On 03/26/2013 10:18 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: There is some timeout code already in the buildfarm client. It was originally put there to help us when we got CVS hangs, a not infrequent occurrence in the early days, so it's currently only used if configured for the checkout phase, but it could easily be used to create a build timeout which would kill the whole process group if the timeout expired. It wouldn't work on Windows, and of course it won't solve whatever problem caused the hang in the first place, but it still might be worth doing. +1 --- at least then we'd get reports of failures, rather than the current behavior where the animal just stops reporting. yeah I have had multiple buildfarm members running into similiar issues (like the still-unexplained issues on spoonbill from a year back: http://www.postgresql.org/message-id/4fe4b674.3020...@kaltenbrunner.cc) so I would really like to see an option for a global timeout. I have been looking at it briefly. There are a few wrinkles, but it's on the TODO list. cheers andrew Stefan -- 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] allowing privileges on untrusted languages
On 1/11/13 10:25 AM, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: It turned out that actually getting rid of lanpltrusted would be too invasive, especially because some language handlers use it to determine their own behavior. So instead the lanpltrusted attribute now just determined what the default privileges of the language are, and all the checks the require superuserness to do anything with untrusted languages are removed. Hmm ... that worries me a bit. It seems like system security will now require being sure that the permissions on the language match the lanpltrusted setting. Even if the code is right today, there's a lot of scope for future oversights with security implications. Don't know what we could do to mitigate that. I think altogether this patch does not introduce any more reasons to be careful then any other security-related patch. The ACL stuff is already spread out over too many places, and you could argue that this patch reduces some of that surface area. In particular, have you thought carefully about upgrade scenarios? Will a dump-and-restore of a pre-9.3 installation end up with safe language privileges? Untrusted languages in pre-9.3 installations cannot have any privileges, because GRANT denies that. If you grant some anyway (e.g., set the trusted bit, grant, re-remove trusted bit), then, well, you get what you asked for, expect now it actually works. In the same vein, I'm worried that the proposed change in pg_dump will do the wrong thing when looking at a pre-9.3 server. Is any server-version-dependent behavior needed there? That shouldn't be a problem for the same reasons. What might actually be a problem in this area is that, AFAICT, pg_dump does not save privileges granted to objects in extensions. -- 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] allowing privileges on untrusted languages
On 1/19/13 8:45 AM, Kohei KaiGai wrote: I think, it is a time to investigate separation of database superuser privileges into several fine-grained capabilities, like as operating system doing. https://github.com/torvalds/linux/blob/master/include/uapi/linux/capability.h The Linux capabilities system exists because there is no normal file system object to attach the privileges to. If there were /dev/somethings for all of these things, there would not no need for the capabilities thing. In this case, the privileges system already exists. We just need to use it. -- 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] replace plugins directory with GUC
On 1/15/13 7:04 AM, Peter Eisentraut wrote: It would seem to be much more in the spirit of things to simply list the allowed plugins in a GUC variable, like some_clever_name_here = $libdir/this, $libdir/that Here is a patch, with some_clever_name = user_loadable_libraries. There are obviously some conflict/transition issues with using user_loadable_libraries vs the plugins directory. I have tried to explain the mechanisms in the documentation, but there are other choices possible in some situations. After thinking about this some more, this is not actually the mechanism I need for my particular application. What I actually needed was something in between shared_preload_libraries and local_preload_libraries, namely that it is loaded into backend processes only (not postmaster), but only by superusers, and typically configured in postgresql.conf. I'll postpone actually implementing that to the next release. I'm not aware of anything that actually uses the plugins mechanism (seeing that the debugger no longer does), so I don't know if this present change would actually be an improvement or not for those applications. So I'd be OK with withdrawing this patch for now. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Catching resource leaks during WAL replay
While looking at bug #7969, it occurred to me that it would be nice if
we could catch resource leaks in WAL redo routines better. It would be
useful during development, to catch bugs earlier, and it could've turned
that replay-stopping error into a warning.
For regular transactions, we use ResourceOwners to track buffer pins
(like in #7969) and other resources. There's no fundamental reason we
couldn't use one during replay. After running a redo routine, there
should be no buffer pins held or other resources held.
Lwlocks are not tracked by resource owners, but we could still easily
warn if any are held after the redo routine exits.
- Heikki
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index e62286f..1558a8a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2282,7 +2282,7 @@ AbortTransaction(void)
* Releasing LW locks is critical since we might try to grab them again
* while cleaning up!
*/
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
/* Clean up buffer I/O and buffer context locks, too */
AbortBufferIO();
@@ -4194,7 +4194,7 @@ AbortSubTransaction(void)
* FIXME This may be incorrect --- Are there some locks we should keep?
* Buffer locks, for example? I don't think so but I'm not sure.
*/
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
AbortBufferIO();
UnlockBuffers();
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 2f91bc8..0bf87f2 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5515,6 +5515,7 @@ StartupXLOG(void)
bool recoveryApply = true;
ErrorContextCallback errcallback;
TimestampTz xtime;
+ ResourceOwner redoOwner;
InRedo = true;
@@ -5523,6 +5524,13 @@ StartupXLOG(void)
(uint32) (ReadRecPtr 32), (uint32) ReadRecPtr)));
/*
+ * Create a resource owner for running redo functions, to catch
+ * resource leaks.
+ */
+ redoOwner = ResourceOwnerCreate(NULL, WAL redo);
+ CurrentResourceOwner = redoOwner;
+
+ /*
* main redo apply loop
*/
do
@@ -5671,6 +5679,21 @@ StartupXLOG(void)
/* Now apply the WAL record itself */
RmgrTable[record-xl_rmid].rm_redo(EndRecPtr, record);
+/*
+ * The redo routine should've released all lwlocks and
+ * other resources.
+ */
+LWLockReleaseAll(true);
+ResourceOwnerRelease(redoOwner, RESOURCE_RELEASE_BEFORE_LOCKS,
+ true, true);
+/*
+ * Locks are *not* released. In hot standby mode, the startup
+ * process holds locks on behalf of transactions running in
+ * the master.
+ */
+ResourceOwnerRelease(redoOwner, RESOURCE_RELEASE_AFTER_LOCKS,
+ true, true);
+
/* Pop the error context stack */
error_context_stack = errcallback.previous;
@@ -5704,6 +5727,9 @@ StartupXLOG(void)
record = ReadRecord(xlogreader, InvalidXLogRecPtr, LOG, false);
} while (record != NULL);
+ CurrentResourceOwner = NULL;
+ ResourceOwnerDelete(redoOwner);
+
/*
* end of main redo apply loop
*/
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 287f19b..03554a1 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -582,7 +582,7 @@ bootstrap_signals(void)
static void
ShutdownAuxiliaryProcess(int code, Datum arg)
{
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
}
/*
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 286ae86..ea0e59d 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -175,7 +175,7 @@ BackgroundWriterMain(void)
* AbortTransaction(). We don't have very many resources to worry
* about in bgwriter, but we do have LWLocks, buffers, and temp files.
*/
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
AbortBufferIO();
UnlockBuffers();
/* buffer pins are released here: */
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 5fb2d81..a29fc54 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -283,7 +283,7 @@ CheckpointerMain(void)
* about in checkpointer, but we do have LWLocks, buffers, and temp
* files.
*/
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
AbortBufferIO();
UnlockBuffers();
/* buffer pins are released here: */
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 8359da6..fe58e8f 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -179,7 +179,7 @@ WalWriterMain(void)
* AbortTransaction(). We don't have very many resources to worry
* about in walwriter, but we do have LWLocks, and perhaps buffers?
*/
- LWLockReleaseAll();
+ LWLockReleaseAll(false);
Re: [HACKERS] pkg-config files for libpq and ecpg
On 3/24/13 1:55 PM, Tom Lane wrote: I experimented a bit with this version of the patch. The hunk that removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build breaks the build for me, so I took it out. What was the error message? Probably not important, but curious. With that gone, I noted that I got this when building in a Fedora RPM environment (ie, the same way I would package PG for Fedora): Name: libpq Description: PostgreSQL libpq library Url: http://www.postgresql.org/ Version: 9.3devel Requires: Requires.private: Cflags: -I/usr/include -I/usr/include/libxml2 Libs: -L/usr/lib64 -lpq Libs.private: -lssl -lcrypto -lkrb5 -lcom_err -lgssapi_krb5 -lcrypt -lldap_r -lpthread The -I for libxml2 seems pretty darn bogus; can't we avoid that? We could, but only in a hardcoded way. As it is, it's part of the general set of flags that we use to build. libxml isn't special in this regard. It only shows up because you have it in a nondefault path. If you had openssl somewhere funny, it would show up as well. At least for the libraries we are currently proposing to pkgconfig-ify, it seems to me that we only want a -I for where we are installing our own headers; there is no need for anything else. That is, echo 'Cflags: -I$(includedir)' seems like plenty. We aren't exposing any other packages' headers in the public header files for these libraries, so there's no need to tell client packages about them. libpq exposes at least openssl and gssapi, so we need those at least. So in general, it won't work so easily to trim this list intelligently. That's something that could perhaps be tuned in the future, but I'd rather offer a few flags too many than not enough. One way to look at this for now is: It's not worse than what pg_config does. Also, I'm underwhelmed by the usefulness of the automatically-generated description lines. It might be better to ask the individual makefiles to set a PKG_CONFIG_DESCRIPTION macro. I think nobody reads that, so it would be a waste of time to maintain it. The URL is more important. I'd be more interested if some Windows thing had already put in the need for a package description (see PGFILEDESC). I'm not strictly against it, though. Lastly, I wonder whether it's worth adding a configure option or some such to control whether pkgconfig files are built, or at least whether they're installed. In a lot of environments this would just be adding useless clutter to the $(libdir). Considering how much other clutter we install without any options, I think it's fine as is. -- 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27.03.2013 20:02, Simon Riggs wrote: On 27 March 2013 17:50, Heikki Linnakangashlinnakan...@vmware.com wrote: Imagine that you have set recovery_config_directory='/foo/' in the master server. Now you want to set up a standby, so you do:' pg_basebackup -D data-standby -R With the -R option, pg_basebackup creates data-standby/recovery.conf to point to the master, with standby_mode='on'. But if you start the server, it will start up as a master, not as a standby, because recovery_config_directory points elsewhere. Yeh, I get it. Same argument applies to all conf files, not just recovery.conf. Sounds like the patch to add -R to pg_basebackup should be revoked as being not well thought out. Or it should be fixed, in which case this works the same. What exactly was wrong with pg_basebackup -R, without recovery_config_directory? - Heikki -- 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] pkg-config files for libpq and ecpg
Peter Eisentraut pete...@gmx.net writes: On 3/24/13 1:55 PM, Tom Lane wrote: I experimented a bit with this version of the patch. The hunk that removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build breaks the build for me, so I took it out. What was the error message? Probably not important, but curious. ecpg's #include of libpq-fe.h failed. I speculate that you didn't notice because you tested on a machine where libpq-fe.h exists in /usr/include. At least for the libraries we are currently proposing to pkgconfig-ify, it seems to me that we only want a -I for where we are installing our own headers; there is no need for anything else. That is, echo 'Cflags: -I$(includedir)' seems like plenty. We aren't exposing any other packages' headers in the public header files for these libraries, so there's no need to tell client packages about them. libpq exposes at least openssl and gssapi, so we need those at least. No, it does not. A client might choose to #include those of its own accord, but then it's the client's problem. Our exported headers do not #include anything more exotic than stdio.h, and it's not the business of the pkg-config switches to provide for anything beyond allowing inclusions of our headers to succeed. 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
Simon Riggs si...@2ndquadrant.com wrote: doc/src/sgml/config.sgml | 17 + So that doc builds could proceed without error I fixed the obvious pasto in config.sgml. -- Kevin Grittner 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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On 27 March 2013 21:05, Heikki Linnakangas hlinnakan...@vmware.com wrote: Same argument applies to all conf files, not just recovery.conf. Sounds like the patch to add -R to pg_basebackup should be revoked as being not well thought out. Or it should be fixed, in which case this works the same. What exactly was wrong with pg_basebackup -R, without recovery_config_directory? You asked me to answer the question. I did. Please answer mine. Why do you think recovery_config_directory are different to config_file with respect to pg_basebackup? It looks like you've discovered a problem with pg_basebackup, not a problem with this patch. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Catching resource leaks during WAL replay
On 27 March 2013 20:40, Heikki Linnakangas hlinnakan...@vmware.com wrote: While looking at bug #7969, it occurred to me that it would be nice if we could catch resource leaks in WAL redo routines better. It would be useful during development, to catch bugs earlier, and it could've turned that replay-stopping error into a warning. For regular transactions, we use ResourceOwners to track buffer pins (like in #7969) and other resources. There's no fundamental reason we couldn't use one during replay. After running a redo routine, there should be no buffer pins held or other resources held. Lwlocks are not tracked by resource owners, but we could still easily warn if any are held after the redo routine exits. I'm inclined to think that the overhead isn't worth the trouble. This is the only bug of its type we had in recent years. Perhaps we need another level of compile for checks that happen only in beta? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] avoid buffer underflow in errfinish()
On Wed, Mar 27, 2013 at 9:03 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Writing it like that suggests that autovac might sometimes be NULL, even if deadlock_state == DS_BLOCKED_BY_AUTOVACUUM. From your explanation above, I gather that's not possible (and I think you're right), so the NULL check is unnecessary. If we think it might be NULL after all, the above makes sense. That makes sense. Thanks for the clarification! - xi -- 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] plpgsql_check_function - rebase for 9.3
Hello
I redesigned output from plpgsql_check_function. Now, it returns table
everytime.
Litlle bit code simplification.
postgres=# \sf fx
CREATE OR REPLACE FUNCTION public.fx(integer)
RETURNS integer
LANGUAGE plpgsql
AS $function$
BEGIN
RETURN (SELECT a FROM t1 WHERE b $1);
END;
$function$
postgres=# select * from plpgsql_check_function('fx(int)');
-[ RECORD 1 ]--
functionid | fx
lineno | 3
statement | RETURN
sqlstate | 42703
message| column b does not exist
detail | [null]
hint | [null]
level | error
position | 32
query | SELECT (SELECT a FROM t1 WHERE b $1)
context| [null]
Regards
Pavel
2013/3/26 Tom Lane t...@sss.pgh.pa.us:
Josh Berkus j...@agliodbs.com writes:
Where are we with this patch? I'm a bit unclear from the discussion on
whether it passes muster or not. Things seem to have petered out.
I took another look at this patch tonight. I think the thing that is
bothering everybody (including Pavel) is that as submitted, pl_check.c
involves a huge amount of duplication of knowledge and code from
pl_exec.c, and to a lesser extent pl_comp.c. It certainly looks like a
maintenance disaster in the making. It doesn't bother me so much that
pl_check.c knows about each syntactic structure in plpgsql: there are
already four or five places you have to touch when adding new syntax.
Rather, it's that there's so much duplication of knowledge about
semantic details, which is largely expressed by copied-and-pasted code
from pl_exec.c. It seems like a safe bet that we'll sometimes miss the
need to fix pl_check.c when we fix something in pl_exec.c. There are
annoying duplications from pl_comp.c as well, eg knowledge of exactly
which magic variables are defined in trigger functions.
Having said all that, it's not clear that we can really do any better.
The only obvious alternative is pushing support for a checking mode
directly into pl_exec.c, which would obfuscate and slow down that code
to an unacceptable degree if Pavel's results at
http://www.postgresql.org/message-id/cafj8prakujmvjpjzfsrye7+ub8jf8wtz5rkxk-0ykxme-k8...@mail.gmail.com
are any indication. (In that message, Pavel proposes shoveling the
problem into the compile code instead, but that seems unlikely to work
IMO: the main problem in pl_check.c as it stands is duplication of code
from pl_exec.c not pl_comp.c. So I think that path could only lead to
duplicating the same code into pl_comp.c.)
So question 1 is do we want to accept that this is the implementation
pathway we're going to settle for, or are we going to hold out for a
better one? I'd be the first in line to hold out if I had a clue how
to move towards a better implementation, but I have none. Are we
prepared to reject this type of feature entirely because of the
code-duplication problem? That doesn't seem attractive either.
But, even granting that this implementation approach is acceptable,
the patch does not seem close to being committable as-is: there's
a lot of fit-and-finish work yet to be done. To make my point I will
just quote from one of the regression test additions:
create or replace function f1()
returns void as $$
declare a1 int; a2 int;
begin
select 10,20 into a1;
end;
$$ language plpgsql;
-- raise warning
select plpgsql_check_function('f1()');
plpgsql_check_function
-
warning:0:4:SQL statement:too many attributies for target variables
Detail: There are less target variables than output columns in query.
Hint: Check target variables in SELECT INTO statement
(3 rows)
Do we like this output format? I don't. The unlabeled, undocumented
fields crammed into a single line with colon separators are neither
readable nor useful. If we actually need these fields, why aren't we
splitting the output into multiple columns? (I'm also wondering why
the patch bothers with an option to emit this same info in XML. Surely
there is vanishingly small use-case for mechanical examination of this
output.)
This example also shows that the user-visible messages have had neither
editing from a native speaker of English, nor even attention from a
spell checker. I don't fault Pavel for that (his English is far better
than my Czech) but this patch has been passed by at least one reviewer
who is a native speaker. Personally I'd also say that neither the
Detail nor Hint messages in this example are worth the electrons they
take up, as they add nothing useful to the basic error message. I'd be
embarrassed to be presenting output like this to end users of Postgres.
(The code comments are in even worse shape than the user-visible
messages, by the by, where they exist at all.)
A somewhat related point is that it doesn't look like any thought
has been taken about localized message output.
This stuff is certainly all fixable if we
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 27 March 2013 17:14, Brendan Jurd dire...@gmail.com wrote:
On 28 March 2013 00:21, Dean Rasheed dean.a.rash...@gmail.com wrote:
The patch is also allowing '{{},{},{}}' which is described up-thread
as a 2-D empty array. That's pretty misleading, since it has length 3
(in the first dimension). '{{},{}}' and '{{}}' are both more empty,
but neither is completely empty.
I'm not saying that the current situation is not broken. I'm just
questioning whether the fix is actually any less confusing than what
we have now.
Well the fix is primarily about 1-D empty arrays, and in that respect
it is much less confusing than what we have now.
Maybe. But even in 1-D, it's still jumping from having one empty array
to infinitely many starting at different indexes, e.g., '{}'::int[] !=
'[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
convinced about its usefulness.
Also, it is incompatible with the choice made for empty ranges, which
are all normalised to a single unique empty range value --- the empty
set, with no start- or end-points. I find that quite logical, and so
to me, it makes most sense to also have a single unique empty array,
which is then necessarily dimensionless.
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] Catching resource leaks during WAL replay
Simon Riggs si...@2ndquadrant.com writes: On 27 March 2013 20:40, Heikki Linnakangas hlinnakan...@vmware.com wrote: While looking at bug #7969, it occurred to me that it would be nice if we could catch resource leaks in WAL redo routines better. It would be useful during development, to catch bugs earlier, and it could've turned that replay-stopping error into a warning. I'm inclined to think that the overhead isn't worth the trouble. This is the only bug of its type we had in recent years. I agree that checking for resource leaks after each WAL record seems too expensive compared to what we'd get for it. But perhaps it's worth making a check every so often, like at restartpoints? 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] Catching resource leaks during WAL replay
On 27 March 2013 23:01, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 27 March 2013 20:40, Heikki Linnakangas hlinnakan...@vmware.com wrote: While looking at bug #7969, it occurred to me that it would be nice if we could catch resource leaks in WAL redo routines better. It would be useful during development, to catch bugs earlier, and it could've turned that replay-stopping error into a warning. I'm inclined to think that the overhead isn't worth the trouble. This is the only bug of its type we had in recent years. I agree that checking for resource leaks after each WAL record seems too expensive compared to what we'd get for it. But perhaps it's worth making a check every so often, like at restartpoints? +1 -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 09:39, Dean Rasheed dean.a.rash...@gmail.com wrote:
On 27 March 2013 17:14, Brendan Jurd dire...@gmail.com wrote:
Well the fix is primarily about 1-D empty arrays, and in that respect
it is much less confusing than what we have now.
Maybe. But even in 1-D, it's still jumping from having one empty array
to infinitely many starting at different indexes, e.g., '{}'::int[] !=
'[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
convinced about its usefulness.
We already have the ability to define lower bounds other than 1 on
arrays, and it would be inconsistent to allow that for arrays with
elements, but not for arrays without. I could imagine somebody
wanting to create an empty zero-based array, and then iteratively
append elements to it.
Also, it is incompatible with the choice made for empty ranges,
To me it doesn't make sense to try to draw parallels between arrays
and ranges, they really are completely different things.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
Brendan Jurd dire...@gmail.com writes:
On 28 March 2013 09:39, Dean Rasheed dean.a.rash...@gmail.com wrote:
Maybe. But even in 1-D, it's still jumping from having one empty array
to infinitely many starting at different indexes, e.g., '{}'::int[] !=
'[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
convinced about its usefulness.
We already have the ability to define lower bounds other than 1 on
arrays, and it would be inconsistent to allow that for arrays with
elements, but not for arrays without.
Yeah, if '[1:1]={0}'::int[] is distinct from '[2:2]={0}'::int[],
it's a bit hard to argue that '[1:0]={}'::int[] must not be
distinct from '[2:1]={}'::int[]. If we were doing this from scratch
we might drop the whole notion of nondefault lower bounds, but that
ship sailed ages ago.
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] [COMMITTERS] pgsql: Allow external recovery_config_directory
On Thu, Mar 28, 2013 at 6:05 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: What exactly was wrong with pg_basebackup -R, without recovery_config_directory? pg_basebackup -R generates automatically recovery.conf inside data folder, so if recovery_config_directory is specified the slave startup will fail. The same problem exists also with the case where a tarball is generated for base backup. Such limitations should be specified in the docs at least. -- Michael
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
We already have the ability to define lower bounds other than 1 on arrays, and it would be inconsistent to allow that for arrays with elements, but not for arrays without. I could imagine somebody wanting to create an empty zero-based array, and then iteratively append elements to it. Hmmm. You know, I think that's why we originally had array_upper() on an empty array return NULL, not 0. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 00:04, Tom Lane t...@sss.pgh.pa.us wrote:
Brendan Jurd dire...@gmail.com writes:
On 28 March 2013 09:39, Dean Rasheed dean.a.rash...@gmail.com wrote:
Maybe. But even in 1-D, it's still jumping from having one empty array
to infinitely many starting at different indexes, e.g., '{}'::int[] !=
'[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
convinced about its usefulness.
We already have the ability to define lower bounds other than 1 on
arrays, and it would be inconsistent to allow that for arrays with
elements, but not for arrays without.
Yeah, if '[1:1]={0}'::int[] is distinct from '[2:2]={0}'::int[],
it's a bit hard to argue that '[1:0]={}'::int[] must not be
distinct from '[2:1]={}'::int[]. If we were doing this from scratch
we might drop the whole notion of nondefault lower bounds, but that
ship sailed ages ago.
You could make the exact same argument for ranges --- if
'[1,1]'::int4range is distinct from '[2,2]'::int4range, why isn't
'[1,1)'::int4range distinct from '[2,2)'::int4range?
I see ranges and arrays as very closely related because the extents of
an array are an integer range, and the extents of an empty array are
an empty range. Moreover, they have almost identical API functions.
There are two internally self-consistent models for handling empty
ranges/arrays --- one in which empty ranges/arrays are considered not
to have lower/upper bounds, and one in which they are. In the first
model, there is only one empty range/array. In the second, there are
infinitely many, all different. Both models can be written in a
consistent way, but what seems inconsistent is to choose one model for
ranges, and change to a different model for arrays.
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] Support for REINDEX CONCURRENTLY
On 2013-03-28 10:18:45 +0900, Michael Paquier wrote: On Thu, Mar 28, 2013 at 3:12 AM, Fujii Masao masao.fu...@gmail.com wrote: Since we call relation_open() with lockmode, ISTM that we should also call relation_close() with the same lockmode instead of NoLock. No? Agreed on that. That doesn't really hold true generally, its often sensible to hold the lock till the end of the transaction, which is what not specifying a lock at close implies. Greetings, Andres Freund -- 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] Support for REINDEX CONCURRENTLY
On 2013-03-19 08:57:31 +0900, Michael Paquier wrote: On Tue, Mar 19, 2013 at 3:24 AM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Mar 13, 2013 at 9:04 PM, Michael Paquier michael.paqu...@gmail.com wrote: I have been working on improving the code of the 2 patches: 1) reltoastidxid removal: snip - Fix a bug with pg_dump and binary upgrade. One valid index is necessary for a given toast relation. Is this bugfix related to the following? appendPQExpBuffer(upgrade_query, - SELECT c.reltoastrelid, t.reltoastidxid + SELECT c.reltoastrelid, t.indexrelid FROM pg_catalog.pg_class c LEFT JOIN - pg_catalog.pg_class t ON (c.reltoastrelid = t.oid) - WHERE c.oid = '%u'::pg_catalog.oid;, + pg_catalog.pg_index t ON (c.reltoastrelid = t.indrelid) + WHERE c.oid = '%u'::pg_catalog.oid AND t.indisvalid + LIMIT 1, Yes. Don't indisready and indislive need to be checked? An index is valid if it is already ready and line. We could add such check for safely but I don't think it is necessary. Note that thats not true for 9.2. live !ready represents isdead there, since the need for that was only recognized after the release. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
Dean Rasheed dean.a.rash...@gmail.com writes:
On 28 March 2013 00:04, Tom Lane t...@sss.pgh.pa.us wrote:
Yeah, if '[1:1]={0}'::int[] is distinct from '[2:2]={0}'::int[],
it's a bit hard to argue that '[1:0]={}'::int[] must not be
distinct from '[2:1]={}'::int[].
You could make the exact same argument for ranges --- if
'[1,1]'::int4range is distinct from '[2,2]'::int4range, why isn't
'[1,1)'::int4range distinct from '[2,2)'::int4range?
Because an array's bounds are a core piece of the identity of the
array. In another universe PG's arrays might not have been defined
like that, but as things stand, they are. We chose to define ranges
differently. That's okay, because ranges *are not arrays*. If they
were the same, we'd not have needed to invent a separate concept.
I see ranges and arrays as very closely related because the extents of
an array are an integer range, and the extents of an empty array are
an empty range. Moreover, they have almost identical API functions.
I think this argument has about as much validity as claiming that
because the subscripts of an array are integers, arrays should behave
like integers.
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] money with 4 digits after dot
I found a workaround: domain type defined as: CREATE DOMAIN currency AS numeric(16,4); Thank you!
