Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove old-style VACUUM FULL (which was known for a little while

2010-02-13 Thread Greg Smith

Robert Haas wrote:

Recording some bookkeeping information in pg_class so that pg_migrator can tell 
what's going on
at a glance seems like the right approach, but I'm fuzzy on the details.
  


November of 2008 was a pretty good month for me, so I enjoyed this 
flashback to it.  That's when the path for how to handle space 
reservation wandered to this same place and then died there:  how to 
know for sure what information to put into the catalog for the upgrade 
utility, before the upgrade utility exists.  Message from Bruce at 
http://archives.postgresql.org/message-id/200901300457.n0u4v1707...@momjian.us 
and my follow-up summarized/linked to the highlights of the earlier 
discussion on that one.


This time around, the way the upgrade is being staged allows a possible 
path through this dependency chain, as noted by Tom:



It would probably be useful to have a utility that runs *in 9.0* and
gets rid of MOVED bits, so that we could drop support for them in 9.1.
But it's not happening for 9.0.


As long as this one gets deprecated nicely here--so the server still 
knows how to deal with the ugly parts, but will not introduce any more 
of them--this should make for a good test case to gain experience with 
handling this whole class of problem.  If the above exercise finishes 
with a clear had we just recorded x in the catalog before 9.0 came 
out we could have done this more easily, I think it would be much more 
likely that a future we should record y in the catalog to improve the 
odds of adding this feature in a way that can upgrade to it in-place 
decision might get made correctly in advance of the upgrade utility 
actually existing.  Right now, just like the 8.4 case, it seems quite 
possible no one will develop a plan in time they can prove will work 
well enough to justify adding speculative catalog support for it.  Much 
easier to figure that out in retrospect though, after the matching 
utility that uses the data exists.


If you think through the implications of that far enough, eventually you 
start to realize that you really can't even add a feature that requires 
an in-place upgrade hack to fix without first having the code that 
performs said hack done.  Otherwise you're never completely sure that 
you put the right catalog pieces and related support code into the 
version you want to upgrade from.  This is why it's not unheard of for 
commercial database products to require a working in-place upgrade code 
*before* the feature change gets committed.


In this case, we get a lucky break in that it's easy to leave support 
for old path in there and punt the problem for now.  I hope that we all 
learn something useful about this class of issue during this opportunity 
to get away with that with little downside.


--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


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


Re: [HACKERS] Confusion over Python drivers {license}

2010-02-13 Thread Greg Smith

Jeff Davis wrote:

Keep in mind that backwards compatibility is not the only issue here;
forwards compatibility matters as well*. A lot of the encoding issues I
wrote up ( http://wiki.postgresql.org/wiki/Driver_development ) will
probably be real bugs in a python3 application using a driver that
doesn't understand encoding properly.
  


Sure, but the benefit of being the de-facto standard to some problem, 
because you're already supported by everybody, is that it's easier to 
attract the most help to move forward too.  The disclaimers on 
investments always read past performance is not indicative of future 
results.  I don't think that's really true in the open-source project 
case.  Generally, I'd rather trust a project that's been keeping up with 
things for a long time already to continue to do so, particularly one 
that's built up a community around it already, rather than risk 
switching to one without a known good track record in that record. 

(Exercise for the reader: consider how what I just said applies in both 
a positive and negative way toward MySQL and its associated development 
model)


--
Greg Smith2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Tim Bunce
On Fri, Feb 12, 2010 at 07:57:15PM -0500, Andrew Dunstan wrote:
 Alex Hunsaker wrote:
   Yes it could allow people who
 can set the plperl.*_init functions to muck with the safe.  As an
 admin I could also do that by setting plperl.on_init and overloading
 subs in the Safe namespace or switching out Safe.pm.
 
 It's quite easy to subvert Safe.pm today, sadly. You don't need
 on*init, nor do you need to replace the System's Safe.pm. I'm not
 going to tell people how here, but it took me all of a few minutes
 to discover and verify when I went looking a few weeks ago, once I
 thought about it.
 
 But that's quite different from us providing an undocumented way to
 expose arbitrary objects to the Safe container. In that case *we*
 become responsible for any insecure uses, and we don't even have the
 luxury of having put large warnings in the docs, because there
 aren't any docs.

I wish I understood better how PostgreSQL developers would be
responsible for a DBA using an undocumented hack. In my view the DBA is
clearly responsible in that case.

If it's not documented it's not supported.

 Another objection is that discussion on this facility has been
 pretty scant. I think that's putting it mildly. Maybe it's been
 apparent to a few people what the implications are, but I doubt it's
 been apparent to many. That makes me quite nervous, which is why I'm
 raising it now.
 
 Tim mentioned in his reply that he'd be happy to put warnings in
 plc_safe_ok.pl. But that file only exists in the source - it's
 turned into header file data as part of the build process, so
 warnings there will be seen by roughly nobody.
 
 I still think if we do this at all it needs to be documented and
 surrounded with appropriate warnings.

I'm away for a day or so (for my wedding anniversary) but I'll have an
updated patch, with a clean well-documented API, for consideration by
Monday morning.

Tim.

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Andrew Dunstan



Tim Bunce wrote:

But that's quite different from us providing an undocumented way to
expose arbitrary objects to the Safe container. In that case *we*
become responsible for any insecure uses, and we don't even have the
luxury of having put large warnings in the docs, because there
aren't any docs.



I wish I understood better how PostgreSQL developers would be
responsible for a DBA using an undocumented hack. In my view the DBA is
clearly responsible in that case.

If it's not documented it's not supported.
  


My feeling is if we provide something we are responsible for it, 
documented or not. Undocumented features with security implications 
raise big red flags in my head. Maybe the difference in perspective 
comes from working on a database as opposed to working on a language.




I still think if we do this at all it needs to be documented and
surrounded with appropriate warnings.



I'm away for a day or so (for my wedding anniversary) but I'll have an
updated patch, with a clean well-documented API, for consideration by
Monday morning.


  


Happy anniversary! But on reflection I think it's too late for this. We 
are a month into the commitfest. Alex's suggestion on how to proceed 
(change the declarations on the relevant items to make them 
inaccessible) makes sense to me.


We've always been fighting time on these PLPerl patches and we're 
moderately lucky to have got as much in as we have.


In case I haven't said it before, your work on this is very much 
appreciated.


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] Small Bug in pgstat display during recovery conflict resolution

2010-02-13 Thread Simon Riggs
Committed, thanks.

On Sun, 2010-02-07 at 21:47 +0100, Andres Freund wrote:
 Hi Simon, Hi all,
 
 
 if (!logged  (wait_s  0 || wait_us  50))
 {
 const char *oldactivitymsg;
 int len;
 
 oldactivitymsg = get_ps_display(len);
 snprintf(waitactivitymsg, sizeof(waitactivitymsg),
  waiting for max_standby_delay (%u s),
  MaxStandbyDelay);
 set_ps_display(waitactivitymsg, false);
 if (len  100)
 len = 100;
 memcpy(waitactivitymsg, oldactivitymsg, len);
 
 pgstat_report_waiting(true);
 
 logged = true;
 }
 ..
 if (logged)
 {
 set_ps_display(waitactivitymsg, false);
 pgstat_report_waiting(false);
 }
 
 That doesnt work because get_ps_display returns the internal buffer. This 
 leads to the situation that after conflict resolution the 
 waiting for max_standby_delay ...
 message is displayed until the next segment starts where its replaced
 again by the 
 ... recovering ... line.
 
 Additionally the old code may print unintialized memory if get_ps_display
  returns a string without a \0 terminator.
 
 The attached patch fixes that.
 
 Andres
-- 
 Simon Riggs   www.2ndQuadrant.com


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


Re: [HACKERS] Testing with concurrent sessions

2010-02-13 Thread Markus Wanner

Hi,

Kevin Grittner wrote:

I just got to the point of having what appears to be a working but
poorly optimized version of serializable transactions,


Cool.


so it is
critical that I create a good set of tests to confirm correct
behavior and monitor for regressions as I optimize.  I see that
you've been working on dtester recently -- should I grab what you've
got here, stick with 0.1, or do you want to package something?


I'm preparing for a 0.1 release, yes. However, I'm trying to keep up 
with the postgres-dtests branch, so that should continue to work with 
the latest dtester code.



If I
should pull from your git, any hints on the best git statements to
merge that in are welcome, I'm still rather new to git, and I tend
not to get things right on a first try without some hints.  :-/


I'd recommend merging postgres-dtests into your branch, so you'll get 
all of the adjustments from there, if I change dtester itself.


Once 0.1 is out, I'm *trying* to remain backwards compatible in future 
releases. However, that holds me from releasing 0.1 in the first place :-)



