Re: [HACKERS] WIP: cross column correlation ...

2011-02-24 Thread Bruce Momjian
Josh Berkus wrote:
> On 2/23/11 7:10 AM, Robert Haas wrote:
> > IME, most bad query plans are caused by either incorrect
> > estimates of selectivity, or wrongheaded notions about what's likely
> > to be cached.  If we could find a way, automated or manual, of
> > providing the planner some better information about the facts of life
> > in those areas, I think we'd be way better off.  I'm open to ideas
> > about what the best way to do that is.
> 
> As previously discussed, I'm fine with approaches which involve
> modifying database objects.  These are auditable and centrally managable
> and aren't devastating to upgrades.
> 
> So thinks like the proposed "CREATE SELECTIVITY" would be OK in a way
> that decorating queries would not.
> 
> Similiarly, I would love to be able to set "cache %" on a per-relation
> basis, and override the whole dubious calculation involving
> random_page_cost for scans of that table.

We should just fine a way of checking what percentage of a table is
already in the shared buffers.  That doesn't help us with the kernel
cache, but it would be a good start and something that doesn't require
user tuning.

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

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

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


Re: [HACKERS] Sync Rep v17

2011-02-24 Thread Jaime Casanova
On Mon, Feb 21, 2011 at 4:06 AM, Fujii Masao  wrote:
>
> PREPARE TRANSACTION and ROLLBACK PREPARED should wait for
> replication as well as COMMIT PREPARED?
>

maybe ROLLBACK PREPARED but i'm not sure... i'm pretty sure we don't
need to wait for PREPARE TRANSACTION, but i could be wrong

> What if fast shutdown is requested while RecordTransactionCommit
> is waiting in SyncRepWaitForLSN? ISTM fast shutdown cannot complete
> until replication has been successfully done (i.e., until at least one
> synchronous standby has connected to the master especially if
> allow_standalone_primary is disabled). Is this OK?
>

i guess that is debatable, IMHO if there is a synch standby then wait
(because the user request durability of the data),
if there isn't a synch rep wait until the timeout (even if
allow_standalone_primary is disabled) because probably this is
a miss configuration or an special case i'm trying to handle (network
broke, data center of standbies explode, etc).

> We should emit WARNING when the synchronous standby with
> wal_receiver_status_interval = 0 connects to the master. Because,
> in that case, a transaction unexpectedly would wait for replication
> infinitely.
>

actually i think we should reject such standby as a synch standby, and
look for another one in the synchronous_standby_names list

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

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


Re: [HACKERS] Sync Rep v17

2011-02-24 Thread Daniel Farina
With some more fooling around, I have also managed to get this elog(WARNING)

if (proc->lwWaitLink == NULL)
elog(WARNING, "could not locate ourselves on 
wait queue");

-- 
fdr

-- 
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] Sync Rep v17

2011-02-24 Thread Daniel Farina
On Tue, Feb 22, 2011 at 11:43 AM, Jaime Casanova  wrote:
> On Sat, Feb 19, 2011 at 11:26 PM, Robert Haas  wrote:
>>
>> DEBUG:  write 0/3027BC8 flush 0/3014690 apply 0/3014690
>> DEBUG:  released 0 procs up to 0/3014690
>> DEBUG:  write 0/3027BC8 flush 0/3027BC8 apply 0/3014690
>> DEBUG:  released 2 procs up to 0/3027BC8
>> WARNING:  could not locate ourselves on wait queue
>> server closed the connection unexpectedly
>>        This probably means the server terminated abnormally
>>        before or while processing the request.
>> The connection to the server was lost. Attempting reset: DEBUG:
>
> you can make this happen more easily, i just run "pgbench -n -c10 -j10
> test" and qot that warning and sometimes a segmentation fault and
> sometimes a failed assertion

I have also reproduced this. Notably, things seem fine as long as
pgbench is confined to one backend, but as soon as two are used (-c 2)
by the feature I can get segfaults.

In the UI department, I am finding it somewhat difficult to affirm
that I am, in fact, synchronously replicating anything in the HA
scenario (where I never want to block. However, by enjoying the patch
at DEBUG3 and running what I think to be syncrepped and non-syncrepped
runs, I believe that I am not committing user error (seeing syncrep
waiting vs. lack thereof).  This is in part hard to confirm because
the single-backend performance (if DEBUG3 is to be believed, I will
write a real torture test later) of syncrep is actually very good, I
was expecting a more perceptible performance dropoff. But then again,
I imagine the real kicker will happen when we can run concurrent
backends. Also, Amazon EBS doesn't have the fastest disks, and within
an availability zone network latency is awfully low.

I can't quite explain what I was seeing before w.r.t.  memory usage,
and more pressingly, a very slow recover. I turned off hot standby and
was messing around and, before I knew it, the server was caught up. I
do not know if that was just coincidence (probably) or overhead
imposed by HS. The very high RES number was linux fooling me, as most
of it was SHR and in SHMEM.

-- 
fdr

-- 
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] Fwd: psql include file using relative path

2011-02-24 Thread Robert Haas
On Thu, Feb 24, 2011 at 6:21 PM, Gurjeet Singh  wrote:
>     psql has the ability to execute commands from a file, but if one wishes
> to develop and provide a modularized set of sql files, then psql is not very
> helpful because the \i command can open file paths either if they are
> absolute paths or if they are palced correctly relative to psql's current
> working directory.
>
> Attached patch adds a new meta-command to psql, '\ir' that allows the user
> to process files relative to currently processing file.

Please add this patch to the currently open CommitFest at:

https://commitfest.postgresql.org/action/commitfest_view/open

-- 
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] Allow pg_archivecleanup to ignore extensions

2011-02-24 Thread Robert Haas
On Tue, Feb 8, 2011 at 2:57 AM, Greg Smith  wrote:
> One bit of feedback I keep getting from people who archive their WAL files
> is that the fairly new pg_archivecleanup utility doesn't handle the case
> where those archives are compressed.  As the sort of users who are concerned
> about compression are also often ones with giant archives they struggle to
> cleanup, they would certainly appreciate having a bundled utility to take
> care of that.

Please add this patch to the currently open CommitFest at

https://commitfest.postgresql.org/action/commitfest_view/open

-- 
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] WIP: collect frequency statistics for arrays

2011-02-24 Thread Robert Haas
On Wed, Feb 23, 2011 at 10:00 AM, Alexander Korotkov
 wrote:
> WIP patch of statistics collection for arrays is attached.

Please add this patch to the currently open CommitFest at

https://commitfest.postgresql.org/action/commitfest_view/open

-- 
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] wCTE behaviour

2011-02-24 Thread Marko Tiikkaja

On 2011-02-25 1:36 AM, Tom Lane wrote:

Marko Tiikkaja  writes:

I fixed an issue with the portal logic, and now we use
PORTAL_ONE_RETURNING for wCTE queries, even if the main query is not a
DML or does not have RETURNING.  This also means that we materialize the
results of the main query sometimes unnecessarily, but that doesn't look
like an easy thing to fix.  PORTAL_ONE_RETURNING as a name is also a bit
misleading now, so maybe that needs changing..


Why is it necessary to hack the portal logic at all?  The patch seems to
work for me without that.  (I've fixed quite a few bugs though, so maybe
what this is really doing is masking a problem elsewhere.)


Without hacking it broke when PQdescribePrepared was called on a 
prepared query like:


WITH t AS (DELETE FROM foo)
SELECT 1;

Not sure if that's an actual problem, but it seemed like something worht 
fixing.



Also, why are we forbidding wCTEs in cursors?  Given the current
definitions, that case seems to work fine too: the wCTEs will be
executed as soon as you fetch something from the cursor.  Are you
just worried about not allowing a case that might be hard to support
later?


Honestly, I have no idea.  It might be a leftover from the previous 
design.  If it looks like it's easy to support, then go for it.



Regards,
Marko Tiikkaja

--
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] Named restore points

2011-02-24 Thread Robert Haas
On Thu, Feb 24, 2011 at 10:28 AM, Euler Taveira de Oliveira
 wrote:
> The following patch implements the Thom's suggestions.
>
> [1] http://archives.postgresql.org/message-id/4d48209c.7050...@timbira.com

Committed with some additional wordsmithing.

-- 
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] wCTE behaviour

2011-02-24 Thread Tom Lane
Marko Tiikkaja  writes:
> I fixed an issue with the portal logic, and now we use 
> PORTAL_ONE_RETURNING for wCTE queries, even if the main query is not a 
> DML or does not have RETURNING.  This also means that we materialize the 
> results of the main query sometimes unnecessarily, but that doesn't look 
> like an easy thing to fix.  PORTAL_ONE_RETURNING as a name is also a bit 
> misleading now, so maybe that needs changing..

Why is it necessary to hack the portal logic at all?  The patch seems to
work for me without that.  (I've fixed quite a few bugs though, so maybe
what this is really doing is masking a problem elsewhere.)

Also, why are we forbidding wCTEs in cursors?  Given the current
definitions, that case seems to work fine too: the wCTEs will be
executed as soon as you fetch something from the cursor.  Are you
just worried about not allowing a case that might be hard to support
later?

regards, tom lane

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


[HACKERS] Fwd: psql include file using relative path

2011-02-24 Thread Gurjeet Singh
psql has the ability to execute commands from a file, but if one wishes
to develop and provide a modularized set of sql files, then psql is not very
helpful because the \i command can open file paths either if they are
absolute paths or if they are palced correctly relative to psql's current
working directory.

Attached patch adds a new meta-command to psql, '\ir' that allows the user
to process files relative to currently processing file.

Also attached is a sample use case ir_sample. One can extract it _anywhere_
on the filesystem and invoke psql with the path to main.sql and the rest of
the files will be automatically included from that location.

Sample session:

[/tmp]$ psql -f ~/dev/ir_sample/main.sql
processing main.sql
BEGIN
processing subdir1/1.sql
processing subdir1/2.sql
processing subdir2/1.sql
processing subdir2/subdir2.1/1.sql
processing subdir2/2.sql
processing subdir2/3.sql
COMMIT

And here's what the sample's directory structure and files look like:

[ir_sample]$ find ./ -name "*.sql" | while read f; do echo === $f ; cat
$f; done
=== ./main.sql 
\echo processing main.sql
BEGIN;
\ir subdir1/1.sql
\ir subdir1/2.sql
\ir subdir2/1.sql
\ir subdir2/2.sql
\ir subdir2/3.sql
COMMIT;
=== ./subdir1/1.sql 
\echo processing subdir1/1.sql
=== ./subdir1/2.sql 
\echo processing subdir1/2.sql
=== ./subdir2/subdir2.1/1.sql 
\echo processing subdir2/subdir2.1/1.sql
=== ./subdir2/1.sql 
\echo processing subdir2/1.sql
\ir subdir2.1/1.sql
=== ./subdir2/2.sql 
\echo processing subdir2/2.sql
=== ./subdir2/3.sql 
\echo processing subdir2/3.sql

