Re: [HACKERS] inherit support for foreign tables

2014-07-10 Thread Etsuro Fujita

(2014/07/10 18:12), Shigeru Hanada wrote:

2014-06-24 16:30 GMT+09:00 Etsuro Fujita :

(2014/06/23 18:35), Ashutosh Bapat wrote:



Selecting tableoid on parent causes an error, "ERROR:  cannot extract
system attribute from virtual tuple". The foreign table has an OID which
can be reported as tableoid for the rows coming from that foreign table.
Do we want to do that?



No.  I think it's a bug.  I'll fix it.



IIUC, you mean that tableoid can't be retrieved when a foreign table
is accessed via parent table,


No.  What I want to say is that tableoid *can* be retrieved when a 
foreign table is accessed via the parent table.


Thanks,

Best regards,
Etsuro Fujita


--
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] Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

2014-07-10 Thread Jeff Davis
On Mon, 2014-07-07 at 01:21 -0700, Jeff Davis wrote:
> On Sun, 2014-07-06 at 21:11 -0700, Jeff Davis wrote:
> > On Wed, 2014-04-16 at 12:50 +0100, Nicholas White wrote:
> > > Thanks for the detailed feedback, I'm sorry it took so long to
> > > incorporate it. I've attached the latest version of the patch, fixing
> > > in particular:

As innocent as this patch seemed at first, it actually opens up a lot of
questions.

Attached is the (incomplete) edit of the patch so far.

Changes from your patch:

* changed test to be locale-insensitive
* lots of refactoring in the execution itself
* fix offset 0 case
* many test improvements
* remove bitmapset and just use an array bitmap
* fix error message typo

Open Issues:

I don't think exposing the frame options is a good idea. That's an
internal concept now, but putting it in windowapi.h will mean that it
needs to live forever.

The struct is private, so there's no easy hack to access the frame
options directly. That means that we need to work with the existing API
functions, which is OK because I think that everything we want to do can
go into WinGetFuncArgInPartition(). If we do the same thing for
WinGetFuncArgInFrame(), then first/last/nth also work.

That leaves the questions:
 * Do we want IGNORE NULLS to work for every window function, or only a
specified subset?
 * If it only works for some window functions, is that hard-coded or
driven by the catalog?
 * If it works for all window functions, could it cause some
pre-existing functions to behave strangely?

Also, I'm re-thinking Dean's comments here:

http://www.postgresql.org/message-id/CAEZATCWT3=P88nv2ThTjvRDLpOsVtAPxaVPe=MaWe-x=guh...@mail.gmail.com

He brings up a few good points. I will look into the frame vs. window
option, though it looks like you've already at least fixed the crash.
His other point about actually eliminating the NULLs from the window
itself is interesting, but I don't think it works. IGNORE NULLS ignores
*other* rows with NULL, but (per spec) does not ignore the current row.
That sounds awkward if you've already removed the NULL rows from the
window, but maybe there's something that could work.

And there are a few other things I'm still looking into, but hopefully
they don't raise new issues.

Regards,
Jeff Davis

*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 13164,13169  SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;
--- 13164,13170 
   lag(value any
   [, offset integer
   [, default any ]])
+  [ { RESPECT | IGNORE } NULLS ]
 


***
*** 13178,13184  SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;
 default are evaluated
 with respect to the current row.  If omitted,
 offset defaults to 1 and
!default to null

   
  
--- 13179,13187 
 default are evaluated
 with respect to the current row.  If omitted,
 offset defaults to 1 and
!default to null. If
!IGNORE NULLS is specified then the function will be evaluated
!as if the rows containing nulls didn't exist.

   
  
***
*** 13191,13196  SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;
--- 13194,13200 
   lead(value any
[, offset integer
[, default any ]])
+  [ { RESPECT | IGNORE } NULLS ]
 


***
*** 13205,13211  SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;
 default are evaluated
 with respect to the current row.  If omitted,
 offset defaults to 1 and
!default to null

   
  
--- 13209,13217 
 default are evaluated
 with respect to the current row.  If omitted,
 offset defaults to 1 and
!default to null. If
!IGNORE NULLS is specified then the function will be evaluated
!as if the rows containing nulls didn't exist.

   
  
***
*** 13299,13309  SELECT xmlagg(x) FROM (SELECT x FROM test ORDER BY y DESC) AS tab;

 
  The SQL standard defines a RESPECT NULLS or
! IGNORE NULLS option for lead, lag,
! first_value, last_value, and
! nth_value.  This is not implemented in
! PostgreSQL: the behavior is always the
! same as the standard's default, namely RESPECT NULLS.
  Likewise, the standard's FROM FIRST or FROM LAST
  option for nth_value is not implemented: only the
  default FROM FIRST behavior is supported.  (You can achieve
--- 13305,13314 

 
  The SQL standard defines a RESPECT NULLS or
! IGNORE NULLS option for first_value,
! last_value, and nth_value.  This is not
! implemented in PostgreSQL: the behavior is
! always the same as the standard's default, namely RESPECT NULLS.
  Likewise, the standard's FROM FIRST or FROM

Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes

2014-07-10 Thread Andres Freund
On 2014-07-04 19:27:10 +0530, Rahila Syed wrote:
> + /* Allocates memory for compressed backup blocks according to the 
> compression
> + * algorithm used.Once per session at the time of insertion of first XLOG
> + * record.
> + * This memory stays till the end of session. OOM is handled by making 
> the
> + * code proceed without FPW compression*/
> + static char *compressed_pages[XLR_MAX_BKP_BLOCKS];
> + static bool compressed_pages_allocated = false;
> + if (compress_backup_block != BACKUP_BLOCK_COMPRESSION_OFF &&
> + compressed_pages_allocated!= true)
> + {
> + size_t buffer_size = VARHDRSZ;
> + int j;
> + if (compress_backup_block == BACKUP_BLOCK_COMPRESSION_SNAPPY)
> + buffer_size += snappy_max_compressed_length(BLCKSZ);
> + else if (compress_backup_block == BACKUP_BLOCK_COMPRESSION_LZ4)
> + buffer_size += LZ4_compressBound(BLCKSZ);
> + else if (compress_backup_block == BACKUP_BLOCK_COMPRESSION_PGLZ)
> + buffer_size += PGLZ_MAX_OUTPUT(BLCKSZ);
> + for (j = 0; j < XLR_MAX_BKP_BLOCKS; j++)
> + {   compressed_pages[j] = (char *) malloc(buffer_size);
> + if(compressed_pages[j] == NULL)
> + {
> + 
> compress_backup_block=BACKUP_BLOCK_COMPRESSION_OFF;
> + break;
> + }
> + }
> + compressed_pages_allocated = true;
> + }

Why not do this in InitXLOGAccess() or similar?

>   /*
>* Make additional rdata chain entries for the backup blocks, so that we
>* don't need to special-case them in the write loop.  This modifies the
> @@ -1015,11 +1048,32 @@ begin:;
>   rdt->next = &(dtbuf_rdt2[i]);
>   rdt = rdt->next;
>  
> + if (compress_backup_block != BACKUP_BLOCK_COMPRESSION_OFF)
> + {
> + /* Compress the backup block before including it in rdata chain 
> */
> + rdt->data = CompressBackupBlock(page, BLCKSZ - 
> bkpb->hole_length,
> + 
> compressed_pages[i], &(rdt->len));
> + if (rdt->data != NULL)
> + {
> + /*
> +  * write_len is the length of compressed block 
> and its varlena
> +  * header
> +  */
> + write_len += rdt->len;
> + bkpb->hole_length = BLCKSZ - rdt->len;
> + /*Adding information about compression in the 
> backup block header*/
> + bkpb->block_compression=compress_backup_block;
> + rdt->next = NULL;
> + continue;
> + }
> + }
> +

So, you're compressing backup blocks one by one. I wonder if that's the
right idea and if we shouldn't instead compress all of them in one run to
increase the compression ratio.


> +/*
>   * Get a pointer to the right location in the WAL buffer containing the
>   * given XLogRecPtr.
>   *
> @@ -4061,6 +4174,50 @@ RestoreBackupBlockContents(XLogRecPtr lsn, BkpBlock 
> bkpb, char *blk,
>   {
>   memcpy((char *) page, blk, BLCKSZ);
>   }
> + /* Decompress if backup block is compressed*/
> + else if (VARATT_IS_COMPRESSED((struct varlena *) blk)
> + && 
> bkpb.block_compression!=BACKUP_BLOCK_COMPRESSION_OFF)
> + {
> + if (bkpb.block_compression == BACKUP_BLOCK_COMPRESSION_SNAPPY)
> + {
> + int ret;
> + size_t compressed_length = VARSIZE((struct varlena *) 
> blk) - VARHDRSZ;
> + char *compressed_data = (char *)VARDATA((struct varlena 
> *) blk);
> + size_t s_uncompressed_length;
> +
> + ret = snappy_uncompressed_length(compressed_data,
> + compressed_length,
> + &s_uncompressed_length);
> + if (!ret)
> + elog(ERROR, "snappy: failed to determine 
> compression length");
> + if (BLCKSZ != s_uncompressed_length)
> + elog(ERROR, "snappy: compression size mismatch 
> %d != %zu",
> + BLCKSZ, s_uncompressed_length);
> +
> + ret = snappy_uncompress(compressed_data,
> + compressed_length,
> + page);
> + if (ret != 0)
> + elog(ERROR, "snappy: decompression failed: %d", 
> ret);
> + }
> +  

Re: [HACKERS] Minmax indexes

2014-07-10 Thread Fujii Masao
On Thu, Jul 10, 2014 at 6:16 AM, Alvaro Herrera
 wrote:
> Claudio Freire wrote:
>
>> An aggregate to generate a "compressed set" from several values
>> A function which adds a new value to the "compressed set" and returns
>> the new "compressed set"
>> A function which tests if a value is in a "compressed set"
>> A function which tests if a "compressed set" overlaps another
>> "compressed set" of equal type
>>
>> If you can define different compressed sets, you can use this to
>> generate both min/max indexes as well as bloom filter indexes. Whether
>> we'd want to have both is perhaps questionable, but having the ability
>> to is probably desirable.
>
> Here's a new version of this patch, which is more generic the original
> versions, and similar to what you describe.

I've not read the discussion so far at all, but I found the problem
when I played with this patch. Sorry if this has already been discussed.

=# create table test as select num from generate_series(1,10) num;
SELECT 10
=# create index testidx on test using minmax (num);
CREATE INDEX
=# alter table test alter column num type text;
ERROR:  could not determine which collation to use for string comparison
HINT:  Use the COLLATE clause to set the collation explicitly.

Regards,

-- 
Fujii Masao


-- 
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 multi-byte characters as escape in SIMILAR TO and SUBSTRING

2014-07-10 Thread Fujii Masao
On Fri, Jul 11, 2014 at 12:49 PM, Jeff Davis  wrote:
> Attached is a small patch to $SUBJECT.
>
> In master, only single-byte characters are allowed as an escape. Of
> course, with the patch it must still be a single character, but it may
> be multi-byte.

+1

Probably you know that, multi-byte character is already allowed
as escape in LIKE. There seems no reason why we don't support
multi-byte escape character in SIMILAR TO and SUBSTRING.

Could you add the patch into next CF?

The patch doesn't contain the change of the document. But I think that
it's better to document what character is allowed as escape in LIKE,
SIMILAR TO and SUBSTRING.

Regards,

-- 
Fujii Masao


-- 
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_upgrade and toast tables bug discovered

2014-07-10 Thread Alvaro Herrera
Bruce Momjian wrote:
> On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote:

> > I have thought some more on this.  I thought I would need to open
> > pg_class in C and do complex backend stuff, but I now realize I can do
> > it from libpq, and just call ALTER TABLE and I think that always
> > auto-checks if a TOAST table is needed.  All I have to do is query
> > pg_class from libpq, then construct ALTER TABLE commands for each item,
> > and it will optionally create the TOAST table if needed.  I just have to
> > use a no-op ALTER TABLE command, like SET STATISTICS.
> 
> Attached is the backend part of the patch.  I will work on the
> pg_upgrade/libpq/ALTER TABLE part later.

Uh, why does this need to be in ALTER TABLE?  Can't this be part of
table creation done by pg_dump?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


[HACKERS] Allow multi-byte characters as escape in SIMILAR TO and SUBSTRING

2014-07-10 Thread Jeff Davis
Attached is a small patch to $SUBJECT.

In master, only single-byte characters are allowed as an escape. Of
course, with the patch it must still be a single character, but it may
be multi-byte.

Regards,
Jeff Davis

*** a/src/backend/utils/adt/regexp.c
--- b/src/backend/utils/adt/regexp.c
***
*** 688,698  similar_escape(PG_FUNCTION_ARGS)
  		elen = VARSIZE_ANY_EXHDR(esc_text);
  		if (elen == 0)
  			e = NULL;			/* no escape character */
! 		else if (elen != 1)
! 			ereport(ERROR,
! 	(errcode(ERRCODE_INVALID_ESCAPE_SEQUENCE),
! 	 errmsg("invalid escape string"),
!   errhint("Escape string must be empty or one character.")));
  	}
  
  	/*--
--- 688,704 
  		elen = VARSIZE_ANY_EXHDR(esc_text);
  		if (elen == 0)
  			e = NULL;			/* no escape character */
! 		else
! 		{
! 			int escape_mblen = pg_verify_mbstr_len(GetDatabaseEncoding(), e,
!    elen, false);
! 
! 			if (escape_mblen > 1)
! ereport(ERROR,
! 		(errcode(ERRCODE_INVALID_ESCAPE_SEQUENCE),
! 		 errmsg("invalid escape string"),
! 		 errhint("Escape string must be empty or one character.")));
! 		}
  	}
  
  	/*--
***
*** 722,781  similar_escape(PG_FUNCTION_ARGS)
  
  	while (plen > 0)
  	{
! 		char		pchar = *p;
  
! 		if (afterescape)
  		{
! 			if (pchar == '"' && !incharclass)	/* for SUBSTRING patterns */
! *r++ = ((nquotes++ % 2) == 0) ? '(' : ')';
! 			else
  			{
  *r++ = '\\';
  *r++ = pchar;
  			}
