Re: [HACKERS] [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)

[David G. Johnston]

On Thu, Mar 19, 2015 at 3:41 PM, David Christensen 
wrote:

> The two-arg form of the current_setting() function will allow a
> fallback value to be returned instead of throwing an error when an
> unknown GUC is provided.  This would come in most useful when using
> custom GUCs; e.g.:
>
>   -- returns current setting of foo.bar, or 'default' if not set
>   SELECT current_setting('foo.bar', 'default')
>

​​This doesn't actually change the GUC in the system, right?  Do we want a
side-effect version of this?

There is already a two-arg form where the second argument is a boolean -
there needs to be tests that ensure proper behavior and function lookup
resolution.

No docs.

David J.
​

Re: [HACKERS] Account for cost and selectivity of HAVING quals

[David G. Johnston]

On Tue, Oct 31, 2017 at 4:31 PM, Tels  wrote:

>
> ​​
> That looks odd to me, it first uses output_tuples in a formula, then
> overwrites the value with a new value. Should these lines be swapped?
>

​IIUC it is correct: the additional total_cost comes from processing every
output group to check whether it is qualified - since every group is
checked the incoming output_tuples from the prior grouping is used.  The
side-effect of the effort is that the number of output_tuples has now been
reduced to only those matching the qual - and so it now must take on a new
value to represent this.

David J.​

Re: [HACKERS] PostgreSQL 10 parenthesized single-column updates can produce errors

[David G. Johnston]

On Tue, Oct 31, 2017 at 3:43 PM, Tom Lane  wrote:

> According to the spec, the elements of a parenthesized
> SET list should be assigned from the fields of a composite RHS.  If
> there's just one element of the SET list, the RHS should be a single-field
> composite value, and this syntax should result in extracting its field.
> This patch doesn't do that; it preserves our previous broken behavior,
> and thus just puts off the day of reckoning that inevitably will come.
>

​Definitely moderates my opinion in my concurrent emai​l...though
postponement is not strictly bad given the seeming frequency of the
existing problematic syntax in the wild already.

David J.

Re: [HACKERS] PostgreSQL 10 parenthesized single-column updates can produce errors

[David G. Johnston]

On Tue, Oct 31, 2017 at 3:14 PM, Rob McColl  wrote:

>
>> I believe that this is not an intended change or behavior, but is instead
>> an unintentional side effect of 906bfcad7ba7cb3863fe0e2a7810be8e3cd84fbd
>> Improve handling of "UPDATE ... SET (column_list) = row_constructor". (
>> https://github.com/postgres/postgres/commit/906bfcad7ba7cb3
>> 863fe0e2a7810be8e3cd84fbd).
>>
>
​At this point the intent of 906bfcad doesn't really matter - and given the
number of complaints in the month since ​v10 went live I'm tending to lean
toward bringing the pre-10 behavior back. There is no ambiguity involved
here and the breakage of existing applications seems considerably worse
than the technical oddity of allowing (val) to be interpreted as a
row_constructor in this situation.  From a standards perspective we are
strictly more permissive so no new problem there.

On a related note, the 10.0 syntax guide is wrong, it needs to break out
the parenthesized single-column and multi-column variants separately:

Presently: ( column_name [, ...] ) = [ ROW ] ( { expression | DEFAULT } [,
...] )

Basically one cannot specify only a single column_name AND omit ROW

It would have been nice if the syntax for that variant would have been:

( column_name, column_name [, ...] ) = [ ROW ] ( { expression | DEFAULT },
{ expression | DEFAULT } [, ...] )
​
If the v10 behavior remains the above change should be made as well as
adding:

( column_name ) = ROW ( expression | DEFAULT )

If we revert 10 to the pre-10 behavior the existing syntax will work.

David J.

[HACKERS] Remove inbound links to sql-createuser

[David G. Johnston]

Since CREATE USER is officially an alias for CREATE ROLE other parts of the
documentation should point to CREATE ROLE, not CREATE USER.  Most do but I
noticed when looking at CREATE DATABASE that it did not.  Further searching
turned up the usage in client-auth.sgml.  That one is questionable since we
are indeed talking about LOGIN roles there but we are already pointing to
sql-alterrole instead of sql-alteruser and so changing the create variation
to point to sql-createrole seems warranted.

Attached, and below.

David J.

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 722f3da813..99921ba079 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -998,9 +998,9 @@ omicron bryanh  guest1
 separate from operating system user passwords. The password for
 each database user is stored in the pg_authid system
 catalog. Passwords can be managed with the SQL commands
- and
+ and
 ,
-e.g., CREATE USER foo WITH PASSWORD 'secret',
+e.g., CREATE ROLE foo WITH LOGIN PASSWORD
'secret',
 or the psql
 command \password.
 If no password has been set up for a user, the stored password
diff --git a/doc/src/sgml/ref/create_database.sgml
b/doc/src/sgml/ref/create_database.sgml
index 3e35c776ea..f63f1f92ac 100644
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -45,7 +45,7 @@ CREATE DATABASE name
   
To create a database, you must be a superuser or have the special
CREATEDB privilege.
-   See .
+   See .
   

   


cleanup-createuser-links.patch
Description: Binary data

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

Re: [HACKERS] Interest in a SECURITY DEFINER function current_user stack access mechanism?

[David G. Johnston]

On Wed, Oct 18, 2017 at 2:30 PM, Nico Williams <n...@cryptonector.com>
wrote:

> On Wed, Oct 18, 2017 at 02:13:29PM -0700, David G. Johnston wrote:
> > > More useful than this, for me, would be a way to get the top-most user.
> >
> > That would be "session_user"?
>
> It's not quite since there's a difference between SET SESSION
> AUTHORIZATION and SET SESSION ROLE.
>
> But yes, it's what I'm using now.
>

​True, though at that point the superuser who wants to cover their tracks
could probably just edit your functions...​


> Really?  Why?  I mean, there's an implicit function invocation stack
> already.  Reifying some bits of the function call stack is useful.  I
> can't think of how this particular reification would be dangerous or set
> a bad precedent.
>

​Nothing concrete...​
​

>
> Hmmm, oh, I forgot about GET DIAGNOSTICS!  The stack is already exposed
> to SQL.  Maybe we could add a CURRENT_USER item to GET STACKED
> DIAGNOSTICS or to the PG_CONTEXT.
>

Ideally if implementing what you describe we'd want it accessible from any
procedural language​, not just pl/pgsql.

Also, GET STACKED DIAGNOSTICS is documented as being exposed only within an
exception handler.


> > If I was in position to dive deeper I wouldn't foreclose on the stack
> idea
> > but I'd be inclined to see if something else could be made to work with
> > reasonable effort.
>
> I would think that the more general approach, if easy enough to
> implement, would be better.  I can (and will) live with using
> session_user instead of current_user, for now.  But I'm willing to
> contribute a patch


​I'd probably expose the stack as an array...

David J.
​

Re: [HACKERS] Interest in a SECURITY DEFINER function current_user stack access mechanism?

[David G. Johnston]

On Wed, Oct 18, 2017 at 2:08 PM, Nico Williams <n...@cryptonector.com>
wrote:

> On Wed, Oct 18, 2017 at 01:43:30PM -0700, David G. Johnston wrote:
>
> More useful than this, for me, would be a way to get the top-most user.
>
>
​That would be "session_user"?​

> Introducing the concept of a stack at the SQL level here seems, at
> > first glance, to be over-complicating things.
>
> Because of the current implementation of invocation of SECURITY DEFINER
> functions, a stack is trivial to build, since it's a list of nodes
> allocated on the C stack in fmgr_security_definer().
>

​Not saying its difficult (or not) to code in C; but exposing that to SQL
seems like a big step.

If I was in position to dive deeper I wouldn't foreclose on the stack idea
but I'd be inclined to see if something else could be made to work with
reasonable effort.

David J.​

Re: [HACKERS] Interest in a SECURITY DEFINER function current_user stack access mechanism?

[David G. Johnston]

On Wed, Oct 18, 2017 at 1:26 PM, Nico Williams 
wrote:

> On Wed, Oct 18, 2017 at 10:15:01PM +0200, Pavel Stehule wrote:
> > there is a function session_user() already
>
> But it doesn't do this.  Are you saying that I should add a
> session_user(int)?
>
>
​Regardless of the merits of the proposed feature, the function
"session_user" is SQL-defined and should not be modified or enhanced.

I could see "calling_role()" being useful - it returns the same value as
"current_role" normally and in security invoker functions while in a
security definer function it would return whatever current_role would have
returned if the function was a security invoker (i.e., the role that the
system will put back into effect once the security definer function
returns).

Introducing the concept of a stack at the SQL level here seems, at first
glance, to be over-complicating things.

David J.

[HACKERS] v10 telease note for pg_basebackup refers to old --xlog-method argument

[David G. Johnston]

diff --git a/doc/src/sgml/release-10.sgml b/doc/src/sgml/release-10.sgml
index 116f7224da..f1f7cfed5f 100644
--- a/doc/src/sgml/release-10.sgml
+++ b/doc/src/sgml/release-10.sgml
@@ -242,7 +242,7 @@

  
   This changes pg_basebackup's
-  -X/--xlog-method default to
stream.
+  -X/--wal-method default to
stream.
   An option value none has been added to reproduce
the old
   behavior.  The pg_basebackup option
-x
   has been removed (instead, use -X fetch).

Also attached.

David J.
diff --git a/doc/src/sgml/release-10.sgml b/doc/src/sgml/release-10.sgml
index 116f7224da..f1f7cfed5f 100644
--- a/doc/src/sgml/release-10.sgml
+++ b/doc/src/sgml/release-10.sgml
@@ -242,7 +242,7 @@
 
  
   This changes pg_basebackup's
-  -X/--xlog-method default to 
stream.
+  -X/--wal-method default to 
stream.
   An option value none has been added to reproduce the 
old
   behavior.  The pg_basebackup option 
-x
   has been removed (instead, use -X fetch).

-- 
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] search path security issue?

[David G. Johnston]

On Thu, Oct 5, 2017 at 3:05 PM, Joshua D. Drake <j...@commandprompt.com>
wrote:

> On 10/05/2017 02:54 PM, David G. Johnston wrote:
>
>> On Thu, Oct 5, 2017 at 2:37 PM, Joshua D. Drake <j...@commandprompt.com
>> <mailto:j...@commandprompt.com>>wrote:
>>
>> I get being able to change my search_path on the fly but it seems
>> odd that as user foo I can change my default search path?
>>
>>
>> Seems down-right thoughtful of us to allow users to change their own
>> defaults instead of forcing them to always change things on-the-fly or bug
>> a DBA to change the default for them.
>>
>
> It seems that if a super user changes the search path with ALTER
> USER/ROLE, then the user itself should not (assuming not an elevated
> privilege) should not be able to change it. Again, I get being able to do
> it with SET but a normal user shouldn't be able to reset a super user
> determined setting.
>

If the superuser really wanted to enforce it they could, a bit verbosely,
set the default for the user for every database explicitly so that the
database+role setting takes precedence over the role-only setting.​

I get where you are coming from but there is no demonstrated
security-related reason to enforce such a restriction and so adding the
book-keeping necessary to track "who done it" (i.e. mere presence of a
value is an insufficient distinguishing characteristic) has a cost but no
benefit (if the superuser is upset that someone changed their own default
search_path the fact that said user is entrusted with login credentials
should be questioned).

David J.

Re: [HACKERS] search path security issue?

[David G. Johnston]

On Thu, Oct 5, 2017 at 2:37 PM, Joshua D. Drake 
wrote:

> I get being able to change my search_path on the fly but it seems odd that
> as user foo I can change my default search path?
>

Seems down-right thoughtful of us to allow users to change their own
defaults instead of forcing them to always change things on-the-fly or bug
a DBA to change the default for them.

David J.
​

Re: [HACKERS] Re: issue: record or row variable cannot be part of multiple-item INTO list

[David G. Johnston]

On Tue, Sep 19, 2017 at 11:29 AM, Tom Lane  wrote:

> ​T​
> hat
> ​ ​
> doesn't work today, and this patch doesn't fix it, but it does create
> enough confusion that we never would be able to fix it.
>
> I'd be much happier if there were some notational difference
> between I-want-the-composite-variable-to-absorb-a-composite-column
> and I-want-the-composite-variable-to-absorb-N-scalar-columns.
> For backwards compatibility with what happens now, the latter would
> have to be the default.


​So, using "()" syntax​

s,t: scalar text
c,d: (text, text)

treat all numbers below as text; and the ((1,2),) as ("(1,2)",)

A. SELECT 1 INTO s; -- today 1, this patch is the same

B. SELECT 1, 2 INTO s; -- today 1, this patch is the same

C. SELECT 1, 2 INTO (s); -- ERROR syntax - scalars cannot be tagged with
(), this patch N/A

D. SELECT 1, 2 INTO s, t; -- today 1, 2, this patch is the same

E. SELECT 1, 2 INTO c; -- today (1,2), this patch is the same

F. SELECT 1, 2 INTO (c); --ERROR "1" cannot be converted to (text, text),
this patch N/A

1. SELECT (1,2) INTO c; -- today ((1,2),); this patch is the same

2. SELECT (1,2) INTO (c); -- (1,2) -- this patch N/A

3. SELECT (1,2),(3,4) INTO c,d; -- ERROR syntax -- this patch gives [I
think...it can be made to give] (1,2),(3,4)

4. SELECT (1,2),(3,4) INTO c,(d); -- ERROR syntax -- this patch N/A

5. SELECT (1,2),(3,4) INTO (c),d; -- ERROR syntax -- this patch N/A

6. SELECT (1,2),(3,4) INTO (c),(d); -- (1,2),(3,4) -- this patch N/A

!. SELECT 1, (2,3), 4 INTO s, (c), t -- 1, (2,3), 4 -- this patch N/A
@. SELECT 1, 2, 3, 4 INTO s, (c), t -- ERROR "2" cannot be made into (text,
text) -- this patch N/A

