Re: [HACKERS] psql: Activate pager only for height, not width

[Brendan Jurd]

On Tue, 30 May 2017 at 05:30 Christoph Berg  wrote:

> Oh interesting, I didn't know about pager_min_lines. That sounds
> useful as well. +1 on the analogous pager_min_cols option.
>

On closer inspection, I note that psql already has a 'columns' \pset
option, which does control the width for triggering the pager, but also
controls the width for wrapping and auto-expanded mode purposes.  So, I can
set 'columns' to get the pager behaviour that I want, but I also get
unwanted effects where I'd rather let the default (terminal width) apply.

So if we were to add a 'pager_min_cols', we'd have to decide how it
interacts with 'columns'.  For example, to determine whether to trigger the
pager, we look for 'pager_min_cols' first, and if that is not set, then
fall back to 'columns'.  For all other width purposes, 'columns' would
continue to apply as present.

However, my feeling is that this is becoming a bit too fiddly.  If
'columns' did not already exist then 'pager_min_cols' would make more
sense, but as it does already exist, my preference is to leave 'columns'
as-is, and go for a height-only option to 'pager' instead.

Thoughts?

Cheers,
BJ

[HACKERS] psql: Activate pager only for height, not width

[Brendan Jurd]

Hello hackers,

I am often frustrated by the default behaviour of the psql pager, which
will activate a pager if the output is deemed to be "too wide" for the
terminal, regardless of the number of lines output, and of the
pager_min_lines setting.

This behaviour is sometimes desirable, but in my use patterns it is more
often the case that I want the pager to activate for output longer than
terminal height, whereas for output a little wider than the terminal, I am
happy for there to be some wrapping.  This is especially the case with "\d"
output for tables, where, at 80 columns, very often the only wrapping is in
the table borders and constraint/trigger definitions.

Usually I turn the pager off completely, and only switch it on when I am
about to execute something that will return many rows, but what I'd really
like is some way to tell psql to activate the pager as normal for height,
but to ignore width.  My first thought was an alternate mode to \pset pager
-- to {'on' | 'off' | 'always'} we could add 'height'.

Another option is to add the ability to specify the number of columns which
psql considers "too wide", analogous to pager_min_lines.  I could then set
pager_min_cols to something around 150 which would work nicely for my
situation.

I don't have strong opinions about how the options are constructed, as long
as it is possible to obtain the behaviour.

I would be happy to produce a patch, if this seems like an acceptable
feature add.

Regards,
BJ

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

[Brendan Jurd]

As an extra data point, if you try this in Python (psycopg2) you get an
exception:

psycopg2.ProgrammingError: autocommit cannot be used inside a transaction

I think this exception is a legitimate response.  If the user switches on
autocommit mode inside a transaction, it was most likely not on purpose.
Chances are, they didn't realise autocommit was off in the first place.

Even if we assume that it was done deliberately, it's difficult to know
exactly what the user intended.  It seems to hinge on a subtlety of what
the user understands autocommit mode to mean -- either "issue an implicit
COMMIT after each statement", or "ensure there is never an open
transaction".

I feel that raising an error is a sane move here -- it is reasonable to
insist that the user make their intention unambiguous.

Cheers,
BJ



On Sat, 6 Aug 2016 at 15:30 Amit Kapila  wrote:

> On Thu, Aug 4, 2016 at 7:46 PM, Pavel Stehule 
> wrote:
>
> > 2016-08-04 15:37 GMT+02:00 Amit Kapila :
> >>
> >> > I dislike automatic commit or rollback here.
> >> >
> >>
> >> What problem you see with it, if we do so and may be mention the same
> >> in docs as well.  Anyway, I think we should make the behaviour of both
> >> ecpg and psql same.
> >
> >
> > Implicit COMMIT can be dangerous
> >
>
> Not, when user has specifically requested for autocommit mode as 'on'.
> I think here what would be more meaningful is that after "Set
> AutoCommit On", when the first command is committed, it should commit
> previous non-pending committed commands as well.
>
> >>
> >> Not sure what benefit we will get by raising warning.  I think it is
> >> better to choose one behaviour (automatic commit or leave the
> >> transaction open as is currently being done in psql) and make it
> >> consistent across all clients.
> >
> >
> > I am not sure about value of ecpg for this case. It is used by 0.0001%
> > users. Probably nobody in Czech Republic knows this client.
> >
>
> Sure, but that doesn't give us the license for being inconsistent in
> behaviour across different clients.
>
> > Warnings enforce the user do some decision
> >
>
> They could be annoying as well, especially if that happens in scripts.
>
>
> --
> 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] Relax requirement for INTO with SELECT in pl/pgsql

[Brendan Jurd]

On Tue, 22 Mar 2016 at 10:34 Robert Haas  wrote:

> Yeah, I think requiring PERFORM is stupid and annoying.  +1 for
> letting people write a SELECT with no target.
>

Apologies for being late on the thread, but another +1 from me.  I've often
been frustrated by the dissonance of PERFORM against SQL.

Cheers,
BJ

Re: [HACKERS] proposal: make NOTIFY list de-duplication optional

[Brendan Jurd]

On Sat, 6 Feb 2016 at 12:50 Tom Lane  wrote:

> Robert Haas  writes:
> > I agree with what Merlin said about this:
> >
> http://www.postgresql.org/message-id/CAHyXU0yoHe8Qc=yc10ahu1nfia1tbhsg+35ds-oeueuapo7...@mail.gmail.com
>
> Yeah, I agree that a GUC for this is quite unappetizing.
>

How would you feel about a variant for calling NOTIFY?

The SQL syntax could be something like "NOTIFY [ALL] channel, payload"
where the ALL means "just send the notification already, nobody cares
whether there's an identical one in the queue".

Likewise we could introduce a three-argument form of pg_notify(text, text,
bool) where the final argument is whether you are interested in removing
duplicates.

Optimising the remove-duplicates path is still probably a worthwhile
endeavour, but if the user really doesn't care at all about duplication, it
seems silly to force them to pay any performance price for a behaviour they
didn't want, no?

Cheers,
BJ

Re: [HACKERS] psql: add \pset true/false

[Brendan Jurd]

On Fri, 30 Oct 2015 at 00:51 Tom Lane  wrote:

> The really key argument that hasn't been addressed here is why does such
> a behavior belong in psql, rather than elsewhere?  Surely legibility
> problems aren't unique to psql users.  Moreover, there are exactly
> parallel facilities for other datatypes on the server side: think
> DateStyle or bytea_output.  So if you were trying to follow precedent
> rather than invent a kluge, you'd have submitted a patch to create a GUC
> that changes the output of boolout().
>

I find Tom's analogy to datestyle and bytea_output convincing.

+1 for a GUC that changes the behaviour of boolout.

And also +1 for doing anything at all to improve on the t/f output.  Those
glyphs are way too similar to each other.

I think U+2713 and U+2717 (✓ and ✗) are the obvious choices for a boolean,
but if we have a GUC we can set this to taste.

Cheers,
BJ

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

On Thu, 16 Jul 2015 at 08:37 Gurjeet Singh gurj...@singh.im wrote:

 OK. Please send a new patch with the changes you agree to, and I can mark
 it ready for committer.


Done.  Please find attached patch v3.  I have changed proportion to
fraction, and made other wording improvements per your suggestions.

Cheers,
BJ
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14806,14811  SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
--- 14806,14817 
/row
  
row
+entryliteralfunctionpg_notification_queue_usage()/function/literal/entry
+entrytypedouble/type/entry
+entryfraction of the asynchronous notification queue currently occupied (0-1)/entry
+   /row
+ 
+   row
 entryliteralfunctionpg_my_temp_schema()/function/literal/entry
 entrytypeoid/type/entry
 entryOID of session's temporary schema, or 0 if none/entry
***
*** 14945,14954  SET search_path TO replaceableschema/ optional, replaceableschema/, ..
  primarypg_listening_channels/primary
 /indexterm
  
 para
  functionpg_listening_channels/function returns a set of names of
! channels that the current session is listening to.  See xref
! linkend=sql-listen for more information.
 /para
  
 indexterm
--- 14951,14969 
  primarypg_listening_channels/primary
 /indexterm
  
+indexterm
+ primarypg_notification_queue_usage/primary
+/indexterm
+ 
 para
  functionpg_listening_channels/function returns a set of names of
! asynchronous notification channels that the current session is listening
! to.  functionpg_notification_queue_usage/function returns the
! fraction of the total available space for notifications currently
! occupied by notifications that are waiting to be processed, as a
! typedouble/type in the range 0-1.
! See xref linkend=sql-listen and xref linkend=sql-notify
! for more information.
 /para
  
 indexterm
*** a/doc/src/sgml/ref/notify.sgml
--- b/doc/src/sgml/ref/notify.sgml
***
*** 166,171  NOTIFY replaceable class=PARAMETERchannel/replaceable [ , replaceable cla
--- 166,176 
 current transaction so that cleanup can proceed.
/para
para
+The function functionpg_notification_queue_usage/function returns the
+proportion of the queue that is currently occupied by pending notifications.
+See xref linkend=functions-info for more information.
+   /para
+   para
 A transaction that has executed commandNOTIFY/command cannot be
 prepared for two-phase commit.
/para
*** a/src/backend/commands/async.c
--- b/src/backend/commands/async.c
***
*** 371,376  static bool asyncQueueIsFull(void);
--- 371,377 
  static bool asyncQueueAdvance(volatile QueuePosition *position, int entryLength);
  static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe);
  static ListCell *asyncQueueAddEntries(ListCell *nextNotify);
+ static double asyncQueueUsage(void);
  static void asyncQueueFillWarning(void);
  static bool SignalBackends(void);
  static void asyncQueueReadAllNotifications(void);
***
*** 1362,1387  asyncQueueAddEntries(ListCell *nextNotify)
  }
  
  /*
!  * Check whether the queue is at least half full, and emit a warning if so.
!  *
!  * This is unlikely given the size of the queue, but possible.
!  * The warnings show up at most once every QUEUE_FULL_WARN_INTERVAL.
   *
!  * Caller must hold exclusive AsyncQueueLock.
   */
