Re: [HACKERS] Optimizing pglz compressor

[Amit Kapila]

On Monday, July 01, 2013 1:36 PM Heikki Linnakangas wrote:
> On 26.06.2013 16:37, Amit Kapila wrote:
> > On Wednesday, June 26, 2013 2:15 AM Heikki Linnakangas wrote:
> >> Can you also try the attached patch, please? It's the same as
> before,
> >> but in this version, I didn't replace the prev and next pointers in
> >> PGLZ_HistEntry struct with int16s. That avoids some table lookups,
> at
> >> the expense of using more memory. It's closer to what we have
> without
> >> the patch, so maybe that helps on your system.
> >
> > Yes it helped a lot on my system.
> 
> Ok, good. Strange, I did not expect such a big difference.
> 
> > There was minor problem in you patch, in one of experiments it
> crashed.
> > Fix is not to access 0th history entry in function pglz_find_match(),
> > modified patch is attached.
> 
> Thanks, good catch! I thought that a pointer to the 0th entry would
> never make it into the prev/next fields, but it does. In fact, we never
> store a NULL there anymore, a pointer to the 0th entry is now always
> used to mean 'invalid'. I adjusted the patch to remove the NULL check,
> and only check for the 0th entry.
> 
> Committed.

Thanks, will update the WAL Optimization patch based on this and post the
new patch and data on the corresponding thread.

With Regards,
Amit Kapila.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Move unused buffers to freelist

[Amit Kapila]

On Tuesday, July 02, 2013 12:00 AM Robert Haas wrote:
> On Sun, Jun 30, 2013 at 3:24 AM, Amit kapila 
> wrote:
> > Do you think it will be sufficient to just wake bgwriter when the
> buffers in freelist drops
> > below low watermark, how about it's current job of flushing dirty
> buffers?
> 
> Well, the only point of flushing dirty buffers in the background
> writer is to make sure that backends can allocate buffers quickly.  If
> there are clean buffers already in the freelist, that's not a concern.
>  So...
> 
> > I mean to ask that if for some scenario where there are sufficient
> buffers in freelist, but most
> > other buffers are dirty, will delaying flush untill number of buffers
> fall below low watermark is okay.
> 
> ...I think this is OK, or at least we should assume it's OK until we
> have evidence that it isn't.

Sure, after completing my other review work of Commit Fest, I will devise
the solution
for the suggestions summarized in previous mail and then start a discussion
about same.


With Regards,
Amit Kapila.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for fail-back without fresh backup

[Amit Kapila]

On Friday, June 28, 2013 10:41 AM Sawada Masahiko wrote:
> On Wed, Jun 26, 2013 at 1:40 PM, Amit Kapila 
> wrote:
> > On Tuesday, June 25, 2013 10:23 AM Amit Langote wrote:
> >> Hi,
> >>
> >> >
> >> >> So our proposal on this problem is that we must ensure that
> master
> >> should
> >> > not make any file system level changes without confirming that the
> >> >> corresponding WAL record is replicated to the standby.
> >> >
> >> >   How will you take care of extra WAL on old master during
> recovery.
> >> If it
> >> > plays the WAL which has not reached new-master, it can be a
> problem.
> >> >
> >>
> >> I am trying to understand how there would be extra WAL on old master
> >> that it would replay and cause inconsistency. Consider how I am
> >> picturing it and correct me if I am wrong.
> >>
> >> 1) Master crashes. So a failback standby becomes new master forking
> the
> >> WAL.
> >> 2) Old master is restarted as a standby (now with this patch,
> without
> >> a new base backup).
> >> 3) It would try to replay all the WAL it has available and later
> >> connect to the new master also following the timeline switch (the
> >> switch might happen using archived WAL and timeline history file OR
> >> the new switch-over-streaming-replication-connection as of 9.3,
> >> right?)
> >>
> >> * in (3), when the new standby/old master is replaying WAL, from
> where
> >> is it picking the WAL?
> >Yes, this is the point which can lead to inconsistency, new
> standby/old master
> >will replay WAL after the last successful checkpoint, for which he
> get info from
> >control file. It is picking WAL from the location where it was
> logged when it was active (pg_xlog).
> >
> >> Does it first replay all the WAL in pg_xlog
> >> before archive? Should we make it check for a timeline history file
> in
> >> archive before it starts replaying any WAL?
> >
> > I have really not thought what is best solution for problem.
> >
> >> * And, would the new master, before forking the WAL, replay all the
> >> WAL that is necessary to come to state (of data directory) that the
> >> old master was just before it crashed?
> >
> > I don't think new master has any correlation with old master's data
> directory,
> > Rather it will replay the WAL it has received/flushed before start
> acting as master.
> when old master fail over, WAL which ahead of new master might be
> broken data. so that when user want to dump from old master, there is
> possible to fail dump.
> it is just idea, we extend parameter which is used in recovery.conf
> like 'follow_master_force'. this parameter accepts 'on' and 'off', is
> effective only when standby_mode is set to on.
> 
> if both parameters 'follow_master_force' and 'standby_mode' is set to
> 'on',
> 1. when standby server starts and starts to recovery, standby server
> skip to apply WAL which is in  pg_xlog, and request WAL from latest
> checkpoint LSN to master server.
> 2. master server receives LSN which is standby server latest
> checkpoint, and compare between LSN of standby and LSN of master
> latest checkpoint. if those LSN match, master will send WAL from
> latest checkpoint LSN. if not, master will inform standby that failed.
> 3. standby will fork WAL, and apply WAL which is sent from master
> continuity.

Please consider if this solution has the same problem as mentioned by Robert 
Hass in below mail:
http://www.postgresql.org/message-id/ca+tgmoy4j+p7jy69ry8gposmmdznyqu6dtionprcxavg+sp...@mail.gmail.com


> in this approach, user who want to dump from old master will set 'off'
> to follow_master_force and standby_mode, and gets the dump of old
> master after master started. OTOH, user who want to starts replication
> force will set 'on' to both parameter.

I think before going into solution of this problem, it should be confirmed by 
others whether such a problem
needs to be resolved as part of this patch.

I have seen that Simon Riggs is a reviewer of this Patch and he hasn't 
mentioned his views about this problem.
So I think it's not worth inventing a solution. 

Rather I think if all other things are resolved for this patch, then may be in 
end we can check with Committer, 
if he thinks that this problem needs to be solved as a separate patch.

With Regards,
Amit Kapila.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Atri Sharma]

On Tue, Jul 2, 2013 at 1:02 AM, Robert Haas  wrote:
> On Sun, Jun 30, 2013 at 8:30 AM, Atri Sharma  wrote:
>> I have been reading the recent discussion and was researching a bit, and I 
>> think that we should really go with the idea of randomising the input 
>> data(if it is not completely presorted), to ensure that we do not get 
>> quadratic complexity.
>
> That doesn't ensure any such thing.  It just makes it less likely.
> But we have existing guards that also make that unlikely, so I'm not
> sure what we'd be gaining.
>
>> One easy way to do that could be to take a sample of the data set, and take 
>> a pivot out of it. Still a better way could be to take multiple samples 
>> which are spread of the data set, select a value from each of them, and then 
>> take a cumulative pivot(median,maybe).
>
> We pretty much do that already.
>
>> This shouldn't be too complex, and should give us a fixed nlogn complexity 
>> even for wild data sets, without affecting existing normal data sets that 
>> are present in every day transactions. I even believe that those data sets 
>> will also benefit from the above optimisation.
>
> The only method of selecting a pivot for quicksort that obtain O(n lg
> n) run time with 100% certainty is have a magical oracle inside the
> computer that tells you in fixed time and with perfect accuracy which
> pivot you should select.
>
> If you want to get a useful response to your emails, consider
> including a statement of what you think the problem is and why you
> think your proposed changes will help.  Consider offering a test case
> that performs badly and an analysis of the reason why.

Right, thanks for that. I will keep that in mind.

I was thinking about *mostly sorted* datasets, consider the following:

10 11 12 4 5 6 1 2

(Just off my head, sorry if I missed something).

Now, the above data set is made up of number of rotation of a sorted
dataset, so is mostly sorted, albeit with some disordering.

My point is that these kind of datasets(not necessarily the above one)
can lead to a bad choice of pivot, and hence give us a complexity
which is below NlogN.

I know we have a check for pre sorted inputs, but wasn't sure how we
deal with mostly sorted inputs, as quick sort likes disorder in input.

I agree with Claudio's idea. One thing to keep in mind is that we
don't do quick sort for large data sets anyway, and move to external
merge sort for it. So, we could think of using median of medians
algorithm for the purpose.

Another thing I would like to investigate is our implementation of
quick sort's performance(and maybe external merge sort as well) on
multiword keys.

Regards,

Atri


--
Regards,

Atri
l'apprenant


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [PATCH] Add an ldapoption to disable chasing LDAP referrals

[James Sewell]

Hey All,

This patch request grew from this post (of mine) to pgsql-general:

http://www.postgresql.org/message-id/cabuevezouae-g1_oejagujjmem675dnystwybp4d_wz6om+...@mail.gmail.com

The patch adds another available LDAP option (ldapnochaseref) for
search+bind mode in the pg_hba.conf fil. If set to 1 (0 is default) then it
performs a ldap_set_option which disables chasing of any LDAP references
which are returned as part of the search LDIF.

I can think of two use cases for this:

   1. (the case which spawned my email) A valid search is performed, but
   for some reason a "ref:" with a non responsive LDAP server is returned as
   well, which causes the authentication to time out (could be intermittent if
   DNS round robin or similar is used and some of the LDAP servers are not
   functioning / a packet dropping firewall is in the way).
   2. (a case I found when testing with AD) A valid search is performed and
   6 "ref:" entries are returned, which all must be chased before
   authentication can succeed. Setting ldapnochaseref speeds up authentication
   with no negative cost (assuming you understand your LDAP schema).

I think it's work noting that this setting seems to be the default for
ldapsearch on Linux these days.

Hopefully I found all the documentation that I was meant to update, let me
know if not though.

Cheers,

James Sewell
PostgreSQL Team Lead / Solutions Architect
_

[image:
http://www.lisasoft.com/sites/lisasoft/files/u1/2013hieghtslogan_0.png]

Level 2, 50 Queen St,
Melbourne, VIC, 3000

P: 03 8370 8000   F: 03 8370 8099  W: www.lisasoft.com

-- 


--
The contents of this email are confidential and may be subject to legal or 
professional privilege and copyright. No representation is made that this 
email is free of viruses or other defects. If you have received this 
communication in error, you may not copy or distribute any part of it or 
otherwise disclose its contents to anyone. Please advise the sender of your 
incorrect receipt of this correspondence.
<>

pgsql_ldapnochaseref_v1.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Tom Lane]

Craig Ringer  writes:
> On 07/02/2013 02:39 AM, Robert Haas wrote:
>> I'm actually
>> not clear that it would be all that bad to assume fixed operator
>> names, as we apparently do in a few places despite the existence of
>> operator classes.  But if that is bad, then I don't know how using @+
>> and @- instead helps anything.

> Personally I'm not clear why it's bad to reserve certain fundamental
> operators like '+' and '-', requiring that they have particular semantics.

It is bad.  It's against project policy, not least because we have
assorted *existing* datatypes for which "obvious" operator names like
"=" do not have all the properties you might expect.

If you need a more concrete example of why that sort of thinking is
bad, you might consider the difference between < and ~<~ for type text.
If we hard-wired knowledge about operator behavior to operator names,
it would be impossible for the system to understand that both of those
operators represent sorting-related behaviors.

Or to be even more concrete: if we allow RANGE to suppose that there's
only one possible definition of "+" for a datatype, we're effectively
supposing that there's only one possible sort ordering for that type.
Which is already a wrong assumption, and has been since Postgres was
still at Berkeley.  If you go this way, you won't be able to support
both WINDOW ... ORDER BY foo USING < RANGE ... and WINDOW ... ORDER BY
foo USING ~<~ RANGE ... because you won't know which addition operator
to apply.

(And yeah, I'm aware that the SQL standard only expects RANGE to support
sort keys that are of numeric, datetime, or interval type.  I would hope
that we have higher expectations than that.  Even if we don't, it's not
exactly hard to credit that people might have multiple ideas about how
to sort interval values.)

There are indeed still some places where we rely on operator names to
mean something, but we need to get away from that idea not add more.
Ideally, any property the system understands about an operator or
function should be explicitly declared through opclass membership or
some similar representation.  We've made substantial progress in that
direction in the last fifteen years.  I don't want to reverse that
progress in the name of minor expediencies, especially not ones that
fail to support flexibility that has been in the system for a couple
or three decades already.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Large object + FREEZE?

[Tatsuo Ishii]

Now that we have COPY FREEZE, I'm thinking about adding similar option
to creating large objects. In 9.3 the maximum size of large objects
are increased. That means, the first access to a large object will
trigger more writes because of hint bit updation. Also subsequent
VACUUM may trigger that as well. If we could freeze arge objects while
creating, it could reduce the writes dramatically as COPY FREEZE
already does.

Opinions?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Eliminating PD_ALL_VISIBLE, take 2

[Jeff Davis]

On Mon, 2013-07-01 at 16:05 -0400, Robert Haas wrote:
> Well, I don't believe there's any way to really eliminate the
> contention concern completely.  There's no way around the fact that it
> means more access to the visibility map, and I've seen recent (albeit
> circumstantial thus far) evidence that that can be a real problem.
> The buffer mapping locks are a problem, too, so anything that means
> more page accesses can't be taken lightly.  I agree your proposed
> changes reduce the chances of problems; I don't agree that they
> eliminate them.

If you have a 1000-page table that is being accessed concurrently, that
requires 1000 pins. My patch would make that 1001, which doesn't sound
very scary to me.

1. Do you agree that concurrent access to 1000-page tables is not a
problem with the design of my patch?

2. Can you be more specific about the scenarios that you *are* concerned
about? Preferably in a form that could be tested on a 64-core box; but
at least some kind of analysis involving numbers. "More page accesses"
is scary, but completely meaningless without saying how *many* more and
in which situations.

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] Eliminating PD_ALL_VISIBLE, take 2

[Jeff Davis]

On Mon, 2013-07-01 at 20:59 -0400, Robert Haas wrote: 
> One of several relevant emails is at:
> 
> http://www.postgresql.org/message-id/51a7473c.6070...@vmware.com
> 
> It is definitely possible that I am mixing up two different things.
> But if I am, I don't know what the other one is.

I believe you are mixing up two different things. The patch in the
commitfest now doesn't cause that problem at all.

The thread above is about one proposal in which Andres "basically
suggested treating all visible as frozen". I threw out the idea that my
proposal was not necessarily in conflict with that one, although others
pointed out some problems with combining them.

However, that only matters if Andres's proposal is going to actually
make it in. Heikki also made a very interesting proposal related to
freezing here:

http://www.postgresql.org/message-id/51a7553e.5070...@vmware.com

and that seems compatible with my proposal (which is one of the
advantages you list).

So, if you object because we're moving toward another incompatible
proposal that is more desirable, then I understand that. It can be a bit
frustrating to me though if my proposal is rejected because one of
several proposals is in conflict. (Not that it's necessarily wrong to do
so, but I'm sure you can see how that is frustrating.)

I'll see if I can help out with Heikki's patch. If it starts to look
like it's going to make it, will you drop this particular objection?

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] Eliminating PD_ALL_VISIBLE, take 2

[Robert Haas]