IOW, this patch, if "c" is the only target (#1) and is composite pretend
the user wrote "INTO c.1, c.2" and assign each output column as a scalar in
one-by-one fashion.  If "c" is not the only target column (#3) assign a
single output column to it.  This maintains compatibility and clean syntax
at the cost of inconsistency.

The alternate, backward compatible, option introduces mandatory () in the
syntax for all composite columns in a multi-variable target (# 3-5 errors,
#6 valid) while it changes the behavior if present on a single variable
target (#1 vs #2).

So, allowing #3 to work makes implementing #2 even more unappealing.
Making only #2 and #6 work seems like a reasonable alternative position.

The last option is to fix #1 to return (1,2) - cleanly reporting an error
if possible, must like we just did with SRFs, and apply the patch thus
gaining both consistency and a clean syntax at the expense of limited
backward incompatibility.

Arrays not considered; single-column composites might end up looking like
scalars when presented to a (c) target.

Hope this helps someone besides me understand the problem space.

David J.

Re: [HACKERS] Re: issue: record or row variable cannot be part of multiple-item INTO list

[David G. Johnston]

On Tue, Sep 19, 2017 at 2:12 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:

> "David G. Johnston" <david.g.johns...@gmail.com> writes:
> > Actually, this does work, just not the way one would immediately expect.
>
> On closer inspection, what's actually happening in your example is that
> you're getting the SELECT's ct1 result crammed into the first column of
> c1, and then a default NULL is stuck into its second column:
>
> > ​ct1: (text, text)​
>
> > DO $$
> > SELECT ('1', '2')::ct1 INTO c1;
> > RAISE NOTICE '%', c1;
> > END;
> > $$;
>
> > ​Notice: ("(1,2)",)
>
> Notice the parens/comma positioning.  It's only because text is such
> a lax datatype that you didn't notice the problem.
>
>
​I saw exactly what you described - that it didn't error out and that the
text representation of the single output composite was being stored in the
first field of the target composite.  i.e., that it "worked".  Does that
fact that it only works if the first field of the composite type is text
change your opinion that the behavior is OK to break?

David J.

Re: [HACKERS] Re: issue: record or row variable cannot be part of multiple-item INTO list

[David G. Johnston]

On Tue, Sep 19, 2017 at 11:29 AM, Tom Lane  wrote:

> Aside from being inconsistent, it doesn't cover all
> the cases --- what if you have just one query output column, that is
> composite, and you'd like it to go into a composite variable?  That
> doesn't work today, and this patch doesn't fix it, but it does create
> enough confusion that we never would be able to fix it.
>
​
Actually, this does work, just not the way one would immediately expect.

​ct1: (text, text)​

DO $$
SELECT ('1', '2')::ct1 INTO c1;
RAISE NOTICE '%', c1;
END;
$$;

​Notice: ("(1,2)",)

And so, yes, my thinking has a backward compatibility problem.  But one
that isn't fixable when constrained by backward compatibility - whether
this patch goes in or not.

David J.

Re: [HACKERS] issue: record or row variable cannot be part of multiple-item INTO list

[David G. Johnston]

On Tuesday, September 19, 2017, Tom Lane  wrote:

> Pavel Stehule > writes:
> > 2017-09-14 12:33 GMT+02:00 Anthony Bykov  >:
> >> As far as I understand, this patch adds functionality (correct me if I'm
> >> wrong) for users. Shouldn't there be any changes in doc/src/sgml/ with
> the
> >> description of new functionality?
>
> > It removes undocumented limit. I recheck plpgsql documentation and there
> > are not any rows about prohibited combinations of data types.
>
> I remain of the opinion that this patch is a fundamentally bad idea.
> It creates an inconsistency between what happens if the INTO target list
> is a single record/row variable (it absorbs all the columns of the query
> output) and what happens if a record/row variable is part of a
> multi-variable target list (now it just absorbs one column, which had
> better be composite).  That's going to confuse people, especially since
> you haven't documented it.  But even with documentation, it doesn't seem
> like good design.  Aside from being inconsistent, it doesn't cover all
> the cases --- what if you have just one query output column, that is
> composite, and you'd like it to go into a composite variable?  That
> doesn't work today, and this patch doesn't fix it, but it does create
> enough confusion that we never would be able to fix it.
>
>
I think it's worth definitively addressing the limitations
noted, documenting how they are resolved/handled, and then give the
programmer more flexibility while, IMO, marginally increasing complexity.
For me we've programmed the "convenience case" and punted on the
technically correct solution.  i.e., we could have chosen to force the user
to write select (c1, c2)::ct into vct.

I'd be much happier if there were some notational difference
> between I-want-the-composite-variable-to-absorb-a-composite-column
> and I-want-the-composite-variable-to-absorb-N-scalar-columns.


If we change to considering exactly one output column for each target var
this seems unnecessary.  Then the one special case is today's single
composite column target and multiple output columns.  If there is only one
select column it has to be composite.

David J.

Re: [HACKERS] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur

[David G. Johnston]

On Thursday, September 14, 2017, Robert Haas  wrote:

> On Thu, Sep 14, 2017 at 3:23 AM, Tsunakawa, Takayuki
> > wrote:
> > Sorry again, but how can we handle this?  A non-PG-developer, Tels (and
> possibly someone else, IIRC) claimed that the behavior be changed during
> the beta period.  Why should we do nothing?
>
> Because we do not have consensus on changing it.  I've decided that
> you're right, but several other people are saying "no".  I can't just
> go change it in the face of objections.
>
>
Add my +1 for changing this to the official tally.

David J.

Re: [HACKERS] Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

[David G. Johnston]

On Thu, Sep 14, 2017 at 8:41 AM, Stephen Frost  wrote:

> Robert, all,
>
> * Robert Haas (robertmh...@gmail.com) wrote:
>
> > >
> > > I vote for rejecting it.  DDL compatibility is less valuable than other
> > > compatibility.  The hypothetical affected application can change its
> DDL to
> > > placate PostgreSQL and use that modified DDL for all other databases,
> too.
> >
> > OK.  Any other votes?
>
> I haven't been as close to this as others, so perhaps my vote is only
> 0.5 on this specific case, but that's my feeling on it.
>

I think we are being consistent as a project by enforcing strictness of
input in this situation so I'll toss my +0.5/+1​ here as well.

David J.

Re: [HACKERS] psql: new help related to variables are not too readable

[David G. Johnston]

On Wed, Sep 13, 2017 at 12:46 PM, Fabien COELHO  wrote:

>
> Hello Tom,
>
> Probably it needs some rebase after Tom committed result status variables.
>>>
>>
>> As it is a style thing, ISTM that the patch is ready if most people agree
>>> that it is better this way and there is no strong veto against.
>>>
>>
>> FWIW, I think it's a bad idea.  We already nearly-doubled the vertical
>> space required for this variable list.  That was a heavy cost --- and we
>> already got at least one complaint about it --- but it seemed warranted
>> to avoid having to deal with very constrained variable descriptions.
>> This proposes to make the vertical space nearly triple what it was in v10.
>> In a typical-size window that's going to have a pretty severe impact on
>> how much of the list you can see at once.  And the readability gain is
>> (at least to my eyes) very marginal.
>>
>
> Ok, you do not like it. As Pavel said, it is subjective. When it is a
> matter of taste, people tend to differ, someone will always complain, one
> way or another, and they are neither right nor wrong.
>
> So, is it a -1 or a veto?
>
> If it is the later, the patch can be marked as "Rejected" and everybody
> will get more time for other things:-)
>
> If it is a not a veto, people can continue to give their opinions.
> Personnally I'm fine with a pager, so vertical spacing is fine. I just do
> not like paging horizontally.


​-1​

​I don't fully by the "it is subjective" argument - I'm by no means an
expert but there are many people out there who have done considerable
research on human perception that there is at least room for non-subjective
evaluation.  Below is my attempt.​

​If I was going to try and read it like a book I'd want the extra
white-space to make doing so easier (white-space gives the eye a breather
when done with a particular concept) - and the length wouldn't really
matter since I'd just make a single pass and be done with it.  But the
planned usage is for quick lookup of options that you know (or at least
suspect) exist and which you probably have an approximate idea of how they
are spelled.  The all-caps and left-justified block headers are distinct
enough to scan down - though I'd consider indenting 4 spaces instead of 2
to make that even easier (less effort to ignore the indented lines since
ignoring nothing is easier than ignoring something).​  Having more fit on
one screen makes that vertical skimming considerably easier as well (no
break and re-acquire when scrolling in a new page).

So I'll agree that in an absolute sense reading the whole of the content in
its condensed form is more difficult than if there were blank lines in
between each block, but usability for the intended purpose is better in the
current form.

David J.

Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

[David G. Johnston]

On Fri, Sep 8, 2017 at 2:09 PM, Tom Lane  wrote:

> Pavel Stehule  writes:
> > personally I prefer syntax without FOR keyword - because following
> keyword
> > must be reserved keyword
>
> > SET x = .., y = .. SELECT ... ;
>
> Nope.  Most of the statement-starting keywords are *not* fully reserved;
> they don't need to be as long as they lead off the statement.  But this
> proposal would break that.  We need to put FOR or IN or another
> already-fully-reserved keyword after the SET list, or something's going
> to bite us.
>

Just throwing it ​out there but can we making putting SET inside a CTE work?

David J.

Re: [HACKERS] [Proposal] Allow users to specify multiple tables in VACUUM commands

[David G. Johnston]

On Wed, Aug 30, 2017 at 4:08 PM, Bossart, Nathan 
wrote:

> On 8/30/17, 5:37 PM, "Michael Paquier"  wrote:
> > Yeah... Each approach has its cost and its advantages. It may be
> > better to wait for more opinions, no many people have complained yet
> > that for example a list of columns using twice the same one fails.
>
> Sounds good to me.
>
> > +VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [  > class="PARAMETER">table_name ] [, ...]
> > I just noticed that... But regarding the docs, I think that you have
> > misplaced the position of "[, ...]", which should be inside the
> > table_name portion in the case of what I quote here, no?
>
> I think that's what I had initially, but it was changed somewhere along
> the line.  It is a little more complicated for the versions that accept
> column lists.
>
> VACUUM ... ANALYZE [ [ table_name [ (column_name [, ...] ) ] ] [, ...] ]
>
> ISTM that we need the extra brackets here to clarify that the table and
> column list combination is what can be provided in a list.  Does that
> make sense?  Or do you think we can omit the outermost brackets here?
>

​Inspired by the syntax documentation for EXPLAIN:

​VACUUM [ ( option [, ...] ) ] [ table_def [, ...] ]

where option can be one of:
FULL
FREEZE
VERBOSE
DISABLE_PAGE_SKIPPING

and where table_def is:
table_name [ ( column_name [, ... ] ) ]

David J.

Re: [HACKERS] show "aggressive" or not in autovacuum logs

[David G. Johnston]

On Mon, Aug 28, 2017 at 2:26 AM, Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp> wrote:

> https://www.postgresql.org/docs/devel/static/runtime-config-client.html
>
> >
> ​V​
> ACUUM performs an aggressive scan
>

​Maybe this should gets its own thread/patch but I'll tack this on here
since it all seems related.

​That paragraph you linked has a couple of typos:

"Although users can set this value anywhere from zero to two billions,
VACUUM" ...

should be 'two billion' (i.e., drop the "s")

"...so that a periodical manual VACUUM has..."

'periodic' - though the description in the linked 24.1.5 is somewhat
clearer (and longer) - the gap exists for the benefit of routine vacuum
invocations to detect the need for an aggressive vacuum as part of a normal
operating cycle rather than the last routine vacuum being non-aggressive
and shortly thereafter an auto-vacuum anti-wraparound run is performed.

Current:

VACUUM will silently limit the effective value to 95% of
autovacuum_freeze_max_age, so that a periodical manual VACUUM has a chance
to run before an anti-wraparound autovacuum is launched for the table.

My interpretation:

VACUUM will silently limit the effective value to 95% of
autovacuum_freeze_max_age so that a normal scan has a window within which
to detect the need to convert itself to an aggressive scan and preempt the
need for an untimely autovacuum initiated anti-wraparound scan.


As noted in the 24.1.5 that "normal scan" can be time scheduled or an
update driven auto-vacuum one.  It could be manual (though not really
periodic) if one is monitoring the aging and is notified to run on manually
when the age falls within the gap.

"Aggressive" sounds right throughout all of this.

Up-thread:
INFO:  vacuuming "public.it" in aggressive mode

Maybe:
INFO:  aggressively vacuuming "public.it"
INFO:  vacuuming "public.it"

Likewise:
LOG:  automatic vacuum of table "postgres.public.pgbench_branches": mode:
aggressive, index scans: 0

could be:
LOG:  automatic aggressive vacuum of table
"postgres.public.pgbench_branches", index scans: 0

Having read the docs and come to understand what "aggressive" means two
wordings work for me (i.e., leaving non-aggressive unadorned).

David J.

Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

[David G. Johnston]

On Mon, Aug 21, 2017 at 2:30 PM, Simon Riggs  wrote:

>
> > The patch applies cleanly to current master and all tests run without
> > failures.
> >
> > I also test against all current supported versions (9.2 ... 9.6) and
> didn't
> > find any issue.
> >
> > Changed status to "ready for commiter".
>
> I get the problem, but not this solution. It's too specific and of
> zero other value, yet not even exactly specific to the issue. We
> definitely don't want --no-extension-comments, but --no-comments
> removes ALL comments just to solve a weird problem. (Meta)Data loss,
> surely?
>

​It was argued, successfully IMO, to have this capability independent of
any particular problem to be solved.  In that context I haven't given the
proposed implementation much thought.

I do agree that this is a very unappealing solution for the stated problem
and am hoping someone will either have an ingenious idea to solve it the
right way or is willing to hold their nose and put in a hack.

David J.

Re: [HACKERS] dubious error message from partition.c

[David G. Johnston]

On Tue, Aug 8, 2017 at 8:34 PM, Tom Lane  wrote:

> A small suggestion is that it'd be better to write it like "Specified
> upper bound \"%s\" precedes lower bound \"%s\"."  I think "succeeds" has
> more alternate meanings than "precedes", so the wording you have seems
> more confusing than it needs to be.  (Of course, the situation could be
> the opposite in other languages, but translators have the ability to
> reverse the ordering if they need to.)
>
> Or you could just go with "follows" instead of "succeeds".
>

​"exceeds" seems to be the word the original sentence was looking for.  If
using a single word I kinda like reversing the direction and using
"precedes"​ though.  "follows" makes it sound like a puppy :)

"is greater than" is a bit more verbose but an option as well - one that
seems more natural in this space - and yes I've reverted back to
lower->upper with this wording.  I think the hard "x" in exceeds is what
turned me off to it.

David J.

Re: [HACKERS] Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

[David G. Johnston]

On Tue, Aug 8, 2017 at 4:33 PM, Dean Rasheed 
wrote:

> On 8 August 2017 at 19:22, Robert Haas  wrote:
> > On Fri, Jul 21, 2017 at 4:24 AM, Dean Rasheed 
> wrote:
> >> Also drop the constraint prohibiting finite values after an unbounded
> >> column, and just document the fact that any values after MINVALUE or
> >> MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
> >> multiple times, which was needlessly verbose.
> >
> > I would like to (belatedly) complain about this part of this commit.
> > Now you can do stuff like this:
> >
> > rhaas=# create table foo (a int, b text) partition by range (a, b);
> > CREATE TABLE
> > rhaas=# create table foo1 partition of foo for values from (minvalue,
> > 0) to (maxvalue, 0);
> > CREATE TABLE
> >
> > The inclusion of 0 has no effect, but it is still stored in the
> > catalog, shows up in \d foo1, and somebody might think it does
> > something.  I think we should go back to requiring bounds after
> > minvalue or maxvalue to be minvalue or maxvalue.
>

​The commit message indicates one would just specify "unbounded", not
min/maxvalue​, which indeed seems to be a better word for it.

Can we, presently, stick "null" in place of the "0"?

Well perhaps verbosity-reduction isn't sufficient justification but I
> still think this is correct because logically any values in the bound
> after MINVALUE/MAXVALUE are irrelevant, so it seems overly restrictive
> to force all later values to be MINVALUE/MAXVALUE when the code will
> just ignore those values.
>
>
But semantically using
​unbounded
 is correct - and referencing the principle of "write once, read many"
people are probably going to care a lot more about the \d display than the
original SQL - and \d showing some randomly chosen value and mentally doing
the gymnastics to think "oh, wait, it doesn't actually mater what this
value is)" compared to it showing "unbounded" and it being obvious that
"any value" will work, seems like a win.


> I don't think we should allow values after MINVALUE/MAXVALUE to be
> omitted altogether because we document multi-column ranges as being
> most easily understood according to the rules of row-wise comparisons,
> and they require equal numbers of values in each row.
>

I wouldn't change the underlying representation but from a UX perspective
being able to ?omit the explicit specification and let the system default
appropriately would be worth considering - though probably not worth the
effort.

​The complaint regarding \d could be solved by figuring out on-the-fly
whether the particular column is presently bounded or not and diplaying
"unbounded" for the later ones regardless of what value is stored in the
catalog.  "Unbounded (0)" would communicate both aspects.​  In the "be
liberal in what you accept" department that would seem to be a win.

​David J.​

Re: [HACKERS] reload-through-the-top-parent switch the partition table

[David G. Johnston]

On Thu, Aug 3, 2017 at 8:53 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > So maybe --load-via-partition-root if nobody likes my previous
> > suggestion of --partition-data-via-root ?
>
> WFM.
>

​+1

David J.​

Re: [HACKERS] reload-through-the-top-parent switch the partition table

[David G. Johnston]

On Thursday, August 3, 2017, Rushabh Lathia 
wrote:

>
>
> --use-partitioned-table [partitioned_name, ...] # if
> names are omitted it defaults to all the partitioned tables.
>
> Here user need to specify the root relation name in the option - and any
> partition table have that as a ROOT, will load the data through
> top-parent-relation.
>
>
This is indeed what I intended to convey.  Only specify "root" names.

David J.

Re: [HACKERS] reload-through-the-top-parent switch the partition table

[David G. Johnston]

