Re: [HACKERS] ALTER TABLE SET STATISTICS requires AccessExclusiveLock

2010-07-19 Thread Simon Riggs
On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote:
  But it seems
 that it's far from clear what to do about it, and it's not the job of
 this patch to fix it anyway.

Agreed.

 Regarding the actual patch, it looks mostly good.  Questions:
 
 1. Why in rewriteSupport.c are we adding a call to
 heap_inplace_update() in some situations?  Doesn't seem like this is
 something we should need or want to be monkeying with.

Hmm, yes, that looks like a hangover. Will change. No others similar.

 2. Instead of AlterTableGreatestLockLevel(), how about
 AlterTableGetLockLevel()?  Yeah, it's going to be the highest lock
 level required by any subcommand, but it seems mildly overspecified.
 I don't feel strongly about this one, though, if someone has a strong
 contrary opinion...

I felt it indicated the process it's using. Happy to change.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and 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] Hostnames in pg_hba.conf

2010-07-19 Thread Peter Eisentraut
On tor, 2010-02-11 at 14:13 +0100, Bart Samwel wrote:
 I've been working on a patch to add hostname support to pg_hba.conf.
 It's not ready for public display yet, but I would just like to run a
 couple of issues / discussion points past everybody.

What's the status of this?


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


Re: [HACKERS] SHOW TABLES

2010-07-19 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


 1.  \d isn't exactly the most intuitive thing ever


Seems fairly mnemomic to me (d=describe) and it packs a 
*lot* of information into a single letter (see below). 
Things that are done often should have short keystrokes, 
and not require learning Yet Another Meta-Language.

 And it's pretty clear that we have been heading into some
 increasingly cryptic bits of fruit salad of
 \dfzb+-meta-bucky-alt-foo

No arguments there, but that's the nature of the beast. I don't 
think it's as bad as is made out, however, as \d covers 99% of 
everyday usage and certainly the show tables that started 
this thread.

 Having SHOW THIS and SHOW THAT which are a bit more readily
 guessed would be somewhat nice.

I'm not sure why easily guessed is thrown out in this thread as 
such a great thing. To achieve that goal, we simply need the 
help system that has been proposed many times: entering in 
SHOW anything gives you a quick rundown of the backslash system.

As far as SHOW THIS, there is a big difference from a plain \dt 
and \d tablename. The former could be emulated quite easily 
with a SHOW command (although even our \dt prints out more information 
than mysql's SHOW TABLES), but the latter includes a crazy amount 
of information that would lead to quite a large SHOW... statement. 
Also, if it were made a server-side thing, how would you return things 
like indexes on a table in a SRF? Have a meta-column describing what 
the other columns represent? Ugly.

 information_schema doesn't have some useful things that we'd like
 ait to have
...
 Alas, I don't see a good way to improve on this :-(

newsysviews seems the way out of that particular mess. I'm also not 
particularly opposed to adding new views or columns to 
information_schema. We would still support the standard by 
having all the required views and columns.

 The \? commands are *solely* for psql, and it would be nice to
 have the Improvement work on server side so it's not only usable
 with the one client.

Agreed, but is there some other command-line client? If it's not 
command-line, free-form SQL typing, it inevitably already has 
support for querying the catalogs built in. At least, every GUI, 
app, and driver I can think of does.

 I've seen too many QA scripts that do awk parsing of output of
 psql \d commands that are vulnerable to all kinds of awfulness.

They should be querying information_schema.

 I'd sure like to be able to write queries that *don't* involve
 array smashing or using grep on \z output to analyze object
 permissions.

Yeah, that would be a better information_schema. :)

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201007191011
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkxEXS0ACgkQvJuQZxSWSshLKwCffkfe0T3tELInxRqG7yCDS5Vr
Ku8AoLUtOu7tTplGZZLPOEuDfKHt+EEm
=Oubu
-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] SHOW TABLES

2010-07-19 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


Robert Haas (robertmh...@gmail.com) wrote:
 I think LIST COMMENTS ON SYSTEM AGGREGATES would be an epic 
 step forward in usability.

Perhaps. But it would behoove you to come up with a less er...
arcane example. I've been using Postgres a long time, and I can 
count the number of times I've needed to see comments on system 
aggregates on my hand. With at least four fingers left over.
...
 in the alphabet soup paragraph above.  I don't think there's
 anything WRONG with letting \dFp show text search dictionaries and
 \dfwS+ list system window functions with additional detail - but I'd
 like an alternative that emphasizes ease of remembering over brevity,
 works in every client, and can be extended in whatever reasonable ways
 the community decides are worth having.
...
I don't know that I'd necessarily remember all those any better, and would 
certainly not enjoy typing out:

LIST TEST SEARCH DICTIONARIES

I don't have to remember \dFp - all I have to remember is \?. For the 
more common ones that I use day to day and don't have to look up 
(\d \dt \df \l etc.) the advantage of a two or three character 
string is strong.

(There is some devil's advocate in there - a standard cross client 
(and dare I say it, cross RDBMS?) way would be nice)

...
 being powerful rings totally hollow for me.  For ordinary, day to day
 tasks like listing all my tables, or looking at the details of a
 particular table, they're great.  I use them all the time and would
 still use them even if some other syntax were available.  But there is
 no reasonable way to pass options to them, and that to me is a pretty
 major drawback.

Well, there's the rub. You're arguing this from a hacker's persepective, 
while the SHOW syntax seems to be overwhelmingly agreed upon to be either 
helpful for clueless noobs, or some nice syntactic sugar for average users.

 I'm not sure where to draw the line but implementing a proper shortcut
 interface for cammands is something taht should be done on the client side
 because not every client is the same and the needs of psql might be
 radically different from any other client (like pgadmin or a fancy Web 2.0
 AJAX thingy - those will likely always use custom catalog queries).
 Maybe a differnet way to look at the whole thing is to reconsider our own
 catalogs (anyone remember newsysview?) and add a bunch of views to abstract
 away most of the current complexity for these usecases?

Yep, agreed. Now, if we can just agree to put information_schema in the default 
search_path, because nobody enjoys having to type out information_schema...

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201007191021
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkxEYDgACgkQvJuQZxSWSsikFwCdGo88Ehdcm8OHi2+VxISTG60Y
b9sAoLsetxcpdMSconsCwj+3Xa1fCCzo
=3aM1
-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] SHOW TABLES