! 			afterescape = false;
! 		}
! 		else if (e && pchar == *e)
! 		{
! 			/* SQL99 escape character; do not send to output */
! 			afterescape = true;
  		}
! 		else if (incharclass)
  		{
! 			if (pchar == '\\')
  *r++ = '\\';
! 			*r++ = pchar;
! 			if (pchar == ']')
! incharclass = false;
! 		}
! 		else if (pchar == '[')
! 		{
! 			*r++ = pchar;
! 			incharclass = true;
! 		}
! 		else if (pchar == '%')
! 		{
! 			*r++ = '.';
! 			*r++ = '*';
! 		}
! 		else if (pchar == '_')
! 			*r++ = '.';
! 		else if (pchar == '(')
! 		{
! 			/* convert to non-capturing parenthesis */
! 			*r++ = '(';
! 			*r++ = '?';
! 			*r++ = ':';
! 		}
! 		else if (pchar == '\\' || pchar == '.' ||
!  pchar == '^' || pchar == '$')
! 		{
! 			*r++ = '\\';
! 			*r++ = pchar;
  		}
- 		else
- 			*r++ = pchar;
- 		p++, plen--;
  	}
  
  	*r++ = ')';
--- 728,816 
  
  	while (plen > 0)
  	{
! 		char	pchar = *p;
! 		int		mblen = pg_encoding_verifymb(GetDatabaseEncoding(), p, plen);
! 
! 		Assert(mblen > 0);
  
! 		if (mblen == 1)
  		{
! 			if (afterescape)
! 			{
! if (pchar == '"' && !incharclass)	/* for SUBSTRING patterns */
! 	*r++ = ((nquotes++ % 2) == 0) ? '(' : ')';
! else
! {
! 	*r++ = '\\';
! 	*r++ = pchar;
! }
! afterescape = false;
! 			}
! 			else if (e && pchar == *e)
! 			{
! /* SQL99 escape character; do not send to output */
! afterescape = true;
! 			}
! 			else if (incharclass)
! 			{
! if (pchar == '\\')
! 	*r++ = '\\';
! *r++ = pchar;
! if (pchar == ']')
! 	incharclass = false;
! 			}
! 			else if (pchar == '[')
! 			{
! *r++ = pchar;
! incharclass = true;
! 			}
! 			else if (pchar == '%')
! 			{
! *r++ = '.';
! *r++ = '*';
! 			}
! 			else if (pchar == '_')
! *r++ = '.';
! 			else if (pchar == '(')
! 			{
! /* convert to non-capturing parenthesis */
! *r++ = '(';
! *r++ = '?';
! *r++ = ':';
! 			}
! 			else if (pchar == '\\' || pchar == '.' ||
! 	 pchar == '^' || pchar == '$')
  			{
  *r++ = '\\';
  *r++ = pchar;
  			}
! 			else
! *r++ = pchar;
! 			p++, plen--;
  		}
! 		else
  		{
! 			if (afterescape)
! 			{
  *r++ = '\\';
! memcpy(r, p, mblen);
! r += mblen;
! afterescape = false;
! 			}
! 			else if (e && elen == mblen && memcmp(e, p, mblen) == 0)
! 			{
! /* SQL99 escape character; do not send to output */
! afterescape = true;
! 			}
! 			else
! 			{
! memcpy(r, p, mblen);
! r += mblen;
! 			}
! 
! 			p += mblen;
! 			plen -= mblen;
  		}
  	}
  
  	*r++ = ')';

-- 
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_upgrade and toast tables bug discovered

2014-07-10 Thread Bruce Momjian
On Thu, Jul 10, 2014 at 06:38:26PM -0400, Bruce Momjian wrote:
> On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote:
> > Well, we are going to need to call internal C functions, often bypassing
> > their typical call sites and the assumption about locking, etc.  Perhaps
> > this could be done from a plpgsql function.  We could add and drop a
> > dummy column to force TOAST table creation --- we would then only need a
> > way to detect if a function _needs_ a TOAST table, which was skipped in
> > binary upgrade mode previously.
> > 
> > That might be a minimalistic approach.
> 
> I have thought some more on this.  I thought I would need to open
> pg_class in C and do complex backend stuff, but I now realize I can do
> it from libpq, and just call ALTER TABLE and I think that always
> auto-checks if a TOAST table is needed.  All I have to do is query
> pg_class from libpq, then construct ALTER TABLE commands for each item,
> and it will optionally create the TOAST table if needed.  I just have to
> use a no-op ALTER TABLE command, like SET STATISTICS.
> 
> I am in Asia the next two weeks but will work on it after I return.

Attached is the backend part of the patch.  I will work on the
pg_upgrade/libpq/ALTER TABLE part later.

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

  + Everyone has their own god. +
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
new file mode 100644
index bdfeb90..6e8ef36
*** a/src/backend/catalog/toasting.c
--- b/src/backend/catalog/toasting.c
*** create_toast_table(Relation rel, Oid toa
*** 165,180 
  	if (rel->rd_rel->reltoastrelid != InvalidOid)
  		return false;
  
! 	/*
! 	 * Check to see whether the table actually needs a TOAST table.
! 	 *
! 	 * If an update-in-place toast relfilenode is specified, force toast file
! 	 * creation even if it seems not to need one.
! 	 */
! 	if (!needs_toast_table(rel) &&
! 		(!IsBinaryUpgrade ||
! 		 !OidIsValid(binary_upgrade_next_toast_pg_class_oid)))
! 		return false;
  
  	/*
  	 * If requested check lockmode is sufficient. This is a cross check in
--- 165,211 
  	if (rel->rd_rel->reltoastrelid != InvalidOid)
  		return false;
  
! 	if (!IsBinaryUpgrade)
! 	{
! 		if (!needs_toast_table(rel))
! 			return false;
! 	}
! 	else
! 	{
! 		/*
! 		 * Check to see whether the table needs a TOAST table.
! 		 *
! 		 * If an update-in-place TOAST relfilenode is specified, force TOAST file
! 		 * creation even if it seems not to need one.  This handles the case
! 		 * where the old cluster needed a TOAST table but the new cluster
! 		 * would not normally create one.
! 		 */
! 		 
! 		/*
! 		 * If a TOAST oid is not specified, skip TOAST creation as we will do
! 		 * it later so we don't create a TOAST table whose OID later conflicts
! 		 * with a user-supplied OID.  This handles cases where the old cluster
! 		 * didn't need a TOAST table, but the new cluster does.
! 		 */
! 		if (!OidIsValid(binary_upgrade_next_toast_pg_class_oid))
! 			return false;
! 
! 		/*
! 		 * If a special TOAST value has been passed in, it means we are in
! 		 * cleanup mode --- we are creating needed TOAST tables after all user
! 		 * tables with specified OIDs have been created.  We let the system
! 		 * assign a TOAST oid for us.  The tables are empty so the missing
! 		 * TOAST tables were not a problem.
! 		 */
! 		if (binary_upgrade_next_toast_pg_class_oid == OPTIONALLY_CREATE_TOAST_OID)
! 		{
! 			/* clear it so it is no longer used */
! 			binary_upgrade_next_toast_pg_class_oid = InvalidOid;
! 
! 			if (!needs_toast_table(rel))
! return false;
! 		}
! 	}
  
  	/*
  	 * If requested check lockmode is sufficient. This is a cross check in
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
new file mode 100644
index f39017c..7aff509
*** a/src/include/catalog/binary_upgrade.h
--- b/src/include/catalog/binary_upgrade.h
***
*** 14,19 
--- 14,22 
  #ifndef BINARY_UPGRADE_H
  #define BINARY_UPGRADE_H
  
+ /* pick a OID that will never be normally used for TOAST table */
+ #define OPTIONALLY_CREATE_TOAST_OID	1
+ 
  extern PGDLLIMPORT Oid binary_upgrade_next_pg_type_oid;
  extern PGDLLIMPORT Oid binary_upgrade_next_array_pg_type_oid;
  extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_type_oid;

-- 
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] how many changes about backend mode from 7.2.2 to 8.4.0?

2014-07-10 Thread 土卜皿
hi, tom
  thank you very much! this is what I wanted!

Dillon


2014-07-11 10:59 GMT+08:00 Tom Lane :