On Mon, Jul 1, 2013 at 8:23 PM, Jeff Davis  wrote:
> Can you point me to that criticism? Why can't you just drop the VM
> completely if it becomes corrupted?
>
> (You might be referring to another idea of mine that was related to
> Andres's proposal for "getting rid of freezing".)

One of several relevant emails is at:

http://www.postgresql.org/message-id/51a7473c.6070...@vmware.com

It is definitely possible that I am mixing up two different things.
But if I am, I don't know what the other one is.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Craig Ringer]

On 07/02/2013 02:39 AM, Robert Haas wrote:
> I'm actually
> not clear that it would be all that bad to assume fixed operator
> names, as we apparently do in a few places despite the existence of
> operator classes.  But if that is bad, then I don't know how using @+
> and @- instead helps anything.

Personally I'm not clear why it's bad to reserve certain fundamental
operators like '+' and '-', requiring that they have particular semantics.

Want to use "+" as an alias for || because your Java programmers are
used to writing + for string concatenation? Um, don't do that.

Existing code would be unaffected since RANGE couldn't ever be used in
existing code. At worst, weird user-defined implementations of "+" and
"-" would result in bizarre window function behaviour if the operators
were unsuitable. Exceeding available memory could certainly be an issue
in cases like "+" as concatenation.

The main advantage I see of adding opclass entries for this is that it
makes it explicit when the operators have semantics suitable for use in
range windows.

I don't have a strong opinion on whether we should just use "+" and "-"
or whether we really need an opclass.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Eliminating PD_ALL_VISIBLE, take 2

[Jeff Davis]

On Mon, 2013-07-01 at 16:05 -0400, Robert Haas wrote:
> The other concern I remember being expressed (and not just by me, but
> by a number of people) is that your patch turns loss of a visibility
> map bit into a data corruption scenario, which it currently isn't.
> Right now, if your visibility map gets corrupted, you can always
> recover by deleting it.  Under your proposal that would no longer be
> possible.  I think that's sufficient grounds to reject the patch by
> itself, even if there were no other issues.  If that doesn't strike
> you as very dangerous, I'm baffled as to why not.

Can you point me to that criticism? Why can't you just drop the VM
completely if it becomes corrupted?

(You might be referring to another idea of mine that was related to
Andres's proposal for "getting rid of freezing".)

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] fixing pg_ctl with relative paths

[Josh Kupershmidt]

On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao  wrote:
> On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt  wrote:
>> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao  wrote:
>>> Though this is a corner case, the patch doesn't seem to handle properly the 
>>> case
>>> where "-D" appears as other option value, e.g., -k option value, in
>>> postmaster.opts
>>> file.
>>
>> Could I see a command-line example of what you mean?
>
> postmaster -k "-D", for example. Of course, it's really a corner case :)

Oh, I see. I was able to trip up strip_datadirs() with something like

$ PGDATA="/my/data/" postmaster -k "-D" -S 100 &
$ pg_ctl -D /my/data/ restart

that example causes pg_ctl to fail to start the server after stopping
it, although perhaps you could even trick the server into starting
with the wrong options. Of course, similar problems exists today in
other cases, such as with the relative paths issue this patch is
trying to address, or a datadir containing embedded quotes.

I am eager to see the relative paths issue fixed, but maybe we need to
bite the bullet and sort out the escaping of command-line options in
the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \"
quote" can consistently be used by pg_ctl {start|stop|restart} before
we can fix this wart.

Josh


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: PL/Lua (was: [HACKERS] plpython implementation)

[Andreas Karlsson]

On 07/02/2013 01:54 AM, Luis Carvalho wrote:

Peter Eisentraut wrote:

On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote:

The project is maintained -- I don't know how to say when something is
well-maintained, but small frequency of code updates is not one of my
criteria;


The bug tracker contains bugs about build problems with PG 8.4, 9.2, and
9.3, which have not been addressed.


Done (it took me a while to see the bug tracker in pgfoundry...) BTW, thanks
for the patch; I'll release a new version of PL/Lua once PG 9.3 is out.


It might be worth looking at the feature set of PL/v8 which currently 
seems to be larger than PL/Perl, PL/Python and PL/tcl. Including having 
the possibility to implement window functions.


http://pgxn.org/dist/plv8/doc/plv8.html#Window.function.API

Nice job with PL/Lua,
Andreas

--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: PL/Lua (was: [HACKERS] plpython implementation)

[Luis Carvalho]

Peter Eisentraut wrote:
> On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote:
> > The project is maintained -- I don't know how to say when something is
> > well-maintained, but small frequency of code updates is not one of my
> > criteria; 
> 
> The bug tracker contains bugs about build problems with PG 8.4, 9.2, and
> 9.3, which have not been addressed.

Done (it took me a while to see the bug tracker in pgfoundry...) BTW, thanks
for the patch; I'll release a new version of PL/Lua once PG 9.3 is out.

Cheers,
Luis

-- 
Computers are useless. They can only give you answers.
-- Pablo Picasso

-- 
Luis Carvalho (Kozure)
lua -e 'print((("lexcarva...@no.gmail.spam.com"):gsub("(%u+%.)","")))'


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Tom Lane]

Robert Haas  writes:
> On Mon, Jul 1, 2013 at 3:28 PM, Alvaro Herrera  
> wrote:
>> Currently, all operator classes are tied to access methods.  Since
>> nobody seems to have any great idea about creating an access method that
>> requires addition and subtraction, would it make sense to have operator
>> classes that exist solely to support keeping track of such operators for
>> the various datatypes?

We certainly could envision adding "+" and "-" items to btree opfamilies,
with the proviso that they'd have to be optional so as not to break
existing extensions that create btree opfamilies.  If we went that way,
I'd suggest that what we actually add to the definition is *not*
operators, but functions --- that is, specify that function slots 2 and
3 can contain addition (resp subtraction) functions that are compatible
with the family's ordering behavior.  Otherwise you'd have the planner
trying to match WHERE clauses to the extra operators, which is somewhere
between wrong and dangerous.  But a function that isn't actually called
by the index AM is not going to pose a hazard of being misapplied.
Besides, it'd likely be easier not harder for the window-function
machinery to work with a function than an operator.

> I suppose if we really wanted to do this, it would make more sense to
> have a new kind of object, maybe CREATE TYPE INTERFACE, rather than
> shoehorning it into the operator class machinery.  It seems like a
> fairly heavyweight solution, however.

Yeah, there's something to be said for not wedging this into the index
opclass infrastructure.  I'd be happier about building such new
infrastructure if we could unify this requirement with Peter's
"transforms" feature, which also seems to need more datatype-related
knowledge than we have in the catalogs now.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Add /coverage/ to .gitignore

[Fabrízio de Royes Mello]

Hi all,

When we run...

./configure --enable-converage
make coverage-html

...the output is generated into /coverage/ directory. The attached patch
add /converage/ to .gitignore.

Regards,

-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


gitignore-coverage.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Josh Berkus]

On 07/01/2013 12:05 AM, ian link wrote:
> Definitely not this week. Hopefully for next commit fest.
> 

OK, marked "Returned with Feedback".  It'll be up to you to add it to
the next commitfest if you think it's ready by then.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: PL/Lua (was: [HACKERS] plpython implementation)

[Peter Eisentraut]

On Mon, 2013-07-01 at 18:15 -0400, Luis Carvalho wrote:
> The project is maintained -- I don't know how to say when something is
> well-maintained, but small frequency of code updates is not one of my
> criteria; 

The bug tracker contains bugs about build problems with PG 8.4, 9.2, and
9.3, which have not been addressed.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for REINDEX CONCURRENTLY

[Michael Paquier]

On Tue, Jul 2, 2013 at 7:36 AM, Fujii Masao  wrote:
> On Mon, Jul 1, 2013 at 9:31 AM, Michael Paquier
>  wrote:
>> Hi all,
>>
>> Please find attached an updated version of the patch removing
>> reltoastidxid (with and w/o context diffs), patch fixing the vacuum
>> full issue. With this fix, all the comments are addressed.
>
> Thanks for updating the patch!
>
> I have one question related to VACUUM FULL problem. What happens
> if we run VACUUM FULL when there is an invalid toast index? The invalid
> toast index is rebuilt and marked as valid, i.e., there can be multiple valid
> toast indexes?
The invalid toast indexes are not rebuilt. With the design of this
patch, toast relations can only have one valid index at the same time,
and this is also the path taken by REINDEX CONCURRENTLY for toast
relations. This process is managed by this code in cluster.c, only the
valid index of toast relation is taken into account when rebuilding
relations:
***
*** 1393,1410  swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,

/*
 * If we're swapping two toast tables by content, do the same for their
!* indexes.
 */
if (swap_toast_by_content &&
!   relform1->reltoastidxid && relform2->reltoastidxid)
!   swap_relation_files(relform1->reltoastidxid,
!   relform2->reltoastidxid,
target_is_pg_class,
swap_toast_by_content,
is_internal,
InvalidTransactionId,
InvalidMultiXactId,
mapped_tables);

/* Clean up. */
heap_freetuple(reltup1);
--- 1392,1421 

/*
 * If we're swapping two toast tables by content, do the same for their
!* valid index. The swap can actually be safely done only if
the relations
!* have indexes.
 */
if (swap_toast_by_content &&
!   relform1->relkind == RELKIND_TOASTVALUE &&
!   relform2->relkind == RELKIND_TOASTVALUE)
!   {
!   Oid toastIndex1, toastIndex2;
!
!   /* Get valid index for each relation */
!   toastIndex1 = toast_get_valid_index(r1,
!
 AccessExclusiveLock);
!   toastIndex2 = toast_get_valid_index(r2,
!
 AccessExclusiveLock);
!
!   swap_relation_files(toastIndex1,
!   toastIndex2,
target_is_pg_class,
swap_toast_by_content,
is_internal,
InvalidTransactionId,
InvalidMultiXactId,
mapped_tables);
+   }

Regards,
--
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for REINDEX CONCURRENTLY

[Fujii Masao]

On Mon, Jul 1, 2013 at 9:31 AM, Michael Paquier
 wrote:
> Hi all,
>
> Please find attached an updated version of the patch removing
> reltoastidxid (with and w/o context diffs), patch fixing the vacuum
> full issue. With this fix, all the comments are addressed.

Thanks for updating the patch!

I have one question related to VACUUM FULL problem. What happens
if we run VACUUM FULL when there is an invalid toast index? The invalid
toast index is rebuilt and marked as valid, i.e., there can be multiple valid
toast indexes?

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_ctl and -h/help

[Michael Paquier]

On Tue, Jul 2, 2013 at 1:43 AM, Bruce Momjian  wrote:
> Agreed --- attached patch applied.  I also noticed that we sometimes
> test for -? then --help, but other times do things in the opposite
> order, and the same for -V/--version, so I made that consistent.
>
> However, I also noticed that while we document -? before --help, we test
> for --help before -?, and the same for -V/--version.  Should I make
> those even more consistent by always testing for the single-letter
> option first?
I am not sure if this is worth doing for all the binaries, the output
result and the return code being the same in all the cases. Having an
undocumented -h was somehow different because it caused the spec of
pg_ctl and friends to behave differently than what was documented. My
2c.
--
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

[Nicholas White]

> pg_get_viewdef() needs to be updated

Ah, good catch - I've fixed this in the attached. I also discovered that
there's a parent-child hierarchy of WindowDefs (using relname->name), so
instead of cloning the WindowDef (in parse_agg.c) if the frameOptions are
different (e.g. by adding the ignore-nulls flag) I create a child of the
WindowDef and override the frameOptions. This has the useful side-effect of
making pg_get_viewdef work as expected (the previous iteration of the patch
produced a copy of the window definintion, not the window name, as it was
using a nameless clone), although the output has parentheses around the
view name:

> lag(i.i, 2) IGNORE NULLS OVER (w) AS lagged_by_2

I've updated the test cases accordingly. Thanks -

Nick


lead-lag-ignore-nulls.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

PL/Lua (was: [HACKERS] plpython implementation)

[Luis Carvalho]

Hi all,

Claudio Freire wrote:
> On Mon, Jul 1, 2013 at 2:29 AM, james  wrote:
> > On 01/07/2013 02:43, Claudio Freire wrote:
> >>
> >> In essence, you'd have to use another implementation. CPython guys
> >> have left it very clear they don't intend to "fix" that, as they don't
> >> consider it a bug. It's just how it is.
> >
> > Given how useful it is to have a scripting language that can be used outside
> > of the database as well as inside it, would it be reasonable to consider
> > 'promoting' pllua?
> >
> > My understanding is that it (lua) is much cleaner under the hood (than
> > CPython).
> > Although I do recognise that Python as a whole has always had more traction.
> 
> Well, that, or you can use another implementation. There are many, and
> PyPy should be seriously considered given its JIT and how much faster
> it is for raw computation power, which is what a DB is most likely
> going to care about. I bet PyPy's sandboxing is a lot better as well.


 
I think that 'promoting' PL/Lua would be too early, but it'd be a great
addition. The latest version, for instance, can run LuaJIT which has a FFI
(check the example in "Anonymous Blocks" at PL/Lua's docs.) I think there are
two main problems: finding maintainers in the core, and lack of popularity to
warrant its promotion (the two problems are related, of course.)


Peter Eisentraut wrote:
> On 7/1/13 1:29 AM, james wrote:
> > Given how useful it is to have a scripting language that can be used
> > outside
> > of the database as well as inside it, would it be reasonable to consider
> > 'promoting' pllua?
> 
> You can start promoting pllua by making it work with current PostgreSQL
> versions.  It hasn't been updated in 5 years, and doesn't build cleanly
> last I checked.
> 
> Having a well-maintained and fully featured pllua available would surely
> be welcome by many.

Thanks for the feedback. Actually, PL/Lua's latest version (1.0) was out one
month ago,

http://pgfoundry.org/frs/?group_id=1000314

but the previous version took around 4 years. I was waiting for bug reports,
since I deemed PL/Lua to be fairly featured, but I have now declared it
"stable".

The project is maintained -- I don't know how to say when something is
well-maintained, but small frequency of code updates is not one of my
criteria; Lua, for instance, took six years between versions 5.2 and 5.1.
BTW, just out of curiosity, when was the last time PL/Tcl was updated?

I think that the project is also fully featured, but I'd appreciate any
comments on the contrary (that is, feature requests.) I might be mistaken, but
PL/Lua has all the features that PL/Python, PL/Perl, and PL/Tcl have, but, for
example, features a trusted flavor when PL/Python does not, and has proper
type mappings, which PL/Perl does not (everything is translated to text.)

PL/Lua 1.0 adds anonymous blocks and a TRUNCATE trigger, and it should run on
PostgreSQL 9.2. It can be used with Lua 5.1, 5.2, and LuaJIT 2.0 (if you want
speed and an easy C interface through a FFI, you should try LuaJIT!)

I'd like to take this opportunity to kindly ask the PostgreSQL doc maintainers
to include PL/Lua in the table at Appendix H.3:

Name: PL/Lua
Language: Lua
Website: http://pgfoundry.org/projects/pllua/

Cheers,
Luis

-- 
Computers are useless. They can only give you answers.
-- Pablo Picasso

-- 
Luis Carvalho (Kozure)
lua -e 'print((("lexcarva...@no.gmail.spam.com"):gsub("(%u+%.)","")))'


-- 
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] Add more regression tests for CREATE OPERATOR

[Robins Tharakan]

On 26 June 2013 02:26, Robins Tharakan  wrote:

> So technically I hope this regression patch I submitted could go through
> since this feedback isn't towards that patch, but in my part I am quite
> intrigued about this test (and how it passes) and probably I'd get back on
> this thread about this particular commented out test in question, as time
> permits.
>
>
Attached is an updated (cumulative) patch, that takes care of the issue
mentioned above and tests two more cases that were skipped earlier.

--
Robins Tharakan
diff --git a/src/test/regress/expected/create_operator.out 
b/src/test/regress/expected/create_operator.out
index 8656864..2e6c764 100644
--- a/src/test/regress/expected/create_operator.out
+++ b/src/test/regress/expected/create_operator.out
@@ -29,3 +29,145 @@ CREATE OPERATOR #%# (
 -- Test comments
 COMMENT ON OPERATOR ## (int4, NONE) IS 'bad right unary';
 ERROR:  operator does not exist: integer ##