2010-07-19 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


Kevin Grittner wrote:

 Any solution which only works within psql isn't a solution for a
 large part of the problem space people are trying to address.  One
 important goal is that if someone spends a day to whip up a GUI
 query tool (as I did when I first started working in Java), it's
 easy to get displays like we get from the psql backslash commands
 (as it was in Sybase, which is what we were using at the time,
 through sp_help and related stored procedures).

I don't agree that this is an important goal. Certainly someone 
writing a GUI (or a new driver) should be expected to be familiar 
with the system catalogs. Moreover, a GUI relies on an underlying 
driver, and every driver should already be providing things like 
a list of tables natively.

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201007191030
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkxEYZ0ACgkQvJuQZxSWSsiIlQCfdXDgTqletVez/r+pKHY4EcW6
QAsAoPLUmblzN2aNEw5DveHEav3XyB/K
=TGq1
-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] SHOW TABLES

2010-07-19 Thread Kevin Grittner
Stephen Frost sfr...@snowman.net wrote:
 
 You think that the users of the libpq() interface (or even the
 protocol itself) are going to handle getting \dt-type output back
 somehow..?
 
If you look back in the thread, you'll see that I admitted my
ignorance of whether this could be properly implemented in the back
end without a protocol change.  Ignorance being bliss, I can revel
in the dreams of *having* such a feature without being dragged down
by the potential pain of its implementation.  ;-)
 
I know, though, that the JDBC spec supports such things -- you can
keep pulling ResultSet objects off the wire, each with its own
distinct set of columns.  (That is, each ResultSet has its own
ResultSetMetaData which specifies how many columns that particular
ResultSet has, what the column names are, what the data type is for
each column, etc.  Each ResultSet returned from a response stream
for a request can be entirely different in all of these
characteristics.)
 
 As what, a single-column result of type text?
 
No, that would be horrible.  That has been mentioned as a
possibility, and it makes me shudder.
 
 And then they'll use non-fixed-width fonts, undoubtably, which
 means the results will end up looking rather ugly, even if we put
 in the effort to format the results.
 
With, for example, Sybase's sp_help, each result set can be listed
any way the client chooses -- I've seen it put into character format
like the psql \d commands, I've seen each result set put into a
table for brower-based query tools, and I've seen each result set
put into a JTable for Java Swing applications.  If a client gets
back a series of result sets, the sky is the limit.
 
-Kevin

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


Re: [HACKERS] SHOW TABLES

2010-07-19 Thread Stephen Frost
* Kevin Grittner (kevin.gritt...@wicourts.gov) wrote:
 I know, though, that the JDBC spec supports such things -- you can
 keep pulling ResultSet objects off the wire, each with its own
 distinct set of columns.  (That is, each ResultSet has its own
 ResultSetMetaData which specifies how many columns that particular
 ResultSet has, what the column names are, what the data type is for
 each column, etc.  Each ResultSet returned from a response stream
 for a request can be entirely different in all of these
 characteristics.)

I'm pretty sure we don't have the ability to support that at either the
libpq or the protocol level today, but in general I do think it's a good
idea and would be good to support.  To be honest, I seem to recall
someone talking about working on it (or perhaps it's on the TODO?).  In
any case, long way there from here.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] SHOW TABLES

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 10:25 AM, Greg Sabino Mullane g...@turnstep.com wrote:
 in the alphabet soup paragraph above.  I don't think there's
 anything WRONG with letting \dFp show text search dictionaries and
 \dfwS+ list system window functions with additional detail - but I'd
 like an alternative that emphasizes ease of remembering over brevity,
 works in every client, and can be extended in whatever reasonable ways
 the community decides are worth having.
 ...
 I don't know that I'd necessarily remember all those any better, and would
 certainly not enjoy typing out:

 LIST TEST SEARCH DICTIONARIES

 I don't have to remember \dFp - all I have to remember is \?. For the
 more common ones that I use day to day and don't have to look up
 (\d \dt \df \l etc.) the advantage of a two or three character
 string is strong.

I don't think anyone is proposing getting rid of backslash commands.
That would be nuts.  What's being proposed is to try to create a
better way to list objects, a way that involves some server-side
support so that clients don't need to muck about with system catalogs
quite so much.

I like psql as well as anyone, but saying that it's easy to do this
stuff because you can use \? to get the appropriate backslash command
seems to me to be missing the point.  Suppose you regularly use
PGadmin to access the database, or some other graphical client, and
you want to find a query to list the comments on every item in the
database.  Good luck!  You'll need to figure out how to use psql,
discover that it has a -E switch (of which I was ignorant for my first
10 years of using PostgreSQL), and get the query out of there.  Then
you'll find that the query psql uses is more than 50 lines long and
also wrong: it omits half the object types.  Woohoo!

And even supposing that you fix the query, there's no guarantee that
it won't be wrong again when PG version X+1 comes out.  Take a look at
describe.c.  It's riddled with special cases for particular PG
versions, special cases that must be replicated in every other client
that wants to work with multiple PG versions.  So, basically, our
advice to anyone who wants a simple, portable way to list objects of
particular types in a cross-PG version compatible way is - copy the
logic in describe.c, adapt it to your application, and update it every
time a new major release comes out.  Is that really the best we can
do, and do we really think that's adequate?

 being powerful rings totally hollow for me.  For ordinary, day to day
 tasks like listing all my tables, or looking at the details of a
 particular table, they're great.  I use them all the time and would
 still use them even if some other syntax were available.  But there is
 no reasonable way to pass options to them, and that to me is a pretty
 major drawback.

 Well, there's the rub. You're arguing this from a hacker's persepective,
 while the SHOW syntax seems to be overwhelmingly agreed upon to be either
 helpful for clueless noobs, or some nice syntactic sugar for average users.