! static void
! asyncQueueFillWarning(void)
  {
! 	int			headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
! 	int			tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
! 	int			occupied;
! 	double		fillDegree;
! 	TimestampTz t;
  
  	occupied = headPage - tailPage;
  
  	if (occupied == 0)
! 		return;	/* fast exit for common case */
  
  	if (occupied  0)
  	{
--- 1363,1399 
  }
  
  /*
!  * SQL function to return the fraction of the notification queue currently
!  * occupied.
!  */
! Datum
! pg_notification_queue_usage(PG_FUNCTION_ARGS)
! {
! 	double usage;
! 
! 	LWLockAcquire(AsyncQueueLock, LW_SHARED);
! 	usage = asyncQueueUsage();
! 	LWLockRelease(AsyncQueueLock);
! 
! 	PG_RETURN_FLOAT8(usage);
! }
! 
! /*
!  * Return the fraction of the queue that is currently occupied.
   *
!  * The caller must hold AysncQueueLock in (at least) shared mode.
   */
! static double
! asyncQueueUsage(void)
  {
! 	int		headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
! 	int		tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
! 	int		occupied;
  
  	occupied = headPage - tailPage;
  
  	if (occupied == 0)
! 		return (double) 0;		/* fast exit for common case */
  
  	if (occupied  0)
  	{
***
*** 1389,1396  asyncQueueFillWarning(void)
  		occupied += QUEUE_MAX_PAGE + 1;
  	}
  
! 	fillDegree = (double) occupied / (double) ((QUEUE_MAX_PAGE + 1) / 2);
  
  	if (fillDegree  0.5)
  		return;
  
--- 1401,1424 
  		occupied += QUEUE_MAX_PAGE + 1;
  	}
  

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

On Fri, 17 Jul 2015 at 23:14 Robert Haas robertmh...@gmail.com wrote:

 Committed.  I changed one remaining use of proportion to fraction,
 fixed an OID conflict, and reverted some unnecessary whitespace
 changes.


Thanks Robert.  Sorry I missed a proportion in my latest version, and
thanks for catching it.

Cheers,
BJ

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

On Fri, 26 Jun 2015 at 06:03 Gurjeet Singh gurj...@singh.im wrote:

 Patch reviewed following the instructions on
 https://wiki.postgresql.org/wiki/Reviewing_a_Patch


Thank you for your review, Gurjeet.



 s/proportion/fraction/


I think of these as synonymous -- do you have any particular reason to
prefer fraction?  I don't feel strongly about it either way, so I'm quite
happy to go with fraction if folks find that more expressive.



 + * The caller must hold (at least) shared AysncQueueLock.

 A possibly better wording: The caller must hold AysncQueueLock in (at
 least) shared mode.


Yes, that is more accurate.



 Unnecessary whitespace changes in pg_proc.h for existing functions.


I did group the asynchronous notification functions together, which seemed
reasonable as there are now three of them, and changed the tabbing between
the function name and namespace ID to match, as is done elsewhere in
pg_proc.h.  I think those changes improve readability, but again I don't
feel strongly about it.


+DESCR(get the current usage of the asynchronous notification queue);

 A possibly better wording: get the fraction of the asynchronous
 notification queue currently in use


I have no objections to your wording.

Cheers,
BJ

Re: [HACKERS] Tab completion for TABLESAMPLE

[Brendan Jurd]

On Fri, 19 Jun 2015 at 21:05 Petr Jelinek p...@2ndquadrant.com wrote:

 On 2015-06-19 09:08, Brendan Jurd wrote:
  I
  think it would be convenient and user-friendly to complete the opening
  bracket -- it would make it perfectly clear that an argument is required
  for the syntax to be valid.
 

 Agreed, here is updated version which does that.


Thanks Petr, I have tested your v2 and it works well.  This is ready for
committer.

Cheers,
BJ

Re: [HACKERS] Tab completion for TABLESAMPLE

[Brendan Jurd]

On Sun, 14 Jun 2015 at 20:44 Petr Jelinek p...@2ndquadrant.com wrote:

 looks like I omitted psql tab completion from the TABLESAMPLE patch. The
 attached patch adds it.


Hi Petr,

I'm doing an initial review of this patch.

It applies and compiles cleanly.  Code style is consistent with its
surroundings.

With the patch applied, psql correctly offers a list of existing
tablesample methods for completion whenever the previous word is
'TABLESAMPLE'.  I did notice that the syntax of the TABLESAMPLE clause
requires brackets after the method name, but the tab completion does not
complete the opening bracket.  Whereas, say, ALTER FUNCTION does.  I think
it would be convenient and user-friendly to complete the opening bracket --
it would make it perfectly clear that an argument is required for the
syntax to be valid.

Otherwise, no issues with the patch.

Cheers,
BJ

Re: [HACKERS] Tab completion for CREATE SEQUENCE

[Brendan Jurd]

On Tue, 16 Jun 2015 at 00:52 Vik Fearing v...@2ndquadrant.fr wrote:

 While reviewing the seqam patches, I noticed that psql has tab
 completion for ALTER SEQUENCE, but not for CREATE SEQUENCE.

 The attached trivial patch fixes that.


Hi Vik,

I'm doing an initial review of this patch.

It applies cleanly, although for some reason the patch seems to introduce
carriage returns in the line endings.  `patch` warned about those and then
very sensibly ignored them.  Everything compiled fine after applying.

The new code's style is not quite in accordance with the surrounds.  I
think that the comments introducing each section should be de-indented to
the first column.

RESTART, SET SCHEMA, OWNER TO, and RENAME TO are not valid
completions for CREATE SEQUENCE.  It looks like these have been blindly
copy-pasted from ALTER SEQUENCE.  Likewise, the valid completion START
[WITH] is missing.

CREATE TEMP[ORARY] SEQUENCE is not supported, and I think it probably
should be.  I think that the patch could be improved with support for the
TEMP form fairly easily.

Apart from the above points, the new completions worked as advertised.

Docs: none needed.

Tests: we don't currently have any test coverage for psql tab completion.

I'm marking this Waiting on Author.  Once the problems have been
corrected, it should be ready for a committer.

Cheers,
BJ

Re: [HACKERS] proposal: row_to_array function

[Brendan Jurd]

On Thu, 2 Apr 2015 at 05:00 Merlin Moncure mmonc...@gmail.com wrote:

 On Sun, Mar 29, 2015 at 1:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  While I don't have a problem with hstore_to_array, I don't think that
  row_to_array is a very good idea; it's basically encouraging people to
  throw away SQL datatypes altogether and imagine that everything is text.

...

 
  (In any case, those who insist can get there through row_to_json, no?)

 You have a point.  What does attached do that to_json does not do
 besides completely discard type information?


FWIW, I think row_to_array is nice, and I would make use of it.  If you
have a record, and you want to iterate over its fields in a generic way, at
least IMO converting to a text array is an obvious thing to reach for, and
it makes for very clearly intentioned code.  While it's true that you could
go through JSON or hstore to achieve much the same thing, it is a bit of a
circumlocution.

I get Tom's point that smashing to text should not be done frivolously, but
there are circumstances when it's a reasonable move.  Is it possible that
it might be used unwisely?  Yes, but then you could say that about pretty
much everything.

Would it alleviate your concerns at all if the function was named
row_to_text_array, to stress the fact that you are throwing away data types?

If the patch was invasive, I would probably not support it, but from what I
can see it's a pretty cheap add.

Cheers,
BJ

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

On Thu, 18 Jun 2015 at 03:06 Gurjeet Singh gurj...@singh.im wrote:

 I don't see this in the CF app; can you please add it there?


Done.  I did try to add it when I posted the email, but for some reason I
couldn't connect to commitfest.postgresql.org at all.  Seems fine now,
though.

Cheers,
BJ

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

Posting v2 of the patch, incorporating some helpful suggestions from Merlin.

Cheers,
BJ
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14800,14805  SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
--- 14800,14811 
/row
  
row
+entryliteralfunctionpg_notification_queue_usage()/function/literal/entry
+entrytypedouble/type/entry
+entryproportion of the asynchronous notification queue currently occupied (0-1)/entry
+   /row
+ 
+   row
 entryliteralfunctionpg_my_temp_schema()/function/literal/entry
 entrytypeoid/type/entry
 entryOID of session's temporary schema, or 0 if none/entry
***
*** 14939,14948  SET search_path TO replaceableschema/ optional, replaceableschema/, ..
  primarypg_listening_channels/primary
 /indexterm
  
 para
  functionpg_listening_channels/function returns a set of names of
! channels that the current session is listening to.  See xref
! linkend=sql-listen for more information.
 /para
  
 indexterm
--- 14945,14963 
  primarypg_listening_channels/primary
 /indexterm
  
+indexterm
+ primarypg_notification_queue_usage/primary
+/indexterm
+ 
 para
  functionpg_listening_channels/function returns a set of names of
! asynchronous notification channels that the current session is listening
! to.  functionpg_notification_queue_usage/function returns the
! proportion of the total available space for notifications currently
! occupied by notifications that are waiting to be processed, as a
! typedouble/type in the range 0-1.
! See xref linkend=sql-listen and xref linkend=sql-notify
! for more information.
 /para
  
 indexterm
*** a/doc/src/sgml/ref/notify.sgml
--- b/doc/src/sgml/ref/notify.sgml
***
*** 166,171  NOTIFY replaceable class=PARAMETERchannel/replaceable [ , replaceable cla
--- 166,176 
 current transaction so that cleanup can proceed.
/para
para
+The function functionpg_notification_queue_usage/function returns the
+proportion of the queue that is currently occupied by pending notifications.
+See xref linkend=functions-info for more information.
+   /para
+   para
 A transaction that has executed commandNOTIFY/command cannot be
 prepared for two-phase commit.
/para
*** a/src/backend/commands/async.c
--- b/src/backend/commands/async.c
***
*** 371,376  static bool asyncQueueIsFull(void);
--- 371,377 
  static bool asyncQueueAdvance(volatile QueuePosition *position, int entryLength);
  static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe);
  static ListCell *asyncQueueAddEntries(ListCell *nextNotify);
+ static double asyncQueueUsage(void);
  static void asyncQueueFillWarning(void);
  static bool SignalBackends(void);
  static void asyncQueueReadAllNotifications(void);
***
*** 1362,1387  asyncQueueAddEntries(ListCell *nextNotify)
  }
  
  /*
!  * Check whether the queue is at least half full, and emit a warning if so.
!  *
!  * This is unlikely given the size of the queue, but possible.
!  * The warnings show up at most once every QUEUE_FULL_WARN_INTERVAL.
   *
!  * Caller must hold exclusive AsyncQueueLock.
   */
! static void
! asyncQueueFillWarning(void)
  {
! 	int			headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
! 	int			tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
! 	int			occupied;
! 	double		fillDegree;
! 	TimestampTz t;
  
  	occupied = headPage - tailPage;
  
  	if (occupied == 0)
! 		return;	/* fast exit for common case */
  
  	if (occupied  0)
  	{
--- 1363,1399 
  }
  
  /*
!  * SQL function to return the proportion of the notification queue currently
!  * occupied.
!  */
! Datum
! pg_notification_queue_usage(PG_FUNCTION_ARGS)
! {
! 	double usage;
! 
! 	LWLockAcquire(AsyncQueueLock, LW_SHARED);
! 	usage = asyncQueueUsage();
! 	LWLockRelease(AsyncQueueLock);
! 
! 	PG_RETURN_FLOAT8(usage);
! }
! 
! /*
!  * Return the proportion of the queue that is currently occupied.
   *
!  * The caller must hold (at least) shared AysncQueueLock.
   */
! static double
! asyncQueueUsage(void)
  {
! 	int		headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
! 	int		tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
! 	int		occupied;
  
  	occupied = headPage - tailPage;
  
  	if (occupied == 0)
! 		return (double) 0;		/* fast exit for common case */
  
  	if (occupied  0)
  	{
***
*** 1389,1396  asyncQueueFillWarning(void)
  		occupied += QUEUE_MAX_PAGE + 1;
  	}
  
! 	fillDegree = (double) occupied / (double) ((QUEUE_MAX_PAGE + 1) / 2);
  
  	if (fillDegree  0.5)
  		return;
  
--- 1401,1424 
  		occupied += QUEUE_MAX_PAGE + 1;
  	}
  
! 	return (double) occupied / (double) ((QUEUE_MAX_PAGE + 1) / 2);
! }
! 
! /*
!  * Check whether the queue is at least half full, and emit a warning if so.
!  *
!  * This is unlikely given the size of the queue, but possible.
! 

Re: [HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

On Thu, 18 Jun 2015 at 08:19 Merlin Moncure mmonc...@gmail.com wrote:


 scratch that.  that note already exists in sql-notify.html.  Instead,
 I'd modify that section to note that you can check queue usage with
 your new function.


I have already done so.  Under the paragraph about the queue filling up, I
have added:

The function functionpg_notify_queue_saturation/function returns the
proportion of the queue that is currently occupied by pending
notifications.

A link from here back to the section in System Information Functions might
be sensible?

I will rename the function with _usage as you suggest, and add the
explanation of the return value in the docs.

Cheers,
BJ

[HACKERS] [PATCH] Function to get size of asynchronous notification queue

[Brendan Jurd]

Hello hackers,

I present a patch to add a new built-in function
pg_notify_queue_saturation().

The purpose of the function is to allow users to monitor the health of
their notification queue.  In certain cases, a client connection listening
for notifications might get stuck inside a transaction, and this would
cause the queue to keep filling up, until finally it reaches capacity and
further attempts to NOTIFY error out.

The current documentation under LISTEN explains this possible gotcha, but
doesn't really suggest a useful way to address it, except to mention that
warnings will show up in the log once you get to 50% saturation of the
queue.  Unless you happen to be eyeballing the logs when it happens, that's
not a huge help.  The choice of 50% as a threshold is also very much
arbitrary, and by the time you hit 50% the problem has likely been going on
for quite a while.  If you want your nagios (or whatever) to say, alert you
when the queue goes over 5% or 1%, your options are limited and awkward.

The patch has almost no new code.  It makes use of the existing logic for
the 50% warning.  I simply refactored that logic into a separate function
asyncQueueSaturation, and then added pg_notify_queue_saturation to make
that available in SQL.

I am not convinced that pg_notify_queue_saturation is the best possible
name for this function, and am very much open to other suggestions.

The patch includes documentation, a regression test and an isolation test.

Cheers,
BJ
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14800,14805  SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
--- 14800,14811 
/row
  
row
+entryliteralfunctionpg_notify_queue_saturation()/function/literal/entry
+entrytypedouble/type/entry
+entryproportion of the asynchronous notification queue currently occupied/entry
+   /row
+ 
+   row
 entryliteralfunctionpg_my_temp_schema()/function/literal/entry
 entrytypeoid/type/entry
 entryOID of session's temporary schema, or 0 if none/entry
***
*** 14939,14948  SET search_path TO replaceableschema/ optional, replaceableschema/, ..
  primarypg_listening_channels/primary
 /indexterm
  
 para
  functionpg_listening_channels/function returns a set of names of
! channels that the current session is listening to.  See xref
! linkend=sql-listen for more information.
 /para
  
 indexterm
--- 14945,14962 
  primarypg_listening_channels/primary
 /indexterm
  
+indexterm
+ primarypg_notify_queue_saturation/primary
+/indexterm
+ 
 para
  functionpg_listening_channels/function returns a set of names of
! asynchronous notification channels that the current session is listening
! to.  functionpg_notify_queue_saturation/function returns the proportion
! of the total available space for notifications currently occupied by
! notifications that are waiting to be processed.  See
! xref linkend=sql-listen and xref linkend=sql-notify
! for more information.
 /para
  
 indexterm
*** a/doc/src/sgml/ref/notify.sgml
--- b/doc/src/sgml/ref/notify.sgml
***
*** 166,171  NOTIFY replaceable class=PARAMETERchannel/replaceable [ , replaceable cla
--- 166,175 
 current transaction so that cleanup can proceed.
/para
para
+The function functionpg_notify_queue_saturation/function returns the
+proportion of the queue that is currently occupied by pending notifications.
+   /para
+   para
 A transaction that has executed commandNOTIFY/command cannot be
 prepared for two-phase commit.
/para
*** a/src/backend/commands/async.c
--- b/src/backend/commands/async.c
***
*** 371,376  static bool asyncQueueIsFull(void);
--- 371,377 
  static bool asyncQueueAdvance(volatile QueuePosition *position, int entryLength);
  static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe);
  static ListCell *asyncQueueAddEntries(ListCell *nextNotify);
+ static double asyncQueueSaturation(void);
  static void asyncQueueFillWarning(void);
  static bool SignalBackends(void);
  static void asyncQueueReadAllNotifications(void);
***
*** 1362,1387  asyncQueueAddEntries(ListCell *nextNotify)
  }
  
  /*
!  * Check whether the queue is at least half full, and emit a warning if so.
!  *
!  * This is unlikely given the size of the queue, but possible.
!  * The warnings show up at most once every QUEUE_FULL_WARN_INTERVAL.
   *
!  * Caller must hold exclusive AsyncQueueLock.
   */