Independent of versions and releases: if you build your tests on top of 
dtester, I'm more than willing to support you with that. If you keep 
your code in a git repository, I could even provide patches, in case I 
need (or want) to change the dtester interface.


Regards

Markus Wanner

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Teodor Sigaev

However, that does make it even uglier to have category shoehorned in as
part of a different field.  Back to wanting 5-key syscaches ...

Sigh.


I see your point. May be it's better to introduce new system table? pg_amorderop 
to store ordering operations for index.


--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/

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


Re: [HACKERS] Streaming Replication docs

2010-02-13 Thread Greg Stark
On Fri, Feb 12, 2010 at 9:14 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 One glaring issue with the current documentation layout is that the
 documentation for the various options in recovery.conf is split in two
 places.

I've been trying to explain to someone how to set this all up and they
keep asking me why there's a separate recovery.conf file. I thought it
made sense back when it was to control PITR and was a transient file
which only applied as long as it was recovering but it seems now to
have become part of the normal setup of the server.

It seems like it would be much easier to keep things straight if you
could set up the master and all the slaves identically and they would
know when they had to store or retrieve files from the WAL archive and
when they had to make or listen for streaming connections based on
their master/slave status -- which is the only bit which needs to be
stored outside the configuration so it can be changed dynamically.

The master/slave status would be indicated by a trigger file or some
external master election daemon connecting and issuing a query calling
a magic sql function.

But having the separate recovery.conf seems kind of pointless legacy
structure at this point.
-- 
greg

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Tom Lane
Teodor Sigaev teo...@sigaev.ru writes:
 I see your point. May be it's better to introduce new system table? 
 pg_amorderop 
 to store ordering operations for index.

We could, but that approach doesn't scale to wanting more categories
in the future --- you're essentially decreeing that every new category
of opclass-associated operator will require a new system catalog,
along with all the infrastructure needed for that.  That guarantees
that the temptation to take shortcuts will remain high.

If we didn't already have the plus/minus-for-WINDOW-RANGE example
staring us in the face, I might think that an extensible solution
wasn't needed here ... but we do so I think we really need to allow
for multiple categories in some form.

Now on the flip side, adding new catalogs would allow flexibility to
add columns that aren't there in pg_amop, which could come in handy
if some future category requires auxiliary data that's not needed for
the existing category of index search operators.  But since the two
examples we have at hand don't appear to need any extra data, this
argument isn't real strong.

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] knngist patch support

2010-02-13 Thread Joshua Tolley
On Sat, Feb 13, 2010 at 01:31:44PM -0500, Robert Haas wrote:
 On Sat, Feb 13, 2010 at 1:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Teodor Sigaev teo...@sigaev.ru writes:
  I see your point. May be it's better to introduce new system table? 
  pg_amorderop
  to store ordering operations for index.
 
  We could, but that approach doesn't scale to wanting more categories
  in the future --- you're essentially decreeing that every new category
  of opclass-associated operator will require a new system catalog,
  along with all the infrastructure needed for that.  That guarantees
  that the temptation to take shortcuts will remain high.
 
 Yeah.  PFA a patch to allow 5-key syscaches.

(Realizing I'm a lurker in this conversation, and hoping not to ask irritating
questions) Do we need to rename SearchSysCache et al. to SearchSysCache1,
etc.? It seems to me that requires changes to all kinds of software without
any real need. The four lines of PL/LOLCODE that inspired this thought aren't
themselves a great burden, but when combined with everyone else using
SearchSysCache already...

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] knngist patch support

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 2:03 PM, Joshua Tolley eggyk...@gmail.com wrote:
 On Sat, Feb 13, 2010 at 01:31:44PM -0500, Robert Haas wrote:
 On Sat, Feb 13, 2010 at 1:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Teodor Sigaev teo...@sigaev.ru writes:
  I see your point. May be it's better to introduce new system table? 
  pg_amorderop
  to store ordering operations for index.
 
  We could, but that approach doesn't scale to wanting more categories
  in the future --- you're essentially decreeing that every new category
  of opclass-associated operator will require a new system catalog,
  along with all the infrastructure needed for that.  That guarantees
  that the temptation to take shortcuts will remain high.

 Yeah.  PFA a patch to allow 5-key syscaches.

 (Realizing I'm a lurker in this conversation, and hoping not to ask irritating
 questions) Do we need to rename SearchSysCache et al. to SearchSysCache1,
 etc.? It seems to me that requires changes to all kinds of software without
 any real need. The four lines of PL/LOLCODE that inspired this thought aren't
 themselves a great burden, but when combined with everyone else using
 SearchSysCache already...