+-- Show deprecated message. => is deprecated now
+CREATE OPERATOR => (
+   leftarg = int8, -- right unary
+   procedure = numeric_fac
+);
+WARNING:  => is deprecated as an operator name
+DETAIL:  This name may be disallowed altogether in future versions of 
PostgreSQL.
+-- Should fail. CREATE OPERATOR requires USAGE on SCHEMA
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_op1;
+CREATE SCHEMA schema_op1;
+GRANT USAGE ON SCHEMA schema_op1 TO PUBLIC;
+REVOKE USAGE ON SCHEMA schema_op1 FROM regress_rol_op1;
+SET ROLE regress_rol_op1;
+CREATE OPERATOR schema_op1.#*# (
+   leftarg = int8, -- right unary
+   procedure = numeric_fac
+);
+ERROR:  permission denied for schema schema_op1
+ROLLBACK;
+-- Should fail. SETOF type functions not allowed as argument (testing leftarg)
+BEGIN TRANSACTION;
+CREATE OPERATOR #*# (
+   leftarg = SETOF int8,
+   procedure = numeric_fac
+);
+ERROR:  SETOF type not allowed for operator argument
+ROLLBACK;
+-- Should fail. SETOF type functions not allowed as argument (testing rightarg)
+BEGIN TRANSACTION;
+CREATE OPERATOR #*# (
+   rightarg = SETOF int8,
+   procedure = numeric_fac
+);
+ERROR:  SETOF type not allowed for operator argument
+ROLLBACK;
+-- Should work. Sample text-book case
+BEGIN TRANSACTION;
+CREATE OR REPLACE FUNCTION fn_op2(boolean, boolean)
+RETURNS boolean AS $$
+SELECT NULL::BOOLEAN;
+$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR === (
+LEFTARG = boolean,
+RIGHTARG = boolean,
+PROCEDURE = fn_op2,
+COMMUTATOR = ===,
+NEGATOR = !==,
+RESTRICT = contsel,
+JOIN = contjoinsel,
+SORT1, SORT2, LTCMP, GTCMP, HASHES, MERGES
+);
+ROLLBACK;
+-- Should fail. Invalid attribute
+CREATE OPERATOR #@%# (
+   leftarg = int8, -- right unary
+   procedure = numeric_fac,
+   invalid_att = int8
+);
+WARNING:  operator attribute "invalid_att" not recognized
+-- Should fail. At least leftarg or rightarg should be mandatorily specified
+CREATE OPERATOR #@%# (
+   procedure = numeric_fac
+);
+ERROR:  at least one of leftarg or rightarg must be specified
+-- Should fail. Procedure should be mandatorily specified
+CREATE OPERATOR #@%# (
+   leftarg = int8
+);
+ERROR:  operator procedure must be specified
+-- Should fail. CREATE OPERATOR requires USAGE on TYPE
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_op3;
+CREATE TYPE type_op3 AS ENUM ('new', 'open', 'closed');
+CREATE FUNCTION fn_op3(type_op3, int8)
+RETURNS int8 AS $$
+SELECT NULL::int8;
+$$ LANGUAGE sql IMMUTABLE;
+REVOKE USAGE ON TYPE type_op3 FROM regress_rol_op3;
+REVOKE USAGE ON TYPE type_op3 FROM PUBLIC;  -- Need to do this so that 
regress_rol_op3 is not allowed USAGE via PUBLIC
+SET ROLE regress_rol_op3;
+CREATE OPERATOR #*# (
+   leftarg = type_op3,
+   rightarg = int8,
+   procedure = fn_op3
+);
+ERROR:  permission denied for type type_op3
+ROLLBACK;
+-- Should fail. CREATE OPERATOR requires USAGE on TYPE (need to check 
separately for rightarg)
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_op4;
+CREATE TYPE type_op4 AS ENUM ('new', 'open', 'closed');
+CREATE FUNCTION fn_op4(int8, type_op4)
+RETURNS int8 AS $$
+SELECT NULL::int8;
+$$ LANGUAGE sql IMMUTABLE;
+REVOKE USAGE ON TYPE type_op4 FROM regress_rol_op4;
+REVOKE USAGE ON TYPE type_op4 FROM PUBLIC;  -- Need to do this so that 
regress_rol_op3 is not allowed USAGE via PUBLIC
+SET ROLE regress_rol_op4;
+CREATE OPERATOR #*# (
+   leftarg = int8,
+   rightarg = type_op4,
+   procedure = fn_op4
+);
+ERROR:  permission denied for type type_op4
+ROLLBACK;
+-- Should fail. CREATE OPERATOR requires EXECUTE on function
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_op5;
+CREATE TYPE type_op5 AS ENUM ('new', 'open', 'closed');
+CREATE FUNCTION fn_op5(int8, int8)
+RETURNS int8 AS $$
+SELECT NULL::int8;
+$$ LANGUAGE sql IMMUTABLE;
+REVOKE EXECUTE ON FUNCTION fn_op5(int8, int8) FROM regress_rol_op5;
+REVOKE EXECUTE ON FUNCTION fn_op5(int8, int8) FROM PUBLIC;-- Need to do this 
so that regress_rol_op3 is not allowed EXECUTE via PUBLIC
+SET ROLE regress_rol_op5;
+CREATE OPERATOR #*# (
+   l

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Robert Haas]

On Mon, Jul 1, 2013 at 4:31 PM, Claudio Freire  wrote:
> What?
>
> A median of medians algorithm will guarantee floor(N/2) elements on
> the smaller. That's the definition of median.
>
> Note that I'm referring to picking the actual median of all tuples,
> not just a sample. That's slow, but it guarantees O(n log n).

Ah, OK.  Well, yes, that would guarantee O(n lg n).  But, as you say,
it would be slow.  :-)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] extensible external toast tuple support

[Robert Haas]

On Mon, Jul 1, 2013 at 3:49 PM, Andres Freund  wrote:
>> I must be missing something.  At that point, yes, you'd like to avoid
>> re-toasting unnecessarily, but ISTM you've already bought the farm.
>> Unless I'm misunderstanding the code as written, you'd just end up
>> writing the indirect pointer back out to disk in that scenario.
>> That's not gonna work...
>
> You didn't misunderstand anything unfortunately. I broke that somewhere
> along the road, probably when factoring things out to
> toast_datum_differs(). Fixed...
> Which shows that we need tests. I've added some using a function in
> regress.c that makes all datums indirect. Together with a triggers that
> allows some testing.

OK, that's nifty.

+   elog(ERROR, "deleteing a tuple indirect datums 
doesn't make sense");

That's spelled wrong.

+   Assert(VARATT_IS_EXTERNAL_ONDISK(old_value));
+
+   /* fast path for the common case where we have the toast oid available 
*/
+   if (VARATT_IS_EXTERNAL_ONDISK(old_value) &&
+   VARATT_IS_EXTERNAL_ONDISK(new_value))
+   return memcmp((char *) old_value, (char *) new_value,
+ VARSIZE_EXTERNAL(old_value)) != 0;

You've already asserted VARATT_IS_EXTERNAL_ONDISK(old_value), so there
can't be any need to test that condition again.

+   changed = memcmp(VARDATA_ANY(old_data), VARDATA_ANY(new_data),
+ VARSIZE_ANY_EXHDR(old_data)) != 0;
+   if (!changed)
+   *needs_rewrite = true;
+
+   /* heap_tuple_untoast_attr copied data */
+   pfree(old_data);
+   if (new_value != new_data)
+   pfree(new_data);
+   return changed;

So... basically, we always set needs_rewrite to !changed, except when
VARATT_IS_EXTERNAL_ONDISK(new_value).  In that latter case we return
after doing memcmp().  That's kind of byzantine and could probably be
simplified by pushing the first if-clause of this function up into the
caller and then making the function's job to simply compare the datums
on the assumption that the new datum is not external on-disk.  Then
you wouldn't need this funny three-way return value done as two
Booleans.

But backing up a minute, this is really a significant behavior change
that is independent of the purpose of the rest of this patch.  What
you're proposing here is that every time we consider toasting a value
on update, we should first check whether it's byte-for-byte equivalent
to the old value.  That may or may not be a good idea - it will suck
if, for example, a user repeatedly updates a very long string by
changing only the last character thereof, but it will win in other
cases.  Whether it's a good idea or not, I think it deserves to be a
separate patch.  For purposes of this patch, I think you should just
assume that any external-indirect value needs to be retoasted, just as
we currently assume for untoasted values.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Alvaro Herrera]

Robert Haas escribió:
> On Mon, Jul 1, 2013 at 3:57 PM, Magnus Hagander  wrote:

> > I still think a better option to that would be to get psql to provide
> > a link to the full documentation there.
> 
> It seems like clutter to me, but I'll defer to whatever the consensus is.

I second the idea of having psql's \h provide a link to the command's
specific reference page.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_resetxlog -m documentation not up to date

[Alvaro Herrera]

Peter Eisentraut wrote:
> On 6/19/13 9:57 PM, Alvaro Herrera wrote:
> > Peter Eisentraut wrote:
> >> Ping.  This ought to be fixed before 9.3 goes out.
> > 
> > Will fix.
> > 
> >> On Sat, 2013-04-27 at 21:22 -0400, Peter Eisentraut wrote:
> >>> The pg_resetxlog -m option was changed from
> 
> The man page lists the -m option as -m mxid,mxid, but the --help output
> has -m XID,XID.  Is that correct?  Do we consider MXIDs to be a subset
> of XIDs?

Yeah, mxids are stored in places that normally store Xids (namely a
tuples' Xmax field).  That said, I don't see any reason not to have
--help show -m MXID,MXID.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Peter Geoghegan]

On Mon, Jul 1, 2013 at 12:32 PM, Robert Haas  wrote:
>> I have been reading the recent discussion and was researching a bit, and I 
>> think that we should really go with the idea of randomising the input 
>> data(if it is not completely presorted), to ensure that we do not get 
>> quadratic complexity.
>
> That doesn't ensure any such thing.  It just makes it less likely.
> But we have existing guards that also make that unlikely, so I'm not
> sure what we'd be gaining.

+1

-- 
Peter Geoghegan


-- 
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] fallocate / posix_fallocate for new WAL file creation (etc...)

[Greg Smith]

On 7/1/13 3:44 PM, Robert Haas wrote:

Yeah.  If the patch isn't going to be a win on RHEL 5, I'd consider
that a good reason to scrap it for now and revisit it in 3 years.
There are still a LOT of people running RHEL 5, and the win isn't big
enough to engineer a more complex solution.


I'm still testing, expect to have this wrapped up on my side by 
tomorrow.  So much of the runtime here is the file setup/close that 
having a 2:1 difference in number of writes, what happens on the old 
platforms, it is hard to get excited about.


I don't think the complexity to lock out RHEL5 here is that bad even if 
it turns out to be a good idea.  Just add another configure check for 
fallocate, and on Linux if it's not there don't use posix_fallocate 
either.  Maybe 5 lines of macro code?  RHEL5 sure isn't going anyway 
anytime soon, but at the same time there won't be that many 9.4 
deployments on that version.


I've been digging into the main situation where this feature helps, and 
it won't be easy to duplicate in a benchmark situation.  Using Linux's 
fallocate works as a hint that the whole 16MB should be allocated at 
once, and therefore together on disk if feasible.  The resulting WAL 
files should be less prone to fragmentation.  That's actually the 
biggest win of this approach, but I can't easily duplicate the sort of 
real-world fragmentation I see on live servers here.Given that, I'm 
leaning toward saying that unless there's a clear regression on older 
platforms, above the noise floor, this is still the right thing to do.


I fully agree that this needs to fully automatic--no GUC--before it's 
worth committing.  If we can't figure out the right thing to do now, 
there's little hope anyone else will in a later tuning expedition.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] changeset generation v5-01 - Patches & git tree

[Simon Riggs]

On 27 June 2013 23:18, Tom Lane  wrote:


> Exactly what is the argument that says performance of this
> function is sufficiently critical to justify adding both the maintenance
> overhead of a new pg_class index, *and* a broken-by-design syscache?
>

I think we all agree on changing the syscache.

I'm not clear why adding a new permanent index to pg_class is such a
problem. It's going to be a very thin index. I'm trying to imagine a use
case that has pg_class index maintenance as a major part of its workload
and I can't. An extra index on pg_attribute and I might agree with you. The
pg_class index would only be a noticeable % of catalog rows for very thin
temp tables, but would still even then be small; that isn't even necessary
work since we all agree that temp table overheads could and should be
optimised away somwhere. So blocking a new index because of that sounds
strange.

What issues do you foresee? How can we test them?

Or perhaps we should just add the index and see if we later discover a
measurable problem workload?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] Bugfix and new feature for PGXS

[Andrew Dunstan]

On 07/01/2013 04:39 PM, Peter Eisentraut wrote:

On 6/29/13 1:54 PM, Andrew Dunstan wrote:

I haven't seen a response to this. One thing we are missing is
documentation. Given that I'm inclined to commit all of this (i.e.
cedric's patches 1,2,3, and 4 plus my addition).

Could someone post an updated set of patches that is currently under
consideration?



See what I actually committed today.





I'm also inclined to backpatch it, since without that it seems to me
unlikely packagers will be able to make practical use of it for several
years, and the risk is very low.

Actually, the risk of makefile changes is pretty high, especially in
cases involving advanced features such as vpath.  GNU make hasn't been
as stable is one might think, lately.  We should carefully consider
exactly which parts are worth backpatching.





These changes are fairly small and mostly non-invasive, but if I've 
broken something we should find out about it fairly quickly, I hope.


cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_resetxlog -m documentation not up to date

[Peter Eisentraut]

On 6/19/13 9:57 PM, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
>> Ping.  This ought to be fixed before 9.3 goes out.
> 
> Will fix.
> 
>> On Sat, 2013-04-27 at 21:22 -0400, Peter Eisentraut wrote:
>>> The pg_resetxlog -m option was changed from

The man page lists the -m option as -m mxid,mxid, but the --help output
has -m XID,XID.  Is that correct?  Do we consider MXIDs to be a subset
of XIDs?



-- 
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] Add regression tests for DISCARD

[Robins Tharakan]

On 17 June 2013 18:14, Marko Kreen  wrote:

> Perhaps existing tests in guc.sql should be merged into it?
>
>
Thanks Marko for pointing out about guc.sql.

Please find attached a patch to move DISCARD related tests from guc.sql to
discard.sql. It adds an extra test for a DISCARD PLANS line, although I
amn't sure on how to validate that its working.

Personally, I wouldn't call this a great patch, since most of the tests
were already running, although in a generic script. The separation of
DISCARD related tests to another file is arguably good for the long-term
though.

--
Robins Tharakan


regress_discard_v3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] proposal: simple date constructor from numeric values

[Peter Eisentraut]

On 7/1/13 3:47 AM, Pavel Stehule wrote:
> and it is a part of our ToDo: "Add function to allow the creation of
> timestamps using parameters"
> 
> so we can have a functions with signatures

I would just name them date(...), time(...), etc.

> CREATE OR REPLACE FUNCTION construct_date(year int, month int DEFAULT
> 1, day int DEFAULT 1) RETURNS date;

I would not use default values for this one.

> CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int
> DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0);

If we are using integer datetime storage, we shouldn't use floats to
construct them.



-- 
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] Bugfix and new feature for PGXS

[Peter Eisentraut]

On 6/29/13 1:54 PM, Andrew Dunstan wrote:
> I haven't seen a response to this. One thing we are missing is
> documentation. Given that I'm inclined to commit all of this (i.e.
> cedric's patches 1,2,3, and 4 plus my addition).

Could someone post an updated set of patches that is currently under
consideration?

> I'm also inclined to backpatch it, since without that it seems to me
> unlikely packagers will be able to make practical use of it for several
> years, and the risk is very low.

Actually, the risk of makefile changes is pretty high, especially in
cases involving advanced features such as vpath.  GNU make hasn't been
as stable is one might think, lately.  We should carefully consider
exactly which parts are worth backpatching.



-- 
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] Randomisation for ensuring nlogn complexity in quicksort

[Claudio Freire]

On Mon, Jul 1, 2013 at 5:12 PM, Robert Haas  wrote:
> On Mon, Jul 1, 2013 at 3:54 PM, Claudio Freire  wrote:
>> On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas  wrote:
 This shouldn't be too complex, and should give us a fixed nlogn complexity 
 even for wild data sets, without affecting existing normal data sets that 
 are present in every day transactions. I even believe that those data sets 
 will also benefit from the above optimisation.
>>>
>>> The only method of selecting a pivot for quicksort that obtain O(n lg
>>> n) run time with 100% certainty is have a magical oracle inside the
>>> computer that tells you in fixed time and with perfect accuracy which
>>> pivot you should select.
>>
>> Doesn't a linear median algorithm (say median of medians) get you O(n lg n)?
>>
>> Granted, with a huge constant (I think 4)... but it should still be O(n lg 
>> n).
>
> No.   Thinking about this a little more, I believe the way it works
> out is that any algorithm for picking the median that guarantees that
> a certain *percentage* of the tuples will be in the smaller partition
> will have O(n lg n) complexity, but any algorithm that only guarantees
> that a fixed *number* of tuples in the smaller partition is still
> quadratic in complexity.  In the case of a median algorithm, you're
> only guaranteed to have 2 elements in the smaller partition, which is
> a constant.  If you take a median of medians, you're guaranteed to
> have 8 elements in the smaller partition, which is bigger, but still a
> constant.


What?

A median of medians algorithm will guarantee floor(N/2) elements on
the smaller. That's the definition of median.

Note that I'm referring to picking the actual median of all tuples,
not just a sample. That's slow, but it guarantees O(n log n).


-- 
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] plpython implementation

[Peter Eisentraut]

On 7/1/13 1:29 AM, james wrote:
> Given how useful it is to have a scripting language that can be used
> outside
> of the database as well as inside it, would it be reasonable to consider
> 'promoting' pllua?

You can start promoting pllua by making it work with current PostgreSQL
versions.  It hasn't been updated in 5 years, and doesn't build cleanly
last I checked.

