Re: [HACKERS] Extending grant insert on tables to sequences

[Jaime Casanova]

On Fri, Aug 22, 2008 at 10:19 PM, Bruce Momjian <[EMAIL PROTECTED]> wrote:
>
> Added to September commit fest.
>

updating the patch with one that only extends inserts. though, i
haven't look at the col level privs patch yet.

-- 
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. (593) 87171157
Index: doc/src/sgml/ref/grant.sgml
===
RCS file: /var/lib/postgresql/CVSREPO/pgsql/doc/src/sgml/ref/grant.sgml,v
retrieving revision 1.70
diff -c -r1.70 grant.sgml
*** doc/src/sgml/ref/grant.sgml	3 Jul 2008 15:59:55 -	1.70
--- doc/src/sgml/ref/grant.sgml	1 Sep 2008 06:43:08 -
***
*** 401,410 
 
  
 
! Granting permission on a table does not automatically extend 
! permissions to any sequences used by the table, including 
! sequences tied to SERIAL columns.  Permissions on 
! sequence must be set separately.
 
  
 
--- 401,410 
 
  
 
! Granting INSERT permissions on a table automatically extends it 
! to any sequences owned by the table, including sequences tied to 
! 	SERIAL columns. All other permissions must be set 
! 	separately.
 
  
 
Index: src/backend/catalog/aclchk.c
===
RCS file: /var/lib/postgresql/CVSREPO/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.147
diff -c -r1.147 aclchk.c
*** src/backend/catalog/aclchk.c	19 Jun 2008 00:46:03 -	1.147
--- src/backend/catalog/aclchk.c	1 Sep 2008 06:06:26 -
***
*** 361,366 
--- 361,396 
  	}
  
  	ExecGrantStmt_oids(&istmt);
+ 
+ 	/*
+ 	 * If the objtype is a relation and the privileges includes INSERT
+ 	 * then extend it as USAGE to the sequences owned by the 
+ 	 * relation
+ 	 */
+ 	if (istmt.objtype == ACL_OBJECT_RELATION && 
+ 		(istmt.all_privs || (istmt.privileges & ACL_INSERT))) 
+ 	{
+ 		InternalGrant istmt_seq;
+ 
+ 		istmt_seq.is_grant = istmt.is_grant;
+ 		istmt_seq.objtype = ACL_OBJECT_SEQUENCE;
+ 		istmt_seq.grantees = istmt.grantees;
+ 		istmt_seq.grant_option = istmt.grant_option;
+ 		istmt_seq.behavior = istmt.behavior;
+ 
+ 		istmt_seq.all_privs = false;
+ 		istmt_seq.privileges = ACL_NO_RIGHTS;
+ 
+ 		istmt_seq.privileges |= ACL_USAGE;
+ 
+ 		istmt_seq.objects = NIL;
+ 		foreach(cell, istmt.objects)
+ 	istmt_seq.objects = list_concat(istmt_seq.objects,
+ getOwnedSequences(lfirst_oid(cell)));
+  
+ 		if (istmt_seq.objects != NIL)
+ 			ExecGrantStmt_oids(&istmt_seq);
+ 	} 
  }
  
  /*
Index: src/test/regress/expected/dependency.out
===
RCS file: /var/lib/postgresql/CVSREPO/pgsql/src/test/regress/expected/dependency.out,v
retrieving revision 1.7
diff -c -r1.7 dependency.out
*** src/test/regress/expected/dependency.out	3 Jul 2008 15:59:55 -	1.7
--- src/test/regress/expected/dependency.out	1 Sep 2008 06:18:43 -
***
*** 13,19 
  -- can't drop neither because they have privileges somewhere
  DROP USER regression_user;
  ERROR:  role "regression_user" cannot be dropped because some objects depend on it
! DETAIL:  access to table deptest
  DROP GROUP regression_group;
  ERROR:  role "regression_group" cannot be dropped because some objects depend on it
  DETAIL:  access to table deptest
--- 13,20 
  -- can't drop neither because they have privileges somewhere
  DROP USER regression_user;
  ERROR:  role "regression_user" cannot be dropped because some objects depend on it
! DETAIL:  access to sequence deptest_f1_seq
! access to table deptest
  DROP GROUP regression_group;
  ERROR:  role "regression_group" cannot be dropped because some objects depend on it
  DETAIL:  access to table deptest

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

[HACKERS] [Patch Review] TRUNCATE Permission

[Ryan Bradetich]

Hello all,

Robert Haas submitted the TRUNCATE permissions patch for the September
commit fest:
http://archives.postgresql.org/message-id/[EMAIL PROTECTED]

I had some extra time tonight, so I downloaded, installed and reviewed this
patch.
Here are my findings:

1. I found the patch style to be consistent with the surrounding code.
2. The patch provides documentation updates and regression test updates.
3. The patch applies (with some fuzz) to the latest GIT tree.

Three issues I found with the patch via code reading and verified via
testing:

1. File: src/backend/catalog/aclchk.c:
Function: pg_class_aclmask():

I believe the ACL_TRUNCATE trigger should be added to this check and mask.

/*
 * Deny anyone permission to update a system catalog unless
 * pg_authid.rolcatupdate is set.   (This is to let superusers
protect
 * themselves from themselves.)  Also allow it if
allowSystemTableMods.
 *
 * As of 7.4 we have some updatable system views; those shouldn't be
 * protected in this way.  Assume the view rules can take care of
 * themselves.  ACL_USAGE is if we ever have system sequences.
 */
if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE)) &&
IsSystemClass(classForm) &&
classForm->relkind != RELKIND_VIEW &&
!has_rolcatupdate(roleid) &&
!allowSystemTableMods)
{
#ifdef ACLDEBUG
elog(DEBUG2, "permission denied for system catalog update");
#endif
mask &= ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_USAGE);
}


Here is where it is visible via the psql interface:

template1=# select rolname, rolcatupdate from pg_authid;
 rolname | rolcatupdate
-+--
 rbrad   | t
(1 row)

template1=# select has_table_privilege('pg_class', 'delete');
 has_table_privilege
-
 t
(1 row)

template1=# select has_table_privilege('pg_class', 'truncate');
 has_table_privilege
-
 t
(1 row)

template1=# update pg_authid set rolcatupdate = false;
UPDATE 1

template1=# select has_table_privilege('pg_class', 'delete');
 has_table_privilege
-
 f
(1 row)

template1=# select has_table_privilege('pg_class', 'truncate');
 has_table_privilege
-
 t
(1 row)


I do not believe this is a huge issue since truncate is prohibited on the
system catalogs
by the truncate_check_rel().

template1=# truncate pg_authid;
ERROR:  permission denied: "pg_authid" is a system catalog


  2. The src/test/regress/sql/privileges.sql regression test has tests for
  the has_table_privilege() function.  It looks like all the other
permissions
  are tested in this function, but there is not a test for the TRUNCATE
  privilege.


  3.  I believe you missed a documentation update in the ddl.sgml file:

   There are several different privileges: SELECT,
   INSERT, UPDATE, DELETE,
   REFERENCES, TRIGGER,
   CREATE, CONNECT, TEMPORARY,
   EXECUTE, and USAGE.


I played around with the patch for an hour or so tonight and I did not
observer any other unusual behaviors.

Hopefully this review is useful.  It is my first patch review for a
commit-fest.
I will update the wiki with a link to this email message for my review.

Thanks,

- Ryan

Re: [HACKERS] Our CLUSTER implementation is pessimal

[Martijn van Oosterhout]