! static void
! asyncQueueFillWarning(void)
  {
! 	int			headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
! 	int			tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
! 	int			occupied;
! 	double		fillDegree;
! 	TimestampTz t;
  
  	occupied = headPage - tailPage;
  
  	if (occupied == 0)
! 		return;	/* fast exit for common case */
  
  	if (occupied  0)
  	{
--- 1363,1399 
  }
  
  

Re: [HACKERS] Function to get size of notification queue?

[Brendan Jurd]

Hi Kevin,

I never found a direct solution to this problem.  I still feel that a
function to find the size of the notification queue would be a handy
feature to have, and I would be willing to take a shot at writing such a
feature.  However, given the tumbleweed/ response to my original email,
it's likely that effort would be a waste of time.

Cheers,
BJ


On Tue, 16 Jun 2015 at 03:40 kjsteuer kjste...@gmail.com wrote:

 Hi BJ,

 What approach did you end up using?

 Thanks,

 Kevin



 --
 View this message in context:
 http://postgresql.nabble.com/Function-to-get-size-of-notification-queue-tp5738461p5853923.html
 Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Function to get size of notification queue?

[Brendan Jurd]

On Tue, 16 Jun 2015 at 05:36 Merlin Moncure mmonc...@gmail.com wrote:

 On Mon, Jun 15, 2015 at 2:12 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  Brendan Jurd wrote:
  However, given the tumbleweed/ response to my original email,
  it's likely that effort would be a waste of time.
 
  I think tumbleweed responses are more in line with hmm, this guy might
  well be right, but I don't know right now. next email.  When people
  come up with really useless proposals, they tend to figure out pretty
  quickly.

 +1

 It took me a lot longer than it should have to figure this out, but
 lack of comment does not in any way indicate a response is bad.  Most
 commonly it means, interesting idea, why don't you code it up and see
 what happens?.  Suggestions, even very good ones (except when related
 to bona fide bugs) are remarkably unlikely to elicit, good idea,
 let's do that!.


Álvaro, Merlin,

Thanks for your comments.  I understand what you're saying, and I do agree
for the most part.  However I've also seen the downside of this, where
nobody comments much on the original proposal, and only after sinking
substantial effort into creating a patch do others appear to forcefully
oppose the idea that led to the patch.  I do understand why it happens this
way, but that doesn't make it any less of a deterrent.

If you see a proposal on the list and you think interesting idea, why
don't you code it up and see what happens, I would humbly and respectfully
encourage you to type exactly those words in to your email client and let
the author of the proposal know.  None of us are telepaths, silence is
ambiguous, and sometimes even a very small encouragement is all that is
needed to provoke action.

Back to the $subject at hand -- I have had a quick look into async.c and
can see that the logic to test for queue size in asyncQueueFillWarning()
could easily be factored out and exposed via an SQL function.  My original
idea was to have the function return the number of notifications in the
queue, but in fact given the way notifications are stored, it would be much
easier to return a float showing the fraction of the maximum queue size
that is currently occupied.  This would actually be more useful for the
use-case I described, where I am wanting to monitor for rogue processes
filling up the queue.

I will take Merlin's advice, code something up and see what happens.

Cheers,
BJ

Re: [HACKERS] Function to get size of notification queue?

[Brendan Jurd]

On Tue, 16 Jun 2015 at 07:52 Merlin Moncure mmonc...@gmail.com wrote:

 It goes back to the adage, 'Everyone wants to be an author but nobody
 wants to write'.


A more accurate version would be Everyone wants to be an author, some want
to write, but nobody likes being rejected by publishers.


 For posterity, I think your idea is pretty good, especially if the
 current slru based implementation supports it without a lot of extra
 work.


Thank you for saying so, and yes, adding the function is pretty much
trivial.  I already have a patch that works, and will submit it once I've
added docs and tests.

Adding a new built-in function is not free though so I think to
 move forwards with this you'd also have to show some more
 justification. Perhaps a real world example demonstrating the problem
 reduced down to an executable case.


Well the docs already describe this situation.  The notification queue is
finite, listening clients with long-running transactions could cause it to
blow out, and if it does blow out, Bad Things will ensue.  At the moment,
there is no good way to find out whether this is happening.

From SQL Commands / NOTIFY / Notes:

There is a queue that holds notifications that have been sent but not yet
processed by all listening sessions. If this queue becomes full,
transactions calling NOTIFY will fail at commit. The queue is quite large
(8GB in a standard installation) and should be sufficiently sized for
almost every use case. However, no cleanup can take place if a session
executes LISTEN and then enters a transaction for a very long time. Once
the queue is half full you will see warnings in the log file pointing you
to the session that is preventing cleanup. In this case you should make
sure that this session ends its current transaction so that cleanup can
proceed.


So, it's straightorward to simulate the problem scenario.  Make two client
connections A and B to the same server.  Client A executes LISTEN a;,
then BEGIN;.  Client B submits some notifications on channel a, e.g.,
SELECT pg_notify('a', 'Test queue saturation ' || s::text) FROM
generate_series(1, 1) s;.  The queue will start filling up, and will
never reduce unless and until client A ends its transaction.  If client B
keeps on submitting notifications, the queue will eventually fill
completely and then client B's session will ERROR out.

Cheers,
BJ

Re: [HACKERS] Minimum supported version of Python?

[Brendan Jurd]

On 16 March 2014 11:55, Tom Lane t...@sss.pgh.pa.us wrote:
 Our documentation claims that the minimum Python version for plpython
 is 2.3.  However, an attempt to build with that on an old Mac yielded
 a bunch of failures in the plpython_types regression test, all of the
 form

...
 Personally I have no desire to put any effort into fixing this, and
 thus suggest that we just change the documentation to specify that 2.5
 is the minimum Python version since 9.0.

+1 for updating the documentation.  2.5 has been around since 2006 so
we are offering a huge range of compatibility as it stands.  Versions
earlier than 2.5 are probably only of interest to historians at this
point.

Cheers,
BJ


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

Re: [HACKERS] proposal: simple date constructor from numeric values

[Brendan Jurd]

On 1 July 2013 17:47, Pavel Stehule pavel.steh...@gmail.com wrote:
 2013/6/29 Pavel Stehule pavel.steh...@gmail.com:
 long time I am thinking about simple function for creating date or
 timestamp values based on numeric types without necessity to create
 format string.

 What do you think about this idea?
 I found so same idea was discussed three years ago

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


I suggested something similar also:

http://www.postgresql.org/message-id/AANLkTi=W1wtcL7qR4PuQaQ=uoabmjsusz6qgjtucx...@mail.gmail.com

The thread I linked died off without reaching a consensus about what
the functions ought to be named, although Josh and Robert were
generally supportive of the idea.

The function signatures I have been using in my own C functions are:

* date(year int, month int, day int) returns date
* datetime(year int, month int, day int, hour int, minute int, second
int) returns timestamp


Cheers,
BJ


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

Re: [HACKERS] proposal: simple date constructor from numeric values

[Brendan Jurd]

On 3 July 2013 21:41, Pavel Stehule pavel.steh...@gmail.com wrote:
 I am thinking so for these functions exists some consensus - minimally
 for function date(year, month, int) - I dream about this function
 ten years :)

 I am not sure about datetime:
 a) we use timestamp name for same thing in pg
 b) we can simply construct timestamp as sum of date + time, what is
 little bit more practical (for me), because it doesn't use too wide
 parameter list.

I agree.  I've got no issues with using date + time arithmetic to
build a timestamp.

 what do you think about names?

 make_date
 make_time

I am fine with those names.  'make', 'construct', 'build', etc. are
all reasonable verbs for what the functions do, but 'make' is nice and
short, and will be familiar to people who've used a 'mktime'.

 I don't would to use to_date, to_time functions, a) because these
 functions use formatted input, b) we hold some compatibility with
 Oracle.

Yes, I agree.

Cheers,
BJ


-- 
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] [9.4 CF 1] The Commitfest Slacker List

[Brendan Jurd]

On 25 June 2013 04:13, Joshua D. Drake j...@commandprompt.com wrote:
 On 06/24/2013 10:59 AM, Andres Freund wrote:
 On 2013-06-24 10:50:42 -0700, Josh Berkus wrote:
 This project is enormously stingy with giving credit to people.  It's
 not like it costs us money, you know.
 I am all for introducing a Contributed by reviewing: ... section in
 the release notes.

 It should be listed with the specific feature.

I don't have a strong opinion about whether the reviewers ought to be
listed all together or with each feature, but I do feel very strongly
that they should be given some kind of credit.

Reviewing is often not all that much fun (compared with authoring) and
as Josh points out, giving proper credit has a bang for buck
incentive value that is hard to argue with.

Cheers,
BJ


-- 
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] Kudos for Reviewers -- straw poll

[Brendan Jurd]

On 26 June 2013 03:17, Josh Berkus j...@agliodbs.com wrote:
 How should reviewers get credited in the release notes?

 a) not at all
 b) in a single block titled Reviewers for this version at the bottom.
 c) on the patch they reviewed, for each patch

A weak preference for (c), with (b) running a close second.  As others
have suggested, a review that leads to significant commitable changes
to the patch should bump the credit to co-authorship.

 Should there be a criteria for a creditable review?

 a) no, all reviews are worthwhile
 b) yes, they have to do more than it compiles
 c) yes, only code reviews should count

(b), the review should at least look at usabililty, doc, and
regression test criteria even if there is no in-depth code analysis.

 Should reviewers for 9.4 get a prize, such as a t-shirt, as a
 promotion to increase the number of non-submitter reviewers?

 a) yes
 b) no
 c) yes, but submitters and committers should get it too

Provisionally (b), if we first try giving proper credit, and that
still doesn't drum up enough reviewing, then look to further incentive
schemes.  No need to jump the gun.

Cheers,
BJ


-- 
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] Hard to Use WAS: Hard limit on WAL space

[Brendan Jurd]

On 15 June 2013 14:43, Craig Ringer cr...@2ndquadrant.com wrote:
 The #1 question I see on Stack Overflow has to be confusion about
 pg_hba.conf, mostly from people who have no idea it exists, don't understand
 how to configure it, etc.

The totally non-obvious name of the file probably has something to do
with that.  It should be called 'auth.conf'.

Cheers,
BJ


-- 
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] Hard to Use WAS: Hard limit on WAL space

[Brendan Jurd]

On 15 June 2013 16:18, Craig Ringer cr...@2ndquadrant.com wrote:
 On 06/15/2013 02:08 PM, Brendan Jurd wrote:
 On 15 June 2013 14:43, Craig Ringer cr...@2ndquadrant.com wrote:
 The #1 question I see on Stack Overflow has to be confusion about
 pg_hba.conf, mostly from people who have no idea it exists, don't understand
 how to configure it, etc.
 The totally non-obvious name of the file probably has something to do
 with that.  It should be called 'auth.conf'.
 Not convinced; since it only controls one facet of auth - it doesn't
 define users, passwords, grants, etc ...

When somebody is setting up postgres for the first time, and they list
the contents of the config directory, you want them to have some idea
what each of the files is for.  If they see something called
'auth.conf', they'll get the right general idea.  An understanding of
the nuances (like that it doesn't control user accounts) will come
once they open up the file -- which they may well do, because it is
called 'auth.conf', and 'auth' is a thing you want to configure.

If they see something called 'pg_hba.conf', they may very reasonably
assume that it is some internal/advanced stuff that they don't need to
worry about just yet, because what the heck is a 'pg_hba'?  The 'pg'
is unnecessary and the 'hba' is an internal jargon term that we've
ill-advisedly allowed to leak out into the filename.

If you really feel that 'auth.conf' is too imprecise, maybe something
like 'conn-auth.conf' would be more your style.

Cheers,
BJ


-- 
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] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 14 June 2013 03:53, David E. Wheeler da...@justatheory.com wrote:
 Similar things should have dissimilar names. I propose:

 bikeshedding

  Old  |New
 --+--
  array_dims   | array_desc

array_bounds?

  array_ndims  | array_depth
  array_length | array_size
  array_lower  | array_start
  array_upper  | array_finish

 The last two are meh, but it’s a place to start…

I think that even with the most dissimilar names we can come up with,
this is going to confuse people.  But it is still better than doing
nothing.

I wonder whether, if we go in this direction, we could still use some
of the work I did on deprecating zero-D arrays.  Let's say the old
functions keep doing what they do now, and we teach them to treat all
empty arrays the same way they currently treat zero-D arrays (return
NULL).  The new functions treat zero-D arrays as though they were 1-D
empty with default bounds, and we add CARDINALITY per ArrayGetNItems.

This way, applications would not be broken by upgrading, and we'd be
giving people a way to opt-in to a better API.

Cheers,
BJ


-- 
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] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 12 June 2013 18:22, Dean Rasheed dean.a.rash...@gmail.com wrote:
 +1 for having a function to return the total number of elements in an
 array, because that's something that's currently missing from SQL.

 However, I think that CARDINALITY() should be that function.

 I'm not convinced that having CARDINALITY() return the length of the
 first dimension is more spec-compatible, since our multi-dimensional
 arrays aren't nested arrays, and it seems unlikely that they ever will
 be. I'd argue that it's at least equally spec-compatible to have
 CARDINALITY() return the total number of elements in the array, if you
 think of a multi-dimensional array as a collection of elements
 arranged in a regular pattern.

It's true that our multidims aren't nested, but they are the nearest
thing we have.  If we want to keep the door open for future attempts
to nudge multidim arrays into closer approximation of nested arrays,
it would be better to have the nested interpretation of CARDINALITY.
Given what we've just gone through with array_length, it seems that
once we select a behaviour for CARDINALITY, we will be stuck with it
permanently.

The problem with thinking of our multidim arrays as just a weirdly
crumpled arrangement of a single collection, is that we've already
abused the nesting syntax for declaring them.

 Also, the spec describes CARDINALITY() and UNNEST() using the same
 language, and I think it's implicit in a couple of places that
 CARDINALITY() should match the number of rows returned by UNNEST(),
 which we've already implemented as fully unnesting every element.

 We're about to add ORDINALITY to UNNEST(), and to me it would be very
 odd to have the resulting maximum ordinality exceed the array's
 cardinality.

Yeah, that makes sense.  Well the good news is that either way,
CARDINALITY will do what people want in the most common case where the
array is one-dimensional.

Multidim arrays are why we can't have nice things.

Cheers,
BJ


-- 
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] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 13 June 2013 04:26, Merlin Moncure mmonc...@gmail.com wrote:
 On Wed, Jun 12, 2013 at 1:20 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Josh Berkus j...@agliodbs.com writes:
 On 06/12/2013 11:01 AM, Tom Lane wrote:
 I'm going to be disappointed if all we can get out of this is
 a cardinality() function, and nothing is done about the empty-array
 semantics.

I would be disappointed too, but on the other hand, CARDINALITY is
required by the spec and anything would be better than nothing.

 Meh.  Robert was pretty vocal about it, but it wasn't clear to me that
 his was the majority opinion, and in any case there wasn't much
 consideration given to compromises falling somewhere between no
 changes and the rather drastic solution Brendan proposed.

I'm all for looking into possible compromises, and will happily take
any improvements to this mess I think I can get past the compatibility
maximalist caucus.

 regression=# select array_dims('{}'::int[]) is null;
  ?column?
 --
  t
 (1 row)

 Whatever you think the dimensions of that are, surely they're not
 unknown.

I don't think anyone has actually tried to defend the behaviour of the
array functions w.r.t. empty arrays.  Even the opponents of the
original proposal agreed that the behaviour was silly, they just
didn't want to fix it, on account of the upgrade burden.

 But, couldn't that be solved by deprecating that function and
 providing a more sensible alternatively named version?

And what would you name that function?  array_dims2?  I can't think of
a name that makes the difference in behaviour apparent.  Can you
imagine the documentation for that?

array_dims - Returns the dimensions of the array, unless it is empty
in which case NULL.
array_proper_dims - Returns the dimensions of the array.
array_ndims - Returns the number of dimension, unless it is empty in
which case NULL.
array_proper_ndims - Returns the number of dimensions.

... and so on for _length, _upper and _lower.

Cheers,
BJ


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

[HACKERS] Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 12 June 2013 04:43, Josh Berkus j...@agliodbs.com wrote:
 What's the status on this patch and current approach to ZDA?

Alright, it might be a good idea to have a quick recap.

Last time, on Arrays Of Our Lives ...

So I proposed and posted a patch aimed at deprecating zero-D arrays,
and replacing them with 1-D empty arrays.

That kicked off a long discussion, with some folks (Robert, Merlin,
Florian et al) vigorously opposing any change in the behaviour of the
array_(length|ndims|lower|upper) functions.  To those folks, the
behaviour would have to be a lot worse than it currently is to justify
breaking compatibility with existing applications.

The idea of using a GUC to smooth over the compatibility break was
suggested, and firmly rejected.

The idea of creating an entirely new type with nicer behaviours came
up, but that wouldn't really fly because arrays have already hogged
all the best syntax.

Since compatibility breakage is so contentious, I suggested that we
forget about changing the array representation and just add new
functions with more sensible behaviours:

* cardinality(anyarray) to return the length of the first dimension,
zero if empty, and
* array_num_items(anyarray) to return the total number of element
positions per ArrayGetNItems, zero if empty.

