Re: [HACKERS] Aussie timezone database changes incoming

2014-10-05 Thread Jim Nasby

On 10/4/14, 3:25 PM, Bruce Momjian wrote:

On Sat, Oct  4, 2014 at 03:01:45PM -0500, Jim Nasby wrote:

On 10/4/14, 2:58 PM, Bruce Momjian wrote:

I've committed changes for this in advance of the upcoming 9.4beta3

release.  Hopefully, if this is seriously bad for anyone, we'll hear
about it from beta testers before it gets into any official back-branch
releases.

The changes for the Russian Federation timezones taking effect October
26 reinforces our need to get a new set of minor releases out soon.  In
fact, those storing future dates might already need those updates.

This is why I wish we had a data type that stored the timezone that was 
originally in effect. :/

Uh, if we stored the _offset_ that was in effect at the time of storage,

Oh heck no. You NEVER want to use offsets instead of real timezones (something 
that far too many programmers don't seem to understand).

it would actually be _worse_ because the new timezone database would not
adjust existing stored values.  If we stored the name of the time zone,
I am not sure how that would help us here.


If we stored the name of the timezone then updates to the TZ database would 
take effect when the data was read back. Of course that doesn't help with 
indexes, but at least you can reindex (and I don't think it'd be to hard to be 
more clever than a bulk reindex).

Aside from that, I don't like that we throw away information (namely, what 
timezone was used when the record was written). If we stored the timezone you 
could actually find that out when you read the data back.


--
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] pgbench throttling latency limit

2014-10-05 Thread Fabien COELHO


Hello Heikki,

Here are new patches, again the first one is just refactoring, and the second 
one contains this feature. I'm planning to commit the first one shortly, and 
the second one later after people have had a chance to look at it.


I looked at it. It looks ok, but for a few spurious spacing changes here 
and there. No big deal.


I tested it, everything I tested behaved as expected, so it is ok for me.

--
Fabien.


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


Re: [HACKERS] proposal for 9.5: monitoring lock time for slow queries

2014-10-05 Thread Gregory Smith

On 8/13/14, 5:14 AM, MauMau wrote:
I'm interested in b, too.  I was thinking of proposing a performance 
diagnostics feature like Oracle's wait events (V$SYSTEM_EVENT and 
V$SESSION_EVENT).  So, if you do this, I'd like to contribute to the 
functional design, code and doc review, and testing.


I already wrote up a design like this once: 
http://www.postgresql.org/message-id/509300f7.5000...@2ndquadrant.com


The main problem when I tried to code it was figuring out how to store 
the data.  When I wrote that, I thought I could just stuff it into a 
shared memory block the way pg_stat_statements did.  That didn't really 
work out.  I think it's manageable now because the Dynamic Shared Memory 
management written since then has the right sort of shape to do the job.


This is one of those jobs where I think the coding itself is the hard 
part, not the design nor the review.  What I really want is something 
that dumps this data into memory, then a second process that persists to 
disk in batches.  I think that's the only way we'll get high performance 
on reads while still saving enough data to be useful after a crash.


We're working on getting a few things in this area fully funded to dig 
into them harder.  The idea of designing for high-speed audit logs into 
memory and then persisting to disk has a lot of overlap with this one 
too, and that may get picked up too.


--
Greg Smith greg.sm...@crunchydatasolutions.com
Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] Add generate_series(numeric, numeric)

2014-10-05 Thread Ali Akbar
Hi
Oops, it seems that I have been too hasty here. With a fresh mind I looked
at my own patch again and found two bugs:


 - Incorrect calculation of each step's value, making stuff crash, it is
 necessary to switch to the context of the function to perform operations on
 a temporary variable first


- i think you can use the fctx-current variable without temporary variable
(there's comment in the add_var function: Full version of add functionality
on variable level (handling signs). result might point to one of the
operands too without danger.). But you _must_ switch the context first
because add_var will allocate new array for the data and freeing the old
one.

- numeric can be NaN. We must reject it as first, finish and last parameter.
- numeric datatype is large, but there are limitations. According to doc,
the limit is: up to 131072 digits before the decimal point; up to 16383
digits after the decimal point. How can we check if the next step
overflows? As a comparison, in int.c, generate_series_step_int4 checks if
its overflows, and stop the next call by setting step to 0. Should we do
that?

~ will try to fix the patch later

-- 
Ali Akbar


[HACKERS] RLS with check option - surprised design

2014-10-05 Thread Pavel Stehule
Hello

I am playing with RLS. I created simple table

table_data (inserted_by text, v integer);

I created two policies

create policy p1 on data with check (inserted_by = session_user);
create policy p2 on data with check (v between 10 and 1000);

I was surprised so p2 effectively disables p1;

next a message:

ERROR:  new row violates WITH CHECK OPTION for data
DETAIL:  Failing row contains (2014-10-05 12:28:30.79652, petr, 1000).

Doesn't inform about broken policy.

Regards

Pavel


Re: [HACKERS] Add generate_series(numeric, numeric)