I use the darn database, too.  The machinations I've gone through to
get some of the information are ridiculously complex.  As Larry Wall
one said, a good programming language should make simple things simple
and complicated things possible; so, I don't believe that having a
simple interface and a powerful interface are mutually exclusive
goals.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] ALTER TABLE SET STATISTICS requires AccessExclusiveLock

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 2:46 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote:
  But it seems
 that it's far from clear what to do about it, and it's not the job of
 this patch to fix it anyway.

 Agreed.

 Regarding the actual patch, it looks mostly good.  Questions:

 1. Why in rewriteSupport.c are we adding a call to
 heap_inplace_update() in some situations?  Doesn't seem like this is
 something we should need or want to be monkeying with.

 Hmm, yes, that looks like a hangover. Will change. No others similar.

 2. Instead of AlterTableGreatestLockLevel(), how about
 AlterTableGetLockLevel()?  Yeah, it's going to be the highest lock
 level required by any subcommand, but it seems mildly overspecified.
 I don't feel strongly about this one, though, if someone has a strong
 contrary opinion...

 I felt it indicated the process it's using. Happy to change.

Cool.  I think with those two changes it's time to commit this.  It's
possible there's some case we've overlooked, but I think that we've
been over this fairly thoroughly, so hopefully not.  Anyway, we're
doing this at the beginning of the 9.1 cycle rather than the end, so
there's hopefully time for any lingering bugs to be found...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] leaky views, yet again

2010-07-19 Thread Robert Haas
2010/7/8 KaiGai Kohei kai...@ak.jp.nec.com:
 (2010/07/08 22:08), Robert Haas wrote:
 I think we pretty much have conceptual agreement on the shape of the
 solution to this problem: when a view is created with CREATE SECURITY
 VIEW, restrict functions and operators that might disclose tuple data
 from being pushed down into view (unless, perhaps, the user invoking
 the view has sufficient privileges to execute the underlying query
 anyhow, e.g. superuser or view owner).  What we have not resolved is
 exactly how we're going to decide which functions and operators might
 do such a dastardly thing.  I think the viable options are as follows:

 1. Adopt Heikki's proposal of treating indexable operators as non-leaky.

 http://archives.postgresql.org/pgsql-hackers/2010-06/msg00291.php

 Or, perhaps, and even more restrictively, treat only
 hashable/mergeable operators as non-leaky.

 2. Add an explicit flag to pg_proc, which can only be set by
 superusers (and which is cleared if a non-superuser modifies it in any
 way), allowing a function to be tagged as non-leaky.  I believe that
 it would be reasonable to set this flag for all of our built-in
 indexable operators (though I'd read the code before doing it), but it
 would remove the need for the assumption that third-party operators
 are equally sane.

 CREATE OR REPLACE FUNCTION doesnt_leak() RETURNS integer AS $$SELECT
 42$$ IMMUTABLE SEAWORTHY; -- doesn't leak

 This problem is not going away, so we should try to decide on something.

 I'd like to vote the second option, because this approach will be also
 applied on another aspect of leaky views.

 When leaky and non-leaky functions are chained within a WHERE clause,
 it will be ordered by the cost of functions. So, we have possibility
 that leaky functions are executed earlier than non-leaky functions.

 It will not be an easy work to mark built-in functions as either leaky
 or non-leaky, but it seems to me the most straight forward solution.

Does anyone else have an opinion on this?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] leaky views, yet again

2010-07-19 Thread Heikki Linnakangas
On 09/07/10 06:47, KaiGai Kohei wrote:
 When leaky and non-leaky functions are chained within a WHERE clause,
 it will be ordered by the cost of functions. So, we have possibility
 that leaky functions are executed earlier than non-leaky functions.

No, that needs to be forbidden as part of the fix. Leaky functions must
not be executed before all the quals from the view are evaluated.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] leaky views, yet again

2010-07-19 Thread Heikki Linnakangas

On 19/07/10 20:08, Robert Haas wrote:

2010/7/8 KaiGai Koheikai...@ak.jp.nec.com:

(2010/07/08 22:08), Robert Haas wrote:

I think we pretty much have conceptual agreement on the shape of the
solution to this problem: when a view is created with CREATE SECURITY
VIEW, restrict functions and operators that might disclose tuple data
from being pushed down into view (unless, perhaps, the user invoking
the view has sufficient privileges to execute the underlying query
anyhow, e.g. superuser or view owner).  What we have not resolved is
exactly how we're going to decide which functions and operators might
do such a dastardly thing.  I think the viable options are as follows:

1. Adopt Heikki's proposal of treating indexable operators as non-leaky.

http://archives.postgresql.org/pgsql-hackers/2010-06/msg00291.php

Or, perhaps, and even more restrictively, treat only
hashable/mergeable operators as non-leaky.

2. Add an explicit flag to pg_proc, which can only be set by
superusers (and which is cleared if a non-superuser modifies it in any
way), allowing a function to be tagged as non-leaky.  I believe that
it would be reasonable to set this flag for all of our built-in
indexable operators (though I'd read the code before doing it), but it
would remove the need for the assumption that third-party operators
are equally sane.

CREATE OR REPLACE FUNCTION doesnt_leak() RETURNS integer AS $$SELECT
42$$ IMMUTABLE SEAWORTHY; -- doesn't leak

This problem is not going away, so we should try to decide on something.


I'd like to vote the second option, because this approach will be also
applied on another aspect of leaky views.

When leaky and non-leaky functions are chained within a WHERE clause,
it will be ordered by the cost of functions. So, we have possibility
that leaky functions are executed earlier than non-leaky functions.

It will not be an easy work to mark built-in functions as either leaky
or non-leaky, but it seems to me the most straight forward solution.


Does anyone else have an opinion on this?


I have a bad feeling that marking functions explicitly as seaworthy is 
going to be hard to get right, and every time you get that wrong it's a 
security issue. On the other hand, if it's enough from a performance 
point of view to review and mark only a few built-in functions like 
index operators, maybe it's ok.


It would be easier to assess this if we had a patch to play with that 
contained all the planner changes to keep track of the seaworthiness of 
functions and apply the quals in right order. You could then try out 
different scenarios to see what the performance is like.


I guess what I'm saying is write a patch, and I'll shoot it down when 
you're done :-/. But hopefully the planner changes don't depend much on 
how we deduce which quals are leaky.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] SHOW TABLES

