Re: [HACKERS] Minmax indexes

2013-09-17 Thread Jaime Casanova
On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
 On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 Hi,

 Here's a reviewable version of what I've dubbed Minmax indexes.

 Thanks for the patch, but I seem to have immediately hit a snag:

 pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
 PANIC:  invalid xlog record length 0


fwiw, this seems to be triggered by ANALYZE.
At least i can trigger it by executing ANALYZE on the table (attached
is a stacktrace of a backend exhibiting the failure)

Another thing is this messages i got when compiling:

mmxlog.c: In function ‘minmax_xlog_revmap_set’:
mmxlog.c:161:14: warning: unused variable ‘blkno’ [-Wunused-variable]
bufpage.c: In function ‘PageIndexDeleteNoCompact’:
bufpage.c:1066:18: warning: ‘lastused’ may be used uninitialized in
this function [-Wmaybe-uninitialized]


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
Program received signal SIGABRT, Aborted.
0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64  ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el 
directorio.
(gdb) bt
#0  0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f442881c6f0 in *__GI_abort () at abort.c:92
#2  0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546
#3  0x0076fdca in elog_finish (elevel=elevel@entry=22, 
fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304
#4  0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', 
info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966
#5  0x004a9bb9 in mmSetHeapBlockItemptr 
(rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, 
blkno=blkno@entry=6, offno=offno@entry=1)
at mmrevmap.c:169
#6  0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, 
rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, 
tup=tup@entry=0x7f441f84cff8, 
itemsz=16) at minmax.c:1410
#7  0x004a9464 in rerun_summarization (numnonsummarized=22, 
nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, 
idxRel=0x7f4429054c88)
at minmax.c:1205
#8  mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268
#9  0x0077388f in FunctionCall2Coll 
(flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, 
arg1=arg1@entry=140736552432368, arg2=arg2@entry=0)
at fmgr.c:1326
#10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, 
stats=stats@entry=0x0) at indexam.c:715
#11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, 
acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 
'\000', 
elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf 
expression opcode 0xfa, 
vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) 
at analyze.c:634
#12 0x00557fef in analyze_rel (relid=relid@entry=16384, 
vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267
---Type return to continue, or q return to quit---
#13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, 
relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized 
out, 
bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', 
isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249
#14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, 
queryString=optimized out, context=optimized out, params=0x0, 
dest=optimized out, 
completionTag=optimized out) at utility.c:682
#15 0x0069c587 in PortalRunUtility (portal=0x1b33198, 
utilityStmt=0x1af5678, isTopLevel=1 '\001', dest=0x1af5a00, 
completionTag=0x7fffc836f920 )
at pquery.c:1187
#16 0x0069d299 in PortalRunMulti (portal=portal@entry=0x1b33198, 
isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1af5a00, 
altdest=altdest@entry=0x1af5a00, 
completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:1318
#17 0x0069df32 in PortalRun (portal=portal@entry=0x1b33198, 
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', 
dest=dest@entry=0x1af5a00, altdest=altdest@entry=0x1af5a00, 
completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:816
#18 0x0069afdb in exec_simple_query (query_string=0x1af4c18 analyze 
t1;) at postgres.c:1048
#19 PostgresMain (argc=optimized out, argv=argv@entry=0x1ab0a70, 
dbname=0x1ab08f0 postgres, username=optimized out) at postgres.c:3992
#20 0x0046559e in BackendRun (port=0x1ab2770) at postmaster.c:4083
#21 BackendStartup (port=0x1ab2770) at postmaster.c:3772
#22 ServerLoop () at postmaster.c:1583
#23 0x0065230e in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x1a8df00) at postmaster.c:1239
#24 0x00465ec5 in main (argc=3, 

Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-17 Thread Samrat Revagade
  syncrep.c: In function ‘SyncRepReleaseWaiters’:
  syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used
  [-Wunused-but-set-variable]
 

 Sorry I forgot fix it.

 I have attached the patch which I modified.


Attached patch combines documentation patch and source-code patch.

-- 
Regards,

Samrat Revgade


synchronous_transfer_v7.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] PostgreSQL 9.3 beta breaks some extensions make install

2013-09-17 Thread Cédric Villemain
  Apt.pgdg got the patch present in postgresql head applyed.
 
 Erm, isn't apt.postgresql.org supposed to ship the *official*
 PostgreSQL versions? Given that this issue affects all distros, I
 don't see why Ubuntu/Debian need to be patched separately.

Well, the patches are applyed on the debian packages (not only in 
apt.pgdg.org).
The packages provided by apt.postgresql.org are based on the 'official 
packages' from debian. (if you allow me this circle)

 Does anyone else think this is problematic? By slipping patches into
 distro-specific packages, you're confusing users (like me) and
 bypassing the PostgreSQL QA process.

The QA is about distribution of packages here. Debian applies 14 patches 
on 9.3 builds, not only the things about vpath we're talking about.

 PS: Where are the sources used to build packages on
 apt.postgresql.org?

9.3:
http://anonscm.debian.org/loggerhead/pkg-postgresql/postgresql-9.3/trunk/changes

-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

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


Re: [HACKERS] Minmax indexes

2013-09-17 Thread Thom Brown
On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
  On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
 wrote:
 
  Hi,
 
  Here's a reviewable version of what I've dubbed Minmax indexes.
 
  Thanks for the patch, but I seem to have immediately hit a snag:
 
  pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
  PANIC:  invalid xlog record length 0
 

 fwiw, this seems to be triggered by ANALYZE.
 At least i can trigger it by executing ANALYZE on the table (attached
 is a stacktrace of a backend exhibiting the failure)

 Another thing is this messages i got when compiling:
 
 mmxlog.c: In function ‘minmax_xlog_revmap_set’:
 mmxlog.c:161:14: warning: unused variable ‘blkno’ [-Wunused-variable]
 bufpage.c: In function ‘PageIndexDeleteNoCompact’:
 bufpage.c:1066:18: warning: ‘lastused’ may be used uninitialized in
 this function [-Wmaybe-uninitialized]
 


I'm able to run ANALYSE manually without it dying:

pgbench=# analyse pgbench_accounts;
ANALYZE
pgbench=# analyse pgbench_accounts;
ANALYZE
pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid);
PANIC:  invalid xlog record length 0

-- 
Thom


Re: [HACKERS] PL/pgSQL, RAISE and error context

2013-09-17 Thread Jeevan Chalke
Hi Marko,

I have reviewed this patch.

1. Patch applies well.
2. make and make install is fine
3. make check is fine too.

But as Peter pointed out plperl regression tests are failing.

I just did grep on .sql files and found following files which has RAISE
statement into it. These files too need expected output changes. Please run
these testcases to get diffs.

./src/pl/plperl/sql/plperl_elog.sql
./src/pl/plpython/sql/plpython_error.sql
./src/pl/plpython/sql/plpython_setof.sql
./src/pl/plpython/sql/plpython_quote.sql
./contrib/sepgsql/sql/label.sql
./contrib/sepgsql/sql/ddl.sql

Code changes looks fine to me.

Thanks


-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation


Re: [HACKERS] insert throw error when year field len 4 for timestamptz datatype

2013-09-17 Thread Rushabh Lathia
On Mon, Sep 16, 2013 at 7:22 PM, Haribabu kommi
haribabu.ko...@huawei.comwrote:

   *On *14 August 2013 Rushabh Lathia wrote:**

 ** **

 postgres=# create table test ( a timestamptz);

 CREATE TABLE

 ** **

 -- Date with year 1000

 postgres=#  insert into test values ( 'Sat Mar 11 23:58:48 1000 IST');***
 *

 INSERT 0 1

 ** **

 -- Now try with year 1 it will return error

 postgres=#  insert into test values ( 'Sat Mar 11 23:58:48 1 IST');**
 **

 ERROR:  invalid input syntax for type timestamp with time zone: Sat Mar
 11 23:58:48 1 IST 

 LINE 1: insert into test values ( 'Sat Mar 11 23:58:48 1 IST');

 ** **

 here error coming from timestamptz_in() - datefields_to_timestamp() -**
 **

 DecodeDateTime() stack.

 ** **

 Looking more at the DecodeDateTime() function, here error coming while
 trying

 to Decode year field which is 1 in the our test. For year field
 ftype is

 DTK_NUMBER, and under DTK_NUMBER for this case if drop in to following
 condition:

 ** **

 else if (flen  4)

 {

 dterr = DecodeNumberField(flen, field[i], fmask,

  tmask, tm,

  fsec, is2digits);

 if (dterr  0)

 return dterr;

 }

 ** **

 because flen in out case flen is 5 (1).

 ** **

 As per the comment above DecodeNumberField(), it interpret numeric
 string as a

 concatenated date or time field. So ideally we should be into
 DecodeNumberField

 function only with (fmask  DTK_DATE_M) == 0 or (fmask  DTK_TIME_M) ==
 0,

 right ??

 ** **

 So, I tried the same and after that test working fine.

 ** **

 PFA patch and share your input/suggestions.

 ** **

 Patch applies cleanly to HEAD. As this patch tries to improve in inserting
 the date of the year value to be more than 4 in length.

 But it didn’t solve all the ways to insert the year field more than 4 in
 length. Please check the following test.

 ** **

 ** **

 postgres=# insert into test values ('10001010 10:10:10 IST');

 INSERT 0 1

 postgres=# insert into test values ('100011010 10:10:10 IST');

 ERROR:  invalid input syntax for type timestamp with time zone: 100011010
 10:10:10 IST at character 26

 STATEMENT:  insert into test values ('100011010 10:10:10 IST');

 ERROR:  invalid input syntax for type timestamp with time zone: 100011010
 10:10:10 IST

 LINE 1: insert into test values ('100011010 10:10:10 IST');

 ^

 ** **

 I feel it is better to provide the functionality of inserting year field
 more than 4 in length in all flows.


+1. Nice catch.

Here is the latest version of patch which handles the functionality in all
flows.
Could you test it and share you comments.

Thanks,
Rushabh Lathia
www.EnterpriseDB.com


timestamptz_fix_with_testcase_v2.patch
Description: Binary data

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


Re: [HACKERS] record identical operator

2013-09-17 Thread Hannu Krosing
On 09/16/2013 04:01 PM, Kevin Grittner wrote:
 Hannu Krosing ha...@2ndquadrant.com wrote:

 Lots of people were bitten when (undocumented) hash
 functions were changed thus breaking hash-based partitioning.
 Nobody can be affected by the new operators in this patch unless
 they choose to use them to compare two records.  They don't work
 for any other type and they don't come into play unless you
 specifically request them.

Maybe the binary equality operator should be named 
for really deeply equal

to distinguish it from === which would be merely NOT DISTINCT FROM

we could even go one step further and define = to mean the same.

?

This could fit better in conceptual sequence of operator 'strength'

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ



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


[HACKERS] Typo fix in spgtextproc.c

2013-09-17 Thread Etsuro Fujita
I ran into a typo.  Attached is a patch.

Thanks,

Best regards,
Etsuro Fujita


typofix-20130917.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


[HACKERS] Docs fix in advanced.sgml

2013-09-17 Thread Etsuro Fujita
I think the document in advanced.sgml should be corrected, though I might
misunderstand the rules of usage.  Attached is a patch.

Thanks,

Best regards,
Etsuro Fujita


docsfix-20130917.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] Minor inheritance/check bug: Inconsistent behavior

2013-09-17 Thread Rushabh Lathia
Hi Amit.

I gone through the mail thread discussion regarding this issue and reviewed
you patch.

-- Patch get applied cleanly on Master branch
-- Make and Make Install fine
-- make check also running cleanly

In the patch code changes looks good to me.

This patch having two part:

1) Allowed TableOid system column in the CHECK constraint
2) Throw an error if other then TableOid system column in CHECK constraint.

I noticed that you added test coverage for 1) but the test coverage for 2)
is missing..

I added the test coverage for 2) in the attached patch.

Marking this as Ready for committer.

Regards,
Rushabh





On Sun, Sep 15, 2013 at 2:31 PM, Amit Kapila amit.kapil...@gmail.comwrote:

 Bruce Momjian wrote:
 On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote:
   I have done the initial analysis and prepared a patch, don't know if
   anything more I can do until
   someone can give any suggestions to further proceed on this bug.
 
   So, I guess we never figured this out.
 
  I can submit this bug-fix for next commitfest if there is no objection
 for doing so.
  What is your opinion?

  Yes, good idea.

 I had rebased the patch against head and added the test case to validate
 it.
 I will upload this patch to commit fest.

 With Regards,
 Amit Kapila.
 EnterpriseDB: http://www.enterprisedb.com


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




-- 
Rushabh Lathia
www.EnterpriseDB.com


sys_col_constr_v2.patch
Description: Binary data

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


Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-17 Thread Asif Naeem
Hi,

I did put some time review the patch, please see my findings below i.e.

1. It seems that you have used strdup() on multiple places in the patch,
e.g. in the below code snippet is it going to lead crash if newp-ident is
NULL because of strdup() failure ?

static EPlan *
 find_plan(char *ident, EPlan **eplan, int *nplans)
 {
 ...
  newp-ident = strdup(ident);
 ...
 }


2. Can we rely on return value of asprintf() instead of recompute length as
strlen(cmdbuf) ?

  if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 
 0)