There have been attempts to add a cardinality function in the past, as
it is required by the SQL spec, but these attempts have stalled when
trying to decide how it should handle multidim arrays.  Having it
return the length of the first dimension is the more spec-compatible
way to go, but some folks argued that it should work as
ArrayGetNItems, because we don't already have a function for that at
the SQL level.  Therefore I propose we add cardinality() per the spec,
and another function to expose ArrayGetNItems.

And that's about where we got to, when the whole discussion was put on
a time-out to make room for the beta.

I am withdrawing the original zero-D patch in favour of the proposed
new functions.  If you have an opinion about that, please do chime in.
 Depending on how that goes I may post a patch implementing my new
proposal in the next few days.

Cheers,
BJ


-- 
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] 9.3: Empty arrays returned by array_remove()

[Brendan Jurd]

On 31 May 2013 02:52, Dean Rasheed dean.a.rash...@gmail.com wrote:
 Testing 9.3beta, it seems that array_remove() may return an empty 1-d
 array whose upper bound is lower than its lower bound. I know that we
 discussed allowing this kind of array, but I don't think that
 discussion reached any conclusion, other than to agree that the
 current empty 0-d array behaviour would be kept in 9.3.


That's right, zero-D is still the only supported representation of an
empty array, so when array_remove() yields an empty array it ought to
be zero-D.  Good catch.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 8 April 2013 16:09, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 On the specific issue of CARDINALITY, I guess we need to decide
 whether we are going to pretend that our array/matrix thing is
 actually nested.  I first argued that we should not.   But it occurred
 to me that if we do pretend, it would at least leave the door ajar if
 we want to do something to make our arrays more nest-like in future,
 without disrupting the behaviour of CARDINALITY.

 This seems to be exactly the same uncertainty that we couldn't resolve
 back in the 8.4 devel cycle, for exactly the same reasons.  I don't see
 that the discussion has moved forward any :-(


I had a poke around in the archives, and it seems to me that the major
argument that was advanced in favour of making cardinality() return
the total number of items was ... we don't have anything that does
that yet.  That's why I'm proposing we add array_num_items as well --
I do think there should be a function for this, I just don't think
cardinality fits the bill.

Cheers,
BJ


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

[HACKERS] Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 9 April 2013 09:24, Josh Berkus j...@agliodbs.com wrote:
 As much as I have a keen interest in this feature, it isn't (AFAIK)
 being considered for 9.3.  Given that it's generated a fair amount of
 controversy, could we table it until 9.3 beta?  There's still plenty of
 unresolved 9.3 patches in the queue.

No problem.  I certainly wasn't expecting it to run for 90 messages
when I started out.  I'll pipe down for now and resume after the beta.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 7 April 2013 01:43, Kevin Grittner kgri...@ymail.com wrote:
 Brendan Jurd dire...@gmail.com wrote:
 Indeed it does not prohibit nesting arrays inside other arrays, but
 the multidim arrays that Postgres allows you to create are not the
 same thing as nested arrays.

 Your interpretation matches mine all around.  It is unfortunate
 that we have hijacked the standard's syntax for arrays to add a
 matrix feature.

It really is unfortunate.  I wonder if it was done in an attempt to
mimic Oracle behaviour.

 It seems to leave us without any way forward on
 these issues that I like very much.

On the specific issue of CARDINALITY, I guess we need to decide
whether we are going to pretend that our array/matrix thing is
actually nested.  I first argued that we should not.   But it occurred
to me that if we do pretend, it would at least leave the door ajar if
we want to do something to make our arrays more nest-like in future,
without disrupting the behaviour of CARDINALITY.

It is unlikely that we ever would make such a change, but given the
intensity of the opposition to any kind of SQL-level behavioural
changes we've had on this thread, I don't want to create any more
barriers to future efforts to comport with the spec.

So how about:

* we add CARDINALITY, make it work like array_length(a, 1) except that
it returns zero for empty arrays, and
* we add array_num_items, which exposes the internal ArrayGetNItems,
and returns zero for empty arrays.

As in:

CARDINALITY(ARRAY[[1,2], [3,4], [5,6]]) = 3
array_num_items(ARRAY[[1,2], [3,4], [5,6]]) = 6

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 6 April 2013 01:59, Kevin Grittner kgri...@ymail.com wrote:
 Brendan Jurd dire...@gmail.com wrote:

 The language specifically allows for zero elements, and does not
 contemplate multiple dimensions.

 I don't remember anything in the spec which would prohibit the data
 type of an array element from itself being an array, however.

Indeed it does not prohibit nesting arrays inside other arrays, but
the multidim arrays that Postgres allows you to create are not the
same thing as nested arrays.

I believe that a purely to-spec implementation would allow you to make
an array-of-int-arrays, but since each element is its own separate
collection there would be no requirement that they have the same
cardinality as each other.

For example, ARRAY[[1], [2,3], [4,5,6]] is a valid collection per the
spec, but Postgres won't let you create this, because Postgres is
trying to create a 2-D matrix of integers, rather than a collection of
collections of integers.

The inability to extend multidim arrays in Postgres is another
manifestation of this matrix-oriented design.  Extending nested
collections is a no-brainer.  Extending matrices while ensuring they
remain perfectly regular ... not so much.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 5 April 2013 07:43, Tom Lane t...@sss.pgh.pa.us wrote:
 Well, if we're going to take that hard a line on it, then we can't
 change anything about array data storage or the existing functions'
 behavior; which leaves us with either doing nothing at all, or
 inventing new functions that have saner behavior while leaving the
 old ones in place.

And then we are in the awkward position of trying to explain the
differences in behaviour between the old and new functions ...
presumably with a dash of for historical reasons and a sprinkling of
to preserve compatibility in every other paragraph.

The other suggestion that had been tossed around elsewhere upthread
was inventing a new type that serves the demand for a straightforward
mutable list, which has exactly one dimension, and which may be
sensibly empty.  Those few who are interested in dimensions = 2 could
keep on using arrays, with all their backwards-compatible silliness
intact, and everybody else could migrate to lists at their leisure.

I don't hate the latter idea from a user perspective, but from a
developer perspective I suspect there are valid objections to be made.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 5 April 2013 13:04, Tom Lane t...@sss.pgh.pa.us wrote:
 (There's been a remarkable lack of attention to the question
 of spec compliance in this thread, btw.  Surely the standard has
 something to say on the matter of zero-length arrays?)

From 4.10 in my draft copy of Foundation, arrays are one of two
collection types (the other being multisets), and:

  A collection is a composite value comprising zero or more elements,
each a value of some data type DT

  The number of elements in C is the cardinality of C

  An array is a collection A in which each element is associated with
exactly one ordinal position in A. If n is
the cardinality of A, then the ordinal position p of an element is an
integer in the range 1 (one) ≤ p ≤ n.

The language specifically allows for zero elements, and does not
contemplate multiple dimensions.  The specification for the array
constructor syntax (6.36) and array element reference by subscript
(6.23) also make it fairly clear that only 1-D arrays were being
considered.

I'd say we've already gone way off-menu by having multidims.  A more
compliant approach would have been to implement arrays as 1-D only,
and then maybe have a separate thing (matrices?) for multidims.

While I was in there I noticed CARDINALITY, which would be pretty easy
to add and would at least provide a more productive way to get the
real length of an array without disrupting existing functionality:

cardinality expression ::=
CARDINALITY left paren collection value expression right paren

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 5 April 2013 15:05, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 While I was in there I noticed CARDINALITY, which would be pretty easy
 to add and would at least provide a more productive way to get the
 real length of an array without disrupting existing functionality:

 Yeah, that would at least fix the null-result-for-empty-array problem
 for that particular functionality.  Still, this is ammunition for the
 position that null results for empty arrays are just broken.

 BTW ... if you check the archives you will find that we had
 cardinality() for a short while, and removed it before 8.4 release,
 because we couldn't agree on what it ought to return when given a
 multi-dimensional array.  I'm afraid that issue is still unresolved.

Well for what it's worth I would expect cardinality() to return the
total number of elements in the array (per ArrayGetNItems).  It's
consistent with the spec's identification of an array as a
collection.  You can chunk the elements into dimensions however you
want, but it's still a collection of elements, and the cardinality is
still the number of elements.

The nesting interpretation doesn't accord with our internal
representation, nor with our requirement that multidim arrays be
regular, nor with the fact that we can't put an array of texts inside
an array of ints.  Our array input syntaxes for multidim arrays look
nest-ish but what they produce is not nested.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 4 April 2013 01:10, Tom Lane t...@sss.pgh.pa.us wrote:
 I think though that the upthread argument that we'd have multiple
 interpretations of the same thing is bogus.  To me, the core idea that's
 being suggested here is that '{}' should mean a zero-length 1-D array,
 not a zero-D array as formerly.  We would still need a way to represent
 zero-D arrays, if only because they'd still exist on-disk in existing
 databases (assuming we're not willing to break pg_upgrade for this).

Tom,

My thought was that on-disk zero-D arrays should be converted into
empty 1-D arrays (with default lower bounds of course) when they are
read by array_recv.  Any SQL operation on your zero-D arrays would
therefore resolve as though they were 1-D.  A pg_dump/restore would
result in the arrays being 1-D on the restore side.  If pg_upgrade
conserves the zero-D array in binary form, that's okay since the
receiving end will just treat it as 1-D out of array_recv anyway.

My intention was that the zero-D array could continue to live
indefinitely in binary form, but would never be observable as such by
any application code.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 4 April 2013 15:11, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 My thought was that on-disk zero-D arrays should be converted into
 empty 1-D arrays (with default lower bounds of course) when they are
 read by array_recv.

 Huh?  array_recv would not get applied to datums coming off of disk.

My mistake, sorry for the noise.

 In any case, the whole exercise is pointless if we don't change the
 visible behavior of array_dims et al.  So I think the idea that this
 would be without visible consequence is silly.  What's up for argument
 is just how much incompatibility is acceptable.

I don't know that anyone was suggesting there would be no visible
consequences of any kind.  I was hoping that we could at least
represent on-disk zero-D arrays as though they were 1-D.

If that's not going to fly, and we are stuck with continuing to allow
zero-D as a valid representation, then perhaps your '[]=' syntax would
be the way to proceed.  It would not be terribly difficult to rework
the patch along those lines, although I have to admit allow empty
arrays with dimensions is not nearly so satisfying a title as
exorcise zero-dimensional arrays.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 1 April 2013 21:57, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Mar 26, 2013 at 4:39 PM, Brendan Jurd dire...@gmail.com wrote:
 On 27 March 2013 06:47, Robert Haas robertmh...@gmail.com wrote:
 rhaas=# select '{}'::int4[] = '{}'::int4[];

 The good news is, if anybody out there is using that idiom to test for
 emptiness, they will not be disrupted by the change.

 According to the discussion downthread, apparently they will, because
 you're introducing an infinitude of empty arrays, not all of which
 compare equal to '{}'::int4.

It is not possible to construct e.g. '[3:2]={}' or '{{}, {}}' in
existing applications, so there is no way for that idiom in existing
applications to be broken by upgrading.  If testing for equality with
'{}' works now, it will also work post-upgrade.

The only way for it to stop working is if somebody upgrades, and
*then* goes out of their way to create an empty array with nondefault
lower bounds, and then tries to compare that array against the empty
array with default lower bounds, to test for emptiness.  Which would
be silly.

Big picture: A very large number of users wouldn't be using arrays at
all, and of those who are, probably a vanishingly small number
(perhaps zero) care about how emptiness interacts with multiple
dimensions or nondefault lower bounds.  We're talking about a corner
case inside a corner case here.

For most folks, this upgrade would break nothing.  A few (myself
included) will want to grep their code for
array_(lower|upper|length|dims) call sites and maybe make some tweaks.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 2 April 2013 10:59, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Apr 1, 2013 at 6:40 PM, Brendan Jurd dire...@gmail.com wrote:
 It is not possible to construct e.g. '[3:2]={}' or '{{}, {}}' in
 existing applications, so there is no way for that idiom in existing
 applications to be broken by upgrading.  If testing for equality with
 '{}' works now, it will also work post-upgrade.

 Sure, they've probably got to have at least
 some kind of application change before the wheels really start to come
 off, but as soon as some array that's empty but not equal to {} creeps
 into their application by whatever means, they've got trouble.

Constructing an array with nondefault bounds isn't happening by
accident, you'd have to know about, and deliberately make use of, the
obscure '[m:n]={}' syntax for array literals.  How is it going to
creep in?

I note the total absence of people rending their garments over the
fact that '{foo}' does not equal '[2:2]={foo}'.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 2 April 2013 11:34, David E. Wheeler da...@kineticode.com wrote:
 On Apr 1, 2013, at 4:59 PM, Robert Haas robertmh...@gmail.com wrote:

 I think the only people for whom nothing will break are the people who
 aren't using arrays in the first place.  Anyone who is is likely to
 have dependencies on the way array_lower/upper work today.

 Well, what if we add new functions that return 0 for empty arrays, but leave 
 the existing ones alone? Perhaps call them array_size(), array_first_index(), 
 and array_last_index(). Then nothing has to break, and we can decide 
 independently if we want to deprecate the older functions in a future 
 release. Or not.

I think having an 'array_size' and an 'array_length' that behave
differently would be legitimately confusing, and I can't think of any
alternative function name that would concisely explain the difference
in behaviour -- 'array_length_without_the_stupid_nulls' is just too
long.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 28 March 2013 20:34, Dean Rasheed dean.a.rash...@gmail.com wrote:
 Is the patch also going to allow empty arrays in higher dimensions
 where not just the last dimension is empty?

It doesn't allow that at present.

 It seems as though, if
 it's allowing 1-by-0 arrays like '{{}}' and '[4:4][8:7]={{}}', it
 should also allow 0-by-0 arrays like '[4:3][8:7]={}', and 0-by-3
 arrays like '[4:3][11:13]={}'.

I think the array literal parser would reject this on the grounds that
the brace structure doesn't match the dimension specifiers.  You could
modify that check to respect zero length in dimensions other than the
innermost one, but it's hard to say whether it would be worth the
effort.

It might be instructive to hear from somebody who does (or intends to)
use multidim arrays for some practical purpose, but I don't even know
whether such a person exists within earshot of this list.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 28 March 2013 00:21, Dean Rasheed dean.a.rash...@gmail.com wrote:
 The patch is also allowing '{{},{},{}}' which is described up-thread
 as a 2-D empty array. That's pretty misleading, since it has length 3
 (in the first dimension). '{{},{}}' and '{{}}' are both more empty,
 but neither is completely empty.

 I'm not saying that the current situation is not broken. I'm just
 questioning whether the fix is actually any less confusing than what
 we have now.

Well the fix is primarily about 1-D empty arrays, and in that respect
it is much less confusing than what we have now.  As for multidim
arrays, I don't use them in the field, so I don't have a strong
opinion about how (or even whether) we should support empty multidim.
I included the '{{}}' syntax following comments from Tom that we
should consider supporting that if we were to get rid of zero-D, but I
don't really have any skin in that game.

Since we require multidim arrays to be regular, perhaps they would
need extents in all dimensions to be practically useful ... if you
wanted to define a blank tic-tac-toe board using a 2-D array, for
example, you'd probably define it as 3x3 with NULL values in each
position, rather than trying to make it empty.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 28 March 2013 09:39, Dean Rasheed dean.a.rash...@gmail.com wrote:
 On 27 March 2013 17:14, Brendan Jurd dire...@gmail.com wrote:
 Well the fix is primarily about 1-D empty arrays, and in that respect
 it is much less confusing than what we have now.

 Maybe. But even in 1-D, it's still jumping from having one empty array
 to infinitely many starting at different indexes, e.g., '{}'::int[] !=
 '[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
 convinced about its usefulness.

We already have the ability to define lower bounds other than 1 on
arrays, and it would be inconsistent to allow that for arrays with
elements, but not for arrays without.  I could imagine somebody
wanting to create an empty zero-based array, and then iteratively
append elements to it.

 Also, it is incompatible with the choice made for empty ranges,

To me it doesn't make sense to try to draw parallels between arrays
and ranges, they really are completely different things.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 26 March 2013 22:57, Robert Haas robertmh...@gmail.com wrote:
 They hate it twice as much when the change is essentially cosmetic.
 There's no functional problems with arrays as they exist today that
 this change would solve.


We can't sensibly test for whether an array is empty.  I'd call that a
functional problem.

The NULL return from array_{length,lower,upper,ndims} is those
functions' way of saying their arguments failed a sanity check.  So we
cannot distinguish in a disciplined way between a valid, empty array,
and bad arguments.  If the zero-D implementation had been more
polished and say, array_ndims returned zero, we had provided an
array_empty function, or the existing functions threw errors for silly
arguments instead of returning NULL, then I'd be more inclined to see
your point.  But as it stands, the zero-D implementation has always
been half-baked and slightly broken, we just got used to working
around it.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 27 March 2013 06:47, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Mar 26, 2013 at 9:02 AM, Brendan Jurd dire...@gmail.com wrote:
 We can't sensibly test for whether an array is empty.  I'd call that a
 functional problem.

 Sure you can.  Equality comparisons work just fine.

 rhaas=# select '{}'::int4[] = '{}'::int4[];

The good news is, if anybody out there is using that idiom to test for
emptiness, they will not be disrupted by the change.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 26 March 2013 00:30, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 On 25 March 2013 13:02, Josh Berkus j...@agliodbs.com wrote:
 Brendan, how hard would it be to create a GUC for backwards-compatible
 behavior?

 Good idea.

 No, it *isn't* a good idea.  GUCs that change application-visible
 semantics are dangerous.  We should have learned this lesson by now.


They are?  Well okay then.  I'm not deeply attached to the GUC thing,
it just seemed like a friendly way to ease the upgrade path.  But if
it's dangerous somehow I'm happy to drop it.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 21 March 2013 10:45, Brendan Jurd dire...@gmail.com wrote:
  src/test/isolation/expected/timeouts.out|  16 +-
  src/test/isolation/specs/timeouts.spec  |   8 +-

Oops, looks like some unrelated changes made their way into the
original patch.  Apologies.  Here's a -v2 patch, sans stowaways.

Cheers,
BJ


zero-length-array-v2.diff.bz2
Description: BZip2 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

[HACKERS] Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: Should array_length() Return NULL)

[Brendan Jurd]

On 26 March 2013 05:26, Greg Stark st...@mit.edu wrote:
 I'm not as sanguine as Tom is about how likely these corner cases will
 be met actually. As far as I can tell checking IS NULL on
 array_length() was the supported way to check for 0-length arrays
 previously

Correct.  There was no other way to check for empty arrays, because
every empty array was zero-D, and zero-D returns NULL from all array
interrogation functions, even array_ndims (which is totally bonkers).


 At least if it's a clean break then everyone on 9.3 knows they need to
 do IS NULL and everyone on 9.4 knows they need to check for 0.

And, as I pointed out in at the top of the thread, you can write
coalesce(array_length(a,1), 0) = 0 if you want to be confident your
code will continue work in either case, and for some folks I expect
that will be the least painful way to upgrade from 9.3 - 9.4.

Cheers,
BJ


-- 
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] adding support for zero-attribute unique/etc keys