> =?UTF-8?B?5Zyf5Y2c55q/?=  writes:
> > I am sorry, maybe I should make my question clearer, I only want to know,
> > in 8.4.0 or newer version,
> >  whether I can debug posgres in the bare backend mode (in
>  usingddd-postgres
> > , he said
> that "
> > There are two ways to debug postgres (a) in the interactive mode and (b)
> in
> > the bare backend mode"
>
> Yeah, that still works as well or poorly as it ever did, though the
> startup process is a bit different: you need to say "postgres --single"
> to launch a standalone backend.
>
> But really, nobody does it that way anymore.  There is no advantage to it,
> unless maybe you're trying to debug a startup-time crash.  The standalone
> backend interface is very unpleasant to use compared to psql: no readline
> command editing, no nice formatting of output, no tab completion or help,
> etc etc.  For debugging of normal query operation it's far better to fire
> up a psql session and then gdb the connected backend process.
>
> There's more info here:
> https://wiki.postgresql.org/wiki/Developer_FAQ#gdb
>
> and if gdb isn't giving you useful symbolic information, see
> the setup tips here:
>
> https://wiki.postgresql.org/wiki/Getting_a_stack_trace_of_a_running_PostgreSQL_backend_on_Linux/BSD
>
> regards, tom lane
>


Re: [HACKERS] how many changes about backend mode from 7.2.2 to 8.4.0?

2014-07-10 Thread Tom Lane
=?UTF-8?B?5Zyf5Y2c55q/?=  writes:
> I am sorry, maybe I should make my question clearer, I only want to know,
> in 8.4.0 or newer version,
>  whether I can debug posgres in the bare backend mode (in  usingddd-postgres
> , he said that "
> There are two ways to debug postgres (a) in the interactive mode and (b) in
> the bare backend mode"

Yeah, that still works as well or poorly as it ever did, though the
startup process is a bit different: you need to say "postgres --single"
to launch a standalone backend.

But really, nobody does it that way anymore.  There is no advantage to it,
unless maybe you're trying to debug a startup-time crash.  The standalone
backend interface is very unpleasant to use compared to psql: no readline
command editing, no nice formatting of output, no tab completion or help,
etc etc.  For debugging of normal query operation it's far better to fire
up a psql session and then gdb the connected backend process.

There's more info here:
https://wiki.postgresql.org/wiki/Developer_FAQ#gdb

and if gdb isn't giving you useful symbolic information, see
the setup tips here:
https://wiki.postgresql.org/wiki/Getting_a_stack_trace_of_a_running_PostgreSQL_backend_on_Linux/BSD

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] how many changes about backend mode from 7.2.2 to 8.4.0?

2014-07-10 Thread 土卜皿
hi,
  thanks a lot!


2014-07-11 10:10 GMT+08:00 Michael Paquier :

> On Fri, Jul 11, 2014 at 10:50 AM, 土卜皿  wrote:
> > NOTE: fedora 17 x86_64, 7.2.2 can not be compiled in the env, I choose
> 8.4.0
> > because I think old version is easier to understand than newer version!
> Are you aware that Postgres 7.2 has been released in 2002? It is EOL
> (end-of-life) since 2005 by looking at the release notes.
>
I know the released time.


>
> > I want to study pg's external sort (in tuplesort.c )through 8.4.0's
> source
> > code, and use ddd to do it, according to usingddd-postgres 's
> description:
> If you are planning to do some development in the future with or for
> Postgres, you would get a better insight by looking at more recent
> code. Here are some guidelines for example to use git, which is really
> helpful as a base infrastructure when studying the code:
> https://wiki.postgresql.org/wiki/Working_with_GIT
>
> seemly the wiki you said has no information about debug

I am sorry, maybe I should make my question clearer, I only want to know,
in 8.4.0 or newer version,
 whether I can debug posgres in the bare backend mode (in  usingddd-postgres
, he said that "
There are two ways to debug postgres (a) in the interactive mode and (b) in
the bare backend mode"
), if yes, what should I do?  thanks in advance!

BEST REGARDS
Dillon


Re: [HACKERS] how many changes about backend mode from 7.2.2 to 8.4.0?

2014-07-10 Thread Michael Paquier
On Fri, Jul 11, 2014 at 10:50 AM, 土卜皿  wrote:
> NOTE: fedora 17 x86_64, 7.2.2 can not be compiled in the env, I choose 8.4.0
> because I think old version is easier to understand than newer version!
Are you aware that Postgres 7.2 has been released in 2002? It is EOL
(end-of-life) since 2005 by looking at the release notes.

> I want to study pg's external sort (in tuplesort.c )through 8.4.0's source
> code, and use ddd to do it, according to usingddd-postgres 's description:
If you are planning to do some development in the future with or for
Postgres, you would get a better insight by looking at more recent
code. Here are some guidelines for example to use git, which is really
helpful as a base infrastructure when studying the code:
https://wiki.postgresql.org/wiki/Working_with_GIT

Hope this helps.
Regards,
-- 
Michael


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


[HACKERS] how many changes about backend mode from 7.2.2 to 8.4.0?

2014-07-10 Thread 土卜皿
hi, all
NOTE: fedora 17 x86_64, 7.2.2 can not be compiled in the env, I choose
8.4.0 because I think old version is easier to understand than newer
version!

I want to study pg's external sort (in tuplesort.c )through 8.4.0's source
code, and use ddd to do it, according to usingddd-postgres
 's description:

 In the backend mode, you don’t create a separate postmaster process and
don’t use psql at all. Instead, you start up postgres from the
command line and directly interact with it.

and after starting  postgres in ddd, a backend interactive interface works
as doc's Figure 5, and I found version 7.2.2's source postgres'c has
backend interactive interface, but my version is 8.4.0, which seemly has
not backend interactive interface, so I want to ask

(1) whether I must use another method to debug relative external sort code
(in tuplesort.c ), as some post said that I can execute "psql, select select
pg_backend_pid(), and gdb pid"  , any advice will be appreciated!

BEST REGARDS
Dillon Peng


Re: [HACKERS] Allowing NOT IN to use ANTI joins

2014-07-10 Thread Tom Lane
I wrote:
> We could no doubt fix this by also insisting that the left-side vars
> be provably not null, but that's going to make the patch even slower
> and even less often applicable.  I'm feeling discouraged about whether
> this is worth doing in this form.

Hm ... actually, there might be a better answer: what about transforming

   WHERE (x,y) NOT IN (SELECT provably-not-null-values FROM ...)

to

   WHERE  AND x IS NOT NULL AND y IS NOT NULL

?

Of course this would require x/y not being volatile, but if they are,
we're not going to get far with optimizing the query anyhow.

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] Allowing NOT IN to use ANTI joins

2014-07-10 Thread Tom Lane
David Rowley  writes:
> Attached is the updated version of the patch.

I spent some time fooling with this patch, cleaning up the join-alias
issue as well as more-cosmetic things.  However, while testing it
I realized that the whole thing is based on a false premise: to equate
a NOT IN with an antijoin, we'd have to prove not only that the inner
side is non-nullable, but that the outer comparison values are too.
Here's an example:

regression=# create table zt1 (f1 int);
CREATE TABLE
regression=# insert into zt1 values(1);
INSERT 0 1
regression=# insert into zt1 values(2);
INSERT 0 1
regression=# insert into zt1 values(null);
INSERT 0 1
regression=# create table zt2 (f1 int not null);
CREATE TABLE
regression=# insert into zt2 values(1);
INSERT 0 1

With the patch in place, we get

regression=# explain select * from zt1 where f1 not in (select f1 from zt2);
QUERY PLAN 
---
 Hash Anti Join  (cost=64.00..144.80 rows=1200 width=4)
   Hash Cond: (zt1.f1 = zt2.f1)
   ->  Seq Scan on zt1  (cost=0.00..34.00 rows=2400 width=4)
   ->  Hash  (cost=34.00..34.00 rows=2400 width=4)
 ->  Seq Scan on zt2  (cost=0.00..34.00 rows=2400 width=4)
 Planning time: 0.646 ms
(6 rows)

regression=# select * from zt1 where f1 not in (select f1 from zt2);
 f1 

  2
   
(2 rows)

which is the wrong answer, as demonstrated by comparison to the result
without optimization:

regression=# alter table zt2 alter column f1 drop not null;
ALTER TABLE
regression=# explain select * from zt1 where f1 not in (select f1 from zt2);
  QUERY PLAN   
---
 Seq Scan on zt1  (cost=40.00..80.00 rows=1200 width=4)
   Filter: (NOT (hashed SubPlan 1))
   SubPlan 1
 ->  Seq Scan on zt2  (cost=0.00..34.00 rows=2400 width=4)
 Planning time: 0.163 ms
(5 rows)

regression=# select * from zt1 where f1 not in (select f1 from zt2);
 f1 

  2
(1 row)

We could no doubt fix this by also insisting that the left-side vars
be provably not null, but that's going to make the patch even slower
and even less often applicable.  I'm feeling discouraged about whether
this is worth doing in this form.

For the archives' sake, I attach an updated version with the fixes
I'd applied so far.

regards, tom lane

diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 3e7dc85..4b44662 100644
*** a/src/backend/optimizer/plan/subselect.c
--- b/src/backend/optimizer/plan/subselect.c
*** SS_process_ctes(PlannerInfo *root)
*** 1195,1205 
   * If so, form a JoinExpr and return it.  Return NULL if the SubLink cannot
   * be converted to a join.
   *
