Re: [HACKERS] Support for REINDEX CONCURRENTLY

[Michael Paquier]

Hi,

I noticed some errors in the comments of the patch committed. Please
find attached a patch to correct that.
Regards,
--
Michael


20130704_reltoastidxid_comments.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] Review: Patch to compute Max LSN of Data Pages

[Amit Kapila]

On Wednesday, July 03, 2013 7:41 PM Robert Haas wrote:
> On Wed, Jul 3, 2013 at 9:51 AM, Amit Kapila 
> wrote:
> > amit@linux:~> md test
> > amit@linux:~> cd test
> > amit@linux:~/test> ln -s ~/test link_test
> > amit@linux:~/test> ls
> > link_test
> > amit@linux:~/test> cd link_test
> > amit@linux:~/test/link_test> ls
> > link_test
> > amit@linux:~/test/link_test> cd link_test
> > amit@linux:~/test/link_test/link_test> cd link_test
> > amit@linux:~/test/link_test/link_test/link_test> pwd
> > /home/amit/test/link_test/link_test/link_test
> > amit@linux:~/test/link_test/link_test/link_test>
> 
> So what?

It can cause error "too many levels of symbolic links"

Point was that in case of symlinks we only want to allow PG_ paths, so that
such situation can never occur.
However I think this is not important to handle by this utility, so we can
remove such handling.

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

[HACKERS] Mention in bgworker docs that db connection needs shmem access

[Michael Paquier]

Hi all,

The bgworker documentation does not explicitely mention that a
bgworker using BGWORKER_BACKEND_DATABASE_CONNECTION needs to have
BGWORKER_SHMEM_ACCESS set as well.

Just to mention, a bgworker using only db connection flag and not
shmem flag will fail at atart-up with this error.
background worker "hello signal worker": must attach to shared memory
in order to request a database connection

Please find attached a patch that improves documentation a bit by
mentioning this restriction. This should normally be backpatched to
9.3 stable.

Thanks,
--
Michael


20130704_bgworker_doc.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] Fwd: [Still Failing] pg-quilter/postgres#111 (master - 82b0102)

[Maciek Sakrejda]

I apologize. I guess the Travis CI integration is a little better than I
expected. I'm traveling but will turn off notifications as soon as I have a
chance to.

Per Peter's comment there is more info in the GitHub project, and I welcome
any feedback. I'll follow up with more once this is a little more mature.

Re: [HACKERS] request a new feature in fuzzystrmatch

[Alvaro Herrera]

Josh Berkus wrote:
> On 07/03/2013 04:05 PM, Alvaro Herrera wrote:
> > Josh Berkus escribió:
> >> Liming,
> >>
> >> Given that this patch will not be ready for commit this week, but has
> >> gotten a review, I am marking it as "Returned with Feedback".   Please
> >> keep working on it, and as soon as you have a new version of the patch,
> >> add it to the September commitfest.  Thanks!
> > 
> > I don't think this patch has gotten any useful review actually.
> 
> Oh?  I thought it was at the state where Liming had to do a whole bunch
> of changes before it could be reviewed further?

No, the comments have been solely on the details of the extension
definition files and the like.  There has been not a single comment
about the new algorithm being proposed, which is the interesting stuff
in this patch.  Review can certainly proceed without regard for the sql
files at all.

-- 
Á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] [9.4 CF 1] The Commitfest Slacker List

[Greg Sabino Mullane]

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


Josh Berkus replied:
>> I won't go into details here because frankly why I have no time 
>> for reviewing a patch is none of your business. 
>
> Then just send an email saying "Sorry, I don't have any time for patch
> review this time.  Maybe next time".   It's pretty simple.

Hope about you not publically shame people in a volunteer project? 
That's pretty simple.

> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

I must have missed the page where patch review is defined as part of 
a committer's job.

> Possibly "slacker" was a poor choice of word given translations; in
> colloquial American English it's a casual term, even affectionate under
> some conditions.  I'll make sure to use different words if I ever end up
> doing a list again.

Please, don't ever do a list again. And yes, "slacker" was an extremely 
poor choice of word. This American English speaker certainly has a 
hard time viewing it as "affectionate". I think the whole thread would 
have been better received with a subject line of "Commitfest needs help".

- -- 
Greg Sabino Mullane g...@turnstep.com
PGP Key: 0x14964AC8 201307032150
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAlHU1QQACgkQvJuQZxSWSsgoMgCfcUm/MnYzsUaqVWq3DvTh2kAi
sYwAoLAijh3SkCbv2c7visToyqPAOWMG
=xoKV
-END PGP SIGNATURE-




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

Re: [HACKERS] event trigger API documentation?

[Peter Eisentraut]

On Mon, 2013-05-06 at 17:17 +0200, Dimitri Fontaine wrote:
> Please find attached a patch against the documentation, containing a
> full code example of what I had in mind. 

Applied with some tweaks.




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

Re: [HACKERS] Fwd: [Still Failing] pg-quilter/postgres#111 (master - 82b0102)

[Peter Geoghegan]

On Wed, Jul 3, 2013 at 5:36 PM, Andrew Dunstan  wrote:
> Does anyone know why I received the message below, or how to turn it off?
>
> Sending out notifications such as this (bogus in this case anyway) is
> unfriendly.

This was an oversight. I'm sorry you were affected in this way.

My colleague Maciek Sakrejda is working on a tool that monitors
pgsql-hackers and ultimately structures patches as pseudo pull
requests on a mirror of Postgres on Github. This includes doing things
like running the regression tests via travis CI.

Since the cat is now out of the bag before we managed to iron out some
outstanding issues, you might as well look at the github page:

https://github.com/deafbybeheading/pg-quilter

-- 
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

[HACKERS] Fwd: [Still Failing] pg-quilter/postgres#111 (master - 82b0102)

[Andrew Dunstan]

Does anyone know why I received the message below, or how to turn it off?

Sending out notifications such as this (bogus in this case anyway) is 
unfriendly.


cheers

andrew



 Original Message 
Subject:[Still Failing] pg-quilter/postgres#111 (master - 82b0102)
Date:   Wed, 03 Jul 2013 23:24:41 +
From:   Travis CI 
To: 



 The build is still failing.

Repository  pg-quilter/postgres
Build #111 	https://travis-ci.org/pg-quilter/postgres/builds/8717058 
 

Changeset 
https://github.com/pg-quilter/postgres/compare/2ef085d0e696...82b0102650cf 
 


Commit  82b0102 (master)
Message Install all a Makefile's extension controls, not just the first.

Bug introduced by commit 6697aa2bc25c83b88d6165340348a31328c35de6 and
reported by Robert Haas.
Author  Andrew Dunstan
Duration28 seconds

	You can configure recipients for build notifications in your 
configuration file 
. 
Further documentation about Travis CI can be found here 
. 
For help please join our IRC channel irc.freenode.net#travis.


Want Travis CI for your private repositories? 
 

Powered by 
 






--
Sent 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

[Michael Paquier]

On Thu, Jul 4, 2013 at 8:05 AM, Alvaro Herrera  wrote:
> Josh Berkus escribió:
>> Liming,
>>
>> Given that this patch will not be ready for commit this week, but has
>> gotten a review, I am marking it as "Returned with Feedback".   Please
>> keep working on it, and as soon as you have a new version of the patch,
>> add it to the September commitfest.  Thanks!
>
> I don't think this patch has gotten any useful review actually.
Exactly. I didn't check the content of the patch, just looked quickly
at the changes done for the extension upgrade.
--
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] [PATCH] Add an ldapoption to disable chasing LDAP referrals

[James Sewell]

Heya,

I see what you are saying, the problem as I see it is that the action we
are taking here is "disable chasing ldap referrals". If the name is
ldapreferrals and we use a boolean then setting it to 1  reads in a counter
intuitive manner:

  "set ldapreferals=true to disable chasing LDAP referrals."

Perhaps you are fine with this though if it's documented? It does work in
the inverse way to pam_ldap, where setting to true enables referral
chasing. pam_ldap works like so:

  not set  : library default
  set to 0 : disable referral chasing
  set to 1 : enable referral chasing

The other option would be to have the default value (of the parameter) be
true and set the boolean to false to disable it. I can't find any other
examples of this though - I assume having a one off like this in the code
is a bad thing also?

I'm happy to let you guys decide.

Cheers,
James

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



On Wed, Jul 3, 2013 at 6:12 PM, Magnus Hagander  wrote:

>
> On Wed, Jul 3, 2013 at 3:04 AM, James Sewell wrote:
>
>> Hey Peter,
>>
>> You are correct, it is the same  as the referrals option in pam_ldap.
>> It's also the -C (sometimes -R - it seems ldapsearch options are pretty
>> non-standard) option in ldapsearch.
>>
>> As far as I'm aware you can't pass this in an LDAP URL, primarily because
>> this never gets sent to the LDAP server. The server always returns an LDIF
>> with inline references, this just determines if you chase them client side
>> or just list them as is.
>>
>> I could be missing something here, but using:
>>
>>  ldapreferrals={0|1}
>>
>> Would require a three state type, as we need a way of not interfering
>> with the library defaults? To 'enable' the new behavior here using a
>> boolean you would need to set ldapreferrals=false - which with the normal
>> way of dealing with config booleans would alter the default behavior if the
>> option was not specified.
>>
>> How do you feel about:
>>
>>   ldapdisablereferrals=(0|1)
>>
>>
> I agree with Peter that the negative thing is bad. l don't see the
> problem, really. If you don't specify it, you rely on library defaults. If
> you do specify it, we lock it to that setting. I don't see the need to
> specifically have a setting to rely on library defaults - just remove it
> from the line and you get that.
>
> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.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.
<>

Re: [HACKERS] dynamic background workers

[Michael Paquier]

On Wed, Jul 3, 2013 at 11:19 PM, Robert Haas  wrote:
> On Mon, Jun 24, 2013 at 3:51 AM, Michael Paquier
>  wrote:
>> 3) Why not adding an other function in worker_spi.c being the opposite
>> of worker_spi_launch to stop dynamic bgworkers for a given index
>> number? This would be only a wrapper of pg_terminate_backend, OK, but
>> at least it would give the user all the information needed to start
>> and to stop a dynamic bgworker with a single extension, here
>> worker_spi.c. It can be painful to stop
>
> Well, there's currently no mechanism for the person who starts a new
> backend to know the PID of the process that actually got started.  I
> plan to write a patch to address that problem, but it's not this
> patch.
OK. Understood, this functionality would be a good addition to have.

>> 4) Not completely related to this patch, but one sanity check in
>> SanityCheckBackgroundWorker:bgworker.c is not listed in the
>> documentation: when requesting a database connection, bgworker needs
>> to have access to shmem. It looks that this should be also fixed in
>> REL9_3_STABLE.
>
> That's fine; I think it's separate from this patch.  Please feel free
> to propose something.
I'll send a patch about that.

>> 6) Just wondering something: it looks that the current code is not
>> able to identify what was the way used to start a given bgworker.
>> Would it be interesting to be able to identify if a bgworker has been
>> registered though RegisterBackgroundWorker or
>> RegisterDynamicBackgroundWorker?
>
> I don't think that's a good thing to expose.
My concerns here are covered by the functionality you propose in 1),
where a user who launched a custom bgworker would know its PID, this
would allow users to keep track of which bgworker has been started
dynamically.

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] [9.4 CF 1] The Commitfest Slacker List

[Wolfe Whalen]

First of all, I'd like to give a big Thank You to all the hackers and
slackers that make Postgres great.  You've really done an amazing job.

I'll step up and take a healthy portion of the blame here.  I enjoy the
awesome features & fixes that all of you put out year after year, but I
have yet to contribute anything.  For what it's worth, I'm sorry.  If
more guys like me could find some time to step up our game a little, we
wouldn't even be having this conversation.  But we're still left with
the fact that there is too much code and not enough review.

Honestly, there is a lot of work for committers to do even when all the
patches have been through sufficient code review.  Slogging through 4
months of CF patches without adequate review is enough to make anybody
throw up their hands and quit.

The best thing we can do to improve Postgres right now and over the long
term is to make sure that doesn't happen.  Anybody who is on this list
is a valued Postgres contributor, and that's why Josh has been reaching
out.  Maybe the term slacker offended some people, but he's basically
saying that code review is an essential contribution, perhaps more
essential than most new patches themselves at this point.  He's asking
for help from those of you with the skill, experience, and established
desire to contribute to the growth of Postgres.

Coordinating volunteers for anything is a frustrating process.  I'm sure
some people on the list shouldn't be on the list, and some people who
should be on the list aren't.  Maybe the list shouldn't have been sent
in the first place.  But it's a call for help born out of frustration,
and one that we could remedy by stepping up our communication a bit.  5
days is too short?  How abut 7 or 10?  No time to review a patch this
CF?  Okay, good to know.

He's trying to tell us how we can best contribute.  If I manage to find
the time to contribute something over the next couple of weeks, it would
border on the absurd if my contribution was some sort of patch
submission.  Pretty much nothing I could write would be a more valuable
contribution than code review at this point.

So I'll drop Josh an email and let him know how much time I might be
able to contribute and when, and I'd suggest other people do the same. 
Even if it's just a quick "Hey, I'm pretty busy the next couple weeks so
I'm not going to be able to review anything this CF."  Being armed with
a little more information about the potential volunteer pool would
probably make his job as CFM much easier.

Thanks again to all of you for all your hard work.

Best regards,

Wolfe Whalen

-- 
  Wolfe Whalen
  wo...@quios.net


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

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Bruce Momjian]