Having a well-maintained and fully featured pllua available would surely
be welcome by many.


-- 
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] GIN improvements part 1: additional information

[Heikki Linnakangas]

On 01.07.2013 13:28, Alexander Korotkov wrote:

Thanks! So, we have a lot of stuff and you give the points for further
work. Could you please verify my plan of work on these patches:
1) Solving questions of archives.postgresql.org/**
message-id/51CEA13C.8040103@**vmware.com
for
packed postinglists.
2) Extract additional info patch based on packed postinglists.
3) Rewrite interface of fast scan. Do CPU and IO benchmarking.
4) Do IO benchmarking of index ordering.
Cleanup, comments and READMEs are assumed in each item.


Yep, sounds good!

- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] "pg_ctl promote" exit status

[Peter Eisentraut]

On 7/1/13 12:47 PM, Bruce Momjian wrote:
>> Approximately none of these changes seem correct to me.  For example,
>> why is failing to open the PID file 6, or failing to start the server 7?
> 
> Well, according to that URL, we have:
> 
>   6   program is not configured
>   7   program is not running

There is also

4   user had insufficient privilege

> I just updated the pg_ctl.c comments to at least point to a valid URL
> for this.  I think we can just call this item closed because I am still
> unclear if these return codes should be returned by pg_ctl or the
> start/stop script.
> 
> Anyway, while I do think pg_ctl could pass a little more information
> back about failure via its return code, I am unclear if LSB is the right
> approach.

Yeah, a lot of these things are unclear and not used in practice, so
it's probably better to stick to exit code 1, unless there is a clear
use case.  The "status" case is different, because there the exit code
can be passed out by the init script directly.


-- 
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] Randomisation for ensuring nlogn complexity in quicksort

[Robert Haas]

On Mon, Jul 1, 2013 at 3:54 PM, Claudio Freire  wrote:
> On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas  wrote:
>>> This shouldn't be too complex, and should give us a fixed nlogn complexity 
>>> even for wild data sets, without affecting existing normal data sets that 
>>> are present in every day transactions. I even believe that those data sets 
>>> will also benefit from the above optimisation.
>>
>> The only method of selecting a pivot for quicksort that obtain O(n lg
>> n) run time with 100% certainty is have a magical oracle inside the
>> computer that tells you in fixed time and with perfect accuracy which
>> pivot you should select.
>
> Doesn't a linear median algorithm (say median of medians) get you O(n lg n)?
>
> Granted, with a huge constant (I think 4)... but it should still be O(n lg n).

No.   Thinking about this a little more, I believe the way it works
out is that any algorithm for picking the median that guarantees that
a certain *percentage* of the tuples will be in the smaller partition
will have O(n lg n) complexity, but any algorithm that only guarantees
that a fixed *number* of tuples in the smaller partition is still
quadratic in complexity.  In the case of a median algorithm, you're
only guaranteed to have 2 elements in the smaller partition, which is
a constant.  If you take a median of medians, you're guaranteed to
have 8 elements in the smaller partition, which is bigger, but still a
constant.

The reason why this doesn't matter much in practice is because the
data distribution that causes quadratic behavior for median-of-medians
is not one which is likely to occur in the real world and will
probably only come up if chosen by an adversary who is attempting to
make your life miserable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Joshua D. Drake]

On 07/01/2013 07:20 AM, David Fetter wrote:


On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote:

On 6/28/13 2:27 PM, David Fetter wrote:

You can run \! man from within psql,

And if you're on Windows, you're Sadly Out of Luck with that.  Is
there an equivalent we could #ifdef in for that platform?


If you are using psql on Windows extensively, you probably have one of
mingw, cygwin, or pgadmin handy, all of which can get you to the
documentation.  I don't think it's worth devising a mechanism for those
not covered by this.


With deepest respect, failing to provide documentation to users on our
widest-deployed platform seems pretty hostile to me.  There was an
earlier suggestion that we provide URLs, which seems like a decent way
forward as those environments so locked down as to disallow outbound
HTTP are pretty rare, and non-networked computers are even more rare.



Although I agree with the sentiment the idea that postgres more widely 
deployed on windows than other platforms is rather laughable. The only 
metrics we have are "downloads" which doesn't count cause linux ships 
with postgres with a simple yum or apt-get.


Whatever solution we decide, we should not push this responsibility off 
on pgadmin as pgadmin is not part of PostgreSQL but a third party tool. 
The "standard" postgresql client is psql (for good or bad) and we should 
support psql fully on all platforms.


Sincerely,

Joshua D. Drake



--
Command Prompt, Inc. - http://www.commandprompt.com/  509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
   a rose in the deeps of my heart. - W.B. Yeats


--
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] Documentation/help for materialized and recursive views

[Robert Haas]

On Mon, Jul 1, 2013 at 3:57 PM, Magnus Hagander  wrote:
> How different are they really? Yes, they are very different from an
> implementation standpoint, from an enduser perspective they really are
> not. If they were, they'd probably be called something else.

They're different because they consume storage, require effort to
update, and aren't always up-to-date.  Those things are all quite
user-visible.

> I still think a better option to that would be to get psql to provide
> a link to the full documentation there.

It seems like clutter to me, but I'll defer to whatever the consensus is.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Eliminating PD_ALL_VISIBLE, take 2

[Robert Haas]

On Mon, Jul 1, 2013 at 1:21 PM, Jeff Davis  wrote:
> On Sun, 2013-06-30 at 22:58 -0400, Robert Haas wrote:
>> I thought that Jeff withdrew this patch.
>
> No -- was there a reason you thought that?

I thought I remembered you saying you were going to abandon it in the
face of objections.

> I know it could use another
> round of testing before commit, and there may be a couple other things
> to clear up. But I don't want to invest a lot of time there right now,
> because, as I understand it, you still object to the patch anyway.
>
> I am still not entirely clear on the objections to this patch:
>
> 1. Contention was a concern, but I believe I have mitigated it. Strictly
> speaking, additional pins may be acquired, but the cost of those pin
> operations will be spread over a lot of other work.
>
> 2. There are quite a few different ideas about where we're going with
> PD_ALL_VISIBLE and freezing, but it seems like removing PD_ALL_VISIBLE
> is potentially compatible with most of them.
>
> Any others?
>
> The patch reduces code complexity and reduces writes during a data load.

Well, I don't believe there's any way to really eliminate the
contention concern completely.  There's no way around the fact that it
means more access to the visibility map, and I've seen recent (albeit
circumstantial thus far) evidence that that can be a real problem.
The buffer mapping locks are a problem, too, so anything that means
more page accesses can't be taken lightly.  I agree your proposed
changes reduce the chances of problems; I don't agree that they
eliminate them.

The other concern I remember being expressed (and not just by me, but
by a number of people) is that your patch turns loss of a visibility
map bit into a data corruption scenario, which it currently isn't.
Right now, if your visibility map gets corrupted, you can always
recover by deleting it.  Under your proposal that would no longer be
possible.  I think that's sufficient grounds to reject the patch by
itself, even if there were no other issues.  If that doesn't strike
you as very dangerous, I'm baffled as to why not.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Peter Eisentraut]

On 7/1/13 12:34 PM, Jeff Davis wrote:
> On Sun, 2013-06-30 at 16:09 -0400, Andrew Dunstan wrote:
>> It was originally generated. Since then it's been maintained by hand.
> 
> What is the procedure for maintaining it by hand?

Edit away.

> Why are
> HAVE_POSIX_SIGNALS and HAVE_SYNC_FILE_RANGE in there (though commented
> out), but not HAVE_POSIX_FADVISE?

Because maintaining it by hand is prone to inconsistencies.  I wouldn't
worry about it.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Magnus Hagander]

On Mon, Jul 1, 2013 at 9:26 PM, Robert Haas  wrote:
> On Mon, Jul 1, 2013 at 10:20 AM, David Fetter  wrote:
>> With deepest respect, failing to provide documentation to users on our
>> widest-deployed platform seems pretty hostile to me.
>
> Yes, that would be pretty hostile.  However, we don't do anything that
> remotely resembles that statement, nor has anyone proposed any such
> thing.
>
> Personally, I think this whole thread is much ado about nothing.
> Magnus is basically arguing that people might expect that CREATE VIEW
> ought to tell you about CREATE MATERIALIZED VIEW also, but I don't
> find that argument to have a whole lot of merit.  Views and
> materialized views are pretty different things; it is a bit like

How different are they really? Yes, they are very different from an
implementation standpoint, from an enduser perspective they really are
not. If they were, they'd probably be called something else..

> asking why Googling for "dog" does not give you information on hot
> dogs.  The output of psql's \h command is intended to be a succinct

I'd personally say it's more like googling for dog gives me hits
specifically around "dog breeding" and not just dogs themselves.


> synopsis summarizing the salient syntax (try saying that five times
> fast), not a comprehensive reference.  If you want the latter, read
> the fine manual.  I admit that this particular case is slightly more
> prone to confusion than some, but I'm just not that exercised about
> it.  Every bit of detail we add to the \h output is better for the
> people who otherwise would have been unhappy, but it's worse for all
> the people who did need it because it's more to read through.

True.


> Regardless of whether you agree with or disagree with the above
> statement, building a high-quality documentation reader into psql so
> that users who are running Windows but not mingw, cygwin, or pgAdmin
> can access the documentation more easily doesn't seem like the correct
> solution to this problem.  I don't really object if somebody wants to
> do it (although someone else may object) but it's certainly taking the
> long way around as far as this particular confusion is concerned.

I still think a better option to that would be to get psql to provide
a link to the full documentation there.

pgAdmin could also do that, but doesn't - it gets you a link to the
main documentation, but not a context sensitive one IIRC.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Claudio Freire]

On Mon, Jul 1, 2013 at 4:32 PM, Robert Haas  wrote:
>> This shouldn't be too complex, and should give us a fixed nlogn complexity 
>> even for wild data sets, without affecting existing normal data sets that 
>> are present in every day transactions. I even believe that those data sets 
>> will also benefit from the above optimisation.
>
> The only method of selecting a pivot for quicksort that obtain O(n lg
> n) run time with 100% certainty is have a magical oracle inside the
> computer that tells you in fixed time and with perfect accuracy which
> pivot you should select.


Doesn't a linear median algorithm (say median of medians) get you O(n lg n)?

Granted, with a huge constant (I think 4)... but it should still be O(n lg n).


-- 
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] extensible external toast tuple support

[Andres Freund]

On 2013-06-28 11:25:50 -0400, Robert Haas wrote:
> On Fri, Jun 28, 2013 at 10:53 AM, Andres Freund  
> wrote:
> >> Why does toast_insert_or_update() need to go through all the
> >> rigamarole in toast_datum_differs()?  I would have thought that it
> >> could simply treat any external-indirect value as needing to be
> >> detoasted and retoasted, since the destination is the disk anyhow.
> >
> > We could do that, yes. But I think it might be better not to: If we
> > simplify the tuples used in a query to not reference ondisk tuples
> > anymore and we then UPDATE using that new version I would rather not
> > retoast all the unchanged columns.
> >
> > I can e.g. very well imagine that we decide to resolve toasted Datums to
> > indirect Datums during an UPDATE if there are multiple BEFORE UPDATE
> > triggers to avoid detoasting in each and every one of them. Such a tuple
> > will then passed to heap_update...
> 
> I must be missing something.  At that point, yes, you'd like to avoid
> re-toasting unnecessarily, but ISTM you've already bought the farm.
> Unless I'm misunderstanding the code as written, you'd just end up
> writing the indirect pointer back out to disk in that scenario.
> That's not gonna work...

You didn't misunderstand anything unfortunately. I broke that somewhere
along the road, probably when factoring things out to
toast_datum_differs(). Fixed...
Which shows that we need tests. I've added some using a function in
regress.c that makes all datums indirect. Together with a triggers that
allows some testing.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 7b2c83aa337e639d2467359ab7b0cc00a2f36cde Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Tue, 11 Jun 2013 23:25:26 +0200
Subject: [PATCH] Add support for multiple kinds of external toast datums

There are several usecases where our current representation of external toast
datums is limiting:
* adding new compression schemes
* avoidance of repeated detoasting
* externally decoded toast tuples

For that support 'tags' on external (varattrib_1b_e) varlenas which recoin the
current va_len_1be field to store the tag (or type) of a varlena. To determine
the actual length a macro VARTAG_SIZE(tag) is added which can be used to map
from a tag to the actual length.

This patch adds support for 'indirect' tuples which point to some externally
allocated memory containing a toast tuple. It also implements the stub for a
different compression algorithm.

0.3
---
 src/backend/access/heap/tuptoaster.c | 189 ---
 src/include/access/tuptoaster.h  |   5 +
 src/include/postgres.h   |  84 +++---
 src/test/regress/expected/indirect_toast.out | 151 ++
 src/test/regress/input/create_function_1.source  |   5 +
 src/test/regress/output/create_function_1.source |   4 +
 src/test/regress/parallel_schedule   |   2 +-
 src/test/regress/regress.c   |  92 +++
 src/test/regress/serial_schedule |   1 +
 src/test/regress/sql/indirect_toast.sql  |  61 
 10 files changed, 552 insertions(+), 42 deletions(-)
 create mode 100644 src/test/regress/expected/indirect_toast.out
 create mode 100644 src/test/regress/sql/indirect_toast.sql

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index fc37ceb..fd3362f 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -44,9 +44,6 @@
 
 #undef TOAST_DEBUG
 
-/* Size of an EXTERNAL datum that contains a standard TOAST pointer */
-#define TOAST_POINTER_SIZE (VARHDRSZ_EXTERNAL + sizeof(struct varatt_external))
-
 /*
  * Testing whether an externally-stored value is compressed now requires
  * comparing extsize (the actual length of the external data) to rawsize
@@ -87,11 +84,11 @@ static struct varlena *toast_fetch_datum_slice(struct varlena * attr,
  * heap_tuple_fetch_attr -
  *
  *	Public entry point to get back a toasted value from
- *	external storage (possibly still in compressed format).
+ *	external source (possibly still in compressed format).
  *
  * This will return a datum that contains all the data internally, ie, not
- * relying on external storage, but it can still be compressed or have a short
- * header.
+ * relying on external storage or memory, but it can still be compressed or
+ * have a short header.
  --
  */
 struct varlena *
