Re: [HACKERS] lowering privs in SECURITY DEFINER function
On Wed, 2011-04-06 at 18:33 -0300, Alvaro Herrera wrote: > (Consider, for example, that you may want to enable a user to run some > operation to which he is authorized, but you want to carry out some > privileged operation before/after doing so: for example, disable > triggers, run an update, re-enable triggers.) I'm not sure I understand the use case. If it's within one function, why not just do it all as the privileged user in the security definer function? The only reason I can think of it if you wanted to make the unprivileged operation arbitrary SQL. But in the example you give, with triggers disabled, it's not safe to allow the user to execute arbitrary operations. In other words, if you wrap an unprivileged operation inside of privileged operations, it seems like the unprivileged operation then becomes privileged. Right? 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] trivial patch: show SIREAD pids in pg_locks
On Fri, 2011-04-01 at 13:00 -0400, Dan Ports wrote: > While looking into a SSI bug, I noticed that we don't actually display > the pid of the holding transaction, even though we have that > information available. Is there a chance that the PID will reference a backend that has either terminated or is idle? That might be confusing. 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] Problem with pg_upgrade?
On Wed, 2011-03-30 at 16:46 -0400, Robert Haas wrote: > I don't really > understand why this is an issue in the first place, though. Surely we > must be setting the XID counter on the new cluster to match the one on > the old cluster, and migrating the relfrozenxid and datfrozenxid > settings, so why does it matter if someone runs vacuum freeze? Because autovacuum may run before those things are properly set, as Bruce said in the original email: "I am concerned that somehow autovaccum is running in frozen mode before I have restored the frozen xids for the table or database." I think some kind of hidden GUC might be the best option. I tend to agree that a third option to the "autovacuum" setting would be confusing. 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] Problem with pg_upgrade?
On Tue, 2011-03-29 at 21:43 -0300, Alvaro Herrera wrote:
> I think it would be better to have
> some sort of option to disable autovacuum completely which would be used
> only during pg_upgrade.
Sounds reasonable to me.
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] Problem with pg_upgrade?
On Tue, 2011-03-29 at 15:52 -0400, Bruce Momjian wrote:
> Does anyone have any other suggestions on how to make sure autovacuum
> does not run in freeze mode?
Can you run in single user mode?
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: Sync Rep and shutdown Re: [HACKERS] Sync Rep v19
On Fri, 2011-03-18 at 10:27 -0400, Robert Haas wrote:
> ERRCODE_(WARNING_?)REPLICATION_WAIT_CANCELLED
>
> ...which might have something to recommend it.
Works for me.
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: Sync Rep and shutdown Re: [HACKERS] Sync Rep v19
On Fri, 2011-03-18 at 08:27 -0400, Robert Haas wrote: > On Thu, Mar 17, 2011 at 6:00 PM, Jeff Davis wrote: > > On Wed, 2011-03-16 at 13:35 -0400, Robert Haas wrote: > >> 2. If a query cancel interrupt is received (pg_cancel_backend or ^C), > >> then cancel the sync rep wait and issue a warning before acknowledging > >> the commit. > > > > When I saw this commit, I noticed that the WARNING doesn't have an > > errcode(). It seems like it should -- this is the kind of thing that the > > client is likely to care about, and may want to handle specially. > > Should I invent ERRCODE_WARNING_TRANSACTION_NOT_REPLICATED? I think it's reasonable to invent a new code here. Perhaps use the word "synchronous" rather than "replicated", though? 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: Sync Rep and shutdown Re: [HACKERS] Sync Rep v19
On Wed, 2011-03-16 at 13:35 -0400, Robert Haas wrote: > 2. If a query cancel interrupt is received (pg_cancel_backend or ^C), > then cancel the sync rep wait and issue a warning before acknowledging > the commit. When I saw this commit, I noticed that the WARNING doesn't have an errcode(). It seems like it should -- this is the kind of thing that the client is likely to care about, and may want to handle specially. 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] Better estimates of index correlation
On Sun, 2011-03-13 at 19:40 -0400, Tom Lane wrote: > It strikes me that it'd be possible to have btvacuumcleanup directly > measure order correlation when it's processing a btree index, yielding a > reliable answer for any btree index regardless of number of columns. > We could do that by comparing the heap block numbers of adjacent > index entries' TIDs and counting the number of up-transitions (block > number larger than previous index entry) versus down-transitions (block > number smaller than previous). Link to previous discussion: http://archives.postgresql.org/pgsql-hackers/2008-10/msg01279.php 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: << >> -|- ops vs empty range
On Fri, 2011-03-11 at 08:37 -0500, Bruce Momjian wrote: > Where are we on this? The options are: 1. Rip out empty ranges. Several people have been skeptical of their usefulness, but I don't recall anyone directly saying that they should be removed. Robert Haas made the point that range types aren't closed under UNION: http://archives.postgresql.org/pgsql-hackers/2011-02/msg01045.php So the additional nice mathematical properties provided by empty ranges are not as important (because it wouldn't be perfect anyway). 2. Change the semantics. Erik Rijkers suggested that we define all operators for empty ranges, perhaps using NULL semantics: http://archives.postgresql.org/pgsql-hackers/2011-02/msg00942.php And Kevin Grittner suggested that there could be discrete ranges of zero length yet a defined starting point: http://archives.postgresql.org/pgsql-hackers/2011-02/msg01042.php 3. Leave empty ranges with the existing "empty set" semantics. Nathan Boley made a good point here: http://archives.postgresql.org/pgsql-hackers/2011-02/msg01108.php Right now it's #3, and I lean pretty strongly toward keeping it. Without #3, people will get confused when fairly simple operations fail in a data-dependent way (at runtime). With #3, people will run into problems only in situations where it is fairly dubious to have an empty range anyway (and therefore likely a real error), such as finding ranges "left of" an empty range. Otherwise, I'd prefer #1 to #2. I think #2 is a bad path to take, and we'll end up with a lot of unintuitive and error-prone operators. 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] Parallel make problem with git master
On Sat, 2011-03-05 at 18:33 -0500, Bruce Momjian wrote:
> I am seeing the following compile problem with gmake -j2:
>
For what it's worth, I'm still seeing this problem too:
http://archives.postgresql.org/pgsql-hackers/2010-12/msg00123.php
I can reproduce it every time.
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] Sync Rep v17
On Wed, 2011-02-23 at 22:42 -0800, Daniel Farina wrote: > Oh, yes, this reproduces past shutdowns/startups, and there's quite a > few txids before I catch up. I'm also comfortable poking around with > gdb (I have already recompiled with debugging symbols and > optimizations off and was poking around, especially at > MemoryContextStats(TopMemoryContext), but was not rewarded. Where is all of that memory going during recovery? Recovery shouldn't use much memory at all, as far as I can tell. What's even allocating memory at all? 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] disposition of remaining patches
On Fri, 2011-02-25 at 15:44 -0800, Josh Berkus wrote: > Hmmm, I don't follow this. The user can only disable syncrep for their > own transactions. If they don't care about the persistence of their > transaction post-failover, why should the DBA care? I think that's the difference between failover and switchover, right? At least Slony makes such a distinction, as well. 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] btree_gist (was: CommitFest progress - or lack thereof)
On Sat, 2011-02-12 at 18:53 +0300, Oleg Bartunov wrote: > > It sure seems like > > http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and > > should be improved, in general.. If this module is really still just a > > test bed for GiST, then perhaps it's not a big deal.. > > No, it's quite useful and used in many projects, since it's the only way > to create multicolumn gist indexes like (tsvector,date). +1 btree_gist is essentially required for exclusion constraints to be useful in a practical way. In fact, can you submit it for the next commitfest to be included in core? That would allow range types and exclusion constraints to be used out-of-the-box in 9.2. Only if you think it's reasonable to put it in core, of course. If extensions are easy enough to install, maybe that's not really necessary. 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: empty ranges
On Fri, 2011-02-11 at 15:14 -0500, Robert Haas wrote: > On Fri, Feb 11, 2011 at 3:03 PM, Jeff Davis wrote: > > Well, there is a certain amount of localized clarity, I will agree with > > that. The complexity comes when you accidentally rely on some > > transformation which seems logically sound, but could result in a > > transient empty range, which then throws an error. > > But by this argument you also need to support discontiguous ranges, don't you? > > I mean, if you want to insist that A intersect B has to still be a > legal range, what about A union B? > I responded to a similar question/point here: http://archives.postgresql.org/pgsql-hackers/2011-02/msg01073.php 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: empty ranges
On Fri, 2011-02-11 at 14:19 -0600, Kevin Grittner wrote: > Well, in the receipt number example there are multiple ranges in use > for each year, and ranges for multiple years. If we get to the idea > of a multi-ranges, this would be very handy for certain types of > reports -- especially for auditors. It's not that we can't do with > with discrete begin and end columns -- we do that now; but it seemed > a potentially beneficial use of ranges for us, if they can represent > the needed states. People already talk about these as ranges, just > in terms of the common English understanding of the word. I think that might indicate that the word "range" is a little too loose. The definition that I've been using is more like a mathematical interval. > Perhaps it was a mistake to get so concrete rather than conceptual > -- basically, it seems like it could be a useful concept for any > planned or scheduled range with an indeterminate end point, which > you want to "reserve" up front and record in progress until > complete. Maybe this is a range... would it be served by: (5, INF) or: [5, INF) ? That's already supported, and it means "all points greater than 5". > Of course, the *most* useful places for our shop to have ranges are > temporal. Many (most?) of those are situations where you start with > a range with an unknown end and later (often years later) fill in > the end of the range based on some event which finally closes it. > Again, two discreet dates with a null-capable end-date work, but I > can see where ranges could potentially be more powerful. Ranges support infinite boundaries, but do not support NULL (previous discussion concluded that NULL boundaries were likely to be confusing and served no obvious use case not handled by infinity). 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: empty ranges
On Fri, 2011-02-11 at 14:14 -0500, Robert Haas wrote: > > It's really that it has nice mathematical properties coming from set > > theory. Take the distributive law: > > > > A UNION (B INTERSECT C) = (A UNION B) INTERSECT (A UNION C) > > But the basic range type isn't even closed under UNION. An excellent point. Allow me to move the target a little: WHERE A && B AND A && C and: WHERE A && (B INTERSECT C) That seems like a logically sound transformation, but if (B INTERSECT C) is empty, it relies on the empty range for those two to be equivalent. And that would be a runtime error, caught during testing only if you're lucky. Now, I agree that lack of closure on UNION exhibits many of the problems that I am pointing out related to forbidding empty ranges. However, I'm not sure if that means we should give up on either. 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: empty ranges
On Fri, 2011-02-11 at 13:50 -0500, Robert Haas wrote: > On Fri, Feb 11, 2011 at 1:11 PM, Jeff Davis wrote: > > Similarly, "intersection" of ranges is somewhat analogous to > > multiplication of numbers. > > I had a feeling that we might be going in this direction. It strikes > me that this case is a bit like division by zero. Except that we do happen to allow the value zero and wait 'til someone divides by it before throwing an error. So I think that's more of a point toward allowing empty ranges than rejecting them. > It's kind of a > nuisance that dividing by zero throws an error and we COULD fix that > by making it return NULL or NaN or some new distinguished value DbZ. But empty ranges are actually quite well-defined, in a way similar to an empty set. * it can meaningfully result in a non-empty range at a later stage of computation * it increases the number of tautologies, rather than decreasing them like NULL I guess what I'm saying is that DbZ doesn't seem particularly useful to carry along, while and empty range plausibly is. > But then we'd have to define what happens when you feed DbZ into every > other operation in the system, and similarly here. If your point is that empty ranges need to be handled specially sometimes, I agree. That is the semantic cost which I identified in the original email. Are the benefits worth it? > If we define two > non-overlapping ranges as intersecting to NULL, or as throwing an > error, then everything else is clear after that. Well, there is a certain amount of localized clarity, I will agree with that. The complexity comes when you accidentally rely on some transformation which seems logically sound, but could result in a transient empty range, which then throws an error. 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: empty ranges
On Fri, 2011-02-11 at 13:08 -0600, Kevin Grittner wrote: > It makes more sense in the context of a range of some type with a > clearly defined granularity. Our accounting system, for example, > can assign a new range of receipt IDs for each calendar year. If > you want a variable to represent the receipts for traffic receipts > for 2012, you might, in preparation for the upcoming year, define > something to declare the range as '[12TR01,12TR01)'. When > the first receipt is assigned as 12TR01, that is updated to > '[12TR01,12TR02)'. Just as an off-the-cuff example. > > Basically, with a type having well-defined granularity, a [) range > could usefully represent, "start to last used", and start out empty. I think this is trying to make a range into something that's not. A range is a set of values with the additional constraint that there are no "gaps". Trying to incorporate a "start value" is adding extra information in there, and it's not really a part of the same algebra. It sounds more like a contiguous sequence with a "start value" and a "current value" to me. Sequences have other useful operations, like "next", and things like "overlaps" don't really seem to make sense (at least not in a practical way that I can tell). 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: empty ranges
On Fri, 2011-02-11 at 10:28 -0800, Josh Berkus wrote:
> I guess I'm having trouble tying the concept of empty ranges to any
> reality external to the database.
That's true, but in the same sense as zero has no meaning outside of the
database.
It's really that it has nice mathematical properties coming from set
theory. Take the distributive law:
A UNION (B INTERSECT C) = (A UNION B) INTERSECT (A UNION C)
If (B INTERSECT C) is empty, then the result should be A. With empty
ranges, that holds on both sides. Without them, it doesn't hold on the
left side.
People implicitly use this kind of logic all the time when constructing
queries. If one form works, and the other throws an error, it will cause
confusion.
> For example, what would the time range:
>
> '('15:15:00','15:15:00')'
>
> ... represent exactly? "A non-existant point in time which might or
> might not be near 3:15 PM"?
That's meaningless and will throw an error. An empty range is not
anchored at any particular point, so any two empty ranges are equal.
> BUT ... if I, in one of my applications, accidentally defined something
> as having the range '('15:15:00','15:15:00')', I would *want* the
> database to through an error and not accept it.
Absolutely. That kind of input should throw an error (and does).
> So, if we allow empty ranges of this kind, I would want a GUC for
> "allow_empty_ranges".
I think that would be the least desirable option. If we don't like empty
ranges, let's prohibit them entirely.
Or, there are always check constraints...
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
[HACKERS] Range Types: empty ranges
Do we want empty ranges? The philosophy is that they are essentially the "zero" value of any range type. Like the number zero, it allows closure over operations that would otherwise return an error. For instance, the number zero is useful because you can do things like: f(x) = 5x + 3; And even if x is zero, the function is still defined, and even produces a more "real" number like 3. Sure, when you try to divide by zero, you have a problem, but otherwise it works. Similarly, "intersection" of ranges is somewhat analogous to multiplication of numbers. I have a feeling that there are a lot of applications that might use intersection in combination with other operators, like overlaps or contains. If we do allow empty ranges, I think it will be seamless in most of those situations because "overlaps" and "contains" are well-defined for empty ranges. But if we don't allow empty ranges, I suspect that it will cause some user surprise, because depending on the order of operations an empty range may be created (causing an error) or not. The cost, of course, is that not all operations are well-defined for empty ranges. I think those are mostly operators like those mentioned in the other thread: ">>" (strictly right of), "<<" (strictly left of), and "-|-" (adjacent); and perhaps "&>" and "&<". These are probably used a little less frequently, and should probably not be used in a context where empty ranges are permitted (if they are, it's likely a mistake and an error should be thrown). My feeling is that we should let the operation proceed as far as it is well-defined, and no further; and I think that means empty ranges should be allowed. Thoughts? Do the benefits outweigh the costs? 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: << >> -|- ops vs empty range
On Fri, 2011-02-11 at 12:03 -0500, Robert Haas wrote: > For what it's worth, my completely uninformed opinion is that > comparison operators shouldn't error out. I haven't read the patch so > I'm not sure what those operators are defined to do, though. ">>" means "strictly right of" "<<" means "strictly left of" "-|-" means "adjacent" (touching but not overlapping) I'm open to suggestion about how those behave with empty ranges. 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: << >> -|- ops vs empty range
On Fri, 2011-02-11 at 15:09 +0100, Erik Rijkers wrote:
> On Wed, February 9, 2011 09:35, Jeff Davis wrote:
> > Updated patch.
> >
>
> The operators << >> and -|- have the following behavior with empty ranges:
>
> testdb=# select '-'::int4range << range(200,300);
> ERROR: empty range
> testdb=# select '-'::int4range >> range(200,300);
> ERROR: empty range
> testdb=# select '-'::int4range -|- range(200,300);
> ERROR: empty range
>
> I'm not sure if that is deliberate behavior, but they seem
> almost bugs to me.
It's deliberate, but it looks like the error messages could use some
improvement.
> Wouldn't it be better (and more practical) if these would
> return false (or perhaps NULL, for 'unknown') ?
I'm hesitant to return NULL when the inputs are known.
If we were to define these functions for empty ranges, I would think
they would all return true.
"<<" and ">>" ("strictly left of" and "strictly right of", respectively)
could be seen to start out as true and return false if it finds a point
overlapping or on the other side.
The primary use case for "-|-" (adjacent) is to see if your ranges are
contiguous and non-overlapping. For empty ranges, that seems to be true.
I'm not disagreeing with your interpretation really. I think that
different people will assume different behavior, and so it's more likely
to cause confusion. An error early on will allow them to do something
like:
CASE WHEN myrange? THEN myrange -|- range(10,20) ELSE TRUE END
So that they (and anyone who reads their query) can see explicitly
what's happening, without looking in the manual for details.
I'm open to suggestion, however. If we can get a reasonable consensus on
the values these functions should return, I'll change it.
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 Type constructors
On Thu, 2011-02-10 at 13:07 -0500, Robert Haas wrote: > According to our documentation[1], RANGE is reserved in SQL:2008 and > SQL:2003, which makes it more imaginable to reserve it than it would > be otherwise. Oh, interesting. > I believe that in a previous email you mentioned that > you were hoping to implement RANGE JOIN, and I will just note that the > restrictions of the grammar require that any keyword that immediately > follows the previous expression and precedes JOIN must be fully > reserved. I'm not sure if you meant that a range join would literally > use the syntax RANGE JOIN, but if so then you're going to have to > argue for fully reserving RANGE anyway, in which case there'd be no > special reason not to allow RANGE [1,10) to mean just that. On the > other hand, if a RANGE JOIN just means a regular join on some funky > operator, and there's no other reason to reserve range, I wouldn't do > it just to get a nicer syntax here. It's mostly just a regular join on a funky operator. We may want that operator to allow a new plan (range merge join); but I think we can determine that it's a range join from the use of the operator. I'll have to look into that more. > Have you done investigation of what RANGE is used to mean in the SQL > spec? Is what you're implementing (a) spec, (b) similar idea, but not > the spec, or (c) something completely different? I'm guessing (c) but > I have no idea what the spec is using it for. (c) was my intention. I did take a brief look at the spec a while back, but I'll take a more detailed look. I think it only has to do with window specifications. This might solve the constructor problem nicely if we could do things like: RANGE[10,20) But I have a feeling that will either cause a bizarre problem with the grammar, or someone will think it's not very SQL-like. 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] postponing some large patches to 9.2
On Thu, 2011-02-10 at 09:46 -0500, Robert Haas wrote: > On Tue, Feb 8, 2011 at 7:58 PM, Jeff Davis wrote: > > On the flip side, if we don't provide review to WIP patches during the > > 3rd commitfest, how do we expect to get anything close to committable on > > the 1st commitfest of the next cycle? > > I'm not sure exactly what you're going for here, because I don't think > I've ever proposed any special treatment of patches in the third > CommitFest, I actually meant 4th (this one). I forgot that the July one was actually a part of the 9.1 cycle. > But if > you were to say that WIP patches *in general* get a lot less review > than non-WIP patches, I would agree with you. > > To some extent, I think that's inevitable. It's not fun to review WIP > patches. Agreed, but it doesn't really apply to this situation. There was still a week left, and the reviewer was still reviewing. So I found it jarring when you said that it had received enough review, and bounced it. In my opinion, if we're going to entertain WIP patches during a commitfest, we shouldn't bounce them early for being WIP. We can bounce them for other causes, like "waiting on author" or "we couldn't find a reviewer" or "we're out of time". > I've found that it's > nearly always better to post specific questions that you want to know > the answer to, rather than a patch where people have to guess what > parts you want feedback on. Well, I've certainly posted some specific questions. I don't expect to get an answer to all of them right away, and certainly many have been answered -- but I didn't just throw the code out and wait. For instance: http://archives.postgresql.org/message-id/1297230650.27157.398.camel@jdavis Anyway, I don't think any of this affected the patch, I was just surprised. I'll leave it at that, because I'm sure you're busy wrapping up this commitfest. 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 (catversion.h)
On Thu, 2011-02-10 at 15:38 +0100, Erik Rijkers wrote: > I've removed the change to catversion.h (18 lines, starting at 4985) from the > patch file; then it > applies cleanly. I should mention that the last patch changed the representation to be more compact. So, if you have any existing test data it will need to be reloaded to work with the latest. 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 (catversion.h)
On Thu, 2011-02-10 at 12:04 -0500, Tom Lane wrote: > "Erik Rijkers" writes: > > On Wed, February 9, 2011 09:35, Jeff Davis wrote: > >> Updated patch. > > > I just wanted to mention that this latest patch doesn't quite apply as-is, > > because of catversion changes. > > Just a note: standard practice is for submitted patches to *not* touch > catversion.h. The committer will add that change before committing. > Otherwise, it's just guaranteed to cause merge problems such as this > one. (It's not unreasonable to mention the need for a catversion bump > in the description of the patch, if you think the committer might not > realize it.) OK, I'll remove that then. I originally put it there so that I wouldn't mix up data directories with a patch I'm reviewing, but I agree that it seems easier this way. 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 - efficiency
On Wed, 2011-02-09 at 18:07 -0500, Chris Browne wrote:
> rangetest@localhost-> create index i2 on some_data (range(whensit));
> CREATE INDEX
If you make this a GiST index, it should work.
The rewrites so that it can use a btree are an interesting idea though.
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 - efficiency
On Wed, 2011-02-09 at 16:20 -0500, Chris Browne wrote:
> rangetest@localhost-> explain analyze select * from some_data where
> '[2010-01-01,2010-02-01)'::daterange @> whensit;
>QUERY PLAN
> -
> Seq Scan on some_data (cost=0.00..634.00 rows=1 width=8) (actual
> time=1.045..111.739 rows=390 loops=1)
>Filter: ('[ 2010-01-01, 2010-02-01 )'::daterange @> whensit)
> Total runtime: 111.780 ms
> (3 rows)
>
> This, alas, reverts to a seq scan on the table, rather than restricting
> itself to the tuples of interest.
>
> I realize that, after a fashion, I'm using this backwards. But when I'm
> doing temporal stuff, that tends to be the pattern:
Yes. The index is a btree index on a normal column, so range types can't
exactly help with that directly -- except maybe as a rewrite like you
say.
One thing you might try is a functional index on (range(whensit)) and
then do: where '...' @> range(whensit).
Does that work for you?
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] postponing some large patches to 9.2
On Tue, 2011-02-08 at 15:10 -0500, Chris Browne wrote:
> It's more than a bit sad... The RangeType change has the massive merit
> of enabling some substantial development changes, where we can get rid
> of whole classes of comparison clauses, and hopefully whole classes of
> range errors. That was my favorite would-be feature for 9.1.
I appreciate the support.
If you take the feature for a quick spin before the next commitfest,
that would be a big help. If I get it in the first commitfest of 9.2
that may mean some follow-up features, like RANGE KEYs/FKs, and maybe
even RANGE JOIN might have a chance for 9.2 as well. Or, maybe some
other features might find it useful, like partitioning or audit logs.
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
Updated patch. Changes: * Addressed Erik's review comments. * Fixed issue with "range @> elem" found by Erik. * Merged with latest HEAD * Changed representation to be more efficient and more robust (could use some testing though, because I just did this tonight) TODO: * send/recv -- just noticed this tonight, no reason not to do it Open Items: * Maybe typmod * grammar -- ask for btree opclass, or compare function? * catalog -- store btree opclass, or compare function? * should non-superusers be able to create range types? * constructor issues I just posted about * SQL length function --immutable/stable/volatile? As always, my repo is here: http://git.postgresql.org/gitweb?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes Regards, Jeff Davis rangetypes-20110208.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
[HACKERS] Range Types - representation and alignment
After some significant prior discussion: Here is what I've found: Doing the simple thing is extremely wasteful. Let's take TSRANGE, for instance: 4 bytes type oid 1 flag byte 8 bytes lower bound 8 bytes upper bound But when constructing the value itself, it starts off with VARHDRSZ bytes. It may later be compacted to a short (1 byte) header, but it starts off as 4. So that means: 4 bytes VARHDRSZ 4 bytes type oid 1 flag byte 7 pad bytes to get back on a 'd' align boundary 8 bytes lower bound 8 bytes upper bound Total: 32 bytes. When compacted into the tuple, it might be 29. We can't skip those pad bytes, because we need to honor the subtype's alignment. If we move the flag byte to the end, the representation works out much better: 4 bytes VARHDRSZ 4 bytes type oid 8 bytes lower bound 8 bytes upper bound 1 flag byte Total: 25 bytes, turns into about 22 bytes when compacted into the tuple. It's a little awkward to read that way, but the savings are worth it. The flag byte is necessary to know whether there are lower and/or upper bounds, so we need to peek ahead to length - 1, and then continue scanning forward through the attributes. So, I'll implement this approach. 22 bytes represents 37.5% overhead above the good ol' PERIOD data type (a lean 16 bytes), but we can make up some of that if using unbounded ranges. For instance, a half-open range like "[5, INF)" would only take 14 bytes. 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 Type constructors
On Wed, 2011-02-09 at 15:39 +0900, Itagaki Takahiro wrote: > On Wed, Feb 9, 2011 at 14:50, Jeff Davis wrote: > > 1. > > The obvious constructor would be: > > range(1, 10) > > But is that [1, 10), (1, 10], (1, 10), or [1, 10]? We need to support > > all 4, and it's not obvious how to do that easily. > > here is the same issue in table partitioning. Also, We might use the > syntax for our partitioning in the future. Just for reference, > DB2 uses EXCLUSIVE and INCLUSIVE keywords to specify boundaries. > > CREATE TABLE ... PARTITION BY RANGE (...) > (STARTING 0 EXCLUSIVE ENDING 100 INCLUSIVE) Interesting. It needs to be usable in normal expressions, however, so it may require some adaptation. That's how arrays do it: there's a special Expr node that represents an array expression. Maybe the same thing could be used for range types, but I fear that there may be some grammar conflicts. I doubt we'd want to fully reserve the keyword "range". 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] REVIEW Range Types
On Tue, 2011-02-08 at 20:43 +0100, Erik Rijkers wrote: > -- > Documentation: > -- > > Section 9.18: > table 9-42. range functions: > The following functions are missing (I encountered them in the regression > tests): > contained_by() > range_eq() I opted not to document functions that mostly exist to implement operators. Some people might prefer the names, but then they don't benefit from GiST index searches. So, I left this as-is for now. > section 'Constructing Ranges' (8.16.6): > In the code example, remove the following line: > "-- the int4range result will appear in the canonical format" > it doesn't make sense there. At this place "canonical format" has not been > discussed; > maybe it is not even discussed anywhere. Thank you. I have added a forward reference. > also (same place): >'where "_" is used to mean "exclusive" and "" is used to mean > "inclusive".' > should be: >'where "_" is used to mean "exclusive" and "i" is used to mean > "inclusive".' Thank you, fixed. > And btw: it should mention here that the range*inf* functions, > an underscore to separate 'range' from the rest of the function name, e.g.: >range_linfi_() => infinite lower bound, inclusive upper bound I tried to clarify that section. Thank you for the review! Updated patch forthcoming. 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
[HACKERS] Range Type constructors
There are two issues I'd like to discuss related to constructing range types from other values. 1. The obvious constructor would be: range(1, 10) But is that [1, 10), (1, 10], (1, 10), or [1, 10]? We need to support all 4, and it's not obvious how to do that easily. The solution that I came up with is not particularly clean, but is quite practical: range(1, 10) -> [1, 10) range__(1, 10) -> (1, 10) range_i(1, 10) -> (1, 10] rangei_(1, 10) -> [1, 10) rangeii(1, 10) -> [1, 10] The last two letters refer to the lower and upper bounds, respectively. A "i" means "inclusive" and an "_" means "exclusive". range() is an alias for rangei_(), because that's the most common representation to use. I realize this isn't a clean solution, and better ideas are welcome. This one actually is quite natural to use I think: short to type and easy to remember (for me at least ;). It gets a little stranger for trying to construct unbounded ranges from other values. Again, there are four possibilities: range_uinfi(5) -> [5, INF) range_uinf_(5) -> (5, INF) range_linfi(5) -> (-INF, 5] range_linf_(5) -> (-INF, 5) And again, not exactly clean, but they work. Constructing a singleton range is easy, fortunately, because only something like "[5,5]" makes sense, "[5,5)" doesn't. So there's just a single-argument version of range: range(5) -> [5,5] 2. The second issue is with the type system. In order for the polymorphic constructors to work, they need to be able to determine the data types of their inputs to construct the range. I am using get_fn_expr_argtype() to accomplish that, but it's not always guaranteed to work. That was the problem Erik ran into: the "range @> elem" operator was implicitly constructing a range on the right side based on the type of the right operand; but was being called in contexts where the types aren't known (like the selectivity estimator). The fix was easy: get the type from the range operand (which is actually stored with the range). But that fix won't work for the constructors above, because there is no range argument to start from. So: in what contexts are functions called that get_fn_expr_argtype() might fail; and are the above constructors at risk for that? Is there a better way? 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
On Tue, 2011-02-08 at 09:10 -0800, Jeff Davis wrote: > On Mon, 2011-02-07 at 18:23 +0100, Dimitri Fontaine wrote: > > I would think > > > > CREATE TYPE foo AS RANGE (bar) USING (btree_ops); > > > > The USING clause is optional, because you generally have a default btree > > opclass for the datatype. > > There are other options, like "CANONICAL", so where do those fit? > > If CREATE TYPE already has an options list, it seems a little strange to > add grammar to support this feature. "USING" doesn't seem to mean a lot, > except that we happen to use it in other contexts to mean "operator > class". For the user-facing part, how about just passing it as a parameter called "SUBTYPE_OPCLASS"? It sounds a little on the "internal detail" side, but so do some other type definition parameters. As for the catalog, I'm inclined to leave the compare function in there directly and just add a dependency on the opclass. That way, it's only one syscache lookup rather than two, to get the compare function oid. Then again, perhaps that doesn't matter anyway. Thoughts? 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] postponing some large patches to 9.2
On Tue, 2011-02-08 at 14:25 -0500, Robert Haas wrote: > > The patch is a million little decisions: names, catalog structure, > > interface, representation, general usability, grammar, functionality, > > etc. Without some checkpoint, the chances that everyone agrees with all > > of these decisions at the beginning of the next commitfest is zero. > > > > Is the commitfest not the right place to do this? If not, then when? > > That's a fair question, and I do understand the difficulty. I think a > CommitFest is the right place to do that. On the other hand, as I'm > sure you realize, I'm not keen to hold up 9.1beta for a feature that > isn't going to be committed until 9.2. I'm not asking you to hold it up. Just don't mark it "returned with feedback" when that is not true, and a week still remains. Erik is still looking at it, and that might generate some interesting discussion. > ...everyone who has been thinking about doing something for the release > wakes up and submits it, often half-finished, often at the very last > minute. On the flip side, if we don't provide review to WIP patches during the 3rd commitfest, how do we expect to get anything close to committable on the 1st commitfest of the next cycle? > Although it doesn't > feel like it at the moment, we have actually made great strides in > absorbing large patches. I agree completely. 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] postponing some large patches to 9.2
On Tue, 2011-02-08 at 11:56 -0500, Robert Haas wrote: > It's a 5400 line patch that wasn't completed until the middle of the > current CommitFest. Nobody has ever submitted a major feature patch > of that size that got done in a single CommitFest, to my recollection, > or even half that size. My concern is that, aside from code, my patch didn't make much progress this commitfest. And the code progress was mostly me working through my own TODO list on things like GiST support -- which didn't represent any real decisions, it was mostly just a matter of code. 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] postponing some large patches to 9.2
On Tue, 2011-02-08 at 06:57 -0500, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: > > - Range Types. This is a large patch which was submitted for the > > first time to the last CommitFest of the cycle, and the first version > > that had no open TODO items was posted yesterday, three-quarters of > > the way through that last CommitFest. Some good review has been done. > > While more is probably needed, I think we should feel good about > > what's been accomplished and mark this one Returned with Feedback. > > I don't agree w/ punting Range Types. Range Types were discussed as far > back as the 2010 developer meeting, were discussed quite a bit again > starting in October and throughout the fall, and Jeff has regularly > been posting updates to it. Given how thorough Jeff is, my feeling is > that this patch is more than ready for beta. I appreciate the sentiment, but in addition to some cleanup, any patch like this at least requires some discussion. It's a language change we'll be supporting for a long time. At minimum, we're a couple hundred emails shy of a real consensus on the naming ;) 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] postponing some large patches to 9.2
On Mon, 2011-02-07 at 22:37 -0500, Robert Haas wrote: > - Range Types. This is a large patch which was submitted for the > first time to the last CommitFest of the cycle, and the first version > that had no open TODO items was posted yesterday, three-quarters of > the way through that last CommitFest. Some good review has been done. > While more is probably needed, I think we should feel good about > what's been accomplished and mark this one Returned with Feedback. I submitted this clearly marked WIP, so I expected that it would likely be pushed to 9.2. However, I don't feel like I have the kind of feedback that will help me get it committed next commitfest. I did get some review, and that was helpful, but it was mostly on isolated details. The patch is a million little decisions: names, catalog structure, interface, representation, general usability, grammar, functionality, etc. Without some checkpoint, the chances that everyone agrees with all of these decisions at the beginning of the next commitfest is zero. Is the commitfest not the right place to do this? If not, then when? 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
On Mon, 2011-02-07 at 18:23 +0100, Dimitri Fontaine wrote: > I would think > > CREATE TYPE foo AS RANGE (bar) USING (btree_ops); > > The USING clause is optional, because you generally have a default btree > opclass for the datatype. There are other options, like "CANONICAL", so where do those fit? If CREATE TYPE already has an options list, it seems a little strange to add grammar to support this feature. "USING" doesn't seem to mean a lot, except that we happen to use it in other contexts to mean "operator class". 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] WIP: RangeTypes
On Mon, 2011-02-07 at 20:32 +0200, Peter Eisentraut wrote: > Have you considered a grammar approach like for arrays, so that you > would write something like > > CREATE TABLE ... ( > foo RANGE OF int > ); > > instead of explicitly creating a range type for every scalar type in > existence? I think that that might be easier to use in the common case. It would be nice, but the type system just isn't powerful enough to express things like that right now, as far as I can tell. That works for arrays because every type in PG has a second pg_type entry for the array type. I don't think we want to do something similar for range types -- especially if there are alternative range types for a given base type. 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
On Mon, 2011-02-07 at 13:33 +0100, Dimitri Fontaine wrote: > Hi, > > Jeff Davis writes: > > * Should pg_range reference the btree opclass or the compare function > > directly? > > I would say yes. We use the btree opclass in other similar situations. Ok, but what should the parameter to CREATE TYPE ... AS RANGE be then? CREATE TYPE foo AS RANGE ( SUBTYPE = ... SUBTYPE_BTREE_OPERATOR_CLASS = ... ); is a little verbose. Ideas? > Is there any reason to restrict who's get to use the feature? I don't > see any… Mostly just paranoia. If they define a strange canonical function, maybe that would cause a problem. Then again, they would have to define that in C to cause a problem anyway. I'll leave it as superuser-only for now, and see if anyone else raises potential problems. > > * Should the SQL (inlinable) function "length", which relies on > > polymorphic "-", be immutable, strict, or volatile? > > I would think stable: polymorphic means that the function > implementing the "-" operator depends on the argument. I don't recall > that it depends on them in a volatile way… except if you change the > operator definition, which is possible to do (so not immutable). I was concerned about someone defining "-" with a stable or volatile function as the definition. I'm not sure if that is a reasonable concern or not. > > * Later we might consider whether we should include btree_gist in > > core, to make range types more useful with exclusion constraints > > out-of-the-box. This should be left for later, I'm just including this > > for the archives so it doesn't get lost. > > I would expect the extension to have something to offer here. Yes. With extensions and PGXN, I would hope that installing btree_gist would not be much of a problem. However, I eventually intend to submit features like "RANGE KEY", a language extension that would need something like btree_gist to work very well at all. Technically btree_gist is not required, but in practice it is necessary to use ranges and exclusion constraints together effectively. 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 - cache lookup failed for function
On Sun, 2011-02-06 at 20:10 +0100, Erik Rijkers wrote: > I was trying > where intrange @> integer > > which admittedly is not in the documentation, > but does already half work, and would be really > convenient to have. As it stands the construct > seems to fail after ANALYZE, when there is more > than 1 row: Thank you for the report! I actually did make some mention of that in the documentation, albeit brief (in the operators table, using timestamp as an example). I have a fix for it. There may still be an issue with the constructors like range(1,2), so I'll look into it a little more, but an updated patch should come soon. 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] SSI patch version 14
On Sat, 2011-02-05 at 14:43 -0600, Kevin Grittner wrote: > "Kevin Grittner" wrote: > > > So now that I'm sure we actually do need code there, I'll add it. > > In working on this I noticed the apparent need to move two calls to > PredicateLockTuple a little bit to keep them inside the buffer lock. > Without at least a share lock on the buffer, it seems that here is a > window where a read could miss the MVCC from a write and the write > could fail to see the predicate lock. Please see whether this seems > reasonable: > > http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=7841a22648c3f4ae46f674d7cf4a7c2673cf9ed2 What does PredicateLockTuple do that needs a share lock? Does a pin suffice? 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
[HACKERS] Range Types
New patch. All known TODO items are closed, although I should do a cleanup pass over the code and docs. Fixed in this patch: * Many documentation improvements * Added INT8RANGE * Renamed PERIOD[TZ] -> TS[TZ]RANGE * Renamed INTRANGE -> INT4RANGE * Improved parser's handling of whitespace and quotes * Support for PL/pgSQL functions with ANYRANGE arguments/returns * Make "subtype_float" function no longer a requirement for GiST, but it should still be supplied for the penalty function to be useful. There are some open issues remaining, however: * Is typmod worth doing? I could complete it pretty quickly, but it involves introducing a new Node type, which seems a little messy for the benefit. * Should pg_range reference the btree opclass or the compare function directly? * Should subtype_cmp default to the default btree opclass's compare function? * Right now only superusers can define a range type. Should we open it up to normal users? * Should the SQL (inlinable) function "length", which relies on polymorphic "-", be immutable, strict, or volatile? * Later we might consider whether we should include btree_gist in core, to make range types more useful with exclusion constraints out-of-the-box. This should be left for later, I'm just including this for the archives so it doesn't get lost. Regards, Jeff Davis rangetypes-20110205.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] WIP: RangeTypes
On Sun, 2011-01-30 at 17:14 -0500, Tom Lane wrote:
> Jeff Davis writes:
> > On Sun, 2011-01-30 at 02:55 +, Thom Brown wrote:
> >> postgres=# select '[18,20]'::numrange @> 19;
> >> ERROR: operator does not exist: numrange @> integer
> >> LINE 1: select '[18,20]'::numrange @> 19;
> >> ^
> >> HINT: No operator matches the given name and argument type(s). You
> >> might need to add explicit type casts.
>
> > It's because it doesn't know the type on the right side, and assumes
> > it's an int4.
>
> Well, yeah, it is an int4. The question ought to be phrased "why does
> the parser fail to promote the int4 to numeric?". There might be some
> excuse for an "operator is not unique" here, but I don't understand the
> above failure --- it should be able to use an implicit coercion from
> int4 to numeric.
The problem exists for arrays, as well, so I think this is just a
limitation of the type system.
Regards,
Jeff Davis
postgres=# select ARRAY[1.4,1.5,1.6]::numeric[] || 5.0;
?column?
---
{1.4,1.5,1.6,5.0}
(1 row)
postgres=# select ARRAY[1.4,1.5,1.6]::numeric[] || 5;
ERROR: operator does not exist: numeric[] || integer
LINE 1: select ARRAY[1.4,1.5,1.6]::numeric[] || 5;
^
HINT: No operator matches the given name and argument type(s). You
might need to add explicit type casts.
--
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] SSI patch version 14
On Tue, 2011-02-01 at 11:01 -0600, Kevin Grittner wrote:
> My compiler doesn't.
Strange. Maybe it requires -O2?
> Would it make sense to elog here, rather than
> Assert? I'm not clear on the rules for that.
elog looks fine there to me, assuming we have the default case. I'm not
100% clear on the rules, either. I think invalid input/corruption are
usually elog (so they can be caught in non-assert builds); but other
switch statements have them as well ("unrecognized node...").
> A small push dealing with all the above issues and adding a little
> to comments:
>
> http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=538ff57691256de0341e22513f59e9dc4dfd998f
>
> Let me know if any of that still needs work to avoid confusion and
> comply with PostgreSQL coding conventions. Like I said, I'm not
> totally clear whether elog is right here, but it seems to me a
> conceptually similar case to some I found elsewhere that elog was
> used.
Looks good. It also looks like it contains a bugfix for subtransactions,
right?
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] SSI patch version 14
On Mon, 2011-01-31 at 17:55 -0600, Kevin Grittner wrote: > http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=6360b0d4ca88c09cf590a75409cd29831afff58b > > With confidence that it works, I looked it over some more and now > like this a lot. It is definitely more readable and should be less > fragile in the face of changes to MVCC bit-twiddling techniques. Of > course, any changes to the HTSV_Result enum will require changes to > this code, but that seems easier to spot and fix than the > alternative. Thanks for the suggestion! One thing that confused me a little about the code is the default case at the end. The enum is exhaustive, so the default doesn't really make sense. The compiler warning you are silencing is the uninitialized variable xid (right?), which is clearly a spurious warning. Since you have the "Assert(TransactionIdIsValid(xid))" there anyway, why not just initialize xid to InvalidTransactionId and get rid of the default case? I assume the "Assert(false)" is there to detect if someone adds a new enum value, but the compiler should issue a warning in that case anyway (and the comment next to Assert(false) is worded in a confusing way). This is all really minor stuff, obviously. Also, from a code standpoint, it might be possible to early return in the HEAPTUPLE_RECENTLY_DEAD case where visible=false. It looks like it will be handled quickly afterward (at TransactionIdPrecedes), so you don't have to change anything, but I thought I would mention it. > Having gotten my head around it, I embellished here: > > http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=f9307a41c198a9aa4203eb529f9c6d1b55c5c6e1 > > Do those changes look reasonable? None of that is really > *necessary*, but it seemed cleaner and clearer that way once I > looked at the code with the changes you suggested. Yes, I like those changes. 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] SSI patch version 15
On Mon, 2011-01-31 at 23:35 +0200, Heikki Linnakangas wrote: > Yeah, I can commit this. Jeff, are you satisfied with this patch now? > I'm glad you're reviewing this, more eyeballs helps a lot with a big > patch like this. I think the patch is very close. I am doing my best in my free time to complete a thorough review. If you have other patches to review/commit then I will still be making progress reviewing SSI. However, I would recommend leaving yourself some time to think on this one if you don't already understand the design well. 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] SSI patch version 14
On Mon, 2011-01-31 at 15:30 -0600, Kevin Grittner wrote: > I'll try to set this up and see if I can get it to pass the check > and dcheck make targets. Can we assume that the performance impact > would be too small to matter when we know for sure that hint bits > have already been set? I think that's a safe assumption. If there is some kind of noticeable difference in conflict rates or runtime, that probably indicates a bug in the new or old code. 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] SSI patch version 14
On Mon, 2011-01-31 at 14:38 -0600, Kevin Grittner wrote: > It is at least as likely that I'm missing something. If I'm > following you, we're talking about these 24 lines of code, where > "valid" is the what was just returned from > HeapTupleSatisfiesVisibility: Yes. > (1) Do you see a case where this would do the wrong thing? Can you > describe that or (even better) provide a test case to demonstrate > it? No, I don't see any incorrect results. > (2) I haven't gotten my head around how HTSV helps or is even the > right thing. It primarily just encapsulates the access to the tuple header fields. I think that avoiding the messy logic of hint bits, tuple locks, etc., is a significant win for readability and maintainability. > If I want to try the switch statement from your recent > post, what should I use as the OldestXmin value on the call to HTSV? I believe RecentGlobalXmin should work. And I don't think the original switch statement I posted did the right thing for HEAPTUPLE_LIVE. I think that case needs to account for the visible flag (if it's live but not visible, that's the same as insert-in-progress for your purposes). 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] SSI patch version 14
On Mon, 2011-01-31 at 13:55 -0600, Kevin Grittner wrote: > Jeff Davis wrote: > > > I don't think this function really cares about the visibility with > > respect to the current snapshot, right? > > What it cares about is whether some other particular top level > transaction wrote a tuple which we *would* read except that it is > not visible to us because that other top level transaction is > concurrent with ours. Or a tuple that you *are* reading, but is being deleted concurrently, right? Or has been deleted by an overlapping transaction? > If so, we want to flag a read-write conflict > out from our transaction and in to that other transaction. It still seems like HTSV would suffice, unless I'm missing something. I think "visible" is still needed though: it matters in the cases HEAPTUPLE_RECENTLY_DEAD and HEAPTUPLE_LIVE. For the former, it only allows an early exit (if !visible); but for the latter, I think it's required. 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] SSI patch version 14
On Mon, 2011-01-31 at 13:32 -0600, Kevin Grittner wrote: > Ah, now I see what you're talking about. Take a look at where that > "valid" flag come from -- the CheckForSerializableConflictOut are > all place right after calls to HeapTupleSatisfiesVisibility. The > "valid" value is what HeapTupleSatisfiesVisibility returned. Is it > possible that the hint bits will not be accurate right after that? > With that in mind, do you still see a problem with how things are > currently done? Oh, ok. The staleness of the hint bit was a fairly minor point though. Really, I think this should be using HTSV to separate concerns better and improve readability. My first reaction was to try to find out what the function was doing that's special. If it is doing something special, and HTSV is not what you're really looking for, a comment to explain would be helpful. As an example, consider that Robert Haas recently suggested using an infomask bit to mean frozen, rather than actually removing the xid, to save the xid as forensic information. If that were to happen, your code would be reading an xid that may have been re-used. 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] SSI patch version 14
On Mon, 2011-01-31 at 07:26 -0600, Kevin Grittner wrote: > > And why are you reading the infomask directly? Do the existing > > visibility functions not suffice? > > It's possible we re-invented some code somewhere, but I'm not clear > on what code from this patch might use what existing function. Could > you provide specifics? In CheckForSerializableConflictOut(), it takes a boolean "valid". Then within the function, it tries to differentiate: 1. Valid with no indication that it will be deleted. 2. Valid, but delete in progress 3. Invalid For #1, you are using the hint bit (not the real transaction status), and manually checking whether it's just a lock or a real delete. For #2 you are assuming any other xmax means that the transaction is in progress (which it may not be, because the hint bit might not be set for some time). I assume that will cause additional false positives. If you used HeapTupleSatisfiesVacuum(), you could do something like: case HEAPTUPLE_LIVE: return; case HEAPTUPLE_RECENTLY_DEAD: case HEAPTUPLE_DELETE_IN_PROGRESS: xid = HeapTupleHeaderGetXmax(tuple->t_data); break; case HEAPTUPLE_INSERT_IN_PROGRESS: xid = HeapTupleHeaderGetXmin(tuple->t_data); break; case HEAPTUPLE_DEAD: return; This is not identical to what's happening currently, and I haven't thought this through thoroughly yet. For instance, "recently dead and invalid" would be checking on the xmax instead of the xmin. Perhaps you could exit early in that case (if you still keep the "valid" flag), but that will happen soon enough anyway. I don't think this function really cares about the visibility with respect to the current snapshot, right? It really cares about what other transactions are interacting with the tuple and how. And I think HTSV meets that need a little better. > > The biggest issue on my mind is what to do about Hot Standby. The > > authors have a plan, but there is also some resistance to it: > > > > > http://archives.postgresql.org/message-id/23698.1295566...@sss.pgh.pa.us > > > > We don't need a perfect solution for 9.1, but it would be nice if > > we had a viable plan for 9.2. > > I don't recall any real opposition to what I sketched out in this > post, which came after the above-referenced one: > > http://archives.postgresql.org/message-id/4d39d5ec022500039...@gw.wicourts.gov > > Also, that opposition appears to be based on a misunderstanding of > the first alternative, which was for sending at most one snapshot per > commit or rollback of a serializable read write transaction, with > possible throttling. The alternative needs at most two bits per > commit or rollback of a serializable read write transaction; although > I haven't checked whether that can be scared up without adding a > whole byte. Read only transactions have nothing to do with the > traffic under either alternative. Ok, great. When I read that before I thought that WAL might need to be sent for implicit RO transactions. I will read it more carefully again. 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] SSI patch version 14
1. In CheckForSerializableConflictIn(), I think the comment above may be out of date. It says: "A tuple insert is in conflict only if there is a predicate lock against the entire relation." That doesn't appear to be true if, for example, there's a predicate lock on the index page that the tuple goes into. I examined it with gdb, and it calls the function, and the function does identify the conflict. 2. Also in the comment above CheckForSerializableConflictIn(), I see: "The call to this function also indicates that we need an entry in the serializable transaction hash table, so that this write's conflicts can be detected for the proper lifetime, which is until this transaction and all overlapping serializable transactions have completed." which doesn't make sense to me. The transaction should already have an entry in the hash table at this point, right? 3. The comment above CheckForSerializableConflictOut() seems to trail off, as though you may have meant to write more. It also seems to be out of date. And why are you reading the infomask directly? Do the existing visibility functions not suffice? I have made it through predicate.c, and I have a much better understanding of what it's actually doing. I can't claim that I have a clear understanding of everything involved, but the code looks like it's in good shape (given the complexity involved) and well-commented. I am marking the patch Ready For Committer, because any committer will need time to go through the patch; and the authors have clearly applied the thought, care, and testing required for something of this complexity. I will continue to work on it, though. The biggest issue on my mind is what to do about Hot Standby. The authors have a plan, but there is also some resistance to it: http://archives.postgresql.org/message-id/23698.1295566...@sss.pgh.pa.us We don't need a perfect solution for 9.1, but it would be nice if we had a viable plan for 9.2. 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] WIP: RangeTypes
Another updated patch. Improvements: * Full GiST support - Thanks to Alexander Korotkov for sending me a new picksplit algorithm for my "temporal" project on pgfoundry. I modified it for use with range types, including a (hopefully) intelligent way of handling empty and unbounded ranges. * Quite a few tests added, some cleanup done Open items: * naming issues: - period -> tsrange ? - periodtz -> tstzrange ? - intrange -> int4range * add int8range * Documentation improvements - CREATE TYPE - ANYRANGE - Data Types section * Thom Brown and Tom Lane pointed out that the type inferencing should be able to promote int4 to numeric for queries like: select '[18,20]'::numrange @> 19; * Should the SQL function length(), which relies on polymorphic "-", be marked immutable, stable, or volatile? * representation or alignment issues * parser should be improved to handle spaces and quoting better * Should btree_gist be pulled into core to make it easier to use exclusion constraints with range types? * Typmod (optional) Regards, Jeff Davis rangetypes-20110130.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] WIP: RangeTypes
[ trying a third time to send this message, apparently there were infrastructure problems before ] On Sun, 2011-01-30 at 02:55 +, Thom Brown wrote: > postgres=# select '[18,20]'::numrange @> 19; > ERROR: operator does not exist: numrange @> integer > LINE 1: select '[18,20]'::numrange @> 19; >^ > HINT: No operator matches the given name and argument type(s). You > might need to add explicit type casts. It's because it doesn't know the type on the right side, and assumes it's an int4. select '[18,20]'::numrange @> 19.0; works. 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] WIP: RangeTypes
On Sat, 2011-01-29 at 14:42 -0500, Tom Lane wrote: > Jeff Davis writes: > > On Fri, 2011-01-28 at 21:52 +, Thom Brown wrote: > > Also, if I try the same, but with a different name for the type, I get > > the same error. Why does that restriction exist? Can't you have > > types which happen to use the exact same subtype? > > > At first, that's how I designed it. Then, I realized that the type > > system needs to know the range type from the element type in order for > > something like ANYRANGE to work. > > That seems like a fairly bad restriction. In a datatype with multiple > useful sort orderings, it'd be desirable to be able to create a range > type for each such ordering, no? I'd be inclined to think of a range > type as being defined by element type plus a btree opfamily. Maybe it'd > be okay to insist on that combination as being unique. I couldn't find another way to make a function with a definition like: range(ANYELEMENT, ANYELEMENT) returns ANYRANGE work. And it seemed worse to live without a constructor like that. Ideas? Also, it's not based on the btree opfamily right now. It's just based on a user-supplied compare function. I think I could change it to store the opfamily instead, if you think that's a better idea. 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] WIP: RangeTypes
On Sat, 2011-01-29 at 11:00 -0800, David E. Wheeler wrote: > I think I'm just revealing my ignorance of these index types and what > they're good for. My impression has been that GIN was a better but > less-full-featured alternative to GiST and getting better with Tom's > recent fixes for its handling of NULLs. But, uh, obviously not. The idea of GIN is that you store multiple entries for each tuple you insert. So, inserting a tuple containing the document 'hello world' would store the keys "hello" and "world" both pointing back to that tuple. It also makes sense for arrays. But ranges are arbitrarily long, and don't have any defined "step", so that means an infinite number of keys. GiST works better for 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] WIP: RangeTypes
On Fri, 2011-01-28 at 14:15 -0500, Chris Browne wrote: > Mind you, timestamptzrange seems a mite *long* to me. Right. I think we might need to compromise here an use some shorter names. tsrange/tstzrange/numrange seem reasonable to me. > Making sure it's consistent with int4, int8, bigint sure seems like a > good idea. OK, I'll change intrange to int4range, and add int8range. int2range doesn't seem useful, though. 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] WIP: RangeTypes
On Fri, 2011-01-28 at 10:41 -0800, David E. Wheeler wrote: > +1 in principal. I think we should try to avoid the user of the term > "period" if possible, and I see definite benefits to a simple model of > $typename . 'range'; Interesting, I didn't realize that PERIOD was such an undesirable type name. > Is there GIN support? GIN seems to be the preferred index type for > this sort of thing, no? GiST is the natural index access method if we approach ranges as a spatial type. I don't quite know what you have in mind for GIN; what keys would you extract from the value '[1.23,4.56)' ? 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] WIP: RangeTypes
On Fri, 2011-01-28 at 21:52 +, Thom Brown wrote: > > This is not very graceful: > > > > postgres=# CREATE TYPE numrange AS RANGE (SUBTYPE=numeric, > > SUBTYPE_CMP=numeric_cmp); > > ERROR: duplicate key value violates unique constraint > > "pg_range_rgnsubtype_index" > > DETAIL: Key (rngsubtype)=(1700) already exists. You're right, that should be a much nicer error message. > > Also, if I try the same, but with a different name for the type, I get > > the same error. Why does that restriction exist? Can't you have > > types which happen to use the exact same subtype? At first, that's how I designed it. Then, I realized that the type system needs to know the range type from the element type in order for something like ANYRANGE to work. There's a workaround though: create a domain over numeric, and then create a range over mynumeric. =# create domain mynumeric as numeric; CREATE DOMAIN =# create type numrange2 as range (subtype=numeric, subtype_cmp=numeric_cmp); ERROR: duplicate key value violates unique constraint "pg_range_rgnsubtype_index" DETAIL: Key (rngsubtype)=(1700) already exists. =# create type numrange2 as range (subtype=mynumeric, subtype_cmp=numeric_cmp); CREATE TYPE =# select range(1.1::mynumeric,2.2::mynumeric); range -- [ 1.1, 2.2 ) (1 row) > Also, how do you remove a range type which coincides with a system > range type. For example: > > postgres=# CREATE TYPE numrange AS RANGE (SUBTYPE=interval, >SUBTYPE_CMP=interval_cmp); > CREATE TYPE > postgres=# drop type numrange; > ERROR: cannot drop type numrange because it is required by the database > system > > Is this because I shouldn't have been able to create this type in the > first place? The types are in two different schemas. It's just as though you created a table called pg_class. To drop the one you created, do: DROP TYPE public.numrange; 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] WIP: RangeTypes
On Fri, 2011-01-28 at 09:17 -0800, David Fetter wrote: > For consistency, and in order not to continue our atrocious naming > tradition, I'd like to propose that the above be named timestamprange > (tsrange for short) and timestamptzrange (tstzrange for short). No real objection, but I'd like to see if someone else will second it. Also, I don't think aliases are very easy to define. They appear to all be special cases in the backend code, without catalog support. Should I use domains? If not, I think we'll have to stick to one name. > > - INTRANGE (int4) > > int4range/intrange and the missing bigintrange/int8range I thought about adding int8range, and the first time around that's what I tried. But then I realized that the literal "4" is interpreted as an int4, meaning that "range(1,10)" would be interpreted as int4range, so int8range was slightly annoying to use because you have to cast the literals. Also, the storage is not particularly efficient right now anyway, so if you need int8range, you could probably use numrange instead. I don't mind either way. If you think someone will use it, I'll add it. > Should there also be a timerange and a timetzrange? I thought about it, and I realized that I've never seen the "time" type used. Again, I'll add it if someone will use it. Keep in mind that it's fairly easy for people to add their own range types. The most difficult part is defining the "canonical" function if it is applicable, and the "subtype_float" function which is necessary for GiST. 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] SSI patch version 14
On Tue, 2011-01-25 at 15:22 -0600, Kevin Grittner wrote: > Jeff Davis wrote: > > > I think just annotating RWConflict.in/outLink and > > PredTranList.available/activeList with the types of things they > > hold would be a help. > > > > Also, you say something about RWConflict and "when the structure > > is not in use". Can you elaborate on that a bit? > > Please let me know whether this works for you: > > http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=325ec55e8c9e5179e2e16ff303af6afc1d6e732b Looks good. 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] WIP: RangeTypes
Updated patch. Changes: * Documentation for operators/functions * a comprehensive set of operators and functions * BTree opclass * Hash opclass * built-in range types: - PERIOD (timestamp) - PERIODTZ (timestamptz) - DATERANGE (date) - INTRANGE (int4) - NUMRANGE (numeric) * added subtype float function to the API, which will be useful for GiST * created canonical functions for intrange and daterange, so that: '[1,5]'::intrange = '[1,6)'::intrange * added length() function, written in SQL as: select upper($1) - lower($1) which uses polymorphic "-" operator to avoid the need to give the subtype subtract function and return type to the generic API Open items: * More documentation work * Settle any representation/alignment concerns * Should the new length() function be marked as immutable, stable, or volatile? It uses the polymorphic "-" operator, and I suppose someone could define a non-immutable version of that before calling length(). Then again, it is likely to be inlined anyway, right? * GiST - docs - catalog work - implementation * typmod support (optional) This is nearing completion. GiST is by far the most amount of effort remaining that I'm aware of. Comments about the API, naming, representation, interface, funcationality, grammar, etc. are welcome. Regards, Jeff Davis rangetypes-20110127.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] SSI patch version 14
On Tue, 2011-01-25 at 05:57 -0500, Dan Ports wrote: > This summary is right on. I would add one additional detail or > clarification to the last point, which is that rather than checking for > a cycle, we're checking for a transaction with both "in" and "out" > conflicts, which every cycle must contain. To clarify, this means that it will get some false positives, right? For instance: T1: get snapshot T2: get snapshot insert R1 commit T1: read R1 write R2 T3: get snapshot read R2 T3: commit T1: commit -- throws error T1 has a conflict out to T2, and T1 has a conflict in from T3. T2 has a conflict in from T1. T3 has a conflict out to T1. T1 is canceled because it has both a conflict in and a conflict out. But the results are the same as a serial order of execution: T3, T1, T2. Is there a reason we can't check for a real cycle, which would let T1 succeed? 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] SSI patch version 14
On Tue, 2011-01-25 at 09:41 -0600, Kevin Grittner wrote: > Yep. For the visual thinkers out there, the whole concept can be > understood by looking at the jpeg file that's in the Wiki page: > > http://wiki.postgresql.org/images/e/eb/Serialization-Anomalies-in-Snapshot-Isolation.png Yes, that helped a lot throughout the review process. Good job keeping it up-to-date! > Yes, that would work. It would lower one type of overhead a little > and allow RO transactions to be released from SSI tracking earlier. > The question is how to determine it without taking too much time > scanning the finished transaction list for every active read write > transaction every time you start a RO transaction. I don't think > that it's a trivial enough issue to consider for 9.1; it's certainly > one to put on the list to look at for 9.2. It's OK to leave it to 9.2. But if it's a RO deferrable transaction, it's just going to go to sleep in that case anyway; so why not look for an opportunity to get a safe snapshot right away? 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] SSI patch version 14
On Tue, 2011-01-25 at 11:17 -0600, Kevin Grittner wrote: > > The heavy use of SHMQueue is quite reasonable, but for some reason > > I find the API very difficult to read. I think it would help (at > > least me) quite a lot to annotate (i.e. with a comment in the > > struct) the various lists and links with the types of data being > > held. > > We've tried to comment enough, but when you have your head buried in > code you don't always recognize how mysterious something can look. > Can you suggest some particular places where more comments would be > helpful? I think just annotating RWConflict.in/outLink and PredTranList.available/activeList with the types of things they hold would be a help. Also, you say something about RWConflict and "when the structure is not in use". Can you elaborate on that a bit? I'll address the rest of your comments in a later email. 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] SSI patch version 14
on and writing to the SLRU. I don't see where it's determining that the predicate locks can be safely released. Couldn't the oldest transaction still have relevant predicate locks? * In RegisterSerializableTransactionInt, if it doesn't get an sxact, it goes into summarization. But summarization assumes that it has at least one finished xact. Is that always true? If you have enough memory to hold a transaction for each connection, plus max_prepared_xacts, plus one, I think that's true. But maybe that could be made more clear? I'll keep working on this patch. I hope I can be of some help getting this committed, because I'm looking forward to this feature. And I certainly think that you and Dan have applied the amount of planning, documentation, and careful implementation necessary for a feature like this. 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] SSI and Hot Standby
On Fri, 2011-01-21 at 18:52 -0600, Kevin Grittner wrote: > My assumption is that when we have a safe snapshot (which should be > pretty close to all the time), we immediately provide it to any > serializable transaction requesting a snapshot, except it seems to > make sense to use the new DEFERRABLE mode to mean that you want to > use the *next* one to arrive. How would it handle this situation: 1. Standby has safe snapshot S1 2. Primary does a VACUUM which removes some stuff visible in S1 3. Standby can't replay the VACUUM because it still has S1, but also can't get a new S2 because the WAL needed for that is behind the VACUUM So, S1 needs to be discarded. What do we do on the standby while there is no safe snapshot? I suppose throw errors -- I can't think of anything else. > This would effectively cause the point in time which was visible to > serializable transactions to lag behind what is visible to other > transactions by a variable amount, but would ensure that a > serializable transaction couldn't see any serialization anomalies. > It would also be immune to serialization failures from SSI logic; > but obviously, standby-related cancellations would be in play. I > don't know whether the older snapshots would tend to increase the > standby-related cancellations, but it wouldn't surprise me. I'm also a little concerned about the user-understandability here. Is it possible to make the following guarantees in this approach: 1. If transactions are completing on the primary, new snapshots will be taken on the standby; and 2. If no write transactions are in progress on the primary, then the standby will get a snapshot that represents the exact same data as on the primary? That would be fairly easy to explain to users. If there is a visibility lag, then we just say "finish the write transactions, and progress will be made". And if the system is idle, they should see identical data. 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] WIP: RangeTypes
On Fri, 2011-01-21 at 12:31 -0500, Robert Haas wrote: > On Thu, Jan 20, 2011 at 4:29 AM, Jeff Davis wrote: > > New patch. I added a lot of generic range functions, and a lot of > > operators. > > > > There is still more work to do, this is just an updated patch. The > > latest can be seen on the git repository, as well: > > So is this 9.2 material at this point? Regardless of whether it's eligible to be in 9.1, I plan to keep working on it. I would appreciate some overall feedback during this commitfest. Much of the code is there, so it would be helpful if we could settle issues like representation, functionality, interface, catalog, API, grammar, and naming. Otherwise, those issues will just be a reason to bounce it from commitfest-next, as well. 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] WIP: RangeTypes
New patch. I added a lot of generic range functions, and a lot of operators. There is still more work to do, this is just an updated patch. The latest can be seen on the git repository, as well: http://git.postgresql.org/gitweb?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes Regards, Jeff Davis rangetypes-20110119.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] SSI and Hot Standby
On Wed, 2011-01-19 at 19:05 -0600, Kevin Grittner wrote: > If we don't do something like this, do we just provide REPEATABLE > READ on the standby as the strictest level of transaction isolation? > If so, do we generate an error on a request for SERIALIZABLE, warn > and provide degraded behavior, or just quietly give them REPEATABLE > READ behavior? > > Thoughts? Hopefully there is a better option available. We don't want to silently give wrong results. Maybe we should bring back the compatibility GUC? It could throw an error unless the user sets the compatibility GUC to turn "serializable" into "repeatable read". 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] WIP: RangeTypes
When defining generic range functions, there is quite a bit of extra
complexity needed to handle special cases.
The special cases are due to:
* empty ranges
* ranges with infinite boundaries
* ranges with NULL boundaries
* ranges with exclusive bounds (e.g. "(" or ")").
Infinite bounds, and exclusive bounds can both be handled somewhat
reasonably, and the complexity can be somewhat hidden. Empty ranges are
a special case, but can be handled at the top of the generic function in
a straightforward way.
NULL bounds, however, have been causing me a little frustration. A
reasonable interpretation of boolean operators that operate on ranges
might be: "true or false if we can prove it from only the inputs; else
NULL". This gets a little interesting because a NULL value as a range
boundary isn't 100% unknown: it's known to be on one side of the other
bound (assuming that the other side is known). This is similar to how
AND and OR behave for NULL. For instance, take the simple definition of
"contains":
r1.a <= r2.a AND r1.b >= r2.b
(where "a" is the lower bound and "b" is the upper)
Consider r1: [NULL, 10], r2: [20, NULL]. Contains should return "false"
according to our rule above, because no matter what the values of r1.a
and r2.b, the ranges can't possibly overlap.
So, now, more complexity needs to be added. We can be more redundant and
do:
r1.a <= r2.a AND r1.b <= r2 AND r1.a <= r2.b AND r1.b >= r2.a
That seems a little error-prone and harder to understand.
Then, when we have functions that operate on ranges and return ranges,
we're not dealing with 3VL exactly, but some other intuition about what
NULL should do. The semantics get a lot more complicated and hard to
reason about. For instance, what about:
(NULL, 5) INTERSECT (3, NULL)
Should that evaluate to NULL, (NULL, NULL), or throw an error? What
about:
(NULL, 5) MINUS (NULL, 7)
(NULL, 5) MINUS (3, NULL)
I feel like I'm making this too complicated. Should I just scope out
NULL range boundaries for the first cut, and leave room in the
representation so that it can be added when there is a more thorough
proposal for NULL range boundaries?
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] WIP: RangeTypes
Updated patch. Summary of changes: * More generic functions * pg_dump support * remove typmod support until it can be done correctly * added some tests There is still quite a bit left, including (numbers match up with previous TODO list): 1. Generic functions -- still more work to do here. Handling the combination of continuous range semantics with NULLs requires quite a lot of special cases, because it's hard to share code among functions. Even something as simple as "equals" is not as trivial as it sounds. Perhaps I'm missing some cleaner abstractions, or perhaps I'm over-thinking the null semantics. 3. perhaps fix typmod 4. documentation 5. more tests 7. better parser Regards, Jeff Davis rangetypes-20110114.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] kill -KILL: What happens?
On Thu, 2011-01-13 at 11:14 -0800, David Fetter wrote: > I get that we can't prevent all pilot error, but I was hoping we could > bullet-proof this a little more, especially in light of a certain > extremely popular server OS's OOM killer's default behavior. That's a good point. I'm not sure how much action can reasonably be taken, however. > Yes, I get that that behavior is crazy, and stupid, and that people > should shut it off, but it *is* our problem if we let the postmaster > start (or continue) when it's set that way. As an aside, linux has actually changed the heuristic: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=a63d83f427fbce97a6cea0db2e64b0eb8435cd10 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] WIP: RangeTypes
On Tue, 2011-01-11 at 11:13 -0800, David Fetter wrote: > > 3. Typmod -- There is still one annoyance about typmod remaining. I need > > to treat it like an array in find_typmod_coercion_function(), and then > > create a coercion expression. Is it worth it? Would typmod on a range be > > confusing, or should I just finish this item up? > > Probably not worth it for the first round. OK, I'll block typmods for range types for now. > > 4. Docs > > Happy to help evenings this week :) > > > 5. Tests > > Same. What do you have so far? Great! I think the best tests would be around the ANYRANGE type mechanism to see if anything seems wrong or limiting. Particularly, its interaction with ANYELEMENT. > > 7. Right now the parse function is quite dumb. Is there some example > > code I should follow to make sure I get this right? > > KISS is a fine principle. Do you really need it smart on the first > round? :) Well, it needs to be correct ;) Specifically, I think there will be a problem if there is a multibyte character following a backslash. There may be other problems, as well. I could probably get these fixed, but it might be better to follow patterns in other code. I'll look into it. > > 8. In order to properly support the various combinations of ANYRANGE and > > ANYELEMENT in a function definition (which are all important), we need > > to be able to determine the range type given a subtype. That means that > > each subtype can only have one associated range, which sounds somewhat > > limiting, but it can be worked around by using domains. I don't think > > this is a major limitation. Comments? > > As we get a more nuanced type system, this is one of the things that > will need to get reworked, so I'd say it's better not to put too much > effort into things that a refactor of the type system > <http://wiki.postgresql.org/wiki/Refactor_Type_System> would make much > better, at least right now. Sounds good. I don't think this is an actual problem, so I'll consider this a non-issue unless someone else has a comment. > > Also related to representation: > > > > * Right now I always align the subtypes within the range according to > > typalign. I could avoid that by packing the bytes tightly, and then > > copying them around later. Suggestions? And what should the overall > > alignment of the range type be? > > For the first cut, the simplest possible. OK. It's already about as simple as it can get, but might be fairly wasteful. > > * If it's a fixed-length type, we can save the varlena header byte on > > the overall range; but we lose the ability to save space when one of the > > boundaries of the range is missing (NULL or INF), and it would > > complicate the code a little. Thoughts? > > Probably not worth complicating the code at this stage. KISS again :) OK. 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] Something fishy about the current Makefiles
On Thu, 2011-01-06 at 12:27 -0500, Robert Haas wrote: > I've noticed something like this as well, but haven't been able to > figure out exactly what is going wrong. I've been having build problems ever since: http://archives.postgresql.org/message-id/1291256879.25389.6.ca...@jdavis-ux.asterdata.local It's gotten much worse recently, where changing a c file seems to require multiple top-level "make install" commands to take effect (as Tom describes). I don't think it's limited to the access/ subdirectory, though. 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] Allowing multiple concurrent base backups
On Tue, 2011-01-11 at 23:07 +0100, Magnus Hagander wrote: > I think keeping the flexibility is important. If it does add an extra > step I think that's ok once we have pg_basebackup, but it must be > reasonably *safe*. Corrupt backups from forgetting to exclude a file > seems not so. Agreed. > But if the problem is you forgot to exclude it, can't you just remove > it at a later time? If you think you are recovering the primary, and it's really the backup, then you get corruption. It's too late to remove a file after that (unless you have a backup of your backup ;) ). If you think you are restoring a backup, and it's really a primary that crashed, then you run into one of the two problems that I mentioned (which are less severe than corruption, but very annoying). 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] Allowing multiple concurrent base backups
On Tue, 2011-01-11 at 22:56 +0200, Heikki Linnakangas wrote: > > 1. If it's a primary recovering from a crash, and there is a > > backup_label file, and the WAL referenced in the backup_label exists, > > then it does a bunch of extra work during recovery; and > > 2. In the same situation, if the WAL referenced in the backup_label > > does not exist, then it PANICs with a HINT to tell you to remove the > > backup_label. > > > > Is this an opportunity to solve these problems and simplify the code? > > It won't change the situation for pg_start_backup(), but with the patch > the base backups done via streaming won't have those issues, because > backup_label is not created (with that name) in the master. Do you think we should change the backup protocol for normal base backups to try to fix this? Or do you think that the simplicity of unrestricted file copy is worth these problems? We could probably make some fairly minor changes, like making a file on the primary and telling users to exclude it from any base backup. The danger, of course, is that they do copy it, and their backup is compromised. 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] Allowing multiple concurrent base backups
On Tue, 2011-01-11 at 20:17 +0200, Heikki Linnakangas wrote: > So, this patch modifies the internal do_pg_start/stop_backup functions > so that in addition to the traditional mode of operation, where a > backup_label file is created in the data directory where it's backed up > along with all other files, the backup label file is be returned to the > caller, and the caller is responsible for including it in the backup. > The code in replication/basebackup.c includes it in the tar file that's > streamed the client, as "backup_label". Perhaps we can use this more intelligent form of base backup to differentiate between: a. a primary that has crashed while a backup was in progress; and b. an online backup that is being restored. Allowing the user to do an unrestricted file copy as a base backup doesn't allow us to make that differentiation. That lead to the two bugs that we fixed in StartupXLOG(). And right now there are still two problems remaining (albeit less severe): 1. If it's a primary recovering from a crash, and there is a backup_label file, and the WAL referenced in the backup_label exists, then it does a bunch of extra work during recovery; and 2. In the same situation, if the WAL referenced in the backup_label does not exist, then it PANICs with a HINT to tell you to remove the backup_label. Is this an opportunity to solve these problems and simplify the code? 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] SSI and 2PC
On Mon, 2011-01-10 at 11:50 -0600, Kevin Grittner wrote: > I'm trying not to panic here, but I haven't looked at 2PC before > yesterday and am just dipping into the code to support it, and time > is short. Can anyone give me a pointer to anything I should read > before I dig through the 2PC code, which might accelerate this? I don't see much about 2PC outside of twophase.c. Regarding the original post, I agree that we should have two phase-commit support for SSI. We opted not to support it for notifications, but there was a fairly reasonable argument why users wouldn't value the combination of 2PC and NOTIFY. I don't expect this to be a huge roadblock for the feature though. It seems fairly contained. I haven't read the 2PC code either, but I don't expect that you'll need to change the rest of your algorithm just to support it. 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
[HACKERS] WIP: RangeTypes
Ok, I have made some progress. This is still a proof-of-concept patch,
but the important pieces are working together.
Synopsis:
CREATE TYPE numrange AS RANGE (SUBTYPE=numeric,
SUBTYPE_CMP=numeric_cmp);
SELECT range_eq('[1,2.2)'::numrange,'[1,2.2]');
SELECT range_lbound('(3.7,9]'::numrange);
SELECT range(6.7);
SELECT '-'::numrange; -- empty
SELECT '[1, NULL]'::numrange; -- ] will become )
SELECT '(INF, 3)'::numrange;
I haven't completed many of the other generic functions, because I'd
like to make sure I'm on the right track first. The important thing
about the functions above is that they show ANYRANGE working in
conjunction with ANYELEMENT in various combinations, which was a
significant part of this patch.
Here are the open items:
1. Generic functions -- most of which are fairly obvious. However, I
want to make sure I'm on the right track first.
2. GiST -- I'll need a mechanism to implement the "penalty" function,
and perhaps I'll also need additional support for the picksplit
function. For the "penalty" function, I think I'll need to require a
function to convert the subtype into a float, and I can use that to find
a distance (which can be the penalty). That should also satisfy anything
that picksplit might need.
3. Typmod -- There is still one annoyance about typmod remaining. I need
to treat it like an array in find_typmod_coercion_function(), and then
create a coercion expression. Is it worth it? Would typmod on a range be
confusing, or should I just finish this item up?
4. Docs
5. Tests
6. pg_dump -- should be pretty easy; I just want to settle some of the
other stuff first.
7. Right now the parse function is quite dumb. Is there some example
code I should follow to make sure I get this right?
8. In order to properly support the various combinations of ANYRANGE and
ANYELEMENT in a function definition (which are all important), we need
to be able to determine the range type given a subtype. That means that
each subtype can only have one associated range, which sounds somewhat
limiting, but it can be worked around by using domains. I don't think
this is a major limitation. Comments?
9. Representation -- right now I store the OID of the range type in the
range itself, much like arrays, in order to call the find the functions
to operate on the subtype. Robert has some justifiable concerns about
that 4-byte overhead. Possible ideas:
* Forget about ANYRANGE altogether, and generate new catalog entries
for the generic functions for each new range type defined. I don't
particularly like this approach because it makes it very difficult to
define new generic functions.
* Somehow fix the type system so that we know the specific types of
arguments in all situations. I don't know if this is feasible.
* Store a 8- or 16-bit unique number in pg_range, and store that
number in the representation. That would be pretty ugly, and limit the
total possible range types defined at once, but it saves a couple bytes
per value.
* Try to somehow mimic what records do. Records use a global array and
use the typmod as an index into that array. It looks like a hack to me,
but might be worth borrowing anyway.
Also related to representation:
* Right now I always align the subtypes within the range according to
typalign. I could avoid that by packing the bytes tightly, and then
copying them around later. Suggestions? And what should the overall
alignment of the range type be?
* If it's a fixed-length type, we can save the varlena header byte on
the overall range; but we lose the ability to save space when one of the
boundaries of the range is missing (NULL or INF), and it would
complicate the code a little. Thoughts?
Regards,
Jeff Davis
rangetypes-20110110.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] Compatibility GUC for serializable
On Mon, 2011-01-10 at 11:29 -0800, Josh Berkus wrote: > On 1/10/11 10:47 AM, Kevin Grittner wrote: > > If they're not using SERIALIZABLE, this patch will have no impact on > > them at all. If they are using SELECT FOR UPDATE *with* > > SERIALIZABLE, everything will function exactly as it is except that > > there may be some serialization failures which they weren't getting > > before, either from the inevitable (but hopefully minimal) false > > positives inherent in the technique or because they missed covering > > something. > > Right, that's what I'm worried about. If we must have a GUC, perhaps we could publish a sunset one release in the future. 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] WIP: Range Types
On Sat, 2011-01-08 at 21:58 -0500, Robert Haas wrote: > I mean, one semi-obvious possibility is to write one set of C > functions that can have multiple SQL-level definitions bound to it. > Then when the function is called, it can peek at flinfo->fn_oid to > figure out which incarnation was called and then get the typo info > from there. That's ugly, though. That would work, but it is pretty ugly. It means it would be very difficult for users to write new generic functions, because they would need to add a new catalog entry for every existing range type. Then again, wasting 4 bytes per row is not ideal, either. And maybe users could still write some useful generic functions if they were a combination of other generic functions, using the polymorphic system. > It'd be really nice if we could just arrange for the info on which > type anyrange actually is at the moment to be available in the right > place. Storing it on disk to work around that is pretty horrible, but > maybe there's no other reasonable option. I was surprised when I saw the solution for records. Maybe we should consider something like that as a last resort (if it's possible for non-record types)? I'd rather give up typmod than anyrange. It also might be worth figuring out why input functions get the type oid and output functions do not. I see this comment above getTypeIOParam(): * As of PostgreSQL 8.1, output functions receive only the value itself * and not any auxiliary parameters, so the name of this routine is now * a bit of a misnomer ... it should be getTypeInputParam. So, why was it eliminated? If the type output function got the type OID, would it be enough to use fn_expr_get_argtype() for the other generic functions? 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] WIP: Range Types
On Sat, 2011-01-08 at 20:32 -0500, Robert Haas wrote: > On Sat, Jan 8, 2011 at 4:05 PM, Jeff Davis wrote: > > On Sat, 2011-01-08 at 15:47 -0500, Robert Haas wrote: > >> On Sat, Jan 8, 2011 at 3:12 PM, Jeff Davis wrote: > >> > Any ideas? Maybe, with alignment and a "flags" byte (to hold > >> > inclusivity, infinite boundaries, etc.), the extra 4 bytes doesn't cost > >> > much, anyway? > >> > >> I'd be really reluctant to bloat the range representation by 4 bytes > >> to support an anyrange type. Better to defer this until the great day > >> when we get a better typmod system, at least IMHO. > > > > Can you elaborate? How can we have generic functions without ANYRANGE? > > > > And without generic functions, how do we make it easy for users to > > specify a new range type? > > Oh, hmm. What generic functions did you have in mind? Well, input/output, comparisons, overlaps, intersection, minus, and all the necessary GiST support functions. Without generic functions, the only choices we have are: * force the user to write and specify them all -- which doesn't leave much left of my feature (I think the interface would be all that's left). * somehow generate the functions at type creation time Any other ideas? 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] WIP: Range Types
On Sat, 2011-01-08 at 13:05 -0800, Jeff Davis wrote: > On Sat, 2011-01-08 at 15:47 -0500, Robert Haas wrote: > > On Sat, Jan 8, 2011 at 3:12 PM, Jeff Davis wrote: > > > Any ideas? Maybe, with alignment and a "flags" byte (to hold > > > inclusivity, infinite boundaries, etc.), the extra 4 bytes doesn't cost > > > much, anyway? > > > > I'd be really reluctant to bloat the range representation by 4 bytes > > to support an anyrange type. Better to defer this until the great day > > when we get a better typmod system, at least IMHO. > > Can you elaborate? How can we have generic functions without ANYRANGE? > > And without generic functions, how do we make it easy for users to > specify a new range type? Another thought: If we use timestamps, then that's 8 bytes each, meaning 16 bytes. Then, there is the VARHDRSZ (now we're at 20), the flag byte (21), and the range type oid (25). With alignment (if it's aligned at all), that's either 28 or 32 bytes, which is starting to seem ridiculous. Making it always varlena is kind of nice, because then if the upper or lower bound is special (NULL or infinity), then we can omit it and save some space. But I'm starting to think that it's not worth it, and we should detect whether the subtype is fixed, and if so, make the range type fixed length. That will save on the varlena header. Any suggestions on how to represent/align these ranges? 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] WIP: Range Types
On Sat, 2011-01-08 at 15:47 -0500, Robert Haas wrote: > On Sat, Jan 8, 2011 at 3:12 PM, Jeff Davis wrote: > > Any ideas? Maybe, with alignment and a "flags" byte (to hold > > inclusivity, infinite boundaries, etc.), the extra 4 bytes doesn't cost > > much, anyway? > > I'd be really reluctant to bloat the range representation by 4 bytes > to support an anyrange type. Better to defer this until the great day > when we get a better typmod system, at least IMHO. Can you elaborate? How can we have generic functions without ANYRANGE? And without generic functions, how do we make it easy for users to specify a new range type? 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] WIP: Range Types
When writing the generic range output function, it needs to know the specific range type in order to call the subtype's output function. Records accomplish this by using a special cache based on the typmod, apparently, which looks like a hack to me. Arrays accomplish this by storing the specific type in every array value. That seems very wasteful in the case of range types (which only hold two values). I thought I could get away with using get_fn_expr_argtype() for most of the generic functions, but apparently that can't always provide an answer. Any ideas? Maybe, with alignment and a "flags" byte (to hold inclusivity, infinite boundaries, etc.), the extra 4 bytes doesn't cost much, anyway? 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] WIP: Range Types
On Fri, 2011-01-07 at 08:21 +, Florian Weimer wrote: > <http://www.postgresql.org/docs/8.4/static/xoper-optimization.html> > > I'm wondering if one of these hint functions can be reused to compute > range lengths. Interesting idea. However, I don't really see a way to make that work. These functions are tied to the operator, so it would be awkward to try to connect it to the GiST support functions. Also, it doesn't seem to be an exact fit, because the RESTRICT function is used to compute the selectivity as of right now using current statistics. 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] WIP: Range Types
On Wed, 2011-01-05 at 12:07 -0800, Jeff Davis wrote: > The current design for range types doesn't ask for add or subtract. > Although it might be interesting to try to use such an interface for > range types, it introduces a lot of complexity and makes it easier to > cause subtle problems (consider that addition of timestamps and > intervals is not commutative). A consequence of this design is that some generic range functions, like "length" or "distance" would need to rely on the polymorphism of "+" and "-" to work. I'm also not sure if a constructor like "range(start, offset) returns anyrange" could be made to work generically at all, because the start and offset may be two different types (and a function that takes ANYELEMENT requires that all ANYELEMENT arguments are the same type). Does anyone see a problem with 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] WIP: Range Types
On Thu, 2011-01-06 at 09:30 +0900, Hitoshi Harada wrote:
> Robert Haas originally began to propose the idea of type
> interface to get together three of KNN-GIST, range type and window
> frame issue. For KNN-GIST, it was committed by extending pg_amop
> without considering others and range type will be as well. Not getting
> them together might be the answer.
We may end up combining all of these concepts into type interfaces
later. Now that we have multiple potential users of type interfaces, it
will be easier to design type interfaces to work well for all of them.
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] WIP: Range Types
On Wed, 2011-01-05 at 10:41 -0800, David Fetter wrote: > On Thu, Jan 06, 2011 at 02:25:01AM +0900, Hitoshi Harada wrote: > > For any type to calculate boundary based on RANGE clause in window > > functions, we need some type interface mechanism in the core to know > > how to add / subtract values to reach the boundary from the current > > value. For example, > > > > SELECT count(*) OVER (ORDER BY n_int RANGE BETWEEN 10 PRECEDING AND 5 > > FOLLOWING) FROM tbl; > > I'm not sure I get the connection between this type of range and the > "range types" Jeff is working on. Jeff's work involves a way to > create types which represent ranges over types which have some kind of > ordering, although not necessarily a successor operation. > > Had you planned to cast to an integer range in the process of doing > this window? I don't think Harada-san needs the type infrastructure itself, just the interface to find the "difference type" (e.g. "interval" from "timestamp") as well as functions like add and subtract (potentially two interesting subtract functions). Without knowing which function to call, there is no way to find the window boundaries given the current row. The current design for range types doesn't ask for add or subtract. Although it might be interesting to try to use such an interface for range types, it introduces a lot of complexity and makes it easier to cause subtle problems (consider that addition of timestamps and intervals is not commutative). Even if add and subtract were associated with a range type, there's no way to tell which range type to pick given the window function syntax (multiple range types could be defined over the same subtype). I think the interface question should be addressed more directly with a "type interfaces" patch. 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] WIP: Range Types
On Tue, 2011-01-04 at 16:45 -0800, Josh Berkus wrote: > On 1/4/11 10:18 AM, Jeff Davis wrote: > > The main drawback here is that only a select group of people will be > > defining discrete range types at all, because it would require them to > > define a function first. Perhaps that's for the best, because, (as Tom > > pointed out) we don't want someone using floats and then specifying a > > granule of '0.01'. > > Frankly, I'm still not convinced that *anyone* will really need discrete > range types Well, *need* is a standard that can never be met. But with something like a date range, it's very possible that a discrete version matches the real-world problem more closely than a continuous one. If you use only continuous ranges, then be careful to stick with exactly one convention, or you will likely get wrong results (I think this point has already been established). That sounds easy, but consider: * If you want to know whether two ranges are adjacent (a common requirement), then you need to use "[ )" or "( ]". * If you need to map a single point into a range, the only thing that makes sense is "[ ]". * If your query contains current_date, you'll probably want ranges that are either in "( ]" or "[ ]" form. * If you are mixing data sets, they may use different conventions. You can work around all of these problems by making the query more complex (and more error-prone). But I wouldn't like to give up on discrete ranges for types where it really makes sense (dates, IPs, integers). 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] WIP: Range Types
On Tue, 2011-01-04 at 23:04 +0900, Hitoshi Harada wrote: > 2011/1/4 Jeff Davis : > > I have been updating my work in progress here: > > > > http://git.postgresql.org/gitweb?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes > > > > Right now, it's not in a reviewable state, but those interested can > > glance through the code. > > > > Quick synopsis (for illustration purposes only; don't expect much from > > the current code): > > > > CREATE TYPE numrange > >AS RANGE (SUBTYPE=numeric, SUBTYPE_CMP=numeric_cmp); > > I am interested in how you define increment/decrement operation of > range value in discrete types. The window functions and PARTITION also > want to represent RANGE but there's no clear solution. > > Sorry if it's already been discussed since I didn't track the threads. The user would specify a "canonical" function like: CREATE TYPE int4range AS RANGE (SUBTYPE=int4, SUBTYPE_CMP=btint4cmp, CANONICAL=my_int4range_canonical); That function would be called when constructing ranges on input or after a computation, and could change something like (1,4] into [2,4] if you prefer the latter form. So the range types would not have increments, decrements, granules, or knowledge about the "difference" type (e.g. "interval" is the difference type for timestamp). What support do you need/want from range types to help with new window function features? Also, partitioning might have some use for range types to represent range partitions. Comments are welcome. 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] WIP: Range Types
On Tue, 2011-01-04 at 14:18 +, Florian Weimer wrote: > * Jeff Davis: > > > 4. For the GiST penalty function, and perhaps some picksplit algorithms, > > it might be nice to know the length of a range, or do some other kinds > > of math. It introduces a lot of complexity to try to define math > > functions for each subtype, and try to make sure they behave sanely. So > > I was thinking that the user might need to specify a function that > > converts the subtype into a float that approximates a value's position > > in the total order. > > Doesn't the eqsel hint already provide this information? > Can you clarify what you mean? I don't know what the "eqsel hint" is. 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] WIP: Range Types
On Tue, 2011-01-04 at 12:21 -0500, Robert Haas wrote: > > It doesn't allow for all of the suggested features. In particular, it > > would not allow "granules" to be specified for discrete ranges. But on > > balance, it seems like this is the most conceptually simple and I think > > it satisfies the primary use cases. > > Maybe I'm missing something, but it seems like this approach could > support granules. You just have to define the canonicalize function > in terms of the granule. I meant that it doesn't support them as an explicit, user-visible concept. The main drawback here is that only a select group of people will be defining discrete range types at all, because it would require them to define a function first. Perhaps that's for the best, because, (as Tom pointed out) we don't want someone using floats and then specifying a granule of '0.01'. While we're talking about it, one question I had is: should the canonicalize function be: /* works on the deserialized information right before serialization */ canonical(&flags, &lower_bound, &upper_bound) or /* works on the serialized form right after serialization */ range = canonical(range) I would lean toward the latter because it's simpler on the user (and allows non-C functions). But perhaps an efficiency argument could be made for the former because it could avoid one round of deserialize/reserialize when the representation is not already in canonical form. 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
[HACKERS] WIP: Range Types
I have been updating my work in progress here: http://git.postgresql.org/gitweb?p=users/jdavis/postgres.git;a=log;h=refs/heads/rangetypes Right now, it's not in a reviewable state, but those interested can glance through the code. Quick synopsis (for illustration purposes only; don't expect much from the current code): CREATE TYPE numrange AS RANGE (SUBTYPE=numeric, SUBTYPE_CMP=numeric_cmp); Much of the previous discussion seemed to focus on two issues: 1. Reconciling discrete ranges (like ranges of integers) and continuous ranges (like ranges of numeric). I liked Robert's suggestion here: http://archives.postgresql.org/message-id/aanlktiks_x93_k82b4f_ga634wci0oeb9ftrurf28...@mail.gmail.com which says that the user can just define a "canonicalize" function that will take a range as input (or perhaps the logical pieces of a range) and put it into an appropriate canonical representation. For instance, int4range_canonical might take (1,4] and turn it into [2,4]. This is similar to a few other ideas, but Robert's idea seems to require the least effort by the person defining the range type, because postgresql can still handle representation. It doesn't allow for all of the suggested features. In particular, it would not allow "granules" to be specified for discrete ranges. But on balance, it seems like this is the most conceptually simple and I think it satisfies the primary use cases. 2. Representational issues. There are many possibilities here: a. flags for inclusivity, start, and offset b. flags for inclusivity, start, and end c. if it's a discrete range, start and end only might suffice d. if it's a discrete range, perhaps something involving "granules" (a) might be interesting, and for some data types might be more compact, but it introduces a new datatype that is distinct from the range's subtype: the "difference type" (that is, for timestamps it's "interval"). This approach seemed reasonable on paper, but it involves a lot of extra complexity, and introduces some strange assumptions (using an offset of "1 month" versus "30 days" can't be allowed). (c) and (d) are rejected because they require different code paths for discrete and continuous ranges. I chose (b). This is the simplest. If desired, we could still allow the user to specify their own serialize/deserialize functions, which can get most of the benefits of the other ones anyway. Other issues: 1. I plan to introduce an ANYRANGE type. 2. We need to use the subtype's IO functions, but those may not be immutable. So, rather than create new IO functions for each range type, I was thinking that I'd use just three (anyrange_i_in, anyrange_s_in, and anyrange_v_in), and select the right one at definition time, based on the subtype's IO functions' volatility. That seems like a bit of a hack -- any better ideas? 3. Right now I allow user-defined parse/deparse functions to be specified. In almost all cases, I would think that we want the text format to be something like: [ 2010-01-01, 2011-01-01 ) where the brackets denote inclusivity, and the left and right sides can be optionally double-quoted. Is it even worth having these parse/deparse functions, or should we just force the "obvious" format? 4. For the GiST penalty function, and perhaps some picksplit algorithms, it might be nice to know the length of a range, or do some other kinds of math. It introduces a lot of complexity to try to define math functions for each subtype, and try to make sure they behave sanely. So I was thinking that the user might need to specify a function that converts the subtype into a float that approximates a value's position in the total order. Any better ideas? Overall: I think this is one of the simpler designs. Conceptually, defining new ranges of different granularity with ease sounds like a great idea -- but it introduces a lot of complexity (and generated a lot of different opinions), so it was not included in this design. Similarly, I am leaning away from lots of user-specified options unless there is a real use case. Any suggestions or comments welcome. 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] Old git repo
On Thu, 2010-12-30 at 11:02 -0500, Tom Lane wrote: > I'm with Magnus on this: the risk of confusion seems to greatly > outweigh any possible benefit from keeping it. There is no reason for > anyone to use that old repo unless they are still working with a local > clone of it, and even if they do have a local clone, such a clone is > self-sufficient. The reason I originally asked for it to be kept around was not because it's hard to rebase, but because there might be references to SHA1s from that repo floating around. I don't think these would be very common, nor critical, but I know I wrote a few emails that included things like "look at this commit". Personally, my utility for the old repo is not much (if it was anything important, I wouldn't have relied on the unofficial repo). But we should probably give a little bit of warning for folks that might want to rebase or translate some old notes. 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