!  * The only non-obvious input parameter is available_rels: this is the set
!  * of query rels that can safely be referenced in the sublink expression.
!  * (We must restrict this to avoid changing the semantics when a sublink
!  * is present in an outer join's ON qual.)  The conversion must fail if
!  * the converted qual would reference any but these parent-query relids.
   *
   * On success, the returned JoinExpr has larg = NULL and rarg = the jointree
   * item representing the pulled-up subquery.  The caller must set larg to
--- 1195,1208 
   * If so, form a JoinExpr and return it.  Return NULL if the SubLink cannot
   * be converted to a join.
   *
!  * If under_not is true, the caller actually found NOT (ANY SubLink),
!  * so that what we must try to build is an ANTI not SEMI join.
!  *
!  * available_rels is the set of query rels that can safely be referenced
!  * in the sublink expression.  (We must restrict this to avoid changing the
!  * semantics when a sublink is present in an outer join's ON qual.)
!  * The conversion must fail if the converted qual would reference any but
!  * these parent-query relids.
   *
   * On success, the returned JoinExpr has larg = NULL and rarg = the jointree
   * item representing the pulled-up subquery.  The caller must set larg to
*** SS_process_ctes(PlannerInfo *root)
*** 1222,1228 
   */
  JoinExpr *
  convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
! 			Relids available_rels)
  {
  	JoinExpr   *result;
  	Query	   *parse = root->parse;
--- 1225,1231 
   */
  JoinExpr *
  convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
! 			bool under_not, Relids available_rels)
  {
  	JoinExpr   *result;
  	Query	   *parse = root->parse;
*** convert_ANY_sublink_to_join(PlannerInfo 
*** 1237,1242 
--- 1240,1254 
  	Assert(sublink->subLinkType == ANY_SUBLINK);
  
  	/*
+ 	 * Per SQL spec, NOT IN is not ordinarily equivalent to an anti-join, so
+ 	 * that by default we have to fail when under_not.  However, if we can
+ 	 * prove that the sub-select's output columns are all certainly not NULL,
+ 	 * then it's 

Re: [HACKERS] IMPORT FOREIGN SCHEMA statement

2014-07-10 Thread Michael Paquier
On Fri, Jul 11, 2014 at 4:06 AM, Tom Lane  wrote:

> I wrote:
> > So I propose we invent a couple more import options, say
> > import_default and import_collate, and make the postgres_fdw
> > code do the obvious thing with them.  import_default should
> > probably default to false, but I'm about halfway tempted to
> > say that import_collate should default to true --- if you're
> > importing a collatable column and you don't have a matching
> > locale locally, it seems like it'd be better if we complain
> > about that by default.
>
> I've committed this patch with that addition and some minor additional
> cleanup.
>
Thanks!
-- 
Michael


Re: [HACKERS] Missing autocomplete for CREATE DATABASE

2014-07-10 Thread David Fetter
On Thu, Jul 10, 2014 at 08:32:01PM +0100, Magnus Hagander wrote:
> It seems psql is missing autocomplete entries for LC_COLLATE and
> LC_CTYPE for the CREATE DATABASE command. Attached patch adds that.
> 
> I doubt this is important enough to backpatch - thoughts?

I don't see how it could hurt to fix this bug.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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_upgrade and toast tables bug discovered

2014-07-10 Thread Bruce Momjian
On Thu, Jul 10, 2014 at 06:17:14PM -0400, Bruce Momjian wrote:
> Well, we are going to need to call internal C functions, often bypassing
> their typical call sites and the assumption about locking, etc.  Perhaps
> this could be done from a plpgsql function.  We could add and drop a
> dummy column to force TOAST table creation --- we would then only need a
> way to detect if a function _needs_ a TOAST table, which was skipped in
> binary upgrade mode previously.
> 
> That might be a minimalistic approach.

I have thought some more on this.  I thought I would need to open
pg_class in C and do complex backend stuff, but I now realize I can do
it from libpq, and just call ALTER TABLE and I think that always
auto-checks if a TOAST table is needed.  All I have to do is query
pg_class from libpq, then construct ALTER TABLE commands for each item,
and it will optionally create the TOAST table if needed.  I just have to
use a no-op ALTER TABLE command, like SET STATISTICS.

I am in Asia the next two weeks but will work on it after I return.

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

  + Everyone has their own god. +


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Greg Stark
On Thu, Jul 10, 2014 at 10:29 PM, Alvaro Herrera
 wrote:
>
> What I think should happen is that if the value is changed, the index
> sholud be rebuilt right there.

I disagree. It would be a non-orthogonal interface if ALTER TABLE
sometimes causes the index to be rebuilt and sometimes just makes a
configuration change. I already see a lot of user confusion when some
ALTER TABLE commands rewrite the table and some are quick meta data
changes.

Especially in this case where the type of configuration being changed
is just an internal storage parameter and the user visible shape of
the index is unchanged it would be weird to rebuild the index.

IMHO the "right" thing to do is just to say this parameter is
read-only and have the AM throw an error when the user changes it. But
even that would require an AM callback for the AM to even know about
the change.

-- 
greg


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Jeff Janes
On Thu, Jul 10, 2014 at 2:30 PM, Jaime Casanova 
wrote:

> On Thu, Jul 10, 2014 at 3:50 PM, Josh Berkus  wrote:
> > On 07/10/2014 12:20 PM, Alvaro Herrera wrote:
> >>> So I guess the only thing left is to issue a NOTICE when said alter
> >>> > takes place (I don't see that on the patch, but maybe it's there?)
> >> That's not in the patch.  I don't think we have an appropriate place to
> >> emit such a notice.
> >
> > What do you mean by "don't have an appropriate place"?
> >
> > The suggestion is that when a user does:
> >
> > ALTER INDEX foo_minmax SET PAGES_PER_RANGE=100
> >
> > they should get a NOTICE:
> >
> > "NOTICE: changes to pages per range will not take effect until the index
> > is REINDEXed"
> >
> > otherwise, we're going to get a lot of "I Altered the pages per range,
> > but performance didn't change" emails.
> >
>
> How is this different from "ALTER TABLE foo SET (FILLFACTOR=80); " or
> from "ALTER TABLE foo ALTER bar SET STORAGE EXTERNAL; " ?
>
> we don't get a notice for these cases either
>


I think those are different.  They don't rewrite existing data in the
table, but they are applied to new (and updated) data. My understanding is
that changing  PAGES_PER_RANGE will have no effect on future data until a
re-index is done, even if the entire table eventually turns over.

Cheers,

Jeff


Re: [HACKERS] Pg_upgrade and toast tables bug discovered

2014-07-10 Thread Bruce Momjian
On Thu, Jul 10, 2014 at 11:11:19PM +0200, Andres Freund wrote:
> On 2014-07-10 16:33:40 -0400, Bruce Momjian wrote:
> > On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote:
> > > > Agreed.  I am now thinking we could harness the code that already exists
> > > > to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN.  We
> > > > would just need an entry point to call it from pg_upgrade, either via an
> > > > SQL command that checks (and hopefully doesn't do anything else), or a C
> > > > function that does it, e.g. VACUUM would be trivial to run on every
> > > > database, but I don't think it tests that;  is _could_ in binary_upgrade
> > > > mode.  However, the idea of having a C function plug into the guts of
> > > > the server and call internal functions makes me uncomforable.
> > > 
> > > Well, pg_upgrade_support's charter is basically to provide access to
> > > the guts of the server in ways we wouldn't normally allow; all that
> > > next-OID stuff is basically exactly that.  So I don't think this is
> > > such a big deal.  It needs to be properly commented, of course.
> > 
> > If you look at how oid assignment is handled, it is done in a very
> > surgical way, i.e. pg_upgrade_support sets a global variable, and the
> > variable triggers different behavior in a CREATE command.  This change
> > would be far more invasive than that.
> 
> Meh. It's only somewhat surgical because there's pg_upgrade specific
> code sprinkled in the backend at strategic places. That's the contrary
> of a clear abstraction barrier. And arguably a function call from a SQL
> C function has a much clearer abstraction barrier.

Well, we are going to need to call internal C functions, often bypassing
their typical call sites and the assumption about locking, etc.  Perhaps
this could be done from a plpgsql function.  We could add and drop a
dummy column to force TOAST table creation --- we would then only need a
way to detect if a function _needs_ a TOAST table, which was skipped in
binary upgrade mode previously.

That might be a minimalistic approach.

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

  + Everyone has their own god. +


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


Re: [HACKERS] Missing autocomplete for CREATE DATABASE

2014-07-10 Thread Vik Fearing
On 07/10/2014 09:32 PM, Magnus Hagander wrote:
> It seems psql is missing autocomplete entries for LC_COLLATE and
> LC_CTYPE for the CREATE DATABASE command. Attached patch adds that.
> 
> I doubt this is important enough to backpatch - thoughts?

It won't apply to current head, but otherwise I see no problem with it.

I have no opinion on backpatching it.
-- 
Vik


-- 
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] Minmax indexes

2014-07-10 Thread Josh Berkus
On 07/10/2014 02:30 PM, Jaime Casanova wrote:
> How is this different from "ALTER TABLE foo SET (FILLFACTOR=80); " or
> from "ALTER TABLE foo ALTER bar SET STORAGE EXTERNAL; " ?
> 
> we don't get a notice for these cases either

Good idea.  We should also emit notices for those.  Well, maybe not for
fillfactor.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://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] Proposal for CSN based snapshots

2014-07-10 Thread Alvaro Herrera
Heikki Linnakangas wrote:

> Attached is a new patch. It now keeps the current pg_clog unchanged,
> but adds a new pg_csnlog besides it. pg_csnlog is more similar to
> pg_subtrans than pg_clog: it's not WAL-logged, is reset at startup,
> and segments older than GlobalXmin can be truncated.
> 
> This addresses the disk space consumption, and simplifies pg_upgrade.
> 
> There are no other significant changes in this new version, so it's
> still very much WIP. But please take a look!

Thanks.  I've been looking at this.  It needs a rebase; I had to apply
to commit 89cf2d52030 (Wed Jul 2 13:11:05 2014 -0400) because of
conflicts with the commit after that; it applies with some fuzz.  I read
the discussion in this thread and the README, to try to understand how
this is supposed to work.  It looks pretty good.

One thing not quite clear to me is the now-static RecentXmin,
GetRecentXmin(), AdvanceGlobalRecentXmin, and the like.  I found the
whole thing pretty confusing.  I noticed that ShmemVariableCache->recentXmin
is read without a lock in GetSnapshotData(), but exclusive ProcArrayLock
is taken to write to it in GetRecentXmin().  I think we can write it
without a lock also, since we assume that storing an xid is atomic.
With that change, I think you can make the acquisition of ProcArray Lock
to walk the pgproc list use shared mode, not exclusive.

Another point is that RecentXmin is no longer used directly, except in
TransactionIdIsActive().  But that routine is only used for an Assert()
now, so I think it's fine to just have GetRecentXmin() return the value
and not set RecentXmin as a side effect; my point is that ISTM
RecentXmin can be removed altogether, which makes that business simpler.


As far as GetOldestXmin() is concerned, that routine now ignores its
arguments.  Is that okay?  I imagine it's just a natural consequence of
how things work now.   [ ... reads some more code ... ]  Oh, now it's
only used to determine a freeze limit.  I think you should just remove
the arguments and make that whole thing simpler.  I was going to suggest
that AdvanceRecentGlobalXmin() could receive a possibly-NULL Relation
argument to pass down to GetOldestSnapshotGuts() which can make use of
it for a tigther limit, but since OldestXmin is only for freezing, maybe
there is no point in this extra complication.

Regarding the pendingGlobalXmin stuff, I didn't find any documentation.
I think you just need to write a very long comment on top of
AdvanceRecentGlobalXmin() to explain what it's doing.  After going over
the code half a dozen times I think I understand what's idea, and it
seems sound.

Not sure about the advancexmin_counter thing.  Maybe in some cases
sessions will only run 9 transactions or less and then finish, in which
case it will only get advanced at checkpoint time, which would be way
too seldom.  Maybe it would work to place the counter in shared memory:

acquire(spinlock);
if (ShmemVariableCache->counter++ >= some_number)
{
   SI don't understand this:hmemVariableCache->counter = 0;
   do_update = true;
}
release(spinlock);
if (do_update)
   AdvanceRecentGlobalXmin();
(Maybe we can forgo the spinlock altogether?  If the value gets out of
hand once in a while, it shouldn't really matter)  Perhaps the "some
number" should be scaled to some fraction of MaxBackends, but not less
than X (32?) and no more than Y (128?).  Or maybe just use constant 10,
as the current code does.  But it'd be total number of transactions, not
transaction in the current backend.


I think it'd be cleaner to have a distinct typedef to use when
XLogRecPtr is used as a snapshot representation.  Right now there is no
clarity on whether we're interested in the xlog position itself or it's
a snapshot value.


HeapTupleSatisfiesVacuum[X]() and various callers needs update to their
comments: when OldestXmin is mentioned, it should be OldestSnapshot.

I noticed that SubTransGetTopmostTransaction() is now only called from
predicate.c, and it's pretty constrained in what it wants.  I'm not
implying that we want to do anything in your patch about it, other than
perhaps add a note to it that we may want to examine it later for
possible changes.

I haven't gone over the transam.c, clog.c changes yet.

I attach a couple of minor tweaks to the README, mostly for clarity (but
also update clog -> csnlog in a couple of places).

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README
index d11c817..23479b6 100644
--- a/src/backend/access/transam/README
+++ b/src/backend/access/transam/README
@@ -247,24 +247,24 @@ What we actually enforce is strict serialization of commits and rollbacks
 with snapshot-taking. We use the LSNs generated by Write-Ahead-Logging as
 a convenient monotonically increasing counter, to serialize commits with
 snapshots. Each commit is naturally assigned an LSN; it's the LSN of the
-comm

Re: [HACKERS] Minmax indexes

2014-07-10 Thread Alvaro Herrera
Josh Berkus wrote:
> On 07/10/2014 12:20 PM, Alvaro Herrera wrote:
> >> So I guess the only thing left is to issue a NOTICE when said alter
> >> > takes place (I don't see that on the patch, but maybe it's there?)
> > That's not in the patch.  I don't think we have an appropriate place to
> > emit such a notice.
> 
> What do you mean by "don't have an appropriate place"?

What I think should happen is that if the value is changed, the index
sholud be rebuilt right there.  But there is no way to have this occur
from the generic tablecmds.c code.  Maybe we should extend the AM
interface so that they are notified of changes and can take action.
Inserting AM-specific code into tablecmds.c seems pretty wrong to me --
existing stuff for WITH CHECK OPTION views notwithstanding.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Jaime Casanova
On Thu, Jul 10, 2014 at 3:50 PM, Josh Berkus  wrote:
> On 07/10/2014 12:20 PM, Alvaro Herrera wrote:
>>> So I guess the only thing left is to issue a NOTICE when said alter
>>> > takes place (I don't see that on the patch, but maybe it's there?)
>> That's not in the patch.  I don't think we have an appropriate place to
>> emit such a notice.
>
> What do you mean by "don't have an appropriate place"?
>
> The suggestion is that when a user does:
>
> ALTER INDEX foo_minmax SET PAGES_PER_RANGE=100
>
> they should get a NOTICE:
>
> "NOTICE: changes to pages per range will not take effect until the index
> is REINDEXed"
>
> otherwise, we're going to get a lot of "I Altered the pages per range,
> but performance didn't change" emails.
>

How is this different from "ALTER TABLE foo SET (FILLFACTOR=80); " or
from "ALTER TABLE foo ALTER bar SET STORAGE EXTERNAL; " ?

we don't get a notice for these cases either

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
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_upgrade and toast tables bug discovered

2014-07-10 Thread Andres Freund
On 2014-07-10 16:33:40 -0400, Bruce Momjian wrote:
> On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote:
> > > Agreed.  I am now thinking we could harness the code that already exists
> > > to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN.  We
> > > would just need an entry point to call it from pg_upgrade, either via an
> > > SQL command that checks (and hopefully doesn't do anything else), or a C
> > > function that does it, e.g. VACUUM would be trivial to run on every
> > > database, but I don't think it tests that;  is _could_ in binary_upgrade
> > > mode.  However, the idea of having a C function plug into the guts of
> > > the server and call internal functions makes me uncomforable.
> > 
> > Well, pg_upgrade_support's charter is basically to provide access to
> > the guts of the server in ways we wouldn't normally allow; all that
> > next-OID stuff is basically exactly that.  So I don't think this is
> > such a big deal.  It needs to be properly commented, of course.
> 
> If you look at how oid assignment is handled, it is done in a very
> surgical way, i.e. pg_upgrade_support sets a global variable, and the
> variable triggers different behavior in a CREATE command.  This change
> would be far more invasive than that.

Meh. It's only somewhat surgical because there's pg_upgrade specific
code sprinkled in the backend at strategic places. That's the contrary
of a clear abstraction barrier. And arguably a function call from a SQL
C function has a much clearer abstraction barrier.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Josh Berkus
On 07/10/2014 12:20 PM, Alvaro Herrera wrote:
>> So I guess the only thing left is to issue a NOTICE when said alter
>> > takes place (I don't see that on the patch, but maybe it's there?)
> That's not in the patch.  I don't think we have an appropriate place to
> emit such a notice.

What do you mean by "don't have an appropriate place"?

The suggestion is that when a user does:

ALTER INDEX foo_minmax SET PAGES_PER_RANGE=100

they should get a NOTICE:

"NOTICE: changes to pages per range will not take effect until the index
is REINDEXed"

otherwise, we're going to get a lot of "I Altered the pages per range,
but performance didn't change" emails.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://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] Pg_upgrade and toast tables bug discovered

2014-07-10 Thread Bruce Momjian
On Thu, Jul 10, 2014 at 10:46:30AM -0400, Robert Haas wrote:
> > Agreed.  I am now thinking we could harness the code that already exists
> > to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN.  We
> > would just need an entry point to call it from pg_upgrade, either via an
> > SQL command that checks (and hopefully doesn't do anything else), or a C
> > function that does it, e.g. VACUUM would be trivial to run on every
> > database, but I don't think it tests that;  is _could_ in binary_upgrade
> > mode.  However, the idea of having a C function plug into the guts of
> > the server and call internal functions makes me uncomforable.
> 
> Well, pg_upgrade_support's charter is basically to provide access to
> the guts of the server in ways we wouldn't normally allow; all that
> next-OID stuff is basically exactly that.  So I don't think this is
> such a big deal.  It needs to be properly commented, of course.

If you look at how oid assignment is handled, it is done in a very
surgical way, i.e. pg_upgrade_support sets a global variable, and the
variable triggers different behavior in a CREATE command.  This change
would be far more invasive than that.

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

  + Everyone has their own god. +


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


Re: [HACKERS] TODO : Allow parallel cores to be used by vacuumdb [ WIP ]

2014-07-10 Thread Jeff Janes
On Fri, Jun 27, 2014 at 4:10 AM, Dilip kumar  wrote:
> On 27 June 2014 02:57, Jeff Wrote,
>
>> Based on that, I find most importantly that it doesn't seem to
>> correctly vacuum tables which have upper case letters in the name,
>> because it does not quote the table names when they need quotes.
>
> Thanks for your comments
>
> There are two problem
> First -> When doing the vacuum of complete database that time if any table 
> with upper case letter, it was giving error
> --FIXED by adding quotes for table name
>
> Second -> When user pass the table using -t option, and if it has uppercase 
> letter
> --This is the existing problem (without parallel implementation),

Just for the record, I don't think the second one is actually a bug.
If someone uses -t option from the command line, they are required to
provide the quotes if quotes are needed, just like they would need to
in psql.  That can be annoying to do from a shell, as you then need to
protect the quotes themselves from the shell, but that is the way it
is.

vacuumdb -t '"CrAzY QuOtE"'
or
vacuumdb -t \"CrAzY\ QuOtE\"

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


Re: [HACKERS] tweaking NTUP_PER_BUCKET

2014-07-10 Thread Tomas Vondra
On 9.7.2014 16:07, Robert Haas wrote:
> On Tue, Jul 8, 2014 at 5:16 PM, Tomas Vondra  wrote:
>> Thinking about this a bit more, do we really need to build the hash
>> table on the first pass? Why not to do this:
>>
>> (1) batching
>> - read the tuples, stuff them into a simple list
>> - don't build the hash table yet
>>
>> (2) building the hash table
>> - we have all the tuples in a simple list, batching is done
>> - we know exact row count, can size the table properly
>> - build the table
> 
> We could do this, and in fact we could save quite a bit of memory if
> we allocated say 1MB chunks and packed the tuples in tightly instead
> of palloc-ing each one separately.  But I worry that rescanning the
> data to build the hash table would slow things down too much.

I did a quick test of how much memory we could save by this. The
attached patch densely packs the tuples into 32kB chunks (1MB seems way
too much because of small work_mem values, but I guess this might be
tuned based on number of tuples / work_mem size ...).

Tested on query like this (see the first message in this thread how to
generate the tables):

 QUERY PLAN
---
 Aggregate  (cost=2014697.64..2014697.65 rows=1 width=33) (actual
time=63796.270..63796.271 rows=1 loops=1)
   ->  Hash Left Join  (cost=318458.14..1889697.60 rows=5016
width=33) (actual time=2865.656..61778.592 rows=5000 loops=1)
 Hash Cond: (o.id = i.id)
 ->  Seq Scan on outer_table o  (cost=0.00..721239.16
rows=5016 width=4) (actual time=0.033..2676.234 rows=5000 loops=1)
 ->  Hash  (cost=193458.06..193458.06 rows=1006 width=37)
(actual time=2855.408..2855.408 rows=1000 loops=1)
   Buckets: 1048576  Batches: 1  Memory Usage: 703125kB
   ->  Seq Scan on inner_table i  (cost=0.00..193458.06
rows=1006 width=37) (actual time=0.044..952.802 rows=1000 loops=1)
 Planning time: 1.139 ms
 Execution time: 63889.056 ms
(9 rows)

I.e. it creates a single batch with ~700 MB of tuples. Without the
patch, top shows this:

 VIRTRESSHR S  %CPU %MEM   COMMAND
  2540356 1,356g   5936 R 100,0 17,6   postgres: EXPLAIN

and the MemoryContextStats added to MultiExecHash shows this:

HashBatchContext: 1451221040 total in 182 blocks; 2826592 free (11
chunks); 1448394448 used

So yeah, the overhead is pretty huge in this case - basicaly 100%
overhead, because the inner table row width is only ~40B. With wider
rows the overhead will be lower.

Now, with the patch it looks like this:

 VIRTRESSHR S  %CPU %MEM   COMMAND
  1835332 720200   6096 R 100,0  8,9   postgres: EXPLAIN

HashBatchContext: 729651520 total in 21980 blocks; 0 free (0 chunks);
729651520 used

So, pretty much no overhead at all. It was slightly faster too (~5%) but
I haven't done much testing so it might be measurement error.

This patch is pretty independent of the other changes discussed here
(tweaking NTUP_PER_BUCKET / nbuckets) so I'll keep it separate.

regards
Tomas
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 589b2f1..18fd4a9 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -47,6 +47,7 @@ static void ExecHashSkewTableInsert(HashJoinTable hashtable,
 		int bucketNumber);
 static void ExecHashRemoveNextSkewBucket(HashJoinTable hashtable);
 
+static char * chunk_alloc(HashJoinTable hashtable, int tupleSize);
 
 /* 
  *		ExecHash
@@ -130,6 +131,9 @@ MultiExecHash(HashState *node)
 	if (node->ps.instrument)
 		InstrStopNode(node->ps.instrument, hashtable->totalTuples);
 
+	/* print some context stats */
+	MemoryContextStats(hashtable->batchCxt);
+	
 	/*
 	 * We do not return the hash table directly because it's not a subtype of
 	 * Node, and so would violate the MultiExecProcNode API.  Instead, our
@@ -223,6 +227,8 @@ ExecEndHash(HashState *node)
 	ExecEndNode(outerPlan);
 }
 
+/* 32kB chunks by default */
+#define CHUNK_SIZE	(32*1024L)
 
 /* 
  *		ExecHashTableCreate
@@ -294,6 +300,10 @@ ExecHashTableCreate(Hash *node, List *hashOperators, bool keepNulls)
 	hashtable->spaceAllowedSkew =
 		hashtable->spaceAllowed * SKEW_WORK_MEM_PERCENT / 100;
 
+	hashtable->chunk_data = NULL;
+	hashtable->chunk_used = 0;
+	hashtable->chunk_length = 0;
+
 	/*
 	 * Get info about the hash functions to be used for each hash key. Also
 	 * remember whether the join operators are strict.
@@ -717,8 +727,8 @@ ExecHashTableInsert(HashJoinTable hashtable,
 
 		/* Create the HashJoinTuple */
 		hashTupleSize = HJTUPLE_OVERHEAD + tuple->t_len;
-		hashTuple = (HashJoinTuple) MemoryContextAlloc(hashtable->batchCxt,
-	   hashTupleSize);
+		hashTuple = (HashJoinTuple) chunk_alloc(hashtable, hashTupleSize);
+
 		hashTuple->hashvalue = 

[HACKERS] Missing autocomplete for CREATE DATABASE

2014-07-10 Thread Magnus Hagander
It seems psql is missing autocomplete entries for LC_COLLATE and
LC_CTYPE for the CREATE DATABASE command. Attached patch adds that.

I doubt this is important enough to backpatch - thoughts?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3bb727f..f746ddc 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2046,7 +2046,7 @@ psql_completion(const char *text, int start, int end)
 	{
 		static const char *const list_DATABASE[] =
 		{"OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "CONNECTION LIMIT",
-		NULL};
+		 "LC_COLLATE", "LC_CTYPE", NULL};
 
 		COMPLETE_WITH_LIST(list_DATABASE);
 	}

-- 
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] Minmax indexes

2014-07-10 Thread Alvaro Herrera
Claudio Freire wrote:
> On Wed, Jul 9, 2014 at 6:16 PM, Alvaro Herrera  
> wrote:
> > Another thing I noticed is that version 8 of the patch blindly believed
> > the "pages_per_range" declared in catalogs.  This meant that if somebody
> > did "alter index foo set pages_per_range=123" the index would
> > immediately break (i.e. return corrupted results when queried).  I have
> > fixed this by storing the pages_per_range value used to construct the
> > index in the metapage.  Now if you do the ALTER INDEX thing, the new
> > value is only used when the index is recreated by REINDEX.
> 
> This seems a lot like parameterizing.

I don't understand what that means -- care to elaborate?

> So I guess the only thing left is to issue a NOTICE when said alter
> takes place (I don't see that on the patch, but maybe it's there?)

That's not in the patch.  I don't think we have an appropriate place to
emit such a notice.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] IMPORT FOREIGN SCHEMA statement