Regards,
-- 
Gurjeet Singh
EnterpriseDB Corporation 
The Enterprise PostgreSQL  Company
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d7cdcf6..bc3949e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -785,7 +785,8 @@ exec_command(const char *cmd,
 
 
 	/* \i is include file */
-	else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0)
+	/* \ir is include file relative to currently processed file */
+	else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0 || strcmp(cmd, "ir") == 0)
 	{
 		char	   *fname = psql_scan_slash_option(scan_state,
    OT_NORMAL, NULL, true);
@@ -798,7 +799,7 @@ exec_command(const char *cmd,
 		else
 		{
 			expand_tilde(&fname);
-			success = (process_file(fname, false) == EXIT_SUCCESS);
+			success = (process_file(fname, false, (cmd[1] == 'r')) == EXIT_SUCCESS);
 			free(fname);
 		}
 	}
@@ -1971,14 +1972,18 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
  * Read commands from filename and then them to the main processing loop
  * Handler for \i, but can be used for other things as well.  Returns
  * MainLoop() error code.
+ *
+ * If use_relative_path is true and filename is not an absolute path, then open
+ * the file from where the currently processed file (if any) is located.
  */
 int
-process_file(char *filename, bool single_txn)
+process_file(char *filename, bool single_txn, bool use_relative_path)
 {
 	FILE	   *fd;
 	int			result;
 	char	   *oldfilename;
 	PGresult   *res;
+	char	   *last_slash = NULL;
 
 	if (!filename)
 		return EXIT_FAILURE;
@@ -1986,6 +1991,29 @@ process_file(char *filename, bool single_txn)
 	if (strcmp(filename, "-") != 0)
 	{
 		canonicalize_path(filename);
+
+		if (use_relative_path && pset.inputfile)
+		{
+			/* find the / that splits the file from its path */
+			last_slash = strrchr(pset.inputfile, '/');
+
+			if (last_slash && !is_absolute_path(filename))
+			{
+size_t dir_len = (last_slash - pset.inputfile) + 1;
+size_t file_len = strlen(filename);
+
+char *relative_file = pg_malloc(dir_len + 1 + file_len + 1);
+
+relative_file[0] = '\0';
+strncat(relative_file, pset.inputfile, dir_len);
+strcat(relative_file, filename);
+
+canonicalize_path(relative_file);
+
+filename = relative_file;
+			}
+		}
+
 		fd = fopen(filename, PG_BINARY_R);
 	}
 	else
diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index 852d645..9d0c31c 100644
--- a/src/bin/psql/command.h
+++ b/src/bin/psql/command.h
@@ -27,7 +27,7 @@ typedef enum _backslashResult
 extern backslashResult HandleSlashCmds(PsqlScanState scan_state,
 PQExpBuffer query_buf);
 
-extern int	process_file(char *filename, bool single_txn);
+extern int	process_file(char *filename, bool single_txn, bool use_relative_path);
 
 extern bool do_pset(const char *param,
 		const char *value,
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index ac5edca..d459934 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -184,6 +184,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\copy ...  perform SQL COPY with data stream to the client host\n"));
 	fprintf(output, _("  \\echo [STRING] write string to standard output\n"));
 	fprintf(output, _("  \\i FILEexecute commands from file\n"));
+	fprintf(output, _

Re: [HACKERS] WIP: cross column correlation ...

2011-02-24 Thread Josh Berkus
On 2/23/11 7:10 AM, Robert Haas wrote:
> IME, most bad query plans are caused by either incorrect
> estimates of selectivity, or wrongheaded notions about what's likely
> to be cached.  If we could find a way, automated or manual, of
> providing the planner some better information about the facts of life
> in those areas, I think we'd be way better off.  I'm open to ideas
> about what the best way to do that is.

As previously discussed, I'm fine with approaches which involve
modifying database objects.  These are auditable and centrally managable
and aren't devastating to upgrades.

So thinks like the proposed "CREATE SELECTIVITY" would be OK in a way
that decorating queries would not.

Similiarly, I would love to be able to set "cache %" on a per-relation
basis, and override the whole dubious calculation involving
random_page_cost for scans of that table.

The great thing about object decorations is that we could then collect
data on which ones worked and which didn't through the performance list
and then use those to improve the query planner.  I doubt such would
work with query decorations.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

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


Re: [HACKERS] Binary in/out for aclitem

2011-02-24 Thread Radosław Smogura
Tom Lane  Wednesday 23 February 2011 22:30:04
> =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
> > Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
> > mask, as well definition is more general then def of PGSQL. In any way it
> > require that rights mades bit array.
> 
> You're going in quite the wrong direction here.  The consensus as I
> understood it was that we should just use the text representation in
> binary mode too, rather than inventing a separate representation that's
> going to put a whole new set of constraints on what can happen to the
> internal representation.  The proposal you have here has no redeeming
> social value whatever, because nobody cares about the I/O efficiency
> for aclitem (and even if anyone did, you've made no case that this would
> actually be more efficient to use on the client side).
> 
>   regards, tom lane

Look at it. Pass call to in/out.

Regards,
Radek
diff --git a/.gitignore b/.gitignore
index 1be11e8..0d594f9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,5 @@ objfiles.txt
 /GNUmakefile
 /config.log
 /config.status
+/nbproject/private/
+/nbproject
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 691ba3b..fa151cd 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -33,6 +33,7 @@
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/syscache.h"
+#include "libpq/pqformat.h"
 
 
 typedef struct
@@ -77,7 +78,8 @@ static const char *getid(const char *s, char *n);
 static void putid(char *p, const char *s);
 static Acl *allocacl(int n);
 static void check_acl(const Acl *acl);
-static const char *aclparse(const char *s, AclItem *aip);
+static const char *aclparse(const char *s, AclItem *aip, bool binary);
+static Datum aclitem_common_in_recv(const char* s, bool binary);
 static bool aclitem_match(const AclItem *a1, const AclItem *a2);
 static int	aclitemComparator(const void *arg1, const void *arg2);
 static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
@@ -222,6 +224,8 @@ putid(char *p, const char *s)
  *
  *		This routine is called by the parser as well as aclitemin(), hence
  *		the added generality.
+ * 
+ * @param binary if we parse for binary mode or text mode
  *
  * RETURNS:
  *		the string position in 's' immediately following the ACL
@@ -230,7 +234,7 @@ putid(char *p, const char *s)
  *		  UID/GID, id type identifier and mode type values.
  */
 static const char *
-aclparse(const char *s, AclItem *aip)
+aclparse(const char *s, AclItem *aip, bool binary)
 {
 	AclMode		privs,
 goption,
@@ -249,20 +253,20 @@ aclparse(const char *s, AclItem *aip)
 		/* we just read a keyword, not a name */
 		if (strcmp(name, "group") != 0 && strcmp(name, "user") != 0)
 			ereport(ERROR,
-	(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+	(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)),
 	 errmsg("unrecognized key word: \"%s\"", name),
 	 errhint("ACL key word must be \"group\" or \"user\".")));
 		s = getid(s, name);		/* move s to the name beyond the keyword */
 		if (name[0] == '\0')
 			ereport(ERROR,
-	(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+	(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)),
 	 errmsg("missing name"),
 	 errhint("A name must follow the \"group\" or \"user\" key word.")));
 	}
 
 	if (*s != '=')
 		ereport(ERROR,
-(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)),
  errmsg("missing \"=\" sign")));
 
 	privs = goption = ACL_NO_RIGHTS;
@@ -315,7 +319,7 @@ aclparse(const char *s, AclItem *aip)
 break;
 			default:
 ereport(ERROR,
-		(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+		(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)),
 	  errmsg("invalid mode character: must be one of \"%s\"",
 			 ACL_ALL_RIGHTS_STR)));
 		}
@@ -337,7 +341,7 @@ aclparse(const char *s, AclItem *aip)
 		s = getid(s + 1, name2);
 		if (name2[0] == '\0')
 			ereport(ERROR,
-	(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+	(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)),
 	 errmsg("a name must follow the \"/\" sign")));
 		aip->ai_grantor = get_role_oid(name2, false);
 	}