On Wed, Jul  3, 2013 at 03:34:06PM -0700, Josh Berkus wrote:
> On 07/03/2013 03:08 PM, Robert Haas wrote:
> > You are way out of line.  You have no right to expect ANYONE to
> > participate in patch review and commit.  Michael is doing us a favor
> > by maintaining ECPG even though he's not heavily involved in the
> > project any more and has other things to do with his time.
> 
> That's a good point.  I hadn't considered (or realized the extent of)
> the occasional and specific nature of Michael's involvement with the
> project these days.  My apologies, then, Michael.
> 
> Is there anyone else on the committer list with similar circumstances?

I spend my available time going through old emails and finding issues
that never made it to a commit-fest, but need doing.  I am currently in
November, 2012.

I am volunteering that information, and do not in any way feel I have to
justify my time commitment to anyone, except perhaps my employer.   If
you want, you can remove my commit bit and I will just post all my
patches for others to commit --- hard to see how that is an improvement.
I will also remind you that before there were commit-fests, Tom and I
pretty much did all that work of committing non-committer's patches.

But my big feedback is, our community has no right to be asking about
committer circumstances.  This is a voluntteer project, and people work
as they want.  The extrapolation of Josh's approach is that committers
have to do work that the community wants to maintain their commit
rights, but their commit rights are helping the community, so why would
people care if you take them away --- you only hurt the community
further by doing so.

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

[Josh Berkus]

On 07/03/2013 04:05 PM, Alvaro Herrera wrote:
> Josh Berkus escribió:
>> Liming,
>>
>> Given that this patch will not be ready for commit this week, but has
>> gotten a review, I am marking it as "Returned with Feedback".   Please
>> keep working on it, and as soon as you have a new version of the patch,
>> add it to the September commitfest.  Thanks!
> 
> I don't think this patch has gotten any useful review actually.

Oh?  I thought it was at the state where Liming had to do a whole bunch
of changes before it could be reviewed further?

-- 
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: [HACKERS] [PATCH] Add transforms feature

[Josh Berkus]

Peter,

I've been playing with the new patch, and haven't been able to reproduce
the load problem created by the original patch.  So that seems fixed.

I'm not comfortable with having all of the transform mappings in the
main contrib/ directory though.  Can we add a subdirectory called
"transforms" containing all of these?

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

[Alvaro Herrera]

Josh Berkus escribió:
> Liming,
> 
> Given that this patch will not be ready for commit this week, but has
> gotten a review, I am marking it as "Returned with Feedback".   Please
> keep working on it, and as soon as you have a new version of the patch,
> add it to the September commitfest.  Thanks!

I don't think this patch has gotten any useful review actually.

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

[Josh Berkus]

Liming,

Given that this patch will not be ready for commit this week, but has
gotten a review, I am marking it as "Returned with Feedback".   Please
keep working on it, and as soon as you have a new version of the patch,
add it to the September commitfest.  Thanks!

-- 
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: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Robert Haas]

On Wed, Jul 3, 2013 at 6:34 PM, Josh Berkus  wrote:
> On 07/03/2013 03:08 PM, Robert Haas wrote:
>> You are way out of line.  You have no right to expect ANYONE to
>> participate in patch review and commit.  Michael is doing us a favor
>> by maintaining ECPG even though he's not heavily involved in the
>> project any more and has other things to do with his time.
>
> That's a good point.  I hadn't considered (or realized the extent of)
> the occasional and specific nature of Michael's involvement with the
> project these days.  My apologies, then, Michael.
>
> Is there anyone else on the committer list with similar circumstances?

I would say that everyone on the committer list and every other list
has the right to choose the level of their involvement.  We can't
really complain about what people choose to do or not do except to the
extent that they impose burdens on other people.  For example, we
typically expect that if a committer commits a patch that breaks
something, that committer will promptly fix it.  People who are not
willing to do that should not commit (or be allowed to commit).  And
people who submit patches for review should also review patches: they
are asking other people to do work, so they should also contribute
work.

To the best of my knowledge, no one has ever been made a committer on
the condition that they spend a certain minimum number of hours per
week, month, or CommitFest on patch review, and I don't think we have
any right to expect that they do that.  Rather, we should be thanking
the people who do choose to do more than their share of patch review,
whether they are committers or not.  And the only people who need to
be called out are people who impose work on others without doing any
themselves.  People who contribute a small amount but ask nothing for
it are a good thing, not a bad thing, again, regardless of whether
they have a commit bit.

-- 
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] [9.4 CF 1] The Commitfest Slacker List

[Josh Berkus]

On 07/03/2013 03:08 PM, Robert Haas wrote:
> You are way out of line.  You have no right to expect ANYONE to
> participate in patch review and commit.  Michael is doing us a favor
> by maintaining ECPG even though he's not heavily involved in the
> project any more and has other things to do with his time.

That's a good point.  I hadn't considered (or realized the extent of)
the occasional and specific nature of Michael's involvement with the
project these days.  My apologies, then, Michael.

Is there anyone else on the committer list with similar circumstances?

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

[Alvaro Herrera]

Liming Hu escribió:

> I do not have a 9.3 environment. I did not change any previous existing code.

git checkout REL9_3_STABLE

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

[Liming Hu]

On Mon, Jul 1, 2013 at 8:09 AM, Alvaro Herrera  wrote:
> 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
Hi Alvaro,

I do not have a 9.3 environment. I did not change any previous existing code.
Can anybody help me on the upgrade?

Thanks,

Liming


--
Liming Hu
cell: (435)-512-4190
Seattle Washington


-- 
Sent 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

[Liming Hu]

On Sat, Jun 29, 2013 at 7:55 PM, Michael Paquier
 wrote:
> On Sun, Jun 30, 2013 at 5:37 AM, Joe Conway  wrote:
>> 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?
> When upgrading this extension here is what you should do:
> - Remove fuzzystrmatch--1.0.sql
> - Add fuzzystrmatch--1.1.sql with the new definitions
> - Add fuzzystrmatch--1.0--1.1.sql to be able to perform an upgrade
> between 1.0 and 1.1
> - Let fuzzystrmatch--unpackaged--1.0.sql as-is
>
Hi Michael,

Thanks. What should be included in  fuzzystrmatch--1.0--1.1.sql for the upgrade?
Liming

> Then by having a quick glance at the patch you sent...
> - fuzzystrmatch--unpackaged--1.0.sql is renamed internally to 1.1.sql
> - fuzzystrmatch--1.0.sql remains, and is renamed to 1.1 internally
> - fuzzystrmatch--1.0--1.1.sql is not provided
>
> Regards,
> --
> Michael



--
Liming Hu
cell: (435)-512-4190
Seattle Washington


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

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Robert Haas]

On Wed, Jul 3, 2013 at 5:16 PM, Josh Berkus  wrote:
> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

You are way out of line.  You have no right to expect ANYONE to
participate in patch review and commit.  Michael is doing us a favor
by maintaining ECPG even though he's not heavily involved in the
project any more and has other things to do with his time.  I think
you're about two emails away from him having him resign in disgust,
and if he does, then the burden of reviewing and committing ECPG
patches is going to fall on someone else.  Do you expect Tom or Noah
or Simon or myself to pick up the slack after you've driven him away?
I suppose you probably do, and that is absolutely wrong and really
pretty offensive.

I think it's completely appropriate for you to remind people who have
submitted patches for review but not reviewed any that they need to do
that part of it, too.  Fair is fair.  But you cannot enforce mandatory
volunteerism on people just because they are committers.  Maybe you
think the world would be a better place if committers who didn't pull
their weight had their commit bits pulled, or that it doesn't matter
if you drive them to resign in disgust.  I respectfully disagree.
Yeah, committers who are completely idle and never do anything
probably shouldn't have a commit bit.  But someone like Michael who
reliably maintains ECPG is an asset to the project whether he chooses
to do anything else or not.  I'm flabbergasted that you can't see
that.

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


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

Re: [HACKERS] Bugfix and new feature for PGXS

[Andrew Dunstan]

On 07/03/2013 05:52 PM, Robert Haas wrote:

On Mon, Jul 1, 2013 at 5:04 PM, Andrew Dunstan  wrote:

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.

Turns out you broke something.  Specifically, contrib/spi now only
installs autoinc.control instead of all the control files for
extensions in that directory.


...

I'm not sure whether this is as simple as changing $< to $^ in the
pgxs.mk's installcontrol rule, or if something more is required.
Could you take a look?



will do.

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] [PATCH] Remove useless USE_PGXS support in contrib

[Josh Berkus]

Peter, Cedric, etc.:

Where are we on this patch?  Seems like discussion died out.  Should it
be bounced?

-- 
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: [HACKERS] Re: [COMMITTERS] pgsql: Revert "Hopefully-portable regression tests for CREATE/ALTER/DRO

[Robert Haas]

On Wed, Jul 3, 2013 at 3:04 PM, Fabien COELHO  wrote:
>> The buildfarm is sad.
>
> Do you have a pointer about the issue?
>
> Is it that builds are performed in some particular locale?

Tom posted an example of the error message on the related
pgsql-hackers thread.  On some systems, no locales can be created,
period.

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

[Robert Haas]

On Mon, Jul 1, 2013 at 5:04 PM, Andrew Dunstan  wrote:
> 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.

Turns out you broke something.  Specifically, contrib/spi now only
installs autoinc.control instead of all the control files for
extensions in that directory.

Before:

/usr/bin/install -c -m 644 ./autoinc.control ./insert_username.control
./moddatetime.control ./refint.control ./timetravel.control
'/Users/rhaas/project/share/postgresql/extension/'
/usr/bin/install -c -m 644 ./autoinc--1.0.sql
./autoinc--unpackaged--1.0.sql ./insert_username--1.0.sql
./insert_username--unpackaged--1.0.sql ./moddatetime--1.0.sql
./moddatetime--unpackaged--1.0.sql ./refint--1.0.sql
./refint--unpackaged--1.0.sql ./timetravel--1.0.sql
./timetravel--unpackaged--1.0.sql
'/Users/rhaas/project/share/postgresql/extension/'
/usr/bin/install -c -m 755  autoinc.so insert_username.so
moddatetime.so refint.so timetravel.so
'/Users/rhaas/project/lib/postgresql/'
/usr/bin/install -c -m 644 ./autoinc.example ./insert_username.example
./moddatetime.example ./refint.example ./timetravel.example
'/Users/rhaas/project/share/doc//postgresql/extension/'

Now:

/usr/bin/install -c -m 644 autoinc.control
'/Users/rhaas/project/share/postgresql/extension/'
/usr/bin/install -c -m 644 autoinc--1.0.sql
autoinc--unpackaged--1.0.sql insert_username--1.0.sql
insert_username--unpackaged--1.0.sql moddatetime--1.0.sql
moddatetime--unpackaged--1.0.sql refint--1.0.sql
refint--unpackaged--1.0.sql timetravel--1.0.sql
timetravel--unpackaged--1.0.sql
'/Users/rhaas/project/share/postgresql/extension/'
/usr/bin/install -c -m 644 autoinc.example insert_username.example
moddatetime.example refint.example timetravel.example
'/Users/rhaas/project/share/doc//postgresql/extension/'
/usr/bin/install -c -m 755  autoinc.so insert_username.so
moddatetime.so refint.so timetravel.so
'/Users/rhaas/project/lib/postgresql/'

I'm not sure whether this is as simple as changing $< to $^ in the
pgxs.mk's installcontrol rule, or if something more is required.
Could you take a look?

Thanks,

-- 
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] [9.4 CF 1] The Commitfest Slacker List

[Andres Freund]

On 2013-07-03 14:16:09 -0700, Josh Berkus wrote:
> On 07/03/2013 02:03 PM, Michael Meskes wrote:
> > I won't go into details here because frankly why I have no time for 
> > reviewing a
> > patch is none of your business. 
> 
> Then just send an email saying "Sorry, I don't have any time for patch
> review this time.  Maybe next time".   It's pretty simple.
> 
> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

I find it absurd to expect anybody - including committers and regular
contributors - to be involved in the project all the time. It's one
thing to call somebody out who regularly commits his/her own stuff but
doesn't do CF work, something entirely different to do it to people who
didn't have time (or even chose to invest it differently!) to contribute
lately. The project does *NOT* own them.

I'd be at least as pissed as Michael seems to be if I were in his shoes.

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

[Josh Berkus]

On 06/29/2013 01:37 PM, Joe Conway wrote:
> On 06/25/2013 01:37 PM, Liming Hu wrote:
>>> please remove "dameraulevenshteinnocompatible" related stuff, I
>>> just followed the template you created. 
>>> "dameraulevenshteinnocompatible" was used in my first testing.
> 
>>> diff -cNr
>>> /opt/src/pgsql-git/master/contrib/fuzzystrmatch/fuzzystrmatch--1.0.sql
>>>
>>>
> diff -cNr
> /opt/src/pgsql-git/master/contrib/fuzzystrmatch/fuzzystrmatch--unpackaged--1.0.sql
> 
> 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?

I believe that's correct.


-- 
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: [HACKERS] [v9.4] row level security -- needs a reviewer

[Josh Berkus]

Hackers,

Please, oh please, won't someone review this?  This patch has been 3
years in the making, so I suspect that it will NOT be a fast review.

-- 
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: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Josh Berkus]

On 07/03/2013 02:03 PM, Michael Meskes wrote:
> I won't go into details here because frankly why I have no time for reviewing 
> a
> patch is none of your business. 

Then just send an email saying "Sorry, I don't have any time for patch
review this time.  Maybe next time".   It's pretty simple.

I'm not going to apologize for expecting *committers* to participate in
patch review and commit.

> As I said I didn't even notice this email in the first hand until I was
> approached from other people and called a slacker in communication not
related
> to the CF at all.

Ah, now, *that* wasn't my intent, sorry about that.  It's rather a
surprise to me that anyone off of the -hackers list would care.

Possibly "slacker" was a poor choice of word given translations; in
colloquial American English it's a casual term, even affectionate under
some conditions.  I'll make sure to use different words if I ever end up
doing a list again.