2014-07-10 Thread Tom Lane
I wrote:
> So I propose we invent a couple more import options, say
> import_default and import_collate, and make the postgres_fdw
> code do the obvious thing with them.  import_default should
> probably default to false, but I'm about halfway tempted to
> say that import_collate should default to true --- if you're
> importing a collatable column and you don't have a matching
> locale locally, it seems like it'd be better if we complain
> about that by default.

I've committed this patch with that addition and some minor additional
cleanup.

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] pgindent weirdness

2014-07-10 Thread Bruce Momjian
On Thu, Jul 10, 2014 at 12:02:50PM -0400, Robert Haas wrote:
> In contrib/test_shm_mq/test.c, the 9.4 pgindent run
> (0a7832005792fa6dad171f9cadb8d587fe0dd800) did this:
> 
> -PG_MODULE_MAGIC;
> -PG_FUNCTION_INFO_V1(test_shm_mq);
> +PG_MODULE_MAGIC; PG_FUNCTION_INFO_V1(test_shm_mq);
>  PG_FUNCTION_INFO_V1(test_shm_mq_pipelined);
> 
> This is obviously not an improvement.  Is there some formatting rule
> that I violated in the original code that lead to this, or what?

Uh, ever other case of PG_MODULE_MAGIC had blank lines before/after this
define.  I went and added that to test_shm_mq/test.c, and adjusted other
blank lines to be consistent.  I did not modify 9.4 as this cosmetic.

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

  + Everyone has their own god. +


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Claudio Freire
On Wed, Jul 9, 2014 at 6:16 PM, Alvaro Herrera  wrote:
> Another thing I noticed is that version 8 of the patch blindly believed
> the "pages_per_range" declared in catalogs.  This meant that if somebody
> did "alter index foo set pages_per_range=123" the index would
> immediately break (i.e. return corrupted results when queried).  I have
> fixed this by storing the pages_per_range value used to construct the
> index in the metapage.  Now if you do the ALTER INDEX thing, the new
> value is only used when the index is recreated by REINDEX.

