Re: [HACKERS] Review: Non-inheritable check constraints
On Fri, Oct 7, 2011 at 21:30, Nikhil Sontakke wrote: > Hi Alex, > > I guess we both are in agreement with each other :) > > After sleeping over it, I think that check is indeed dead code with this new > non-inheritable check constraints functionality in place. So unless you have > some other comments, we can mark this as 'Ready for Commiter'. > > Again, thanks for the thorough review and subsequent changes! PFA an updated patch with the check removed and a comment or two added. I've also marked it ready. non_inh_check_v3.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Intermittent regression test failure from index-only scans patch
Robert Haas writes: > On Oct 8, 2011, at 11:04 AM, Tom Lane wrote: >> I'm inclined to fix this by changing the test to examine idx_tup_read >> not idx_tup_fetch. Alternatively, we could have the test force >> enable_indexonlyscan off. Thoughts? > No preference. I ended up doing it the second way (ie enable_indexonlyscan = off) because it turns out that pg_stat_user_tables doesn't have the idx_tup_read column --- we track that count per index, not per table. I could have complicated the test's stats queries some more, but it seemed quite not relevant to the goals of the test. > Should we have another counter for heap fetches avoided? Seems like that > could be useful to know. Hm. I'm hesitant to add another per-table (or per index?) statistics counter because of the resultant bloat in the stats file. But it wouldn't be a bad idea for somebody to take two steps back and rethink what we're counting in this area. The current counter definitions are mostly backwards-compatible with pre-8.1 behavior, and it seems like the goalposts have moved enough that maybe it's time to break compatibility. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Schema grants for creating and dropping objects
It seems that in order to create an object in a given schema, I must have been granted create privilege on the schema. But in order to drop that object I require usage privilege. This means that with the right privilege settings I can create objects that I cannot subsequently drop, or can drop an object that I cannot recreate. I assume this is a bug but if it's intended behaviour I'd love to hear the rationale. I checked this on 8.3, 8.4, 9.0 and 9.1 all with the same results. Best regards. __ Marc signature.asc Description: This is a digitally signed message part
Re: [HACKERS] unite recovery.conf and postgresql.conf
On Tue, Sep 27, 2011 at 1:33 AM, Fujii Masao wrote: > > Though there is still ongonig discussion, since there is no objection about > the above two changes, I revised the patch that way. And I fixed the minor > bug handling the default value of recovery_target_timeline wrongly. > Attached is the revised version of the patch. This patch no longer applies as it conflicts with the following commit: commit d56b3afc0376afe491065d9eca6440b3cc7b1346 Author: Tom Lane Date: Sun Oct 2 16:50:04 2011 -0400 Restructure error handling in reading of postgresql.conf. Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types - typo + NULL string constructor
On Sat, 2011-10-08 at 12:44 -0700, Jeff Janes wrote: > When I apply this to head, "make check" fails with: > > create type textrange_en_us as range(subtype=text, collation="en_US"); > + ERROR: collation "en_US" for encoding "SQL_ASCII" does not exist > > Is this a problem? e.g. will it break the build-farm? > > "make check LANG=en_US" does pass Thank you for pointing that out. I think I need to remove those before commit, but I just wanted them in there now to exercise that part of the code. Is there a better way to test collations like that? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types - typo + NULL string constructor
On Sun, Oct 2, 2011 at 12:05 PM, Jeff Davis wrote: > On Sun, 2011-10-02 at 11:32 +0200, Florian Pflug wrote: >> Looking at the patch, I noticed that it's possible to specify the default >> boundaries ([], [), (] or ()) per individual float type with the >> DEFAULT_FLAGS clause of CREATE TYPE .. AS RANGE. I wonder if that doesn't >> do more harm then good - it makes it impossible to deduce the meaning of >> e.g. numericrange(1.0, 2.0) without looking up the definition of >> numericrange. >> >> I suggest we pick one set of default boundaries, ideally '[)' since that >> is what all the built-in canonization functions produce, and stick with it. > > Done. > > Also, made the range parsing even more like records with more code > copied verbatim. And fixed some parsing tests along the way. When I apply this to head, "make check" fails with: create type textrange_en_us as range(subtype=text, collation="en_US"); + ERROR: collation "en_US" for encoding "SQL_ASCII" does not exist Is this a problem? e.g. will it break the build-farm? "make check LANG=en_US" does pass Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] index-only scans
On Oct 8, 2011, at 11:52 AM, Tom Lane wrote: > I'm inclined to do the last part immediately, since there's a > performance argument for it, and then work on revising the executor > implementation. Sounds great. Thanks for working on this. ...Robert -- 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] Intermittent regression test failure from index-only scans patch
On Oct 8, 2011, at 11:04 AM, Tom Lane wrote: > I'm inclined to fix this by changing the test to examine idx_tup_read > not idx_tup_fetch. Alternatively, we could have the test force > enable_indexonlyscan off. Thoughts? No preference. Should we have another counter for heap fetches avoided? Seems like that could be useful to know. ...Robert -- 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] libpq, PQdescribePrepared -> PQftype, PQfmod, no PQnullable
I'll point to rather different reasoning... Libpq is not returning tables, or relations, for that matter, but rather the results of queries. It is reasonable to expect to know which attributes of a table are or are not nullable, and that is commonly available as an attribute of pg_attribute, however... General purpose queries are nowhere near so predetermined. Indeed, whether a column is nullable may not be at all visible, as the value of a column may be computed by a function and thereby be quite opaque to static analysis. That makes me think that guessing which attributes of a query may be null seems like a pretty futile exercise. At first blush, we could simplify to PQnullable() always returning true, but that's not terribly revealing. However, often, there mayn't be a much better solution that isn't really tough to implement. I'd not be keen on people putting much effort into futile exercises ; better to work on things that are "less futile."
[HACKERS] REVIEW: Optimizing box_penalty
I tried to review the "Optimizing box_penalty" patch: https://commitfest.postgresql.org/action/patch_view?id=600 as posted here: http://archives.postgresql.org/message-id/4e088690.5080...@enterprisedb.com The patch no longer applies to source code, due to other recent GiST changes. Parts of it were modifying functions which no longer exist. I picked out the bits which still seemed relevant, and a patch with that is attached. The improvement in REINDEX time is now only about 0.25% on my machine at home, which is small enough that it could easily be from code shifting around. (Or such shifts might be hiding a larger actual savings.) In any event, this patch doesn't seem to be justified as a performance patch, based on my benchmarks today. On the other hand, this patch leaves the code a few lines shorter and eliminates some unnecessary Datum wrapping, PG_FUNCTION_ARGS parameters on a static function, and allows that function to be called directly rather than using DirectFunctionCall2(). I find the resulting code a little cleaner and easier to read. I would prefer to see it applied on that basis, personally. Since the author is a committer, and this is a pretty minor code style patch at this point, I'll mark it "Ready for Committer" and leave it to Heikki to decide what to do with it. -Kevin penalty-v2.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] SET variable - Permission issues
Josh writes: > [ unhappy about users being able to freely adjust work_mem etc ] Really, if you're letting users issue arbitrary SQL queries, there simply isn't any way to prevent them from beating your server into the ground. I don't think that inserting a hack to prevent specific configuration variables from being adjusted is going to help you against an uncooperative user. You'd be better off to rethink the "let them issue SQL queries directly" part of your design. The reason that the specific variables you mention (as well as some others that bear on such things) are USERSET and not SUSET is precisely that we are not trying to constrain the amount of resources an uncooperative user can consume. If we did try to do that, quite a lot of design decisions would have to be revisited, and there would be a number of unpleasant tradeoffs to be made. GUC privilege levels are just the tip of the iceberg. 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: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)
On Sat, 2011-10-08 at 18:43 +0400, Alexander Korotkov wrote: > I meant that penalty can be determined as sum of difference of old and > new bounds of range, i.e. penalty = subtype_diff_float(new_lower, > old_lower) + subtype_diff_float(old_upper, new_upper). > When we insert [100,200) into [10,+inf), union([100,200), [10,+inf)) > = [10,+inf), so penalty = subtype_diff_float(10,10) > + subtype_diff_float(+inf, +inf) = 0 + 0 = 0. > When we insert [100,200) into [10,), union([100,200), [10, > +inf)) = [100,+inf), so penalty = subtype_diff_float(100,10) > + subtype_diff_float(+inf, +inf) = 99900 + 0 = 99900. > OK, I like that. I will make the change. > But, there are still the problem, when we'are inserting open interval > when there is no such open intervals yet. For example, we're going to > insert [0,+inf), while root page contains [0,10), [10,20), [20,30). > Each penalty will be infinity, while it seems to be better to insert > it into [0,10). But, it seems to me to be general limitation of > current GiST interface, when we have to express penalty in a single > float. That seems like an acceptable limitation. I don't think my solution handles it any better. Regards, Jeff Davis > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix Leaky View Problem
On Sat, Oct 08, 2011 at 09:11:08AM +0200, Kohei KaiGai wrote: > 2011/10/8 Noah Misch : > > On Sun, Oct 02, 2011 at 07:16:33PM +0200, Kohei KaiGai wrote: > >> My preference is still also WITH(security_barrier=...) syntax. > >> > >> The arguable point was the behavior when a view is replaced without > >> explicit WITH clause; > >> whether we should consider it was specified a default value, or we > >> should consider it means > >> the option is preserved. > >> If we stand on the viewpoint that object's attribute related to > >> security (such as ownership, > >> acl, label, ...) should be preserved, the security barrier also shall > >> be preserved. > >> On the other hand, we can never know what options will be added in the > >> future, right now. > >> Thus, we may need to sort out options related to security and not at > >> DefineVirtualRelation(). > >> > >> However, do we need to limit type of the options to be preserved to > >> security related? > >> It is the first case that object with arbitrary options can be replaced. > >> It seems to me we have no matter, even if we determine object's > >> options are preserved > >> unless an explicit new value is provided. > > > > Currently, you can predict how CREATE OR REPLACE affects a given object > > characteristic with a simple rule: if the CREATE OR REPLACE statement can > > specify a characteristic, we don't preserve its existing value. ?Otherwise, > > we > > do preserve it. ?Let's not depart from that rule. > > > > Applying that rule to the proposed syntax, it shall not preserve the > > existing > > security_barrier value. ?I think that is acceptable. ?If it's not > > acceptable, we > > need a different syntax -- perhaps CREATE SECURITY VIEW. > > > No. It also preserves the security-barrier flag, when we replace a view > without > SECURITY option. The only difference is that we have no way to turn off > security-barrier flag explicitly, right now. > > The major reason why I prefer reloptions rather than "SECURITY" option is > that allows to reuse the existing capability to store a property of relation. > It seems to me both of syntax enables to achieve the rule to preserve flags > when a view is replaced. Yes, there are no technical barriers to implementing either behavior with either syntax. However, CREATE OR REPLACE VIEW ... WITH (...) has a precedent guiding its behavior: if a CREATE OR REPLACE statement can specify a characteristic, we don't preserve its existing value. > >> Any other ideas? > > > > Suppose we permitted pushdown of unsafe predicates when the user can read > > the > > involved columns anyway, a generalization of the idea from the first > > paragraph > > of [1]. ?Would that, along with LEAKPROOF, provide enough strategies for > > shoring > > up performance to justify removing unsafe views entirely? > > > The problem was that we do all the access control decision at the > executor stage, > but planner has to make a plan prior to execution. So, it was also reason why > we > have tried to add LEAKPROOF flag to functions. Yes; we'd need to invalidate relevant plans in response to anything that changes access control decisions. GRANT and ALTER ... OWNER TO already do that, but we'd need to cover pg_authid/pg_auth_members changes, SET ROLE, SET SESSION AUTHORIZATION, and probably a few other things. That might be a substantial project in its own right. -- 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] Intermittent regression test failure from index-only scans patch
=?ISO-8859-1?Q?C=E9dric_Villemain?= writes: > 2011/10/8 Tom Lane : >> The diff indicates that the idx_scan count advanced but idx_tup_fetch >> did not, which is not so surprising here because tenk2 hasn't been >> modified in some time. If the autovacuum daemon managed to mark it >> all-visible before the stats test runs, then an index-only scan will >> happen, and bingo, no idx_tup_fetch increment (because indeed no heap >> tuple was fetched). >> >> I'm inclined to fix this by changing the test to examine idx_tup_read >> not idx_tup_fetch. Alternatively, we could have the test force >> enable_indexonlyscan off. Thoughts? > No preferences, but is it interesting to add a "vacuum freeze" > somewhere and check expected result after index-only scan ? (for both > idx_tup_read and idx_tup_fetch) This test is only trying to make sure that the stats collection machinery is working. I don't think that we should try to coerce things so that it can check something as context-sensitive as whether an index-only scan happened. It's too fragile already --- we've seen non-reproducible failures here many times before. 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] index-only scans
I wrote: > Robert Haas writes: >> Not really. We have detected a small performance regression when both >> heap and index fit in shared_buffers and an index-only scan is used. >> I have a couple of ideas for improving this. One is to store a >> virtual tuple into the slot instead of building a regular tuple, but >> what do we do about tuples with OIDs? [ that's done ] > I was also toying with the notion of pushing the slot fill-in into the > AM, so that the AM API is to return a filled TupleSlot not an > IndexTuple. This would not save any cycles AFAICT --- at least in > btree, we still have to make a palloc'd copy of the IndexTuple so that > we can release lock on the index page. The point of it would be to > avoid the assumption that the index's internal storage has exactly the > format of IndexTuple. Don't know whether we'd ever have any actual use > for that flexibility, but it seems like it wouldn't cost much to > preserve the option. BTW, I concluded that that would be a bad idea, because it would involve the index AM in some choices that we're likely to want to change. In particular it would foreclose ever doing anything with expression indexes, without an AM API change. Better to just define the AM's responsibility as to hand back a tuple defined according to the index's columns. >> Another is to avoid locking the >> index buffer multiple times - right now it locks the index buffer to >> get the TID, and then relocks it to extract the index tuple (after >> checking that nothing disturbing has happened meanwhile). It seems >> likely that with some refactoring we could get this down to a single >> lock/unlock cycle, but I haven't figured out exactly where the TID >> gets copied out. > Yeah, maybe, but let's get the patch committed before we start looking > for second-order optimizations. On reflection I'm starting to think that the above would be a good idea because there are a couple of bogosities in the basic choices this patch made. In particular, I'm thinking about how we could use an index on f(x) to avoid recalculating f() in something like select f(x) from tab where f(x) < 42; assuming that f() is expensive but immutable. The planner side of this is already a bit daunting, because it's not clear how to recognize that an index on f(x) is a covering index (the existing code is going to think that x itself needs to be available). But the executor side is a real problem, because it will fail to make use of the f() value fetched from the index anytime the heap visibility test fails. I believe that we should rejigger things so that when an index-only scan is selected, the executor *always* works from the data supplied by the index. Even if it has to visit the heap --- it will do that but just to consult the tuple's visibility data, and then use what it got from the index anyway. This means we'd build the plan node's filter quals and targetlist to reference the index tuple columns not the underlying table's. (Which in passing gets rid of the behavior you were complaining about that EXPLAIN VERBOSE shows a lot of columns that aren't actually being computed.) In order to make this work, we have to remove the API wart that says the index AM is allowed to choose to not return the index tuple. And that ties into what you were saying above. What we ought to do is have _bt_readpage() save off copies of the whole tuples not only the TIDs when it is extracting data from the page. This is no more net copying work than what happens now (assuming that all the tuples get fetched) since we won't need the per-tuple memcpy that occurs now in bt_getindextuple. The tuples can go into a page-sized workspace buffer associated with the BTScanPosData structure, and then just be referenced in-place in that workspace with no second copy step. I'm inclined to do the last part immediately, since there's a performance argument for it, and then work on revising the executor implementation. 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] Intermittent regression test failure from index-only scans patch
2011/10/8 Tom Lane : > I notice that several members of the buildfarm have been consistently > showing the same regression test failure since the index-only scans > patch went in: > > *** > /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/expected/stats.out > Sat Oct 8 03:20:05 2011 > --- > /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/results/stats.out > Sat Oct 8 12:30:55 2011 > *** > *** 94,100 > WHERE st.relname='tenk2' AND cl.relname='tenk2'; > ?column? | ?column? | ?column? | ?column? > --+--+--+-- > ! t | t | t | t > (1 row) > > SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, > --- 94,100 > WHERE st.relname='tenk2' AND cl.relname='tenk2'; > ?column? | ?column? | ?column? | ?column? > --+--+--+-- > ! t | t | t | f > (1 row) > > SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, > > > The diff indicates that the idx_scan count advanced but idx_tup_fetch > did not, which is not so surprising here because tenk2 hasn't been > modified in some time. If the autovacuum daemon managed to mark it > all-visible before the stats test runs, then an index-only scan will > happen, and bingo, no idx_tup_fetch increment (because indeed no heap > tuple was fetched). > > I'm inclined to fix this by changing the test to examine idx_tup_read > not idx_tup_fetch. Alternatively, we could have the test force > enable_indexonlyscan off. Thoughts? No preferences, but is it interesting to add a "vacuum freeze" somewhere and check expected result after index-only scan ? (for both idx_tup_read and idx_tup_fetch) > > 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 > -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SET variable - Permission issues
Hello all, While working on an application I have been developing for about two years now, I have come across a base-limitation in PostgreSQL for three separate problems. I was talking with other members of the community recently and they agreed that the issue has some merit so I thought I would bring it up to the list. First a bit about my environment to better understand the issue. My application allows users to connect directly to the database and issue commands as they see fit with their own database credentials, which operate under various permission restrictions. While I understand this is an unusual way of using the database, this is simply the design of my application. With that being said, I believe this issue is also important to address for other scenarios as well (like an organization that offers shared hosting of a PostgreSQL instance). The actual issue is that many important variables within the PostgreSQL configuration file can be overridden by any (non superuser) account using SET. The first instance of this I ran into was with statement_timeout, which I was hoping to use to limit the amount of time a query can run for before the query is cancelled. This was crucial to the architecture of my application to assure that infinite queries did not bog down the system and deny service to other users running their own queries. The problem is that regardless of what is set in the configuration, users of any privilege level can override the value in the postgresql.conf with the SET command. This was frustrating, but not the end of the world. I solved it by using a perl script that monitors the running queries and kills any process that runs over the allotted time. This isn't perfect though, since a user could issue enough commands to prevent the perl script from being able to function correctly. The second variable that I have recently come to realize exists causes a much more serious problem. This would be the work_mem setting. This variable allows any user to override the system default (again using SET) and issue some commands that may intentionally eat up huge chunks of RAM. This one seems a lot more serious as I have no way of monitoring what this value is set to. Also, if the attacker acts fast enough (and uses the statement_timeout issue as well), they can completely DoS the server. Finally, the third variable that has caused my application grief is the SEED value used by the RANDOM function that can also be set by any user.I realize I should be using a better pseudorandom number generator for anything important, but since I did not know that PostgreSQL's random is beyond a simple case of low-entropy and is actually completely deterministic for all my users (because they can each SET their own SEED values without restrictions), I now need to rework quite a bit of code to resolve this. These are the three variables which have been problematic so far in my application, but I am sure there are others I simply have not noticed (or are harmless in my application, but may cause problems in another scenario). As far as a solution to the problem, some discussion was made about adding more variables for system maximums (like for statement_timeout and work_mem), but this does not cover all cases (like SEED) and adds unnecessary limitations to the system. Some back-end users may actually need to alter these variables beyond what is set as the maximum but cannot do so without altering the configuration file. The other option that was discussed was to add privileges for variables (i.e. adding GRANT and REVOKE privileges for the SET command). This is obviously a bit more effort but I feel it would be the best option as it will add a lot more flexibility into the security of PostgreSQL. If anybody has any other solutions for how to temporarily resolve these problems I would definitely love to hear it but I think it is a good idea to discuss this problem and find a solution that everyone is comfortable with. Thanks for taking the time to think this over, -Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq, PQdescribePrepared -> PQftype, PQfmod, no PQnullable
The obvious typos: ,--- I/Alex (Thu, 06 Oct 2011 19:42:13 -0400) * | (may use pg_attribute.attnotnull on t1, t2, is I didn't see the 'create's. (may use pg_attribute.attnotnull on t1, t2, if I didn't see the 'create's. | Now, for this statement, I can easily identify non-nullable columns. Now, for this statement, I can easily identify the non-nullable columns: -- Alex -- goncharov.a...@gmail.com -- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [OT?] Time-zone database down [was: Re: timezone buglet?]
On 10/07/2011 11:02 PM, Greg Stark wrote: All that said I think this is far murkier than you all seem to think. Copyright law is one of the most complex areas of the law and this is one of the least well defined parts of copyright law. Hi Greg: I don't think "we all" think this issue is clear. Quoting relevant case law and considering what position to hold or what action to take is what I would call due diligence. If somebody wants to hire a lawyer that might be advisable as well. I think "wait and see whether this is a true violation" is a perfectly valid legal position to hold and is not pretending in any way that this issue is "clear"... -- Mark Mielke -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Intermittent regression test failure from index-only scans patch
I notice that several members of the buildfarm have been consistently showing the same regression test failure since the index-only scans patch went in: *** /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/expected/stats.out Sat Oct 8 03:20:05 2011 --- /home/pgbuildfarm/workdir/HEAD/pgsql.27150/src/test/regress/results/stats.out Sat Oct 8 12:30:55 2011 *** *** 94,100 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? | ?column? | ?column? --+--+--+-- ! t| t| t| t (1 row) SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, --- 94,100 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? | ?column? | ?column? --+--+--+-- ! t| t| t| f (1 row) SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, The diff indicates that the idx_scan count advanced but idx_tup_fetch did not, which is not so surprising here because tenk2 hasn't been modified in some time. If the autovacuum daemon managed to mark it all-visible before the stats test runs, then an index-only scan will happen, and bingo, no idx_tup_fetch increment (because indeed no heap tuple was fetched). I'm inclined to fix this by changing the test to examine idx_tup_read not idx_tup_fetch. Alternatively, we could have the test force enable_indexonlyscan off. Thoughts? 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: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)
On Sat, Oct 8, 2011 at 1:01 PM, Jeff Davis wrote: > On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > > > The first thing caught my eye in existing GiST code is idea of > > subtype_float. float8 has limited precision and can't respresent, for > > example, varlena values good enough. Even if we have large int8 value > > we can loose lower bits, but data distribution can be so that these > > bits are valuable. Wouldn't it better to have function like > > subtype_diff_float which returns difference between two values of > > subtype as an float? Using of such function could make penalty more > > sensible to even small difference between values, and accordingly more > > relevant. > > The reason I did it that way is for unbounded ranges. With > subtype_diff_float, it's difficult for the GiST code to differentiate > between [10,) and [10,), because infinity minus anything is > infinity. But when inserting the range [100,200), the penalty for the > first one should be zero and the second one should have some positive > penalty, right? > I meant that penalty can be determined as sum of difference of old and new bounds of range, i.e. penalty = subtype_diff_float(new_lower, old_lower) + subtype_diff_float(old_upper, new_upper). When we insert [100,200) into [10,+inf), union([100,200), [10,+inf)) = [10,+inf), so penalty = subtype_diff_float(10,10) + subtype_diff_float(+inf, +inf) = 0 + 0 = 0. When we insert [100,200) into [10,), union([100,200), [10,+inf)) = [100,+inf), so penalty = subtype_diff_float(100,10) + subtype_diff_float(+inf, +inf) = 99900 + 0 = 99900. But, there are still the problem, when we'are inserting open interval when there is no such open intervals yet. For example, we're going to insert [0,+inf), while root page contains [0,10), [10,20), [20,30). Each penalty will be infinity, while it seems to be better to insert it into [0,10). But, it seems to me to be general limitation of current GiST interface, when we have to express penalty in a single float. -- With best regards, Alexander Korotkov.
Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace
Julien Tachoires 10/07/11 10:17 AM >>> > Here's a patch to allow TOAST tables to be moved to a different > tablespace. This item has been picked up from the TODO list. > Main idea is to consider that a TOAST table can have its own > tablespace. Thanks for the patch. Please add this to the open CommitFest at: https://commitfest.postgresql.org/action/commitfest_view/open That will help ensure we don't lose track of it before the next review cycle. For more information on the review and commit process, see this page: http://wiki.postgresql.org/wiki/CommitFest We are currently well in to a CF and still have patches which nobody has yet volunteered to review. If you could help with that, it would be great! https://commitfest.postgresql.org/action/commitfest_view/inprogress -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade - add config directory setting
Bruce Momjian wrote:
> OK, I have modified the postmaster in PG 9.2 to allow output of the data
> directory, and modified pg_ctl to use that, so starting in PG 9.2 pg_ctl
> will work cleanly for config-only directories.
>
> I will now work on pg_upgrade to also use the new flag to find the data
> directory from a config-only install. However, this is only available
> in PG 9.2, and it will only be in PG 9.3 that you can hope to use this
> feature (if old is PG 9.2 or later). I am afraid the symlink hack will
> have to be used for several more years, and if you are supporting
> upgrades from pre-9.2, perhaps forever.
>
> I did find that it is possible to use pg_ctl -w start on a config-only
> install using this trick:
>
> su -l postgres \
> -c "env PGPORT=\"5432\" /usr/lib/postgresql-9.1/bin/pg_ctl start -w \
> -t 60 -s -D /var/lib/postgresql/9.1/data/ \
> -o '-D /etc/postgresql-9.1/ \
> --data-directory=/var/lib/postgresql/9.1/data/ \
> --silent-mode=true'"
>
> Unfortunately pg_upgrade doesn't support the -o option which would make
> this possible for pg_upgrade.
>
> One idea would be to add -o/-O options to pg_upgrade 9.2 to allow this
> to work even with old installs, but frankly, this is so confusing I am
> not sure we want to encourage people to do things like this. Of course,
> the symlink hack is even worse, so maybe there is some merit to this.
OK, the attached patch adds -o/-O options to pg_upgrade to mimick pg_ctl
-o, and documents the 'Gentoo method' for allowing pg_upgrade to handle
pre-9.2 upgrades for config-only installs. I think this closes the
issue, with no backpatching required for it to work for new PG 9.2.
Users will have to continue using the symlink method for new PG 9.1.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index 3ab1b5c..9892b97
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*** parseCommandLine(int argc, char *argv[])
*** 39,44
--- 39,46
{"new-datadir", required_argument, NULL, 'D'},
{"old-bindir", required_argument, NULL, 'b'},
{"new-bindir", required_argument, NULL, 'B'},
+ {"old-options", required_argument, NULL, 'o'},
+ {"new-options", required_argument, NULL, 'O'},
{"old-port", required_argument, NULL, 'p'},
{"new-port", required_argument, NULL, 'P'},
*** parseCommandLine(int argc, char *argv[])
*** 93,99
getcwd(os_info.cwd, MAXPGPATH);
! while ((option = getopt_long(argc, argv, "d:D:b:B:cgG:kl:p:P:u:v",
long_options, &optindex)) != -1)
{
switch (option)
--- 95,101
getcwd(os_info.cwd, MAXPGPATH);
! while ((option = getopt_long(argc, argv, "d:D:b:B:cgG:kl:o:O:p:P:u:v",
long_options, &optindex)) != -1)
{
switch (option)
*** parseCommandLine(int argc, char *argv[])
*** 141,146
--- 143,161
log_opts.filename = pg_strdup(optarg);
break;
+ case 'o':
+ old_cluster.pgopts = pg_strdup(optarg);
+ break;
+
+ case 'O':
+ new_cluster.pgopts = pg_strdup(optarg);
+ break;
+
+ /*
+ * Someday, the port number option could be removed and
+ * passed using -o/-O, but that requires postmaster -C
+ * to be supported on all old/new versions.
+ */
case 'p':
if ((old_cluster.port = atoi(optarg)) <= 0)
{
*** Options:\n\
*** 242,247
--- 257,264
-G, --debugfile=FILENAME output debugging activity to file\n\
-k, --linklink instead of copying files to new cluster\n\
-l, --logfile=FILENAMElog session activity to file\n\
+ -o, --old-options=OPTIONS old cluster options to pass to the server\n\
+ -O, --new-options=OPTIONS new cluster options to pass to the server\n\
-p, --old-port=OLDPORTold cluster port number (default %d)\n\
-P, --new-port=NEWPORTnew cluster port number (default %d)\n\
-u, --user=NAME clusters superuser (default \"%s\")\n\
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 0fb16ed..b7e4ea5
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*** typedef struct
*** 189,194
--- 189,195
char *pgdata; /* pathname for cluster's $PGDATA directory */
char *pgconfig; /* pathname for cluster's config file directory */
char *bindir; /* pathname for cluster's executable directory */
+ char *pgopts; /* options to pass to the server, like pg_ctl -o */
unsigned short port; /* port number where postmaster is waiting */
uint32 major_version; /* PG_VERSION of cluster */
char major_version_str[64]; /* string PG_VERSION of cluster */
diff --g
Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)
On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > The first thing caught my eye in existing GiST code is idea of > subtype_float. float8 has limited precision and can't respresent, for > example, varlena values good enough. Even if we have large int8 value > we can loose lower bits, but data distribution can be so that these > bits are valuable. Wouldn't it better to have function like > subtype_diff_float which returns difference between two values of > subtype as an float? Using of such function could make penalty more > sensible to even small difference between values, and accordingly more > relevant. The reason I did it that way is for unbounded ranges. With subtype_diff_float, it's difficult for the GiST code to differentiate between [10,) and [10,), because infinity minus anything is infinity. But when inserting the range [100,200), the penalty for the first one should be zero and the second one should have some positive penalty, right? Maybe we can still use subtype_diff_float by calling it on various pairs of bounds to come up with a reasonable cost? I'm open to suggestion. I'd just like to make sure that unbounded ranges are a part of the consideration. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] Fix Leaky View Problem
2011/10/8 Noah Misch : > On Sun, Oct 02, 2011 at 07:16:33PM +0200, Kohei KaiGai wrote: >> My preference is still also WITH(security_barrier=...) syntax. >> >> The arguable point was the behavior when a view is replaced without >> explicit WITH clause; >> whether we should consider it was specified a default value, or we >> should consider it means >> the option is preserved. >> If we stand on the viewpoint that object's attribute related to >> security (such as ownership, >> acl, label, ...) should be preserved, the security barrier also shall >> be preserved. >> On the other hand, we can never know what options will be added in the >> future, right now. >> Thus, we may need to sort out options related to security and not at >> DefineVirtualRelation(). >> >> However, do we need to limit type of the options to be preserved to >> security related? >> It is the first case that object with arbitrary options can be replaced. >> It seems to me we have no matter, even if we determine object's >> options are preserved >> unless an explicit new value is provided. > > Currently, you can predict how CREATE OR REPLACE affects a given object > characteristic with a simple rule: if the CREATE OR REPLACE statement can > specify a characteristic, we don't preserve its existing value. Otherwise, we > do preserve it. Let's not depart from that rule. > > Applying that rule to the proposed syntax, it shall not preserve the existing > security_barrier value. I think that is acceptable. If it's not acceptable, > we > need a different syntax -- perhaps CREATE SECURITY VIEW. > No. It also preserves the security-barrier flag, when we replace a view without SECURITY option. The only difference is that we have no way to turn off security-barrier flag explicitly, right now. The major reason why I prefer reloptions rather than "SECURITY" option is that allows to reuse the existing capability to store a property of relation. It seems to me both of syntax enables to achieve the rule to preserve flags when a view is replaced. >> Any other ideas? > > Suppose we permitted pushdown of unsafe predicates when the user can read the > involved columns anyway, a generalization of the idea from the first paragraph > of [1]. Would that, along with LEAKPROOF, provide enough strategies for > shoring > up performance to justify removing unsafe views entirely? > The problem was that we do all the access control decision at the executor stage, but planner has to make a plan prior to execution. So, it was also reason why we have tried to add LEAKPROOF flag to functions. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
