Re: [HACKERS] Window-functions patch handling of aggregates
Yeah, it seems like adding a flag like iswindowable to aggregate functions is the safest option. It would be nice if it represented an abstract property of the state function or final function rather than just works with the implementation of window functions. I'm not sure what that property is though - isidempotent? isreentrant? Maybe just a vague isrepeatable? -- Greg On 24 Dec 2008, at 20:16, Hitoshi Harada umi.tan...@gmail.com wrote: 2008/12/25 Tom Lane t...@sss.pgh.pa.us: Gregory Stark st...@enterprisedb.com writes: Tom Lane t...@sss.pgh.pa.us writes: Unless we want to move the goalposts on what an aggregate is allowed to do internally, we're going to have to change this to re- aggregate repeatedly. Neither prospect is appetizing in the least. Does it currently copy the state datum before calling the final function? Would that help? No. The entire point of what we have now formalized as aggregates with internal-type transition values is that the transition value isn't necessarily a single palloc chunk. For stuff like array_agg, the performance costs of requiring that are enormous. On looking at what array_agg does, it seems the issue there is that the final-function actually deletes the working state when it thinks it's done with it (see construct_md_array). It would probably be possible to fix things so that it doesn't do that when it's called by a WindowAgg instead of a regular Agg. What I'm more concerned about is what third-party code will break. contrib/intagg has done this type of thing since forever, and I'm sure people have copied that... I have concerned it once before on the first design of the window functions. I don't have much idea about all over the aggregate functions but at least count(*) does some assumption of AggState in its context. So I concluded when the window functions are introduced it must be announced that if you use aggregate in the window context, you must be sure it supports window as well as aggregate. It is because currently (= 8.3) aggregates are assumed it is called in AggState only but the assumption would be broken now. It is designed, and announcing helps much third party code to support or not to support window functions (i.e. you can stop by error if window is not supported by the function). 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] Window-functions patch handling of aggregates
2008/12/25 Greg Stark greg.st...@enterprisedb.com:
Yeah, it seems like adding a flag like iswindowable to aggregate functions
is the safest option.
It would be nice if it represented an abstract property of the state
function or final function rather than just works with the implementation
of window functions. I'm not sure what that property is though -
isidempotent? isreentrant? Maybe just a vague isrepeatable?
No, I meant wrinting such like:
Datum
some_trans_fn(PG_FUNCTION_ARGS)
{
if (fcinfo-context IsA(fcinfo-context, WindowAggState))
elog(ERROR, some_agg does not support window aggregate);
...
}
rather than adding column to catalog. To add flag you must add new
syntax for CREATE AGGREGATE, which is slightly more painful.
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
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1348)
I updated the patch set of SE-PostgreSQL and related stuff (r1348) [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1348.patch [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1348.patch [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1348.patch [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1348.patch [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1348.patch Draft of the SE-PostgreSQL documentation is here: http://wiki.postgresql.org/wiki/SEPostgreSQL (It also should be updated for the recent changes...) List of updates: - The patches are rebased to the latest CVS HEAD. Currently, previous ones (r1324) are not suitable for this. - It put a copied relkind value on pg_attribute.attkind. This change enables to reduce per tuple lookups for RELOID, and improve robustness of security model. - bugfix: heap_getsysattr() could return NULL, when enhanced security feature is disabled. It is fixed to return an alternative label/default acl. - errcode_for_file_access() is applied on filesystem related errors, instead of ERRCODE_SELINUX_ERROR. - Reloptions related code for Row-level ACLs feature is flattened. Now it invokes rowaclXXX() without PGACE hooks, because there is an active effort to support variable kind of reloptions now. - The default_row_acl got stored as text represenation due to incorrect table dump. (We should not put it as security id.) - bugfix: Makefile in src/test/sepgsql Request for comments: The current heap_reloptions() requires reloption-parser not to raise an error when validate = false. However, it makes a matter when we store default_row_acl as a entry of reloptions. The input handler of AclItem[] can raise an error if given input string has invalid format or users. What solutions can be considered? - Implement its own AclItem[] parser which does not raise an error on validate = false. - Set dependencies on users which appears in default Row-ACLs. - Remove default Row-level ACLs feature. - Any other idea? And, I have a question. Is the new reloption framework designed to store strings? The latest one support Bool, Int and Real, doen't it? Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Window-functions patch handling of aggregates
2008/12/25 Hitoshi Harada umi.tan...@gmail.com:
2008/12/25 Greg Stark greg.st...@enterprisedb.com:
Yeah, it seems like adding a flag like iswindowable to aggregate functions
is the safest option.
It would be nice if it represented an abstract property of the state
function or final function rather than just works with the implementation
of window functions. I'm not sure what that property is though -
isidempotent? isreentrant? Maybe just a vague isrepeatable?
No, I meant wrinting such like:
Datum
some_trans_fn(PG_FUNCTION_ARGS)
{
if (fcinfo-context IsA(fcinfo-context, WindowAggState))
elog(ERROR, some_agg does not support window aggregate);
...
}
rather than adding column to catalog. To add flag you must add new
syntax for CREATE AGGREGATE, which is slightly more painful.
enhancing of CREATE AGGREGATE syntax should be better, it could solve
problem with compatibility.
regards
Pavel Stehule
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
--
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 handling of aggregates
2008/12/25 Pavel Stehule pavel.steh...@gmail.com:
2008/12/25 Hitoshi Harada umi.tan...@gmail.com:
2008/12/25 Greg Stark greg.st...@enterprisedb.com:
Yeah, it seems like adding a flag like iswindowable to aggregate functions
is the safest option.
It would be nice if it represented an abstract property of the state
function or final function rather than just works with the implementation
of window functions. I'm not sure what that property is though -
isidempotent? isreentrant? Maybe just a vague isrepeatable?
No, I meant wrinting such like:
Datum
some_trans_fn(PG_FUNCTION_ARGS)
{
if (fcinfo-context IsA(fcinfo-context, WindowAggState))
elog(ERROR, some_agg does not support window aggregate);
...
}
rather than adding column to catalog. To add flag you must add new
syntax for CREATE AGGREGATE, which is slightly more painful.
enhancing of CREATE AGGREGATE syntax should be better, it could solve
problem with compatibility.
Most of the aggregates have no problem with this issue and the rest
are fixable like array_agg. So adding a column and syntax is too much,
I guess. My suggestion of raising error is urgent patch for third
party aggregates that are copied from contrib/intagg.
But if there is a chance of general approach to call repeatable
aggregate other than WindowAgg, that should be considered.
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] Proposed Patch to Improve Performance of Multi-BatchHash Join for Skewed Data Sets
There is almost zero penalty for selecting incorrect MCV tuples to buffer in memory. Since the number of MCVs is approximately 100, the overhead is keeping these 100 tuples in memory where they *might* not be MCVs. The cost is the little extra memory and the checking of the MCVs which is very fast. I looked at this some more. I'm a little concerned about the way we're maintaining the in-memory hash table. Since the highest legal statistics target is now 10,000, it's possible that we could have two orders of magnitude more MCVs than what you're expecting. As I read the code, that could lead to construction of an in-memory hash table with 64K slots. On a 32-bit machine, I believe that works out to 16 bytes per partition (12 and 4), which is a 1MB hash table. That's not necessarily problematic, except that I don't think you're considering the size of the hash table itself when evaluating whether you are blowing out work_mem, and the default size of work_mem is 1MB. I also don't really understand why we're trying to control the size of the hash table by flushing tuples after the fact. Right now, when the in-memory table fills up, we just keep adding tuples to it, which in turn forces us to flush out other tuples to keep the size down. This seems quite inefficient - not only are we doing a lot of unnecessary allocating and freeing, but those flushed slots in the hash table degrade performance (because they don't stop the scan for an empty slot). It seems like we could simplify things considerably by adding tuples to the in-memory hash table only to the point where the next tuple would blow it out. Once we get to that point, we can skip the isAMostCommonValue() test and send any future tuples straight to temp files. (This would also reduce the memory consumption of the in-memory table by a factor of two.) We could potentially improve on this even further if we can estimate in advance how many MCVs we can fit into the in-memory hash table before it gets blown out. If, for example, we have only 1MB of work_mem but there 10,000 MCVs, getMostCommonValues() might decide to only hash the first 1,000 MCVs. Even if we still blow out the in-memory hash table, the earlier MCVs are more frequent than the later MCVs, so the ones that actually make it into the table are likely to be more beneficial. I'm not sure exactly how to do this tuning though, since we'd need to approximate the size of the tuples... I guess the query planner makes some effort to estimate that but I'm not sure how to get at it. However, the join with Part and LineItem *should* show a benefit but may not because of a limitation of the patch implementation (not the idea). The MCV optimization is only enabled currently when the probe side is a sequential scan. This limitation is due to our current inability to determine a stats tuple of the join attribute on the probe side for other operators. (This should be possible - help please?). Not sure how to get at this either, but I'll take a look and see if I can figure it out. Merry Christmas, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Automatic view update rules
On Mon, Dec 22, 2008 at 8:53 AM, Bernd Helmle maili...@oopsware.de wrote: --On Mittwoch, November 26, 2008 10:54:01 +0100 Bernd Helmle maili...@oopsware.de wrote: Okay, i've finally managed to create an updated version with (hopefully) all issues mentioned by Robert adressed. Hi Bernd, 1) i found a crash type bug, try this: create table foo ( id integer not nullprimary key, namevarchar(30) ) with oids; create view foo_view as select oid, * from foo; with this you will get an error like this one: ERROR: RETURNING list's entry 1 has different type from column oid but if you make this: alter table foo add column description text; create view foo_view as select oid, * from foo; then, the server crash. STATEMENT: create or replace view v_foo as select oid, * from foo; LOG: server process (PID 16320) was terminated by signal 11: Segmentation fault LOG: terminating any other active server processes maybe the better solution is to not allow such a view to be updatable 2) Another less important bug, the WITH CHECK OPTION is accepted even when that functionality is not implemented. updatable_views=# create or replace view v2 as select * from foo where id 10 with check option; NOTICE: CREATE VIEW will create implicit INSERT/UPDATE/DELETE rules CREATE VIEW 3) one final point: seems like you'll have to update the rules regression test (attached the regression.diffs) -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 regression.diffs Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [idea] a copied relkind in pg_attribute
On Wed, Dec 24, 2008 at 7:05 PM, KaiGai Kohei kai...@ak.jp.nec.com wrote: The other need an explanation. A database superuser is allowed to update system catalog by hand, if it is allowed by the security policy. For example, he will be able to update relkind in some of pg_class, even if it never happen in general DDLs. If a relkind is changed from 'r' to 'c', we deal pg_attribute entries pointing the tuple as db_tuple class, not db_column class, because they are not already columns. It means we fundamentally have to check permissions on pg_attribute when pg_class is updated, or pg_attribute should have its identifier information. I think the later approach is more simple. and what stops a superuser (if it's allowed by the security policy) from changing pg_attribute.attkind? protecting a DBA (DataBase Aniquilator) from shooting himself on the foot in situations like this one is not a good approach, IMHO... Please consider an another instance. In filesystem, 'x' permission bit has different meaning between files and directries. If a derectory without no child files is handled as a regular file suddenly, it can make a confusion. It is a similar situation. doesn't understand this... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] Hot standby and b-tree killed items
Perhaps we should listen to the people that have said they don't want queries cancelled, even if the alternative is inconsistent answers. I think an alternative to that would be if the wal backlog is too big, let current queries finish and let incoming queries wait till the backlog gets smaller. fell free to ignore me, as a non-hacker I`m not even supposed to be reading this list :-] Greetings Marcin -- 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] [idea] a copied relkind in pg_attribute
Jaime Casanova wrote:
On Wed, Dec 24, 2008 at 7:05 PM, KaiGai Kohei kai...@ak.jp.nec.com wrote:
The other need an explanation. A database superuser is allowed
to update system catalog by hand, if it is allowed by the security
policy. For example, he will be able to update relkind in some
of pg_class, even if it never happen in general DDLs.
If a relkind is changed from 'r' to 'c', we deal pg_attribute
entries pointing the tuple as db_tuple class, not db_column class,
because they are not already columns.
It means we fundamentally have to check permissions on pg_attribute
when pg_class is updated, or pg_attribute should have its identifier
information. I think the later approach is more simple.
and what stops a superuser (if it's allowed by the security policy)
from changing pg_attribute.attkind?
There are various kind of permission in the security policy.
When someone tries to change pg_attribute.attkind, he should have
db_column:{relabelfrom} privilege, because it makes changes to
its object class which is a part of security attribute.
However, when someone tries to change pg_attribute.attnotnull,
he should have db_column:{setattr}, because this operation does not
make changes in its security attribute.
In this case, db_column:{setattr} should be allowed to the client
who connected as a superuser, but db_column:{relabelfrom} should
not be allowed.
If you are not familiar to SELinux, please consider differences
between file ownership and 'w' permission on UNIX filesystem.
protecting a DBA (DataBase Aniquilator) from shooting himself
on the foot in situations like this one is not a good approach, IMHO...
It is not an issue the attkind tries to resolve.
If its object class (part of security attribute) is determined by external
factors, we have to lookup the external factors for each pg_attribute, and
we also have to check permissions on referrer side on changes in external
factors fundamentally.
It is a costly operatin, despite the factor (relkind) is almost constant.
Please consider an another instance. In filesystem, 'x' permission
bit has different meaning between files and directries. If a derectory
without no child files is handled as a regular file suddenly, it can
make a confusion. It is a similar situation.
doesn't understand this...
I wanted to introduce it is a strange behavior that same object is
handled as another sort of one depending on external factors.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei kai...@ak.jp.nec.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] uuids on freebsd
On Wed, Dec 24, 2008 at 10:08:01AM -0800, David Lee Lambert wrote: On Dec 17, 2:30 pm, Andrew Gierth and...@tao11.riddles.org.uk wrote: Has anyone ever managed to get uuid generation working on FreeBSD? [...] ([...] The only solution I could come up with was to knock off a quick uuid-freebsd module that uses the base system uuid functions rather than the ossp ones. I could put this on pgfoundry if there isn't likely to be a real fix in the near future.) +1 for putting it on pgFoundry. It's now on pgfoundry :) Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com 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
[HACKERS] Unused include/storage/itempos.h
Hi, I found include/storage/itempos.h is not included from any sources. What is it for? Should we remove it from the source tree? struct ItemSubpositionData typedef ItemSubpositionData *ItemSubposition; 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
[HACKERS] NOTIFY extra parameter
Any plans to enable the extra parameter string for notifies? Apparently, it was seriously considered as it is a member of PGnotify and has a reserved spot in the 'A' command sent by the backend. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] contrib/pg_stat_statements 1226
Here is an updated version of contrib/pg_stat_statements patch. Alex Hunsaker bada...@gmail.com wrote: I think the explain_analyze_format guc is a clever way of getting around the explain analyze verbose you proposed earlier. But I dont see any doc updates for it. Documentation is added. How about just pg_stat_statements.track ? I renamed the variables to: - pg_stat_statements.limit - pg_stat_statements.track - pg_stat_statements.saved_file I also modified assign_custom_variable_classes() to accept '_' as a prefix character, not only 0-9A-Za-z. I do like the consistency of having the custom gucs be the same as the module name, easy to grep or google for. Should I also rename variables used in auto_explain module? It uses 'explain.*' now. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center pg_stat_statements.1226.tar.gz Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