-- 
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: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Michael Meskes]

On Wed, Jul 03, 2013 at 12:34:50PM -0700, Josh Berkus wrote:
> If you didn't feel obligated, you wouldn't be pissed at me.  You'd just
> blow it off (like Bruce did).  I think you're angry with me because you
> feel guilty.

That is outrageous bullshit!

> My *personal* viewpoint is that all committers should feel obligated to

And my *personal* viewpoint is that nobody should be offended like this. But
apparently I don't get my wish either.

> review and commit patches from other contributors.  That's why they're
> committers in the first place.  Certainly if a committer looks at the CF
> application and notices that 80% of the reviewing and committing is
> being done by three people, none of whom have any more "spare time" than
> they do, they should feel obligated to help those people out.

How many patches did you review? You don't have to be a committer to do that.

> We have a problem with patch reviewing and committing in this project;
> it's not being done in a timely fashion in general (every CF last year
> ended late), and the people who are doing most of the work feel
> overworked and frustrated.  This problem is getting worse every year,
> and will kill the project if it continues on its current trajectory.

As if publicly blaming people for not behaving the way you would like them to
will do the project a lot of good.

Let me stress again that you didn't even try talking to the people in question
in private before, nor did you bother putting your *suggestion* up for
discussion before flaming people.

Also let me stress again that I did *not* put a patch into the CF.

> 3) getting most of our existing contributors to shoulder their fair
> share of patch review.
> 
> (3) is what I'm addressing on this thread.  The reason I volunteered to
> be CFM this time was directly because of our discussion in Ottawa of how
> the review process wasn't working.  I decided to find out *why* it
> wasn't working, and the first obvious thing I ran across was that most
> of our current and our long-term contributors weren't doing any patch
> review.  For CF1, the number of people submitting patches outnumbered
> those who had volunteered for review 2 to 1.  That *is* the review
> problem in a nutshell; everybody wants someone else to do the work.

Great, I wasn't part of any discussion as I didn't make it to Ottawa this time.
Neither am I part of the problem with 0 patches, but still I've got to shoulder
the blame in a less than friendly way.

> I don't think it's too much to ask people who are listed on the project
> developers page as major contributors to review one patch per CommitFest
> most of the time.  If they did just *one* it would substantially
> decrease the workload on the people who are currently doing the vast
> majority of review and commit.

You didn't ask! You blamed and offended people! 

I won't go into details here because frankly why I have no time for reviewing a
patch is none of your business. 

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL


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

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Michael Meskes]

On Wed, Jul 03, 2013 at 04:03:08PM -0400, Bruce Momjian wrote:
> I do understand Josh's frustration that something different had to be
> done.

As a matter of fact I do, too. I just think the style of blaming people in
public like this is not ideal.

As I said I didn't even notice this email in the first hand until I was
approached from other people and called a slacker in communication not related
to the CF at all.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL


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

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Michael Meskes]

On Wed, Jul 03, 2013 at 09:47:13AM -0400, Robert Haas wrote:
> An attempt to prod a few more people into helping review.
> 
> I can see that this pissed you off, and I'm sorry about that.  But I
> don't think that was his intent.

I hoped for this kind of answer from him but ...

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL


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

Re: [HACKERS] refresh materialized view concurrently

[Tom Lane]

Kevin Grittner  writes:
> Further testing shows that any UPDATE or DELETE statement acquires
> a RowExclusiveLock on every index on the table and holds it until
> end of transaction, whether or not any rows are affected and
> regardless of whether an index scan or a seqscan is used.  In fact,
> just an EXPLAIN of an UPDATE or DELETE does so.  It is only INSERT
> which releases the locks at the end of the statement.

Hm, possibly the planner is taking those locks.  I don't think the
executor's behavior is any different.  But the planner only cares about
indexes in SELECT/UPDATE/DELETE, since an INSERT has no interest in
scanning the target table.

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] [9.4 CF 1] The Commitfest Slacker List

[Bruce Momjian]

On Wed, Jul  3, 2013 at 12:34:50PM -0700, Josh Berkus wrote:
> Michael Meskes wrote:
> >> So, as an experiment, call it a mixed result.  I would like to have some
> >> other way to motivate reviewers than public shame.  I'd like to have
> > 
> > Doesn't shame imply that people knew that were supposed to review patches in
> > the first place? An implication that is not true, at least for some on your
> > list. I think I better not bring up the word I would describe your email 
> > with,
> > just for the fear of mistranslating it.
> 
> If you didn't feel obligated, you wouldn't be pissed at me.  You'd just
> blow it off (like Bruce did).  I think you're angry with me because you
> feel guilty.

People are supposed to feel guilty because they are not volunteering
enough time, or enough time in the places the community wants?  How does
that make sense?  Doesn't that contradict the term "volunteer"?

Also, don't assume everyone has the thick skin I do.

I do understand Josh's frustration that something different had to be
done.

-- 
  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] refresh materialized view concurrently

[Kevin Grittner]

Tom Lane  wrote:
> Kevin Grittner  writes:

>> we acquire locks on all indexes even for a HOT UPDATE which uses
>> a seqscan, and hold those until end of transaction.  Is there a
>> reason for that?
>
> Sounds dubious to me; although in the HOT code it might be that
> there's no convenient place to release the index locks.

Further testing shows that any UPDATE or DELETE statement acquires
a RowExclusiveLock on every index on the table and holds it until
end of transaction, whether or not any rows are affected and
regardless of whether an index scan or a seqscan is used.  In fact,
just an EXPLAIN of an UPDATE or DELETE does so.  It is only INSERT
which releases the locks at the end of the statement.

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

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

[Andres Freund]

On 2013-07-03 21:07:03 +0200, Fabien COELHO wrote:
> 
> >>Here is a v2 which is more likely to work under VPATH.
> 
> Here is a POC v4 which relies on multiple --schedule instead of creating
> concatenated schedule files.
> 
> -- 
> Fabien.

> diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
> index d5935b6..8a39f7d 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)/parallel_big_schedule 
> $(srcdir)/resultmap
>  
>  install-tests: all install install-lib installdirs-tests
>   $(MAKE) -C $(top_builddir)/contrib/spi install
> @@ -137,19 +137,43 @@ tablespace-setup:
>  ## Run tests
>  ##
>  
> +# installcheck vs check:
> +# - whether test is run against installed or compiled version
> +# test schedules: parallel, parallel_big, standby
> +# serial schedules can be derived from parallel schedules
> +
> +derived_schedules = serial_schedule serial_big_schedule
> +
> +serial_%: parallel_%
> + echo "# this file is generated automatically, do not edit!" > $@
> + egrep '^(test|ignore):' $< | \
> + while read op list ; do \
> +   for test in $$list ; do \
> + echo "$$op $$test" ; \
> +   done ; \
> + done >> $@
> +

This won't work on windows all that easily. Maybe we should instead add
a "--run-serially" parameter to pg_regress?

> -installcheck: all tablespace-setup
> - $(pg_regress_installcheck) $(REGRESS_OPTS) 
> --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
> +# after installation, serial
> +installcheck: all tablespace-setup serial_schedule
> + $(pg_regress_installcheck) $(REGRESS_OPTS) \
> +   --schedule=serial_schedule $(EXTRA_TESTS)

Why do we use the serial schedule for installcheck anyway? Just because
of max_connections?

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] [9.4 CF 1] The Commitfest Slacker List

[Cédric Villemain]

Le mercredi 3 juillet 2013 21:03:42, Christopher Browne a écrit :
> On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain 
wrote:
> > > Clearly I ticked off a bunch of people by publishing "the list".  On
> > > the other hand, in the 5 days succeeding the post, more than a dozen
> > > additional people signed up to review patches, and we got some of the
> > > "ready for committer" patches cleared out -- something which nothing
> > > else I did, including dozens of private emails, general pleas to this
> > > mailing list, mails to the RRReviewers list, served to accomplish, in
> > > this or previous CFs.
> > 
> > Others rules appeared, like the 5 days limit.
> > To me it outlines that some are abusing the CF app and pushing there
> > useless
> > patches (not still ready or complete, WIP, ...
> 
> Seems to me that "useless" overstates things, but it does seem fair to
> say that some patches are not sufficiently well prepared to be efficiently
> added into Postgres.

Oops! You just made me realized my choice of words was not good at all!
I didn't considered under this angle, I just meant that some patches were 
added to CF to help patches authors, it was a good idea, but this had some 
unexpected corner case. Sorry for the confusion.
-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation


signature.asc
Description: This is a digitally signed message part.

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Josh Berkus]

Michael Meskes wrote:
>> So, as an experiment, call it a mixed result.  I would like to have some
>> other way to motivate reviewers than public shame.  I'd like to have
> 
> Doesn't shame imply that people knew that were supposed to review patches in
> the first place? An implication that is not true, at least for some on your
> list. I think I better not bring up the word I would describe your email with,
> just for the fear of mistranslating it.

If you didn't feel obligated, you wouldn't be pissed at me.  You'd just
blow it off (like Bruce did).  I think you're angry with me because you
feel guilty.

My *personal* viewpoint is that all committers should feel obligated to
review and commit patches from other contributors.  That's why they're
committers in the first place.  Certainly if a committer looks at the CF
application and notices that 80% of the reviewing and committing is
being done by three people, none of whom have any more "spare time" than
they do, they should feel obligated to help those people out.

We have a problem with patch reviewing and committing in this project;
it's not being done in a timely fashion in general (every CF last year
ended late), and the people who are doing most of the work feel
overworked and frustrated.  This problem is getting worse every year,
and will kill the project if it continues on its current trajectory.

There are *only* three ways out of this hole, all three of which I'm
trying to address:

1) more automation and better tools in order to reduce the total time
required of each reviewer/committer;

2) a program of recruitment of new reviewers, including giving respect
and recognition to people for their reviewing efforts

3) getting most of our existing contributors to shoulder their fair
share of patch review.

(3) is what I'm addressing on this thread.  The reason I volunteered to
be CFM this time was directly because of our discussion in Ottawa of how
the review process wasn't working.  I decided to find out *why* it
wasn't working, and the first obvious thing I ran across was that most
of our current and our long-term contributors weren't doing any patch
review.  For CF1, the number of people submitting patches outnumbered
those who had volunteered for review 2 to 1.  That *is* the review
problem in a nutshell; everybody wants someone else to do the work.

I don't think it's too much to ask people who are listed on the project
developers page as major contributors to review one patch per CommitFest
most of the time.  If they did just *one* it would substantially
decrease the workload on the people who are currently doing the vast
majority of review and commit.

On 07/03/2013 11:24 AM, Cédric Villemain wrote:
> You're looking at a short term, big effect.
> And long term ? Will people listed still be interested to participate
in a
> project which stamps people ?
>
> With or without review, it's a shame if people stop proposing patches
because
> they are not sure to get time to review other things *in time*.

Yes, I am, because the CF is only supposed to be 30 days long, and I
plan to finish it on time.  That's my job as CFM.

Several people on this thread have raised the fear of discouraging patch
submitters, but the consistent evidence is that we have more submissions
than we can currently handle.  I'd rather have half as many submissions,
but do a really good job of reviewing, improving, and integrating those
than the current mess.

Furthermore, there are quite a number of people who are submitting
patches on paid company time.  For those people, "submit one, review
one" has to be an ironclad rule so that they can tell their bosses that
they *have* to spend time on patch review.  Otherwise, the review
doesn't happen.

-- 
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: [HACKERS] refresh materialized view concurrently

[Tom Lane]

Kevin Grittner  writes:
> OK.  I had seen that no locks were held after the insert and wasn't
> aware that we acquired and then released them for each insert
> within a transaction.  On the other hand, we acquire locks on all
> indexes even for a HOT UPDATE which uses a seqscan, and hold those
> until end of transaction.  Is there a reason for that?

Sounds dubious to me; although in the HOT code it might be that there's
no convenient place to release the index locks.

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] Improvement of checkpoint IO scheduler for stable transaction responses

[Gavin Flower]

On 04/07/13 01:31, Robert Haas wrote:

On Wed, Jul 3, 2013 at 4:18 AM, KONDO Mitsumasa
 wrote:

I tested and changed segsize=0.25GB which is max partitioned table file size and
default setting is 1GB in configure option (./configure --with-segsize=0.25).
Because I thought that small segsize is good for fsync phase and background disk
write in OS in checkpoint. I got significant improvements in DBT-2 result!

This is interesting.  Unfortunately, it has a significant downside:
potentially, there will be a lot more files in the data directory.  As
it is, the number of files that exist there today has caused
performance problems for some of our customers.  I'm not sure off-hand
to what degree those problems have been related to overall inode
consumption vs. the number of files in the same directory.

If the problem is mainly with number of of files in the same
directory, we could consider revising our directory layout.  Instead
of:

base/${DBOID}/${RELFILENODE}_{FORK}

We could have:

base/${DBOID}/${FORK}/${RELFILENODE}

That would move all the vm and fsm forks to separate directories,
which would cut down the number of files in the main-fork directory
significantly.  That might be worth doing independently of the issue
you're raising here.  For large clusters, you'd even want one more
level to keep the directories from getting too big:

base/${DBOID}/${FORK}/${X}/${RELFILENODE}

...where ${X} is two hex digits, maybe just the low 16 bits of the
relfilenode number.  But this would be not as good for small clusters
where you'd end up with oodles of little-tiny directories, and I'm not
sure it'd be practical to smoothly fail over from one system to the
other.


16 bits ==> 4 hex digits

Could you perhaps start with 1 hex digit, and automagically increase it 
to 2, 3, .. as needed?  There could be a status file at that level, that 
would indicate the current number of hex digits, plus a temporary 
mapping file when in transition.


Cheers,
Gavin

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Josh Berkus]

Tatsuo,