This seems a lot like parameterizing. So I guess the only thing left
is to issue a NOTICE when said alter takes place (I don't see that on
the patch, but maybe it's there?)


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


[HACKERS] pgindent weirdness

2014-07-10 Thread Robert Haas
In contrib/test_shm_mq/test.c, the 9.4 pgindent run
(0a7832005792fa6dad171f9cadb8d587fe0dd800) did this:

-PG_MODULE_MAGIC;
-PG_FUNCTION_INFO_V1(test_shm_mq);
+PG_MODULE_MAGIC; PG_FUNCTION_INFO_V1(test_shm_mq);
 PG_FUNCTION_INFO_V1(test_shm_mq_pipelined);

This is obviously not an improvement.  Is there some formatting rule
that I violated in the original code that lead to this, or what?

-- 
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] RLS Design

2014-07-10 Thread Robert Haas
On Wed, Jul 9, 2014 at 2:13 AM, Stephen Frost  wrote:
> Robert,
>
> * Robert Haas (robertmh...@gmail.com) wrote:
>> If you're going to have predicates be table-level and access grants be
>> table-level, then what's the value in having policies?  You could just
>> do:
>>
>> ALTER TABLE table_name GRANT ROW ACCESS TO role_name USING quals;
>
> Yes, this would be possible (and is nearly identical to the original
> patch, except that this includes per-role considerations), however, my
> thinking is that it'd be simpler to work with policy names rather than
> sets of quals, to use when mapping to roles, and they would potentially
> be useful later for other things (eg: for setting up which policies
> should be applied when, or which should be OR' or AND"d with other
> policies, or having groups of policies, etc).

Hmm.  I guess that's reasonable.  Should the policy be a per-table
object (like rules, constraints, etc.) instead of a global object?

You could do:

ALTER TABLE table_name ADD POLICY policy_name (quals);
ALTER TABLE table_name POLICY FOR role_name IS policy_name;
ALTER TABLE table_name DROP POLICY policy_name;

-- 
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] wrapping in extended mode doesn't work well with default pager

2014-07-10 Thread Greg Stark
On Mon, Jul 7, 2014 at 8:35 AM, Sergey Muraviov
 wrote:
> So what's wrong with the patch?
> And what should I change in it for 9.5?

Possibly nothing. The concern was tha it's modifying the output in
cases where the output is not \expanded and/or not wrapped. Now I've
mostly convinced myself that those cases should change to be
consistent with the wrapped output but there was at least one tool
depending on that format (check_postgres) so perhaps it's not
worthwhile and having it be inconsistent would be safer.

-- 
greg


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


Re: [HACKERS] Pg_upgrade and toast tables bug discovered

2014-07-10 Thread Robert Haas
On Wed, Jul 9, 2014 at 12:09 PM, Bruce Momjian  wrote:
>> To me, that sounds vastly more complicated and error-prone than
>> forcing the TOAST tables to be added in a second pass as Andres
>> suggested.
>>
>> But I just work here.
>
> Agreed.  I am now thinking we could harness the code that already exists
> to optionally add a TOAST table as part of ALTER TABLE ADD COLUMN.  We
> would just need an entry point to call it from pg_upgrade, either via an
> SQL command that checks (and hopefully doesn't do anything else), or a C
> function that does it, e.g. VACUUM would be trivial to run on every
> database, but I don't think it tests that;  is _could_ in binary_upgrade
> mode.  However, the idea of having a C function plug into the guts of
> the server and call internal functions makes me uncomforable.

Well, pg_upgrade_support's charter is basically to provide access to
the guts of the server in ways we wouldn't normally allow; all that
next-OID stuff is basically exactly that.  So I don't think this is
such a big deal.  It needs to be properly commented, of course.

-- 
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] IMPORT FOREIGN SCHEMA statement

2014-07-10 Thread Tom Lane
Ronan Dunklau  writes:
> Similarly to what we do for the schema, we should also check that the server 
> in the parsed statement is the one we are importing from.

Hm.  The FDW would really have to go out of its way to put the wrong thing
there, so is this worth the trouble?  It's not like the creation schema
where you can easily omit it from the command; the syntax requires 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] IMPORT FOREIGN SCHEMA statement

2014-07-10 Thread Tom Lane
I wrote:
> Michael Paquier  writes:
>> I guess that this implementation is enough as a first shot, particularly
>> regarding the complexity that default expression import would bring up for
>> postgres_fdw (point raised during the review, but not really discussed
>> afterwards).

> Yeah.  I'm actually more concerned about the lack of collation import,
> but that's unfixable unless we can figure out how to identify collations
> in a platform-independent way.

I had second thoughts about this: now that the code is generating a
textual CREATE FOREIGN TABLE command, it would actually be just about
trivial to pull back the text representation of the remote's default
expression and attempt to apply it to the foreign table.  Likewise we
could ignore the platform-dependency issues in collations and just
attempt to apply whatever collation name is reported by the remote.

Arguably both of these things would be too risky to do by default,
but if we make them be controlled by IMPORT options, why not?

In the case of a default expression, AFAICS there are two possible
failure modes.  The local server might not have some function or
operator used by the remote; in which case, you'd get a pretty easy
to understand error message, and it would be obvious what to do if
you wanted to fix it.  Or we might have a similarly-named function
or operator that behaves differently (an important special case
is where the function is volatile and you'd get a different answer
locally, eg nextval()).  Well, that's a problem, and we'd need to
document it, but I don't see that that risk is so bad that we should
refuse to import any default expressions ever.  Especially when
common cases like "0" or "now()" can be expected to work fine.

Similarly, in the case of a collation name, we might not have the
same locale name, which would be annoying but not dangerous.  Or
we might have a similarly-named locale that actually has sorting
rules different from the remote's.  But that would only be a real
risk when pulling from a remote that's on a different OS from the
local server, and in any case whatever discrepancy might exist is
unlikely to be worse than failing to import a collation spec at all.
Not importing the remote's spec is *guaranteed* to be wrong.

So I propose we invent a couple more import options, say
import_default and import_collate, and make the postgres_fdw
code do the obvious thing with them.  import_default should
probably default to false, but I'm about halfway tempted to
say that import_collate should default to true --- if you're
importing a collatable column and you don't have a matching
locale locally, it seems like it'd be better if we complain
about that by default.

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] Proposing pg_hibernate

2014-07-10 Thread MauMau

Hello,

I've finished reviewing the code.  I already marked this patch as waiting on 
author.  I'll be waiting for the revised patch, then proceed to running the 
program when the patch seems reasonable.


(12)
Like worker_spi, save and restore errno in signal handlers.


(13)
Remove the following comment, and similar ones if any, because this module 
is to be included in 9.5 and above.


/*
 * In Postgres version 9.4 and above, we use the dynamic background worker
 * infrastructure for BlockReaders, and the BufferSaver process does the
 * legwork of registering the BlockReader workers.
 */


(14)
Expand the body following functions at their call sites and remove the 
function definition, because they are called only once.  It would be 
straightforward to see the processing where it should be.


* DefineGUCs
* CreateDirectory


(15)
I don't understand the use case of pg_hibernator.enabled.  Is this really 
necessary?  In what situations does this parameter "really" matter?  If it's 
not crucial, why don't we remove it and make the specification simpler?



(16)
As mentioned above, you can remove CreateDirectory().  Instead, you can just 
mkdir() unconditionally and check the result, without first stat()ing.



(17)
Use AllocateDir/ReadDir/FreeDir instead of opendir/readdir/closedir in the 
server process.  Plus, follow the error handling style of other source files 
using AllocateDir.



(18)
The local variable hibernate_dir appears to be unnecessary because it always 
holds SAVE_LOCATION as its value.  If so, remove it.



(19)
I think the severity below should better be WARNING, but I don't insist.

ereport(LOG, (errmsg("registration of background worker failed")));


(20)
"iff" should be "if".

/* Remove the element from pending list iff we could register a worker 
successfully. */




(21)
How is this true?  Does the shared system catalog always have at least one 
shared buffer?


  /* Make sure the first buffer we save belongs to global object. */
  Assert(buf->database == InvalidOid);
...
   * Special case for global objects. The sort brings them to the
   * front of the list.


(22)
The format should be %u, not %d.

 ereport(log_level,
   (errmsg("writer: writing block db %d filenode %d forknum %d blocknum 
%d",

 database_counter, prev_filenode, prev_forknum, buf->blocknum)));


(23)
Why is the first while loop in BlockReaderMain() necessary?  Just opening 
the target save file isn't enough?




(24)
Use MemoryContextAllocHuge().  palloc() can only allocate chunks up to 1GB.

 * This is not a concern as of now, so deferred; there's at least one other
 * place that allocates (NBuffers * (much_bigger_struct)), so this seems to
 * be an acceptable practice.
 */

saved_buffers = (SavedBuffer *) palloc(sizeof(SavedBuffer) * NBuffers);


(25)
It's better for the .save files to be created per tablespace, not per 
database.  Tablespaces are more likely to be allocated on different storage 
devices for I/O distribution and capacity.  So, it would be more natural to 
expect that we can restore buffers more quickly by letting multiple Block 
Readers do parallel I/O on different storage devices.



(26)
Reading the below description in the documentation, it seems that Block 
Readers can exit with 0 upon successful completion, because bgw_restart_time 
is set to BGW_NEVER_RESTART.