return fail_lo_xact(\\lo_import, own_transaction);
   bufptr = cmdbuf + strlen(cmdbuf);


3. There seems naming convention for functions like pfree (that seems
similar to free()), pstrdup etc; but psprintf seems different than sprintf.
Can't we use pg_asprintf instead in the patch, as it seems that both
(psprintf and pg_asprintf) provide almost same functionality ?

4. It seems that ret  0 need to be changed to rc  0 in the following
code i.e.

int
 pg_asprintf(char **ret, const char *format, ...)
 {
  va_list  ap;
  int   rc;
  va_start(ap, format);
  rc = vasprintf(ret, format, ap);
  va_end(ap);
  if (ret  0)
  {
   fprintf(stderr, _(out of memory\n));
   exit(EXIT_FAILURE);
  }
  return rc;
 }


5. It seems that in the previously implementation, error handling was not
there, is it appropriate here that if there is issue in duplicating
environment, quit the application ? i.e.

/*
  * Handy subroutine for setting an environment variable var to val
  */
 static void
 doputenv(const char *var, const char *val)
 {
  char*s;
  pg_asprintf(s, %s=%s, var, val);
  putenv(s);
 }



Please do let me know if I missed something or more info is required. Thank
you.

Regards,
Muhammad Asif Naeem


On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote:

 Peter Eisentraut wrote:

  The attached patch should speak for itself.

 Yeah, it's a very nice cleanup.

  I have supplied a few variants:
 
  - asprintf() is the standard function, supplied by libpgport if
  necessary.
 
  - pg_asprintf() is asprintf() with automatic error handling (like
  pg_malloc(), etc.)
 
  - psprintf() is the same idea but with palloc.

 Looks good to me, except that pg_asprintf seems to be checking ret
 instead of rc.

 Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
 I don't see that we use the integer return value anywhere.  Callers
 interested in the return value can use asprintf directly (and you have
 already inserted callers that do nonstandard things using direct
 asprintf).

 --
 Á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] [PATCH] Add use of asprintf()

2013-09-17 Thread Asif Naeem
On Tue, Sep 17, 2013 at 3:13 PM, Asif Naeem anaeem...@gmail.com wrote:

 Hi,

 I did put some time review the patch, please see my findings below i.e.

 1. It seems that you have used strdup() on multiple places in the patch,
 e.g. in the below code snippet is it going to lead crash if newp-ident is
 NULL because of strdup() failure ?

 static EPlan *
 find_plan(char *ident, EPlan **eplan, int *nplans)
 {
 ...
  newp-ident = strdup(ident);
 ...
 }


 2. Can we rely on return value of asprintf() instead of recompute length
 as strlen(cmdbuf) ?

   if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 
 0)
return fail_lo_xact(\\lo_import, own_transaction);
   bufptr = cmdbuf + strlen(cmdbuf);


 3. There seems naming convention for functions like pfree (that seems
 similar to free()), pstrdup etc; but psprintf seems different than sprintf.
 Can't we use pg_asprintf instead in the patch, as it seems that both
 (psprintf and pg_asprintf) provide almost same functionality ?

 4. It seems that ret  0 need to be changed to rc  0 in the following
 code i.e.

 int
 pg_asprintf(char **ret, const char *format, ...)
 {
  va_list  ap;
  int   rc;
  va_start(ap, format);
  rc = vasprintf(ret, format, ap);
  va_end(ap);
  if (ret  0)
  {
   fprintf(stderr, _(out of memory\n));
   exit(EXIT_FAILURE);
  }
  return rc;
 }


It seems this point is already mentioned by Alvaro. Thanks.



 5. It seems that in the previously implementation, error handling was not
 there, is it appropriate here that if there is issue in duplicating
 environment, quit the application ? i.e.

 /*
  * Handy subroutine for setting an environment variable var to val
  */
 static void
 doputenv(const char *var, const char *val)
 {
  char*s;
  pg_asprintf(s, %s=%s, var, val);
  putenv(s);
 }



 Please do let me know if I missed something or more info is required.
 Thank you.

 Regards,
 Muhammad Asif Naeem


 On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera 
 alvhe...@2ndquadrant.comwrote:

 Peter Eisentraut wrote:

  The attached patch should speak for itself.

 Yeah, it's a very nice cleanup.

  I have supplied a few variants:
 
  - asprintf() is the standard function, supplied by libpgport if
  necessary.
 
  - pg_asprintf() is asprintf() with automatic error handling (like
  pg_malloc(), etc.)
 
  - psprintf() is the same idea but with palloc.

 Looks good to me, except that pg_asprintf seems to be checking ret
 instead of rc.

 Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
 I don't see that we use the integer return value anywhere.  Callers
 interested in the return value can use asprintf directly (and you have
 already inserted callers that do nonstandard things using direct
 asprintf).

 --
 Á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] pg_stat_statements: calls under-estimation propagation

2013-09-17 Thread Fujii Masao
On Sun, Sep 15, 2013 at 3:54 PM, samthakur74 samthaku...@gmail.com wrote:


 You have added this email to the commit fest, but it contains no patch.

 Please add the email with the actual patch.

  I hope its attached now!

You seem to have forgotten to include the pg_stat_statements--1.2.sql
and pg_stat_statements--1.1--1.2.sql in the patch.

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] record identical operator

2013-09-17 Thread Kevin Grittner
Andres Freund and...@2ndquadrant.com wrote:
 On 2013-09-16 15:26:08 -0700, Kevin Grittner wrote:
 I can understand claiming that the risk is acceptable but
 arguing it's not there seems extremly strange to me.

 It's not a risk.  It's why the operator exists. 

 Pft. It's fine if the materialized view code uses it. I don't see
 the danger there.
 It's about users discovering it. If they notice it, they will use
 it because its a crapload faster than normal row comparisons.

To have clean semantics, I think the operator should mean that the
stored format of the row is the same.  Regarding the array null
bitmap example, I think it would be truly weird if the operator
said that the stored format was the same, but this happened:

test=# select pg_column_size(ARRAY[1,2,3]);
 pg_column_size

 36
(1 row)

test=# select pg_column_size((ARRAY[1,2,3,NULL])::int4[3]);
 pg_column_size

 44
(1 row)

They have the same stored format, but a different number of
bytes?!?

 And deals with NULLs in a simpler way.

That might be addressed by leaving room to implement IS NOT
DISTINCT FROM as an operator.  See below.

 Perhaps the name of the operator (===) or the fact that I've
 been using the shorthand description of identical instead of
 writing out record image equals in these emails has confused
 you.

 If you really think that confusing you is the correct
 description for concerns about users not understanding
 limitations (which you didn't seem to know about), go ahead. Way
 to go to solicit feedback.

I see how that could be taken in a pejorative way; that was not
intended.  I apologize.  I was rushing a bit on that email to make
an appointment shortly afterward.  The point was to suggest that
bad communication on my part about the intent and concept of the
operator may have contributed to you saying that there was a risk
of it working as intended.  There is perhaps a risk that someone
will think that it does something other than what is intended, but
when you say that there is a risk that it will behave exactly as
intended, it does suggest that we're talking past each other.

 I can stop using the shorter description and have absolutely no
 attachment to the operator name, if that helps.

 You're unfortunately going to need operators if you want
 mergejoins to work, right?

Yes, operators are needed for that.

 If not I'd have said name it matview_needs_update() or something
 like that... But yes, === certainly seems like a bad idea.

I've come to agree with that.  The appointment was to meet with a
local PostgreSQL user who I've talked to before.  He reminded me
that his pet peeve was that he couldn't use IS [ NOT ] DISTINCT
FROM with the ALL | ANY construct because the IS [ NOT ] DISTINCT
FROM predicate isn't an operator.


Hannu Krosing ha...@2ndquadrant.com wrote:

 Maybe the binary equality operator should be named 
 for really deeply equal

 to distinguish it from === which would be merely NOT DISTINCT FROM

 we could even go one step further and define = to mean the same.

 ?

 This could fit better in conceptual sequence of operator 'strength'

I'm inclined to go with this.  It would leave the door open to
implementing IS NOT DISTINCT FROM on a type-by-type basis.  My one
concern is that it doesn't make room for a has no user-visible
differences from operator; but I'm not sure whether that is
something that there really is any use for.  But if we want to
reserve name space for it just in case someone has a use for it,
that would be between IS NOT DISTINCT FROM and has the same
storage representation as, so that would leave:

  =  equals (but doesn't necessarily look the same as)
  ===    IS NOT DISTINCT FROM as an operator
     reserved for has no user-visible differences from
  =  stored image is the same

Of course, that begs the question of whether == is already taken.
If not, we could knock one '=' off of everything above except for
equals.  What existing uses are known for == ?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] UTF8 national character data type support WIP patch and list of open issues.

2013-09-17 Thread Arulappan, Arul Shaji


-Original Message-
From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-
ow...@postgresql.org] On Behalf Of MauMau

Hello,

I think it would be nice for PostgreSQL to support national character
types
largely because it should ease migration from other DBMSs.

[Reasons why we need NCHAR]
--
1. Invite users of other DBMSs to PostgreSQL.  Oracle, SQL Server,
MySQL, etc.
all have NCHAR support.  PostgreSQL is probably the only database out
of major
ones that does not support NCHAR.
Sadly, I've read a report from some Japanese government agency that the
number
of MySQL users exceeded that of PostgreSQL here in Japan in 2010 or
2011.  I
wouldn't say that is due to NCHAR support, but it might be one reason.
I want
PostgreSQL to be more popular and regain those users.

2. Enhance the open image of PostgreSQL by implementing more features
of SQL
standard.  NCHAR may be a wrong and unnecessary feature of SQL standard
now
that we have Unicode support, but it is defined in the standard and
widely
implemented.

3. I have heard that some potential customers didn't adopt PostgreSQL
due to
lack of NCHAR support.  However, I don't know the exact reason why they
need
NCHAR.

The use case we have is for customer(s) who are modernizing their
databases on mainframes. These applications are typically written in
COBOL which does have extensive support for National Characters.
Supporting National Characters as in-built data types in PostgreSQL is,
not to exaggerate, an important criteria in their decision to use
PostgreSQL or not. (So is Embedded COBOL. But that is a separate issue.)




4. I guess some users really want to continue to use ShiftJIS or EUC_JP
for
database encoding, and use NCHAR for a limited set of columns to store
international text in Unicode:
- to avoid code conversion between the server and the client for
performance
- because ShiftJIS and EUC_JP require less amount of storage (2 bytes
for most
Kanji) than UTF-8 (3 bytes) This use case is described in chapter 6 of
Oracle
Database Globalization Support Guide.
--


I think we need to do the following:

[Minimum requirements]
--
1. Accept NCHAR/NVARCHAR as data type name and N'...' syntactically.
This is already implemented.  PostgreSQL treats NCHAR/NVARCHAR as
synonyms for
CHAR/VARCHAR, and ignores N prefix.  But this is not documented.

2. Declare support for national character support in the manual.
1 is not sufficient because users don't want to depend on undocumented
behavior.  This is exactly what the TODO item national character
support
in PostgreSQL TODO wiki is about.

3. Implement NCHAR/NVARCHAR as distinct data types, not as synonyms so
that:
- psql \d can display the user-specified data types.
- pg_dump/pg_dumpall can output NCHAR/NVARCHAR columns as-is, not as
CHAR/VARCHAR.
- To implement additional features for NCHAR/NVARCHAR in the future, as
described below.
--


Agreed. This is our minimum requirement too. 

Rgds,
Arul Shaji








[Optional requirements]
--
1. Implement client driver support, such as:
- NCHAR host variable type (e.g. NCHAR var_name[12];) in ECPG, as
specified
in the SQL standard.
- national character methods (e.g. setNString, getNString,
setNCharacterStream) as specified in JDBC 4.0.
I think at first we can treat these national-character-specific
features as the
same as CHAR/VARCHAR.

2. NCHAR/NVARCHAR columns can be used in non-UTF-8 databases and always
contain
Unicode data.
I think it is sufficient at first that NCHAR/NVARCHAR columns can only
be used
in UTF-8 databases and they store UTF-8 strings.  This allows us to
reuse the
input/output/send/recv functions and other infrastructure of
CHAR/VARCHAR.
This is a reasonable compromise to avoid duplication and minimize the
first
implementation of NCHAR support.

3. Store strings in UTF-16 encoding in NCHAR/NVARCHAR columns.
Fixed-width encoding may allow faster string manipulation as described
in
Oracle's manual.  But I'm not sure about this, because UTF-16 is not a
real
fixed-width encoding due to supplementary characters.

This would definitely be a welcome addition. 



--


I don't think it is good to implement NCHAR/NVARCHAR types as
extensions like
contrib/citext, because NCHAR/NVARCHAR are basic types and need
client-side
support.  That is, client drivers need to be aware of the fixed
NCHAR/NVARCHAR
OID values.

How do you think we should implement NCHAR support?

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




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

Re: [HACKERS] record identical operator

2013-09-17 Thread Rod Taylor
On Tue, Sep 17, 2013 at 8:23 AM, Kevin Grittner kgri...@ymail.com wrote:


 Of course, that begs the question of whether == is already taken.
 If not, we could knock one '=' off of everything above except for
 equals.  What existing uses are known for == ?


== is already taken as a common typo in plpgsql scripts. I strongly prefer
if this remained an error.