2010-07-19 Thread David Fetter
On Mon, Jul 19, 2010 at 02:12:19PM -, Greg Sabino Mullane wrote:
 
 -BEGIN PGP SIGNED MESSAGE-
 Hash: RIPEMD160
 
 
  1.  \d isn't exactly the most intuitive thing ever
 
 Seems fairly mnemomic to me (d=describe) and it packs a 
 *lot* of information into a single letter (see below). 
 Things that are done often should have short keystrokes, 
 and not require learning Yet Another Meta-Language.
 
  And it's pretty clear that we have been heading into some
  increasingly cryptic bits of fruit salad of
  \dfzb+-meta-bucky-alt-foo
 
 No arguments there, but that's the nature of the beast. I don't 
 think it's as bad as is made out, however, as \d covers 99% of 
 everyday usage and certainly the show tables that started 
 this thread.

It covers 0% of cases where people are not using psql.

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

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

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


Re: [HACKERS] SHOW TABLES

2010-07-19 Thread Joshua D. Drake
On Mon, 2010-07-19 at 10:23 -0700, David Fetter wrote:
 On Mon, Jul 19, 2010 at 02:12:19PM -, Greg Sabino Mullane wrote:
  
  -BEGIN PGP SIGNED MESSAGE-
  Hash: RIPEMD160
  
  
   1.  \d isn't exactly the most intuitive thing ever
  
  Seems fairly mnemomic to me (d=describe) and it packs a 
  *lot* of information into a single letter (see below). 
  Things that are done often should have short keystrokes, 
  and not require learning Yet Another Meta-Language.
  
   And it's pretty clear that we have been heading into some
   increasingly cryptic bits of fruit salad of
   \dfzb+-meta-bucky-alt-foo
  
  No arguments there, but that's the nature of the beast. I don't 
  think it's as bad as is made out, however, as \d covers 99% of 
  everyday usage and certainly the show tables that started 
  this thread.
 
 It covers 0% of cases where people are not using psql.

Which is probably 85% of our users. (No I have no actual metric)

Every single corp I have walked into is using at least PgAdmin as well.

Until this project as a whole recognizes that for the majority to the
populace the command line is dead; we will continue to have these
arguments for no apparent reason but to make sure spam filters are still
reading our emails.


JD


-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579
Consulting, Training, Support, Custom Development, Engineering


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


Re: [HACKERS] leaky views, yet again

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 1:19 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 I guess what I'm saying is write a patch, and I'll shoot it down when
 you're done :-/. But hopefully the planner changes don't depend much on how
 we deduce which quals are leaky.

Oh, great...  I love that.

/me rolls eyes

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] SHOW TABLES

2010-07-19 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160

David Fetter wrote:

 No arguments there, but that's the nature of the beast. I don't 
 think it's as bad as is made out, however, as \d covers 99% of 
 everyday usage and certainly the show tables that started 
 this thread.

 It covers 0% of cases where people are not using psql.

Yes, and everything else already has a show tables. See 
for example, PPA:

http://phppgadmin.sourceforge.net/images/4.png

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201007191342
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkxEj4kACgkQvJuQZxSWSshrwgCg65eIziE2SW8XhdTSHwVMzxnm
ynIAoLPOc0yuKyrE2kaaJFq5UiDb45Nd
=veva
-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] SHOW TABLES

2010-07-19 Thread David Fetter
On Mon, Jul 19, 2010 at 09:31:06AM -0500, Kevin Grittner wrote:
 Stephen Frost sfr...@snowman.net wrote:
  
  You think that the users of the libpq() interface (or even the
  protocol itself) are going to handle getting \dt-type output back
  somehow..?
  
 If you look back in the thread, you'll see that I admitted my
 ignorance of whether this could be properly implemented in the back
 end without a protocol change.  Ignorance being bliss, I can revel
 in the dreams of *having* such a feature without being dragged down
 by the potential pain of its implementation.  ;-)
  
 I know, though, that the JDBC spec supports such things -- you can
 keep pulling ResultSet objects off the wire, each with its own
 distinct set of columns.  (That is, each ResultSet has its own
 ResultSetMetaData which specifies how many columns that particular
 ResultSet has, what the column names are, what the data type is for
 each column, etc.  Each ResultSet returned from a response stream
 for a request can be entirely different in all of these
 characteristics.)

Would something like this do?  Thanks to Andrew Gierth for helping me
figure out how to get this working :)

CREATE OR REPLACE FUNCTION multi_result()
RETURNS SETOF REFCURSOR
LANGUAGE plpgsql
AS $$
DECLARE
r RECORD;
ref REFCURSOR;
BEGIN
FOR r IN
SELECT table_name
FROM information_schema.tables
WHERE table_schema = 'information_schema'
LOOP
ref := 'multi_result_' || quote_ident(r.table_name);
OPEN ref FOR EXECUTE 'SELECT * FROM information_schema.' || 
quote_ident(r.table_name); /* Not really needed. */
RETURN NEXT ref;
ref := NULL;
END LOOP;
RETURN;
END;
$$;

BEGIN;
SELECT * FROM multi_result();
FETCH FORWARD ALL FROM multi_result_views;
ROLLBACK;

  As what, a single-column result of type text?
  
 No, that would be horrible.  That has been mentioned as a

+1 on the shuddering.  Add also nausea. :P

  And then they'll use non-fixed-width fonts, undoubtably, which
  means the results will end up looking rather ugly, even if we put
  in the effort to format the results.
  
 With, for example, Sybase's sp_help, each result set can be listed
 any way the client chooses -- I've seen it put into character format
 like the psql \d commands, I've seen each result set put into a
 table for brower-based query tools, and I've seen each result set
 put into a JTable for Java Swing applications.  If a client gets
 back a series of result sets, the sky is the limit.

Ad astra per PostgreSQL!

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

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

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


Re: [HACKERS] Reworks of DML permission checks

2010-07-19 Thread Robert Haas
2010/7/12 KaiGai Kohei kai...@ak.jp.nec.com:
 (2010/07/10 5:53), Robert Haas wrote:
 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com:
 The attached patch tries to rework DML permission checks.

 It was mainly checked at the ExecCheckRTEPerms(), but same logic was
 implemented in COPY TO/FROM statement and RI_Initial_Check().

 This patch tries to consolidate these permission checks into a common
 function to make access control decision on DML permissions. It enables
 to eliminate the code duplication, and improve consistency of access
 controls.

 This patch is listed on the CommitFest page, but I'm not sure if it
 represents the latest work on this topic.  At a minimum, it needs to
 be rebased.

 I am not excited about moving ExecCheckRT[E]Perms to some other place
 in the code.  It seems to me that will complicate back-patching with
 no corresponding advantage.  I'd suggest we not do that.    The COPY
 and RI code can call ExecCheckRTPerms() where it is. Maybe at some
 point we will have a grand master plan for how this should all be laid
 out, but right now I'd prefer localized changes.


 OK, I rebased and revised the patch not to move ExecCheckRTPerms()
 from executor/execMain.c.
 In the attached patch, DoCopy() and RI_Initial_Check() calls that
 function to consolidate dml access control logic.

