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

2013-12-21 Thread Peter Geoghegan
On Fri, Dec 20, 2013 at 1:12 PM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 There are probably other ways to make that general idea work though.  I
 didn't follow this thread carefully, but is the idea that there would be
 many promise tuples live at any one time, or only one?  Because if
 there's only one, or a very limited number, it might be workable to
 sleep on that tuple's lock instead of the xact's lock.

 Only one.

 heap_update() and heap_delete() also grab a heavy-weight lock on the tuple,
 before calling XactLockTableWait(). _bt_doinsert() does not, but it could.
 Perhaps we can take advantage of that.

I am skeptical of this approach. It sounds like you're saying that
you'd like to intersperse value and row locking, such that you'd
definitely get a row lock on your first attempt after detecting a
duplicate. With respect, I dismissed this months ago. Why should it be
okay to leave earlier, actually inserted index tuples (from earlier
unique indexes) behind? You still have to delete those (that is, the
heap tuple) on conflict, and what you outline is sufficiently
hand-wavey for me to strongly doubt the feasibility of making earlier
btree tuples not behave as pseudo value locks ***in all relevant
contexts***. How exactly do you determine that row versions were
*deleted*? How do you sensibly differentiate between updates and
deletes, or do you? What of lock starvation hazards? Perhaps I've
misunderstood, but detecting and reasoning about deletedness like this
seems like a major modularity violation, even by the standards of the
btree AM. Do XactLockTableWait() callers have to re-check
tuple-deletedness both before and after their XactLockTableWait()
call? For regular non-upserting inserters too?

I think that the way forward is to refine my design in order to
upgrade locks from exclusive buffer locks to something else, managed
by the lock manager but perhaps through an additional layer of
indirection. As previously outlined, I'm thinking of a new SLRU-based
granular value locking infrastructure built for this purpose, with
btree inserters marking pages as having an entry in this table. That
doesn't sound like much fun to go and implement, but it's reasonably
well precedented, if authoritative transaction processing papers are
anything to go by, as previously noted [1].

I hate to make a plausibility argument, particularly at this late
stage, but: no one, myself included, has managed to find any holes in
the semantics implied by my implementation in the last few months. It
is relatively easy to reason about, and doesn't leave the idea of an
amcanunique abstraction in tatters, nor does it expand the already
byzantine tuple locking infrastructure in a whole new direction. These
are strong advantages. It really isn't hard to imagine a totally sound
implementation of the same idea -- what I do with buffer locks, but
without actual buffer locks and their obvious attendant disadvantages,
and without appreciably regressing the performance of non-upsert
use-cases. AFAICT, there is way less uncertainty around doing this,
unless you think that unprincipled deadlocking is morally defensible,
which I don't believe you or anyone else does.

[1] 
http://www.postgresql.org/message-id/cam3swzq9xmm8bzynx3memy1amqckqxuusy8t1ifqzz999u_...@mail.gmail.com
-- 
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


Re: [HACKERS] make_interval ??

2013-12-21 Thread Pavel Stehule
Hello

here is patch

postgres=# select make_interval(years := 1, months := 6);
 make_interval
---
 1 year 6 mons
(1 row)

postgres=# select make_interval(weeks := 3);
 make_interval
---
 21 days
(1 row)

postgres=# select make_interval(days := 10);
 make_interval
---
 10 days
(1 row)

postgres=# select make_interval(hours := 2, mins := 10, secs := 25.33);
 make_interval
---
 02:10:25.33
(1 row)

Regards

Pavel


2013/12/21 Josh Berkus j...@agliodbs.com

 On 12/20/2013 04:44 PM, Gavin Flower wrote:
  On 21/12/13 13:40, Josh Berkus wrote:
  On 12/20/2013 03:09 PM, Gavin Flower wrote:
  What about leap years?
  What about them?
 
  some years have 365 days others have 366, so how any days in an interval
  of 2 years?, 4 years?

 Your question isn't relevant to this patch.  It's not defining the
 interval type, just creating an alternate constructor for it.

 (the answer is, it depends on what timestamp you're adding it to ...)

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


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

commit a49f69a9b1fb3337a8bf5cf3f198b6505c8f4115
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Thu Dec 12 18:08:47 2013 +0100

initial implementation make_timestamp

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a411e3a..cf7fcfe 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -6714,6 +6714,32 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
row
 entry
  indexterm
+  primarymake_interval/primary
+ /indexterm
+ literal
+  function
+   make_interval(parameteryears/parameter typeint/type DEFAULT 0,
+   parametermonths/parameter typeint/type DEFAULT 0,
+   parameterweeks/parameter typeint/type DEFAULT 0,
+   parameterdays/parameter typeint/type DEFAULT 0,
+   parameterhours/parameter typeint/type DEFAULT 0,
+   parametermins/parameter typeint/type DEFAULT 0,
+   parametersecs/parameter typedouble precision/type DEFAULT 0.0)
+  /function
+ /literal
+/entry
+entrytypeinterval/type/entry
+entry
+ Create interval from years, months, weeks, days, hours, minutes and
+ seconds fields
+/entry
+entryliteralmake_interval(days := 10)/literal/entry
+entryliteral10 days/literal/entry
+   /row
+
+   row
+entry
+ indexterm
   primarymake_time/primary
  /indexterm
  literal
@@ -6735,6 +6761,78 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
row
 entry
  indexterm
+  primarymake_timetz/primary
+ /indexterm
+ literal
+  function
+   make_timetz(parameterhour/parameter typeint/type,
+   parametermin/parameter typeint/type,
+   parametersec/parameter typedouble precision/type,
+   optional parametertimezone/parameter typetext/type /optional)
+  /function
+ /literal
+/entry
+entrytypetime with time zone/type/entry
+entry
+ Create time with time zone from hour, minute and seconds fields
+/entry
+entryliteralmake_timetz(8, 15, 23.5)/literal/entry
+entryliteral08:15:23.5+01/literal/entry
+   /row
+
+   row
+entry
+ indexterm
+  primarymake_timestamp/primary
+ /indexterm
+ literal
+  function
+   make_timestamp(parameteryear/parameter typeint/type,
+   parametermonth/parameter typeint/type,
+   parameterday/parameter typeint/type,
+   parameterhour/parameter typeint/type,
+   parametermin/parameter typeint/type,
+   parametersec/parameter typedouble precision/type)
+  /function
+ /literal
+/entry
+entrytypetimestamp/type/entry
+entry
+ Create timestamp from year, month, day, hour, minute and seconds fields
+/entry
+entryliteralmake_timestamp(1-23, 7, 15, 8, 15, 23.5)/literal/entry
+entryliteral2013-07-15 08:15:23.5/literal/entry
+   /row
+
+   row
+entry
+ indexterm
+  primarymake_timestamptz/primary
+ /indexterm
+ literal
+  function
+   make_timestamptz(parameteryear/parameter typeint/type,
+   parametermonth/parameter typeint/type,
+   parameterday/parameter typeint/type,
+   parameterhour/parameter typeint/type,
+   parametermin/parameter typeint/type,
+   parametersec/parameter typedouble precision/type,
+   optional parametertimezone/parameter typetext/type /optional)
+  /function
+ /literal
+/entry
+entrytypetimestamp with time zone/type/entry
+entry
+ 

[HACKERS] ISN extension bug?

2013-12-21 Thread Fabien COELHO


Hello devs,

ISTM that there is an issue on the ISMN type:

 sh psql
 psql (9.3.2)
 Type help for help.
 # CREATE EXTENTION isn;
 # SELECT ISMN 'M123456782';
  M-1234-5678-5

*** The 2 is changed to 5 in the display...

 # SELECT ISMN 'M123456785';
 ERROR:  invalid check digit for ISMN number: M123456785, should be 2
 LINE 1: SELECT ISMN 'M123456785';
^
 # SELECT ISMN 'M-1234-5678-5';
 ERROR:  invalid check digit for ISMN number: M-1234-5678-5, should be 2
 LINE 1: SELECT ISMN 'M-1234-5678-5';
^

--
Fabien.


--
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] shared memory message queues

2013-12-21 Thread Andres Freund
On 2013-12-20 22:04:05 +0100, Andres Freund wrote:
 Hi,
 
 On 2013-12-18 15:23:23 -0500, Robert Haas wrote:
  It sounds like most people who have looked at this stuff are broadly
  happy with it, so I'd like to push on toward commit soon, but it'd be
  helpful, Andres, if you could review the comment additions to
  shm-mq-v2.patch and see whether those address your concerns.  If so,
  I'll see about improving the overall comments for shm-toc-v1.patch as
  well to clarify the points that seem to have caused a bit of
  confusion; specific thoughts on what ought to be covered there, or any
  other review, is most welcome.
 
 Some things:

One more thing:
static uint64
shm_mq_get_bytes_written(volatile shm_mq *mq, bool *detached)
{
uint64  v;

SpinLockAcquire(mq-mq_mutex);
v = mq-mq_bytes_written;
*detached = mq-mq_detached;
SpinLockRelease(mq-mq_mutex);

return mq-mq_bytes_written;
}

Note how you're returning mq-mq_bytes_written instead of v.

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] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread David Rowley
On Tue, Dec 17, 2013 at 10:51 PM, David Rowley dgrowle...@gmail.com wrote:

 On Mon, Dec 16, 2013 at 9:36 PM, Hannu Krosing ha...@2ndquadrant.comwrote:

  On 12/16/2013 08:39 AM, David Rowley wrote:


  Any other ideas or +1's for any of the existing ones?

 +1, inverse good :)


 In the attached patch I've renamed negative to inverse. I've also disabled
 the inverse functions when an expression in an aggregate contains a
 volatile function.


I've attached an updated patch in which I've done some more work to the
documents and also added some more sanity checking around CREATE AGGREGATE.
The previous patch allowed CREATE AGGREGATE to accept a transition function
which was strict and an inverse transition function which was not, or vise
versa.

I've also added a bunch of regression tests around create aggregate which
were missing in the previous patch.

Regards

David Rowley


inverse_transition_functions_v1.1.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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread Erik Rijkers
On Sat, December 21, 2013 10:53, David Rowley wrote:

  [inverse_transition_functions_v1.1.patch.gz ]

Hi,

I know, $subject says WIP, but I assumed it's supposed to compile, so I tried 
to get this to run on linux (Centos 5.0).
gcc 4.8.2,
with

./configure  --prefix=/home/aardvark/pg_stuff/pg_installations/pgsql.inverse 
--with-pgport=6554  --enable-depend
--enable-cassert --enable-debug --with-openssl --with-perl --with-libxml 
--with-libxslt --with-ossp-uuid

but although applying the patch and configure were OK, the compile went awry:

[...]
make[4]: Entering directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/backend'
make[4]: Nothing to be done for `submake-errcodes'.
make[4]: Leaving directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/backend'
make[3]: Leaving directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/common'
make -C catalog schemapg.h
make[3]: Entering directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/backend/catalog'
cd ../../../src/include/catalog  '/home/aardvark/perl-5.19/bin/perl' 
./duplicate_oids
1220
1221
1800
1801
1970
1971
make[3]: Leaving directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/backend/catalog'
make[2]: Leaving directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src/backend'
make[1]: Leaving directory 
`/home/aardvark/pg_stuff/pg_sandbox/pgsql.inverse/src'
-- [2013.12.21 11:35:29 inverse] make contrib
dblink.c:55:28: fatal error: utils/fmgroids.h: No such file or directory
 #include utils/fmgroids.h
^
compilation terminated.
make[1]: *** [dblink.o] Error 1
make: *** [all-dblink-recurse] Error 2
-- make returned 2 - abort

( although that output mentions perl 5,19, I also tried with regular perl 5.18, 
cassert on/off, etc.; every combination
yielded this same compile error )


Thanks,

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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread David Rowley
On Sat, Dec 21, 2013 at 11:47 PM, Erik Rijkers e...@xs4all.nl wrote:

 On Sat, December 21, 2013 10:53, David Rowley wrote:

   [inverse_transition_functions_v1.1.patch.gz ]

 Hi,

 I know, $subject says WIP, but I assumed it's supposed to compile, so I
 tried to get this to run on linux (Centos 5.0).
 gcc 4.8.2,
 with


snip


 cd ../../../src/include/catalog  '/home/aardvark/perl-5.19/bin/perl'
 ./duplicate_oids
 1220
 1221
 1800
 1801
 1970
 1971


Thanks for the report. It seems the windows build does not check for
duplicate OIDs and the linux one does.
I'll just need to change the Oids of the new functions I've added, only it
seems the unused_oids perl script does not like windows.

I'll try to get you a working patch shortly.

Regards

David Rowley


Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread David Rowley
On Sun, Dec 22, 2013 at 12:12 AM, David Rowley dgrowle...@gmail.com wrote:

 On Sat, Dec 21, 2013 at 11:47 PM, Erik Rijkers e...@xs4all.nl wrote:

 On Sat, December 21, 2013 10:53, David Rowley wrote:

   [inverse_transition_functions_v1.1.patch.gz ]

 Hi,

 I know, $subject says WIP, but I assumed it's supposed to compile, so I
 tried to get this to run on linux (Centos 5.0).
 gcc 4.8.2,
 with


 snip


 cd ../../../src/include/catalog  '/home/aardvark/perl-5.19/bin/perl'
 ./duplicate_oids
 1220
 1221
 1800
 1801
 1970
 1971


 Thanks for the report. It seems the windows build does not check for
 duplicate OIDs and the linux one does.
 I'll just need to change the Oids of the new functions I've added, only it
 seems the unused_oids perl script does not like windows.

 I'll try to get you a working patch shortly.


Please find attached an updated patch which should remove the duplicate OID
problem you saw.

Regards

David Rowley


 Regards

 David Rowley




inverse_transition_functions_v1.2.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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread Erik Rijkers
On Sat, December 21, 2013 12:38, David Rowley wrote:
 [ inverse_transition_functions_v1.2.patch.gz  ]
 Please find attached an updated patch which should remove the duplicate OID
 problem you saw.

That fixes it, thanks

There is 1 of 141 failed tests:

window   ... FAILED

but that's easily ignored for now.

I'll do some testing later,

Thanks

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] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread David Rowley
On Sun, Dec 22, 2013 at 12:49 AM, Erik Rijkers e...@xs4all.nl wrote:

 On Sat, December 21, 2013 12:38, David Rowley wrote:
  [ inverse_transition_functions_v1.2.patch.gz  ]
  Please find attached an updated patch which should remove the duplicate
 OID
  problem you saw.

 That fixes it, thanks

 There is 1 of 141 failed tests:

 window   ... FAILED


That's strange, it passes here.

Would you be able to send me the regression diff file?

Regards

David Rowley


 but that's easily ignored for now.

 I'll do some testing later,

 Thanks

 Erik Rijkers









Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread Erik Rijkers
On Sat, December 21, 2013 12:52, David Rowley wrote:
 On Sun, Dec 22, 2013 at 12:49 AM, Erik Rijkers e...@xs4all.nl wrote:

 On Sat, December 21, 2013 12:38, David Rowley wrote:
  [ inverse_transition_functions_v1.2.patch.gz  ]
  Please find attached an updated patch which should remove the duplicate
 OID
  problem you saw.

 That fixes it, thanks

 There is 1 of 141 failed tests:

 window   ... FAILED


 That's strange, it passes here.

 Would you be able to send me the regression diff file?


attached...


regression.diffs
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] Negative Transition Aggregate Functions (WIP)

2013-12-21 Thread David Rowley
On Sun, Dec 22, 2013 at 1:01 AM, Erik Rijkers e...@xs4all.nl wrote:

 On Sat, December 21, 2013 12:52, David Rowley wrote:
  On Sun, Dec 22, 2013 at 12:49 AM, Erik Rijkers e...@xs4all.nl wrote:
 
  On Sat, December 21, 2013 12:38, David Rowley wrote:
   [ inverse_transition_functions_v1.2.patch.gz  ]
   Please find attached an updated patch which should remove the
 duplicate
  OID
   problem you saw.
 
  That fixes it, thanks
 
  There is 1 of 141 failed tests:
 
  window   ... FAILED
 
 
  That's strange, it passes here.
 
  Would you be able to send me the regression diff file?
 

 attached...


Thanks

This was just down to some missing trailing white space on the expected
results. I've fixed these up and attached another patch.

Regards

David Rowley


inverse_transition_functions_v1.3.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] shared memory message queues

2013-12-21 Thread Peter Eisentraut
This patch didn't make it out of the 2013-11 commit fest.  You should
move it to the next commit fest (probably with an updated patch) before
January 15th.



-- 
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] Race condition in b-tree page deletion

2013-12-21 Thread Peter Eisentraut
This patch didn't make it out of the 2013-11 commit fest.  You should
move it to the next commit fest (probably with an updated patch)
before January 15th, if it is not resolved before then.



-- 
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] Wait free LW_SHARED acquisition - v0.2

2013-12-21 Thread Peter Eisentraut
This patch didn't make it out of the 2013-11 commit fest.  You should
move it to the next commit fest (probably with an updated patch)
before January 15th.



-- 
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] better atomics - v0.2

2013-12-21 Thread Peter Eisentraut
This patch didn't make it out of the 2013-11 commit fest.  You should
move it to the next commit fest (probably with an updated patch)
before January 15th.



-- 
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] ECPG FETCH readahead, was: Re: ECPG fixes

2013-12-21 Thread Peter Eisentraut
This patch didn't make it out of the 2013-11 commit fest.  You should
move it to the next commit fest (probably with an updated patch)
before January 15th.



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


Re: [HACKERS] [COMMITTERS] pgsql: Upgrade to Autoconf 2.69

2013-12-21 Thread Peter Eisentraut
On Fri, 2013-12-20 at 10:54 -0300, Alvaro Herrera wrote:
 I don't see how can the pg_upgrade check fail in this way but not the
 regular regression test.  This patch includes the following hunk to
 pg_config.h.in:
 
 +/* Enable large inode numbers on Mac OS X 10.5.  */
 +#ifndef _DARWIN_USE_64_BIT_INODE
 +# define _DARWIN_USE_64_BIT_INODE 1
 +#endif
 
 
 Evidently something is not going well in ReadRecord.  It should have
 reported the read failure, but didn't.  That seems a separate bug that
 needs fixed.

This is enabling large-file support on OS X, so that seems kind of
important.  It's not failing with newer versions of OS X, so that leaves
the following possibilities, I think:

- Large files never worked on 10.5.  That would be strange because
Autoconf explicitly refers to that version.

- New OS X versions have this on by default (could someone check?), so
it already worked before, and it's something specific to 10.5 that's
broken.

- It's something specific to PowerPC or endianness.

- That build farm member has a strange setup.

If anyone has any of these components, please help isolate the problem.

It's conceivable that inconsistent include file order would cause this
kind of problem, but we're pretty strict about including the right
things first.

It someone can reproduce the problem, it would also help dissecting the
pg_upgrade test script to see exactly where the failure is introduced.




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


[HACKERS] pg_upgrade: make the locale comparison more tolerant

2013-12-21 Thread Pavel Raiskup
Hello pg-hackers!

I tried to look at the problem resulting in changes mentioned here:
http://www.postgresql.org/message-id/20121002155857.ge30...@momjian.us

If the system locale is changed e.g. from en_US.utf8 to en_US.utf-8 before
upgrading the data stack for newer server, pg_upgrade fails.  It is quite
awkward to ask users to change locale.conf contents just for upgrade
purposes; is there other known workaround?

I attached patch aimed to remove this hurdle.  After its application, the
in-place upgrade finished successfully for me under mentioned conditions,
though I am not 100% sure about its correctness.

Thanks, Pavel
From e54180b146edba871a9a4389f7cada0df1674587 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup prais...@redhat.com
Date: Sat, 21 Dec 2013 01:27:01 +0100
Subject: [PATCH] pg_upgrade: more tolerating locale comparison

Locale strings specified like 'cs_CZ.utf8' and 'cs_CZ.UTF-8'
should be treat as equivalent.
This patch switches from case-insensitive comparison of the whole
locale strings to more tolerant comparison:  It tries separately
compare the right side of the locale string (after the dot
character; the encoding part).  That string is firstly decoded by
pg_valid_client_encoding() from  pg_wchar.h and then are compared
corresponding codes.  Remaining part of locale string is case
insensitive string comparison.  If the encoding detection is not
possible (e.g. no encoding part of locale string), keep the
complete case insensitive comparison.

This resolves yet another complication of in-place upgrading.

---
 contrib/pg_upgrade/check.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 411689a..697a16a 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -9,6 +9,7 @@
 
 #include postgres_fe.h
 
+#include mb/pg_wchar.h
 #include pg_upgrade.h
 
 
@@ -393,6 +394,36 @@ set_locale_and_encoding(ClusterInfo *cluster)
 	PQfinish(conn);
 }
 
+/*
+ * equivalent_locale()
+ *
+ * Best effort locale comparison.  Return false if we are not 100% sure the
+ * locale is equivalent.
+ */
+static bool
+equivalent_locale(const char *loca, const char *locb)
+{
+	int enca, encb;
+	const char *chara, *charb;
+	int lencmp;
+
+	if (!(chara = strrchr(loca, '.')) ||
+		!(charb = strrchr(locb, '.')))
+		/* locale string does not contain encoding part */
+		return (pg_strcasecmp(loca, locb) == 0);
+
+	chara++;
+	charb++;
+
+	enca = pg_valid_server_encoding(chara);
+	encb = pg_valid_server_encoding(charb);
+
+	if (enca  0 || encb  0 || enca != encb)
+		return (pg_strcasecmp(loca, locb) == 0);
+
+	lencmp = chara - loca;
+	return (pg_strncasecmp(loca, locb, lencmp) == 0);
+}
 
 /*
  * check_locale_and_encoding()
@@ -409,13 +440,13 @@ check_locale_and_encoding(ControlData *oldctrl,
 	 * They also often use inconsistent hyphenation, which we cannot fix, e.g.
 	 * UTF-8 vs. UTF8, so at least we display the mismatching values.
 	 */
-	if (pg_strcasecmp(oldctrl-lc_collate, newctrl-lc_collate) != 0)
+	if (!equivalent_locale(oldctrl-lc_collate, newctrl-lc_collate))
 		pg_fatal(lc_collate cluster values do not match:  old \%s\, new \%s\\n,
 			   oldctrl-lc_collate, newctrl-lc_collate);
-	if (pg_strcasecmp(oldctrl-lc_ctype, newctrl-lc_ctype) != 0)
+	if (!equivalent_locale(oldctrl-lc_ctype, newctrl-lc_ctype))
 		pg_fatal(lc_ctype cluster values do not match:  old \%s\, new \%s\\n,
 			   oldctrl-lc_ctype, newctrl-lc_ctype);
-	if (pg_strcasecmp(oldctrl-encoding, newctrl-encoding) != 0)
+	if (!equivalent_locale(oldctrl-encoding, newctrl-encoding))
 		pg_fatal(encoding cluster values do not match:  old \%s\, new \%s\\n,
 			   oldctrl-encoding, newctrl-encoding);
 }
-- 
1.8.4.2


-- 
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] SQL objects UNITs

2013-12-21 Thread Stephen Frost
* Andrew Dunstan (and...@dunslane.net) wrote:
 That having been said, having a flag we can set to
 dump the extension contents normally rather than just dumping a CREATE
 EXTENSION statement seems completely reasonable to me.
 
 ALTER EXTENSION foo SET (dump_members = true/false);
 
 It's even got use cases outside of what Dimitri wants to do, like
 dumping and restoring an extension that you've manually modified
 without losing your changes.
 
 Yeah, seems like it might have merit.

I like the simplicity of this approach as well, but I believe Tom had
concerns about having some extensions behave quite different from
others (hence the earlier suggetsion to name the 'dumpable' ones
something different).  That said, I'm starting to wonder about a few
different options that might be handy- having the extension be dumpable
(or maybe an option to pg_dump to dump them from the DB, or not), and
perhaps an option to have the version # included in the dump (or an
option to exclude it, such as when run by pg_upgrade..?).  Perhaps
similar things for pg_restore.

In any case, this is certainly the way I had been hoping the discussion
would go..

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] SQL objects UNITs

2013-12-21 Thread Dimitri Fontaine
Stephen Frost sfr...@snowman.net writes:
   That said, I'm starting to wonder about a few
 different options that might be handy- having the extension be dumpable
 (or maybe an option to pg_dump to dump them from the DB, or not), and
 perhaps an option to have the version # included in the dump (or an
 option to exclude it, such as when run by pg_upgrade..?).  Perhaps
 similar things for pg_restore.

 In any case, this is certainly the way I had been hoping the discussion
 would go..

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

-- 
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] SQL objects UNITs

2013-12-21 Thread Stephen Frost
Dimitri,

* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote:
 Stephen Frost sfr...@snowman.net writes:
That said, I'm starting to wonder about a few
  different options that might be handy- having the extension be dumpable
  (or maybe an option to pg_dump to dump them from the DB, or not), and
  perhaps an option to have the version # included in the dump (or an
  option to exclude it, such as when run by pg_upgrade..?).  Perhaps
  similar things for pg_restore.
 
  In any case, this is certainly the way I had been hoping the discussion
  would go..
 
   http://www.postgresql.org/message-id/18778.1354753...@sss.pgh.pa.us

If you'd like to add to the discussion, then please do so.  This isn't
adding anything.  Tom brings up good points in that thread from last
year but my suggestion was about having a few options *in addition* to
keeping track of how extensions were installed and using that as the
default.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] WITHIN GROUP patch

2013-12-21 Thread Tom Lane
[ still hacking away at this patch ]

Andrew Gierth and...@tao11.riddles.org.uk writes:
 Tom == Tom Lane t...@sss.pgh.pa.us writes:
  Tom Not wanting to consider the sort args when there's more than one
  Tom doesn't square with forcing them to be considered when there's
  Tom just one.  It's the same aggregate after all,

 This logic is only applied in the patch to aggregates that _aren't_
 hypothetical.

 (thinking out loud:) It might be more consistent to also add the
 condition that the single sort column not be a variadic arg. And/or
 the condition that it be the same type as the result. Or have a flag
 in pg_aggregate to say this agg returns one of its sorted input
 values, so preserve the collation.

I eventually decided that we were overthinking this problem.  At least
for regular ordered-set aggregates, we can just deem that the collation
of the aggregate is indeterminate unless all the inputs (both direct and
aggregated) agree on the collation to use.  This gives us the right
answer for all the standard aggregates, which have at most one collatable
input, and it's very unclear that anything more complicated would be an
improvement.  I definitely didn't like the hack that was in there that'd
force a sort column to absorb a possibly-unrelated collation.

For hypotheticals, I agree after reading the spec text that we're supposed
to unify the collations of matching hypothetical and aggregated arguments
to determine the collation to use for sorting that column.  I see that
the patch just leaves these columns out of the determination of the
aggregate's result collation.  That's okay for the time being at least,
since we have no hypotheticals with collatable output types, but I wonder
if anyone wants to argue for some other rule (and if so, what)?

regards, tom lane


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


Re: [HACKERS] SQL objects UNITs

2013-12-21 Thread Robert Haas
On Sat, Dec 21, 2013 at 12:10 PM, Dimitri Fontaine
dimi...@2ndquadrant.fr wrote:
 Stephen Frost sfr...@snowman.net writes:
   That said, I'm starting to wonder about a few
 different options that might be handy- having the extension be dumpable
 (or maybe an option to pg_dump to dump them from the DB, or not), and
 perhaps an option to have the version # included in the dump (or an
 option to exclude it, such as when run by pg_upgrade..?).  Perhaps
 similar things for pg_restore.

 In any case, this is certainly the way I had been hoping the discussion
 would go..

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

Fortunately, nobody's proposing that exact design, and I think there
are more recent emails where Tom expressed at least some support for
the idea of installing an extension purely via SQL, and in fact backed
the idea of being able to dump-and-restore the extension members as
superior to storing blobs in the catalog.

If you want to go beat your head against the wall, I don't blame you,
but it's not going to help us make any progress here.

-- 
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] SQL objects UNITs

2013-12-21 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sat, Dec 21, 2013 at 12:10 PM, Dimitri Fontaine
 dimi...@2ndquadrant.fr wrote:
 Stephen Frost sfr...@snowman.net writes:
 That said, I'm starting to wonder about a few
 different options that might be handy- having the extension be dumpable
 (or maybe an option to pg_dump to dump them from the DB, or not), and
 perhaps an option to have the version # included in the dump (or an
 option to exclude it, such as when run by pg_upgrade..?).  Perhaps
 similar things for pg_restore.
 
 In any case, this is certainly the way I had been hoping the discussion
 would go..
 
 http://www.postgresql.org/message-id/18778.1354753...@sss.pgh.pa.us

 Fortunately, nobody's proposing that exact design, and I think there
 are more recent emails where Tom expressed at least some support for
 the idea of installing an extension purely via SQL, and in fact backed
 the idea of being able to dump-and-restore the extension members as
 superior to storing blobs in the catalog.

AFAICT, what I was complaining about there was the idea that the
per-extension behavior had to be specified via switches to pg_dump
in order to get a valid dump.  That doesn't seem too workable ---
you think your nightly backup script will know that?  But the idea that
it's an alterable property of each extension, *stored in the database*,
does not fall foul of that complaint.

regards, tom lane


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