> Because I did not register the patch into CF page myself. I should
> have not posted it until I find any patch which I can take care
> of. Sorry for this.

My apologies!  I did post the list of patches I'd added to the CF in my
"patch sweep" to -hackers, but I forgot to match it against the list of
contributors who weren't reviewing.   Sorry about that.

In general, I prefer to do the patch sweep 5 days out from the start of
the CF so that I have time to email people about whether or not their
patches should have been included.   However, this time an emergency
cropped up just before the CF started and I found myself doing the patch
sweep the day before the CF, which didn't leave time for an email response.

This is one of those areas where better tooling could help a lot.

-- 
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: [HACKERS] preserving forensic information when we freeze

[Marko Kreen]

On Wed, Jul 3, 2013 at 8:07 PM, Andres Freund  wrote:
> On 2013-07-02 16:29:56 -0400, Robert Haas wrote:
>> There's no possibility of getting confused here; if X is still around
>> at all, it's xmax is of the same generation as Y's xmin.  Otherwise,
>> we've had an undetected XID wraparound.
>
> Another issue I thought about is what we will return for SELECT xmin
> FROM blarg; Some people use that in their applications (IIRC
> skytools/pqg/londiste does so) and they might get confused if we
> suddently return xids from the future. On the other hand, not returning
> the original xid would be a shame as well...

Skytools/pqg/londiste use only txid_* APIs, they don't look at
4-byte xids on table rows.

-- 
marko


-- 
Sent 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 COLLATE

[Andrew Dunstan]

On 07/03/2013 03:00 PM, Alvaro Herrera wrote:

Robert Haas escribió:


I agree.  I think it'd be a good idea to get the buildfarm to run the
existing collate.utf8.linux test regularly on platforms where it
passes,

+1



I can probably whip up a module for it. (I created the buildfarm module 
system so people could create their own addons, but sadly nobody seems 
to have risen to the bait.)


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

[Andrew Dunstan]

On 07/03/2013 02:50 PM, Josh Berkus wrote:

On 07/03/2013 07:43 AM, Robert Haas wrote:

Let's have a new schedule called minute-check with the objective to run the

common tests in 60 secs.

Note that we're below 60s even with assert and CLOBBER_CACHE_ALWAYS, at
least on my laptop.



I find that hard to believe. On friarbird, which has 
CLOBBER_CACHE_ALWAYS turned on, "make check" takes 110 minutes. The same 
machine runs nightjar which takes 34 seconds.


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] [PATCH] big test separation POC

[Fabien COELHO]

Here is a v2 which is more likely to work under VPATH.


Here is a POC v4 which relies on multiple --schedule instead of creating 
concatenated schedule files.


--
Fabien.diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index d5935b6..8a39f7d 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)/parallel_big_schedule $(srcdir)/resultmap
 
 install-tests: all install install-lib installdirs-tests
 	$(MAKE) -C $(top_builddir)/contrib/spi install
@@ -137,19 +137,43 @@ tablespace-setup:
 ## Run tests
 ##
 
+# installcheck vs check:
+# - whether test is run against installed or compiled version
+# test schedules: parallel, parallel_big, standby
+# serial schedules can be derived from parallel schedules
+
+derived_schedules = serial_schedule serial_big_schedule
+
+serial_%: parallel_%
+	echo "# this file is generated automatically, do not edit!" > $@
+	egrep '^(test|ignore):' $< | \
+	while read op list ; do \
+	  for test in $$list ; do \
+	echo "$$op $$test" ; \
+	  done ; \
+	done >> $@
+
 REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
 
+# before installation, parallel
 check: all tablespace-setup
-	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(TEMP_CONF) $(EXTRA_TESTS)
+	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TEMP_CONF) \
+	  --schedule=$(srcdir)/parallel_schedule $(EXTRA_TESTS)
 
-installcheck: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+# after installation, serial
+installcheck: all tablespace-setup serial_schedule
+	$(pg_regress_installcheck) $(REGRESS_OPTS) \
+	  --schedule=serial_schedule $(EXTRA_TESTS)
 
+# after installation, parallel
 installcheck-parallel: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
+	$(pg_regress_installcheck) $(REGRESS_OPTS) $(MAXCONNOPT) \
+	  --schedule=$(srcdir)/parallel_schedule $(EXTRA_TESTS)
 
+# after installation
 standbycheck: all
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/standby_schedule --use-existing
+	$(pg_regress_installcheck) $(REGRESS_OPTS) \
+	  --schedule=$(srcdir)/standby_schedule --use-existing
 
 # old interfaces follow...
 
@@ -157,11 +181,19 @@ runcheck: check
 runtest: installcheck
 runtest-parallel: installcheck-parallel
 
-bigtest: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
+bigtest: installcheck-big
+
+# test = after installation, serial
+installcheck-big: all tablespace-setup serial_schedule serial_big_schedule
+	$(pg_regress_installcheck) $(REGRESS_OPTS) \
+	  --schedule=serial_schedule \
+	  --schedule=serial_big_schedule
 
+# check = before installation, parallel
 bigcheck: all tablespace-setup
-	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
+	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) \
+	  --schedule=$(srcdir)/parallel_schedule \
+	  --schedule=$(srcdir)/parallel_big_schedule
 
 
 ##
@@ -173,6 +205,6 @@ clean distclean maintainer-clean: clean-lib
 	rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX)
 	rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
 # things created by various check targets
-	rm -f $(output_files) $(input_files)
+	rm -f $(output_files) $(input_files) $(derived_schedules)
 	rm -rf testtablespace
 	rm -rf $(pg_regress_clean_files)
diff --git a/src/test/regress/parallel_big_schedule b/src/test/regress/parallel_big_schedule
new file mode 100644
index 000..9434abf
--- /dev/null
+++ b/src/test/regress/parallel_big_schedule
@@ -0,0 +1 @@
+test: numeric_big
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
deleted file mode 100644
index ceeca73..000
--- a/src/test/regress/serial_schedule
+++ /dev/null
@@ -1,140 +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: oid

[HACKERS] Re: [COMMITTERS] pgsql: Revert "Hopefully-portable regression tests for CREATE/ALTER/DRO

[Fabien COELHO]

The buildfarm is sad.


Do you have a pointer about the issue?

Is it that builds are performed in some particular locale?

--
Fabien.


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

Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List

[Christopher Browne]

On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain wrote:

> > Clearly I ticked off a bunch of people by publishing "the list".  On the
> > other hand, in the 5 days succeeding the post, more than a dozen
> > additional people signed up to review patches, and we got some of the
> > "ready for committer" patches cleared out -- something which nothing
> > else I did, including dozens of private emails, general pleas to this
> > mailing list, mails to the RRReviewers list, served to accomplish, in
> > this or previous CFs.
>
> Others rules appeared, like the 5 days limit.
> To me it outlines that some are abusing the CF app and pushing there
> useless
> patches (not still ready or complete, WIP, ...


Seems to me that "useless" overstates things, but it does seem fair to
say that some patches are not sufficiently well prepared to be efficiently
added into Postgres.

> So, as an experiment, call it a mixed result.  I would like to have some
> > other way to motivate reviewers than public shame.  I'd like to have
> > some positive motivations for reviewers, such as public recognition by
> > our project and respect from hackers, but I'm doubting that those are
> > actually going to happen, given the feedback I've gotten on this list to
> > the idea.
>
> You're looking at a short term, big effect.
> And long term ? Will people listed still be interested to participate in a
> project which stamps people ?
>
> With or without review, it's a shame if people stop proposing patches
> because
> they are not sure to get time to review other things *in time*.


Well, if the project is hampered by not being able to get *all* the
changes that people imagine that they want to put in, then we have a
real problem of needing a sort of "triage" to determine which changes
will be accepted, and which will not.

Perhaps we need an extra status in the CommitFest application, namely
one that characterizes:
   Insufficiently Important To Warrant Review

That's too long a term.  Perhaps "Not Review-worthy" expresses it better?
-- 
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

Re: [HACKERS] preserving forensic information when we freeze

[Robert Haas]

On Wed, Jul 3, 2013 at 1:07 PM, Andres Freund  wrote:
> Well, nothing would prevent using the HeapTupleHeaderGetRawXmin() in
> those places. Exactly the number of callsites is what makes me think
> that somebody will get this wrong in the future.

Well, I guess I could go rework the whole patch that way.  It's a fair
request, but I kinda doubt it's going to make the patch smaller.

>> > * I think rewrite_heap_dead_tuple needs to check for a frozen xmin and
>> >   store that. We might looking at a chain which partially was done in
>> >   <9.4. Not sure if that's a realistic scenario, but I'd rather be safe.
>>
>> IIUC, you're talking about the scenario where we have an update chain
>> X -> Y, where X is dead but not actually removed and Y is
>> (forensically) frozen.   We're examining tuple Y and trying to
>> determine whether X has been entered in rs_unresolved_tups.  If, as I
>> think you're proposing, we consider the xmin of Y to be
>> FrozenTransactionId, we will definitely not find it - because the way
>> it got into the table in the first place was based on the value
>> returned by HeapTupleHeaderGetUpdateXid().  And that value is certain
>> not to be FrozenTransactionId, because we never set the xmax of a
>> tuple to FrozenTransactionId.
>
> I am thinking of something slightly different. rewrite_heap_dead_tuple()
> now removes tuples/xids from the unresolved table that could be from a
> different xid epoch since it unconditionally does a HASH_REMOVE if it
> finds an entry doing a lookup using the *preserved* xid. Earlier that
> was harmless since for frozen tuples it only ever used
> FrozenTransactionId which obviously cannot be part of a valid chain and
> couldn't even get entered into unresolved_tups.
>
> I am not sure at all if that actually can be harmful but there isn't any
> reason we would need to do the delete so I wouldn't. There can be
> complex enough situation where later parts of a ctid chain are dead and
> earlier ones are recently dead and such that I would rather be cautious.

OK, I think I see your point, and I think you're right.

>> There's no possibility of getting confused here; if X is still around
>> at all, it's xmax is of the same generation as Y's xmin.  Otherwise,
>> we've had an undetected XID wraparound.
>
> Another issue I thought about is what we will return for SELECT xmin
> FROM blarg; Some people use that in their applications (IIRC
> skytools/pqg/londiste does so) and they might get confused if we
> suddently return xids from the future. On the other hand, not returning
> the original xid would be a shame as well...

Yeah.  I think the system columns that we have today are pretty much
crap.  I wonder if we couldn't throw them out and replace them with
some kind of functions that you can pass a row to.  That would allow
us to expose a lot more detail without adding a bajillion hidden
columns, and for a bonus we'd save substantially on catalog bloat.

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

[Alvaro Herrera]

Robert Haas escribió:

> I agree.  I think it'd be a good idea to get the buildfarm to run the
> existing collate.utf8.linux test regularly on platforms where it
> passes,

+1

-- 
Á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] Fix pgstattuple/pgstatindex to use regclass-type as the argument

[Fujii Masao]

On Wed, Jun 26, 2013 at 12:39 AM, Robert Haas  wrote:
> On Thu, Jun 20, 2013 at 2:32 PM, Fujii Masao  wrote:
>> On Thu, Jun 20, 2013 at 11:43 AM, Satoshi Nagayasu  wrote:
>>> (2013/06/17 4:02), Fujii Masao wrote:

 On Sat, Mar 9, 2013 at 3:23 PM, Satoshi Nagayasu  wrote:
>
> It is obviously easy to keep two types of function interfaces,
> one with regclass-type and another with text-type, in the next
> release for backward-compatibility like below:
>
> pgstattuple(regclass)  -- safer interface.
> pgstattuple(text)  -- will be depreciated in the future release.


 So you're thinking to remove pgstattuple(oid) soon?
>>>
>>>
>>> AFAIK, a regclass type argument would accept an OID value,
>>> which means regclass type has upper-compatibility against
>>> oid type.
>>>
>>> So, even if the declaration is changed, compatibility could
>>> be kept actually. This test case (in sql/pgstattuple.sql)
>>> confirms that.
>>>
>>> 
>>> select * from pgstatindex('myschema.test_pkey'::regclass::oid);
>>>  version | tree_level | index_size | root_block_no | internal_pages |
>>> leaf_pages | empty_pages | deleted_pages | avg_leaf_density |
>>> leaf_fragmentation
>>> -+++---+++-+---+--+
>>>2 |  0 |   8192 | 1 |  0 |
>>> 1 |   0 | 0 | 0.79 |0
>>> (1 row)
>>> 
>>>
>>>
> Having both interfaces for a while would allow users to have enough
> time to rewrite their applications.
>
> Then, we will be able to obsolete (or just drop) old interfaces
> in the future release, maybe 9.4 or 9.5. I think this approach
> would minimize an impact of such interface change.
>
> So, I think we can clean up function arguments in the pgstattuple
> module, and also we can have two interfaces, both regclass and text,
> for the next release.
>
> Any comments?


 In the document, you should mark old functions as deprecated.
>>>
>>>
>>> I'm still considering changing the function name as Tom pointed
>>> out. How about "pgstatbtindex"?
>>
>> Or just adding pgstatindex(regclass)?
>>
>>> In fact, pgstatindex does support only BTree index.
>>> So, "pgstatbtindex" seems to be more appropriate for this function.
>>
>> Can most ordinary users realize "bt" means "btree"?
>>
>>> We can keep having both (old) pgstatindex and (new) pgstatbtindex
>>> during next 2-3 major releases, and the old one will be deprecated
>>> after that.
>>
>> Since pg_relpages(oid) doesn't exist, pg_relpages() is in the same
>> situation as pgstatindex(), i.e., we cannot just replace pg_relpages(text)
>> with pg_relpages(regclass) for the backward-compatibility. How do you
>> think we should solve the pg_relpages() problem? Rename? Just
>> add pg_relpages(regclass)?
>
> Adding a function with a new name seems likely to be smoother, since
> that way you don't have to worry about problems with function calls
> being thought ambiguous.