@@ -99,13 +96,35 @@ heap_tuple_fetch_attr(struct varlena * attr)
 {
 	struct varlena *result;
 
-	if (VARATT_IS_EXTERNAL(attr))
+	if (VARATT_IS_EXTERNAL_ONDISK(attr))
 	{
 		/*
 		 * This is an external stored plain value
 		 */
 		result = toast_fetch_datum(attr);
 	}
+	else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
+	{
+		/*
+		 * copy into the caller's memory context. That's not required in all
+		 * cases but sufficient for now since thi

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Robert Haas]

On Mon, Jul 1, 2013 at 2:17 PM, Jeff Davis  wrote:
> On Tue, 2013-07-02 at 02:13 +0900, Fujii Masao wrote:
>> Even in that case, if a user can easily know which platform posix_fallocate
>> should be used in, we can commit the patch with the configurable GUC
>> parameter.
>
> I disagree here. We're not talking about a huge win; this speedup may
> not even be detectable on a running system.
>
> I think Robert summarized the reason for the patch best: "I mean, if
> posix_fallocate() is faster, then it's just faster, right?". But if we
> need a new GUC, and DBAs now have one more thing they need to test about
> their platform, then that argument goes out the window.

Yeah.  If the patch isn't going to be a win on RHEL 5, I'd consider
that a good reason to scrap it for now and revisit it in 3 years.
There are still a LOT of people running RHEL 5, and the win isn't big
enough to engineer a more complex solution.  But ultimately RHEL 5,
like any other old system, will go away.

However, Greg's latest testing results seem to show that there isn't
much to worry about, so we may be fine anyway.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Robert Haas]

On Mon, Jul 1, 2013 at 3:28 PM, Alvaro Herrera  wrote:
> Robert Haas escribió:
>> On Sun, Jun 30, 2013 at 11:54 PM, ian link  wrote:
>
>> > It seems pretty clear that assuming '+' and '-' are addition and 
>> > subtraction
>> > is a bad idea. I don't think it would be too tricky to add support for new
>> > operator strategies. Andrew Gierth suggested calling these new strategies
>> > "offset -" and "offset +", which I think describes it pretty well. I
>> > assigned the operator itself to be "@+" and "@-" but that can obviously be
>> > changed. If this sounds like a good path to you guys, I will go ahead and
>> > implement the operators for the appropriate types. Please let me know if I
>> > am misunderstanding something - I am still figuring stuff out :)
>>
>> I don't think I understand the design you have in mind.  I'm actually
>> not clear that it would be all that bad to assume fixed operator
>> names, as we apparently do in a few places despite the existence of
>> operator classes.  But if that is bad, then I don't know how using @+
>> and @- instead helps anything.
>
> Yeah.
>
> Currently, all operator classes are tied to access methods.  Since
> nobody seems to have any great idea about creating an access method that
> requires addition and subtraction, would it make sense to have operator
> classes that exist solely to support keeping track of such operators for
> the various datatypes?

I suppose if we really wanted to do this, it would make more sense to
have a new kind of object, maybe CREATE TYPE INTERFACE, rather than
shoehorning it into the operator class machinery.  It seems like a
fairly heavyweight solution, however.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Andrew Dunstan]

On 07/01/2013 03:26 PM, Robert Haas wrote:

Regardless of whether you agree with or disagree with the above
statement, building a high-quality documentation reader into psql so
that users who are running Windows but not mingw, cygwin, or pgAdmin
can access the documentation more easily doesn't seem like the correct
solution to this problem.  I don't really object if somebody wants to
do it (although someone else may object) but it's certainly taking the
long way around as far as this particular confusion is concerned.



FWIW, in my reasonably substantial experience, virtually every user on 
Windows uses pgadmin3. Use of psql is fairly rare.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Robert Haas]

On Sun, Jun 30, 2013 at 8:30 AM, Atri Sharma  wrote:
> I have been reading the recent discussion and was researching a bit, and I 
> think that we should really go with the idea of randomising the input data(if 
> it is not completely presorted), to ensure that we do not get quadratic 
> complexity.

That doesn't ensure any such thing.  It just makes it less likely.
But we have existing guards that also make that unlikely, so I'm not
sure what we'd be gaining.

> One easy way to do that could be to take a sample of the data set, and take a 
> pivot out of it. Still a better way could be to take multiple samples which 
> are spread of the data set, select a value from each of them, and then take a 
> cumulative pivot(median,maybe).

We pretty much do that already.

> This shouldn't be too complex, and should give us a fixed nlogn complexity 
> even for wild data sets, without affecting existing normal data sets that are 
> present in every day transactions. I even believe that those data sets will 
> also benefit from the above optimisation.

The only method of selecting a pivot for quicksort that obtain O(n lg
n) run time with 100% certainty is have a magical oracle inside the
computer that tells you in fixed time and with perfect accuracy which
pivot you should select.

If you want to get a useful response to your emails, consider
including a statement of what you think the problem is and why you
think your proposed changes will help.  Consider offering a test case
that performs badly and an analysis of the reason why.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Alvaro Herrera]

Robert Haas escribió:
> On Sun, Jun 30, 2013 at 11:54 PM, ian link  wrote:

> > It seems pretty clear that assuming '+' and '-' are addition and subtraction
> > is a bad idea. I don't think it would be too tricky to add support for new
> > operator strategies. Andrew Gierth suggested calling these new strategies
> > "offset -" and "offset +", which I think describes it pretty well. I
> > assigned the operator itself to be "@+" and "@-" but that can obviously be
> > changed. If this sounds like a good path to you guys, I will go ahead and
> > implement the operators for the appropriate types. Please let me know if I
> > am misunderstanding something - I am still figuring stuff out :)
> 
> I don't think I understand the design you have in mind.  I'm actually
> not clear that it would be all that bad to assume fixed operator
> names, as we apparently do in a few places despite the existence of
> operator classes.  But if that is bad, then I don't know how using @+
> and @- instead helps anything.

Yeah.

Currently, all operator classes are tied to access methods.  Since
nobody seems to have any great idea about creating an access method that
requires addition and subtraction, would it make sense to have operator
classes that exist solely to support keeping track of such operators for
the various datatypes?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Robert Haas]

On Mon, Jul 1, 2013 at 10:20 AM, David Fetter  wrote:
> With deepest respect, failing to provide documentation to users on our
> widest-deployed platform seems pretty hostile to me.

Yes, that would be pretty hostile.  However, we don't do anything that
remotely resembles that statement, nor has anyone proposed any such
thing.

Personally, I think this whole thread is much ado about nothing.
Magnus is basically arguing that people might expect that CREATE VIEW
ought to tell you about CREATE MATERIALIZED VIEW also, but I don't
find that argument to have a whole lot of merit.  Views and
materialized views are pretty different things; it is a bit like
asking why Googling for "dog" does not give you information on hot
dogs.  The output of psql's \h command is intended to be a succinct
synopsis summarizing the salient syntax (try saying that five times
fast), not a comprehensive reference.  If you want the latter, read
the fine manual.  I admit that this particular case is slightly more
prone to confusion than some, but I'm just not that exercised about
it.  Every bit of detail we add to the \h output is better for the
people who otherwise would have been unhappy, but it's worse for all
the people who did need it because it's more to read through.

Regardless of whether you agree with or disagree with the above
statement, building a high-quality documentation reader into psql so
that users who are running Windows but not mingw, cygwin, or pgAdmin
can access the documentation more easily doesn't seem like the correct
solution to this problem.  I don't really object if somebody wants to
do it (although someone else may object) but it's certainly taking the
long way around as far as this particular confusion is concerned.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MVCC catalog access

[Robert Haas]

On Mon, Jul 1, 2013 at 10:04 AM, Andres Freund  wrote:
> This is really cool stuff.

Thanks.

> I have to say, if the thing that primarily suffers is pretty extreme DDL
> in extreme situations I am not really worried. Anybody running anything
> close to the territory of such concurrency won't perform that much DDL.

/me wipes brow.

> Something picked up when quickly scanning over the last version of the
> patch:
>
>> +/*
>> + * Staleness detection for CatalogSnapshot.
>> + */
>> +static bool CatalogSnapshotStale = true;
>>
>>  /*
>>   * These are updated by GetSnapshotData.  We initialize them this way
>> @@ -177,6 +188,9 @@ GetTransactionSnapshot(void)
>>   else
>>   CurrentSnapshot = 
>> GetSnapshotData(&CurrentSnapshotData);
>>
>> + /* Don't allow catalog snapshot to be older than xact 
>> snapshot. */
>> + CatalogSnapshotStale = true;
>> +
>>   FirstSnapshotSet = true;
>>   return CurrentSnapshot;
>>   }
>> @@ -184,6 +198,9 @@ GetTransactionSnapshot(void)
>>   if (IsolationUsesXactSnapshot())
>>   return CurrentSnapshot;
>>
>> + /* Don't allow catalog snapshot to be older than xact snapshot. */
>> + CatalogSnapshotStale = true;
>> +
>>   CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData);
>>
>>   return CurrentSnapshot;
>> @@ -207,6 +224,54 @@ GetLatestSnapshot(void)
>>  }
>
> Do we really need to invalidate snapshots in either situation? Isn't it
> implied, that if it's still valid, according to a) no invalidation via local
> invalidation messages b) no invalidations from other backends, there
> shouldn't be any possible differences when you only look at the catalog?

I had the same thought, removed that code, and then put it back.  The
problem is that if we revive an older snapshot "from the dead", so to
speak, our backend's advertised xmin might need to go backwards, and
that seems unsafe - e.g. suppose another backend has updated a tuple
but not yet committed.  We don't see any invalidation messages so
decide reuse our existing (old) snapshot and begin a scan.  After
we've looked at the page containing the new tuple (and decided not to
see it), vacuum nukes the old tuple (which we then also don't see).
Bad things ensue.  It might be possible to avoid the majority of
problems in this area via an appropriate set of grotty hacks, but I
don't want to go there.

> And if it needs to change, we could copy the newly generated snapshot
> to the catalog snapshot if it's currently valid.

Yeah, I think there's room for further fine-tuning there.  But I think
it would make sense to push the patch at this point, and then if we
find cases that can be further improved, or things that it breaks, we
can fix them.  This area is complicated enough that I wouldn't be
horribly surprised if we end up having to fix a few more problem cases
or even revert the whole thing, but I think we've probably reached the
point where further review has less value than getting the code out
there in front of more people and seeing where (if anywhere) the
wheels come off out in the wild.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] big test separation POC

[Fabien COELHO]

While testing patch, I found that make installcheck breaks with your patch
and gives following error:


Indeed, I did not put the dependency for that target, I really tested 
"check" & "bigcheck". The attached patch adds the needed dependency for 
installcheck, and I could run it. I checked that no other target seems to 
be missing a dependency...