IF foo == bar THEN ...


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-17 Thread Fujii Masao
On Tue, Sep 17, 2013 at 3:45 PM, Samrat Revagade
revagade.sam...@gmail.com wrote:

  syncrep.c: In function ‘SyncRepReleaseWaiters’:
  syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used
  [-Wunused-but-set-variable]
 

 Sorry I forgot fix it.

 I have attached the patch which I modified.


 Attached patch combines documentation patch and source-code patch.

I set up synchronous replication with synchronous_transfer = all, and then I ran
pgbench -i and executed CHECKPOINT in the master. After that, when I executed
CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by
synchronous_transfer feature.

How does synchronous_transfer work with cascade replication? If it's set to all
in the sender-side standby, it can resolve the data page inconsistency between
two standbys?

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] insert throw error when year field len 4 for timestamptz datatype

2013-09-17 Thread Haribabu kommi
On Tue, 17 September 2013 14:33 Rushabh Lathia wrote:
On Mon, Sep 16, 2013 at 7:22 PM, Haribabu kommi 
haribabu.ko...@huawei.commailto:haribabu.ko...@huawei.com wrote:
On 14 August 2013 Rushabh Lathia wrote:
 postgres=# create table test ( a timestamptz);
CREATE TABLE
 -- Date with year 1000
postgres=#  insert into test values ( 'Sat Mar 11 23:58:48 1000 IST');
INSERT 0 1
 -- Now try with year 1 it will return error
postgres=#  insert into test values ( 'Sat Mar 11 23:58:48 1 IST');
ERROR:  invalid input syntax for type timestamp with time zone: Sat Mar 11 
23:58:48 1 IST
LINE 1: insert into test values ( 'Sat Mar 11 23:58:48 1 IST');
Patch applies cleanly to HEAD. As this patch tries to improve in inserting 
the date of the year value to be more than 4 in length.
But it didn't solve all the ways to insert the year field more than 4 in 
length. Please check the following test.

 postgres=# insert into test values ('10001010 10:10:10 IST');
INSERT 0 1
postgres=# insert into test values ('100011010 10:10:10 IST');
ERROR:  invalid input syntax for type timestamp with time zone: 100011010 
10:10:10 IST at character 26
STATEMENT:  insert into test values ('100011010 10:10:10 IST');
ERROR:  invalid input syntax for type timestamp with time zone: 100011010 
10:10:10 IST
LINE 1: insert into test values ('100011010 10:10:10 IST');
^
 I feel it is better to provide the functionality of inserting year field 
 more than 4 in length in all flows.

+1. Nice catch.

Here is the latest version of patch which handles the functionality in all 
flows.
Could you test it and share you comments.

I am getting some other failures with the updated patch also, please check the 
following tests.

select date 'January 8, 19990';
select timestamptz 'January 8, 199910 01:01:01 IST';
INSERT INTO TIMESTAMPTZ_TST VALUES(4, '10001 SAT 8 MAR 10:10:10 IST');

you can get the test scripts from regress test files of date.sql, timetz.sql, 
timestamp.sql and timestamptz.sql
and modify according to the patch for verification.

I feel changing the year value to accept the length (4) is not simple.
So many places the year length crossing more than length 4 is not considered.
Search in the code with  and correct all related paths.

Regards,
Hari babu.



Re: [HACKERS] Fix picksplit with nan values

2013-09-17 Thread Alexander Korotkov
On Mon, Sep 16, 2013 at 4:13 PM, Andrew Gierth
and...@tao11.riddles.org.ukwrote:

  Alexander == Alexander Korotkov aekorot...@gmail.com writes:

  Alexander 2) NaN coordinates should be processed in GiST index scan
  Alexander like in sequential scan.

 postgres=# select * from pts order by a - '(0,0)' limit 10;
 a
 --
  (1,1)
  (7,nan)
  (9,nan)
  (11,nan)
  (4,nan)
  (nan,6)
  (2,1)
  (1,2)
  (2,2)
  (3,1)
 (10 rows)

 postgres=# set enable_indexscan=false;
 SET

 postgres=# select * from pts order by a - '(0,0)' limit 10;
a
 ---
  (1,1)
  (2,1)
  (1,2)
  (2,2)
  (3,1)
  (1,3)
  (3,2)
  (2,3)
  (4,1)
  (1,4)
 (10 rows)

 this data set was created by:
 insert into pts
   select point(i,j)
 from (select generate_series(1,100)::float8 union all select 'nan')
 s1(i),
  (select generate_series(1,100)::float8 union all select 'nan')
 s2(j)
order by random();


Thanks, Andrew! Good spot.
I didn't examine order by operators for work with NaNs.
I think this time problem is in GiST itself rather than in opclass. I'm
going to fix it in a separate patch.

--
With best regards,
Alexander Korotkov.


Re: [HACKERS] record identical operator

2013-09-17 Thread Hannu Krosing
On 09/17/2013 02:46 PM, Rod Taylor wrote:
 On Tue, Sep 17, 2013 at 8:23 AM, Kevin Grittner kgri...@ymail.com
 mailto:kgri...@ymail.com wrote:


 Of course, that begs the question of whether == is already taken.
 If not, we could knock one '=' off of everything above except for
 equals.  What existing uses are known for == ?


 == is already taken as a common typo in plpgsql scripts. I strongly
 prefer if this remained an error.

 IF foo == bar THEN ...
That was also my reason for not suggesting == .
It is too widely used in other systems for simple equality check.

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ



Re: [HACKERS] record identical operator

2013-09-17 Thread Hannu Krosing
On 09/17/2013 12:52 AM, Andres Freund wrote:
 On 2013-09-16 15:26:08 -0700, Kevin Grittner wrote:
 I can understand claiming that the risk is acceptable but arguing
 it's not there seems extremly strange to me.
 It's not a risk.  It's why the operator exists. 
 Pft. It's fine if the materialized view code uses it. I don't see the
 danger there.
 It's about users discovering it. If they notice it, they will use it
 because its a crapload faster than normal row comparisons. And deals
 with NULLs in a simpler way.
This is why I suggested naming the operator  with four '='s
as this should make the user ponder why there are so many equal signs.
And there are other languages where more equal signs mean stronger
equality.

Basically it is equality with possible false negatives.

If a user wants to use it for speeding up simple equality, it should be
used as
WHERE t.*  sample AND t.* = sample


 Perhaps the name
 of the operator (===) or the fact that I've been using the
 shorthand description of identical instead of writing out record
 image equals in these emails has confused you.
 If you really think that confusing you is the correct description for
 concerns about users not understanding limitations (which you didn't
 seem to know about), 
IIRC he did start with a visible limitation of = being too weak equality
for some fields
 go ahead. Way to go to solicit feedback.

 I can stop using
 the shorter description and have absolutely no attachment to the
 operator name, if that helps.
 You're unfortunately going to need operators if you want mergejoins to
 work, right? If not I'd have said name it matview_needs_update() or
 something like that... But yes, === certainly seems like a bad idea.
but having === as an operator form of IS NOT DISTINCT FROM
seems like a good idea to me.

We will get an operator which most people expect, with sufficiently
strange name to make people look up the docs and we will not be
violating SQL spec.

And it prepares their heads for stronger semantics of  :)

 Greetings,

 Andres Freund




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


Re: [HACKERS] Minmax indexes

2013-09-17 Thread Jaime Casanova
On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote:
 On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
  On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
  wrote:
 
  Hi,
 
  Here's a reviewable version of what I've dubbed Minmax indexes.
 
  Thanks for the patch, but I seem to have immediately hit a snag:
 
  pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax
  (aid);
  PANIC:  invalid xlog record length 0
 

 fwiw, this seems to be triggered by ANALYZE.
 At least i can trigger it by executing ANALYZE on the table (attached
 is a stacktrace of a backend exhibiting the failure)


 I'm able to run ANALYSE manually without it dying:


try inserting some data before the ANALYZE, that will force a
resumarization which is mentioned in the stack trace of the failure

-- 
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] New statistics for WAL buffer dirty writes

2013-09-17 Thread Fujii Masao
On Wed, Sep 11, 2013 at 12:43 PM, Satoshi Nagayasu sn...@uptime.jp wrote:
 (2013/09/10 22:48), Peter Eisentraut wrote:
 On 9/10/13 3:37 AM, Satoshi Nagayasu wrote:
 Thanks for checking. Revised one attached.

 Please fix compiler warning:

 walwriter.c: In function ‘WalWriterMain’:
 walwriter.c:293:3: warning: implicit declaration of function
 ‘pgstat_send_walwriter’ [-Wimplicit-function-declaration]

 Thanks. Fixed.

The patch looks good to me. I have some comments:

The description of pg_stat_reset_shared() should mention
pg_stat_walwriter in the document.

We should implment something like pg_stat_reset_shared('all') so that
we can easily reset
all cluster-wide statistics counters to zero?

Some background workers may write WAL because WAL buffer is full. So you seem to
need to change those processes so that they also can increase the
xlog_dirty_writes
counter.

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] Freezing without write I/O

2013-09-17 Thread Peter Eisentraut
On 9/16/13 9:59 AM, Heikki Linnakangas wrote:
 Here's a rebased version of the patch, including the above-mentioned
 fixes. Nothing else new.

It still fails to apply.  You might need to rebase it again.


-- 
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

2013-09-17 Thread Thom Brown
On 17 September 2013 14:37, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote:
  On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com
 wrote:
 
  On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
   On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
   wrote:
  
   Hi,
  
   Here's a reviewable version of what I've dubbed Minmax indexes.
  
   Thanks for the patch, but I seem to have immediately hit a snag:
  
   pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax
   (aid);
   PANIC:  invalid xlog record length 0
  
 
  fwiw, this seems to be triggered by ANALYZE.
  At least i can trigger it by executing ANALYZE on the table (attached
  is a stacktrace of a backend exhibiting the failure)
 
 
  I'm able to run ANALYSE manually without it dying:
 

 try inserting some data before the ANALYZE, that will force a
 resumarization which is mentioned in the stack trace of the failure


I've tried inserting 1 row then ANALYSE and 10,000 rows then ANALYSE, and
in both cases there's no error.  But then trying to create the index again
results in my original error.

-- 
Thom


Re: [HACKERS] Freezing without write I/O

2013-09-17 Thread Andres Freund
On 2013-09-17 09:41:47 -0400, Peter Eisentraut wrote:
 On 9/16/13 9:59 AM, Heikki Linnakangas wrote:
  Here's a rebased version of the patch, including the above-mentioned
  fixes. Nothing else new.
 
 It still fails to apply.  You might need to rebase it again.

FWIW, I don't think it's too important that this specific patch applies
all the time. From the look I had and the discussions onlist, what it
needs so far is mostly architecture review and such.

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] logical changeset generation v6

2013-09-17 Thread Peter Eisentraut
On 9/15/13 11:30 AM, Andres Freund wrote:
 On 2013-09-15 11:20:20 -0400, Peter Eisentraut wrote:
 On Sat, 2013-09-14 at 22:49 +0200, Andres Freund wrote:
 Attached you can find the newest version of the logical changeset
 generation patchset.

 You probably have bigger things to worry about, but please check the
 results of cpluspluscheck, because some of the header files don't
 include header files they depend on.
 
 Hm. I tried to get that right, but it's been a while since I last
 checked. I don't regularly use cpluspluscheck because it doesn't work in
 VPATH builds... We really need to fix that.
 
 I'll push a fix for that to the git tree, don't think that's worth a
 resend in itself.

This patch set now fails to apply because of the commit Rename various
freeze multixact variables.



-- 
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_stat_statements: calls under-estimation propagation

2013-09-17 Thread samthakur74
You seem to have forgotten to include the pg_stat_statements--1.2.sql
and pg_stat_statements--1.1--1.2.sql in the patch.


 Sorry again. Please find updated patch attached.



 http://www.postgresql.org/mailpref/pgsql-hackers

 NAMLhttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=macro_viewerid=instant_html%21nabble%3Aemail.namlbase=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespacebreadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml




On Tue, Sep 17, 2013 at 5:47 PM, Fujii Masao-2 [via PostgreSQL] 
ml-node+s1045698n5771213...@n5.nabble.com wrote:

 On Sun, Sep 15, 2013 at 3:54 PM, samthakur74 [hidden 
 email]http://user/SendEmail.jtp?type=nodenode=5771213i=0
 wrote:
 
 
  You have added this email to the commit fest, but it contains no
 patch.
 
  Please add the email with the actual patch.
 
   I hope its attached now!

 You seem to have forgotten to include the pg_stat_statements--1.2.sql
 and pg_stat_statements--1.1--1.2.sql in the patch.

 Regards,

 --
 Fujii Masao


 --
 Sent via pgsql-hackers mailing list ([hidden 
 email]http://user/SendEmail.jtp?type=nodenode=5771213i=1)

 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


 --
  If you reply to this email, your message will be added to the discussion
 below:

 http://postgresql.1045698.n5.nabble.com/pg-stat-statements-calls-under-estimation-propagation-tp5738128p5771213.html
  To unsubscribe from pg_stat_statements: calls under-estimation
 propagation, click 
 herehttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=unsubscribe_by_codenode=5738128code=c2FtdGhha3VyNzRAZ21haWwuY29tfDU3MzgxMjh8ODM4MzYxMTcy
 .
 NAMLhttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=macro_viewerid=instant_html%21nabble%3Aemail.namlbase=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespacebreadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml



pg_stat_statements-identification-v4.patch.gz (9K) 
http://postgresql.1045698.n5.nabble.com/attachment/5771248/0/pg_stat_statements-identification-v4.patch.gz




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/pg-stat-statements-calls-under-estimation-propagation-tp5738128p5771248.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] record identical operator

