[HACKERS] Why do we allow indexes to contain the same column more than once?
Is there some use case for that I'm not seeing? postgres=***# create index tti on tt(i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i); CREATE INDEX Time: 15.891 ms postgres=***# commit; COMMIT Time: 11.191 ms postgres=# \d tt Table public.tt ++-+---+ | Column | Type | Modifiers | ++-+---+ | i | integer | | ++-+---+ Indexes: tti btree (i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i, i) postgres=# \d tti Index public.tti ++-++ | Column | Type | Definition | ++-++ | i | integer | i | | i1 | integer | i | | i2 | integer | i | | i3 | integer | i | | i4 | integer | i | | i5 | integer | i | | i6 | integer | i | | i7 | integer | i | | i8 | integer | i | | i9 | integer | i | | i10| integer | i | | i11| integer | i | | i12| integer | i | | i13| integer | i | | i14| integer | i | | i15| integer | i | | i16| integer | i | | i17| integer | i | | i18| integer | i | | i19| integer | i | | i20| integer | i | | i21| integer | i | | i22| integer | i | | i23| integer | i | | i24| integer | i | | i25| integer | i | | i26| integer | i | | i27| integer | i | | i28| integer | i | | i29| integer | i | | i30| integer | i | | i31| integer | i | ++-++ btree, for table public.tt -- greg -- 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] Why do we allow indexes to contain the same column more than once?
Hi, On 2014-05-01 13:55:46 +0100, Greg Stark wrote: Is there some use case for that I'm not seeing? postgres=***# create index tti on tt(i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i,i); CREATE INDEX Time: 15.891 ms Can be useful if different opclasses are used for the individual columns. Other than that I am not seing much use. But what would we gain by prohibiting it except possibly breaking people's upgrades? Greetings, Andres Freund -- 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] improving \dt++ in psql
=?UTF-8?Q?Filip_Rembia=C5=82kowski?= filip.rembialkow...@gmail.com writes: I was (as a long time Pg user) dead sure that psql really sometimes cares about the number of plus signs that you add to meta-command No, it never has done so AFAIK. Can't we have higher levels of verbosiness? I think that for most commands, two levels of verbosity are already complicating the code in describe.c about as much as it can stand :-( Things like the infrastructure to mark particular columns as translatable would really need some significant work if we want to adopt multiple pluses as a standard design element. 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] quiet inline configure check misses a step for clang
Hi, On 2014-04-03 12:47:00 +0200, Andres Freund wrote: The current quiet inline test doesn't work for clang. As e.g. evidenced in http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=gulldt=2014-04-03%2007%3A49%3A26stg=configure configure thinks it's not quiet. Which means that postgres compiled with a recent clang will be noticably slower than it needs to be. The reason for that is that clang is smart and warns about static inline if they are declared locally in the .c file, but not if they are declared in a #included file. That seems to be a reasonable behaviour... I think that needs to be fixed. We either can make the configure test considerably more complex or simply drop the requirement for quiet inline. I still think we really need to fix this. I have three possible solutions: a) Add an external file (in the source tree) that's included in the configure test. b) Have a compiler specific override and specify USE_INLINE there. c) Drop the requirement of quiet inlines. a) would probably the best, but I haven't yet found a non ugly solution :( Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Successor of MD5 authentication, let's use SCRAM
On Fri, Sep 13, 2013 at 5:31 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Sep 12, 2013 at 11:33 AM, Magnus Hagander mag...@hagander.net wrote: Well, undocumented and OpenSSL tend to go hand in hand a lot. Or, well, it might be documented, but not in a useful way. I wouldn't count on it. The OpenSSL code is some of the worst-formatted spaghetti code I've ever seen, and the reason I know that is because whenever I try to do anything with OpenSSL I generally end up having to read it, precisely because, as you say, the documentation is extremely incomplete. I hate to be critical of other projects, but everything I've ever done with OpenSSL has been difficult, and I really think we should try to get less dependent on it rather than more. I have nothing exciting to add but I happened to be reading this old thread and thought the above post was relevant again these days. -- greg -- 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] Why do we allow indexes to contain the same column more than once?
Andres Freund and...@2ndquadrant.com writes: On 2014-05-01 13:55:46 +0100, Greg Stark wrote: Is there some use case for that I'm not seeing? Can be useful if different opclasses are used for the individual columns. Other than that I am not seing much use. Yeah. This is more plausible for the advanced index types like GIST, where different opclasses might possibly do significantly different things. We do prohibit it for unique/pkey constraints: regression=# create table tt(f1 int, unique(f1,f1)); ERROR: column f1 appears twice in unique constraint LINE 1: create table tt(f1 int, unique(f1,f1)); ^ which I think is per SQL spec, and anyway there would be no way that the constraint code would select different opclasses for the same column, so it's clearly redundant. 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] quiet inline configure check misses a step for clang
Andres Freund and...@2ndquadrant.com writes: I still think we really need to fix this. I have three possible solutions: a) Add an external file (in the source tree) that's included in the configure test. b) Have a compiler specific override and specify USE_INLINE there. c) Drop the requirement of quiet inlines. a) would probably the best, but I haven't yet found a non ugly solution :( Why is (a) so hard again? (If you're wondering where to put the file, I'd vote for the config/ directory.) 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] quiet inline configure check misses a step for clang
On 2014-05-01 10:10:46 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I still think we really need to fix this. I have three possible solutions: a) Add an external file (in the source tree) that's included in the configure test. b) Have a compiler specific override and specify USE_INLINE there. c) Drop the requirement of quiet inlines. a) would probably the best, but I haven't yet found a non ugly solution :( Why is (a) so hard again? (If you're wondering where to put the file, I'd vote for the config/ directory.) If we accept that the test includes a external file that's included in the source tree, it's not hard. What'd be hard is to generate a second file via autoconf. I looked at doing the latter and it seemed far to complex. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small doc patch for pg_replication_slots
Le 01/05/2014 04:56, Robert Haas a écrit : On Tue, Apr 29, 2014 at 5:39 AM, Thomas Reiss thomas.re...@dalibo.com wrote: You can find attached a small patch to fix the pg_replication_slots documentation. The slot_type and plugin are not in the appropriate order, slot_name and plugin have a wrong type and xmin appears two times. Without the patch, the description of catalog_xmin is: entryThe literalxmin/literal, or oldest transaction ID, that this slot forces to be retained in the system catalogs. /entry With the patch it's: entryThe oldest transaction that this slot needs the database to retain. literalVACUUM/literal cannot remove catalog tuples deleted by any later transaction. That's only one word different from the language for xmin, which doesn't seem quite right. Committed after fixing that. ...Robert Thanks -- Thomas Reiss Consultant Dalibo http://dalibo.com - http://dalibo.org -- 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] Clock sweep not caching enough B-Tree leaf pages?
Jim Nasby j...@nasby.net wrote: In our case this could maybe be handled by simply not incrementing counts when there's no eviction... but I'm more a fan of separate polls/clocks, because that means you can do things like a LFU for active and an LRU for inactive. I have hesitated to mention some benchmarks I did for optimal caching techniques for a database load, because they were so old, but maybe the ideas might spark something of value in the discussion. I'm talking about early 1985 on 80286 hardware on DOS with a Terminate and Stay Resident (TSR) cache external to the database. The external cache used LRU caching, and I was looking at what caching I could do inside the database to improve real database workloads which tended to include both OLTP and reporting. I found two types of caches improved performance. Neither was a substitute for the LRU cache closer to the hardware, and eliminating either reduced performance over having both. One was index-specific -- each connection caused to be held in cache the last page at each level of the index. This proved useful because in our real life applications it turned out that the next random access on an index was very often the same or near the previous. The other was a weighted average of access counts -- each access bumped a count and after a certain number of bumps all counts were reduced by 25%. This was accomplished by setting each count to the sum of it's existing value shifted right by one and shifted right by two. I understand that with the much larger RAM caches available 30 years later there could be some problems with passing all the counts atomically without causing a noticeable pause, and the higher connection counts may cause more contention issues. But if those issues could be solved (or somehow dodged for a proof of concept benchmark) it might be interesting to see how that worked out. FWIW, I recall that we used a one byte counter for each page, running 0 to 255. I don't recall the number at which we effectively multiplied by 0.75, and there was nothing particularly magic about that multiplier other than it was pretty fast and worked better that 0.5 in my benchmarks. I also don't remember what we used as the initial value on a page load. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Clock sweep not caching enough B-Tree leaf pages?
Kevin Grittner kgri...@ymail.com wrote: each connection caused to be held in cache the last page at each level of the index. Apologies for ambiguous terminology there. To be clear: the most recently accessed page at each level of the index. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Display of timestamp in pg_dump custom format
On Thu, May 1, 2014 at 12:33:51PM +1200, Gavin Flower wrote: On 01/05/14 12:04, Bruce Momjian wrote: On Thu, May 1, 2014 at 08:27:49AM +1200, Gavin Flower wrote: On 01/05/14 02:51, Bruce Momjian wrote: The table of contents for pg_restore -l shows the time the archive was made as local time (it uses ctime()): ; Archive created at Wed Apr 30 10:03:28 2014 Is this clear enough that it is local time? Should we display this better, perhaps with a time zone designation? I think it would be good to include the time zone, as we are all very international these days - and in Australia, adjacent states have different dates for the summer time transition! Personally, I would like to see the date in the format 2014-04-30, but having the day of the week is good. Milliseconds might be useful, if you want to check logs files. OK, I will work on it for 9.5. Thanks. So the it would then read something like: ; Archive created at Wed 2014-04-30 10:03:28.042 NZST (but with the correct appropriate time zone designation)? I think we would use a numeric offset. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Obsolete coding in fork_process.c
fork_process.c quoth: /* * Flush stdio channels just before fork, to avoid double-output problems. * Ideally we'd use fflush(NULL) here, but there are still a few non-ANSI * stdio libraries out there (like SunOS 4.1.x) that coredump if we do. * Presently stdout and stderr are the only stdio output channels used by * the postmaster, so fflush'ing them should be sufficient. */ fflush(stdout); fflush(stderr); Is there any reason not to change this to just fflush(NULL)? We dropped support for SunOS 4.1 quite some time ago ... While it's still true that the postmaster proper doesn't need to fflush anything but stdout and stderr, this coding seems a bit less than safe when you consider the possibility of third-party libraries loaded into the postmaster process. 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] Obsolete coding in fork_process.c
On Thu, May 01, 2014 at 12:13:28PM -0400, Tom Lane wrote: fork_process.c quoth: /* * Flush stdio channels just before fork, to avoid double-output problems. * Ideally we'd use fflush(NULL) here, but there are still a few non-ANSI * stdio libraries out there (like SunOS 4.1.x) that coredump if we do. * Presently stdout and stderr are the only stdio output channels used by * the postmaster, so fflush'ing them should be sufficient. */ fflush(stdout); fflush(stderr); Is there any reason not to change this to just fflush(NULL)? We dropped support for SunOS 4.1 quite some time ago ... Modern systems have other fflush(NULL) problems: http://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207692.html http://perl5.git.perl.org/metaconfig.git/blob/master:/U/perl/fflushall.U -- Noah Misch 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] quiet inline configure check misses a step for clang
On 2014-05-01 10:10:46 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
I still think we really need to fix this. I have three possible
solutions:
a) Add an external file (in the source tree) that's included in the
configure test.
b) Have a compiler specific override and specify USE_INLINE there.
c) Drop the requirement of quiet inlines.
a) would probably the best, but I haven't yet found a non ugly solution :(
Why is (a) so hard again?
(If you're wondering where to put the file, I'd vote for the config/
directory.)
Patch attached.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 67a8595f07a93d50276f1f4dc967b33a5137ae25 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Thu, 1 May 2014 20:40:31 +0200
Subject: [PATCH] Fix quiet inline configure test on newer clang compilers.
The test used to just define a static inline function inline and check
whether that causes a warning. But newer clang versions warn about
unused static inline functions when defined inside a .c file, but not
when defined in a included header. Change the test to cope.
This caused postgres compiled with newer clang versions to not use
inline functions, potentially leading to performance degradations.
---
config/c-compiler.m4| 14 --
config/test_quiet_include.h | 5 +
configure | 2 +-
3 files changed, 18 insertions(+), 3 deletions(-)
create mode 100644 config/test_quiet_include.h
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 4ba3236..1b9a2db 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -19,7 +19,17 @@ fi])# PGAC_C_SIGNED
# PGAC_C_INLINE
# -
-# Check if the C compiler understands inline functions.
+# Check if the C compiler understands inline functions without being
+# noisy about unused static inline functions. Some older compilers
+# understand inline functions (tested by AC_C_INLINE) but warn about
+# them if they aren't used in a translation unit.
+# This test used to just define a inline function, but some compilers
+# (notably clang) got too smart and now warn about unused static
+# inline functions when defined inside a .c file, but not when defined
+# in a included header. Since the latter is what we want to use, test
+# that by excluding a header defined outside autoconf's confines. Not
+# pretty, but it works.
+#
# Defines: inline, PG_USE_INLINE
AC_DEFUN([PGAC_C_INLINE],
[AC_C_INLINE
@@ -28,7 +38,7 @@ AC_CACHE_CHECK([for quiet inline (no complaint if unreferenced)], pgac_cv_c_inli
if test $ac_cv_c_inline != no; then
pgac_c_inline_save_werror=$ac_c_werror_flag
ac_c_werror_flag=yes
-AC_LINK_IFELSE([AC_LANG_PROGRAM([static inline int fun () {return 0;}],[])],
+AC_LINK_IFELSE([AC_LANG_PROGRAM([#include $srcdir/config/test_quiet_include.h],[])],
[pgac_cv_c_inline_quietly=yes])
ac_c_werror_flag=$pgac_c_inline_save_werror
fi])
diff --git a/config/test_quiet_include.h b/config/test_quiet_include.h
new file mode 100644
index 000..d2060c1
--- /dev/null
+++ b/config/test_quiet_include.h
@@ -0,0 +1,5 @@
+/*
+ * For the raison d'etre of this file, check the comment above the definition
+ * of the PGAC_C_INLINE macro in config/c-compiler.m4.
+ */
+static inline int fun () {return 0;}
diff --git a/configure b/configure
index e0dbdfe..9953389 100755
--- a/configure
+++ b/configure
@@ -9725,7 +9725,7 @@ else
ac_c_werror_flag=yes
cat confdefs.h - _ACEOF conftest.$ac_ext
/* end confdefs.h. */
-static inline int fun () {return 0;}
+#include $srcdir/config/test_quiet_include.h
int
main ()
{
--
1.8.5.rc2.dirty
--
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] Obsolete coding in fork_process.c
Noah Misch n...@leadboat.com writes: On Thu, May 01, 2014 at 12:13:28PM -0400, Tom Lane wrote: Is there any reason not to change this to just fflush(NULL)? We dropped support for SunOS 4.1 quite some time ago ... Modern systems have other fflush(NULL) problems: http://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207692.html http://perl5.git.perl.org/metaconfig.git/blob/master:/U/perl/fflushall.U Fun. I doubt that the postmaster's stdin would ever be a pipe, but maybe we'd better leave well enough alone. Should update the comment though. 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] Store data in pg_toast for custom type fails (bug?)
I wrote: I believe I understand what's going on here, and it's not quite as exciting as it first appears. The issue is that we are failing to honor the toasting goes only one level deep rule in the specific case of arrays of composite type. So while it's definitely a nasty bug, it affects only narrow use cases, and doesn't call into question our whole vacuuming strategy or anything like that. I've committed a fix for this. Thanks for the report! 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] quiet inline configure check misses a step for clang
Andres Freund and...@2ndquadrant.com writes: Patch attached. Committed with minor comment-smithing. 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] quiet inline configure check misses a step for clang
On 2014-05-01 16:17:17 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Patch attached. Committed with minor comment-smithing. Thanks. There's another problematic case: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=olinguitodt=2014-05-01%2021%3A22%3A43stg=config configure:9737: ccache gcc -o conftest -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -g -I/usr/local/include/libxml2 -I/usr/local/include -I/usr/include/kerberosV -L/usr/local/lib -lpthread -lcrypto -liconv -L/usr/local/lib -L/usr/local/lib conftest.c -lxslt -lxml2 -lssl -lcrypto -lz -lreadline -ltermcap -lm 5 /usr/local/lib/libxml2.so.14.0: warning: strcpy() is almost always misused, please use strlcpy() /usr/local/lib/libxml2.so.14.0: warning: strcat() is almost always misused, please use strlcat() /usr/local/lib/libxslt.so.3.8: warning: sprintf() is often misused, please use snprintf() configure:9737: $? = 0 configure: failed program was: That's not new. And it seems like a case of really misguided platform specific changes (actually smelling a bit of zealotry). So I'd be prepared to ignore it. Other opinions? How on earth could it be a good idea to warn about this when linking to libraries instead of warning when a function is actually used during compilation? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Obsolete coding in fork_process.c
I wrote:
Noah Misch n...@leadboat.com writes:
On Thu, May 01, 2014 at 12:13:28PM -0400, Tom Lane wrote:
Is there any reason not to change this to just fflush(NULL)? We dropped
support for SunOS 4.1 quite some time ago ...
Modern systems have other fflush(NULL) problems:
http://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207692.html
http://perl5.git.perl.org/metaconfig.git/blob/master:/U/perl/fflushall.U
Fun. I doubt that the postmaster's stdin would ever be a pipe, but
maybe we'd better leave well enough alone. Should update the comment
though.
However ... after looking around I notice that fflush(NULL) is already
being used in parallel pg_dump and pg_upgrade; and at least in the latter
case I'm afraid to change that because it looks like there are probably
other stdio output files open in the process.
I think we should probably go ahead and switch over to using
fflush(NULL). The capability is required by C89 for crissake;
are we really going to rely on hearsay evidence that there are
current platforms that are still broken? Also, I found at least
one claim on the net that this has been fixed in Solaris for awhile:
http://grokbase.com/t/perl/perl5-porters/021hn4z9pq/fflush-null-on-solaris-9
Attached is a draft patch that I was working on before I decided that
making the indicated changes in pg_upgrade was probably a bad idea.
This is mainly to memorialize the places we should look at if we
change it.
BTW, while working on this I noticed that there are a boatload of places
where we use system() or popen() without a prior fflush. I suspect most
of them are safe, or we'd have heard more complaints --- but shouldn't
we clamp down on that?
regards, tom lane
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
index fa0a005..3bf9c6a 100644
*** a/contrib/pg_upgrade/controldata.c
--- b/contrib/pg_upgrade/controldata.c
*** get_control_data(ClusterInfo *cluster, b
*** 114,119
--- 113,120
cluster-bindir,
live_check ? pg_controldata\ : pg_resetxlog\ -n,
cluster-pgdata);
+
+ /* Don't use fflush(NULL); see comments in fork_process.c */
fflush(stdout);
fflush(stderr);
diff --git a/contrib/pg_upgrade/parallel.c b/contrib/pg_upgrade/parallel.c
index f4201e1..7807687 100644
*** a/contrib/pg_upgrade/parallel.c
--- b/contrib/pg_upgrade/parallel.c
*** parallel_exec_prog(const char *log_file,
*** 118,125
/* set this before we start the job */
parallel_jobs++;
! /* Ensure stdio state is quiesced before forking */
! fflush(NULL);
#ifndef WIN32
child = fork();
--- 118,126
/* set this before we start the job */
parallel_jobs++;
! /* Don't use fflush(NULL); see comments in fork_process.c */
! fflush(stdout);
! fflush(stderr);
#ifndef WIN32
child = fork();
*** parallel_transfer_all_new_dbs(DbInfoArr
*** 227,234
/* set this before we start the job */
parallel_jobs++;
! /* Ensure stdio state is quiesced before forking */
! fflush(NULL);
#ifndef WIN32
child = fork();
--- 228,236
/* set this before we start the job */
parallel_jobs++;
! /* Don't use fflush(NULL); see comments in fork_process.c */
! fflush(stdout);
! fflush(stderr);
#ifndef WIN32
child = fork();
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 7c1e59e..cf84fcf 100644
*** a/contrib/pgbench/pgbench.c
--- b/contrib/pgbench/pgbench.c
*** pthread_create(pthread_t *thread,
*** 3328,
--- 3328,3337
return errno;
}
+ /* Don't use fflush(NULL); see comments in fork_process.c */
+ fflush(stdout);
+ fflush(stderr);
+
th-pid = fork();
if (th-pid == -1) /* error */
{
diff --git a/src/backend/postmaster/fork_process.c b/src/backend/postmaster/fork_process.c
index 3e2acdd..28f4ca7 100644
*** a/src/backend/postmaster/fork_process.c
--- b/src/backend/postmaster/fork_process.c
*** fork_process(void)
*** 38,45
/*
* Flush stdio channels just before fork, to avoid double-output problems.
! * Ideally we'd use fflush(NULL) here, but there are still a few non-ANSI
! * stdio libraries out there (like SunOS 4.1.x) that coredump if we do.
* Presently stdout and stderr are the only stdio output channels used by
* the postmaster, so fflush'ing them should be sufficient.
*/
--- 38,50
/*
* Flush stdio channels just before fork, to avoid double-output problems.
! *
! * Ideally we'd use fflush(NULL) here, but that has portability issues ---
! * notably, it's reported that current Solaris as of 2013 still has some
! * odd behaviors in fflush(NULL), such as closing stdin if it's a pipe.
! * While it's not clear that that would affect the postmaster, it still
! * seems best to stay away from fflush(NULL).
! *
* Presently stdout and stderr are the only stdio output channels used by
Re: [HACKERS] quiet inline configure check misses a step for clang
Andres Freund and...@2ndquadrant.com writes: There's another problematic case: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=olinguitodt=2014-05-01%2021%3A22%3A43stg=config /usr/local/lib/libxml2.so.14.0: warning: strcpy() is almost always misused, please use strlcpy() /usr/local/lib/libxml2.so.14.0: warning: strcat() is almost always misused, please use strlcat() /usr/local/lib/libxslt.so.3.8: warning: sprintf() is often misused, please use snprintf() That's not new. And it seems like a case of really misguided platform specific changes (actually smelling a bit of zealotry). So I'd be prepared to ignore it. Other opinions? Yeah, I've noticed those before, and concluded that misguided zealotry is the appropriate classification. I certainly won't do anything about those. (These particular ones, we *can't* do anything about, since the calls are inside libraries we don't control the source of...) How on earth could it be a good idea to warn about this when linking to libraries instead of warning when a function is actually used during compilation? Not only misguided, but crappily implemented zealotry. 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] Obsolete coding in fork_process.c
On Thu, May 1, 2014 at 5:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Noah Misch n...@leadboat.com writes: On Thu, May 01, 2014 at 12:13:28PM -0400, Tom Lane wrote: Is there any reason not to change this to just fflush(NULL)? We dropped support for SunOS 4.1 quite some time ago ... Modern systems have other fflush(NULL) problems: http://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207692.html http://perl5.git.perl.org/metaconfig.git/blob/master:/U/perl/fflushall.U Fun. I doubt that the postmaster's stdin would ever be a pipe, but maybe we'd better leave well enough alone. Should update the comment though. However ... after looking around I notice that fflush(NULL) is already being used in parallel pg_dump and pg_upgrade; and at least in the latter case I'm afraid to change that because it looks like there are probably other stdio output files open in the process. I think we should probably go ahead and switch over to using fflush(NULL). The capability is required by C89 for crissake; are we really going to rely on hearsay evidence that there are current platforms that are still broken? Also, I found at least one claim on the net that this has been fixed in Solaris for awhile: http://grokbase.com/t/perl/perl5-porters/021hn4z9pq/fflush-null-on-solaris-9 I am having a hard time understanding what the real impetus to change this is. IIUC, we have zero reports of the current coding being a problem, so I'm not sure why we want to go make it different. Sure, there are hypothetical problems with the current code, but there are also hypothetical problems with the proposed change, and the current code has survived quite a bit of real-world deployment. I guess it's hard for me to be dead-set against this if we're using that pattern elsewhere, but I won't be very surprised if more weird cases where it doesn't work turn up down the road; and I'm a bit worried that if we let it proliferate we'll find it hard to get rid of if and when those reports turn up. Attached is a draft patch that I was working on before I decided that making the indicated changes in pg_upgrade was probably a bad idea. This is mainly to memorialize the places we should look at if we change it. BTW, while working on this I noticed that there are a boatload of places where we use system() or popen() without a prior fflush. I suspect most of them are safe, or we'd have heard more complaints --- but shouldn't we clamp down on that? I think that would probably be a good idea. I wouldn't be shocked if there are problems there that we've failed to notice. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Obsolete coding in fork_process.c
On Thu, May 01, 2014 at 05:59:08PM -0400, Tom Lane wrote: I wrote: Noah Misch n...@leadboat.com writes: Modern systems have other fflush(NULL) problems: http://www.nntp.perl.org/group/perl.perl5.porters/2013/09/msg207692.html http://perl5.git.perl.org/metaconfig.git/blob/master:/U/perl/fflushall.U Fun. I doubt that the postmaster's stdin would ever be a pipe, but maybe we'd better leave well enough alone. Should update the comment though. However ... after looking around I notice that fflush(NULL) is already being used in parallel pg_dump and pg_upgrade; and at least in the latter case I'm afraid to change that because it looks like there are probably other stdio output files open in the process. Do those programs, operating in those modes, read from stdin or some other long-lived, pipe-backed FILE*? BTW, while working on this I noticed that there are a boatload of places where we use system() or popen() without a prior fflush. I suspect most of them are safe, or we'd have heard more complaints --- but shouldn't we clamp down on that? You need the fflush() when forking and then using stdio in the child before any exec(). Have you caught wind of any system() or popen() implementation having that property? -- Noah Misch 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] Obsolete coding in fork_process.c
Noah Misch n...@leadboat.com writes: On Thu, May 01, 2014 at 05:59:08PM -0400, Tom Lane wrote: However ... after looking around I notice that fflush(NULL) is already being used in parallel pg_dump and pg_upgrade; and at least in the latter case I'm afraid to change that because it looks like there are probably other stdio output files open in the process. Do those programs, operating in those modes, read from stdin or some other long-lived, pipe-backed FILE*? Probably not. However, if that's your criterion, I'd say that I'd much rather assume that the postmaster doesn't have a pipe connected to stdin than that it has no stdio outputs besides stdout/stderr. BTW, while working on this I noticed that there are a boatload of places where we use system() or popen() without a prior fflush. I suspect most of them are safe, or we'd have heard more complaints --- but shouldn't we clamp down on that? You need the fflush() when forking and then using stdio in the child before any exec(). Have you caught wind of any system() or popen() implementation having that property? You're only considering one aspect of the problem. Yeah, you might not get duplicated output unless system() prints something before exec(), but we would also like to have causality: that is, whatever we sent to stdout before calling system() should appear there before anything the child process sends to stdout. 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] Obsolete coding in fork_process.c
On Thu, May 01, 2014 at 08:44:46PM -0400, Tom Lane wrote: Noah Misch n...@leadboat.com writes: On Thu, May 01, 2014 at 05:59:08PM -0400, Tom Lane wrote: However ... after looking around I notice that fflush(NULL) is already being used in parallel pg_dump and pg_upgrade; and at least in the latter case I'm afraid to change that because it looks like there are probably other stdio output files open in the process. Do those programs, operating in those modes, read from stdin or some other long-lived, pipe-backed FILE*? Probably not. Then the use of fflush(NULL) in those programs tells us nothing about the status of the aforementioned HP-UX bug. However, if that's your criterion, I'd say that I'd much rather assume that the postmaster doesn't have a pipe connected to stdin than that it has no stdio outputs besides stdout/stderr. Either way, the beneficiary is theoretical; who can tell which one deserves priority? Let's do what we usually do in the absence of a clear improvement: keep the longstanding behavior. BTW, while working on this I noticed that there are a boatload of places where we use system() or popen() without a prior fflush. I suspect most of them are safe, or we'd have heard more complaints --- but shouldn't we clamp down on that? You need the fflush() when forking and then using stdio in the child before any exec(). Have you caught wind of any system() or popen() implementation having that property? You're only considering one aspect of the problem. Yeah, you might not get duplicated output unless system() prints something before exec(), but we would also like to have causality: that is, whatever we sent to stdout before calling system() should appear there before anything the child process sends to stdout. Good point. I suppose a couple of fflush() calls have negligible cost next to a system() or popen(). Introduce pg_popen()/pg_system(), and adopt a rule that they are [almost] our only callers of raw popen()/system()? -- Noah Misch 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] includedir_internal headers are not self-contained
Tom Lane wrote: I wrote: How about we change common/relpath.[hc] to export a single version of relpath() that takes its arguments as separate plain OIDs, and then make the other versions wrappers that are only declared in some backend header? The wrappers could possibly be macros, allowing things like pg_xlogdump to keep using them as long as they didn't mind importing backend headers. (Though for the RelFileNode case this would imply multiple evaluation of the macro argument, so maybe it's not such a great idea.) Since nobody objected, I've committed something along this line. include/common/ is now free of references to backend headers. Many thanks for the extra effort. The patch is certainly too invasive to consider back-patching into 9.3, though. I feel unsure about this. I agree the patch is quite invasive. Leaving 9.3 in a broken state seems problematic. In particular I'm not sure what would Debian do about the whole issue; would they have to carry the patch for their 9.3 packages? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] includedir_internal headers are not self-contained
Alvaro Herrera alvhe...@2ndquadrant.com writes: Tom Lane wrote: The patch is certainly too invasive to consider back-patching into 9.3, though. I feel unsure about this. I agree the patch is quite invasive. Leaving 9.3 in a broken state seems problematic. In particular I'm not sure what would Debian do about the whole issue; would they have to carry the patch for their 9.3 packages? My recommendation to Christoph upthread was that they just look the other way for the time being, ie, ignore the fact that relpath.h is unusable by freestanding apps in 9.3. Backpatching what I did for 9.4 would be an ABI break, so that seems to me to be out of the question in 9.3. And it's not clear that anybody outside core+contrib really needs relpath.h yet, anyway. (Of course, you could argue that if there are no external users then the ABI break isn't a problem; but if there are, then it is.) 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
[HACKERS] need xmllint on borka
I have been working on making the DocBook XML output valid. The first part was bb4eefe7bf518e42c73797ea37b033a5d8a8e70a, I now have the rest ready, but I'll spare you the mostly mechanical 200kB patch for now. In addition, I'd like to add the attached patch with an xmllint call to make sure things stay valid. But we don't have xmllint installed on borka, where we build the releases. Could someone please install it? --- a/doc/src/sgml/Makefile +++ b/doc/src/sgml/Makefile @@ -76,6 +76,7 @@ override SPFLAGS += -wall -wno-unused-param -wno-empty -wfully-tagged man distprep-man: man-stamp man-stamp: stylesheet-man.xsl postgres.xml + $(XMLLINT) --noout --valid postgres.xml $(XSLTPROC) $(XSLTPROCFLAGS) $(XSLTPROC_MAN_FLAGS) $^ touch $@ @@ -252,11 +253,13 @@ endif xslthtml: xslthtml-stamp xslthtml-stamp: stylesheet.xsl postgres.xml + $(XMLLINT) --noout --valid postgres.xml $(XSLTPROC) $(XSLTPROCFLAGS) $(XSLTPROC_HTML_FLAGS) $^ cp $(srcdir)/stylesheet.css html/ touch $@ htmlhelp: stylesheet-hh.xsl postgres.xml + $(XMLLINT) --noout --valid postgres.xml $(XSLTPROC) $(XSLTPROCFLAGS) $^ %-A4.fo.tmp: stylesheet-fo.xsl %.xml @@ -279,6 +282,7 @@ XMLLINT = xmllint epub: postgres.epub postgres.epub: postgres.xml + $(XMLLINT) --noout --valid $ $(DBTOEPUB) $ -- 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] need xmllint on borka
Peter Eisentraut pete...@gmx.net writes: I have been working on making the DocBook XML output valid. The first part was bb4eefe7bf518e42c73797ea37b033a5d8a8e70a, I now have the rest ready, but I'll spare you the mostly mechanical 200kB patch for now. In addition, I'd like to add the attached patch with an xmllint call to make sure things stay valid. But we don't have xmllint installed on borka, where we build the releases. Could someone please install it? -1. Doesn't this break make man for *any* hacker who doesn't have xmllint installed? Please change the patch so that it runs xmllint if available, rather than turning it into a de facto requirement for anybody who works on documentation. Or if you think you can pass a vote to require it, I'd suggest that the patch had better include documentation additions listing this as a required build tool. (The subtext here is that borka is absolutely not an acceptable place to encounter documentation build failures. By the time we're at that stage of the release cycle, I don't really care what xmllint might have to say; there isn't going to be time to make it happy.) 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] Obsolete coding in fork_process.c
Noah Misch n...@leadboat.com writes: On Thu, May 01, 2014 at 08:44:46PM -0400, Tom Lane wrote: You're only considering one aspect of the problem. Yeah, you might not get duplicated output unless system() prints something before exec(), but we would also like to have causality: that is, whatever we sent to stdout before calling system() should appear there before anything the child process sends to stdout. Good point. I suppose a couple of fflush() calls have negligible cost next to a system() or popen(). Introduce pg_popen()/pg_system(), and adopt a rule that they are [almost] our only callers of raw popen()/system()? Meh. I'm not usually in favor of adopting nonstandard notation, and this doesn't seem like a place to start. In particular, if you don't want to use fflush(NULL) in these proposed wrappers, then call sites are still going to have an issue with needing to do manual fflushes; pg_regress.c's spawn_process is an example: /* * Must flush I/O buffers before fork. Ideally we'd use fflush(NULL) here * ... does anyone still care about systems where that doesn't work? */ fflush(stdout); fflush(stderr); if (logfile) fflush(logfile); pid = fork(); I think that removing the need for fflush(stdout) and fflush(stderr) in this context would mostly result in people forgetting to fflush other output files. I'd rather have the two lines of boilerplate (and a comment about why we're refusing to depend on fflush(NULL)) than take that risk. Now, if you were willing to put fflush(NULL) into the wrappers, I could get on board with that ;-) 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
[HACKERS] Allowing empty target list in SELECT (1b4f7f93b4693858cb983af3cd557f6097dab67b)
Hi, I s the following behavior perceived fix-worthy? -- note the ' 1's in the output s po stgres=# CREATE TABLE test AS SELECT; SELECT 1 postgres=# insert into test select; INSERT 0 1 My guess why this happens is because changes made in the commit in $SUBJECT only pertain to fixing syntax errors and nothing else. -- Amit
Re: [HACKERS] Allowing empty target list in SELECT (1b4f7f93b4693858cb983af3cd557f6097dab67b)
On Fri, May 2, 2014 at 12:57 PM, Amit Langote amitlangot...@gmail.com wrote: I s the following behavior perceived fix-worthy? -- note the ' 1's in the output s po stgres=# CREATE TABLE test AS SELECT; SELECT 1 postgres=# insert into test select; INSERT 0 1 Or maybe, it just means 1 'null' row/record and not no row at all? -- Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] is there a hook by which we can modify input query before postgresql builds raw parse tree
Hi, I want to know is there a way to use a hook to modify the input query before Postgresql parses and builds the parse tree for the query.
Re: [HACKERS] is there a hook by which we can modify input query before postgresql builds raw parse tree
On Fri, May 2, 2014 at 10:07 AM, Rajmohan C csrajmo...@gmail.com wrote: Hi, I want to know is there a way to use a hook to modify the input query before Postgresql parses and builds the parse tree for the query. Uh...the rewriter? -- Regards, Atri *l'apprenant*
Re: [HACKERS] is there a hook by which we can modify input query before postgresql builds raw parse tree
On Fri, May 2, 2014 at 1:37 PM, Rajmohan C csrajmo...@gmail.com wrote: Hi, I want to know is there a way to use a hook to modify the input query before Postgresql parses and builds the parse tree for the query. Not that I know of. I am not sure that enforcing query rewrite on the server side is a good idea at this may trap your applications and have them face unexpected behaviors. FWIW, a presentation about hooks has been done at PGCon 2012, you can refer to it as well for your future work: http://wiki.postgresql.org/images/e/e3/Hooks_in_postgresql.pdf -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] is there a hook by which we can modify input query before postgresql builds raw parse tree
On Fri, May 2, 2014 at 2:06 PM, Michael Paquier michael.paqu...@gmail.com wrote: Not that I know of. I am not sure that enforcing query rewrite on the server side is a good idea at this may trap your applications and have them face unexpected behaviors. Note as well that there is a hook *before* the rewriter and *after* parse/analyze of query called post_parse_analyze_hook in parser/analyze.h. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