[Brendan Jurd]

On 26 March 2013 05:04, Darren Duncan dar...@darrenduncan.net wrote:
 On 2013.03.25 1:17 AM, Albe Laurenz wrote:
 The desired effect can be had today with a unique index:

 CREATE TABLE singleton (id integer);
 CREATE UNIQUE INDEX singleton_idx ON singleton((1));

 Okay, that is helpful, and less of a kludge than what I was doing, but it is
 still a kludge compared to what I'm proposing, which I see as elegant.


FWIW I think an index on (TRUE) expresses the intention more clearly
than an index on () would.

I don't have any objection to the purely logical sense of the
zero-attribute key, but it's hard to see the pragmatic value.

Cheers,
BJ


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

Re: [PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 25 March 2013 13:02, Josh Berkus j...@agliodbs.com wrote:
 On 03/20/2013 04:45 PM, Brendan Jurd wrote:
 Incompatibility:
 This patch introduces an incompatible change in the behaviour of the
 aforementioned array functions -- instead of returning NULL for empty
 arrays they return meaningful values.  Applications relying on the old
 behaviour to test for emptiness may be disrupted.  One can

 As a heavy user of arrays, I support this patch as being worth the
 breakage of backwards compatibility.  However, that means it certainly
 will need to wait for 9.4 to provide adequate warning.

Thanks Josh, I appreciate the support.


 Brendan, how hard would it be to create a GUC for backwards-compatible
 behavior?

Good idea.  I don't know how hard it would be (never messed with GUCs
before), but I'll take a swing at it and see how it works out.

Cheers,
BJ


-- 
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] Single-argument variant for array_length and friends?

[Brendan Jurd]

On 22 March 2013 09:12, Merlin Moncure mmonc...@gmail.com wrote:
 On Thu, Mar 21, 2013 at 2:00 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 lot of postgresql functions calculate with all items in array without
 respect to dimensions - like unnest.

 so concept use outermost dim is not in pg now, and should not be
 introduced if it is possible. More it goes against a verbosity concept
 introduced by ADA and reused in PL/SQL and PL/pgSQL.

 and pl/psm*

Yeah, okay.  That argument works for me.  Let's go for option (a),
only allow the user to omit the dimension argument if the array is
1-D.

We still have the issue that Tom isn't convinced that the feature is
worth pursuing -- Tom, would you please elaborate a little on what you
dislike about it?  I don't see much of a downside (just 3 extra
pg_procs).

Cheers,
BJ


-- 
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] Single-argument variant for array_length and friends?

[Brendan Jurd]

On 21 March 2013 17:08, Pavel Stehule pavel.steh...@gmail.com wrote:
 2013/3/21 Tom Lane t...@sss.pgh.pa.us:
 I'm not entirely convinced that this is a good idea, but if we're going
 to allow it I would argue that array_length(a) should be defined as
 array_length(a, 1).  The other possibilities are too complicated to
 explain in as few words.


 exactly

 +1

Hi Pavel,

Is your +1 to array_length(a) being defined as array_length(a,1), or
to Tom's being unconvinced by the whole proposal?  Or both?

Cheers,
BJ


-- 
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] Single-argument variant for array_length and friends?

[Brendan Jurd]

On 21 March 2013 17:32, Pavel Stehule pavel.steh...@gmail.com wrote:
 If I though about it more, I like to more limit one parametric
 array_length function just for only 1D array. So it is your A use
 case. But I understand so this variant is not orthogonal. Hard to say,
 what is better.


Yes, for me (a) is running a very close 2nd place to (c).  The
strength of (a) is it means we aren't making guesses about the user's
intention.  When a user concocts an expression that is ambiguous, I
feel it is usually good to kick it back to them and ask them to be
more precise.

On the other hand, I find it very natural to interpret what is the
length of my multidim array to mean what is the length of the
outermost dimension of my multidim array, because to me a multidim
array is just an array that contains more arrays.

Cheers,
BJ


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

[PATCH] Exorcise zero-dimensional arrays (Was: Re: [HACKERS] Should array_length() Return NULL)

[Brendan Jurd]

On 17 March 2013 05:19, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 On 16 March 2013 09:07, Tom Lane t...@sss.pgh.pa.us wrote:
 The thing is that that syntax creates an array of zero dimensions,
 not one that has 1 dimension and zero elements.

 I'm going to ask the question that immediately comes to mind: Is there
 anything good at all about being able to define a zero-dimensional
 array?

 Perhaps not.  I think for most uses, a 1-D zero-length array would be
 just as good.  I guess what I'd want to know is whether we also need
 to support higher-dimensional zero-size arrays, and if so, what does
 the I/O syntax for those look like?

 Another fly in the ointment is that if we do redefine '{}' as meaning
 something other than a zero-D array, how will we handle existing
 database entries that are zero-D arrays?


Hello hackers,

I submit a patch to rectify the weird and confusing quirk of Postgres
to use zero dimensions to signify an empty array.

Rationale:
The concept of an array without dimensions is a) incoherent, and b)
leads to astonishing results from our suite of user-facing array
functions.  array_length, array_dims, array_ndims, array_lower and
array_upper all return NULL when presented such an array.

When a user writes ARRAY[]::int[], they expect to get an empty array,
but instead we've been giving them a bizarre semi-functional
proto-array.  Not cool.

Approach:
The patch teaches postgres to regard zero as an invalid number of
dimensions (which it very much is), and allows instead for fully armed
and operational empty arrays.

The simplest form of empty array is the 1-D empty array (ARRAY[] or
'{}') but the patch also allows for empty arrays over multiple
dimensions, meaning that the final dimension has a length of zero, but
the other dimensions may have any valid length.  For example,
'{{},{},{}}' is a 2-D empty array with dimension lengths {3,0} and
dimension bounds [1:3][1:0].

An empty array dimension may have any valid lower bound, but by
default the lower bound will be 1 and the upper bound will therefore
be 0.

Any zero-D arrays read in from existing database entries will be
output as 1-D empty arrays from array_recv.

There is an existing limitation where you cannot extend a multi-D
array by assigning values to subscripts or slices.  I've made no
attempt to address that limitation.

The patch improves some error reporting, particularly by adding
errdetail messages for ereports of problems parsing a curly-brace
array literal.

There is some miscellaneous code cleanup included; I moved the
hardcoded characters '{', '}', etc. into constants, unwound a
superfluous inner loop from ArrayCount, and corrected some mistakes in
code comments and error texts.  If preferred for reviewing, I can
split those changes (and/or the new errdetails) out into a separate
patch.

Incompatibility:
This patch introduces an incompatible change in the behaviour of the
aforementioned array functions -- instead of returning NULL for empty
arrays they return meaningful values.  Applications relying on the old
behaviour to test for emptiness may be disrupted.  One can
future-proof against this by changing e.g.

IF array_length(arr, 1) IS NULL  ...

to

IF coalesce(array_length(arr, 1), 0) = 0  ...

There is also a change in the way array subscript assignment works.
Previously it wasn't possible to make a distinction between assigning
to an empty array and assigning to a NULL, so in either case an
expression like arr[4] := 1 would create [4:4]={1}.  Now there is
a distinction.  If you assign to an empty array you will get {NULL,
NULL, NULL, 1}, whereas if you assign to a NULL you'll get
[4:4]={1}.

Regression tests:
The regression tests were updated to reflect behavioural changes.

Documentation:
A minor update to the prose in chapter 8.15 is included in the patch.

Issues:
Fixed-length arrays (like oidvector) are zero-based, which means that
they are rendered into string form with their dimension info shown.
So an empty oidvector now renders as [0:-1]={}, which is correct but
ugly.  I'm not delighted with this side-effect but I'm not sure what
can be done about it.  I'm open to ideas.

Diffstat:
 doc/src/sgml/array.sgml |   4 +-
 src/backend/executor/execQual.c |  77 +-
 src/backend/utils/adt/array_userfuncs.c |  23 +-
 src/backend/utils/adt/arrayfuncs.c  | 671
+--
 src/backend/utils/adt/arrayutils.c  |   4 +
 src/backend/utils/adt/xml.c |  19 +-
 src/include/c.h |   1 +
 src/include/utils/array.h   |   4 +
 src/pl/plpython/plpy_typeio.c   |   3 -
 src/test/isolation/expected/timeouts.out|  16 +-
 src/test/isolation/specs/timeouts.spec  |   8 +-
 src/test/regress/expected/arrays.out|  55 ++--
 src/test/regress/expected/create_function_3.out |   2

Re: [HACKERS] Should array_length() Return NULL

[Brendan Jurd]

On 16 March 2013 09:07, Tom Lane t...@sss.pgh.pa.us wrote:
 David E. Wheeler da...@justatheory.com writes:
 This surprised me:

 david=# select array_length('{}'::text[], 1);
  array_length
 --
[null]

 I had expecte dit to retur 0. I might expect NULL for a NULL param, but not 
 one that's defined but has no elements.

 The thing is that that syntax creates an array of zero dimensions,
 not one that has 1 dimension and zero elements.  So 0 would be
 incorrect.


I'm going to ask the question that immediately comes to mind: Is there
anything good at all about being able to define a zero-dimensional
array?

I would have thought that anything deserving the name array has
one-or-more dimensions, and that a zero-dimensional array is a weird
way of talking about a scalar value.  In which case '{}'::text[] would
not be a legitimate way to declare one anyway.  Am I missing
something?

Cheers,
BJ


-- 
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] Should array_length() Return NULL

[Brendan Jurd]

On 17 March 2013 05:19, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 On 16 March 2013 09:07, Tom Lane t...@sss.pgh.pa.us wrote:
 The thing is that that syntax creates an array of zero dimensions,
 not one that has 1 dimension and zero elements.

 I'm going to ask the question that immediately comes to mind: Is there
 anything good at all about being able to define a zero-dimensional
 array?

 Perhaps not.  I think for most uses, a 1-D zero-length array would be
 just as good.  I guess what I'd want to know is whether we also need
 to support higher-dimensional zero-size arrays, and if so, what does
 the I/O syntax for those look like?

If I'm reading right, in our current implementation of array
dimensionality, there can be no such thing as a higher-dimensional
zero-length array anyhow.  Postgres doesn't care about how many
dimensions you define for an array, it uses the quacks like a duck
test for number of dimensions.  For example:

postgres=# SELECT ARRAY[1]::int[][], array_dims(ARRAY[1]::int[][]);
 array | array_dims