2013-09-17 Thread Dimitri Fontaine
Kevin Grittner kgri...@ymail.com writes:
   =  equals (but doesn't necessarily look the same as)
   ===    IS NOT DISTINCT FROM as an operator
      reserved for has no user-visible differences from
   =  stored image is the same

I understand the need for more than one equality operator and my
programming language of choice exposes eq, eql, equal and equalp for
solving a very similar situation. Oh, and also offers = and string= and
string-equal, among others.

My vote goes against using a different number of the same character to
name the operators though, as that's not visually helpful enough IMO.

In yet another language whose grammar took inspiration with Prolog,
dealing with binary is made with explicitely using the bit syntax
expression where any datum is boxed into  and .

  http://erlang.org/doc/reference_manual/expressions.html#id78513

It might not be practical to do the same in SQL and have either the
following syntax example or even a unary bit representation operator
to do the same:

  SELECT row(expression, here) = column_name …

That would probably prevent using indexes?

It might be inspiration to name the bit-comparison operator something
like = though, or == or something else. If only we could force the
usage of unicode for the SQL input itself… I'm sure there's a unicode
glyph perfect for what we want here…

About IS NOT DISTINCT FROM, I would propose something where NOT and
DISTINCT are kept as concepts. Not usually is expressed with ! and we
already have a bunch of same as or matches operators with ~, so:

  ~   IS DISTINCT FROM
  !~  IS NOT DISTINCT FROM

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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


Re: [HACKERS] Re: Proposal/design feedback needed: WITHIN GROUP (sql standard ordered set aggregate functions)

2013-09-17 Thread Robert Haas
On Sat, Sep 14, 2013 at 7:23 AM, Andrew Gierth
and...@tao11.riddles.org.uk wrote:
  Peter Please fix compiler warnings:

 Someone should do the same in WaitForBackgroundWorkerStartup so that
 building with -Werror works.

I don't get a warning there.  Can you be more specific about the problem?

-- 
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] unaccent module - two params function should be immutable

2013-09-17 Thread Robert Haas
On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 I have developed the attached patch based on your suggestion.  I did not
 see anything in the code that would make it STABLE, except a lookup of a
 dictionary library:

 dictOid = get_ts_dict_oid(stringToQualifiedNameList(unaccent),
 false);

 yes, it risk, but similar is with timezones, and other external data.

That's a catalog lookup - not a reliance on external data.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


[HACKERS] output/security_label.source referring to abs_builddir instead of libdir

2013-09-17 Thread Hamid Quddus Akhtar
In make check, there are 4 shared libraries that are loaded for test cases
from the src/test/regress folder. Whereas, output of other libraries
contain @libdir@ tag, the output file for security_label (dumm_seclabel)
contains a tag for @abs_builddir@ in the output/security_label.source file.

I believe this an issue, and rather than referring @abs_builddir@, it
should instead contain the tag @libdir@. Also note that while I could
provide a path for dynamic libraries via dlpath option of pg_regress,
this is not honored by security_label test case.

PFA patch for this fix.


Regards.

-- 
*
*
*Hamid Quddus Akhtar*
Configuration Management Team



Ph: +92.333.544.9950
Skype ID: EngineeredVirus
www.enterprisedb.co
http://www.enterprisedb.com/mhttp://www.enterprisedb.com/
*
Follow us on Twitter*
@EnterpriseDB

Visit EnterpriseDB for tutorials, webinars,
whitepapershttp://www.enterprisedb.com/resources-community and
more http://www.enterprisedb.com/resources-community


security_label.source.output.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] json docs fixup

2013-09-17 Thread Robert Haas
On Sat, Sep 14, 2013 at 3:23 PM, Andrew Dunstan and...@dunslane.net wrote:
 While writing slides for pgopen next week, I noticed that the JSON docs on
 json_populate_record and json_populate_recordset contain this sentence:

A column may only be specified once.


 IIRC we removed that restriction  during development, so unless there is a
 squawk I am going to simply remove that sentence where it occurs. Or should
 we say something like:

If a column is specified more than once, the last value is used.

If the latter statement is accurate, I favor including it, versus
saying nothing on the topic.

-- 
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] [RFC] Extend namespace of valid guc names

2013-09-17 Thread Robert Haas
On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote:
 a) allow repeatedly qualified names like a.b.c

 The attached patch does a)

What is the use case for this change?

-- 
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] [RFC] Extend namespace of valid guc names

2013-09-17 Thread Andres Freund
On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
 On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote:
  a) allow repeatedly qualified names like a.b.c
 
  The attached patch does a)
 
 What is the use case for this change?

Check 
http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
or the commit message of the patch.

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] pg_system_identifier()

2013-09-17 Thread Andres Freund
On 2013-09-17 10:57:46 -0400, Robert Haas wrote:
 On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote:
  How about adding new system view with new function which returns
  a single pg_controldata value in text type, and using a cast for
  each column in the view definition?
 
  CREATE VIEW pg_catalog.pg_controldata AS
SELECT pg_controldata('control_version')::integer AS control_version,
   pg_controldata('catalog_version')::integer AS catalog_version,
   pg_controldata('system_identifier')::bigint AS system_identifier,
   ...
   pg_controldata('next_xlog_file')::char(25) AS next_xlog_file,
   ...
   pg_controldata('encoding')::text AS encoding;
 
  Given that the view can work like a SRF, and it allows us to retrieve
  all the values of pg_controldata with appropriate types in single
  record from the view:
 
 I like this idea.  I think having an easy way to get the values with
 the right types will be a plus.  But adding a separate function for
 each field seems excessive, so I think this is a good compromise.

Why not add a single function returning a composite type then? That'd at
least have a chance of returning consistent values for the individual
values that change during runtime. It would also produce proper errors
when you load a view using columns that don't exist anymore instead of
just at runtime.

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] proposal: simple date constructor from numeric values

2013-09-17 Thread Alvaro Herrera
Pavel Stehule escribió:

 fixed - see attached patch

There's a typo tange in some error messages, which has found its way
to the regression tests.

-- 
Á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] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Is it necessary to rewrite table while increasing the scale of datatype numeric?

2013-09-17 Thread Robert Haas
On Sun, Sep 15, 2013 at 8:05 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 But note that the current behavior is worse in this regard.  If you specify
 a scale of 4 at the column level, than it is not possible to distinguish
 between 5.000 and 5. on a per-value basis within that column.  If the
 scale at the column level was taken as the maximum scale, not the only
 allowed one, then that distinction could be recorded. That behavior seems
 more sensible to me (metrologically speaking, regardless of alter table
 performance aspects), but I don't see how to get there from here with
 acceptable compatibility breakage.

I think I'd probably agree with that in a green field, but as you say,
I can't see accepting the backward compatibility break at this point.
After all, you can get variable-precision in a single column by
declaring it as unqualified numeric.

-- 
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] pg_system_identifier()

2013-09-17 Thread Robert Haas
On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote:
 How about adding new system view with new function which returns
 a single pg_controldata value in text type, and using a cast for
 each column in the view definition?

 CREATE VIEW pg_catalog.pg_controldata AS
   SELECT pg_controldata('control_version')::integer AS control_version,
  pg_controldata('catalog_version')::integer AS catalog_version,
  pg_controldata('system_identifier')::bigint AS system_identifier,
  ...
  pg_controldata('next_xlog_file')::char(25) AS next_xlog_file,
  ...
  pg_controldata('encoding')::text AS encoding;

 Given that the view can work like a SRF, and it allows us to retrieve
 all the values of pg_controldata with appropriate types in single
 record from the view:

I like this idea.  I think having an easy way to get the values with
the right types will be a plus.  But adding a separate function for
each field seems excessive, so I think this is a good compromise.

-- 
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] pg_stat_statements: calls under-estimation propagation

2013-09-17 Thread Fujii Masao
On Tue, Sep 17, 2013 at 10:48 PM, samthakur74 samthaku...@gmail.com wrote:




 You seem to have forgotten to include the pg_stat_statements--1.2.sql
 and pg_stat_statements--1.1--1.2.sql in the patch.


 Sorry again. Please find updated patch attached.

pg_stat_statements--1.2.sql is missing. Could you confirm that the patch
includes all the changes?

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_system_identifier()

2013-09-17 Thread Robert Haas
On Tue, Sep 17, 2013 at 10:59 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-09-17 10:57:46 -0400, Robert Haas wrote:
 On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote:
  How about adding new system view with new function which returns
  a single pg_controldata value in text type, and using a cast for
  each column in the view definition?
 
  CREATE VIEW pg_catalog.pg_controldata AS
SELECT pg_controldata('control_version')::integer AS control_version,
   pg_controldata('catalog_version')::integer AS catalog_version,
   pg_controldata('system_identifier')::bigint AS system_identifier,
   ...
   pg_controldata('next_xlog_file')::char(25) AS next_xlog_file,
   ...
   pg_controldata('encoding')::text AS encoding;
 
  Given that the view can work like a SRF, and it allows us to retrieve
  all the values of pg_controldata with appropriate types in single
  record from the view:

 I like this idea.  I think having an easy way to get the values with
 the right types will be a plus.  But adding a separate function for
 each field seems excessive, so I think this is a good compromise.

 Why not add a single function returning a composite type then? That'd at
 least have a chance of returning consistent values for the individual
 values that change during runtime. It would also produce proper errors
 when you load a view using columns that don't exist anymore instead of
 just at runtime.

Hmm.  Yeah, that might be better.

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

2013-09-17 Thread Jaime Casanova
On Tue, Sep 17, 2013 at 8:43 AM, Thom Brown t...@linux.com wrote:
 On 17 September 2013 14:37, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote:
  On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com
  wrote:
 
  On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
   On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
   wrote:
  
   Hi,
  
   Here's a reviewable version of what I've dubbed Minmax indexes.
  
   Thanks for the patch, but I seem to have immediately hit a snag:
  
   pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax
   (aid);
   PANIC:  invalid xlog record length 0
  
 
  fwiw, this seems to be triggered by ANALYZE.
  At least i can trigger it by executing ANALYZE on the table (attached
  is a stacktrace of a backend exhibiting the failure)
 
 
  I'm able to run ANALYSE manually without it dying:
 

 try inserting some data before the ANALYZE, that will force a
 resumarization which is mentioned in the stack trace of the failure


 I've tried inserting 1 row then ANALYSE and 10,000 rows then ANALYSE, and in
 both cases there's no error.  But then trying to create the index again
 results in my original error.


Ok

So, please confirm if this is the pattern you are following:

CREATE TABLE t1(i int);
INSERT INTO t1 SELECT generate_series(1, 1);
CREATE INDEX idx1 ON t1 USING minmax (i);

if that, then the attached stack trace (index_failure_thom.txt) should
correspond to the failure you are looking.

My test was slightly different:

CREATE TABLE t1(i int);
CREATE INDEX idx1 ON t1 USING minmax (i);
INSERT INTO t1 SELECT generate_series(1, 1);
ANALYZE t1;

and the failure happened in a different time, in resumarization
(attached index_failure_jcm.txt)

but in the end, both failures seems to happen for the same reason: a
record of length 0... at XLogInsert time

#4  XLogInsert at xlog.c:966
#5  mmSetHeapBlockItemptr at mmrevmap.c:169
#6  mm_doinsert at minmax.c:1410

