Re: [HACKERS] Grouping Sets

2011-09-19 Thread Joshua Tolley
On Sun, Sep 18, 2011 at 02:08:01PM -0500, David Rinaldi wrote:
 I tried to apply the Grouping Sets Patch to 8.4, but received several Hunks
 failed messages, does anyone know if the failing hunks can be applied
 manually?  Or what version they were applied to specifically?

Your best bet is probably to get the code from approximately the date of the
patch. As far as I know it hasn't been touched in a while, and didn't work
well back when it was being actively developed.

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


pgpeMKakIJ2SX.pgp
Description: PGP signature


Re: [HACKERS] Patch to add a primary key using an existing index

2011-01-25 Thread Joshua Tolley
On Mon, Jan 24, 2011 at 07:01:13PM -0500, Tom Lane wrote:
 One other issue that might be worthy of discussion is that as things
 stand, execution of the ADD CONSTRAINT USING INDEX syntax will cause
 the constraint to absorb the index as an INTERNAL dependency.  That
 means dropping the constraint would make the index go away silently ---
 it no longer has any separate life.  If the intent is just to provide a
 way to get the effect of ALTER ADD PRIMARY KEY CONCURRENTLY, then this
 behavior is probably fine.  But someone who believes DROP CONSTRAINT
 exactly reverses the effects of ADD CONSTRAINT might be surprised.
 Comments?

So you'd manually create an index, attach it to a constraint, drop the
constraint, and find that the index had disappeared? ISTM since you created
the index explicitly, you should have to drop it explicitly as well.

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


signature.asc
Description: Digital signature


Re: [HACKERS] SSI, simplified

2011-01-25 Thread Joshua Tolley
On Sun, Jan 23, 2011 at 07:48:04PM -0600, Kevin Grittner wrote:
 http://www.xtranormal.com/watch/8285377/?listid=20642536

I foresee a whole new set of animated postgres tutorials...

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


signature.asc
Description: Digital signature


Re: [HACKERS] Sync Rep Design

2010-12-30 Thread Joshua Tolley
On Thu, Dec 30, 2010 at 03:24:09PM -0500, Aidan Van Dyk wrote:
 On Thu, Dec 30, 2010 at 3:07 PM, Robert Treat r...@xzilla.net wrote:
 
  If primary crashes while commits are waiting for acknowledgement, those
  transactions will be marked fully committed if the primary database
  recovers, no matter how allow_standalone_primary is set.
 
  This seems backwards; if you are waiting for acknowledgement, wouldn't the
  normal assumption be that the transactions *didnt* make it to any standby,
  and should be rolled back ?
 
 This is the standard 2-phase commit problem.  The primary server *has*
 committed it, it's fsync has returned, and the only thing keeping it
 from returning the commit to the client is that it's waiting on a
 synchronous ack from a slave.

snip

 2) initiate fsync on the primary first
- In this case, the slave is always slightly behind.  If if your
 primary falls over, you don't give commit messages to the clients, but
 if it recovers, it might have committed data, and slaves will still be
 able to catch up.
 
 The thing is that currently, even without replication, #2 can happen.

For what little it's worth, I vote for this option, because it's a problem
that can already happen (as opposed to adding an entirely new type of problem
to the mix).

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


signature.asc
Description: Digital signature


Re: [HACKERS] proposal : cross-column stats

2010-12-13 Thread Joshua Tolley
On Sun, Dec 12, 2010 at 07:10:44PM -0800, Nathan Boley wrote:
 Another quick note: I think that storing the full contingency table is
 wasteful since the marginals are already stored in the single column
 statistics. Look at copulas [2] ( FWIW I think that Josh Tolley was
 looking at this a couple years back ).

Josh Tolley still looks at it occasionally, though time hasn't permitted any
sort of significant work for quite some time. The multicolstat branch on my
git.postgresql.org repository will create an empirical copula each
multi-column index, and stick it in pg_statistic. It doesn't yet do anything
useful with that information, nor am I convinced it's remotely bug-free. In a
brief PGCon discussion with Tom a while back, it was suggested a good place
for the planner to use these stats would be clausesel.c, which is responsible
for handling code such as ...WHERE foo  4 AND foo  5.

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


signature.asc
Description: Digital signature


Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Joshua Tolley
On Mon, Nov 29, 2010 at 11:12:58AM -0500, Tom Lane wrote:
 Andrew Dunstan and...@dunslane.net writes:
  I'm not sure why you need either from. It just seems like a noise 
  word. Maybe we could use pg_execute_query_file() and 
  pg_execute_query_string(), which would be fairly clear and nicely 
  symmetrical.
 
 +1, but I think query is also a noise word here.
 Why not just pg_execute_file and pg_execute_string?
 
   regards, tom lane

While we're bikeshedding, and since I started the thread that has become this
topic, +1 for Tom's naming.

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


signature.asc
Description: Digital signature


Re: [HACKERS] pg_execute_from_file review

2010-11-25 Thread Joshua Tolley
On Thu, Nov 25, 2010 at 10:24:51PM +0100, Dimitri Fontaine wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  I've just looked at pg_execute_from_file[1]. The idea here is to execute all
  the SQL commands in a given file. My comments:
 
 Thanks for your review. Please find attached a revised patch where I've
 changed the internals of the function so that it's split in two and that
 the opr_sanity check passes, per comments from David Wheeler and Tom Lane.

I'll take a look ASAP.

  * I'd like to see the docs slightly expanded, specifically to describe
parameter replacement. I wondered for a while if I needed to set of
parameters in any specific way, before reading the code and realizing they
can be whatever I want.
 
 My guess is that you knew that in the CREATE EXTENSION context, it has
 been proposed to use the notation @extschema@ as a placeholder, and
 you've then been confused. I've refrained from imposing any style with
 respect to what the placeholder would look like in the mecanism-patch.
 
 Do we still want to detail in the docs that there's nothing expected
 about the placeholder syntax of format?

Perhaps such docs will show up with the rest of the EXTENSION work, but I'd
like a brief mention somewhere.

  * Does anyone think it needs representation in the test suite?
 
 Well the patch will get its tests with the arrival of the extension main
 patch, where all contribs are installed using it.

Works for me.

  * Shouldn't it include SPI_push() and SPI_pop()?
 
 ENOCLUE

My guess is yes, because that was widely hailed as a good idea when I did
PL/LOLCODE. I suspect it would only matter if someone were using
pg_execute_from_file within some other function, which isn't entirely
unlikely.

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


signature.asc
Description: Digital signature


[HACKERS] pg_execute_from_file review

2010-11-24 Thread Joshua Tolley
I've just looked at pg_execute_from_file[1]. The idea here is to execute all
the SQL commands in a given file. My comments:

* It applies well enough, and builds fine
* It seems to work, and I've not come up with a way to make it break
* It seems useful, and to follow the limited design discussion relevant to it
* It looks more or less like the rest of the code base, so far as I know

Various questions and/or nits:

* I'd like to see the docs slightly expanded, specifically to describe
  parameter replacement. I wondered for a while if I needed to set of
  parameters in any specific way, before reading the code and realizing they
  can be whatever I want.
* Does anyone think it needs representation in the test suite?
* Is it at all bad to include spi.h in genfile.c? IOW should this function
  live elsewhere? It seems reasonable to me to do it as written, but I thought
  I'd ask.
* In the snippet below, it seems best just to use palloc0():
query_string = (char *)palloc((fsize+1)*sizeof(char));
memset(query_string, 0, fsize+1);
* Shouldn't it include SPI_push() and SPI_pop()?

[1] http://archives.postgresql.org/message-id/m262wf6fnz@2ndquadrant.fr

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


signature.asc
Description: Digital signature


Re: [HACKERS] Synchronous replication - patch status inquiry

2010-09-02 Thread Joshua Tolley
On Wed, Sep 01, 2010 at 04:53:38PM +0900, Fujii Masao wrote:
 - down
   When that situation occurs, the master shuts down immediately.
   Though this is unsafe for the system requiring high availability,
   as far as I recall, some people wanted this mode in the previous
   discussion.

Oracle provides this, among other possible configurations; perhaps that's why
it came up earlier.

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


signature.asc
Description: Digital signature


Re: [HACKERS] grouping sets - updated patch

2010-08-09 Thread Joshua Tolley
On Mon, Aug 09, 2010 at 10:59:26PM +0200, Pavel Stehule wrote:
 Hello
 
 I fixed an issues with empty sets. It just work, but there are some ugly 
 hacks.
 
 It's really needs own planner node - now grouping functions are not
 supported by ORDER BY clause.

I haven't made it through the last version much, but I'll poke through this
instead. I have a few days of family business coming up, and might be
unrespondive during that time.

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


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-06 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 I am sending a updated version.

I've been looking at the changes to gram.y, and noted the comment under 
func_expr
where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
reserved keyword because it's already used in the cube contrib module. But
then the changes to kwlist.h include this:

+ PG_KEYWORD(cube, CUBE, RESERVED_KEYWORD)
...
+ PG_KEYWORD(rollup, ROLLUP, RESERVED_KEYWORD)

...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
realize things like CURRENT_TIME, that also have special entries in the
func_expr grammar, are also reserved keywords, but this all seems at odds with
the comment. What am I missing? Is the comment simply pointing out that the
designation of CUBE and ROLLUP as reserved keywords will have to change at
some point, but it hasn't been implemented yet (or no one has figured out how
to do it)?

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


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-05 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 So Joshua, can you look on code?

Sure... thanks :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-04 Thread Joshua Tolley
On Wed, Aug 04, 2010 at 04:44:05AM +0200, Pavel Stehule wrote:
  Yeah, I seem to have done a poor job of producing the patch based on the
  repository I was working from. That said, it seems Pavel's working actively 
  on
  a patch anyway, so perhaps my updating the old one isn't all that 
  worthwhile.
  Pavel, is your code somewhere that we can get to it?
 
 
 not now. please wait a week.

That works for me. I'm glad to try doing a better job of putting together my
version of the patch, if anyone thinks it's useful, but it seems that since
Pavel's code is due to appear sometime in the foreseeable future, there's not
much point in my doing that.

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


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-04 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 06:21:18AM +0200, Pavel Stehule wrote:
 I hope, so next week you can do own work on this job - I am not a
 native speaker, and my code will need a checking and fixing comments

I haven't entirely figured out how the code in the old patch works, but I
promise I *can* edit comments/docs :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-03 Thread Joshua Tolley
On Tue, Aug 03, 2010 at 12:58:03PM -0700, David Fetter wrote:
 On Mon, Aug 02, 2010 at 11:50:00PM -0600, Josh Tolley wrote:
  In case anyone's interested, I've taken the CTE-based grouping sets
  patch from [1] and made it apply to 9.1, attached. I haven't yet
  done things like checked it for whitespace consistency, style
  conformity, or anything else, but (tuits permitting) hope to figure
  out how it works and get it closer to commitability in some upcoming
  commitfest.
  
  I mention it here so that if someone else is working on it, we can
  avoid duplicated effort, and to see if a CTE-based grouping sets
  implementation is really the way we think we want to go.
  
  [1]
  http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php
 
 I've added back one file in the patch enclosed here.  I'm still
 getting compile fails from
 
 CC=ccache gcc ./configure --prefix=$PG_PREFIX --with-pgport=$PGPORT 
 --with-perl --with-libxml --enable-debug --enable-cassert
 make
 
 Log from that also enclosed.
 

Yeah, I seem to have done a poor job of producing the patch based on the
repository I was working from. That said, it seems Pavel's working actively on
a patch anyway, so perhaps my updating the old one isn't all that worthwhile.
Pavel, is your code somewhere that we can get to it?

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


signature.asc
Description: Digital signature


[HACKERS] GROUPING SETS revisited

2010-08-02 Thread Joshua Tolley
In case anyone's interested, I've taken the CTE-based grouping sets patch from
[1] and made it apply to 9.1, attached. I haven't yet done things like checked
it for whitespace consistency, style conformity, or anything else, but (tuits
permitting) hope to figure out how it works and get it closer to commitability
in some upcoming commitfest.

I mention it here so that if someone else is working on it, we can avoid
duplicated effort, and to see if a CTE-based grouping sets implementation is
really the way we think we want to go.