Could you let me know the example that this problem happens?

For the test, I just implemented the regclass-version of pg_relpages()
(patch attached) and tested some cases. But I could not get that problem.

SELECT pg_relpages('hoge');-- OK
SELECT pg_relpages(oid) FROM pg_class WHERE relname = 'hoge';-- OK
SELECT pg_relpages(relname) FROM pg_class WHERE relname = 'hoge';-- OK

Regards,

-- 
Fujii Masao


pg_relpages_test.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] refresh materialized view concurrently

[Kevin Grittner]

Tom Lane  wrote:
> Robert Haas  writes:
>> Tom Lane  wrote:
>>> Are we somehow not going through ExecOpenIndices?
>
>> I dunno.  I just did a quick black-box test:
>>
>> [ begin; insert; without commit ]
>>
>> No foo_pkey anywhere.
>
> That proves nothing, as we don't keep such locks after the query
> (and there's no reason to AFAICS).  See ExecCloseIndices.

OK.  I had seen that no locks were held after the insert and wasn't
aware that we acquired and then released them for each insert
within a transaction.  On the other hand, we acquire locks on all
indexes even for a HOT UPDATE which uses a seqscan, and hold those
until end of transaction.  Is there a reason for that?

I suppose that since a concurrent refresh is very likely to lock
all these indexes with RowExclusiveLock anyway, as a result of the
DELETE, UPDATE and INSERT statements subsequently issued through
SPI, I might was well acquire that lock when I look at the
definition, and not release it -- so that the subsequent locks are
local to the backend, and therefore faster.

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] New regression test time

[Josh Berkus]

On 07/03/2013 07:43 AM, Robert Haas wrote:
> Let's have a new schedule called minute-check with the objective to run the
>> common tests in 60 secs.

Note that we're below 60s even with assert and CLOBBER_CACHE_ALWAYS, at
least on my laptop.

-- 
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: [HACKERS] Support for REINDEX CONCURRENTLY

[Michael Paquier]

On Thu, Jul 4, 2013 at 3:26 AM, Fujii Masao  wrote:
> On Thu, Jul 4, 2013 at 2:41 AM, Fujii Masao  wrote:
>> On Thu, Jul 4, 2013 at 2:36 AM, Andres Freund  wrote:
>>> On 2013-07-04 02:32:32 +0900, Michael Paquier wrote:
 > Wouldn't it make more sense to fetch the toast index oid in the query
 > ontop instead of making a query for every relation?
>>
>> +1
>> I changed the query that way. Updated version of the patch attached.
>>
>> Also I updated the rules.out because Michael changed the system_views.sql.
>> Otherwise, the regression test would fail.
>>
>> Will commit this patch.
>
> Committed. So, let's get to REINDEX CONCURRENTLY patch!
Thanks for the hard work! I'll work on something based on MVCC
catalogs, so at least lock will be lowered at swap phase and isolation
tests will be added.
--
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] LATERAL quals revisited

[Tom Lane]

I wrote:
> So attached is a draft patch for this.  It's not complete yet because
> there are various comments that are now wrong and need to be updated;
> but I think the code is functioning correctly.

Hm, spoke too soon :-(.  This query causes an assertion failure, with or
without my draft patch:

select c.*,a.*,ss1.q1,ss2.q1,ss3.* from
  int8_tbl c left join (
int8_tbl a left join
  (select q1, coalesce(q2,f1) as x from int8_tbl b, int4_tbl b2) ss1
  on a.q2 = ss1.q1
cross join
lateral (select q1, coalesce(ss1.x,q2) as y from int8_tbl d) ss2
  ) on c.q2 = ss2.q1,
  lateral (select * from int4_tbl i where ss2.y > f1) ss3;

TRAP: FailedAssertion("!(bms_is_subset(phinfo->ph_needed, 
phinfo->ph_may_need))", File: "initsplan.c", Line: 213)

What's happening is that distribute_qual_to_rels concludes (correctly)
that the "ss2.y > f1" clause must be postponed until after the nest of
left joins, since those could null ss2.y.  So the PlaceHolderVar for
ss2.y is marked as being needed at the topmost join level.  However,
find_placeholders_in_jointree had only marked the PHV as being "maybe
needed" to scan the "i" relation, since that's what the syntactic
location of the reference implies.  Since we depend on the assumption
that ph_needed is always a subset of ph_may_need, there's an assertion
that fires if that stops being true, and that's what's crashing.

After some thought about this, I'm coming to the conclusion that lateral
references destroy the ph_maybe_needed optimization altogether: we
cannot derive an accurate estimate of where a placeholder will end up in
the final qual distribution, short of essentially doing all the work in
deconstruct_jointree over again.  I guess in principle we could repeat
deconstruct_jointree until we had stable estimates of the ph_needed
locations, but that would be expensive and probably would induce a lot
of new planner bugs (since the data structure changes performed during
deconstruct_jointree aren't designed to be backed out easily).

The only place where ph_may_need is actually used is in this bit in
make_outerjoininfo():

/*
 * Examine PlaceHolderVars.  If a PHV is supposed to be evaluated within
 * this join's nullable side, and it may get used above this join, then
 * ensure that min_righthand contains the full eval_at set of the PHV.
 * This ensures that the PHV actually can be evaluated within the RHS.
 * Note that this works only because we should already have determined the
 * final eval_at level for any PHV syntactically within this join.
 */
foreach(l, root->placeholder_list)
{
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
Relidsph_syn_level = phinfo->ph_var->phrels;

/* Ignore placeholder if it didn't syntactically come from RHS */
if (!bms_is_subset(ph_syn_level, right_rels))
continue;

/* We can also ignore it if it's certainly not used above this join */
/* XXX this test is probably overly conservative */
if (bms_is_subset(phinfo->ph_may_need, min_righthand))
continue;

/* Else, prevent join from being formed before we eval the PHV */
min_righthand = bms_add_members(min_righthand, phinfo->ph_eval_at);
}

Looking at it again, it's not really clear that skipping placeholders in
this way results in very much optimization --- sometimes we can avoid
constraining join order, but how often?  I tried diking out the check
on ph_may_need from this loop, and saw no changes in the regression test
results (not that that proves a whole lot about optimization of complex
queries).  So I'm pretty tempted to just remove ph_may_need, along with
the machinery that computes it.

Another possibility would be to keep the optimization, but disable it in
queries that use LATERAL.  I don't much care for that though --- seems
too Rube Goldbergish, and in any case I have a lot less faith in the
whole concept now than I had before I started digging into this issue.

Thoughts?

regards, tom lane


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

Re: [HACKERS] Redesigning checkpoint_segments

[Peter Eisentraut]

On 6/6/13 4:09 PM, Heikki Linnakangas wrote:
> On 06.06.2013 20:24, Josh Berkus wrote:
>>> Yeah, something like that :-). I was thinking of letting the estimate
>>> decrease like a moving average, but react to any increases immediately.
>>> Same thing we do in bgwriter to track buffer allocations:
>>
>> Seems reasonable.
> 
> Here's a patch implementing that. Docs not updated yet. I did not change
> the way checkpoint_segments triggers checkpoints - that'll can be a
> separate patch. This only decouples the segment preallocation behavior
> from checkpoint_segments. With the patch, you can set
> checkpoint_segments really high, without consuming that much disk space
> all the time.

I don't understand what this patch, by itself, will accomplish in terms
of the originally stated goals of making checkpoint_segments easier to
tune, and controlling disk space used.  To some degree, it makes both of
these things worse, because you can no longer use checkpoint_segments to
control the disk space.  Instead, it is replaced by magic.

What sort of behavior are you expecting to come out of this?  In testing,
I didn't see much of a difference.  Although I'd expect that this would
actually preallocate fewer segments than the old formula.

Two small issues in the code:

Code change doesn't match comment:

+*
+* XXX: We don't have a good estimate of how many WAL files we should 
keep
+* preallocated here. Quite arbitrarily, use max_advance=5. That's good
+* enough for current use of this function; this only gets called when
+* there are no more preallocated WAL segments available.
 */
installed_segno = logsegno;
-   max_advance = XLOGfileslop;
+   max_advance = CheckPointSegments;

KB should be kB.



-- 
Sent 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 Thu, Jul 4, 2013 at 2:41 AM, Fujii Masao  wrote:
> On Thu, Jul 4, 2013 at 2:36 AM, Andres Freund  wrote:
>> On 2013-07-04 02:32:32 +0900, Michael Paquier wrote:
>>> > Wouldn't it make more sense to fetch the toast index oid in the query
>>> > ontop instead of making a query for every relation?
>
> +1
> I changed the query that way. Updated version of the patch attached.
>
> Also I updated the rules.out because Michael changed the system_views.sql.
> Otherwise, the regression test would fail.
>
> Will commit this patch.

Committed. So, let's get to REINDEX CONCURRENTLY patch!

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] [9.4 CF 1] The Commitfest Slacker List

[Cédric Villemain]

> Clearly I ticked off a bunch of people by publishing "the list".  On the
> other hand, in the 5 days succeeding the post, more than a dozen
> additional people signed up to review patches, and we got some of the
> "ready for committer" patches cleared out -- something which nothing
> else I did, including dozens of private emails, general pleas to this
> mailing list, mails to the RRReviewers list, served to accomplish, in
> this or previous CFs.

Others rules appeared, like the 5 days limit.
To me it outlines that some are abusing the CF app and pushing there useless 
patches (not still ready or complete, WIP, ...)

> So, as an experiment, call it a mixed result.  I would like to have some
> other way to motivate reviewers than public shame.  I'd like to have
> some positive motivations for reviewers, such as public recognition by
> our project and respect from hackers, but I'm doubting that those are
> actually going to happen, given the feedback I've gotten on this list to
> the idea.

You're looking at a short term, big effect.
And long term ? Will people listed still be interested to participate in a 
project which stamps people ?

With or without review, it's a shame if people stop proposing patches because 
they are not sure to get time to review other things *in time*.
-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation


signature.asc
Description: This is a digitally signed message part.

Re: [HACKERS] Add regression tests for COLLATE

[Robert Haas]

On Wed, Jul 3, 2013 at 1:38 PM, Tom Lane  wrote:
> Andres Freund  writes:
>> On 2013-07-03 13:29:18 -0400, Robert Haas wrote:
>>> I think that's a killer blow for this particular patch.  I'm going to
>>> set this to rejected in the CF app.
>
>> Can't we use a alternative expected file for those?
>
> Alternative expected files are a PITA to maintain, at least for
> committers who don't have a platform that can reproduce the alternative
> behavior.  If this test were of somewhat higher value I'd be in favor of
> fixing it that way, but given that it's been seriously constrained by
> the portability issues that were already considered, I'm not sure it's
> worth our trouble.  (There's also no very strong reason to believe that
> we found out all the remaining portability issues.  Maybe we should have
> left it in there for a day, just to see if the buildfarm would show any
> other symptoms besides this one.)

I agree.  I think it'd be a good idea to get the buildfarm to run the
existing collate.utf8.linux test regularly on platforms where it
passes, but this particular approach is valuable mostly because
(supposedly) it was going to work everywhere.  However, it doesn'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] proposal: simple date constructor from numeric values

[Pavel Stehule]

Hello


> So my vote is for make_time(hour int, min int, sec float8).
>

so here is a patch

Regards

Pavel



> regards, tom lane


make_date.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] Add regression tests for COLLATE

[Tom Lane]

Andres Freund  writes:
> On 2013-07-03 13:29:18 -0400, Robert Haas wrote:
>> I think that's a killer blow for this particular patch.  I'm going to
>> set this to rejected in the CF app.

> Can't we use a alternative expected file for those?

Alternative expected files are a PITA to maintain, at least for
committers who don't have a platform that can reproduce the alternative
behavior.  If this test were of somewhat higher value I'd be in favor of
fixing it that way, but given that it's been seriously constrained by
the portability issues that were already considered, I'm not sure it's
worth our trouble.  (There's also no very strong reason to believe that
we found out all the remaining portability issues.  Maybe we should have
left it in there for a day, just to see if the buildfarm would show any
other symptoms besides this one.)

regards, tom lane


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

Re: [HACKERS] Support for REINDEX CONCURRENTLY

[Andres Freund]

On 2013-07-04 02:32:32 +0900, Michael Paquier wrote:
> > Wouldn't it make more sense to fetch the toast index oid in the query
> > ontop instead of making a query for every relation?
> With something like a CASE condition in the upper query for
> reltoastrelid? This code path is not only taken by indexes but also by
> tables. So I thought that it was cleaner and more readable to fetch
> the index OID only if necessary as a separate query.

A left join should do the trick?

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 REINDEX CONCURRENTLY

[Michael Paquier]