On Wed, Aug 2, 2017 at 10:58 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Wed, Aug 2, 2017 at 1:08 PM, Tom Lane  wrote:
> >> --restore-via-partition-root ?
>
> > I worry someone will think that pg_dump is now restoring stuff, but it
> isn't.
>
> Well, the point is that the commands it emits will cause the eventual
> restore to go through the root.  Anyway, I think trying to avoid using
> a verb altogether is going to result in a very stilted option name.
>
> I notice that the option list already includes some references to
> "insert", so maybe "--insert-via-partition-root"?  Although you could
> argue that that's confusing when we're using COPY.


--use-partitioned-table [partition-name, ...]  # if names are omitted it
defaults to all partitioned tables

I don't know that we need to use "root" in the argument name to communicate
"the top-most if partitioned tables are nested".  We have the docs to
describe exactly what it does.  "Partitioned Table" is what we are calling
the main routing table in the docs.  "Use" seems adequate.

FWIW my first thought was "--insert-via-partition-root" and I didn't mind
that "COPY" commands would be affected implicitly.

David J.

Re: [HACKERS] Possible bug in 9.3.17 using operator <>

[David G. Johnston]

On Tue, Aug 1, 2017 at 8:39 AM, Nick Dro  wrote:

> The operator <> seems to not work properly comparing citext types in
> triggers function.
>
> https://stackoverflow.com/questions/45441840/posgresql-
> 9-3-operator-doesnt-give-logical-result
>
> Can someone figure out what is the problem? This seems like a bug.
>

Um, 'jack' and 'Jack' and indeed equal when compared case-insensitively.
You've asked whether they are unequal.  Since they are equal the answer to
your question is "no" (false).  That is the answer you were given.

David J.

Re: [HACKERS] Update description of \d[S+] in \?

[David G. Johnston]

On Mon, Jul 31, 2017 at 7:06 PM, Robert Haas  wrote:

> On Thu, Jul 13, 2017 at 8:40 PM, Amit Langote
>  wrote:
> > On 2017/07/13 19:57, Ashutosh Bapat wrote:
> >> On Thu, Jul 13, 2017 at 12:01 PM, Amit Langote
> >>  wrote:
> >>> The description of \d[S+] currently does not mention that it will list
> >>> materialized views and foreign tables.  Attached fixes that.
> >>>
> >>
> >> I guess the same change is applicable to the description of \d[S+] NAME
> as well.
> >
> > Thanks for the review.  Fixed in the attached.
>
> The problem with this, IMV, is that it makes those lines more than 80
> characters, whereas right now they are not.


​84: ​  \\d[S+] list (foreign) tables, (materialized)
views, and sequences\n
76:   \\d[S+] list (foreign) tables, (mat.) views, and
sequences\n

  And that line seems
> doomed to get even longer in the future.
>

​Cross that bridge when we come to it?

Lumping the tables and views into a single label (I'd go with "relations"
since these are all - albeit non-exclusively - things that can appear in a
FROM clause) would greatly aid things here.  Indexes and sequences would
retain their own identities.  But I seem to recall that elsewhere we call
indexes relations - and I'm not sure about sequences.

I'm partial to calling it "relations and sequences" and letting the reader
check the documentation for what "relations" means in this context.

David J.
​

Re: [HACKERS] [BUGS] BUG #14759: insert into foreign data partitions fail

[David G. Johnston]

On Mon, Jul 31, 2017 at 5:42 PM, Amit Langote  wrote:

>
> On a second thought though, I think we should list the foreign table
> partitions' limitations in only one place, that is, the CREATE FOREIGN
> TABLE reference page.  Listing them under 5.10.2.3. seems a bit off to me,
> because other limitations listed there are those of the new partitioned
> table objects, such as lack of global index constraints, etc.  Lack of
> tuple-routing to foreign partitions does not seem to me of the similar
> nature.  Also, the same text is no longer repeated in 3 different places.
>
> Thoughts on the updated patch?
>

Overall, works for me.

grammar (add a couple of commas for flow) and style (dropping the first
"the")

current: "(both the user-defined constraints such as CHECK or
NOT NULL clauses and the partition constraint)"
proposed: "(both user-defined constraints, such as CHECK or
NOT NULL clauses, and the partition constraint)"

Thanks!

David J.

Re: [HACKERS] [BUGS] BUG #14759: insert into foreign data partitions fail

[David G. Johnston]

On Tue, Jul 25, 2017 at 11:29 PM, Amit Langote <
langote_amit...@lab.ntt.co.jp> wrote:

> > I'm curious what the other limitations are...
>
> When I first wrote that documentation line (I am assuming you're asking
> about "although these have some limitations that normal tables do not"), I
> was thinking about the fact that the core system does not enforce
> (locally) any constraints defined on foreign tables.  Since we allow
> inserting data into partitions directly, it is imperative that we enforce
> the "partition constraint" along with the traditional constraints such as
> NOT NULL and CHECK constraints, which we can do for local table partitions
> but not for foreign table ones.
>
> Anyway, attached patch documents all these limitations about foreign table
> partitions more prominently.
>

​The revised patch down-thread looks good.  Thanks.

I indeed was referring to the paragraph you quoted.

​I would probably just   s/For example/In particular/   and call it good -
or maybe also tell the user that all the limitations are listed in the
notes section for create foreign table (though my first thoughts are all
quite wordy).

David J.

Re: [HACKERS] pg_upgrade failed if view contain natural left join condition

[David G. Johnston]

On Thu, Jul 20, 2017 at 6:53 AM, Tom Lane  wrote:

> tushar  writes:
> > postgres=# create table t(n int);
> > CREATE TABLE
> > postgres=# create table t1(a int);
> > CREATE TABLE
> > postgres=# create view ttt1 as SELECT e.n FROM t e NATURAL LEFT JOIN t1
> d;
> > CREATE VIEW
>
> You realize of course that that's a pretty useless join definition.
> Still, yes, we do need to reverse-list the view with correct syntax.
> Probably t LEFT JOIN t1 ON TRUE would do it.
>

Per the docs:

"If there are no common column names, NATURAL behaves like CROSS JOIN."

I'm being a bit pedantic here but since NATURAL is a replacement for
"ON/USING" it would seem more consistent to describe it, when no matching
columns are found, as "behaves like specifying ON TRUE" instead.   Maybe
"behaves like specifying ON TRUE, causing a CROSS JOIN to occur instead."

I find it a bit strange, though not surprising, that it doesn't devolve to
"ON FALSE".

David J.

Re: [HACKERS] Domains and arrays and composites, oh my

[David G. Johnston]

On Thursday, July 13, 2017, Tom Lane  wrote:
>
> regression=# select * from fdc();
>   fdc
> ---
>  (1,2)
> (1 row)
>
>
Select (fdc).* from fdc();  is considerably more intuitive that the cast.
Does that give the expected multi-column result?

David J.

Re: [HACKERS] CAST vs ::

[David G. Johnston]

On Thursday, July 13, 2017, Tom Lane  wrote:

> Maybe we can hack ruleutils to use
> the CAST syntax only in this specific context.
>

Given the lack of complaints, and ubiquity of ::, this would seem ideal
and sufficient. While there is something to be said for using standard
compliant syntax changing just this like doesn't seem like it would move
the goalposts meaningfully.

David J.

Re: [HACKERS] \set AUTOROLLBACK ON

[David G. Johnston]

On Mon, Jun 26, 2017 at 12:19 PM, David Fetter  wrote:

> On Mon, Jun 26, 2017 at 04:00:55PM +0200, Joel Jacobson wrote:
> > Hi hackers,
> >
> > A colleague of mine wondered if there is a way to always run
> > everything you type into psql in a db txn and automatically rollback
> > it as soon as it finish.
> > I couldn't think of any way to do so, but thought it would be a nice
> > feature and probably quite easy to add to psql, so I thought I should
> > suggest it here.
> >
> > The typical use-case is you are doing something in production that you
> > just want to
> > a) test if some query works like expected and then rollback
> > or,
> > b) read-only queries that should not commit any changes anyway, so
> > here the rollback would just be an extra layer of security, since your
> > SELECT might call volatile functions that are actually not read-only
> >
> > Thoughts?
>
> Multi-statement transactions:
>
> Would flavor of BEGIN TRANSACTION undo the feature?
> If not, would it auto-munge COMMIT into a ROLLBACK?
>

​We already have SET TRANSACTION READ ONLY.

If you begin a transaction and do not issue an explicit commit when the
session closes the default action is ROLLBACK.

At some point if you want to use SQL features you need to write SQL - not
pass command-line arguments to the client.

See also ".psqlrc" and shell functions/aliases.

This doesn't seem like material to build into psql but since the proposal
lacks an envisioned usage its hard to say conclusively.  Interplay with the
various ways to source SQL, and existing arguments, is a prime area of
concern.

Side effects:
>
> Let's imagine you have a function called
> ddos_the_entire_internet(message TEXT), or something less drastic
> which nevertheless has side effects the DB can't control.
>
> How should this mode handle it?  Should it try to detect calls to
> volatile functions, or should it just silently fail to do what
> it's promised to do?
>

​It doesn't need to promise anything more than what happens today if
someone manually keys in

BEGIN;
[...]
ROLLBACK;

​using psql prompts.

David J.

Re: [HACKERS] Adding connection id in the startup message

[David G. Johnston]

On Wed, Jun 21, 2017 at 12:15 PM, Robert Haas  wrote:
> I think the problem is real,
> but I'm not sure that this is the best solution.  On the other hand,
> I'm also not entirely sure I understand the proposal yet.

Given the problems with changing the protocol it does seem like
generalizing away from "connection id" to "arbitrary payload" would be
useful.

Like, add a new "attachment map" area where any application-aware node
that encounters the message can attach data.  If some node further
downstream sends a response message the contents of the attachment map
would be sent back as-is.

The advantage of "connection id" is that the messages are still of
fixed size whereas the more flexible arrangement would necessitate
that message sizes vary.  In return middleware gets a place to store
session information that can be used more broadly than a simple
externally generated connection id.

David J.


-- 
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] Typo in insert.sgml

[David G. Johnston]

On Tue, Jun 20, 2017 at 1:35 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Tue, Jun 20, 2017 at 2:34 PM, Peter Eisentraut
>>  wrote:
>>> This was not a typo, this was intentional.
>
>> To me, Julien's change seems to make it easier to understand, but
>> maybe I'm misunderstanding what it's trying to say in the first place.
>
> I think there are two subtly different things here: the sequence counters
> will continue to be attached to the identity columns, and the counters
> will continue in sequence (ie, no implied ALTER SEQUENCE RESTART).
> The existing text seems to mean the latter but Julien's change makes it
> about the former.
>
> It could use rewording for clarity, just not this rewording.

Tom, I'm not following your train of thought:

Given one sequenced IDENTITY column in each table (some rows were
delete from each already):
tbl1
4
5
6

tbl2
10
11
12

The point being made is that inserting all rows from tbl1 into tbl2
will result in tbl2 having:
10
11
12
13
14
15

and not:
10
11
12
4
5
6

That the sequence counters continue to be attached seems like it can
and should be taken for granted.  Inserting to a table likewise should
never cause an ALTER SEQUENCE RESTART to occur and so can and should
be taken for granted.

ISTM that both the existing text and Julien's variation say this, just
differently.  I think the word "continue" is part of the problem.  I
also think not being explicit about the fact that its tbl2's sequence
that supplies the new values - instead of using the values already
present in tbl1 - requires one to think harder about the scenario than
need be.

David J.


-- 
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] Optional message to user when terminating/cancelling backend

[David G. Johnston]

On Tue, Jun 20, 2017 at 11:54 AM, Alvaro Herrera
 wrote:
> Satyanarayana Narlapuram wrote:

> Unless you have a lot of users running psql manually, I don't see how
> this is actually very useful or actionable.  What would the user do with
> the information?  Hopefully your users already trust that you'd keep the
> downtime to the minimum possible.
>

Why wouldn't this be useful in application logs?  Spurious dropped
connections during application execution would be alarming.  Seeing a
message from the DBA when looking into those would be a painless and
quick way to alleviate stress.

pg_cancel_backend(, 'please try not to leave sessions in an "idle
in transaction" state...') would also seem like a useful message to
communicate; to user or application.  Sure, some of this can, and
maybe would also need to, be done out-of-band but this communication
channel seems worthy enough to at least evaluate the provided
implementation.

David J.


-- 
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 transactons not fully isolated

[David G. Johnston]

On Tue, Jun 20, 2017 at 12:22 PM, Chapman Flack  wrote:
> I get the reported result (DELETE 0 and a table containing 2 and 3)
> in both 'read committed' and 'read uncommitted'.

Practically speaking those are a single transaction isolation mode.

https://www.postgresql.org/docs/10/static/transaction-iso.html

I think Merlin has mis-read the article he linked to.  The example
being used there never claims to be done under serialization and seems
to describe an example of the perils of relying on the default
isolation level.

David J.


-- 
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] Typo in insert.sgml

[David G. Johnston]

On Tuesday, June 20, 2017, Robert Haas  wrote:

> On Tue, Jun 20, 2017 at 2:34 PM, Peter Eisentraut
> > wrote:
> > On 6/18/17 03:16, Julien Rouhaud wrote:
> >> Patch attached.
> >
> > This was not a typo, this was intentional.
>
> To me, Julien's change seems to make it easier to understand, but
> maybe I'm misunderstanding what it's trying to say in the first place.
>
>
Maybe

[...] that are not identity columns in tbl2. Values for
identity columns will come from the sequence generators for
tbl2.

Otherwise I'd drop the word "continue" altogether and just say "but will
use the sequence counters for".

David J.

Re: [HACKERS] psql's \d and \dt are sending their complaints to different output files

[David G. Johnston]

On Mon, Jun 19, 2017 at 2:53 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> "David G. Johnston" <david.g.johns...@gmail.com> writes:
>> The docs also indicate that we don't include materialized views as
>> part of "\d" which seems like an oversight somewhere.
>
> Where are you reading that?

https://www.postgresql.org/docs/9.6/static/app-psql.html

First sentence:

"For each relation (table, view, index, sequence, or foreign table) or
composite type matching the pattern..."

I was expecting "materialized view" to be one of the parenthetical options.

> Experimentation shows that "\d" does include
> matviews, and that matches the code, which has this as the default
> expansion of \d:
>
> /* standard listing of interesting things */
> success = listTables("tvmsE", NULL, show_verbose, 
> show_system);
>

\dT / "composite type" seems to be a special case.

David J.


-- 
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] psql's \d and \dt are sending their complaints to different output files

[David G. Johnston]

On Mon, Jun 19, 2017 at 2:25 PM, Peter Eisentraut
 wrote:
> On 6/19/17 09:00, Oleksandr Shulgin wrote:
>> I wonder if it is intentional that \d complains on stderr if it cannot
>> find relations to match, but \dt prints the message to the current
>> output file?
>>
>> postgres=# \d xxx
>> Did not find any relation named "xxx".
>> postgres=# \dt xxx
>> No matching relations found.
>>>
> The first command is "show me relation xxx" and that gives an error
> message if it does not exist (and would also create an appropriate exit
> status if certain options are used).
>
> The second command says "show me all relations matched 'xxx'".  The
> result of this is successful execution showing nothing.  The message it
> prints is a "courtesy" message.

The docs indicate the optional argument to both is a pattern so I'm
not seeing why they are treated differently.

The docs also indicate that we don't include materialized views as
part of "\d" which seems like an oversight somewhere.

It sounds like, though isn't specified anywhere, the while you can
write "\dit" to get a subset of all available options omitting all of
the options leaves you with "\d" which is equivalent to specifying
them all.  Which leads on to believe the output from both should be in
sync.

David J.


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

Re: [BUGS] [HACKERS] Re: Postgresql bug report - unexpected behavior of suppress_redundant_updates_trigger

[David G. Johnston]