2014-10-05 Thread Ali Akbar
2014-10-05 15:21 GMT+07:00 Ali Akbar the.ap...@gmail.com:

 Hi
 Oops, it seems that I have been too hasty here. With a fresh mind I looked
 at my own patch again and found two bugs:


 - Incorrect calculation of each step's value, making stuff crash, it is
 necessary to switch to the context of the function to perform operations on
 a temporary variable first


 - i think you can use the fctx-current variable without temporary
 variable (there's comment in the add_var function: Full version of add
 functionality on variable level (handling signs). result might point to one
 of the operands too without danger.). But you _must_ switch the context
 first because add_var will allocate new array for the data and freeing the
 old one.

 - numeric can be NaN. We must reject it as first, finish and last
 parameter.
 - numeric datatype is large, but there are limitations. According to doc,
 the limit is: up to 131072 digits before the decimal point; up to 16383
 digits after the decimal point. How can we check if the next step
 overflows? As a comparison, in int.c, generate_series_step_int4 checks if
 its overflows, and stop the next call by setting step to 0. Should we do
 that?

 ~ will try to fix the patch later

attached the patch. Not checking if it overflows, but testing it with 9 *
10 ^ 131072 works (not resulting in an error). Other modifications:
- doc update
- regresssion tests
- while testing regression test, opr_sanity checks that the
generate_series_numeric function is used twice (once for 2 parameter and
once for the 3 parameter function), so i changed the name to
generate_series_step_numeric and created new function
generate_series_numeric that calls generate_series_step_numeric

-- 
Ali Akbar
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14074,14081  AND
  tbody
   row
entryliteralfunctiongenerate_series(parameterstart/parameter, parameterstop/parameter)/function/literal/entry
!   entrytypeint/type or typebigint/type/entry
!   entrytypesetof int/type or typesetof bigint/type (same as argument type)/entry
entry
 Generate a series of values, from parameterstart/parameter to parameterstop/parameter
 with a step size of one
--- 14074,14081 
  tbody
   row
entryliteralfunctiongenerate_series(parameterstart/parameter, parameterstop/parameter)/function/literal/entry
!   entrytypeint/type, typebigint/type or typenumeric/type/entry
!   entrytypesetof int/type, typesetof bigint/type, or typesetof numeric/type (same as argument type)/entry
entry
 Generate a series of values, from parameterstart/parameter to parameterstop/parameter
 with a step size of one
***
*** 14084,14091  AND
  
   row
entryliteralfunctiongenerate_series(parameterstart/parameter, parameterstop/parameter, parameterstep/parameter)/function/literal/entry
!   entrytypeint/type or typebigint/type/entry
!   entrytypesetof int/type or typesetof bigint/type (same as argument type)/entry
entry
 Generate a series of values, from parameterstart/parameter to parameterstop/parameter
 with a step size of parameterstep/parameter
--- 14084,14091 
  
   row
entryliteralfunctiongenerate_series(parameterstart/parameter, parameterstop/parameter, parameterstep/parameter)/function/literal/entry
!   entrytypeint/type, typebigint/type or typenumeric/type/entry
!   entrytypesetof int/type, typesetof bigint/type or typesetof numeric/type (same as argument type)/entry
entry
 Generate a series of values, from parameterstart/parameter to parameterstop/parameter
 with a step size of parameterstep/parameter
*** a/src/backend/utils/adt/numeric.c
--- b/src/backend/utils/adt/numeric.c
***
*** 28,33 
--- 28,34 
  
  #include access/hash.h
  #include catalog/pg_type.h
+ #include funcapi.h
  #include libpq/pqformat.h
  #include miscadmin.h
  #include nodes/nodeFuncs.h
***
*** 261,266  typedef struct NumericVar
--- 262,279 
  
  
  /* --
+  * Data for generate series
+  * --
+  */
+ typedef struct
+ {
+ 	NumericVar	current;
+ 	NumericVar	finish;
+ 	NumericVar	step;
+ } generate_series_numeric_fctx;
+ 
+ 
+ /* --
   * Some preinitialized constants
   * --
   */
***
*** 1221,1226  numeric_floor(PG_FUNCTION_ARGS)
--- 1234,1346 
  	PG_RETURN_NUMERIC(res);
  }
  