[1] http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/src/backend/parser/Makefile b/src/backend/parser/Makefile
index a8f4c07..fb248a6 100644
*** a/src/backend/parser/Makefile
--- b/src/backend/parser/Makefile
*** override CPPFLAGS := -I. -I$(srcdir) $(C
*** 15,21 
  OBJS= analyze.o gram.o keywords.o kwlookup.o parser.o \
parse_agg.o parse_clause.o parse_coerce.o parse_cte.o parse_expr.o \
parse_func.o parse_node.o parse_oper.o parse_param.o parse_relation.o \
!   parse_target.o parse_type.o parse_utilcmd.o scansup.o
  
  FLEXFLAGS = -CF
  
--- 15,21 
  OBJS= analyze.o gram.o keywords.o kwlookup.o parser.o \
parse_agg.o parse_clause.o parse_coerce.o parse_cte.o parse_expr.o \
parse_func.o parse_node.o parse_oper.o parse_param.o parse_relation.o \
!   parse_target.o parse_type.o parse_utilcmd.o scansup.o parse_gsets.o
  
  FLEXFLAGS = -CF
  
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6b99a10..1b579a8 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
***
*** 34,39 
--- 34,40 
  #include parser/parse_clause.h
  #include parser/parse_coerce.h
  #include parser/parse_cte.h
+ #include parser/parse_gsets.h
  #include parser/parse_oper.h
  #include parser/parse_param.h
  #include parser/parse_relation.h
*** parse_sub_analyze(Node *parseTree, Parse
*** 150,155 
--- 151,313 
  }
  
  /*
+  * process GROUPING SETS
+  */
+ static SelectStmt *
+ makeSelectStmt(List *targetList, List *fromClause)
+ {
+ 	SelectStmt *n = makeNode(SelectStmt);
+ 	n-distinctClause = NULL;
+ 	n-intoClause = NULL;
+ 	n-targetList = targetList;
+ 	n-fromClause = fromClause;
+ 	n-whereClause = NULL;
+ 	n-groupClause = NULL;
+ 	n-havingClause = NULL;
+ 	n-windowClause = NIL;
+ 	n-withClause = NULL;
+ 	n-valuesLists = NIL;
+ 	n-sortClause = NIL;
+ 	n-limitOffset = NULL;
+ 	n-limitCount = NULL;
+ 	n-lockingClause = NIL;
+ 	n-op = SETOP_NONE;
+ 	n-all = false;
+ 	n-larg = NULL;
+ 	n-rarg = NULL;
+ 	return n;
+ }
+ 
+ static List *
+ makeStarTargetList(void)
+ {
+ 	ResTarget *rt = makeNode(ResTarget);
+ 	
+ 	rt-name = NULL;
+ 	rt-indirection = NIL;
+ 	rt-val = (Node *) makeNode(ColumnRef);
+ 	((ColumnRef *) rt-val)-fields = list_make1(makeNode(A_Star));
+ 	rt-location = -1;
+ 
+ 	return list_make1(rt);
+ }
+  
+ static SelectStmt *
+ transformGroupingSets(ParseState *pstate, SelectStmt *stmt)
+ {
+ 	if (stmt-groupClause  IsA(stmt-groupClause, GroupByClause))
+ 	{
+ 		GroupingSetsSpec *gss = (GroupingSetsSpec *) expandGroupingSets(pstate, 
+ 		(List *)((GroupByClause *)stmt-groupClause)-fields);
+ 	
+ 		if (pstate-p_hasGroupingSets)
+ 		{
+ 			CommonTableExpr *cte = makeNode(CommonTableExpr);
+ 			SelectStmt  *cteedstmt;
+ 			int	ngroupingsets = list_length(gss-set_list) + (gss-has_empty_set ? 1 : 0);
+ 			bool	all = ((GroupByClause *) stmt-groupClause)-all;
+ 		
+ 			cteedstmt = makeSelectStmt(NIL, NIL);
+ 			cteedstmt-intoClause = stmt-intoClause;
+ 			cteedstmt-sortClause = stmt-sortClause;
+ 			cteedstmt-limitOffset = stmt-limitOffset;
+ 			cteedstmt-limitCount = stmt-limitCount;
+ 			cteedstmt-lockingClause = stmt-lockingClause;
+ 		
+ 			cte-ctename = **g**;
+ 			cte-ctequery = (Node *) stmt;
+ 			cte-location = -1;
+ 		
+ 			cteedstmt-withClause = makeNode(WithClause);
+ 			cteedstmt-withClause-ctes = list_make1(cte);
+ 			cteedstmt-withClause-recursive = false;
+ 			cteedstmt-withClause-location = -1;
+ 		
+ 			/* when is more than one grouping set, then we should generate setop node */
+ 			if (ngroupingsets  1)
+ 			{
+ /* add quuery under union all for every grouping set */
+ SelectStmt	*larg = NULL;
+ SelectStmt	*rarg;
+ ListCell*lc;
+ 			
+ foreach(lc, gss-set_list)
+ {
+ 	List	*groupClause;
+ 
+ 	Assert(IsA(lfirst(lc), List));
+ 	groupClause = (List *) lfirst(lc);
+ 
+ 	if (larg == NULL)
+ 	{
+ 		larg = makeSelectStmt(copyObject(stmt-targetList),
+ 	list_make1(makeRangeVar(NULL, **g**, -1)));
+ 		larg-groupClause = (Node *) groupClause;
+ 		larg-havingClause = copyObject(stmt-havingClause);
+ 	}
+ 	else
+ 	{
+ 		SelectStmt	*setop = makeSelectStmt(NIL, NIL);
+ 	
+ 		rarg = makeSelectStmt(copyObject(stmt-targetList),
+ 	list_make1

Re: [HACKERS] Synchronous replication

2010-07-27 Thread Joshua Tolley
On Tue, Jul 27, 2010 at 01:41:10PM +0900, Fujii Masao wrote:
 On Tue, Jul 27, 2010 at 12:36 PM, Joshua Tolley eggyk...@gmail.com wrote:
  Perhaps I'm hijacking the wrong thread for this, but I wonder if the quorum
  idea is really the best thing for us. I've been thinking about Oracle's way 
  of
  doing things[1]. In short, there are three different modes: availability,
  performance, and protection. Protection appears to mean that at least one
  standby has applied the log; availability means at least one standby has
  received the log info (it doesn't specify whether that info has been fsynced
  or applied, but presumably does not mean applied, since it's distinct from
  protection mode); performance means replication is asynchronous. I'm not
  sure this method is perfect, but it might be simpler than the quorum 
  behavior
  that has been considered, and adequate for actual use cases.
 
 In my case, I'd like to set up one synchronous standby on the near rack for
 high-availability, and one asynchronous standby on the remote site for 
 disaster
 recovery. Can Oracle's way cover the case?

I don't think it can support the case you're interested in, though I'm not
terribly expert on it. I'm definitely not arguing for the syntax Oracle uses,
or something similar; I much prefer the flexibility we're proposing, and agree
with Yeb Havinga in another email who suggests we spell out in documentation
some recipes for achieving various possible scenarios given whatever GUCs we
settle on.

 availability mode with two standbys might create a sort of similar 
 situation.
 That is, since the ACK from the near standby arrives in first, the near 
 standby
 acts synchronous and the remote one does asynchronous. But the ACK from the
 remote standby can arrive in first, so it's not guaranteed that the near 
 standby
 has received the log info before transaction commit returns a success to the
 client. In this case, we have to failover to the remote standby even if it's 
 not
 under control of a clusterware. This is a problem for me.

My concern is that in a quorum system, if the quorum number is less than the
total number of replicas, there's no way to know *which* replicas composed the
quorum for any given transaction, so we can't know which servers to fail to if
the master dies. This isn't different from Oracle, where it looks like
essentially the quorum value is always 1. Your scenario shows that all
replicas are not created equal, and that sometimes we'll be interested in WAL
getting committed on a specific subset of the available servers. If I had two
nearby replicas called X and Y, and one at a remote site called Z, for
instance, I'd set quorum to 2, but really I'd want to say wait for server X
and Y before committing, but don't worry about Z.

I have no idea how to set up our GUCs to encode a situation like that :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] Synchronous replication

2010-07-27 Thread Joshua Tolley
On Tue, Jul 27, 2010 at 10:53:45PM +0900, Fujii Masao wrote:
 On Tue, Jul 27, 2010 at 10:12 PM, Joshua Tolley eggyk...@gmail.com wrote:
  My concern is that in a quorum system, if the quorum number is less than the
  total number of replicas, there's no way to know *which* replicas composed 
  the
  quorum for any given transaction, so we can't know which servers to fail to 
  if
  the master dies.
 
 What about checking the current WAL receive location of each standby by
 using pg_last_xlog_receive_location()? The standby which has the newest
 location should be failed over to.

That makes sense. Thanks.

  This isn't different from Oracle, where it looks like
  essentially the quorum value is always 1. Your scenario shows that all
  replicas are not created equal, and that sometimes we'll be interested in 
  WAL
  getting committed on a specific subset of the available servers. If I had 
  two
  nearby replicas called X and Y, and one at a remote site called Z, for
  instance, I'd set quorum to 2, but really I'd want to say wait for server X
  and Y before committing, but don't worry about Z.
 
  I have no idea how to set up our GUCs to encode a situation like that :)
 
 Yeah, quorum commit alone cannot cover that situation. I think that
 current approach (i.e., quorum commit plus replication mode per standby)
 would cover that. In your example, you can choose recv, fsync or
 replay as replication_mode in X and Y, and choose async in Z.

Clearly I need to read through the GUCs and docs better. I'll try to keep
quiet until that's finished :)


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


signature.asc
Description: Digital signature


Re: [HACKERS] Synchronous replication

2010-07-26 Thread Joshua Tolley
On Thu, Jul 22, 2010 at 10:37:12AM +0200, Yeb Havinga wrote:
 Fujii Masao wrote:
 Initially I also expected the quorum to behave like described by  
 Aidan/option 2. Also, IMHO the name quorom is a bit short, like having  
 maximum but not saying a max_something.

 quorum_min_sync_standbys
 quorum_max_sync_standbys

Perhaps I'm hijacking the wrong thread for this, but I wonder if the quorum
idea is really the best thing for us. I've been thinking about Oracle's way of
doing things[1]. In short, there are three different modes: availability,
performance, and protection. Protection appears to mean that at least one
standby has applied the log; availability means at least one standby has
received the log info (it doesn't specify whether that info has been fsynced
or applied, but presumably does not mean applied, since it's distinct from
protection mode); performance means replication is asynchronous. I'm not
sure this method is perfect, but it might be simpler than the quorum behavior
that has been considered, and adequate for actual use cases.

[1]
http://download.oracle.com/docs/cd/B28359_01/server.111/b28294/protection.htm#SBYDB02000
alternatively, http://is.gd/dLkq4

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


signature.asc
Description: Digital signature


Re: [HACKERS] cross column correlation revisted

2010-07-15 Thread Joshua Tolley
On Thu, Jul 15, 2010 at 12:04:21PM +0200, Hans-Jürgen Schönig wrote:
 hello ...
 
 a view is already nice but i think it is still too narrow. 
 the problem is: you don't want a view for every potential join.
 in addition to that - ideally there is not much left of a view when it comes 
 to checking for costs.
 so, i think, this is not the kind of approach leading to total success here.

The prolem is a very big one, and it's helpful to try solving it one piece at
a time, so the single table and view-based cases are probably good starting
points.

 one side question: does anybody happen to know how this is one in oracle or 
 db2?

Neither appear to handle multi-column statistics in any form.

[1] http://download.oracle.com/docs/cd/B13789_01/appdev.101/b10802/d_stats.htm
[2]
http://www.ibm.com/developerworks/data/library/techarticle/dm-0606fechner/index.html

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


signature.asc
Description: Digital signature


Re: [HACKERS] cross column correlation revisted

2010-07-14 Thread Joshua Tolley
On Wed, Jul 14, 2010 at 01:21:19PM +0200, Yeb Havinga wrote:
 Heikki Linnakangas wrote:
 However, the problem is how to represent and store the  
 cross-correlation. For fields with low cardinality, like gender and  
 boolean breast-cancer-or-not you can count the prevalence of all the  
 different combinations, but that doesn't scale. Another often cited  
 example is zip code + street address. There's clearly a strong  
 correlation between them, but how do you represent that?

 For scalar values we currently store a histogram. I suppose we could  
 create a 2D histogram for two columns, but that doesn't actually help  
 with the zip code + street address problem.
 In my head the neuron for 'principle component analysis' went on while  
 reading this. Back in college it was used to prepare input data before  
 feeding it into a neural network. Maybe ideas from PCA could be helpful?

I've been playing off and on with an idea along these lines, which builds an
empirical copula[1] to represent correlations between columns where there
exists a multi-column index containing those columns. This copula gets stored
in pg_statistic. There are plenty of unresolved questions (and a crash I
introduced and haven't had time to track down), but the code I've been working
on is here[2] in the multicolstat branch. Most of the changes are in
analyze.c; no user-visible changes have been introduced. For that matter,
there aren't any changes yet actually to use the values once calculated (more
unresolved questions get in the way there), but it's a start.

[1] http://en.wikipedia.org/wiki/Copula_(statistics)
[2] http://git.postgresql.org/gitweb?p=users/eggyknap/postgres.git

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


signature.asc
Description: Digital signature


Re: [HACKERS] cross column correlation revisted

2010-07-14 Thread Joshua Tolley
On Wed, Jul 14, 2010 at 04:41:01PM +0200, PostgreSQL - Hans-Jürgen Schönig 
wrote:
 hello ...
 
 look at the syntax i posted in more detail:
 
 ALTER TABLE x SET CORRELATION STATISTICS FOR (x.id = y.id AND x.id2 = 
  y.id2)
  
 it says X and Y ...
 the selectivity of joins are what i am most interested in. cross correlation 
 of columns within the same table are just a byproduct.
 the core thing is: how can i estimate the number of rows returned from a join?

All the discussion of this topic that I've seen has been limited to the single
table case. The hard problem in that case is coming up with something you can
precalculate that will actually be useful during query planning, without
taking too much disk, memory, CPU, or something else. Expanding the discussion
to include join relations certainly still has valid use cases, but is even
harder, because you've also got to keep track of precisely how the underlying
relations are joined, so you know in what context the statistics remain valid.
So it makes sense to tackle the single table version first. Once it's
implemented somehow, and has been proven sufficiently effective to merit the
increased code size and complexity, we can consider expanding it to joined
relations.

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


signature.asc
Description: Digital signature


[HACKERS] Out of date comment in xlogutils.c

2010-07-08 Thread Joshua Tolley
I noticed the following out-of-date bits in a comment in xlogutils.c:

   * LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE), for reading from the main
   * fork.
   *
-  * (Getting the lock is not really necessary, since we expect that this is
-  * only used during single-process XLOG replay, but some subroutines such
-  * as MarkBufferDirty will complain if we don't. And hopefully we'll get
-  * hot standby support in the future, where there will be backends running
-  * read-only queries during XLOG replay.)
-  *
   * The returned buffer is exclusively-locked.
   *
   * For historical reasons, instead of a ReadBufferMode argument, this only

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] Add _PG_init to PL language handler documentation

2010-06-09 Thread Joshua Tolley
On Tue, Jun 08, 2010 at 04:18:08PM -0400, Robert Haas wrote:
 On Wed, May 26, 2010 at 11:24 PM, Joshua Tolley eggyk...@gmail.com wrote:
  On Wed, May 26, 2010 at 03:47:25PM -0400, Robert Haas wrote:
  On Tue, May 25, 2010 at 4:34 AM, Jonathan Leto jal...@gmail.com wrote:
   This tiny doc patch adds _PG_init to the skeleton example code for a
   PL. The information is quite valuable to PL authors, who might miss it
   when it is described in the shared library documentation.
 
  I'm not sure it does much good to add it to the template without any
  explanation of what it does.  What might make more sense is to add a
  cross-reference to the section on dynamic loading, where this is
  documented.
 
  +1. How about the attached (which, incidentally, tested successfully on my
  box, because I've managed to achieve doc building nirvana through blindly
  flailing about until it worked...)?
 
 I've committed a doc change along these lines, but I thought your
 version was a little wordy and maybe not in the best spot, so I did it
 a bit differently.  Hopefully it's good - if not, I can always change
 it again...

You're far from the first to label one of my productions wordy :) Your
version suits me fine (btw, in case anyone else is looking for it, it's here:
http://archives.postgresql.org/pgsql-committers/2010-06/msg00060.php)

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] Add _PG_init to PL language handler documentation

2010-05-26 Thread Joshua Tolley
On Wed, May 26, 2010 at 03:47:25PM -0400, Robert Haas wrote:
 On Tue, May 25, 2010 at 4:34 AM, Jonathan Leto jal...@gmail.com wrote:
  This tiny doc patch adds _PG_init to the skeleton example code for a
  PL. The information is quite valuable to PL authors, who might miss it
  when it is described in the shared library documentation.
 
 I'm not sure it does much good to add it to the template without any
 explanation of what it does.  What might make more sense is to add a
 cross-reference to the section on dynamic loading, where this is
 documented.

+1. How about the attached (which, incidentally, tested successfully on my
box, because I've managed to achieve doc building nirvana through blindly
flailing about until it worked...)?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/plhandler.sgml b/doc/src/sgml/plhandler.sgml
index b82a5da..1162bcf 100644
*** a/doc/src/sgml/plhandler.sgml
--- b/doc/src/sgml/plhandler.sgml
***
*** 95,101 
 /para
  
 para
! This is a template for a procedural-language handler written in C:
  programlisting
  #include postgres.h
  #include executor/spi.h
--- 95,104 
 /para
  
 para
! This is a template for a procedural-language handler written in C. The
! documentation on C-language functions found in xref linkend=xfunc-c
! applies to language handlers as well as any other C functions, and may be
! helpful to understand some portions of this code.
  programlisting
  #include postgres.h
  #include executor/spi.h


signature.asc
Description: Digital signature


Re: [HACKERS] Specification for Trusted PLs?

2010-05-21 Thread Joshua Tolley
On Fri, May 21, 2010 at 1:36 PM, David Fetter da...@fetter.org wrote:
 On Fri, May 21, 2010 at 03:15:27PM -0400, Tom Lane wrote:
 As long as you can't do database access except via SPI, that should
 be covered.  So I guess the next item on the list is no, or at least
 restricted, access to functions outside the PL's own language.

 No access seems pretty draconian.

 How about limiting such access to functions of equal or lower
 trustedness?  Surely an untrusted function shouldn't be restricted
 from calling other untrusted functions based on the language they're
 written in.

Agreed. As long as a trusted language can do things outside the
database only by going through a database and calling some function to
which the user has rights, in an untrusted language, that seems decent
to me. A user with permissions to launch_missiles() would have a
function in an untrusted language to do it, but there's no reason an
untrusted language shouldn't be able to say SELECT
launch_missiles().

--
Joshua Tolley / eggyknap
End Point Corporation

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


Re: [HACKERS] Specification for Trusted PLs?

2010-05-21 Thread Joshua Tolley
On Fri, May 21, 2010 at 2:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Joshua Tolley eggyk...@gmail.com writes:
 Agreed. As long as a trusted language can do things outside the
 database only by going through a database and calling some function to
 which the user has rights, in an untrusted language, that seems decent
 to me. A user with permissions to launch_missiles() would have a
 function in an untrusted language to do it, but there's no reason an
 untrusted language shouldn't be able to say SELECT

 s/untrusted/trusted/ here, right?

Er, right. Sorry.


 launch_missiles().

 To me, as long as they go back into the database via SPI, anything they
 can get to from there is OK.  What I meant to highlight upthread is that
 we don't want trusted functions being able to access other functions
 directly without going through SQL.  As an example, a PL that has FFI
 capability sufficient to allow direct access to heap_insert() would
 have to be considered untrusted.

That I can definitely agree with.

--
Joshua Tolley / eggyknap
End Point Corporation

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


[HACKERS] wal_level and continuous archiving documentation

2010-05-13 Thread Joshua Tolley
I was reading through
http://www.postgresql.org/docs/9.0/static/continuous-archiving.html and
noticed that wal_level isn't mentioned where I'd expect it to be.
Specifically, there's a paragraph that starts, To enable WAL archiving, set
the archive_mode configuration parameter to on, and specify the shell command
to use in the archive_command configuration parameter. There follows a long
discussion of archive_command, but no further discussion of other
configuration settings for several paragraphs, suggesting that those two
configuration changes are the only ones required to end up with a useful
archive. However, further on, it discusses wal_level:

When wal_level is minimal some SQL commands are optimized to avoid WAL
logging, as described in Section 14.4.7. If archiving or streaming
replication were turned on during execution of one of these statements,
WAL would not contain enough information for archive recovery.

ISTM wal_archive should make an appearance where the docs bring up
archive_mode and archive_command, to say wal_level must be set to 'archive'
or 'hot_standby', so all required configuration changes are mentioned close
together.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-08 Thread Joshua Tolley
On Wed, Apr 07, 2010 at 01:07:21PM -0400, Robert Haas wrote:
 On Wed, Apr 7, 2010 at 10:46 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Simon Riggs si...@2ndquadrant.com writes:
  When there is a specific reject rule, why does the server say
  FATAL:  no pg_hba.conf entry
 
  It's intentional.  We try to expose the minimum amount of knowledge
  about the contents of pg_hba.conf to potential attackers.
 
 The problem with the message is not that it's uninformative, but that
 it's counterfactual.
 
 ...Robert

I agree (I noticed and was bothered by this today, as a matter of irrelevant
fact). I can support the idea of exposing as little as possible of
pg_hba.conf, but ISTM the no pg_hba.conf entry is exposing too much, by that
standard. Just say something like connection disallowed and leave it at that
-- either it's disallowed by lack of a rule, or by existence of a reject
rule, or by something else entirely. As long as the message isn't clearly
wrong in the reject case, as it is now.

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


signature.asc
Description: Digital signature


Re: [HACKERS] machine-readable pg_controldata?

2010-03-04 Thread Joshua Tolley
On Thu, Mar 04, 2010 at 10:54:15PM +0100, Magnus Hagander wrote:
 2010/3/4 Josh Berkus j...@agliodbs.com:
  pg_controldata --catalog_version
 
  Even better would be the ability to get everything which is in
  pg_controldata currently as part of a system view in a running
  PostgreSQL; I can get most of them, but certainly not all.
 
 +1 for having all the information available from inside the backend,
 if that's technically possible (which I assume it should be)

I'd love to see pg_config's various bits of information in there as well. I
just haven't gotten around to writing it. But +1 from me, FWIW.

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


signature.asc
Description: Digital signature


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


[HACKERS] Making pg_config and pg_controldata output available via SQL

2010-02-03 Thread Joshua Tolley
I'd really like to see the data from pg_config and pg_controldata available
through SQL, such as by adding output to pg_show_all_settings(), or adding new
SRFs named something like pg_config() and pg_controldata(). Does this make as
much sense to the rest of the world as it does to me? In particular it's
useful to be able to find $libdir without requiring pg_config, as some
packagers tend not to include it in anything put the -dev packages, but all
those settings seem useful to have on hand, and in at least most cases
shouldn't be tough to expose via SQL. Comments?

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


signature.asc
Description: Digital signature


Re: [HACKERS] Making pg_config and pg_controldata output available via SQL

2010-02-03 Thread Joshua Tolley
On Wed, Feb 03, 2010 at 02:32:47PM -0500, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  I'd really like to see the data from pg_config and pg_controldata available
  through SQL, such as by adding output to pg_show_all_settings(), or adding 
  new
  SRFs named something like pg_config() and pg_controldata(). Does this make 
  as
  much sense to the rest of the world as it does to me? In particular it's
  useful to be able to find $libdir without requiring pg_config, as some
  packagers tend not to include it in anything put the -dev packages, but all
  those settings seem useful to have on hand, and in at least most cases
  shouldn't be tough to expose via SQL. Comments?
 
 I wonder whether there's a security issue there.  Telling an attacker
 whether you've been built with feature X seems like possibly useful
 info that he couldn't otherwise get without local machine access.
 In particular, we already try to avoid exposing server filesystem
 path information.

I'd wondered the same thing, without spending enough time on it to come to a
conclusion beyond perhaps making the functions executable only by superuser
would suffice.

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


signature.asc
Description: Digital signature


Re: [HACKERS] development setup and libdir

2010-01-30 Thread Joshua Tolley
On Sat, Jan 30, 2010 at 04:50:11PM -0700, James William Pye wrote:
 That install of PG that you're using will *probably not* have debugging 
 information.
 
 Now, granted, you might not need PG with debugging for some problems, but
 chances are that you'll come across one (or two or fifty or so) where you
 *will* need it.

Not just debug information (the --enable-debug flag to the configure script),
but also --enable-cassert, --enable-depend, and/or others you're likely to
want to use during development.

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


signature.asc
Description: Digital signature


Re: [HACKERS] per-user pg_service.conf

2010-01-13 Thread Joshua Tolley
On Wed, Jan 13, 2010 at 11:49:59PM +0200, Peter Eisentraut wrote:
 I was surprised/annoyed to find out that there is no way to have
 per-user pg_service.conf, something like ~/.pg_service.conf (well,
 except by export PGSYSCONFDIR).  That would be easy to add.  Comments?

+1 from me. I was similarly surprised to learn the same thing recently, but
admit I didn't take the time see how easily it could be changed.

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


signature.asc
Description: Digital signature


[HACKERS] Hot standby documentation

2010-01-07 Thread Joshua Tolley
Having concluded I really need to start playing with hot standby, I started
looking for documentation on the subject. I found what I was looking for; I
also found this page[1], which, it seems, ought to mention hot standby.
Comments?

[1] http://developer.postgresql.org/pgdocs/postgres/high-availability.html

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


signature.asc
Description: Digital signature


Re: [HACKERS] Time to run initdb is mostly figure-out-the-timezone work

2009-12-18 Thread Joshua Tolley
On Fri, Dec 18, 2009 at 06:20:39PM +0100, Guillaume Lelarge wrote:
 Le 18/12/2009 18:07, Tom Lane a écrit :
  On current Fedora 11, there is a huge difference in initdb time if you
  have TZ set versus if you don't: I get about 18 seconds versus less than
  four.
 I have the exact same issue:

For whatever it's worth, I get it too, on Ubuntu 9.04... ~4s without TZ vs.
~1.8s with TZ.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Need a mentor, and a project.

2009-12-07 Thread Joshua Tolley
On Mon, Dec 07, 2009 at 09:53:32AM +0100, Albe Laurenz wrote:
 abindra wrote:
  Next quarter I am planning to do an Independent Study course 
  where the main objective would be to allow me to get familiar 
  with the internals of Postgres by working on a project(s). I 
  would like to work on something that could possibly be 
  accepted as a patch.
  
  This is (I think) somewhat similar to what students do during 
  google summer and I was hoping to get some help here in terms of:
  1. A good project to work on for a newbie.
  2. Would someone be willing to be a mentor? It would be nice 
  to be able to get some guidance on a one-to-one basis.
 
 I would start with the TODO list: http://wiki.postgresql.org/wiki/Todo
 These are things for which there is a consensus that it would be
 a good idea to implement them. Pick things that look interesting to
 you, and try to read the discussions in the archives that lead
 to the TODO items.

I agree the TODO list is a good place to start. Other good sources include the
-hackers list and comments in the code. I was surprised when I began taking an
interest in PostgreSQL how rarely interesting projects mentioned on -hackers
made it into the TODO list; I've come to realize that the TODO contains, in
general, very non-controversial items everyone is pretty sure we could use,
whereas -hackers ranges freely over other topics which are still very
interesting but often more controversial or less obviously necessary.
Committed patches both large and small address TODO list items fairly rarely,
so don't get too hung up on finding something from the TODO list alone.

 Bring the topic up in the hackers list, say that you would like
 to work on this or that TODO item, present your ideas of how you
 want to do it. Ask about things where you feel insecure.
 If you get some support, proceed to write a patch. Ask for
 directions, post half-baked patches and ask for comments.
 
 That is because you will probably receive a good amount of
 critizism and maybe rejection, and if you invest a couple of
 months into working on something that nobody knows about *and*
 your work gets rejected, that is much worse than drawing fire
 right away.

+1. Especially when developing a complex patch, and especially when you're new
to the community, you need to avoid working in a vacuum, for social as well as
technical reasons. The more complex a patch, the more consensus you'll
eventually need to achieve before getting it committed, in general, and it
helps to gain that consensus early on, rather than after you've written a lot
of code. The keyword proposal might be a useful search term when digging in
the -hackers archives for historical examples.

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


signature.asc
Description: Digital signature


Re: [HACKERS] PL/Python array support

2009-12-02 Thread Joshua Tolley
On Fri, Nov 20, 2009 at 12:00:24AM +0200, Peter Eisentraut wrote:
 On fre, 2009-11-13 at 18:46 +0300, Teodor Sigaev wrote:
  CREATE OR REPLACE FUNCTION incr(stuff int[]) RETURNS int[] AS $$
  for x in stuff:
   yield x+1
  $$
  LANGUAGE 'plpythonu';
  
  # select incr(ARRAY[1,2,3]);
  ERROR:  invalid memory alloc request size 18446744073709551608
  CONTEXT:  while creating return value
  PL/Python function incr
 
 Fixed with additional error check and regression test.  (The problem
 could be more simply demonstrated by returning any non-sequence from the
 function.)  Thanks for catching it.

My last email claimed that the regression test needed some additional changes
to its expected output, and further claimed that it had the regression test's
diff attached. As was helpfully pointed out off-list, it actually wasn't
attached. Trying again..

-- Josh
*** /home/josh/devel/pgsrc/pg85/src/pl/plpython/expected/plpython_types.out 
2009-12-01 20:39:52.0 -0700
--- /home/josh/devel/pgsrc/pg85/src/pl/plpython/results/plpython_types.out  
2009-12-01 20:40:04.0 -0700
***
*** 580,582 
--- 580,589 
   {abc,def}
  (1 row)
  
+ CREATE FUNCTION test_type_conversion_array_error() RETURNS int[] AS $$
+ return 5
+ $$ LANGUAGE plpythonu;
+ SELECT * FROM test_type_conversion_array_error();
+ ERROR:  PL/Python: return value of function with array return type is not a 
Python sequence
+ CONTEXT:  while creating return value
+ PL/Python function test_type_conversion_array_error

==



signature.asc
Description: Digital signature


Re: [HACKERS] PL/Python array support

2009-12-01 Thread Joshua Tolley
On Fri, Nov 20, 2009 at 12:00:24AM +0200, Peter Eisentraut wrote:
 On fre, 2009-11-13 at 18:46 +0300, Teodor Sigaev wrote:
  CREATE OR REPLACE FUNCTION incr(stuff int[]) RETURNS int[] AS $$
  for x in stuff:
   yield x+1
  $$
  LANGUAGE 'plpythonu';
  
  # select incr(ARRAY[1,2,3]);
  ERROR:  invalid memory alloc request size 18446744073709551608
  CONTEXT:  while creating return value
  PL/Python function incr
 
 Fixed with additional error check and regression test.  (The problem
 could be more simply demonstrated by returning any non-sequence from the
 function.)  Thanks for catching it.

I've finally gotten around to getting a review done, and basically it looks
good to me. There appears to be a problem with the tests in that the expected
output doesn't include the test_type_conversion_array_error() function
mentioned in sql/plpython_types.sql. Diff generated by the regression test is
attached. Other than that problem, though, the code looks fine to me (should I
presume to judge Peter's code? :). Aside from the problem mentioned above, the
tests work fine, and seem fairly comprehensive. Other testing I've done also
passes.

This patch doesn't include any documentation; my reading of the PL/Python docs
suggests that's probably acceptable, as the existing docs don't talk about its
array handling. That said, it might be useful to include an example, to show
for instance that identical PL/Python code could create either an array of a
type or a set of rows of that type:

5432 j...@josh*# create function return_set() returns setof int as $$ return
(1, 2, 3, 4, 5) $$ language plpythonu;
CREATE FUNCTION
5432 j...@josh*# create function return_arr() returns int[] as $$ return (1,
2, 3, 4, 5) $$ language plpythonu;
CREATE FUNCTION
5432 j...@josh*# select return_arr();
 return_arr  
-
 {1,2,3,4,5}
(1 row)

5432 j...@josh*# select * from return_set();
 return_set 

  1
  2
  3
  4
  5
(5 rows)

Perhaps that's overkill, though.

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


signature.asc
Description: Digital signature


Re: [HACKERS] plperl and inline functions -- first draft

2009-11-29 Thread Joshua Tolley
On Sat, Nov 28, 2009 at 10:15:40PM -0500, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  Makes sense on both counts. Thanks for the help. How does the attached look?
 
 Applied with minor corrections, mainly around the state save/restore
 logic.  I also put in some code to fix the memory leak noted by Tim Bunce,
 but am waiting for some confirmation that it's right before
 back-patching the pre-existing bug of the same ilk.
 
   regards, tom lane

Yay, and thanks. For the record, I'm can't claim to know whether your fix is
the Right Thing or not, so I'm witholding comment.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Application name patch - v4

2009-11-28 Thread Joshua Tolley
On Sat, Nov 28, 2009 at 06:47:49PM -0500, Tom Lane wrote:
 Dave Page dp...@pgadmin.org writes:
  Updated application name patch, including a GUC assign hook to clean
  the application name of any unsafe characters, per discussion.
 
 Applied with assorted editorialization.  There were a couple of
 definitional issues that I don't recall if we had consensus on:
 
 1. The patch prevents non-superusers from seeing other users'
 application names in pg_stat_activity.  This seems at best pretty
 debatable to me.  Yes, it supports usages in which you want to put
 security-sensitive information into the appname, but at the cost of
 disabling (perfectly reasonable) usages where you don't.  If we made
 the app name universally visible, people simply wouldn't put security
 sensitive info in it, the same as they don't put it on the command line.
 Should we change this?
 
 (While I'm looking at it, I wonder why client_addr and client_port
 are similarly hidden.)

I vote for showing it to everyone, superuser or otherwise, though I can't
really say why I feel that way. 

 2. I am wondering if we should mark application_name as
 GUC_NO_RESET_ALL.  As-is, the value sent at libpq initialization
 will be lost during RESET ALL, which would probably surprise people.
 On the other hand, not resetting it might surprise other people.
 If we were able to send it in the startup packet then this wouldn't
 be a problem, but we are far from being able to do that.

Nothing I've written uses RESET ALL, but if it did, I expect it would be
because whatever the connection was being used for in the past differs
substantially from whatever I plan to use it for in the future, which seems a
suitable time also to change application_name. I vote against
GUC_NO_RESET_ALL.

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


signature.asc
Description: Digital signature


Re: [HACKERS] plperl and inline functions -- first draft

2009-11-19 Thread Joshua Tolley
On Wed, Nov 18, 2009 at 12:38:00PM +0200, Alexey Klyukin wrote:
 Yes, current_call_data can't be allocate in the SPI memory context, since 
 it's used to extract the result after SPI_finish is called, although it 
 doesn't lead to problems here since no result is returned. Anyway, I'd move 
 SPI_connect after the current_call_data initialization.
 
 I also noticed that no error context is set in the inline handler, not sure 
 whether it really useful except for the sake of consistency, but in case it 
 is - here is the patch:

Makes sense on both counts. Thanks for the help. How does the attached look?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE FUNCTION replaceablefuncname/r
*** 59,69 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. A PL/Perl function must
!always return a scalar value.  You can return more complex structures
!(arrays, records, and sets) by returning a reference, as discussed below.
!Never return a list.
/para
  
note
--- 59,81 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
+ 
+PL/Perl also supports anonymous code blocks called with the
+xref linkend=sql-do endterm=sql-do-title
+statement:
+ 
+ programlisting
+ DO $$
+ # PL/Perl function body
+ $$ LANGUAGE plperl;
+ /programlisting
+ 
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!return a value; PL/Perl functions created with CREATE FUNCTION must always
!return a scalar value. You can return more complex structures (arrays,
!records, and sets) by returning a reference, as discussed below.  Never
!return a list.
/para
  
note
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...86337f3 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***
*** 0 
--- 1,9 
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ CONTEXT: PL/Perl anonymous code block
+ DO $$ use Config; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
+ CONTEXT: PL/Perl anonymous code block
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..88b73f3 100644
*** a/src/pl

Re: [HACKERS] plperl and inline functions -- first draft

2009-11-17 Thread Joshua Tolley
On Wed, Nov 18, 2009 at 09:35:35AM +1100, Brendan Jurd wrote:
 2009/11/17 Joshua Tolley eggyk...@gmail.com:
  On Sun, Nov 15, 2009 at 12:10:33PM +1100, Brendan Jurd wrote:
  I noticed that there was a fairly large amount of bogus/inconsistent
  whitespace
 ...
 
  Thanks -- I tend to forget whitespace :)
 
  In the documentation you refer to this feature as inline functions.
  I think this might be mixing up the terminology
 ...
  I can accept that argument. The attached patch modifies the documentation, 
  and
  fixes another inconsistency I found.
 
 
 Cool.  I have no gripes with the revised patch.  I'm marking this as
 ready for committer now.  Thanks!

Thanks to you, as well, and Andrew for his work.

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


signature.asc
Description: Digital signature


Re: [HACKERS] plperl and inline functions -- first draft

2009-11-17 Thread Joshua Tolley
On Tue, Nov 17, 2009 at 06:05:19PM -0500, Andrew Dunstan wrote:


 Alexey Klyukin wrote:

 I've noticed that the patch doesn't install current_call_data before calling 
 plperl_call_perl_func, although it saves and restores its previous value. 
 This breaks spi code, which relies on current_call_data-prodesc, i.e.:

 postgres=# DO $$ $result = spi_exec_query(select 1); $$ LANGUAGE plperl;
   

 Yeah, good catch. We need to lift some stuff out of  
 plperl_func_handler(), because this code bypasses that. Not only setting  
 the call_data but also connectin g to the SPI manager and maybe one or  
 two other things.

I kept thinking I had to test SPI, but I guess I hadn't ever done it. The
attached takes care of such stuff, I think.

 Also, a call to to plperl_call_perl_func should be cast to void to avoid a 
 possible compiler warning (although It doesn't emit one on my system):

 (void) plperl_call_perl_func(desc, fake_fcinfo);

 Right.

I don't get the warning either, and didn't realize it could produce one.
Thanks -- that change is also in the attached version.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE FUNCTION replaceablefuncname/r
*** 59,69 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. A PL/Perl function must
!always return a scalar value.  You can return more complex structures
!(arrays, records, and sets) by returning a reference, as discussed below.
!Never return a list.
/para
  
note
--- 59,81 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
+ 
+PL/Perl also supports anonymous code blocks called with the
+xref linkend=sql-do endterm=sql-do-title
+statement:
+ 
+ programlisting
+ DO $$
+ # PL/Perl function body
+ $$ LANGUAGE plperl;
+ /programlisting
+ 
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!return a value; PL/Perl functions created with CREATE FUNCTION must always
!return a scalar value. You can return more complex structures (arrays,
!records, and sets) by returning a reference, as discussed below.  Never
!return a list.
/para
  
note
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...a955581 .
*** a/src/pl/plperl/expected/plperl_do.out

Re: [HACKERS] plperl and inline functions -- first draft

2009-11-16 Thread Joshua Tolley
On Sun, Nov 15, 2009 at 12:10:33PM +1100, Brendan Jurd wrote:
 I noticed that there was a fairly large amount of bogus/inconsistent
 whitespace in the patch, particularly in the body of
 plperl_inline_handler().  Some of the lines were indented with tabs,
 others with spaces.  You should stick with tabs.  There were also a
 lot of lines with a whole lot of trailing whitespace at the end.

Thanks -- I tend to forget whitespace :)

 In the documentation you refer to this feature as inline functions.
 I think this might be mixing up the terminology ... although the code
 refers to inline handlers internally, the word inline doesn't
 appear in the user-facing documentation for the DO command.  Instead
 they are referred to as anonymous code blocks.  I think it would
 improve consistency if the PL/Perl mention used the same term.

I can accept that argument. The attached patch modifies the documentation, and
fixes another inconsistency I found.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE FUNCTION replaceablefuncname/r
*** 59,69 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. A PL/Perl function must
!always return a scalar value.  You can return more complex structures
!(arrays, records, and sets) by returning a reference, as discussed below.
!Never return a list.
/para
  
note
--- 59,81 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
+ 
+PL/Perl also supports anonymous code blocks called with the
+xref linkend=sql-do endterm=sql-do-title
+statement:
+ 
+ programlisting
+ DO $$
+ # PL/Perl function body
+ $$ LANGUAGE plperl;
+ /programlisting
+ 
 The body of the function is ordinary Perl code. In fact, the PL/Perl
!glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!return a value; PL/Perl functions created with CREATE FUNCTION must always
!return a scalar value. You can return more complex structures (arrays,
!records, and sets) by returning a reference, as discussed below.  Never
!return a list.
/para
  
note
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...a955581 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***
*** 0 
--- 1,7 
+ DO $$
+   $a = 'This is a test';
+   elog

Re: [HACKERS] plperl and inline functions -- first draft

2009-11-12 Thread Joshua Tolley
On Fri, Nov 06, 2009 at 09:53:20PM -0500, Andrew Dunstan wrote:
 Joshua Tolley wrote:
  I looked through the
 regression tests and didn't find any that used plperl -- should we add one 
 for
 this (or for this and all kinds of other stuff)? Is there some way to make
 running the regression test conditional on having built --with-perl in the
 first place?

 Look in src/pl/plperl/{sql,expected}

 cheers

 andrew

FWIW, I've added this to the upcoming commitfest page.

https://commitfest.postgresql.org/action/patch_view?id=206

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


signature.asc
Description: Digital signature


Re: [HACKERS] plperl and inline functions -- first draft

2009-11-09 Thread Joshua Tolley
On Fri, Nov 06, 2009 at 09:53:20PM -0500, Andrew Dunstan wrote:


 Joshua Tolley wrote:
  I looked through the
 regression tests and didn't find any that used plperl -- should we add one 
 for
 this (or for this and all kinds of other stuff)? Is there some way to make
 running the regression test conditional on having built --with-perl in the
 first place?

   

 Look in src/pl/plperl/{sql,expected}

Ok, updated patch attached. As far as I know, this completes all outstanding
issues:

1) weird comment in plperl.c is corrected and formatted decently
2) plperlu vs. plperl actually works (thanks again, Andrew)
3) docs included
4) regression tests included