On Wed, Jul 3, 2013 at 11:16 PM, Andres Freund  wrote:
> On 2013-07-03 10:03:26 +0900, Michael Paquier wrote:
>> index 9ee9ea2..23e0373 100644
>> --- a/src/bin/pg_dump/pg_dump.c
>> +++ b/src/bin/pg_dump/pg_dump.c
>> @@ -2778,10 +2778,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>   PQExpBuffer upgrade_query = createPQExpBuffer();
>>   PGresult   *upgrade_res;
>>   Oid pg_class_reltoastrelid;
>> - Oid pg_class_reltoastidxid;
>>
>>   appendPQExpBuffer(upgrade_query,
>> -   "SELECT c.reltoastrelid, 
>> t.reltoastidxid "
>> +   "SELECT c.reltoastrelid "
>> "FROM pg_catalog.pg_class c LEFT 
>> JOIN "
>> "pg_catalog.pg_class t ON 
>> (c.reltoastrelid = t.oid) "
>> "WHERE c.oid = 
>> '%u'::pg_catalog.oid;",
>> @@ -2790,7 +2789,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>   upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
>>
>>   pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0, 
>> PQfnumber(upgrade_res, "reltoastrelid")));
>> - pg_class_reltoastidxid = atooid(PQgetvalue(upgrade_res, 0, 
>> PQfnumber(upgrade_res, "reltoastidxid")));
>>
>>   appendPQExpBuffer(upgrade_buffer,
>>  "\n-- For binary upgrade, must preserve 
>> pg_class oids\n");
>> @@ -2803,6 +2801,10 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>   /* only tables have toast tables, not indexes */
>>   if (OidIsValid(pg_class_reltoastrelid))
>>   {
>> + PQExpBuffer index_query = createPQExpBuffer();
>> + PGresult   *index_res;
>> + Oid indexrelid;
>> +
>>   /*
>>* One complexity is that the table definition might 
>> not require
>>* the creation of a TOAST table, and the TOAST table 
>> might have
>> @@ -2816,10 +2818,23 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>> "SELECT 
>> binary_upgrade.set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n",
>> 
>> pg_class_reltoastrelid);
>>
>> - /* every toast table has an index */
>> + /* Every toast table has one valid index, so fetch it 
>> first... */
>> + appendPQExpBuffer(index_query,
>> +   "SELECT c.indexrelid 
>> "
>> +   "FROM 
>> pg_catalog.pg_index c "
>> +   "WHERE c.indrelid = 
>> %u AND c.indisvalid;",
>> +   
>> pg_class_reltoastrelid);
>> + index_res = ExecuteSqlQueryForSingleRow(fout, 
>> index_query->data);
>> + indexrelid = atooid(PQgetvalue(index_res, 0,
>> +
>> PQfnumber(index_res, "indexrelid")));
>> +
>> + /* Then set it */
>>   appendPQExpBuffer(upgrade_buffer,
>> "SELECT 
>> binary_upgrade.set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n",
>> -   
>> pg_class_reltoastidxid);
>> +   indexrelid);
>> +
>> + PQclear(index_res);
>> + destroyPQExpBuffer(index_query);
>
> Wouldn't it make more sense to fetch the toast index oid in the query
> ontop instead of making a query for every relation?
With something like a CASE condition in the upper query for
reltoastrelid? This code path is not only taken by indexes but also by
tables. So I thought that it was cleaner and more readable to fetch
the index OID only if necessary as a separate query.

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

[Simon Riggs]

On 3 July 2013 15:43, Robert Haas  wrote:
>
> > Let's have a new schedule called minute-check with the objective to run
> the
> > common tests in 60 secs.
> >
> > We can continue to expand the normal schedules from here.
> >
> > Anybody that wants short tests can run that, everyone else can run the
> full
> > test suite.
> >
> > We should be encouraging people to run the full test suite, not the fast
> > one.
>
> I like this idea, or some variant of it (fastcheck?).


I thought about that, but then people will spend time trying to tune it.

If its called minute-check and it runs in 60 secs then they'll leave it
alone...

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

Re: [HACKERS] Add regression tests for COLLATE

[Andres Freund]

On 2013-07-03 13:29:18 -0400, Robert Haas wrote:
> On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas  wrote:
> >> Please revert.
> >
> > OK, sure, but just for my education and general enlightenment ... what
> > platform is that?
> 
> Ah, never mind.  I see that the buildfarm is quickly turning red -
> NetBSD, OmniOS, and HP/UX are all unhappy.
> 
> I think that's a killer blow for this particular patch.  I'm going to
> set this to rejected in the CF app.

Can't we use a alternative expected file for those?

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

[Robert Haas]

On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas  wrote:
>> Please revert.
>
> OK, sure, but just for my education and general enlightenment ... what
> platform is that?

Ah, never mind.  I see that the buildfarm is quickly turning red -
NetBSD, OmniOS, and HP/UX are all unhappy.

I think that's a killer blow for this particular patch.  I'm going to
set this to rejected in the CF app.

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

[Robert Haas]

On Wed, Jul 3, 2013 at 1:11 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I did remove those, plus made some other cleanups, and committed this.
>>  I think that there's now some duplication between this and the
>> collate.linux.utf8 test that should be cleaned up by removing the
>> duplicative stuff from that test, assuming this holds up OK when the
>> buildfarm - and other developers - test it out.
>
> I don't even need to test it:
>
> regression=# CREATE COLLATION collate_coll2 FROM "C";
> ERROR:  nondefault collations are not supported on this platform
>
> Please revert.

OK, sure, but just for my education and general enlightenment ... what
platform is that?

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


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

Re: [HACKERS] Add regression tests for COLLATE

[Tom Lane]

Robert Haas  writes:
> I did remove those, plus made some other cleanups, and committed this.
>  I think that there's now some duplication between this and the
> collate.linux.utf8 test that should be cleaned up by removing the
> duplicative stuff from that test, assuming this holds up OK when the
> buildfarm - and other developers - test it out.

I don't even need to test it:

regression=# CREATE COLLATION collate_coll2 FROM "C";
ERROR:  nondefault collations are not supported on this platform

Please revert.

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] proposal: simple date constructor from numeric values

[Pavel Stehule]

2013/7/3 Tom Lane :
> Alvaro Herrera  writes:
>> Peter Eisentraut escribió:
>>> On 7/1/13 3:47 AM, Pavel Stehule wrote:
 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.
>
>> I think this is wrong.  Datetime storage may be int, but since they're
>> microseconds underneath, we'd be unable to specify a full-resolution
>> timestamp if we didn't have float ms or integer ľs.  So either the
>> seconds argument should allow fractions (probably not a good idea), or
>> we should have another integer argument for microseconds (not
>> milliseconds as the above signature implies).
>
> FWIW, I'd vote for allowing the seconds to be fractional.  That's the
> way the user perceives things:
>
> regression=# select '12:34:56.789'::time;
>  time
> --
>  12:34:56.789
> (1 row)
>
> Moreover, an integer microseconds argument would be a shortsighted idea
> because it wires the precision limit into the function API.  As long as
> we make the seconds argument be float8, it will work fine even when the
> underlying precision switches to, say, nanoseconds.
>
> And lastly, those default arguments are a bad idea as well.  There's no
> reasonable use-case for make_time(12); that's almost certainly an error.
> Even more so for make_time().  While you could make some case for
> make_time(12,34) being useful, I don't think it buys much compared
> to writing out make_time(12,34,0), and having just one function
> signature is that much less cognitive load on users.
>

I had a plan use DEFAULT only for usec parameter (if it was used).
Seconds was mandatory parameter.

After tests on prototype I think so fractional sec is better. Separate
value (in usec) is really big number - not practical for hand writing

> So my vote is for make_time(hour int, min int, sec float8).

+1

Pavel
>
> 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] preserving forensic information when we freeze

[Andres Freund]

On 2013-07-02 16:29:56 -0400, Robert Haas wrote:
> On Mon, Jun 24, 2013 at 8:12 AM, Andres Freund  wrote:
> > Agreed. And it also improves the status quo when debugging. I wish this
> > would have been the representation since the beginning.
> >
> > Some remarks:
> > * I don't really like that HeapTupleHeaderXminFrozen() now is frequently
> >   performed without checking for FrozenTransactionId. I think the places
> >   where that's done are currently safe, but it seems likely that we
> >   introduce bugs due to people writing similar code.
> >   I think replacing *GetXmin by a wrapper that returns
> >   FrozenTransactionId if the hint bit tell us so would be a good
> >   idea. Then add *GetRawXmin for the rest (probably mostly display).
> >   Seems like it would make the patch actually smaller.
> 
> I did think about this approach, but it seemed like it would add
> cycles in a significant number of places.  For example, consider the
> HeapTupleSatisfies() routines, which are perhaps PostgreSQL's finest
> example of a place where you DON'T want to add any cycles.  All of the
> checks on xmin are conditional on HeapTupleHeaderXminCommitted()
> having been found already to be false.  That implies that the frozen
> bits aren't set either, so if HeapTupleHeaderGetXmin() were to recheck
> the bits it would be a waste.  As I got to the end of the patch I was
> a little dismayed by the number of places that did need adjustment to
> check both things, but there are still plenty of important places that
> don't.

Well, nothing would prevent using the HeapTupleHeaderGetRawXmin() in
those places. Exactly the number of callsites is what makes me think
that somebody will get this wrong in the future.

> > * The PLs imo should set fn_xmin to FrozenTransactionId if the hint bit
> >   tell us the tuple is frozen.
> 
> Why?  I thought about that, but it seemed to me that the purpose of
> that caching was to avoid confusing two functions whose pg_proc tuples
> ended up at the same TID.
> [good reasoning]

Man. I hate this hack. I wonder what happens if somebody does a VACUUM
FULL of pg_class and there are a bunch of functions created in the same
transaction...

> > * I think rewrite_heap_dead_tuple needs to check for a frozen xmin and
> >   store that. We might looking at a chain which partially was done in
> >   <9.4. Not sure if that's a realistic scenario, but I'd rather be safe.
> 
> IIUC, you're talking about the scenario where we have an update chain
> X -> Y, where X is dead but not actually removed and Y is
> (forensically) frozen.   We're examining tuple Y and trying to
> determine whether X has been entered in rs_unresolved_tups.  If, as I
> think you're proposing, we consider the xmin of Y to be
> FrozenTransactionId, we will definitely not find it - because the way
> it got into the table in the first place was based on the value
> returned by HeapTupleHeaderGetUpdateXid().  And that value is certain
> not to be FrozenTransactionId, because we never set the xmax of a
> tuple to FrozenTransactionId.

I am thinking of something slightly different. rewrite_heap_dead_tuple()
now removes tuples/xids from the unresolved table that could be from a
different xid epoch since it unconditionally does a HASH_REMOVE if it
finds an entry doing a lookup using the *preserved* xid. Earlier that
was harmless since for frozen tuples it only ever used
FrozenTransactionId which obviously cannot be part of a valid chain and
couldn't even get entered into unresolved_tups.

I am not sure at all if that actually can be harmful but there isn't any
reason we would need to do the delete so I wouldn't. There can be
complex enough situation where later parts of a ctid chain are dead and
earlier ones are recently dead and such that I would rather be cautious.

> There's no possibility of getting confused here; if X is still around
> at all, it's xmax is of the same generation as Y's xmin.  Otherwise,
> we've had an undetected XID wraparound.

Another issue I thought about is what we will return for SELECT xmin
FROM blarg; Some people use that in their applications (IIRC
skytools/pqg/londiste does so) and they might get confused if we
suddently return xids from the future. On the other hand, not returning
the original xid would be a shame as well...

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] proposal: simple date constructor from numeric values

[Tom Lane]

Alvaro Herrera  writes:
> Peter Eisentraut escribió:
>> On 7/1/13 3:47 AM, Pavel Stehule wrote:
>>> 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.

> I think this is wrong.  Datetime storage may be int, but since they're
> microseconds underneath, we'd be unable to specify a full-resolution
> timestamp if we didn't have float ms or integer µs.  So either the
> seconds argument should allow fractions (probably not a good idea), or
> we should have another integer argument for microseconds (not
> milliseconds as the above signature implies).

FWIW, I'd vote for allowing the seconds to be fractional.  That's the
way the user perceives things:

regression=# select '12:34:56.789'::time;
 time 
--
 12:34:56.789
(1 row)

Moreover, an integer microseconds argument would be a shortsighted idea
because it wires the precision limit into the function API.  As long as
we make the seconds argument be float8, it will work fine even when the
underlying precision switches to, say, nanoseconds.

And lastly, those default arguments are a bad idea as well.  There's no
reasonable use-case for make_time(12); that's almost certainly an error.
Even more so for make_time().  While you could make some case for
make_time(12,34) being useful, I don't think it buys much compared
to writing out make_time(12,34,0), and having just one function
signature is that much less cognitive load on users.

So my vote is for make_time(hour int, min int, sec float8).

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

[Robert Haas]

On Thu, May 9, 2013 at 9:27 PM, Robins Tharakan  wrote:
> One workaround is to use DROP COLLATION IF EXISTS, that conveys the message
> without UTF8 in the message string.
>
> But three other tests use ALTER COLLATION and I see no way but to comment /
> disable them. Currently have them disabled (with comments as to what they
> do, and why they're disabled).
>
> This updated patch doesn't have UTF8 string anywhere.
> Let me know if you prefer to remove the commented out tests completely.

I did remove those, plus made some other cleanups, and committed this.
 I think that there's now some duplication between this and the
collate.linux.utf8 test that should be cleaned up by removing the
duplicative stuff from that test, assuming this holds up OK when the
buildfarm - and other developers - test it out.  Could you prepare a
patch towards that end?

-- 
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] proposal: simple date constructor from numeric values

[Pavel Stehule]

2013/7/3 Alvaro Herrera :
> Peter Eisentraut escribió:
>> On 7/1/13 3:47 AM, Pavel Stehule wrote:
>
>> > 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.
>
> I think this is wrong.  Datetime storage may be int, but since they're
> microseconds underneath, we'd be unable to specify a full-resolution
> timestamp if we didn't have float ms or integer µs.  So either the
> seconds argument should allow fractions (probably not a good idea), or
> we should have another integer argument for microseconds (not
> milliseconds as the above signature implies).

so make_time(hour int, mi int, sec int, usec int DEFAULT 0)

Is good for all ?

Regards

Pavel


>
> --
> Á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] proposal: simple date constructor from numeric values

[Alvaro Herrera]

Peter Eisentraut escribió:
> On 7/1/13 3:47 AM, Pavel Stehule wrote:

> > 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.

I think this is wrong.  Datetime storage may be int, but since they're
microseconds underneath, we'd be unable to specify a full-resolution
timestamp if we didn't have float ms or integer µs.  So either the
seconds argument should allow fractions (probably not a good idea), or
we should have another integer argument for microseconds (not
milliseconds as the above signature implies).