+ 
+ /*
+  * generate_series_numeric() -
+  *
+  *  Generate series of numeric.
+  */
+ Datum
+ generate_series_numeric(PG_FUNCTION_ARGS) {
+ 	return generate_series_step_numeric(fcinfo);
+ }
+ 
+ Datum
+ generate_series_step_numeric(PG_FUNCTION_ARGS)
+ {
+ 	generate_series_numeric_fctx *fctx;
+ 	FuncCallContext *funcctx;
+ 	MemoryContext oldcontext;
+ 
+ 	if (SRF_IS_FIRSTCALL())
+ 	{
+ 		Numeric		start_num = PG_GETARG_NUMERIC(0);
+ 		Numeric		finish_num = PG_GETARG_NUMERIC(1);
+ 		NumericVar	

Re: [HACKERS] Add generate_series(numeric, numeric)

2014-10-05 Thread Ali Akbar
Also, Платон Малюгин, can you add this patch to postgresql commitfest (
http://commitfest.postgresql.org)?

--
Ali Akbar


Re: [HACKERS] RLS with check option - surprised design

2014-10-05 Thread Stephen Frost
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I am playing with RLS. I created simple table
 
 table_data (inserted_by text, v integer);
 
 I created two policies
 
 create policy p1 on data with check (inserted_by = session_user);
 create policy p2 on data with check (v between 10 and 1000);
 
 I was surprised so p2 effectively disables p1;

It doesn't disable it at all- both are applied using OR, as documented
and discussed extensively earlier this year..

I'm not against revisiting that and there has been suggestions about
providing a 'RESTRICTED' policy type which AND's them together, but I
hope it isn't surprising to anyone who has looked at the documentation..
You might also have a policy which applies to all roles and then a more
permissive policy for an 'admin' type of user- look at the Unix passwd
example outlined in the documentation.

 next a message:
 
 ERROR:  new row violates WITH CHECK OPTION for data
 DETAIL:  Failing row contains (2014-10-05 12:28:30.79652, petr, 1000).
 
 Doesn't inform about broken policy.

I'm guessing this is referring to the above policies and so my comments
there apply..  One thing to note about this is that there is an active
discussion about removing the 'DETAIL' part of that error message as it
may be an information leak.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] CREATE IF NOT EXISTS INDEX

2014-10-05 Thread Marti Raudsepp
On Fri, Oct 3, 2014 at 7:25 PM, Fabrízio de Royes Mello
fabriziome...@gmail.com wrote:
 I agree with your grammar change.

Cool.

 The version 5 (attached) contains all discussed until now.

From documentation:

CREATE INDEX ... [ IF NOT EXISTS name | name ] ON ...

Maybe I'm just slow, but it took me a few minutes to understand what
this means. :)

I would add a human-language explanation to IF NOT EXISTS description:
  Index name is required when IF NOT EXISTS is specified


You have resurrected this bit again, which now conflicts with git master...

- write_msg(NULL, reading row-security enabled for table \%s\,
+ write_msg(NULL, reading row-security enabled for table \%s\\n,


  n-concurrent = $4;
+ n-if_not_exists = false;
  n-idxname = $5;

Minor stylistic thing: now that this is a constant, I would move it to
the end together with other constant assignments, and follow the
struct's field ordering (in both code paths):

n-isconstraint = false;
n-deferrable = false;
n-initdeferred = false;
n-if_not_exists = false;

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] RLS with check option - surprised design

2014-10-05 Thread Pavel Stehule
2014-10-05 14:16 GMT+02:00 Stephen Frost sfr...@snowman.net:

 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
  I am playing with RLS. I created simple table
 
  table_data (inserted_by text, v integer);
 
  I created two policies
 
  create policy p1 on data with check (inserted_by = session_user);
  create policy p2 on data with check (v between 10 and 1000);
 
  I was surprised so p2 effectively disables p1;

 It doesn't disable it at all- both are applied using OR, as documented
 and discussed extensively earlier this year..


I didn't watch a discussion about RLS this year.

Please, can you show me some use case, where OR has bigger sense than AND?

Thank you

Pavel


 I'm not against revisiting that and there has been suggestions about
 providing a 'RESTRICTED' policy type which AND's them together, but I
 hope it isn't surprising to anyone who has looked at the documentation..
 You might also have a policy which applies to all roles and then a more
 permissive policy for an 'admin' type of user- look at the Unix passwd
 example outlined in the documentation.

  next a message:
 
  ERROR:  new row violates WITH CHECK OPTION for data
  DETAIL:  Failing row contains (2014-10-05 12:28:30.79652, petr, 1000).
 
  Doesn't inform about broken policy.

 I'm guessing this is referring to the above policies and so my comments
 there apply..  One thing to note about this is that there is an active
 discussion about removing the 'DETAIL' part of that error message as it
 may be an information leak.

 Thanks,

 Stephen



Re: [HACKERS] RLS with check option - surprised design

2014-10-05 Thread Stephen Frost
Pavel,

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 Please, can you show me some use case, where OR has bigger sense than AND?
[...]
  You might also have a policy which applies to all roles and then a more
  permissive policy for an 'admin' type of user- look at the Unix passwd
  example outlined in the documentation.

Thanks,

Stephen


signature.asc
Description: Digital signature


[HACKERS] Proposal for better support of time-varying timezone abbreviations

2014-10-05 Thread Tom Lane
I got interested in the problem discussed in
http://www.postgresql.org/message-id/20714.1412456...@sss.pgh.pa.us
to wit:

 It's becoming clear to me that our existing design whereby zone
 abbreviations represent fixed GMT offsets isn't really good enough.
 I've been wondering whether we could change things so that, for instance,
 EDT means daylight time according to America/New_York and the system
 would consult the zic database to find out what the prevailing GMT offset
 was in that zone on that date.  This would be a lot more robust in the
 face of the kind of foolishness we now see actually goes on.

Here is a fairly detailed design sketch for a solution:

1. Allow tznames entries to consist of an abbreviation and the name of
a zic timezone, for example

MSK Europe/Moscow

instead of the current scheme whereby an abbreviation is defined by a
daylight-savings flag and a numeric GMT offset.  When an abbreviation is
defined this way, the implied offset and DST flag are looked up
dynamically as described below.  (In my message quoted above, I'd imagined
that we'd write a DST flag and a zone name, but it turns out this does not
work because there are cases where the DST property has changed over time.
Yes, really.  So this design mandates that we derive the DST flag by
looking into the zic timezone data.)  Note that we'll still allow the old
style of entries, and indeed prefer that way for cases where an
abbreviation has never changed meaning, because:

* We need that anyway for forwards compatibility of existing custom
abbreviations files.

* It's a lot cheaper to interpret a fixed-meaning zone abbreviation using
the existing logic than to do it as I propose here, so we shouldn't spend
the extra cycles unless necessary.

* Converting every one of the existing abbreviation-file entries would be
really tedious, so I don't want to do it where not necessary.

Also note that this doesn't touch the aspect of the existing design
whereby there are multiple potential abbreviations files.  We still have
the problem that the same abbreviation can be in use in different
timezones, so we have to let users configure which zone they mean by a
given abbreviation.

2. To interpret such an abbreviation in the context of timestamptz input,
look up the referenced zic timezone, and use the meaning of the
abbreviation that prevailed at or most recently before the local time
indicated by the rest of the timestamptz string.  If the abbreviation was
never used before that time in the given zone, use its earliest later
interpretation; or if it was never used at all (ie bad configuration file)
throw error.  Note that this is different from what happens if you give
the underlying zone name directly.  It's always been the case that you
could say, for instance, EST to force interpretation of a datetime as
standard time even when DST is in force, or EDT to force the opposite
interpretation, and this definition preserves that behavior.

3. In the context of timetz input, we only have a time of day not a full
datetime to look at, so it's not entirely clear what to do.  We could
throw an error, but that would result in rejecting some inputs currently
considered valid.  Perhaps we don't really care, since we consider timetz
a deprecated type anyway.  If that doesn't seem OK, we could assume
today's date and the given time-of-day and look up the abbreviation's
meaning as described above.  This would mean that the meaning of, say,
'15:00 MSK'::timetz would change over time --- but that happens now,
whenever we change the contents of the abbreviations file entry for MSK,
so maybe this isn't as horrid as it sounds.

4. I've eyeballed the relevant code a bit, and it seems that the only
implementation aspect that isn't perfectly straightforward is figuring
out how to cram a zic timezone reference into a datetkn table entry.
I suggest that before tackling this feature proper, we bring struct
datetkn into the 21st century by widening it from 12 to 16 bytes, along
the lines of

typedef struct
{
chartoken[TOKMAXLEN + 1];  /* now always null-terminated */
chartype;
int32   value;
} datetkn;

and getting rid of all of the very crufty code that deals with
non-null-terminated token strings and cramming values that don't really
fit into a char-sized field into value.  (We might save more code bytes
that way than we spend on the wider token-table entries :-( ... and we'll
certainly make the code less ugly.)  Having done that, the value can be
large enough to be an index into additional storage appended to a
TimeZoneAbbrevTable.  I imagine it pointing at a struct like this:

struct DynamicTimeZoneAbbrev
{
const pg_tz *tz;  /* zic timezone, or NULL if not yet looked up */
charname[1];  /* zone name (variable length string)
};

We'd resolve the timezone name into a pg_tz pointer only upon first use of
a dynamic abbreviation, since we don't want to force loading of every zone
referenced in the 

Re: [HACKERS] Escaping from blocked send() reprised.

2014-10-05 Thread Andres Freund
On 2014-10-03 18:29:23 +0300, Heikki Linnakangas wrote:
 On 10/03/2014 05:26 PM, Andres Freund wrote:
 On 2014-10-03 17:12:18 +0300, Heikki Linnakangas wrote:
 On 09/28/2014 01:54 AM, Andres Freund wrote:
 0003 Sinval/notify processing got simplified further. There really isn't
   any need for DisableNotifyInterrupt/DisableCatchupInterrupt
   anymore. Also begin_client_read/client_read_ended don't make much
   sense anymore. Instead introduce ProcessClientReadInterrupt (which
   wants a better name).
 There's also a very WIP
 0004 Allows secure_read/write be interrupted when ProcDiePending is
   set. All of that happens via the latch mechanism, nothing happens
   inside signal handlers. So I do think it's quite an improvement
   over what's been discussed in this thread.
   But it (and the other approaches) do noticeably increase the
   likelihood of clients not getting the error message if the client
   isn't actually dead. The likelihood of write() being blocked
   *temporarily* due to normal bandwidth constraints is quite high
   when you consider COPY FROM and similar. Right now we'll wait till
   we can put the error message into the socket afaics.
 
 1-3 need some serious comment work, but I think the approach is
 basically sound. I'm much, much less sure about allowing send() to be
 interrupted.
 
 Yeah, 1-3 seem sane.
 
 I think 3 also needs a careful look. Have you looked through it? While
 imo much less complex than before, there's some complex interactions in
 the touched code. And we have terrible coverage of both catchup
 interrupts and notify stuff...
 
 I only looked at the .patch, I didn't apply it, so I didn't look at the
 context much. But I don't see any fundamental problem with it. I would like
 to have a closer look before it's committed, though.

I'd appreciate that. I don't want to commit it without a careful review
of another committer.

 There's also the concern that using a latch for client communication
 increases the number of syscalls for the same work. We should at least
 try to quantify that...
 
 I'm not too concerned about that, since we only do extra syscalls when the
 socket isn't immediately available for reading/writing, i.e. when we have to
 sleep anyway.

Well, kernels actually do some nice optimizations for blocking reads -
at least for local sockets. Like switching to the other process
immediately and such.
I'm not super concerned either, but I think we should try to measure
it. And if we're failing, we probably should try to address these
problems - if possible in the latch code.

 4 also looks OK to me at a quick glance. It basically
 enables handling the die interrupt immediately, if we're blocked in a read
 or write. It won't be handled in the signal handler, but within the
 secure_read/write call anyway.
 
 What are you thinking about the concern that it'll reduce the likelihood
 of transferring the error message to the client? I tried to reduce that
 by only allowing errors when write() blocks, but that's not an
 infrequent event.
 
 I'm not too concerned about that either. I mean, it's probably true that it
 reduces the likelihood, but I don't particularly care myself. But if we
 care, we could use a timeout there, so that if we receive a SIGTERM while
 blocked on a send(), we wait for a few seconds to see if we can send
 whatever we were sending, before terminating the backend.
 
 
 What should we do with this patch in the commitfest?

I think feature should be ddeclare as 'returned with feedback' for this
commitfest. I've done so.

I don't see much of a reason waiting with patch 1 till the next
commitfest. It imo looks uncontroversial and doesn't have any far
reaching consequences.

 Are you planning to clean up and commit these patches?

I plan to do so one by one, yes. If you'd like to pick up any of them,
feel free to do (after telling me, to avoid duplicated efforts). I don't
feel proprietary about any of them. But I guess you have other stuff
you'd like to work on too ;)

I'll try to send out a version with the stuff you mentioned earlier in
the next couple days.

Greetings,

Andres Freund

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


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


Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations

2014-10-05 Thread Gavin Flower

On 06/10/14 10:33, Tom Lane wrote:

I got interested in the problem discussed in
http://www.postgresql.org/message-id/20714.1412456...@sss.pgh.pa.us
to wit:


It's becoming clear to me that our existing design whereby zone
abbreviations represent fixed GMT offsets isn't really good enough.
I've been wondering whether we could change things so that, for instance,
EDT means daylight time according to America/New_York and the system
would consult the zic database to find out what the prevailing GMT offset
was in that zone on that date.  This would be a lot more robust in the
face of the kind of foolishness we now see actually goes on.

Here is a fairly detailed design sketch for a solution:

1. Allow tznames entries to consist of an abbreviation and the name of
a zic timezone, for example

MSK Europe/Moscow

instead of the current scheme whereby an abbreviation is defined by a
daylight-savings flag and a numeric GMT offset.  When an abbreviation is
defined this way, the implied offset and DST flag are looked up
dynamically as described below.  (In my message quoted above, I'd imagined
that we'd write a DST flag and a zone name, but it turns out this does not
work because there are cases where the DST property has changed over time.
Yes, really.  So this design mandates that we derive the DST flag by
looking into the zic timezone data.)  Note that we'll still allow the old
style of entries, and indeed prefer that way for cases where an
abbreviation has never changed meaning, because:

* We need that anyway for forwards compatibility of existing custom
abbreviations files.

* It's a lot cheaper to interpret a fixed-meaning zone abbreviation using
the existing logic than to do it as I propose here, so we shouldn't spend
the extra cycles unless necessary.

* Converting every one of the existing abbreviation-file entries would be
really tedious, so I don't want to do it where not necessary.

Also note that this doesn't touch the aspect of the existing design
whereby there are multiple potential abbreviations files.  We still have
the problem that the same abbreviation can be in use in different
timezones, so we have to let users configure which zone they mean by a
given abbreviation.

2. To interpret such an abbreviation in the context of timestamptz input,
look up the referenced zic timezone, and use the meaning of the
abbreviation that prevailed at or most recently before the local time
indicated by the rest of the timestamptz string.  If the abbreviation was
never used before that time in the given zone, use its earliest later
interpretation; or if it was never used at all (ie bad configuration file)
throw error.  Note that this is different from what happens if you give
the underlying zone name directly.  It's always been the case that you
could say, for instance, EST to force interpretation of a datetime as
standard time even when DST is in force, or EDT to force the opposite
interpretation, and this definition preserves that behavior.

3. In the context of timetz input, we only have a time of day not a full
datetime to look at, so it's not entirely clear what to do.  We could
throw an error, but that would result in rejecting some inputs currently
considered valid.  Perhaps we don't really care, since we consider timetz
a deprecated type anyway.  If that doesn't seem OK, we could assume
today's date and the given time-of-day and look up the abbreviation's
meaning as described above.  This would mean that the meaning of, say,
'15:00 MSK'::timetz would change over time --- but that happens now,
whenever we change the contents of the abbreviations file entry for MSK,
so maybe this isn't as horrid as it sounds.

4. I've eyeballed the relevant code a bit, and it seems that the only
implementation aspect that isn't perfectly straightforward is figuring
out how to cram a zic timezone reference into a datetkn table entry.
I suggest that before tackling this feature proper, we bring struct
datetkn into the 21st century by widening it from 12 to 16 bytes, along
the lines of

typedef struct
{
 chartoken[TOKMAXLEN + 1];  /* now always null-terminated */
 chartype;
 int32   value;
} datetkn;

and getting rid of all of the very crufty code that deals with
non-null-terminated token strings and cramming values that don't really
fit into a char-sized field into value.  (We might save more code bytes
that way than we spend on the wider token-table entries :-( ... and we'll
certainly make the code less ugly.)  Having done that, the value can be
large enough to be an index into additional storage appended to a
TimeZoneAbbrevTable.  I imagine it pointing at a struct like this:

struct DynamicTimeZoneAbbrev
{
 const pg_tz *tz;  /* zic timezone, or NULL if not yet looked up */
 charname[1];  /* zone name (variable length string)
};

We'd resolve the timezone name into a pg_tz pointer only upon first use of
a dynamic abbreviation, since we don't want to force loading 

Re: [HACKERS] Proposal for better support of time-varying timezone abbreviations

2014-10-05 Thread Tom Lane
Gavin Flower gavinflo...@archidevsys.co.nz writes:
 The use of an /as_at_date/ is far more problematic.  The idea relates to 
 how existing date/times should be treated with respect to the date/time 
 that a pg database is updated with new time zone data files.   In the 
 simplest form: there would be a function in pg that would return the 
 date/time a new time zone data file was entered into the system, so that 
 application software can manually correct when the stored GMT date/time 
 was stored incorrectly because the wring GMT offset was used due to the 
 updated time zone data files not being in place.  Alternatively, pg 
 could offer to do the correction in a one-off action at the time the new 
 zone data files were updated.

Right now there's basically no way to do something like that, since what
we store for timestamptz is just a UTC time instant, with no record of
what GMT offset was involved much less exactly how the offset was
specified in the input.  We'd probably have to (at least) double the
on-disk size of timestamptz values to record that ... which seems like a
mighty high price to pay to fix a corner case.  Not to mention that
nobody's going to be willing to break on-disk compatibility of timestamptz
for this.

In any case, my proposal is just about being able to correctly interpret
historical timezone abbreviations during input, not about changing what
we store as datetime values.

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] CREATE IF NOT EXISTS INDEX

2014-10-05 Thread Fabrízio de Royes Mello
On Sun, Oct 5, 2014 at 9:52 AM, Marti Raudsepp ma...@juffo.org wrote:

  The version 5 (attached) contains all discussed until now.

 From documentation:

 CREATE INDEX ... [ IF NOT EXISTS name | name ] ON ...

 Maybe I'm just slow, but it took me a few minutes to understand what
 this means. :)


Well, I try to show that IF NOT EXISTS require the name. Is this wrong?

Anyway I fixed that way:

CREATE INDEX ... [ IF NOT EXISTS [ name ] ] ON ...

Maybe is better than the last... what you think?


 I would add a human-language explanation to IF NOT EXISTS description:
   Index name is required when IF NOT EXISTS is specified


Ok.


 
 You have resurrected this bit again, which now conflicts with git
master...

 - write_msg(NULL, reading row-security enabled for table \%s\,
 + write_msg(NULL, reading row-security enabled for table \%s\\n,


Ohh... sorry... again... my mistake :-(  now all was fixed.


 
   n-concurrent = $4;
 + n-if_not_exists = false;
   n-idxname = $5;

 Minor stylistic thing: now that this is a constant, I would move it to
 the end together with other constant assignments, and follow the
 struct's field ordering (in both code paths):

 n-isconstraint = false;
 n-deferrable = false;
 n-initdeferred = false;
 n-if_not_exists = false;


Fixed.

Thanks again!

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index e469b17..9b3f3a3 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  refsynopsisdiv
 synopsis
-CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ replaceable class=parametername/replaceable ] ON replaceable class=parametertable_name/replaceable [ USING replaceable class=parametermethod/replaceable ]
+CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ IF NOT EXISTS [ replaceable class=parametername/replaceable ] ] ON replaceable class=parametertable_name/replaceable [ USING replaceable class=parametermethod/replaceable ]
 ( { replaceable class=parametercolumn_name/replaceable | ( replaceable class=parameterexpression/replaceable ) } [ COLLATE replaceable class=parametercollation/replaceable ] [ replaceable class=parameteropclass/replaceable ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [, ...] )
 [ WITH ( replaceable class=PARAMETERstorage_parameter/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) ]
 [ TABLESPACE replaceable class=parametertablespace_name/replaceable ]
@@ -99,6 +99,18 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ replaceable class=parametername/
 
 variablelist
  varlistentry
+  termliteralIF NOT EXISTS/literal/term
+  listitem
+   para
+Do not throw an error if the index already exists. A notice is issued
+in this case. Note that there is no guarantee that the existing index
+is anything like the one that would have been created.
+Index name is required when IF NOT EXISTS is specified.
+   /para
+  /listitem
+ /varlistentry
+
+ varlistentry
   termliteralUNIQUE/literal/term
   listitem
para
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ee10594..8905e30 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -697,7 +697,8 @@ index_create(Relation heapRelation,
 			 bool allow_system_table_mods,
 			 bool skip_build,
 			 bool concurrent,
-			 bool is_internal)
+			 bool is_internal,
+			 bool if_not_exists)
 {
 	Oid			heapRelationId = RelationGetRelid(heapRelation);
 	Relation	pg_class;
@@ -773,10 +774,22 @@ index_create(Relation heapRelation,
 		elog(ERROR, shared relations must be placed in pg_global tablespace);
 
 	if (get_relname_relid(indexRelationName, namespaceId))
+	{
+		if (if_not_exists)
+		{
+			ereport(NOTICE,
+	(errcode(ERRCODE_DUPLICATE_TABLE),
+	 errmsg(relation \%s\ already exists, skipping,
+			indexRelationName)));
+			heap_close(pg_class, RowExclusiveLock);
+			return InvalidOid;
+		}
+
 		ereport(ERROR,
 (errcode(ERRCODE_DUPLICATE_TABLE),
  errmsg(relation \%s\ already exists,
 		indexRelationName)));
+	}
 
 	/*
 	 * construct tuple descriptor for index tuples
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 160f006..5ef6dcc 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -342,7 +342,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
  rel-rd_rel-reltablespace,
  collationObjectId, classObjectId, coloptions, (Datum) 0,
  true, false, false, false,
- true, false, false, true);
+ true, false, false, true, false);
 
 	heap_close(toast_rel, NoLock);
 
diff --git 

[HACKERS] Feasibility of supporting bind params for all command types

2014-10-05 Thread Craig Ringer
Hi all

While looking at an unrelated issue in PgJDBC I noticed that it's
difficult for users and the driver to tell in advance if a given
statement will support bind parameters.

PostgreSQL just treats placeholders as syntax errors for non-plannable
statements at parse time.

This forces users to try to guess whether a given statement can be
parameterised or not, or forces drivers to guess this on behalf of users
and do client-side parameter substitution.

As a result, some code that worked with PgJDBC using the v2 protocol
will fail with the v3 protocol, e.g.

@Test
public void test() throws SQLException {
PGConnection pgc = (PGConnection)conn;
PreparedStatement ps = conn.prepareStatement(SET ROLE ?);
ps.setString(1, somebody);
ps.executeUpdate();
}

This works with the v2 protocol because PgJDBC does client side
parameter binding unless you request sever-side prepare (via SQL-level
PREPARE and EXECUTE).

With the v3 protocol it always uses the extended parse/bind/execute
flow, with unnamed statements.

(Another case where this is quite frustrating is COPY, though PgJDBC has
a wrapper API for COPY that helps cover that up.)

It'd be nice not to force users to do their own escaping of literals in
non-plannable statements. Before embarking on anything like this I
thought I'd check and see if anyone's looked into supporting bind
parameters in utility statements, or if not, if anyone has any ideas
about the feasibility of adding such support.

I didn't have much luck searching for discussion on the matter.


-- 
 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] WAL format and API changes (9.5)

2014-10-05 Thread Michael Paquier
On Fri, Oct 3, 2014 at 9:51 PM, Heikki Linnakangas hlinnakan...@vmware.com
wrote:
 So I now have a refactoring patch ready that I'd like to commit (the
attached two patches together), but to be honest, I have no idea why the
second patch is so essential to performance.
Thanks. I did some more tests with master, master+patch1, master+patch1+CRC
refactoring, but I am not able to see any performance difference with
pgbench (--no-vacuum, -t) and the test suite you provided, just some noise
that barely changed performance. Note that fd.c uses
SYNC_METHOD_FSYNC_WRITETHROUGH, so it is necessary to include xlog.h in it.
-- 
Michael


Re: [HACKERS] CREATE IF NOT EXISTS INDEX

2014-10-05 Thread Marti Raudsepp
On Mon, Oct 6, 2014 at 4:17 AM, Fabrízio de Royes Mello
fabriziome...@gmail.com wrote:
 On Sun, Oct 5, 2014 at 9:52 AM, Marti Raudsepp ma...@juffo.org wrote:
 CREATE INDEX ... [ IF NOT EXISTS name | name ] ON ...

 Maybe I'm just slow, but it took me a few minutes to understand what
 this means. :)

 Well, I try to show that IF NOT EXISTS require the name. Is this wrong?

No, I'm sorry, you misunderstood me. It was totally correct before, it
just wasn't easy to understand at first.

 CREATE INDEX ... [ IF NOT EXISTS [ name ] ] ON ...

I think this one is wrong now. It suggests these are valid syntaxes:
CREATE INDEX ... ON ...
CREATE INDEX ... IF NOT EXISTS ON ... -- wrong
CREATE INDEX ... IF NOT EXISTS name ON ...

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] Feasibility of supporting bind params for all command types

2014-10-05 Thread Tom Lane
Craig Ringer cr...@2ndquadrant.com writes:
 While looking at an unrelated issue in PgJDBC I noticed that it's
 difficult for users and the driver to tell in advance if a given
 statement will support bind parameters.

It's not that hard ;-) ... if it ain't SELECT/INSERT/UPDATE/DELETE,
it won't accept parameters.

 As a result, some code that worked with PgJDBC using the v2 protocol
 will fail with the v3 protocol, e.g.

 @Test
 public void test() throws SQLException {
   PGConnection pgc = (PGConnection)conn;
   PreparedStatement ps = conn.prepareStatement(SET ROLE ?);
   ps.setString(1, somebody);
   ps.executeUpdate();
 }

It's more or less accidental that that works, I think.  I assume that the
statement that actually gets sent to the server looks like

SET ROLE 'something'

which morally ought to be a syntax error: you'd expect the role name
to be an identifier (possibly double-quoted).  Not a singly-quoted string
literal.  We allow a string literal because for some weird reason the SQL
standard says so, but it still feels like a type violation.

 It'd be nice not to force users to do their own escaping of literals in
 non-plannable statements. Before embarking on anything like this I
 thought I'd check and see if anyone's looked into supporting bind
 parameters in utility statements, or if not, if anyone has any ideas
 about the feasibility of adding such support.

I think it might be desirable but it'd be a mess, both as to the
concept/definition and as to the implementation.  How would a parameter
placeholder substitute for an identifier --- for example, what type would
be reported by Describe?  What would you do about parameter placeholders
in expressions in DDL --- for example,

CREATE TABLE mytable (f1 int default ?+? );

Here, the placeholders surely don't represent identifiers, but the system
is going to have a hard time figuring out what datatype they *should*
represent.  Carrying that example a bit further, I wonder what the chances
are of doing something sane or useful with

CREATE TABLE ? (? ? default ?+? );

But if you want to punt on that, I think you just greatly weakened your
argument for the whole thing.

On the implementation side, I'm worried about how we make sure that
parameter placeholders get replaced in a DDL expression that would
normally *not* get evaluated immediately, like the DEFAULT expression 
above.

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] Failure with make check-world for pgtypeslib/dt_test2 with HEAD on OSX

2014-10-05 Thread Michael Paquier
Hi,

This morning while running make check-world on my OSX Mavericks laptop, I
found the following failure:
test pgtypeslib/dt_test2  ... stderr FAILED (test process was
terminated by signal 6: Abort trap)
(lldb) bt
* thread #1: tid = 0x, 0x7fff8052c866
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGSTOP
  * frame #0: 0x7fff8052c866 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x7fff83cb035c libsystem_pthread.dylib`pthread_kill + 92
frame #2: 0x7fff81899bba libsystem_c.dylib`__abort + 145
frame #3: 0x7fff8189a46d libsystem_c.dylib`__stack_chk_fail + 196
frame #4: 0x00010f7cb3bb
libpgtypes.3.dylib`PGTYPESdate_from_asc(str=0x00010f6a2d6c,
endptr=0x7fff5055e488) + 635 at datetime.c:104
frame #5: 0x00010f6a260f dt_test2`main + 255 at dt_test2.pgc:91
frame #6: 0x7fff87acc5fd libdyld.dylib`start + 1
frame #7: 0x7fff87acc5fd libdyld.dylib`start + 1
Bisecting is showing me that this failure has been introduced by 4318dae,
and is reproducible on all the active branches, down to REL9_0_STABLE.

Note that this problem has been introduced after discussing a separate
issue here:
http://www.postgresql.org/message-id/1399399313.27807.28.camel@sussancws0025
Regards,
-- 
Michael


Re: [HACKERS] Failure with make check-world for pgtypeslib/dt_test2 with HEAD on OSX

2014-10-05 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes:
 This morning while running make check-world on my OSX Mavericks laptop, I
 found the following failure:

[ scratches head... ]  Doesn't reproduce on my OSX Mavericks laptop,
either with or without --disable-integer-datetimes.  What compiler
are you using exactly?  Any special build options?

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