"If bgw_restart_time for a background worker is configured as 
BGW_NEVER_RESTART, or if it exits with an exit code of 0 or is terminated by 
TerminateBackgroundWorker, it will be automatically unregistered by the 
postmaster on exit."



(27)
As others said, I also think that Buffer Saver should first write to a temp 
file, say *.tmp, then rename it to *.save upon completion.  That prevents 
the Block Reader from reading possibly corrupted half-baked file that does 
not represent any hibernated state.



Regards
MauMau




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


[HACKERS] timeout of pg_receivexlog --status-interval

2014-07-10 Thread Sawada Masahiko
Hi,

At 1047 line of receivelog.c:CopyStreamPoll(), we set NULL to
timeoutptr variable.
if the value of timeoutprt is set NULL then the process will wait
until can read socket using by select() function as following.

if (timeout_ms < 0)
timeoutptr = NULL;
else
{
timeout.tv_sec = timeout_ms / 1000L;
timeout.tv_usec = (timeout_ms % 1000L) * 1000L;
timeoutptr = &timeout;
}

ret = select(PQsocket(conn) + 1, &input_mask, NULL, NULL, timeoutptr);

But the 1047 line of receivelog.c is never executed because the value
of timeout_ms is NOT allowed less than 0 at CopyStreamReceive which is
only one function calls CopyStreamPoll().
The currently code, if we specify -s to 0 then CopyStreamPoll()
function is never called.
And the pg_receivexlog will be execute PQgetCopyData() and failed, in
succession.

I think that it is contradiction, and should execute select() function
with NULL of fourth argument.
the attached patch allows to execute select() with NULL, i.g.,
pg_receivexlog.c will wait until can  read socket without timeout, if
-s is specified to 0.

Regards,
---
Sawada Masahiko


receivexlog-timout.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 line number as prompt option to psql

2014-07-10 Thread Sawada Masahiko
On Thu, Jul 10, 2014 at 8:35 PM, Jeevan Chalke
 wrote:
> Hi,
>
> Found few more bugs in new code:
>
> A:
> This got bad:
>
> jeevan@ubuntu:~/pg_master$ ./install/bin/psql postgres
> psql (9.5devel)
> Type "help" for help.
>
>
> postgres=# \set PROMPT1 '%/[%l]%R%# '
> postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
> postgres[1]=# select
> postgres[2]-# *
> postgres[3]-# from
> postgres[4]-# abc;
> ERROR:  syntax error at or near "fromabc"
> LINE 1: select*fromabc;
>
>^
> postgres[1]=#
> postgres[1]=#
> postgres[1]=# \e
> ERROR:  syntax error at or near "fromabc"
> LINE 1: select*fromabc;
>^
> postgres[1]=# select*fromabc;
> ERROR:  syntax error at or near "fromabc"
> LINE 1: select*fromabc;
>^
> postgres[1]=#
>
>
> See query text in LINE 1:. This is because, you have removed addition of
> newline character. Related added_nl_pos. Need more investigation here.
> However I don't think these changes are relevant to what you wanted in this
> feature.
> Will you please explain the idea behind these changes ?
>
> Moreover, if you don't want to add newline character, then I guess entire
> logic related to added_nl_pos is NO more required. You may remove this
> variable and its logic altogether, not sure though. Also make sure you
> update the relevant comments while doing so. Here you have removed the code
> which is adding the newline but the comment there still reads:
> /* insert newlines into query buffer between source lines */
>
> Need more thoughts on this.
>
>
> B:
>
> postgres=# \set PROMPT1 '%/[%l]%R%# '
> postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
> postgres[1]=# \e
> postgres[-2147483645]-# limit 1;
>relname
> --
>  pg_statistic
> (1 row)
>
>
> postgres[1]=#
> postgres[1]=# select
> relname
> from
> pg_class
> limit 1;
>
> Logic related to wrapping around the cur_line counter is wrong. Actually
> issue is with newline variable. If number of lines in \e editor goes beyond
> INT_MAX (NOT sure about the practical use), then newline will be -ve which
> then enforces cur_line to be negative. To mimic this I have initialized
> newline = INT_MAX - 1.
>

Thank you for reviewing the patch with variable cases.
I have revised the patch, and attached latest patch.

> A:
> Will you please explain the idea behind these changes ?
I thought wrong about adding new to tail of query_buf.
The latest patch does not change related to them.

> B:
I added the condition of cur_line < 0.


Regards,

---
Sawada Masahiko


psql-line-number_v4.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] Missing IPv6 for pgbuildfarm.org

2014-07-10 Thread Andrew Dunstan


On 07/02/2014 05:08 AM, Torsten Zuehlsdorff wrote:

Hello,

i've tried to setup a FreeBSD 10 machine as buildfarm-member. But it's 
an IPv6 only machine and there is no IPv6 for the homepage.


Can anyone add support for IPv6 to it?




I'm looking into this.

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] psql: show only failed queries

2014-07-10 Thread Pavel Stehule
Hello

here is a proposed patch - autocomplete for known psql variable content

Regards

Pavel


2014-07-10 9:50 GMT+02:00 Pavel Stehule :

>
>
>
> 2014-07-10 7:32 GMT+02:00 Fujii Masao :
>
> On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule 
>> wrote:
>> >> Barring any objection, I will commit this patch except tab-completion
>> >> part.
>>
>> Committed.
>>
>
> Thank you very much
>
>>
>> > It can be a second discussion, but I am thinking so anywhere when we
>> can use
>> > autocomplete, then we should it - Although it should not to substitute
>> > documentation, it is fast help about available options, mainly in
>> situation
>> > where variables can hold a relative different content. I can prepare, if
>> > there will not any objection addition patch with complete autocomplete
>> for
>> > all psql variables described in doc.
>>
>> I have no objection. But I found that the tab-completion for psql
>> variables names
>> are not complete. For example, COMP_KEYWORD_CASE is not displayed.
>> Probably we should address this problem, too.
>>
>
> I prepare patch for next commitfest - it is relative trivial task
>
> Pavel
>
>
>>
>> Regards,
>>
>> --
>> Fujii Masao
>>
>
>
commit 7cba776aea228165c5b65f7077515328a0efa039
Author: root 
Date:   Thu Jul 10 14:54:36 2014 +0200

initial concept

diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 5a397e8..f05dfe2 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -147,6 +147,8 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
 	SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
 
+	SetVariable(pset.vars, "COMP_KEYWORD_CASE", "preserve-upper");
+
 	parse_psql_options(argc, argv, &options);
 
 	/*
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bab0357..41ae499 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3592,6 +3592,79 @@ psql_completion(const char *text, int start, int end)
 	{
 		matches = complete_from_variables(text, "", "");
 	}
+	else if (strcmp(prev2_wd, "\\set") == 0)
+	{
+		if (strcmp(prev_wd, "AUTOCOMMIT") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", "interactive", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "COMP_KEYWORD_CASE") == 0)
+		{
+			static const char *const my_list[] =
+			{"lower", "upper", "preserve-lower", "preserve-upper", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "ECHO") == 0)
+		{
+			static const char *const my_list[] =
+			{"none", "errors", "queries", "all", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "ECHO_HIDDEN") == 0)
+		{
+			static const char *const my_list[] =
+			{"noexec", "off", "on", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "ON_ERROR_ROLLBACK") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", "interactive", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "ON_ERROR_STOP") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "QUIET") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "SINGLELINE") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "SINGLESTEP") == 0)
+		{
+			static const char *const my_list[] =
+			{"on", "off", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "VERBOSITY") == 0)
+		{
+			static const char *const my_list[] =
+			{"default", "verbose", "terse", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+	}
 	else if (strcmp(prev_wd, "\\sf") == 0 || strcmp(prev_wd, "\\sf+") == 0)
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions, NULL);
 	else if (strcmp(prev_wd, "\\cd") == 0 ||

-- 
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 line number as prompt option to psql

2014-07-10 Thread Jeevan Chalke
Hi,

Found few more bugs in new code:

A:
This got bad:

jeevan@ubuntu:~/pg_master$ ./install/bin/psql postgres
psql (9.5devel)
Type "help" for help.

postgres=# \set PROMPT1 '%/[%l]%R%# '
postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
postgres[1]=# select
postgres[2]-# *
postgres[3]-# from
postgres[4]-# abc;
ERROR:  syntax error at or near "fromabc"
LINE 1: select*fromabc;
   ^
postgres[1]=#
postgres[1]=#
postgres[1]=# \e
ERROR:  syntax error at or near "fromabc"
LINE 1: select*fromabc;
   ^
postgres[1]=# select*fromabc;
ERROR:  syntax error at or near "fromabc"
LINE 1: select*fromabc;
   ^
postgres[1]=#


See query text in LINE 1:. This is because, you have removed addition of
newline character. Related added_nl_pos. Need more investigation here.
However I don't think these changes are relevant to what you wanted in this
feature.
Will you please explain the idea behind these changes ?

Moreover, if you don't want to add newline character, then I guess entire
logic related to added_nl_pos is NO more required. You may remove this
variable and its logic altogether, not sure though. Also make sure you
update the relevant comments while doing so. Here you have removed the code
which is adding the newline but the comment there still reads:
/* insert newlines into query buffer between source lines */

Need more thoughts on this.


B:
postgres=# \set PROMPT1 '%/[%l]%R%# '
postgres[1]=# \set PROMPT2 '%/[%l]%R%# '
postgres[1]=# \e
postgres[-2147483645]-# limit 1;
   relname
--
 pg_statistic
(1 row)

postgres[1]=#
postgres[1]=# select
relname
from
pg_class
limit 1;

Logic related to wrapping around the cur_line counter is wrong. Actually
issue is with newline variable. If number of lines in \e editor goes beyond
INT_MAX (NOT sure about the practical use), then newline will be -ve which
then enforces cur_line to be negative. To mimic this I have initialized
newline = INT_MAX - 1.

Thanks

-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company


Re: [HACKERS] Re: how to find the order of joins from Explain command XML plan output in PostgreSQL

2014-07-10 Thread Guillaume Lelarge
Le 9 juil. 2014 20:36, "David G Johnston"  a
écrit :
>
> csrajmohan wrote
> > "EXPLAIN (format XML) " command in PostgreSQL9.3.4 gives the plan chosen
> > by
> > the optimizer in XML format. In my program, I have to extract certain
data
> > about optimizer plan from this XML output. I am using *LibXML2* library
> > for
> > parsing the XML. I had successfully extracted information about which
> > relations are involved and what joins are used by parsing the XML. But
> > I am *unable
> > to extract the* *order of joining the relations from the XML output*. I
> > conceptually understood that the reverse level order traversal of binary
> > tree representation of the XML plan will give correct ordering of joins
> > applied. But I could not figure out how do I get that from the XML? Does
> > libXML2 support anything of this sort? If not how should I proceed to
> > tackle this?
>
> So, since nothing better has been forthcoming in your other two posts on
> this topic I'll just say that likely you will have much better luck using
> SAX-based processing as opposed to DOM-based processing.  I seriously
doubt
> native/core PostgreSQL facilities will allow you to do what you desire.
>
> As you said, hierarchy and physical output order determines the "order of
> joining" within the planner so you have to capture and track such
relational
> information during your processing - which is made much easier if you
simply
> traverse the output node-by-node exactly as a SAX based parser does.
>
> Though pgAdminIII has a visual query display that you might look at for
> inspiration.
>

FWIW, pgadmin's visual explain doesn't (yet?) use XML or json or yaml
output.


Re: [HACKERS] pg_resetxlog to clear backup start/end locations.

2014-07-10 Thread Kyotaro HORIGUCHI
Hello,