actually, if you create a temp table both tests works fine

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
Program received signal SIGABRT, Aborted.
0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64  ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el 
directorio.
(gdb) bt
#0  0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f442881c6f0 in *__GI_abort () at abort.c:92
#2  0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546
#3  0x0076fdca in elog_finish (elevel=elevel@entry=22, 
fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304
#4  0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', 
info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966
#5  0x004a9bb9 in mmSetHeapBlockItemptr 
(rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, 
blkno=blkno@entry=6, offno=offno@entry=1)
at mmrevmap.c:169
#6  0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, 
rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, 
tup=tup@entry=0x7f441f84cff8, 
itemsz=16) at minmax.c:1410
#7  0x004a9464 in rerun_summarization (numnonsummarized=22, 
nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, 
idxRel=0x7f4429054c88)
at minmax.c:1205
#8  mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268
#9  0x0077388f in FunctionCall2Coll 
(flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, 
arg1=arg1@entry=140736552432368, arg2=arg2@entry=0)
at fmgr.c:1326
#10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, 
stats=stats@entry=0x0) at indexam.c:715
#11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, 
acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 
'\000', 
elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf 
expression opcode 0xfa, 
vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) 
at analyze.c:634
#12 0x00557fef in analyze_rel (relid=relid@entry=16384, 
vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267
---Type return to continue, or q return to quit---
#13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, 
relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized 
out, 
bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', 
isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249
#14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, 
queryString=optimized out, context=optimized out, params=0x0, 
dest=optimized out, 
completionTag=optimized out) at utility.c:682
#15 0x0069c587 in PortalRunUtility 

Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2013-09-17 Thread Robert Haas
On Sat, Sep 14, 2013 at 6:27 PM, Peter Geoghegan p...@heroku.com wrote:
 Note that today there is no guarantee that the original waiter for a
 duplicate-inserting xact to complete will be the first one to get a
 second chance, so I think it's hard to question this on correctness
 grounds. Even if they are released in FIFO order, there is no reason
 to assume that the first waiter will win the race with a second. Most
 obviously, the second waiter may not even ever get the chance to block
 on the same xid at all (so it's not really a waiter at all) and still
 be able to insert, if the blocking-xact aborts after the second
 waiter starts its descent but before it checks uniqueness. All this,
 even though the second waiter arrived maybe minutes after the first.

ProcLockWakeup() only wakes as many waiters from the head of the queue
as can all be granted the lock without any conflicts.  So I don't
think there is a race condition in that path.

 So far it's
 been a slippery slope type argument that can be equally well used to
 argue against some facet of almost any substantial patch ever
 proposed.

I don't completely agree with that characterization, but you do have a
point.  Obviously, if the differences in the area of interruptibility,
starvation, deadlock risk, etc. can be made small enough relative to
the status quo can be made small enough, then those aren't reasons to
reject the approach.

But I'm skeptical that you're going to be able to accomplish that,
especially without adversely affecting maintainability.  I think the
way that you're proposing to use lwlocks here is sufficiently
different from what the rest of the system does that it's going to be
hard to avoid system-wide affects that can't easily be caught during
code review; and like Andres, I don't share your skepticism about
alternative approaches.

 For that matter, if the table has more than MAX_SIMUL_LWLOCKS indexes,
 you'll error out.  In fact, if you get the number of indexes exactly
 right, you'll exceed MAX_SIMUL_LWLOCKS in visibilitymap_clear() and
 panic the whole system.

 Oh, come on. We can obviously engineer a solution to that problem. I
 don't think I've ever seen a table with close to 100 *unique* indexes.
 4 or 5 is a very high number. If we just raised on error if someone
 tried to do this with more than 10 unique indexes, I would guess
 that'd we'd get exactly zero complaints about it.

That's not a solution; that's a hack.

 Undetected deadlock is really not much worse than detected deadlock
 here. Either way, it's a bug. And it's something that any kind of
 implementation will need to account for. It's not okay to
 *unpredictably* deadlock, in a way that the user has no control over.
 Today, someone can do an analysis of their application and eliminate
 deadlocks if they need to. That might not be terribly practical much
 of the time, but it can be done. It certainly is practical to do it in
 a localized way. I wouldn't like to compromise that.

I agree that unpredictable deadlocks are bad.  I think the fundamental
problem with UPSERT, MERGE, and this proposal is what happens when the
conflicting tuple is present but not visible to your scan, either
because it hasn't committed yet or because it has committed but is not
visible to your snapshot.  I'm not clear on how you handle that in
your approach.

 If you look at the code, you'll see that I've made very modest
 modifications to LWLockRelease only. I would be extremely surprised if
 the overhead was not only in the noise, but was completely impossible
 to detect through any conventional benchmark. These are the same kind
 of very modest changes made for LWLockAcquireOrWait(), and you said
 nothing about that at the time. Despite the fact that you now appear
 to think that that whole effort was largely a waste of time.

Well, I did have some concerns about the performance impact of that patch:

http://www.postgresql.org/message-id/CA+TgmoaPyQKEaoFz8HkDGvRDbOmRpkGo69zjODB5=7jh3hb...@mail.gmail.com

I also discovered, after it was committed, that it didn't help in the
way I expected:

http://www.postgresql.org/message-id/CA+TgmoY8P3sD=ouvig+xzjmzk5-phunv39rtfyzuqxu8hjt...@mail.gmail.com

It's true that I didn't raise those concerns contemporaneously with
the commit, but I didn't understand the situation well enough at that
time to realize how narrow the benefit was.

I've wished, on a number of occasions, to be able to add more lwlock
primitives.  The problem with that is that if everybody does it, we'll
pretty soon end up with a mess.  I attempted to address that with this
proposal:

http://www.postgresql.org/message-id/ca+tgmob4ye_k5dpo0t07pnf1sokpybo+wj4m4fryos7z4_y...@mail.gmail.com

...but nobody (including me) was very sure that was the right way
forward, and it never went anywhere.  However, I think the basic issue
remains.  I was sad to discover last week that Heikki handled this
problem for the WAL scalability patch by basically copy-and-pasting
much of the lwlock 

Re: [HACKERS] [PERFORM] encouraging index-only scans

2013-09-17 Thread Jim Nasby

On 9/7/13 12:34 AM, Andres Freund wrote:

What I was thinking of was to keep track of the oldest xids on pages
that cannot be marked all visible. I haven't thought about the
statistics part much, but what if we binned the space between
[RecentGlobalXmin, -nextXid) into 10 bins and counted the number of
pages falling into each bin. Then after the vacuum finished we could
compute how far RecentGlobalXmin would have to progress to make another
vacuum worthwile by counting the number of pages from the lowest bin
upwards and use the bin's upper limit as the triggering xid.

Now, we'd definitely need to amend that scheme by something that handles
pages that are newly written to, but it seems something like that
wouldn't be too hard to implement and would make autovacuum more useful.


If we're binning by XID though you're still dependent on scanning to build that 
range. Anything that creates dead tuples will also be be problematic, because 
it's going to unset VM bits on you, and you won't know if it's due to INSERTS 
or dead tuples.

What if we maintained XID stats for ranges of pages in a separate fork? Call it 
the XidStats fork. Presumably the interesting pieces would be min(xmin) and 
max(xmax) for pages that aren't all visible. If we did that at a granularity 
of, say, 1MB worth of pages[1] we're talking 8 bytes per MB, or 1 XidStats page 
per GB of heap. (Worst case alignment bumps that up to 2 XidStats pages per GB 
of heap.)

Having both min(xmin) and max(xmax) for a range of pages would allow for very 
granular operation of vacuum. Instead of hitting every heap page that's not 
all-visible, it would only hit those that are not visible and where min(xmin) 
or max(xmax) were less than RecentGlobalXmin.

One concern is maintaining this data. A key point is that we don't have to 
update it every time it changes; if the min/max are only off by a few hundred 
XIDs there's no point to updating the XidStats page. We'd obviously need the 
XidStats page to be read in, but even a 100GB heap would be either 100 or 200 
XidStats pages.

[1]: There's a trade-off between how much space we 'waste' on XidStats pages 
and how many heap pages we potentially have to scan in the range. We'd want to 
see what this looked like in a real system. The thing that helps here is that 
regardless of what the stats for a particular heap range are, you're not going 
to scan any pages in that range that are already all-visible.
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


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


Re: [HACKERS] relscan_details.h

2013-09-17 Thread Noah Misch
On Mon, Sep 16, 2013 at 11:02:28PM -0300, Alvaro Herrera wrote:
 relscan.h is a bit of an unfortunate header because it requires a lot of
 other headers to compile, and is also required by execnodes.h.  This

Not in my copy of the source tree.  execnodes.h uses the corresponding
typedefs that appear in genam.h and heapam.h.  Indeed, find . -name '*.Po' |
xargs grep -l relscan.h | wc -l only locates 26 files that include relscan.h
directly or indirectly.

 quick patch removes the struct definitions from that file, moving them
 into a new relscan_details.h file; the reworked relscan.h does not need
 any other header, freeing execnodes.h from needlessly including lots of
 unnecessary stuff all over the place.  Only the files that really need
 the struct definition include the new file, and as seen in the patch,
 they are quite few.
 
 execnodes.h is included by 147 backend source files; relscan_details.h
 only 27.  So the exposure area is reduced considerably.  This patch also
 modifies a few other backend source files to add one or both of genam.h
 and heapam.h, which were previously included through execnodes.h.
 
 This is probably missing tweaks for contrib/.  The following modules
 would need to include relscan_details.h: sepgsql dblink pgstattuple
 pgrowlocks.  I expect the effect on third-party modules to be much less
 than by the htup_details.h change.

-1 on header restructuring in the absence of noteworthy compile-time benchmark
improvements.  Besides the obvious benchmark of full-build time, one could
exercise the benefit of fewer header dependencies by modelling the series of
compile times from running git pull; make at each commit for the past
several months.  The latter benchmark is perhaps more favorable.

nm

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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


Re: [HACKERS] Freezing without write I/O

2013-09-17 Thread Alvaro Herrera
Heikki Linnakangas escribió:

 Here's a rebased version of the patch, including the above-mentioned
 fixes. Nothing else new.

Nice work.  I apologize for the conflicts I created yesterday.

I would suggest renaming varsup_internal.h to varsup_xlog.h.

You added a FIXME comment to HeapTupleHeaderIsOnlyLocked.  I think the
answer to the question is yes, because checking for locked might incur
examining the Xmax of tuples, which cannot be done if the page is
mature; perhaps the check for maturity needs to happen only after the
infomask has been checked.

The new stuff in GetNewTransactionId() is said to be very expensive
(it might involve grabbing the ProcArrayLock and scanning the whole
procarray, for example.)  There's a comment about having this be done
only in other processes, and I think that's a good idea, otherwise we
risk adding a lot of latency, randomly, to client-connected processes
which might be latency sensitive.  I think autovacuum is not a good
choice however (it might not even be running).  How about using the
bgwriter or walwriter for this?  As far as I can tell, there's no need
for this process to actually be able to run transactions or scan
catalogs; the ability to lock and scan ProcArray appears sufficient.
Using a RTC (instead of the Xid counter % 128) to determine when to run
this doesn't look like a problem to me.  Something that sleeps for too
long might be, so we would need to ensure that it happens timely.  Not
sure what's an appropriate time for this, however.

Another option is have backends check the Xid counter, and every 128
ticks set a flag in shmem so that the background process knows to
execute the switch.


heap_freeze_tuple() receives one Xid and one MultiXactId; they can be
passed as Invalid to mean forced freezing.  Do we really need to offer
the possibility of freezing xids but not multis, and multis but not
xids?  From a conceptual PoV, it seems to make more sense to me to pass
a boolean force meaning freeze everything, and ignore the numerical
values.


The greatest risk in this patch is the possibility that some
heap_freeze_page() call is forgotten.  I think you covered all cases in
heapam.c.

-- 
Á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] Re: Proposal/design feedback needed: WITHIN GROUP (sql standard ordered set aggregate functions)

2013-09-17 Thread Andrew Gierth
 Robert == Robert Haas robertmh...@gmail.com writes:

  Someone should do the same in WaitForBackgroundWorkerStartup so
  that building with -Werror works.

 Robert I don't get a warning there.  Can you be more specific about
 Robert the problem?

bgworker.c: In function 'WaitForBackgroundWorkerStartup':
bgworker.c:866: warning: 'pid' may be used uninitialized in this function

gcc 4.2.2 / freebsd 8.2

-- 
Andrew (irc:RhodiumToad)


-- 
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] relscan_details.h

2013-09-17 Thread Alvaro Herrera
Noah Misch wrote:
 On Mon, Sep 16, 2013 at 11:02:28PM -0300, Alvaro Herrera wrote:
  relscan.h is a bit of an unfortunate header because it requires a lot of
  other headers to compile, and is also required by execnodes.h.  This
 
 Not in my copy of the source tree.  execnodes.h uses the corresponding
 typedefs that appear in genam.h and heapam.h.  Indeed, find . -name '*.Po' |
 xargs grep -l relscan.h | wc -l only locates 26 files that include relscan.h
 directly or indirectly.

Ah, I see now that I misspoke.  This changeset has been dormant on my
repo for a while, so I confused the detail of what it is about.  The
real benefit of this change is to eliminate indirect inclusion of
genam.h and heapam.h.  The number of indirect inclusions of each of them
is:
HEADwith patch
heapam.h219 118
genam.h 226 121

 -1 on header restructuring in the absence of noteworthy compile-time benchmark
 improvements.  Besides the obvious benchmark of full-build time, one could
 exercise the benefit of fewer header dependencies by modelling the series of
 compile times from running git pull; make at each commit for the past
 several months.  The latter benchmark is perhaps more favorable.

I will have a go at measuring things this way.

-- 
Á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] patch: add MAP_HUGETLB to mmap() where supported (WIP)

2013-09-17 Thread Robert Haas
On Mon, Sep 16, 2013 at 9:13 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 Robert, do you remember why you put the pagesize = sysconf(_SC_PAGE_SIZE);
 call in the new mmap() shared memory allocator?

Hmm, no.  Unfortunately, I don't.  We could try ripping it out and see
if the buildfarm breaks. If it is needed, then the dynamic shared
memory patch I posted probably needs it as well.

-- 
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] relscan_details.h

2013-09-17 Thread Robert Haas
On Tue, Sep 17, 2013 at 2:30 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 -1 on header restructuring in the absence of noteworthy compile-time 
 benchmark
 improvements.  Besides the obvious benchmark of full-build time, one could
 exercise the benefit of fewer header dependencies by modelling the series of
 compile times from running git pull; make at each commit for the past
 several months.  The latter benchmark is perhaps more favorable.

 I will have a go at measuring things this way.