This patch contains a number of copies of the following code:

+   {
+   if (ereport_on_violation)
+   aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+
get_rel_name(relOid));
+   return false;
+   }

What if we don't pass ereport_on_violation down to
ExecCheckRTEPerms(), and just have it return a boolean?  Then
ExecCheckRTPerms() can throw the error if ereport_on_violation is
true, and return false otherwise.

With this patch, ri_triggers.c emits a compiler warning, apparently
due to a missing include.

Otherwise, the changes look pretty sensible, though I haven't tested them yet.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] TODO 9.0 done items removed

2010-07-19 Thread Alvaro Herrera
Excerpts from Bruce Momjian's message of sáb jul 17 21:21:52 -0400 2010:
 Since we branched 9.1 before we released Postgres 9.0, I had to remove
 the 9.0 TODO items before 9.0 was released, or people might have marked
 items as done when they were done only in 9.1.

Should we create a TodoDone90 page like this one?
http://wiki.postgresql.org/wiki/TodoDone84

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


Re: [HACKERS] TODO 9.0 done items removed

2010-07-19 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Bruce Momjian's message of s??b jul 17 21:21:52 -0400 2010:
  Since we branched 9.1 before we released Postgres 9.0, I had to remove
  the 9.0 TODO items before 9.0 was released, or people might have marked
  items as done when they were done only in 9.1.
 
 Should we create a TodoDone90 page like this one?
 http://wiki.postgresql.org/wiki/TodoDone84

Sure.  How is that done?  :-O

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

  + None of us is going to be here forever. +

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


[HACKERS] Patch review: make RAISE without arguments work like Oracle

2010-07-19 Thread David Fetter
I'd like to mark this as Ready for Committer :)

Review below.

Cheers,
David.

== Submission review ==

* Is the patch in context diff format?

Yes.

* Does it apply cleanly to the current CVS HEAD?

Yes, with a few offsets.

patch -p0  ../reraise_issue_PG_v1.patch 
patching file src/pl/plpgsql/src/pl_exec.c
Hunk #9 succeeded at 2427 (offset 2 lines).
Hunk #10 succeeded at 2663 (offset 2 lines).
patching file src/pl/plpgsql/src/plpgsql.h

* Does it include reasonable tests, necessary doc patches, etc?

No.  Please find new patch attached with one test and one change
to the docs.

== Usability review ==  

Read what the patch is supposed to do, and consider:

* Does the patch actually implement that? 

Yes.

* Do we want that? 

While the discussion was not long or extensive, the consensus
seemed to be yes.

* Do we already have it? 

No.

* Does it follow SQL spec, or the community-agreed behavior? 

Not applicable, as far as I understand the spec.

* Does it include pg_dump support (if applicable)?

Not applicable.

* Are there dangers? 

Old code may depend on the previous behavior.

* Have all the bases been covered?

== Feature test ==

Apply the patch, compile it and test:

* Does the feature work as advertised?

Yes.

* Are there corner cases the author has failed to consider?

Not that I've found.

* Are there any assertion failures or crashes?

No.

**Review should be done with the ''configure'' options ''--enable-cassert'' and 
''--enable-debug'' turned on; see [[Working with CVS]] for a full example.  
Those will help find issues with the code that might otherwise be missed.  A 
copy of PostgreSQL built using these parameters will be substantially slower 
than one built without them.  If you're working on something 
performance-related, such as testing whether a patch slows anything down, be 
sure to build without these flags before testing execution speed.  You can turn 
off the assert testing, the larger of the slowdowns, at server start time by 
putting ''debug_assertions = false'' in your postgresql.conf.  See 
[http://www.postgresql.org/docs/current/static/runtime-config-developer.html 
Developer Options] for more details about that setting; it defaults to true in 
builds done with --enable-cassert.

== Performance review ==

* Does the patch slow down simple tests? 

Not that I've found, although anything involving exceptions in
PL/pgsql performs pretty poorly in stock PostgreSQL.

* If it claims to improve performance, does it?

It makes no such claim.

* Does it slow down other things?

Not that I've found.

== Coding review ==

Read the changes to the code in detail and consider:

* Does it follow the project 
[http://developer.postgresql.org/pgdocs/postgres/source.html coding 
guidelines]? 

Yes.

* Are there portability issues? 

Not that I can see.

* Will it work on Windows/BSD etc? 

No reason it shouldn't.  Haven't tested those platforms.

* Are the comments sufficient and accurate?

Yes.

* Does it do what it says, correctly?

Yes, as far as I can tell.

* Does it produce compiler warnings?

No.

* Can you make it crash?

No.

== Architecture review ==

Consider the changes to the code in the context of the project as a whole:

* Is everything done in a way that fits together coherently with other 
features/modules? 

Yes.

* Are there interdependencies that can cause problems?

Not that I've found, except as noted above re: apps based on the
previous behavior.

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

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 2374,2379  SELECT merge_db(1, 'dennis');
--- 2374,2420 
  
  /para
  /example
+ example id=plpgsql-reraise-example
+ titleRe-Raising Exceptions/title
+ para
+ 
+ This example shows how to re-raise an exception, which helps you
+ write more complex exception-handling in your functions:
+ programlisting
+ CREATE OR REPLACE FUNCTION raisetest()
+ RETURNS VOID
+ LANGUAGE plpgsql
+ AS $$
+ BEGIN
+ BEGIN
+ RAISE syntax_error;
+ EXCEPTION
+ WHEN syntax_error THEN
+BEGIN
+raise notice 'exception thrown in inner block, reraising';
+RAISE;
+EXCEPTION
+WHEN OTHERS THEN
+RAISE NOTICE 'RIGHT - exception caught in innermost block';
+END;
+ END;
+ EXCEPTION
+ WHEN OTHERS THEN
+ RAISE NOTICE 'WRONG - exception caught in outer block';
+ END;
+ $$;
+ 
+ select raisetest();
+ NOTICE:  exception thrown in inner block, reraising