-- 
Á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] Add regression tests for ROLE (USER)

[Robert Haas]

On Thu, May 9, 2013 at 4:29 AM, Fabien COELHO  wrote:
> This updated version works for me and addresses previous comments.
>
> I think that such tests are definitely valuable, especially as many corner
> cases which must trigger errors are covered.
>
> I recommend to apply it.

I'm attaching an update of this patch that renames both the new test
file (user->role), and the regression users that get created.  It also
fixes the serial schedule to match the parallel schedule.

However, before it can get committed, I think this set of tests needs
streamlining.  It does seem to me valuable, but I think it's wasteful
in terms of runtime to create so many roles, do just one thing with
them, and then drop them.  I recommend consolidating some of the
tests.  For example:

+-- Should work. ALTER ROLE with (UN)ENCRYPTED PASSWORD
+CREATE ROLE regress_rol_rol14;
+ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD 'abc';
+DROP ROLE regress_rol_rol14;
+CREATE ROLE regress_rol_rol15;
+ALTER ROLE regress_rol_rol15 WITH UNENCRYPTED PASSWORD 'abc';
+DROP ROLE regress_rol_rol15;
+
+-- Should fail. ALTER ROLE with (UN)ENCRYPTED PASSWORD but no password value
+CREATE ROLE regress_rol_rol16;
+ALTER ROLE regress_rol_rol16 WITH ENCRYPTED PASSWORD;
+DROP ROLE regress_rol_rol16;
+CREATE ROLE regress_rol_rol17;
+ALTER ROLE regress_rol_rol17 WITH UNENCRYPTED PASSWORD;
+DROP ROLE regress_rol_rol17;
+
+-- Should fail. ALTER ROLE with both UNENCRYPTED and ENCRYPTED
+CREATE ROLE regress_rol_rol18;
+ALTER ROLE regress_rol_rol18 WITH ENCRYPTED UNENCRYPTED PASSWORD 'abc';
+DROP ROLE regress_rol_rol18;

There's no reason that couldn't be written this way:

CREATE ROLE regress_rol_rol14;
ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD 'abc';
ALTER ROLE regress_rol_rol14 WITH UNENCRYPTED PASSWORD 'abc';
ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD;
ALTER ROLE regress_rol_rol14 WITH UNENCRYPTED PASSWORD;
ALTER ROLE regress_rol_rol14 WITH ENCRYPTED UNENCRYPTED PASSWORD 'abc';
DROP ROLE regress_rol_rol14;

Considering the concerns already expressed about the runtime of the
test, I think it's important to minimize the number of create/drop
role cycles that the tests perform.

Generally, I think that the tests which return a syntax error are of
limited value and should probably be dropped.  That is unlikely to get
broken by accident.  If the syntax error is being thrown by something
outside of bison proper, that's probably worth testing.  But I think
that testing random syntax variations is a pretty low-value
proposition.

Setting this to "Waiting on Author".

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


regress_role_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

[Brendan Jurd]

On 3 July 2013 21:41, Pavel Stehule  wrote:
> I am thinking so for these functions exists some consensus - minimally
> for function "date"(year, month, int) - I dream about this function
> ten years :)
>
> I am not sure about "datetime":
> a) we use "timestamp" name for same thing in pg
> b) we can simply construct timestamp as sum of date + time, what is
> little bit more practical (for me), because it doesn't use too wide
> parameter list.

I agree.  I've got no issues with using date + time arithmetic to
build a timestamp.

> what do you think about names?
>
> make_date
> make_time

I am fine with those names.  'make', 'construct', 'build', etc. are
all reasonable verbs for what the functions do, but 'make' is nice and
short, and will be familiar to people who've used a 'mktime'.

> I don't would to use to_date, to_time functions, a) because these
> functions use formatted input, b) we hold some compatibility with
> Oracle.

Yes, I agree.

Cheers,
BJ


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

Re: [HACKERS] refresh materialized view concurrently

[Andres Freund]

On 2013-07-03 11:08:32 -0400, Tom Lane wrote:
> Robert Haas  writes:
> > On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane  wrote:
> >> Are we somehow not going through ExecOpenIndices?
> 
> > I dunno.  I just did a quick black-box test:
> 
> > CREATE TABLE foo (a int primary key);
> > BEGIN;
> > INSERT INTO foo VALUES (1);
> > SELECT relation::regclass, locktype, mode, granted FROM pg_locks;
> 
> > I get:
> 
> >  relation |   locktype|   mode   | granted
> > --+---+--+-
> >  pg_locks | relation  | AccessShareLock  | t
> >  foo  | relation  | RowExclusiveLock | t
> >   | virtualxid| ExclusiveLock| t
> >   | transactionid | ExclusiveLock| t
> 
> > No foo_pkey anywhere.
> 
> That proves nothing, as we don't keep such locks after the query
> (and there's no reason to AFAICS).  See ExecCloseIndices.

Should be easy enough to test by hacking LOCK TABLE to lock indexes and
taking out a conflicting lock (SHARE?) in a second transaction. I wonder
if we shouldn't just generally allow that, I remember relaxing that
check before (when playing with CREATE/DROP CONCURRENTLY afair).

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] possible/feasible to specify field and value in error msg?

[Bruce Momjian]

On Wed, Jul  3, 2013 at 11:14:18AM -0400, Bruce Momjian wrote:
> On Wed, Jul  3, 2013 at 10:54:48AM +0200, Willy-Bas Loos wrote:
> > Hi,
> > 
> > I have some complicated query that truncates and fills a table and i get 
> > this
> > message:
> > ERROR:  smallint out of range
> > STATEMENT: 
> > This is in postgres 8.4
> > I don't know where the error is, and the query takes rather long. So it is
> > going to be a bit cumbersome for me to debug this.
> > 
> > Would it be possible/feasible to specify, in future versions of postgres:
> > * what value
> > * which field (of which table)
> > * the offending tuple? (possibly truncated to some threshold nr of 
> > characters)
> > 
> > I ask because i can imagine that, inside the code that handles this, you 
> > might
> > not have access to that information and adding access to it might be
> > inefficient.
> > 
> > I do get the whole query of course, and that is very handy for automated
> > things. But in this case, it doesn't help me.
> 
> We will add optional error details in Postgres 9.3:
> 
>   http://momjian.us/main/blogs/pgblog/2013.html#April_11_2013
> 
> I don't know if an out-of-range error would generate the column name.

I just tested this and it doesn't show the offending column name; 
sorry:

test=> CREATE TABLE test(x smallint);
CREATE TABLE
test=> \set VERBOSITY verbose
test=> INSERT INTO test VALUES (1000);
ERROR:  22003: smallint out of range
LOCATION:  i4toi2, int.c:349

-- 
  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] Patch to add regression tests for SCHEMA

[Robert Haas]

On Wed, May 8, 2013 at 9:19 PM, Robins Tharakan  wrote:
> Please find attached an updated patch with the said changes.
> I'll try to update the other patches (if they pertain to this feedback) and
> update on their respective threads (as well as on Commitfest).

This patch updates the parallel schedule but not the serial schedule.
Please try to remember to update both files when adding new test
files.  Also, please observe the admonitions in the parallel schedule,
at top of file:

# By convention, we put no more than twenty tests in any one parallel group;
# this limits the number of connections needed to run the tests.

In this particular case, I think that adding a new set of tests isn't
the right thing anyway.  Schemas are also known as namespaces, and the
existing "namespace" test is where related test cases live.  Add these
tests there instead.

Rename regression users to regress_rol_nsp1 etc. per convention
established in the CREATE OPERATOR patch.

Setting patch to "Waiting on Author".

-- 
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] dynamic background workers

[Alvaro Herrera]

Andres Freund escribió:

> Just as a datapoint, if you benchmark the numbers of forks that can be
> performed by a single process (i.e. postmaster) the number is easily in
> the 10s of thousands. Now forking that much has some scalability
> implications inside the kernel, but still.
> I'd be surprised if the actual fork is more than 5-10% of the current
> cost of starting a new backend.

I played at having some thousands of registered bgworkers on my laptop,
and there wasn't even that much load.  So yeah, you can have lots of
forks.

-- 
Á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] possible/feasible to specify field and value in error msg?

[Bruce Momjian]

On Wed, Jul  3, 2013 at 10:54:48AM +0200, Willy-Bas Loos wrote:
> Hi,
> 
> I have some complicated query that truncates and fills a table and i get this
> message:
> ERROR:  smallint out of range
> STATEMENT: 
> This is in postgres 8.4
> I don't know where the error is, and the query takes rather long. So it is
> going to be a bit cumbersome for me to debug this.
> 
> Would it be possible/feasible to specify, in future versions of postgres:
> * what value
> * which field (of which table)
> * the offending tuple? (possibly truncated to some threshold nr of characters)
> 
> I ask because i can imagine that, inside the code that handles this, you might
> not have access to that information and adding access to it might be
> inefficient.
> 
> I do get the whole query of course, and that is very handy for automated
> things. But in this case, it doesn't help me.

We will add optional error details in Postgres 9.3:

http://momjian.us/main/blogs/pgblog/2013.html#April_11_2013

I don't know if an out-of-range error would generate the column name.

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

[Robert Haas]

On Tue, May 7, 2013 at 6:40 PM, Robins Tharakan  wrote:
> Have provided an updated patch as per Fabien's recent response on Commitfest
> site.
> Any and all feedback is appreciated.

I think you should rename the roles used here to regress_rol_seq1 etc.
to match the CREATE OPERATOR patch.

And you need to update the expected output.

Setting this one to "Waiting on Author".

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


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

[HACKERS] (trivial patch) remove superfluous semicolons from pg_dump

[Ian Lawrence Barwick]

I noticed an instance of 'appendPQExpBuffer(query, ";");' in pg_dump.c which
seems pointless and mildly confusing. There's another seemingly
useless one in pg_dumpall.c. Attached patch removes both (if that
makes any sense).


Regards

Ian Barwick


pg_dump-cull-semicolons.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] refresh materialized view concurrently

[Tom Lane]

Robert Haas  writes:
> On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane  wrote:
>> Are we somehow not going through ExecOpenIndices?

> I dunno.  I just did a quick black-box test:

> CREATE TABLE foo (a int primary key);
> BEGIN;
> INSERT INTO foo VALUES (1);
> SELECT relation::regclass, locktype, mode, granted FROM pg_locks;

> I get:

>  relation |   locktype|   mode   | granted
> --+---+--+-
>  pg_locks | relation  | AccessShareLock  | t
>  foo  | relation  | RowExclusiveLock | t
>   | virtualxid| ExclusiveLock| t
>   | transactionid | ExclusiveLock| t

> No foo_pkey anywhere.

That proves nothing, as we don't keep such locks after the query
(and there's no reason to AFAICS).  See ExecCloseIndices.

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

[Robert Haas]

On Mon, May 13, 2013 at 8:37 PM, Robins Tharakan  wrote:
> Hi,
>
> Patch to add more regression tests to ASYNC (/src/backend/commands/async).
> Take code-coverage from 39% to 75%.
>
> Any and all feedback is obviously welcome.
>
> p.s.: Please let me know whether these tests are useless or would not be
> committed owing to unrelated reasons. As also, if these tests need to be
> clubbed (bundled up in 2-3) and submitted as a single submit.

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] refresh materialized view concurrently

[Robert Haas]

On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane  wrote:
>>> I don't believe that that happens.  If it does, it's a bug.  Either the
>>> planner or the executor should be taking a lock on each index touched
>>> by a query.
>
>> It seems Kevin's right.  Not sure why that doesn't break.
>
> Are we somehow not going through ExecOpenIndices?

I dunno.  I just did a quick black-box test:

CREATE TABLE foo (a int primary key);
BEGIN;
INSERT INTO foo VALUES (1);
SELECT relation::regclass, locktype, mode, granted FROM pg_locks;

I get:

 relation |   locktype|   mode   | granted
--+---+--+-
 pg_locks | relation  | AccessShareLock  | t
 foo  | relation  | RowExclusiveLock | t
  | virtualxid| ExclusiveLock| t
  | transactionid | ExclusiveLock| t

No foo_pkey anywhere.

-- 
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] refresh materialized view concurrently

[Tom Lane]

Robert Haas  writes:
> On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane  wrote:
>> I don't believe that that happens.  If it does, it's a bug.  Either the
>> planner or the executor should be taking a lock on each index touched
>> by a query.

> It seems Kevin's right.  Not sure why that doesn't break.

Are we somehow not going through ExecOpenIndices?

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

[Robert Haas]

On Wed, Jul 3, 2013 at 2:28 AM, Simon Riggs  wrote:
>> It's sad to simply reject meaningful automated tests on the basis of doubt
>> that they're important enough to belong in every human-in-the-loop test
>> run.
>> I share the broader vision for automated testing represented by these
>> patches.
>
> +1  We should be encouraging people to submit automated tests.

It seems like there is now a clear consensus to proceed here, so I'll
start looking at committing some of these tests.

> Automated testing is about x10-100 faster than manual testing, so I see new
> tests as saving me time not wasting it.

+1.

> Let's have a new schedule called minute-check with the objective to run the
> common tests in 60 secs.
>
> We can continue to expand the normal schedules from here.
>
> Anybody that wants short tests can run that, everyone else can run the full
> test suite.
>
> We should be encouraging people to run the full test suite, not the fast
> one.

I like this idea, or some variant of it (fastcheck?).

-- 
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] refresh materialized view concurrently

[Robert Haas]