@@ -548,6 +552,22 @@ check_acl(const Acl *acl)
  errmsg("ACL arrays must not contain null values")));
 }
 
+static
+Datum aclitem_common_in_recv(const char* s, bool binary)
+{
+AclItem*aip;
+
+aip = (AclItem *) palloc(sizeof(AclItem));
+s = aclparse(s, aip, binary);
+while (isspace((unsigned char) *s))
+++s;
+if (*s)
+ereport(ERROR,
+(errcode((binary ? ERRCODE_INVALID_BINARY_REPRESENTATION : ERRCODE_INVALID_TEXT_REPRESENTATION)

Re: [HACKERS] wCTE: about the name of the feature

2011-02-24 Thread David E. Wheeler
On Feb 24, 2011, at 10:43 AM, Robert Haas wrote:

>> The best idea I have at the moment is to spell out "data modifying
>> command" (or "statement") rather than relying on the acronym.
>> In the code, we could change hasDmlWith to hasModifyingWith, for
>> example.  The error messages could read like
>>data-modifying statement in WITH is not allowed in a view
>> 
>> Comments?
> 
> Great idea.  I had the same complaint when I looked at this patch a
> year ago, but didn't come up with nearly as good an idea as to what to
> do about it.

I like "statement" better than "command," too, but love the acronym DMC. As in, 
"you want to Run [a] DMC." ;-P

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] wCTE: about the name of the feature

2011-02-24 Thread Robert Haas
On Thu, Feb 24, 2011 at 11:20 AM, Tom Lane  wrote:
> The wCTE patch refers to the feature it's adding as "DML WITH".  I'm
> still pretty unhappy with that terminology.  In my view of the world,
> "DML" includes SELECT as well as INSERT/UPDATE/DELETE.  The wikipedia
> entry about the term
> http://en.wikipedia.org/wiki/Data_Manipulation_Language
> agrees that that's at least the majority usage, and even our own docs
> seem to use it to include SELECT as often as not.  Since the distinction
> is absolutely critical to talking about this feature sensibly, I don't
> think it's a good plan to use an acronym that is guaranteed to produce
> uncertainty in the reader's mind.
>
> The best idea I have at the moment is to spell out "data modifying
> command" (or "statement") rather than relying on the acronym.
> In the code, we could change hasDmlWith to hasModifyingWith, for
> example.  The error messages could read like
>        data-modifying statement in WITH is not allowed in a view
>
> Comments?

Great idea.  I had the same complaint when I looked at this patch a
year ago, but didn't come up with nearly as good an idea as to what to
do about it.

-- 
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] SSI bug?

2011-02-24 Thread Kevin Grittner
Heikki Linnakangas  wrote:
> On 23.02.2011 07:20, Kevin Grittner wrote:
>> Dan Ports  wrote:
>>
>>> The obvious solution to me is to just keep the lock on both the
>>> old and new page.
>>
>> That's the creative thinking I was failing to do.  Keeping the
>> old lock will generate some false positives, but it will be rare
>> and those don't compromise correctness -- they just carry the
>> cost of starting the transaction over.
> 
> Sounds reasonable, but let me throw in another idea while we're at
> it: if there's a lock on the index page we're about to delete, we
> could just choose to not delete it. The next vacuum will pick it
> up. Presumably it will happen rarely, so index bloat won't be an
> issue.
 
Yeah, that's probably better.
 
-Kevin

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


Re: [HACKERS] pg_basebackup and wal streaming

2011-02-24 Thread Yeb Havinga

On 2011-02-20 21:37, Dimitri Fontaine wrote:

Hi,

Magnus Hagander  writes:

Better late than never (or?), here's the final cleanup of
pg_streamrecv for moving into the main distribution, per discussion
back in late dec or early jan. It also includes the "stream logs in
parallel to backup" part that was not completed on pg_basebackup.


I can't wait to see the new streaming replication setup docs using that
integrated tool.
I just did some initial playing around with this tool to start testing 
the latest syncrep patch. I'm time boxed for today, but just wanted to 
say: great tool.


mgrid@standby1:~/off/postgresql$ pg_basebackup -x -D /data -vP -h 
192.168.73.34

xlog start point: 0/720
34537/18152 kb g(100%) 1/1 tablespaces
xlog end point: 0/7014740
pg_basebackup: base backup completed.
mgrid@standby1:~/off/postgresql$ pg_ctl -D /data -l logfile start
server starting
mgrid@standby1:~/off/postgresql$ psql postgres
psql (9.1devel)
Type "help" for help.

postgres=# \d
   List of relations
 Schema | Name | Type  | Owner
+--+---+---
 public | aap  | table | mgrid
(1 row)

regards,
Yeb Havinga


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


Re: [HACKERS] SSI bug?

2011-02-24 Thread Heikki Linnakangas

On 23.02.2011 07:20, Kevin Grittner wrote:

Dan Ports  wrote:


The obvious solution to me is to just keep the lock on both the old
and new page.


That's the creative thinking I was failing to do.  Keeping the old
lock will generate some false positives, but it will be rare and
those don't compromise correctness -- they just carry the cost of
starting the transaction over.


Sounds reasonable, but let me throw in another idea while we're at it: 
if there's a lock on the index page we're about to delete, we could just 
choose to not delete it. The next vacuum will pick it up. Presumably it 
will happen rarely, so index bloat won't be an issue.



I was going to bemoan the extra complexity this would add -- but
actually, couldn't we just replace PredicateLockPageCombine with a
call to PredicateLockPageSplit since they'd now do the same thing?


I'd be inclined to leave the external interface alone, in case we
conceive of an even better implementation.


Agreed.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] wCTE: about the name of the feature

2011-02-24 Thread Andrew Dunstan



On 02/24/2011 11:20 AM, Tom Lane wrote:

The wCTE patch refers to the feature it's adding as "DML WITH".  I'm
still pretty unhappy with that terminology.  In my view of the world,
"DML" includes SELECT as well as INSERT/UPDATE/DELETE.  The wikipedia
entry about the term
http://en.wikipedia.org/wiki/Data_Manipulation_Language
agrees that that's at least the majority usage, and even our own docs
seem to use it to include SELECT as often as not.  Since the distinction
is absolutely critical to talking about this feature sensibly, I don't
think it's a good plan to use an acronym that is guaranteed to produce
uncertainty in the reader's mind.

The best idea I have at the moment is to spell out "data modifying
command" (or "statement") rather than relying on the acronym.
In the code, we could change hasDmlWith to hasModifyingWith, for
example.  The error messages could read like
data-modifying statement in WITH is not allowed in a view

Comments?



I think your're absolutely right. DML means that to me too. It's in 
effect the opposite of DDL.


log_statement used "mod" for this category of statements. If we need a 
new acronym, I modestly suggest "CUD" (CRUD without the R). The we could 
make all sorts of puns about "chewing the CUD" :-)


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] wCTE: about the name of the feature