On Mon, Sep 01, 2008 at 12:25:26AM +0100, Gregory Stark wrote:
> The problem is that it does a full index scan and looks up each tuple in the
> order of the index. That means it a) is doing a lot of random i/o and b) has
> to access the same pages over and over again.



> a) We need some way to decide *when* to do a sort and when to do an index
> scan. The planner has all this machinery but we don't really have all the
> pieces handy to use it in a utility statement. This is especially important
> for the case where we're doing a cluster operation on a table that's already
> clustered. In that case an index scan could conceivably actually win (though I
> kind of doubt it). I don't really have a solution for this.

The case I had recently was a table that was hugely bloated. 300MB data
and only 110 live rows. A cluster was instant, a seqscan/sort would
probably be much slower. A VACUUM FULL probably worse :)

Isn't there some compromise. Like say scanning the index to collect a
few thousand records and then sort them the way a bitmap index scan
does. Should be much more efficient that what we have now.

Have a nice day,
-- 
Martijn van Oosterhout   <[EMAIL PROTECTED]>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while 
> boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature

Re: [HACKERS] Is this really really as designed or defined in some standard

[Pavel Stehule]

Hello

2008/8/31 Hannu Krosing <[EMAIL PROTECTED]>:
> It seems that we allow several function arguments to have same
> name (or is it label :)
>
> hannu=# create or replace function ff(a int, a int) returns int language
> plpgsql as $$begin return $1+$2; end;$$;
> CREATE FUNCTION
> hannu=# select ff(1,1);
>  ff
> 
>  2
> (1 row)
>
> hannu=# select ff(1,2);
>  ff
> 
>  3
> (1 row)
>
> hannu=# create or replace function ffa(a int, a int) returns int
> language plpgsql as $$begin return a + a; end;$$;
> CREATE FUNCTION
> hannu=# select ffa(1,2);
>  ffa
> -
>   2
> (1 row)
>
> Is this defined by some standard or just an oversight ?
>

what is problem? You have two diferent functions. I don't see anything wrong.

Pavel

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

-- 
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] Our CLUSTER implementation is pessimal

[Heikki Linnakangas]

Martijn van Oosterhout wrote:

On Mon, Sep 01, 2008 at 12:25:26AM +0100, Gregory Stark wrote:

The problem is that it does a full index scan and looks up each tuple in the
order of the index. That means it a) is doing a lot of random i/o and b) has
to access the same pages over and over again.





a) We need some way to decide *when* to do a sort and when to do an index
scan. The planner has all this machinery but we don't really have all the
pieces handy to use it in a utility statement. This is especially important
for the case where we're doing a cluster operation on a table that's already
clustered. In that case an index scan could conceivably actually win (though I
kind of doubt it). I don't really have a solution for this.


The case I had recently was a table that was hugely bloated. 300MB data
and only 110 live rows. A cluster was instant, a seqscan/sort would
probably be much slower. A VACUUM FULL probably worse :)

Isn't there some compromise. Like say scanning the index to collect a
few thousand records and then sort them the way a bitmap index scan
does. Should be much more efficient that what we have now.


Ideally we would use the planner, and the planner would choose the best 
plan for a bloated table like that (it probably does, I'm not sure) as well.


However, I'm not sure how much we can trust the statistics for a table 
we're about to CLUSTER. Often you run CLUSTER on a table you've just 
loaded or mass-updated.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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

Re: [HACKERS] Is this really really as designed or defined in some standard

[Magnus Hagander]

Pavel Stehule wrote:
> Hello
> 
> 2008/8/31 Hannu Krosing <[EMAIL PROTECTED]>:
>> It seems that we allow several function arguments to have same
>> name (or is it label :)
>>
>> hannu=# create or replace function ff(a int, a int) returns int language
>> plpgsql as $$begin return $1+$2; end;$$;
>> CREATE FUNCTION
>> hannu=# select ff(1,1);
>>  ff
>> 
>>  2
>> (1 row)
>>
>> hannu=# select ff(1,2);
>>  ff
>> 
>>  3
>> (1 row)
>>
>> hannu=# create or replace function ffa(a int, a int) returns int
>> language plpgsql as $$begin return a + a; end;$$;
>> CREATE FUNCTION
>> hannu=# select ffa(1,2);
>>  ffa
>> -
>>   2
>> (1 row)
>>
>> Is this defined by some standard or just an oversight ?
>>
> 
> what is problem? You have two diferent functions. I don't see anything wrong.

Take a look at the second function again. It's certainly not behaviour
that I would expect :-) (I would expect a syntax error)

//Magnus

-- 
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] WIP patch: Collation support

[Radek Strnad]

Ok, so do you suggest to leave it with a notice "reindex database" or start
to solve it somehow?

Regards

Radek Strnad

On Mon, Sep 1, 2008 at 12:08 AM, Alvaro Herrera
<[EMAIL PROTECTED]>wrote:

> Radek Strnad escribió:
>
> > - when creating database with different collation than database cluster,
> the
> > database has to be reindexed. Any idea how to do it? Function
> > ReindexDatabase works only when database is opened.
>
> We have this Todo item:
>
>  Set proper permissions on non-system schemas during db creation
>Currently all schemas are owned by the super-user because they are
>copied from the template1 database. However, since all objects are
>inherited from the template database, it is not clear that setting
>schemas to the db owner is correct.
>
> When this was discussed years ago, one proposed idea was that on the
> first connection the backend should, as a first task, ensure that
> certain administrative chores be done.  The first of those was changing
> the ownership of schemas.  It sounds like this reindexing you propose
> falls into the same category.
>
> --
> Alvaro Herrera
> http://www.CommandPrompt.com/
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support
>

Re: [HACKERS] Is this really really as designed or defined in some standard

[Pavel Stehule]

2008/9/1 Magnus Hagander <[EMAIL PROTECTED]>:
> Pavel Stehule wrote:
>> Hello
>>
>> 2008/8/31 Hannu Krosing <[EMAIL PROTECTED]>:
>>> It seems that we allow several function arguments to have same
>>> name (or is it label :)
>>>
>>> hannu=# create or replace function ff(a int, a int) returns int language
>>> plpgsql as $$begin return $1+$2; end;$$;
>>> CREATE FUNCTION
>>> hannu=# select ff(1,1);
>>>  ff
>>> 
>>>  2
>>> (1 row)
>>>
>>> hannu=# select ff(1,2);
>>>  ff
>>> 
>>>  3
>>> (1 row)
>>>
>>> hannu=# create or replace function ffa(a int, a int) returns int
>>> language plpgsql as $$begin return a + a; end;$$;
>>> CREATE FUNCTION
>>> hannu=# select ffa(1,2);
>>>  ffa
>>> -
>>>   2
>>> (1 row)
>>>
>>> Is this defined by some standard or just an oversight ?
>>>
>>
>> what is problem? You have two diferent functions. I don't see anything wrong.
>
> Take a look at the second function again. It's certainly not behaviour
> that I would expect :-) (I would expect a syntax error)

I see it now - it's really bug

Pavel

>
> //Magnus
>

-- 
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] Auto-explain patch

[ITAGAKI Takahiro]

Dean Rasheed <[EMAIL PROTECTED]> wrote:

> > An arguable part is initializing instruments in ExecutorRun_hook.
> > The initialization should be done in ExecutorStart normally, but
> > it is too late in the hook. Is it safe? or are there any better idea?
> 
> How about adding a new hook to control instrumentation of queries in
> ExecutorStart? Something like:
> 
> typedef bool (*ExecutorDoInstrument_hook_type) (QueryDesc *queryDesc, int 
> eflags);
> extern PGDLLIMPORT ExecutorDoInstrument_hook_type ExecutorDoInstrument_hook;