--
Fabien.diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 7309b00..35d29a4 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -86,7 +86,7 @@ regress_data_files = \
 	$(wildcard $(srcdir)/output/*.source) \
 	$(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \
 	$(wildcard $(srcdir)/data/*.data) \
-	$(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap
+	$(srcdir)/parallel_schedule $(srcdir)/big_schedule $(srcdir)/resultmap
 
 install-tests: all install install-lib installdirs-tests
 	$(MAKE) -C $(top_builddir)/contrib/spi install
@@ -132,13 +132,33 @@ tablespace-setup:
 ## Run tests
 ##
 
+# derive schedules
+derived_schedules = serial_schedule parallel_big_schedule serial_big_schedule
+
+serial_schedule: parallel_schedule
+	echo '# this file is automatically generated, do not edit!' > $@
+	egrep '^(test|ignore):' $< | \
+	while read op list ; do \
+	  for test in $$list ; do \
+	echo "$$op $$test" ; \
+	  done ; \
+	done >> $@
+
+parallel_big_schedule: parallel_schedule big_schedule
+	echo '# this file is automatically generated, do not edit!' > $@
+	cat $^ >> $@
+
+serial_big_schedule: serial_schedule big_schedule
+	echo '# this file is automatically generated, do not edit!' > $@
+	cat $^ >> $@
+
 REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
 
 check: all tablespace-setup
 	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(TEMP_CONF) $(EXTRA_TESTS)
 
-installcheck: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+installcheck: all tablespace-setup serial_schedule
+	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=./serial_schedule $(EXTRA_TESTS)
 
 installcheck-parallel: all tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
@@ -152,11 +172,11 @@ runcheck: check
 runtest: installcheck
 runtest-parallel: installcheck-parallel
 
-bigtest: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
+bigtest: all tablespace-setup serial_big_schedule
+	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=./serial_big_schedule
 
-bigcheck: all tablespace-setup
-	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
+bigcheck: all tablespace-setup parallel_big_schedule
+	$(pg_regress_check) $(REGRESS_OPTS) --schedule=./parallel_big_schedule $(MAXCONNOPT)
 
 
 ##
@@ -166,7 +186,7 @@ bigcheck: all tablespace-setup
 clean distclean maintainer-clean: clean-lib
 # things built by `all' target
 	rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX)
-	rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
+	rm -f pg_regress_main.o pg_regress.o pg_regress$(X) $(derived_schedules)
 # things created by various check targets
 	rm -f $(output_files) $(input_files)
 	rm -rf testtablespace
diff --git a/src/test/regress/big_schedule b/src/test/regress/big_schedule
new file mode 100644
index 000..4058499
--- /dev/null
+++ b/src/test/regress/big_schedule
@@ -0,0 +1,3 @@
+# these are big tests not run by default
+# these test are expected serial, only put one test per line
+test: numeric_big
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
deleted file mode 100644
index d6eaa7a..000
--- a/src/test/regress/serial_schedule
+++ /dev/null
@@ -1,139 +0,0 @@
-# src/test/regress/serial_schedule
-# This should probably be in an order similar to parallel_schedule.
-test: tablespace
-test: boolean
-test: char
-test: name
-test: varchar
-test: text
-test: int2
-test: int4
-test: int8
-test: oid
-test: float4
-test: float8
-test: bit
-test: numeric
-test: txid
-test: uuid
-test: enum
-test: money
-test: rangetypes
-test: strings
-test: numerology
-test: point
-test: lseg
-test: box
-test: path
-test: polygon
-test: circle
-test: date
-test: time
-test: timetz
-test: timestamp
-test: timestamptz
-test: interval
-test: abstime
-test: reltime
-test: tinterval
-test: inet
-test: macaddr
-test: tstypes
-test: comments
-test: geometry
-test: horology
-test: regex
-test: oidjoins
-test: type_sanity
-test: opr_sanity
-test: insert
-test: create_function_1
-test: create_type
-test: create_table
-test: create_function_2
-test: copy
-test: copyselect
-test: create_misc
-test: create_operator
-test: create_index
-test: create_view
-test: create_aggregate
-test: create_function_3
-test: create_cast
-test: constraints
-test: triggers
-

Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division

[Robert Haas]

On Mon, Jul 1, 2013 at 3:19 AM, Jeevan Chalke
 wrote:
> I have re-validated this new patch and it looks good to go in now.
>
> I saw that it's already marked ready for committer.

I don't normally like to commit things over another committer's
objections, but this has +1 votes from four other committers (Stephen,
Noah, Peter E, myself) so I think that's enough reason to move
forward.  So committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] changeset generation v5-01 - Patches & git tree

[Andres Freund]

On 2013-07-01 14:16:55 -0400, Tom Lane wrote:
> Alvaro Herrera  writes:
> > So the question is, do we take the overhead of the new index (which
> > means overhead on DML operations -- supposedly rare) or do we take the
> > overhead of larger WAL records (which means overhead on all DDL
> > operations)?
> 
> > Note we can make either thing apply to only people running logical
> > replication.
> 
> I don't believe you can have or not have an index on pg_class as easily
> as all that.  The choice would have to be frozen at initdb time, so
> people would have to pay the overhead if they thought there was even a
> small possibility that they'd want logical replication later.

It should be possible to create the index in a single database when we
start logical replication in that database? Running the index creation
with a fixed oid shouldn't require too much code. The oid won't be
reused by other pg_class entries since it would be a system one.
Alternatively we could always create the index's pg_class/index entry
but mark it as !indislive when logical replication isn't active for that
database. Then activating it would just require rebuilding that
index.

But then, I am not fully convinced that's worth the trouble since I
don't think pg_class index maintenance is the painspot in DDL atm.

> Flipping the content of WAL records might not be a terribly simple thing
> to do either, but at least in principle it could be done during a
> postmaster restart, without initdb.

The main patch combines various booleans in the heap wal records into a
flags variable, so there should be enough space to keep track of it
without increasing size. Makes size calculations a bit more annoying
though as we use the xlog record length to calculate the heap tuple's
length, but that's not a large problem.
So we could just set the XLOG_HEAP_CONTAINS_CLASSOID flag if wal_level
>= WAL_LEVEL_LOGICAL. Wal decoding then can throw a tantrum if it finds
a record without it and we're done.

We could even make that per database, but that seems to be something for
the future.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Support for RANGE ... PRECEDING windows in OVER

[Robert Haas]

On Sun, Jun 30, 2013 at 11:54 PM, ian link  wrote:
> I found some time and I think I am up to speed now. I finally figured out
> how to add new operator strategies and made a little test operator for
> myself.
>
> It seems pretty clear that assuming '+' and '-' are addition and subtraction
> is a bad idea. I don't think it would be too tricky to add support for new
> operator strategies. Andrew Gierth suggested calling these new strategies
> "offset -" and "offset +", which I think describes it pretty well. I
> assigned the operator itself to be "@+" and "@-" but that can obviously be
> changed. If this sounds like a good path to you guys, I will go ahead and
> implement the operators for the appropriate types. Please let me know if I
> am misunderstanding something - I am still figuring stuff out :)

I don't think I understand the design you have in mind.  I'm actually
not clear that it would be all that bad to assume fixed operator
names, as we apparently do in a few places despite the existence of
operator classes.  But if that is bad, then I don't know how using @+
and @- instead helps anything.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] review: Non-recursive processing of AND/OR lists

[Robert Haas]

On Sun, Jun 30, 2013 at 1:08 PM, Pavel Stehule  wrote:
> 2013/6/30 Gurjeet Singh :
>> On Sun, Jun 30, 2013 at 11:46 AM, Pavel Stehule 
>> wrote:
>>>
>>> 2013/6/30 Gurjeet Singh :
>>> > On Sun, Jun 30, 2013 at 11:13 AM, Pavel Stehule
>>> > 
>>> > wrote:
>>> >
>>> > How about naming those 3 variables as follows:
>>> >
>>> > root_expr_kind
>>> > root_expr_name
>>> > root_bool_expr_type
>>>
>>> +1
>>
>>
>> Thanks. Attached is the patch with that change. I'll update the commitfest
>> entry with a link to this email.
>
> ok
>
> I chechecked it - patched without warnings, all tests passed
>
> It is ready for commit

I think it's a waste of code to try to handle bushy trees.  A list is
not a particularly efficient representation of the pending list; this
will probably be slower than recusing in the common case.  I'd suggest
keeping the logic to handle left-deep trees, which I find rather
elegant, but ditching the pending list.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Move unused buffers to freelist

[Robert Haas]

On Sun, Jun 30, 2013 at 3:24 AM, Amit kapila  wrote:
> Do you think it will be sufficient to just wake bgwriter when the buffers in 
> freelist drops
> below low watermark, how about it's current job of flushing dirty buffers?

Well, the only point of flushing dirty buffers in the background
writer is to make sure that backends can allocate buffers quickly.  If
there are clean buffers already in the freelist, that's not a concern.
 So...

> I mean to ask that if for some scenario where there are sufficient buffers in 
> freelist, but most
> other buffers are dirty, will delaying flush untill number of buffers fall 
> below low watermark is okay.

...I think this is OK, or at least we should assume it's OK until we
have evidence that it isn't.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Jeff Davis]

On Tue, 2013-07-02 at 02:13 +0900, Fujii Masao wrote:
> Even in that case, if a user can easily know which platform posix_fallocate
> should be used in, we can commit the patch with the configurable GUC
> parameter.

I disagree here. We're not talking about a huge win; this speedup may
not even be detectable on a running system.

I think Robert summarized the reason for the patch best: "I mean, if
posix_fallocate() is faster, then it's just faster, right?". But if we
need a new GUC, and DBAs now have one more thing they need to test about
their platform, then that argument goes out the window.

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] changeset generation v5-01 - Patches & git tree

[Tom Lane]

Alvaro Herrera  writes:
> So the question is, do we take the overhead of the new index (which
> means overhead on DML operations -- supposedly rare) or do we take the
> overhead of larger WAL records (which means overhead on all DDL
> operations)?

> Note we can make either thing apply to only people running logical
> replication.

I don't believe you can have or not have an index on pg_class as easily
as all that.  The choice would have to be frozen at initdb time, so
people would have to pay the overhead if they thought there was even a
small possibility that they'd want logical replication later.

Flipping the content of WAL records might not be a terribly simple thing
to do either, but at least in principle it could be done during a
postmaster restart, without initdb.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] changeset generation v5-01 - Patches & git tree

[Alvaro Herrera]

Since this discussion seems to have stalled, let me do a quick summary.
The goal of this subset of patches is to allow retroactive look up of
relations starting from a WAL record.  Currently, the WAL record only
tracks the relfilenode that it affects, so there are two possibilities:

1. we add some way to find out the relation OID from the relfilenode,
2. we augment the WAL record with the relation OID.

Each solution has its drawbacks.  For the former,
* we need a new cache
* we need a new pg_class index
* looking up the relation OID still requires some CPU runtime and memory
  to keep the caches in; run invalidations, etc.

For the latter,
* each WAL record would become somewhat bigger.  For WAL records with a
  payload of 25 bytes (say insert a tuple which is 25 bytes long) this
  means about 7% overhead.

There are some other issues, but these can be solved.  For instance Tom
doesn't want a syscache on top of a non-unique index, and I agree on
that.  But if we agree on this way forward, we can just go a different
route by keeping a separate cache layer.

So the question is, do we take the overhead of the new index (which
means overhead on DML operations -- supposedly rare) or do we take the
overhead of larger WAL records (which means overhead on all DDL
operations)?

Note we can make either thing apply to only people running logical
replication.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Outputting UTC offset with to_char()

[Bruce Momjian]

Applied.  I referenced macros for some of the new constants, e.g.
SECS_PER_HOUR.

---

On Fri, Jun 28, 2013 at 10:04:49PM -0400, Bruce Momjian wrote:
> On Sun, Oct 21, 2012 at 05:40:40PM -0400, Andrew Dunstan wrote:
> > 
> > I'm not sure if this has come up before.
> > 
> > A client was just finding difficulties because to_char() doesn't
> > support formatting the timezone part of a timestamptz numerically
> > (i.e. as +-hhmm) instead of using a timezone name. Is there any
> > reason for that? Would it be something worth having?
> 
> Great idea!  I have developed the attached patch to do this:
> 
>   test=> SELECT to_char(current_timestamp, 'OF');
>to_char
>   -
>-04
>   (1 row)
>   
>   test=> SELECT to_char(current_timestamp, 'TMOF');
>to_char
>   -
>-04
>   (1 row)
>   
>   test=> SET timezone = 'Asia/Calcutta';
>   SET
>   test=> SELECT to_char(current_timestamp, 'OF');
>to_char
>   -
>+05:30
>   (1 row)
>   
>   test=> SELECT to_char(current_timestamp, 'FMOF');
>to_char
>   -
>+5:30
>   (1 row)
> 
> I went with the optional colon and minutes because this is how we output
> it:
> 
>   test=> SELECT current_timestamp;
> now
>   ---
>2013-06-28 22:02:24.773587-04
>  ---
>   (1 row)
>   
>   test=> set timezone = 'Asia/Calcutta';
>   SET
>   test=> SELECT current_timestamp;
>  now
>   --
>2013-06-29 07:32:29.157565+05:30
>  --
>   (1 row)
> 
> -- 
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
> 
>   + It's impossible for everything to be true. +

> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
> new file mode 100644
> index 7c009d8..5765ddf
> *** a/doc/src/sgml/func.sgml
> --- b/doc/src/sgml/func.sgml
> *** SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1
> *** 5645,5650 
> --- 5645,5654 
>   tz
>   lower case time-zone name
>  
> +
> + OF
> + time-zone offset
> +
> 
>
>   
> diff --git a/src/backend/utils/adt/formatting.c 
> b/src/backend/utils/adt/formatting.c
> new file mode 100644
> index 7b85406..4c272ef
> *** a/src/backend/utils/adt/formatting.c
> --- b/src/backend/utils/adt/formatting.c
> *** typedef enum
> *** 600,605 
> --- 600,606 
>   DCH_MS,
>   DCH_Month,
>   DCH_Mon,
> + DCH_OF,
>   DCH_P_M,
>   DCH_PM,
>   DCH_Q,
> *** static const KeyWord DCH_keywords[] = {
> *** 746,751 
> --- 747,753 
>   {"MS", 2, DCH_MS, TRUE, FROM_CHAR_DATE_NONE},
>   {"Month", 5, DCH_Month, FALSE, FROM_CHAR_DATE_GREGORIAN},
>   {"Mon", 3, DCH_Mon, FALSE, FROM_CHAR_DATE_GREGORIAN},
> + {"OF", 2, DCH_OF, FALSE, FROM_CHAR_DATE_NONE},  /* O */
>   {"P.M.", 4, DCH_P_M, FALSE, FROM_CHAR_DATE_NONE},   /* P */
>   {"PM", 2, DCH_PM, FALSE, FROM_CHAR_DATE_NONE},
>   {"Q", 1, DCH_Q, TRUE, FROM_CHAR_DATE_NONE}, /* Q */
> *** static const int DCH_index[KeyWord_INDEX
> *** 874,880 
>   -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>   -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>   -1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1,
> ! DCH_FX, -1, DCH_HH24, DCH_IDDD, DCH_J, -1, -1, DCH_MI, -1, -1,
>   DCH_P_M, DCH_Q, DCH_RM, DCH_, DCH_TZ, DCH_US, -1, DCH_WW, -1, 
> DCH_Y_YYY,
>   -1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc,
>   DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iddd, DCH_j, -1, -1, DCH_mi,
> --- 876,882 
>   -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>   -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>   -1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1,
> ! DCH_FX, -1, DCH_HH24, DCH_IDDD, DCH_J, -1, -1, DCH_MI, -1, DCH_OF,
>   DCH_P_M, DCH_Q, DCH_RM, DCH_, DCH_TZ, DCH_US, -1, DCH_WW, -1, 
> DCH_Y_YYY,
>   -1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc,
>   DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iddd, DCH_j, -1, -1, DCH_mi,
> *** DCH_to_char(FormatNode *node, bool is_in
> *** 2502,2507 
> --- 2504,2519 
>   s += strlen(s);
>   }
>   break;
> + case DCH_OF:
> + INVALID_FOR_INTERVAL;
> + sprintf(s, "%+0*ld", S_FM(n->suffix) ? 0 : 3, 
> tm->tm_gmtoff / 3600);
> + s += strlen(s);
> + if (tm->tm_gmtoff % 3600 != 0)
> + {
> + 

Re: [HACKERS] Eliminating PD_ALL_VISIBLE, take 2

[Jeff Davis]

On Sun, 2013-06-30 at 22:58 -0400, Robert Haas wrote:
> I thought that Jeff withdrew this patch.

No -- was there a reason you thought that? I know it could use another
round of testing before commit, and there may be a couple other things
to clear up. But I don't want to invest a lot of time there right now,
because, as I understand it, you still object to the patch anyway.

I am still not entirely clear on the objections to this patch:

1. Contention was a concern, but I believe I have mitigated it. Strictly
speaking, additional pins may be acquired, but the cost of those pin
operations will be spread over a lot of other work.

2. There are quite a few different ideas about where we're going with
PD_ALL_VISIBLE and freezing, but it seems like removing PD_ALL_VISIBLE
is potentially compatible with most of them.

Any others?

The patch reduces code complexity and reduces writes during a data load.

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] Minor inheritance/check bug: Inconsistent behavior

['Bruce Momjian']

On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote:
> >> I have done the initial analysis and prepared a patch, don't know if
> >> anything more I can do until
> >> someone can give any suggestions to further proceed on this bug.
> 
> >So, I guess we never figured this out.
> 
> I can submit this bug-fix for next commitfest if there is no objection for 
> doing so.
> What is your opinion?

Yes, good idea.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Fujii Masao]

On Tue, Jul 2, 2013 at 1:55 AM, Jeff Davis  wrote:
> On Sun, 2013-06-30 at 18:55 -0400, Greg Smith wrote:
>> This makes platform level testing a lot easier, thanks.  Attached is an
>> updated copy of that program with some error checking.  If the files it
>> creates already existed, the code didn't notice, and a series of write
>> errors happened.  If you set the test up right it's not a problem, but
>> it's better if a bad setup is caught.  I wrapped the whole test with a
>> shell script, also attached, which insures the right test sequence and
>> checks.
>
> Thank you.
>
>> That's glibc helpfully converting your call to posix_fallocate into
>> small writes, because the OS doesn't provide a better way in that
>> kernel.  It's not hard to imagine this being slower than what the WAL
>> code is doing right now.  I'm not worried about correctness issues
>> anymore, but my gut paranoia about this not working as expected on older
>> systems was justified.  Everyone who thought I was just whining owes me
>> a cookie.
>
> So your theory is that it may be slower because there are twice as many
> syscalls (one per 4K page rather than one per 8K page)? Interesting
> observation.
>
>> This is what I plan to benchmark specifically next.
>
> In the interest of keeping this patch moving forward, do you have an
> estimate for when this testing will be complete?
>
>>   If the
>> posix_fallocate approach is actually slower than what's done now when
>> it's not getting kernel acceleration, which is the case on RHEL5 era
>> kernels, we might need to make the configure time test more complicated.
>>   Whether posix_fallocate is defined isn't sensitive enough; on Linux it
>> may be the case that this only is usable when fallocate() is also there.
>
> I'd say that if posix_fallocate is slower than the existing code on
> pretty much any platform, we shouldn't commit the patch at all.

Even in that case, if a user can easily know which platform posix_fallocate
should be used in, we can commit the patch with the configurable GUC
parameter.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[Atri Sharma]

On Mon, Jul 1, 2013 at 10:02 PM, David Fetter  wrote:
> On Mon, Jul 01, 2013 at 04:42:04AM -0700, jasmine wrote:
>> My PostgresSQL (9.2) is crashing after certain load tests. Currently,
>> postgressql is crashing when simulatenously 800 to 1000 threads are run on a
>> 10 million records schema. Not sure, if we have to tweak some more
>> parameters of postgres.
>
> Jasmine,
>
> Please start your own thread rather than replying to some unrelated
> thread.
>
> In the particular case above, please also to provide reproducible test
> cases including the exact version(s) of PostgreSQL to which they apply
> (9.2.4, e.g.) along with output from same so other people can see what
> you're talking about and actually help.

Not sure if it belongs to -general instead of -hackers.

Regards,

Atri

--
Regards,

Atri
l'apprenant


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

[Alvaro Herrera]

I think this is unlikely to work reliably:

+   PG_TRY();
+   {
+   ExtensionControl *control = read_extension_control_file(extname);
+ 
+   if (control)
+   {
+   ereport(ERROR,
+   (errcode(ERRCODE_DUPLICATE_OBJECT),
+errmsg("extension \"%s\" is already available", extname)));
+   }
+   }
+   PG_CATCH();
+   {
+   /* no control file found is good news for us */
+   }
+   PG_END_TRY();

What if read_extension_control_file() fails because of an out-of-memory
error?  I think you need to extend that function to have a more useful
API, not rely on it raising a specific error.  There is at least one
more case when you're calling that function in the same way.

It'd probably work to have a boolean return (found/not found), and
return the ExtensionControl structure through a pointer.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Jeff Davis]

On Sun, 2013-06-30 at 18:55 -0400, Greg Smith wrote:
> This makes platform level testing a lot easier, thanks.  Attached is an 
> updated copy of that program with some error checking.  If the files it 
> creates already existed, the code didn't notice, and a series of write 
> errors happened.  If you set the test up right it's not a problem, but 
> it's better if a bad setup is caught.  I wrapped the whole test with a 
> shell script, also attached, which insures the right test sequence and 
> checks.

Thank you.

> That's glibc helpfully converting your call to posix_fallocate into 
> small writes, because the OS doesn't provide a better way in that 
> kernel.  It's not hard to imagine this being slower than what the WAL 
> code is doing right now.  I'm not worried about correctness issues 
> anymore, but my gut paranoia about this not working as expected on older 
> systems was justified.  Everyone who thought I was just whining owes me 
> a cookie.

So your theory is that it may be slower because there are twice as many
syscalls (one per 4K page rather than one per 8K page)? Interesting
observation.

> This is what I plan to benchmark specifically next.

In the interest of keeping this patch moving forward, do you have an
estimate for when this testing will be complete?

>   If the 
> posix_fallocate approach is actually slower than what's done now when 
> it's not getting kernel acceleration, which is the case on RHEL5 era 
> kernels, we might need to make the configure time test more complicated. 
>   Whether posix_fallocate is defined isn't sensitive enough; on Linux it 
> may be the case that this only is usable when fallocate() is also there.

I'd say that if posix_fallocate is slower than the existing code on
pretty much any platform, we shouldn't commit the patch at all. I would
be quite surprised if that was the case, however.

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] Optimizing pglz compressor

[Bruce Momjian]

On Mon, Jul  1, 2013 at 11:05:37AM +0300, Heikki Linnakangas wrote:
> On 26.06.2013 16:37, Amit Kapila wrote:
> >On Wednesday, June 26, 2013 2:15 AM Heikki Linnakangas wrote:
> >>Can you also try the attached patch, please? It's the same as before,
> >>but in this version, I didn't replace the prev and next pointers in
> >>PGLZ_HistEntry struct with int16s. That avoids some table lookups, at
> >>the expense of using more memory. It's closer to what we have without
> >>the patch, so maybe that helps on your system.
> >
> >Yes it helped a lot on my system.
> 
> Ok, good. Strange, I did not expect such a big difference.
> 
> >There was minor problem in you patch, in one of experiments it crashed.
> >Fix is not to access 0th history entry in function pglz_find_match(),
> >modified patch is attached.
> 
> Thanks, good catch! I thought that a pointer to the 0th entry would
> never make it into the prev/next fields, but it does. In fact, we
> never store a NULL there anymore, a pointer to the 0th entry is now
> always used to mean 'invalid'. I adjusted the patch to remove the
> NULL check, and only check for the 0th entry.
> 
> Committed.

Does someone have new tests comparing this patch with the new
compression methods being considered?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] "pg_ctl promote" exit status

[Bruce Momjian]

On Mon, Jul  1, 2013 at 10:11:23AM -0400, Peter Eisentraut wrote:
> On 6/28/13 10:50 PM, Bruce Momjian wrote:
> > On Mon, Jan 28, 2013 at 09:46:32AM -0500, Peter Eisentraut wrote:
> >> On 1/26/13 4:44 PM, Aaron W. Swenson wrote:
> >>> You are right. Had I read a little further down, it seems that the
> >>> exit status should actually be 7.
> >>
> >> 7 is OK for "not running", but what should we use when the server is not
> >> in standby mode?  Using the idempotent argument that we are discussing
> >> for the stop action, promoting a server that is not a standby should be
> >> a noop and exit successfully.  Not sure if that is what we want, though.
> > 
> > I looked at all the LSB return codes listed here and mapped them to
> > pg_ctl error situations:
> > 
> > 
> > https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
> > 
> > Patch attached.  I did not touch the start/stop return codes.
> 
> Approximately none of these changes seem correct to me.  For example,
> why is failing to open the PID file 6, or failing to start the server 7?

