Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
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)
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
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 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)
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)
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
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)
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)
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)
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
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
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
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)
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
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
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)
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
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
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/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
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
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
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?
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?
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
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
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
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
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?
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?
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
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
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
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
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)
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
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
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.
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
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
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
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
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
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