I think it is not good to have any single-purpose hooks -- a new global
variable "bool force_instrument" would be enough for the purpose ;-)

I'd like to suggest on-demand allocation of instruments instead.
PlanState->instrument is not only a runtime statstics collector, but also
represents whether instrumentation is enabled or not. However, we also
have the same information in EState->es_insrument. If we use it instread
of NULL check, we could initialize instrument fields in executor.

[src/backend/executor/execProcnode.c]
ExecProcNode(PlanState *node)
{
...
if (node->state->es_instrument)
{
if (node->instrument == NULL)
node->instrument = InstrAlloc(1, long_lived_memory_context);
InstrStartNode(node->instrument);
}

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



-- 
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] Attaching error cursor position to invalid constant values

[Heikki Linnakangas]

Tom Lane wrote:

Does anyone think this might be "too chatty"?


No.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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

Re: [HACKERS] Attaching error cursor position to invalid constant values

[Asko Oja]

On Mon, Sep 1, 2008 at 12:59 PM, Heikki Linnakangas <
[EMAIL PROTECTED]> wrote:

> Tom Lane wrote:
>
>> Does anyone think this might be "too chatty"?
>>
>
> No.
>
+1

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

Re: [HACKERS] Is this really really as designed or defined in some standard

[Hannu Krosing]

On Mon, 2008-09-01 at 11:15 +0200, Pavel Stehule wrote:
> 2008/9/1 Magnus Hagander <[EMAIL PROTECTED]>:
> > Pavel Stehule wrote:
> >> Hello
> >>
> >> 2008/8/31 Hannu Krosing <[EMAIL PROTECTED]>:

> >>>
> >>> hannu=# create or replace function ffa(a int, a int) returns int
> >>> language plpgsql as $$begin return a + a; end;$$;
> >>> CREATE FUNCTION
> >>> hannu=# select ffa(1,2);
> >>>  ffa
> >>> -
> >>>   2
> >>> (1 row)
> >>>
> >>> Is this defined by some standard or just an oversight ?
> >>>
> >>
> >> what is problem? You have two diferent functions. I don't see anything 
> >> wrong.
> >
> > Take a look at the second function again. It's certainly not behaviour
> > that I would expect :-) (I would expect a syntax error)
> 
> I see it now - it's really bug

There are a few places, where repeating labels are allowed, for example
select can produce such record 

hannu=# select 1 as a, 2 as a;
 a | a 
---+---
 1 | 2
(1 row)

But it is not allowed in TYPE or table definitions

hannu=# create type aa as (a int, a int); 
ERROR:  column "a" specified more than once

hannu=# create table aa (a int, a int); 
ERROR:  column "a" specified more than once

It probably is also not allowed in function/procedure argument list, but
I was not sure that any standard would not require it.

So, should this be fixed at calling / SQL side (by not allowing
repeating argument names) or at pl side for each pl separately ?

--
Hannu



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

Re: [HACKERS] [PATCH] Make gram.y use palloc/pfree for memory management

[Marko Kreen]

First a correction, overriding malloc/free seems dangerous they
seems to leak out, so correct would be to use YYMALLOC/YYFREE.
This leaves 1.875 potentially leaking, but danger seems small.

On 9/1/08, Tom Lane <[EMAIL PROTECTED]> wrote:
> Marko Kreen <[EMAIL PROTECTED]> writes:
>  > This means gram.y can leak memory if error is throws in
>  > the middle of parsing.
>
> Please offer some evidence for that claim.

The leak occurs when
1. bison does allocation.
2. error is thrown.

Now, normally bison does not do allocation as it has initially 200-item
stack allocated in stack.  When this is full it does allocate.

But I'm not familial enough with bison internals and Postgres parser
structure, on how cause the stack fill up.  It may be that Postgres
parser avoids recursive stack allocations, thus in practice the
leak cannot occur.

-- 
marko

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

Re: [HACKERS] [PATCH] Cleanup of GUC units code

[Marko Kreen]

On 9/1/08, Tom Lane <[EMAIL PROTECTED]> wrote:
> Marko Kreen <[EMAIL PROTECTED]> writes:
>  > - In attempt to preserve maximum range of values for INT64_IS_BUSTED
>  >   systems, the code is written rather non-obvious way.
>
> I do not personally object a bit to making the units comparisons
>  case-insensitive (I think it's mainly Peter who wants to be strict
>  about it).  I don't think there are any other good ideas in this
>  patch, however, and exposing ourselves to intermediate overflows in
>  the name of simplicity is definitely not one.

For all practical purposes, the overflow is insignificant when int64
works.  I'll look if I can avoid it on INT64_IS_BUSTED case.

In the meantime, here is simple patch for case-insensivity.

-- 
marko
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4ca25dc..b10be22 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4182,7 +4182,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 #error XLOG_BLCKSZ must be between 1KB and 1MB
 #endif
 
-			if (strncmp(endptr, "kB", 2) == 0)
+			if (pg_strncasecmp(endptr, "kB", 2) == 0)
 			{
 endptr += 2;
 switch (flags & GUC_UNIT_MEMORY)
@@ -4195,7 +4195,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "MB", 2) == 0)
+			else if (pg_strncasecmp(endptr, "MB", 2) == 0)
 			{
 endptr += 2;
 switch (flags & GUC_UNIT_MEMORY)
@@ -4211,7 +4211,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "GB", 2) == 0)
+			else if (pg_strncasecmp(endptr, "GB", 2) == 0)
 			{
 endptr += 2;
 switch (flags & GUC_UNIT_MEMORY)
@@ -4234,7 +4234,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 			if (hintmsg)
 *hintmsg = gettext_noop("Valid units for this parameter are \"ms\", \"s\", \"min\", \"h\", and \"d\".");
 
-			if (strncmp(endptr, "ms", 2) == 0)
+			if (pg_strncasecmp(endptr, "ms", 2) == 0)
 			{
 endptr += 2;
 switch (flags & GUC_UNIT_TIME)
@@ -4247,7 +4247,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "s", 1) == 0)
+			else if (pg_strncasecmp(endptr, "s", 1) == 0)
 			{
 endptr += 1;
 switch (flags & GUC_UNIT_TIME)
@@ -4260,7 +4260,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "min", 3) == 0)
+			else if (pg_strncasecmp(endptr, "min", 3) == 0)
 			{
 endptr += 3;
 switch (flags & GUC_UNIT_TIME)
@@ -4273,7 +4273,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "h", 1) == 0)
+			else if (pg_strncasecmp(endptr, "h", 1) == 0)
 			{
 endptr += 1;
 switch (flags & GUC_UNIT_TIME)
@@ -4289,7 +4289,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
 		break;
 }
 			}
-			else if (strncmp(endptr, "d", 1) == 0)
+			else if (pg_strncasecmp(endptr, "d", 1) == 0)
 			{
 endptr += 1;
 switch (flags & GUC_UNIT_TIME)

-- 
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] Extending grant insert on tables to sequences

[Stephen Frost]

* Jaime Casanova ([EMAIL PROTECTED]) wrote:
> On Fri, Aug 22, 2008 at 10:19 PM, Bruce Momjian <[EMAIL PROTECTED]> wrote:
> >
> > Added to September commit fest.
> >
> 
> updating the patch with one that only extends inserts. though, i
> haven't look at the col level privs patch yet.

At least initially I wasn't planning to support column-level privileges
for sequences, so I don't think it will affect you much.  Do people
think it makes sense to try and support that?

As your patch appears more ready-for-commit than the column-level
privileges patch, I wouldn't worry about what code might have to move
around, that'll be for me to deal with in a re-sync with HEAD once your
patch is committed.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Window functions patch v04 for the September commit fest