Well, according to that URL, we have:

6   program is not configured
7   program is not running

I just updated the pg_ctl.c comments to at least point to a valid URL
for this.  I think we can just call this item closed because I am still
unclear if these return codes should be returned by pg_ctl or the
start/stop script.

Anyway, while I do think pg_ctl could pass a little more information
back about failure via its return code, I am unclear if LSB is the right
approach.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_ctl and -h/help

[Bruce Momjian]

On Sun, Jun 30, 2013 at 02:29:20PM +0900, Michael Paquier wrote:
> On Sat, Jun 29, 2013 at 10:45 PM, Bruce Momjian  wrote:
> > In studying pg_upgrade's handling of --help, I noticed that pg_ctl
> > supports -h for help, but it is the only tool to do so, and -h is not
> > documented.  I propose we remove -h for help in pg_ctl, and have it
> > support only -? and --help.
> I suppose that it doesn't hurt to have it, but for yes the sake of
> consistency with the other binaries it would make sense to remove it.
> Btw, not even the docs, it is also not listed in the --help message
> findable in code.

Agreed --- attached patch applied.  I also noticed that we sometimes
test for -? then --help, but other times do things in the opposite
order, and the same for -V/--version, so I made that consistent.

However, I also noticed that while we document -? before --help, we test
for --help before -?, and the same for -V/--version.  Should I make
those even more consistent by always testing for the single-letter
option first?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_test_fsync/pg_test_fsync.c b/contrib/pg_test_fsync/pg_test_fsync.c
new file mode 100644
index b978d9e..53f600f
*** a/contrib/pg_test_fsync/pg_test_fsync.c
--- b/contrib/pg_test_fsync/pg_test_fsync.c
*** handle_args(int argc, char *argv[])
*** 146,153 
  
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-h") == 0 ||
! 			strcmp(argv[1], "-?") == 0)
  		{
  			printf("Usage: %s [-f FILENAME] [-s SECS-PER-TEST]\n", progname);
  			exit(0);
--- 146,152 
  
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
  		{
  			printf("Usage: %s [-f FILENAME] [-s SECS-PER-TEST]\n", progname);
  			exit(0);
diff --git a/contrib/pg_test_timing/pg_test_timing.c b/contrib/pg_test_timing/pg_test_timing.c
new file mode 100644
index 0bf9127..e44c535
*** a/contrib/pg_test_timing/pg_test_timing.c
--- b/contrib/pg_test_timing/pg_test_timing.c
*** handle_args(int argc, char *argv[])
*** 49,56 
  
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-h") == 0 ||
! 			strcmp(argv[1], "-?") == 0)
  		{
  			printf("Usage: %s [-d DURATION]\n", progname);
  			exit(0);
--- 49,55 
  
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
  		{
  			printf("Usage: %s [-d DURATION]\n", progname);
  			exit(0);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index 9045e00..9e909ae
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
*** main(int argc, char **argv)
*** 2002,2014 
  	/* support --help and --version even if invoked as root */
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "-h") == 0 || strcmp(argv[1], "--help") == 0 ||
! 			strcmp(argv[1], "-?") == 0)
  		{
  			do_help();
  			exit(0);
  		}
! 		else if (strcmp(argv[1], "-V") == 0 || strcmp(argv[1], "--version") == 0)
  		{
  			puts("pg_ctl (PostgreSQL) " PG_VERSION);
  			exit(0);
--- 2002,2013 
  	/* support --help and --version even if invoked as root */
  	if (argc > 1)
  	{
! 		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
  		{
  			do_help();
  			exit(0);
  		}
! 		else if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
  		{
  			puts("pg_ctl (PostgreSQL) " PG_VERSION);
  			exit(0);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
new file mode 100644
index 1c9f7a5..b2264c9
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*** parse_psql_options(int argc, char *argv[
*** 558,564 
  break;
  			case '?':
  /* Actual help option given */
! if (strcmp(argv[optind - 1], "-?") == 0 || strcmp(argv[optind - 1], "--help") == 0)
  {
  	usage();
  	exit(EXIT_SUCCESS);
--- 558,564 
  break;
  			case '?':
  /* Actual help option given */
! if (strcmp(argv[optind - 1], "--help") == 0 || strcmp(argv[optind - 1], "-?") == 0)
  {
  	usage();
  	exit(EXIT_SUCCESS);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

[Alvaro Herrera]

Very minor comment here: these SGML "id" tags:

+ 

are pretty important, because they become the URL for the specific page
in the reference docs.  So I think you should fix them to be the correct
spelling of the command "alter template for extension", and also perhaps
add an hyphen or two.  Maybe "SQL-ALTER-EXTENSION-FOR-TEMPLATE".  (We're
inconsistent about adding hyphens; most URLs don't have hyphens after
then "sql-" bit, so "sql-altertablespace", but we have some examples of
the opposite such as "sql-commit-prepared" and "sql-drop-owned".)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] fallocate / posix_fallocate for new WAL file creation (etc...)

[Jeff Davis]

On Sun, 2013-06-30 at 16:09 -0400, Andrew Dunstan wrote:
> It was originally generated. Since then it's been maintained by hand.

What is the procedure for maintaining it by hand? Why are
HAVE_POSIX_SIGNALS and HAVE_SYNC_FILE_RANGE in there (though commented
out), but not HAVE_POSIX_FADVISE?

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] Randomisation for ensuring nlogn complexity in quicksort

[David Fetter]

On Mon, Jul 01, 2013 at 04:42:04AM -0700, jasmine wrote:
> My PostgresSQL (9.2) is crashing after certain load tests. Currently,
> postgressql is crashing when simulatenously 800 to 1000 threads are run on a
> 10 million records schema. Not sure, if we have to tweak some more
> parameters of postgres. 

Jasmine,

Please start your own thread rather than replying to some unrelated
thread.

In the particular case above, please also to provide reproducible test
cases including the exact version(s) of PostgreSQL to which they apply
(9.2.4, e.g.) along with output from same so other people can see what
you're talking about and actually help.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: FILTER for aggregates [was Re: [HACKERS] Department of Redundancy Department: makeNode(FuncCall) division]

[Dean Rasheed]

On 1 July 2013 01:44, David Fetter  wrote:
> On Fri, Jun 28, 2013 at 09:22:52PM +0100, Dean Rasheed wrote:
>> On 21 June 2013 06:16, David Fetter  wrote:
>> > Please find attached a patch which allows subqueries in the FILTER
>> > clause and adds regression testing for same.
>> >
>>
>> This needs re-basing/merging following Robert's recent commit to make
>> OVER unreserved.
>
> Please find attached.  Thanks, Andrew Gierth!  In this one, FILTER is
> no longer a reserved word.
>

Looking at this patch again, it appears to be in pretty good shape.

- Applies cleanly to head.
- Compiles with no warnings.
- Includes regression test cases and doc updates.
- Compatible with the relevant part of T612, "Advanced OLAP operations".
- Includes pg_dump support.
- Code changes all look reasonable, and I can't find any corner cases
that have been missed.
- Appears to work as expected. I tested everything I could think of
and couldn't break it.

AFAICT all the bases have been covered. As mentioned upthread, I would
have preferred a few more regression test cases, and a couple of the
tests don't appear to return anything interesting, but I'll leave that
for the committer to decide whether they're sufficient for regression
tests.

I have a few suggestions to improve the docs:

1). In syntax.sgml: "The aggregate_name can also be suffixed with
FILTER as described below". It's not really a suffix to the aggregate
name, since it follows the function arguments and optional order by
clause. Perhaps it would be more consistent with the surrounding text
to say something like

expression is
any value expression that does not itself contain an aggregate
expression or a window function call, and
!order_by_clause and
!filter_clause are optional
!ORDER BY and FILTER clauses as described below.

2). In syntax.sgml: "... or when a FILTER clause is present, each row
matching same". In the context of that paragraph this suggests that
the filter clause only applies to the first form, since that paragraph
is a description of the 4 forms of the aggregate function. I don't
think it's worth mentioning FILTER in this paragraph at all --- it's
adequately described below that.

3). In syntax.sgml: "Adding a FILTER clause to an aggregate specifies
which values of the expression being aggregated to evaluate". How
about something a little more specific, along the lines of

If FILTER is specified, then only input rows for which
the filter_clause evaluates to true are
fed to the aggregate function; input rows for which the
filter_clause evaluates to false or the
null value are discarded.  For example...

4). In select.sgml: "In the absence of a FILTER clause, aggregate
functions...". It doesn't seem right to refer to the FILTER clause at
the top level here because it's not part of the SELECT syntax being
described on this page. Also I think this should include a
cross-reference to the aggregate function syntax section, perhaps
something like:

Aggregate functions, if any are used, are computed across all rows
making up each group, producing a separate value for each group
(whereas without GROUP BY, an aggregate
produces a single value computed across all the selected rows).
+The set of rows fed to the aggregate function can be further filtered
+by attaching a FILTER clause to the aggregate
+function call, see  for more information.
When GROUP BY is present, it is not valid for
the SELECT list expressions to refer to
ungrouped columns except within aggregate functions or if the
ungrouped column is functionally dependent on the grouped columns,
since there would otherwise be more than one possible value to
return for an ungrouped column.  A functional dependency exists if
the grouped columns (or a subset thereof) are the primary key of
the table containing the ungrouped column.

Regards,
Dean


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Randomisation for ensuring nlogn complexity in quicksort

[jasmine]

My PostgresSQL (9.2) is crashing after certain load tests. Currently,
postgressql is crashing when simulatenously 800 to 1000 threads are run on a
10 million records schema. Not sure, if we have to tweak some more
parameters of postgres. 



-
jasmine
--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Randomisation-for-ensuring-nlogn-complexity-in-quicksort-tp5761907p5762011.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Passing fdw_private data from PlanForeignScan to PlanForeignModify

[Bernd Helmle]

--On 13. Juni 2013 18:12:05 -0400 Tom Lane  wrote:


What i tried before was to access (in PlanForeignModify) the RelOptInfo
structure through PlannerInfo->simple_rel_array, assuming the the
resultRelation index points to the right array member. However, that
didn't  work, the fdw_private List is not the one filled by
GetForeignPlan...is  there another way to get back the RelOptInfo worked
on earlier?


It should work ... *if* there was in fact a RelOptInfo worked on
earlier.  There sometimes isn't.  You might need to do something like
what make_modifytable() has to do to call you in the first place:


Sorry for the late feedback, didn't manage to get back to this earlier.

This works indeed, the RelOptInfo structure stored in simple_rel_array (if 
present) allows
a FDW to access its earlier scan state, assigned in GetForeignPlan() for 
example.


Thanks for the hints!

--

Bernd



--
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] fallocate / posix_fallocate for new WAL file creation (etc...)

[Jon Nelson]

On Sun, Jun 30, 2013 at 11:52 PM, Greg Smith  wrote:
> On 6/30/13 9:28 PM, Jon Nelson wrote:
>>
>> The performance of the latter (new) test sometimes seems to perform
>> worse and sometimes seems to perform better (usually worse) than
>> either of the other two. In all cases, posix_fallocate performs
>> better, but I don't have a sufficiently old kernel to test with.
>
>
> This updated test program looks reliable now.  The numbers are very tight
> when I'd expect them to be, and there's nowhere with the huge differences I
> saw in the earlier test program.
>
> Here's results from a few sets of popular older platforms:

If you found yourself with a spare moment, could you run these again
with the number of open/close cycles set high (say, 100) and the
number of rewrites set to 0 and also to 1? Most of the time spent is
actually spent overwriting the files so by reducing or eliminating
that aspect it might be easier to get a handle on the actual
performance difference.



--
Jon


-- 
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] Documentation/help for materialized and recursive views

[David Fetter]

On Mon, Jul 01, 2013 at 10:52:55AM -0400, Peter Eisentraut wrote:
> On 7/1/13 10:20 AM, David Fetter wrote:
> > On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote:
> >> On 6/28/13 2:27 PM, David Fetter wrote:
>  You can run \! man from within psql,
> >>> And if you're on Windows, you're Sadly Out of Luck with that.
> >>> Is there an equivalent we could #ifdef in for that platform?
> >>
> >> If you are using psql on Windows extensively, you probably have
> >> one of mingw, cygwin, or pgadmin handy, all of which can get you
> >> to the documentation.  I don't think it's worth devising a
> >> mechanism for those not covered by this.
> > 
> > With deepest respect, failing to provide documentation to users on
> > our widest-deployed platform seems pretty hostile to me.
> 
> As I argue above, I don't think this is the widest-deployed
> platform.  The actual platform in use is either mingw, which has
> man, or click-and-drool, which has pgadmin, both of which provide
> the documentation.

I'm not going to get into a big definitional wrangle here.  "Has
available" (as in you could install software if you wanted to) is a
pretty long distance from "actually handy," which URLs are a bit
closer to.

> > There was an earlier suggestion that we provide URLs, which seems
> > like a decent way forward as those environments so locked down as
> > to disallow outbound HTTP are pretty rare, and non-networked
> > computers are even more rare.
> 
> Does clicking on links in cmd.exe do anything useful?

Apparently

start URL

works for some large class of URLs.  More details here:
http://www.dwheeler.com/essays/open-files-urls.html

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Shorter iterations of join_info_list

[Antonin Houska]

As far as I understand, deconstruct_recurse() ensures that 
SpecialJoinInfo of a new join always gets added to higher position in 
join_info_list than SJ infos of all joins located below the new join in 
the tree. I wonder if we can rely on that fact sometimes.


One possible use case could be 
placeholder.c:update_placeholder_eval_levels():


  1. The first (in terms of position in join_info_list) join above 
phinfo->ph_var->phrels can be marked as the (exclusive) upper bound for 
all iterations.


  2. The first join for which particular iteration of SJ infos 
identifies the necessity to extend eval_at can be marked in 
join_info_list as (exclusive) lower bound for the next iteration. This 
is because that addition can only affect parents of the join whose 
relations we just added to eval_at. And these essentially can't be 
located at lower positions in join_info_list.


The lower limit could also be used in initsplan.c:check_outerjoin_delay().

Is this worth a patch? It's not much coding but I'd appreciate some 
feedback before I try to do anything.


Thanks,
Antonin Houska (Tony)


--
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] request a new feature in fuzzystrmatch

[Alvaro Herrera]

Joe Conway escribió:

> Actually, given that this change will create version 1.1 of the
> extension, I believe the 1.0 versions of the sql scripts should
> probably be removed entirely. Can someone with more knowledge of the
> extension facility comment on that?

Besides what Michael said, another thing is that you need to ensure that
if the functions are run with the 1.0 definitions, they don't crash
(i.e. you need to check the number of arguments actually passed to
function(s), and ensure no changes are made to the types of previously
existing arguments).  You can test this by installing the 1.0 version in
a 9.3 database, then pg_upgrade, and test the functions without running
the ALTER EXTENSION UPGRADE.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] checking variadic "any" argument in parser - should be array

[Pavel Stehule]

2013/6/29 Pavel Stehule :
> Hello
>
> updated patch - precious Assert, more comments
>
> Regards
>
> Pavel
>

stripped


variadic_any_parser_check-3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] checking variadic "any" argument in parser - should be array

[Alvaro Herrera]

Pavel Stehule escribió:
> Hello
> 
> updated patch - precious Assert, more comments

Pavel, can you please remove quoted text from messages you reply to?
This message has 10kb of completely useless text.

Thanks,

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Documentation/help for materialized and recursive views

[Peter Eisentraut]

On 7/1/13 10:20 AM, David Fetter wrote:
> On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote:
>> On 6/28/13 2:27 PM, David Fetter wrote:
 You can run \! man from within psql,
>>> And if you're on Windows, you're Sadly Out of Luck with that.  Is
>>> there an equivalent we could #ifdef in for that platform?
>>
>> If you are using psql on Windows extensively, you probably have one of
>> mingw, cygwin, or pgadmin handy, all of which can get you to the
>> documentation.  I don't think it's worth devising a mechanism for those
>> not covered by this.
> 
> With deepest respect, failing to provide documentation to users on our
> widest-deployed platform seems pretty hostile to me.

As I argue above, I don't think this is the widest-deployed platform.
The actual platform in use is either mingw, which has man, or
click-and-drool, which has pgadmin, both of which provide the documentation.