2011-02-24 Thread Marko Tiikkaja

On 2011-02-24 6:37 PM +0200, Tom Lane wrote:

OK, I will make those adjustments.  Are you going to do more work on the
documentation part of the patch?  I can stick to working on the code
part meanwhile, if you are.


I am planning on working on the documentation this weekend.


Regards,
Marko Tiikkaja

--
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] wCTE: about the name of the feature

2011-02-24 Thread Tom Lane
Marko Tiikkaja  writes:
> On 2011-02-24 6:20 PM +0200, Tom Lane wrote:
>> The best idea I have at the moment is to spell out "data modifying
>> command" (or "statement") rather than relying on the acronym.
>> In the code, we could change hasDmlWith to hasModifyingWith, for
>> example.  The error messages could read like
>>  data-modifying statement in WITH is not allowed in a view
>> 
>> Comments?

> Out of everything suggested so far, I think this is the best we have, if 
> we can fit the whole thing into out error messages.  Quickly grepping 
> through the patch suggests that we can, at least for the cases in the 
> current patch.

> I also prefer "statement" over "command".

OK, I will make those adjustments.  Are you going to do more work on the
documentation part of the patch?  I can stick to working on the code
part meanwhile, if you are.

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] Possible substitute for PostmasterIsAlive polling loops

2011-02-24 Thread Florian Pflug
On Feb24, 2011, at 04:14 , Robert Haas wrote:
> On Wed, Feb 23, 2011 at 4:54 PM, Tom Lane  wrote:
>> IOW, at least on Linux, you *can* arrange to get a signal when your
>> parent process dies.
> 
> That's pretty cool.
> 
>> Not sure how ugly it'd be to use this call when available and a time
>> delay when not, but it's something to think about.
> 
> Yeah.  It may be worth thinking about whether we want to use the
> postmaster-pipe trick someone was proposing.  That might be more
> portable.


FWIW, I did some experiments which this when it was discussion a while ago.

To actually get a signal when the parent dies, I set the FASYNC flag
on the pipe's receiving end's fd. To be able to signal multiple children
using only one pipe, I set the fd's owner to -getgrp() using
fcntl(F_SETOWN). On both linux and OSX 10.6 this caused SIGIO to be sent
to all processes in the process group once sending end of the pipe was
closed.

I don't have access to any other Unixes to test this on, but I since it
works on OSX chances are high that it'll work on FreeBSD also.

best regards,
Florian Pflug


-- 
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] wCTE: about the name of the feature

2011-02-24 Thread Marko Tiikkaja

On 2011-02-24 6:20 PM +0200, Tom Lane wrote:

The wCTE patch refers to the feature it's adding as "DML WITH".  I'm
still pretty unhappy with that terminology.  In my view of the world,
"DML" includes SELECT as well as INSERT/UPDATE/DELETE.  The wikipedia
entry about the term
http://en.wikipedia.org/wiki/Data_Manipulation_Language
agrees that that's at least the majority usage, and even our own docs
seem to use it to include SELECT as often as not.  Since the distinction
is absolutely critical to talking about this feature sensibly, I don't
think it's a good plan to use an acronym that is guaranteed to produce
uncertainty in the reader's mind.


Agreed.


The best idea I have at the moment is to spell out "data modifying
command" (or "statement") rather than relying on the acronym.
In the code, we could change hasDmlWith to hasModifyingWith, for
example.  The error messages could read like
data-modifying statement in WITH is not allowed in a view

Comments?


Out of everything suggested so far, I think this is the best we have, if 
we can fit the whole thing into out error messages.  Quickly grepping 
through the patch suggests that we can, at least for the cases in the 
current patch.


I also prefer "statement" over "command".


Regards,
Marko Tiikkaja

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


[HACKERS] wCTE: about the name of the feature

2011-02-24 Thread Tom Lane
The wCTE patch refers to the feature it's adding as "DML WITH".  I'm
still pretty unhappy with that terminology.  In my view of the world,
"DML" includes SELECT as well as INSERT/UPDATE/DELETE.  The wikipedia
entry about the term
http://en.wikipedia.org/wiki/Data_Manipulation_Language
agrees that that's at least the majority usage, and even our own docs
seem to use it to include SELECT as often as not.  Since the distinction
is absolutely critical to talking about this feature sensibly, I don't
think it's a good plan to use an acronym that is guaranteed to produce
uncertainty in the reader's mind.

The best idea I have at the moment is to spell out "data modifying
command" (or "statement") rather than relying on the acronym.
In the code, we could change hasDmlWith to hasModifyingWith, for
example.  The error messages could read like
data-modifying statement in WITH is not allowed in a view

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] Review: Fix snapshot taking inconsistencies

2011-02-24 Thread Tom Lane
Marko Tiikkaja  writes:
> On 2011-02-24 5:21 PM, Tom Lane wrote:
>> Oh, did we decide to do it that way?  OK with me, but the submitted docs
>> are woefully inadequate on the point.  This behavior is going to have to
>> be explained extremely clearly (and even so, I bet we'll get bug reports
>> about it :-().

> I'm ready to put more effort into the documentation if the patch is 
> going in, but I really don't want to waste my time just to hear that the 
> patch is not going to be in 9.1.  Does this sound acceptable?

I've found some things I don't like about it, but the only part that
seems far short of being committable is the documentation.

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] Review: Fix snapshot taking inconsistencies

2011-02-24 Thread Marko Tiikkaja

On 2011-02-24 5:21 PM, Tom Lane wrote:

Oh, did we decide to do it that way?  OK with me, but the submitted docs
are woefully inadequate on the point.  This behavior is going to have to
be explained extremely clearly (and even so, I bet we'll get bug reports
about it :-().


I'm ready to put more effort into the documentation if the patch is 
going in, but I really don't want to waste my time just to hear that the 
patch is not going to be in 9.1.  Does this sound acceptable?



Regards,
Marko Tiikkaja

--
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] Named restore points