[Heikki Linnakangas]

Hitoshi Harada wrote:

2008/8/30 Hitoshi Harada <[EMAIL PROTECTED]>:

Here's the latest window functions patch against HEAD. It seems to be
ready for the September commit fest, as added documents, WINDOW clause
feature and misc tests. I guess this would be the window functions
feature freeze for 8.4. The remaining feature will be implemented for
the later release.

This patch consists of:
- windowed aggregates
- cooperate with GROUP BY aggregates
- some optimizations with multiple windows
- ranking functions
- WINDOW clause
- windowing SQL regression tests
- sgml documents

Looking up the total road map, the dropped features are:

- sliding window (window framing)
- lead(), lag(), etc. that reach for random rows
- user defined window functions


Ok, I'm starting to read up on SQL2003 window functions, and to review 
this patch. Here's a long brain-dump of my first thoughts on the design.


Let's list a couple of requirements first:

1. It's important that what gets committed now can be extended to handle 
all of the window function stuff in SQL2003 in the future, as well as 
user-defined-window-functions in the spirit of PostgreSQL extensibility. 
Even if we don't implement all of it in this release.


2. Performance needs to be reasonable, in the face a large number of 
input rows. These are typically used in OLAP queries that process 
gazillions of rows. Some window functions need access to all rows in the 
window frame or partition, so you can't reasonably expect great 
performance with those if the working set is large, but those that 
don't, should work with finite memory requirements, and reasonably fast.


Because of 1., let's focus on the interface for 
user-defined-window-functions first. Ideally, the interface is such that 
all the window functions and aggregates defined in SQL2003, and others 
we might want to have in core or as pgfoundry projects, can be 
implemented using it.


I think there's still some confusion in the design document about what a 
frame is. The terminology section is great, it helped me a lot to get 
started, so thank you for that. However, in the "Actual design in the 
patch" you describe how a window function works, and you talk about a 
window frame, but elsewhere in the doc you note that window frames are 
not implemented yet.


As already discussed, there's two different kind of window expressions: 
window aggregates, and ranking aggregates (you call them window 
functions in your doc). It seems that they are different enough that we 
can't bang them together. For example, you can't use a window frame 
clause with ranking aggregates, and ranking aggregates need to see the 
whole row, whereas window aggregates only see the expressions passed as 
argument.


API for window functions (aka. ranking aggregate functions)


Borrowing ideas from the many approaches listed in the design doc, I 
propose an interface using a set-returning function. Similar to Simon's 
proposal, but taking into account that a ranking function might need to 
see some or all input tuples before returning anything.


For each input tuple, the window function can return any number of 
tuples, including 0. After processing all input values, though, the 
total number of output rows returned must match the total number of 
input values fed into it. The function must eventually output one value 
for each input value, in the same order.


This is a flexible design that allows for different kind of 
implementations; the function can return one output tuple for each input 
tuple (ROW_NUMBER(), RANK() etc.), or it can swallow all input tuples 
first, and only then return all output tuples (e.g. CUME_DIST).


Here's a couple of step-by-step examples of how some functions would 
work. Imagine input values 1, 3, 4, 4, 5:


RANK


Invocation  Input   Value returned  Internal state (after)
1   1   1   (last value 1, count 1, rank=1)
2   3   2   (last value 2, count 1, rank=2)
3   4   3   (last value 4, count 1, rank=3)
4   4   3   (last value 4, count 2, rank=3)
5   5   5   (last value 5, count 1, rank=5)

So RANK returns one tuple after each input tuple. Just like in your 
current patch, keep the last value returned, a row count, and the 
current rank as state.


LAG
---

Internal state is a FIFO queue of values. Each input value is pushed to 
the FIFO, and the tuple that falls out of the queue is returned.


For example, LAG(,2,0):

Invocation  Input row   Value returned  Internal state (after)
1   1   0   (1)
2   3   0   (3, 1)
3   4   1   (4, 3)
4   4   3   (4, 4)
5   5   4   (5, 4)

LEAD


Return nothing for first  input tuples. Then return the curren

Re: [HACKERS] [Patch Review] TRUNCATE Permission

[Ryan Bradetich]

Hello all,

On Sun, Aug 31, 2008 at 11:55 PM, Ryan Bradetich <[EMAIL PROTECTED]>wrote:

>
> I do not believe this is a huge issue since truncate is prohibited on the
> system catalogs
> by the truncate_check_rel().
>
> template1=# truncate pg_authid;
> ERROR:  permission denied: "pg_authid" is a system catalog
>

I thought about this some more.  I believe my suggestion was incorrect.
Since truncate_check_rel() prevents the use of the truncate command on
system catalogs, the TRUNCATE permission should always be stripped
from the system catalogs.

Here is the inconsistency I observed:

template1=# \z pg_catalog.pg_authid

  Access privileges
   Schema   |   Name| Type  |  Access privileges
+---+---+-
 pg_catalog | pg_authid | table | rbrad=arwdDxt/rbrad
(1 row)

  template1=# select rolname, rolcatupdate from pg_authid;
   rolname | rolcatupdate
  -+--
   rbrad   | t
  (1 row)

  template1=# select has_table_privilege('pg_authid', 'truncate');
   has_table_privilege
  -
   t
  (1 row)

  template1=# truncate pg_authid;
  ERROR:  permission denied: "pg_authid" is a system catalog

The TRUNCATE fails even though \z and has_table_privilege() said I had
permissions.
Compare with the DELETE privilege:

  template1=# select has_table_privilege('pg_authid', 'delete');
   has_table_privilege
  -
   t
  (1 row)

template1=# delete from pg_authid;
DELETE 1

Thanks,

- Ryan

Re: [HACKERS] [Patch Review] TRUNCATE Permission

[Tom Lane]

"Ryan Bradetich" <[EMAIL PROTECTED]> writes:
>> I do not believe this is a huge issue since truncate is prohibited on the
>> system catalogs
>> by the truncate_check_rel().

Only when allowSystemTableMods is false.  I think it would be a serious
mistake for your patch to treat the system catalogs differently from
other tables.

> Here is the inconsistency I observed:

It seems to me that you are assuming that lack of a TRUNCATE permission
bit is the only valid reason for a "permission denied" failure.  This is
fairly obviously not so, since multiple permissions typically enter into
any command (consider schema-level permissions for instance).

regards, tom lane

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[David Fetter]

On Mon, Sep 01, 2008 at 09:00:47PM +0300, Heikki Linnakangas wrote:
> Hitoshi Harada wrote:
>> 2008/8/30 Hitoshi Harada <[EMAIL PROTECTED]>:
>>> Here's the latest window functions patch against HEAD. It seems to be
>>> ready for the September commit fest, as added documents, WINDOW clause
>>> feature and misc tests. I guess this would be the window functions
>>> feature freeze for 8.4. The remaining feature will be implemented for
>>> the later release.
>>>
>>> This patch consists of:
>>> - windowed aggregates
>>> - cooperate with GROUP BY aggregates
>>> - some optimizations with multiple windows
>>> - ranking functions
>>> - WINDOW clause
>>> - windowing SQL regression tests
>>> - sgml documents
>>>
>>> Looking up the total road map, the dropped features are:
>>>
>>> - sliding window (window framing)
>>> - lead(), lag(), etc. that reach for random rows
>>> - user defined window functions
>
> Ok, I'm starting to read up on SQL2003 window functions,

Maybe it would be better to read the SQL2008 standard
 :)