If we want to allow 5-key syscaches, we have to add an extra parameter
to SearchSysCache and friends.  So everyone caller of SearchSysCache
is going to break.  (Well, unless we instead leave SearchSysCache
alone and add SearchSysCacheExtended or similar; but that's not really
project style.)

The point of the macros is that if someone some day decides they want
to allow SIX-key syscaches, we don't have to break all those people
again; we can just update the macro definitions.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Hitoshi Harada
2010/2/14 Tom Lane t...@sss.pgh.pa.us:
 Teodor Sigaev teo...@sigaev.ru writes:
 I see your point. May be it's better to introduce new system table? 
 pg_amorderop
 to store ordering operations for index.

 We could, but that approach doesn't scale to wanting more categories
 in the future --- you're essentially decreeing that every new category
 of opclass-associated operator will require a new system catalog,
 along with all the infrastructure needed for that.  That guarantees
 that the temptation to take shortcuts will remain high.

 If we didn't already have the plus/minus-for-WINDOW-RANGE example
 staring us in the face, I might think that an extensible solution
 wasn't needed here ... but we do so I think we really need to allow
 for multiple categories in some form.

I'm not so familiar with knn gist topic, but I think there are not
enough fundamentals yet. Because we started from defining operators if
they can be ordered or indexed, general operator semantics is over
them, not adding columns to pg_amop. It will be something like
datatype-independent human word: add and subtract, take distance,
ordered.
And we don't have time to invent such new world.

Regards.



-- 
Hitoshi Harada

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Hitoshi Harada
2010/2/14 Robert Haas robertmh...@gmail.com:
 If we want to allow 5-key syscaches, we have to add an extra parameter
 to SearchSysCache and friends.  So everyone caller of SearchSysCache
 is going to break.  (Well, unless we instead leave SearchSysCache
 alone and add SearchSysCacheExtended or similar; but that's not really
 project style.)

DirectFunctionCall1/2/3...?

-- 
Hitoshi Harada

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Tom Lane
Hitoshi Harada umi.tan...@gmail.com writes:
 And we don't have time to invent such new world.

Huh?  This is all discussion for 9.1 (or even later).  There's
plenty of time.

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] knngist patch support

2010-02-13 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sat, Feb 13, 2010 at 2:03 PM, Joshua Tolley eggyk...@gmail.com wrote:
 (Realizing I'm a lurker in this conversation, and hoping not to ask 
 irritating
 questions) Do we need to rename SearchSysCache et al. to SearchSysCache1,
 etc.? It seems to me that requires changes to all kinds of software without
 any real need. The four lines of PL/LOLCODE that inspired this thought aren't
 themselves a great burden, but when combined with everyone else using
 SearchSysCache already...

 If we want to allow 5-key syscaches, we have to add an extra parameter
 to SearchSysCache and friends.  So everyone caller of SearchSysCache
 is going to break.  (Well, unless we instead leave SearchSysCache
 alone and add SearchSysCacheExtended or similar; but that's not really
 project style.)

It's fair to stop and think about how this would affect external
packages and what they'd have to do to support building against both new
and old-style backends.

Reflecting on it, it seems to me that the separate SearchSysCacheN()
macros are obviously cleaner and closer to preferred project style than
the existing code with all those explicit zeroes.  So I think there's
a case for migrating to that style even if we didn't have a concern
about the max number of lookup keys.

What would probably be the recommended solution for backwards-compatible
source code is to convert the actual calls to new style, and then
provide a block of macro definitions along the lines of

#if CATALOG_VERSION_NO  something
#define SearchSysCache1(...) SearchSysCache(..., 0, 0, 0)
#define SearchSysCache2(...) SearchSysCache(..., 0, 0)
etc

So that seems okay so far.

What's not clear from this is whether we should reserve SearchSysCache
as an equivalent to SearchSysCache4() and therefore name the underlying
function something different.  That would allow being lazy about
converting individual calls over.

I'm inclined to think not, and here's the reason: with the old call
style it was never apparent from the callee side how many arguments the
caller intended to be valid, and so for example we couldn't have an
Assert that checked it was right.  I'm not sure if we should go so
far as to add an explicit nkeys argument to SearchSysCache, but it's
worth thinking about at least.

regards, tom lane

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


[HACKERS] Documentation build issues on Debian/Ubuntu

2010-02-13 Thread Greg Smith
I can't seem to build the PDF version of the documentation on any of my 
Ubuntu 9.04 systems, and wonder if there's anything that can/should 
should get done about it.


The problem happens like this:

gsm...@gsmith-desktop:~/pgwork/doc/src/sgml$ make postgres-US.pdf
openjade  -D . -D . -c 
/usr/share/sgml/docbook/stylesheet/dsssl/modular/catalog -d 
./stylesheet.dsl -t tex -V tex-backend -i output-print -i include-index 
-V texpdf-output -V '%paper-type%'=USletter -o postgres-US.tex-pdf 
postgres.sgml
openjade:./stylesheet.dsl:621:2:E: flow object not accepted by port; 
only display flow objects accepted