Personally, I'm not particularly in favor of these kinds of changes.
The changes we made last time broke a lot of extensions - including
some proprietary EDB ones that I had to go fix.  I think a lot of
people spent a lot of time fixing broken builds, at EDB and elsewhere,
as well as rebasing patches.  And the only benefit we have to balance
that out is that incremental recompiles are faster, and I'm not really
sure how important that actually is.  On my system, configure takes 25
seconds and make -j3 takes 65 seconds; so, even a full recompile is
pretty darn fast, and an incremental recompile is usually really fast.
 Now granted this is a relatively new system, but still.

I don't want to be too dogmatic in opposing this; I accept that we
should, from time to time, refactor things.  If we don't, superflouous
dependencies will probably proliferate over time.  But personally, I'd
rather do these changes in bulk every third release or so and keep
them to a minimum in between times, so that we're not forcing people
to constantly decorate their extensions with new #if directives.

-- 
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] plpgsql.print_strict_params

2013-09-17 Thread Marko Tiikkaja

Hi,

Attached is a patch with the following changes:

On 16/09/2013 10:57, I wrote:

On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote:

However the sample function provided in the documentation throws a
runtime error due to a missing FROM-clause entry.


Ugh.  I'll look into fixing that.


Fixed.


This lineis in exec_get_query_params() and exec_get_dynquery_params():

  return (no parameters);

Presumably the message will escape translation and this line should be better
written as:
 return _((no parameters));


Nice catch.  Will look into this.  Another option would be to simply
omit the DETAIL line if there were no parameters.  At this very moment
I'm thinking that might be a bit nicer.


Decided to remove the DETAIL line in these cases.


Also, if the offending query parameter contains a single quote, the output
is not escaped:

postgres=# select get_userid(E'foo''');
ERROR:  query returned more than one row
DETAIL:  p1 = 'foo''
CONTEXT:  PL/pgSQL function get_userid(text) line 9 at SQL statement

Not sure if that's a real problem though.


Hmm..  I should probably look at what happens when the parameters to a
prepared statement are currently logged and imitate that.


Yup, they're escaped.  Did just that.  Also copied the parameters:  
prefix on the DETAIL line from there.



The functions added in pl_exec.c - exec_get_query_params() and
exec_get_dynquery_params() do strike me as potentially misnamed,
as they don't actually execute anything but are more utility
functions for generating formatted output.

Maybe format_query_params() etc. would be better?


Agreed, they could use some renaming.


I went with format_expr_params and format_preparedparamsdata.


* Are the comments sufficient and accurate?
exec_get_query_params() and exec_get_dynquery_params()
could do with some brief comments explaining their purpose.


Agreed.


Still agreeing with both of us, added a comment each.

I also added the new keywords to the unreserved_keywords production, as 
per the instructions near the beginning of pl_gram.y.




Regards,
Marko Tiikkaja
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 1077,1082  END;
--- 1077,1106 
  /para
  
  para
+  If literalprint_strict_params/ is enabled for the function,
+  you will get information about the parameters passed to the
+  query in the literalDETAIL/ part of the error message produced
+  when the requirements of STRICT are not met.  You can change this
+  setting on a system-wide basis by setting
+  varnameplpgsql.print_strict_params/, though only subsequent
+  function compilations will be affected.  You can also enable it
+  on a per-function basis by using a compiler option:
+ programlisting
+ CREATE FUNCTION get_userid(username text) RETURNS int
+ AS $$
+ #print_strict_params on
+ DECLARE
+ userid int;
+ BEGIN
+ SELECT users.userid INTO STRICT userid
+ FROM users WHERE users.username = get_userid.username;
+ RETURN userid;
+ END
+ $$ LANGUAGE plpgsql;
+ /programlisting
+ /para
+ 
+ para
   For commandINSERT//commandUPDATE//commandDELETE/ with
   literalRETURNING/, applicationPL/pgSQL/application reports
   an error for more than one returned row, even when
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***
*** 351,356  do_compile(FunctionCallInfo fcinfo,
--- 351,357 
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+   function-print_strict_params = plpgsql_print_strict_params;
  
if (is_dml_trigger)
function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
***
*** 847,852  plpgsql_compile_inline(char *proc_source)
--- 848,854 
function-fn_cxt = func_cxt;
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
+   function-print_strict_params = plpgsql_print_strict_params;
  
plpgsql_ns_init();
plpgsql_ns_push(func_name);
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***
*** 221,226  static Portal exec_dynquery_with_params(PLpgSQL_execstate 
*estate,
--- 221,231 
  PLpgSQL_expr *dynquery, List 
*params,
  const char *portalname, int 
cursorOptions);
  
+ static char *format_expr_params(PLpgSQL_execstate *estate,
+   const 
PLpgSQL_expr *expr);
+ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
+  
const PreparedParamsData *ppd);
+ 
  
  /* --
   * plpgsql_exec_function  Called by the call handler for
***
*** 3391,3408 

Re: [HACKERS] [RFC] Extend namespace of valid guc names

2013-09-17 Thread Robert Haas
On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
 On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  a) allow repeatedly qualified names like a.b.c
 
  The attached patch does a)

 What is the use case for this change?

 Check 
 http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
 or the commit message of the patch.

That's more or less what I figured.  One thing I'm uncomfortable with
is that, while this is useful for extensions, what do we do when we
have a similar requirement for core?  The proposed GUC name is of the
format extension.user_defined_object_name.property_name; but omitting
the extension name would lead to
user_defined_object_name.property_name, which doesn't feel good as a
method for naming core GUCs.  The user defined object name might just
so happen to be an extension name.

More generally, it seems like this is trying to take structured data
and fit into the GUC mechanism, and I'm not sure that's going to be a
real good fit.  I mean, pg_hba.conf could be handled this way.  You
could have hba.1.type = local, hba.1.database = all, hba.1.user = all,
hba.2.type = host, etc.  But I doubt that any of us would consider
that an improvement over the actual approach of having a separate file
with that information.  If it's not a good fit for pg_hba.conf, why is
it a good fit for anything else we want to do?

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

2013-09-17 Thread Thom Brown
On 17 September 2013 22:03, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 Thom Brown wrote:

 Thanks for testing.

  Thanks for the patch, but I seem to have immediately hit a snag:
 
  pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
  PANIC:  invalid xlog record length 0

 Silly mistake I had already made in another patch.  Here's an
 incremental patch which fixes this bug.  Apply this on top of previous
 minmax-1.patch.


Thanks.

Hit another issue with exactly the same procedure:

pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid);
ERROR:  lock 176475 is not held

-- 
Thom


Re: [HACKERS] relscan_details.h

2013-09-17 Thread Alvaro Herrera
Robert Haas escribió:

 Personally, I'm not particularly in favor of these kinds of changes.
 The changes we made last time broke a lot of extensions - including
 some proprietary EDB ones that I had to go fix.  I think a lot of
 people spent a lot of time fixing broken builds, at EDB and elsewhere,
 as well as rebasing patches.  And the only benefit we have to balance
 that out is that incremental recompiles are faster, and I'm not really
 sure how important that actually is.  On my system, configure takes 25
 seconds and make -j3 takes 65 seconds; so, even a full recompile is
 pretty darn fast, and an incremental recompile is usually really fast.
  Now granted this is a relatively new system, but still.

Fortunately the machines I work on now are also reasonably fast.  There
was a time when my desktop was so slow that it paid off to tweak certain
file timestamps to avoid spurious recompiles.  Now I don't have to
worry.  But it still annoys me that I have enough time to context-switch
to, say, the email client or web browser, from where I don't switch back
so quickly; which means I waste five or ten minutes for a task that
should have taken 20 seconds.

Now, htup_details.h was a bit different than the case at hand because
there's evidently lots of code that want to deal with the guts of
tuples, but for scans you mainly want to start one, iterate and finish,
but don't care much about the innards.  So the cleanup work required is
going to be orders of magnitude smaller.

OTOH, back then we had the alternative of doing the split in such a way
that third-party code wouldn't have been affected that heavily, but we
failed to take that into consideration.  I trust we have all learned
that lesson and will keep it in mind for next time.

 I don't want to be too dogmatic in opposing this; I accept that we
 should, from time to time, refactor things.  If we don't, superflouous
 dependencies will probably proliferate over time.  But personally, I'd
 rather do these changes in bulk every third release or so and keep
 them to a minimum in between times, so that we're not forcing people
 to constantly decorate their extensions with new #if directives.

We can batch things, sure, but I don't think doing it only once every
three years is the right tradeoff.  There's not all that much of this
refactoring, after all.

-- 
Á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

2013-09-17 Thread Alvaro Herrera
Thom Brown wrote:

Thanks for testing.

 Thanks for the patch, but I seem to have immediately hit a snag:
 
 pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
 PANIC:  invalid xlog record length 0

Silly mistake I had already made in another patch.  Here's an
incremental patch which fixes this bug.  Apply this on top of previous
minmax-1.patch.

I also renumbered the duplicate OID pointed out by Peter, and fixed the
two compiler warnings reported by Jaime.

Note you'll need to re-initdb in order to get the right catalog entries.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/src/backend/access/minmax/mmrevmap.c b/src/backend/access/minmax/mmrevmap.c
index 3e19f90..76cddde 100644
--- a/src/backend/access/minmax/mmrevmap.c
+++ b/src/backend/access/minmax/mmrevmap.c
@@ -147,12 +147,10 @@ mmSetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk,
 	{
 		xl_minmax_rm_set	xlrec;
 		XLogRecPtr	recptr;
-		XLogRecData	rdata;
+		XLogRecData	rdata[2];
 		uint8		info;
 
 		info = XLOG_MINMAX_REVMAP_SET;
-		if (extend)
-			info |= XLOG_MINMAX_INIT_PAGE;
 
 		xlrec.node = rmAccess-idxrel-rd_node;
 		xlrec.mapBlock = mapBlk;
@@ -160,13 +158,26 @@ mmSetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk,
 		xlrec.heapBlock = heapBlk;
 		ItemPointerSet((xlrec.newval), blkno, offno);
 
-		rdata.data = (char *) xlrec;
-		rdata.len = SizeOfMinmaxRevmapSet;
-		rdata.buffer = rmAccess-currBuf;
-		rdata.buffer_std = false;
-		rdata.next = NULL;
+		rdata[0].data = (char *) xlrec;
+		rdata[0].len = SizeOfMinmaxRevmapSet;
+		rdata[0].buffer = InvalidBuffer;
+		rdata[0].buffer_std = false;
+		rdata[0].next = (rdata[1]);
+
+		rdata[1].data = NULL;
+		rdata[1].len = 0;
+		rdata[1].buffer = rmAccess-currBuf;
+		rdata[1].buffer_std = false;
+		rdata[1].next = NULL;
+
+		if (extend)
+		{
+			info |= XLOG_MINMAX_INIT_PAGE;
+			/* If the page is new, there's no need for a full page image */
+			rdata[0].next = NULL;
+		}
 
-		recptr = XLogInsert(RM_MINMAX_ID, info, rdata);
+		recptr = XLogInsert(RM_MINMAX_ID, info, rdata);
 
 		PageSetLSN(BufferGetPage(rmAccess-currBuf), recptr);
 	}
diff --git a/src/backend/access/minmax/mmxlog.c b/src/backend/access/minmax/mmxlog.c
index ee095a2..758fc5f 100644
--- a/src/backend/access/minmax/mmxlog.c
+++ b/src/backend/access/minmax/mmxlog.c
@@ -158,7 +158,6 @@ minmax_xlog_revmap_set(XLogRecPtr lsn, XLogRecord *record)
 {
 	xl_minmax_rm_set *xlrec = (xl_minmax_rm_set *) XLogRecGetData(record);
 	bool	init;
-	BlockNumber blkno;
 	Buffer	buffer;
 	Page	page;
 
diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index 1e7cbac..d1a3ca7 100644
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -1040,6 +1040,7 @@ PageIndexDeleteNoCompact(Page page, OffsetNumber *itemnos, int nitems)
 		 * page.
 		 */
 		memcpy(pageCopy, page, BLCKSZ);
+		lastused = FirstOffsetNumber;
 		upper = pd_special;
 		PageClearHasFreeLinePointers(page);
 		for (i = 0, itemidptr = itemidbase; i  nline; i++, itemidptr++)
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8109949..192e295 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -784,28 +784,28 @@ DATA(insert (	3474   3831 3831 18 s	3882 4000 0 ));
 /*
  * MinMax int4_ops
  */
-DATA(insert (	3177   23 23 1 s	97	403 0 ));
-DATA(insert (	3177   23 23 2 s	523 403 0 ));
-DATA(insert (	3177   23 23 3 s	96	403 0 ));
-DATA(insert (	3177   23 23 4 s	525 403 0 ));
-DATA(insert (	3177   23 23 5 s	521 403 0 ));
+DATA(insert (	3192   23 23 1 s	97	3847 0 ));
+DATA(insert (	3192   23 23 2 s	523 3847 0 ));
+DATA(insert (	3192   23 23 3 s	96	3847 0 ));
+DATA(insert (	3192   23 23 4 s	525 3847 0 ));
+DATA(insert (	3192   23 23 5 s	521 3847 0 ));
 
 /*
  * MinMax numeric_ops
  */
-DATA(insert (	3192   1700 1700 1 s 1754 403 0 ));
-DATA(insert (	3192   1700 1700 2 s 1755 403 0 ));
-DATA(insert (	3192   1700 1700 3 s 1752 403 0 ));
-DATA(insert (	3192   1700 1700 4 s 1757 403 0 ));
-DATA(insert (	3192   1700 1700 5 s 1756 403 0 ));
+DATA(insert (	3193   1700 1700 1 s 1754 3847 0 ));
+DATA(insert (	3193   1700 1700 2 s 1755 3847 0 ));
+DATA(insert (	3193   1700 1700 3 s 1752 3847 0 ));
+DATA(insert (	3193   1700 1700 4 s 1757 3847 0 ));
+DATA(insert (	3193   1700 1700 5 s 1756 3847 0 ));
 
 /*
  * MinMax text_ops
  */
-DATA(insert (	3193   25 25 1 s	664 403 0 ));
-DATA(insert (	3193   25 25 2 s	665 403 0 ));
-DATA(insert (	3193   25 25 3 s	98	403 0 ));
-DATA(insert (	3193   25 25 4 s	667 403 0 ));
-DATA(insert (	3193   25 25 5 s	666 403 0 ));
+DATA(insert (	3194   25 25 1 s	664 3847 0 ));
+DATA(insert (	3194   25 25 2 s	665 3847 0 ));
+DATA(insert (	3194   25 25 3 s	98	3847 0 ));
+DATA(insert (	3194   25 25 4 s	667 3847 0 ));
+DATA(insert (	3194   25 25 5 s	666 3847 0 ));
 
 #endif   /* PG_AMOP_H */
diff --git a/src/include/catalog/pg_amproc.h 

Re: [HACKERS] Minmax indexes

2013-09-17 Thread Erik Rijkers
On Tue, September 17, 2013 23:03, Alvaro Herrera wrote:

 [minmax-1.patch. + minmax-2-incr.patch. (and initdb)]


The patches apply and compile OK.

I've not yet really tested; I just wanted to mention that  make check  gives 
the following differences:



*** 
/home/aardvark/pg_stuff/pg_sandbox/pgsql.minmax/src/test/regress/expected/opr_sanity.out
2013-09-17 23:18:31.427356703
+0200
--- 
/home/aardvark/pg_stuff/pg_sandbox/pgsql.minmax/src/test/regress/results/opr_sanity.out
 2013-09-17 23:20:48.208150824
+0200
***
*** 1076,1081 
--- 1076,1086 
 2742 |2 | @@@
 2742 |3 | @
 2742 |4 | =
+3847 |1 | 
+3847 |2 | =
+3847 |3 | =
+3847 |4 | =
+3847 |5 | 
 4000 |1 | 
 4000 |1 | ~~
 4000 |2 | 
***
*** 1098,1104 
 4000 |   15 | 
 4000 |   16 | @
 4000 |   18 | =
! (62 rows)

  -- Check that all opclass search operators have selectivity estimators.
  -- This is not absolutely required, but it seems a reasonable thing
--- 1103,1109 
 4000 |   15 | 
 4000 |   16 | @
 4000 |   18 | =
! (67 rows)

  -- Check that all opclass search operators have selectivity estimators.
  -- This is not absolutely required, but it seems a reasonable thing
***
*** 1272,1280 
  WHERE am.amname  'btree' AND am.amname  'gist' AND am.amname  'gin'
  GROUP BY amname, amsupport, opcname, amprocfamily
  HAVING count(*) != amsupport OR amprocfamily IS NULL;
!  amname | opcname | count
! +-+---
! (0 rows)

  SELECT amname, opcname, count(*)
  FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid
--- 1277,1288 
  WHERE am.amname  'btree' AND am.amname  'gist' AND am.amname  'gin'
  GROUP BY amname, amsupport, opcname, amprocfamily
  HAVING count(*) != amsupport OR amprocfamily IS NULL;
!  amname |   opcname   | count
! +-+---
!  minmax | int4_ops| 1
!  minmax | text_ops| 1
!  minmax | numeric_ops | 1
! (3 rows)

  SELECT amname, opcname, count(*)
  FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid

==




Erik Rijkers




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


Re: [HACKERS] Support for REINDEX CONCURRENTLY

2013-09-17 Thread Robert Haas
On Mon, Sep 16, 2013 at 10:38 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-08-29 10:39:09 -0400, Robert Haas wrote:
 I have been of the opinion for some time now that the
 shared-invalidation code is not a particularly good design for much of
 what we need.  Waiting for an old snapshot is often a proxy for
 waiting long enough that we can be sure every other backend will
 process the shared-invalidation message before it next uses any of the
 cached data that will be invalidated by that message.  However, it
 would be better to be able to send invalidation messages in some way
 that causes them to processed more eagerly by other backends, and that
 provides some more specific feedback on whether or not they have
 actually been processed.  Then we could send the invalidation
 messages, wait just until everyone confirms that they have been seen,
 which should hopefully happen quickly, and then proceed.

 Actually, the shared inval code already has that knowledge, doesn't it?
 ISTM all we'd need is have a sequence number of SI entries which has
 to be queryable. Then one can simply wait till all backends have
 consumed up to that id which we keep track of the furthest back backend
 in shmem.

In theory, yes, but in practice, there are a few difficulties.

1. We're not in a huge hurry to ensure that sinval notifications are
delivered in a timely fashion.  We know that sinval resets are bad, so
if a backend is getting close to needing a sinval reset, we kick it in
an attempt to get it to AcceptInvalidationMessages().  But if the
sinval queue isn't filling up, there's no upper bound on the amount of
time that can pass before a particular sinval is read.  Therefore, the
amount of time that passes before an idle backend is forced to drain
the sinval queue can vary widely, from a fraction of a second to
minutes, hours, or days.   So it's kind of unappealing to think about
making user-visible behavior dependent on how long it ends up taking.

2. Every time we add a new kind of sinval message, we increase the
frequency of sinval resets, and those are bad.  So any notifications
that we choose to send this way had better be pretty low-volume.

Considering the foregoing points, it's unclear to me whether we should
try to improve sinval incrementally or replace it with something
completely new.  I'm sure that the above-mentioned problems are
solvable, but I'm not sure how hairy it will be.  On the other hand,
designing something new could be pretty hairy, too.

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

2013-09-17 Thread Alvaro Herrera
Thom Brown wrote:

 Hit another issue with exactly the same procedure:
 
 pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid);
 ERROR:  lock 176475 is not held

That's what I get for restructuring the way buffers are acquired to use
the FSM, and then neglecting to test creation on decently-sized indexes.
Fix attached.

I just realized that xlog replay is also broken.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/src/backend/access/minmax/minmax.c b/src/backend/access/minmax/minmax.c
index 3b41100..47cb05e 100644
--- a/src/backend/access/minmax/minmax.c
+++ b/src/backend/access/minmax/minmax.c
@@ -1510,10 +1510,8 @@ mm_getinsertbuffer(Relation irel, Buffer *buffer, Size itemsz)
 		}
 
 		if (!BufferIsInvalid(*buffer))
-		{
-			MarkBufferDirty(*buffer);
-			UnlockReleaseBuffer(*buffer);
-		}
+			ReleaseBuffer(*buffer);
+
 		*buffer = buf;
 	}
 	else

-- 
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] Assertions in PL/PgSQL

2013-09-17 Thread Marko Tiikkaja

On 2013-09-16 21:24, Pavel Stehule wrote:

2. a failed assert should to raise a exception, that should not be handled
by any exception handler - similar to ERRCODE_QUERY_CANCELED - see
exception_matches_conditions.


I'm not sure what I think about that idea.  I see decent arguments for 
it working either way.  Care to unravel yours a bit more?



Regards,
Marko Tiikkaja


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


Re: [HACKERS] Minmax indexes

2013-09-17 Thread Alvaro Herrera
Erik Rijkers wrote:
 On Tue, September 17, 2013 23:03, Alvaro Herrera wrote:
 
  [minmax-1.patch. + minmax-2-incr.patch. (and initdb)]
 
 
 The patches apply and compile OK.
 
 I've not yet really tested; I just wanted to mention that  make check  gives 
 the following differences:

Oops, I forgot to update the expected file.  I had to comment on this
when submitting minmax-2-incr.patch and forgot.  First, those extra five
operators are supposed to be there; expected file needs an update.  As
for this:

 --- 1277,1288 
   WHERE am.amname  'btree' AND am.amname  'gist' AND am.amname  'gin'
   GROUP BY amname, amsupport, opcname, amprocfamily
   HAVING count(*) != amsupport OR amprocfamily IS NULL;
 !  amname |   opcname   | count
 ! +-+---
 !  minmax | int4_ops| 1
 !  minmax | text_ops| 1
 !  minmax | numeric_ops | 1
 ! (3 rows)

I think the problem is that the query is wrong.  This is the complete query:

SELECT amname, opcname, count(*)
FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid
 LEFT JOIN pg_amproc p ON amprocfamily = opcfamily AND
 amproclefttype = amprocrighttype AND amproclefttype = opcintype
WHERE am.amname  'btree' AND am.amname  'gist' AND am.amname  'gin'
GROUP BY amname, amsupport, opcname, amprocfamily
HAVING count(*) != amsupport OR amprocfamily IS NULL;

I should be, instead, this:

SELECT amname, opcname, count(*)
FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid
 LEFT JOIN pg_amproc p ON amprocfamily = opcfamily AND
 amproclefttype = amprocrighttype AND amproclefttype = opcintype
WHERE am.amname  'btree' AND am.amname  'gist' AND am.amname  'gin'
GROUP BY amname, amsupport, opcname, amprocfamily
HAVING count(*) != amsupport AND (amprocfamily IS NOT NULL);

This query is supposed to check that there are no opclasses with
mismatching number of support procedures; but if the left join returns a
null-extended row for pg_amproc, that means there is no support proc,
yet count(*) will return 1.  So count(*) will not match amsupport, and
the row is supposed to be excluded by the amprocfamily IS NULL clause in
HAVING.

Both queries return empty in HEAD, but only the second one correctly
returns empty with the patch applied.

-- 
Á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] Updatable view columns

2013-09-17 Thread Marko Tiikkaja

On 2013-09-17 12:53, Dean Rasheed wrote:

Thanks for the review. Those changes all look sensible to me.

Here's an updated patch incorporating all your fixes, and rebased to
apply without offsets.


Looks good to me.  Marking this one ready for committer.


Regards,
Marko Tiikkaja


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


Re: [HACKERS] [PERFORM] encouraging index-only scans

2013-09-17 Thread Andres Freund
On 2013-09-17 11:37:35 -0500, Jim Nasby wrote:
 On 9/7/13 12:34 AM, Andres Freund wrote:
 What I was thinking of was to keep track of the oldest xids on pages
 that cannot be marked all visible. I haven't thought about the
 statistics part much, but what if we binned the space between
 [RecentGlobalXmin, -nextXid) into 10 bins and counted the number of
 pages falling into each bin. Then after the vacuum finished we could
 compute how far RecentGlobalXmin would have to progress to make another
 vacuum worthwile by counting the number of pages from the lowest bin
 upwards and use the bin's upper limit as the triggering xid.
 
 Now, we'd definitely need to amend that scheme by something that handles
 pages that are newly written to, but it seems something like that
 wouldn't be too hard to implement and would make autovacuum more useful.
 
 If we're binning by XID though you're still dependent on scanning to
 build that range. Anything that creates dead tuples will also be be
 problematic, because it's going to unset VM bits on you, and you won't
 know if it's due to INSERTS or dead tuples.

I don't think that's all that much of a problem. In the end, it's a good
idea to look at pages shortly after they have been filled/been
touched. Setting hint bits at that point avoid repetitive IO and in many
cases we will already be able to mark them all-visible.
The binning idea was really about sensibly estimating whether a new scan
already makes sense which is currently very hard to judge.

I generally think the current logic for triggering VACUUMs via
autovacuum doesn't really make all that much sense in the days where we
have the visibility map.

 What if we maintained XID stats for ranges of pages in a separate
 fork? Call it the XidStats fork. Presumably the interesting pieces
 would be min(xmin) and max(xmax) for pages that aren't all visible. If
 we did that at a granularity of, say, 1MB worth of pages[1] we're
 talking 8 bytes per MB, or 1 XidStats page per GB of heap. (Worst case
 alignment bumps that up to 2 XidStats pages per GB of heap.)

Yes, I have thought about similar ideas as well, but I came to the
conclusion that it's not worth it. If you want to make the boundaries
precise and the xidstats fork small, you're introducing new contention
points because every DML will need to make sure it's correct.
Also, the amount of code that would require seems to be bigger than
justified by the increase of precision when to vacuum.


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] Support for REINDEX CONCURRENTLY

2013-09-17 Thread Andres Freund
On 2013-09-17 16:34:37 -0400, Robert Haas wrote:
 On Mon, Sep 16, 2013 at 10:38 AM, Andres Freund and...@2ndquadrant.com 
 wrote:
  Actually, the shared inval code already has that knowledge, doesn't it?
  ISTM all we'd need is have a sequence number of SI entries which has
  to be queryable. Then one can simply wait till all backends have
  consumed up to that id which we keep track of the furthest back backend
  in shmem.
 
 In theory, yes, but in practice, there are a few difficulties.

Agreed ;)

 1. We're not in a huge hurry to ensure that sinval notifications are
 delivered in a timely fashion.  We know that sinval resets are bad, so
 if a backend is getting close to needing a sinval reset, we kick it in
 an attempt to get it to AcceptInvalidationMessages().  But if the
 sinval queue isn't filling up, there's no upper bound on the amount of
 time that can pass before a particular sinval is read.  Therefore, the
 amount of time that passes before an idle backend is forced to drain
 the sinval queue can vary widely, from a fraction of a second to
 minutes, hours, or days.   So it's kind of unappealing to think about
 making user-visible behavior dependent on how long it ends up taking.

Well, when we're signalling it's certainly faster than waiting for the
other's snapshot to vanish which can take ages for normal backends. And
we can signal when we wait for consumption without too many
problems.
Also, I think in most of the usecases we can simply not wait for any of
the idle backends, those don't use the old definition anyway.

 2. Every time we add a new kind of sinval message, we increase the
 frequency of sinval resets, and those are bad.  So any notifications
 that we choose to send this way had better be pretty low-volume.

In pretty much all the cases where I can see the need for something like
that, we already send sinval messages, so we should be able to
piggbyback on those.

 Considering the foregoing points, it's unclear to me whether we should
 try to improve sinval incrementally or replace it with something
 completely new.  I'm sure that the above-mentioned problems are
 solvable, but I'm not sure how hairy it will be.  On the other hand,
 designing something new could be pretty hairy, too.

I am pretty sure there's quite a bit to improve around sinvals but I
think any replacement would look surprisingly similar to what we
have. So I think doing it incrementally is more realistic.
And I am certainly scared by the thought of having to replace it without
breaking corner cases all over.

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] [RFC] Extend namespace of valid guc names

2013-09-17 Thread Andres Freund
Hi,

On 2013-09-17 16:24:34 -0400, Robert Haas wrote:
 On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.com 
 wrote:
  On 2013-09-17 10:23:30 -0400, Robert Haas wrote:
  What is the use case for this change?
 
  Check 
  http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de
  or the commit message of the patch.
 
 That's more or less what I figured.  One thing I'm uncomfortable with
 is that, while this is useful for extensions, what do we do when we
 have a similar requirement for core?  The proposed GUC name is of the
 format extension.user_defined_object_name.property_name; but omitting
 the extension name would lead to
 user_defined_object_name.property_name, which doesn't feel good as a
 method for naming core GUCs.  The user defined object name might just
 so happen to be an extension name.

I think that ship has long since sailed. postgresql.conf has allowed
foo.bar style GUCs via custom_variable_classes for a long time, and
these days we don't even require that but allow them generally. Also,
SET, postgres -c, and SELECT set_config() already don't have the
restriction to one dot in the variable name.

I think if we're going to use something like that for postgres, we'll
have to live with the chance of breaking applications (although I don't
think it's that big for most of the variables where I can forsee the use).

 If it's not a good fit for pg_hba.conf, why is it a good fit for
 anything else we want to do?

Well, for now it's primarily there for extensions, not for core
code. Although somebody has already asked when I'd submit the patch
because they could use it for their patch...
I don't really find the pg_hba.conf comparison terribly convincing. As
an extension, how would you prefer to formulate the configuration
in the example? 

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] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2013-09-17 Thread Andres Freund
On 2013-09-17 12:29:51 -0400, Robert Haas wrote:
 But I'm skeptical that you're going to be able to accomplish that,
 especially without adversely affecting maintainability.  I think the
 way that you're proposing to use lwlocks here is sufficiently
 different from what the rest of the system does that it's going to be
 hard to avoid system-wide affects that can't easily be caught during
 code review;

I actually think extending lwlocks to allow downgrading an exclusive
lock is a good idea, independent of this path, and I think there are
some areas of the code where we could use that capability to increase
scalability. Now, that might be because I pretty much suggested using
them in such a way to solve some of the problems :P

I don't think they solve the issue of this patch (holding several nbtree
pages locked across heap operations) though.

 I agree that unpredictable deadlocks are bad.  I think the fundamental
 problem with UPSERT, MERGE, and this proposal is what happens when the
 conflicting tuple is present but not visible to your scan, either
 because it hasn't committed yet or because it has committed but is not
 visible to your snapshot.  I'm not clear on how you handle that in
 your approach.

Hm. I think it should be handled exactly the way we handle it for unique
indexes today. Wait till it's clear whether you can proceed.
At some point we might to extend that logic to more cases, but that
should be separate discussion imo.

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] Patch for fail-back without fresh backup

2013-09-17 Thread Sawada Masahiko
On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Tue, Sep 17, 2013 at 3:45 PM, Samrat Revagade
 revagade.sam...@gmail.com wrote:

  syncrep.c: In function ‘SyncRepReleaseWaiters’:
  syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used
  [-Wunused-but-set-variable]
 

 Sorry I forgot fix it.

 I have attached the patch which I modified.


 Attached patch combines documentation patch and source-code patch.

 I set up synchronous replication with synchronous_transfer = all, and then I 
 ran
 pgbench -i and executed CHECKPOINT in the master. After that, when I executed
 CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by
 synchronous_transfer feature.

Did you set synchronous_standby_names in the standby server?
If so, the master server waits for the standby server which is set to
synchronous_standby_names.
Please let me know detail of this case.


 How does synchronous_transfer work with cascade replication? If it's set to 
 all
 in the sender-side standby, it can resolve the data page inconsistency 
 between
 two standbys?


Currently patch supports the case which two servers are set up SYNC replication.
IWO, failback safe standby is the same as SYNC replication standby.
User can set synchronous_transfer in only master side.

Regards,

---
Sawada Masahiko


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


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-17 Thread Fujii Masao
On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
 On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I set up synchronous replication with synchronous_transfer = all, and then I 
 ran
 pgbench -i and executed CHECKPOINT in the master. After that, when I executed
 CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by
 synchronous_transfer feature.

 Did you set synchronous_standby_names in the standby server?

Yes.

 If so, the master server waits for the standby server which is set to
 synchronous_standby_names.
 Please let me know detail of this case.

Both master and standby have the same postgresql.conf settings as follows:

max_wal_senders = 4
wal_level = hot_standby
wal_keep_segments = 32
synchronous_standby_names = '*'
synchronous_transfer = all

 How does synchronous_transfer work with cascade replication? If it's set to 
 all
 in the sender-side standby, it can resolve the data page inconsistency 
 between
 two standbys?


 Currently patch supports the case which two servers are set up SYNC 
 replication.
 IWO, failback safe standby is the same as SYNC replication standby.
 User can set synchronous_transfer in only master side.

So, it's very strange that CHECKPOINT on the standby gets stuck infinitely.

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] Minor inheritance/check bug: Inconsistent behavior

2013-09-17 Thread Amit Kapila
On Tue, Sep 17, 2013 at 3:29 PM, Rushabh Lathia
rushabh.lat...@gmail.com wrote:
 Hi Amit.

 I gone through the mail thread discussion regarding this issue and reviewed
 you patch.

 -- Patch get applied cleanly on Master branch
 -- Make and Make Install fine
 -- make check also running cleanly

 In the patch code changes looks good to me.

 This patch having two part:

 1) Allowed TableOid system column in the CHECK constraint
 2) Throw an error if other then TableOid system column in CHECK constraint.

 I noticed that you added test coverage for 1) but the test coverage for 2)
 is missing..
   Initially I thought of keeping the test for point-2 as well, but
later left it thinking it might not add much value for adding negative
test for this
   scenario.
 I added the test coverage for 2) in the attached patch.
   Thanks for adding new test.

 Marking this as Ready for committer.

Thanks a ton for reviewing the patch.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


 On Sun, Sep 15, 2013 at 2:31 PM, Amit Kapila amit.kapil...@gmail.com
 wrote:

 Bruce Momjian wrote:
 On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote:
   I have done the initial analysis and prepared a patch, don't know if
   anything more I can do until
   someone can give any suggestions to further proceed on this bug.
 
   So, I guess we never figured this out.
 
  I can submit this bug-fix for next commitfest if there is no objection
  for doing so.
  What is your opinion?

  Yes, good idea.

 I had rebased the patch against head and added the test case to validate
 it.
 I will upload this patch to commit fest.


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


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-17 Thread Sawada Masahiko
On Wed, Sep 18, 2013 at 11:45 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com 
 wrote:
 On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I set up synchronous replication with synchronous_transfer = all, and then 
 I ran
 pgbench -i and executed CHECKPOINT in the master. After that, when I 
 executed
 CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased 
 by
 synchronous_transfer feature.

 Did you set synchronous_standby_names in the standby server?

 Yes.

 If so, the master server waits for the standby server which is set to
 synchronous_standby_names.
 Please let me know detail of this case.

 Both master and standby have the same postgresql.conf settings as follows:

 max_wal_senders = 4
 wal_level = hot_standby
 wal_keep_segments = 32
 synchronous_standby_names = '*'
 synchronous_transfer = all

 How does synchronous_transfer work with cascade replication? If it's set to 
 all
 in the sender-side standby, it can resolve the data page inconsistency 
 between
 two standbys?


 Currently patch supports the case which two servers are set up SYNC 
 replication.
 IWO, failback safe standby is the same as SYNC replication standby.
 User can set synchronous_transfer in only master side.

 So, it's very strange that CHECKPOINT on the standby gets stuck infinitely.


yes I think so.
I was not considering that user set synchronous_standby_names in the
standby server.
it will ocurr
I will fix it considering this case.



Regards,

---
Sawada Masahiko


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


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-17 Thread Sawada Masahiko
On Wed, Sep 18, 2013 at 1:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote:
 On Wed, Sep 18, 2013 at 11:45 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com 
 wrote:
 On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I set up synchronous replication with synchronous_transfer = all, and then 
 I ran
 pgbench -i and executed CHECKPOINT in the master. After that, when I 
 executed
 CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased 
 by
 synchronous_transfer feature.

 Did you set synchronous_standby_names in the standby server?

 Yes.

 If so, the master server waits for the standby server which is set to
 synchronous_standby_names.
 Please let me know detail of this case.

 Both master and standby have the same postgresql.conf settings as follows:

 max_wal_senders = 4
 wal_level = hot_standby
 wal_keep_segments = 32
 synchronous_standby_names = '*'
 synchronous_transfer = all

 How does synchronous_transfer work with cascade replication? If it's set 
 to all
 in the sender-side standby, it can resolve the data page inconsistency 
 between
 two standbys?


 Currently patch supports the case which two servers are set up SYNC 
 replication.
 IWO, failback safe standby is the same as SYNC replication standby.
 User can set synchronous_transfer in only master side.

 So, it's very strange that CHECKPOINT on the standby gets stuck infinitely.


Sorry I sent mail by mistake.

yes I think so.

It waits for corresponding WAL replicated.
Behaviour of synchronous_transfer is similar to
synchronous_standby_names and synchronous replication little.
That is, if  those parameter is set but the standby server doesn't
connect to the master server,
the master server waits for corresponding WAL replicated to standby
server infinitely.

I was not considering that user set synchronous_standby_names in the
standby server.
I will fix it considering this case.

Regards,

---
Sawada Masahiko


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


[HACKERS] Patch for typo in src/bin/psql/command.c

2013-09-17 Thread Ian Lawrence Barwick
Attached.

Regards

Ian Barwick


psql-command-c-typo.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] pg_stat_statements: calls under-estimation propagation

2013-09-17 Thread Sameer Thakur
 You seem to have forgotten to include the pg_stat_statements--1.2.sql
 and pg_stat_statements--1.1--1.2.sql in the patch.
 Sorry again. Please find updated patch attached.

I did not add pg_stat_statements--1.2.sql. I have added that now and
updated the patch again.

The patch attached should contain following file changes
patching file contrib/pg_stat_statements/Makefile
patching file contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql
patching file contrib/pg_stat_statements/pg_stat_statements--1.2.sql
patching file contrib/pg_stat_statements/pg_stat_statements.c
patching file contrib/pg_stat_statements/pg_stat_statements.control
patching file doc/src/sgml/pgstatstatements.sgml

regards
Sameer


pg_stat_statements-identification-v4.patch.gz
Description: GNU Zip compressed data

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


Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

2013-09-17 Thread Peter Geoghegan
On Tue, Sep 17, 2013 at 6:20 PM, Andres Freund and...@2ndquadrant.com wrote:
 I agree that unpredictable deadlocks are bad.  I think the fundamental
 problem with UPSERT, MERGE, and this proposal is what happens when the
 conflicting tuple is present but not visible to your scan, either
 because it hasn't committed yet or because it has committed but is not
 visible to your snapshot.  I'm not clear on how you handle that in
 your approach.

 Hm. I think it should be handled exactly the way we handle it for unique
 indexes today. Wait till it's clear whether you can proceed.

That's what I do, although getting those details right has been of
secondary concern for obvious reasons.

 At some point we might to extend that logic to more cases, but that
 should be separate discussion imo.

This is essentially why I went and added a row locking component over
your objections. Value locks (regardless of implementation)
effectively stop an insertion from finishing, but not from starting.
ISTM that locking the row with value locks held can cause deadlock.
So, unfortunately, we cannot really discuss value locking and row
locking separately, even though I see the appeal of trying to. Gaining
an actual representative notion of the expense of releasing and
re-acquiring the locks is too tightly coupled with how this is handled
and how frequently we need to restart. Plus there may well be other
issues in the same vein that we've yet to consider.

-- 
Peter Geoghegan


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