> and to review  this patch.  Here's a long brain-dump of my first
> thoughts on the design.
>
> Let's list a couple of requirements first:
>
> 1. It's important that what gets committed now can be extended to
> handle  all of the window function stuff in SQL2003 in the future,
> as well as  user-defined-window-functions in the spirit of
> PostgreSQL extensibility.  Even if we don't implement all of it in
> this release.

> 2. Performance needs to be reasonable, in the face a large number of  
> input rows. These are typically used in OLAP queries that process  
> gazillions of rows. Some window functions need access to all rows in the  
> window frame or partition, so you can't reasonably expect great  
> performance with those if the working set is large, but those that  
> don't, should work with finite memory requirements, and reasonably fast.
>
> Because of 1., let's focus on the interface for
> user-defined-window-functions first. Ideally, the interface is such
> that  all the window functions and aggregates defined in SQL2003,
> and others  we might want to have in core or as pgfoundry projects,
> can be  implemented using it.

SQL2008 defines the following:

 ::=
 OVER 

 ::=

| ROW_NUMBER  
| 
| 
| 
| 
| 

 ::=
  RANK
| DENSE_RANK
| PERCENT_RANK
| CUME_DIST

 ::=
NTILE   

 ::=
  
| 

 ::=
  
[   [   ] ] 
[  ]

 ::=
LEAD | LAG

 ::=


 ::=


 ::=


 ::=
RESPECT NULLS | IGNORE NULLS

 ::=
[  ]

 ::=
FIRST_VALUE | LAST_VALUE

 ::=
NTH_VALUE 
[  ] [  ]

 ::=
  
| 

 ::=
  FROM FIRST
| FROM LAST

 ::=
  
| 

 ::=


> For functions like LEAD or CUME_DIST that don't immediately start  
> returning values, the executor will need a tuplestore to buffer input  
> rows until it gets their values from the window function.

For these aggregates, should there be an API that signals that a tuplestore
will be needed?

> The syntax for creating a ranking aggregate might look something like this:
>
> CREATE RANKING AGGREGATE name (
> SFUNC = sfunc,
> STYPE = state_data_type,
> OUTFUNC = outfunc
> )
>
> This is very similar to Simon's proposal, and to current CREATE
> AGGREGATE, but tweaked a bit to fix the problems Hitoshi pointed
> out.
>
> sfunc is called once for each input row. Unlike with normal
> aggregates,  sfunc is passed the whole input row, so that e.g RANK
> can compare it  against the previous row, or LEAD can buffer it.
>
> outfunc is a set-returning function, and it is called until it
> returns  no more rows, after each sfunc invocation.

> Window aggregates
> -
>
> Like normal aggregates, window aggregates process N input values, and  
> return one output value. In normal GROUP BY expressions, the aggregate  
> function is called once each group, but in a window aggregate expression  
> with a window frame (aka sliding window), there is as many "groups" as  
> there is rows.
>
> In fact, any normal aggregate function can also be used as a window  
> aggregate and vice versa. The trivial implementation is to have a buffer  
> to hold all values currently in the window frame, and for each input  
> row, invoke the aggregate function over all the rows currently in the  
> frame. I think we'll need support that, to allow using any built-in or  
> user-defined aggregate as a window aggregate.
>
> However, we also want to provide optimized versions of the common  
> aggregates. Aggregates like COUNT, SUM, and AVG can easily be  
> implemented more efficiently, without rescanning all the tuples in the  
> group.
>
> I propose an extension to the current CREATE AGGREGATE syntax to allow  
> for optimized versions. Currently, the syntax is like:
>
> CREATE AGGREGATE name ( input_data_type [ , ... ] ) (
> SFUNC = sfunc,
> STYPE = state_data_type
> [ , FINALFUNC = ffunc ]
> 

Re: [HACKERS] Is this really really as designed or defined in some standard

[Tom Lane]

Hannu Krosing <[EMAIL PROTECTED]> writes:
> So, should this be fixed at calling / SQL side (by not allowing
> repeating argument names) or at pl side for each pl separately ?

I'm for fixing it just once, ie, in CREATE FUNCTION.  I can't imagine
any scenario where it's a good idea to have duplicate function parameter
names.

However, since this is a behavioral change that could break code that
works now, I think it should be a HEAD-only change; no backpatch.

regards, tom lane

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[Gregory Stark]

Heikki Linnakangas <[EMAIL PROTECTED]> writes:

> sfunc is called once for each input row. Unlike with normal aggregates, sfunc
> is passed the whole input row, so that e.g RANK can compare it against the
> previous row, or LEAD can buffer it.

I'm not sure I follow this bit about being passed the whole input row. How
would that relate to something like lag(x*y, 2) for example?

> outfunc is a set-returning function, and it is called until it returns no more
> rows, after each sfunc invocation.

And in any case I feel like this is abstraction on the cheap. The only reason
it's so general is because it's leaving all the work to the functions to
implement. 

It also means we get no benefit from cases like

SELECT lag(x,5),lag(y,5),lag(z,5)

where the executor could keep one tuplestore and for all of them. For that
matter it could keep one tuplestore even for cases like lag(x,5),lag(y,4).

What would the executor do for a query like

SELECT lead(x,1),lead(y,2),lead(y,3)

It would not only have to keep a tuplestore to buffer the output but it would
have to deal with receiving data from different SRFs at different times. The
best approach I can think of would be to keep a tuplestore for each SRF using
themas queues, reading old values from the head as soon as they all have at
least one new value in them.

And it doesn't answer how to deal with things like

SELECT lag(x,1) OVER (ORDER BY a), lag(x,1) OVER (ORDER BY b)

I, uh, don't actually have any ideas of how to deal with that one :(

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS support!

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

Fwd: [HACKERS] [Patch Review] TRUNCATE Permission

[Ryan Bradetich]

I had intended to send this message to the pgsql-hackers mailing list as
well.

Thanks,

- Ryan

-- Forwarded message --
From: Ryan Bradetich <[EMAIL PROTECTED]>
Date: Mon, Sep 1, 2008 at 2:20 PM
Subject: Re: [HACKERS] [Patch Review] TRUNCATE Permission
To: Tom Lane <[EMAIL PROTECTED]>


Hello Tom,

On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <[EMAIL PROTECTED]> wrote:

> "Ryan Bradetich" <[EMAIL PROTECTED]> writes:
> >> I do not believe this is a huge issue since truncate is prohibited on
> the
> >> system catalogs
> >> by the truncate_check_rel().
>
> Only when allowSystemTableMods is false.  I think it would be a serious
> mistake for your patch to treat the system catalogs differently from
> other tables.
>

Good Point.  Looks like I still have more code reading to do :)

This is Robert Haas's patch for the September 2008 commit-fest.
I am just offering my review.   Gives me a good excuse to dig into
the PostgreSQL code base.  Hopefully this review is useful to the
person committing the patch.


>  > Here is the inconsistency I observed:
>
> It seems to me that you are assuming that lack of a TRUNCATE permission
> bit is the only valid reason for a "permission denied" failure.  This is
> fairly obviously not so, since multiple permissions typically enter into
> any command (consider schema-level permissions for instance).
>

I was just comparing the behavior between DELETE and TRUNCATE.
My last suggestion for the TRUNCATE permission always being removed
on the system tables is obviously bogus because of the allowSystemTableMods
issue you raised.

Does my first suggestion still make sense for removing the TRUNCATE in
pg_class_aclmask() when pg_Authid.rolcatupdate is not set?

Thanks,

- Ryan

[HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[David E. Wheeler]

Howdy,

I'm trying to write a simple function that will return a string with  
the type name of a value. Unfortunately, it keeps dying on me. I don't  
even get any useful debugging information with --enable-cassert, just  
this:


LOG:  server process (PID 96946) was terminated by signal 10: Bus error
LOG:  terminating any other active server processes
LOG:  all server processes terminated; reinitializing

I stuck in a few calls to elog(), and it looks like this is the line  
that's choking:


typeoid = get_fn_expr_argtype(fcinfo->flinfo, 0);

But that's copied directly from enum.c. So I'm pretty mystified. Any  
help would be greatly appreciated.


Here's the complete code:

#include "postgres.h"
#include "fmgr.h"
#include "utils/builtins.h"

#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif

extern Datum type_of (PG_FUNCTION_ARGS);

Datum
type_of(PG_FUNCTION_ARGS)
{
Oidtypeoid;
Datum  result;
char   *typename;

typeoid = get_fn_expr_argtype(fcinfo->flinfo, 0);
if (typeoid == InvalidOid) {
ereport(
ERROR, (
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("could not determine data type of argument to  
type_of()")

)
);
}

typename = format_type_be(typeoid);
result = DirectFunctionCall1(textin, CStringGetDatum(typename));
PG_RETURN_DATUM(result);
}

And I load the function like so:

CREATE OR REPLACE FUNCTION type_of(anyelement)
RETURNS text
AS '$libdir/type_of'
LANGUAGE C STRICT IMMUTABLE;

Thanks,

DAvid


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

Re: [HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[Tom Lane]

"David E. Wheeler" <[EMAIL PROTECTED]> writes:
> Here's the complete code:

Looks like you forgot PG_FUNCTION_INFO_V1(), so what's being passed to
this isn't an fcinfo ...

regards, tom lane

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

[EMAIL PROTECTED]: Re: [HACKERS] [patch] GUC source file and line number]

[Alvaro Herrera]

Greg just sent me this patch, augmenting the one I sent to add source
file and line to GUC vars; Greg's patch adds a column with the default
value of each var.

I forward it to -hackers to have a public Message-Id to link to in the
Commitfest page.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--- Begin Message ---
Attached patch extends the last one Alvaro sent me to add a default_val 
column to pg_settings.  I renumbered the new columns the sourcefile/line 
patch added to fit it into a logical order (the default is just after the 
enumvalues).  That and minimizing the number of times everybody has to 
test in this area are why I wanted to squeeze this in at the same time.


Sample that shows the sort of thing I expect tuning tools will use this 
for:


select name,setting,default_val from pg_settings where not 
setting=default_val;


name|  setting   |default_val
++---
 archive_command| (disabled) |
 client_encoding| UTF8   | SQL_ASCII
 default_text_search_config | pg_catalog.english | pg_catalog.simple
 lc_collate | en_US.UTF-8| C
 lc_ctype   | en_US.UTF-8| C
 lc_messages| en_US.UTF-8|
 lc_monetary| en_US.UTF-8| C
 lc_numeric | en_US.UTF-8| C
 lc_time| en_US.UTF-8| C
 log_timezone   | US/Eastern | UNKNOWN
 max_fsm_pages  | 204800 | 2
 max_stack_depth| 2048   | 100
 server_encoding| UTF8   | SQL_ASCII
 shared_buffers | 4096   | 1024
 TimeZone   | US/Eastern | UNKNOWN
 timezone_abbreviations | Default| UNKNOWN

While there is a "default" for source, it's impossible to distinguish 
cases where someone set the value explicitly, but to the default value, 
unless tool writers have their own database of what the defaults are. 
That situation often happens when people uncomment the parameter in their 
postgresql.conf but don't actually change it from the default.  Example:


select name,setting,default_val,sourcefile,sourceline from pg_settings 
where name='max_connections';


  name   | setting | default_val |   sourcefile 
| sourceline

-+-+-+-+
 max_connections | 100 | 100 | 
/home/gsmith/pgproject/guc/data/postgresql.conf | 61


Hope I didn't introduce any bugs in your otherwise clean patch.

--
* Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MDIndex: doc/src/sgml/catalogs.sgml
===
RCS file: /home/gsmith/cvsrepo/pgsql/doc/src/sgml/catalogs.sgml,v
retrieving revision 2.172
diff -c -r2.172 catalogs.sgml
*** doc/src/sgml/catalogs.sgml  30 Jul 2008 17:05:04 -  2.172
--- doc/src/sgml/catalogs.sgml  1 Sep 2008 23:55:25 -
***
*** 6414,6419 
--- 6414,6436 
Allowed values in enum parameters (NULL for non-enum
values)
   
+  
+   default_val
+   text
+   Default value if the parameter is not explicitly set
+  
+  
+   sourcefile
+   text
+   Input file the current value was set from (if any), helpful 
+   when using configuration include directives
+  
+  
+   sourceline
+   text
+   Line number within the sourcefile the current value was set 
+   from
+  
  
 

Index: src/backend/utils/adt/ri_triggers.c
===
RCS file: /home/gsmith/cvsrepo/pgsql/src/backend/utils/adt/ri_triggers.c,v
retrieving revision 1.109
diff -c -r1.109 ri_triggers.c
*** src/backend/utils/adt/ri_triggers.c 19 May 2008 04:14:24 -  1.109
--- src/backend/utils/adt/ri_triggers.c 1 Sep 2008 23:27:02 -
***
*** 2738,2743 
--- 2738,2744 
snprintf(workmembuf, sizeof(workmembuf), "%d", maintenance_work_mem);
(void) set_config_option("work_mem", workmembuf,
 PGC_USERSET, 
PGC_S_SESSION,
+NULL, 0,
 GUC_ACTION_LOCAL, 
true);
  
if (SPI_connect() != SPI_OK_CONNECT)
***
*** 2827,2832 
--- 2828,2834 
snprintf(workmembuf, sizeof(workmembuf), "%d", old_work_mem);
(void) set_config_option("work_mem", workmembuf,
 PGC_USERSET, 
PGC_S_SESSION,
+NULL, 0,

Re: Fwd: [HACKERS] [Patch Review] TRUNCATE Permission

[Tom Lane]

"Ryan Bradetich" <[EMAIL PROTECTED]> writes:
> On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
>> [ something about "your patch" ]

> This is Robert Haas's patch for the September 2008 commit-fest.
> I am just offering my review.

Sorry about that, I got confused by the reply-to-a-reply.

> Does my first suggestion still make sense for removing the TRUNCATE in
> pg_class_aclmask() when pg_Authid.rolcatupdate is not set?

Probably.  AFAICS it should be treated exactly like ACL_DELETE, so
anyplace that acl-whacking code is doing something for ACL_DELETE and
the patch doesn't add in ACL_TRUNCATE, I'd be suspicious ...

regards, tom lane

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

Re: [HACKERS] [PATCH] allow has_table_privilege(..., 'usage') on sequences

[Jaime Casanova]

On Thu, Aug 7, 2008 at 3:08 AM, Abhijit Menon-Sen <[EMAIL PROTECTED]> wrote:
> I just noticed, to my dismay, that has_table_privilege() does not allow
> me to check for usage privileges on sequences.
>

Maybe we want a new function has_sequence_privilege() instead?

-- 
regards,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. (593) 87171157

-- 
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] Mysterious Bus Error with get_fn_expr_argtype()

[David E. Wheeler]

On Sep 1, 2008, at 16:55, Tom Lane wrote:


"David E. Wheeler" <[EMAIL PROTECTED]> writes:

Here's the complete code:


Looks like you forgot PG_FUNCTION_INFO_V1(), so what's being passed to
this isn't an fcinfo ...


Bah! I knew I had to be missing something really fundamental. Thanks  
Tom.


BTW, anyone have any interest in this function in core? Its purpose is  
to return a string identifying the data type of its argument. It's  
useful for dynamically building queries to pass to PL/pgSQL's EXECUTE  
statement when you don't know the data types of values you're putting  
into the statement.


Thanks,

David


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

Re: [HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[Brendan Jurd]

On Tue, Sep 2, 2008 at 2:03 PM, David E. Wheeler <[EMAIL PROTECTED]> wrote:
> BTW, anyone have any interest in this function in core? Its purpose is to
> return a string identifying the data type of its argument. It's useful for
> dynamically building queries to pass to PL/pgSQL's EXECUTE statement when
> you don't know the data types of values you're putting into the statement.
>

+1.  I've been using a variation on this theme (it returns the type
OID, not a text value) for a couple of years.  I find it very helpful
for troubleshooting queries where I need to track the cast/coercion
behaviour.

For the record, my version of the function is simply:

PG_FUNCTION_INFO_V1(gettype);
Datum gettype(PG_FUNCTION_ARGS)
{
PG_RETURN_OID(get_fn_expr_argtype(fcinfo->flinfo, 0));
}

CREATE OR REPLACE FUNCTION gettype(anyelement) RETURNS oid AS
 'libname', 'gettype'
 LANGUAGE C IMMUTABLE STRICT;

Cheers,
BJ

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

Re: [HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[Neil Conway]

On Mon, Sep 1, 2008 at 9:35 PM, Brendan Jurd <[EMAIL PROTECTED]> wrote:
> +1.  I've been using a variation on this theme (it returns the type
> OID, not a text value) for a couple of years.

Returning regtype seems like the natural choice.

-Neil

-- 
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] Window functions patch v04 for the September commit fest

[Hitoshi Harada]

Thanks for comments,

2008/9/2 Heikki Linnakangas <[EMAIL PROTECTED]>:
>
> Ok, I'm starting to read up on SQL2003 window functions, and to review this
> patch. Here's a long brain-dump of my first thoughts on the design.
>
> Let's list a couple of requirements first:
>
> 1. It's important that what gets committed now can be extended to handle all
> of the window function stuff in SQL2003 in the future, as well as
> user-defined-window-functions in the spirit of PostgreSQL extensibility.
> Even if we don't implement all of it in this release.
>
> 2. Performance needs to be reasonable, in the face a large number of input
> rows. These are typically used in OLAP queries that process gazillions of
> rows. Some window functions need access to all rows in the window frame or
> partition, so you can't reasonably expect great performance with those if
> the working set is large, but those that don't, should work with finite
> memory requirements, and reasonably fast.
>
> Because of 1., let's focus on the interface for
> user-defined-window-functions first. Ideally, the interface is such that all
> the window functions and aggregates defined in SQL2003, and others we might
> want to have in core or as pgfoundry projects, can be implemented using it.
>
> I think there's still some confusion in the design document about what a
> frame is. The terminology section is great, it helped me a lot to get
> started, so thank you for that. However, in the "Actual design in the patch"
> you describe how a window function works, and you talk about a window frame,
> but elsewhere in the doc you note that window frames are not implemented
> yet.

Sorry about that. In my understanding, the "Window Frame" is defined
by clauses such like "ROWS BETWEEN ... ", "RANGE BETWEEN ... " or so,
contrast to "Window Partition" defined by "PARTITION BY" clause. A
frame slides within a partition or there's only one frame if those
clauses are not specified. The current patch has not implemented this
yet. I'll update the docs.

>
> As already discussed, there's two different kind of window expressions:
> window aggregates, and ranking aggregates (you call them window functions in
> your doc). It seems that they are different enough that we can't bang them
> together. For example, you can't use a window frame clause with ranking
> aggregates, and ranking aggregates need to see the whole row, whereas window
> aggregates only see the expressions passed as argument.

I don't like to call the second type "ranking aggregates" because it
may refer to only ranking functions though there are more types of
function like ntile(), lead() and lag(). But "window functions"
doesn't seem appropriate either since it's ambiguous with the general
name of window expressions.

>
> API for window functions (aka. ranking aggregate functions)
> 
>
> Borrowing ideas from the many approaches listed in the design doc, I propose
> an interface using a set-returning function. Similar to Simon's proposal,
> but taking into account that a ranking function might need to see some or
> all input tuples before returning anything.
>
> For each input tuple, the window function can return any number of tuples,
> including 0. After processing all input values, though, the total number of
> output rows returned must match the total number of input values fed into
> it. The function must eventually output one value for each input value, in
> the same order.
>
> This is a flexible design that allows for different kind of implementations;
> the function can return one output tuple for each input tuple (ROW_NUMBER(),
> RANK() etc.), or it can swallow all input tuples first, and only then return
> all output tuples (e.g. CUME_DIST).
>
> Here's a couple of step-by-step examples of how some functions would work.
> Imagine input values 1, 3, 4, 4, 5:
>
> RANK
> 
>
> Invocation  Input   Value returned  Internal state (after)
> 1   1   1   (last value 1, count 1, rank=1)
> 2   3   2   (last value 2, count 1, rank=2)
> 3   4   3   (last value 4, count 1, rank=3)
> 4   4   3   (last value 4, count 2, rank=3)
> 5   5   5   (last value 5, count 1, rank=5)
>
> So RANK returns one tuple after each input tuple. Just like in your current
> patch, keep the last value returned, a row count, and the current rank as
> state.
>
> LAG
> ---
>
> Internal state is a FIFO queue of values. Each input value is pushed to the
> FIFO, and the tuple that falls out of the queue is returned.
>
> For example, LAG(,2,0):
>
> Invocation  Input row   Value returned  Internal state (after)
> 1   1   0   (1)
> 2   3   0   (3, 1)
> 3   4   1   (4, 3)
> 4   4   3   (4, 4)
> 5   5   4

Re: [HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[Tom Lane]

"Neil Conway" <[EMAIL PROTECTED]> writes:
> On Mon, Sep 1, 2008 at 9:35 PM, Brendan Jurd <[EMAIL PROTECTED]> wrote:
>> +1.  I've been using a variation on this theme (it returns the type
>> OID, not a text value) for a couple of years.

> Returning regtype seems like the natural choice.

I was just about to say the same.  Another thought is that you might as
well declare the input type as "any" --- using "anyelement" just causes
the parser to waste a few cycles checking for argument/result type
conflicts that can't exist here.

I don't like gettype() as the function name: it's not particularly
readable and it seems to infringe on application namespace.  It should
be pg_something ... maybe pg_typeof() ?

Oh, another thing: it shouldn't be STRICT.  Nulls have perfectly good
types.

regards, tom lane

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[Hitoshi Harada]

2008/9/2 Gregory Stark <[EMAIL PROTECTED]>:
> And it doesn't answer how to deal with things like
>
> SELECT lag(x,1) OVER (ORDER BY a), lag(x,1) OVER (ORDER BY b)
>
> I, uh, don't actually have any ideas of how to deal with that one :(

For different windows we make different window nodes with different
sort nodes as the patch does.



Regards,


-- 
Hitoshi Harada

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

Re: [HACKERS] Mysterious Bus Error with get_fn_expr_argtype()

[Brendan Jurd]

On Tue, Sep 2, 2008 at 2:57 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
> "Neil Conway" <[EMAIL PROTECTED]> writes:
>
>> Returning regtype seems like the natural choice.
>
> I was just about to say the same.

Yes.  In fact, the way I typically use the function is to write
gettype(whatever)::regtype.  I was too lazy to modify the function to
return regtype, and with "::regtype" weighing in at a mere 9
keystrokes there wasn't much motivation =)

> Another thought is that you might as
> well declare the input type as "any" --- using "anyelement" just causes
> the parser to waste a few cycles checking for argument/result type
> conflicts that can't exist here.

Good tip.

> I don't like gettype() as the function name: it's not particularly
> readable and it seems to infringe on application namespace.  It should
> be pg_something ... maybe pg_typeof() ?
>

Sure, pg_typeof() sounds good.  I only used gettype() because it was a
familiar name (PHP has an analogous builtin function called
gettype()).

> Oh, another thing: it shouldn't be STRICT.  Nulls have perfectly good
> types.
>

Agreed.

Barring any further comments/objections, I'll go ahead and prepare a
patch to add this to core.

Cheers,
BJ

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

[HACKERS] Full page write improvement: new WAL archive command (beta) released

[Koichi Suzuki]

Dear PostgreSQL folks;

New WAL archive commands, to be used in "archive_command" and
"restore_command" are released in

http://pgfoundry.org/projects/pglesslog/

You can download pglesslog 1.1 beta and use this in PostgreSQL8.3.x to
reduce the size of archive log.   You don't need any modification to
PostgreSQL itself.   Size reduction is done by replaceing full page
writes (needed to recovery from incomplete database writes at the crash,
which is not needed in PITR) with corresponding incremental logs.
Full page writes between pg_start_backup and pg_stop_backup are not
replaced to restore incomplete pages during the backup.

This is the first beat release for 32bit environment.   General release
including 64bit environment will be done in a couple of weeks.

The following is the notes from README.lesslog file in the release file.
 Please note that in the case of pgbench, archive log size will be one
seventh of the original.

** What is lesslog?

Lesslog is a set of tools to reduce the size of PostgreSQL archive log.
Lesslog consists of the following  programs.

** What's new in lesslog 1.1?

Lesslog 1.1 can generate incremental log for HEAP2_CLEAN WAL records,
which are created by vacuum and prune (HOT).   Because full page writes
of this WAL records occupies major part of WAL segments, replacing this
with incremental log dramatically reduces the size of WAL when archiving.

** How small archive log will be?

Pglesslog compression rate is shown in pgbench.

1. Environment
   Scale factor: 100
   Connection: 10
   Transactions: 360,000 per connection
   shared_buffers: 32MB
   max_fsm_pages: 204,800
   checkpoint_segments: 1,000
   checkpoint_timeout: 5min
   checkpoint_completion_target: 0.5
   autovacuum: on

2. Size of original/compressed WAL

   WAL segments :1,379
   Original WAL size : 22,064MB
   Compressed WAL size: 3,598MB
   Size ratio: 16% of the original

** Acknowledgement

I appreciate Pavan Deolasee and Heikki Linnakangas for their stimulating
comments and advise.   Without their advises, the solution would have
been much more complicated.

-- 
Koichi Suzuki

-- 
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] Window functions patch v04 for the September commit fest

