[HACKERS] windows consolidated cleanup

[Andrew Dunstan]

The attached patch is intended to clean up a bunch of compiler warnings 
seen on Windows due to mismatches of signedness or constness, unused 
variables, redefined macros and a missing prototype.


It doesn't clean up all the warnings by any means, but it fixes quite a few.

One thing I'm a bit confused about is this type of warning:

  src\backend\utils\misc\guc-file.c(977): warning C4003: not enough actual 
parameters for macro 'GUC_yywrap'


If someone can suggest a good fix That would be nice.

cheers

andrew

*** a/src/backend/main/main.c
--- b/src/backend/main/main.c
***
*** 393,399  get_current_username(const char *progname)
  	return strdup(pw->pw_name);
  #else
  	long		namesize = 256 /* UNLEN */ + 1;
! 	char	   *name;
  
  	name = malloc(namesize);
  	if (!GetUserName(name, &namesize))
--- 393,399 
  	return strdup(pw->pw_name);
  #else
  	long		namesize = 256 /* UNLEN */ + 1;
! 	unsigned char   *name;
  
  	name = malloc(namesize);
  	if (!GetUserName(name, &namesize))
*** a/src/backend/port/win32/socket.c
--- b/src/backend/port/win32/socket.c
***
*** 370,376  pgwin32_recv(SOCKET s, char *buf, int len, int f)
  }
  
  int
! pgwin32_send(SOCKET s, char *buf, int len, int flags)
  {
  	WSABUF		wbuf;
  	int			r;
--- 370,376 
  }
  
  int