Some items of note include that this makes the regression tests add not only
plperl to the test database but also plperlu, which is a new thing. I can't
see why this might cause problems, but thought I'd mention it. The tests
specifically try to verify that plperl doesn't allow 'use Data::Dumper', and
plperlu does. Since Data::Dumper is part of perl core, that seemed safe, but
it is another dependency, and perhaps we don't want to do that. If not, is
there some other useful way of testing plperlu vs. plperl, and does it really
matter?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..d4b2816 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** CREATE FUNCTION replaceablefuncname/r
*** 59,64 
--- 59,75 
  # PL/Perl function body
  $$ LANGUAGE plperl;
  /programlisting
+ 
+PL/Perl also supports inline functions called with the 
+xref linkend=sql-do endterm=sql-do-title
+statement:
+ 
+ programlisting
+ DO $$
+ # PL/Perl function body
+ $$ LANGUAGE plperl;
+ /programlisting
+ 
 The body of the function is ordinary Perl code. In fact, the PL/Perl
 glue code wraps it inside a Perl subroutine. A PL/Perl function must
 always return a scalar value.  You can return more complex structures
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...3706018 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***
*** 0 
--- 1,10 
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ DO $$ elog(NOTICE, This is plperlu); $$ LANGUAGE plperlu;
+ NOTICE:  This is plperlu
+ DO $$ use Data::Dumper; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
+ DO $$ use Data::Dumper; $$ LANGUAGE plperlu;
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59