On Mon, Jun 19, 2017 at 2:10 PM, Tom Lane  wrote:
> Alvaro Herrera  writes:
>> Tom Lane wrote:
>>> ... If the trigger is succeeding (ie,
>>> detecting a no-op update) often enough that it would be worth that,
>>> you've really got an application-stupidity problem to fix.
>
>> ISTM the whole point of suppress_redundant_updates_trigger is to cope
>> with application stupidity.
>
> I think it's a suitable band-aid for limited amounts of stupidity.
> But it eliminates only a small fraction of the total overhead involved
> in a useless update command.  So I remain of the opinion that if that's
> happening a lot, you're better off fixing the problem somewhere upstream.

At first glance I think I'd rather have it do the correct thing all of
the time, even if it takes longer, so that my only trade-off decision
is whether to improve performance by fixing the application.

Ideally if the input tuple wouldn't require compression we wouldn't
bother to decompress the stored tuple.

David J.


-- 
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] how are the rpms configured that are available in PostgreSQL RPM Building Project - Yum Repository

[David G. Johnston]

On Fri, Jun 16, 2017 at 12:40 PM, Cook, Malcolm  wrote:

> Hi,
>
>
>
> I am referring to the contents of https://yum.postgresql.org/
> (specifically version 9.6 rpms for CentoOS7)
>
>
>
> More specifically I wonder if they are configured:--with-python
> --with-tcl --with-pam --with-ldap
>
>
>
> And do they install the Additional Supplied Modules (
> https://www.postgresql.org/docs/9.1/static/contrib.html ) as provided
> when performing a
>
>
>
>make install world
>
>
>
> Please advise if my possibly novice question should better be posed
> elsewhere.  Or if I should expect to find answers in the fine manual.  I
> looked but did not see…
>

​This may provide the info you need.  Note you'll actually need an
installed version to run it against.​

https://www.postgresql.org/docs/current/static/app-pgconfig.html

​This will show what extensions are available in said cluster.

https://www.postgresql.org/docs/current/static/view-pg-available-extensions.html

​David J.​

[HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[David G. Johnston]

On Thursday, June 15, 2017, Tom Lane  wrote:

> Michael Paquier  writes:
> > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> >> (2) My inclination would be not to back-patch.  This change could break
> >> configurations that worked before, and the lack of prior complaints
> >> says that not many people are having a problem with it.
>
> > That's fourteen years without complains, still I cannot imagine any
> > cases where it would be a problem as people who would have faced this
> > problem but not reported it have likely just enforced the FS to handle
> > case-insensitivity for paths.
>
> Well, it's not just about case sensitivity.  The comment for
> SplitDirectoriesString points out that it deals with embedded spaces
> differently, and it also applies canonicalize_path().  I'm too tired
> to think hard about what that part might mean for compatibility, but
> it probably isn't nothing.
>
> Anyway, I agree that this is an appropriate change for HEAD.  Just
> not convinced that we should shove it into minor releases.
>
>
If it's downcasing then careless use of mixed case when the true path on a
case sensitive filesystem is all lower case (not unlikely at all given
default packaging for deb and rpm packages, iirc) would indeed be a
problem.  Those paths are accidentally working right now.  I vote for no
back-patch.

David J.

Re: [HACKERS] logical replication: \dRp+ and "for all tables"

[David G. Johnston]

On Wed, Jun 14, 2017 at 4:10 PM, Tom Lane  wrote:

> I was hoping we'd get some more votes in this thread, but it seems like
> we've only got three, and by my count two of them are for just printing
> "all tables".


The following looks right - given a publication it would nice to know if
its for all tables or not.

  \dRp
!List of publications
! Name|  Owner   | All Tables | Inserts |
Updates | Deletes
!
+--++-+-+-
!  testpib_ins_trunct | regress_publication_user | f  | t   | f
  | f
!  testpub_default| regress_publication_user | f  | t   | t
  | t
​
This [I couldn't find a regression diff entry where "All Tables" is true :(
...]

  \dRp+ testpub3
!Publication testpub3
!  All Tables | Inserts | Updates | Deletes
! +-+-+-
!  f  | t   | t   | t
  Tables:
  "public.testpub_tbl3"
  "public.testpub_tbl3a"

I agree with Tom and Masahiko Sawada, if "All Tables" is false we continue
to show "Tables:\n\t[tables]" but when "All Tables" is true we'd write
something like "Tables: All Tables in Database".  The user can query the
database if they wish to know the names of all those tables exactly.

I suppose we could go further here, say by simplifying "public.tbl1,
public.tbl2", when public only contains two tables, to "public.*".  Or
consider never listing more than some small number of rows and provide a
"show all" option (\dRp++ or just a function/view) that would list every
single table.  But I would go with the default "+" behavior being to show
table names when the listing of tables is fixed and to say "All Tables in
Database" when it is dynamic.  In "+" mode that makes the "All Tables"
boolean redundant though I'd keep it around for consistency.

David J.

Re: [HACKERS] List of hostaddrs not supported

[David G. Johnston]

On Thu, Jun 8, 2017 at 8:18 AM, Robert Haas  wrote:

> Whatever you put in the hostaddr field - or any field other than host
> and port - is one entry.  There is no notion of a list of entries in
> any other field, and no attempt to split any other field on a comma or
> any other symbol.
>
​[...]​


> I think the argument is about whether I made the right decision when I
> scoped the feature, not about whether there's a defect in the
> implementation.
>

Implementation comes into play if the scope decision stands.

​I have no immediate examples but it doesn't seem that we usually go to
great lengths to infer user intent and show hints based upon said
inference.  But we also don't forbid doing so.  So the question of whether
we should implement better error messages here seems to be in scope -
especially since we do allow for lists in some of the sibling fields.​

These are already failing so I'd agree that explicit rejection isn't
necessary - the question seems restricted to usability.  Though I suppose
we need to consider whether there is any problem with the current setup if
indeed our intended separator is also an allowable character - i.e., do we
want to future-proof the syntax by requiring quotes now?

David J.

Re: [HACKERS] PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

[David G. Johnston]

On Wed, Jun 7, 2017 at 11:57 AM, Tom Lane  wrote:

> If people are on board with throwing an error, I'll go see about
> writing a patch.
>

+1 from me.

David J.​

Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

[David G. Johnston]

Stephen,

On Tue, May 30, 2017 at 8:41 PM, Stephen Frost <sfr...@snowman.net> wrote:

> David,
>
> * David G. Johnston (david.g.johns...@gmail.com) wrote:
> > On Fri, May 26, 2017 at 7:47 AM, Stephen Frost <sfr...@snowman.net>
> wrote:
> > > * Robins Tharakan (thara...@gmail.com) wrote:
> > > > Attached is a patch adds a --no-comments argument to pg_dump to skip
> > > > generation of COMMENT statements when generating a backup. This is
> > > crucial
> > > > for non-superusers to restore a database backup in a Single
> Transaction.
> > > > Currently, this requires one to remove COMMENTs via scripts, which is
> > > > inelegant at best.
> > >
> > > Having --no-comments seems generally useful to me, in any case.
> >
> > I​t smacks of being excessive to me.
> > ​
>
> I have a hard time having an issue with an option that exists to exclude
> a particular type of object from being in the dump.
>

​Excessive with respect to the problem at hand.  A single comment in the
dump is unable to be restored.  Because of that we are going to require
people to omit every comment in their database.  The loss of all that
information is in excess of what is required to solve the stated problem
which is how I was thinking of excessive.

I agree that the general idea of allowing users to choose to include or
exclude comments is worth discussion along the same lines of large objects
and privileges.


> > > CREATE EXTENSION IF NOT EXISTS plpgsql ... COMMENT blah;
> >
> > ​A less verbose way to add comments to objects would be nice but we have
> an
> > immediate problem that we either need to solve or document a best
> practice
> > for.
>
> The above would be a solution to the immediate problem in as much as
> adding COMMENT IF NOT EXISTS would be.
>

​I believe it would take a lot longer, possibly even until 12, to get the
linked comment feature committed compared ​to committing COMMENT IF NOT
EXISTS or some variation (or putting in a hack for that matter).


> > COMMENT IF NOT EXISTS has been brought up but it doesn't actually map to
> > what seems to me is the underlying problem...that people don't want a
> > non-functional (usually...) aspect preventing successful restoration.
>
> I tend to disagree with this characterization- I'm of the opinion that
> people are, rightly, confused as to why we bother to try and add a
> COMMENT to an object which we didn't actually end up creating (as it
> already existed), and then throw an error on it to boot.


My characterization does actually describe the current system though.
While I won't doubt that people do hold your belief it is an underlying
mis-understanding as to how PostgreSQL works since comments aren't, as you
say below, actual attributes but rather objects in their own right.  I
would love to have someone solve the systemic problem here.  But the actual
complaint can probably be addressed without it.


>   Were pg_dump a
> bit more intelligent of an application, it would realize that once the
> CREATE ... IF NOT EXISTS returned a notice saying "this thing already
> existed" that it would realize that it shouldn't try to adjust the
> attributes of that object, as it was already existing.


​pg_dump isn't in play during the restore which is where the error occurs.

During restore you have pg_restore+psql - and having cross-statement logic
in those applications is likely a non-starter. That is ultimately the
problem here, and which is indeed fixed by the outstanding proposal of
embedding COMMENT within the CREATE/ALTER object commands.  But today,
comments are independent objects and solving the problem within that domain
will probably prove easier than changing how the system treats comments.


> > COMMENT ON object TRY 'text'  -- i.e., replace the word IS with TRY
>
> Perhaps you could elaborate as to how this is different from IF NOT
> EXISTS?
>
>
​If the comment on plpgsql were removed for some reason the COMMENT ON IF
NOT EXISTS would fire and then it would fail due to permissions.  With
"TRY" whether the comment (or object for that matter) exists or not the new
comment would be attempted and if the permission failure kicked in it
wouldn't care.​


>  If the
> > affected users cannot make that work then maybe we should just remove the
> > comment from the extension.
>
> Removing the comment as a way to deal with our deficiency in this area
> strikes me as akin to adding planner hints.
>

​Maybe, but the proposal you are supporting has been around for years, with
people generally in favor of it, and hasn't happened yet.  At some point
I'd rather hold my nose and fix the problem myself than wait for the
professional to arrive and do it right.  Any, hey, we've had multiple
planner hints since 8.4 ;)

David J.

Re: [HACKERS] pg_config --version-num

[David G. Johnston]

On Tue, May 30, 2017 at 8:07 PM, Tom Lane  wrote:

> Hm, but with this you're trading that problem for "is the right version
> of pg_config in my PATH?".
>

That is probably a solved problem for those who are parsing the output of
--version today.
​

>
> This idea might well be useful for external packages which are always
> built/tested against installed versions of Postgres.  But it seems like
> we need to think harder about what to do for our own usages, and that
> may lead to a different solution altogether.
>

​While we may not want to go to the extreme that is Perl, there being more
than one way to do things does have merit.  Given that we run 5 concurrent
releases of our software and put out new ones annually the version is a
very important piece of information.  There being 5 different ways to get
at that data is not inherently bad since each way likely is most useful to
different subsets of our users.  To allow people to scratch their own itch,
here specifically, seems like an easy win in making the project visibly
responsive to the community.

David J.
​

Re: [HACKERS] pg_config --version-num

[David G. Johnston]

On Tue, May 30, 2017 at 6:36 PM, Michael Paquier 
wrote:

> On Tue, May 30, 2017 at 6:14 PM, Craig Ringer 
> wrote:
> > Attached is a small patch to teach pg_config how to output a
> --version-num
> >
> > With Pg 10, parsing versions got more annoying. Especially with
> > "10beta1", "9.6beta2" etc into the mix. It makes no sense to force
> > tools and scripts to do this when we can just expose a sensible
> > pre-formatted one at no cost to us.
> >
> > Personally I'd like to backpatch this into supported back branches,
> > but just having it in pg 10 would be  a help.
>
> The last threads treating about the same subject are here:
> https://www.postgresql.org/message-id/CAB7nPqTAdAJpX8iK4V3uYJbO2Kmo8
> rhzqjkdsladdrannrg...@mail.gmail.com


​​Tom's comment here:

"whereas adding a pg_config option
entails quite a lot of overhead (documentation, translatable help text,
yadda yadda)."

The proposed doesn't touch the first item and patch author's aren't
expected to handle the second.  Not sure what all the rest entails...but I
cannot imagine it is a considerable amount of stuff that amounts to little
more than boilerplate text paralleling what is already out there for the
existing --version option.  If someone is willing to do that I'd think we
should feel OK with the little bit of translation would that would need to
occur because of it.

The fact that this is even on the radar means that more than likely there
are sensible uses for this capability whether they've been adequately
presented or not.  We don't have someone begging for help here but rather
ultimately a complete patch that can be committed and which would require
pretty much zero maintenance.

While I don't do it presently I could very well imagine value in being able
to inspect installed versions PostgreSQL, including patch levels, without
needing a running server process or the ability to login.

David J.

Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

[David G. Johnston]

On Fri, May 26, 2017 at 7:47 AM, Stephen Frost  wrote:

> Greetings,
>
> * Robins Tharakan (thara...@gmail.com) wrote:
> > Attached is a patch adds a --no-comments argument to pg_dump to skip
> > generation of COMMENT statements when generating a backup. This is
> crucial
> > for non-superusers to restore a database backup in a Single Transaction.
> > Currently, this requires one to remove COMMENTs via scripts, which is
> > inelegant at best.
>
> Having --no-comments seems generally useful to me, in any case.
>

I​t smacks of being excessive to me.
​

> CREATE EXTENSION IF NOT EXISTS plpgsql ... COMMENT blah;
>

​A less verbose way to add comments to objects would be nice but we have an
immediate problem that we either need to solve or document a best practice
for.

COMMENT IF NOT EXISTS has been brought up but it doesn't actually map to
what seems to me is the underlying problem...that people don't want a
non-functional (usually...) aspect preventing successful restoration.

COMMENT ON object TRY 'text'  -- i.e., replace the word IS with TRY

If the attempt to comment fails for any reason log a warning (maybe) but
otherwise ignore the result and continue on without invoking an error.

One suggestion I've seen is to simply "COMMENT ON EXTENSION plpgsql IS
NULL;"  If that works in the scenarios people are currently dealing with
then I'd say we should advise that such an action be taken for those whom
wish to generate dumps that can be loaded by non-super-users.  If the
affected users cannot make that work then maybe we should just remove the
comment from the extension.  People can lookup "plpgsql" in the docs easily
enough and "PL/pgSQL procedural language" doesn't do anything more than
expand the acronym.

David J.

Re: [HACKERS] Allowing dash character in LTREE

[David G. Johnston]

On Sat, May 20, 2017 at 11:26 AM, Cyril Auburtin 
wrote:

> Ah sorry, first time, I thought it didn't pass
>

​You should check our excellent online mailing list archives before
re-sending.

https://www.postgresql.org/list/

David J.
​

Re: [HACKERS] Allowing dash character in LTREE

[David G. Johnston]

On Sat, May 20, 2017 at 1:27 AM, Cyril Auburtin 
wrote:

> It could be useful to allow the `-` char in allowed LTREE label characters  
> (currently
> a-zA-Z0-9_ https://www.postgresql.org/docs/9.6/static/ltree.html)
>
> The reason is to allow to use more easily base64 ids, (like
> https://github.com/dylang/shortid, ...), uuid's too in labels
>
> `-` is also not used as a query operator as a plus
>
>
> source: https://doxygen.postgresql.org/ltree_8h.html#a
> 1cb5d5dd0c364d720854e8a250967dd1
>

​This is the third time you've posted this suggestion.  Its been seen by
the people you are sending it to.

I don't follow how this helps for UUID - they are not usually generated
hierarchically​...

I'll +1 the thought, but I won't be providing a patch.

David J.

Re: [HACKERS] [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur

[David G. Johnston]

On Wed, May 17, 2017 at 12:06 AM, Tsunakawa, Takayuki <
tsunakawa.ta...@jp.fujitsu.com> wrote:

> From: pgsql-hackers-ow...@postgresql.org
> > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
> > Who is right is a judgement call, but I don't think it's self-evident
> that
> > users want to ignore anything and everything that might have gone wrong
> > with the connection to the first server, rather than only those things
> which
> > resemble a down server.  It seems quite possible to me that if we had
> defined
> > it as you are proposing, somebody would now be arguing for a behavior
> change
> > in the other direction.
>
> Judgment call... so, I understood that it's a matter of choosing between
> helping to detect configuration errors early or service continuity.


​This is how I've been reading this thread and I'm tending to agree with
prioritizing service continuity ​over configuration error detection.  As a
client if I have an alternative that ends up working I don't really care
whose fault it is that the earlier options weren't.  I don't have enough
experience to think up plausible scenarios here but I'm sold on the theory.

David J.

Re: [HACKERS] CTE inlining

[David G. Johnston]

On Tue, May 9, 2017 at 12:36 PM, Tom Lane  wrote:

> Also, considering that this behavior has been there since 8.4,
> I think it's sheerest chutzpah to claim that changing the docs in
> v10 would materially reduce the backward-compatibility concerns
> for whatever we might do in v11.
>

​No it won't, but those who are using 10 as their first version would
probably be happy if this was covered in a bit more depth.  Even a comment
like "Unlike most other DBMS PostgreSQL presently executes the subquery in
the CTE​ in relative isolation.  It is suggested that you document any
intentional usage of this optimization fence as a query planning hint so
that should the default change in the future you can update the query to
support whatever official syntax is implemented to retain this behavior.

We cannot really help those who have been using this since 8.4 and won't
read the relevant docs because they don't need to learn anything new; but
we can at least avoid subjecting newer customers.  I'm not that strongly in
favor of it but it would be a nice touch - even if it won't change the
risk/reward calculation in any meaningful way.

David J.

Re: [HACKERS] CTE inlining

[David G. Johnston]

On Tue, May 9, 2017 at 12:15 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 5/5/17 08:43, David Rowley wrote:
> > How about we get the ball rolling on this in v10 and pull that part
> > out of the docs. If anything that'll buy us a bit more wiggle room to
> > change this in v11.
> >
> > I've attached a proposed patch.
>
> If we just tell them that the thing they might have relied on might go
> away, without a replacement to suggest, then we're just confusing and
> scaring them, no?
>

​We'd end up suggesting our OFFSET 0 hack as true protection.  If they know
for a fact that their use of CTE for its barrier properties is not
supported they are also more likely to document intentional usage with
something like:   "-- CHANGE THIS ONCE VERSION 11 IS RELEASED!!! --" which
would make finding the call sites that need to add the new "MATERIALIZED"
​keyword much easier.

David J.

Re: [HACKERS] CTE inlining

[David G. Johnston]

On Thu, May 4, 2017 at 9:22 AM, Andrew Dunstan <
andrew.duns...@2ndquadrant.com> wrote:

>
> Yeah, the idea that this won't cause possibly significant pain is quite
> wrong. Quite by accident I came across an example just this morning where
> rewriting as a CTE makes a big improvement.
>
> I wrote this query:
>
> select (json_populate_record(null::mytype, myjson)).*
> from mytable;
>
>
> It turned out that this was an order of magnitude faster:
>
> with r as
> (
>select json_populate_record(null::mytype, myjson) as x
>from mytable
> )
> select (x).*
> from r;
>

​Except I suspect we at least have a chance to detect the above and not
de-optimize it by evaluating "json_populate_record" once for every column
in mytype.

The now idiomatic solution​ to the above is to use LATERAL so the above CTE
is no longer actually a required workaround.

David J.

Re: [HACKERS] CTE inlining

[David G. Johnston]

On Mon, May 1, 2017 at 7:43 AM, Andreas Karlsson <andr...@proxel.se> wrote:

> On 05/01/2017 04:33 PM, David G. Johnston wrote:
> > On Mon, May 1, 2017 at 7:26 AM, Andreas Karlsson <andr...@proxel.se
> > I am not sure I like decorators since this means adding an ad hoc
> > query hint directly into the SQL syntax which is something which I
> > requires serious consideration.
>
> > ​I would shorten that to "WITH MAT" except that I don't think that
> > having two way to introduce an optimization fence is worthwhile.
>
> You mean OFFSET 0? I have never been a fan of using it as an optimization
> fence. I do not think OFFSET 0 conveys clearly enough to the reader that is
> is an optimization fence.


​I think I was being too literal in my thinking.  Proposing that we
effectively do away with OFFSET 0 and instead recommend "WITH MAT name AS
()" for subqueries requiring standalone evaluation is something I can agree
with.  I too have always though of OFFSET 0 as hack-ish which is why my SRF
usage examples have always used CTEs.

David J.​

Re: [HACKERS] CTE inlining

[David G. Johnston]

On Mon, May 1, 2017 at 7:26 AM, Andreas Karlsson  wrote:

> On 05/01/2017 04:17 PM, David Fetter wrote:
>
>> Maybe we could allow a "decorator" that would tell the planner the CTE
>>> could be inlined?
>>>
>>> WITH INLINE mycte AS ( ...)
>>>
>>
>> +1 for a decorator, -1 for this one.
>>
>
> I am not sure I like decorators since this means adding an ad hoc query
> hint directly into the SQL syntax which is something which I requires
> serious consideration.


​Given that we already have
​"​
prevent optimization
​"​
syntax why do we need a decorator on the CTE?


>
>
> We already have an explicit optimization fence with OFFSET 0, and I
>> think making optimization fences explicit is how we should continue.
>> I'd be more in favor of something along the lines of
>>
>> WITH FENCED/* Somewhat fuzzy.  What fence? */
>> or
>> WITH AT_MOST_ONCE  /* Clearer, but not super precise */
>> or
>> WITH UNIQUE_ATOMIC /* More descriptive, but not super clear without
>> the docs in hand */
>>
>> or something along that line.
>>
>
> What about WITH MATERIALIZED, borrowing from the MySQL terminology
> "materialized subquery"?


​I would shorten that to "WITH MAT" except that I don't think that having
two way to introduce an optimization fence is worthwhile.

If we don't optimize SRFs-in-target-list, and thus avoid multiple function
evaluation for (composite_col).*, I believe a significant number of
intentional optimization fence uses will be safe but behavioral changes.

David J.

Re: [HACKERS] Cached plans and statement generalization

[David G. Johnston]

On Tue, Apr 25, 2017 at 3:24 PM, David Fetter  wrote:

> I don't have an exploit yet.  What concerns me is attackers' access to
> what is in essence the ability to poke at RULEs when they only have
> privileges to read.
>

​If they want to see how it works they can read the source code.  In terms
of runtime data it would limited to whatever the session itself created.
In most cases the presence of the cache would be invisible.  I suppose it
might appear if one were to explain a query, reset the session, explain
another query and then re-explain the original.  If the chosen plan in the
second pass differed because of the presence of the leading query it would
be noticeable but not revealing.  Albeit I'm a far cry from a security
expert...

David J.
​

Re: [HACKERS] Separation walsender & normal backends

[David G. Johnston]

On Tue, Apr 25, 2017 at 2:24 PM, Petr Jelinek 
wrote:

> On 25/04/17 17:13, Fujii Masao wrote:
> > On Tue, Apr 25, 2017 at 11:34 PM, Tom Lane  wrote:
> > OTOH, I believe that logical replication is still useful even without
> > initial table sync feature. So reverting the table sync patch seems
> > possible idea.
> >
>
> I don't think that's good idea, the usefulness if much lower without the
> initial copy. The original patch for this added new commands to
> replication protocol, adding generic SQL interface was result of request
> in the reviews.
>

Haven't followed this feature closely but my first thoughts when recently
reading about it were related to the initial copy and table synchronization
- so I'd have to agree with Petr here.  Full table sync is big and for any
table with activity on it the confidence level of knowing you have
everything is greatly reduced if the system isn't making a guarantee.

David J.

Re: [HACKERS] question: data file update when pg_basebackup in progress

[David G. Johnston]

On Tue, Apr 25, 2017 at 9:08 AM, Rui Hai Jiang  wrote:

> When pg_basebackup is launched, a checkpoint is created first, then all
> files are transferred to the  pg_basebackup client.  Is it possible that a
> data page(say page-N) in a data file is changed after the checkpoint and
> before the pg_basebackup is finished?
>

​I believe so.
​

> If this happens,  is it possible that only part of the changed page be
> transferred to the pg_basebackup client?  i.e.  the pg_basebackup client
> gets page-N with part of the old content and part of the new content. How
> does postgreSQL handle this kind of data page?
>

​The first write to a page after a checkpoint is always recorded in the WAL
as a full page write.  Every ​WAL file since the checkpoint must also be
copied to the backed up system.  The replay of those WAL files is what
brings the remote and local system into sync with respect to all changes
since the backup checkpoint.

David J.

Re: [HACKERS] pg_basebackup issue

[David G. Johnston]

For reference this has been asked, and eventually answered on -general at:

https://www.postgresql.org/message-id/flat/CAKFQuwZDS7nA0SvVnumwjHBxz4CWKQm3bVNTHVeWdtAW_oXNJg%40mail.gmail.com#cakfquwzds7na0svvnumwjhbxz4cwkqm3bvnthvewdtaw_ox...@mail.gmail.com

Further comments below; partly a rehash of the conclusion drawn by Adrian
Klaver on that thread.

On Sun, Apr 23, 2017 at 11:55 AM, chiru r  wrote:

>
> postgres=#
> postgres=# create user backup_admin password 'X';
> CREATE ROLE
> postgres=# create role dba_admin SUPERUSER REPLICATION;
> CREATE ROLE
> postgres=# grant dba_admin to backup_admin;
> GRANT ROLE
> postgres=# alter user backup_admin set role to dba_admin;
> ALTER ROLE
>
> postgres=# \du
>List of roles
> Role name | Attributes
> | Member of
> --+-
> ---+
>  backup_admin |
>  | {dba_admin}
>  dba_admin| Superuser, Cannot login, Replication
> | {}
>  postgres | Superuser, Create role, Create DB, Replication, Bypass
> RLS | {}
>
> [postgres@pgserver ~]$ mkdir online_backups1
> [postgres@pgserver ~]$ /opt/PostgreSQL/9.5/bin/pg_basebackup  --format=t
>   --pgdata=online_backups1 -p 5432 -U backup_admin  -x -z  --verbose
> pg_basebackup: could not connect to server: FATAL:  must be superuser or
> replication role to start walsender
>
> *Please help me why pg_basebackup is throwing FATAL when I use
> backup_admin?.*
>
>
​The pg_basebackup is dying because the role specified, -U backup_admin,
has neither SUPERUSER nor REPLICATION privileges since those two privileges
are not programmed to be passed down via inheritance.  This is a feature.
As noted on the other thread one could ask for another feature (via another
role attribute) that tells PostgreSQL to pass down those privileges via
inheritance.​  That seems like the most useful solution if one believes
that having such an attribute would be an improvement over explicitly
defining whether specific login roles are replication or, the
all-inclusive, superuser.

The reason the "ALTER USER .. SET ROLE TO" doesn't make any difference here
is because pg_backup doesn't specify a database and the table
pg_db_role_setting, which where that command stores its data, is only
consulted after a successful connection to a specific database has been
established.  That doesn't happen here.

*Is there any limitation in pg_basebackup utility ?*
>

​I suppose...
if you look at it from the standpoint that pg_basebackup operates as the
physical data files level and not the SQL level.

Other's with more authority will voice their own opinions but I'm not where
changing the inheritance behavior is an option at this point.  Making ALTER
USER ... SET ROLE work here is plausible but hackish.  The new role
attribute just seems messy.

While I guess I get the appeal of having everything defined via group roles
and implicit inheritance it still sounds like a purely aesthetic dynamic
which is contrary to existing design decisions.

David J.

Re: [HACKERS] Ongoing issues with representation of empty arrays

[David G. Johnston]

On Mon, Apr 10, 2017 at 4:57 PM, Andrew Gierth 
wrote:

> The distinction between the standard representation of '{}' as an array
> with zero dimensions and nonstandard representations as a 1-dimensional
> array with zero elements has come up in a couple of contexts on the IRC
> channel recently.
>

​Just to add to the listing of annoyances.

The following makes me want some way, in SQL, to create an empty
1-dimensional array ...

SELECT array_agg(e_arr)
FROM ( VALUES (ARRAY['1']::text[]), (ARRAY[]::text[]) ) v (e_arr);
--ERROR:  cannot accumulate arrays of different dimensionality

We moved the goals posts nicely with bac27394a1c but not being able to mix
empty and non-empty arrays is problematic.  Ideally an empty array could
become an array of any dimension on-the-fly so that if even explicitly
dimensioned input to array_agg is 1-dimensioned then all empty arrays would
be promoted to 1-dimension and the resultant output would become two
dimensional.  unnest'ing such a structure would pose its own challenges
though...

David J.

Re: [HACKERS] error handling in RegisterBackgroundWorker

[David G. Johnston]

On Tue, Apr 11, 2017 at 11:10 AM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 4/11/17 11:47, David G. Johnston wrote:
> > ​A potential middle-ground is to start, but then only allow superuser
> > connections.
>
> Then you might as well start and allow all connections.  I don't see
> what this buys.
>
>
​If "leave it offline until it gets fixed" is on the table then there is
some underlying reason that we'd not want application (or replication)
users connecting to the database while it is in a degraded state.  Even if
one accepts that premise that doesn't mean that an administrator shouldn't
be allowed to login and do ad-hoc stuff; the goal of the prevention is to
disallow programmed external actors that assume/require that these
background worker processes are active from connecting while they are not.
This middle-ground accommodates that goal​ in a precise manner.

I don't have an opinion as to which extreme is better so in the absence I'm
in favor of "put control in the hands of the administrator" - this just
provides a slightly more usable environment for the administrator to
operate within.

David J.

Re: [HACKERS] error handling in RegisterBackgroundWorker

[David G. Johnston]

On Tue, Apr 11, 2017 at 8:33 AM, Tom Lane  wrote:

> Peter Eisentraut  writes:
> > On 4/10/17 23:22, Tom Lane wrote:
> >> Personally I'd err on the side of "starting up degraded is better than
> >> not starting at all".  Or maybe we should invent a GUC to let DBAs
> >> express their preference on that?
>
> > If we defaulted allow_degraded to yes, then users wouldn't find that
> > setting until they do start up degraded and want to fix things, in which
> > case they could just fix the settings that caused the degraded startup
> > in the first place.
>
> > If we defaulted to no, then I don't think any user would go in and
> > change it.  "Sure, I'll allow degraded startup.  That sounds useful."
>
> Well, they would change it when their server failed to start and they
> needed to start it rather than just rebuild from backups.  I'd be fine
> with defaulting it off.  I just don't want "can't make a loopback socket"
> to be equivalent to "you're screwed and you'll never see your data again".
>

​A potential middle-ground is to start, but then only allow superuser
connections.  At least then if the configuration problem is sitting
postgresql.conf.auto the superuser can issue ALTER SYSETM to fix it; and
can be reassured that worse case they can at least see their data.

If that was a soft setting they could also run a function to enable normal
access if they so choose.  In effect its a "default to off" mode with an
easy way to force the system to become live - but without a GUC (so they
couldn't make the decision permanent...which seems like a feature)

David J.

Re: [HACKERS] Ongoing issues with representation of empty arrays

[David G. Johnston]

On Mon, Apr 10, 2017 at 5:57 PM, Andrew Gierth 
wrote:

> Second is aclitem[], past bug #8395 which was not really resolved; empty
> ACLs are actually 1-dim arrays of length 0, and all the ACL functions
> insist on that, which means that you can't call aclexplode('{}') for
> example:
>
> https://www.postgresql.org/message-id/flat/CA%
> 2BTgmoZdDpTJDUVsgzRhoCctidUqLDyO8bdYwgLD5p8DwHtMcQ%40mail.gmail.com
>
> It's much less clear what to do about this one. Thoughts?
>

​After a quick Google search to get a feel for the landscape...​

At a high level the Access Control System is a high-risk area - it seems
like the above anomaly doesn't outweigh the risk of slogging through and
changing the mechanics.  But maybe I'm being overly paranoid in my
inexperience...

The question I'd have is whether people are doing '{}'::aclitem[] out of
habit or because there is no other way, in SQL, to generate an empty ACL
array?  If its the later we probably should supply one even if its not
documented for external use; none of these other functions are.

I.e., a no-arg "newacl" function where:  array_length(newacl(), 1) => 0

IOW - the response to Bug # 8395 is to make #3 workable with the correct
syntax.  That #2 would continue to return false is maybe annoying but
unless we are prepared (and capable) of making '{}'::aclitem[] an error I
don't see that we should worry about it.

David J.

p.s. A side question is whether a handful of reports and a couple of PGXN
extensions suggest that we should consider user-facing documenting some of
these things.

Re: [HACKERS] [PATCH] Warn users about duplicate configuration parameters

[David G. Johnston]

On Fri, Apr 7, 2017 at 8:29 AM, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:

> Andres, Tatsuo,
>
> Thank you for sharing your thoughts.
>
> > -1 - I frequently just override earlier parameters by adding an
> > include at the end of the file.  Also, with postgresql.auto.conf it's
> > even more common to override parameters.
>
> > -1 from me too by the same reason Andres said.
>
> I see no problem here. After all, it's just warnings. We can even add
> a GUC that disables them specially for experienced users who knows what
> she or he is doing. And/or add special case for postgresql.auto.conf.
>
>
​-1 for learning how the configuration system work via warning messages.

We've recently added pg_file_settings to provide a holistic view and the
docs cover the topic quite well.

David J.
​

Re: [HACKERS] Variable substitution in psql backtick expansion

[David G. Johnston]

On Mon, Apr 3, 2017 at 5:12 AM, Daniel Verite 
wrote:

> Queries can be as complex as necessary, they just have to fit in one line.


​Line continuation in general is missed though I thought something already
when in for 10.0 that improves upon this...​


> In no way at all,in the sense that, either you choose to use an SQL
> evaluator, or a client-side evaluator (if it exists), or a backtick
> expression.
> They are mutually exclusive for a single \if invocation.
>
> Client-side evaluation would have to be called with a syntax
> that is unambiguously different.


​Is that the universe: server, client, shell?

Shell already has backticks required
​Server, being the most common, ideally wouldn't need demarcation
Client thus would want its own symbol pairing to distinguish it from server.

Server doesn't need a leading marker but do we want to limit it to single
statements only and allow an optional trailing semi-colon (or backslash
command) which, if present, would end the "server" portion of the string
and allow for treatment of additional terms on the same line to be parsed
in a different context?

I'm conceptually for the implied "SELECT" idea.  It overlaps with plpgsql
syntax.

David J.

Re: [HACKERS] REFERENCES privilege should not be symmetric (was Re: [GENERAL] Postgres Permissions Article)

[David G. Johnston]

On Fri, Mar 31, 2017 at 10:40 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Fri, Mar 31, 2017 at 11:29 AM, Tom Lane  wrote:
> >> The argument for not back-patching a bug fix usually boils down to
> >> fear of breaking existing applications, but it's hard to see how
> >> removal of a permission check could break a working application ---
> >> especially when the permission check is as hard to trigger as this one.
> >> How many table owners ever revoke their own REFERENCES permission?
>
> > Sure, but that argument cuts both ways.  If nobody ever does that, who
> > will be helped by back-patching this?
> > I certainly agree that back-patching this change is pretty low risk.
> > I just don't think it has any real benefits.
>
> I think the benefit is reduction of user confusion.  Admittedly, since
> Paul is the first person I can remember ever having complained about it,
> maybe nobody else is confused.
>

​After going back-and-forth on this (and not being able to independently
come to the conclusion that what we are adhering to is actually a typo) I'm
going to toss my +1 in with Robert's.  If anyone actually complains about
the behavior and not just the documentation we could consider back-patching
if any release before 10.0 is still under support.

There have been non-bug fix improvements to the docs that didn't get
back-patched covering topics more confusing than this.  Expecting those
learning the system to consult the most recent version of the docs is
standard fare here.  From a practical perspective the revised current docs
will be applicable for past versions as long as one doesn't go a get their
REFERENCES permission revoked somehow.  If they do, and wonder why, the
docs and these list will be able to explain it reasonably well.

David J.​

Re: [HACKERS] increasing the default WAL segment size

[David G. Johnston]

On Wed, Mar 22, 2017 at 9:51 AM, Stephen Frost  wrote:

> Robert,
>
> * Robert Haas (robertmh...@gmail.com) wrote:
> > On Wed, Mar 22, 2017 at 12:22 PM, Stephen Frost 
> wrote:
> > > While I understand that you'd like to separate the concerns between
> > > changing the renaming scheme and changing the default and enabling this
> > > option, I don't agree that they can or should be independently
> > > considered.
> >
> > Well, I don't understand what you think is going to happen here.  Neither
> > Beena nor any other contributor you don't employ is obliged to write a
> > patch for those changes because you'd like them to get made, and Peter
> and
> > I have already voted against including them.  If you or David want to
> write
> > a patch for those changes, post it for discussion, and try to get
> consensus
> > to commit it, that's of course your right.  But the patch will be more
> than
> > three weeks after the feature freeze deadline and will have two committer
> > votes against it from the outset.
>
> This would clearly be an adjustment to the submitted patch, which
> happens regularly during the review and commit process and is part of
> the commitfest process, so I don't agree that holding it to new-feature
> level is correct when it's a change which is entirely relevant to this
> new feature, and one which a committer is asking to be included as part
> of the change.  Nor do I feel particularly bad about asking for feature
> authors to be prepared to rework parts of their feature based on
> feedback during the commitfest process.
>

​Maybe it can be fit in as part of the overall patch set but wouldn't
placing it either:

First. changing the name behavior and use the existing configure-time ​knob
to test it out

or

Second. commit the existing patch relying on the existing behavior and then
implement the rename changes using the new initdb-time knob to test it out.

​in a series make reasoning and discussing the change considerably easier?​


> I would have liked to have realized this oddity with the WAL naming
> scheme for not-16MB-WALs earlier too, but it's unfortunately not within
> my abilities to change that.  That does not mean that we shouldn't be
> cognizant of the impact that this new feature will have in exposing this
> naming scheme, one which there seems to be agreement is bad, to users.
>

> That said, David is taking a look at it to try and be helpful.
>
> Vote-counting seems a bit premature given that there hasn't been any
> particularly clear asking for votes.  Additionally, I believe Peter also
> seemed concerned that the existing naming scheme which, if used with,
> say, 64MB segments, wouldn't match LSNs either, in this post:
> 9795723f-b4dd-f9e9-62e4-ddaf6cd88...@2ndquadrant.com


​While my DBA skills aren't that great I would think that having a system
that relies upon the DBA learning how to mentally map between LSN IDs and
WAL​ files is a failure in UX in the first place.  The hacker-DBA might get
a kick out of being able to operate efficiently with that knowledge and
level of skill but the typical DBA would rather have something like "pg_wal
--lsn " that they can rely upon.  I would think tool builders would
likewise rather rely on us to tell them where to go look instead of
matching up two strings.

This kinda reminds me of the discussion regarding our version number
change.  We are going to expose broken tools that rely on the decimal
version number instead of the official integer one.  Here we expose tools
that rely on the equivalence between LSN and WAL filenames when using 16MB
WAL files.  What I haven't seen defined here is how those tools should be
behaving - i.e., what is our supported API.  If we lack an official way of
working with these values then maybe we shouldn't give users an initdb-time
way to change the WAL file size.

For the uninformed like myself an actual concrete example of an actual
program that would be affected would be helpful.

David J.

Re: [HACKERS] Removing binaries

[David G. Johnston]

On Tue, Mar 21, 2017 at 5:12 AM, Robert Haas  wrote:

>
> Here's another idea: what if we always created the default database at
> initdb time?  For example, if I initdb as rhaas, maybe it should
> create an "rhaas" database for me, so that this works:
>
> initdb
> pg_ctl start
> psql
>
> I think a big part of the usability problem here comes from the fact
> that the default database for connections is based on the username,
> but the default databases that get created have fixed names (postgres,
> template1).  So the default configuration is one where you can't
> connect.  Why the heck do we do it that way?
>
>
​I'd be curious to estimate how many users that have difficulties learning
how all this works actually run a manual initdb prior to beginning their
experimentation.  I suspect the percentage is fairly low.

Doing away with "the default database for psql is one named after the user"
seems like it would be more widely applicable.  I for one tend to name
things after what they do, or are used for, and thus have never benefited
from this particular design decision.

David J.

Re: [HACKERS] Our feature change policy

[David G. Johnston]

On Mon, Mar 20, 2017 at 9:18 AM, Bruce Momjian  wrote:

> .  #3 and #4 would need to be weighted depending on
> whether choosing them would delay progress, e.g. it did delay progress
> on standard-conforming strings, but the delay was determined to be
> reasonable.
>

w.r.t. standard-conforming strings to this day we are in indefinite
deprecation of the backward compatibility mechanics.  We simply made the
choice of whether to use the backward compatibility mode explicit when we
changed the GUC default.  For features with that possibility adding an "D.
Optionally, when applicable, maintain current behavior during release and
later switch the default behavior to the new mode after N years."  Odds are
if we are considering instituting item D we'd be content with discussing
the specifics of the case and not rely on any kind of rule or guideline to
define N.  As evidenced defaults and deprecation are orthogonal concerns.

David J.

Re: [HACKERS] Our feature change policy

[David G. Johnston]

On Mon, Mar 20, 2017 at 8:57 AM, Stephen Frost  wrote:

> Tom,
>
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > Stephen Frost  writes:
> > > * Bruce Momjian (br...@momjian.us) wrote:
> > >> 1.  make the change now and mention it in the release notes
> > >> 2.  #1, but also provide backward compatibility for 5+ years
> > >> 3.  mark the feature as deprecated and remove/change it in 5+ years
> > >> 4.  #3, but issue a warning for deprecated usage
> >



> I was imagining the changes to pg_stat_activity as possibly falling
> under #3/change, meaning that we'd wait for 5 years before actually
> renaming those columns, which is part of why I was objecting to that
> idea.
>

Which ends up boiling down to:

A. Make a change and document it in the release notes

B. If applicable, and desired, provide a 5 year backward​ compatibility
deprecation period (i.e., 3 =>[implies] 2). Generally 2 => 3 but the
deprecation period is undefined.

C. Optionally, if deprecating, provide explicit warnings when the
deprecated feature is used

Guidelines for when to desire the 5 year period would be helpful.  And also
acknowledge that we may wish to choose a shorter period of time, or
institute immediate removal, at our discretion.

Nothing says we cannot go longer than 5 years but given our support policy
I would say that we'd rarely desired to do so intentionally - the burden of
proof falling to those who would want to keep something longer.

David J.

Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)

[David G. Johnston]

On Sun, Mar 12, 2017 at 10:24 AM, Tom Lane  wrote:

>
> One point here is that we need to distinguish problems in the expression,
> which could arise from changing variable values, from some other types of
> mistakes like \elif with no preceding \if.  When you see something like
> that you pretty much have to treat it as a no-op; but I don't think that's
> a problem for scripting usage.
>

​One of my discarded write-ups from last night made a point that we don't
really distinguish between run-time and compile-time errors - possibly
because we haven't had to until now.

​If we detect what would be considered a compile-time error (\elif after
\else for instance) we could treat anything that isn't a conditional
meta-command as a no-op with a warning (and exit in stop-script mode)​.

There are only four commands and a finite number of usage permutations.
Enumerating and figuring out the proper behavior for each should be done.

Thus - ​If the expressions are bad they are considered false but the block
is created

If the flow-control command is bad the system will tell the user why and
how to get back to a valid state - the entire machine state goes INVALID
until a corrective command is encountered.

For instance:

\if
\else
\elif
warning: elif block cannot occur directly within an \else block.  either
start a new \if, \endif the current scope, or type \else to continue
entering commands into the existing else block.  no expression evaluation
has occurred.
\echo 'c'
warning: command ignored in broken \if block scope - see prior correction
options

Yes, that's wordy, but if that was shown the user would be able to
recognize their situation and be able to get back to their desired state.

Figuring out what the valid correction commands are for each invalid state,
and where the user is left, is tedious but mechanical.

So we'd need an INVALID state as well as the existing IGNORE state.

\endif would always work - but take you up one nesting level

The user shouldn't need to memorize the invalid state rules.  While we
could document them and point the reader there having them inline seems
preferable.
​

>
> We could imagine resolving this tension by treating failed \if expressions
> differently in interactive and noninteractive cases.  But I fear that cure
> would be worse than the disease.
>

​I don't think this becomes necessary - we should distinguish the error
types the same in both modes.​

Re: [HACKERS] \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)

[David G. Johnston]

On Sat, Mar 11, 2017 at 5:45 PM, Tom Lane  wrote:

>
> * Whether or not you think it's important not to expand skipped variables,
> I think that it's critical that skipped backtick expressions not be
> executed.
> ​ [...] ​
> I do not think that a skipped \if or \elif
> should evaluate its argument at all.
>
>
​[...]
​

> * I'm not on board with having a bad expression result in failing
> the \if or \elif altogether.  It was stated several times upthread
> that that should be processed as though the result were "false",
> and I agree with that.


​+1​

​Oddly, Corey was using you as support for this position...though without
an actual quote:

"""
Tom was pretty adamant that invalid commands are not executed. So in a case
like this, with ON_ERROR_STOP off:

\if false
\echo 'a'
\elif true
\echo 'b'
\elif invalid
\echo 'c'
\endif

Both 'b' and 'c' should print, because "\elif invalid" should not execute.
The code I had before was simpler, but it missed that.
"""
https://www.postgresql.org/message-id/CADkLM%3De9BY_-
PT96mcs4qqiLtt8t-Fp%3De_AdycW-aS0OQvbC9Q%40mail.gmail.com

Also,

Robert made a comment somewhere along the line about users wanting to
simply re-type the intended line if the "invalid" was interactive and due
to a typo.  That concern is pretty much limited to just the "\if" situation
- if you typo an "\elif" block you can just type "\elif" again and begin
yet another "\elif" block.  I say we live with it and focus on the UX - if
you type \if no matter what happens after you hit enter you are in a
conditional block and will need to \endif at some point. Re-typing the
correct \if command will just make you need another one of them.

David J.

Re: [HACKERS] Should we eliminate or reduce HUP from docs?

[David G. Johnston]

On Fri, Mar 10, 2017 at 3:51 PM, Alvaro Herrera <alvhe...@2ndquadrant.com>
wrote:

> David G. Johnston wrote:
> > On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
>
> > is incomplete.
>
> Sure.  We can just reword that along the lines of " ... and when a
> reload signal is received, see 19.1.3".  Don't you like that?
>
>
​WFM
​

>
> > The order or some of these items is interesting but given the general
> lack
> > of field complaints and questions it mustn't be confusion inducing.  Even
> > this thread isn't an actual complaint but rather concern about signals in
> > general.  Pulling the relevant paragraph out to its own section in 19.1
> was
> > my first reaction as well and has the merit of simplicity.
>
> Simplicity FTW.
>
>
​WFM​

David J.

Re: [HACKERS] Should we eliminate or reduce HUP from docs?

[David G. Johnston]

On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera <alvhe...@2ndquadrant.com>
wrote:

> David G. Johnston wrote:
> > On Fri, Mar 10, 2017 at 1:02 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
> > wrote:
>
> > > There are several ways to cause a config file reload (pg_ctl reload,
> > > pg_reload_conf, direct SIGHUP).  We could have a section in docs
> listing
> > > them all, and then all the other places that say a reload needs to
> occur
> > > simply refer the reader to that section.
> >
> > ​19.1.2 contains a fairly comprehensive coverage of the topic ​- but
> > postgres.conf is not the only thing that gets reloaded.  Specifically,
> > "Client Authentication" (chapter 20) is also affected.
>
> I think we could split 19.1.2 in two parts, where the first one is the
> current content minus the paragraph "The configuration file is reread".
> We'd create "19.1.3 Configuration File Reloads" to contain that
> paragraph, perhaps not with the exact current wording.
>

​If only 19 and 20 need it I would say its a coin-toss.​


> > One theory would be to consider "configuration reload" part of "18.
> Server
> > ... Operation" and document the mechanics there with forward references
> to
> > 19/Configuration and 20/Authentication.
>
> Dunno.  Given that other configuration elements such as config file
> placement are already in chapter 19, it seems strange to put reloading
> behavior in 18.
>
>
​It wouldn't be hateful to cross link to 19 from 20 - but assuming
pg_reload_conf() impacts pg_hba.conf​ (I don't know off-hand) the paragraph

"""
The pg_hba.conf file is read on start-up and when the main server process
receives a SIGHUP signal. If you edit the file on an active system, you
will need to signal the postmaster (using pg_ctl reload or kill -HUP) to
make it re-read the file.
"""

is incomplete.

Is "kill" portable?

The order or some of these items is interesting but given the general lack
of field complaints and questions it mustn't be confusion inducing.  Even
this thread isn't an actual complaint but rather concern about signals in
general.  Pulling the relevant paragraph out to its own section in 19.1 was
my first reaction as well and has the merit of simplicity.

David J.

Re: [HACKERS] Should we eliminate or reduce HUP from docs?

[David G. Johnston]

On Fri, Mar 10, 2017 at 1:02 PM, Alvaro Herrera 
wrote:

> Joshua D. Drake wrote:
>
> > I am a bad speaker, I am writing a talk three weeks before the conference
> > (as opposed to on the plane).
>
> Hah.
>
> > I noticed in the docs we still reference the
> > passing of SIGHUP for reloading conf file but we now have
> pg_reload_conf();
> >
> > It seems the use of pg_reload_conf() would provide a better canonical
> > interface to our users. Especially those users who are not used to
> > interacting with the OS (Windows, Oracle etc...) for databases.
>
> There are several ways to cause a config file reload (pg_ctl reload,
> pg_reload_conf, direct SIGHUP).  We could have a section in docs listing
> them all, and then all the other places that say a reload needs to occur
> simply refer the reader to that section.
>

​19.1.2 contains a fairly comprehensive coverage of the topic ​- but
postgres.conf is not the only thing that gets reloaded.  Specifically,
"Client Authentication" (chapter 20) is also affected.

One theory would be to consider "configuration reload" part of "18. Server
... Operation" and document the mechanics there with forward references to
19/Configuration and 20/Authentication.  The existing content in those
chapters discussing reload would then send the reader back to 18 for "how
to reload" and just state "when to reload" in their particular situations.

Any other spots that warrant the same treatment?

If we are going to touch this area up it might be worth a fresh
consideration of index entries too.

David J.

Re: [HACKERS] bytea_output vs make installcheck

[David G. Johnston]

On Thu, Mar 9, 2017 at 4:45 PM, Neha Khatri  wrote:

> On Fri, Mar 10, 2017 at 6:14 AM, Peter Eisentraut  ndquadrant.com> wrote:
>
>> On 2/14/17 16:50, Jeff Janes wrote:
>> > make installcheck currently fails against a server running
>> > with bytea_output = escape.
>> >
>> > Making it succeed is fairly easy, and I think it is worth doing.
>> >
>> > Attached are two options for doing that.  One overrides bytea_output
>> > locally where-ever needed, and the other overrides it for the entire
>> > 'regression' database.
>>
>> I would use option 2 here (ALTER DATABASE) and be done with it.  Some
>> people didn't like using ALTER DATABASE, but it's consistent with
>> existing use.  If someone wants to change that, that can be independent
>> of this issue.
>
>
> Sorry about the naive question, but if someone has set the GUC
> bytea_output = 'escape', then the intention seem to be to obtain the output
> in 'escape' format for bytea.
> With this, if an installcheck is done, that might also have been done with
> the expectation that the output will be in 'escape' format. In that case,
> how much is it justified to hard code the format for regression database?
> However, I agree that there are not many bytea outputs in the current
> regression suite
>
>
​At a high level (which is all I know here) ​If we leave behind tests that
at least exercise bytea_output='escape'​ mode to ensure it is functioning
properly then I'd have no problem having the testing of other features
dependent upon bytea_output, but that are coded to compare against the
now-default output format, set that runtime configurable mode to that which
they require.  If the choice of output mode is simply a byproduct we should
be free to set it to whatever we need for the currently executing test.

If a simple way of doing this involves fixing the default to what the suite
expects and one-off changing it when testing escape mode stuff that seems
like a reasonable position to take.  Having to set bytea_output when it
isn't the item under test seems like its just going to add noise.

David J.

Re: [HACKERS] UPDATE of partition key

[David G. Johnston]

On Sat, Feb 25, 2017 at 11:11 AM, Greg Stark <st...@mit.edu> wrote:

> On 24 February 2017 at 14:57, David G. Johnston
> <david.g.johns...@gmail.com> wrote:
> > I dislike an error.  I'd say that making partition "just work" here is
> > material for another patch.  In this one an update of the partition key
> can
> > be documented as shorthand for delete-returning-insert with all the
> > limitations that go with that.  If someone acceptably solves the ctid
> > following logic later it can be committed - I'm assuming there would be
> no
> > complaints to making things just work in a case where they only sorta
> > worked.
>
> Personally I don't think there's any hope that there will ever be
> cross-table ctids links. Maybe one day there will be a major new table
> storage format with very different capabilities than today but in the
> current architecture it seems like an impossible leap.
>

​How about making it work without a physical token dynamic?  For instance,
let the server recognize the serialization error but instead of returning
it to the client the server itself tries again.​


> I would expect everyone to come to terms with the basic idea that
> partition key updates are always going to be a corner case. The user
> defined the partition key and the docs should carefully explain to
> them the impact of that definition. As long as that explanation gives
> them something they can work with and manage the consequences of
> that's going to be fine.
>
> What I'm concerned about is that silently giving "wrong" answers in
> regular queries -- not even ones doing the partition key updates -- is
> something the user can't really manage. They have no way to rewrite
> the query to avoid the problem if some other user or part of their
> system is updating partition keys. They have no way to know the
> problem is even occurring.
>
> Just to spell it out -- it's not just "no-op updates" where the user
> sees 0 records updated. If I update all records where
> username='stark', perhaps to set the "user banned" flag and get back
> "9 records updated" and later find out that I missed a record because
> someone changed the department_id while my query was running -- how
> would I even know? How could I possibly rewrite my query to avoid
> that?
>

​But my point is that this isn't a regression from current behavior.  If I
deleted one of those starks and re-inserted them with a different
department_id that brand new record wouldn't be banned.  In short, my take
on this patch is that it is a performance optimization.  Making the UPDATE
command actually work as part of its implementation detail is a happy
byproduct.​

>From the POV of an external observer it doesn't have to matter whether the
update or delete-insert SQL was used.  It would be nice if the UPDATE
version could keep logical identity maintained but that is a feature
enhancement.

Failing if the other session used the UPDATE SQL isn't wrong; and I'm not
against it, I just don't believe that it is better than maintaining the
status quo semantics.

That said my concurrency-fu is not that strong and I don't really have a
practical reason to prefer one over the other - thus I fall back on
maintaining internal consistency.

IIUC ​it is already possible, for those who care to do so, to get a
serialization failure in this scenario by upgrading isolation to repeatable
read.

David J.

Re: [HACKERS] Make subquery alias optional in FROM clause

[David G. Johnston]

 On Fri, Feb 24, 2017 at 9:35 AM, David Fetter  wrote:

>
> > => SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
> > ERROR:  42703: column "?column" does not exist
> > LINE 2: SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
> >^
> > HINT:  Perhaps you meant to reference the column "x.?column?" or the
> > column "x.?column?".
>

This is indirectly pointing out the duplication ​since the hint is
specifying the exact same name twice...

I don't know how far comparing apples and oranges gets us here...and the
assignment of names to expression columns lacking aliases is a bit smarter
than given credit for here - e.g., it uses the name of the function in a
simple function call expression.

There is no risk of naming conflicts in pre-existing queries.  I say we do
something like:  pg_subquery_n and make it known that the value for "n"
will be chosen independent of names already present in the query.  We've
recently reserved the pg_ prefix for roles we might as well leverage that.
These names need only be available for internal needs; as a user I'd expect
it is be noted as an implementation detail that should not be relied upon.
Whether it needs to get exposed for technical reasons (e.g., dump/restore
and explain) I do not know.

David J.

Re: [HACKERS] UPDATE of partition key

[David G. Johnston]

On Friday, February 24, 2017, Simon Riggs  wrote:
>
> 2. I know that DB2 handles this by having the user specify WITH ROW
> MOVEMENT to explicitly indicate they accept the issue and want update
> to work even with that. We could have an explicit option to allow
> that. This appears to be the only way we could avoid silent errors for
> foreign table partitions.
>
>
This does, however, make the partitioning very non-transparent to every
update query just because it is remotely possible a partition-moving update
might occur concurrently.

I dislike an error.  I'd say that making partition "just work" here is
material for another patch.  In this one an update of the partition key can
be documented as shorthand for delete-returning-insert with all the
limitations that go with that.  If someone acceptably solves the
ctid following logic later it can be committed - I'm assuming there would
be no complaints to making things just work in a case where they only sorta
worked.

David J.

Re: [HACKERS] UPDATE of partition key

[David G. Johnston]

On Friday, February 24, 2017, Robert Haas  wrote:

> On Mon, Feb 20, 2017 at 2:58 PM, Amit Khandekar  > wrote:
> > I am inclined to at least have some option for the user to decide the
> > behaviour. In the future we can even consider support for walking
> > through the ctid chain across multiple relfilenodes. But till then, we
> > need to decide what default behaviour to keep. My inclination is more
> > towards erroring out in an unfortunate even where there is an UPDATE
> > while the row-movement is happening. One option is to not get into
> > finding whether the DELETE was part of partition row-movement or it
> > was indeed a DELETE, and always error out the UPDATE when
> > heap_update() returns HeapTupleUpdated, but only if the table is a
> > leaf partition. But this obviously will cause annoyance because of
> > chances of getting such errors when there are concurrent updates and
> > deletes in the same partition. But we can keep a table-level option
> > for determining whether to error out or silently lose the UPDATE.
>
> I'm still a fan of the "do nothing and just document that this is a
> weirdness of partitioned tables" approach, because implementing
> something will be complicated, will ensure that this misses this
> release if not the next one, and may not be any better for users.  But
> probably we need to get some more opinions from other people, since I
> can imagine people being pretty unhappy if the consensus happens to be
> at odds with my own preferences.
>
>
For my own sanity - the move update would complete successfully and break
every ctid chain that it touches.  Any update lined up behind it in the
lock queue would discover their target record has been deleted and
would experience whatever behavior their isolation level dictates for such
a situation.  So multi-partition update queries will fail to update some
records if they happen to move between partitions even if they would
otherwise match the query's predicate.

Is there any difference in behavior between this and a SQL writeable CTE
performing the same thing via delete-returning-insert?

David J.

Re: [HACKERS] Range Partitioning behaviour - query

[David G. Johnston]

On Thu, Feb 23, 2017 at 6:17 PM, Amit Langote  wrote:

> On 2017/02/24 8:38, Venkata B Nagothi wrote:
> > On Thu, Feb 23, 2017 at 3:14 PM, Amit Langote wrote:
> >> Upper bound of a range partition is an exclusive bound.  A note was
> added
> >> recently to the CREATE TABLE page to make this clear.
> >>
> >> https://www.postgresql.org/docs/devel/static/sql-createtable.html
> >
> >
> > Thanks. Actually, my confusion was that the upper bound value would be
> > included when "TO" clause is used in the syntax.
>
> Hmm, TO sounds like it implies inclusive.
>

​I think most common usage of the word ends up being inclusive but the word
itself doesn't really care.​

Dictionary.com has a good example:

"We work from nine to five." - you leave at the beginning of the 5 o'clock
hour (I'm going for casual usage here)

Since our implementation of ranges is half-open the usage here is
consistent with that concept.  That it doesn't match BETWEEN is actually
somewhat nice since you can use ranges for half-open and BETWEEN if you
want to be concise with fully-closed endpoints.  But it is one more thing
to remember.

David J.

Re: [HACKERS] Make subquery alias optional in FROM clause

[David G. Johnston]

On Wed, Feb 22, 2017 at 8:08 AM, Tom Lane  wrote:

> Or else not generate
> a name at all, in which case there simply wouldn't be a way to refer to
> the subquery by name; I'm not sure what that might break though.
>

​Yeah, usually when I want this I don't end up needing refer by name:

First I write:

SELECT * 
FROM 

The decide I need to do some result filtering

SELECT * FROM (

) --ooops, forgot the alias
WHERE ...

Its for interactive use - and in fact I don't think I'd want to leave my
production queries without names.

David J.

Re: [HACKERS] Make subquery alias optional in FROM clause

[David G. Johnston]

On Wed, Feb 22, 2017 at 8:08 AM, Tom Lane  wrote:

> Bernd Helmle  writes:
> >> From time to time, especially during migration projects from Oracle to
> > PostgreSQL, i'm faced with people questioning why the alias in the FROM
> > clause for subqueries in PostgreSQL is mandatory. The default answer
> > here is, the SQL standard requires it.
>
> Indeed.  When I wrote the comment you're referring to, quite a few years
> ago now, I thought that popular demand might force us to allow omitted
> aliases.  But the demand never materialized.  At this point it seems
> clear to me that there isn't really good reason to exceed the spec here.
> It just encourages people to write unportable SQL code.
>

​I'll contribute to the popular demand aspect but given that the error is
good and the fix is very simple its not exactly a strong desire.

My code is already unportable since I choose to use "::" for casting - and
I'm sure quite a few other PostgreSQL-specific things - so the portability
aspect to the argument is already thin for me and moreso given other DBMSes
already relax the requirement.

David J.​

Re: [HACKERS] gitlab post-mortem: pg_basebackup waiting for checkpoint

[David G. Johnston]

On Fri, Feb 17, 2017 at 4:22 PM, Tomas Vondra 
wrote:

> What about adding a paragraph into pg_basebackup docs, explaining that
> with 'fast' it does immediate checkpoint, while with 'spread' it'll wait
> for a spread checkpoint.
>

I agree that a better, and self-contained, explanation of the behaviors
that fast and spread invoke on the server should be included directly in
the pg_basebackup docs.

Additionally, a primary benefit of pg_basebackup is hiding the low-level
details from the user and in that spirit the cross-reference link to
Section 25.3.3 "Making a Base Backup Using the Low Level API" should be
removed.  If there is specific information there that a user of
pg_basebackup needs it should be presented properly in the application
documentation.

The top of pg_basebackup points to the entire 25.3 chapter but the flow
from there is solid - coverage of pg_basebackup occurs and points out the
low level API for those whose needs are not fully served by the bundled
application.  If one uses pg_basebackup they should be able to stop at that
point, go back to the app page, and continue reading and skip all of 25.3.3

The term "spread checkpoint" isn't actually a defined term in our
docs...and aside from the word spread itself describing out a checkpoint
works, it isn't used outside of pg_basebackup docs.  So "it will wait for a
spread checkpoint" doesn't really work - "it will start and then wait for a
normal checkpoint to complete" does.

More holistically (i.e., feel free to skip)

This paragraph from 25.3.3:

"""
This is because it performs a checkpoint, and the I/O required for the
checkpoint will be spread out over a significant period of time, by default
half your inter-checkpoint interval (see the configuration parameter
checkpoint_completion_target). This is usually what you want, because it
minimizes the impact on query processing. If you want to start the backup
as soon as possible, change the second parameter to true.
"""

is good but buried and seems like it would be more visible in Chapter 30.
Reliability and the Write-Ahead Log. To there both the internals and
backbackup pages could point the reader.  There isn't a chapter dedicated
to checkpoints - nor does there need to be - but a section in 30 seems
warranted as being the official reference.  Right now you have to skim the
configuration variables and "WAL Configuration" and "CHECKPOINT" and "base
backup API and pg_basebackup" to cover everything.  A checkpoint chapter
with that paragraph as a focus would allow the other items to simply say
"immediate or normal checkpoint" as needed and redirect the reader for
additional context as to the trade-offs of each - whether done manually or
during some form of backup script.

David J.

Re: [HACKERS] [PROPOSAL] Temporal query processing with range types

[David G. Johnston]

On Wed, Feb 15, 2017 at 12:24 PM, Robert Haas  wrote:

> On Tue, Jan 24, 2017 at 4:32 AM, Peter Moser  wrote:
> >> Using common terms such as ALIGN and NORMALIZE for such a specific
> >> functionality seems a bit wrong.
> >
> > Would ALIGN RANGES/RANGE ALIGN and NORMALIZE RANGES/RANGE NORMALIZE be
> better
> > options? We are also thankful for any suggestion or comments about the
> syntax.
>
> So it seems like an ALIGN or NORMALIZE option is kind of like a JOIN,
> except apparently there's no join type and the optimizer can never
> reorder these operations with each other or with other joins.  Is that
> right?  The optimizer changes in this patch seem fairly minimal, so
> I'm guessing it can't be doing anything very complex here.
>
> + * INPUT:
> + * (r ALIGN s ON q WITH (r.ts, r.te, s.ts, s.te)) c
> + * where q can be any join qualifier, and r.ts, r.te, s.ts,
> and s.t
> e
> + * can be any column name.
> + *
> + * OUTPUT:
> + * (
> + * SELECT r.*, GREATEST(r.ts, s.ts) P1, LEAST(r.te, s.te) P2
> + *  FROM
> + *  (
> + * SELECT *, row_id() OVER () rn FROM r
> + *  ) r
> + *  LEFT OUTER JOIN
> + *  s
> + *  ON q AND r.ts < s.te AND r.te > s.ts
> + *  ORDER BY rn, P1, P2
> + *  ) c
>
> It's hard to see what's going on here.  What's ts?  What's te?  If you
> used longer names for these things, it might be a bit more
> self-documenting.
>

Just reasoning out loud here...​

ISTM ts and te are "temporal [range] start" and "temporal [range] end"​ (or
probably just the common "timestamp start/end")

​From what I can see it is affecting an intersection of the two ranges and,
furthermore, splitting the LEFT range into sub-ranges that match up with
the sub-ranges found on the right side.  From the example above this seems
like it should be acting on self-normalized ranges - but I may be missing
something by evaluating this out of context.

r1 [1, 6] [ts, te] [time period start, time period end]
s1 [2, 3]
s2 [3, 4]
s3 [5, 7]

r LEFT JOIN s ON (r.ts < s.te AND r.te > s.ts)

r1[1, 6],s1[2, 3] => [max(r.ts, s.ts),min(r.te, s.te)] => r1[1, 6],d[2, 3]
r1[1, 6],s2[3, 4] => [max(t.ts, s.ts),min(r.te, s.te)] => r1[1, 6],d[3, 4]
r1[1, 6],s3[5, 7] => [max(t.ts, s.ts),min(r.te, s.te)] => r1[1, 6],d[5, 6]

Thus the intersection is [2,6] but since s1 has three ranges that begin
between 2 and 6 (i.e., 2, 3, and 5) there are three output records that
correspond to those sub-ranges.

The description in the OP basically distinguishes between NORMALIZE and
ALIGN in that ALIGN, as described above, affects an INTERSECTION on the two
ranges - discarding the non-overlapping data - while NORMALIZE performs the
alignment while also retaining the non-overlapping data.

The rest of the syntax seems to deal with selecting subsets of range
records based upon attribute data.

David J.

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[David G. Johnston]

On Thu, Feb 9, 2017 at 12:08 PM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Mon, Feb 6, 2017 at 12:29 PM, Magnus Hagander 
> wrote:
> >>> Here is what I have, 6 votes clearly stated:
> >>> 1. Rename nothing: Daniel,
> >>> 2. Rename directory only: Andres
> >>> 3. Rename everything: Stephen, Vladimir, David S, Michael P (with
> >>> aliases for functions, I could live without at this point...)
>
> > [ vote-counting ]
>
> > Therefore, I plan to go ahead and do #3.  Somebody's probably going to
> > jump in now with another opinion but I think this thread's gone on
> > long enough.
>
> Agreed, let's just get it done.
>
> Although this doesn't really settle whether we ought to do 3a (with
> backwards-compatibility function aliases in core) or 3b (without 'em).
> Do people want to re-vote, understanding that those are the remaining
> choices?
>

​I wouldn't oppose:

CREATE EXTENSION give_me_my_xlog_back;

but my prior thoughts lead me toward not including such functions in the
bootstrap catalog.

David J.

Re: [HACKERS] Idea on how to simplify comparing two sets

[David G. Johnston]

On Wed, Feb 8, 2017 at 10:30 AM, Tom Lane  wrote:

> David Fetter  writes:
> > On Wed, Feb 08, 2017 at 11:22:56AM -0500, Tom Lane wrote:
> >> Yes.  I think a new set-operation keyword would inevitably have to
> >> be fully reserved --- UNION, INTERSECT, and EXCEPT all are --- which
> >> means that you'd break every application that has used that word as
> >> a table, column, or function name.
>
> > I've long wanted a SYMMETRIC DIFFERENCE join type, that being the only
> > elementary set operation not included in join types, but nobody at the
> > SQL standards committee seems to have cared enough to help.
>
> I wonder whether you could shoehorn it in with no new reserved word
> by spelling it "EXCEPT SYMMETRIC", which could be justified by the
> precedent of BETWEEN SYMMETRIC.  But not sure what to do with
> duplicate rows (ie, if LHS has two occurrences of row X and RHS
> has one occurrence, do you output X?)
>

​Without SYMMETRIC its defined to return:

​max(m-n,0)

with SYMMETRIC I'd think that would just change to:

abs(m-n)

Then you still have to apply ALL or DISTINCT on the above result.

David J.

Re: [HACKERS] Idea on how to simplify comparing two sets

[David G. Johnston]

On Tue, Feb 7, 2017 at 8:58 AM, Tom Lane  wrote:

> Joel Jacobson  writes:
> > Currently there is no simple way to check if two sets are equal.
>
> Uh ... maybe check whether SELECT set1 EXCEPT SELECT set2
> and SELECT set2 EXCEPT SELECT set1 are both empty?
>

​SELECT set1 FULL EXCEPT SELECT set2 ?

Matches with the concept and syntax of "FULL JOIN"​.

or

SELECT set1 XOR SELECT set2

That said I'm not sure how much we want to go down this road on our own.
It'd be nice to have when its needed but its not something that gets much
visibility on these lists to suggest a large pent-up demand.

IS NOT DISTINCT FROM doesn't imply bi-direction any better than EXCEPT does
... if we are going to add new syntax I'd say it should at least do that.

David J.

Re: [HACKERS] One-shot expanded output in psql using \G

[David G. Johnston]

On Mon, Jan 30, 2017 at 8:35 AM, Stephen Frost  wrote:

> Tom,
>
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > Stephen Frost  writes:
> > > This particular bike-shedding really doesn't seem to be terribly useful
> > > or sensible, to me.  \gx isn't "consistent" or "descriptive", frankly.
> >
> > Why not?  To me it reads as "\g with an x option".  The "x" refers to
> > the implied "\x", so it's not an arbitrary choice at all.
>
> That's not how '\dx' works, as I pointed out, so I don't see having the
> second character being 'x' to imply "\x mode" makes sense.
>

​It makes perfect sense ... it just not something that we've had the option
to do before (no, I haven't tried to figure out if we've missed an
opportunity or two here).​

​[...]​

without actual consistency across commands which take 'x'
> as a sub-command I don't see the 'descriptive' argument as holding much
> weight either
> ​.
>

​Arguing that something is mnemonic doesn't require any precedence - though
one could wish for better uses of mnemonic naming choices for past and
future items.

In scripting uses of psql I could see wanting to use "\gx" and, say "\gn"
(i.e., always output in non-expanded mode) instead of ";" so that for any
given query I can specify the exact layout I care about and don't have to
jump through hoops to toggle \x back and forth.

Limiting consideration of the use-case of this feature to interactive use
is, IMHO, a mistake.  In the copious use of psql scripting that I do I
would find both options I named above to be useful to directly and
concisely communicate the display intent of each query I execute.

David J.

Re: [HACKERS] One-shot expanded output in psql using \G

[David G. Johnston]

On Mon, Jan 30, 2017 at 8:14 AM, Tom Lane  wrote:

> Stephen Frost  writes:
> > This particular bike-shedding really doesn't seem to be terribly useful
> > or sensible, to me.  \gx isn't "consistent" or "descriptive", frankly.
>
> Why not?  To me it reads as "\g with an x option".  The "x" refers to
> the implied "\x", so it's not an arbitrary choice at all.
>
> The main problem I see with \G is that it's a dead end.  If somebody
> comes along next year and says "I'd like a variant of \g with some other
> frammish", what will we do?  There are no more case variants to use.
>
> In short, really the direction this ought to go in is \g[options] [file]
> which is perfectly consistent with precedents in psql such as \d.
> But there isn't any place where we've decided that upper case means
> a variant of a lower case command.
>

​+1
​

Re: [HACKERS] Query fails when SRFs are part of FROM clause (Commit id: 69f4b9c85f)

[David G. Johnston]

On Fri, Jan 27, 2017 at 3:13 PM, David G. Johnston <
david.g.johns...@gmail.com> wrote:

> In any case the more idiomatic way of writing your query these days (since
> 9.4 came out) is:
>
> SELECT *
> FROM pg_constraint pc
> LEFT JOIN LATERAL generate_series(1, case when contype in ('f','p','u')
> then array_upper(pc.conkey, 1) else 0 end) gs ON true;
>
>
Supposedly ​should work back to 9.3​, mis-remembered when LATERAL was
released.

David J.

Re: [HACKERS] Query fails when SRFs are part of FROM clause (Commit id: 69f4b9c85f)

[David G. Johnston]

On Fri, Jan 27, 2017 at 5:28 AM, Rushabh Lathia 
wrote:

> Consider the below test;
>
> CREATE TABLE tab ( a int primary key);
>
> SELECT  *
> FROM pg_constraint pc,
> CAST(CASE WHEN pc.contype IN ('f','u','p') THEN generate_series(1,
> array_upper(pc.conkey, 1)) ELSE NULL END AS int) AS position;
>
> Above query is failing with "set-valued function called in context that
> cannot
> accept a set". But if I remove the CASE from the query then it working
> just good.
>
> Like:
>
> SELECT  *
> FROM pg_constraint pc,
> CAST(generate_series(1, array_upper(pc.conkey, 1)) AS int) AS position;
>
> This started failing with 69f4b9c85f168ae006929eec44fc44d569e846b9. It
> seems
> check_srf_call_placement() sets the hasTargetSRFs flag and but when the
> SRFs
> at the rtable ofcourse this flag doesn't get set. It seems like missing
> something
> their, but I might be completely wrong as not quire aware of this area.
>
>
I'm a bit surprised that your query actually works...and without delving
into source code its hard to explain why it should/shouldn't or whether the
recent SRF work was intended to impact it.

In any case the more idiomatic way of writing your query these days (since
9.4 came out) is:

SELECT *
FROM pg_constraint pc
LEFT JOIN LATERAL generate_series(1, case when contype in ('f','p','u')
then array_upper(pc.conkey, 1) else 0 end) gs ON true;

generate_series is smart enough to return an empty set (instead of erroring
out) when provided with (1,0) as arguments.

David J.

Re: [HACKERS] One-shot expanded output in psql using \G

[David G. Johnston]

On Fri, Jan 27, 2017 at 8:31 AM, Stephen Frost  wrote:

> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > D'Arcy Cain wrote:
> >
> > > I am a pretty heavy user of psql but I don't think that that would be
> so
> > > helpful.  I assume you mean a new option, let's call it "\X" the
> causes the
> > > next query to be expanded.  I type "\X" then a query.  I realize that
> I made
> > > a mistake and have to redo the query so I have to type "\X" again.  If
> I
> > > forget then I have to run the query yet again.
> >
> > I think the suggestion is that \G replaces \g (which is the same thing
> > as the semicolon).  So you would do this:
> >
> > SELECT * FROM table WHERE table_status = 1; % get a short list; normal
> output
> > SELECT * FROM table WHERE table_id = 123 \G % drill down to one ID
>
> Uh, I figured it was more like \g, which just re-runs the last query..
> As in, you'd do:
>
> table pg_proc; % blargh, I can't read it like this
> \G % ahh, much nicer
>

​This information surprised me.  It was unexpected that the last
successfully executed query remains in the query buffer until the next SQL
(and not meta) command is started.  I was expecting that as soon as result
was returned to the screen the current query buffer would be cleared in
preparation for the next query.​

A sentence or two describing this behavior (or, more generally the query
buffer itself), probably placed at the end of the "Entering SQL Commands"
section, would help to make this common knowledge.

David J.

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[David G. Johnston]

On Thu, Jan 26, 2017 at 4:19 PM, Andres Freund  wrote:

>
> We decided s/pg_xlog/pg_wal/ was necessary because people lost their
> data, and we couldn't come up with a reasonable way to change it without
> the name. The tradeoff is dataloss vs. dealing with directory renaming
> in a few lowlevel tools.  So we decided to change the name.  It seems
> breakage was unavoidable.
>
> For me the renaming of functions, binaries, etc, is different because
> the benefit is long-term avoidance of a bit of confusion, with the
> shorter term price being tool breakages and confusion because
> documentation/guides/... for different versions don't apply anymore, and
> the search terms don't help anymore.  But we're still changing this,
> even though breakage seems avoidable.
>

Well stated and compelling.
​

>
> And that fits into the bigger topic of us doing minor cleanups without a
> lot of concern for backward compatibility, e.g. like us whacking things
> around in pg_stat_activity for most of the last releases - nearly all of
> those could have been done in a compatible manner, without even smelling
> that badly.
>
> I hear these complaints about postgres most frequently: 1) replication
> sucks. 2) way too slow on analytics queries. 3) existing admin tools
> suck. 4) self written admin tools (required due to 3)) constantly break.
>
> There's a lot being done on 1) and 2). There's very little in-core
> progress about 3). We're getting worse on 4).
>