> > > Anyway, I think that making -n option display all the values that -f 
> > > option
> > > changes would be useful. But since that's not a bugfix, we should apply it
> > > only in HEAD.
> > 
> > Agreed.
> 
> Is this a TODO item?

It's not that. The 'bug' was my wrong guess and I've found that
it is done by default from the first..

I'm sorry to have bothered you.

regards,

-- 
Kyotaro Horiguchi
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] Use unique index for longer pathkeys.

2014-07-10 Thread Kyotaro HORIGUCHI
Thank you for taking a look on this patch.

> I took a quick look at this patch, more or less because nobody else did.
> 
> > Duing last CF, I proposed to match long pathkeys against index columns
> > during creating index paths. This worked fine but also it is difficult
> > to make sure that all side-effects are eliminated. Finally Tom Lane
> > suggested to truncate pathkeys while generation of the pathkeys
> > itself. So this patch comes.
> 
> I found your older patch quite straightforward to understand, but the
> new one much more difficult to follow (but that's not saying much, I
> am not very familiar with the planner code in general).

I think it's quite natural to think so.

> Do you have any references to the discussion about the side-effects that
> needed to be eliminated with the earlier patch?

The biggest side-effects (or simplly defect) found so far is
discussed here,

http://www.postgresql.org/message-id/01bd01cf0b4e$9b960ad0$d2c22070$@ets...@lab.ntt.co.jp

This was caused by omitting the membership of the Var under
inspection while cheking if the pathkeys is extensible.

http://www.postgresql.org/message-id/20140107.145900.196068363.horiguchi.kyot...@lab.ntt.co.jp

After all, Tom said that the right way to do this is not such
whacking-a-mole thing but loosen pathkeys previously so that the
planner naturally do what the previous patch did without any
special treat.

http://www.postgresql.org/message-id/5212.1397599...@sss.pgh.pa.us

So the new patch comes.


regards,

-- 
Kyotaro Horiguchi
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] WAL replay bugs

2014-07-10 Thread Kyotaro HORIGUCHI
Hello, The new patch looks good for me.

The usage of this is a bit irregular as a (extension) module but
it is the nature of this 'contrib'. The rearranged page type
detection logic seems neater and keeps to work as intended. This
logic will be changed after the new page type detection scheme
becomes ready by the another patch.


I have some additional comments, which should be the last
ones. All of the comments are about test.sh.

- A question mark seems missing from the end of the message "Has
  build been done with -DBUFFER_CAPTURE included in CFLAGS" in
  test.sh.

- "make check" runs "regress --use-existing" but IMHO the make
  target which is expected to do that is installcheck. I had
  fooled to convince that it should run the postgres which is
  built dedicatedly:(

- postgres processes are left running after
  test_default(custom).sh has stopped halfway. This can be fixed
  with the attached patch, but, to be honest, this seems too
  much. I'll follow your decision whether or not to do this.
  (bufcapt_test_sh_20140710.patch)

- test_default.sh is not only an example script which will run
  while utilize this facility, but the test script for this
  facility itself.

  So I think it would be better be added some queries so that all
  possible page types available for the default testing. What do
  you think about the attached patch?  (hash index is unlogged
  but I dared to put it for clarity.)

  (bufcapt_test_default_sh_20140710.patch)

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/contrib/buffer_capture_cmp/test.sh b/contrib/buffer_capture_cmp/test.sh
index 89740bb..ba5e290 100644
--- a/contrib/buffer_capture_cmp/test.sh
+++ b/contrib/buffer_capture_cmp/test.sh
@@ -117,16 +117,27 @@ pg_ctl -w -D $TEST_STANDBY start
 # Check the presence of custom tests and kick them in priority. If not,
 # fallback to the default tests. Tests need only to be run on the master
 # node.
+
 if [ -f ./test-custom.sh ]; then
-	. ./test-custom.sh
+	TEST_SCRIPT=test-custom.sh
 else
-	. ./test-default.sh
+	TEST_SCRIPT=test-default.sh
 fi
 
+set +e
+bash -e $TEST_SCRIPT
+EXITSTATUS=$?
+set -e
+
 # Time to stop the nodes as tests have run
 pg_ctl -w -D $TEST_MASTER stop
 pg_ctl -w -D $TEST_STANDBY stop
 
+if [ $EXITSTATUS != 0 ]; then
+	echo "$TEST_SCRIPT exited by error"
+	exit 1;
+fi
+
 DIFF_FILE=capture_differences.txt
 
 # Check if the capture files exist. If not, build may have not been
diff --git a/contrib/buffer_capture_cmp/test-default.sh b/contrib/buffer_capture_cmp/test-default.sh
index 5bec503..24091ff 100644
--- a/contrib/buffer_capture_cmp/test-default.sh
+++ b/contrib/buffer_capture_cmp/test-default.sh
@@ -11,4 +11,16 @@
 # cd $ROOT_DIR
 
 # Create a simple table
-psql -c 'CREATE TABLE aa AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE TABLE tbtree AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE INDEX i_tbtree ON tbtree USING btree(a)'
+psql -c 'CREATE TABLE thash AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE INDEX i_thash ON thash USING hash(a)'
+psql -c 'CREATE TABLE tgist AS SELECT POINT(a, a) AS p1 FROM generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tgist ON tgist USING gist(p1)'
+psql -c 'CREATE TABLE tspgist AS SELECT POINT(a, a) AS p1 FROM generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tspgist ON tspgist USING spgist(p1)'
+psql -c 'CREATE TABLE tgin AS SELECT ARRAY[a/10, a%10] as a1 from generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tgin ON tgin USING gin(a1)'
+psql -c 'CREATE SEQUENCE sq1'
+
+

-- 
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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..

2014-07-10 Thread Marko Tiikkaja

On 7/10/14 5:44 AM, Amit Kapila wrote:

Basically, I wanted to say that apart from modified columns, we just
need to pass table OID.  If I am reading correctly, the same is
mentioned by Heikki as well.


Yes, Heikki was talking about that approach.  I was more interested in 
the significantly more invasive approach Tom and Andres talked about 
upthread, which your email was a response to.




.marko


--
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] inherit support for foreign tables

2014-07-10 Thread Shigeru Hanada
Hi Fujita-san,

Sorry for leaving this thread for long time.

2014-06-24 16:30 GMT+09:00 Etsuro Fujita :
> (2014/06/23 18:35), Ashutosh Bapat wrote:
>>
>> Hi,
>> Selecting tableoid on parent causes an error, "ERROR:  cannot extract
>> system attribute from virtual tuple". The foreign table has an OID which
>> can be reported as tableoid for the rows coming from that foreign table.
>> Do we want to do that?
>
>
> No.  I think it's a bug.  I'll fix it.

IIUC, you mean that tableoid can't be retrieved when a foreign table
is accessed via parent table, but it sounds strange to me, because one
of main purposes of tableoid is determine actual source table in
appended results.

Am I missing the point?

-- 
Shigeru HANADA


-- 
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] psql: show only failed queries

2014-07-10 Thread Pavel Stehule
2014-07-10 7:32 GMT+02:00 Fujii Masao :

> On Wed, Jul 9, 2014 at 9:44 PM, Pavel Stehule 
> wrote:
> >> Barring any objection, I will commit this patch except tab-completion
> >> part.
>
> Committed.
>

Thank you very much

>
> > It can be a second discussion, but I am thinking so anywhere when we can
> use
> > autocomplete, then we should it - Although it should not to substitute
> > documentation, it is fast help about available options, mainly in
> situation
> > where variables can hold a relative different content. I can prepare, if
> > there will not any objection addition patch with complete autocomplete
> for
> > all psql variables described in doc.
>
> I have no objection. But I found that the tab-completion for psql
> variables names
> are not complete. For example, COMP_KEYWORD_CASE is not displayed.
> Probably we should address this problem, too.
>

I prepare patch for next commitfest - it is relative trivial task

Pavel


>
> Regards,
>
> --
> Fujii Masao
>


Re: [HACKERS] pg_receivexlog add synchronous mode

2014-07-10 Thread furuyao
This patch is modified the comment.
Each comment is coping as follows.

> Could you update the status of this patch from "Waiting on Author" to
> "Needs Review" when you post the revised version of the patch?
Thank you for pointing out. 
From now on, I will update status when I post the patch.

> +How often should pg_receivexlog
> issue sync
> +commands to ensure the received WAL file is safely
> +flushed to disk without being asked by the server to do so.
> 
> "without being asked by the server to do so" implies that the server asks
> pg_receivexlog to flush WAL files periodically?
I think that the sentence means the following.
Without waiting for the feedback request from the server, select is timed out 
and flush is checked. 

Because I cause misunderstanding, I delete a sentence.

>  Specifying
> +an interval of 0 together the consecutive
> data.
> 
> This text looks corrupted. What does this mean?
> 
> +Not specifying an interval disables issuing fsyncs altogether,
> +while still reporting progress the server.
> 
> No. Even when the option is not specified, WAL files should be flushed
> at WAL file switch, like current pg_receivexlog does. If you want to
> disable the flush completely, you can change the option so that it accepts
> -1 which means to disable the flush, for example.
Fix to description "issuing fsyncs at only  WAL file close". 

> +printf(_("  -F  --fsync-interval=SECS\n"
> + " frequency of syncs to the
> output file (default: file close only)\n"));
> 
> It's better to use "transaction log files" rather than "output file"
> here.
Fix as you pointed out.

> SECS should be INTERVAL for the sake of consistency with
> --stat-interval's help message?
Fix as you pointed out.

> + * fsync_interval controls how often we flush to the received
> + * WAL file, in seconds.
> 
> "seconds" should be "miliseconds"?
Fix as you pointed out.

> The patch changes pg_receivexlog so that it keep processing the
> continuous messages without calling stream_stop(). But as I commented
> before, stream_stop should be called every time the message is received?
> Otherwise pg_basebackup background WAL receiver might not be able to stop
> streaming at proper point.
FIx the calling stream_stop() with 1 message processing is complete. 

> The flush interval is checked and WAL files are flushed only when
> CopyStreamReceive() returns 0, i.e., there is no message available and
> the timeout occurs. Why did you do that? I'm afraid that
> CopyStreamReceive() always waits for at least one second before WAL files
> are flushed even when --fsync-interval is set to 0.
CopyStreamReceive() is according to pg_recvlogical --fsync-interval and 
--status-interval shorter intervals runs the wait.
About specifying an interval of zero.
Every flush to continuously message,  so no problem will wait one second.

> Why don't you update output_last_status when WAL file is flushed and
> closed?
About the closed, add the update step.
About the flush, according to pg_recvlogical, update is performed after an 
interval check before flush. 
Therefore not modify.

Regards,

--
Furuya Osamu



pg_receivexlog-add-fsync-interval-v2.patch
Description: pg_receivexlog-add-fsync-interval-v2.patch

-- 
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] IMPORT FOREIGN SCHEMA statement

2014-07-10 Thread Ronan Dunklau
Le mercredi 9 juillet 2014 21:30:05 Tom Lane a écrit :
> Michael Paquier  writes:
> > I guess that this implementation is enough as a first shot, particularly
> > regarding the complexity that default expression import would bring up for
> > postgres_fdw (point raised during the review, but not really discussed
> > afterwards).
> 
> Yeah.  I'm actually more concerned about the lack of collation import,
> but that's unfixable unless we can figure out how to identify collations
> in a platform-independent way.
> 
>   regards, tom lane

Thank you for working on this patch, I'm not really fond of building strings 
in a FDW for the core to parse them again but I don't see any other solution 
to the problem you mentioned.

Similarly to what we do for the schema, we should also check that the server 
in the parsed statement is the one we are importing from.


-- 
Ronan Dunklau
http://dalibo.com - http://dalibo.org

signature.asc
Description: This is a digitally signed message part.