Re: [HACKERS] Add FET to Default and Europe.txt
On 6 October 2012 22:40, Tom Lane t...@sss.pgh.pa.us wrote: Thus for example MSK apparently now means GMT+4 not GMT+3. We can change the tznames entry for that, but should we get rid of MSD entirely? Some input from the Russians on this list would be helpful. ... Comments? It shouldn't be our job to make decisions like this on behalf of the world. The TZ database clearly changes over time, so doing this accurately would mean we'd need to hold start and stop dates for each code so it can be used appropriately with specific times. Which would mean holding data in much finer detail that the source data, which is a little bizarre. * We should deprecate all use of timezone abbreviations in our internal code, if any. * Ship only the current tz file, as shipped by IANA with as few prep steps as possible * Make the tz file configurable, so people can be more explicit about what *they* mean by certain codes, to avoid the need for choosing between countries. For example, someone may have hardcoded particular codes with the understanding they relate to one particular TZ - if we make changes, we will alter the application logic, so that must be able to be put back for backwards compatibility. -- Simon Riggs 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] Rethinking placement of latch self-pipe initialization
On 7 October 2012 18:27, Tom Lane t...@sss.pgh.pa.us wrote: Sean Chittenden recently reported that 9.2 can crash after logging FATAL: pipe() failed if the kernel is short of file descriptors: http://archives.postgresql.org/pgsql-general/2012-10/msg00202.php The only match to that error text is in initSelfPipe(). What I believe is happening is that InitProcess is calling OwnLatch which calls initSelfPipe, and the latter fails, and then the postmaster thinks that was a backend crash because we have armed the dead-man switch but not set up on_shmem_exit(ProcKill) which would disarm it. It's possible we could fix this by changing the order of operations in InitProcess and OwnLatch, but it'd be tricky, not least because ProcKill calls DisownLatch which asserts that OwnLatch was done. What I think would be a better idea is to fix things so that OwnLatch cannot fail except as a result of internal logic errors, by splitting out the acquisition of the self-pipe into a separate function named say InitializeLatchSupport. The question then becomes where to put the InitializeLatchSupport calls. My first thought is to put them near the signal-setup stanzas for the various processes (ie, the pqsignal calls) similarly to what we did recently for initialization of timeout support. However there might be a better idea. Comments? We still have to consider how Postgres would operate without the latches. I don't see that it can, so a shutdown seems appropriate. Is the purpose of this just to allow a cleaner and more informative shutdown? Or do you think we can avoid? If we did move the init calls, would that alter things for code that creates new used defined latches? -- Simon Riggs 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] Bad Data back Door
Tom Lane wrote: David E. Wheeler da...@justatheory.com writes: On Oct 5, 2012, at 6:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, having said that, I think it has to be the reponsibility of the FDW to apply any required check ... which makes this a bug report against oracle_fdw, not the core system. (FWIW, contrib/file_fdw depends on the COPY code, which will check encoding.) I agree that this is a bug in oracle_fdw (well, potentially; ultimately, it's Oracle that's lying about the encoding of those text values). But I think that it would be much more useful overall -- not to mention more database-like -- for PostgreSQL to provide a way to enforce it. That is, to consider foreign tables to be an input like COPY or SQL, and to validate values before displaying them. It is the FDW's responsibility to deal with this. We expect it to hand back valid tuples; it is not reasonable to disassemble them looking for mistakes (and we couldn't catch most mistakes, anyway). If the interface were defined in terms of text, we could do checking above the FDW level ... but it isn't. As the author I agree that this is a bug in oracle_fdw. This was caused by ignorance on my part: I had assumed that the type input functions would perform the necessary checks, but it seems like that is not the case. I'll look into it. Oracle does not care much about correct encoding. If client character set and database character set are the same, Oracle does not bother to check the data. This is probably how WINDOWS-1252 characters slipped into the UTF-8 database in question. I consider this a bug in Oracle, but never reported it, because I don't have much hope that Oracle would see it as a problem given their habitually sloppy handling of encoding issues. Yours, Laurenz Albe -- 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] Bad Data back Door
On Oct 8, 2012, at 12:25 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: As the author I agree that this is a bug in oracle_fdw. Thanks. Should I file a report somewhere? This was caused by ignorance on my part: I had assumed that the type input functions would perform the necessary checks, but it seems like that is not the case. I'll look into it. Thank you! Oracle does not care much about correct encoding. If client character set and database character set are the same, Oracle does not bother to check the data. This is probably how WINDOWS-1252 characters slipped into the UTF-8 database in question. I consider this a bug in Oracle, but never reported it, because I don't have much hope that Oracle would see it as a problem given their habitually sloppy handling of encoding issues. Yeah, same here. I've been looking into write a function to try to fix poorly-encoded data, though, but haven't got far, because CONVERT() does not indicate failure. If you have any insight on this, I'd appreciate your thoughts on this Stack Overflow question: http://stackoverflow.com/q/12717363/79202 Thanks, David smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] Add FET to Default and Europe.txt
On 08.10.2012 10:03, Simon Riggs wrote: On 6 October 2012 22:40, Tom Lanet...@sss.pgh.pa.us wrote: Thus for example MSK apparently now means GMT+4 not GMT+3. We can change the tznames entry for that, but should we get rid of MSD entirely? Some input from the Russians on this list would be helpful. ... Comments? It shouldn't be our job to make decisions like this on behalf of the world. I wish it wasn't, too. But I don't think there's any upstream for this information. The tz library has abbreviations for timezones, but they're not unique. * Make the tz file configurable, so people can be more explicit about what *they* mean by certain codes, to avoid the need for choosing between countries. For example, someone may have hardcoded particular codes with the understanding they relate to one particular TZ - if we make changes, we will alter the application logic, so that must be able to be put back for backwards compatibility. It is configurable. See http://www.postgresql.org/docs/devel/static/datetime-config-files.html. We're just discussing what the defaults should be. - Heikki -- 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 psql \ds
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
Julien Tachoires jul...@gmail.com writes:
About \ds behaviour, I think to add 2 columns :
- 'LastValue'
- 'Increment'
That would make the command a great deal slower, since it would have to
access each sequence to get that info. I don't object to adding such
columns to \ds+, but I don't think it's a good idea to put them in the
basic command.
Ok.
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern, show_verbose);
+ break;
Cheers,
--
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] Add FET to Default and Europe.txt
On 8 October 2012 09:05, Heikki Linnakangas hlinnakan...@vmware.com wrote: * Make the tz file configurable, so people can be more explicit about what *they* mean by certain codes, to avoid the need for choosing between countries. For example, someone may have hardcoded particular codes with the understanding they relate to one particular TZ - if we make changes, we will alter the application logic, so that must be able to be put back for backwards compatibility. It is configurable. See http://www.postgresql.org/docs/devel/static/datetime-config-files.html. We're just discussing what the defaults should be. The problem there is that the default is Default, so you have no idea what you are accepting and so are unlikely to be brave enough to alter it. If default were named Postgres9 then people would at least understand that hackers had decided a few things and they might want to override it. So I think we need 2 new settings: one called Postgres92, one called Postgres93. 93 has the new settings and is the default. Default would no longer map to anything, to make sure we have an explicit break of compatibility. -- Simon Riggs 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] Add FET to Default and Europe.txt
Am 08.10.12 11:07, schrieb Simon Riggs: On 8 October 2012 09:05, Heikki Linnakangas hlinnakan...@vmware.com wrote: * Make the tz file configurable, so people can be more explicit about what *they* mean by certain codes, to avoid the need for choosing between countries. For example, someone may have hardcoded particular codes with the understanding they relate to one particular TZ - if we make changes, we will alter the application logic, so that must be able to be put back for backwards compatibility. It is configurable. See http://www.postgresql.org/docs/devel/static/datetime-config-files.html. We're just discussing what the defaults should be. The problem there is that the default is Default, so you have no idea what you are accepting and so are unlikely to be brave enough to alter it. If default were named Postgres9 then people would at least understand that hackers had decided a few things and they might want to override it. So I think we need 2 new settings: one called Postgres92, one called Postgres93. 93 has the new settings and is the default. Default would no longer map to anything, to make sure we have an explicit break of compatibility. Removing the timezone abbreviations from the default settings is probably the wrong approach. People use them, I use them, and my original request to add FET came from the fact that someone wanted to use it. As long as we have a type timestamp with timezone, there should also be a way use and specify timezones in a usual format - by default. I think the problem we face is more of a maintainer nature than of a technical nature. Someone has to maintain this information. But then all other projects, mostly BSDs, that I was involved with, managed to keep this information more or less up to date. A good starting point would be to take the timezone information directly from the the files IANA distributes, instead of manually copying and maintaining them in a separate file. If no one else does, I can try to maintain these files. Those who know about my work, know that I do a lot of time related software (support for time signal receivers in OpenBSD, e.g.). So my vote - if I have one - is to keep the TZ information, but maybe maintain it better, if needed. Oh, and as a side note, I did not check how often the TZ database gets updated in PostgreSQL, if someone actually maintains it, I did not want to imply you do a bad job ;) -- 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] Add FET to Default and Europe.txt
On 8 October 2012 11:14, Marc Balmer m...@msys.ch wrote: So I think we need 2 new settings: one called Postgres92, one called Postgres93. 93 has the new settings and is the default. Default would no longer map to anything, to make sure we have an explicit break of compatibility. Removing the timezone abbreviations from the default settings is probably the wrong approach. People use them, I use them, and my original request to add FET came from the fact that someone wanted to use it. Sorry for any confusion. I've not suggested removing timezone abbreviations, nor do I wish to do so. I did suggest that we do not rely upon TZ abbreviations in any code shipped by PostgreSQL project, but I suspect that is a non-problem anyway. -- Simon Riggs 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] more suitable messages for analyze on hot standby.
On 08.10.2012 04:04, Tomonari Katsumata wrote: I work with streaming replication and hot standby. And I noticed that error message is not proper when I issue ANALYZE command to standby. ... This is not big problem, but error message should report actual thing. please check it. Thanks, committed with some stylistic changes. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Promoting a standby during base backup (was Re: [HACKERS] Switching timeline over streaming replication)
On 04.10.2012 20:07, Fujii Masao wrote: On Thu, Oct 4, 2012 at 4:59 PM, Heikki Linnakangas But I wonder why promoting a standby renders the backup invalid in the first place? Fujii, Simon, can you explain that? Simon had the same question and I answered it before. http://archives.postgresql.org/message-id/cahgqgwfu04oo8yl5sucdjvq3brni7wtfzty9wa2kxtznhic...@mail.gmail.com --- You say If the standby is promoted to the master during online backup, the backup fails. but no explanation of why? I could work those things out, but I don't want to have to, plus we may disagree if I did. If the backup succeeds in that case, when we start an archive recovery from that backup, the recovery needs to cross between two timelines. Which means that we need to set recovery_target_timeline before starting recovery. Whether recovery_target_timeline needs to be set or not depends on whether the standby was promoted during taking the backup. Leaving such a decision to a user seems fragile. pg_control is backed up last, it would contain the new timeline. No need to set recovery_target_timeline. pg_basebackup -x ensures that all required files are included in the backup and we can start recovery without restoring any file from the archive. But if the standby is promoted during the backup, the timeline history file would become an essential file for recovery, but it's not included in the backup. That is true. We could teach it to include the timeline history file, though. The situation may change if your switching-timeline patch has been committed. It's useful if we can continue the backup even if the standby is promoted. It wouldn't help with pg_basebackup -x, although it would allow streaming replication to fetch the timeline history file. I guess it's best to keep that restriction for now. But I'll add a TODO item for this. - Heikki -- 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] Libxml2 load error on Windows
On Mon, Jun 18, 2012 at 02:08:29PM +0500, Talha Bin Rizwan wrote: PostgreSQL 9.2 Windows build is having trouble with the XML support: http://postgresql.1045698.n5.nabble.com/9-2-beta1-libxml2-can-t-be-loaded-on-Windows-td5710672.html Therefore, I used first approach to get libxml2 version number and include the #define HAVE_XMLSTRUCTUREDERRORCONTEXT in pg_config.h if libxml2 version is greater than 20703 (which effectively is 2.7.3). Please find attached patch libxml2_win_v1.patch. Tom fixed this differently; effectively, we now use a check like you describe on all platforms, not just Windows: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=470d0b9789981bc91a8ef2654911d80ab6a6be57 Since that commit makes your patch obsolete, I'm marking it Rejected. Even so, thank you for calling attention to the problem and submitting this fix. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PQntuples and PQgetvalue problem.
Hello,
In my C++ program, i want to partition sql table, there is sample code below.
It is working right. the problem is after func called 2 or 3 times in func1,
PQntuples results 0 although tablename exist. Also when i disabled PQntuples
at if statement, other psql function PQgetvalue(res) return error that is row
number 0 is out of range 0..-1there are a couple function call same as func1(to
check tablename ), at func. when i disabled one of them another at PQntuples
funcx return 0. i check so may times. it is so weird.
So i think you can help me.
Best regards.
Zafer,
Sample code :
func1{
char cmd[100] = ;
snprintf(cmd,sizeof(cmd),select tablename from pg_tables where
tablename='%s', tablename);
PGresult *res = PQexec(conn, cmd );
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
logger.error(PQexec: %s, PQerrorMessage(conn));
PQclear(res);
wexit(1);
}
if (PQntuples(res) == 0)
//
}
func {
PGresult *res;
func1();
res=PQexec(conn,Select count(*) from persons);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
printf(PQexec: %s, PQerrorMessage(conn));
PQclear(res);
exit(1);
}
if( atoi(PQgetvalue(res,0,0)) != 0)
// some code PQexec()}}
Re: [HACKERS] Visual Studio 2012 RC
On Sun, Oct 07, 2012 at 08:30:22PM +0200, Brar Piening wrote:
Noah Misch wrote:
I'm marking this patch Waiting on Author, but the changes needed to
get it Ready for Committer are fairly trivial. Thanks, nm
Thanks for your review and sorry for my delayed response - I've been on
vacation.
I'll look into adressing your comments and suggestions within the next
few days.
Thanks. I decided to try a 32-bit build, but Solution::DeterminePlatform
detected it as x64. Its shibboleth is no longer valid; the cl.exe shipping
with VS 2012 Express for Desktop has a /favor option for both architectures:
32clhelp:/favor:blend|ATOM select processor to optimize for, one of:
64clhelp:/favor:blend|AMD64|INTEL64|ATOM select processor to optimize for,
one of:
Overlaying the first attached change fixed detection for this particular
compiler, but I have not checked compatibility with older versions. Do you
have VS 2008 and/or VS 2010 handy? Having worked around that, the build
eventually failed like this:
Creating library Debug\postgres\postgres.lib and object
Debug\postgres\postgres.exp
postgres.exp : error LNK2001: unresolved external symbol
_xmm@41f0
[c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj]
postgres.exp : error LNK2001: unresolved external symbol
_xmm@80008000
[c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj]
postgres.exp : error LNK2001: unresolved external symbol
_xmm@8000800080008000
[c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj]
.\Debug\postgres\postgres.exe : fatal error LNK1120: 3 unresolved externals
[c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj]
The command exited with code 1120.
Done executing task Link -- FAILED.
This compiler emits _xmm symbols automatically, where needed. The second
attached change lets the build complete and pass tests, but I can't readily
explain why it's necessary. In the 64-bit build, the _xmm symbols export
normally (albeit, I presume, needlessly). I hoped to find some rationale for
the preexisting gendef.pl exclusion of _real, which seems to resemble _xmm in
origin and use. Magnus/anyone, can you shed light on our exclusion of _real
symbols from .def files?
In any event, if these incremental changes seem sane to you, please merge them
into your next version.
nm
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index e51665c..218ca8d 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -63,13 +63,12 @@ sub DeterminePlatform
{
my $self = shift;
- # Determine if we are in 32 or 64-bit mode. Do this by seeing if CL has
- # 64-bit only parameters.
+ # Examine CL help output to determine if we are in 32 or 64-bit mode.
$self-{platform} = 'Win32';
- open(P, cl /? 2NUL|) || die cl command not found;
+ open(P, cl /? 21 |) || die cl command not found;
while (P)
{
- if (/^\/favor:/)
+ if (/for x64/)
{
$self-{platform} = 'x64';
last;
diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index ab65c46..8ef0422 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -40,6 +40,7 @@ while ($ARGV[0]/*.obj)
next if $pieces[6] =~ /^\(/;
next if $pieces[6] =~ /^__real/;
next if $pieces[6] =~ /^__imp/;
+ next if $pieces[6] =~ /^__xmm/;
next if $pieces[6] =~ /NULL_THUNK_DATA$/;
next if $pieces[6] =~ /^__IMPORT_DESCRIPTOR/;
next if $pieces[6] =~ /^__NULL_IMPORT/;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Promoting a standby during base backup (was Re: [HACKERS] Switching timeline over streaming replication)
On 4 October 2012 18:07, Fujii Masao masao.fu...@gmail.com wrote:
On Thu, Oct 4, 2012 at 4:59 PM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
On 03.10.2012 18:15, Amit Kapila wrote:
On Tuesday, October 02, 2012 4:21 PM Heikki Linnakangas wrote:
Hmm, should a base backup be aborted when the standby is promoted? Does
the promotion render the backup corrupt?
I think currently it does so. Pls refer
1.
do_pg_stop_backup(char *labelfile, bool waitforarchive)
{
..
if (strcmp(backupfrom, standby) == 0 !backup_started_in_recovery)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg(the standby was promoted during
online backup),
errhint(This means that the backup
being
taken is corrupt
and should not be used.
Try taking another
online
backup.)));
..
}
Okay. I think that check in do_pg_stop_backup() actually already ensures
that you don't end up with a corrupt backup, even if the standby is promoted
while a backup is being taken. Admittedly it would be nicer to abort it
immediately rather than error out at the end.
But I wonder why promoting a standby renders the backup invalid in the first
place? Fujii, Simon, can you explain that?
Simon had the same question and I answered it before.
http://archives.postgresql.org/message-id/cahgqgwfu04oo8yl5sucdjvq3brni7wtfzty9wa2kxtznhic...@mail.gmail.com
---
You say
If the standby is promoted to the master during online backup, the
backup fails.
but no explanation of why?
I could work those things out, but I don't want to have to, plus we
may disagree if I did.
If the backup succeeds in that case, when we start an archive recovery from
that
backup, the recovery needs to cross between two timelines. Which means that
we need to set recovery_target_timeline before starting recovery. Whether
recovery_target_timeline needs to be set or not depends on whether the standby
was promoted during taking the backup. Leaving such a decision to a user seems
fragile.
I accepted your answer before, but I think it should be challenged
now. This is definitely a time when you really want that backup, so
invalidating it for such a weak reason is not useful, even if I
understand your original thought.
Something that has concerned me is that we don't have an explicit
timeline change record. We *say* we do that at shutdown checkpoints,
but that is recorded in the new timeline. So we have the strange
situation of changing timeline at two separate places.
When we change timeline we really should generate one last WAL on the
old timeline that marks an explicit change of timeline and a single
exact moment when the timeline change takes place. With PITR we are
unable to do that, because any timeline can fork at any point. With
smooth switchover we have a special case that is not anything goes
and there is a good case for not incrementing the timeline at all.
This is still a half-formed thought, but at least you should know I'm
in the debate.
--
Simon Riggs 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] sortsupport for text
Do you think you can follow through on this soon, Robert? I don't believe that there are any outstanding issues. I'm not going to make an issue of the fact that strxfrm() hasn't been taken advantage of. If you could please post a new revision, with the suggested alterations (that you agree with), I'd be happy to take another look. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Regarding identifying a foreign scan
On Sun, Oct 7, 2012 at 10:29 AM, Tom Lane t...@sss.pgh.pa.us wrote: Atri Sharma atri.j...@gmail.com writes: Does that mean that using (some) global storage is the cause of the problem? If you're using global storage for state that needs to be replicated per-scan, yes, probably. But it's hard to be sure when you're being so vague about what you're doing. yeah -- the problem here is that Atri is not using ForeignScanState-fdw_state properly for storage (this in jdbc-fdw). I think he knows what to do -- he's going to fix it up. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] odd alter_generic regression failures
We seem to have an intermittent failure on the alter_generic tests that look like this: SET SESSION AUTHORIZATION regtest_alter_user1; CREATE FUNCTION alt_func1(int) RETURNS int LANGUAGE sql AS 'SELECT $1 + 1'; + ERROR: permission denied for language sql CREATE FUNCTION alt_func2(int) RETURNS int LANGUAGE sql AS 'SELECT $1 - 1'; + ERROR: permission denied for language sql CREATE AGGREGATE alt_agg1 ( sfunc1 = int4pl, basetype = int4, stype1 = int4, initcond = 0 ); This has been seen on at least two buildfarm members: chough (Windows/MSVC) and smilodon (NetBSD/gcc). The fact that it's intermittent is rather worrying and puzzling. 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] Deparsing DDL command strings
Tom Lane t...@sss.pgh.pa.us writes: going to be required below that ... but the point I'm trying to make is that it would be a one-and-done task. Adding a requirement to be able to decompile raw parse trees will be a permanent drag on every type of SQL feature addition. I'll show some examples of very involved command (CREATE and ALTER TABLE are the most complex we have I think) and some very simple commands (DROP TABLE is one of the simplest), so that we can make up our minds on that angle. Then we want to qualify object names. Some type names have already been taken care of apparently by the parser here, relation names not yet and we need to cope with non existing relation names. Which is exactly what you *won't* be able to do anything about when looking at a raw parse tree. It's just a different presentation of the query string. So, I'm currently adding the deparsing to the existing only event we have, which is ddl_command_start. That's maybe not the best place where to do it, I even now wonder if we can do it there at all. Doing the same thing at ddl_command_end would allow us have all the information we need and leave nothing to magic guesses: full schema qualification of all objects involved, main object(s) OIDs available, all the jazz. Well, yeah. Anything else is magic not code. Well, prepending an object name with the first entry of the current search_path as its schema is not that far a stretch when the object is being created, as far as I see it. It's more reasonable to document that the rewritten no-ambiguities command string is only available for ddl_command_end events, though. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] [COMMITTERS] pgsql: Disable _FORTIFY_SOURCE with ICC
On 5 October 2012 04:37, Tom Lane t...@sss.pgh.pa.us wrote: A bit later: testing on an F17 box (glibc-2.15-56.fc17.x86_64) confirms Peter G's complaint, and also confirms that putting the above into port/linux.h (instead of the template file) fixes the problem. Don't know how to disable it on ICC, but I suppose there's some way to test for ICC via an #if. I was using F17 + glibc 2.15 too. http://www.acsu.buffalo.edu/~charngda/icc.html indicates that ICC defines a macro __ICC, which evaluates to an integer constant representing the ICC version numbers. I guess that that particular source isn't very trustworthy, but that's what we have a build farm for. Small patch that hopefully fixes everything for everyone is attached. However, when I build at -O2 with the system GCC (on the same F17 box with no changes to CFLAGS; just ./configure --with-libxml --prefix=/home/peter/pgsql --enable-depend) this patch (when applied to today's master) sees the following warnings raised: zic.c: In function ‘writezone’: zic.c:1752:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1753:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1754:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1755:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1756:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1757:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1758:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1759:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1760:3: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1772:4: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c:1785:4: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c: In function ‘puttzcode64’: zic.c:1514:2: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] zic.c: In function ‘puttzcode’: zic.c:1505:2: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] In file included from gram.y:13502:0: scan.c: In function ‘yy_try_NUL_trans’: scan.c:16247:23: warning: unused variable ‘yyg’ [-Wunused-variable] -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services fortify_source_fix.2012_10_08.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking placement of latch self-pipe initialization
Simon Riggs si...@2ndquadrant.com writes: We still have to consider how Postgres would operate without the latches. I don't see that it can, so a shutdown seems appropriate. Is the purpose of this just to allow a cleaner and more informative shutdown? Or do you think we can avoid? The point is that right now, if a new backend fails its initialization at this specific step, that gets translated into a database-wide crash and restart cycle, for no good reason. It should just result in that particular session failing. If we did move the init calls, would that alter things for code that creates new used defined latches? Only to the extent that it'd have to make sure it called the initialization function at some appropriate point. It's not like required initialization functions are a foreign concept in our code. 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] enhanced error fields
On 20 August 2012 14:09, Pavel Stehule pavel.steh...@gmail.com wrote: (eelog-2012-08-20.diff) This patch has the following reference to relerror.c: *** a/src/include/utils/rel.h --- b/src/include/utils/rel.h *** *** 394,397 typedef struct StdRdOptions --- 394,402 extern void RelationIncrementReferenceCount(Relation rel); extern void RelationDecrementReferenceCount(Relation rel); + /* routines in utils/error/relerror.c */ Indeed, the new C file has been separately added to a makefile: ! OBJS = assert.o elog.o relerror.o However, the new file itself does not appear in this patch. Therefore, the code does not compile. Please post a revision with the new file included. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
On Thu, Oct 4, 2012 at 6:12 AM, Amit kapila amit.kap...@huawei.com wrote: 1. One new configuration parameter wal_receiver_timeout is added to detect timeout at receiver task. 2. Existing parameter replication_timeout is renamed to wal_sender_timeout. -1 from me on a backward compatibility break here. I don't know what else to call the new GUC (replication_server_timeout?) but I'm not excited about breaking existing conf files, nor do I particularly like the proposed new names. -- 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] Missing OID define
On Fri, Oct 5, 2012 at 3:32 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: Phil Sorber wrote: Thom Brown and I were doing some hacking the other day and came across this missing define. We argued over who was going to send the patch in and I lost. So here it is. +1 I have been missing this #define. After extensive review, I have committed this patch. -- 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
[HACKERS] why can't plpgsql return a row-expression?
Hi, PL/pgsql seems to have a strange restriction regarding RETURN. Normally, you can write RETURN expression. But if the function returns a row type, then you can only write RETURN variable or RETURN NULL. rhaas=# create type xyz as (a int, b int); CREATE TYPE rhaas=# select row(1,2)::xyz; row --- (1,2) (1 row) rhaas=# create or replace function return_xyz() returns xyz as $$ rhaas$# begin rhaas$# return row(1,2)::xyz; rhaas$# end$$ language plpgsql; ERROR: RETURN must specify a record or row variable in function returning row LINE 3: return row(1,2)::xyz; ^ Off the top of my head, I can't think of any reason for this restriction, nor can I find any code comments or anything in the commit log which explains the reason for it. Does anyone know why we don't allow this? -- 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] [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
On Monday, October 08, 2012 7:38 PM Robert Haas wrote: On Thu, Oct 4, 2012 at 6:12 AM, Amit kapila amit.kap...@huawei.com wrote: 1. One new configuration parameter wal_receiver_timeout is added to detect timeout at receiver task. 2. Existing parameter replication_timeout is renamed to wal_sender_timeout. -1 from me on a backward compatibility break here. I don't know what else to call the new GUC (replication_server_timeout?) but I'm not excited about breaking existing conf files, nor do I particularly like the proposed new names. How about following: 1. replication_client_timeout -- shouldn't it be client as new configuration is for wal receiver 2. replication_standby_timeout If we introduce a new parameter for wal receiver, wouldn't replication_timeout be confusing for user? With Regards, Amit Kapila. -- 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 psql \ds
On Mon, Oct 8, 2012 at 3:49 AM, Julien Tachoires jul...@gmail.com wrote:
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
Julien Tachoires jul...@gmail.com writes:
About \ds behaviour, I think to add 2 columns :
- 'LastValue'
- 'Increment'
That would make the command a great deal slower, since it would have to
access each sequence to get that info. I don't object to adding such
columns to \ds+, but I don't think it's a good idea to put them in the
basic command.
Ok.
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern,
show_verbose);
+ break;
What happens if the user does this:
\dis
--
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] Improving psql \ds
Julien Tachoires jul...@gmail.com writes:
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern, show_verbose);
+ break;
No, that's not acceptable, because it will break cases such as \dstv.
The \dxxx command namespace is already direly overloaded; I'm not sure
cramming more behaviors into it is a good idea anyway. Perhaps
something along the line of \seqs would be a usable substitute.
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] Improving psql \ds
2012/10/8 Robert Haas robertmh...@gmail.com:
On Mon, Oct 8, 2012 at 3:49 AM, Julien Tachoires jul...@gmail.com wrote:
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
Julien Tachoires jul...@gmail.com writes:
About \ds behaviour, I think to add 2 columns :
- 'LastValue'
- 'Increment'
That would make the command a great deal slower, since it would have to
access each sequence to get that info. I don't object to adding such
columns to \ds+, but I don't think it's a good idea to put them in the
basic command.
Ok.
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern,
show_verbose);
+ break;
What happens if the user does this:
\dis
hmm, you're right, this is not a good idea.
--
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 can't plpgsql return a row-expression?
Robert Haas robertmh...@gmail.com writes: ERROR: RETURN must specify a record or row variable in function returning row Off the top of my head, I can't think of any reason for this restriction, nor can I find any code comments or anything in the commit log which explains the reason for it. Does anyone know why we don't allow this? Laziness, probably. Feel free to have at it. 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] Add FET to Default and Europe.txt
Bruce Momjian br...@momjian.us writes: Could we pull an abbreviation list from one of the BSDs? And that would be authoritative why? 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] Add FET to Default and Europe.txt
On Mon, Oct 8, 2012 at 11:10:08AM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Could we pull an abbreviation list from one of the BSDs? And that would be authoritative why? It would be somone else maintaining it. :-) -- Bruce Momjian br...@momjian.ushttp://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] Add FET to Default and Europe.txt
Am 08.10.12 16:53, schrieb Bruce Momjian: On Mon, Oct 8, 2012 at 12:14:15PM +0200, Marc Balmer wrote: A good starting point would be to take the timezone information directly from the the files IANA distributes, instead of manually copying and maintaining them in a separate file. If no one else does, I can try to maintain these files. Those who know about my work, know that I do a lot of time related software (support for time signal receivers in OpenBSD, e.g.). Could we pull an abbreviation list from one of the BSDs? Imo, the data should be pulled from the official source. Else an unneeded dependency from BSD is likely created... -- 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] Add FET to Default and Europe.txt
On Mon, Oct 8, 2012 at 05:15:18PM +0200, Marc Balmer wrote: Am 08.10.12 16:53, schrieb Bruce Momjian: On Mon, Oct 8, 2012 at 12:14:15PM +0200, Marc Balmer wrote: A good starting point would be to take the timezone information directly from the the files IANA distributes, instead of manually copying and maintaining them in a separate file. If no one else does, I can try to maintain these files. Those who know about my work, know that I do a lot of time related software (support for time signal receivers in OpenBSD, e.g.). Could we pull an abbreviation list from one of the BSDs? Imo, the data should be pulled from the official source. Else an unneeded dependency from BSD is likely created... I thought there was no official source of abbreviations that deals with duplicates by choosing the most appropriate one. Is there? Where is it? -- Bruce Momjian br...@momjian.ushttp://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] Improving psql \ds
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
Julien Tachoires jul...@gmail.com writes:
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern, show_verbose);
+ break;
No, that's not acceptable, because it will break cases such as \dstv.
The \dxxx command namespace is already direly overloaded; I'm not sure
cramming more behaviors into it is a good idea anyway. Perhaps
something along the line of \seqs would be a usable substitute.
Ok. Does having both '\ds' and , let's call it '\seqs', won't be
source of confusion for the user ?
--
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] Add FET to Default and Europe.txt
Simon Riggs si...@2ndquadrant.com writes: Sorry for any confusion. I've not suggested removing timezone abbreviations, nor do I wish to do so. I did suggest that we do not rely upon TZ abbreviations in any code shipped by PostgreSQL project, but I suspect that is a non-problem anyway. No, we don't rely on them AFAIK; certainly we don't use them in data output. But people expect to be able to use them in data input. The idea of tracking date ranges in which a TZ abbreviation is valid is an interesting one, but it's vastly more effort than I for one am willing to put into the problem --- not just in coding the infrastructure, but gathering and maintaining the source data. [ reflects for a bit... ] I guess in principle we could pull change information out of the IANA database rather than having to maintain it ourselves. But still what you're suggesting is an awful lot of work. The other thing that the abbreviation list files are doing for us is providing a user-configurable way to resolve conflicting abbreviations, for instance IST (the Indians and the Israelis both use this, but not to mean the same thing). This requirement isn't ever going to go away. The Default list represents some considered judgements as to what the most widely useful set of abbreviations is. We don't get to abdicate the position of having to make those judgements; nobody would thank us for breaking their database configurations because we can't decide. I still think that we need some input from actual Russians as to what is the currently most useful set of abbreviations for Russian time zones. Armchair speculation from the other side of the globe isn't going to be helpful to them. 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] sortsupport for text
On Mon, Oct 8, 2012 at 8:47 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: Do you think you can follow through on this soon, Robert? I don't believe that there are any outstanding issues. I'm not going to make an issue of the fact that strxfrm() hasn't been taken advantage of. If you could please post a new revision, with the suggested alterations (that you agree with), I'd be happy to take another look. I don't have any plans to work on this further. I think all of the criticism that has been leveled at this patch is 100% bogus, and I greatly dislike the changes that have been proposed. That may not be fair, but it's how I feel, and in light of that feeling it does not make sense for me to pursue this. Sorry. -- 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] Move postgresql_fdw_validator into dblink
Hanada-san, The attached patch is a revised one according to my previous suggestion. It re-defines PQconninfoOption *options as static variable with NULL initial value, then, PQconndefaults() shall be invoked at once. The default options never changed during duration of the backend process, so here is no reason why we allocate and free this object for each validator invocation. If it is also OK for you, I'd like to take over this patch to comitter. This patch is prerequisite of postgresql_fdw, so I hope this patch getting merged soon. Thanks, 2012/9/20 Kohei KaiGai kai...@kaigai.gr.jp: Hanada-san, I checked your patch. It can be applied to the latest master branch without any conflicts, and regression tests were fine. Unlike the original postgresql_fdw_validator(), the new dblink_fdw_validator() has wise idea; that pulls list of connection options from libpq, instead of self-defined static table. I basically agree not to have multiple source of option list. + /* +* Get list of valid libpq options. It contains default values too, but we +* don't care the values. Obtained list is allocated with malloc, so we +* must free it before leaving this function. +*/ + options = PQconndefaults(); + if (options == NULL) + ereport(ERROR, + (errcode(ERRCODE_FDW_OUT_OF_MEMORY), +errmsg(out of memory), +errdetail(could not get libpq default connection options))); I doubt whether PQconndefaults needs to be invoked for each validator calls. At least, supported option list of libpq should be never changed during run-time. So, I think PQconndefaults() should be called only once at first invocation, then option list can be stored in static variables or somewhere. As source code comments says, it is allocated with malloc, thus, we don't worry about unintentional release. :-) I don't think other part of this patch is arguable. Thanks, 2012/9/13 Shigeru HANADA shigeru.han...@gmail.com: Kaigai-san, (2012/09/13 16:56), Kohei KaiGai wrote: What about your plan to upstream contrib/pgsql_fdw module on the upcoming commit-fest? I will post pgsql_fdw patch (though it will be renamed to postgresql_fdw) in opening CF (2012 Sep), as soon as I resolve a bug in ANALYZE support, maybe on tomorrow. :-) Even though I understand the point I noticed (miss-synchronization of sub- transaction block between local and remote side) is never easy problem to solve, it is worth to get the patch on the table of discussion. In my opinion, it is an idea to split-off the transaction control portion as a limitation of current version. For example, just raise an error when the foreign-table being referenced in sub-transaction block; with explicit description in the document. I agree not to support synchronize TX block between remote and local, at least in next CF (I mean keeping remote TX open until local COMMIT or ABORT). It would require 2PC and many issues to be solved, so I'd like to focus fundamental part first. OTOH, using foreign tables in sub-transaction seems essential to me. Anyway, let me pick up your patch for reviewing. And, I hope you to prepare contrib/pgsql_fdw patch based on this patch. Thanks for your volunteer :-) Regards, -- Shigeru HANADA -- KaiGai Kohei kai...@kaigai.gr.jp -- KaiGai Kohei kai...@kaigai.gr.jp dblink_fdw_validator.v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add FET to Default and Europe.txt
On 08.10.2012 18:26, Tom Lane wrote: The other thing that the abbreviation list files are doing for us is providing a user-configurable way to resolve conflicting abbreviations, for instance IST (the Indians and the Israelis both use this, but not to mean the same thing). This requirement isn't ever going to go away. Locale-specific abbreviation lists would be nice. While abbreviations are not unique globally, they have established meanings in particular countries and regions. For example, IST is not unique, but if you are in India (en_IN) and spell out IST, you probably mean Indian Standard time. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] static-if-inline
The ilist patch from Andres Freund introduces a cute trick for defining
maybe-inline functions, which works regardless of whether the compiler
supports inlining, and eliminates the need to write the code twice
(first in the header and also the .c file.) It's really quite a simple
thing, but the whole topic on inlining has generated a lot of debate.
So to remove that controversy out of said patch, here's a preliminary
one: get rid of duplicate definitions in sortsupport.c, list.c, mcxt.c
(which are currently defined as static inline in their respective
headers for compilers that support it, and as regular functions in the
.c files for those that don't).
What's being done in this patch is exactly what would be done in the
ilist code. So if there are problems with this, please speak up.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/src/backend/nodes/list.c
--- b/src/backend/nodes/list.c
***
*** 15,20
--- 15,23
*/
#include postgres.h
+ /* see pg_list.h */
+ #define PG_LIST_INCLUDE_DEFINITIONS
+
#include nodes/pg_list.h
***
*** 1224,1254 list_copy_tail(const List *oldlist, int nskip)
}
/*
- * pg_list.h defines inline versions of these functions if allowed by the
- * compiler; in which case the definitions below are skipped.
- */
- #ifndef USE_INLINE
-
- ListCell *
- list_head(const List *l)
- {
- return l ? l-head : NULL;
- }
-
- ListCell *
- list_tail(List *l)
- {
- return l ? l-tail : NULL;
- }
-
- int
- list_length(const List *l)
- {
- return l ? l-length : 0;
- }
- #endif /* ! USE_INLINE */
-
- /*
* Temporary compatibility functions
*
* In order to avoid warnings for these function definitions, we need
--- 1227,1232
*** a/src/backend/utils/mmgr/mcxt.c
--- b/src/backend/utils/mmgr/mcxt.c
***
*** 21,26
--- 21,29
#include postgres.h
+ /* see palloc.h */
+ #define MCXT_INCLUDE_DEFINITIONS
+
#include utils/memutils.h
***
*** 696,723 repalloc(void *pointer, Size size)
}
/*
- * MemoryContextSwitchTo
- * Returns the current context; installs the given context.
- *
- * palloc.h defines an inline version of this function if allowed by the
- * compiler; in which case the definition below is skipped.
- */
- #ifndef USE_INLINE
-
- MemoryContext
- MemoryContextSwitchTo(MemoryContext context)
- {
- MemoryContext old;
-
- AssertArg(MemoryContextIsValid(context));
-
- old = CurrentMemoryContext;
- CurrentMemoryContext = context;
- return old;
- }
- #endif /* ! USE_INLINE */
-
- /*
* MemoryContextStrdup
* Like strdup(), but allocate from the specified context
*/
--- 699,704
*** a/src/backend/utils/sort/sortsupport.c
--- b/src/backend/utils/sort/sortsupport.c
***
*** 15,20
--- 15,23
#include postgres.h
+ /* See sortsupport.h */
+ #define SORTSUPPORT_INCLUDE_DEFINITIONS
+
#include fmgr.h
#include utils/lsyscache.h
#include utils/sortsupport.h
***
*** 29,78 typedef struct
/*
- * sortsupport.h defines inline versions of these functions if allowed by the
- * compiler; in which case the definitions below are skipped.
- */
- #ifndef USE_INLINE
-
- /*
- * Apply a sort comparator function and return a 3-way comparison result.
- * This takes care of handling reverse-sort and NULLs-ordering properly.
- */
- int
- ApplySortComparator(Datum datum1, bool isNull1,
- Datum datum2, bool isNull2,
- SortSupport ssup)
- {
- int compare;
-
- if (isNull1)
- {
- if (isNull2)
- compare = 0; /* NULL = NULL */
- else if (ssup-ssup_nulls_first)
- compare = -1; /* NULL NOT_NULL */
- else
- compare = 1; /* NULL NOT_NULL */
- }
- else if (isNull2)
- {
- if (ssup-ssup_nulls_first)
- compare = 1; /* NOT_NULL NULL */
- else
- compare = -1; /* NOT_NULL NULL */
- }
- else
- {
- compare = (*ssup-comparator) (datum1, datum2, ssup);
- if (ssup-ssup_reverse)
- compare = -compare;
- }
-
- return compare;
- }
- #endif /* ! USE_INLINE */
-
- /*
* Shim function for calling an old-style comparator
*
* This is essentially an inlined version of FunctionCall2Coll(), except
--- 32,37
*** a/src/include/c.h
--- b/src/include/c.h
***
*** 744,749 typedef NameData *Name;
--- 744,764
#endif /* HAVE__BUILTIN_TYPES_COMPATIBLE_P */
+ /*
+ * Inlining support
+ *
+ * Allow modules to declare functions that are inlined if the compiler supports
+ * it. The functions must be defined in its header, protected by a special
+ * #ifdef that the .c file defines. If the compiler does not support inlines,
+ * the function definitions are pulled in by the .c file as regular (not
+ * inline) symbols; the prototype must also exist in the header in that case.
+ */
+ #ifdef USE_INLINE
+ #define STATIC_IF_INLINE static inline
+ #else
+ #define
Re: [HACKERS] Add FET to Default and Europe.txt
Heikki Linnakangas hlinnakan...@vmware.com writes: On 08.10.2012 18:26, Tom Lane wrote: The other thing that the abbreviation list files are doing for us is providing a user-configurable way to resolve conflicting abbreviations, for instance IST (the Indians and the Israelis both use this, but not to mean the same thing). This requirement isn't ever going to go away. Locale-specific abbreviation lists would be nice. Yeah, that's a good thought, although the lack of standardization of locale names seems to make it a bit hard to implement. My first idea was look for a tznames file matching the value of LC_TIME, and if found, concatenate its contents with 'Default'. But there are enough ways to spell en_IN to make that a bit problematic, and besides which a user in India might well be running with C locale anyway. Still, there might be a useful incremental usability gain there. We aren't ever going to get out of the need for user configurability though. For instance, if a user in India writes EST, is he thinking of the Australian or American meaning? It's plausible that either might be preferred, depending on who that user interacts with regularly. 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] static-if-inline
Alvaro Herrera alvhe...@2ndquadrant.com writes: What's being done in this patch is exactly what would be done in the ilist code. So if there are problems with this, please speak up. Stylistic gripe: in palloc.h you didn't follow the pattern of #ifndef USE_INLINE extern ... #endif before (rather than after) the function definitions. Nitpicky I know, but if we're trying to create a pattern, we should be consistent. The comment added in c.h has some grammar issues, too. Looks good otherwise. 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] Warnings from fwrite() in git head
Bruce Momjian br...@momjian.us writes: I am seeing the following warnings in git head from zic.c: zic.c:1505: warning: ignoring return value of âfwriteâ, declared with attribute warn_unused_result Yeah, this is probably a consequence of the _FORTIFY_SOURCE addition. I believe that ratchets up warning pickiness as well as other things. I'm inclined to think we should make zic.c bail out on write errors. Otherwise, make install could fail to notice an out-of-disk-space situation during install. The analogy you are drawing to ignoring errors while writing log messages is quite faulty. We're overdue for another round of syncing the tz code with upstream, too. Might be best to do that first, just in case Olson's crew already fixed this. 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] Warnings from fwrite() in git head
On Mon, Oct 8, 2012 at 8:58 AM, Bruce Momjian br...@momjian.us wrote: I am seeing the following warnings in git head from zic.c: zic.c:1505: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result ... Seems casting to void is not enough. Not sure why the error just started appearing for me last week. I don't see any recent gcc updates. This is gcc version 4.4.5-8 on Debian Squeeze. I think it was the addition of _FORTIFY_SOURCE I get the warnings in zic.c, but also I was also getting them in a different file. But now I only see zic ones in newest HEAD. Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Logging parameters values on bind error
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Hi,
We were struggling today with some java code binding values violating
some constraints in a prepared statement.
We don't own the code and couldn't make tests with it. So we tried to
find if PostgreSQL was able to log binded values when the BIND
operation fail and found that this is not possible in current release:
the error raised while planing the statement is not caught.
PFA a patch that catch any error while creating the query plan and add
parameters values to the error message if log_statement or
log_min_duration_statement would have logged it.
Comments?
- --
Jehan-Guillaume de Rorthais
http://www.dalibo.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlBy/cgACgkQXu9L1HbaT6Li5QCdEa9Zc4g302znpHmrwB9dnRBI
JSwAnR2Poil0QAP6b+TflM2ebDCPLq3G
=HU3H
-END PGP SIGNATURE-
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***
*** 1715,1726 exec_bind_message(StringInfo input_message)
pq_getmsgend(input_message);
! /*
! * Obtain a plan from the CachedPlanSource. Any cruft from (re)planning
! * will be generated in MessageContext. The plan refcount will be
! * assigned to the Portal, so it will be released at portal destruction.
! */
! cplan = GetCachedPlan(psrc, params, false);
/*
* Now we can define the portal.
--- 1715,1738
pq_getmsgend(input_message);
! PG_TRY();
! {
! /*
! * Obtain a plan from the CachedPlanSource. Any cruft from (re)planning
! * will be generated in MessageContext. The plan refcount will be
! * assigned to the Portal, so it will be released at portal destruction.
! */
!
! cplan = GetCachedPlan(psrc, params, false);
! }
! PG_CATCH();
! {
! if (check_log_statement(portal-stmts) || check_log_duration(msec_str, false) == 2)
! errdetail_params(params);
!
! PG_RE_THROW();
! }
! PG_END_TRY();
/*
* Now we can define the portal.
--
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 can't plpgsql return a row-expression?
2012/10/8 Tom Lane t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com writes: ERROR: RETURN must specify a record or row variable in function returning row Off the top of my head, I can't think of any reason for this restriction, nor can I find any code comments or anything in the commit log which explains the reason for it. Does anyone know why we don't allow this? Laziness, probably. Feel free to have at it. I wrote patch some years ago. It was rejected from performance reasons - because every row had to be casted to resulted type. Pavel regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] sortsupport for text
On 8 October 2012 16:30, Robert Haas robertmh...@gmail.com wrote: I don't have any plans to work on this further. I think all of the criticism that has been leveled at this patch is 100% bogus, and I greatly dislike the changes that have been proposed. That may not be fair, but it's how I feel, and in light of that feeling it does not make sense for me to pursue this. Sorry. If it was the case that you were only 50% of the way to getting something committable, I guess I'd understand; this is, after all, a volunteer effort, and you are of course free to pursue or not pursue whatever you like. It's more like 95% though, so I have a hard time understanding this attitude. The only criticism that I imagine that you could be referring to is my criticism (which was seconded by Tom) concerning the approach to sizing a buffer that is used as state between successive calls to the sortsupport routine. This was a fairly trivial point even within the context of this patch. It was regrettable that it got so out of hand, but that certainly wasn't my intention. I must say that I find this disheartening as a reviewer, particularly since it comes from someone who reviews a lot of patches (including most of my own), and has often complained about the difficulties that reviewers face. Is it really so hard for you to accept a criticism from me? I didn't expect you to fully accept my criticism; I only expected you to take it into consideration, and possibly let it go for the sake of progress. For what it's worth, I have high regard for your work generally. In all sincerity, this appears to me to be a slight, and I find it upsetting, both on a personal level, and because it creates a concern about being able to work with you in the future, which is certainly something that I've benefited from in the past, and hope to continue to benefit from. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Logging parameters values on bind error
Jehan-Guillaume de Rorthais j...@dalibo.com writes: PFA a patch that catch any error while creating the query plan and add parameters values to the error message if log_statement or log_min_duration_statement would have logged it. If that works, it's only by accident --- you're not supposed to add more info to an error object after the error has been thrown. What's worse, if the original error had a DETAIL message, you're overwriting it. The right way to do this type of thing, which is also cheaper than PG_TRY on most platforms, is to set up an ErrorContextCallback stack entry to call a function that adds extra information as a CONTEXT line. There are lots of examples in the code base. However ... I'm unconvinced that this can work safely at all. Note the check in errdetail_params that causes it to not do anything in an aborted transaction. Once you have caught an error, the current transaction really has to be considered aborted, even if xact.c hasn't been told about it yet. So this patch is cheating with both hands, and it will break given the right combination of error and parameter datatypes. A less dangerous approach would be to only attempt to print parameter values that were supplied in text format. 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] why can't plpgsql return a row-expression?
Pavel Stehule pavel.steh...@gmail.com writes: 2012/10/8 Tom Lane t...@sss.pgh.pa.us: Laziness, probably. Feel free to have at it. I wrote patch some years ago. It was rejected from performance reasons - because every row had to be casted to resulted type. I don't recall that patch in any detail, but it's not apparent to me that an extra cast step *must* be required to implement this. In the cases that are supported now, surely we could notice that no additional work is required. It's also worth commenting that if we were to switch the storage of composite-type plpgsql variables to HeapTuple, as has been suggested here for other reasons, the performance tradeoffs in this area would likely change completely anyway. 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] Bad Data back Door
On Oct 5, 2012, at 6:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, having said that, I think it has to be the reponsibility of the FDW to apply any required check ... which makes this a bug report against oracle_fdw, not the core system. (FWIW, contrib/file_fdw depends on the COPY code, which will check encoding.) FWIW, I believe that dblink does not check encoding. Best, 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] Bad Data back Door
David E. Wheeler da...@justatheory.com writes: On Oct 5, 2012, at 6:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, having said that, I think it has to be the reponsibility of the FDW to apply any required check ... which makes this a bug report against oracle_fdw, not the core system. (FWIW, contrib/file_fdw depends on the COPY code, which will check encoding.) FWIW, I believe that dblink does not check encoding. In dblink's case, that boils down to trusting a remote instance of Postgres to get this right, which doesn't seem totally unreasonable. But I wouldn't object to adding checks there if someone wanted to submit a patch. 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] Bad Data back Door
On Oct 8, 2012, at 11:13 AM, Tom Lane t...@sss.pgh.pa.us wrote: FWIW, I believe that dblink does not check encoding. In dblink's case, that boils down to trusting a remote instance of Postgres to get this right, which doesn't seem totally unreasonable. But I wouldn't object to adding checks there if someone wanted to submit a patch. Yeah, I found this because we had a dblink to another PostgreSQL server's table with data populated from oracle_fdw. I guess trusting is reasonable, though. I wonder about dbi-link, though… 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] Bad Data back Door
On 10/08/2012 02:13 PM, Tom Lane wrote: David E. Wheeler da...@justatheory.com writes: On Oct 5, 2012, at 6:12 PM, Tom Lane t...@sss.pgh.pa.us wrote: Now, having said that, I think it has to be the reponsibility of the FDW to apply any required check ... which makes this a bug report against oracle_fdw, not the core system. (FWIW, contrib/file_fdw depends on the COPY code, which will check encoding.) FWIW, I believe that dblink does not check encoding. In dblink's case, that boils down to trusting a remote instance of Postgres to get this right, which doesn't seem totally unreasonable. But I wouldn't object to adding checks there if someone wanted to submit a patch. It does do: PQsetClientEncoding(conn, GetDatabaseEncodingName()); I'd be mildly reluctant to do anything more except possibly as an option, unless it could be shown to have minimal performance impact. 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] [v9.3] Row-Level Security
On 8 October 2012 15:57, Kohei KaiGai kai...@kaigai.gr.jp wrote: The attached patch is a refreshed version towards the latest master branch, to fix up patch conflicts. Here is no other difference from the previous revision. Thanks, I had another look at this over the weekend and I found couple of additional problems (test cases attached): 1). It is possible to define a RLS qual that refers back to the table that it's defined on, in such a way that causes infinite recursion in the planner, giving ERROR: stack depth limit exceeded. I think it would be preferable to trap this and report a more meaningful error back to the user, along similar lines to a self-referencing view. 2). In other cases it is possible to define a RLS qual that refers to another table with a RLS qual in such a way that the second table's RLS qual is not checked, thus allowing a user to bypass the security check. 3). If a RLS qual refers to a view it errors, since the RLS quals are added after rule expansion, and so the view is not rewritten. To me this suggests that perhaps the expansion of RLS quals should be done in the rewriter. I've not thought that through in any detail, but ISTM that a RIR rule could add a table with a RLS qual, and a RLS qual could add a relation with a RIR rule that needs expanding, and so the 2 need to be processed together. This could also make use of the existing recursion-checking code in the rewriter. Regards, Dean test.sql Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] getopt() and strdup()
A while ago I noticed that in some places we strdup/pg_strdup() optarg strings from getopt(), and in some places we don't. If we needed the strdup(), the missing cases should generate errors. If we don't need them, the strdup() is unnecessary, and research confirms they are unnecessary. Should we remove the extra strdup/pg_strdup() calls, for consistency. I think we might have had old platforms that required it, but none are still supported today. -- Bruce Momjian br...@momjian.ushttp://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] Improving psql \ds
On 09/10/12 03:53, Robert Haas wrote:
On Mon, Oct 8, 2012 at 3:49 AM, Julien Tachoires jul...@gmail.com wrote:
2012/10/8 Tom Lane t...@sss.pgh.pa.us:
Julien Tachoires jul...@gmail.com writes:
About \ds behaviour, I think to add 2 columns :
- 'LastValue'
- 'Increment'
That would make the command a great deal slower, since it would have to
access each sequence to get that info. I don't object to adding such
columns to \ds+, but I don't think it's a good idea to put them in the
basic command.
Ok.
The other problem you're going to have here is that there is in fact no
such command as \ds (nor \ds+); rather, it's a special case of
\dtsvi. As such, putting any relkind-specific info into the result is
problematic. I think you're going to have to invent a different command
if you want there to be sequence-specific columns.
Yes, that's why I plan to create a new function listSequences() and
call it from src/bin/psql/command.c :
+ /* \d* commands */
+ else if (cmd[0] == 'd')
+ {
...
+ case 's':
+ success = listSequences(pattern, show_verbose);
+ break;
What happens if the user does this:
\dis
Hmm...
'According' to Terry Prachett,that is probably short hand for telling
the backend to go to Hell = 'dis' is apparently another name for 'Hell'!
(In his book 'Truth', commander Vimes has a personal dis-organiser - as
the 'dis-organiser' is run by a demon from Hell.)
Now back to more appropriate comments...
--
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] Add FET to Default and Europe.txt
On Mon, Oct 8, 2012 at 11:54 AM, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: On 08.10.2012 18:26, Tom Lane wrote: The other thing that the abbreviation list files are doing for us is providing a user-configurable way to resolve conflicting abbreviations, for instance IST (the Indians and the Israelis both use this, but not to mean the same thing). This requirement isn't ever going to go away. Locale-specific abbreviation lists would be nice. Yeah, that's a good thought, although the lack of standardization of locale names seems to make it a bit hard to implement. My first idea was look for a tznames file matching the value of LC_TIME, and if found, concatenate its contents with 'Default'. But there are enough ways to spell en_IN to make that a bit problematic, and besides which a user in India might well be running with C locale anyway. Still, there might be a useful incremental usability gain there. We aren't ever going to get out of the need for user configurability though. For instance, if a user in India writes EST, is he thinking of the Australian or American meaning? It's plausible that either might be preferred, depending on who that user interacts with regularly. That sounds pretty cool, but coolness isn't the right way to evaluate whether this is good or not. If we introduce cases where peoples' expectations are liable to be disappointed (e.g. - they get the wrong EST, and report a bug on that), then we lose. We have, in effect, been treating the handling of time zones in a manner where we're imagining there's general agreement on their interpretation. We presently get bug reports (and are losing/getting it not as right as would be nice) in cases where we leave TZ symbols out, where they could have been included. The scenario where we could unambiguously include time zones is where the symbols are unique. If we were to include *all* uniquely-named symbols, that would minimize the number of complaints about missing zones, whilst evading the cases where the symbols are non-unique. That might be worth considering, though it'll certainly attract complaints in that some odd-ball zones would be included whilst well-known ones wouldn't. I would tend to think that local variations (e.g. - having a list for LC_TIME=en_IN) heads us into a morass of complexity. As you suggest, two different people using en_IN might have different preferences for what EST should mean. That being said, if we had a way to configure preferred localizations, people could set up their own set of associations. You want your own morass? You can build it yourself... -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- 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] [v9.3] Row-Level Security
2012/10/8 Dean Rasheed dean.a.rash...@gmail.com: On 8 October 2012 15:57, Kohei KaiGai kai...@kaigai.gr.jp wrote: The attached patch is a refreshed version towards the latest master branch, to fix up patch conflicts. Here is no other difference from the previous revision. Thanks, I had another look at this over the weekend and I found couple of additional problems (test cases attached): 1). It is possible to define a RLS qual that refers back to the table that it's defined on, in such a way that causes infinite recursion in the planner, giving ERROR: stack depth limit exceeded. I think it would be preferable to trap this and report a more meaningful error back to the user, along similar lines to a self-referencing view. 2). In other cases it is possible to define a RLS qual that refers to another table with a RLS qual in such a way that the second table's RLS qual is not checked, thus allowing a user to bypass the security check. 3). If a RLS qual refers to a view it errors, since the RLS quals are added after rule expansion, and so the view is not rewritten. To me this suggests that perhaps the expansion of RLS quals should be done in the rewriter. I've not thought that through in any detail, but ISTM that a RIR rule could add a table with a RLS qual, and a RLS qual could add a relation with a RIR rule that needs expanding, and so the 2 need to be processed together. This could also make use of the existing recursion-checking code in the rewriter. Thanks for your checks. I missed some cases that you suggested. The reason why we need to put RLS expansion at planner stage is requirement towards plan cache invalidation. Due to special case handling for superuser, plan cache has to be invalidated if user-id to run executor was switched since planner stage. The planner shall be invoked again, but not rewritter, on its invalidation. Probably, it make sense to invoke rewriter's logic to solve RLS policy from planner stage (that allows plan-cache invalidation). Let me investigate the code of rewriter. Best regards, -- 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] getopt() and strdup()
On 10/08/2012 02:40 PM, Bruce Momjian wrote: A while ago I noticed that in some places we strdup/pg_strdup() optarg strings from getopt(), and in some places we don't. If we needed the strdup(), the missing cases should generate errors. If we don't need them, the strdup() is unnecessary, and research confirms they are unnecessary. Should we remove the extra strdup/pg_strdup() calls, for consistency. I think we might have had old platforms that required it, but none are still supported today. ISTR there was a requirement at least on some platforms to use strdup/pg_strdup if the individual argument could be deformed, or the argument vector could be deformed. But maybe my memory is astray. I'm all for consistency, but only if we're darn sure it's not going to break things. 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] Add FET to Default and Europe.txt
Christopher Browne cbbro...@gmail.com writes: The scenario where we could unambiguously include time zones is where the symbols are unique. If we were to include *all* uniquely-named symbols, that would minimize the number of complaints about missing zones, whilst evading the cases where the symbols are non-unique. That might be worth considering, though it'll certainly attract complaints in that some odd-ball zones would be included whilst well-known ones wouldn't. That sounds good in the abstract ... however, consider that two of the ambiguous abbreviations are EST and CST, which means that taking a hard line would piss off every American east of the Mississippi, likewise over half of Canada, not to mention some proportion of Australians. Can you say user revolt? Projects have been forked for less. We can't just refuse to deal with this ambiguity. We have to have some very-low-pain way to install settings that will please those large fractions of our user base. Moreover, if that very-low-pain way isn't the exact same way it's been done for the last half dozen releases, you'll already have managed to annoy those selfsame large fractions. You'd better have a good reason for changing it. 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] embedded list v3
Hi Peter,
On Monday, October 08, 2012 09:43:51 PM Peter Geoghegan wrote:
Pendantry: This should be in alphabetical order:
! OBJS = stringinfo.o ilist.o
Argh. Youve said that before. Somehow I reintroduced it...
I notice that the patch (my revision) produces a whole bunch of
warnings like this with Clang, though not GCC:
[peter@peterlaptop cache]$ clang -O2 -g -Wall -Wmissing-prototypes
-Wpointer-arith -Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -g -I../../../../src/include
-D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -I/usr/include/libxml2 -c -o
catcache.o catcache.c -MMD -MP -MF .deps/catcache.Po
clang: warning: argument unused during compilation:
'-fexcess-precision=standard'
catcache.c:457:21: warning: expression result unused [-Wunused-value]
CatCache *ccp = slist_container(CatCache, cc_next,
cache_iter.cur);
^~
../../../../src/include/lib/ilist.h:721:3: note: expanded from macro
'slist_container'
(AssertVariableIsOfTypeMacro(ptr, slist_node *),...
^
../../../../src/include/c.h:735:2: note: expanded from macro
'AssertVariableIsOfTypeMacro'
StaticAssertExpr(__builtin_types_compatible_p(__typeof__(varname),
typename), \
^
../../../../src/include/c.h:710:46: note: expanded from macro
'StaticAssertExpr' ({ StaticAssertStmt(condition, errmessage); true; })
^
../../../../src/include/c.h:185:15: note: expanded from macro 'true'
#define true((bool) 1)
^ ~
*** SNIP SNIP SNIP ***
catcache.c:1818:21: warning: expression result unused [-Wunused-value]
CatCache *ccp = slist_container(CatCache, cc_next,
iter.cur); ^~~~
../../../../src/include/lib/ilist.h:722:3: note: expanded from macro
'slist_container'
AssertVariableIsOfTypeMacro(((type*)NULL)-membername,
slist_node), \
^
../../../../src/include/c.h:735:2: note: expanded from macro
'AssertVariableIsOfTypeMacro'
StaticAssertExpr(__builtin_types_compatible_p(__typeof__(varname),
typename), \
^
../../../../src/include/c.h:710:46: note: expanded from macro
'StaticAssertExpr' ({ StaticAssertStmt(condition, errmessage); true; })
^
../../../../src/include/c.h:185:15: note: expanded from macro 'true'
#define true((bool) 1)
^ ~
28 warnings generated.
I suppose that this is something that's going to have to be addressed
sooner or later.
That looks like an annoying warning. Split of StaticAssertExpr into
StaticAssertExpr and StaticAssertExprBool?
This seems kind of arbitrary:
/* The socket number we are listening for connections on */
int PostPortNumber;
+
/* The directory names for Unix socket(s) */
char *Unix_socket_directories;
+
/* The TCP listen address(es) */
char *ListenAddresses;
Yep, no idea why I added the spaces.
This looks funny:
+ #endif /* defined(USE_INLINE) ||
+ *
defined(ILIST_DEFINE_FUNCTIONS) */
I tend to consider the 80-column thing a recommendation more than a
requirement.
Thats pgindent's doing. I couldn't convince it not to do that short of making
it a multiline comment with 's.
A further stylistic gripe is that this:
+ #define dlist_container(type, membername, ptr)
\
+ (AssertVariableIsOfTypeMacro(ptr, dlist_node *),
\
+ AssertVariableIsOfTypeMacro(((type *) NULL)-membername, dlist_node),
\ +((type *)((char *)(ptr) - offsetof(type, membername)))
\
+ )
Should probably look like this:
+ #define dlist_container(type, membername, ptr)
\
+ (AssertVariableIsOfTypeMacro(ptr, dlist_node *),
\
+ AssertVariableIsOfTypeMacro(((type *) NULL)-membername, dlist_node),
\ +((type *)((char *)(ptr) - offsetof(type, membername
Why? I find the former better readable.
This is a little unclear:
+ * dlist_foreach (iter, db-tables)
+ * {
+ * // inside an *_foreach the iterator's .cur field can be used to
access + * // the current element
This comment:
+ * Singly linked lists are *not* circularly linked (how could they be?)
in + * contrast to the doubly linked lists. As no pointer to the last
list element
Isn't quite accurate, if I've understood you correctly; it is surely
possible in principle to have a circular singly-linked list.
Its complete crap. One shouldn't write
Re: [HACKERS] getopt() and strdup()
Bruce Momjian br...@momjian.us writes: A while ago I noticed that in some places we strdup/pg_strdup() optarg strings from getopt(), and in some places we don't. If we needed the strdup(), the missing cases should generate errors. If we don't need them, the strdup() is unnecessary, and research confirms they are unnecessary. Should we remove the extra strdup/pg_strdup() calls, for consistency. What research? Given the number of different ways argv[] is handled on different platforms (cf ps_status.c), I am very unwilling to trust that it's safe to hang onto an argv string for long without strdup'ing it. I think we might have had old platforms that required it, but none are still supported today. And what's your grounds for stating that? All the alternatives in ps_status.c are still live code AFAICS. My feeling is it's more likely to be a good idea to be adding strdup's than removing them. 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] sortsupport for text
On Mon, Oct 8, 2012 at 12:26 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: If it was the case that you were only 50% of the way to getting something committable, I guess I'd understand; this is, after all, a volunteer effort, and you are of course free to pursue or not pursue whatever you like. It's more like 95% though, so I have a hard time understanding this attitude. The only criticism that I imagine that you could be referring to is my criticism (which was seconded by Tom) concerning the approach to sizing a buffer that is used as state between successive calls to the sortsupport routine. This was a fairly trivial point even within the context of this patch. It was regrettable that it got so out of hand, but that certainly wasn't my intention. I must say that I find this disheartening as a reviewer, particularly since it comes from someone who reviews a lot of patches (including most of my own), and has often complained about the difficulties that reviewers face. Is it really so hard for you to accept a criticism from me? I didn't expect you to fully accept my criticism; I only expected you to take it into consideration, and possibly let it go for the sake of progress. For what it's worth, I have high regard for your work generally. In all sincerity, this appears to me to be a slight, and I find it upsetting, both on a personal level, and because it creates a concern about being able to work with you in the future, which is certainly something that I've benefited from in the past, and hope to continue to benefit from. Hey, if me deciding I don't want to work on a patch any more is going to make you feel slighted, then you're out of luck. The archives are littered with people who have decided to stop working on things because the consensus position on list was different than the approach that they personally favored, and I have as much right to do that as anyone else. I think that you are perfectly entitled to feel disappointed that I don't like your suggestions, even as I am disappointed that the patch encountered so much resistance. But let's not turn it into a personal issue. I don't think that you're a bad person because you don't like my approach, and I hope you don't think I'm a bad person because I don't like yours. If you do, then that's just something we're both going to have to live with, because I am not going to make a policy of working on things because other people will be offended if I don't. As to what the substantive issue is, well, yeah, I don't like the memory allocation strategy that you and Tom are proposing. In the worst case it chews up a lot of extra memory, and in the best case there's no measurable performance benefit - or at least nobody done any work to demonstrate that there is one. I've already pointed out that, in fact, the strategy of doing power-of-two allocations is rarely followed for very large allocations, a point to which I believe no one has responded substantively. Now I don't think anyone else is required to respond to that point, but neither am I required to revise the patch into a form that I don't agree with. Equally, I will not presume to commit it in a form that you and Tom do not agree with. That would be asinine, and I try (not always successfully) not to be an ass. It is not as if anyone has phrased this as a maybe-we-should sort of argument; there have been quite definite arguments on both sides over apparently strongly-held positions. I had hoped that this was going to be a quick and non-controversial win, but 74 email messages later it has become clear that it will be neither of those things. I do not have a problem with accepting review feedback from you or from anyone else. There are many examples in the archives of cases where I have improved my code based on review feedback; indeed, the possibility of getting high-quality feedback from the very smart developers who inhabit this mailing list is for me a principle attraction of working on PostgreSQL, not to mention a major reason why PostgreSQL is as good a product as it is. However, that general truth does not oblige me to accept any particular piece of feedback as well-founded, and this is hardly the first suggestion that I have argued against in four years as a PostgreSQL developer, nor the first of my patches to land on the cutting-room floor. Nor do all of my suggestions for other people's patches get adopted either. -- 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] why repl_gram.h?
On Oct 6, 2012 3:00 AM, Peter Eisentraut pete...@gmx.net wrote: src/backend/replication/Makefile calls bison with -d to create a repl_gram.h header file, but that is not used anywhere. Is this an oversight? That's probably a copy/paste caused oversight. I don't recall any particular reason for it. /Magnus
[HACKERS] MemSetLoop ignoring the 'val' parameter
Hi,
#define MemSetLoop(start, val, len) \
do \
{ \
long * _start = (long *) (start); \
long * _stop = (long *) ((char *) _start + (Size) (len)); \
\
while (_start _stop) \
*_start++ = 0; \
} while (0)
The 'val' parameter is ignored.
Currently it doesn't matter because MemSetLoop is only used with a 0 parameter
and only so in mcxt.c but it looks like it should be fixed anyway.
Andres
--
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] MemSetLoop ignoring the 'val' parameter
On Monday, October 08, 2012 10:39:27 PM Andres Freund wrote: The 'val' parameter is ignored. Trivial patch attached. -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From a4056e1110961c64b56d61a88c0d472c58a80579 Mon Sep 17 00:00:00 2001 From: Andres Freund and...@anarazel.de Date: Mon, 8 Oct 2012 22:55:14 +0200 Subject: [PATCH] Fix MemSetLoop to not ignore the val parameter This typo didn't have any bad consequences as the only in-core callsite passes in 0 which is the value that was assigned instead of val anyway. --- src/include/c.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/c.h b/src/include/c.h index 925d961..8c3ca8c 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -689,7 +689,7 @@ typedef NameData *Name; long * _stop = (long *) ((char *) _start + (Size) (len)); \ \ while (_start _stop) \ - *_start++ = 0; \ + *_start++ = val; \ } while (0) -- 1.7.12.289.g0ce9864.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] Support for REINDEX CONCURRENTLY
On 10/5/12 9:57 PM, Michael Paquier wrote: In the current version of the patch, at the beginning of process a new index is created. It is a twin of the index it has to replace, meaning that it copies the dependencies of old index and creates twin entries of the old index even in pg_depend and pg_constraint also if necessary. So the old index and the new index have exactly the same data in catalog, they are completely decoupled, and you do not need to worry about the OID replacements and the visibility consequences. Yeah, what's the risk to renaming an index during concurrent access? The only thing I can think of is an old backend referring to the wrong index name in an elog. That's certainly not great, but could possibly be dealt with. Are there any other things that are directly tied to the name of an index (or of any object for that matter)? -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] sortsupport for text
On 8 October 2012 21:35, Robert Haas robertmh...@gmail.com wrote: Hey, if me deciding I don't want to work on a patch any more is going to make you feel slighted, then you're out of luck. The archives are littered with people who have decided to stop working on things because the consensus position on list was different than the approach that they personally favored, and I have as much right to do that as anyone else. Sure you do. However, I doubt very many of those who gave up did so over so trivial an issue as to how to grow a buffer somewhere, and those that did usually did not go on to become major contributors, and certainly not committers. The buffer thing is, as far as I know, the single solitary point of contention with this patch. We're talking about 2 lines of code. To give up now is just petulant. There is no other way of looking at it. It is not as if anyone has phrased this as a maybe-we-should sort of argument; there have been quite definite arguments on both sides over apparently strongly-held positions. I had hoped that this was going to be a quick and non-controversial win, but 74 email messages later it has become clear that it will be neither of those things. Many of those 74 emails concerned my completely unrelated digression into exploiting strxfrm(); we spent a ridiculously long time discussing how to size this buffer, but it still wasn't anything like 74 messages. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Placement of permissions checks for large object operations
While looking at the 64-bit large object patch, I happened to notice that the LO permissions checks that were added a release or two back seem to be done exceedingly inefficiently. To wit, they're done in lo_read() and lo_write(), which means that if the user say reads an 8MB large object with an 8K buffer size, we'll repeat the same permissions check 1000 times. This is particularly bizarre on the read side, where the code has gone to probably-unreasonable lengths to make dead certain that it will get the same answer each time. But even on the write side, it's not apparent to me that it's useful or sensible to allow a few writes and then stop allowing them if some other transaction commits an ACL change meanwhile. (There would be a race condition there anyway, since our transaction might examine the ACL just before somebody else changes it.) I thought about proposing that the permissions checks be done in lo_open instead. However, that would result in semantic changes --- for instance, it's legal and sometimes useful to supply INV_WRITE and not actually write, if you want up-to-date read results. So we can't really enforce permissions at open time based on the supplied flags. However, it wouldn't be hard to make the code apply the checks at most once per lo_open, by keeping flags in the LargeObjectDesc entries showing whether we've already checked read or write privilege on each descriptor. So the check would be made only during the first lo_read or lo_write call on a given descriptor. Comments? 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] MemSetLoop ignoring the 'val' parameter
Andres Freund and...@2ndquadrant.com writes: The 'val' parameter is ignored. This is not broken. Read the comments for MemSetTest. 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] MemSetLoop ignoring the 'val' parameter
On Tuesday, October 09, 2012 12:56:16 AM Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: The 'val' parameter is ignored. This is not broken. Read the comments for MemSetTest. Ah. I was surprised about that already. The comment says that val has to be constant though, not that it has to be zero. In my understanding 1 is constant as well. Also, why do we even pass in a 'val' parameter in that case? Feeling a little slow here... Andres -- 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] Support for REINDEX CONCURRENTLY
On 10/8/12 6:12 PM, Tom Lane wrote: Jim Nasby j...@nasby.net writes: Yeah, what's the risk to renaming an index during concurrent access? SnapshotNow searches for the pg_class row could get broken by *any* transactional update of that row, whether it's for a change of relname or some other field. A lot of these problems would go away if we rejiggered the definition of SnapshotNow to be more like MVCC. We have discussed that in the past, but IIRC it's not exactly a simple or risk-free change in itself. Still, maybe we should start thinking about doing that instead of trying to make REINDEX CONCURRENTLY safe given the existing infrastructure. Yeah, I was just trying to remember what other situations this has come up in. My recollection is that there's been a couple other cases where that would be useful. My recollection is also that such a change would be rather large... but it might be smaller than all the other work-arounds that are needed because we don't have that... -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Support for REINDEX CONCURRENTLY
On 10/8/12 5:08 PM, Andres Freund wrote: On Monday, October 08, 2012 11:57:46 PM Jim Nasby wrote: On 10/5/12 9:57 PM, Michael Paquier wrote: In the current version of the patch, at the beginning of process a new index is created. It is a twin of the index it has to replace, meaning that it copies the dependencies of old index and creates twin entries of the old index even in pg_depend and pg_constraint also if necessary. So the old index and the new index have exactly the same data in catalog, they are completely decoupled, and you do not need to worry about the OID replacements and the visibility consequences. Yeah, what's the risk to renaming an index during concurrent access? The only thing I can think of is an old backend referring to the wrong index name in an elog. That's certainly not great, but could possibly be dealt with. We cannot have two indexes with the same oid in the catalog, so the two different names will have to have different oids. Unfortunately the indexes oid is referred to by other tables (e.g. pg_constraint), so renaming the indexes while differering in the oid isn't really helpful :(... Hrm... the claim was made that everything relating to the index, including pg_depend and pg_contstraint, got duplicated. But I don't know how you could duplicate a constraint without also playing name games. Perhaps name games are being played there as well... Right now I don't see anything that would make switching oids easier than relfilenodes. Yeah... in order to make either of those schemes work I think there would need to non-trivial internal changes so that we weren't just passing around raw OIDs/filenodes. BTW, it occurs to me that this problem might be easier to deal with if we had support for accessing the catalog with the same snapshot as the main query was using... IIRC that's been discussed in the past for other issues. -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] PQping command line tool
On 10/4/12 11:34 AM, Greg Sabino Mullane wrote: I was wondering recently if there was any command line tool that utilized PQping() or PQpingParams(). I searched the code and couldn't find anything and was wondering if there was any interest to have something like this included? I wrote something for my purposes of performing a health check that also supports nagios style status output. It's probably convenient for scripting purposes as well. I'm not sure how useful this information would be. Most health checks (Nagios or otherwise) really only care if things are working all the up to point A or not, where point A is usually a simple query such as SELECT 1. Knowing various failure states as returned by PQping* does not seem to fit into such tools - any failure needs to be handled manually. For whatever reason our Nagios setup telnets to the port to see if it's open, which means we get a ton of messages in the log about authentication errors or some such. It'd be useful to us to have a utility that could cleanly validate the server was up and communicating, without having to actually login. -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Deparsing DDL command strings
On 10/5/12 11:15 AM, Tom Lane wrote: Also, we need to normalize that command string. Tools needing to look at it won't want to depend on random white spacing and other oddities. Instead, they'll get to depend on the oddities of parse transformations (ie, what's done in the raw grammar vs. what's done in parse_analyze), which is something we whack around regularly. Besides which, you've merely asserted this requirement without providing any evidence that it's important at all, let alone important enough to justify the kind of maintenance burden that you want to saddle us with. I definitely want to be able to parse DDL commands to be able to either enforce things or to drive other parts of the system based on what's changing. Without the ability to capture (and parse) DDL commands I'm stuck creating wrapper functions around anything I want to capture and then trying to ensure that everyone uses the wrappers and not the raw DDL commands. Event triggers that just spit out raw SQL give me the first part of this, but not the second part: I'm still stuck trying to parse raw SQL on my own. Having normalized SQL to parse should make that a bit easier, but ideally I'd like to be able to pull specific elements out of a command. I'd want to be able to do things like: IF command is ALTER TABLE THEN FOR EACH subcommand IF subcommand IS DROP COLUMN THEN do something that needs to know what column is being dropped ELSE IF subcommand IS ADD COLUMN THEN do something that needs to know the definition of the column being added I don't think every bit of that has to be dealt with by the event trigger code itself. For example, if you're adding a column to a table and the entries have already been made in the catalog, you could query to get the details of the column definition if you were given an OID into pg_attributes. Having said all that, an event system that spits back the raw SQL would certainly be better than nothing. But realize that people would still need to do parsing on it (ie: replication solutions will need to know what table just got ALTER'd). -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] MemSetLoop ignoring the 'val' parameter
Andres Freund and...@2ndquadrant.com writes: On Tuesday, October 09, 2012 12:56:16 AM Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: The 'val' parameter is ignored. This is not broken. Read the comments for MemSetTest. Ah. I was surprised about that already. The comment says that val has to be constant though, not that it has to be zero. In my understanding 1 is constant as well. Also, why do we even pass in a 'val' parameter in that case? Well, first off, the callers should not be aware of the detail that MemSetTest insists on a val of zero, so they have to pass val even though it's unused by the current implementation of MemSetLoop. The callers are responsible for not passing a volatile value there, but it's hard to dodge that problem given that we're dealing with macros; if the value changes on repeat evaluation we're screwed anyway. However, nonvolatile is not constant. For instance, it's perfectly fine to pass MemSetTest/Loop a variable for the val that is sometimes zero and sometimes not. If we changed the coding as you suggest, the compiler would probably generate less efficient code since it wouldn't realize (unless it was quite smart) that MemSetLoop is always filling with zeroes. 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] getopt() and strdup()
On Mon, Oct 8, 2012 at 04:33:29PM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: A while ago I noticed that in some places we strdup/pg_strdup() optarg strings from getopt(), and in some places we don't. If we needed the strdup(), the missing cases should generate errors. If we don't need them, the strdup() is unnecessary, and research confirms they are unnecessary. Should we remove the extra strdup/pg_strdup() calls, for consistency. What research? Given the number of different ways argv[] is handled on different platforms (cf ps_status.c), I am very unwilling to trust that it's safe to hang onto an argv string for long without strdup'ing it. I think we might have had old platforms that required it, but none are still supported today. And what's your grounds for stating that? All the alternatives in ps_status.c are still live code AFAICS. My feeling is it's more likely to be a good idea to be adding strdup's than removing them. Well, what we have now is either wrong or over-kill --- I don't know for sure which. -- Bruce Momjian br...@momjian.ushttp://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] Support for REINDEX CONCURRENTLY
On Tue, Oct 9, 2012 at 8:14 AM, Jim Nasby j...@nasby.net wrote: Hrm... the claim was made that everything relating to the index, including pg_depend and pg_contstraint, got duplicated. But I don't know how you could duplicate a constraint without also playing name games. Perhaps name games are being played there as well... Yes, it is what was originally intended. Please note the pg_constraint entry was not duplicated correctly in the first version of the patch because of a bug I already fixed. I will provide another version soon if necessary. Right now I don't see anything that would make switching oids easier than relfilenodes. Yeah... in order to make either of those schemes work I think there would need to non-trivial internal changes so that we weren't just passing around raw OIDs/filenodes. BTW, it occurs to me that this problem might be easier to deal with if we had support for accessing the catalog with the same snapshot as the main query was using... IIRC that's been discussed in the past for other issues. Yes, it would be better and helpful to have such a mechanism even for other operations. -- Michael Paquier http://michael.otacoo.com
Re: [HACKERS] Add FET to Default and Europe.txt
On Mon, Oct 8, 2012 at 4:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: We can't just refuse to deal with this ambiguity. We have to have some very-low-pain way to install settings that will please those large fractions of our user base. Moreover, if that very-low-pain way isn't the exact same way it's been done for the last half dozen releases, you'll already have managed to annoy those selfsame large fractions. You'd better have a good reason for changing it. As previously noted, there are two topics that are being discussed here: - how to allow users to configure their own timezone abbreviations and - how to keep the timezone abbreviations that we ship updated wrt zic changes Regarding configurability, we currently allow users (=administrators) to change their abbreviations to whatever they like to use. The Default file we provide has been taken from the set that used to be a list that we compiled in back in the 8.x days (and we had this egregious GUC australian_timezones that decided whether CST/EST was regarded to be US or Australian timezones - there was no such hack for any of the other ambiguities). These timezone abbreviations have developed over several decades, most of these decades without global communication in mind. They are full of inconsistencies and irregularities and IMHO any way to select a proper set for someone automatically is doomed to solve the problem only partially. Another funny ambiguity is that the EST timezone in Austalia could mean both Eastern Standard Time and Eastern Summer Time, having different offsets depending on what month of the year you're in... The fact that we don't hear more complaints about the sets actually suggests that most people are happy with our Default set. In fact, Marc could just add his FET timezone to his own installations and use it from there. His request to add it to our Default set however is perfectly valid and should be discussed. One thing that could be improved about configurability is the fact that if you're not an administrator of the database, i.e. if you don't have write access to where the timezones are defined, you're pretty much out of luck being able to define your own abbreviations. This is true in a shared hosting environment for example. Wrt updating the timezone abbreviation files, I guess what we need is a parser for the zic database, our own timezone files and a script that compares the two datasets and spits out any conflicts so that we can clean them up after manual inspection of the differences. I will see if I can come up with some scripts in this direction. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] TODO item: teach pg_dump about sparsely-stored large objects
The backend code for large objects goes to some lengths to be intelligent about sparsely-written blobs: if you seek out to the middle of nowhere and write a few bytes, you don't end up allocating space in pg_largeobject for all the byte positions you skipped over. However, pg_dump knows nothing of this. If you pg_dump a sparsely-stored large object, it will tediously transfer all those nonexistent zeros from server to client, and write them into the resulting archive. And then when you restore, the blob isn't sparse anymore ... those zeroes become non-virtual on the database side too. Admittedly, this is no different than what happens when you try to back up a sparsely-stored Unix file, at least with simpler backup tools. But it seems to me we should try a bit harder. There are a couple of stumbling blocks to making that happen: * How should pg_dump find out where there are holes? It would be easy if it were to look into pg_largeobject, but that would destroy the ability to use pg_dump as non-superuser. I think we'd really have to provide some API to read from a blob in a sparse-storage-aware manner. The first idea that comes to mind is some way to tell lo_read to stop reading when it hits a gap (instead of manufacturing zeroes) and then a new whence option for lo_lseek that tells it to seek to the next non-dummy data in the blob. * How do we get pg_dump to make use of the knowledge once it's got it? The current code in that area is a masterpiece of ugly unreadability; it's near impossible to tell what connects to what else, and there are assorted magic switches that completely change the behavior of major interface functions. I'd kind of want to rewrite the whole mess before trying to change its behavior. I'm also pretty certain that we'd need an archive format change, though we've certainly done those before so that isn't a fatal objection. I'm not planning to do anything about this myself, but if someone is looking for a project, here's one. 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] Visual Studio 2012 RC
One more thing -- I tried a build with NLS support and encountered this: src\backend\utils\adt\pg_locale.c(746): error C2039: 'lc_handle' : is not a member of 'threadlocaleinfostruct' [c:\cygwin\home\nm\src\pg\postgresql\postgres.vcxproj] That's in the function IsoLocaleName(), which digs around inside a locale_t. I suppose the (undocumented) content of that structure changed in this newer CRT (MSVCR110D). I apologize for the slow drip of problem reports; I just happen to be trying additional configurations with your patch as a foundation. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