2011-02-24 Thread Euler Taveira de Oliveira

Em 08-02-2011 17:35, Thom Brown escreveu:

This could do with a bit more documentation about usage.  Below the
Backup Control Functions table
(http://developer.postgresql.org/pgdocs/postgres/functions-admin.html#FUNCTIONS-ADMIN-BACKUP-TABLE),
each function has a paragraph detailing what it does.


I forgot to check it.


Also, I notice you can easily write over a label.  The case I'm
thinking of is someone in psql creating a named restore point, then
later on, they go in again, accidentally cursor up and select the
previous statement and create it again.  Would this mean that the
previous label is lost, or would it be the case that any subsequent
duplicate labels would have no effect unless the WAL files with the
original label in were consumed?  In either case, a note in the docs
about this would be useful.

This is a limitation that I pointed out [1] but people decided to postpone 
named restore point management. The first one is used as restore point. I 
added it in the attached patch.



And I don't see these label creations getting logged either.  Could we
output that to the log because at least then users can grep the
directory for labels, and, in most cases, the time they occurred?

Good point. I included location instead of time; time is already supplied by 
log file.


The following patch implements the Thom's suggestions.


[1] http://archives.postgresql.org/message-id/4d48209c.7050...@timbira.com


--
  Euler Taveira de Oliveira
  http://www.timbira.com/
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 736eb67..fe7e42b 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** postgres=# select pg_start_backup('label
*** 14070,14075 
--- 14070,14084 
 
  
 
+pg_create_restore_point creates a named transaction log record
+that can be used as recovery point, and then returns the transaction log
+record location. The given name can be used in  that specifies the point up to which recovery
+will proceed. Avoid creating restore points that have the same name, recovery
+stops at the first one.
+
+ 
+
  pg_current_xlog_location displays the current transaction log write
  location in the same format used by the above functions.  Similarly,
  pg_current_xlog_insert_location displays the current transaction log
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3ba1f29..b4eb4ac 100644
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*** XLogRestorePoint(const char *rpName)
*** 8144,8149 
--- 8144,8153 
  
  	RecPtr = XLogInsert(RM_XLOG_ID, XLOG_RESTORE_POINT, &rdata);
  
+ 	ereport(LOG,
+ 			(errmsg("restore point \"%s\" created at %X/%X",
+ 	rpName,	RecPtr.xlogid, RecPtr.xrecoff)));
+ 
  	return RecPtr;
  }
  

-- 
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] Review: Fix snapshot taking inconsistencies

2011-02-24 Thread Tom Lane
Marko Tiikkaja  writes:
> On 2011-02-24 2:31 AM, Tom Lane wrote:
>> The connection is the question of where to do CommandCounterIncrement
>> between successive DML WITH operations in a single command.

> .. what?  We decided *not* to do any CommandCounterIncrements between 
> DML WITHs.

Oh, did we decide to do it that way?  OK with me, but the submitted docs
are woefully inadequate on the point.  This behavior is going to have to
be explained extremely clearly (and even so, I bet we'll get bug reports
about 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] WIP: cross column correlation ...

2011-02-24 Thread Bruce Momjian
Robert Haas wrote:
> On Wed, Feb 23, 2011 at 10:30 PM, Bruce Momjian  wrote:
> > Robert Haas wrote:
> >> If you want to take the above as in any way an exhaustive survey of
> >> the landscape (which it isn't), C seems like a standout, maybe
> >> augmented by the making the planner able to notice that A1 = x1 AND A2
> >> = x2 is equivalent to (A1,A2) = (x1, x2) so you don't have to rewrite
> >> queries as much.
> >>
> >> I don't really know how to handle the join selectivity problem. ?I am
> >> not convinced that there is a better solution to that than decorating
> >> the query. ?After all the join selectivity depends not only on the
> >> join clause itself, but also on what you've filtered out of each table
> >> in the meantime.
> >
> > Thinking some more, I think another downside to the "decorate the query"
> > idea is that many queries use constants that are supplied only at
> > runtime, so there would be no way to hard-code a selectivity value into
> > a query when you don't know the value. ?Could a selectivity function
> > handle that?
> 
> Beats me.  What do you have in mind?

My point is just that many queries have constants who's values are not
known at the time the query is written, so any system should have a way
to handle that somehow.  This is why query decoration is usually not a
good solution, and why something more flexible that is stored as part of
the column is preferred.

Perhaps a selectivity function that has easy access to the computed
selectivity of the constant involved might be a win.  For example, for
the zip code/state code case we could have something like:

function mysel(zip, state) { pgsel(zip)}

meaning we would still use the selectivities found in the optimizer
statistics (pgsel), but modify them in some way.  In the case above, the
selectivity only comes from the zip code.  You could also do things like:

function mysel(x, y) { pgsel(x) * pgsel(y) * 0.001}

Such functions have a higher probability of working for all queries
involving that column.

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

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

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


Re: [HACKERS] Review: Fix snapshot taking inconsistencies

2011-02-24 Thread Alvaro Herrera
Excerpts from Alvaro Herrera's message of mié feb 23 21:35:13 -0300 2011:
> Excerpts from Tom Lane's message of mié feb 23 19:39:23 -0300 2011:

> > My recollection is that this was pretty tightly coupled to the wCTE
> > patch.  I had been intending to review them together, and have just
> > now come up for air enough to start doing that.  Do you really want
> > to review this one separately?
> 
> Dunno.  If you're gonna pick it up I guess my time is best spent
> elsewhere.  There was some restructuring in code in postgres.c to be
> done near this patch, which wasn't attacked at all by Marko AFAICS.
> Maybe I should be looking at that instead.

... but then, if I did that, I could create merge conflicts for you, so
please have at it.  I think pure beautification code cleanups can way
till 9.2 starts.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Binary in/out for aclitem

2011-02-24 Thread rsmogura

On Thu, 24 Feb 2011 08:38:35 -0600, Merlin Moncure wrote:

On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane  wrote:

=?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
Here is extended version, has version field (N_ACL_RIGHTS*2) and 
reserved
mask, as well definition is more general then def of PGSQL. In any 
way it

require that rights mades bit array.


You're going in quite the wrong direction here.  The consensus as I
understood it was that we should just use the text representation in
binary mode too, rather than inventing a separate representation 
that's
going to put a whole new set of constraints on what can happen to 
the
internal representation.  The proposal you have here has no 
redeeming

social value whatever, because nobody cares about the I/O efficiency
for aclitem (and even if anyone did, you've made no case that this 
would

actually be more efficient to use on the client side).


+1 on this.  binary wire format is a win generally when one of the 
two

properties is true:

1) the receiving application is putting it into a binary structure
that is similar to what the backend sends, and conversion is
non-trivial (timestamps, geo types, etc)
2) text format needs lots of escaping (bytea, arrays etc)