On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane  wrote:
> Kevin Grittner  writes:
>> Robert Haas  wrote:
>>> I doubt very much that this is safe.  And even if it is safe
>>> today, I think it's a bad idea, because we're likely to try to
>>> reduce lock levels in the future.  Taking no lock on a relation
>>> we're opening, even an index, seems certain to be a bad idea.
>
> I'm with Robert on this.
>
>> What we're talking about is taking a look at the index definition
>> while the indexed table involved is covered by an ExclusiveLock.
>> Why is that more dangerous than inserting entries into an index
>> without taking a lock on that index while the indexed table is
>> covered by a RowExclusiveLock, as happens on INSERT?
>
> I don't believe that that happens.  If it does, it's a bug.  Either the
> planner or the executor should be taking a lock on each index touched
> by a query.

It seems Kevin's right.  Not sure why that doesn't break.

-- 
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] refresh materialized view concurrently

[Tom Lane]

Kevin Grittner  writes:
> Robert Haas  wrote:
>> I doubt very much that this is safe.  And even if it is safe
>> today, I think it's a bad idea, because we're likely to try to
>> reduce lock levels in the future.  Taking no lock on a relation
>> we're opening, even an index, seems certain to be a bad idea.

I'm with Robert on this.

> What we're talking about is taking a look at the index definition
> while the indexed table involved is covered by an ExclusiveLock.
> Why is that more dangerous than inserting entries into an index
> without taking a lock on that index while the indexed table is
> covered by a RowExclusiveLock, as happens on INSERT? 

I don't believe that that happens.  If it does, it's a bug.  Either the
planner or the executor should be taking a lock on each index touched
by a query.

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] dynamic background workers

[Robert Haas]

On Mon, Jun 24, 2013 at 3:51 AM, Michael Paquier
 wrote:
> 3) Why not adding an other function in worker_spi.c being the opposite
> of worker_spi_launch to stop dynamic bgworkers for a given index
> number? This would be only a wrapper of pg_terminate_backend, OK, but
> at least it would give the user all the information needed to start
> and to stop a dynamic bgworker with a single extension, here
> worker_spi.c. It can be painful to stop

Well, there's currently no mechanism for the person who starts a new
backend to know the PID of the process that actually got started.  I
plan to write a patch to address that problem, but it's not this
patch.

> 4) Not completely related to this patch, but one sanity check in
> SanityCheckBackgroundWorker:bgworker.c is not listed in the
> documentation: when requesting a database connection, bgworker needs
> to have access to shmem. It looks that this should be also fixed in
> REL9_3_STABLE.

That's fine; I think it's separate from this patch.  Please feel free
to propose something.

> 5) Why not adding some documentation? Both dynamic and static
> bgworkers share the same infrastructure, so some lines in the existing
> chapter might be fine?

I'll take a look.

> 6) Just wondering something: it looks that the current code is not
> able to identify what was the way used to start a given bgworker.
> Would it be interesting to be able to identify if a bgworker has been
> registered though RegisterBackgroundWorker or
> RegisterDynamicBackgroundWorker?

I don't think that's a good thing to expose.

-- 
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] refresh materialized view concurrently

[Kevin Grittner]

Robert Haas  wrote:
> Hitoshi Harada  wrote:
>> Other than these, I've found index is opened with NoLock,
>> relying on ExclusiveLock of parent matview, and ALTER INDEX SET
>> TABLESPACE or something similar can run concurrently, but it is
>> presumably safe.  DROP INDEX, REINDEX would be blocked by the
>> ExclusiveLock.
>
> I doubt very much that this is safe.  And even if it is safe
> today, I think it's a bad idea, because we're likely to try to
> reduce lock levels in the future.  Taking no lock on a relation
> we're opening, even an index, seems certain to be a bad idea.

What we're talking about is taking a look at the index definition
while the indexed table involved is covered by an ExclusiveLock.
Why is that more dangerous than inserting entries into an index
without taking a lock on that index while the indexed table is
covered by a RowExclusiveLock, as happens on INSERT? 
RowExclusiveLock is a much weaker lock, and we can't add entries to
an index without looking at its definition.  Should we be taking
out locks on every index for every INSERT?  If not, what makes that
safe while merely looking at the definition is too risky?

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Support for REINDEX CONCURRENTLY

[Andres Freund]

On 2013-07-03 10:03:26 +0900, Michael Paquier wrote:
> +static int
> +toast_open_indexes(Relation toastrel,
> +LOCKMODE lock,
> +Relation **toastidxs,
> +int *num_indexes)
> + /*
> +  * Free index list, not necessary as relations are opened and a valid 
> index
> +  * has been found.
> +  */
> + list_free(indexlist);

Missing "anymore" or such.

> index 9ee9ea2..23e0373 100644
> --- a/src/bin/pg_dump/pg_dump.c
> +++ b/src/bin/pg_dump/pg_dump.c
> @@ -2778,10 +2778,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>   PQExpBuffer upgrade_query = createPQExpBuffer();
>   PGresult   *upgrade_res;
>   Oid pg_class_reltoastrelid;
> - Oid pg_class_reltoastidxid;
>  
>   appendPQExpBuffer(upgrade_query,
> -   "SELECT c.reltoastrelid, 
> t.reltoastidxid "
> +   "SELECT c.reltoastrelid "
> "FROM pg_catalog.pg_class c LEFT JOIN 
> "
> "pg_catalog.pg_class t ON 
> (c.reltoastrelid = t.oid) "
> "WHERE c.oid = '%u'::pg_catalog.oid;",
> @@ -2790,7 +2789,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>   upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
>  
>   pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0, 
> PQfnumber(upgrade_res, "reltoastrelid")));
> - pg_class_reltoastidxid = atooid(PQgetvalue(upgrade_res, 0, 
> PQfnumber(upgrade_res, "reltoastidxid")));
>  
>   appendPQExpBuffer(upgrade_buffer,
>  "\n-- For binary upgrade, must preserve 
> pg_class oids\n");
> @@ -2803,6 +2801,10 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>   /* only tables have toast tables, not indexes */
>   if (OidIsValid(pg_class_reltoastrelid))
>   {
> + PQExpBuffer index_query = createPQExpBuffer();
> + PGresult   *index_res;
> + Oid indexrelid;
> +
>   /*
>* One complexity is that the table definition might 
> not require
>* the creation of a TOAST table, and the TOAST table 
> might have
> @@ -2816,10 +2818,23 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
> "SELECT 
> binary_upgrade.set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n",
> 
> pg_class_reltoastrelid);
>  
> - /* every toast table has an index */
> + /* Every toast table has one valid index, so fetch it 
> first... */
> + appendPQExpBuffer(index_query,
> +   "SELECT c.indexrelid "
> +   "FROM 
> pg_catalog.pg_index c "
> +   "WHERE c.indrelid = 
> %u AND c.indisvalid;",
> +   
> pg_class_reltoastrelid);
> + index_res = ExecuteSqlQueryForSingleRow(fout, 
> index_query->data);
> + indexrelid = atooid(PQgetvalue(index_res, 0,
> +
> PQfnumber(index_res, "indexrelid")));
> +
> + /* Then set it */
>   appendPQExpBuffer(upgrade_buffer,
> "SELECT 
> binary_upgrade.set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n",
> -   
> pg_class_reltoastidxid);
> +   indexrelid);
> +
> + PQclear(index_res);
> + destroyPQExpBuffer(index_query);

Wouldn't it make more sense to fetch the toast index oid in the query
ontop instead of making a query for every relation?

Looking good!

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: Patch to compute Max LSN of Data Pages

[Robert Haas]

On Wed, Jul 3, 2013 at 9:51 AM, Amit Kapila  wrote:
> amit@linux:~> md test
> amit@linux:~> cd test
> amit@linux:~/test> ln -s ~/test link_test
> amit@linux:~/test> ls
> link_test
> amit@linux:~/test> cd link_test
> amit@linux:~/test/link_test> ls
> link_test
> amit@linux:~/test/link_test> cd link_test
> amit@linux:~/test/link_test/link_test> cd link_test
> amit@linux:~/test/link_test/link_test/link_test> pwd
> /home/amit/test/link_test/link_test/link_test
> amit@linux:~/test/link_test/link_test/link_test>

So what?

-- 
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: Patch to compute Max LSN of Data Pages

[Amit Kapila]

> -Original Message-
> From: Robert Haas [mailto:robertmh...@gmail.com]
> Sent: Wednesday, July 03, 2013 6:40 PM
> To: Amit Kapila
> Cc: Andres Freund; Josh Berkus; pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Review: Patch to compute Max LSN of Data Pages
> 
> On Wed, Jul 3, 2013 at 8:44 AM, Amit Kapila 
> wrote:
> >> Why does this patch need all of this fancy path-handling logic -
> >> specifically, remove_parent_refernces() and make_absolute_path()?
> >> Surely its needs are not that different from pg_ctl or pg_resetxlog
> or
> >> pg_controldata.  If they all need it and it's duplicated, we should
> >> fix that.  Otherwise, why the special logic here?
> >>
> >> I don't think we need getLinkPath() either.  The server has no
> trouble
> >> finding its files by just using a pathname that follows the symlink.
> >> Why do we need anything different here?  The whole point of symlinks
> >> is that you can traverse them transparently, without knowing where
> >> they point.
> >
> > It is to handle negative scenario's like if there is any recursion in
> path.
> > However if you feel this is not important, it can be removed.
> 
> I'm having a hard time imagining a situation where that would be a
> problem.  If the symlink points to itself somehow, the OS will throw
> an error.  If your filesystem is so badly hosed that the directory
> structure is more fundamentally broken than the OS's circular-symlink
> detection code can handle, whether or not this utility works is a
> second-order consideration.  What kind of scenario are you imagining?


amit@linux:~> md test 
amit@linux:~> cd test 
amit@linux:~/test> ln -s ~/test link_test 
amit@linux:~/test> ls 
link_test 
amit@linux:~/test> cd link_test 
amit@linux:~/test/link_test> ls 
link_test 
amit@linux:~/test/link_test> cd link_test 
amit@linux:~/test/link_test/link_test> cd link_test 
amit@linux:~/test/link_test/link_test/link_test> pwd 
/home/amit/test/link_test/link_test/link_test 
amit@linux:~/test/link_test/link_test/link_test>

Platform details

Suse - 11.2
Kernel - 3.0.13

This is to avoid when user has given some path where db files are present.

> >> I think it would be a good idea to have a mode that prints out the
> max
> >> LSN found in *each data file* scanned, and then prints out the
> overall
> >> max found at the end.  In fact, I think that should perhaps be the
> >> default mode, with -q, --quiet to disable it.
> >
> > Printing too many LSN for each file might fill user's screen and he
> might be
> > needing only overall LSN.
> > Should we keep -p --printall as option to print all LSN's and keep
> default
> > as overall max LSN?
> 
> Honestly, I have a hard time imagining the use case for that mode.
> This isn't a tool that people should be running regularly, and some
> output that lends a bit of confidence that the tool is doing the right
> thing seems like a good thing.  Keep in mind it's likely to run for
> quite a while, too, and this would provide a progress indicator.  I'll
> defer to whatever the consensus is here but my gut feeling is that if
> you don't want that extra output, there's a good chance you're
> misusing the tool.

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] [9.4 CF 1] The Commitfest Slacker List

[Robert Haas]

On Wed, Jul 3, 2013 at 9:21 AM, Michael Meskes  wrote:
> On Tue, Jul 02, 2013 at 04:00:22PM -0400, Robert Haas wrote:
>> make you review patches against your will.  Don't take it for more
>> than what Josh meant it as.
>
> And that was what?

An attempt to prod a few more people into helping review.

I can see that this pissed you off, and I'm sorry about that.  But I
don't think that was his intent.

-- 
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] Improvement of checkpoint IO scheduler for stable transaction responses

[Andres Freund]

On 2013-07-03 17:18:29 +0900, KONDO Mitsumasa wrote:
> Hi,
> 
> I tested and changed segsize=0.25GB which is max partitioned table file size 
> and
> default setting is 1GB in configure option (./configure --with-segsize=0.25).
> Because I thought that small segsize is good for fsync phase and background 
> disk
> write in OS in checkpoint. I got significant improvements in DBT-2 result!
> 
> * Performance result in DBT-2 (WH340)
>   | NOTPM90%tileAverage  Maximum
>  -+---
>  original_0.7 (baseline)  | 3474.62  18.348328  5.73936.977713
>  fsync + write| 3586.85  14.459486  4.96027.266958
>  fsync + write + segsize=0.25 | 3661.17  8.288164.11717.23191
> 
> Changing segsize with my checkpoint patches improved original over 50% at 
> 90%tile
> and maximum response time.

Hm. I wonder how much of this could be gained by doing a
sync_file_range(SYNC_FILE_RANGE_WRITE) (or similar) either while doing
the original checkpoint-pass through the buffers or when fsyncing the
files. Presumably the smaller segsize is better because we don't
completely stall the system by submitting up to 1GB of io at once. So,
if we were to do it in 32MB chunks and then do a final fsync()
afterwards we might get most of the benefits.

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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

[Amit Kapila]

On Wednesday, July 03, 2013 6:29 PM Simon Riggs wrote:
On 3 July 2013 07:45, Amit Kapila  wrote:
 
>>> postgresql.auto.conf is similar enough to postgresql.conf that it will
prevent tab completion from working as it does now. As a result it will
cause
>>> people to bring that file up for editing when we wish to avoid that.
  
>>This new file postgresql.auto.conf will be created inside a new directory
>>"config", so both will be in different directories.
>>  Will there be any confusion with tab completion for different
directories?

> Tab completion will not be confused, no.

> But I think everything else will be. 
>How will I know that some settings have been set by ALTER SYSTEM and some
by other means?

Other means by hand editing postgresql.auto.conf?
If not we can check in pg_settings, it shows sourcefile. So if the setting
is from new file postgresql.auto.conf, this means it is set by ALTER SYSTEM.


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