> There was an
> earlier suggestion that we provide URLs, which seems like a decent way
> forward as those environments so locked down as to disallow outbound
> HTTP are pretty rare, and non-networked computers are even more rare.

Does clicking on links in cmd.exe do anything useful?


-- 
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] New regression test time

[Andres Freund]

On 2013-07-01 07:14:23 -0700, David Fetter wrote:
> > If we had a different set of tests, that would be a valid argument.  But
> > we don't, so it's not.  And nobody has offered to write a feature to
> > split our tests either.

> With utmost respect, this just isn't true.  There is a "make coverage"
> target that probably doesn't get enough exercise, but it's just the
> kind of infrastructure you're describing.

Uh? Isn't make coverage a target for collecting the generated coverage
data? Afaik it itself does *NOT* depend on any checks being run. And it
only does something sensible if --enable-coverage is passed to
./configure anyway.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] big test separation POC

[Samrat Revagade]

Hi Fabien,

On Mon, Jul 1, 2013 at 10:42 AM, Fabien COELHO  wrote:

>
>   - I do not understand why the makefile specifies $(srcdir) before
>>>local files in some places.
>>>
>>
>> For VPATH builds :-)
>>
>
> Here is a v2 which is more likely to work under VPATH.


I really appreciate your efforts. I am reviewing your patch.

While testing patch, I found that make installcheck breaks with your patch
and gives following error:

== running regression test queries==
pg_regress: could not open file "./serial_schedule" for reading: No such
file or directory

looks like you forgot to add entry for serial_schedule.

Following sequence of commands will reproduces this error:
1)  ./configure
2) make
3) make install
4) initialize the database cluster (initdb)
5) start the server
6) make installcheck

I will post more review comments if there are any.


-- 
Regards,

Samrat Revgade

Re: [HACKERS] Documentation/help for materialized and recursive views

[David Fetter]

On Mon, Jul 01, 2013 at 10:05:24AM -0400, Peter Eisentraut wrote:
> On 6/28/13 2:27 PM, David Fetter wrote:
> >> You can run \! man from within psql,
> > And if you're on Windows, you're Sadly Out of Luck with that.  Is
> > there an equivalent we could #ifdef in for that platform?
> 
> If you are using psql on Windows extensively, you probably have one of
> mingw, cygwin, or pgadmin handy, all of which can get you to the
> documentation.  I don't think it's worth devising a mechanism for those
> not covered by this.

With deepest respect, failing to provide documentation to users on our
widest-deployed platform seems pretty hostile to me.  There was an
earlier suggestion that we provide URLs, which seems like a decent way
forward as those environments so locked down as to disallow outbound
HTTP are pretty rare, and non-networked computers are even more rare.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] LDAP: bugfix and deprecated OpenLDAP API

[Albe Laurenz]

Peter Eisentraut wrote:
> Btw., I just checked the source code of Apache, PHP, and PAM, and they
> are all unconditionally building with LDAP_DEPRECATED.  So maybe there
> is no hurry about this.

I don't think that the old API functions will go away until there
is a new standard for the LDAP C API, but I have no inside knowledge
of OpenLDAP.

Yours,
Laurenz Albe

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] LDAP: bugfix and deprecated OpenLDAP API

[Albe Laurenz]

Magnus Hagander wrote:
> On Tue, Feb 5, 2013 at 10:39 AM, Albe Laurenz  wrote:
>> I found a small bug in the implementation of LDAP connection
>> parameter lookup.

[...]

>> As coded now, the timeout won't work - if the LDAP server
>> is down, ldap_simple_bind will wait for the network
>> timeout, which will be quite longer than 2 seconds.
>>
>> The attached patch ldap-bug.patch fixes this problem;
>> unfortunately I found no way that works both with OpenLDAP
>> and Windows LDAP, so I had to add an #ifdef.
>>
>> I think that this patch should be applied and backpatched.
> 
> So just to be clear - the difference is we're going from implicit
> anonymous bind, to an explicit one? We're not actually causing an
> extra bind compared to previous versions?

No, it was an explicit bind before as well.
The bug I discovered is that if you ldap_simple_bind, the
timeout set in ldap_result does not work for network timeouts.

It always waits until the TCP connection is established or fails,
which can take much longer than PGLDAP_TIMEOUT seconds.

With OpenLDAP you can set the LDAP_OPT_NETWORK_TIMEOUT option
to enforce a timeout, but on Windows you have to use the
nonstandard function ldap_connect.

>> I also tried to fix the problem mentioned in
>> http://www.postgresql.org/message-id/CA+TgmoYnj=Es3L_0Q8+ijR4tVhvztW1fb=7c9k9gemzwqhp...@mail.gmail.com
>> that we use deprecated OpenLDAP functions, see the attached
>> ldap-undeprecate.patch.
>>
>> I added a file ldap.c in src/port with my own implementation
>> of some of the functions that OpenLDAP has deprecated.
>> With that, the code changes necessary are pretty minimal.
> 
> Doesn't this need a version check against OpenSSL at some point, or a
> configure check? Are we just assuming that all versions that people
> ever use have the function deprecated? (That's probably not entirely
> unreasonable, just double checking)

I checked, and it should work fine since OpenLDAP 2.0.0, released in 2000.
The current version is 2.4.35.
In 2.0.0 the old API was not yet deprecated, but the new functions are
already there so that PostgreSQL should work.

I don't know if that would require a check, but it should be simple
to add a configure test if we are sufficiently paranoid.

I'll be on vacation from Wednesday on until July 20th.

Yours,
Laurenz Albe

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] New regression test time

[David Fetter]

On Sat, Jun 29, 2013 at 02:59:35PM -0700, Josh Berkus wrote:
> On 06/29/2013 02:14 PM, Andrew Dunstan wrote:
> > AIUI: They do test feature use and errors that have cropped up in the
> > past that we need to beware of. They don't test every bug we've ever
> > had, nor do they exercise every piece of code.
> 
> If we don't have a test for it, then we can break it in the future and
> not know we've broken it until .0 is released.  Is that really a
> direction we're happy going in?
> 
> > Maybe there is a good case for these last two in a different set of tests.
> 
> If we had a different set of tests, that would be a valid argument.  But
> we don't, so it's not.  And nobody has offered to write a feature to
> split our tests either.

With utmost respect, this just isn't true.  There is a "make coverage"
target that probably doesn't get enough exercise, but it's just the
kind of infrastructure you're describing.

> I have to say, I'm really surprised at the level of resistance
> people on this list are showing to the idea of increasing test
> coverage. I thought that Postgres was all about reliability?   For a
> project as mature as we are, our test coverage is abysmal, and I
> think I'm starting to see why.

Burdening hackers with extra time in ordinary compile cycles is the
wrong direction.  If anything, we should probably look at what tests
only routinely get run by our CI system--currently the buildfarm--and
which ones developers could reasonably be expected to wait until
post-push to run in day-to-day development.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] "pg_ctl promote" exit status

[Peter Eisentraut]

On 6/28/13 10:50 PM, Bruce Momjian wrote:
> On Mon, Jan 28, 2013 at 09:46:32AM -0500, Peter Eisentraut wrote:
>> On 1/26/13 4:44 PM, Aaron W. Swenson wrote:
>>> You are right. Had I read a little further down, it seems that the
>>> exit status should actually be 7.
>>
>> 7 is OK for "not running", but what should we use when the server is not
>> in standby mode?  Using the idempotent argument that we are discussing
>> for the stop action, promoting a server that is not a standby should be
>> a noop and exit successfully.  Not sure if that is what we want, though.
> 
> I looked at all the LSB return codes listed here and mapped them to
> pg_ctl error situations:
> 
>   
> https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
> 
> Patch attached.  I did not touch the start/stop return codes.

Approximately none of these changes seem correct to me.  For example,
why is failing to open the PID file 6, or failing to start the server 7?




-- 
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] Documentation/help for materialized and recursive views

[Peter Eisentraut]

On 6/28/13 2:27 PM, David Fetter wrote:
>> You can run \! man from within psql,
> And if you're on Windows, you're Sadly Out of Luck with that.  Is
> there an equivalent we could #ifdef in for that platform?

If you are using psql on Windows extensively, you probably have one of
mingw, cygwin, or pgadmin handy, all of which can get you to the
documentation.  I don't think it's worth devising a mechanism for those
not covered by this.



-- 
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] MVCC catalog access

[Andres Freund]

On 2013-06-28 23:14:23 -0400, Robert Haas wrote:
> Here's a further update of this patch.  In this version, I added some
> mechanism to send a new kind of sinval message that is sent when a
> catalog without catcaches is updated; it doesn't apply to all
> catalogs, just to whichever ones we want to have this treatment.  That
> means we don't need to retake snapshots for those catalogs on every
> access, so backend startup requires just one extra MVCC snapshot as
> compared with current master.  Assorted cleanup has been done, along
> with the removal of a few more SnapshotNow references.

This is really cool stuff.

> It's still possible to construct test cases that perform badly by
> pounding the server with 1000 clients running Andres's
> readonly-busy.sql.  Consider the following test case: use a DO block
> to create a schema with 10,000 functions in it and then DROP ..
> CASCADE.  When the server is unloaded, the extra MVCC overhead is
> pretty small.

> Well, now the create is 52% slower and the drop is a whopping 4.7x
> slower.  It's worth digging into the reasons just a bit.  I was able
> to speed up this case quite a bit - it was 30x slower a few hours ago
> - by adding a few new relations to the switch in
> RelationInvalidatesSnapshotsOnly().  But the code still takes one MVCC
> snapshot per object dropped, because deleteOneObject() calls
> CommandCounterIncrement() and that, as it must, invalidates our
> previous snapshot.

I have to say, if the thing that primarily suffers is pretty extreme DDL
in extreme situations I am not really worried. Anybody running anything
close to the territory of such concurrency won't perform that much DDL.

> We could, if we were inclined to spend the effort,
> probably work out that although we need to change curcid, the rest of
> the snapshot is still OK, but I'm not too convinced that it's worth
> adding an even-more-complicated mechanism for this.  We could probably
> also optimize the delete code to increment the command counter fewer
> times, but I'm not convinced that's worth doing either.

I am pretty convinced we shouldn't do either for now.

Something picked up when quickly scanning over the last version of the
patch:

> +/*
> + * Staleness detection for CatalogSnapshot.
> + */
> +static bool CatalogSnapshotStale = true;
>  
>  /*
>   * These are updated by GetSnapshotData.  We initialize them this way
> @@ -177,6 +188,9 @@ GetTransactionSnapshot(void)
>   else
>   CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData);
>  
> + /* Don't allow catalog snapshot to be older than xact snapshot. 
> */
> + CatalogSnapshotStale = true;
> +
>   FirstSnapshotSet = true;
>   return CurrentSnapshot;
>   }
> @@ -184,6 +198,9 @@ GetTransactionSnapshot(void)
>   if (IsolationUsesXactSnapshot())
>   return CurrentSnapshot;
>  
> + /* Don't allow catalog snapshot to be older than xact snapshot. */
> + CatalogSnapshotStale = true;
> +
>   CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData);
>  
>   return CurrentSnapshot;
> @@ -207,6 +224,54 @@ GetLatestSnapshot(void)
>  }

Do we really need to invalidate snapshots in either situation? Isn't it
implied, that if it's still valid, according to a) no invalidation via local
invalidation messages b) no invalidations from other backends, there
shouldn't be any possible differences when you only look at the catalog?

And if it needs to change, we could copy the newly generated snapshot
to the catalog snapshot if it's currently valid.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] XLogInsert scaling, revisited

[Andres Freund]

On 2013-06-26 18:52:30 +0300, Heikki Linnakangas wrote:
> >* Could you document the way slots prevent checkpoints from occurring
> >   when XLogInsert rechecks for full page writes? I think it's correct -
> >   but not very obvious on a glance.
>
> There's this in the comment near the top of the file:
>
>  * To update RedoRecPtr or fullPageWrites, one has to make sure that all
>  * subsequent inserters see the new value. This is done by reserving all the
>  * insertion slots before changing the value. XLogInsert reads RedoRecPtr
> and
>  * fullPageWrites after grabbing a slot, so by holding all the slots
>  * simultaneously, you can ensure that all subsequent inserts see the new
>  * value.  Those fields change very seldom, so we prefer to be fast and
>  * non-contended when they need to be read, and slow when they're changed.
>
> Does that explain it well enough? XLogInsert holds onto a slot while it
> rechecks for full page writes.

I am a bit worried about that logic. We're basically reverting to the
old logic whe xlog writing is an exlusive affair. We will have to wait
for all the other queued inserters before we're finished. I am afraid
that that will show up latencywise.

I have two ideas to improve on that:
a) Queue the backend that does WALInsertSlotAcquire(true) at the front
of the exclusive waiters in *AcquireOne. That should be fairly easy.
b) Get rid of WALInsertSlotAcquire(true) by not relying on
blocking all slot acquiration. I think with some trickery we can do that
safely:
In CreateCheckpoint() we first acquire the insertpos_lck and read
CurrBytePos as a recptr. Set some shared memory variable, say,
PseudoRedoRecPtr, that's now used to check whether backup blocks need to
be made. Release insertpos_lck. Then acquire each slot once, but without
holding the other slots. That guarantees that all XLogInsert()ing
backends henceforth see our PseudoRedoRecPtr value. Then just proceed in
CreateCheckpoint() as we're currently doing except computing RedoRecPtr
under a spinlock.
If a backend reads PseudoRedoRecPtr before we've set RedoRecPtr
accordingly, all that happens is that we possibly have written a FPI too
early.

Makes sense?

Greetings,

Andres Freund

--
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Review: Display number of changed rows since last analyze

[Albe Laurenz]

Magnus Hagander wrote:
>>> On Mon, Jun 17, 2013 at 1:49 PM, Albe Laurenz  
>>> wrote:
 I think that the column name is ok as it is, even if it
 is a bit long - I cannot come up with a more succinct
 idea.  Perhaps "n_changed_since_analyze" could be shortened
 to "n_mod_since_analyze", but that's not much of an improvement.
>>>
>>> AFAICT it's related to "n_live_tup", and "n_dead_tup". How about just
>>> "n_mod_tup"? Though that doesn't convey that it's since the last
>>> analyze, I guess.
>>>
>>> But given that both n_dead_tup and n_live_tup don't really indicate
>>> that they're not "since the beginning of stats" in the name (which
>>> other stats counters are), I'm not sure that's a problem? It would be
>>> a name that sounds more similar to the rest of the table.
>>
>> I don't get that.
>>
>> As far as I know, n_dead_tup and n_live_tup are estimates for
>> the total number of live and dead tuples, period.
>>
>> Both numbers are not reset to zero when ANALYZE (or even VACUUM)
>> takes place.
>
> No, but they are zero *until* vacuum runs.
> 
> The point I was trying to make was that they are showing an absolute
> number. Unlike for example n_tup_inserted and friends which show the
> total number of  since stat reset.

Ok, I understand you now.

All the old names are fairly intuitive in my opinion.

"Number of life tuples since the statistics were reset" doesn't make
a lot of sense to me, so I would automatically read that as an absolute
number.

But it would not be clear to me that "n_mod_tuples" are counted
since the last ANALYZE (different from other columns); I would
jump to the conclusion that it is a sum of n_tup_ins, n_tup_upd
and n_tup_del.

So I think that a name that it less likely to cause confusion
would be better that a short, but misleading name.

Yours,
Laurenz Albe

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] New regression test time

[Amit kapila]

On Monday, July 01, 2013 8:37 AM Josh Berkus wrote:
On 06/30/2013 12:33 AM, Amit kapila wrote:
>
> On Sunday, June 30, 2013 11:37 AM Fabien COELHO wrote:
 If we had a different set of tests, that would be a valid argument.  But
 we don't, so it's not.  And nobody has offered to write a feature to
 split our tests either.
>
>>> I have done a POC. See:
>
>>> https://commitfest.postgresql.org/action/patch_view?id=1170
>
>> I think it is better to submit for next commit fest which is at below link:
>>
>> https://commitfest.postgresql.org/action/commitfest_view?id=19

> I would argue for doing this in this CF, just so that we can have the
> benefit of the extra tests for the next 3 months, and so that Andrew can
> work on the buildfarm additions.

My mail was just a suggestion, as in general after start of CF all new patch 
submissions for review 
are done in next CF. 
However as for this particular patch, there are quite a few other dependent 
patches, so it will be good
if this patch gets committed in this CF only.
Also as a CFM, you are better person to decide about it.


With Regards,
Amit Kapila.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers