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

2014-01-19 Thread David Rowley
On Sun, Jan 19, 2014 at 5:27 PM, David Rowley dgrowle...@gmail.com wrote:


  It's probably far more worth it for the bool and/or aggregates. We
 could just
  keep track of the values aggregated and the count of values as true
 and return
  true if those are the same in the case of AND, then check the true
 count
  is  0 in the case of OR. I'd feel more strongly to go and do that
 if I'd
  actually ever used those aggregates for anything.

 That, OTOH, would be worthwhile I think. I'll go do that, though probably
 not today. I hope to get to it sometime tomorrow.


 I've commited a patch to the github repo to do this.

 https://github.com/david-rowley/postgres/commit/121b0823753cedf33bb94f646df3176b77f28500
 but I'm not sure if we can keep it as I had to remove the sort op as I
 explained above.



I think I'm going to have to revert the patch which implements the inverse
transition function for bool_and and bool_or.
I tested on an instance of 9.3.2 and the following queries use index scans.

create table booltest (b boolean not null);
insert into booltest (b) select false from generate_series(1,2) g(n);
insert into booltest (b) values(true);

create index booltest_b_idx ON booltest(b);
vacuum analyze booltest;

explain select bool_or(b) from booltest;
explain select bool_and(b) from booltest;

I'm guessing there is no way to have an internal state type on the
aggregate and a sort operator on the aggregate.

I wonder if it is worth creating naive inverse transition functions similar
to max()'s and min()'s inverse transition functions. I guess on average
they've got about a 50% chance of being used and likely for some work loads
it would be a win.

What's your thoughts?

Regards
David Rowley


Re: [HACKERS] array_length(anyarray)

2014-01-19 Thread Dean Rasheed
On 18 January 2014 03:07, Marko Tiikkaja ma...@joh.to wrote:
 On 1/12/14, 5:53 AM, I wrote:

 On 1/9/14, 2:57 PM, Dean Rasheed wrote:

 How it should behave for multi-dimensional arrays is less clear, but
 I'd argue that it should return the total number of elements, i.e.
 cardinality('{{1,2},{3,4}}'::int[][]) = 4. That would make it
 consistent with the choices we've already made for unnest() and
 ordinality:
- cardinality(foo) = (select count(*) from unnest(foo)).
- unnest with ordinality would always result in ordinals in the range
 [1, cardinality].


 Ignoring my proposal, this seems like the most reasonable option.  I'll
 send an updated patch along these lines.


 Here's the patch as promised.  Thoughts?


A couple of points:

The answer for empty (zero dimensional) arrays is wrong --- you need
special case handling for this case to return 0. In fact why not
simply use ArrayGetNItems()?

In the docs, in the table of array functions, I think it would
probably be useful to make the entry for array_length say see also
cardinality, otherwise people might just stop reading there. I
suspect that in over 90% of cases, cardinality will be the more
appropriate function to use rather than array_length.

Regards,
Dean


-- 
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: Patch to add support of IF NOT EXISTS to others CREATE statements

2014-01-19 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes:
 * Robert Haas (robertmh...@gmail.com) wrote:
 I kind of don't see the point of having IF NOT EXISTS for things that
 have OR REPLACE, and am generally in favor of implementing OR REPLACE
 rather than IF NOT EXISTS where possible.  The point is usually to get
 the object to a known state, and OR REPLACE will generally accomplish
 that better than IF NOT EXISTS.  However, if the object has complex
 structure (like a table that contains data) then replacing it is a
 bad plan, so IF NOT EXISTS is really the best you can do - and it's
 still useful, even if it does require more care.

 This patch is in the most recent commitfest and marked as Ready for
 Committer, so I started reviewing it and came across the above.

 I find myself mostly agreeing with the above comments from Robert, but
 it doesn't seem like we've really done a comprehensive review of the
 various commands to make a 'command' decision on each as to if it should
 have IF NOT EXISTS or OR REPLACE options.

There's been pretty extensive theorizing about this in the past (try
searching the pghackers archives for CINE and COR), and I think the
rough consensus was that it's hard to do COR sensibly for objects
containing persistent state (ie tables) or with separately-declarable
substructure (again, mostly tables, though composite types have some of
the same issues).  However, if COR does make sense then CINE is an
inferior alternative, because of the issue about not knowing the resulting
state of the object for sure.

Given this list I would absolutely reject CINE for aggregates (why in the
world would we make them act differently from functions?), and likewise
for casts, collations, operators, and types.  I don't see any reason not
to prefer COR for these object kinds.  There is room for argument about
the text search stuff, though, because of the fact that some of the text
search object types have separately declarable substructure.

 The one difficulty that I do see with the 'OR REPLACE' option is when we
 can't simply replace an existing object due to dependencies on the
 existing definition of that object.  Still, if that's the case, wouldn't
 you want an error?

The main knock on COR is that there's no way for the system to completely
protect itself from the possibility that you replaced the object
definition with something that behaves incompatibly.  For instance, if we
had COR for collations and you redefined a collation, that might (or might
not) break indexes whose ordering depends on that collation.  However,
we already bought into that type of risk when we invented COR for
functions, and by and large there have been few complaints about it.
The ability to substitute an improved version of a function seems to be
worth the risks of substituting a broken version.

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] GiST support for inet datatypes

2014-01-19 Thread Emre Hasegeli
2014-01-19 Andreas Karlsson andr...@proxel.se:
 Hi,

 I will review your two patches (gist support + selectivity). This is part 1
 of my review. I will look more into the actual GiST implementation in a
 couple of days, but thought I could provide you with my initial input right
 away.

Thank you for looking at it.


 inet-gist
 -

 General:

 I like the idea of the patch and think the  operator is useful for
 exclusion constraints, and that indexing the contains operator is useful for
 IP-address lookups. There is an extension, ip4r, which adds a GiST indexed
 type for IP ranges but since we have the inet type in core I think it should
 have GiST indexes.

 I am not convinced an adjacent operator is useful for the inet type, but if
 it is included it should be indexed just like -|- of ranges. We should try
 to keep these lists of indexed operators the same.

I added it just not to leave negotor field empty. It can also be useful with
exclusion constraints but not with GiST support. I did not add GiST support
for it and the not equals operator because they do not fit the index
structure. I can just remove the operator for now.


 Compilation:

 Compiled without errors.

 Regression tests:

 One of the broken regression tests seems unrelated to the selectivity
 functions.

 -- Make a list of all the distinct operator names being used in particular
 -- strategy slots.

 I think it would be fine just to add the newly indexed operators here, but
 the more indexed operators we get in the core the less useful this test
 becomes.

I did not add the new operators to the list because I do not feel right
about supporting , =, , = symbols for these operators.
They should be @, @=, @, @= to be consistent with other types.


 I am a bit suspicious about your memcmp based optimization in bitncommon,
 but it could be good. Have you benchmarked it compared to doing the same
 thing with a loop?

I did, when I was writing that part. I will be happy to do it again. I will
post the results.


 Documentation:

 Please use examples in the operator table which will evaluate to true, and
 for the  case an example where not both sides are the same.

I will change in the next version.


 I have not found a place either in the documentation where it is documented
 which operators are documented. I would suggest just adding a short note
 after the operators table.

I will add in the next version.


 inet-selfuncs
 -

 Compilation:

 There were some new warnings caused by this patch (with gcc 4.8.2). The
 warnings were all about the use of uninitialized variables (right,
 right_min_bits, right_order) in inet_hist_overlap_selectivity. Looking at
 the code I see that they are harmless but you should still rewrite the loop
 to silence the warnings.

I will fix in the next version.


 Regression tests:

 There are two broken

 -- Insist that all built-in pg_proc entries have descriptions

 Here you should just add descriptions for the new functions in pg_proc.h.

I will add in the next version.


 -- Check that all opclass search operators have selectivity estimators.

 Fails due to missing selectivity functions for the operators. I think you
 should at least for now do like the range types and use areajoinsel,
 contjoinsel, etc for the join selectivity estimation.

I did not support them in the first version because I could not decide
the way. I have better options than using the the geo_selfuncs for these
operators. The options are from simple to complex:

0) just use network_overlap_selectivity
1) fix network_overlap_selectivity with a constant between 0 and 1
2) calculate this constant by using masklens of all buckets of the histogram
3) calculate this constant by using masklens of matching buckets of
   the histogram
4) store STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM for this
   types, calculate this constant with it
5) create another kind of histogram for masklens, calculate this constant
   with it

I do not know how to do 4 or 5, so I will try 3 for the next version. Do you
think it is a good idea?


 Source code:

 The selectivity estimation functions, network_overlap_selectivity and
 network_adjacent_selectivity, should follow the naming conventions of the
 other selectivity estimation functions. They are named things like
 tsmatchsel, tsmatchjoinsel, and rangesel.

I will rename it to inetoverlapsel, 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] array_length(anyarray)

2014-01-19 Thread Marko Tiikkaja



On 1/19/14, 9:12 AM, Dean Rasheed wrote:

On 18 January 2014 03:07, Marko Tiikkaja ma...@joh.to wrote:

Here's the patch as promised.  Thoughts?



A couple of points:

The answer for empty (zero dimensional) arrays is wrong --- you need
special case handling for this case to return 0.


How embarrassing.  I don't know why I removed that check or how I didn't 
catch the clearly wrong answer in the test output.



In fact why not
simply use ArrayGetNItems()?


Even better.  Changed.


In the docs, in the table of array functions, I think it would
probably be useful to make the entry for array_length say see also
cardinality, otherwise people might just stop reading there. I
suspect that in over 90% of cases, cardinality will be the more
appropriate function to use rather than array_length.


I don't see this as a huge improvement, but even worse, I don't see a 
way to naturally fit it into the description.



New version attached, without the doc change.


Regards,
Marko Tiikkaja
*** a/doc/src/sgml/array.sgml
--- b/doc/src/sgml/array.sgml
***
*** 338,343  SELECT array_length(schedule, 1) FROM sal_emp WHERE name = 
'Carol';
--- 338,356 
  2
  (1 row)
  /programlisting
+ 
+  functioncardinality/function returns the total number of elements in an
+  array across all dimensions.  It is effectively the number of rows a call to
+  functionunnest/function would yield:
+ 
+ programlisting
+ SELECT cardinality(schedule) FROM sal_emp WHERE name = 'Carol';
+ 
+  cardinality 
+ -
+4
+ (1 row)
+ /programlisting
   /para
   /sect2
  
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 11009,11014  SELECT NULLIF(value, '(none)') ...
--- 11009,11017 
  primaryarray_upper/primary
/indexterm
indexterm
+ primarycardinality/primary
+   /indexterm
+   indexterm
  primarystring_to_array/primary
/indexterm
indexterm
***
*** 11167,11172  SELECT NULLIF(value, '(none)') ...
--- 11170,11186 
 row
  entry
   literal
+   functioncardinality/function(typeanyarray/type)
+  /literal
+ /entry
+ entrytypeint/type/entry
+ entryreturns the total number of elements in the array, or 0 if the 
array is empty/entry
+ entryliteralcardinality(ARRAY[[1,2],[3,4]])/literal/entry
+ entryliteral4/literal/entry
+/row
+row
+ entry
+  literal
functionstring_to_array/function(typetext/type, 
typetext/type optional, typetext/type/optional)
   /literal
  /entry
*** a/src/backend/utils/adt/arrayfuncs.c
--- b/src/backend/utils/adt/arrayfuncs.c
***
*** 1740,1745  array_length(PG_FUNCTION_ARGS)
--- 1740,1757 
  }
  
  /*
+  * array_cardinality:
+  *returns the total number of elements in an array
+  */
+ Datum
+ array_cardinality(PG_FUNCTION_ARGS)
+ {
+   ArrayType  *v = PG_GETARG_ARRAYTYPE_P(0);
+   PG_RETURN_INT32(ArrayGetNItems(ARR_NDIM(v), ARR_DIMS(v)));
+ }
+ 
+ 
+ /*
   * array_ref :
   *  This routine takes an array pointer and a subscript array and returns
   *  the referenced item as a Datum.  Note that for a pass-by-reference
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 840,845  DATA(insert OID = 2092 (  array_upper PGNSP PGUID 12 1 0 0 
0 f f f f t f i 2
--- 840,847 
  DESCR(array upper dimension);
  DATA(insert OID = 2176 (  array_length   PGNSP PGUID 12 1 0 0 0 f f f 
f t f i 2 0 23 2277 23 _null_ _null_ _null_ _null_ array_length _null_ _null_ 
_null_ ));
  DESCR(array length);
+ DATA(insert OID = 3179 (  cardinalityPGNSP PGUID 12 1 0 0 0 f f f f t f i 
1 0 23 2277 _null_ _null_ _null_ _null_ array_cardinality _null_ _null_ 
_null_ ));
+ DESCR(array cardinality);
  DATA(insert OID = 378 (  array_appendPGNSP PGUID 12 1 0 0 0 f f f f f f i 
2 0 2277 2277 2283 _null_ _null_ _null_ _null_ array_push _null_ _null_ 
_null_ ));
  DESCR(append element onto end of array);
  DATA(insert OID = 379 (  array_prepend   PGNSP PGUID 12 1 0 0 0 f f f 
f f f i 2 0 2277 2283 2277 _null_ _null_ _null_ _null_ array_push _null_ 
_null_ _null_ ));
*** a/src/include/utils/array.h
--- b/src/include/utils/array.h
***
*** 204,209  extern Datum array_dims(PG_FUNCTION_ARGS);
--- 204,210 
  extern Datum array_lower(PG_FUNCTION_ARGS);
  extern Datum array_upper(PG_FUNCTION_ARGS);
  extern Datum array_length(PG_FUNCTION_ARGS);
+ extern Datum array_cardinality(PG_FUNCTION_ARGS);
  extern Datum array_larger(PG_FUNCTION_ARGS);
  extern Datum array_smaller(PG_FUNCTION_ARGS);
  extern Datum generate_subscripts(PG_FUNCTION_ARGS);
*** a/src/test/regress/expected/arrays.out
--- b/src/test/regress/expected/arrays.out
***
*** 1455,1460  select array_length(array[[1,2,3], [4,5,6]], 3);
--- 1455,1502 
   
  (1 row)
  
+ select 

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

2014-01-19 Thread David Rowley
On Sat, Jan 18, 2014 at 11:34 AM, David Rowley dgrowle...@gmail.com wrote:


 The latest version of the patch is attached.


I've attached an updated version of the patch.

I'm now using github to track the changes on the patch, so I've included
the commit sha in the file name of the latest commit that this patch
includes, but I've also included the date.

Please see https://github.com/david-rowley/postgres/commits/invtrans for
what's been changed.

Right now I don't think there is very much left to do. Perhaps the
documents need some examples of creating inverse transition functions, I
was not sure, so I left them out for now.

Regards

David Rowley



 Regards

 David Rowley




inverse_transition_functions_d00df99_2014-01-20.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] Feature request: Logging SSL connections

2014-01-19 Thread Magnus Hagander
On Fri, Jan 17, 2014 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Magnus Hagander mag...@hagander.net writes:
  Applied, thanks!

 Minor bikeshedding: the messages would read better, to my eye, as

 user=%s database=%s SSL enabled (protocol=%s, cipher=%s)

 Putting enabled where it is requires extra mental gymnastics on
 the part of the reader.  And why the random change between =
 in one part of the string and :  in the new part?  You could take
 that last point a bit further and make it


Makes sense.


user=%s database=%s SSL=enabled (protocol=%s, cipher=%s)

 but I'm not sure if that's an improvement.


I don't think it is, so I've applied the first suggestion.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] array_length(anyarray)

2014-01-19 Thread Dean Rasheed
On 19 January 2014 11:43, Marko Tiikkaja ma...@joh.to wrote:


 On 1/19/14, 9:12 AM, Dean Rasheed wrote:

 On 18 January 2014 03:07, Marko Tiikkaja ma...@joh.to wrote:

 Here's the patch as promised.  Thoughts?


 A couple of points:

 The answer for empty (zero dimensional) arrays is wrong --- you need
 special case handling for this case to return 0.


 How embarrassing.  I don't know why I removed that check or how I didn't
 catch the clearly wrong answer in the test output.


 In fact why not
 simply use ArrayGetNItems()?


 Even better.  Changed.


 In the docs, in the table of array functions, I think it would
 probably be useful to make the entry for array_length say see also
 cardinality, otherwise people might just stop reading there. I
 suspect that in over 90% of cases, cardinality will be the more
 appropriate function to use rather than array_length.


 I don't see this as a huge improvement, but even worse, I don't see a way to
 naturally fit it into the description.


Hmm. Looking at that page in the docs, it currently doesn't even
mention that array_length returns NULL for empty arrays, or more
generally for arrays that don't have the requested dimension. To
someone unfamiliar with postgresql arrays, that could lead to a nasty
surprise.

How about having the array_length docs say something like

   returns the length of the requested array dimension, or NULL if the
   array is empty or does not have the requested dimension.  To get the
   total number of array elements across all dimensions, use
   functioncardinality/.

If we did that, we should probably also add the or NULL if the array
is empty or does not have the requested dimension clause to the
array_upper and array_lower docs, and or NULL if the array is empty
to the array_dims and array_ndims docs.

That might seem overly pedantic, but it's quite annoying when API
documentation doesn't fully specify the behaviour, and you're forced
to use trial-and-error to find out how the functions behave.

Regards,
Dean


-- 
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] array_length(anyarray)

2014-01-19 Thread Marko Tiikkaja

On 1/19/14, 2:12 PM, Dean Rasheed wrote:

That might seem overly pedantic, but it's quite annoying when API
documentation doesn't fully specify the behaviour, and you're forced
to use trial-and-error to find out how the functions behave.


For what it's worth, I was thinking the same thing when I was looking at 
that table.  Nearly *all* of them are completely inadequate at 
explaining the finer details, and the user has to experiment to figure 
out what actually happens.  I seem to recall other similar examples in 
our documentation for functions.


Personally I would like to see this fixed for all functions, not just 
array functions.  But I think that should be a separate patch.



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] Changeset Extraction v7.0 (was logical changeset generation)

2014-01-19 Thread Stefan Kaltenbrunner
On 01/18/2014 02:31 PM, Robert Haas wrote:
 On Thu, Jan 16, 2014 at 10:15 PM, Craig Ringer cr...@2ndquadrant.com wrote:
 Anybody who actually uses SHIFT_JIS as an operational encoding, rather
 than as an input/output encoding, is into pain and suffering. Personally
 I'd be quite happy to see it supported as client_encoding, but forbidden
 as a server-side encoding. That's not the case right now - so since we
 support it, we'd better guard against its quirks.
 
 I think that *is* the case right now.  pg_wchar.h sayeth:
 
 /* followings are for client encoding only */
 PG_SJIS,/* Shift JIS
 (Winindows-932) */

while you have that file open: s/Winindows-932/Windows-932 maybe?



Stefan


-- 
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] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread Amit Kapila
On Thu, Dec 5, 2013 at 7:54 PM, MauMau maumau...@gmail.com wrote:
 Hello,

 I've removed a limitation regarding event log on Windows with the attached
 patch.  I hesitate to admit this is a bug fix and want to regard this an
 improvement, but maybe it's a bug fix from users' perspective.  Actually, I
 received problem reports from some users.


 [Problem]
 pg_ctl always uses event source PostgreSQL to output messages to the event
 log.  Some example messages are:

 server starting
 server started

 This causes the problems below:

 1. Cannot distinguish PostgreSQL instances because they use the same event
 source.

 2. If you use multiple installations of PostgreSQL on the same machine,
 whether they are the same or different versions, Windows event viewer
 reports an error event source not found in the following sequence.  Other
 system administration software which deal with event log may show other
 anomalies.
 2-1 Install the first PostgreSQL (e.g. 9.3) into dir_1, register
 dir_1\lib\pgevent.dll for event source PostgreSQL, then creates
 instance_1.
 2-2 Install the second PostgreSQL (e.g. 9.2) into dir_2 as part of some
 packaged application, register dir_2\lib\pgevent.dll for event source
 PostgreSQL,

Today, I was trying to reproduce this issue and found that if user tries
to register event source second time with same name, we just replace
the previous event source's path in registry.
Shouldn't we try to stop user at this step only, means if he tries to
register with same event source name more than once return error,
saying event source with same name already exists?


Another thing is after register of pgevent.dll, if I just try to start
PostgreSQL
it shows below messages in EventLog. Although the message has information
about server startup, but the start of Description is something similar to
what you were reporting event source not found.
Am I missing something?

Steps
1. installation of PostgreSQL from source code using Install.bat in
mdvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll
4. Modify postgresql.conf to set log_destination= 'eventlog'
5. pg_ctl.exe start -D ..\..\Data

Log Name:  Application
Source:PostgreSQL
Date:  1/19/2014 7:49:54 PM
Event ID:  0
Task Category: None
Level: Information
Keywords:  Classic
User:  N/A
Computer:  WIN-NGNN8PKIMD7
Description:
The description for Event ID 0 from source PostgreSQL cannot be found.
Either the component that raises this event is not installed on your
local computer or the installation is corrupted. You can install or
repair the component on the local computer.

If the event originated on another computer, the display information
had to be saved with the event.

The following information was included with the event:

LOG:  ending log output to stderr
HINT:  Future log output will go to log destination eventlog.


the message resource is present but the message is not found in the
string/message table


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


Re: [HACKERS] Deprecations in authentication

2014-01-19 Thread Magnus Hagander
On Sat, Jan 18, 2014 at 3:59 PM, Andrew Dunstan and...@dunslane.net wrote:


 On 01/16/2014 08:01 AM, Magnus Hagander wrote:


 On Wed, Jan 15, 2014 at 6:57 PM, Tom Lane t...@sss.pgh.pa.us mailto:
 t...@sss.pgh.pa.us wrote:

 Magnus Hagander mag...@hagander.net mailto:mag...@hagander.net

 writes:
  One thing I noticed - in MSVC, the config parameter krb5
 (equivalent of
  the removed --with-krb5) enabled *both* krb5 and gssapi, and
 there is no
  separate config parameter for gssapi. Do we want to rename that
 one to
  gss, or do we want to keep it as krb5? Renaming it would break
  otherwise working environments, but it's kind of weird to leave
 it...

 +1 for renaming --- anybody who's building with krb5 and
 expecting to,
 you know, actually *get* krb5 would probably rather find out about
 this
 change at build time instead of down the road a ways.

 A compromise position would be to introduce a gss parameter while
 leaving
 krb5 in place as a deprecated (perhaps undocumented?) synonym for it.
 But I think that's basically confusing.


 Yeah, I'm not sure it actually helps much.


 Andrew - is this going to cause any issues wrt the buildfarm, by any
 chance?


 None of my Windows buildfarm members builds with krb5. Mastodon does,
 although it seems to have gone quiet for 16 days (Dave - might be worth a
 check). Probably the result of renaming krb5 would be just that the build
 would proceed without it. From memory I don't thing the config settings are
 sanity checked.

 (We need some more, and more modern, Windows buildfarm members.)


Thanks, pushed with the rename. That'll keep things less confusing going
forward at least :)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


[HACKERS] Add value to error message when size extends

2014-01-19 Thread Maor Lipchuk
Hi all,

We have encountered an issue when executing an insert command,
when one of the values' length was bigger than the column size limitation.

the error message which been displayed was value too long for type char...
but there was no indication which value has exceeded the limited size.
(See bug #8880)

Attached is a WIP patch which attend to make the message clearer.

Regards,
Maor and Daniel
From c77ce40572b2079f3f107bd05c563331e000737a Mon Sep 17 00:00:00 2001
From: Maor Lipchuk mlipc...@redhat.com
Date: Sun, 19 Jan 2014 18:13:52 +0200
Subject: [PATCH] varchar.c[WIP]: Add value to error message when size extends

When trying to insert a value into to a column which has size limitation
the error message being presented is value too long for type char...
but there is no inication which value has exceeded the size.
(See bug #8880)

The proposed fix was to add the value the user tried to insert,
so she can get a clue which value has been problematic.
---
 src/backend/utils/adt/varchar.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 502ca44..baf4d88 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -619,7 +619,7 @@ varchar(PG_FUNCTION_ARGS)
 			if (s_data[i] != ' ')
 ereport(ERROR,
 		(errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-	  errmsg(value too long for type character varying(%d),
+	  errmsg(value (%s) too long for type character varying(%d), s_data,
 			 maxlen)));
 	}
 
-- 
1.7.1


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

2014-01-19 Thread Florian Pflug
On Jan19, 2014, at 05:27 , David Rowley dgrowle...@gmail.com wrote:
 I just finished implementing the inverse transition functions for bool_and
 and bool_or, these aggregates had a sort operator which I assume would have
 allowed an index scan to be performed, but since I had to change the first
 argument of these aggregates to internal and that meant I had to get rid of
 the sort operator...

Why does having transition type internal prevent you from specifying a
sort operator? The sort operator's argument types must match the *input*
type of the aggregate, not the transition type.

Here's a pure SQL implementation of an optimized bool_and called myand_agg
that uses state type bigint[] and specifies a sort operator.

  create or replace function myboolagg_fwd(counts bigint[], value bool)
  returns bigint[] as $$
select array[
  counts[1] + case value when true then 0 else 1 end,
  counts[2] + case value when true then 1 else 0 end
]
  $$ language sql strict immutable;

  create or replace function myboolagg_inv(counts bigint[], value bool)
  returns bigint[] as $$
select array[
  counts[1] - case value when true then 0 else 1 end,
  counts[2] - case value when true then 1 else 0 end
]
  $$ language sql strict immutable;

  create or replace function myboolagg_and(counts bigint[])
  returns bool as $$
select case counts[1] when 0 then true else false end
  $$ language sql strict immutable;

  create aggregate myand_agg (bool) (
stype = bigint[],
sfunc = myboolagg_fwd,
invfunc = myboolagg_inv,
finalfunc = myboolagg_and,
sortop = ,
initcond = '{0,0}'
  );

With this, doing

  create table boolvals as
select i, random()  0.5 as v from generate_series(1,1) i;
  create index on boolvals(v);

  explain analyze select myand_agg(v) from boolvals;

yields

 Result  (cost=0.33..0.34 rows=1 width=0) (actual time=0.067..0.067 rows=1 
loops=1)
   InitPlan 1 (returns $0)
 -  Limit  (cost=0.29..0.33 rows=1 width=1) (actual time=0.061..0.061 
rows=1 loops=1)
   -  Index Only Scan using boolvals_v_idx on boolvals  
(cost=0.29..474.41 rows=9950 width=1) (actual time=0.061..0.061 rows=1 loops=1)
 Index Cond: (v IS NOT NULL)
 Heap Fetches: 1
 Total runtime: 0.100 ms

which looks fine, no?

best regards,
Florian Pflug



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


Re: [HACKERS] Add value to error message when size extends

2014-01-19 Thread Tom Lane
Maor Lipchuk mlipc...@redhat.com writes:
 We have encountered an issue when executing an insert command,
 when one of the values' length was bigger than the column size limitation.

 the error message which been displayed was value too long for type char...
 but there was no indication which value has exceeded the limited size.
 (See bug #8880)

 Attached is a WIP patch which attend to make the message clearer.

This sort of thing has been proposed before ...

We have a message style guideline that says that primary error messages
should fit on one line, which is generally understood to mean maybe
80 characters.  Complaining about a too-long varchar string in this style
seems practically guaranteed to violate that.  More, putting the string
contents before the meat of the message is the worst possible choice if
a client user interface decides to truncate the message.  And if the
string were *really* long, say in the millions of characters, it's not
unlikely that this would end up causing the message to get replaced by
out of memory, which is totally counterproductive.

You could avoid the first two of those objections by putting the string
contents into a DETAIL message; but not the third one.

Aside from that, it's less than clear whether this approach would actually
help much: maybe the string is readily identifiable as to which column
it's meant for, and maybe not.  People have speculated about ways to
name the target column in the error report, which would probably be
far more useful; but it's not real clear how to do that in our system
structure.

One thing that occurs to me just now is that perhaps we could report
the failure as if it were a syntax error, that is with an error cursor
pointing to where in the triggering SQL query the coercion is being done.
(Years ago this would not have been possible because we didn't always
keep around the query text until runtime, but I think we do now.)
It would take some work to make that happen, and I'm not sure it would
really resolve the usability problem, but it seems worth proposing.
An advantage is that over time such a facility could be extended to
run-time errors happening in any function not just this particular one.

regards, tom lane


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


[HACKERS] PGCon 2014 - last chance

2014-01-19 Thread Dan Langille
Today is your last chance to submit a proposal for PGCon 2014.

PGCon 2014 will be on 20-24 May 2014 at University of Ottawa.

* 20-21 (Tue-Wed) tutorials
* 22-23 (Thu-Fri) talks - the main part of the conference
* 24 (Sat) The Unconference (very popular in 2013, the first year)

See http://www.pgcon.org/2014/


We are now accepting proposals for the main part of the conference (22-23 May).
Proposals can be quite simple. We do not require academic-style papers.

If you are doing something interesting with PostgreSQL, please submit
a proposal.  You might be one of the backend hackers or work on a
PostgreSQL related project and want to share your know-how with
others. You might be developing an interesting system using PostgreSQL
as the foundation. Perhaps you migrated from another database to
PostgreSQL and would like to share details.  These, and other stories
are welcome. Both users and developers are encouraged to share their
experiences.

Here are a some ideas to jump start your proposal process:

- novel ways in which PostgreSQL is used
- migration of production systems from another database
- data warehousing
- tuning PostgreSQL for different work loads
- replication and clustering
- hacking the PostgreSQL code
- PostgreSQL derivatives and forks
- applications built around PostgreSQL
- benchmarking and performance engineering
- case studies
- location-aware and mapping software with PostGIS
- The latest PostgreSQL features and features in development
- research and teaching with PostgreSQL
- things the PostgreSQL project could do better
- integrating PostgreSQL with 3rd-party software

Both users and developers are encouraged to share their experiences.

The schedule is:

1 Dec 2013 Proposal acceptance begins
19 Jan 2014 Proposal acceptance ends
19 Feb 2014 Confirmation of accepted proposals

NOTE: the call for lightning talks will go out very close to the conference.
Do not submit lightning talks proposals until then.

See also http://www.pgcon.org/2014/papers.php

Instructions for submitting a proposal to PGCon 2014 are available
from: http://www.pgcon.org/2014/submissions.php

-- 
Dan Langille - http://langille.org



signature.asc
Description: Message signed with OpenPGP using GPGMail


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

2014-01-19 Thread David Rowley
On Mon, Jan 20, 2014 at 5:53 AM, Florian Pflug f...@phlo.org wrote:

 On Jan19, 2014, at 05:27 , David Rowley dgrowle...@gmail.com wrote:
  I just finished implementing the inverse transition functions for
 bool_and
  and bool_or, these aggregates had a sort operator which I assume would
 have
  allowed an index scan to be performed, but since I had to change the
 first
  argument of these aggregates to internal and that meant I had to get
 rid of
  the sort operator...

 Why does having transition type internal prevent you from specifying a
 sort operator? The sort operator's argument types must match the *input*
 type of the aggregate, not the transition type.

 Here's a pure SQL implementation of an optimized bool_and called myand_agg
 that uses state type bigint[] and specifies a sort operator.

   create or replace function myboolagg_fwd(counts bigint[], value bool)
   returns bigint[] as $$
 select array[
   counts[1] + case value when true then 0 else 1 end,
   counts[2] + case value when true then 1 else 0 end
 ]
   $$ language sql strict immutable;

   create or replace function myboolagg_inv(counts bigint[], value bool)
   returns bigint[] as $$
 select array[
   counts[1] - case value when true then 0 else 1 end,
   counts[2] - case value when true then 1 else 0 end
 ]
   $$ language sql strict immutable;

   create or replace function myboolagg_and(counts bigint[])
   returns bool as $$
 select case counts[1] when 0 then true else false end
   $$ language sql strict immutable;

   create aggregate myand_agg (bool) (
 stype = bigint[],
 sfunc = myboolagg_fwd,
 invfunc = myboolagg_inv,
 finalfunc = myboolagg_and,
 sortop = ,
 initcond = '{0,0}'
   );

 With this, doing

   create table boolvals as
 select i, random()  0.5 as v from generate_series(1,1) i;
   create index on boolvals(v);

   explain analyze select myand_agg(v) from boolvals;

 yields

  Result  (cost=0.33..0.34 rows=1 width=0) (actual time=0.067..0.067 rows=1
 loops=1)
InitPlan 1 (returns $0)
  -  Limit  (cost=0.29..0.33 rows=1 width=1) (actual time=0.061..0.061
 rows=1 loops=1)
-  Index Only Scan using boolvals_v_idx on boolvals
  (cost=0.29..474.41 rows=9950 width=1) (actual time=0.061..0.061 rows=1
 loops=1)
  Index Cond: (v IS NOT NULL)
  Heap Fetches: 1
  Total runtime: 0.100 ms

 which looks fine, no?


hmm, yeah you're right. I guess I didn't quite think through what the sort
comparison was comparing with, for some reason I had it in my head that it
was the aggregate state and not another value in a btree index.

I've applied that patch again and put in the sort operators.

Thanks for looking at that.

Regards

David Rowley


 best regards,
 Florian Pflug




Re: [HACKERS] Add value to error message when size extends

2014-01-19 Thread Marti Raudsepp
On Sun, Jan 19, 2014 at 8:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Complaining about a too-long varchar string in this style
 seems practically guaranteed to violate that.

Agreed. But I think it would be useful to add the length of the string
being inserted; we already have it in the len variable.

 One thing that occurs to me just now is that perhaps we could report
 the failure as if it were a syntax error

That would be cool, if it can be made to work.

Regards,
Marti


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


Re: [HACKERS] Add value to error message when size extends

2014-01-19 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes:
 Agreed. But I think it would be useful to add the length of the string
 being inserted; we already have it in the len variable.

Hm, maybe, but I'm having a hard time visualizing cases in which it
helps much.

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] improve the help message about psql -F

2014-01-19 Thread Marti Raudsepp
2014/1/17 Jov am...@amutu.com
 but in the psql --help,-F say:

 set field separator (default: |)

 if user don't read the offical doc carefully,he can use:

 psql -F , -c 'select ...'

 But can't get what he want.
 It is a bad user Experience.

+1 from me, patch applies and is helpful.

After patching this line in psql --help is 82 characters long; I think
it's best to keep help screens below 80 characters wide (although
there's already 1 other line violating this rule).

I think the word set is pretty useless there anyway, maybe remove
that so the message becomes field separator for unaligned output
(default: |)

PS: There isn't an open CommitFest to add this to. Shouldn't we always
open a new CF when the last one goes in progress? If there's no date,
it may be simply called next

Regards,
Marti


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

2014-01-19 Thread Marti Raudsepp
On Tue, Nov 19, 2013 at 6:38 PM, Andres Freund and...@2ndquadrant.com wrote:
 The attached patches compile and make check successfully on linux x86,
 amd64 and msvc x86 and amd64. This time and updated configure is
 included. The majority of operations still rely on CAS emulation.

Note that the atomics-generic-acc.h file has ® characters in the
Latin-1 encoding (Intel ® Itanium ®). If you have to use weird
characters, I think you should make sure they're UTF-8

Regards,
Marti


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


Re: [HACKERS] Add value to error message when size extends

2014-01-19 Thread Tom Lane
Marti Raudsepp ma...@juffo.org writes:
 On Sun, Jan 19, 2014 at 8:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 One thing that occurs to me just now is that perhaps we could report
 the failure as if it were a syntax error

 That would be cool, if it can be made to work.

Just as a five-minute proof-of-concept hack, attached is a patch that
makes varchar() report an error position if it can get one.  This is
*very* far from being production quality --- debug_query_string is the
wrong thing to rely on in general, and we'd certainly want to encapsulate
the logic rather than have individual functions know about how to do it.
But here's some testing that shows that the idea seems to have promise
from a usability standpoint:

regression=# create table test (f1 varchar(10), f2 varchar(5), f3 varchar(10));
CREATE TABLE

regression=# insert into test values ('a', 'b', 'foobar');
INSERT 0 1

regression=# insert into test values ('foobar', 'foobar', 'foobar');
ERROR:  value too long for type character varying(5)
LINE 1: insert into test values ('foobar', 'foobar', 'foobar');
   ^

regression=# update test set f2 = f3 where f1 = 'a';
ERROR:  value too long for type character varying(5)
LINE 1: update test set f2 = f3 where f1 = 'a';
 ^

The first error case points out a limitation of relying on the contents of
the string to figure out where your problem is.  The error-cursor approach
has its own limitations, of course; for instance the second case might not
be thought to be all that helpful.

regards, tom lane

diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 502ca44..4438ed8 100644
*** a/src/backend/utils/adt/varchar.c
--- b/src/backend/utils/adt/varchar.c
***
*** 19,24 
--- 19,25 
  #include access/tuptoaster.h
  #include libpq/pqformat.h
  #include nodes/nodeFuncs.h
+ #include tcop/tcopprot.h
  #include utils/array.h
  #include utils/builtins.h
  #include mb/pg_wchar.h
*** varchar(PG_FUNCTION_ARGS)
*** 617,626 
  	{
  		for (i = maxmblen; i  len; i++)
  			if (s_data[i] != ' ')
  ereport(ERROR,
  		(errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
  	  errmsg(value too long for type character varying(%d),
! 			 maxlen)));
  	}
  
  	PG_RETURN_VARCHAR_P((VarChar *) cstring_to_text_with_len(s_data,
--- 618,645 
  	{
  		for (i = maxmblen; i  len; i++)
  			if (s_data[i] != ' ')
+ 			{
+ int			pos = 0;
+ 
+ if (debug_query_string 
+ 	fcinfo-flinfo-fn_expr)
+ {
+ 	int location = exprLocation(fcinfo-flinfo-fn_expr);
+ 
+ 	if (location = 0)
+ 	{
+ 		/* Convert offset to character number */
+ 		pos = pg_mbstrlen_with_len(debug_query_string,
+    location) + 1;
+ 	}
+ }
+ 
  ereport(ERROR,
  		(errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
  	  errmsg(value too long for type character varying(%d),
! 			 maxlen),
! 		 errposition(pos)));
! 			}
  	}
  
  	PG_RETURN_VARCHAR_P((VarChar *) cstring_to_text_with_len(s_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] GiST support for inet datatypes

2014-01-19 Thread Andreas Karlsson

On 01/19/2014 11:10 AM, Emre Hasegeli wrote:

I am not convinced an adjacent operator is useful for the inet type, but if
it is included it should be indexed just like -|- of ranges. We should try
to keep these lists of indexed operators the same.


I added it just not to leave negotor field empty. It can also be useful with
exclusion constraints but not with GiST support. I did not add GiST support
for it and the not equals operator because they do not fit the index
structure. I can just remove the operator for now.


Sounds good. None of the other  implementations have a negator.


I think it would be fine just to add the newly indexed operators here, but
the more indexed operators we get in the core the less useful this test
becomes.


I did not add the new operators to the list because I do not feel right
about supporting , =, , = symbols for these operators.
They should be @, @=, @, @= to be consistent with other types.


I agree, but I am not sure if it is ok to break backward compatibility here.


-- Check that all opclass search operators have selectivity estimators.

Fails due to missing selectivity functions for the operators. I think you
should at least for now do like the range types and use areajoinsel,
contjoinsel, etc for the join selectivity estimation.


I did not support them in the first version because I could not decide
the way. I have better options than using the the geo_selfuncs for these
operators. The options are from simple to complex:

0) just use network_overlap_selectivity
1) fix network_overlap_selectivity with a constant between 0 and 1
2) calculate this constant by using masklens of all buckets of the histogram
3) calculate this constant by using masklens of matching buckets of
the histogram
4) store STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM for this
types, calculate this constant with it
5) create another kind of histogram for masklens, calculate this constant
with it

I do not know how to do 4 or 5, so I will try 3 for the next version. Do you
think it is a good idea?


Solutions 0 and 1 does not really sound better than using geo_selfuncs.

--
Andreas Karlsson


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


[HACKERS] What is happening on buildfarm member crake?

2014-01-19 Thread Tom Lane
Since contrib/test_shm_mq went into the tree, crake has been crashing
intermittently (maybe one time in three) at the contribcheck step.
While there's not conclusive proof that test_shm_mq is at fault, the
circumstantial evidence is pretty strong.  For instance, in the
postmaster log for the last failure,
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crakedt=2014-01-19%2013%3A26%3A08
we see

LOG:  registering background worker test_shm_mq
LOG:  starting background worker process test_shm_mq
LOG:  worker process: test_shm_mq (PID 12585) exited with exit code 1
LOG:  unregistering background worker test_shm_mq
LOG:  registering background worker test_shm_mq
LOG:  registering background worker test_shm_mq
LOG:  registering background worker test_shm_mq
LOG:  starting background worker process test_shm_mq
LOG:  starting background worker process test_shm_mq
LOG:  starting background worker process test_shm_mq
LOG:  worker process: test_shm_mq (PID 12588) exited with exit code 1
LOG:  unregistering background worker test_shm_mq
LOG:  worker process: test_shm_mq (PID 12586) exited with exit code 1
LOG:  unregistering background worker test_shm_mq
LOG:  worker process: test_shm_mq (PID 12587) exited with exit code 1
LOG:  unregistering background worker test_shm_mq
LOG:  server process (PID 12584) was terminated by signal 11: Segmentation fault
LOG:  terminating any other active server processes

Comparing the PIDs makes it seem pretty likely that what's crashing is the
backend running the test_shm_mq test.  Since the connected psql doesn't
notice any problem, most likely the crash occurs during backend shutdown
after psql has disconnected (which would also explain why no current
query gets shown in the crash report).

crake is running F16 x86_64, which I don't have installed here.  So I
tried to reproduce this on RHEL6 and F19 x86_64 machines, with build
options matching what crake says it's using, with absolutely no success.
I don't think any other buildfarm critters are showing this either, which
makes it look like it's possibly specific to the particular compiler
version crake has got.  Or maybe there's some other factor needed to
trigger it.  Anyway, I wonder whether Andrew could get a stack trace from
the core dump next time it happens.

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] What is happening on buildfarm member crake?

2014-01-19 Thread Tom Lane
I wrote:
 I don't think any other buildfarm critters are showing this either,

Scratch that --- closer inspection of the buildfarm logs shows that
kouprey and lapwing have suffered identical failures, though much less
often than crake.  That lets out the theory that it's x86_64 specific, or
even 64-bit specific.  Still have no idea why I can't reproduce it here.

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] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Today, I was trying to reproduce this issue and found that if user tries
to register event source second time with same name, we just replace
the previous event source's path in registry.
Shouldn't we try to stop user at this step only, means if he tries to
register with same event source name more than once return error,
saying event source with same name already exists?


I'm OK with either.  If we add the check, I think that would be another 
patch.  However, I'm afraid the check won't be much effective, because the 
packaged application then unregister and register (i.e. regsvr32 /u and then 
regsvr32 /i) blindly.




Another thing is after register of pgevent.dll, if I just try to start
PostgreSQL
it shows below messages in EventLog. Although the message has information
about server startup, but the start of Description is something similar to
what you were reporting event source not found.
Am I missing something?

Steps
1. installation of PostgreSQL from source code using Install.bat in
mdvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll


Please specify pgevent.dll, not pgevent32.dll.

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


Re: [HACKERS] GiST support for inet datatypes

2014-01-19 Thread Andreas Karlsson

Here comes part 2 of 2 of the review.

inet-gist
-

In general the code looks good and I think your approach makes sense. 
Not an expert on GiST though so I would like a second opinion on this. 
Maybe there is some clever trick which would make the index more 
efficient, but the design I see is simple and logical. Like this much 
more than the incorrect GiST index for inet in btree_gist.


The only thing is that I think your index would do poorly on the case 
where all values share a prefix before the netmask but have different 
values after the netmask, since gist_union and gist_penalty does not 
care about the bits after the netmask. Am I correct? Have you thought 
anything about this?


To create such data:

CREATE TABLE a (ip inet);
INSERT INTO a SELECT '127.0.0.0/16'::inet + s FROM generate_series(1, 
pow(2, 16)::int) s;

CREATE INDEX aidx ON a USING gist (ip);

Saw also some minor things too.

Typo in comment, weird sentence:

 * The main question to calculate the union is that they have how many
 * bits in common. [...]

I do not like how you called the result key i inet_union_gist() tmp. 
Something like result or result_ip would be better.


Typo in comment, reverse should be inverse:

 * Penalty is reverse of the common IP bits of the two addresses.

Typo in comment, missing be:

 * Values bigger than 1 are used when the common IP bits cannot
 * calculated.

Language can be improved in this comment. Both ways to split are by the 
union of the keys, it is more of a split by address prefix.


 * The second and the important way is to split by the union of the keys.
 * Union of the keys calculated same way with the inet_gist_union function.
 * The first and the last biggest subnets created from the calculated
 * union. In this case addresses contained by the first subnet will be put
 * to the left bucket, address contained by the last subnet will be put to
 * the right bucket.

Could you not just use memcmp in inet_gist_same() instead of bitncmp() 
since it only cares about equality?


There is an extra empty line at the end of network_gist.c

inet-selfuncs
-

Here I have to honestly admit I am not sure if I can tell if your 
selectivity function is correct. Your explanation sounds convincing and 
the general idea of looking at the histogram is correct. I am just have 
no idea if the details are right.


DEFAULT_NETWORK_OVERLAP_SELECTIVITY should be renamed 
DEFAULT_NETWORK_OVERLAP_SEL and moved to the .c file.


Typo in comment, constrant - constant.

There is an extra empty line at the end of network_selfuncs.c

--
Andreas Karlsson


--
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] Fix double-inclusion of pg_config_os.h when building extensions with Visual Studio

2014-01-19 Thread Craig Ringer
On 01/17/2014 07:41 PM, Magnus Hagander wrote:
 Regardless of where the other thread goes, this seems like something we
 should fix. Thus - applied, with minor changes to the comment, thanks.

Thanks.

 My understanding is that this change alone doesn't actually help us very
 much, so I haven't backpatched it anywhere. Let me know if that
 understanding was incorrect, and it would actually help as a backpatch.

I don't think there's any point backpatching it - as you say, by its
self it doesn't help tons. There's little or no evidence that anyone's
doing standalone Visual Studio based builds at the moment, and if they
are they'll have already had to define WIN32 themselves so they won't
notice.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


Re: [HACKERS] Add value to error message when size extends

2014-01-19 Thread Maor Lipchuk
Hi Tom and Marti
Thank you so much for your respond.

The solution Tom proposed is much more better, and it will be a great
solution for clarifying many issues regarding this error.

Regards,
Maor


On 01/19/2014 10:00 PM, Tom Lane wrote:
 Marti Raudsepp ma...@juffo.org writes:
 On Sun, Jan 19, 2014 at 8:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 One thing that occurs to me just now is that perhaps we could report
 the failure as if it were a syntax error
 
 That would be cool, if it can be made to work.
 
 Just as a five-minute proof-of-concept hack, attached is a patch that
 makes varchar() report an error position if it can get one.  This is
 *very* far from being production quality --- debug_query_string is the
 wrong thing to rely on in general, and we'd certainly want to encapsulate
 the logic rather than have individual functions know about how to do it.
 But here's some testing that shows that the idea seems to have promise
 from a usability standpoint:
 
 regression=# create table test (f1 varchar(10), f2 varchar(5), f3 
 varchar(10));
 CREATE TABLE
 
 regression=# insert into test values ('a', 'b', 'foobar');
 INSERT 0 1
 
 regression=# insert into test values ('foobar', 'foobar', 'foobar');
 ERROR:  value too long for type character varying(5)
 LINE 1: insert into test values ('foobar', 'foobar', 'foobar');
^
 
 regression=# update test set f2 = f3 where f1 = 'a';
 ERROR:  value too long for type character varying(5)
 LINE 1: update test set f2 = f3 where f1 = 'a';
  ^
 
 The first error case points out a limitation of relying on the contents of
 the string to figure out where your problem is.  The error-cursor approach
 has its own limitations, of course; for instance the second case might not
 be thought to be all that helpful.
Yes, but in this case you will know specifically which column is the
problematic one.
The highlight of your approach gains much more benefit when
updating/inserting multiple values in one update/insert command.
 
   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] What is happening on buildfarm member crake?

2014-01-19 Thread Andrew Dunstan


On 01/19/2014 04:14 PM, Tom Lane wrote:

I wrote:

I don't think any other buildfarm critters are showing this either,

Scratch that --- closer inspection of the buildfarm logs shows that
kouprey and lapwing have suffered identical failures, though much less
often than crake.  That lets out the theory that it's x86_64 specific, or
even 64-bit specific.  Still have no idea why I can't reproduce it here.


I can give you an account on the machine if you want to play.

Also crake does produce backtraces  on core dumps, and they are at the 
bottom of the buildfarm log. The latest failure backtrace is reproduced 
below.


   == stack trace: 
/home/bf/bfr/root/HEAD/inst/data-C/core.12584 ==
   [New LWP 12584]
   [Thread debugging using libthread_db enabled]
   Using host libthread_db library /lib64/libthread_db.so.1.
   Core was generated by `postgres: buildfarm contrib_regression_test_shm_mq'.
   Program terminated with signal 11, Segmentation fault.
   #0  SetLatch (latch=0x1c) at pg_latch.c:509
   509  if (latch-is_set)
   #0  SetLatch (latch=0x1c) at pg_latch.c:509
   #1  0x0064c04e in procsignal_sigusr1_handler 
(postgres_signal_arg=optimized out) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/storage/ipc/procsignal.c:289
   #2  signal handler called
   #3  _dl_fini () at dl-fini.c:190
   #4  0x00361ba39931 in __run_exit_handlers (status=0, listp=0x361bdb1668, 
run_list_atexit=true) at exit.c:78
   #5  0x00361ba399b5 in __GI_exit (status=optimized out) at exit.c:100
   #6  0x006485a6 in proc_exit (code=0) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/storage/ipc/ipc.c:143
   #7  0x00663abb in PostgresMain (argc=optimized out, argv=optimized out, 
dbname=0x12b8170 contrib_regression_test_shm_mq, username=optimized out) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/tcop/postgres.c:4225
   #8  0x0062220f in BackendRun (port=0x12d6bf0) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:4083
   #9  BackendStartup (port=0x12d6bf0) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:3772
   #10 ServerLoop () at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:1583
   #11 PostmasterMain (argc=optimized out, argv=optimized out) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:1238
   #12 0x0045e2e8 in main (argc=3, argv=0x12b7430) at 
/home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/main/main.c:205

cheers

andrew


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


Re: [HACKERS] What is happening on buildfarm member crake?

2014-01-19 Thread Robert Haas
On Sun, Jan 19, 2014 at 7:53 PM, Andrew Dunstan and...@dunslane.net wrote:
 Also crake does produce backtraces  on core dumps, and they are at the
 bottom of the buildfarm log. The latest failure backtrace is reproduced
 below.

== stack trace:
 /home/bf/bfr/root/HEAD/inst/data-C/core.12584 ==
[New LWP 12584]
[Thread debugging using libthread_db enabled]
Using host libthread_db library /lib64/libthread_db.so.1.
Core was generated by `postgres: buildfarm
 contrib_regression_test_shm_mq'.
Program terminated with signal 11, Segmentation fault.
#0  SetLatch (latch=0x1c) at pg_latch.c:509
509  if (latch-is_set)
#0  SetLatch (latch=0x1c) at pg_latch.c:509
#1  0x0064c04e in procsignal_sigusr1_handler
 (postgres_signal_arg=optimized out) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/storage/ipc/procsignal.c:289
#2  signal handler called
#3  _dl_fini () at dl-fini.c:190
#4  0x00361ba39931 in __run_exit_handlers (status=0,
 listp=0x361bdb1668, run_list_atexit=true) at exit.c:78
#5  0x00361ba399b5 in __GI_exit (status=optimized out) at
 exit.c:100
#6  0x006485a6 in proc_exit (code=0) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/storage/ipc/ipc.c:143
#7  0x00663abb in PostgresMain (argc=optimized out,
 argv=optimized out, dbname=0x12b8170 contrib_regression_test_shm_mq,
 username=optimized out) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/tcop/postgres.c:4225
#8  0x0062220f in BackendRun (port=0x12d6bf0) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:4083
#9  BackendStartup (port=0x12d6bf0) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:3772
#10 ServerLoop () at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:1583
#11 PostmasterMain (argc=optimized out, argv=optimized out) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/postmaster/postmaster.c:1238
#12 0x0045e2e8 in main (argc=3, argv=0x12b7430) at
 /home/bf/bfr/root/HEAD/pgsql.25562/../pgsql/src/backend/main/main.c:205

Hmm, that looks an awful lot like the SIGUSR1 signal handler is
getting called after we've already completed shmem_exit.  And indeed
that seems like the sort of thing that would result in dying horribly
in just this way.  The obvious fix seems to be to check
proc_exit_inprogress before doing anything that might touch shared
memory, but there are a lot of other SIGUSR1 handlers that don't do
that either.  However, in those cases, the likely cause of a SIGUSR1
would be a sinval catchup interrupt or a recovery conflict, which
aren't likely to be so far delayed that they arrive after we've
already disconnected from shared memory.  But the dynamic background
workers stuff adds a new possible cause of SIGUSR1: the postmaster
letting us know that a child has started or died.  And that could
happen even after we've detached shared memory.

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

2014-01-19 Thread Robert Haas
On Fri, Jan 17, 2014 at 5:45 AM, Marko Tiikkaja ma...@joh.to wrote:
 On 1/17/14 11:26 AM, Pavel Stehule wrote:

 After some thinking I don't think so this design is not good. It  changing
 a working with exception (error) levels - and it is not consistent with
 other PostgreSQL parts.

 A benefit is less than not clean configuration. Better to solve similar
 issues via specialized plpgsql extensions or try to help me push
 plpgsql_check_function to core. It can be a best holder for this and
 similar checks.


 I see these as being two are different things.  There *is* a need for a
 full-blown static analyzer for PL/PgSQL, but I don't think it needs to be in
 core.  However, there seems to be a number of pitfalls we could warn the
 user about with little effort in core, and I think those are worth doing.

I don't want to be overly negative, but that sounds sort of like
you're saying that it's not worth having a good static analyzer in
core, but that you are in favor of having a bad one.

I personally tend to think a static analyzer is a better fit than
adding a whole laundry list of pragmas.  Most people won't remember to
turn them all on anyway, and those who do will find that it gets
pretty tedious after we have more than about two of them.

-- 
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] [v9.4] row level security

2014-01-19 Thread Craig Ringer
On 01/18/2014 03:27 AM, Gregory Smith wrote:
 With my advocacy hat on, I'd like to revisit this idea now that there's
 a viable updatable security barrier view submission.  I thought the most
 serious showstopper feedback from the last CF's RLS submission was that
 this needed to be sorted out first.  Reworking KaiGai's submission to
 merge against Dean's new one makes it viable again in my mind, and I'd
 like to continue toward re-reviewing it as part of this CF in that
 light.

I had hoped to have this done weeks ago, but was blocked on getting a
viable approach to updatable security barrier views in place. I really
appreciate Dean, with his greater experience and skill in this area,
looking into it.

As it is I'm spending today reworking the RLS patch on top of the new
approach to updatable security barrier views.

Then it'll be a matter of tests, lots and lots of tests of every weird
corner I can think of.

 Admittedly it's not ideal to try and do that at the same time
 the barrier view patch is being modified, but I see that as a normal CF
 merge of things based on other people's submissions.

I tend to agree - and the whole idea of reworking RLS on top of
updatable security barrier views is that it makes the patch for RLS's UI
and catalogs largely independent from the mechanics of filtering rows.

 I mentioned advocacy because the budding new PostgreSQL test instances
 I'm seeing now will lose a lot of momentum if we end up with no user
 visible RLS features in 9.4.  The pieces we have now can assemble into
 something that's useful, and I don't think that goal is unreasonably far
 away.

If it's possible, getting _something_ into 9.4 would be great. I'm aware
of multiple interested users who originally expected this in 9.3. That
hasn't worked out, but it'd be great to make 9.4.

-- 
 Craig Ringer   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] currawong is not a happy animal

2014-01-19 Thread Craig Ringer
On 01/18/2014 12:26 AM, David Rowley wrote:
 -#if defined(_WIN32)
 +#if defined(_WIN32)  !defined(WIN32)

That makes sense, since we force WIN32 in the build system. I should've
seen that. Thanks for catching it.

-- 
 Craig Ringer   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] ALTER TABLE ... SET TABLESPACE pg_default

2014-01-19 Thread Craig Ringer
On 01/17/2014 05:28 AM, Stephen Frost wrote:
 Greetings,
 
 Harking back to 10 years ago when tablespaces were added, it looks 
 like we originally figured that users didn't need permissions to 
 create tables in the database default, per 2467394e.  That strikes
 me as perfectly fair.  Unfortunately, the later addition of ALTER
 TABLE ... SET TABLESPACE (af4de814) didn't get the memo about the
 default tablespace being special in this regard and refuses to let 
 a user move their tables into the default tablespace, even though
 they can do so via 'CREATE TABLE ... AS SELECT * FROM ...'.
 
 Barring objections, I'll add the same conditional around the
 AclCheck in ATPrepSetTableSpace() as exists in DefineRelation() to
 allow users to ALTER TABLE ... SET TABLESPACE into the database's
 default tablespace and backpatch accordingly.

Sounds sensible. I just stumbled across a report of this bug, too:

http://stackoverflow.com/questions/21193127/avoid-users-to-create-tables-on-default-tablespace


-- 
 Craig Ringer   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)

2014-01-19 Thread Florian Pflug
On Jan19, 2014, at 20:00 , David Rowley dgrowle...@gmail.com wrote:
 I've applied that patch again and put in the sort operators.

I've push a new version to https://github.com/fgp/postgres/tree/invtrans
This branch includes the following changes

* A bunch of missing declaration for *_inv functions

* An assert that the frame end doesn't move backwards - I realized that
 it is after all easy to do that, if it's done after the loop which adds
 the new values, not before.

* EXPLAIN VERBOSE ANALYZE now shows the max. number of forward aggregate
 transitions per row and aggregate. It's a bit imprecise, because it doesn't
 track the count per aggregate, but it's still a good metric for how well
 the inverse transition functions work. If the number is close to one, you
 know that very few rescans are happening.

* I've also renamed INVFUNC to INVSFUNC. That's a pretty invasive change, and
 it's the last commit, so if you object to that, then you can merge up to
 eafa72330f23f7c970019156fcc26b18dd55be27 instead of
 de3d9148be9732c4870b76af96c309eaf1d613d7.

A few more things I noticed, all minor stuff

* do_numeric_discard()'s inverseTransValid flag is unnecessary. First, if the
 inverse transition function returns NULL once, we never call it again, so the
 flag won't have any practical effect. And second, assume we ever called the
 forward transition function after the inverse fail, and then retried the 
inverse.
 In the case of do_numeric_discard(), that actually *could* allow the inverse
 to suddenly succeed - if the call to the forward function increased the dscale
 beyond that of the element we tried to remove, removal would suddenly be
 possible again. We never do that, of course, and it seems unlikely we ever
 will. But it's still weird to have code which serves no other purpose than to
 pessimize a case which would otherwise just work fine.

* The state == NULL checks in all the strict inverse transition functions are
 redundant.

I haven't taken a close look at the documentation yet, I hope to be able to
do that tomorrow. Otherwise, things look good as far as I'm concerned.

best regards,
Florian Pflug



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


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread Amit Kapila
On Mon, Jan 20, 2014 at 4:05 AM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Today, I was trying to reproduce this issue and found that if user tries
 to register event source second time with same name, we just replace
 the previous event source's path in registry.
 Shouldn't we try to stop user at this step only, means if he tries to
 register with same event source name more than once return error,
 saying event source with same name already exists?


 I'm OK with either.  If we add the check, I think that would be another
 patch.

   Do you think without this the problem reported by you is resolved completely.
   User can hit same problem, if he tries to follow similar steps mentioned in
   your first mail. I had tried below steps based on description in your
   first mail:

   Steps
1. installation of PostgreSQL from source code (master) using Install.bat in
msvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent.dll
4. Modify postgresql.conf to set log_destination= 'eventlog'
5. event_source = 'PostgreSQL'
6. pg_ctl.exe start -D ..\..\Data
7. psql -d postgres
8. Drop table t1; --try dropping some non-existant table
9. Check in Event viewer, you can find proper error.
10. NO Problem till above step.
11. Installation of PostgreSQL from source code (9.2) using Install.bat in
msvc directory
12. initdb -D 9.2_data_dir
13. regsvr32 /n /i:PostgreSQL install_9.2_dir_path\lib\pgevent.dll
14. Remove 9.2 installation (i have just renamed the install folder)
15. now perform step 8 again on master (with or without restart of server)
16. Open Event Viewer, you can see the message
 event source not found

Now this could have been avoided, if in step-13 we use different
event source name, but I think that will happen even without
this patch.

 However, I'm afraid the check won't be much effective, because the
 packaged application then unregister and register (i.e. regsvr32 /u and then
 regsvr32 /i) blindly.

If user register/unregister different versions of pgevent.dll blindly,
then I think
any fix in this area could not prevent the error event source not found


 Another thing is after register of pgevent.dll, if I just try to start
 PostgreSQL
 it shows below messages in EventLog. Although the message has information
 about server startup, but the start of Description is something similar to
 what you were reporting event source not found.
 Am I missing something?

 Steps
 1. installation of PostgreSQL from source code using Install.bat in
 mdvc directory
 2. initdb -D data_dir
 3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll


 Please specify pgevent.dll, not pgevent32.dll.

Typo, I was using prevent.dll only, I think the reason is I need to restart
Event Viewer after register command.


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


Re: [HACKERS] ALTER SYSTEM SET typos and fix for temporary file name management

2014-01-19 Thread Amit Kapila
On Sat, Jan 18, 2014 at 7:59 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 Hi all,

 After going through commit 65d6e4c (introducing ALTER SYSTEM SET), I
 noticed a couple of typo mistakes as well as (I think) a weird way of
 using the temporary auto-configuration name postgresql.auto.conf.temp
 in two different places, resulting in the patch attached.

.tmp suffix is used at couple other places in code as well.
snapmgr.c
XactExportFilePath(pathtmp, topXid, list_length(exportedSnapshots), .tmp);
receivelog.c
snprintf(tmppath, MAXPGPATH, %s.tmp, path);

Although similar suffix is used at other places, but still I think it might be
better to define for current case as here prefix (postgresql.auto.conf) is also
fixed and chances of getting it changed are less. However even if we don't
do, it might be okay as we are using such suffixes at other places as well.


 It might be an overkill to use a dedicated variable for the temporary
 autoconfiguration file (here PG_AUTOCONF_FILENAME_TEMP), but I found
 kind of weird the way things are currently done on master branch. IMO,
 it would reduce bug possibilities to have everything managed with a
 single variable.

 Typos at least should be fixed :)

- appendStringInfoString(buf, # Do not edit this file manually! \n);
- appendStringInfoString(buf, # It will be overwritten by ALTER
SYSTEM command. \n);
+ appendStringInfoString(buf, # Do not edit this file manually!\n);
+ appendStringInfoString(buf, # It will be overwritten by ALTER
SYSTEM command.\n);

Same change in initdb.c is missing.

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


Re: [HACKERS] Funny representation in pg_stat_statements.query.

2014-01-19 Thread Kyotaro HORIGUCHI
Thank you.

tgl  Hello, I noticed that pg_stat_statements.query can have funny values.
tgl 
tgl I don't think that's an acceptable reason for lobotomizing the parser's
tgl ability to print error cursors, which is what your first patch does
tgl (and without even any documentation that would keep someone from
tgl changing it back).

Yes, Nevertheless I don't think that replacement the keywords
with the fuction 'now' cannot results in error and the function
'now' itself cannot throw errors before significant changes is
made, of course there is no never.

tgl The second patch might be all right, though I'm a bit disturbed by its
tgl dependency on gram.h constants.  The numeric values of those change on a
tgl regular basis, and who's to say that these are exactly the set of tokens
tgl that we care about?

I felt the same anxiety. The code seems largely unstable because
mainly of the last reason. There seems no viable (and stable)
means to check and/or keep the pertinence of the token set. So I
also begged(?) for the third way.

tgl In the end, really the cleanest fix for this would be to get rid of the
tgl grammar's translation of these special functions into hacky expressions.
tgl They ought to get translated into some new node type(s) that could be
tgl reverse-listed in standard form by ruleutils.c.  I've wanted to do that
tgl for years, but never got around to it ...

Yes, that's what somewhat worried me. That way enables us to use
the lexical locations without extra care. (although also seems to
be a bit too heavy labor for the gain..)

 - CURRENT_TIME and the like are parsed into the nodes directly
   represents the node specs in gram.y

 - add transform.. uh.. transformDatetimeFuncs or something to
   transform the nodes into executable form, perhaps. But it
   should be after pg_stat_statements refer to the query tree.

 - modify ruleutils.c in corresponding part if needed.

Since this becomes more than a simple bug fix, I think it is too
late for 9.4 now. I'll work on this in the longer term.

Thanks for the suggestion.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


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


Re: [HACKERS] [BUGS] surprising to_timestamp behavior

2014-01-19 Thread Jeevan Chalke

 I went to review this, and found that there's not actually a patch
 attached ...

 regards, tom lane


Attached. Sorry for that.

-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 6ae426b..56fb359 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -2846,16 +2846,18 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out)
 	{
 		if (n-type != NODE_TYPE_ACTION)
 		{
+			/* Separator, consume one character from input string */
 			s++;
-			/* Ignore spaces when not in FX (fixed width) mode */
-			if (isspace((unsigned char) n-character)  !fx_mode)
-			{
-while (*s != '\0'  isspace((unsigned char) *s))
-	s++;
-			}
 			continue;
 		}
 
+		/* Ignore spaces when not in FX (fixed width) mode */
+		if (!fx_mode)
+		{
+			while (*s != '\0'  isspace((unsigned char) *s))
+s++;
+		}
+
 		from_char_set_mode(out, n-key-date_mode);
 
 		switch (n-key-id)
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 87a6951..9ebc0f7 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -2965,3 +2965,76 @@ SELECT to_char('2012-12-12 12:00'::timestamptz, '-MM-DD HH:MI:SS TZ');
 (1 row)
 
 RESET TIME ZONE;
+-- White spaces in input string, not matching with format string
+SELECT to_timestamp('2011-12-18 23:38:15', '-MM-DD  HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 03:38:15 2011 PST
+(1 row)
+
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD  HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 23:38:15 2011 PST
+(1 row)
+
+SELECT to_timestamp('2011-12-18   23:38:15', '-MM-DD  HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 23:38:15 2011 PST
+(1 row)
+
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 23:38:15 2011 PST
+(1 row)
+
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD  HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 23:38:15 2011 PST
+(1 row)
+
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD   HH24:MI:SS');
+ to_timestamp 
+--
+ Sun Dec 18 03:38:15 2011 PST
+(1 row)
+
+SELECT to_date('2011 12  18', ' MM DD');
+  to_date   
+
+ 12-18-2011
+(1 row)
+
+SELECT to_date('2011 12  18', ' MM  DD');
+  to_date   
+
+ 12-18-2011
+(1 row)
+
+SELECT to_date('2011 12  18', ' MM   DD');
+  to_date   
+
+ 12-08-2011
+(1 row)
+
+SELECT to_date('2011 12 18', '  MM DD');
+  to_date   
+
+ 02-18-2011
+(1 row)
+
+SELECT to_date('2011  12 18', '  MM DD');
+  to_date   
+
+ 12-18-2011
+(1 row)
+
+SELECT to_date('2011   12 18', '  MM DD');
+  to_date   
+
+ 12-18-2011
+(1 row)
+
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index fe9a520..19c1cf4 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -477,3 +477,20 @@ SELECT '2012-12-12 12:00 America/New_York'::timestamptz;
 SELECT to_char('2012-12-12 12:00'::timestamptz, '-MM-DD HH:MI:SS TZ');
 
 RESET TIME ZONE;
+
+-- White spaces in input string, not matching with format string
+SELECT to_timestamp('2011-12-18 23:38:15', '-MM-DD  HH24:MI:SS');
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD  HH24:MI:SS');
+SELECT to_timestamp('2011-12-18   23:38:15', '-MM-DD  HH24:MI:SS');
+
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD HH24:MI:SS');
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD  HH24:MI:SS');
+SELECT to_timestamp('2011-12-18  23:38:15', '-MM-DD   HH24:MI:SS');
+
+SELECT to_date('2011 12  18', ' MM DD');
+SELECT to_date('2011 12  18', ' MM  DD');
+SELECT to_date('2011 12  18', ' MM   DD');
+
+SELECT to_date('2011 12 18', '  MM DD');
+SELECT to_date('2011  12 18', '  MM DD');
+SELECT to_date('2011   12 18', '  MM DD');

-- 
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: variant of regclass

2014-01-19 Thread Amit Khandekar
Hi,

I have begun the review as part of the commitfest. Below are my comments




*_guts() functions are defined as returning Datum, while they are actually
returning Oid. They should be defined as returning Oid.
Also the PG_RETURN_OID() has been still used in some of the *_guts()
functions. They should use 'return';



In pg_proc.h, to_regproc() has been defined as function returning
*regclass* type.



I, as a user would be happier if we also have to_regprocedure() and
to_regoperator(). The following query looks a valid use-case where one
needs to find if a particular function exists. Using to_regproc('sum') does
not make sense here because it will return InvalidOid, which will not tell
us whether that is because there is no such function or whether there are
duplicate function names.
select * from pg_proc where oid = to_regprocedure('sum(int)');




The changes in parse_type.c look all good, except some cosmetic changes:

The parameters are not aligned one below the other for these two lines :
typenameTypeIdAndModMissingOk(ParseState *pstate, const TypeName *typeName,
 Oid *typeid_p, int32 *typmod_p)

Similar thing for typenameTypeIdAndMod_guts().


--

Please also add some testcases in the regression tests.



On 14 January 2014 12:58, Yugo Nagata nag...@sraoss.co.jp wrote:

 Here is the patch to implement to_regclass, to_regproc, to_regoper,
 and to_regtype. They are new functions similar to regclass, regproc,
 regoper, and regtype except that if requested object is not found,
 returns InvalidOid, rather than raises an error.

 On Tue, 31 Dec 2013 12:10:56 +0100
 Pavel Stehule pavel.steh...@gmail.com wrote:

  2013/12/31 Tatsuo Ishii is...@postgresql.org
 
On 12/31/2013 02:38 AM, Tatsuo Ishii wrote:
Before proceeding the work, I would like to make sure that
 followings
are complete list of new functions. Inside parentheses are
corresponding original functions.
   
toregproc (regproc)
toregoper (regoper)
toregclass (regclass)
toregtype (regtype)
   
Pardon the bikeshedding, but those are hard to read for me.  I would
prefer to go with the to_timestamp() model and add an underscore to
those names.
  
   I have no problem with adding to_. Objection?
  
 
  I like to_reg* too
 
  Regards
 
  Pavel
 
 
  
   Best regards,
   --
   Tatsuo Ishii
   SRA OSS, Inc. Japan
   English: http://www.sraoss.co.jp/index_en.php
   Japanese: http://www.sraoss.co.jp
  
  
   --
   Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
   To make changes to your subscription:
   http://www.postgresql.org/mailpref/pgsql-hackers
  


 --
 Yugo Nagata nag...@sraoss.co.jp


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




[HACKERS] NOT Null constraint on foreign table not working

2014-01-19 Thread Rushabh Lathia
Hello,

Please consider the following test:

create database foo;
\c foo

create table foo_test ( a int );

\c postgres

create extension if not exists postgres_fdw;
create server foo_server foreign data wrapper postgres_fdw options ( dbname
 'foo' );
create user mapping for current_user server foo_server;

create foreign table foo_test ( a int not null) server foo_server;

-- insert should return error for because NOT NULL constraint on column a
postgres=# insert into foo_test values ( null );
INSERT 0 1

postgres=# select * from foo_test;
 a
---

(1 row)

-- clean up
drop foreign table foo_test;
drop server foo_server cascade;
\c postgres
drop database foo;


Analysis:

As per the PG documentation it says that foreign table do support the
NOT NULL, NULL and DEFAULT.

http://www.postgresql.org/docs/devel/static/sql-createforeigntable.html

But when I tried the NOT NULL constraint, its not working for the foreign
tables.
Looking at the code into ExecInsert(), for the foreign table missed to call
ExecConstraints(). I am not sure whether it is intentional that  we not
calling
ExecConstraints() in case of foreign server or its missed.
Do share your thought on this.

I quickly fix the issue by adding ExecConstraints() call for foreign table
and
now test behaving as expected. PFA patch for the same.

Regards,
Rushabh Lathia
www.EnterpriseDB.com
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 6f0f47e..240126c 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -229,6 +229,12 @@ ExecInsert(TupleTableSlot *slot,
 	else if (resultRelInfo-ri_FdwRoutine)
 	{
 		/*
+		 * Check the constraints of the tuple
+		 */
+		if (resultRelationDesc-rd_att-constr)
+			ExecConstraints(resultRelInfo, slot, estate);
+
+		/*
 		 * insert into foreign table: let the FDW do it
 		 */
 		slot = resultRelInfo-ri_FdwRoutine-ExecForeignInsert(estate,

-- 
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] using rpmbuild with PostgreSQL 9.2.6 source code

2014-01-19 Thread Pavel Stehule
Hello

you need installed devel packages

Regards

Pavel Stehule


2014/1/20 Sameer Kumar sameer.ku...@ashnik.com

 Hi,

 I have downloaded the tar source code of PostgreSQL and also the SPEC
 file. I am trying to use rpmbuild command but I always get below error:

 error: Failed build dependencies:
 uuid-devel is needed by postgresql92-9.2.6-1PGDG.el6.ppc64
 selinux-policy = 3.9.13 is needed by
 postgresql92-9.2.6-1PGDG.el6.ppc64
 systemd-units is needed by postgresql92-9.2.6-1PGDG.el6.ppc64


 I have attached the list of installed packages on my server.

 Best Regards,
 *Sameer Kumar | Database Consultant*

 *ASHNIK PTE. LTD. *101 Cecil Street, #11-11 Tong Eng Building, Singapore
 069533
 M : *+65 8110 0350 %2B65%208110%200350* T: +65 6438 3504 |
 www.ashnik.com
 www.facebook.com/ashnikbiz | www.twitter.com/ashnikbiz

 [image: email patch]

 This email may contain confidential, privileged or copyright material and
 is solely for the use of the intended recipient(s).


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


image002.jpg

Re: [HACKERS] NOT Null constraint on foreign table not working

2014-01-19 Thread Rushabh Lathia
Please consider attached patch here as earlier attached wrong patch.

Sorry for the inconvenience.


On Mon, Jan 20, 2014 at 1:21 PM, Rushabh Lathia rushabh.lat...@gmail.comwrote:

 Hello,

 Please consider the following test:

 create database foo;
 \c foo

 create table foo_test ( a int );

 \c postgres

 create extension if not exists postgres_fdw;
 create server foo_server foreign data wrapper postgres_fdw options (
 dbname  'foo' );
 create user mapping for current_user server foo_server;

 create foreign table foo_test ( a int not null) server foo_server;

 -- insert should return error for because NOT NULL constraint on column a
 postgres=# insert into foo_test values ( null );
 INSERT 0 1

 postgres=# select * from foo_test;
  a
 ---

 (1 row)

 -- clean up
 drop foreign table foo_test;
 drop server foo_server cascade;
 \c postgres
 drop database foo;


 Analysis:

 As per the PG documentation it says that foreign table do support the
 NOT NULL, NULL and DEFAULT.

 http://www.postgresql.org/docs/devel/static/sql-createforeigntable.html

 But when I tried the NOT NULL constraint, its not working for the foreign
 tables.
 Looking at the code into ExecInsert(), for the foreign table missed to call
 ExecConstraints(). I am not sure whether it is intentional that  we not
 calling
 ExecConstraints() in case of foreign server or its missed.
 Do share your thought on this.

 I quickly fix the issue by adding ExecConstraints() call for foreign table
 and
 now test behaving as expected. PFA patch for the same.

 Regards,
 Rushabh Lathia
 www.EnterpriseDB.com




-- 
Rushabh Lathia
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 6f0f47e..f745f5e 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -229,6 +229,12 @@ ExecInsert(TupleTableSlot *slot,
 	else if (resultRelInfo-ri_FdwRoutine)
 	{
 		/*
+		 * Check the constraints of the tuple
+		 */
+		if (resultRelationDesc-rd_att-constr)
+			ExecConstraints(resultRelInfo, slot, estate);
+
+		/*
 		 * insert into foreign table: let the FDW do it
 		 */
 		slot = resultRelInfo-ri_FdwRoutine-ExecForeignInsert(estate,
@@ -641,6 +647,9 @@ ExecUpdate(ItemPointer tupleid,
 	}
 	else if (resultRelInfo-ri_FdwRoutine)
 	{
+		if (resultRelationDesc-rd_att-constr)
+			ExecConstraints(resultRelInfo, slot, estate);
+
 		/*
 		 * update in foreign table: let the FDW do it
 		 */

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