Let's take the numeric type for example...if we were debating the
binary wire format for that type, I would be arguing for the backend
to send a string for the binary wire format unless someone could
present a solid case that the postgres format dropped right into a
popular numeric library in C, etc (AFAIK, it doesn't).  Almost
everyone that gets a numeric will directly translate it to a string 
or

a hardware binary representation which the backend can't send.

Even if you could make the case for aclitem on performance grounds,
you still have to get past tom's objection (which I agree with) that
the performance benefit outweighs having to deal with making and
(especially) maintaining the binary wire format.  It should be
becoming obvious to everyone the binary formats are becoming
increasingly popular, and sooner or later backwards compatibility
issues and other unresolved issues pertaining to them have to be 
dealt

with.  Point being, let's not make that more difficult than it has to
be.

merlin


Thanks, but actually I didn't realized final direction, "pass to text" 
or "create something really extensive", I didn't treat aclitem IO as 
live or dead case, just all. I always treat performance really serious, 
but I'm not psychopathic to check aclitem IO!!!


Btw, In my opinion binary format will be popular not for speed, but for 
that it is internal strict, and pass in many situations more useful 
informations (e.g. types for structs, arrays), it is just easier to 
maintain on driver side. But it is still unpopular maybe due to missing 
methods :), and few others.


Regards,
Radek


--
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] Binary in/out for aclitem

2011-02-24 Thread Merlin Moncure
On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane  wrote:
> =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
>> Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
>> mask, as well definition is more general then def of PGSQL. In any way it
>> require that rights mades bit array.
>
> You're going in quite the wrong direction here.  The consensus as I
> understood it was that we should just use the text representation in
> binary mode too, rather than inventing a separate representation that's
> going to put a whole new set of constraints on what can happen to the
> internal representation.  The proposal you have here has no redeeming
> social value whatever, because nobody cares about the I/O efficiency
> for aclitem (and even if anyone did, you've made no case that this would
> actually be more efficient to use on the client side).

+1 on this.  binary wire format is a win generally when one of the two
properties is true:

1) the receiving application is putting it into a binary structure
that is similar to what the backend sends, and conversion is
non-trivial (timestamps, geo types, etc)
2) text format needs lots of escaping (bytea, arrays etc)

Let's take the numeric type for example...if we were debating the
binary wire format for that type, I would be arguing for the backend
to send a string for the binary wire format unless someone could
present a solid case that the postgres format dropped right into a
popular numeric library in C, etc (AFAIK, it doesn't).  Almost
everyone that gets a numeric will directly translate it to a string or
a hardware binary representation which the backend can't send.

Even if you could make the case for aclitem on performance grounds,
you still have to get past tom's objection (which I agree with) that
the performance benefit outweighs having to deal with making and
(especially) maintaining the binary wire format.  It should be
becoming obvious to everyone the binary formats are becoming
increasingly popular, and sooner or later backwards compatibility
issues and other unresolved issues pertaining to them have to be dealt
with.  Point being, let's not make that more difficult than it has to
be.

merlin

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


Re: [HACKERS] PostgreSQL FDW update

2011-02-24 Thread Robert Haas
On Thu, Feb 24, 2011 at 8:13 AM, Shigeru HANADA
 wrote:
>
> On Wed, 23 Feb 2011 20:30:05 +0900
> Shigeru HANADA  wrote:
>
>>
>> On Tue, 22 Feb 2011 11:33:25 -0500
>> Robert Haas  wrote:
>> > Is anyone actually working on a new version of this patch sufficiently
>> > rapidly that we can expect a new version in the next day or two?
>> >
>> > If not, I think we mark this one Returned with Feedback and revisit it for 
>> > 9.2.
>>
>> I'm working on it.
>>
>> Fixes for new FDW API have been done, but there are some problems in
>> SQL generation codes, such as SELECT clause optimization (omitting
>> unused column from SELECT clause).  It would take a while, but I'll
>> post revised version of the patch tomorrow.
>
> Attached is a revised version of postgresql_fdw patch.  I started from
> Heikki's latest patch, and modified some points:
>
> 1) use new FDW API
> 2) use EXTENSION framework
> 3) SELECT clause optimization (use NULL for unused columns)
> 4) show remote query in EXPLAIN output
>
> WHERE clause pushdown was implemented in Heikki's version, so I didn't
> touch around it.  Now I'm working on cost estimation and connection
> management, but they would need some more work.

So this is still work-in-progress?  When do you expect a final version?

-- 
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] pl/python tracebacks

2011-02-24 Thread Jan Urbański
On 24/02/11 14:10, Peter Eisentraut wrote:
> On tor, 2010-12-23 at 14:56 +0100, Jan Urbański wrote:
>> For errors originating from Python exceptions add the traceback as the
>> message detail. The patch tries to mimick Python's traceback.py module
>> behaviour as close as possible, icluding interleaving stack frames
>> with source code lines in the detail message. Any Python developer
>> should instantly recognize these kind of error reporting, it looks
>> almost the same as an error in the interactive Python shell.
> 
> I think the traceback should go into the CONTEXT part of the error.  The
> context message that's already there is now redundant with the
> traceback.
> 
> You could even call errcontext() multiple times to build up the
> traceback, but maybe that's not necessary.

Hm, perhaps, I put it in the details, because it sounded like the place
to put information that is not that important, but still helpful. It's
kind of natural to think of the traceback as the detail of the error
message. But if you prefer context, I'm fine with that. You want me to
update the patch to put the traceback in the context?

Jan

-- 
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] pl/python tracebacks

2011-02-24 Thread Peter Eisentraut
On tor, 2010-12-23 at 14:56 +0100, Jan Urbański wrote:
> For errors originating from Python exceptions add the traceback as the
> message detail. The patch tries to mimick Python's traceback.py module
> behaviour as close as possible, icluding interleaving stack frames
> with source code lines in the detail message. Any Python developer
> should instantly recognize these kind of error reporting, it looks
> almost the same as an error in the interactive Python shell.