[Heikki Linnakangas]

David Fetter wrote:

On Mon, Sep 01, 2008 at 09:00:47PM +0300, Heikki Linnakangas wrote:

Ok, I'm starting to read up on SQL2003 window functions,


Maybe it would be better to read the SQL2008 standard
 :)


Ah, thanks!

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[Tom Lane]

Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> David Fetter wrote:
>> On Mon, Sep 01, 2008 at 09:00:47PM +0300, Heikki Linnakangas wrote:
>>> Ok, I'm starting to read up on SQL2003 window functions,
>> 
>> Maybe it would be better to read the SQL2008 standard
>>  :)

> Ah, thanks!

Er, s/2008 standard/200n draft with uncertain chances of passage/

It's not like we haven't seen a SQL draft go down in flames before.

regards, tom lane

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

Re: [HACKERS] Is this really really as designed or defined in some standard

[Pavel Stehule]

2008/9/1 Tom Lane <[EMAIL PROTECTED]>:
> Hannu Krosing <[EMAIL PROTECTED]> writes:
>> So, should this be fixed at calling / SQL side (by not allowing
>> repeating argument names) or at pl side for each pl separately ?
>
> I'm for fixing it just once, ie, in CREATE FUNCTION.  I can't imagine
> any scenario where it's a good idea to have duplicate function parameter
> names.
>
> However, since this is a behavioral change that could break code that
> works now, I think it should be a HEAD-only change; no backpatch.