! pgwin32_send(SOCKET s, const char *buf, int len, int flags)
  {
  	WSABUF		wbuf;
  	int			r;
***
*** 380,386  pgwin32_send(SOCKET s, char *buf, int len, int flags)
  		return -1;
  
  	wbuf.len = len;
! 	wbuf.buf = buf;
  
  	/*
  	 * Readiness of socket to send data to UDP socket may be not true: socket
--- 380,386 
  		return -1;
  
  	wbuf.len = len;
! 	wbuf.buf = (char *) buf;
  
  	/*
  	 * Readiness of socket to send data to UDP socket may be not true: socket
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 63,69 
--- 63,78 
  #include "utils/syscache.h"
  
  #ifdef WIN32
+ /*
+  * This Windows file defines StrNCpy. We don't need it here, so we undefine
+  * it to keep the compiler quiet, and undefine it again after the file is
+  * included, so we don't accidentally use theirs.
+  */
+ #undef StrNCpy
  #include 
+ #ifdef StrNCpy
+ #undef STrNCpy
+ #endif
  #endif
  
  #define		MAX_L10N_DATA		80
***
*** 555,561  strftime_win32(char *dst, size_t dstlen, const wchar_t *format, const struct tm
  	dst[len] = '\0';
  	if (encoding != PG_UTF8)
  	{
! 		char	   *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
  
  		if (dst != convstr)
  		{
--- 564,571 
  	dst[len] = '\0';
  	if (encoding != PG_UTF8)
  	{
! 		char	   *convstr = pg_do_encoding_conversion((unsigned char *) dst, 
! 		len, PG_UTF8, encoding);
  
  		if (dst != convstr)
  		{
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
***
*** 1561,1567  normalize_locale_name(char *new, const char *old)
  static void
  setup_collation(void)
  {
! #ifdef HAVE_LOCALE_T
  	int			i;
  	FILE	   *locale_a_handle;
  	char		localebuf[NAMEDATALEN];
--- 1561,1567 
  static void
  setup_collation(void)
  {
! #if defined(HAVE_LOCALE_T) && !defined(WIN32)
  	int			i;
  	FILE	   *locale_a_handle;
  	char		localebuf[NAMEDATALEN];
***
*** 1687,1696  setup_collation(void)
  		printf(_("No usable system locales were found.\n"));
  		printf(_("Use the option \"--debug\" to see details.\n"));
  	}
! #else			/* not HAVE_LOCALE_T */
  	printf(_("not supported on this platform\n"));
  	fflush(stdout);
! #endif   /* not HAVE_LOCALE_T */
  }
  
  /*
--- 1687,1696 
  		printf(_("No usable system locales were found.\n"));
  		printf(_("Use the option \"--debug\" to see details.\n"));
  	}
! #else			/* not HAVE_LOCALE_T && not WIN32 */
  	printf(_("not supported on this platform\n"));
  	fflush(stdout);
! #endif   /* not HAVE_LOCALE_T  && not WIN32*/
  }
  
  /*
***
*** 2387,2393  setlocales(void)
--- 2387,2396 
  #ifdef WIN32
  typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
  
+ /* Windows API define missing from some versions of MingW headers */
+ #ifndef  DISABLE_MAX_PRIVILEGE
  #define DISABLE_MAX_PRIVILEGE	0x1
+ #endif
  
  /*
   * Create a restricted token and execute the specified process with it.
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
***
*** 1461,1468  typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, L
  typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
  typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);
  
! /* Windows API define missing from MingW headers */
  #define DISABLE_MAX_PRIVILEGE	0x1
  
  /*
   * Create a restricted token, a job object sandbox, and execute the specified
--- 1461,1470 
  typedef BOOL (WINAPI * __Assign

Re: [HACKERS] Extension Packaging

[David E. Wheeler]

On Apr 23, 2011, at 1:03 PM, Dimitri Fontaine wrote:

> Daniele Varrazzo  writes:
>> For my extension I'm less concerned by having the install sql named in
>> different ways or by the upgrade sql as all these files are generated
>> by scripts. You may find useful this one
> 
> You can also generate that reliably in SQL.  You install your extension
> with CREATE EXTENSION then run the query over pg_depend and you have it
> all.  Then you can test this upgrade script you just got in SQL.  Tom
> also has a version that does the necessary string replacements using sed
> from a bash script rather than the SQL replace() function.
> 
>  http://archives.postgresql.org/pgsql-hackers/2011-02/msg01208.php
>  http://archives.postgresql.org/pgsql-hackers/2011-02/msg01438.php

Nice. Did you and Tom ever work out the difference in results?

  http://archives.postgresql.org/pgsql-hackers/2011-02/msg01572.php

I'd like to see this documented somewhere, perhaps in 

  http://developer.postgresql.org/pgdocs/postgres/extend-extensions.html

Thanks,

David



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

Re: [HACKERS] time-delayed standbys

[Jaime Casanova]

On Tue, Apr 19, 2011 at 9:47 PM, Robert Haas  wrote:
>
> That is, a standby configured such that replay lags a prescribed
> amount of time behind the master.
>
> This seemed easy to implement, so I did.  Patch (for 9.2, obviously) attached.
>

This crashes when stoping recovery to a target (i tried with a named
restore point and with a poin in time) after executing
pg_xlog_replay_resume(). here is the backtrace. I will try to check
later but i wanted to report it before...

#0  0xb537 in raise () from /lib/libc.so.6
#1  0xb777a922 in abort () from /lib/libc.so.6
#2  0x08393a19 in errfinish (dummy=0) at elog.c:513
#3  0x083944ba in elog_finish (elevel=22, fmt=0x83d5221 "wal receiver
still active") at elog.c:1156
#4  0x080f04cb in StartupXLOG () at xlog.c:6691
#5  0x080f2825 in StartupProcessMain () at xlog.c:10050
#6  0x0811468f in AuxiliaryProcessMain (argc=2, argv=0xbfa326a8) at
bootstrap.c:417
#7  0x0827c2ea in StartChildProcess (type=StartupProcess) at postmaster.c:4488
#8  0x08280b85 in PostmasterMain (argc=3, argv=0xa4c17e8) at postmaster.c:1106
#9  0x0821730f in main (argc=3, argv=0xa4c17e8) at main.c:199


-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL

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

[HACKERS] pg_upgrade copy_file not needed on Win32

[Bruce Momjian]

I have applied the attached patch which prevents copy_file() from being
compiled on Win32, per report from Andrew Dunstan.  copy_file() is not
used on Win32 and generates a warning.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/file.c b/contrib/pg_upgrade/file.c
new file mode 100644
index f8f7233..0552541
*** a/contrib/pg_upgrade/file.c
--- b/contrib/pg_upgrade/file.c
***
*** 12,20 
  #include 
  
  
- static int	copy_file(const char *fromfile, const char *tofile, bool force);
  
! #ifdef WIN32
  static int	win32_pghardlink(const char *src, const char *dst);
  #endif
  
--- 12,21 
  #include 
  
  
  
! #ifndef WIN32
! static int	copy_file(const char *fromfile, const char *tofile, bool force);
! #else
  static int	win32_pghardlink(const char *src, const char *dst);
  #endif
  
*** linkAndUpdateFile(pageCnvCtx *pageConver
*** 126,131 
--- 127,133 
  }
  
  
+ #ifndef WIN32
  static int
  copy_file(const char *srcfile, const char *dstfile, bool force)
  {
*** copy_file(const char *srcfile, const cha
*** 220,225 
--- 222,228 
  
  	return 1;
  }
+ #endif
  
  
  /*

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

[HACKERS] pgwin32_send

[Andrew Dunstan]

The declaration of pgwin32_send in src/backend/port/win32/socket.c has:

 int pgwin32_send(SOCKET s, char *buf, int len, int flags)


but the on my Linux machine, the prototype for send() is:

 ssize_t send(int sockfd, const void *buf, size_t len, int flags);


Is there any reason not to make the second argument a "const char *" and 
thus avoid a whine from gcc on Windows?


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] Extension Packaging

[Daniele Varrazzo]

On Thu, Apr 21, 2011 at 4:16 AM, Tom Lane  wrote:
> "David E. Wheeler"  writes:

>> * Another, minor point: If I release a new version with no changes to the 
>> code (as I've done today, just changing the make stuff and documentation), 
>> it's kind of annoying that I'd need to have a migration script from the old 
>> version to the new version that's…empty. But I dunno, maybe not such a big 
>> deal. It's useful to have it there with a comment in it: “No changes.”
>
> If you did not actually change the contents of the install script, you
> should not change its version number either.

Sorry, I'm not entirely convinced. If I release an extension 1.0, then
find a bug in the C code and fix it in 1.0.1, arguably "make install"
will put the .so in the right place and the 1.0.1 code will be picked
up by new sessions. But pg_extension still knows 1.0 as extension
version, and ALTER EXTENSION ... UPGRADE fails because no update path
is knows.

There is also a dangerous asymmetry: If I'm not mistaken the library
.so has no version number, so there can be only one version in the
system: an update changing code and sql requires ALTER EXTENSION to be
run as soon as possible, or some sql function from the old extension
may try to call non-existing functions in the library - or worse the
wrong ones or with wrong parameters. OTOH library-only changes don't
strictly require ALTER EXTENSION - and trying to issue the command
would fail if no path to the default version is available (leaving the
admin puzzled about whether he installed the upgrade properly).

Is an empty upgrade file the only way to get the extension metadata
right? I wouldn't find it particularly bad, but better have it that
have library-metadata mismatches or inconsistencies in the upgrade
procedures I think.

-- Daniele

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

[HACKERS] code cleanups

[Andrew Dunstan]

getopt.c is looking a bit ancient. In particular, getopt() has no 
prototype and old style arguments. Is there any reason not to bring this 
into the 1990s (or is that 1980s?)


gcc also doesn't like the way perl does unused attributes on Windows, 
and it generates quite a few warnings about it. A little experimentation 
suggests this would probably clean it up if placed at the start of plperl.h:


   #if defined(__GNUC_)
   #define PERL_UNUSED_DECL __attribute__ ((unused))
   #endif

Thoughts?

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] Patch for pg_upgrade to turn off autovacuum

[Bruce Momjian]

Bruce Momjian wrote:
> Bruce Momjian wrote:
> > Tom Lane wrote:
> > > Bruce Momjian  writes:
> > > > I thought some more about this and I don't want autovacuum to run on the
> > > > old server.  This is because pg_dumpall --binary-upgrade --schema-only
> > > > grabs the datfrozenxid for all the databases at the start, then connects
> > > > to each database to gets the relfrozenxids.  I don't want to risk any
> > > > advancement of either of those during the pg_dumpall run.
> > > 
> > > Why?  It doesn't really matter --- if you grab a value that is older
> > > than the latest, it's still valid.  As Robert said, you're
> > > over-engineering this, and thereby introducing potential failure modes,
> > > for no gain.
> > 
> > Uh, I am kind of paranoid about pg_upgrade because it is trying to do
> > something Postgres was never designed to do.  I am a little worried that
> > we would be assuming that pg_dumpall always does the datfrozenxid first
> > and if we ever did it last we would have relfrozenxids before the
> > datfrozenxid.  I am worried if we don't prevent autovacuum on the old
> > server that pg_upgrade will be more fragile to changes in other parts of
> > the system.
> 
> Hold, I overstated the fragility issue above.  I now realize that the
> old system is not going to change and that I only need to worry about
> future changes, where are handled by the new -b flag, so maybe we can
> get away with only stopping autovacuum on the new server, but I would
> need someone to verify that, and this would be a change in the way 9.0
> pg_upgrade operated because it did disable autovacuum on the old and new
> servers with 99.9% reliability.

Well, having seen no replies, I am going to apply the version of the
patch in a few days that keeps the old vacuum-disable behavior for older
releases, and uses the -b flag for newer ones by testing the catalog
version, e.g.:

snprintf(cmd, sizeof(cmd),
 SYSTEMQUOTE "\"%s/pg_ctl\" -l \"%s\" -D \"%s\" "
 "-o \"-p %d %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE,
 bindir, output_filename, datadir, port,
 (cluster->controldata.cat_ver >=
BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
"-c autovacuum=off -c autovacuum_freeze_max_age=20",
 log_opts.filename);

I know people like that pg_upgrade doesn't care much about what version
it is running on, but it is really the ability of pg_upgrade to ignore
changes made to the server that is really why pg_upgrade is useful, and
this change makes pg_upgrade even more immune to such changes.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] Extension Packaging

[Dimitri Fontaine]

Daniele Varrazzo  writes:
> For my extension I'm less concerned by having the install sql named in
> different ways or by the upgrade sql as all these files are generated
> by scripts. You may find useful this one

You can also generate that reliably in SQL.  You install your extension
with CREATE EXTENSION then run the query over pg_depend and you have it
all.  Then you can test this upgrade script you just got in SQL.  Tom
also has a version that does the necessary string replacements using sed
from a bash script rather than the SQL replace() function.

  http://archives.postgresql.org/pgsql-hackers/2011-02/msg01208.php
  http://archives.postgresql.org/pgsql-hackers/2011-02/msg01438.php

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr

-- 
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] Fix for Perl 5.14

[Reini Urban]

2011/4/23 Andrew Dunstan :
> On 04/23/2011 03:02 AM, Alex Hunsaker wrote:
>>
>> Perl 5.14.0-RC1 came out a few days ago...
>>
>> There is a minor compile time error due to the API changing a bit:
>> plperl.c:929:3: error: lvalue required as left operand of assignment
>>
>> This is due to GvCV() no longer returning an lvalue, instead they want
>> us to use the new GvCV_set macro. (see
>>
>> http://search.cpan.org/~jesse/perl-5.14.0-RC1/pod/perldelta.pod#GvCV()_and_GvGP()_are_no_longer_lvalues)
>>
>> Unfortunately  that macro is not available on older perls so the
>> attached provides our own macro when GvCV_set is not defined.
>>
>> Tested with 5.14.0-rc1 and 5.12.3.
>>
>> The -head patch applies with fuzz to 9.0. The 8.4 patch applies clean
>> to 8.4 and with fuzz to 8.3 and 8.2.
>
>
> How nice of them not to fix it in ppport.h. I thought this is exactly the
> sort of thing it's for.

It's not so easy to convert
  foo = GvCV(bah);
to a
  GvCV_set(foo, bar);
with a ppport.h macro automatically.

But yes, the backport GvCV_set => lvalue GvCV should be in ppport.h.
It is not yet
-- 
Reini Urban

-- 
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] smallserial / serial2

[Mike Pultz]

Hey Tom,

 

I'm sure there are plenty of useful tables with <= 32k rows in them? I have
a table for customers that uses a smallint (since the customer id is
referenced all over the place)- due to the nature of our product, we're
never going to have more than 32k customers, but I still want the benefit of
the sequence.

 

And since serial4 and serial8 are simply pseudo-types- effectively there for
convenience, I'd argue that it should simply be there for completeness- just
because it may be less used, doesn't mean it shouldn't be convenient?

 

Also, in another case, I'm using it in a small table used to constrain a
bigger table- eg:

 

create table stuff (

   id serial2 unique

);

 

create table data (

   id serial8 unique,

   stuff smallint not null,

   foreign key(stuff) references stuff(id) on update cascade on delete
restrict

);

 

Where our "data" table has ~700 million rows right now.

 

And yes- I guess there's nothing to stop me from using a smallint in the
data table (thus getting the size savings), and reference a int in the stuff
table, but it seems like bad form to me to have a foreign key constraint
between two different types.

 

Mike

 

-Original Message-
From: Tom Lane [mailto:t...@sss.pgh.pa.us] 
Sent: Thursday, April 21, 2011 10:26 AM
To: Mike Pultz
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] smallserial / serial2 

 

"Mike Pultz" <  m...@mikepultz.com> writes:

> I use tables all the time that have sequences on smallint's; I'd like 

> to simplify my create files by not having to create the sequence 

> first, but I also don't want to give up those 2 bytes per column!

 

A sequence that can only go to 32K doesn't seem all that generally useful
...

 

Are you certain that you're really saving anything?  More likely than not,
the "saved" 2 bytes are going to disappear into alignment padding of a later
column or of the whole tuple.  Even if it really does help for your case,
that's another reason to doubt that it's generally useful.

 

regards, tom lane

Re: [HACKERS] Proposed fix for NOTIFY performance degradation

[Tom Lane]

Gianni Ciolli  writes:
> [ proposes lobotomization of duplicate-elimination behavior in NOTIFY ]

I think this change is likely to be penny-wise and pound-foolish.
The reason the duplicate check is in there is that things like triggers
may just do "NOTIFY my_table_changed".  If the trigger is fired N times
in a transaction, and you don't have duplicate-elimination in NOTIFY,
then you get N duplicate messages to no purpose.  And the expense of
actually sending (and processing) those messages is a lot higher than
suppressing them would be.

With the proposed change, the simplest case of just one such trigger is
still covered, but not two or more.  I don't think this is good enough.
It's basically throwing the responsibility on the application programmer
to avoid duplicates --- and in most scenarios, it will cost much more
to suppress duplicates in PL code than to do it here.

When I started to read this patch I was hoping to see some clever scheme
for detecting dups at lower cost than what we currently do, like perhaps
hashing.  I'm not impressed with just abandoning the responsibility,
though.

One idea we might consider is to offer two forms of NOTIFY, one that
suppresses dups and one that doesn't, so that in cases where the app
programmer knows his code doesn't generate (many) dups he can tell us
not to bother checking.

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] Some TODO items for collations

[Tom Lane]

I think the collations patch has now gone about as far as it's going to
get for 9.1.  There are a couple of areas that ought to be on the TODO
list for future versions, though:

* Integrating collations with text search configurations.  There are
several places in the tsearch code that currently have hard-wired uses
of DEFAULT_COLLATION_OID to control case-folding and character
classification behavior.  The most obvious way to generalize that would
be to have the tsearch operators/functions respond to COLLATE, but it
seems to me that that's likely a bad idea --- the appropriate collation
to use for these behaviors needs to be tied to the active text search
dictionary or configuration, probably.  Or maybe we should reverse that
and extend the notion of a collation object to include a reference to a
text search configuration.  It needs thought.  One other point is that
we can't easily put a collation selection into tsearch configurations
so long as collation names are platform-specific.  Should we have a TODO
item to find a way of providing platform-independent collation names?

* Integrating collations with to_char() and related functions.  Their
current behavior is a bit schizophrenic, in that things like TMMonth
will do case-folding according to the function's input COLLATE property,
but the month name itself is determined according to the LC_TIME GUC.
Not sure if we should extend the notion of a collation to cover all
of the LC_foo categories --- if we do, we'll have to think about the
interaction with the legacy GUC variables.

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] Fix for Perl 5.14

[Alex Hunsaker]

On Sat, Apr 23, 2011 at 07:00, Andrew Dunstan  wrote:
>
>
> On 04/23/2011 03:02 AM, Alex Hunsaker wrote:
>> ...
>> There is a minor compile time error due to the API changing a bit:
>> plperl.c:929:3: error: lvalue required as left operand of assignment
>> ...
>> Unfortunately  that macro is not available on older perls so the
>> attached provides our own macro when GvCV_set is not defined.

> How nice of them not to fix it in ppport.h. I thought this is exactly the
> sort of thing it's for.

Hrm, I have the latest released version of Devel::PPPort, 3.19. I went
poking around and found the have a newer "developer" release (3.19_03)
at http://search.cpan.org/~mhx/Devel-PPPort-3.19_03/ (released
2011-04-13). I see a few things for 5.14 but nothing about GvCV_set.
Maybe I'm doing something wrong? I'm just diffing its ppport.h with
ours. For the curious its attached.

> This looks OK, but I think we need to wait until they remove the RC tag.

Makes sense, Ill repost once they do ;).


ppport.patch.gz
Description: GNU Zip compressed data

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

Re: [HACKERS] What Index Access Method Functions are really needed?

[Kevin Grittner]

Yves Weißig wrote:
 
> I can not find "amcostestimate" in hash.h or anywhere, or am I
> missing something?
 
Search for hashcostestimate here:
 
http://git.postgresql.org/gitweb?p=postgresql.git;a=blob;f=src/backend/utils/adt/selfuncs.c
 
To find that, I ran this query:
 
select * from pg_am;
 
That showed me that the name for the hash implementation of that
function was hashcostestimate.  Then I ran this in my bash shell, at
the root of the source tree:
 
find -name '*.h' -or -name '*.c' \
 | egrep -v '^\./src/test/.+/tmp_check/' \
 | xargs egrep -n '\bhashcostestimate\b'
 
-Kevin


-- 
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] Collation patch's handling of wcstombs/mbstowcs is sheerest fantasy

[Tom Lane]

I wrote:
> * Where they're not, install the locale_t with uselocale(), do
> mbstowcs or wcstombs, and revert to the former locale_t setting.
> This is ugly as sin, and not thread-safe, but of course lots of
> the backend is not thread-safe.

I've been corrected on that: uselocale() *is* thread safe, at least in
glibc (it only affects the locale used by the current thread).  And it's
"only a few instructions" according to Jakub Jelinek.  So a temporary
setting via uselocale is exactly what you're supposed to do for any
locale-sensitive function that hasn't got a *_l variant.

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] pgbench \for or similar loop

[Dimitri Fontaine]

Greg Smith  writes:
> Kevin Grittner wrote:
>> I'm not clear on exactly what you're proposing there, but the thing
>> I've considered doing is having threads to try to keep a FIFO queue
>> populated with a configurable transaction mix, while a configurable
>> number of worker threads pull those transactions off the queue and...
>
> This is like the beginning of an advertisement for how Tsung is useful for
> simulating complicated workloads.  The thought of growing pgbench to reach
> that level of capabilities makes my head hurt.

+1 for having a look at Tsung here.  You'll be glad not to have to
reinvent all what it already does.

> When faced with this same issue, the sysbench team decided to embed Lua as
> their scripting language; sample scripts:

I would tend to prefer some scheme (guile comes to the mind but that's
GPL), being an Emacs user.  Also I've seen projects pick lua then down
the road regret the choice (http://julien.danjou.info/blog/2008.html).

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr

-- 
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] fsync reliability

[Matthew Woodcraft]

On 2011-04-22 21:55, Greg Smith wrote:
> On 04/22/2011 09:32 AM, Simon Riggs wrote:
>> OK, that's good, but ISTM we still have a hole during
>> RemoveOldXlogFiles() where we don't fsync or open/close the file, just
>> rename it.
> 
> This is also something that many applications rely upon working as hoped
> for here, even though it's not technically part of POSIX.  Early
> versions of ext4 broke that, and it caused a giant outcry of
> complaints. 
> http://www.h-online.com/open/news/item/Ext4-data-loss-explanations-and-workarounds-740671.html
> has a good summary.  This was broken on ext4 from around 2.6.28 to
> 2.6.30, but the fix for it was so demanded that it's even been ported by
> the relatively lazy distributions to their 2.6.28/2.6.29 kernels.

As far as I can make out, the current situation is that this fix (the
auto_da_alloc mount option) doesn't work as advertised, and the ext4
maintainers are not treating this as a bug.

See https://bugzilla.kernel.org/show_bug.cgi?id=15910

-M-


-- 
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] best way to test new index?

[Yves Weißig]

Thanks for the answer Kevin!

I am developing an encoded bitmap index, as proposed in
http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.44.7570&rank=1

Automated, in my words, would have meant... some SQL-Statements which
create tables, do inserts, deletes, selects and so on and the results
can be compared to a master file or whatsoever. On low level I tend to
go your way and debug with elog, I just thought somebody might have a
different approach on debugging such code.

Yves

Am 22.04.2011 18:30, schrieb Kevin Grittner:
> Yves Weißig wrote:
>  
>> Ok, but I thought more like an automated test, or a test which
>> checks if the interface is correctly implemented.
>  
> I'm not aware of any automated tests which would test whether a new
> index type is correctly implemented.  For starters, how would the
> test know when the AM should be used, or what correct results would
> be?  Perhaps if this was a drop-in replacement for btree you could
> cobble something together so that the standard regression tests
> (`make check` and such) would use your index type instead of btree,
> yielding similar results; but you haven't given us the slightest
> clue whether that's the sort of index you're developing.
>  
>> what would be the best way to debug in pg? elog? asserts?
>  
> If there are clear guidelines on this anywhere, I've missed it or
> forgotten it.  Here's what I tend to go by:
>  
> (1)  If it's a message which is expected enough to warrant
> translation to all the supported languages, use ereport.
>  
> (2)  If it's something which should be rare and technical enough not
> to warrant burdening the translators, use elog.
>  
> (3)  If it's something which "should never happen" unless there is a
> programming error within the PostgreSQL code, it should not normally
> be checked in production (just development and through beta
> testing), and it is sane to panic (effectively restarting
> PostgreSQL) when the condition is encountered, use Assert.
>  
> That's all independent of what logging level you use.  You may want
> to sprinkle your code with elog calls at the DEBUG level during
> development, and consider which might be worth keeping at which
> debug levels later on.  Sometimes debug calls are conditioned on
> #ifdef conditions so that they're not in the executable without a
> special build option.
>  
> -Kevin
> 

-- 
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] What Index Access Method Functions are really needed?

[Yves Weißig]

That is exactly the point, Kevin, I read the documentation and found out
that "amgettuple" and "amgetbitmap" are optional. I just wondered if
there is more information about this topic?
Additionally, I can not find "amcostestimate" in hash.h or anywhere, or
am I missing something? So "All of them" lacks some exceptions, or?

Am 22.04.2011 17:27, schrieb Kevin Grittner:
> Leonardo Francalanci  wrote:
 another question regarding indexes. Sadly I can't find enough 
 info in the documentation. Which of the functions are needed in
 order for a index to work?
>>>
>>> All of them.
>>
>>
>> Maybe I completely misunderstood the question, but some functions
>> are "optionals", such as amgetbitmap, right?
>>
>> http://www.postgresql.org/docs/9.0/static/index-functions.html
>  
> Browsing that page, I think these are the two relevant bits
> regarding exceptions to the rule:
>  
> | The amgettuple function need only be provided if the access method
> | supports "plain" index scans. If it doesn't, the amgettuple field
> | in its pg_am row must be set to zero.
>  
> | The amgetbitmap function need only be provided if the access
> | method supports "bitmap" index scans. If it doesn't, the
> | amgetbitmap field in its pg_am row must be set to zero.
>  
> I have no idea which of these would be useful for the AM being
> discussed.
>  
> -Kevin
> 

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

[HACKERS] Proposed fix for NOTIFY performance degradation

[Gianni Ciolli]

Hi,

while measuring NOTIFY execution time, I noticed a significant
performance drop.

Please find a patch attached, together with some tests; more details
are shown below.

Best regards,
Dr. Gianni Ciolli - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gianni.cio...@2ndquadrant.it | www.2ndquadrant.it

---8<--8<--8<--8<--8<--8<--8<--8<--8<---

h1. The problem

We record pending notifications in a transaction-based list.
Notifications are recorded in the same order as they are issued.

Because of PostgreSQL's asynchronous notification semantics, we don't
need to record two identical notifications; we record a notification
only if there are no duplicates.

This is implemented by scanning the list and checking for duplicates.
The list is scanned backwards because the last element is easily
accessible, which is a sensible optimisation in the case of many
notifications which are identical to the previous one (scenario A).

However, scanning the list is quite expensive in the case of many
notifications which are not identical to the previous one (scenario B,
see Test 1 below when m > 1).

h1. Proposed solution

To check only the last element in that list, which is efficient in
both scenarios (see Test 2 below).

h1. Tests

"PostgreSQL HEAD" has been fetched as after commit #a0e8df52 (Wed Apr
20 22:49:37 2011 -0400).

Test 1 has been executed against PostgreSQL HEAD.

Test 2 has been executed against patched version of PostgreSQL HEAD.

In the tables below:

* "n" denotes the number of notifications issued in a single
  transaction;

* "m" denotes the number of distinct channels used for these
  notifications;

* "iter" is the number of times each transaction has been repeated (to
  reduce the importance of occasional spikes);

* "avg_usec" denotes the average time in microseconds required by each
  NOTIFY statement.

h2. Test 1 - PostgreSQL HEAD 

   n   |   m   | iter | avg_usec 
---+---+--+--
10 | 1 |   10 |   43.730
   100 | 1 |   10 |   37.630
  1000 | 1 |   10 |   42.990
 1 | 1 |   10 |   36.225
10 |10 |   10 |   43.960
   100 |   100 |   10 |   46.537
  1000 |  1000 |   10 |  126.115
 1 | 1 |   10 |  906.501

h2. Test 2 - patched PostgreSQL

   n   |   m   | iter | avg_usec 
---+---+--+--
10 | 1 |   10 |   43.810
   100 | 1 |   10 |   38.256
  1000 | 1 |   10 |   36.950
 1 | 1 |   10 |   36.638
10 |10 |   10 |   44.830
   100 |   100 |   10 |   38.684
  1000 |  1000 |   10 |   38.924
 1 | 1 |   10 |   38.032
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 588e9f2..16c0d96 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -2160,7 +2160,6 @@ NotifyMyFrontEnd(const char *channel, const char *payload, int32 srcPid)
 static bool
 AsyncExistsPendingNotify(const char *channel, const char *payload)
 {
-	ListCell   *p;
 	Notification *n;
 
 	if (pendingNotifies == NIL)
@@ -2175,9 +2174,11 @@ AsyncExistsPendingNotify(const char *channel, const char *payload)
 	 * backwards in order to make duplicate-elimination a tad faster when the
 	 * same condition is signaled many times in a row. So as a compromise we
 	 * check the tail element first which we can access directly. If this
-	 * doesn't match, we check the whole list.
+	 * doesn't match, we used to check the whole list; we don't do that
+	 * anymore, because it was inefficient in the case when many distinct
+	 * channels are notified.
 	 *
-	 * As we are not checking our parents' lists, we can still get duplicates
+	 * Also, as we are not checking our parents' lists, we can get duplicates
 	 * in combination with subtransactions, like in:
 	 *
 	 * begin;
@@ -2192,15 +2193,6 @@ AsyncExistsPendingNotify(const char *channel, const char *payload)
 		strcmp(n->payload, payload) == 0)
 		return true;
 
-	foreach(p, pendingNotifies)
-	{
-		n = (Notification *) lfirst(p);
-
-		if (strcmp(n->channel, channel) == 0 &&
-			strcmp(n->payload, payload) == 0)
-			return true;
-	}
-
 	return false;
 }
 


test.sql
Description: application/sql

-- 
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] SSI non-serializalbe UPDATE performance (was: getting to beta)

[Kevin Grittner]

> Dan Ports  wrote:
 
> Even under these conditions I couldn't reliably see a slowdown. My
> latest batch of results (16 backends, median of three 10 minute
> runs) shows a difference well below 1%. In a couple of cases I saw
> the code with the SSI checks running faster than with them removed,
> so this difference seems in the noise.
 
Now that Dan has finished the comparative performance tests, and the
version with SSI sometimes tests faster, sometimes slower, with the
difference always a fraction of a percent even on a fairly unusual
worst case environment (tmpfs and dataset fits in cache buffers), I
think we can take this off the 9.1 "must fix" list.  We've seen much
larger changes with no explanation other than an assumption that the
executable code heppens to line up on line cache boundaries
differently.
 
Unless there is an objection, I'll move this item from "Blockers for
Beta1" to "Not Blockers for Beta1" on the Wiki page, pending results
of the point raised below.
 
> we might as well bail out of these functions early if there are no
> serializable transactions running. Kevin points out we can do this
> by checking if PredXact->SxactGlobalXmin is invalid. I would add
> that we can do this safely without taking any locks, even on
> weak-memory-ordering machines. Even if a new serializable
> transaction starts concurrently, we have the appropriate buffer
> page locked, so it's not able to take any relevant SIREAD locks.
 
Even though this didn't show any difference in Dan's performance
tests, it seems like reasonable insurance against creating a new
bottleneck in very high concurrency situations.
 
Dan, do you have a patch for this, or should I create one?
 
-Kevin

-- 
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] Fix for Perl 5.14

[Andrew Dunstan]

On 04/23/2011 03:02 AM, Alex Hunsaker wrote:

Perl 5.14.0-RC1 came out a few days ago...

There is a minor compile time error due to the API changing a bit:
plperl.c:929:3: error: lvalue required as left operand of assignment

This is due to GvCV() no longer returning an lvalue, instead they want
us to use the new GvCV_set macro. (see
http://search.cpan.org/~jesse/perl-5.14.0-RC1/pod/perldelta.pod#GvCV()_and_GvGP()_are_no_longer_lvalues)

Unfortunately  that macro is not available on older perls so the
attached provides our own macro when GvCV_set is not defined.

Tested with 5.14.0-rc1 and 5.12.3.

The -head patch applies with fuzz to 9.0. The 8.4 patch applies clean
to 8.4 and with fuzz to 8.3 and 8.2.



How nice of them not to fix it in ppport.h. I thought this is exactly 
the sort of thing it's for.


This looks OK, but I think we need to wait until they remove the RC tag.

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

[HACKERS] Fix for Perl 5.14

[Alex Hunsaker]

Perl 5.14.0-RC1 came out a few days ago...

There is a minor compile time error due to the API changing a bit:
plperl.c:929:3: error: lvalue required as left operand of assignment

This is due to GvCV() no longer returning an lvalue, instead they want
us to use the new GvCV_set macro. (see
http://search.cpan.org/~jesse/perl-5.14.0-RC1/pod/perldelta.pod#GvCV()_and_GvGP()_are_no_longer_lvalues)

Unfortunately  that macro is not available on older perls so the
attached provides our own macro when GvCV_set is not defined.

Tested with 5.14.0-rc1 and 5.12.3.

The -head patch applies with fuzz to 9.0. The 8.4 patch applies clean
to 8.4 and with fuzz to 8.3 and 8.2.
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 926,932  plperl_trusted_init(void)
  		if (!isGV_with_GP(sv) || !GvCV(sv))
  			continue;
  		SvREFCNT_dec(GvCV(sv)); /* free the CV */
! 		GvCV(sv) = NULL;		/* prevent call via GV */
  	}
  	hv_clear(stash);
  
--- 926,932 
  		if (!isGV_with_GP(sv) || !GvCV(sv))
  			continue;
  		SvREFCNT_dec(GvCV(sv)); /* free the CV */
! 		GvCV_set(sv, NULL);		/* prevent call via GV */
  	}
  	hv_clear(stash);
  
*** a/src/pl/plperl/plperl.h
--- b/src/pl/plperl/plperl.h
***
*** 49,54 
--- 49,59 
  (U32)HeKUTF8(he))
  #endif
  
+ /* supply GvCV_set if it's missing - ppport.h doesn't supply it, unfortunately */
+ #ifndef GvCV_set
+ #define GvCV_set(gv, cv)		(GvCV(gv) = cv)
+ #endif
+ 
  /* declare routines from plperl.c for access by .xs files */
  HV		   *plperl_spi_exec(char *, int);
  void		plperl_return_next(SV *);
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***
*** 700,706  plperl_trusted_init(void)
  		if (!isGV_with_GP(sv) || !GvCV(sv))
  			continue;
  		SvREFCNT_dec(GvCV(sv)); /* free the CV */
! 		GvCV(sv) = NULL;		/* prevent call via GV */
  	}
  	hv_clear(stash);
  	/* invalidate assorted caches */
--- 700,706 
  		if (!isGV_with_GP(sv) || !GvCV(sv))
  			continue;
  		SvREFCNT_dec(GvCV(sv)); /* free the CV */
! 		GvCV_set(sv, NULL);		/* prevent call via GV */
  	}
  	hv_clear(stash);
  	/* invalidate assorted caches */
*** a/src/pl/plperl/plperl.h
--- b/src/pl/plperl/plperl.h
***
*** 43,48 
--- 43,53 
  #undef bool
  #endif
  
+ /* supply GvCV_set if it's missing - ppport.h doesn't supply it, unfortunately */
+ #ifndef GvCV_set
+ #define GvCV_set(gv, cv)		(GvCV(gv) = cv)
+ #endif
+ 
  /* routines from spi_internal.c */
  int			spi_DEBUG(void);
  int			spi_LOG(void);

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