---+
 {1}   | [1:1]

postgres=# SELECT ARRAY[ARRAY[1]]::int[];
 array
---
 {{1}}

postgres=# SELECT ARRAY[ARRAY[1]]::int[][];
 array
---
 {{1}}

Some array functions just plain don't work with multiple dimensions:

postgres=# SELECT array_append(ARRAY[ARRAY[1]]::int[][], ARRAY[2]);
ERROR:  function array_append(integer[], integer[]) does not exist

So, to answer your question, no, I don't think we would need to
support it, at least not unless/until there is a major change and
number of dimensions becomes more meaningful.  You can start out with
a zero-length array (which has one dimension) and then add nested
arrays to it if you want to -- it will then ipso facto have multiple
dimensions.

 Another fly in the ointment is that if we do redefine '{}' as meaning
 something other than a zero-D array, how will we handle existing
 database entries that are zero-D arrays?


I would go with zero-length 1-D.  It's almost certainly what the
author intended.

I'd be more worried about the possibility of, say, PL/pg functions in
the field that rely on our existing bizarre behaviours to test for an
empty array, like IF array_length(A) IS NULL, or IF array_dims(A) IS
NULL.  I'm pretty sure I have some such tests in my applications, and
I still think breaking them is a reasonable price to pay for greater
sanity.

Cheers,
BJ


-- 
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] Should array_length() Return NULL

[Brendan Jurd]

On 17 March 2013 06:27, Tom Lane t...@sss.pgh.pa.us wrote:
 What I'm concerned about here is whether these expressions shouldn't
 be yielding different data values:



 Right now, if we did make them produce what they appear to mean, the
 array I/O functions would have a problem with representing the results:


 So I think we'd need to fix that before we could go very far in this
 direction.

I agree.  I am starting to work on that very thing.

I noticed that there are a whole bunch of errmsgs in ArrayCount and
ReadArrayStr that just say malformed array literal with no detail
message at all.  Not very helpful.  I'm tempted to improve that on my
way past.

Cheers,
BJ


-- 
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] Considering Gerrit for CFs

[Brendan Jurd]

On 7 February 2013 08:07, Josh Berkus j...@agliodbs.com wrote:
 The existing Gerrit community would be keen to have the PostgreSQL
 project as a major user, though, and would theoretically help with
 modification needs.  Current major users are OpenStack, Mediawiki,
 LibreOffice and QT.

Do we actually have any requirements that are known to be uncatered
for in gerrit's standard feature set?

Cheers,
BJ


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

[HACKERS] Function to get size of notification queue?

[Brendan Jurd]

Hi folks,

I have a project which uses Postgres asynchronous notifications pretty
heavily.  It has a particularly Fun failure mode which causes the
notification queue to fill up.  To better debug this problem I'd like
to be able to monitor the size of the notification queue over time.

It doesn't look like we have a pg_notify_queue_size() or equivalent.
Should we?  Or would I be better off just watching the size of the
pg_notify/ directory on disk?

Cheers,
BJ


-- 
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] operator dependency of commutator and negator, redux

[Brendan Jurd]

On 20 December 2012 11:51, Tom Lane t...@sss.pgh.pa.us wrote:

 While reconsidering the various not-too-satisfactory fixes we thought of
 back then, I had a sudden thought.  Instead of having a COMMUTATOR or
 NEGATOR forward reference create a shell operator and link to it,
 why not simply *ignore* such references?  Then when the second operator
 is defined, go ahead and fill in both links?


Ignore with warning sounds pretty good.  So it would go something like this?

# CREATE OPERATOR  (... COMMUTATOR );
WARNING: COMMUTATOR  (foo, foo) undefined, ignoring.
CREATE OPERATOR

# CREATE OPERATOR  (... COMMUTATOR );
CREATE OPERATOR


Cheers,
BJ


-- 
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] External Open Standards

[Brendan Jurd]

On 24 May 2012 05:30, Peter Eisentraut pete...@gmx.net wrote:
 On mån, 2012-05-21 at 15:34 +1000, Brendan Jurd wrote:
 I'd be okay with just adding a note in the manual under Date/Time
 Output to the effect of Note: ISO 8601 specifies the use of uppercase
 letter 'T' to separate the date and time. Postgres uses a space for
 improved readability, in line with other database systems and RFC
 3339.

 But that is wrong.  We use the space because SQL says so.  The reason we
 have all those other formats is for readability or consistency or some
 secondary standard.  But the format of the ISO format is exactly what
 the SQL standard says, without any other considerations.


Did you miss my follow-on message on this thread?  You are responding
to a message that has been superceded.  I saw what you wrote earlier,
and updated my suggestions for the docs 2 days ago.

Please see

http://archives.postgresql.org/message-id/cadxjzo34ma54imgd7ruadcc-r6lq7w-ta8gqwvahkua7bsv...@mail.gmail.com

Cheers,
BJ

-- 
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] External Open Standards

[Brendan Jurd]

On 22 May 2012 02:58, Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 The problem is that people think that ISO means ISO 8601, whereas it
 actually means ISO 9075.  I can see how that's an easy mistake to make,
 though.

 ... especially since we keep referring to 8601 in our own docs.
 Does this mean we should do a global s/8601/9075/ in the docs?

Negative on the global replace.  Some date/time functions legitimately
refer to 8601 (see the page on 'extract' for examples).

I had thought that to_char/to_timestamp might refer to 8601 for the
ISO week date feature, but there doesn't appear to be an explicit
mention on that page.  Perhaps there should be?

The text in 8.5.2. Date/Time Output seems to be the culprit here.  It
(thrice) names 8601 as the governing standard for the ISO output
datestyle, and it has some ancillary wording that would become awkward
if we just subbed in 9075 instead.

Currently the first few sentences read:

The output format of the date/time types can be set to one of the
four styles ISO 8601, SQL (Ingres), traditional POSTGRES (Unix date
format), or German. The default is the ISO format. (The SQL standard
requires the use of the ISO 8601 format. The name of the SQL output
format is a historical accident.) 

I would suggest something like:

The output format of the date/time types can be set to one of the
four styles ISO, SQL (Ingres), traditional POSTGRES (Unix date
format), or German. The default is the ISO format, which refers to ISO
9075, the SQL standard.  (The name of the SQL output format is a
historical accident, it does not refer to the SQL standard.) 

The note I suggested earlier would probably need to change in light of
the above, too.  Perhaps along these lines:

Note: the output format specified by ISO 9075 is identical to that
specified in ISO 8601, except that the uppercase letter 'T' separating
the date part from the time part is replaced with a space for the sake
of readability.

Cheers,
BJ

-- 
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] External Open Standards

[Brendan Jurd]

On 20 May 2012 01:52, Daniel Farina dan...@heroku.com wrote:
 The documentation is misleading to the point of our support for ISO
 8601-strict parsing.

 http://archives.postgresql.org/pgsql-hackers/2012-02/msg01237.php

 A very fine point, but I discovered it not out of curiosity, but a
 fairly angry user on Twitter.

 We can define the problem away since the space-inclusive format is so
 common...so much so, that it is codified in RFC 3339
 (http://www.ietf.org/rfc/rfc3339.txt).  The only problem, then, is the
 DATESTYLE ISO labeling: changing that would be really painful, so
 perhaps another solution is to parse the T demanded by 8601,
 presuming no other details come to light.

We may be wandering a bit off-topic from Simon's OP, but I'll bite.
We already do *parse* the 'T' in datetime input:

postgres=# select timestamp '2012-05-21T15:05';
  timestamp
-
 2012-05-21 15:05:00
(1 row)

What we don't do is *output* the 'T', but this is pretty easy to
workaround, e.g., to_char(now(), '-MM-DDTHH24:MI:SS').  The
scope of  actually wanting the 'T' is surely pretty minor?

I'd be okay with just adding a note in the manual under Date/Time
Output to the effect of Note: ISO 8601 specifies the use of uppercase
letter 'T' to separate the date and time. Postgres uses a space for
improved readability, in line with other database systems and RFC
3339.

Cheers,
BJ

-- 
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 tracker tool we need

[Brendan Jurd]

On 18 April 2012 13:44, Tom Lane t...@sss.pgh.pa.us wrote:
 ... I think you'll find a lot of that data could be mined out of our
 historical commit logs already.  I know I make a practice of mentioning
 bug # whenever there is a relevant bug number, and I think other
 committers do too.  It wouldn't be 100% coverage, but still, if we could
 bootstrap the tracker with a few hundred old bugs, we might have
 something that was immediately useful, instead of starting from scratch
 and hoping it would eventually contain enough data to be useful.

Just as a data point, git tells me that there are 387 commits where
the commit log message matches '#\d+', and 336 where it matches 'bug
#\d+'.

Cheers,
BJ

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

[HACKERS] Clobbered parameter names via DECLARE in PL/PgSQL

[Brendan Jurd]

Hello hackers,

It turns out that in a PL/PgSQL function, you can DECLARE a variable
using the same name as one of the function parameters.  This has the
effect of clobbering the parameter, for example:

CREATE OR REPLACE FUNCTION declare_clobber(foo int)
RETURNS int LANGUAGE plpgsql AS $$
DECLARE
foo text;
BEGIN
RETURN foo;
END;
$$;

SELECT declare_clobber(1);
== NULL

On the other hand, PL/PgSQL does protect against duplicate definitions
within DECLARE:

CREATE OR REPLACE FUNCTION declare_clobber(foo int)
RETURNS int LANGUAGE plpgsql AS $$
DECLARE
foo int;
foo text;
BEGIN
RETURN foo;
END;
$$;
== ERROR:  duplicate declaration at or near foo

And it also protects against using a DECLAREd name as a parameter alias:

CREATE OR REPLACE FUNCTION declare_clobber(foo int)
RETURNS int LANGUAGE plpgsql AS $$
DECLARE
bar int;
bar ALIAS FOR $1;
BEGIN
RETURN bar;
END;
$$;
== ERROR:  duplicate declaration at or near bar

I would suggest that if the user DECLAREs a variable with the same
name as a parameter, it is very evidently a programming error, and we
should raise the same duplicate declaration error.  I haven't yet
looked at how difficult this would be to fix, but if there are no
objections I would like to attempt a patch.

Cheers,
BJ

-- 
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] Clobbered parameter names via DECLARE in PL/PgSQL

[Brendan Jurd]

On 15 April 2012 17:55, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/4/15 Brendan Jurd dire...@gmail.com:
 It turns out that in a PL/PgSQL function, you can DECLARE a variable
 using the same name as one of the function parameters.  This has the
 effect of clobbering the parameter, for example:

...

 I would suggest that if the user DECLAREs a variable with the same
 name as a parameter, it is very evidently a programming error, and we
 should raise the same duplicate declaration error.  I haven't yet
 looked at how difficult this would be to fix, but if there are no
 objections I would like to attempt a patch.


 I disagree - variables and parameters are in different namespace so
 you can exactly identify variable and parameter. More - it is
 compatibility break.


They may technically be in different namespaces, but the fact that the
declared variable quietly goes ahead and masks the parameter locally,
seems like a recipe for unexpected consequences.  It certainly was in
my case, and I doubt I'm the first or the last to make that mistake.

Under these conditions, you now have foo which refers to the
variable, and declare_clobber.foo which refers to the parameter.
Not exactly a model of clarity, and it's also quite easy to miss the
part of the PL/PgSQL docs mentioning this notation.

Perhaps it's a failure of imagination on my part, but I can't think of
a legitimate reason for a programmer to deliberately use the same name
to refer to a declared variable and a function parameter.  What would
be the benefit?

Cheers,
BJ

-- 
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] Clobbered parameter names via DECLARE in PL/PgSQL

[Brendan Jurd]

On 15 April 2012 18:54, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/4/15 Brendan Jurd dire...@gmail.com:
 Perhaps it's a failure of imagination on my part, but I can't think of
 a legitimate reason for a programmer to deliberately use the same name
 to refer to a declared variable and a function parameter.  What would
 be the benefit?

 it depends on level of nesting blocks. For simple functions there
 parameter redeclaration is clean bug, but for more nested blocks and
 complex procedures, there should be interesting using some local
 variables with same identifier like some parameters and blocking
 parameter's identifier can be same unfriendly feature like RO
 parameters in previous pg versions.

 I understand your motivation well, but solution should be warning, not
 blocking. I think.

I can accept that ... but I wonder about the implementation of such a
warning.  Can we raise a WARNING message on CREATE [OR REPLACE]
FUNCTION?  If so, should there be a way to switch it off?  If so,
would this be implemented globally, or per-function?  Would it be a
postgres run-time setting, or an extension to CREATE FUNCTION syntax,
or something within the PL/pgSQL code (like Perl's 'use strict')?

Cheers,
BJ

-- 
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] Our regex vs. POSIX on longest match

[Brendan Jurd]

On 5 March 2012 17:23, Robert Haas robertmh...@gmail.com wrote:
 This is different from what Perl does, but I think Perl's behavior
 here is batty: given a+|a+b+ and the string aaabbb, it picks the first
 branch and matches only aaa.

Yeah, this is sometimes referred to as ordered alternation,
basically that the branches of the alternation are prioritised in the
same order in which they are described.  It is fairly commonplace
among regex implementations.

 apparently, it selects the syntactically first
 branch that can match, regardless of the length of the match, which
 strikes me as nearly pure evil.

As long as it's documented that alternation prioritises in this way, I
don't feel upset about it.  At least it still provides you with a
sensible way to get whatever you want from your RE; if you want a
shorter alternative to be preferred, put it up the front.  Ordered
alternation also gives you a way to specify which of several
same-length alternatives you would prefer to be matched, which can
come in handy.  It also means you can specify less-complex
alternatives before more-complex ones, which can have performance
advantages.

I do agree with you that if you *don't* do ordered alternation, then
it is right to treat alternation as greedy by default.

Cheers,
BJ

-- 
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] Our regex vs. POSIX on longest match

[Brendan Jurd]

On 4 March 2012 17:53, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 I'll admit that this is a pretty obscure point, but we do appear to be
 in direct violation of POSIX here.

 How so?  POSIX doesn't contain any non-greedy constructs.  If you use
 only the POSIX-compatible greedy constructs, the behavior is compliant.

While it's true that POSIX doesn't contemplate non-greed, after
reading the spec I would have expected an expression *as a whole* to
still prefer the longest match, insofar as that is possible while
respecting non-greedy particles.  I just ran some quick experiments in
Perl, Python and PHP, using 'xy1234' ~ 'y*?\d+'.  All returned
'y1234', which to my mind is the most obvious answer, and one which
still makes sense in light of what POSIX has to say.  Whereas Postgres
(and presumably Tcl) return 'y1'.