Re: [HACKERS] standard_conforming_strings

2010-07-19 Thread Peter Eisentraut
On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote:
 On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote:
 
  I think there are two ways we can do this, seeing that most appear to be
  in favor of doing it in the first place:  Either we just flip the
  default, make a note in the release notes, and see what happens.  Or we
  spend some time now and make, say, a list of driver versions and
  application versions that work with standard_conforming_strings = on,
  and then decide based on that, and also make that list a public resource
  for packagers etc.
 
 Do both. Turn them on, then make a list and inform driver maintainers who 
 need to update. They've got a year, after all.

Here we go then:
http://wiki.postgresql.org/wiki/Standard_conforming_strings



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


Re: [HACKERS] SHOW TABLES

2010-07-19 Thread Kevin Grittner
David Fetter da...@fetter.org wrote:
 
 Would something like this do?  Thanks to Andrew Gierth for helping
 me figure out how to get this working :)
 
 CREATE OR REPLACE FUNCTION multi_result()
 RETURNS SETOF REFCURSOR
 
With appropriate tweaks to JDBC and the other drivers, this would
cover a lot of ground.  You might be able to cover the last little
bit by returning a SETOF some record with (at least) three columns,
one of which would be filled in each row: REFCURSOR (for a result
set), INTEGER (for a row count), and something which could carry an
object which would map to a SQLWarning (which can be used with
SQLSTATE '0' to deliver informational text or '01xxx' for actual
warnings).  A JDBC execute request (as opposed to executeUpdate or
executeQuery) may get back any combination of the above in an
ordered fashion.  Essentially, this meta result set would need to be
hidden within the Statement object.
 
http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/sql/Statement.html#execute%28java.lang.String%29
 
http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/sql/Statement.html#getWarnings%28%29
 
-Kevin

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


Re: [HACKERS] Parsing of aggregate ORDER BY clauses

2010-07-19 Thread Hitoshi Harada
2010/7/19 Tom Lane t...@sss.pgh.pa.us:
 I looked into the problem reported here:
 http://archives.postgresql.org/pgsql-bugs/2010-07/msg00119.php

 1. Postpone coercion of the function inputs till after processing of
 the ORDER BY/DISTINCT decoration.  This isn't too good because then
 we'll be using the wrong data type for deciding the semantics of
 ORDER BY/DISTINCT.  That could lead to bizarre behavior or even
 crashes, eg if we try to use numeric sort operators on a value that
 actually has been coerced to float8.  We could possibly go back and
 re-do the decisions about data types but it'd be a mess.

 2. Split the processing of aggregates with ORDER BY/DISTINCT so that the
 sorting/uniqueifying is done in a separate expression node that can work
 with the native types of the given columns, and only after that do we
 perform coercion to the aggregate function's input types.  This would be
 logically the cleanest thing, perhaps, but it'd represent a very major
 rework of the patch, with really no hope of getting it done for 9.0.

 3. Do something so that we can still match t::text to t.  This
 seems pretty awful on first glance but it's not actually that bad,
 because in the case we care about the cast will be marked as having
 been applied implicitly.  Basically, instead of just equal() comparisons
 in findTargetlistEntrySQL99(), we'd strip off any implicit cast at the
 top of either expression, and only then do equal().  Since the implicit
 casts are, by definition, things the user didn't write, this would still
 have the expected behavior of matching expressions that were identical
 when the user wrote them.

 #3 seems the sanest fix, but I wonder if anyone has an objection or
 better idea.

I didn't look at the code yet, #2 sounds like the way to go. But I see
the breakage is unacceptable for 9.0, so #3 is the choice for 9.0 and
will we fix it as #2 for 9.1 or later?


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] Reworks of DML permission checks

2010-07-19 Thread KaiGai Kohei
(2010/07/20 3:13), Robert Haas wrote:
 2010/7/12 KaiGai Koheikai...@ak.jp.nec.com:
 (2010/07/10 5:53), Robert Haas wrote:
 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com:
 The attached patch tries to rework DML permission checks.

 It was mainly checked at the ExecCheckRTEPerms(), but same logic was
 implemented in COPY TO/FROM statement and RI_Initial_Check().

 This patch tries to consolidate these permission checks into a common
 function to make access control decision on DML permissions. It enables
 to eliminate the code duplication, and improve consistency of access
 controls.

 This patch is listed on the CommitFest page, but I'm not sure if it
 represents the latest work on this topic.  At a minimum, it needs to
 be rebased.

 I am not excited about moving ExecCheckRT[E]Perms to some other place
 in the code.  It seems to me that will complicate back-patching with
 no corresponding advantage.  I'd suggest we not do that.The COPY
 and RI code can call ExecCheckRTPerms() where it is. Maybe at some
 point we will have a grand master plan for how this should all be laid
 out, but right now I'd prefer localized changes.


 OK, I rebased and revised the patch not to move ExecCheckRTPerms()
 from executor/execMain.c.
 In the attached patch, DoCopy() and RI_Initial_Check() calls that
 function to consolidate dml access control logic.
 
 This patch contains a number of copies of the following code:
 
 +   {
 +   if (ereport_on_violation)
 +   aclcheck_error(ACLCHECK_NO_PRIV, 
 ACL_KIND_CLASS,
 +
 get_rel_name(relOid));
 +   return false;
 +   }
 
 What if we don't pass ereport_on_violation down to
 ExecCheckRTEPerms(), and just have it return a boolean?  Then
 ExecCheckRTPerms() can throw the error if ereport_on_violation is
 true, and return false otherwise.
 
All the error messages are indeed same, so it seems to me fair enough.

As long as we don't need to report the error using aclcheck_error_col(),
instead of aclcheck_error(), this change will keep the code simple.
If it is preferable to show users the column-name in access violations,
we need to raise an error from ExecCheckRTEPerms().

 With this patch, ri_triggers.c emits a compiler warning, apparently
 due to a missing include.
 
Oh, sorry, I'll fix it soon.