I agree - it's could break only 100% wrong code, but it could problems
in minor update.

Could you backpach only warning?

regards
Pavel

>
>regards, tom lane
>

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[Heikki Linnakangas]

Gregory Stark wrote:

Heikki Linnakangas <[EMAIL PROTECTED]> writes:


sfunc is called once for each input row. Unlike with normal aggregates, sfunc
is passed the whole input row, so that e.g RANK can compare it against the
previous row, or LEAD can buffer it.


I'm not sure I follow this bit about being passed the whole input row. How
would that relate to something like lag(x*y, 2) for example?


Well, sfunc needs to be passed not only the whole input tuple, but also 
the values of the arguments.


Hmm. The spec says that the offset argument of lead and lag needs to be 
a numeric literal. To enforce that at parse time, we'd have to hard-code 
that into the grammar. Or we could check it at run-time, and throw an 
error if its value changes across invocations of the sfunc function.



outfunc is a set-returning function, and it is called until it returns no more
rows, after each sfunc invocation.


And in any case I feel like this is abstraction on the cheap. The only reason
it's so general is because it's leaving all the work to the functions to
implement. 


It also means we get no benefit from cases like

SELECT lag(x,5),lag(y,5),lag(z,5)

where the executor could keep one tuplestore and for all of them. For that
matter it could keep one tuplestore even for cases like lag(x,5),lag(y,4).


True. I guess it comes down to how much complexity we're willing to have 
in the executor node to handle that more efficiently.


Note that the memory usage is the same either way, except for the extra 
constant overhead of multiple tuplestores, because each tuplestore 
inside the lag implementation only needs to store its own column.



What would the executor do for a query like

SELECT lead(x,1),lead(y,2),lead(y,3)

It would not only have to keep a tuplestore to buffer the output but it would
have to deal with receiving data from different SRFs at different times. The
best approach I can think of would be to keep a tuplestore for each SRF using
themas queues, reading old values from the head as soon as they all have at
least one new value in them.


Hitoshi solved that creating a separate Window node for each window 
function. So the plan tree for that would look something like:


Window (lead(x,1))
  Window (lead(y,2))
Window (lead(y,3))
  Seq Scan ...

That keeps the Window node implementation quite simple because it only 
needs to handle one window function at a time.



And it doesn't answer how to deal with things like

SELECT lag(x,1) OVER (ORDER BY a), lag(x,1) OVER (ORDER BY b)

I, uh, don't actually have any ideas of how to deal with that one :(


The above handles that by putting extra sort nodes in between the Window 
nodes. Not too efficient, but I don't see any way around it.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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

Re: [HACKERS] Window functions patch v04 for the September commit fest

[David Fetter]

On Tue, Sep 02, 2008 at 02:42:25AM -0400, Tom Lane wrote:
> Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> > David Fetter wrote:
> >> On Mon, Sep 01, 2008 at 09:00:47PM +0300, Heikki Linnakangas wrote:
> >>> Ok, I'm starting to read up on SQL2003 window functions,
> >> 
> >> Maybe it would be better to read the SQL2008 standard
> >>  :)
> 
> > Ah, thanks!
> 
> Er, s/2008 standard/200n draft with uncertain chances of passage/
> 
> It's not like we haven't seen a SQL draft go down in flames before.

Do you think that anything in the windowing functions section will
disappear?

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

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

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