Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Martijn van Oosterhout wrote: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( See pg_list.h (list_head) for an example. I think we can tolerate this for the three functions in pg_list.h because they are so few and so tiny, but I'm not sure about PGACE because they are a large lot. On the other hand, turning them to real functions would be a performance hit. Really? C99 requires it and MSVC does support it. At least the other compilers whose name I remembered (HP, Sun) support it also. I'd be surprised if a compiler didn't since it's the form of inline that most matches what people expect to happen. Do you have an example? I have no preference either of them, because it is not an essence of my patches whether its security hooks are implemented as inline, or not. IIRC, indeed, some of compiler also supported static inline. However, it also seems to me that PostgreSQL implementation tend to avoid to use inline functions actively. For example, heap_getattr() and fastgetattr() are implemented as macros, even if they have a bit complex conditional branches, which can be rewritten more simple with inline functions. If we have any policy to use inline functions, I'll follow this. Thanks, http://www.greenend.org.uk/rjk/2003/03/inline.html http://hi.baidu.com/junru/blog/item/4d8db11339050c856438db7a.html Have a nice day, -- 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] Latest version of Hot Standby patch
On Wed, 2009-01-14 at 08:27 +0200, Heikki Linnakangas wrote: Thanks for picking this up, despite my report was so vague. Simon Riggs wrote: Best way seems to be to do (almost) the same thing as ExtendSubtrans() during recovery, so we zero out pages at the point that recovering transactions get written to pg_subtrans. Yep. Do we have the same bug with ExtendCLOG? No, the clog issues WAL records for that. 8 times less than subtrans would if we allowed it to, so its not as bad performance wise either. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Assertion failure in plpgsql with INSTEAD OF rule
Tom Lane wrote: So what I'm thinking is: 1. We can't redefine the SPI interface in back branches, so there's probably little alternative but to remove those Asserts there. Committed this. 2. In HEAD, I think we should have SPI return a new SPI_OK_REWRITTEN code for this case, and have plpgsql respond to that by always setting found = false. With that, the Asserts can stay (in fact the new path should assert !mod_stmt, since it shouldn't have found any canSetTag query). I'll look into this as a separate patch. -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Martijn van Oosterhout wrote: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( See pg_list.h (list_head) for an example. I think we can tolerate this for the three functions in pg_list.h because they are so few and so tiny, but I'm not sure about PGACE because they are a large lot. On the other hand, turning them to real functions would be a performance hit. Really? C99 requires it and MSVC does support it. Apparently we're stuck with C89 for some time yet; maybe an upgrade can be argued, if a large gain by the jump to C99 can be demonstrated: http://archives.postgresql.org/pgsql-hackers/2008-11/msg01494.php -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] solaris libpq threaded build fails
Andrew Chernow wrote: The problem with the current check is its only an AC_CHECK_FUNCS. We need an AC_SEARCH_LIBS first so the proper -llibrary is appended to LIBS, which is used by AC_CHECK_FUNCS. AC_SEARCH_LIBS(gethostbyname_r, c nsl) Just don't put c in there. You usually don't want an explicit -lc to appear in your link commands. -- 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] solaris libpq threaded build fails
AC_SEARCH_LIBS(gethostbyname_r, c nsl) Just don't put c in there. You usually don't want an explicit -lc to appear in your link commands. Correct. Copied that from an internal project, which I should fix. -- 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
Re: [HACKERS] Documenting pglesslog
Koichi Suzuki wrote: Pg_readahead is a tool to prefetch data pages before redoing, based on the contents of archive/active WAL segment. For 8.3 and 8.4 without sync.rep, this works together with restore_command. Pg_readahead analyze WAL segment, schedule and issue posix_fadvise() to prefetch data pages quickly before redoing. Discussions and materials will be found at http://archives.postgresql.org/pgsql-hackers/2008-10/msg01372.php So far, external command implemantation speeds up PITR up to six times! Therefore, overall recovery time can be a little longer than that with FPW. Now that 8.4 is using fsync, sounds like something that should be integrated into the core code, rather than as a /contrib. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] reloptions with a namespace
Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: This uses a new parse node. You need at least equalfuncs.c support for that, and outfuncs would be advisable. Added. One possible disadvantage is that a command like this works, but does nothing: alvherre=# alter table foo set (test.foo = 1); Uh, why doesn't it throw an error? We throw error for unrecognized reloptions in general. I wasn't sure of the best place to add a check. I have added it to transformRelOptions; I am not entirely comfortable with it, because it works, but it still allows this: alvherre=# alter index f set (toast.fillfactor = 20); ALTER INDEX The original case now fails correctly with alvherre=# alter table foo set (test.foo = 1); ERROR: 22023: unrecognized parameter namespace test UBICACIÓN: transformRelOptions, /pgsql/source/04toastopt/src/backend/access/common/reloptions.c:490 alvherre=# alter table foo set (toast.foo = 1); ERROR: 22023: unrecognized parameter foo UBICACIÓN: parseRelOptions, /pgsql/source/04toastopt/src/backend/access/common/reloptions.c:694 -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
KaiGai Kohei wrote: Martijn van Oosterhout wrote: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( See pg_list.h (list_head) for an example. I think we can tolerate this for the three functions in pg_list.h because they are so few and so tiny, but I'm not sure about PGACE because they are a large lot. On the other hand, turning them to real functions would be a performance hit. Really? C99 requires it and MSVC does support it. At least the other compilers whose name I remembered (HP, Sun) support it also. I'd be surprised if a compiler didn't since it's the form of inline that most matches what people expect to happen. Do you have an example? I have no preference either of them, because it is not an essence of my patches whether its security hooks are implemented as inline, or not. IIRC, indeed, some of compiler also supported static inline. However, it also seems to me that PostgreSQL implementation tend to avoid to use inline functions actively. For example, heap_getattr() and fastgetattr() are implemented as macros, even if they have a bit complex conditional branches, which can be rewritten more simple with inline functions. I thought one advantage of using macros is that we force the inlining, while I think inline compiler directives are more of a hint, but maybe the compiler knows better than we do in some cases. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Martijn van Oosterhout klep...@svana.org writes: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( Really? C99 requires it and MSVC does support it. Wrong. What C99 requires is a uselessly cumbersome form of inline that is not compatible with the GCC feature. We did actually implement C99-compatible inlines in one or two places (in the sorting code IIRC), but it's not something that I want to put up with on a large scale. If you ran the current sepostgres patch through an actual C99 compiler, it would fail. 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Bruce Momjian br...@momjian.us writes: KaiGai Kohei wrote: However, it also seems to me that PostgreSQL implementation tend to avoid to use inline functions actively. I thought one advantage of using macros is that we force the inlining, The (only) good thing about macros is they're portable: they work, and work the same, on every C compiler. This cannot be said of inline. 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] reloptions with a namespace
Alvaro Herrera escreveu: I wasn't sure of the best place to add a check. I have added it to transformRelOptions; I am not entirely comfortable with it, because it works, but it still allows this: IMHO it's the appropriate place. alvherre=# alter index f set (toast.fillfactor = 20); ALTER INDEX Maybe you need to add relopt_kind test in your validation code. -- Euler Taveira de Oliveira http://www.timbira.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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Tom Lane wrote: If you ran the current sepostgres patch through an actual C99 compiler, it would fail. The current one (r1408) is reworked to use normal functions, and inlines are eliminated. :-) http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/include/security/sepgsql.h http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/backend/security/pgaceHooks.c Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
On Wed, Jan 14, 2009 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Bruce Momjian br...@momjian.us writes:
KaiGai Kohei wrote:
However, it also seems to me that PostgreSQL implementation tend to
avoid to use inline functions actively.
I thought one advantage of using macros is that we force the inlining,
The (only) good thing about macros is they're portable: they work,
and work the same, on every C compiler. This cannot be said of inline.
Just out of curiosity, does C89, or whatever standard we follow, allow this?
int
somefunc(int x)
{
int foo[x];
/* use foo[] for scratch space */
}
Obviously this is a bad plan if x can be a big number because you
might crash your stack, but suppose we know that's not an issue? It
seems a shame to have to do palloc/pfree in a situation like this.
...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] reloptions with a namespace
Euler Taveira de Oliveira wrote:
Alvaro Herrera escreveu:
I wasn't sure of the best place to add a check. I have added it to
transformRelOptions; I am not entirely comfortable with it, because it
works, but it still allows this:
IMHO it's the appropriate place.
I think the best place would be parseRelOptions. The problem is that
transformRelOptions does not apply any semantics to the values it's
parsing; it doesn't know about the relopt_kind for example. That stuff
is only known by parseRelOptions, but when the options reach that point,
they have already lost the namespace (due to transformRelOptions).
alvherre=# alter index f set (toast.fillfactor = 20);
ALTER INDEX
Maybe you need to add relopt_kind test in your validation code.
No clean way to do that :-(
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/access/common/reloptions.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.18
diff -c -p -r1.18 reloptions.c
*** src/backend/access/common/reloptions.c 12 Jan 2009 21:02:14 - 1.18
--- src/backend/access/common/reloptions.c 14 Jan 2009 14:32:26 -
*** add_string_reloption(int kind, char *nam
*** 390,397
}
/*
! * Transform a relation options list (list of DefElem) into the text array
! * format that is kept in pg_class.reloptions.
*
* This is used for three cases: CREATE TABLE/INDEX, ALTER TABLE SET, and
* ALTER TABLE RESET. In the ALTER cases, oldOptions is the existing
--- 390,399
}
/*
! * Transform a relation options list (list of ReloptElem) into the text array
! * format that is kept in pg_class.reloptions, including only those options
! * that are in the passed namespace. The output values do not include the
! * namespace.
*
* This is used for three cases: CREATE TABLE/INDEX, ALTER TABLE SET, and
* ALTER TABLE RESET. In the ALTER cases, oldOptions is the existing
*** add_string_reloption(int kind, char *nam
*** 402,414
* in the list (it will be or has been handled by interpretOidsOption()).
*
* Note that this is not responsible for determining whether the options
! * are valid.
*
* Both oldOptions and the result are text arrays (or NULL for default),
* but we declare them as Datums to avoid including array.h in reloptions.h.
*/
Datum
! transformRelOptions(Datum oldOptions, List *defList,
bool ignoreOids, bool isReset)
{
Datum result;
--- 404,416
* in the list (it will be or has been handled by interpretOidsOption()).
*
* Note that this is not responsible for determining whether the options
! * are valid, but it does check that the namespaces given are known.
*
* Both oldOptions and the result are text arrays (or NULL for default),
* but we declare them as Datums to avoid including array.h in reloptions.h.
*/
Datum
! transformRelOptions(Datum oldOptions, List *defList, char *namspace,
bool ignoreOids, bool isReset)
{
Datum result;
*** transformRelOptions(Datum oldOptions, Li
*** 444,454
/* Search for a match in defList */
foreach(cell, defList)
{
! DefElem*def = lfirst(cell);
! int kw_len = strlen(def-defname);
if (text_len kw_len text_str[kw_len] == '='
! pg_strncasecmp(text_str, def-defname, kw_len) == 0)
break;
}
if (!cell)
--- 446,468
/* Search for a match in defList */
foreach(cell, defList)
{
! ReloptElem *def = lfirst(cell);
! int kw_len;
+ /* ignore if not in the same namespace */
+ if (namspace == NULL)
+ {
+ if (def-nmspc != NULL)
+ continue;
+ }
+ else if (def-nmspc == NULL)
+ continue;
+ else if (pg_strcasecmp(def-nmspc, namspace) != 0)
+ continue;
+
+ kw_len = strlen(def-optname);
if (text_len kw_len text_str[kw_len] == '='
! pg_strncasecmp(text_str, def-optname, kw_len) == 0)
break;
}
if (!cell)
*** transformRelOptions(Datum oldOptions, Li
*** 468,474
*/
foreach(cell, defList)
{
! DefElem*def = lfirst(cell);
if (isReset)
{
--- 482,493
*/
foreach(cell, defList)
{
! ReloptElem*def = lfirst(cell);
!
! if (def-nmspc pg_strcasecmp(def-nmspc, toast) != 0)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg(unrecognized parameter namespace \%s\, def-nmspc)));
if (isReset)
{
*** transformRelOptions(Datum oldOptions, Li
*** 483,504
const char *value;
Size len;
! if (ignoreOids pg_strcasecmp(def-defname, oids) == 0)
continue;
/*
! * Flatten the DefElem into a text string like name=arg. If we
! * have just name, assume name=true is meant.
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Robert Haas robertmh...@gmail.com writes:
Just out of curiosity, does C89, or whatever standard we follow, allow this?
int
somefunc(int x)
{
int foo[x];
/* use foo[] for scratch space */
}
It's not in C89 but look up alloca.
We don't use it anywhere in postgres currently so it's kind of unlikely we
would start now.
I think C99 does allow what you typed, and I think gcc has an extension to
allow it too.
Obviously this is a bad plan if x can be a big number because you
might crash your stack, but suppose we know that's not an issue? It
seems a shame to have to do palloc/pfree in a situation like this.
palloc really isn't that expensive, unless you're allocating tons of tiny
objects or you're in a tight loop it's not worth worrying about.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!
--
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] A single escape required for log_filename
On Wednesday 14 January 2009 05:23:53 Tom Lane wrote: However, since there's no standard strftime escape for epoch, Robert's proposal to rip out the functionality would break any existing code that still depends on this formatting option. If it came down to this, then I'd say rip it out. Naming log files by epoch isn't exactly a user-friendly practice anyway, and there are equivalent but more readable formatting options available. -- 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] A single escape required for log_filename
On Wed, 2009-01-14 at 18:37 +0200, Peter Eisentraut wrote: On Wednesday 14 January 2009 05:23:53 Tom Lane wrote: However, since there's no standard strftime escape for epoch, Robert's proposal to rip out the functionality would break any existing code that still depends on this formatting option. If it came down to this, then I'd say rip it out. Naming log files by epoch isn't exactly a user-friendly practice anyway, and there are equivalent but more readable formatting options available. +1 Joshua D. Drake -- PostgreSQL - XMPP: jdr...@jabber.postgresql.org Consulting, Development, Support, Training 503-667-4564 - http://www.commandprompt.com/ The PostgreSQL Company, serving since 1997 -- 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] solaris libpq threaded build fails
Andrew Chernow wrote:
Forgot to mention, there is an easy fix:
~]# LDFLAGS=-lnsl ./configure --enable-thread-safety
But I assume that only works if I use gethostbyname_r(), right?
No, works for gethostbyname as well. They are all in libnsl.
But we do check for that in thread_test.c.
The problem with the current check is its only an AC_CHECK_FUNCS.
We need an AC_SEARCH_LIBS first so the proper -llibrary is
appended to LIBS, which is used by AC_CHECK_FUNCS.
AC_SEARCH_LIBS(gethostbyname_r, c nsl) AC_CHECK_FUNCS([strerror_r
getpwuid_r gethostbyname_r])
(AC_CHECK_FUNCS from configure.in line 1371)
OK, patch attached and applied to CVS HEAD. The nsl (not 'nls') library
check was removed in Postgres 8.2 here:
http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/configure.in.diff?r1=1.448;r2=1.445;f=h
The new code is more specific by testing for gethostbyname_r() and only
on Solaris. I also added a comment about why it was re-added.
The next question is do we backpatch this back to 8.2?
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.626
diff -c -c -r1.626 configure
*** configure 11 Jan 2009 18:02:15 - 1.626
--- configure 14 Jan 2009 16:37:21 -
***
*** 7743,7748
--- 7743,7836
fi
+ # Required for thread_test.c on Solaris 2.5:
+ case $host_os in
+ solaris*)
+ { echo $as_me:$LINENO: checking for library containing gethostbyname_r 5
+ echo $ECHO_N checking for library containing gethostbyname_r... $ECHO_C 6; }
+ if test ${ac_cv_search_gethostbyname_r+set} = set; then
+ echo $ECHO_N (cached) $ECHO_C 6
+ else
+ ac_func_search_save_LIBS=$LIBS
+ cat conftest.$ac_ext _ACEOF
+ /* confdefs.h. */
+ _ACEOF
+ cat confdefs.h conftest.$ac_ext
+ cat conftest.$ac_ext _ACEOF
+ /* end confdefs.h. */
+
+ /* Override any GCC internal prototype to avoid an error.
+Use char because int might match the return type of a GCC
+builtin and then its argument prototype would still apply. */
+ #ifdef __cplusplus
+ extern C
+ #endif
+ char gethostbyname_r ();
+ int
+ main ()
+ {
+ return gethostbyname_r ();
+ ;
+ return 0;
+ }
+ _ACEOF
+ for ac_lib in '' nsl; do
+ if test -z $ac_lib; then
+ ac_res=none required
+ else
+ ac_res=-l$ac_lib
+ LIBS=-l$ac_lib $ac_func_search_save_LIBS
+ fi
+ rm -f conftest.$ac_objext conftest$ac_exeext
+ if { (ac_try=$ac_link
+ case (($ac_try in
+ *\* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+ *) ac_try_echo=$ac_try;;
+ esac
+ eval echo \\$as_me:$LINENO: $ac_try_echo\) 5
+ (eval $ac_link) 2conftest.er1
+ ac_status=$?
+ grep -v '^ *+' conftest.er1 conftest.err
+ rm -f conftest.er1
+ cat conftest.err 5
+ echo $as_me:$LINENO: \$? = $ac_status 5
+ (exit $ac_status); } {
+ test -z $ac_c_werror_flag ||
+ test ! -s conftest.err
+} test -s conftest$ac_exeext
+$as_test_x conftest$ac_exeext; then
+ ac_cv_search_gethostbyname_r=$ac_res
+ else
+ echo $as_me: failed program was: 5
+ sed 's/^/| /' conftest.$ac_ext 5
+
+
+ fi
+
+ rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+ conftest$ac_exeext
+ if test ${ac_cv_search_gethostbyname_r+set} = set; then
+ break
+ fi
+ done
+ if test ${ac_cv_search_gethostbyname_r+set} = set; then
+ :
+ else
+ ac_cv_search_gethostbyname_r=no
+ fi
+ rm conftest.$ac_ext
+ LIBS=$ac_func_search_save_LIBS
+ fi
+ { echo $as_me:$LINENO: result: $ac_cv_search_gethostbyname_r 5
+ echo ${ECHO_T}$ac_cv_search_gethostbyname_r 6; }
+ ac_res=$ac_cv_search_gethostbyname_r
+ if test $ac_res != no; then
+ test $ac_res = none required || LIBS=$ac_res $LIBS
+
+ fi
+
+ ;;
+ esac
# Cygwin:
{ echo $as_me:$LINENO: checking for library containing shmget 5
echo $ECHO_N checking for library containing shmget... $ECHO_C 6; }
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.585
diff -c -c -r1.585 configure.in
*** configure.in 11 Jan 2009 18:02:17 - 1.585
--- configure.in 14 Jan 2009 16:37:21 -
***
*** 870,875
--- 870,881
AC_SEARCH_LIBS(crypt, crypt)
# Solaris:
AC_SEARCH_LIBS(fdatasync, [rt posix4])
+ # Required for thread_test.c on Solaris 2.5:
+ case $host_os in
+ solaris*)
+ AC_SEARCH_LIBS(gethostbyname_r, nsl)
+ ;;
+ esac
# Cygwin:
AC_SEARCH_LIBS(shmget, cygipc)
--
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] solaris libpq threaded build fails
On 1/14/09, Bruce Momjian br...@momjian.us wrote: OK, patch attached and applied to CVS HEAD. The nsl (not 'nls') library check was removed in Postgres 8.2 here: As long as you are looking at this, can you take a peek at this patch? http://archives.free.net.ph/message/20081116.053100.15b5801d.fi.html We had a similar problem on hpux 10.20. This was more invasive change though.. At the time, Tom was ambivalent and Dunstan voted that the platform was too old, so it never went in. merlin -- 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] solaris libpq threaded build fails
Merlin Moncure wrote: On 1/14/09, Bruce Momjian br...@momjian.us wrote: OK, patch attached and applied to CVS HEAD. The nsl (not 'nls') library check was removed in Postgres 8.2 here: As long as you are looking at this, can you take a peek at this patch? http://archives.free.net.ph/message/20081116.053100.15b5801d.fi.html We had a similar problem on hpux 10.20. This was more invasive change though.. At the time, Tom was ambivalent and Dunstan voted that the platform was too old, so it never went in. Thanks, I looked it over. It does have the danger of affecting other platforms, so there would have to be more checks in there. Also the calling of the function with all null pointers seems dangerous, though I am not sure how else we could test this. It might be safer to create a conflicting function prototype and see if that throws a warning. I did a Google search and it seems there isn't an easy way to do the configure check except the approach you have taken. Your changes to thread.c seem fine. Comp.programming.threads, has a pretty ugly autoconf example in their FAQ with the conclusion: http://www.lambdacs.com/cpt/FAQ.html Whom do I shoot? take your pick :-( Let me see if I can work up a more minimal patch. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] A single escape required for log_filename
If it came down to this, then I'd say rip it out. Naming log files by epoch isn't exactly a user-friendly practice anyway, and there are equivalent but more readable formatting options available. There are other alternatives but they're all ugly. For example, we could make %0 (or some sequence that isn't taken) evaluate to the empty string, or we could add another GUC to control the interpretation of this GUC. But I don't think the ugliness is justified in this instance. ...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] solaris libpq threaded build fails
Bruce Momjian wrote: Also the calling of the function with all null pointers seems dangerous, Its only trying to compile it, AC_TRY_COMPILE, not execute it. I don't think? the NULL pointers could ever raise havoc. -- 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
Re: [HACKERS] New patch for Column-level privileges
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: The attached patch put invocations of markColumnForSelectPriv() at transformJoinUsingClause() to mark those columns are used. Thanks for the update! Attached is a patch which: - incorporates KaiGai's latest patches to deal with JOINs and NATURAL JOINs - adds regression tests following Tom's suggestion to check whole-row vars in the face of column add/deletes - adds regression tests for NATURAL JOIN and successful JOINs with table sub-sets - reworks pg_attribute_aclmask() to remove the looping component - adds a new pg_attribute_aclcheck_all() to handle the ANY/ALL needs of execMain and the looping - removes special handling of system columns, they can still be granted/revoked, but they won't be included in ANY/ALL tests and a table-wide REVOKE won't affect them. After thinking about it for a while, I felt this was the most sensible compromise between code complexity, following the SQL spec, and user freedom. - split out adding column revokes for table-level commands into a add_col_revokes function to clean up ExecGrant_Relation a bit. - when handling table-level revokes, skips over columns which do not have an ACL defined, since it clearly has no effect except to force creation of a default ACL that's just clutter. Comments, testing, etc, most appreciated! Thanks, Stephen colprivs_2009011401.diff.gz Description: Binary data signature.asc Description: Digital signature
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1403)
It's not in C89 but look up alloca. I know about alloca... We don't use it anywhere in postgres currently so it's kind of unlikely we would start now. :-( Obviously this is a bad plan if x can be a big number because you might crash your stack, but suppose we know that's not an issue? It seems a shame to have to do palloc/pfree in a situation like this. palloc really isn't that expensive, unless you're allocating tons of tiny objects or you're in a tight loop it's not worth worrying about. Yeah... but... It really depends on what you compare it to. It's cheap compared to 99% of the functions in the code base - perhaps so. But it's darn expensive compared to moving the stack pointer. I have seen profiles for PostgreSQL and other systems where memory management is a sizable percentage of the CPU time, so it is not silly to worry about economizing. ...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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
On Wed, Jan 14, 2009 at 09:52:20AM -0500, Tom Lane wrote: Martijn van Oosterhout klep...@svana.org writes: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( Really? C99 requires it and MSVC does support it. Wrong. What C99 requires is a uselessly cumbersome form of inline that is not compatible with the GCC feature. We did actually implement C99-compatible inlines in one or two places (in the sorting code IIRC), but it's not something that I want to put up with on a large scale. I was talking about static inline, where C99 agrees completely with GCC and is significantly more portable. Have a nice day, -- Martijn van Oosterhout klep...@svana.org 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] Proposed Patch to Improve Performance of Multi-BatchHash Join for Skewed Data Sets
Here is a cleaned-up version. I fixed a number of whitespace issues, improved a few comments, and rearranged one set of nested if-else statements (hopefully without breaking anything in the process). Josh / eggyknap - Can you rerun your performance tests with this version of the patch? To help with testing, we have constructed a patch specifically for testing. The patch is the same as Robert's version except that it tracks and prints out statistics during the join on how many tuples are affected and has the enable_hashjoin_usestatmcvs variable defined so that it is easy to turn on/off skew handling. This is useful as although the patch reduces the number of I/Os performed, this improvement may not be seen in some queries which are dominated by other cost factors (non-skew joins, CPU time, time to scan input relations, etc.). The sample output looks like this: LI-P Values: 100 Skew: 0.27 Est. tuples: 59986052.00 Batches: 512 Est. Save: 16114709.99 Total Inner Tuples: 200 IM Inner Tuples: 83 Batch Zero Inner Tuples: 3941 Batch Zero Potential Inner Tuples: 3941 Total Outer Tuples: 59986052 IM Outer Tuples: 16074146 Batch Zero Outer Tuples: 98778 Batch Zero Potential Outer Tuples: 98778 Total Output Tuples: 59986052 IM Output Tuples: 16074146 Batch Zero Output Tuples: 98778 Batch Zero Potential Output Tuples: 98778 Percentage less tuple IOs than HHJ: 25.98 The other change is that the system calculates the skew and will not use the in-memory skew partition if the skew is less than 1%. Finally, we have attached some performance results for the TPCH 10G data set (skew factors z=1 and z=2). For the Customer-Orders-Lineitem-Part query that Josh was testing, we see no overall time difference that is significant compared to experimental error (although there is I/O benefit for the Lineitem-Part join). This query cost is dominated by the non-skew joins of Customer-Orders and Orders-Lineitem and output tuple construction. The joins with skew, Lineitem-Supplier and Lineitem-Part, show significantly improved performance. Note how the statistics show that the percentage I/O savings is directly proportional to the skew. However, the overall query time savings is always less than this as there are other costs such as reading the relations, performing the hash comparisons, building the output tuples, etc. that are unaffected by the optimization. At this point, we await further feedback on what is necessary to get this patch accepted. We would also like to thank Josh and Robert again for their review time. Sincerely, Ramon Lawrence and Bryce Cutt histojoin_testing.patch Description: histojoin_testing.patch TPC-H 10G Skew Factor Z=1 results - LI-P Regular HJ: (time in milliseconds) 990344 1022562 1071250 1003219 1049000 989953 Average: 1021054.667 LI-P Skew-enabled HJ: (time in milliseconds) 883593 960860 934906 1007282 937406 948078 Average: 945354.1667 % Difference: 7.4% LI-P Values: 100 Skew: 0.27 Est. tuples: 59986052.00 Batches: 512 Est. Save: 16114709.99 Total Inner Tuples: 200 IM Inner Tuples: 83 Batch Zero Inner Tuples: 3941 Batch Zero Potential Inner Tuples: 3941 Total Outer Tuples: 59986052 IM Outer Tuples: 16074146 Batch Zero Outer Tuples: 98778 Batch Zero Potential Outer Tuples: 98778 Total Output Tuples: 59986052 IM Output Tuples: 16074146 Batch Zero Output Tuples: 98778 Batch Zero Potential Output Tuples: 98778 Percentage less tuple IOs than HHJ: 25.98 LI-P-S Regular HJ: (time in milliseconds) 1833016 1567515 1504625 Average: 1635052 LI-P-S Skew-enabled HJ: (time in milliseconds) 883593 1280297 1423984 Average: 1195958 % Difference: 27% LI-S Values: 100 Skew: 0.19 Est. tuples: 59986052.00 Batches: 32 Est. Save: 11097357.16 Total Inner Tuples: 10 IM Inner Tuples: 78 Batch Zero Inner Tuples: 3123 Batch Zero Potential Inner Tuples: 3125 Total Outer Tuples: 59986052 IM Outer Tuples: 11563695 Batch Zero Outer Tuples: 1577432 Batch Zero Potential Outer Tuples: 1693632 Total Output Tuples: 59986052 IM Output Tuples: 11563695 Batch Zero Output Tuples: 1577432 Batch Zero Potential Output Tuples: 1693632 Percentage less tuple IOs than HHJ: 19.61 (LI-S)-P Values: 100 Skew: 0.27 Est. tuples: 59986052.00 Batches: 512 Est. Save: 16114709.99 Total Inner Tuples: 200 IM Inner Tuples: 83 Batch Zero Inner Tuples: 3941 Batch Zero Potential Inner Tuples: 3941 Total Outer Tuples: 59986052 IM Outer Tuples: 16074146 Batch Zero Outer Tuples: 98778 Batch Zero Potential Outer Tuples: 98778 Total Output Tuples: 59986052 IM Output Tuples: 16074146 Batch Zero Output Tuples: 98778 Batch Zero Potential Output Tuples: 98778 Percentage less tuple IOs than HHJ: 25.98 TPC-H 10G Skew Factor Z=2 results - LI-P Regular HJ: (time in milliseconds) 505672 424922 303250 361610 358125 Average: 390715.8 LI-P Skew-enabled HJ: (time in milliseconds) 219078 210078 212938 210094 212500 Average: 212937.6 % difference: 45.5%
Re: [HACKERS] solaris libpq threaded build fails
Bruce Momjian wrote:
Merlin Moncure wrote:
On 1/14/09, Bruce Momjian br...@momjian.us wrote:
OK, patch attached and applied to CVS HEAD. The nsl (not 'nls') library
check was removed in Postgres 8.2 here:
As long as you are looking at this, can you take a peek at this patch?
http://archives.free.net.ph/message/20081116.053100.15b5801d.fi.html
We had a similar problem on hpux 10.20. This was more invasive change
though.. At the time, Tom was ambivalent and Dunstan voted that the
platform was too old, so it never went in.
Thanks, I looked it over. It does have the danger of affecting other
platforms, so there would have to be more checks in there. Also the
calling of the function with all null pointers seems dangerous, though I
am not sure how else we could test this. It might be safer to create a
conflicting function prototype and see if that throws a warning.
I did a Google search and it seems there isn't an easy way to do the
configure check except the approach you have taken. Your changes to
thread.c seem fine.
Comp.programming.threads, has a pretty ugly autoconf example in their
FAQ with the conclusion:
http://www.lambdacs.com/cpt/FAQ.html
Whom do I shoot?
take your pick :-(
Let me see if I can work up a more minimal patch.
OK, I ended up doing a compile test as you suggested because I was
worried that a mismatched 'const' might throw an error.
The patch is very similar to the one posted, though perhaps a little
cleaner.
I don't need the #undef _XOPEN_SOURCE_EXTENDED in autoconf because I am
testing for the 5-argument version.
Would someone please test this to make sure it works on their platforms.
Is there any objection to applying this to 8.4? While the operating
system is old, it seems we are having new users use threading on these
older operating systems, hence the need for a patch.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.628
diff -c -c -r1.628 configure
*** configure 14 Jan 2009 18:10:21 - 1.628
--- configure 14 Jan 2009 22:00:25 -
***
*** 20379,20384
--- 20379,20440
+ if test $enable_thread_safety = yes -a x$ac_cv_func_gethostbyname_r = xyes ; then
+ { echo $as_me:$LINENO: checking gethostbyname_r argument count 5
+ echo $ECHO_N checking gethostbyname_r argument count... $ECHO_C 6; }
+ # Check to see if 5-argument call generates an error
+ cat conftest.$ac_ext _ACEOF
+ /* confdefs.h. */
+ _ACEOF
+ cat confdefs.h conftest.$ac_ext
+ cat conftest.$ac_ext _ACEOF
+ /* end confdefs.h. */
+ #include netdb.h
+ int
+ main ()
+ {
+ (void) gethostbyname_r (NULL, NULL, NULL, 0, NULL);
+ ;
+ return 0;
+ }
+ _ACEOF
+ rm -f conftest.$ac_objext
+ if { (ac_try=$ac_compile
+ case (($ac_try in
+ *\* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+ *) ac_try_echo=$ac_try;;
+ esac
+ eval echo \\$as_me:$LINENO: $ac_try_echo\) 5
+ (eval $ac_compile) 2conftest.er1
+ ac_status=$?
+ grep -v '^ *+' conftest.er1 conftest.err
+ rm -f conftest.er1
+ cat conftest.err 5
+ echo $as_me:$LINENO: \$? = $ac_status 5
+ (exit $ac_status); } {
+ test -z $ac_c_werror_flag ||
+ test ! -s conftest.err
+} test -s conftest.$ac_objext; then
+
+ cat confdefs.h \_ACEOF
+ #define GETHOSTBYNAME_R_ARGCNT 3
+ _ACEOF
+ { echo $as_me:$LINENO: result: 5 5
+ echo ${ECHO_T}5 6; }
+ else
+ echo $as_me: failed program was: 5
+ sed 's/^/| /' conftest.$ac_ext 5
+
+
+ cat confdefs.h \_ACEOF
+ #define GETHOSTBYNAME_R_ARGCNT 5
+ _ACEOF
+ { echo $as_me:$LINENO: result: 3 5
+ echo ${ECHO_T}3 6; }
+ fi
+
+ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ fi
# This test makes sure that run tests work at all. Sometimes a shared
# library is found by the linker, but the runtime linker can't find it.
Index: configure.in
===
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.587
diff -c -c -r1.587 configure.in
*** configure.in 14 Jan 2009 18:10:21 - 1.587
--- configure.in 14 Jan 2009 22:00:25 -
***
*** 1417,1422
--- 1417,1430
AC_SUBST(LDAP_LIBS_FE)
AC_SUBST(LDAP_LIBS_BE)
+ if test $enable_thread_safety = yes -a x$ac_cv_func_gethostbyname_r = xyes ; then
+ AC_MSG_CHECKING([gethostbyname_r argument count])
+ # Check to see if 5-argument call generates an error
+ AC_TRY_COMPILE([#include netdb.h],
+ [(void) gethostbyname_r (NULL, NULL, NULL, 0, NULL);],
+ [AC_DEFINE(GETHOSTBYNAME_R_ARGCNT, 3, [Define to the number of arguments gethostbyname_r accepts]) AC_MSG_RESULT(5)],
+ [AC_DEFINE(GETHOSTBYNAME_R_ARGCNT, 5, [Define to the number of arguments gethostbyname_r
Re: [HACKERS] async notification patch for dblink
What is the status on this? --- Marcus Kempe wrote: This patch adds the ability to retrieve async notifications using dblink, via the addition of the function dblink_get_notify. It is written against cvs head, includes documentation and regression testing. It compiles, tests and works well. I would be interested in some feedback on the regression test. My initial thought was to test the function as thoroughly as possible. So I perform listen and notify commands within the test to be able to test all aspects of the code. Even though this works well for me, I get the feeling that this is not the correct way to do it. I can find no other testing of the listen/notify functionality in the regression tests, and I imagine this is for good reason. If someone would care to explain this, and maybe give a hint about what amount of testing is appropriate for this fairly trivial patch, it would be appreciated. Best regards, Marcus Kempe [ Attachment, skipping... ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] solaris libpq threaded build fails
Bruce Momjian br...@momjian.us writes: Is there any objection to applying this to 8.4? Yes. I don't think we should bother with a one-operating-system patch for an OS version that was obsolete ten years ago. (Even if I am still running it ;-).) If we do this, the next thing will be trying to work around whatever threading bugs exist in the platform, and you can be sure there are some. 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] solaris libpq threaded build fails
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Is there any objection to applying this to 8.4? Yes. I don't think we should bother with a one-operating-system patch for an OS version that was obsolete ten years ago. (Even if I am still running it ;-).) If we do this, the next thing will be trying to work around whatever threading bugs exist in the platform, and you can be sure there are some. True, but from my reading AIX was also in the mix. We can just wait for another bug report, of course. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] solaris libpq threaded build fails
Bruce Momjian br...@momjian.us writes: Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Is there any objection to applying this to 8.4? Yes. I don't think we should bother with a one-operating-system patch for an OS version that was obsolete ten years ago. (Even if I am still running it ;-).) If we do this, the next thing will be trying to work around whatever threading bugs exist in the platform, and you can be sure there are some. True, but from my reading AIX was also in the mix. We can just wait for another bug report, of course. I think the appropriate level of effort is just to document that we don't support threading on HPUX 10.x. And the same for whatever ancient AIX version might have the same problem ... 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] Statement-level triggers and inheritance
Added to TODO: Have statement-level triggers fire for all tables in an inheritance hierarchy --- Greg Sabino Mullane wrote: [ There is text before PGP section. ] -BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 NotDashEscaped: You need GnuPG to verify this message Looks like inheritance causes a statement-level trigger to fire on the last evaluated table in the inheritance chain. Is this the desired behavior? If so, is there any way to predict or drive which child table will be last evaluated? Or any way to have a statement-level trigger fire on the parent table without using the ONLY syntax? I'm converting a parent table from using rules to triggers and would like to use a statement-level trigger to effect this rather than row-level, but don't want to silently prevent moving rows to the child table(s) because the caller forgot to specify 'ONLY'. Test case: CREATE OR REPLACE FUNCTION trigtest() RETURNS TRIGGER LANGUAGE plpgsql AS $_$ BEGIN RAISE NOTICE 'Trigger on table %, level is %', TG_TABLE_NAME, TG_LEVEL; RETURN NULL; END; $_$; DROP TABLE IF EXISTS abc CASCADE; CREATE TABLE abc AS SELECT 123::int AS id; CREATE TRIGGER abctrig1 AFTER UPDATE ON abc FOR EACH STATEMENT EXECUTE PROCEDURE trigtest(); CREATE TRIGGER abctrig2 AFTER UPDATE ON abc FOR EACH ROW EXECUTE PROCEDURE trigtest(); UPDATE abc SET id = id; -- Outputs both as expected: -- NOTICE: Trigger on table abc, level is ROW -- NOTICE: Trigger on table abc, level is STATEMENT CREATE TABLE abckid() INHERITS (abc); UPDATE abc SET id = id; -- Outputs the row-level only: -- NOTICE: Trigger on table abc, level is ROW CREATE TRIGGER abckidtrig AFTER UPDATE ON abckid FOR EACH STATEMENT EXECUTE PROCEDURE trigtest(); UPDATE abc SET id = id; -- Outputs row-level on parent, statement-level on child: -- NOTICE: Trigger on table abc, level is ROW -- NOTICE: Trigger on table abckid, level is STATEMENT CREATE TABLE abckid2() INHERITS (abc); UPDATE abc SET id = id; -- Outputs row-level on parent only: -- NOTICE: Trigger on table abc, level is ROW CREATE TRIGGER abckid2trig AFTER UPDATE ON abckid2 FOR EACH STATEMENT EXECUTE PROCEDURE trigtest(); UPDATE abc SET id = id; -- Outputs row-level on parent, statement-level on one (the latest?) child only: -- NOTICE: Trigger on table abc, level is ROW -- NOTICE: Trigger on table abckid2, level is STATEMENT UPDATE ONLY abc SET id = id; -- Outputs row-level on parent, statement-level on parent: -- NOTICE: Trigger on table abc, level is ROW -- NOTICE: Trigger on table abc, level is STATEMENT -- Greg Sabino Mullane g...@turnstep.com End Point Corporation PGP Key: 0x14964AC8 200811281627 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkkwY5AACgkQvJuQZxSWSsgK8gCeIeAJ1P45EOciwYOBlseezjMt s5EAoM01zRA41nqYJnt4YzY8cmy6SOtc =J1YY -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] pg_stat_all_tables vs NULLs
Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander mag...@hagander.net writes: I've noticed that pg_stat_all_tables returns NULL for idx_scan and idx_tup_fetch if there are no indexes present on a table. Is this actually intended, or is that something that should be fixed? Hmm. I suspect it's an implementation artifact rather than something that was consciously chosen, but on reflection it doesn't seem like a bad thing. If we just COALESCE'd it to zero (which I assume is what you have in mind) then there would be no distinction in the view between you have no indexes and there are indexes but they aren't being used. But does it make sense to look for that information in pg_stat_*_tables, really? If you want to know if an index exists for a table, you'd normally go look in the system tables, not the statistics views, I think. I'd vote to leave it alone, I think. I can go for that as well though. I'd say Let's document it instead then, but it seems the stats views documentation is very short on what actually goes in the fields. But I guess we could just add a (NULL if no indexes are present) to that? In the long term it might be worthwhile to rewrite that section of the docs to focus more on the stats views (giving each it's own section with more information bout it than just a list of fields) and less on the underlying implementation functions. But that's a different day ;-) I don't see any clean place to put this information in our documentation, and since this is the first report of confusion, I don't think we can easily document this. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Visibility map, partial vacuums
Would someone tell me why 'autovacuum_freeze_max_age' defaults to 200M when our wraparound limit is around 2B? Also, is anything being done about the concern about 'vacuum storm' explained below? --- Gregory Stark wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Hmm. It just occurred to me that I think this circumvented the anti-wraparound vacuuming: a normal vacuum doesn't advance relfrozenxid anymore. We'll need to disable the skipping when autovacuum is triggered to prevent wraparound. VACUUM FREEZE does that already, but it's unnecessarily aggressive in freezing. Having seen how the anti-wraparound vacuums work in the field I think merely replacing it with a regular vacuum which covers the whole table will not actually work well. What will happen is that, because nothing else is advancing the relfrozenxid, the age of the relfrozenxid for all tables will advance until they all hit autovacuum_max_freeze_age. Quite often all the tables were created around the same time so they will all hit autovacuum_max_freeze_age at the same time. So a database which was operating fine and receiving regular vacuums at a reasonable pace will suddenly be hit by vacuums for every table all at the same time, 3 at a time. If you don't have vacuum_cost_delay set that will cause a major issue. Even if you do have vacuum_cost_delay set it will prevent the small busy tables from getting vacuumed regularly due to the backlog in anti-wraparound vacuums. Worse, vacuum will set the freeze_xid to nearly the same value for all of the tables. So it will all happen again in another 100M transactions. And again in another 100M transactions, and again... I think there are several things which need to happen here. 1) Raise autovacuum_max_freeze_age to 400M or 800M. Having it at 200M just means unnecessary full table vacuums long before they accomplish anything. 2) Include a factor which spreads out the anti-wraparound freezes in the autovacuum launcher. Some ideas: . we could implicitly add random(vacuum_freeze_min_age) to the autovacuum_max_freeze_age. That would spread them out evenly over 100M transactions. . we could check if another anti-wraparound vacuum is still running and implicitly add a vacuum_freeze_min_age penalty to the autovacuum_max_freeze_age for each running anti-wraparound vacuum. That would spread them out without being introducing non-determinism which seems better. . we could leave autovacuum_max_freeze_age and instead pick a semi-random vacuum_freeze_min_age. This would mean the first set of anti-wraparound vacuums would still be synchronized but subsequent ones might be spread out somewhat. There's not as much room to randomize this though and it would affect how much i/o vacuum did which makes it seem less palatable to me. 3) I also think we need to put a clamp on the vacuum_cost_delay. Too many people are setting it to unreasonably high values which results in their vacuums never completing. Actually I think what we should do is junk all the existing parameters and replace it with a vacuum_nice_level or vacuum_bandwidth_cap from which we calculate the cost_limit and hide all the other parameters as internal parameters. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? Oh, I get it now, 'distdir'. Let me look. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? Uh, I usually run the tag program from the /src directory where there is no 'distdir', but if you want 'distdir' added, just let me know. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] reloptions with a namespace
Alvaro Herrera escreveu: Euler Taveira de Oliveira wrote: Alvaro Herrera escreveu: I wasn't sure of the best place to add a check. I have added it to transformRelOptions; I am not entirely comfortable with it, because it works, but it still allows this: IMHO it's the appropriate place. I think the best place would be parseRelOptions. The problem is that transformRelOptions does not apply any semantics to the values it's parsing; it doesn't know about the relopt_kind for example. That stuff is only known by parseRelOptions, but when the options reach that point, they have already lost the namespace (due to transformRelOptions). [Looking at your patch...] You're right. The only command that doesn't use parseRelOptions() is ALTER INDEX. Is it the case to add a call at index_reloptions() too? If it is not the case, I think you need to pass 'nmspc.relopt=value' instead of 'relopt=value' at transformRelOptions(). Of course, you'll need to add some code at index_reloptions() to validate reloptions. The following message doesn't say much. Isn't it the case to replace 'opt_definition' with 'OptWith' variant at gram.y? euler=# create index fooi on foo(a) with (nmspc.relopt = 32); ERROR: syntax error at or near . LINHA 1: create index fooi on foo(a) with (nmspc.relopt = 32); ^ -- Euler Taveira de Oliveira http://www.timbira.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] Visibility map, partial vacuums
Bruce Momjian wrote: Would someone tell me why 'autovacuum_freeze_max_age' defaults to 200M when our wraparound limit is around 2B? Presumably because of this (from the docs): The commit status uses two bits per transaction, so if autovacuum_freeze_max_age has its maximum allowed value of a little less than two billion, pg_clog can be expected to grow to about half a gigabyte. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documenting pglesslog
Pg_readahead uses posix_fadvise, which is included in Greg's patch and I've already posted pg_readahead patch integrated into the core. Integration with snc.rep. will be a separate patch which will be posted in a couple of days. 2009/1/14 Bruce Momjian br...@momjian.us: Koichi Suzuki wrote: Pg_readahead is a tool to prefetch data pages before redoing, based on the contents of archive/active WAL segment. For 8.3 and 8.4 without sync.rep, this works together with restore_command. Pg_readahead analyze WAL segment, schedule and issue posix_fadvise() to prefetch data pages quickly before redoing. Discussions and materials will be found at http://archives.postgresql.org/pgsql-hackers/2008-10/msg01372.php So far, external command implemantation speeds up PITR up to six times! Therefore, overall recovery time can be a little longer than that with FPW. Now that 8.4 is using fsync, sounds like something that should be integrated into the core code, rather than as a /contrib. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1403)
Martijn van Oosterhout wrote: On Wed, Jan 14, 2009 at 09:52:20AM -0500, Tom Lane wrote: Martijn van Oosterhout klep...@svana.org writes: On Tue, Jan 13, 2009 at 10:05:45AM -0300, Alvaro Herrera wrote: pgace.h: you have a bunch of static inline functions in here. As far as I know this doesn't work in compilers other than GCC :-( Really? C99 requires it and MSVC does support it. Wrong. What C99 requires is a uselessly cumbersome form of inline that is not compatible with the GCC feature. We did actually implement C99-compatible inlines in one or two places (in the sorting code IIRC), but it's not something that I want to put up with on a large scale. I was talking about static inline, where C99 agrees completely with GCC and is significantly more portable. As I noted yesterday, I have no preference between inline and real one. However, I don't think it is a good idea to apply such an arguable manner because of current v8.4 development schedule. All patches are available here for a long time: [1/5] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1408.patch [2/5] http://sepgsql.googlecode.com/files/sepostgresql-utils-8.4devel-3-r1408.patch [3/5] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1408.patch [4/5] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1408.patch [5/5] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1408.patch I would like committer to begin their reviews. If necessary, I can rework/update them with my highest priority. 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] tuplestore potential performance problem
Has this been addressed? --- Hitoshi Harada wrote: 2008/12/3 Tom Lane t...@sss.pgh.pa.us: If this means a lot of contortion/complication in the upper-level code, seems like it'd be better to address the performance issue within tuplestore/buffile. We could keep separate buffers for write and read perhaps. But do you have real evidence of a performance problem? I'd sort of expect the kernel's disk cache to mitigate this pretty well. regards, tom lane I don't have real evidence but reasoned it. No strace was done. So it may not be cased by flushing out but this commit gets performance quite better, to earlier patch performance, around 44sec from around 76sec. http://git.postgresql.org/?p=~davidfetter/window_functions/.git;a=commitdiff;h=87d9b8ac5dca9fae5f3ac4f3218d8fb4eca8b5b0;hp=f1976a9d002b20006ac31ca85db27abcf56e9ea2 where pos = -1 means spool all rows until the end. The earlier approach was buffering all the table and the newer Heikki's approach was buffer on row by row while reading. The newest is buffering row by row while reading during in memory, and holding all the remaining tuples before reading after out to file, something like hybrid method. 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 -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Visibility map, partial vacuums
Andrew Dunstan wrote: Bruce Momjian wrote: Would someone tell me why 'autovacuum_freeze_max_age' defaults to 200M when our wraparound limit is around 2B? Presumably because of this (from the docs): The commit status uses two bits per transaction, so if autovacuum_freeze_max_age has its maximum allowed value of a little less than two billion, pg_clog can be expected to grow to about half a gigabyte. Oh, that's interesting; thanks. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Visibility map, partial vacuums
Bruce Momjian br...@momjian.us writes: Would someone tell me why 'autovacuum_freeze_max_age' defaults to 200M when our wraparound limit is around 2B? I suggested raising it dramatically in the post you quote and Heikki pointed it controls the maximum amount of space the clog will take. Raising it to, say, 800M will mean up to 200MB of space which might be kind of annoying for a small database. It would be nice if we could ensure the clog got trimmed frequently enough on small databases that we could raise the max_age. It's really annoying to see all these vacuums running 10x more often than necessary. The rest of the thread is visible at the bottom of: http://article.gmane.org/gmane.comp.db.postgresql.devel.general/107525 Also, is anything being done about the concern about 'vacuum storm' explained below? I'm interested too. -- 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
Re: [HACKERS] visibility maps and heap_prune
Is this something for 8.4 CVS? --- Pavan Deolasee wrote: On Mon, Dec 8, 2008 at 11:33 AM, Pavan Deolasee pavan.deola...@gmail.comwrote: On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: If you see a straightforward way, please submit a patch! Will do that. Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all tuples in the page are visible to all transactions and there are no DEAD line pointers in the page. The second check is required so that VACUUM takes up the page. We could slightly distinguish the two cases (one where the page requires vacuuming only because of DEAD line pointers and the other where the page-tuples do not require any visibility checks), but I thought its not worth the complexity. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com [ Attachment, skipping... ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] visibility maps
Is there anything to do with the below issue? --- Pavan Deolasee wrote: On Wed, Dec 17, 2008 at 7:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think what you are suggesting is that we should set the visibility map bit while dead line pointers (tombstones) still remain. If that's what you meant it's a bad idea. No, I'm not suggesting that. I understand the problem there. I was merely explaining how HOT-prune may some time (when there are no DEAD line pointers created) help us set the visibility bit early. OTOH I think we can still set PD_ALL_VISIBLE page header flag even when there are DEAD line pointers. Thanks, Pavan -- Pavan Deolasee 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 -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? Uh, I usually run the tag program from the /src directory where there is no 'distdir', but if you want 'distdir' added, just let me know. No, I'm just pointing out that the distdir rule has a find invocation, and it excludes directories named CVS. So if we exclude .git on the make_[ce]tags scripts, maybe we should on make distdir too. On the other hand, I'm not sure who (if anyone) runs make distdir, so maybe this is not an issue. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera wrote: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? Uh, I usually run the tag program from the /src directory where there is no 'distdir', but if you want 'distdir' added, just let me know. No, I'm just pointing out that the distdir rule has a find invocation, and it excludes directories named CVS. So if we exclude .git on the make_[ce]tags scripts, maybe we should on make distdir too. On the other hand, I'm not sure who (if anyone) runs make distdir, so maybe this is not an issue. Oh, OK, I didn't realize that. Done. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. I have to admit that I find this whole idea a bit weird. Is it common to symlink the tags file? What I did was tell my editor that the tags file is to be found on the root source directory. It seems the most natural way to do this. Am I alone in this? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] New patch for Column-level privileges
Stephen, I could find a strange behavior unfortunatelly. :-)
(But it is obvious one, don't worry.)
postgres=# CREATE TABLE t1 (a int, b int, c int);
CREATE TABLE
postgres=# ALTER TABLE t1 DROP COLUMN c;
ALTER TABLE
postgres=# \c - ymj
psql (8.4devel)
You are now connected to database postgres as user ymj.
postgres= SELECT 1 FROM t1;
DEBUG: pg_attribute_aclmask: t1.a required: 0002 allowed:
DEBUG: pg_attribute_aclmask: t1.b required: 0002 allowed:
?column?
--
(0 rows)
It is a case to be failed because 'ymj' has no privileges on t1
and its columns, but it does not raise any error.
In this case, t1 has three columns but one of them has already dropped.
Your pg_attribute_aclcheck_all() tries to check all the general columns
on the given relation, and it returns ACLCHECK_OK if one of them are
allowed and (how == ACLMASK_ANY).
+ AclResult
+ pg_attribute_aclcheck_all(Oid table_oid, Oid roleid, AclMode mode,
+ AclMaskHow how)
+ {
:
+ for (curr_att = 1; curr_att = nattrs; curr_att++)
+ {
+ if (pg_attribute_aclmask(table_oid,
+ curr_att, roleid, mode, ACLMASK_ANY) != 0)
+ {
+ if (how == ACLMASK_ANY)
+ return ACLCHECK_OK;
+ }
+ else
+ {
+ if (how == ACLMASK_ALL)
+ return ACLCHECK_NO_PRIV;
+ }
+ }
But pg_attribute_aclmask() returns the required privileges set as is,
if target attribute has been already dropped, then pg_attribute_aclcheck_all()
considers it a column is allowed to select at least.
+ AclMode
+ pg_attribute_aclmask(Oid table_oid, AttrNumber att_number, Oid roleid,
+AclMode mask, AclMaskHow how)
+ {
:
+ /* Skip dropped columns */
+ if (((Form_pg_attribute) GETSTRUCT(attTuple))-attisdropped)
+ {
+ /* if we have a detoasted copy, free it */
+ if (relacl (Pointer) relacl != DatumGetPointer(relaclDatum))
+ pfree(relacl);
+
+ ReleaseSysCache(attTuple);
+ ReleaseSysCache(classTuple);
+
+ return mask;
+ }
I think pg_attribute_aclcheck_all() should skip dropped columns,
even if it need a finding-up system cache once more.
And, pg_attribute_aclmask() should raise an error for a request
to dropped column, as if it could not be found.
BTW, what is the official reviewer's opinion?
It seems to me most of the issues on column-level privileges are
resolved, so it is almost ready for getting merged.
Thanks,
Stephen Frost wrote:
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
The attached patch put invocations of markColumnForSelectPriv()
at transformJoinUsingClause() to mark those columns are used.
Thanks for the update!
Attached is a patch which:
- incorporates KaiGai's latest patches to deal with JOINs and
NATURAL JOINs
- adds regression tests following Tom's suggestion to check
whole-row vars in the face of column add/deletes
- adds regression tests for NATURAL JOIN and successful JOINs
with table sub-sets
- reworks pg_attribute_aclmask() to remove the looping component
- adds a new pg_attribute_aclcheck_all() to handle the ANY/ALL
needs of execMain and the looping
- removes special handling of system columns, they can still be
granted/revoked, but they won't be included in ANY/ALL tests and a
table-wide REVOKE won't affect them. After thinking about it for a
while, I felt this was the most sensible compromise between code
complexity, following the SQL spec, and user freedom.
- split out adding column revokes for table-level commands into a
add_col_revokes function to clean up ExecGrant_Relation a bit.
- when handling table-level revokes, skips over columns which do not
have an ACL defined, since it clearly has no effect except to force
creation of a default ACL that's just clutter.
Comments, testing, etc, most appreciated!
Thanks,
Stephen
--
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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera escreveu: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. I have to admit that I find this whole idea a bit weird. Is it common to symlink the tags file? What I did was tell my editor that the tags file is to be found on the root source directory. It seems the most natural way to do this. Am I alone in this? No, I do it too. -- Euler Taveira de Oliveira http://www.timbira.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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Euler Taveira de Oliveira wrote: Alvaro Herrera escreveu: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. I have to admit that I find this whole idea a bit weird. Is it common to symlink the tags file? What I did was tell my editor that the tags file is to be found on the root source directory. It seems the most natural way to do this. Am I alone in this? No, I do it too. Well, I just tell my editor to look for the tags file in the current directory so I don't have to configure it. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Bruce Momjian wrote: Euler Taveira de Oliveira wrote: Alvaro Herrera escreveu: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. I have to admit that I find this whole idea a bit weird. Is it common to symlink the tags file? What I did was tell my editor that the tags file is to be found on the root source directory. It seems the most natural way to do this. Am I alone in this? No, I do it too. Well, I just tell my editor to look for the tags file in the current directory so I don't have to configure it. But you had to configure the make_etags script :-) It just seems that the cruft is in a different place. Don't the tags symlinks show up in diff output and stuff like that? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Re: [COMMITTERS] pgsql: Make 'find' syntax consistent; add .git exclusion to make_ctags.
Alvaro Herrera wrote: Bruce Momjian wrote: Euler Taveira de Oliveira wrote: Alvaro Herrera escreveu: Bruce Momjian wrote: Alvaro Herrera wrote: Bruce Momjian wrote: Log Message: --- Make 'find' syntax consistent; add .git exclusion to make_ctags. Should this consider the distdir target in /GNUmakefile.in too? It only is looking for _directories_ to put 'tags' into. I have to admit that I find this whole idea a bit weird. Is it common to symlink the tags file? What I did was tell my editor that the tags file is to be found on the root source directory. It seems the most natural way to do this. Am I alone in this? No, I do it too. Well, I just tell my editor to look for the tags file in the current directory so I don't have to configure it. But you had to configure the make_etags script :-) It just seems that the cruft is in a different place. Don't the tags symlinks show up in diff output and stuff like that? No because they are not part of CVS. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] New patch for Column-level privileges
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Stephen, I could find a strange behavior unfortunatelly. :-) Glad you're finding these, honestly. Better to find them and deal with them now than after a release. It is a case to be failed because 'ymj' has no privileges on t1 and its columns, but it does not raise any error. In this case, t1 has three columns but one of them has already dropped. Your pg_attribute_aclcheck_all() tries to check all the general columns on the given relation, and it returns ACLCHECK_OK if one of them are allowed and (how == ACLMASK_ANY). Yeah, I'm not too suprised at that, honestly, it was awkward to deal with dropped columns in pg_attribute_aclmask(). I think pg_attribute_aclcheck_all() should skip dropped columns, even if it need a finding-up system cache once more. And, pg_attribute_aclmask() should raise an error for a request to dropped column, as if it could not be found. I've implemented these changes, and updated the regression tests to check for this. Updated patch is attached. BTW, what is the official reviewer's opinion? It seems to me most of the issues on column-level privileges are resolved, so it is almost ready for getting merged. I tend to doubt Tom's had a chance to review it again yet, which is fine, though I'm certainly hopeful the recent focus and our combined efforts mean this patch can be included for 8.4. My personal opinion is that it's ready for beta testing (which is kind of what this feels like we're doing here) and barring any serious issues found during testing is good to go for inclusion. As for areas where there could still be some improvment, I'd love to hear your thoughts and opinions (and others!) on how column-level privileges are handled in ExecuteGrantStmt and into ExecGrant_Relation and how we might improve it. I don't like the nested for() loops in ExecuteGrantStmt, but I don't see any easy way to resolve that and keep the SQL-required syntax. As for ExecGrant_Relation, it'd be nice if we could shorten it somehow by either combining the relation and column level handling into a single piece of code, or maybe refactoring it into seperate functions which could be called from both pieces.. For both of these, I'm not sure it's really practical or would end up being better than what's there, but that's what I'd look at next with this patch and how it might be improved. Thanks! Stephen colprivs_2009011402.diff.gz Description: Binary data signature.asc Description: Digital signature
Re: [HACKERS] tuplestore potential performance problem
2009/1/15 Bruce Momjian br...@momjian.us: Has this been addressed? It is mentioned at http://archives.postgresql.org/pgsql-hackers/2008-12/msg01849.php * Look at tuplestore performance issues. The tuplestore_in_memory() thing is just a band-aid, we ought to try to solve it properly. tuplestore_advance seems like a weak spot as well. but not solved yet. It seems to me that to solve this the tuplestore's inside design should be changed much. In-file state doesn't use memory any more but it should be re-used for writing buffer, whereas the current desgin uses BufFile to do it, which causes tell/seek overhead for repeated put/get operation. And this is not for 8.4, I guess. 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] New patch for Column-level privileges
BTW, what is the official reviewer's opinion? It seems to me most of the issues on column-level privileges are resolved, so it is almost ready for getting merged. I tend to doubt Tom's had a chance to review it again yet, which is fine, though I'm certainly hopeful the recent focus and our combined efforts mean this patch can be included for 8.4. My personal opinion is that it's ready for beta testing (which is kind of what this feels like we're doing here) and barring any serious issues found during testing is good to go for inclusion. As for areas where there could still be some improvment, I'd love to hear your thoughts and opinions (and others!) on how column-level privileges are handled in ExecuteGrantStmt and into ExecGrant_Relation and how we might improve it. I don't like the nested for() loops in ExecuteGrantStmt, but I don't see any easy way to resolve that and keep the SQL-required syntax. What is your concern? In my personal opinion, it is quite natural to apply nested-loop to handle multiple columns within multiple tables. At least, I don't have a smart idea to handle two-dimensional data structure withou nested-loop. As for ExecGrant_Relation, it'd be nice if we could shorten it somehow by either combining the relation and column level handling into a single piece of code, or maybe refactoring it into seperate functions which could be called from both pieces.. It seems to me ExecGrant_Relation() is a bit larger than other ExecGrant_()s. My preference is to clip out column-privilege part into ExecGrant_Attribute() and invoke it for each given columns. But, it is just my preference. Please ask it official commiters/reviewers. 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] Visibility map, partial vacuums
Gregory Stark wrote: Bruce Momjian br...@momjian.us writes: Would someone tell me why 'autovacuum_freeze_max_age' defaults to 200M when our wraparound limit is around 2B? I suggested raising it dramatically in the post you quote and Heikki pointed it controls the maximum amount of space the clog will take. Raising it to, say, 800M will mean up to 200MB of space which might be kind of annoying for a small database. It would be nice if we could ensure the clog got trimmed frequently enough on small databases that we could raise the max_age. It's really annoying to see all these vacuums running 10x more often than necessary. Well, if it's a small database, you might as well just vacuum it. The rest of the thread is visible at the bottom of: http://article.gmane.org/gmane.comp.db.postgresql.devel.general/107525 Also, is anything being done about the concern about 'vacuum storm' explained below? I'm interested too. The additional vacuum_freeze_table_age (as I'm now calling it) setting I discussed in a later thread should alleviate that somewhat. When a table is autovacuumed, the whole table is scanned to freeze tuples if it's older than vacuum_freeze_table_age, and relfrozenxid is advanced. When different tables reach the autovacuum threshold at different times, they will also have their relfrozenxids set to different values. And in fact no anti-wraparound vacuum is needed. That doesn't help with read-only or insert-only tables, but that's not a new problem. -- 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