​I would posit, broadly, and without any evidence, that changes like we are
discussing here, will go toward helping on 3 since (hopefully) as
PostgreSQL becomes a more appealing option in the marketplace more quality
developers will be drawn toward using their skills to improve its
ecosystem.  Mandating uniformity in areas like this speaks toward
professionalism.

Given that "there's a lot being done on #1" it would seem that right now is
an excellent time to make these changes - so that the new guides and tools
that leverage those enhancements can use the WAL terminology and have its
presence be consistently present throughout the system.

If there was a reasonably short horizon for features or capabilities that
make this kind of renaming breaking easier to accommodate I would say we
could probably wait for it to be completed first.  But AFAIK there isn't
anything in the works that would allow individual users to easily enable a
backward-compatibility mode for the mid-level API that we are largely
touching here.

It may be that this is a straw that breaks the camel's back for some users
of PostgreSQL who are fed up with #4 ... I don't know ... but I'm
reasonably confident that new users a couple of years from now would have a
better experience with our product with these changes in place.  Its an
aggressive play for growth - and that entails risk and upsetting those
invested in the status-quo (which are are already doing per your #1).

David J.

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[David G. Johnston]

On Thu, Jan 26, 2017 at 3:28 PM, Andres Freund  wrote:

> I *personally* don't think it's worth
> changing all this without taking more care about backward compat than
> we're apparently willing to do.  I'm ok with loosing that argument.  I
> just don't think the previous concensus for a narrower change can be
> used for the wider one
> ​.
>

​It would probably help others if you could brain dump a bit as to the
benefit of the status-quo compared to both the 9.6 behavior and the all-in
one.

Honestly, the reference to the camel sticking their nose in a tent was
supposed to be taken humorously, the serious explanation that it meant to
convey was already very well presented by Robert.  The only reason I added
the wiki link was to give those who would have no clue about the metaphor a
chance at understanding it.

I actually took your response as:  "why the f**k is he talking about
camels?" and started laughing...

Dave