What I found surprising here is that our implementation allows an
earlier quantifier to clobber the greediness of a later quantifier.
There's a certain disregard for the intentions of the author of the
regex.  If I had wanted the whole expression to be non-greedy, I could
have written 'y*?\d+?'.  But since I wrote 'y*?\d+', it is clear that
I meant for the first atom to be non-greedy, and for the second to be
greedy.

 The issue that is obscure is, once you define some non-greedy
 constructs, how to define how they should act in combination with greedy
 ones.  I'm not sure to what extent the engine's behavior is driven by
 implementation restrictions and to what extent it's really the sanest
 behavior Henry could think of.  I found a comment from him about it:
 http://groups.google.com/group/comp.lang.tcl/msg/c493317cc0d10d50
 but it's short on details as to what alternatives he considered.

Thanks for the link; it is always good to get more insight into
Henry's approach.  I'm beginning to think I should just start reading
everything he ever posted to comp.lang.tcl ...

Cheers,
BJ

-- 
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] Our regex vs. POSIX on longest match

[Brendan Jurd]

On 5 March 2012 04:34, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 On 4 March 2012 17:53, Tom Lane t...@sss.pgh.pa.us wrote:
 Brendan Jurd dire...@gmail.com writes:
 While it's true that POSIX doesn't contemplate non-greed, after
 reading the spec I would have expected an expression *as a whole* to
 still prefer the longest match, insofar as that is possible while
 respecting non-greedy particles.

 It's not apparent to me that a construct that is outside the spec
 shouldn't be expected to change the overall behavior.  In particular,
 what if the RE contains only non-greedy quantifiers --- would you still
 expect longest match overall?  Henry certainly didn't.

Well, no, but then that wouldn't be prefer the longest match, insofar
as that is possible while
 respecting non-greedy particles.  If all the quantifiers in an
expression are non-greedy, then it is trivially true that the only way
to respect the author's intention is to return the shortest match.

 Well, that's just an arbitrary example.  The cases I remember people
 complaining about in practice were the other way round: greedy
 quantifier followed by non-greedy, and they were unhappy that the
 non-greediness was effectively not respected (because the overall RE was
 taken as greedy).

I am unhappy about the reverse example too, and for the same reasons.

If we look at 'xy1234' ~ 'y*\d+?', Postgres gives us 'y1234'
(disregarding the non-greediness of the latter quantifier), and Python
gives us 'y1' (respecting both quantifiers).

So in Postgres, no matter how you mix the greediness up, some of your
quantifiers will not be respected.

 So you can't fix the issue by pointing to POSIX and
 saying overall greedy is always the right thing.

... insofar as that is possible while respecting non-greedy particles.

I will take Henry's word for it that this problem is harder than it
looks, but in any case, I think we may not presume to know better than
the author of the regex about the greediness of his quantifiers.

 Another thing I've seen people complain about is that an alternation
 (anything with top-level |) is always taken as greedy overall.

Yeah, that is quirky.

Cheers,
BJ

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

[HACKERS] Our regex vs. POSIX on longest match

[Brendan Jurd]

Hello folks,

I am in the process of accelerating down the rabbit hole of regex
internals.  Something that came up during my reading, is that a POSIX
compliant regex engine ought to always prefer the longest possible
match, when multiple matches are possible beginning from the same
location in the string. [1]

I wasn't sure that that was how our regex engine worked, and indeed,
on checking the manual [2] I found that our regex engine uses a
strange sort of inductive greediness to determine whether the
longest or the shortest possible match ought to be preferred.  The
greediness of individual particles in the regex are taken into
account, and at the top level the entire expression is concluded to be
either greedy, or non-greedy.

I'll admit that this is a pretty obscure point, but we do appear to be
in direct violation of POSIX here.  I am getting the impression that
our engine works this way to route around some of the performance
issues that can arise in trying out every possible match with an
NFA-style engine.

I find it a bit unfortunate that POSIX is so unambiguous about this,
while our engine's treatment is, well ... quite arcane by comparison.
At minimum, I think we should be more explicit in the manual that this
behaviour flips POSIX the proverbial bird.  Several paragraphs south,
there is a sentence reading Incompatibilities of note include ... the
longest/shortest-match (rather than first-match) matching semantics,
but in the context it seems as though the author is talking about an
incompatibility with Perl, not with POSIX.

Thoughts?

Cheers,
BJ

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html
[2] 
http://www.postgresql.org/docs/9.0/static/functions-matching.html#FUNCTIONS-POSIX-REGEXP

-- 
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] Future of our regular expression code

[Brendan Jurd]

On 19 February 2012 15:49, Tom Lane t...@sss.pgh.pa.us wrote:
 That sounds great.

 BTW, if you don't have it already, I'd highly recommend getting a copy
 of Friedl's Mastering Regular Expressions.  It's aimed at users not
 implementers, but there is a wealth of valuable context information in
 there, as well as a really good not-too-technical overview of typical
 implementation techniques for RE engines.  You'd probably still want one
 of the more academic presentations such as the dragon book for
 reference, but I think Freidl's take on it is extremely useful.

Thanks for the recommendations Tom.  I've now got Friedl, and there's
a dead-tree copy of 'Compilers' making its gradual way to me (no
ebook).
I've also been reading the article series by Russ Cox linked upthread
-- it's good stuff.

Are you far enough into the backrefs bug that you'd prefer to see it
through, or would you like me to pick it up?

Cheers,
BJ

-- 
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] Future of our regular expression code

[Brendan Jurd]

On 20 February 2012 10:42, Tom Lane t...@sss.pgh.pa.us wrote:
 I have also got
 a bunch of text about the colormap management code, which I think
 is interesting right now because that is what we are going to have
 to fix if we want decent performance for Unicode \w and related
 classes (cf the other current -hackers thread about regexes).
 I was hoping to prevail on you to pick that part up as your first
 project.  I will commit what I've got in a few minutes --- look
 for src/backend/regex/README in that commit.

Okay, I've read through your README content, it was very helpful.
I'll now go chew through some more reading material and then start
studying our existing regex source code.  Once I'm firing on all
cylinders with this stuff, I'll begin to tackle the colormap.

Cheers,
BJ

-- 
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] Future of our regular expression code

[Brendan Jurd]

On 19 February 2012 06:52, Tom Lane t...@sss.pgh.pa.us wrote:
 Yeah ... if you *don't* know the difference between a DFA and an NFA,
 you're likely to find yourself in over your head.  Having said that,
 this is eminently learnable stuff and pretty self-contained, so somebody
 who had the time and interest could make themselves into an expert in
 a reasonable amount of time.

I find myself in possession of both time and interest.  I have to
admit up-front that I don't have experience with regex code, but I do
have some experience with parsers generally, and I'd like to think
some of that skillset would transfer to this problem.  I also find
regexes fascinating and extremely useful, so learning more about them
will be no hardship.

I'd happily cede to an expert, should one appear, but otherwise I'm
all for moving the regex code into a discrete library, and I'm
volunteering to take a swing at it.

Cheers,
BJ

-- 
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] Arithmetic operators for macaddr type

[Brendan Jurd]

On 12 December 2011 15:59, Pavel Stehule pavel.steh...@gmail.com wrote:
 2011/12/12 Brendan Jurd dire...@gmail.com:
 I just bumped into a situation where I wanted to do a little macaddr
 arithmetic in postgres.  I note that the inet type has support for
 bitwise AND, OR and NOT, as well as subtraction, but macaddr has none
 of the above.

 +1


Here is a patch for $SUBJECT.  I merely added support for ~,  and |
operators for the macaddr type.  The patch itself is rather trivial,
and includes regression tests and a doc update.

For the documentation, I did think about adding a new table for the
macaddr operators, but in the end decided it would probably be an
overkill.  If others think a table would be better, I'm happy to
revise it.

I also considered adding a function which would return the numeric
value of the MAC as a bigint, but figured I might save that for a
separate patch.

Cheers,
BJ
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8300,8306  CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple
 para
  The typemacaddr/type type also supports the standard relational
  operators (literalgt;/literal, literallt;=/literal, etc.) for
! lexicographical ordering.
 /para
  
/sect1
--- 8300,8308 
 para
  The typemacaddr/type type also supports the standard relational
  operators (literalgt;/literal, literallt;=/literal, etc.) for
! lexicographical ordering, and the bitwise arithmetic operators
! (literal~/literal, literalamp;/literal and literal|/literal)
! for NOT, AND and OR.
 /para
  
/sect1
*** a/src/backend/utils/adt/mac.c
--- b/src/backend/utils/adt/mac.c
***
*** 242,247  hashmacaddr(PG_FUNCTION_ARGS)
--- 242,300 
  }
  
  /*
+  * Arithmetic functions: bitwise NOT, AND, OR.
+  */
+ Datum
+ macaddr_not(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result-a = ~addr-a;
+ 	result-b = ~addr-b;
+ 	result-c = ~addr-c;
+ 	result-d = ~addr-d;
+ 	result-e = ~addr-e;
+ 	result-f = ~addr-f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ Datum
+ macaddr_and(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr1 = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *addr2 = PG_GETARG_MACADDR_P(1);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result-a = addr1-a  addr2-a;
+ 	result-b = addr1-b  addr2-b;
+ 	result-c = addr1-c  addr2-c;
+ 	result-d = addr1-d  addr2-d;
+ 	result-e = addr1-e  addr2-e;
+ 	result-f = addr1-f  addr2-f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ Datum
+ macaddr_or(PG_FUNCTION_ARGS)
+ {
+ 	macaddr	   *addr1 = PG_GETARG_MACADDR_P(0);
+ 	macaddr	   *addr2 = PG_GETARG_MACADDR_P(1);
+ 	macaddr	   *result;
+ 
+ 	result = (macaddr *) palloc(sizeof(macaddr));
+ 	result-a = addr1-a | addr2-a;
+ 	result-b = addr1-b | addr2-b;
+ 	result-c = addr1-c | addr2-c;
+ 	result-d = addr1-d | addr2-d;
+ 	result-e = addr1-e | addr2-e;
+ 	result-f = addr1-f | addr2-f;
+ 	PG_RETURN_MACADDR_P(result);
+ }
+ 
+ /*
   *	Truncation function to allow comparing mac manufacturers.
   *	From suggestion by Alex Pilosov a...@pilosoft.com
   */
*** a/src/include/catalog/pg_operator.h
--- b/src/include/catalog/pg_operator.h
***
*** 1116,1121  DESCR(greater than);
--- 1116,1128 
  DATA(insert OID = 1225 (  =	   PGNSP PGUID b f f 829 829	 16 1223 1222 macaddr_ge scalargtsel scalargtjoinsel ));
  DESCR(greater than or equal);
  
+ DATA(insert OID = 3141 (  ~	   PGNSP PGUID l f f	  0 829 829 0 0 macaddr_not - - ));
+ DESCR(bitwise not);
+ DATA(insert OID = 3142 (  	   PGNSP PGUID b f f	829 829 829 0 0 macaddr_and - - ));
+ DESCR(bitwise and);
+ DATA(insert OID = 3143 (  |	   PGNSP PGUID b f f	829 829 829 0 0 macaddr_or - - ));
+ DESCR(bitwise or);
+ 
  /* INET type (these also support CIDR via implicit cast) */
  DATA(insert OID = 1201 (  =	   PGNSP PGUID b t t 869 869	 16 1201 1202 network_eq eqsel eqjoinsel ));
  DESCR(equal);
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 2037,2042  DATA(insert OID = 834 (  macaddr_ge			PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 16
--- 2037,2045 
  DATA(insert OID = 835 (  macaddr_ne			PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 16 829 829 _null_ _null_ _null_ _null_	macaddr_ne _null_ _null_ _null_ ));
  DATA(insert OID = 836 (  macaddr_cmp		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 23 829 829 _null_ _null_ _null_ _null_	macaddr_cmp _null_ _null_ _null_ ));
  DESCR(less-equal-greater);
+ DATA(insert OID = 3138 (  macaddr_not		PGNSP PGUID 12 1 0 0 0 f f f t f i 1 0 829 829 _null_ _null_ _null_ _null_	macaddr_not _null_ _null_ _null_ ));
+ DATA(insert OID = 3139 (  macaddr_and		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 829 829 829 _null_ _null_ _null_ _null_	macaddr_and _null_ _null_ _null_ ));
+ DATA(insert OID = 3140 (  macaddr_or		PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 829 829 829 _null_ _null_ _null_

[HACKERS] Arithmetic operators for macaddr type

[Brendan Jurd]

Hello folks,

I just bumped into a situation where I wanted to do a little macaddr
arithmetic in postgres.  I note that the inet type has support for
bitwise AND, OR and NOT, as well as subtraction, but macaddr has none
of the above.

These operations are easy to perform in C, but relatively a pain to do
in SQL, especially as there doesn't seem to be a direct way to get a
macaddr into a plain numeric form.

I can't see any reason why postgres shouldn't support these operations
on macaddr.  I'd like to add them as fully realised operators in core.
 Would that be acceptable?

Cheers,
BJ

-- 
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] Thoughts on SELECT * EXCLUDING (...) FROM ...?

[Brendan Jurd]

On 1 November 2011 00:14, Andrew Dunstan and...@dunslane.net wrote:
 On 10/30/2011 10:00 PM, Christopher Browne wrote:
 I don't think I wish it.  We're telling our developers not to use select
 *, and I don't think having select * except  would change that policy,
 beyond requiring us to waste time explaining :

 No, we're not changing policy.  The fact that PGDG added this to 9.2 does
 *not* imply our policy was wrong.


 That's fine, and it's a good policy. A good policy might well exclude use of
 a number of available features (e.g. one place I know bans doing joins with
 ',' instead of explicit join operators). But I don't think it helps us
 decide what to support.

 The fact is that if you have 100 columns and want 95 of them, it's very
 tedious to have to specify them all, especially for ad hoc queries where the
 house SQL standards really don't matter that much.

I couldn't agree more with Andrew's comment.  What's good for an ad
hoc psql query isn't congruent with what's good for your application
queries.

We could have  * EXCLUDING  and still say that it is undesirable in
all the same contexts that  *  is undesirable.

Cheers,
BJ

-- 
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] Is there really no interest in SQL Standard?

[Brendan Jurd]

On 16 September 2011 16:24, Susanne Ebrecht susa...@2ndquadrant.com wrote:
 Isn't it possible to create a closed mailing list - a list that won't get
 published - on which
 I can discuss SQL Standard stuff with the folk who wants to support me?

 I don't fear to make decisions on my own - but speaking for the whole
 project without
 getting feedback - is a worse feeling.

 Usually, when I feel unsure how I should decide I just bother Peter - but I
 would prefer
 to have some more ppl in my background.

I for one would definitely be interested in reading such a list.

Cheers,
BJ

-- 
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] Moving core timestamp typedefs/macros somewhere else

[Brendan Jurd]

On 8 September 2011 10:22, Tom Lane t...@sss.pgh.pa.us wrote:
 If you believe the idea I suggested a few days ago that we ought to try
 to push basic typedefs into a separate set of headers, then this could
 be the first instance of that, which would lead to naming it something
 like datatype/timestamp.h.  If that seems premature, then I guess it
 ought to go into utils/, but then we need some other name because
 utils/timestamp.h is taken.

The separate headers for basic typedefs makes perfect sense to me.

Cheers,
BJ

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

Re: [HACKERS] postgresql.conf archive_command example

[Brendan Jurd]

On 31 August 2011 04:39, Peter Eisentraut pete...@gmx.net wrote:
 I think it would be useful to add the following explanation and sample
 to the postgresql.conf sample file:


Good idea Peter, +1.

Cheers,
BJ

-- 
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] extract(epoch from infinity) is not 0

[Brendan Jurd]

On 14 July 2011 06:58, Alvaro Herrera alvhe...@commandprompt.com wrote:
 I don't find the proposed behavior all that suprising, which the
 original behavior surely is.  I guess the bigger question is whether the
 values that timestamptz_part() returns for other cases (than epoch)
 should also be different from 0 when an 'infinity' timestamp is passed.
 (In other words, why should 0 be the assumed return value here?)


Well, for example, how do you go about answering the question what is
the day-of-month of the infinite timestamp?  The question is
nonsense; it doesn't have a defined day of month, so I think we should
be returning NULL or throwing an error.  Returning zero is definitely
wrong.  I think throwing an error is the better way to go, as the user
probably didn't intend to ask an incoherent question.

It makes sense to special-case 'epoch' because it effectively converts
the operation into interval math; if we ask how many seconds from
1970-01-01 00:00 UTC until the infinite timestamp? the answer is
genuinely infinite seconds.  So +1 for the proposed change for
epoch, and let's throw an error for the other date fields instead of
returning zero.

Cheers,
BJ

-- 
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] extract(epoch from infinity) is not 0

[Brendan Jurd]

On 14 July 2011 08:16, Robert Haas robertmh...@gmail.com wrote:
 On Jul 13, 2011, at 4:21 PM, Brendan Jurd dire...@gmail.com wrote:
 Well, for example, how do you go about answering the question what is
 the day-of-month of the infinite timestamp?  The question is
 nonsense; it doesn't have a defined day of month, so I think we should
 be returning NULL or throwing an error.  Returning zero is definitely
 wrong.  I think throwing an error is the better way to go, as the user
 probably didn't intend to ask an incoherent question.

 It makes sense to special-case 'epoch' because it effectively converts
 the operation into interval math; if we ask how many seconds from
 1970-01-01 00:00 UTC until the infinite timestamp? the answer is
 genuinely infinite seconds.  So +1 for the proposed change for
 epoch, and let's throw an error for the other date fields instead of
 returning zero.

 I'd rather we avoid throwing an error, because that sometimes forces people 
 who want to handle that case to use a subtransaction to catch it, which is 
 quite slow.

SELECT CASE WHEN isfinite(ts) THEN extract(day from ts) ELSE NULL END

Cheers,
BJ

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

Re: [HACKERS] minor patch submission: CREATE CAST ... AS EXPLICIT

[Brendan Jurd]

On 18 June 2011 09:49, Brendan Jurd dire...@gmail.com wrote:
 Hi Fabien,

 I'm taking a look at this patch for the commitfest.  On first reading
 of the patch, it looked pretty sensible to me, but I had some trouble
 applying it to HEAD:

 error: patch failed: doc/src/sgml/ref/create_cast.sgml:20
 error: doc/src/sgml/ref/create_cast.sgml: patch does not apply
 error: patch failed: src/backend/parser/gram.y:499
 error: src/backend/parser/gram.y: patch does not apply
 error: patch failed: src/include/parser/kwlist.h:148
 error: src/include/parser/kwlist.h: patch does not apply
 error: patch failed: src/test/regress/expected/create_cast.out:27
 error: src/test/regress/expected/create_cast.out: patch does not apply
 error: patch failed: src/test/regress/sql/create_cast.sql:27
 error: src/test/regress/sql/create_cast.sql: patch does not apply

 Perhaps the patch could use a refresh?

The author has yet to reply to the above -- we are still lacking a
patch version that applies cleanly to HEAD.  I have marked this patch
'Waiting on Author'.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 24 June 2011 03:48, Alvaro Herrera alvhe...@commandprompt.com wrote:
 I have touched next_token() and next_token_expand() a bit more, because
 it seemed to me that they could be simplified further (the bit about
 returning the comma in the token, instead of being a boolean return,
 seemed strange).  Please let me know what you think.

Cool.  I didn't like the way it returned the comma either, but I
thought it would be best if I kept the changes in my patch to a
minimum.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 22 June 2011 00:47, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011:
 because pamservice - is known keyword, but 'pamservice' is some
 literal without any mean. You should to use a makro token_is_keyword
 more often.

 Yeah, I wondered about this too (same with auth types, i.e. do we accept
 quoted hostssl and so on or should that by rejected?).  I opted for
 leaving it alone, but maybe this needs to be fixed.  (Now that I think
 about it, what we should do first is verify whether it works with quotes
 in the unpatched code).

 AFAICS, this is only important in places where the syntax allows either
 a keyword or an identifier.  If only a keyword is possible, there is no
 value in rejecting it because it's quoted.  And, when you do the test,
 I think you'll find that it would be breaking hba files that used to
 work (though admittedly, it's doubtful that there are any such in the
 field).

Yes, I agree, and this was my thinking when I came up against this
while writing the original patch.  It doesn't help to treat hostssl
differently than hostssl, because quoted identifiers are meaningless
in that context.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 22 June 2011 14:01, Pavel Stehule pavel.steh...@gmail.com wrote:
 ook, now is clean, so this is majority opinion.

 Please, can you send a final patch.

I don't have any further changes to add to Alvaro's version 3, which
is already up on the CF app.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 21 June 2011 06:06, Alvaro Herrera alvhe...@commandprompt.com wrote:
 Excerpts from Alvaro Herrera's message of lun jun 20 12:19:37 -0400 2011:
 Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011:

  b) probably you can simplify a memory management using own two
  persistent memory context - and you can swap it. Then functions like
  free_hba_record, clean_hba_list, free_lines should be removed.

 Yeah, I reworked the patch with something like that over the weekend.
 Not all of it though.  I'll send an updated version shortly.

 Here it is, please let me know what you think.  I took the liberty of
 cleaning up some things that were clearly historical leftovers.