[HACKERS] plperl and inline functions -- first draft

2009-11-05 Thread Joshua Tolley
I've been trying to make pl/perl support 8.5's inline functions, with the
attached patch. The basics seem to be there, with at least one notable
exception, namely that plperl functions can do stuff only plperlu should do. I
presume this is because I really don't understand yet how plperl's trusted
interpreter initialization works, and have simply copied what looked like
important stuff from the original plperl call handler. I tested with this to
prove it:

DO $$ qx{touch test.txt}; $$ language plperl;

This works both with plperl and plperlu. Hints, anyone? Comments?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler _null_ plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 
  DATA(insert ( plpgsql		t t plpgsql_call_handler plpgsql_inline_handler plpgsql_validator $libdir/plpgsql _null_ ));
  DATA(insert ( pltcl		t t pltcl_call_handler _null_ _null_ $libdir/pltcl _null_ ));
  DATA(insert ( pltclu		f f pltclu_call_handler _null_ _null_ $libdir/pltcl _null_ ));
! DATA(insert ( plperl		t t plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
! DATA(insert ( plperlu		f f plperl_call_handler plperl_inline_handler plperl_validator $libdir/plperl _null_ ));
  DATA(insert ( plpythonu	f f plpython_call_handler _null_ _null_ $libdir/plpython _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..33ede1b 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** static plperl_call_data *current_call_da
*** 144,149 
--- 144,150 
   * Forward declarations
   **/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,870 
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
--- 863,872 
  
  
  /*
!  * plperl_call_handler and plperl_inline_handler are the only
!  * externally-visible parts of the plperl call interface.  The Postgres function
!  * and trigger managers call plperl_call_handler to execute a perl function, and
!  * call plperl_inline_handler to execute plperl code in a DO statement.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
*** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 
--- 897,952 
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+ FunctionCallInfoData fake_fcinfo;
+ FmgrInfo flinfo;
+ plperl_proc_desc desc;
+ HeapTuple	langTup;
+ Form_pg_language langStruct;
+ 
+ MemSet(fake_fcinfo, 0, sizeof(fake_fcinfo));
+ MemSet(flinfo, 0, sizeof(flinfo));  
+ MemSet(desc, 0, sizeof(desc));
+ fake_fcinfo.flinfo = flinfo;
+ flinfo.fn_oid = InvalidOid;  
+ flinfo.fn_mcxt = CurrentMemoryContext; 
+ 
+ desc.proname = ;
+ desc.fn_readonly = 0;
+ 
+ /
+ * Lookup the pg_language tuple by Oid
+ /
+ langTup = SearchSysCache(LANGOID,
+ ObjectIdGetDatum(codeblock-langOid),
+ 0, 0, 0);
+ if (!HeapTupleIsValid(langTup))
+ {
+ elog(ERROR, cache lookup failed for language with OID %d,
+ codeblock-langOid);
+ }
+ langStruct = (Form_pg_language) GETSTRUCT

Re: [HACKERS] plperl and inline functions -- first draft

2009-11-05 Thread Joshua Tolley
On Thu, Nov 05, 2009 at 05:51:45PM -0500, Andrew Dunstan wrote:
 Joshua Tolley wrote:
 I've been trying to make pl/perl support 8.5's inline functions, with the
 attached patch. 

 Wow, this is the second time this week that people have produced patches  
 for stuff I was about to do. Cool!

Well, I warmed up with PL/LOLCODE :)

 The basics seem to be there, with at least one notable
 exception, namely that plperl functions can do stuff only plperlu should do. 
 I
 presume this is because I really don't understand yet how plperl's trusted
 interpreter initialization works, and have simply copied what looked like
 important stuff from the original plperl call handler. 


 I'll check that out.

Many thanks.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Feature Suggestion: PL/Js

2009-10-07 Thread Joshua Tolley
On Wed, Oct 07, 2009 at 10:22:02AM -0400, Alvaro Herrera wrote:
 Kiswono Prayogo escribió:
  by using latest v8 engine from google, is it possible to build PL/Js
  just like other PL in Postgre? such as PL/PHP
  what should i learn if i want to build PL/Js?
 
 I think Josh Tolley has some slides on how we built PL/LOLCODE that
 could prove useful.

Said slides are available here:
http://www.pgcon.org/2009/schedule/events/159.en.html

I hope they can be useful.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Feature Suggestion: PL/Js

2009-10-07 Thread Joshua Tolley
On Wed, Oct 07, 2009 at 11:29:15AM -0400, Alvaro Herrera wrote:
 Joshua Tolley escribió:
  On Wed, Oct 07, 2009 at 10:22:02AM -0400, Alvaro Herrera wrote:
   Kiswono Prayogo escribió:
by using latest v8 engine from google, is it possible to build PL/Js
just like other PL in Postgre? such as PL/PHP
what should i learn if i want to build PL/Js?
   
   I think Josh Tolley has some slides on how we built PL/LOLCODE that
   could prove useful.
 
 Huh, I didn't mean we built but he built!

I didn't feel like you were stealing credit. I certainly couldn't have built
it, such as it is, without support from -hackers. :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] WIP - syslogger infrastructure changes

2009-09-26 Thread Joshua Tolley
On Sat, Sep 26, 2009 at 11:43:46AM -0400, Tom Lane wrote:
 complete but more complex solution.  (dup2 works on Windows, no?)

Unless I'm misreading syslogger.c, dup2() gets called on every platform.

I've started the wiki page in question:
http://wiki.postgresql.org/wiki/Logging_Brainstorm

It doesn't have anything linking to it right now, which might be a bad thing.

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


signature.asc
Description: Digital signature


Re: [HACKERS] syslog_line_prefix

2009-09-25 Thread Joshua Tolley
On Fri, Sep 25, 2009 at 03:19:36PM -0400, Tom Lane wrote:
 However, I don't think I actually believe the premise of this patch,
 which is that sending log information to both stderr and syslog is
 a useful thing to do

Actually the thing I want is to be able to send some stuff to syslog, and some
to a file, and other stuff to another file. This patch doesn't do all that,
but lays the necessary groundwork.

For instance, the various applications that parse query output all require
specific log line formats and various other configurations to be able to
understand our logs, and even then still have problems dealing with multi-line
entries. This is a pain. Such applications should be able to have their own
machine-readable logs, like csvlog. Unfortunately csvlogs are almost entirely
unreadable by humans, so I'd also like to have a human readable log somewhere.
These two logs need not necessarily contain the same information.

Loads of people seem to want to be able to have separate per-database log
files, which something like this could also allow.

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


signature.asc
Description: Digital signature


Re: [HACKERS] syslog_line_prefix

2009-09-25 Thread Joshua Tolley
On Fri, Sep 25, 2009 at 04:18:08PM -0400, Robert Haas wrote:
  I would even more like to have some things send to CSV and some things
  sent to text.
 
 This patch won't help, then.

No, it won't, but as said before, it lays the groundwork, namely letting the
syslogger know things about the log messages it gets (rather than just having
an opaque string), and route messages various places, accordingly.

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


signature.asc
Description: Digital signature


Re: [HACKERS] syslog_line_prefix

2009-09-25 Thread Joshua Tolley
On Fri, Sep 25, 2009 at 05:04:45PM -0400, Robert Haas wrote:
 On Fri, Sep 25, 2009 at 4:58 PM, Joshua Tolley eggyk...@gmail.com wrote:
  On Fri, Sep 25, 2009 at 03:19:36PM -0400, Tom Lane wrote:
  However, I don't think I actually believe the premise of this patch,
  which is that sending log information to both stderr and syslog is
  a useful thing to do
 
  Actually the thing I want is to be able to send some stuff to syslog, and 
  some
  to a file, and other stuff to another file. This patch doesn't do all that,
  but lays the necessary groundwork.
 
 I don't think it does anything of the sort.  Getting to that point by
 adding GUCs is quickly going to produce obviously unacceptable numbers
 of GUCs.  Or if it isn't, then I'd like to hear the whole designed
 laid out.  I think Magnus's idea of a separate config file is much
 more likely to be succesful than what we have here, but that too will
 require some design that hasn't been done yet.

This doesn't approach the issue of how precisely you'd configure a more
complex logging scheme, because clearly that will be complex. The whole
purpose here is to let the syslogger know stuff about the log messages it
gets, so it can act on them intelligently.

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


signature.asc
Description: Digital signature


Re: [HACKERS] syslog_line_prefix

2009-09-25 Thread Joshua Tolley
On Fri, Sep 25, 2009 at 05:04:45PM -0400, Robert Haas wrote:
 On Fri, Sep 25, 2009 at 4:58 PM, Joshua Tolley eggyk...@gmail.com wrote:
  On Fri, Sep 25, 2009 at 03:19:36PM -0400, Tom Lane wrote:
  However, I don't think I actually believe the premise of this patch,
  which is that sending log information to both stderr and syslog is
  a useful thing to do
 
  Actually the thing I want is to be able to send some stuff to syslog, and 
  some
  to a file, and other stuff to another file. This patch doesn't do all that,
  but lays the necessary groundwork.
 
 I don't think it does anything of the sort.  Getting to that point by
 adding GUCs is quickly going to produce obviously unacceptable numbers
 of GUCs.  Or if it isn't, then I'd like to hear the whole designed
 laid out.  I think Magnus's idea of a separate config file is much
 more likely to be succesful than what we have here, but that too will
 require some design that hasn't been done yet.
 
 ...Robert

Having just sent two messages to the discussion about the wrong patch, I'll
apologize, and shut up now :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] Feedback on getting rid of VACUUM FULL

2009-09-17 Thread Joshua Tolley
On Wed, Sep 16, 2009 at 09:48:20PM -0400, Tom Lane wrote:
 Seems like there would
 be lots of situations where short exclusive-lock intervals could be
 tolerated, even though not long ones.  So that's another argument
 for being able to set an upper bound on how many tuples get moved
 per call.

Presumably this couldn't easily be an upper bound on the time spent moving
tuples, rather than an upper bound on the number of tuples moved?

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


signature.asc
Description: Digital signature


Re: [HACKERS] WIP: generalized index constraints

2009-09-15 Thread Joshua Tolley
On Tue, Sep 15, 2009 at 11:21:14PM +1000, Brendan Jurd wrote:
 2009/9/15 Jeff Davis pg...@j-davis.com:
  Attached is the latest version.
 
 
 The new error message for a conflict is:
 
 ERROR:  index constraint violation detected
 DETAIL:  tuple conflicts with existing data
 
 How about also including the name of the constraint (or index) that
 was violated?  I could imagine this error message being frustrating
 for someone who had a table with multiple index constraints, as they
 wouldn't know which one had raised the conflict.

Perhaps the tuple that caused the violation as well, like UNIQUE index
violations already do? Even if we know what constraint has been tripped, we
might not know what value did it.

j...@josh# create table a (a integer);
j...@josh*# create unique index a_unique on a (a);
j...@josh*# insert into a values (1), (2), (3);
j...@josh*# insert into a values (8), (3), (4);
ERROR:  duplicate key value violates unique constraint a_unique
DETAIL:  Key (a)=(3) already exists.

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


signature.asc
Description: Digital signature


Re: [HACKERS] WIP: generalized index constraints

2009-09-15 Thread Joshua Tolley
On Tue, Sep 15, 2009 at 05:52:35PM -0400, Tom Lane wrote:
 Jeff Davis pg...@j-davis.com writes:
  On Tue, 2009-09-15 at 14:42 -0700, Jeff Davis wrote:
  operator constraints
  operator exclusion constraints
  operator conflict constraints
  conflict operator constraints
  operator index constraints
  index constraints
  generalized index constraints
  something else?
 
  Just to add a couple more permutations of Robert Haas's suggestions:
 
   exclusion operator constraints
   exclusive operator constraints
 
 To my ear, operator exclusion constraints or exclusive operator
 constraints seem reasonable; the other permutations of that phrase
 simply aren't good English.

I was having a hard time coming up with a name that was adequately
short-and-sweet, and still conveyed the idea of both operator and index,
which seems important so as to designate between these and the constraints
we've had all along. Perhaps indexed operator constraints?

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


signature.asc
Description: Digital signature


Re: [HACKERS] autovacuum_max_workers docs

2009-09-13 Thread Joshua Tolley
On Sun, Sep 13, 2009 at 10:54:21PM +0300, Peter Eisentraut wrote:
 On fre, 2009-09-11 at 07:39 -0600, Joshua Tolley wrote:
 While your discovery is accurate and the change makes it consistent with
 other similar parameters, note that the previous wording is also
 completely correct.  This while way of phrasing things is suboptimal.
 
 I've committed it anyway for now.

Although I understand we also need a way to demonstrate which options can be
set interactively, and which can't, I'd love to see changing this option
requires a restart or ... a reload, if only because I'm always interpreting
the docs wrong in that respect. That said, if I ever come up with woeding I'm
especially proud of, I'll submit a less-trivial patch.

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


signature.asc
Description: Digital signature


[HACKERS] autovacuum_max_workers docs

2009-09-11 Thread Joshua Tolley
The current docs for autovacuum_max_workers suggest it should be modifiable
with a reload, unless I'm reading in awfully silly ways this morning (which
isn't entirely out of the question). Anyway, in the 8.3.7 and 8.5devel
instances I've tried, autovacuum_max_workers can only be set at server start.
I propose this:

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7c82835..26a8ddf 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3589,8 +3589,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH 
csv;
para
 Specifies the maximum number of autovacuum processes (other than the
 autovacuum launcher) which may be running at any one time.  The default
-is three.  This parameter can only be set in
-the filenamepostgresql.conf/ file or on the server command line.
+is three.  This parameter can only be set at server start.
/para
   /listitem
  /varlistentry

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


signature.asc
Description: Digital signature


Re: [HACKERS] Implementation of GROUPING SETS (T431: Extended grouping capabilities)

2009-09-02 Thread Joshua Tolley
On Thu, Sep 03, 2009 at 01:19:25AM +0400, Олег Царев wrote:
 After week-lengthed investigation, now i 'm sure - my level of
 qualification not enough for implementation task GROUPING SETS.
 I require documentation about the executor and the planner, i can't
 understand scheme of work by source code.
 Many code, many cases, but very little information what is it and
 how thos work. May be i stupid.

I doubt you're stupid; a stupid person wouldn't know what GROUPING SETS meant,
wouldn't bother finding out, and certainly wouldn't bother trying to implement
it. It's very helpful that you've let us know you're not working on it. That
way Pavel, if he finds he has time and interest, or someone else, can work on
it without fear of conflicting with what you're doing. Thanks for your work;
please don't get discouraged!

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


signature.asc
Description: Digital signature


Re: [HACKERS] hba load error and silent mode

2009-08-24 Thread Joshua Tolley
On Mon, Aug 24, 2009 at 02:57:02PM +0200, Magnus Hagander wrote:
 On Mon, Aug 24, 2009 at 14:39, Andrew Dunstanand...@dunslane.net wrote:
 
  Last night I had to deal with a puzzled user of version 8.4  who found
  postgres refused to start but didn't log any error.  It turned out that
  there was an error in the pg_hba.conf file, and the client was running in
  silent mode (the SUSE default).
 
  This seems like a bug, and it's certainly not very postgres-like behaviour.
 
  Can we move the call to hba_load() in postmaster.c down a bit so it occurs
  after the SysLogger is set up? ISTM we really need an absolute minimum of
  code between the call to pmdaemonize() and SysLogger_Start().
 
 I can see other reasons that this would be good, so +1 from me unless
 there is any specific reason we can't start it earlier.

+1 from me, too, under the same condition.  Since logging in really
interesting ways depends on a separate process, any logging before then will
be abnormal, and any logs we create will probably show up in a relatively
unexpected place. The Principle of Least Surprise suggests we minimize that
possibility.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Hot standby?

2009-08-12 Thread Joshua Tolley
On Tue, Aug 11, 2009 at 11:19:18PM -0400, Robert Haas wrote:
 On Tue, Aug 11, 2009 at 9:44 PM, Greg Starkgsst...@mit.edu wrote:
  On Tue, Aug 11, 2009 at 10:13 PM, Robert Haasrobertmh...@gmail.com wrote:
  On Tue, Aug 11, 2009 at 4:07 PM, Josh Berkusj...@agliodbs.com wrote:
  So really, the streaming replication patch should be called hot
  standby,
 
  No.  AIUI, hot standby means that when your primary falls over, the
  secondary automatically promotes itself and takes over.
 
  No! This is *not* what hot standby means, at least not in the Oracle 
  world.
 
 I'm perplexed by this.  For example: http://en.wikipedia.org/wiki/Hot_standby
 
 Admittedly, wikipedia is not an authoritative source, but I've always
 understood cold/warm/hot just as Peter described them upthread.  Cold
 means it's on the shelf.  Warm means it's plugged in, but you have to
 have to do something to get it going.  Hot means it just takes over
 when needed.

After all this, perhaps we can at least conclude that calling it cold,
warm, or hot anything is confusing, because no one can agree on what that
means. I propose we leave off finding a naming that includes temperature.

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


signature.asc
Description: Digital signature


Re: [HACKERS] Review: Patch for contains/overlap of polygons

2009-08-09 Thread Joshua Tolley
On Sun, Aug 09, 2009 at 02:29:44PM -0400, Robert Haas wrote:
 On Sun, Aug 9, 2009 at 2:20 PM, Bruce Momjianbr...@momjian.us wrote:
  This is a nice section layout for a patch review report --- should we
  provide an email template like this one for reviewers to use?
 
 We could, but it might be over-engineering.  Those particular section
 headers might not be applicable to someone else's review.

I've just added a link to this email to the Reviewing a Patch wiki page
(http://wiki.postgresql.org/wiki/Reviewing_a_Patch). Do with it as you see fit
:)

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-08-04 Thread Joshua Tolley
On Tue, Aug 04, 2009 at 01:28:00PM -0400, Stephen Frost wrote:
 Thanks to Joshua, there weren't really many changes I found for the
 docs.  Here they are anyway:

Yay, I was useful! :)

 How about:
 
 Replaces current privileges with the default privileges, as set using
 xref linkend=sql-alterschema endterm=sql-alterschema-title, for
 this object type in its current schema.
 The literalWITH GRANT OPTION/literal parameter is not applicable
 because it is copied from default privileges.
 Note: This can emphasis role=boldrevoke/emphasis privileges!  This
 will clear all existing privileges first!  If no default privilege has
 been set, the object will have all privileges revoked!
 
 Otherwise, I thought the docs looked pretty good.

No complaints here, FWIW.

- Josh


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER TABLE ... ALTER COLUMN ... SET DISTINCT

2009-08-02 Thread Joshua Tolley
On Sun, Aug 02, 2009 at 01:23:27PM -0400, Tom Lane wrote:
 There wasn't any response to this comment:
 
 marcin mank marcin.m...@gmail.com writes:
  ALTER COLUMN SET DISTINCT
  feels like adding a unique constraint.
 
  ALTER COLUMN SET STATISTICS DISTINCT ?
  ALTER COLUMN SET STATISTICS NDISTINCT ?
 
 I don't want to add a new keyword, but SET STATISTICS DISTINCT would
 be an easy change.  Comments?

+1

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 11:14:19AM +0200, Petr Jelinek wrote:
 Joshua Tolley wrote:
 Am I the only one that gets this on make check, with this version (from
 src/test/regress/log/initdb.log):

 selecting default shared_buffers ... 32MB
 creating configuration files ... ok
 creating template1 database in 
 /home/josh/devel/pgsrc/pg85/src/test/regress/./tmp_check/data/base/1 ... 
 FATAL:  relation pg_namespace_default_acl already exists
 child process exited with exit code 1
 initdb: data directory 
 /home/josh/devel/pgsrc/pg85/src/test/regress/./tmp_check/data not removed 
 at user's request
   
 Certainly never happened to me. Are you using any special parameters or  
 something (altho I don't have the slightest idea what could cause that)  
 ? I run make check on patches using gcc under debian and msvc on vista  
 before sending them.

I figured as much. I can't seem to get past this, despite a make distclean.
Suggestions, anyone?

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 03:50:06PM -0400, Andrew Dunstan wrote:
 Joshua Tolley wrote:
 I figured as much. I can't seem to get past this, despite a make distclean.
 Suggestions, anyone?

 try a fresh checkout and reapply the patch?

[ a couple git clean, git reset, make clean, etc. commands later... ]

Yeah, well, it works now. What's more, the problems I had with make check the
first time I tried this are no longer.

I've done all the looking I can think to do at the patch in its existing form,
and don't have any complaints. I realize, though, that there are open
questions about how this should work with, given the GRANT ON ALL patch. I'm
not sure I have comments in that regard, but I'll try to come up with some, or
at least to convince myself I don't have any for a better reason than just
that I wouldn't know what I was talking about. In the meantime, I think this
one is ready to be marked as ... something else. Ready for committer?

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote:
 while writing some basic docs I found bug in dependency handling when  
 doing SET on object type that already had some default privileges.  
 Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT  
 OPTION behaves like REVOKE now). And there is also initial version of  
 those basic docs included (but you have to pardon my english as I didn't  
 pass it to Stephen for proofreading due to discovery of that bug).

Immediately after concluding I was done with my review, I realized I'd
completely forgotten to look at the docs. I've made a few changes based solely
on my opinions of what sounds better and what's more consistent with the
existing documentation. Do with them as you see fit. :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-25 Thread Joshua Tolley
On Sat, Jul 25, 2009 at 08:41:12PM -0400, Robert Haas wrote:
 On Sat, Jul 25, 2009 at 8:39 PM, Joshua Tolleyeggyk...@gmail.com wrote:
  On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote:
  while writing some basic docs I found bug in dependency handling when
  doing SET on object type that already had some default privileges.
  Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT
  OPTION behaves like REVOKE now). And there is also initial version of
  those basic docs included (but you have to pardon my english as I didn't
  pass it to Stephen for proofreading due to discovery of that bug).
 
  Immediately after concluding I was done with my review, I realized I'd
  completely forgotten to look at the docs. I've made a few changes based 
  solely
  on my opinions of what sounds better and what's more consistent with the
  existing documentation. Do with them as you see fit. :)
 
 Did you intend to attach something to this email?
 
 ...Robert

Well, yes, now that you mention it :) Trying again...

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 34679d8..3eb92a4 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -3130,6 +3130,70 @@
  /sect1
 
 
+ sect1 id=catalog-pg-namespace-default-acl
+  titlestructnamepg_namespace_default_acl/structname/title
+
+  indexterm zone=catalog-pg-namespace-default-acl
+   primarypg_namespace_default_acl/primary
+  /indexterm
+
+  para
+   The catalog structnamepg_namespace_default_acl/ stores default
+   privileges for newly created objects inside the schema.
+  /para
+
+  table
+   titlestructnamepg_namespace/ Columns/title
+
+   tgroup cols=4
+thead
+ row
+  entryName/entry
+  entryType/entry
+  entryReferences/entry
+  entryDescription/entry
+ /row
+/thead
+
+tbody
+ row
+  entrystructfielddefaclnamespace/structfield/entry
+  entrytypeoid/type/entry
+  entryliterallink linkend=catalog-pg-namespacestructnamepg_namespace/structname/link.oid/literal/entry
+  entryThe OID of the namespace associated with this entry/entry
+ /row
+
+ row
+  entrystructfielddefaclgrantobjtype/structfield/entry
+  entrytypechar/type/entry
+  entry/entry
+  entry
+   literalr/ = table, literalv/ = view,
+   literalf/ = function, literalS/ = sequence
+  /entry
+ /row
+
+ row
+  entrystructfielddefacllist/structfield/entry
+  entrytypeaclitem[]/type/entry
+  entry/entry
+  entry
+   Access privileges that the object should have on creation.
+   This is NOT a mask, it's exactly what the object will get.
+   See
+   xref linkend=sql-alterschema endterm=sql-alterschema-title,
+   xref linkend=sql-grant endterm=sql-grant-title and
+   xref linkend=sql-revoke endterm=sql-revoke-title
+   for details.
+  /entry
+ /row
+/tbody
+   /tgroup
+  /table
+
+ /sect1
+
+
  sect1 id=catalog-pg-opclass
   titlestructnamepg_opclass/structname/title
 
diff --git a/doc/src/sgml/ref/alter_schema.sgml b/doc/src/sgml/ref/alter_schema.sgml
index 2458d19..62f4c2a 100644
--- a/doc/src/sgml/ref/alter_schema.sgml
+++ b/doc/src/sgml/ref/alter_schema.sgml
@@ -23,18 +23,46 @@ PostgreSQL documentation
 synopsis
 ALTER SCHEMA replaceablename/replaceable RENAME TO replaceablenewname/replaceable
 ALTER SCHEMA replaceablename/replaceable OWNER TO replaceablenewowner/replaceable
+
+ALTER SCHEMA replaceablename/replaceable { SET | ADD } DEFAULT PRIVILEGES { { ON default_privileges 
+	TO { [ GROUP ] replaceable class=PARAMETERrolename/replaceable | PUBLIC } [, ...] [ WITH GRANT OPTION ] } [AND ...] } [...]
+
+where replaceable class=PARAMETERdefault_privileges/replaceable is:
+
+{ { TABLE | VIEW } { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
+[,...] | ALL [ PRIVILEGES ] } |
+  SEQUENCE { { USAGE | SELECT | UPDATE }
+[,...] | ALL [ PRIVILEGES ] } |
+  FUNCTION { EXECUTE | ALL [ PRIVILEGES ] } }
+  
+ALTER SCHEMA replaceablename/replaceable DROP DEFAULT PRIVILEGES { { ON drop_default_privileges
+	FROM { [ GROUP ] replaceable class=PARAMETERrolename/replaceable | PUBLIC } [, ...] } [AND ...] } [...]
+
+where replaceable class=PARAMETERdrop_default_privileges/replaceable is:
+
+{ { TABLE | VIEW } [ GRANT OPTION FOR ]
+	{ { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
+[,...] | ALL [ PRIVILEGES ] } |
+  SEQUENCE [ GRANT OPTION FOR ] 
+	{ { USAGE | SELECT | UPDATE } [,...] | ALL [ PRIVILEGES ] } |
+  FUNCTION [ GRANT OPTION FOR ] 
+	{ EXECUTE | ALL [ PRIVILEGES ] } }
 /synopsis
  /refsynopsisdiv
 
- refsect1
+ refsect1 id=sql-alterschema-description
   titleDescription/title
 
   para
-   commandALTER SCHEMA/command changes the definition of a schema.
+   You must own the schema to use commandALTER SCHEMA/.
   /para
 
+ /refsect1
+
+ refsect1 id=sql-alterschema-description

Re: [HACKERS] When is a record NULL?

2009-07-24 Thread Joshua Tolley
On Thu, Jul 23, 2009 at 06:46:25PM -0700, David E. Wheeler wrote:
 Yes, but given that the standard says that `ROW(1, NULL)` is NULL, then I 
 would expect it to be NOT DISTINCT from `ROW(2, NULL)`.

Wait, didn't we decide upthread that the standard said ROW(1, NULL) isn't
NULL?

(From Tom):
 This is per SQL standard.  IS NULL is true if *all* the record's  

 fields are null; IS NOT NULL is true if *none* of them are.   


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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-24 Thread Joshua Tolley
On Sun, Jul 19, 2009 at 06:13:32PM +0200, Petr Jelinek wrote:
 Hello,

 while writing some basic docs I found bug in dependency handling when  
 doing SET on object type that already had some default privileges.  
 Attached patch fixes it, it also fixes thinko in parser (DROPing GRANT  
 OPTION behaves like REVOKE now). And there is also initial version of  
 those basic docs included (but you have to pardon my english as I didn't  
 pass it to Stephen for proofreading due to discovery of that bug).

Am I the only one that gets this on make check, with this version (from
src/test/regress/log/initdb.log):

selecting default shared_buffers ... 32MB
creating configuration files ... ok
creating template1 database in 
/home/josh/devel/pgsrc/pg85/src/test/regress/./tmp_check/data/base/1 ... FATAL: 
 relation pg_namespace_default_acl already exists
child process exited with exit code 1
initdb: data directory 
/home/josh/devel/pgsrc/pg85/src/test/regress/./tmp_check/data not removed at 
user's request

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-22 Thread Joshua Tolley
On Wed, Jul 22, 2009 at 08:54:19PM -0400, Robert Haas wrote:
 I am gathering that this patch is still a bit of a WIP. 

I don't consider it a WIP. Petr posted a patch a couple of days ago, but I've
not been able to verify its changes or perform some additional testing I had
in mind, because of my own schedule. I probably should have made that clear a
while ago. I consider the ball entirely in my court on this one, and plan to
complete my review using the latest version of the patch as soon as my time
permits; I expect this will happen before Saturday.

 I am also a bit unsure as to whether Josh Tolley is still conducting a
 more in-depth review.  Josh?

Yes, I am, but if you've read this far you know that already :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Joshua Tolley
On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote:
 Hello,

 this is first public version of our DefaultACLs patch as described on  
 http://wiki.postgresql.org/wiki/DefaultACL .

I've been asked to review this. I can't get it to build, because of a missing
semicolon on line 1608. I fixed it in my version, and it applied cleanly to
head (with some offset hunks in gram.y). I've not yet finished building and
testing; results to follow later.

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] DefaultACLs

2009-07-17 Thread Joshua Tolley
On Tue, Jul 14, 2009 at 11:10:00PM +0200, Petr Jelinek wrote:
 Hello,

 this is first public version of our DefaultACLs patch as described on  
 http://wiki.postgresql.org/wiki/DefaultACL .

Ok, here's my first crack at a comprehensive review. There's more I need to
look at, eventually. Some of these are very minor stylistic comments, and some
are probably just because I've much less of a clue, in general, than I'd like
to think I have.

First, as you've already pointed out, this needs documentation. 

Once I added the missing semicolon mentioned earlier, it applies and builds
fine. The regression tests, however, seem to assume that they'll be run as the
postgres user, and the privileges test failed. Here's part of a diff between
expected/privileges.out and results/privileges.out as an example of what I
mean:

  ALTER SCHEMA regressns DROP DEFAULT PRIVILEGES ON TABLE ALL FROM
regressuser2;
***
*** 895,903 
  (1 row)
  
  SELECT relname, relacl FROM pg_class WHERE relname = 'acltest2';
!  relname  |relacl
! --+--
!  acltest2 | {postgres=arwdDxt/postgres,regressgroup1=r/postgres}
  (1 row)
  
  CREATE FUNCTION regressns.testfunc1() RETURNS int AS 'select 1;' LANGUAGE
sql;
--- 895,903 
  (1 row)
  
  SELECT relname, relacl FROM pg_class WHERE relname = 'acltest2';
!  relname  |  relacl  
! --+--
!  acltest2 | {josh=arwdDxt/josh,regressgroup1=r/josh}
  (1 row)
  
  CREATE FUNCTION regressns.testfunc1() RETURNS int AS 'select 1;' LANGUAGE
sql;

Very minor stylistic or comment issues:

* There's a stray newline added in pg_class.h (no other changes were made to
  that file by this patch)
* It feels to me like the comment Continue with standard grant in aclchk.c
  interrupts the flow of the code, though such a comment was likely useful
  when the patch was being written.
* pg_namespace_default_acl.h:71 should read objects stored *in* pg_class
* The comment at the beginning of InsertPgClassTuple() in catalog/heap.c
  should probably be updated to say that relation's ACLs aren't always NULL by
  default
* copy_from in gram.y got changed to to_from, but to_from isn't ever used in
  the default ACL grammar. I wondered if this was changed so you could use the
  same TO/FROM code as COPY uses, and then you decided to hardcode TO and FROM?

In my perusal of the patch, I didn't see any code that screamed at me as
though it were a bad idea; quite likely there weren't any really bad ideas but
I can't say with confidence I'd have spotted them if there were. The addition
of both the NSPDEFACLOBJ_* defines alongside the NSP_ACL_OBJ_* defines kinda
made me think there were too many sets of constants that had to be kept in
sync, but I'm not sure that's much of an issue in reality, given how unlikely
it is that schema object types to which default ACLs should apply are likely
to be added or removed.

I don't know how patches that require catalog version changes are generally
handled; should the patch include that change?

More testing to follow.

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


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] [v8.5] Security checks on largeobjects

2009-07-16 Thread Joshua Tolley
On Thu, Jul 16, 2009 at 01:49:14PM +0900, KaiGai Kohei wrote:
 Joshua,
 
 I found your name as a reviewer at the commitfest.postgresql.org.
 
 However, I don't think the initial proposal of the largeobject
 security is now on the state to be reviewed seriously.
 
 In my preference, it should be reworked based on the TOASTing
 approach as discussed in this thread previously, if we can
 find a reasonable solution for the issue I raised before.

For whatever it's worth, I consider my capability limited to making sure the
patch applies and coming up with a few interesting ways of testing it out, but
not seriously studying the code and knowing when there might be a competitive
alternative implementation. I don't yet understand the issues that have been
raised, but will quit working to review the patch if you feel it's not ready.
Thanks for letting me know; I hope a solution to the problems you've brought
up is forthcoming.

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


signature.asc
Description: Digital signature


[HACKERS] git.postgresql.org vs. REL8_1_STABLE

2009-07-10 Thread Joshua Tolley
Am I the only one having problems building 8.1 from git? (Am I the only one
building 8.1 from git?) In a clean repository, I've checked out REL8_1_STABLE,
configured with only one argument, to set --prefix, and make gives me this:

make[4]: Entering directory
`/home/josh/devel/pgsrc/pg81/src/backend/access/common'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv
-I../../../../src/include -D_GNU_SOURCE   -c -o indexvalid.o indexvalid.c
In file included from ../../../../src/include/executor/execdesc.h:19,
 from ../../../../src/include/executor/executor.h:17,
 from ../../../../src/include/executor/execdebug.h:17,
 from indexvalid.c:19:
../../../../src/include/nodes/execnodes.h:23:29: error: nodes/tidbitmap.h: No
such file or directory
In file included from ../../../../src/include/executor/execdesc.h:19,
 from ../../../../src/include/executor/executor.h:17,
 from ../../../../src/include/executor/execdebug.h:17,
 from indexvalid.c:19:
../../../../src/include/nodes/execnodes.h:934: error: expected
specifier-qualifier-list before ‘TIDBitmap’
../../../../src/include/nodes/execnodes.h:959: error: expected
specifier-qualifier-list before ‘TIDBitmap’
make[4]: *** [indexvalid.o] Error 1
make[4]: Leaving directory
`/home/josh/devel/pgsrc/pg81/src/backend/access/common'
make[3]: *** [common-recursive] Error 2
make[3]: Leaving directory `/home/josh/devel/pgsrc/pg81/src/backend/access'
make[2]: *** [access-recursive] Error 2
make[2]: Leaving directory `/home/josh/devel/pgsrc/pg81/src/backend'
make[1]: *** [all] Error 2 
make[1]: Leaving directory `/home/josh/devel/pgsrc/pg81/src'
make: *** [all] Error 2 

Indeed, src/include/nodes has no tidbitmap.h file (it shows up in
REL8_2_STABLE).

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


signature.asc
Description: Digital signature


Re: [HACKERS] [pgsql-www] commitfest.postgresql.org

2009-07-09 Thread Joshua Tolley
On Thu, Jul 09, 2009 at 02:35:04PM -0300, Dickson S. Guedes wrote:
 This could help, maybe with a RSS in that (like in git).

+1 for the RSS feed, if only because I think it sounds neat :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] *_collapse_limit, geqo_threshold

2009-07-08 Thread Joshua Tolley
On Wed, Jul 08, 2009 at 09:26:35PM -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  That was my first reaction too, but now I'm wondering whether we  
  shouldn't just do #1.  #2 is a planner hint, too, just not a very good  
  one.  If, as you suggest, it isn't actually useful, then why keep it  
  at all? (On the other hand, if someone thinks they need it, it would  
  be interesting to know the use case, and think about the best way to  
  address it.)
 
 Well, I can cite one reasonably plausible use case: when you have an
 umpteen-way join you need to execute a lot, and you don't want to pay
 for an exhaustive search, but GEQO doesn't reliably find a good plan.
 What you can do is let the system do an exhaustive search once to find
 the best plan, then you rearrange the query to specify that join order
 via JOINs, and turn off join collapsing.  Presto, good plan every time
 with very little planning time expended.

 Now, your answer will probably be that we should provide some better
 mechanism for re-using a previously identified plan structure.  No
 doubt that would be ideal, but the amount of effort required to get
 there is nontrivial, and I'm not at all convinced it would be repaid
 in usefulness.  Whereas what I describe above is something that costs
 us nearly nothing to provide.  The usefulness might be marginal too,
 but on the basis of cost/benefit ratio it's a clear win.

This sounds like planner hints to me. The argument against hinting, AIUI, is
that although the plan you've guaranteed via hints may be a good one today,
when the data change a bit your carefully crafted plan happens to become a bad
one, but you're no longer around to change the hints accordingly. Entire
stored plans, or predetermined seeds for GEQO's random number generator all
boil down to saying, I want you to use this plan henceforth and forever. 

  It occurs to me that one way to make GEQO less scary would be to take
  out the nondeterminism by resetting its random number generator for
  each query.  You might get a good plan or an awful one, but at least
  it'd be the same one each time.  DBAs like predictability.
 
  Hmm, that doesn't sound appealing to me, but I'm only a DBA at need.
 
 I was imagining a GUC that would make the reset optional, in case anyone
 really does want to have unstable plans.  I don't have much doubt about
 what typical users will prefer though.

Do we know that GEQO plans are, in reality, less stable than than usual
planner? Certainly on paper it appears they could be, but the mailing lists
are full of emails about this query's plan changed and performance suddenly
tanked; how do I fix it? so I'm unconvinced this is a problem unique to GEQO.
Which in turn boils down to we need real world data to look at.

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


signature.asc
Description: Digital signature


Re: [HACKERS] First CommitFest: July 15th

2009-07-07 Thread Joshua Tolley
On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
 On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolleyeggyk...@gmail.com wrote:
  On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
  As far as I'm aware, there's been no code
  review yet either, which would probably be a good idea.
 
  I don't have loads of time in the coming days, but IIRC I've taken a glance 
  at
  a past version of the code, and would be willing to do so again, if it would
  be useful.
 
 If you can look over it, that would be great. i'm not really qualified
 to review perl code, and we always prefer to have at least two sets of
 eyeballs on anything that we put into production for obvious reasons.

On the assumption that other folks' testing has included bug hunting and the
like, I've spent the review time I was able to muster up figuring out the code
and looking for stuff that scared me. I didn't find anything that jumped out.
I did wonder if the %ACTION hash in Handler.pm ought not perhaps include a
flag to indicate that that action needs authentication, and have the handler
take care of that instead of the individual actions. Perhaps a similar
technique could be profitably employed for the titles. Oh, and in Patch.pm,
s/with/which in patches with have been Committed.

Finally, I ran Perl::Critic, and attached an (admittedly untested) patch to
clean up the things it whined about.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/perl-lib/PgCommitFest/CommitFestTopic.pm b/perl-lib/PgCommitFest/CommitFestTopic.pm
index 3e101fc..92113ac 100644
--- a/perl-lib/PgCommitFest/CommitFestTopic.pm
+++ b/perl-lib/PgCommitFest/CommitFestTopic.pm
@@ -80,7 +80,8 @@ EOM
 sub search {
 	my ($r) = @_;
 	my $id = $r-cgi_id();
-	my $d = $r-db-select_one(EOM, $id) if defined $id;
+	my $d;
+$d = $r-db-select_one(EOM, $id) if defined $id;
 SELECT id, name FROM commitfest_view WHERE id = ?
 EOM
 	$r-error_exit('CommitFest not found.') if !defined $d;
diff --git a/perl-lib/PgCommitFest/DB.pm b/perl-lib/PgCommitFest/DB.pm
index 4719007..adeecdb 100644
--- a/perl-lib/PgCommitFest/DB.pm
+++ b/perl-lib/PgCommitFest/DB.pm
@@ -118,7 +118,7 @@ sub select_one {
 sub tidy {
 	my ($self) = @_;
 	$self-{'dbh'}-rollback() if $self-{'dirty'};
-	return undef;
+	return;
 }
 
 sub update {
diff --git a/perl-lib/PgCommitFest/Handler.pm b/perl-lib/PgCommitFest/Handler.pm
index d94e042..19c4424 100644
--- a/perl-lib/PgCommitFest/Handler.pm
+++ b/perl-lib/PgCommitFest/Handler.pm
@@ -103,9 +103,9 @@ EOM
 		$pg_login_db-disconnect;
 		if (defined $u) {
 			my $random_bits;
-			open(RANDOM_BITS, '/dev/urandom') || die /dev/urandom: $!;
-			sysread(RANDOM_BITS, $random_bits, 16);
-			close(RANDOM_BITS);
+			open(my $RANDOM_BITS, '', '/dev/urandom') || die /dev/urandom: $!;
+			sysread($RANDOM_BITS, $random_bits, 16);
+			close($RANDOM_BITS);
 			my $session_cookie = unpack(H*, $random_bits);
 			$r-db-insert('session', { 'id' = $session_cookie,
 'userid' = $u-{'userid'} });
diff --git a/perl-lib/PgCommitFest/Patch.pm b/perl-lib/PgCommitFest/Patch.pm
index fe9a720..aebec55 100644
--- a/perl-lib/PgCommitFest/Patch.pm
+++ b/perl-lib/PgCommitFest/Patch.pm
@@ -161,7 +161,8 @@ sub view {
 	my ($r) = @_;
 	my $aa = $r-authenticate();
 	my $id = $r-cgi_id();
-	my $d = $r-db-select_one(EOM, $id) if defined $id;
+my $d;
+	$d = $r-db-select_one(EOM, $id) if defined $id;
 SELECT id, name, commitfest_id, commitfest, commitfest_topic_id,
 	commitfest_topic, patch_status, author, reviewers, date_closed
 FROM patch_view WHERE id = ?
diff --git a/perl-lib/PgCommitFest/Request.pm b/perl-lib/PgCommitFest/Request.pm
index de6d32f..c1042ba 100644
--- a/perl-lib/PgCommitFest/Request.pm
+++ b/perl-lib/PgCommitFest/Request.pm
@@ -22,7 +22,7 @@ $CGI::DISABLE_UPLOADS = 1;  # No need for uploads at present.
 sub new {
 	my ($class, $db) = @_;
 	my $cgi = CGI::Fast-new();
-	return undef if !defined $cgi;
+	return if !defined $cgi;
 	bless {
 		'cgi' = $cgi,
 		'control' = {},
diff --git a/perl-lib/PgCommitFest/WebControl.pm b/perl-lib/PgCommitFest/WebControl.pm
index 7443a60..11af6bd 100644
--- a/perl-lib/PgCommitFest/WebControl.pm
+++ b/perl-lib/PgCommitFest/WebControl.pm
@@ -42,7 +42,7 @@ sub choice {
 	$self-{'value_key'} = 'id' if !defined $self-{'value_key'};
 	$self-{'text_key'} = 'name' if !defined $self-{'text_key'};
 	$self-{'choice'} = $choice_list;
-	return undef;
+	return;
 }
 
 sub db_value {


signature.asc
Description: Digital signature


Re: [HACKERS] First CommitFest: July 15th

2009-07-02 Thread Joshua Tolley
On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
 As far as I'm aware, there's been no code
 review yet either, which would probably be a good idea.

I don't have loads of time in the coming days, but IIRC I've taken a glance at
a past version of the code, and would be willing to do so again, if it would
be useful.

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


signature.asc
Description: Digital signature


Re: [HACKERS] First CommitFest: July 15th

2009-07-02 Thread Joshua Tolley
On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
 On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolleyeggyk...@gmail.com wrote:
  On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
  As far as I'm aware, there's been no code
  review yet either, which would probably be a good idea.
 
  I don't have loads of time in the coming days, but IIRC I've taken a glance 
  at
  a past version of the code, and would be willing to do so again, if it would
  be useful.
 
 If you can look over it, that would be great. i'm not really qualified
 to review perl code, and we always prefer to have at least two sets of
 eyeballs on anything that we put into production for obvious reasons.

Is git://git.postgresql.org/git/pgcommitfest.git still the right place to get
the source?

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


signature.asc
Description: Digital signature


Re: [HACKERS] 8.5 development schedule

2009-07-01 Thread Joshua Tolley
On Wed, Jul 01, 2009 at 11:51:28AM -0400, Robert Haas wrote:
 Tom wrote upthread:
 # If you find yourself with nothing else to do except review a new patch
 # during beta, then sure, go do it.  But is there *really* nothing you
 # could be doing to help expedite the beta?
 
 My honest answer to this question is that I have no idea. It was
 pretty clear to me what I was supposed to be doing during CommitFest
 (reviewing patches) but a lot less clear to me what I should be doing
 during beta.  I know that there was an open items list, but it was
 never really clear to me how I should help with that. 

My feelings as well. During beta, there was clearly work for those with
experience in the areas with open items, and probably for committers
generally, but each time I reviewed the open items list I found little I could
do to help. If there's something I should have found, I'd love for someone to
point it out; in the meantime I tested betas and release candidates in
situtations common to my use of PostgreSQL, and found that it worked to my
satisfaction, after which I was left trying to figure out what to do, and
started dabbling in a patch or two that interested me.

 If a hamster
 and an elephant are trying to sit on the same bench, the hamster does
 not want the elephant to assert that he is a hamster; he wants the
 elephant to announce his choice of seat prior to putting his bottom in

Thanks, Robert -- this made me giggle :)

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


signature.asc
Description: Digital signature


Re: [HACKERS] Query progress indication - an implementation

2009-06-29 Thread Joshua Tolley
On Mon, Jun 29, 2009 at 02:07:23PM -0400, Tom Lane wrote:
 Josh Berkus j...@agliodbs.com writes:
  So, while an actual % completed indicator would be perfect, a query 
  steps completed, current step = would still be very useful and a large 
  improvement over what we have now.
 
 I think this is pretty much nonsense --- most queries run all their plan
 nodes concurrently to some extent.  You can't usefully say that a query
 is on some node, nor measure progress by whether some node is done.

What about showing the outermost node where work has started?

--
Josh / eggyknap
End Point Corp.
www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] Multi-Dimensional Histograms

2009-06-29 Thread Joshua Tolley
On Mon, Jun 29, 2009 at 10:22:15PM -0400, Robert Haas wrote:
 I'm finding myself unable to follow all the terminology on this thead.
  What's dimension reduction? 

For instance, ask a bunch of people a bunch of survey questions, in hopes of
predicting some value (for instance, whether or not these people will
die of a heart attack in the next three years). After waiting three years,
discover that some of the questions didn't really help you predict the
outcome. Remove them from the dataset. That's dimension reduction.

 What's PCA?

Principal Component Analysis,
http://en.wikipedia.org/wiki/Principal_components_analysis

--
Josh / eggyknap
End Point, Corp.
http://www.endpoint.com


signature.asc
Description: Digital signature


[HACKERS] Dtrace probes documentation

2009-05-28 Thread Joshua Tolley
The dtrace probes documentation [1] spells each probe name with dashes
(transaction-start, transaction-commit, etc.). Yet as far as I can see,
dtrace only works if you spell the probe names with double underscores
(transaction__start, transaction__commit, etc.). Why the discrepancy?
Obvious patch attached, in case this needs to be changed.

- Josh / eggyknap

1: http://www.postgresql.org/docs/8.4/static/dynamic-trace.html

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index ea8b017..672a1fe 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1070,95 +1070,95 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
tbody
 
 row
- entrytransaction-start/entry
+ entrytransaction__start/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires at the start of a new transaction.
   arg0 is the transaction id./entry
 /row
 row
- entrytransaction-commit/entry
+ entrytransaction__commit/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires when a transaction completes successfully.
   arg0 is the transaction id./entry
 /row
 row
- entrytransaction-abort/entry
+ entrytransaction__abort/entry
  entry(LocalTransactionId)/entry
  entryProbe that fires when a transaction completes unsuccessfully.
   arg0 is the transaction id./entry
 /row
 row
- entryquery-start/entry
+ entryquery__start/entry
  entry(const char *)/entry
  entryProbe that fires when the processing of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-done/entry
+ entryquery__done/entry
  entry(const char *)/entry
  entryProbe that fires when the processing of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-parse-start/entry
+ entryquery__parse__start/entry
  entry(const char *)/entry
  entryProbe that fires when the parsing of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-parse-done/entry
+ entryquery__parse__done/entry
  entry(const char *)/entry
  entryProbe that fires when the parsing of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-rewrite-start/entry
+ entryquery__rewrite__start/entry
  entry(const char *)/entry
  entryProbe that fires when the rewriting of a query is started.
   arg0 is the query string./entry
 /row
 row
- entryquery-rewrite-done/entry
+ entryquery__rewrite__done/entry
  entry(const char *)/entry
  entryProbe that fires when the rewriting of a query is complete.
   arg0 is the query string./entry
 /row
 row
- entryquery-plan-start/entry
+ entryquery__plan__start/entry
  entry()/entry
  entryProbe that fires when the planning of a query is started./entry
 /row
 row
- entryquery-plan-done/entry
+ entryquery__plan__done/entry
  entry()/entry
  entryProbe that fires when the planning of a query is complete./entry
 /row
 row
- entryquery-execute-start/entry
+ entryquery__execute__start/entry
  entry()/entry
  entryProbe that fires when the execution of a query is started./entry
 /row
 row
- entryquery-execute-done/entry
+ entryquery__execute__done/entry
  entry()/entry
  entryProbe that fires when the execution of a query is complete./entry
 /row
 row
- entrystatement-status/entry
+ entrystatement__status/entry
  entry(const char *)/entry
  entryProbe that fires anytime the server process updates its
   structnamepg_stat_activity/.structfieldcurrent_query/ status.
   arg0 is the new status string./entry
 /row
 row
- entrycheckpoint-start/entry
+ entrycheckpoint__start/entry
  entry(int)/entry
  entryProbe that fires when a checkpoint is started.
   arg0 holds the bitwise flags used to distinguish different checkpoint
   types, such as shutdown, immediate or force./entry
 /row
 row
- entrycheckpoint-done/entry
+ entrycheckpoint__done/entry
  entry(int, int, int, int, int)/entry
  entryProbe that fires when a checkpoint is complete.
   (The probes listed next fire in sequence during checkpoint processing.)
@@ -1167,20 +1167,20 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
   removed and recycled respectively./entry
 /row
 row
- entryclog-checkpoint-start/entry
+ entryclog__checkpoint__start/entry
  entry(bool)/entry
  entryProbe that fires when the CLOG portion of a checkpoint is started.
   arg0 is true for normal checkpoint, false for shutdown
   checkpoint./entry
 /row
 row
- entryclog-checkpoint-done/entry
+ entryclog__checkpoint__done/entry
  entry(bool)/entry
  entryProbe that fires when the CLOG portion of a checkpoint is
-  

Re: [HACKERS] Dtrace probes documentation

2009-05-28 Thread Joshua Tolley
On Thu, May 28, 2009 at 06:28:14PM -0400, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  The dtrace probes documentation [1] spells each probe name with dashes
  (transaction-start, transaction-commit, etc.). Yet as far as I can see,
  dtrace only works if you spell the probe names with double underscores
  (transaction__start, transaction__commit, etc.). Why the discrepancy?
 
 Read 26.4.3 and .4.  I don't know why they have this bizarre set of
 conventions, but the single-hyphen version is the spelling
 most visible to end users.

I thought it might be something like that. I've been playing with SystemTap,
and found that only the double-underscore version works for ... well, anything.
See http://blog.endpoint.com/2009/05/postgresql-with-systemtap.html for
details. Perhaps it's worth noting in the documentation that SystemTap users
will need to use the double-underscore version?

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] explain analyze rows=%.0f

2009-05-28 Thread Joshua Tolley
On Thu, May 28, 2009 at 11:12:42PM -0400, Robert Haas wrote:
 On Thu, May 28, 2009 at 11:00 PM, Euler Taveira de Oliveira
  Don't you think is too strange having, for example, 6.67 rows?
 
 No stranger than having it say 7 when it's really not.  Actually mine
 mostly come out 1 when the real value is somewhere between 0.5 and
 1.49.  :-(

+1. It would help users realize more quickly that some of the values in the
EXPLAIN output are, for instance, *average* number of rows *per iteration* of a
nested loop, say, rather than total rows found in all loops. That's an
important distinction that isn't immediately clear to the novice EXPLAIN
reader, but would become so very quickly as users tried to figure out how a
scan could come up with a fractional row.

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-26 Thread Joshua Tolley
On Tue, May 26, 2009 at 09:55:55AM -0400, Dave Page wrote:
 from the pgAdmin perspective. We
 already use libxml2, but JSON would introduce another dependency for
 us.

...and using XML introduces a dependency for those that apps that don't already
use some XML parser. I realize that since the pool of apps that care to
mechanically parse EXPLAIN output is small, it wouldn't necessarily be a big
deal to hand each of them a new dependency in the form of a parser for XML,
JSON, etc. But we know the least common denominator is to return a set of
tuples; let's make sure that really is unworkable before forcing even that
dependency. 

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-25 Thread Joshua Tolley
On Mon, May 25, 2009 at 07:14:56AM -0400, Robert Haas wrote:
 Many people who responded to this
 thread were fine with the idea of some sort of options syntax, but we
 had at least four different proposals for how to implement it:
 
 Robert Haas: EXPLAIN (foo 'bar', baz 'bletch', ...) query
 Pavel Stehule: explain_query(query, options...) [exact format of
 options not specified]
 Andrew Dunstan:  SET explain_format = 'foo, baz'; EXPLAIN query
 Josh Tolley: EXPLAIN (foo, baz, ...) query [also suggested by me as an
 idea I rejected]

I hadn't actually caught that there were two ideas on the table with syntax
similar to EXPLAIN (...) query, and don't mean to champion either of the
two specifically (at least until I've read closely enough to note the
difference). I just kinda liked the EXPLAIN (some options of some sort)
query syntax better than other proposals. 

That said, I think I'm changing my vote in favor of Pavel. It's my guess that
some variant of his version would be the easiest to make compliant with the
bit I'm most interested in, which is not being limited to, say,
XML/JSON/YAML/etc. in the output. Applications that use PostgreSQL will of
necessity need to know how to handle data presented in tables, so let's
present our explain results as a table. As has been said in other branches of
this thread, that way we don't force applications also to support
XML/JSON/YAML/etc. We might consider providing functions to convert the
tabular result to one or more of those formats, but at its inception, the data
should live as tuples in a relation.

In other messages, I've advocated actually inserting the data into a table. I
think that was a mistake. Who makes the table? What's it called? What schema
is it in? Who cleans it up when we're done with it? ...etc. I'd much rather
see a bunch of rows returned as a set, which I can then insert into a table,
pass into a function for reformatting, or just consume in an application.

All of which leads me to this variant of the functional approach as my answer:

SELECT * FROM pg_explain_query(query, options in a still-unspecified
format);

I could then do things like this:

CREATE TABLE explain_results AS SELECT * FROM pg_explain_query(...);

and this:

SELECT xmlify_a_record(pg_explain_query(...));

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-25 Thread Joshua Tolley
On Mon, May 25, 2009 at 10:55:48AM -0400, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  The Oracle version, as it fills the table of explain results, gives
  each number an id and the id of its parent row, which behavior we
  could presumably copy.  I'm definitely keen to keep a human-readable
  EXPLAIN such as we have now, to augment the table-based proposal, but
  a table would provide the more flexible output we'd need for more
  detailed reporting, a simple interface for applications to consume the
  EXPLAIN data without human intervention, and a convenient platform
  from whence the data can be transformed to XML, JSON, etc.  for those
  that are so inclined.
 
 I would think a table would be considerably *less* flexible --- you
 could not easily change the output column set.  Unless you're imagining
 just dumping something equivalent to the current output into a text
 column.  Which would be flexible, but it'd hardly have any of the
 other desirable properties you list.

I'm not sure I see why it would be less flexible. I'm imagining we define some
record type, and a function that returns a set of those records. The fields in
the record would include data element this version of explain could possibly
return. Then you call a function to explain a query, passing it some options
to select which of those data elements you're interested in. The function
returns the same data type at each call, filling with NULLs the fields you've
told it you're uninterested in. Changes between versions would modify this
data type, but provided consumer applications have specified the columns
they're interested in (rather than, say, SELECT *) that shouldn't bother
anyone.

The functions to change this into some other format would probably need to be
more intelligent than just that. That seems a fair price to pay.

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-25 Thread Joshua Tolley
On Mon, May 25, 2009 at 11:22:24AM -0400, Tom Lane wrote:
 Joshua Tolley eggyk...@gmail.com writes:
  I'm not sure I see why it would be less flexible. I'm imagining we define 
  some
  record type, and a function that returns a set of those records.
 
 I'm unimpressed by the various proposals to change EXPLAIN into a
 function.  Quoting the command-to-explain is going to be a pain in the
 neck. 

Yeah, that's been bugging me, despite my recent support of that plan.

 And can you really imagine using it manually, especially if it
 returns so many fields that you *have to* write out the list of fields
 you actually want, else the result is unreadable?  It's going to be just
 as much of something you can only use through a helper application as
 the XML way would be.

Good point. The reason, as I remember it, that we wanted to be able to specify
what fields are returned was so that fields that are expensive to calculate
are calculated only when the user wants them. If that's the only
consideration, perhaps we should have a standard version and a FULL version,
e.g.

EXPLAIN [ANALYZE] [FULL] query

...where FULL would indicate the user wanted the all available statistics, not
just the cheap ones. Somewhere in there we'd also need an indicator to say we
wanted an output format other than the usual text version (such as the WITH
XML clause I think someone suggested); I maintain it's all just a table of
data, and should be represented the same way we represent any other table of
data.

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-24 Thread Joshua Tolley
On Sun, May 24, 2009 at 11:57:13AM -0400, Andrew Dunstan wrote:


 Robert Haas wrote:
 EXPLAIN ('hash_detail', 'on') query...
   

 Oops, I should have written EXPLAIN (hash_detail 'on') query... can't
 follow my own syntax.

   
 I am sorry - this is really strange syntax . Who will use this syntax?
 For some parser is little bit better function call, than parametrized
 statement. Some dificulties with options should be fixed with named
 param (we are speaking about 8.5).

 select explain_xml(select ..., true as hash_detail, ...)
 

 See to me THAT is a really strange syntax, so I guess we need some more 
 votes.


   

 Both of these seem both odd an unnecessary. Why not just have a setting  
 called, say, explain_format which governs the output?

set explain_format = 'xml, verbose';
explain select * from foo;

 No new function or syntax would be required.

A further possibility: Oracle's equivalent of EXPLAIN doesn't actually output
anything to the screen, but rather fills in a (temporary?) table somewhere with
details of the query plan. I mostly found this irritating when working with
Oracle, because each time I used it I had to look up an example query to
generate output like PostgreSQL's EXPLAIN, which is generally what I really
wanted. But since we'd still have the old EXPLAIN behavior available, perhaps
something such as an Oracle-like table filler would be useful.

Such a proposal doesn't answer the need to allow users to specify, for
performance and other reasons, the precise subset of statistics they're
interested in; for whatever it's worth, my current favorite contender in that
field is EXPLAIN (a, b, c) query.

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] generic options for explain

2009-05-24 Thread Joshua Tolley
On Sun, May 24, 2009 at 06:53:29PM -0400, Tom Lane wrote:
 Greg Smith gsm...@gregsmith.com writes:
  On Sun, 24 May 2009, Pavel Stehule wrote:
  we should have a secondary function explain_query(query_string,
  option) that returns setof some.
 
  +1.  The incremental approach here should first be adding functions that 
  actually do the work required.  Then, if there's a set of those that look 
  to be extremely useful, maybe at that point it's worth talking about how 
  to integrate them into the parser.  Starting with the parser changes 
  rather than the parts that actually do the work is backwards.  If you do 
  it the other way around, at all times you have a patch that actually 
  provides immediate useful value were it to be committed.
 
  Something that returns a setof can also be easily used to implement the 
  dump EXPLAIN to a table feature Josh Tolley brought up (which is another 
  common request in this area).
 
 A serious problem with EXPLAIN via a function returning set, or with
 putting the result into a table, is that set results are logically
 unordered, just as table contents are.  So from a strict point of view
 this only makes sense when the output format is designed to not depend
 on row ordering to convey information.  We could certainly invent such
 a format, but I think it's a mistake to go in this direction for
 EXPLAIN output that is similar to the current output.

The Oracle version, as it fills the table of explain results, gives each number
an id and the id of its parent row, which behavior we could presumably copy.
I'm definitely keen to keep a human-readable EXPLAIN such as we have now, to
augment the table-based proposal, but a table would provide the more flexible
output we'd need for more detailed reporting, a simple interface for
applications to consume the EXPLAIN data without human intervention, and a
convenient platform from whence the data can be transformed to XML, JSON, etc.
for those that are so inclined.

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] Implementation of GROUPING SETS (T431: Extended grouping capabilities)

2009-05-13 Thread Joshua Tolley
On Wed, May 13, 2009 at 06:29:41AM +0200, Pavel Stehule wrote:
 2009/5/13 Joshua Tolley eggyk...@gmail.com:
  On Tue, May 12, 2009 at 11:20:14PM +0200, Pavel Stehule wrote:
  this patch has some bugs but it is good prototype (it's more stable
  than old patch):
 
  I'm not sure if you're at the point that you're interested in bug reports, 
  but
  here's something that didn't behave as expected:
 
  5432 j...@josh*# create table gsettest (prod_id integer, cust_id integer,
  quantity integer);
  CREATE TABLE
  5432 j...@josh*# insert into gsettest select floor(random() * 10)::int,
  floor(random() * 20)::int, floor(random() * 10)::int from generate_series(1,
  100);
  INSERT 0 100
  5432 j...@josh*# select prod_id, cust_id, sum(quantity) from gsettest group 
  by
  cube (prod_id, cust_id) order by 1, 2;
   prod_id | cust_id | sum
  -+-+-
        5 |       7 |   4
        8 |      16 |   3
        9 |      19 |   8
        4 |      13 |   3
        8 |       8 |  15
        5 |       2 |   4
        7 |       6 |   7
        6 |       6 |   3
  /snip
 
  Note that the results aren't sorted. The following, though, works around it:
 
 I thing, so result should not be sorted - it's same like normal group by.

Normal GROUP BY wouldn't have ignored the ORDER BY clause I included.

- Josh


signature.asc
Description: Digital signature


Re: [HACKERS] Implementation of GROUPING SETS (T431: Extended grouping capabilities)

2009-05-12 Thread Joshua Tolley
On Tue, May 12, 2009 at 11:20:14PM +0200, Pavel Stehule wrote:
 this patch has some bugs but it is good prototype (it's more stable
 than old patch):

I'm not sure if you're at the point that you're interested in bug reports, but
here's something that didn't behave as expected:

5432 j...@josh*# create table gsettest (prod_id integer, cust_id integer,
quantity integer);
CREATE TABLE
5432 j...@josh*# insert into gsettest select floor(random() * 10)::int,
floor(random() * 20)::int, floor(random() * 10)::int from generate_series(1,
100);
INSERT 0 100
5432 j...@josh*# select prod_id, cust_id, sum(quantity) from gsettest group by
cube (prod_id, cust_id) order by 1, 2;
 prod_id | cust_id | sum 
-+-+-
   5 |   7 |   4
   8 |  16 |   3
   9 |  19 |   8
   4 |  13 |   3
   8 |   8 |  15
   5 |   2 |   4
   7 |   6 |   7
   6 |   6 |   3
/snip

Note that the results aren't sorted. The following, though, works around it:

5432 j...@josh*# select * from (select prod_id, cust_id, sum(quantity) from
gsettest group by cube (prod_id, cust_id)) f order by 1, 2;
 prod_id | cust_id | sum 
-+-+-
   0 |   2 |   8
   0 |   4 |   8
   0 |   5 |   2
   0 |   7 |  11
   0 |   8 |   7
   0 |   9 |   1
   0 |  12 |   3
   0 |  14 |   7
   0 |  16 |   5
   0 |  17 |   8
   0 |  18 |   9
   0 |  19 |   2
   0 | |  71
/snip

EXPLAIN output is as follows:
5432 j...@josh*# explain select prod_id, cust_id, sum(quantity) from gsettest
group by cube (prod_id, cust_id) order by 1, 2;
QUERY PLAN
---
 Append  (cost=193.54..347.71 rows=601 width=9)
   CTE **g**
 -  Sort  (cost=135.34..140.19 rows=1940 width=12)
   Sort Key: gsettest.prod_id, gsettest.cust_id
   -  Seq Scan on gsettest  (cost=0.00..29.40 rows=1940 width=12)
   -  HashAggregate  (cost=53.35..55.85 rows=200 width=12)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=12)
   -  HashAggregate  (cost=48.50..51.00 rows=200 width=8)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=8)
   -  HashAggregate  (cost=48.50..51.00 rows=200 width=8)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=8)
   -  Aggregate  (cost=43.65..43.66 rows=1 width=4)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=4)
(13 rows)

...and without the ORDER BY clause just to prove that it really is the reason
for the Sort step...

5432 j...@josh*# explain select prod_id, cust_id, sum(quantity) from gsettest
group by cube (prod_id, cust_id);
   QUERY PLAN

 Append  (cost=82.75..236.92 rows=601 width=9)
   CTE **g**
 -  Seq Scan on gsettest  (cost=0.00..29.40 rows=1940 width=12)
   -  HashAggregate  (cost=53.35..55.85 rows=200 width=12)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=12)
   -  HashAggregate  (cost=48.50..51.00 rows=200 width=8)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=8)
   -  HashAggregate  (cost=48.50..51.00 rows=200 width=8)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=8)
   -  Aggregate  (cost=43.65..43.66 rows=1 width=4)
 -  CTE Scan on **g**  (cost=0.00..38.80 rows=1940 width=4)
(11 rows)

I'm hoping I'll get a chance to poke at the patch some. This could be very
useful...

- Josh / eggyknap


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER TABLE ... ALTER COLUMN ... SET DISTINCT

2009-05-04 Thread Joshua Tolley
On Mon, May 04, 2009 at 10:13:31PM -0400, Robert Haas wrote:

nit
 +   own analysis indicates otherwie).  When set to a negative value, which
s/otherwie/otherwise
/nit


A question: why does attdistinct become entry #5 instead of going at the end?
I assume it's because the order here controls the column order, and it makes
sense to have attdistinct next to attstattarget, since they're related. Is
that right? Thanks in advance...

 --- 185,210 
* 
*/
   
 ! #define Natts_pg_attribute  19
   #define Anum_pg_attribute_attrelid  1
   #define Anum_pg_attribute_attname   2
   #define Anum_pg_attribute_atttypid  3
   #define Anum_pg_attribute_attstattarget 4
 ! #define Anum_pg_attribute_attdistinct   5
 ! #define Anum_pg_attribute_attlen6
 ! #define Anum_pg_attribute_attnum7

- Josh / eggyknap


signature.asc
Description: Digital signature


[HACKERS] Lifetime of FmgrInfo

2009-04-16 Thread Joshua Tolley
I was browsing PL/pgSQL source, and saw this line (pl_comp.c:151):

function = (PLpgSQL_function *) fcinfo-flinfo-fn_extra

It then does some work to determine whether the result in function is
valid or not. So I got to wondering, what's the lifetime of the
FunctionCallInfoinfo object passed to the call handler function? Or in
other words, what memory context is it in? And is there some way I could
find that out more easily than digging through the source?

- Josh / eggyknap


signature.asc
Description: Digital signature


  1   2   >