Thanks,
-- 
KaiGai Kohei kai...@ak.jp.nec.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] standard_conforming_strings

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 5:04 PM, Peter Eisentraut pete...@gmx.net wrote:
 On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote:
 On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote:

  I think there are two ways we can do this, seeing that most appear to be
  in favor of doing it in the first place:  Either we just flip the
  default, make a note in the release notes, and see what happens.  Or we
  spend some time now and make, say, a list of driver versions and
  application versions that work with standard_conforming_strings = on,
  and then decide based on that, and also make that list a public resource
  for packagers etc.

 Do both. Turn them on, then make a list and inform driver maintainers who 
 need to update. They've got a year, after all.

 Here we go then:
 http://wiki.postgresql.org/wiki/Standard_conforming_strings

Looks like a good start.  We might want to make an open items for
9.1 page and, as a first such open item, add a link to this page.

Since it seems we have consensus, I will commit the patch.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] suppress automatic recovery after back crash

2010-07-19 Thread Robert Haas
On Sat, Jul 17, 2010 at 10:16 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 On Sun, 2010-06-27 at 20:54 -0400, Robert Haas wrote:
 automatic_restart = true      # reinitialize after backend crash?

 automatic_restart makes me think when does that happen?.

 Can we call this restart_after_crash? Or similar.

 +1.  automatic_restart is close to content-free.

OK, committed that way.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] standard_conforming_strings

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 8:32 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Jul 19, 2010 at 5:04 PM, Peter Eisentraut pete...@gmx.net wrote:
 On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote:
 On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote:

  I think there are two ways we can do this, seeing that most appear to be
  in favor of doing it in the first place:  Either we just flip the
  default, make a note in the release notes, and see what happens.  Or we
  spend some time now and make, say, a list of driver versions and
  application versions that work with standard_conforming_strings = on,
  and then decide based on that, and also make that list a public resource
  for packagers etc.

 Do both. Turn them on, then make a list and inform driver maintainers who 
 need to update. They've got a year, after all.

 Here we go then:
 http://wiki.postgresql.org/wiki/Standard_conforming_strings

 Looks like a good start.  We might want to make an open items for
 9.1 page and, as a first such open item, add a link to this page.

 Since it seems we have consensus, I will commit the patch.

So it turns out that (at least) the hstore and ECPG regression tests
depend on the default setting of standard_conforming_strings.  I have
taken a crack at fixing this...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] Reworks of DML permission checks

2010-07-19 Thread KaiGai Kohei
The attached patch is the revised one.

* It was rebased to the latest git HEAD.
* Prototype of ExecCheckRTEPerms() was changed; it become to return
  a bool value to inform the caller its access control decision, and
  its 'ereport_on_violation' argument has gone.
* ExecCheckRTPerms() calls aclcheck_error() when ExecCheckRTEPerms()
  returned false, and 'ereport_on_violation' is true.
* Add '#include executor/executor.h' on the ri_triggers.c.

Thanks,

(2010/07/20 9:24), KaiGai Kohei wrote:
 (2010/07/20 3:13), Robert Haas wrote:
 2010/7/12 KaiGai Koheikai...@ak.jp.nec.com:
 (2010/07/10 5:53), Robert Haas wrote:
 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com:
 The attached patch tries to rework DML permission checks.

 It was mainly checked at the ExecCheckRTEPerms(), but same logic was
 implemented in COPY TO/FROM statement and RI_Initial_Check().

 This patch tries to consolidate these permission checks into a common
 function to make access control decision on DML permissions. It enables
 to eliminate the code duplication, and improve consistency of access
 controls.

 This patch is listed on the CommitFest page, but I'm not sure if it
 represents the latest work on this topic.  At a minimum, it needs to
 be rebased.

 I am not excited about moving ExecCheckRT[E]Perms to some other place
 in the code.  It seems to me that will complicate back-patching with
 no corresponding advantage.  I'd suggest we not do that.The COPY
 and RI code can call ExecCheckRTPerms() where it is. Maybe at some
 point we will have a grand master plan for how this should all be laid
 out, but right now I'd prefer localized changes.


 OK, I rebased and revised the patch not to move ExecCheckRTPerms()
 from executor/execMain.c.
 In the attached patch, DoCopy() and RI_Initial_Check() calls that
 function to consolidate dml access control logic.

 This patch contains a number of copies of the following code:

 +   {
 +   if (ereport_on_violation)
 +   aclcheck_error(ACLCHECK_NO_PRIV, 
 ACL_KIND_CLASS,
 +
 get_rel_name(relOid));
 +   return false;
 +   }

 What if we don't pass ereport_on_violation down to
 ExecCheckRTEPerms(), and just have it return a boolean?  Then
 ExecCheckRTPerms() can throw the error if ereport_on_violation is
 true, and return false otherwise.

 All the error messages are indeed same, so it seems to me fair enough.
 
 As long as we don't need to report the error using aclcheck_error_col(),
 instead of aclcheck_error(), this change will keep the code simple.
 If it is preferable to show users the column-name in access violations,
 we need to raise an error from ExecCheckRTEPerms().
 
 With this patch, ri_triggers.c emits a compiler warning, apparently
 due to a missing include.

 Oh, sorry, I'll fix it soon.
 
 Thanks,


-- 
KaiGai Kohei kai...@ak.jp.nec.com


pgsql-v9.1-reworks-dml-checks.3.patch
Description: application/octect-stream

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


Re: [HACKERS] [COMMITTERS] pgsql: pgindent run for 9.0, second run

2010-07-19 Thread Robert Haas
On Tue, Jul 6, 2010 at 3:19 PM, Bruce Momjian momj...@postgresql.org wrote:
 Log Message:
 ---
 pgindent run for 9.0, second run

It appears that the expletive git mirror has deduced the wrong
contents for this commit.  Apparently as a result, when I build from
git master, the dblink regression tests fail.

Can someone please fix this?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread Robert Haas
On Sun, Jul 18, 2010 at 2:00 PM, David Christensen da...@endpoint.com wrote:
 Updated the commitfest entry with the patch, updated the title to reflect the 
 actual name of the command, and marked as ready for committer.

I took a look at this patch.  One problem is that it doesn't handle
the case where there is no database connection (for example, shut down
the database with pg_ctl, then do select 1, then do \conninfo).  I've
fixed that in the attached version.