Okay, yeah, the MemoryContext approach is far more elegant than what I
had.  Boy was I ever barking up the wrong tree.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 21 June 2011 11:11, Alvaro Herrera alvhe...@commandprompt.com wrote:
 I realize I took out most of the fun of this patch from you, but -- are
 you still planning to do some more exhaustive testing of it?  I checked
 some funny scenarios (including include files and groups) but it's not
 all that unlikely that I missed some cases.  I also didn't check any
 auth method other than ident, md5 and trust, 'cause I don't have what's
 required for anything else.  (It's a pity that the regression tests
 don't exercise anything else.)

I've pretty much tested all the things I can think to test, and I'm in
the same boat as you re auth methods.  If there are bugs, I think they
will be of the obscure kind, that only reveal themselves under
peculiar circumstances.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 21 June 2011 13:51, Pavel Stehule pavel.steh...@gmail.com wrote:
 I have one question. I can't find any rules for work with tokens, etc,
 where is quotes allowed and disallowed?

 I don't see any other issues.

I'm not sure I understand your question, but quotes are allowed
anywhere and they always act to remove any special meaning the token
might otherwise have had.  It's much like quoting a column name in
SQL.

I didn't change any of the hba parsing rules, so escaping and whatnot
should all work the way it did before.  The only difference should be
that after parsing, keywords will only be evaluated for fields where
they matter.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 21 June 2011 14:34, Pavel Stehule pavel.steh...@gmail.com wrote:
 I don't understand to using a macro

 #define token_is_keyword(t, k)  (!t-quoted  strcmp(t-string, k) == 0)

 because you disallowed a quoting?

Well, a token can only be treated as a special keyword if it is unquoted.

As an example, in the 'database' field, the bare token 'replication'
is a keyword meaning the pseudo-database for streaming rep.  Whereas
the quoted token replication would mean a real database which is
called 'replication'.

Likewise, the bare token 'all' in the username field is a keyword --
it matches any username.  Whereas the quoted token all would only
match a user named 'all'.

Therefore, token_is_keyword only returns true where the token matches
the given string as is also unquoted.

Does that make sense?

Cheers,
BJ

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

[HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 16 June 2011 00:22, Pavel Stehule pavel.steh...@gmail.com wrote:
 I try to apply your patch, but it is finished with some failed hinks.

 Please, can you refresh your patch

Hi Pavel,

Thanks for taking a look.  I have attached v2 of the patch, as against
current HEAD.  I've also added the new patch to the CF app.  I look
forward to your comments.

Cheers,
BJ


hba-keywords-v2.diff.bz2
Description: BZip2 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] minor patch submission: CREATE CAST ... AS EXPLICIT

[Brendan Jurd]

On 22 May 2011 07:27, Fabien COELHO coe...@cri.ensmp.fr wrote:

 Hello Tom,

 Add AS EXPLICIT to CREATE CAST This gives a name to the default case
 of CREATE CAST, which creates a cast which must be explicitely invoked.

 I'm not sure this is a good idea.  The CREATE CAST syntax is in the SQL
 standard, and this isn't it.  Now I realize that we've extended that
 statement already to cover some functionality that's not in the
 standard, but that doesn't mean we should create unnecessarily
 nonstandard syntax for cases that are in the standard.

 The standard provides only one case, so CAST is good enough a name.

 Once you start creating alternatives with distinct semantics, then it helps
 to give the initial one a name as well to be able to discuss them with
 something else that the remaining case, or when there is no option,
 especially as there is something to discuss.

 Note that the standard is still supported just the same, and the
 documentation already underlines that AS * stuff is a pg extension,
 nothing is really changed. Maybe the documentation could be clearer about
 where the standard stops and where extensions start, even now without an AS
 EXPLICIT clause.

 If a commercial vendor did that, wouldn't you castigate them for trying to
 create vendor lock-in?

 I'm more concerned with explaining things to students, and its good to have
 words and logic for that.

 With respect to the standard, it seems good enough to me if (1) the standard
 is well supported and (2) the documentation clearly says which parts are
 extensions. If you really want to keep to the standard, then do not offer
 any extension.

 Moreover, this stuff is really minor compared to RULEs or many other things
 specifics to pg, and the lock-in is light, you just have to remove AS
 EXPLICIT to get away, no big deal.


Hi Fabien,

I'm taking a look at this patch for the commitfest.  On first reading
of the patch, it looked pretty sensible to me, but I had some trouble
applying it to HEAD:

error: patch failed: doc/src/sgml/ref/create_cast.sgml:20
error: doc/src/sgml/ref/create_cast.sgml: patch does not apply
error: patch failed: src/backend/parser/gram.y:499
error: src/backend/parser/gram.y: patch does not apply
error: patch failed: src/include/parser/kwlist.h:148
error: src/include/parser/kwlist.h: patch does not apply
error: patch failed: src/test/regress/expected/create_cast.out:27
error: src/test/regress/expected/create_cast.out: patch does not apply
error: patch failed: src/test/regress/sql/create_cast.sql:27
error: src/test/regress/sql/create_cast.sql: patch does not apply

Perhaps the patch could use a refresh?

Also, for what it's worth, I buy into the argument for adding AS
EXPLICIT.  This stuff is all an extension to the SQL standard already;
it might as well have a well-rounded syntax.

Cheers,
BJ

-- 
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] Fwd: Keywords in pg_hba.conf should be field-specific

[Brendan Jurd]

On 18 June 2011 13:43, Alvaro Herrera alvhe...@commandprompt.com wrote:
 Is this really a WIP patch?  I'm playing a bit with it currently, seems
 fairly sane.


In this case, the WIP designation is meant to convey warning: only
casual testing has beeen done.  I tried it out with various
permutations of pg_hba.conf, and it worked as advertised in those
tests, but I have not made any attempt to formulate a more rigorous
testing regimen.

In particular I haven't tested that the more exotic authentication
methods still work properly, and I can't recall whether I tested
recursive file inclusion and group membership.

Is that a wrongful use of the WIP designation?

Cheers,
BJ

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

[HACKERS] DOCS: SGML identifier may not exceed 44 characters

[Brendan Jurd]

Hi folks,

I was working on a little docs patch today, and when I tried to
`make`, openjade choked on an identifier in information_schema.sgml,
which is very much unrelated to my changes:

openjade:information_schema.sgml:828:60:Q: length of name token must
not exceed NAMELEN (44)

Here is a trivial patch to shut openjade up.  This particular id does
not appear to be referred to anywhere else in the docs yet.

The identifier appears to have been introduced in commit
2e2d56fea97f43cf8c40a87143bc10356e4ed4d4 on Feb 9 this year.

I'm using openjade 1.3.2.

Cheers,
BJ

diff --git a/doc/src/sgml/information_schema.sgml
b/doc/src/sgml/information_schema.sgml
index 2febb4c..5fdbd51 100644
--- a/doc/src/sgml/information_schema.sgml
+++ b/doc/src/sgml/information_schema.sgml
@@ -825,7 +825,7 @@
   /table
  /sect1

- sect1 id=infoschema-collation-character-set-applicability
+ sect1 id=infoschema-collation-charset-applicability
   titleliteralcollation_character_set_applicability/literal/title

   para

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

Re: [DOCS] [HACKERS] DOCS: SGML identifier may not exceed 44 characters

[Brendan Jurd]

On 1 June 2011 06:36, Peter Eisentraut pete...@gmx.net wrote:
 It looks like the original DocBook distribution has a limit of 44, but
 someone patched it to 256 on your installation.

 But it seems like no one else has seen this problem yet, so it's quite
 suspicious, since surely people have built the documentation in the last
 few months.


The relevant value on my machine seems to be:

/usr/share/sgml/docbook/sgml-dtd-4.2/docbook.dcl:81:  NAMELEN44

This file belongs to the Gentoo package app-text/docbook-sgml-dtd
4.2-r2, which is the current stable version for the 4.2 slot.

I would hazard to suggest that nobody else is seeing this problem
because I'm the only one building the docs on Gentoo =)

Still, the 44 character limit does seem to be per the upstream
distribution, and the identifier I patched above is the first one to
violate it.  Are there any solid reasons we shouldn't just comply with
it?

Cheers,
BJ

-- 
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] Getting a bug tracker for the Postgres project

[Brendan Jurd]

On 31 May 2011 11:52, Robert Haas robertmh...@gmail.com wrote:
 I have used RT and I found that the
 web interface was both difficult to use and unwieldly for tickets
 containing large numbers of messages.

A big loud ditto from me on this point.

Cheers,
BJ

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