make: *** [postgres-US.tex-pdf] Segmentation fault
make: *** Deleting file `postgres-US.tex-pdf'

These flow object not accepted by port errors showing up and then 
causing a crash go back a long ways:  
http://archives.postgresql.org/pgsql-docs/2003-12/msg00024.php


There used to be a warning in the docs about not building with openjade 
1.4, which is certainly what I've got here:


$ openjade --version
openjade:I: OpenJade version 1.4devel
openjade:I: OpenSP version 1.5.2

The problems here don't seem limited to this project; I see a similar 
report of the same style of crash against some Linux kernel docbooks at 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=521148


That wanders toward suggesting it's a 32/64 bit issue, but that's not 
true here--I get the same crash on both architectures.


Any suggestions?

--
Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com   www.2ndQuadrant.us


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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 3:02 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 What would probably be the recommended solution for backwards-compatible
 source code is to convert the actual calls to new style, and then
 provide a block of macro definitions along the lines of

 #if CATALOG_VERSION_NO  something
 #define SearchSysCache1(...) SearchSysCache(..., 0, 0, 0)
 #define SearchSysCache2(...) SearchSysCache(..., 0, 0)
 etc

 So that seems okay so far.

Where would we put this block of code?

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sat, Feb 13, 2010 at 3:02 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 What would probably be the recommended solution for backwards-compatible
 source code is to convert the actual calls to new style, and then
 provide a block of macro definitions along the lines of
 
 #if CATALOG_VERSION_NO  something
 #define SearchSysCache1(...) SearchSysCache(..., 0, 0, 0)
 #define SearchSysCache2(...) SearchSysCache(..., 0, 0)
 etc
 
 So that seems okay so far.

 Where would we put this block of code?

*We* wouldn't.  This is a solution that might be used by pg_foundry
projects for instance.  Some people want to distribute loadable modules
for which the same source code can be compiled against multiple PG
releases.  They'd stick those macro definitions, conditionalized as
above, into their own source file.

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] knngist patch support

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 2:38 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hitoshi Harada umi.tan...@gmail.com writes:
 And we don't have time to invent such new world.

 Huh?  This is all discussion for 9.1 (or even later).  There's
 plenty of time.

Just to be clear, I was intending this patch, at least, to be applied
now.  I actually think there's a good argument that we should do at
least this much for 9.0, namely that now is probably the time when
there are the fewest outstanding patches that will be broken by this.
If we try to apply this for the first 9.1 CommitFest, then (1) it'll
have to be completely redone and (2) it'll force massive rebasing.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Just to be clear, I was intending this patch, at least, to be applied
 now.  I actually think there's a good argument that we should do at
 least this much for 9.0, namely that now is probably the time when
 there are the fewest outstanding patches that will be broken by this.
 If we try to apply this for the first 9.1 CommitFest, then (1) it'll
 have to be completely redone and (2) it'll force massive rebasing.

What I think we should do is not change SearchSysCache for 9.0,
but provide the SearchSysCacheN macros as syntactic sugar, and
go around and change (at least most of) the call sites to use the
macros.  This is clearly cleaner source code, and with that method
we'll still be ABI-compatible in 9.0 for anybody who doesn't fix their
source right away.

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] knngist patch support

2010-02-13 Thread Teodor Sigaev

Reflecting on it, it seems to me that the separate SearchSysCacheN()
macros are obviously cleaner and closer to preferred project style than
the existing code with all those explicit zeroes.  So I think there's
a case for migrating to that style even if we didn't have a concern
about the max number of lookup keys.


http://www.sigaev.ru/misc/syscache-0.1.gz

Patch enlarges number of arguments of SearchSysCache and friends to 5 and 
introduces SearchSysCacheN (and friends) macroses. All 
SearchSysCache/SearchSysCacheCopy/SearchSysCacheExists/SearchSysCacheList/GetSysCacheOid 
calls are converted into calling macroses.


For compatibility, we could rename SearchSysCache into SearchSysCacheSomething 
and provide SearchSysCache function for old code.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Dimitri Fontaine
Tom Lane t...@sss.pgh.pa.us writes:
 Teodor Sigaev teo...@sigaev.ru writes:
 I see your point. May be it's better to introduce new system table? 
 pg_amorderop 
 to store ordering operations for index.

 We could, but that approach doesn't scale to wanting more categories
 in the future --- you're essentially decreeing that every new category
 of opclass-associated operator will require a new system catalog,
 along with all the infrastructure needed for that.  That guarantees
 that the temptation to take shortcuts will remain high.

On the other hand here's how the fine manual define an operator class:

  Operator classes are so called because one thing they specify is the
  set of WHERE-clause operators that can be used with an index (i.e.,
  can be converted into an index-scan qualification). An operator class
  can also specify some support procedures that are needed by the
  internal operations of the index method, but do not directly
  correspond to any WHERE-clause operator that can be used with the
  index.

 If we didn't already have the plus/minus-for-WINDOW-RANGE example
 staring us in the face, I might think that an extensible solution
 wasn't needed here ... but we do so I think we really need to allow
 for multiple categories in some form.

Agreed.

And we're talking about the basic operators + and -, and about a
distance or metric operator. Those remind me of groups and etc.

  http://en.wikipedia.org/wiki/Group_(mathematics)
  http://en.wikipedia.org/wiki/Abelian_group
  http://en.wikipedia.org/wiki/Ring_(mathematics)
  http://en.wikipedia.org/wiki/Metric_space

A group is defined by a data type, an operator (+), and an identity
element. If the group is abelian, the given operation is associative and
commutative, and each element of the data type has an inverse.

Then there's the metric space which is a data type with a distance
function. This function must be non-negative, commutative, etc.

So I guess what we need here is a Operator Group to define our plus and
minus operators, and the fact that it's a group says (by convention,
like the total ordering of a BTree) that the + is commutative and the -
its opposite. Or we have an option called abelian for specifying the
commutativity?

Then I'm for tricking the maths to say that our notion of a metric space
will apply only to our notion of an Operator Group, unless someone
really insists on separating the concepts. So an Operator Group defines
2 strategies that you have to attach to your + and - operators, and a
support function which is the distance, and optional.

Now, we want to have Groups able to work on more than one datatype, for
example talking about the distance of a point to a circle or a box. So
we need an Operator Group Family, don't we?

How much does all that help in this case?
-- 
dim

PS: I'm sad to have this discussion after having read it's too late for
9.0. The KNN-Gist stuff with extended ORDER BY indexing was too good 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


[HACKERS] idle in txn query cancellation

2010-02-13 Thread Andres Freund
Hi all,

I know it is late in the cycle, but I still think that the current behaviour 
of ERRORing during execution of a query but FATALing during IDLE IN 
TRANSACTION is very confusing to the user. Especially as you are not even able 
to read the reason for getting disconnected because the client doesnt expect 
input in that state...

The first patch adds the capability to add a flag to ereport like:
ereport(ERROR | LOG_NO_CLIENT)
Tom earlier suggested using COMERROR but thats just a version of LOG which 
doesnt report to the client. The patch makes that to be a synonym of LOG | 
LOG_NO_CLIENT.
While its not the most pretty API I dont think its that bad because the 
directionality is somewhat a property of the loglevel. Beside it would 
generate a lot of useless noise and breakage.

The second patch changes the FATAL during cancelling an idle in txn query into  
ERROR | LOG_NO_CLIENT.
To avoid breaking the known state there also may no ready for query message 
get sent. The patch ensures that by setting and checking a 
silent_error_while_idle variable.

That way the client will not see that an error occured until the next command 
sent but I dont think there is a solution to that in 9.0 timeframe if at all.

The patch only adds that for the recovery conflict path for now.

What do you think? Is it worth applying something like that now? If yes I 
would try to test the patch some more (obviously the patch survives the 
regression tests, but they do not seem to check the extended query protocol at 
all).

One could argue that the LOG_NO_CLIENT flag should be added when a idle 
transaction gets terminated by force but I wouldn't bother.

On a related note I would also like to get rid of the restriction that a 
normal query cancellation will only be done if no subtransactions are stacked. 
But I guess its too late for that? (I have a patch ready, some cleanup would 
be needed)
The latter works by:
- adding a explicit error code (which should be done regardless of this 
discussion)
- avoiding to catch such error at a few places (plperl, plpython)
- recursively aborting the subtransactions once the mainloop is reached
- relying on the fact that the cancellation signal will get resent
- possibly escalating to a FATAL if nothing happens after a certain number of 
tries

Andres


PS: I know I sort-of-promised a patch earlier, but it didn't work out time-
wise.
From 2171feda1a91b0e77c1c0788abb3fc3183503b79 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sat, 13 Feb 2010 22:30:41 +0100
Subject: [PATCH 2/2] Dont FATAL a IDLE IN TRANSACTIOn backend during conflict
 resolution. Instead avoid sending the error (and a later read for
 query) to the client to avoid confusing the client state.

---
 src/backend/tcop/postgres.c |   41 -
 1 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 13734f0..e9c622e 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*** static int	UseNewLine = 0;		/* Use EOF a
*** 176,181 
--- 176,187 
  static bool RecoveryConflictPending = false;
  static ProcSignalReason	RecoveryConflictReason;
  
+ /*
+  * Are we disallowed from sending a ready for query message right
+  * now because it would confuse the frontend?
+  */
+ bool silent_error_while_idle = false;
+ 
  /* 
   *		decls for routines only used in this file
   * 
*** ProcessInterrupts(void)
*** 2917,2934 
  			RecoveryConflictPending = false;
  			DisableNotifyInterrupt();
  			DisableCatchupInterrupt();
! 			if (DoingCommandRead)
! ereport(FATAL,
! 		(errcode(ERRCODE_ADMIN_SHUTDOWN),
! 		 errmsg(terminating connection due to conflict with recovery),
! 		 errdetail_recovery_conflict(),
! 		 errhint(In a moment you should be able to reconnect to the
!   database and repeat your command.)));
! 			else
  ereport(ERROR,
  		(errcode(ERRCODE_QUERY_CANCELED),
  		 errmsg(canceling statement due to conflict with recovery),
  		 errdetail_recovery_conflict()));
  		}
  
  		/*
--- 2923,2948 
  			RecoveryConflictPending = false;
  			DisableNotifyInterrupt();
  			DisableCatchupInterrupt();
! 			if (DoingCommandRead){
! /*
!  * We cant issue a normal ERROR here because the
!  * client doesnt expect the server to send an error at
!  * that point.
!  * We also may not send a ready for query/Z message
!  * because that would be unexpected as well.
!  */
! silent_error_while_idle = true;
! ereport(ERROR | LOG_NO_CLIENT,
! 		(errcode(ERRCODE_QUERY_CANCELED),
! 		 errmsg(canceling statement due to conflict with recovery),
! 		 errdetail_recovery_conflict()));
! 			}
! 			else{
  ereport(ERROR,
  		(errcode(ERRCODE_QUERY_CANCELED),

Re: [HACKERS] PostgreSQL::PLPerl::Call - Simple interface for calling SQL functions from PostgreSQL PL/Perl

2010-02-13 Thread David E. Wheeler
On Feb 12, 2010, at 3:10 PM, Tim Bunce wrote:

 I've appended the POD documentation and attached the (rough but working)
 test script.
 
 I plan to release the module to CPAN in the next week or so.
 
 I'd greatly appreciate any feedback.

I like the idea overall, and anything that can simplify the interface is more 
than welcome. However:

* I'd rather not have to specify a signature for a non-polymorphic function.
* I'd like to be able to use Perl code to call the functions as discussed
  previously, something like:

  my $count_sql = SP-tl_activity_stats_sql(
  [ statistic = $stat, person_id = $pid ],
  $debug
  );

  For a Polymorphic function, perhaps it could be something like:

  my $count = SP-call(
  tl_activity_stats_sql = [qw(text[] int)],
  [ statistic = $stat, person_id = $pid ],
  $debug
  );

  The advantage here is that I'm not writing functions inside strings, and only 
provide the signature when I need to disambiguate between polymorphic variants.

Anyway, That's just interface arguing. The overall idea is sound and very much 
appreciated.

Best,

David


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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread David E. Wheeler
On Feb 13, 2010, at 6:32 AM, Andrew Dunstan wrote:

 My feeling is if we provide something we are responsible for it, documented 
 or not. Undocumented features with security implications raise big red flags 
 in my head. Maybe the difference in perspective comes from working on a 
 database as opposed to working on a language.

I'm confused. Doesn't on_plperl_init already give us this? Isn't any of the 
stuff loaded by that GUC then available from inside the PLPerl Safe compartment?

Best,

David


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


[HACKERS] 0 0 return tag

2010-02-13 Thread Bruce Momjian
I have updated the attached C comment about why we return 0 0 for some
tags.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/tcop/pquery.c
===
RCS file: /cvsroot/pgsql/src/backend/tcop/pquery.c,v
retrieving revision 1.134
diff -c -c -r1.134 pquery.c
*** src/backend/tcop/pquery.c	2 Jan 2010 16:57:52 -	1.134
--- src/backend/tcop/pquery.c	13 Feb 2010 22:45:08 -
***
*** 1318,1326 
  	 * If a command completion tag was supplied, use it.  Otherwise use the
  	 * portal's commandTag as the default completion tag.
  	 *
! 	 * Exception: clients will expect INSERT/UPDATE/DELETE tags to have
! 	 * counts, so fake something up if necessary.  (This could happen if the
! 	 * original query was replaced by a DO INSTEAD rule.)
  	 */
  	if (completionTag  completionTag[0] == '\0')
  	{
--- 1318,1331 
  	 * If a command completion tag was supplied, use it.  Otherwise use the
  	 * portal's commandTag as the default completion tag.
  	 *
! 	 * Exception: Clients expect INSERT/UPDATE/DELETE tags to have
! 	 * counts, so fake them with zeros.  This can happen with DO INSTEAD
! 	 * rules if there is no replacement query of the same type as the
! 	 * original.  We print 0 0 here because technically there is no
! 	 * query of the matching tag type, and printing a non-zero count for
! 	 * a different query type seems wrong, e.g.  an INSERT that does
! 	 * an UPDATE instead should not print 0 1 if one row
! 	 * was updated.  See QueryRewrite(), step 3, for details.
  	 */
  	if (completionTag  completionTag[0] == '\0')
  	{

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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Andrew Dunstan



David E. Wheeler wrote:

On Feb 13, 2010, at 6:32 AM, Andrew Dunstan wrote:

  

My feeling is if we provide something we are responsible for it, documented or 
not. Undocumented features with security implications raise big red flags in my 
head. Maybe the difference in perspective comes from working on a database as 
opposed to working on a language.



I'm confused. Doesn't on_plperl_init already give us this? Isn't any of the 
stuff loaded by that GUC then available from inside the PLPerl Safe compartment?


  


No (and if it does it's a bug). Try it and see.

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] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread David E. Wheeler
On Feb 13, 2010, at 2:46 PM, Andrew Dunstan wrote:

 I'm confused. Doesn't on_plperl_init already give us this? Isn't any of the 
 stuff loaded by that GUC then available from inside the PLPerl Safe 
 compartment? 
 
 No (and if it does it's a bug). Try it and see.

Then what's the point of it?

David


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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Andrew Dunstan



David E. Wheeler wrote:

On Feb 13, 2010, at 2:46 PM, Andrew Dunstan wrote:

  
I'm confused. Doesn't on_plperl_init already give us this? Isn't any of the stuff loaded by that GUC then available from inside the PLPerl Safe compartment? 
  

No (and if it does it's a bug). Try it and see.



Then what's the point of it?


  


To perform initialisation, such as setting a value in %_SHARED.

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] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread David E. Wheeler
On Feb 13, 2010, at 3:35 PM, Andrew Dunstan wrote:

 To perform initialisation, such as setting a value in %_SHARED.

Hrm. Well, as a DBA, I'd *really* like to be able to make some things available 
from within a Safe container, such as Devel::NYTProf::PgPLPerl or the 
PostgreSQL::PLPerl::Call module that Tim's working on. Right now I can do that 
by hacking warnings.pm directly or by the method you  figured out a few weeks 
ago, which isn't really all that nasty.

I'm not sure that Tim's interface is the best approach to giving DBAs the 
ability to do this from within PostgreSQL, but I'm hard-pressed to come up with 
a better interface. But I do think it should be allowed.

Best,

David


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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Jeff Davis
On Sat, 2010-02-13 at 13:28 -0500, Tom Lane wrote:
 If we didn't already have the plus/minus-for-WINDOW-RANGE example
 staring us in the face, I might think that an extensible solution
 wasn't needed here ... but we do so I think we really need to allow
 for multiple categories in some form.

Is this also an opportunity to provide the infrastructure to assign
meaning to operators?

Related discussion:
http://archives.postgresql.org/pgsql-hackers/2009-10/msg01443.php

Regards,
Jeff Davis


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


Re: [HACKERS] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Tom Lane
David E. Wheeler da...@kineticode.com writes:
 I'm not sure that Tim's interface is the best approach to giving DBAs
 the ability to do this from within PostgreSQL, but I'm hard-pressed to
 come up with a better interface. But I do think it should be allowed.

I think the fact that security worries have been raised is enough reason
to go slow on this.  Let's plan to think about it some more, and put it
into 9.1 if no real problems emerge.

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] Package namespace and Safe init cleanup for plperl [PATCH]

2010-02-13 Thread Andrew Dunstan



David E. Wheeler wrote:

On Feb 13, 2010, at 3:35 PM, Andrew Dunstan wrote:

  

To perform initialisation, such as setting a value in %_SHARED.



Hrm. Well, as a DBA, I'd *really* like to be able to make some things available 
from within a Safe container, such as Devel::NYTProf::PgPLPerl or the 
PostgreSQL::PLPerl::Call module that Tim's working on. Right now I can do that 
by hacking warnings.pm directly or by the method you  figured out a few weeks 
ago, which isn't really all that nasty.

I'm not sure that Tim's interface is the best approach to giving DBAs the 
ability to do this from within PostgreSQL, but I'm hard-pressed to come up with 
a better interface. But I do think it should be allowed.
  


I am not saying we shouldn't. But that's a new feature, and we do have a 
process to follow. I think it's long past the time when we can accept 
new features which have had almost no discussion.


And I am saying that any facility we provide for it has to be known and 
documented.


Clearly there are modules which could probably be used with reasonable 
safety. Math::BigInt and MIME::Base64 are examples that come to mind as 
likely candidates.


But I am not prepared to commit a patch providing this facility based on 
just having a variable declared using use vars rather than my, 
without a word in the docs, and with almost no discussion on -hackers, 
and that discussion starting two weeks after the start of the final 
commitfest. That's just not the way we do things round here, AIUI.


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


[HACKERS] psycopg2 license changed

2010-02-13 Thread Federico Di Gregorio
Hi *,

I just wanted all interested people know that psycopg2 2.0.14 to be
released in the next few days will be under the LGPL3 + OpenSSL
exception (example code and tests under the LGPL3 alone because they are
never linked to OpenSSL).

The Zope 2 and 3 adapters will be splitted out into their own packages
after next release (i.e., during 2.0.15 development) to facilitate
easy_install, etc., and will be available under LGPL3 or ZPL.

Changes are already available from public git repository:

git://luna.dndg.it/public/psycopg2


I hope this makes everybody happy, have fun,
federico

-- 
Federico Di Gregorio   f...@initd.org
 Everything will be OK at the end. If it's not OK, it's not the end.
  -- Unknown




signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove old-style VACUUM FULL (which was known for a little while

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 3:34 AM, Greg Smith g...@2ndquadrant.com wrote:
 Robert Haas wrote:
 Recording some bookkeeping information in pg_class so that pg_migrator can
 tell what's going on
 at a glance seems like the right approach, but I'm fuzzy on the details.

 November of 2008 was a pretty good month for me, so I enjoyed this flashback
 to it.  That's when the path for how to handle space reservation wandered to
 this same place and then died there:  how to know for sure what information
 to put into the catalog for the upgrade utility, before the upgrade utility
 exists.  Message from Bruce at
 http://archives.postgresql.org/message-id/200901300457.n0u4v1707...@momjian.us
 and my follow-up summarized/linked to the highlights of the earlier
 discussion on that one.

Sure.  I think there's an a critical difference between the two
discussions: the framework I'm proposing is general and applicable to
almost any upgrade situation that changes the ODF in any way, and
provides a general way of eventually desupporting ODFs we no longer
want.  The previous discussion was about a space reservation system
which couldn't be made to work for a variety of reasons, including
uncertainty about what the future needs might be, and lack of any sort
of bookkeeping system (such as the one I'm proposing here) for
tracking the current state of the system.

 If you think through the implications of that far enough, eventually you
 start to realize that you really can't even add a feature that requires an
 in-place upgrade hack to fix without first having the code that performs
 said hack done.  Otherwise you're never completely sure that you put the
 right catalog pieces and related support code into the version you want to
 upgrade from.  This is why it's not unheard of for commercial database
 products to require a working in-place upgrade code *before* the feature
 change gets committed.

Agreed.

 In this case, we get a lucky break in that it's easy to leave support for
 old path in there and punt the problem for now.  I hope that we all learn
 something useful about this class of issue during this opportunity to get
 away with that with little downside.

Yep.

...Robert

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-13 Thread Bruce Momjian
Boszormenyi Zoltan wrote:
  Ah, I didn't even see that that section needed to be updated.  Good
  catch.  I'd suggest the following wording:
 
  For a commandSELECT/command or commandCREATE TABLE AS/command
  command, the tag is SELECT rows... [and the rest as you have it]
 
  We should probably also retitle that section from Retrieving Result
  Information for Other Commands to Retrieving Other Result
  Information and adjust the text of the opening sentence similarly.
 
  Also I think you need to update the docs for PQcmdtuples to mention
  that it not works for SELECT and CTAS.
 

  Ok, I will update libpq.sgml where this section resides.
  What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?
  
 
  Sorry, CTAS = CREATE TABLE AS SELECT.

 
 Okay, new patch is attached. Please read the docs changes, and comment.

I have reviewed this patch and made some adjustments, attached.  The
major change is that our return of 0 0 in certain cases must remain,
though I have improved the C comment explaining it with a separate CVS
commit:

/*
 * If a command completion tag was supplied, use it.  Otherwise use the
 * portal's commandTag as the default completion tag.
 *
 * Exception: Clients expect INSERT/UPDATE/DELETE tags to have
 * counts, so fake them with zeros.  This can happen with DO INSTEAD
 * rules if there is no replacement query of the same type as the
 * original.  We print 0 0 here because technically there is no
 * query of the matching tag type, and printing a non-zero count for
 * a different query type seems wrong, e.g.  an INSERT that does
 * an UPDATE instead should not print 0 1 if one row
 * was updated.  See QueryRewrite(), step 3, for details.
 */

I have removed the part of the patch that chagned 0 0;  it seems to
run fine without it.  The rest of my adjustments were minor.

One major part of the patch seems to be the collection of the
PORTAL_ONE_SELECT switch label into the label below it, which is a nice
cleanup.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/libpq.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.297
diff -c -c -r1.297 libpq.sgml
*** doc/src/sgml/libpq.sgml 5 Feb 2010 03:09:04 -   1.297
--- doc/src/sgml/libpq.sgml 14 Feb 2010 03:11:00 -
***
*** 2869,2880 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Result Information for Other Commands/title
  
 para
! These functions are used to extract information from
! structnamePGresult/structname objects that are not
! commandSELECT/ results.
 /para
  
 variablelist
--- 2869,2879 
/sect2
  
sect2 id=libpq-exec-nonselect
!titleRetrieving Other Result Information/title
  
 para
! These functions are used to extract other information from
! structnamePGresult/structname objects.
 /para
  
 variablelist
***
*** 2925,2936 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of an commandINSERT/, commandUPDATE/,
!commandDELETE/, commandMOVE/, commandFETCH/, or
!commandCOPY/ statement, or an commandEXECUTE/ of a
!prepared query that contains an commandINSERT/,
!commandUPDATE/, or commandDELETE/ statement.  If the
!command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
--- 2924,2935 
 This function returns a string containing the number of rows
 affected by the acronymSQL/ statement that generated the
 structnamePGresult/. This function can only be used following
!the execution of a commandSELECT/, commandCREATE TABLE AS/,
!commandINSERT/, commandUPDATE/, commandDELETE/,
!commandMOVE/, commandFETCH/, or commandCOPY/ statement,
!or an commandEXECUTE/ of a prepared query that contains an
!commandINSERT/, commandUPDATE/, or commandDELETE/ 
statement.
!If the command that generated the structnamePGresult/ was anything
 else, functionPQcmdTuples/ returns an empty string. The caller
 should not free the return value directly. It will be freed when
 the associated structnamePGresult/ handle is passed to
Index: doc/src/sgml/protocol.sgml
===
RCS file: 

Re: [HACKERS] knngist patch support

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 3:58 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Just to be clear, I was intending this patch, at least, to be applied
 now.  I actually think there's a good argument that we should do at
 least this much for 9.0, namely that now is probably the time when
 there are the fewest outstanding patches that will be broken by this.
 If we try to apply this for the first 9.1 CommitFest, then (1) it'll
 have to be completely redone and (2) it'll force massive rebasing.

 What I think we should do is not change SearchSysCache for 9.0,
 but provide the SearchSysCacheN macros as syntactic sugar, and
 go around and change (at least most of) the call sites to use the
 macros.  This is clearly cleaner source code, and with that method
 we'll still be ABI-compatible in 9.0 for anybody who doesn't fix their
 source right away.

Let me just think out loud for a second about the roadmap for knngist
at this point.

1. Add support for 5-key syscaches using either my patch or Teodor's
or some third version that may crop up.  This could possibly be split
into one version that converts everything to using macros (for 9.0)
and a second version that actually increases the maximum number of
keys from 4 to 5 (for 9.1).

2. Modify pg_amop by adding a new column amopcategory, probably either
int2 or maybe even just char.  Add this key to both unique indices on
pg_amop.  Modify CREATE OPERATOR CLASS (and maybe ALTER OPERATOR
FAMILY?) to support some new syntax to add order-by-op operators to
opclasses/families (I'm thinking OPERATOR ORDER strategy-number
operator vs. the usual OPRATOR strategy-number operator).  Also
add an amcanorderbyop flag to pg_am.  Teach the planner to generate an
index scan path when amcanorderbyop is set on the AM and a suitable
order-by-op clause is present.

3. Modify GIST to use the infrastructure introduced by #2 for the new
- operator.

I'm not prepared to endorse doing #3 in core for 9.0, but I wonder if
it would be feasible to think about doing #1 and #2 and putting
something into contrib for #3.  The last round of knngist patches had
a planner patch which basically did #2 (with the problems previously
discussed, to which we now seem to have a fairly clean and
straightforward solution) and then the itself and proc patches did
#3.  The planner patch is actually quite small, so maybe it's not out
of the question to think that it might go in, with some suitable
fixing up along the lines discussed here and upthread.

On the other hand, maybe I'm getting too ambitious.

...Robert

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


Re: [HACKERS] [PATCH] Provide rowcount for utility SELECTs

2010-02-13 Thread Robert Haas
On Sat, Feb 13, 2010 at 10:15 PM, Bruce Momjian br...@momjian.us wrote:
 Boszormenyi Zoltan wrote:
  Ah, I didn't even see that that section needed to be updated.  Good
  catch.  I'd suggest the following wording:
 
  For a commandSELECT/command or commandCREATE TABLE AS/command
  command, the tag is SELECT rows... [and the rest as you have it]
 
  We should probably also retitle that section from Retrieving Result
  Information for Other Commands to Retrieving Other Result
  Information and adjust the text of the opening sentence similarly.
 
  Also I think you need to update the docs for PQcmdtuples to mention
  that it not works for SELECT and CTAS.
 
 
  Ok, I will update libpq.sgml where this section resides.
  What's a CTA, btw? Do you mean CTE, a.k.a. Common Table Expression?
 
 
  Sorry, CTAS = CREATE TABLE AS SELECT.
 

 Okay, new patch is attached. Please read the docs changes, and comment.

 I have reviewed this patch and made some adjustments, attached.  The
 major change is that our return of 0 0 in certain cases must remain,
 though I have improved the C comment explaining it with a separate CVS
 commit:

    /*
     * If a command completion tag was supplied, use it.  Otherwise use the
     * portal's commandTag as the default completion tag.
     *
     * Exception: Clients expect INSERT/UPDATE/DELETE tags to have
     * counts, so fake them with zeros.  This can happen with DO INSTEAD
     * rules if there is no replacement query of the same type as the
     * original.  We print 0 0 here because technically there is no
     * query of the matching tag type, and printing a non-zero count for
     * a different query type seems wrong, e.g.  an INSERT that does
     * an UPDATE instead should not print 0 1 if one row
     * was updated.  See QueryRewrite(), step 3, for details.
     */

 I have removed the part of the patch that chagned 0 0;  it seems to
 run fine without it.  The rest of my adjustments were minor.

 One major part of the patch seems to be the collection of the
 PORTAL_ONE_SELECT switch label into the label below it, which is a nice
 cleanup.

Thanks for taking the time to review this.  I think your adjustments are good.

...Robert

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


Re: [HACKERS] knngist patch support

2010-02-13 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 ...
 2. Modify pg_amop by adding a new column amopcategory, probably either
 int2 or maybe even just char.
 ...
 I'm not prepared to endorse doing #3 in core for 9.0, but I wonder if
 it would be feasible to think about doing #1 and #2 and putting
 something into contrib for #3.

No, we are not touching the system catalogs for this in 9.0, much less
fooling with any planner logic.  If it had been submitted a few months
earlier that could have happened, but we are barely 48 hours away from
alpha freeze.  The only part of this that I think can even be considered
at this point is to do a backwards-compatible source code upgrade that
will decouple the various call sites from knowledge of exactly how many
key columns the syscaches allow.  And even that is not for the benefit
of this feature; it's mainly to minimize breakage of other patches
that will be developed between now and whenever knngist does land.
(IOW, I agree with your point that making the call syntax change now
is the least painful time to do that.)

regards, tom lane

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


[HACKERS] CommitFest Status Summary - 2010-02-14

2010-02-13 Thread Robert Haas
We're down to 5 patches remaining, and 1 day remaining, so it's time
to try to wrap things up.

* Fix large object support in pg_dump.  I think this is just waiting
for a second opinion on whether the approach is correct.  I've been
meaning to look at it, but haven't gotten enough round tuits; maybe
someone else would like to take a look?  This is an open item, so we
should really try to deal with it.
* Faster CREATE DATABASE by delaying fsync.  This is really two
patches now, one of which is apparently to be backpatched.  Greg Stark
was going to commit it, but perhaps if he isn't around someone else
should pick it up.
* Provide rowcount for utility SELECTs.  Bruce just posted an updated
version of this patch.  I think it's pretty much ready to go.
* Package namespace and Safe init cleanup for plperl.  Andrew Dunstan
is taking care of this one, I believe.
* Listen / Notify rewrite.  This is the only one of the remaining
patches that is not marked as Ready for Committer, but I think it
would be good if someone (probably Tom) at least took a look at it.
I'm not sure if it's committable at this point, but we should at least
try to provide some good feedback.

...Robert

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


Re: [HACKERS] Writeable CTEs and empty relations

2010-02-13 Thread Robert Haas
On Fri, Feb 12, 2010 at 9:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Feb 12, 2010 at 8:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 We could possibly put in some hack to disallow OLD/NEW references in
 the WITH queries, but that got past my threshold of ugliness, so
 I'm not going to commit it without further discussion.

 On the face of it it's not obvious to me why you wouldn't just do
 that.  If it's not valid to reference them there, then just don't let
 it happen (now comes the part where you tell me why it's not that
 simple).

 Well, there's no obvious-to-the-user reason why it shouldn't work.
 If we hack it, then an example like I gave will give a no such
 table error, and I'll bet long odds we'll get bug reports about it.

 (Now maybe we could suppress the bug reports if we could get it to
 print something more like NEW can't be referenced in WITH, but
 doing that seems significantly harder --- the way that I can see
 to do it would be to not have NEW/OLD in the namespace while parsing
 WITH, and that would lead to a pretty stupid error message.)

Oh, I get it.  I thought you were saying that it was semantically
nonsensical, but now I think you're saying that there's some
implementation artifact that prevents us from supporting it.
Personally, I don't use NEW and OLD in UPDATE queries, so it wouldn't
bother me to just disallow them, but (1) I might be in the minority
and (2) even if I'm not, the ugliness factor is something to consider.
 So I'm not sure what the right thing to do is, but we need to make up
our mind...

If we do decide to hold off on this, then we should probably have a
discussion of what the right way to fix the rule issues is for 9.1.
If there's no non-ugly way to do this, then we might as well go ahead
and do something ugly.  If there is a non-ugly way to fix it, we
should figure out what it is so that those who may be interested in
having this feature can see about coding it up.

...Robert

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


Re: [HACKERS] idle in txn query cancellation

2010-02-13 Thread Simon Riggs
On Sat, 2010-02-13 at 22:37 +0100, Andres Freund wrote:
 I know it is late in the cycle

No problem here. Thanks for your diligence. Will review.

-- 
 Simon Riggs   www.2ndQuadrant.com


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