However, I'm also wondering about the output format.  I feel like it
would make sense to use a format similar to what we do for \c, which
looks like this:

You are now connected to database %s.

It prints out more parameters if they've changed.  The longest
possible version is:

You are now connected to database %s on host %s at port %s as user %s.

My suggestion is that we use the same format, except (1) always
include all the components, since that's the point; (2) don't include
the word now; and (3) if there is no host, then print via local
socket rather than on host...port  So where the current patch
prints:

Connected to database: rhaas, user: rhaas, port: 5432 via local
domain socket

I would propose to print instead:

You are connected to database rhaas via local socket as user rhaas.

If people strongly prefer the way the patch does it now, I don't think
it's horrible... but it seems like it would be nicer to be somewhat
consistent with the existing message.  Thoughts?

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


psql-conninfo-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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread David Christensen

On Jul 19, 2010, at 10:34 PM, Robert Haas wrote:

 On Sun, Jul 18, 2010 at 2:00 PM, David Christensen da...@endpoint.com wrote:
 Updated the commitfest entry with the patch, updated the title to reflect 
 the actual name of the command, and marked as ready for committer.
 
 I took a look at this patch.  One problem is that it doesn't handle
 the case where there is no database connection (for example, shut down
 the database with pg_ctl, then do select 1, then do \conninfo).  I've
 fixed that in the attached version.

Thanks, I hadn't considered that case.

 However, I'm also wondering about the output format.  I feel like it
 would make sense to use a format similar to what we do for \c, which
 looks like this:
 
 You are now connected to database %s.
 
 It prints out more parameters if they've changed.  The longest
 possible version is:
 
 You are now connected to database %s on host %s at port %s as user %s.
 
 My suggestion is that we use the same format, except (1) always
 include all the components, since that's the point; (2) don't include
 the word now; and (3) if there is no host, then print via local
 socket rather than on host...port  So where the current patch
 prints:
 
 Connected to database: rhaas, user: rhaas, port: 5432 via local
 domain socket
 
 I would propose to print instead:
 
 You are connected to database rhaas via local socket as user rhaas.
 
 If people strongly prefer the way the patch does it now, I don't think
 it's horrible... but it seems like it would be nicer to be somewhat
 consistent with the existing message.  Thoughts?


+1 from me; I don't care what color the bikeshed is, as long as it gets the 
point across, which this does, and is consistent to boot.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.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] Explicit psqlrc

2010-07-19 Thread Robert Haas
On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Jun 23, 2010 at 9:17 AM, gabrielle gor...@gmail.com wrote:
 On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@gmail.com wrote:
 Well, that might be a good idea, too, but my expectation is that:

 psql -f one -f two -f three

 ought to behave in a manner fairly similar to:

 cat one two three  all
 psql -f all

 and it sounds like with this patch that's far from being the case.

 Correct.  Each is handled individually.

 Should I continue to check on ON_ERROR_ROLLBACK results, or bounce
 this back to the author?

 It doesn't hurt to continue to review and find other problems so that
 the author can try to fix them all at once, but it certainly seems
 clear that it's not ready to commit at this point, so it definitely
 needs to go back to the author for a rework.

Since it has been over a month since this review was posted and no new
version of the patch has appeared, I think we should mark this patch
as Returned with Feedback.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread Robert Haas
On Mon, Jul 19, 2010 at 11:41 PM, David Christensen da...@endpoint.com wrote:
 I took a look at this patch.  One problem is that it doesn't handle
 the case where there is no database connection (for example, shut down
 the database with pg_ctl, then do select 1, then do \conninfo).  I've
 fixed that in the attached version.

 Thanks, I hadn't considered that case.

For some reason, I end up crashing a lot of databases during
development (must be my lousy coding).  So I've had occasion to run
across this, ahem, a few times...

 +1 from me; I don't care what color the bikeshed is, as long as it gets the 
 point across, which this does, and is consistent to boot.

Great, committed that way.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread David Christensen
 I would propose to print instead:
 
 You are connected to database rhaas via local socket as user rhaas.


One minor quibble here; you lose the ability to see which pg instance you're 
running on if there are multiple ones running on different local sockets, so 
maybe either the port or the socket path should show up here still.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread Robert Haas
On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com wrote:
 I would propose to print instead:

 You are connected to database rhaas via local socket as user rhaas.


 One minor quibble here; you lose the ability to see which pg instance you're 
 running on if there are multiple ones running on different local sockets, so 
 maybe either the port or the socket path should show up here still.

Doh.  Will fix.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread Robert Haas
On Tue, Jul 20, 2010 at 12:07 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com 
 wrote:
 I would propose to print instead:

 You are connected to database rhaas via local socket as user rhaas.


 One minor quibble here; you lose the ability to see which pg instance you're 
 running on if there are multiple ones running on different local sockets, so 
 maybe either the port or the socket path should show up here still.

 Doh.  Will fix.

Something like the attached?

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


psql-conninfo-fix.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] [COMMITTERS] pgsql: pgindent run for 9.0, second run

2010-07-19 Thread Andrew Dunstan



Robert Haas wrote:

On Tue, Jul 6, 2010 at 3:19 PM, Bruce Momjian momj...@postgresql.org wrote:
  

Log Message:
---
pgindent run for 9.0, second run



It appears that the expletive git mirror has deduced the wrong
contents for this commit.  Apparently as a result, when I build from
git master, the dblink regression tests fail.

Can someone please fix this?

  


I despaired of this repo being anything like reliable months ago. AFAIK 
it is using a known to be broken version of fromcvs.


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] psql \conninfo command (was: Patch: psql \whoami option)

2010-07-19 Thread David Christensen

On Jul 19, 2010, at 11:10 PM, Robert Haas wrote:

 On Tue, Jul 20, 2010 at 12:07 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com 
 wrote:
 I would propose to print instead:
 
 You are connected to database rhaas via local socket as user rhaas.
 
 
 One minor quibble here; you lose the ability to see which pg instance 
 you're running on if there are multiple ones running on different local 
 sockets, so maybe either the port or the socket path should show up here 
 still.
 
 Doh.  Will fix.
 
 Something like the attached?


Looks good to me.

Regards,

David
--
David Christensen
End Point Corporation
da...@endpoint.com





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