I think the traceback should go into the CONTEXT part of the error.  The
context message that's already there is now redundant with the
traceback.

You could even call errcontext() multiple times to build up the
traceback, but maybe that's not necessary.



-- 
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] Sync Rep v17

2011-02-24 Thread Fujii Masao
On Tue, Feb 22, 2011 at 2:38 PM, Fujii Masao  wrote:
> I've read about two-tenths of the patch, so I'll submit another comments
> about the rest later. Sorry for the slow reviewing...

Here are another comments:

+   {"synchronous_standby_names", PGC_SIGHUP, WAL_REPLICATION,
+   gettext_noop("List of potential standby names to 
synchronise with."),
+   NULL,
+   GUC_LIST_INPUT | GUC_IS_NAME

Why did you add GUC_IS_NAME here? I don't think that it's reasonable
to limit the length of this parameter to 63. Because dozens of standby
names might be specified in the parameter.

SyncRepQueue->qlock should be initialized by calling SpinLockInit?

+ * Portions Copyright (c) 2010-2010, PostgreSQL Global Development Group

Typo: s/2010/2011

sync_replication_timeout_client would mess up the "wait-forever" option.
So, when allow_standalone_primary is disabled, ISTM that
sync_replication_timeout_client should have no effect.

Please check max_wal_senders before calling SyncRepWaitForLSN for
non-replication case.

SyncRepRemoveFromQueue seems not to be as short-term as we can
use the spinlock. Instead, LW lock should be used there.

+   old_status = get_ps_display(&len);
+   new_status = (char *) palloc(len + 21 + 1);
+   memcpy(new_status, old_status, len);
+   strcpy(new_status + len, " waiting for sync rep");
+   set_ps_display(new_status, false);
+   new_status[len] = '\0'; /* truncate off " waiting" */

Updating the PS display should be skipped if update_process_title is false.

+   /*
+* XXX extra code needed here to maintain sorted invariant.

Yeah, such a code is required. I think that we can shorten the time
it takes to find an insert position by searching the list backwards.
Because the given LSN is expected to be relatively new in the queue.

+* Our approach should be same as racing car - slow in, fast 
out.
+*/

Really? Even when removing the entry from the queue, we need
to search the queue as well as we do in the add-entry case.
Why don't you make walsenders remove the entry from the queue,
instead?

+   longtimeout = SyncRepGetWaitTimeout();

+   bool timeout = false;

+   else if (timeout > 0 &&
+   
TimestampDifferenceExceeds(GetCurrentTransactionStopTimestamp(),
+   
now, timeout))
+   {
+   release = true;
+   timeout = true;
+   }

You seem to mix up two "timeout" variables.

+   if (proc->lwWaitLink == MyProc)
+   {
+   /*
+* Remove ourselves from middle of queue.
+* No need to touch head or tail.
+*/
+   proc->lwWaitLink = MyProc->lwWaitLink;
+   }

When we find ourselves, we should break out of the loop soon,
instead of continuing the loop to the end?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] pl/python tracebacks

2011-02-24 Thread Peter Eisentraut
On lör, 2011-02-12 at 10:07 +0100, Jan Urbański wrote:
> > PLyUnicode_AsString(PyObject *unicode)
> > {
> > PyObject   *o = PLyUnicode_Bytes(unicode);
> > char   *rv = pstrdup(PyBytes_AsString(o));
> > 
> > Py_XDECREF(o);
> > return rv;
> > }
> > 
> > PyString_AsString is used all over the place without any pfrees. But
> I
> > have no Idea how that pstrdup() is getting freed if at all.
> > 
> > Care to enlighten me ?
> 
> Ooops, seems that this hack that's meant to improve compatibility with
> Python3 makes it leak. I wonder is the pstrdup is necessary here,

The result of PyBytes_AsString(o) points into the internal storage of o,
which is released (effectively freed) by the decref on the next line.
So you'd better make a copy if you want to keep using it.


-- 
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] Invitation to Cluster Hackers meeting at pgCon

2011-02-24 Thread Tatsuo Ishii
Hi,

I will be there.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

> All,
> 
> This year we will be having a "Cluster Hackers" summit at pgCon.  You
> are invited if you are currently working on any PostgreSQL replication
> or clustering solution, or core features to support such solutions.
> This includes, but is not limited to: PostgreXC, GridSQL, Postgres-R,
> Slony-I, Bucardo, binary replication, pgPool, Skytools, Aster Data,
> Greenplum, HadoopDB, or others.
> 
> This is a working meeting, intended to coordinate development of joint
> open source projects in order to enhance PostgreSQL clustering.  It is
> not a meeting for users, just hackers.  The meeting is sponsored by NTT
> Open Source.
> 
> Tuesday May 17th
> 9am to 5pm
> University of Ottawa Campus (room TBD)
> 
> This is concurrent with the first day of tutorials at pgCon.
> 
> Please RSVP if you plan to be there.  Please also make a note if you
> need assistance with travel costs (We probably will not have funds
> for assistance, but I'll let you know if we do).
> 
> RSVP:
> https://spreadsheets.google.com/viewform?formkey=dEFINzFPYlROTFZBVjJEYVFTbWZIUXc6MQ
> 
> Tentative schedule is:
> 
> 09:00-09:30 - Introduction, organize schedule
> 
> 09:30-12:00 - Status updates on clustering projects
>   and on core-code clustering projects
> 
> 12:00-13:00 - Break for lunch
> 
> 13:00-17:00 - Breakout sessions: discuss specific projects
>   in small groups
> (extra room will be available for this)
> 
> 19:00   Dinner (optional, not sponsored)
> 
> 
> 
> 
> -- 
>   -- Josh Berkus
>  PostgreSQL Experts Inc.
>  http://www.pgexperts.com
> 
> -- 
> 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] pl/python tracebacks

2011-02-24 Thread Peter Eisentraut
On lör, 2011-02-12 at 02:00 -0700, Alex Hunsaker wrote:
> PyString_AsString is used all over the place without any pfrees. But I
> have no Idea how that pstrdup() is getting freed if at all.

pstrdup() like palloc() allocates memory from the current memory
context, which is freed automatically at some useful time, often at the
end of the query.  It is very common throughout the PostgreSQL code that
memory is not explicitly freed.  See src/backend/utils/mmgr/README for
more information.


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