Re: [HACKERS] pg_dump dump catalog ACLs

[David G. Johnston]

On Wed, Mar 2, 2016 at 1:54 PM, Stephen Frost  wrote:

> * Joe Conway (m...@joeconway.com) wrote:
> > On 03/01/2016 08:00 AM, Tom Lane wrote:
> > > Joe Conway  writes:
> > >> Would it be a terrible idea to add some attribute to ACLs which can be
> > >> used to indicate they should not be dumped (and supporting syntax)?
> > >
> > > Yes, we'd need some way to mark non-null ACLs as being "built-in
> > > defaults".  I do not see the need to have SQL syntax supporting that
> > > though.
> >
> > I was thinking the supporting syntax might be used by extensions, for
> > example.
>
> I tend to agree with Tom that we don't really need SQL syntax for this.
>
> > > Actually, wouldn't you need to mark individual aclitems as built-in
> > > or not?  Consider a situation where we have some function foo() that
> > > by default has EXECUTE permission granted to some built-in "pg_admin"
> > > role.  If a given installation then also grants EXECUTE to "joe",
> > > what you really want to have happen is for pg_dump to dump only the
> > > grant to "joe".  Mentioning pg_admin's grant would tie the dump to
> > > a particular major PG version's idea of what the built-in roles are,
> > > which is what I'm arguing we need to avoid.
> >
> > Yes, I guess it would need to be a per aclitem attribute.
>
> Agreed.
>
> > > I guess this could also be addressed by having two separate aclitem[]
> > > columns, one that is expected to be frozen after initdb and one for
> > > user-added grants.
> >
> > Yeah, that would work, but seems kind of ugly.
>
> Rather than have two aclitem[] columns in every catalog, since this
> information is only used by pg_dump and not during normal operation, we
> could use the approach that pg_description took and have an independent
> catalog table which just contains all non-NULL "system" ACLs.  We could
> populate it at the bottom of system_views.sql, so that we don't have to
> explicitly think about updating that table whenever there's a change to
> what the default ACLs are.
>
> I don't see any reason it couldn't be used by extensions also, though
> we'd have to do a bit more work on pg_dump to make it actually dump
> out any non-default ACLs for extension-owned objects.
>

​It sounds like this train of thought would resolve this complaint?

​
http://www.postgresql.org/message-id/CADmxfmmz-ATwptaidTSAF0XE=cpeikmyc00sj6t9xf6kcv5...@mail.gmail.com

​Namely allowing users to edit permissions on extension objects and have
those changes dumped and then restored after the dependent CREATE EXTENSION
command is executed during pg_restore.

Did I interpret that right?

David J.
​

Re: [HACKERS] pg_dump dump catalog ACLs

[David G. Johnston]

On Wed, Mar 2, 2016 at 2:44 PM, Joe Conway  wrote:

> On 03/02/2016 12:54 PM, Stephen Frost wrote:
> > * Joe Conway (m...@joeconway.com) wrote:
> >> On 03/01/2016 08:00 AM, Tom Lane wrote:
> >>> Yes, we'd need some way to mark non-null ACLs as being "built-in
> >>> defaults".  I do not see the need to have SQL syntax supporting that
> >>> though.
> >>
> >> I was thinking the supporting syntax might be used by extensions, for
> >> example.
> >
> > I tend to agree with Tom that we don't really need SQL syntax for this.
>
> > I don't see any reason it couldn't be used by extensions also, though
> > we'd have to do a bit more work on pg_dump to make it actually dump
> > out any non-default ACLs for extension-owned objects.
>
> Without any syntax, what does the extension do, directly insert into
> this special catalog table?
>
>
​The desire in the thread I linked was for the user, not the extension, to
alter the permissions.  In that context (and possibly here as well)
PostgreSQL would (somehow?) recognize the ​target as being special and thus
requiring adding or updating an entry into the supplemental catalog table
when the usual GRANT/REVOKE SQL command is issued.

​In effect any object dependent upon an EXTENSION or that already exists in
this special catalog table would need to have the supplemental procedure
executed.

David J.
​

[HACKERS] WHERE clause not used when index is used (9.5)

[David G. Johnston]

Placing this specific message onto -bugs while keeping -hackers and
removing -novice.

Editing subject to include version and remove list identifiers.

There is continuing discussion on -hackers though mostly about how to do
this right in the future.  The specific problem stems from an attempted
performance improvement which is likely to be reverted.

David J.


On Tue, Mar 1, 2016 at 8:40 AM, Tom Lane  wrote:

> Tobias Florek  writes:
> > When creating an index to use for an ORDER BY clause, a simple query
> > starts to return more results than expected.  See the following detailed
> > log.
>
> Ugh.  That is *badly* broken.  I thought maybe it had something to do with
> the "abbreviated keys" work, but the same thing happens if you change the
> numeric column to integer, so I'm not very sure where to look.  Who's
> touched btree key comparison logic lately?
>
> (Problem is reproducible in 9.5 and HEAD, but not 9.4.)
>
>
> > Create enough test data for planer to use an index (if exists) for the
> > condition.
>
> > CREATE TABLE "index_cond_test" AS
> > SELECT
> >   (10 + random() * 10)::int AS "final_score",
> >   round((10 + random() * 10)::numeric, 5) "time_taken"
> > FROM generate_series(1, 1) s;
>
>
> > Run control query without an index (will be less than 1 rows). Pay
> > attention to tuples of (20,a) with a > 11.
>
> > SELECT *
> > FROM "index_cond_test"
> > WHERE (final_score, time_taken) < (20, 11)
> > ORDER BY final_score DESC, time_taken ASC;
>
>
> > Or wrapped in count(*), to make it even more obvious
>
> > SELECT count(*) FROM ( SELECT *
> >FROM "index_cond_test"
> >WHERE (final_score, time_taken) < (20, 11)
> >ORDER BY final_score DESC, time_taken ASC) q;
>
> > Create the index
>
> > CREATE INDEX "index_cond_test_ranking" ON "index_cond_test" USING
> btree (final_score DESC, time_taken ASC);
>
> > Run test query (will return all 1 rows)
>
> > SELECT *
> > FROM "index_cond_test"
> > WHERE (final_score, time_taken) < (20, 11)
> > ORDER BY final_score DESC, time_taken ASC;
>
> > or wrapped
>
> > SELECT count(*) FROM ( SELECT *
> >FROM "index_cond_test"
> >WHERE (final_score, time_taken) < (20, 11)
> >ORDER BY final_score DESC, time_taken ASC) q;
>
> regards, tom lane
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Tue, Mar 8, 2016 at 3:57 PM, Corey Huinker 
wrote:

>
>> I'm pretty meh about the whole idea of this function, though,
>> actually, and I don't see a single clear +1 vote for this
>> functionality upthread.  (Apologies if I've missed one.)  In the
>> absence of a few of those, I recommend we reject this.
>>
>
> Just David and Vik so far. The rest were either against(Simon),
> meh(Robert) or +1ed/-1ed the backpatch, leaving their thoughts on the
> function itself unspoken.
>
> Happy to make the changes above if we're moving forward with it.
>

​I'll toss a user-land +1 for this.

David J.

Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

Adding -hackers for consideration in the Commitfest.

Thanks!

David J.

>>>Original request by me

http://www.postgresql.org/message-id/CAKFQuwZqjz-je3Z=8jdodym3jm-n2ul4cuqy5vh8n75e5v1...@mail.gmail.com

When executing a query using \watch in psql the first execution of the
query includes "Title is [...]" when \pset title is in use.  Subsequent
executions do not.  Once that first display goes off-screen the information
in the title is no longer readily accessible.  If using \watch for a
long-running monitoring query it can be helpful to incorporate some context
information into the title.

-- Forwarded message --
From: Michael Paquier 
Date: Thu, Jan 28, 2016 at 6:01 AM
Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
interations
To: "David G. Johnston" 
Cc: Tom Lane , "pgsql-gene...@postgresql.org" <
pgsql-gene...@postgresql.org>


On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
 wrote:
> On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <
michael.paqu...@gmail.com>
> wrote:
>>
>> On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
>>  wrote:
>> > So how about:
>> >
>> > + snprintf(title, strlen(myopt.title) + 50,
>> > + _("Watch every %lds\t%s\t%s"),
>> > +  sleep, head_title, asctime(localtime(&timer)));
>>
>> I would just keep the timestamp and the title separated so what do you
>> think about that instead?
>> Watch every Xs   $timestamp
>> $head_title
>
>
> That works.  I like having the title immediately above the table.
>
> The other option that came to mind would be to place the time information
> after the table display while leaving the title before it.  On an output
> that requires more vertical space than is available in the terminal one
> would no longer have to scroll up to confirm last execution time.  If
doing
> this I'd probably get rid of any logic that attempts to center the time
> information on the table and simply leave it left-aligned.

​And the example:
​
OK, attached is an updated patch. How does that look?

   Watch every 5sFri Jan 29 13:06:31 2016

This is a medium length title
repeat


--
 

(1 row)
​
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..3241d27 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,8 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	bool		 *head_title = NULL;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,6 +3035,18 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title = pg_malloc(strlen(myopt.title) + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+		title = pg_malloc(50);
+
 	for (;;)
 	{
 		int			res;
@@ -3045,8 +3058,13 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
- sleep, asctime(localtime(&timer)));
+		if (head_title)
+			snprintf(title, strlen(myopt.title) + 50,
+	 _("Watch every %lds\t%s\n%s"),
+	 sleep, asctime(localtime(&timer)), head_title);
+		else
+			snprintf(title, 50, _("Watch every %lds\t%s"),
+	 sleep, asctime(localtime(&timer)));
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3059,7 +3077,11 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		if (res == 0)
 			break;
 		if (res == -1)
+		{
+			pg_free(title);
+			pg_free(head_title);
 			return false;
+		}
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,6 +3106,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
+	pg_free(title);
+	pg_free(head_title);
 	return true;
 }
 

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

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Thu, Mar 10, 2016 at 8:58 AM, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
> wrote:
> > On 10 March 2016 at 06:53, Michael Paquier 
> > wrote:
> >>
> >> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >>  wrote:
> >> > Robert Haas wrote:
> >> >> I'm pretty meh about the whole idea of this function, though,
> >> >> actually, and I don't see a single clear +1 vote for this
> >> >> functionality upthread.  (Apologies if I've missed one.)  In the
> >> >> absence of a few of those, I recommend we reject this.
> >> >
> >> > +1
> >>
> >> I'm meh for this patch.
> >
> >
> > "meh" == +1
> >
> > I thought it meant -1
>
> In my case it meant, like, -0.5.  I don't really like adding lots of
> utility functions like this to the default install, because I'm not
> sure how widely they get used and it gradually increases the size of
> the code, system catalogs, etc.  But I also don't want to block
> genuinely useful things.  So basically, I'm not excited about this
> patch, but I don't want to fight about it either.
>

​I tend to think we err toward this too much.  This seems like development
concerns trumping usability.  Consider that anything someone took the time
to write and polish to make committable to core was obviously genuinely
useful to them - and for every person capable of actually taking things
that far there are likely many more like myself who cannot but have
encountered the, albeit minor, usability annoyance that this kind of
function seeks to remove.

I really don't care that the codebase is marginally larger or that there is
another few records in the catalog - I hope that our code organization and
performance is capable of handling it (and many more).

The overhead of adding an entirely new concept to core would give me more
pause in that situation but this function in particular simply fleshes out
what the user community sees to be a minor yet notable lack in our existing
offering of the generate_series() feature.  Its threshold for meeting
"genuinely" should be considerably lower than for more invasive or complex
features such as those that require entirely new syntax (e.g., the recently
rejected ALTER ROLE patch) to implement.

If something can be done with a user-defined function on stock PostgreSQL,
especially for concepts or features we already have, the decision to commit
a c-language function that someone else has written and others have
reviewed should, IMO, be given the benefit of assumed usefulness.

My $0.02

David J.

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Thu, Mar 10, 2016 at 9:30 AM, Michael Paquier 
wrote:

> On Thu, Mar 10, 2016 at 4:58 PM, Robert Haas 
> wrote:
> > On Thu, Mar 10, 2016 at 10:30 AM, Simon Riggs 
> wrote:
> >> On 10 March 2016 at 06:53, Michael Paquier 
> >> wrote:
> >>>
> >>> On Wed, Mar 9, 2016 at 12:13 AM, Alvaro Herrera
> >>>  wrote:
> >>> > Robert Haas wrote:
> >>> >> I'm pretty meh about the whole idea of this function, though,
> >>> >> actually, and I don't see a single clear +1 vote for this
> >>> >> functionality upthread.  (Apologies if I've missed one.)  In the
> >>> >> absence of a few of those, I recommend we reject this.
> >>> >
> >>> > +1
> >>>
> >>> I'm meh for this patch.
> >>
> >>
> >> "meh" == +1
> >>
> >> I thought it meant -1
> >
> > In my case it meant, like, -0.5.  I don't really like adding lots of
> > utility functions like this to the default install, because I'm not
> > sure how widely they get used and it gradually increases the size of
> > the code, system catalogs, etc.  But I also don't want to block
> > genuinely useful things.  So basically, I'm not excited about this
> > patch, but I don't want to fight about it either.
>
> I am of the same feeling, at -0.5. I don't feel like putting -1 for
> this patch, as I don't really understand why this is worth adding more
> complexity in the code for something that can be done with
> generate_series using timestamps. Also, why only dates? And why not
> other units like hours or seconds?
>

​A date is a type, hours and seconds are not types.  To use hours and
seconds you need timestamp (I guess we could offer a "time" version of this
function too) which we already have.  Also, not choosing to implement
something else generally shouldn't preclude something that exists and have
genuine value from being committed.

Obviously there is some user-land annoyance at having to play with
timestamp when all one really cares about is date.  Given the prevalence of
date usage in user-land this is not surprising.

We're not forcing anyone to review this that doesn't see that it is worth
their time.  We are asking th
​at​
the people that the community has placed in a position of authority spend
some a limited amount of effort reviewing a minor addition that has been
deemed desirable and that has already been reviewed and deemed something
that meets the project's technical requirements.
​  The expectation is that the amount of ongoing support this function
would require is similar to that of the existing generate_series functions.​


​This is something that can be easily added by the user as a SQL function -
its complexity cannot be so great as to be deemed a risk to the system but
including its c-language variant.  As you said, we already do something
very similar for timestamps so the marginal complexity being added
shouldn't be significant.

If you are going to -1 or -0.5 for "adds too much complexity" it would be
helpful to know specifics.  Scanning the thread the only real concern was
dealing with infinity - which is already a complexity the current functions
have so there is no "additional" there - but maybe I've missed something.

I understand Robert's position and while I find it to be community-hostile
this is an open source project and so I accept that this is a possible
outcome.  But as soon as he asked for some +1s he got them (mostly without
reasons but the reality is that the request was for desire) and a few "I
vote -0.5 since my dislike is only half-baked".  And the fact is that a
single +1 for the idea likely represents many people at large who would use
the function if present while I suspect most of those who could offer an
informed -1 are already on this list.  The vast majority probably don't
care either way as long as we don't introduce bugs.

David J.

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Thu, Mar 10, 2016 at 11:45 AM, Robert Haas  wrote:

> On Thu, Mar 10, 2016 at 11:33 AM, David G. Johnston
>  wrote:
> > I tend to think we err toward this too much.  This seems like development
> > concerns trumping usability.  Consider that anything someone took the
> time
> > to write and polish to make committable to core was obviously genuinely
> > useful to them - and for every person capable of actually taking things
> that
> > far there are likely many more like myself who cannot but have
> encountered
> > the, albeit minor, usability annoyance that this kind of function seeks
> to
> > remove.
>
> Sure, an individual function like this has almost no negative impact.
> On the other hand, working around its absence is also trivial.  You
> can create a wrapper function that does exactly this in a couple of
> lines of SQL.  In my opinion, saying that people should do that in
> they need it has some advantages over shipping it to everyone.  If you
> don't have it and you want it, you can easily get it.  But what if you
> have it and you don't want it, for example because what you really
> want is a minimal postgres installation?


I'd rather cater to the people who are content that everything in
PostgreSQL is of high quality and ignore those things that they have no
immediate need for - and then are happy to find out that when they have a
new need PostgreSQL already provides them well thought-out and tested tool
that they can use.

We purport to be highly extensible but that doesn't preclude us from being
well-stocked at the same time.

And by not including these kinds of things in core the broader ecosystem is
harmed since not every provider or PostgreSQL services is willing or
capable of allowing random third-party extensions; nor is adding 20
"important and generally useful to me but an annoyance to the project"
functions to every database I create a trivial exercise.  But it is trivial
to ignore something that exists but that I have no need for.

You can't take anything in
> core back out again, or at least not easily.  Everything about core is
> expanding very randomly - code size, shared memory footprint, all of
> it.  If you think that has no downside for users, I respectfully
> disagree.


Attempts to limit that expansion seemingly happen "very randomly" as well.​

If there is a goal and demand for a "minimalist" installation then we don't
seem to have anything going on that is actively trying to make it a
reality.  I'm likely being a bit myopic here but at the same time the
increased footprint from JSON, parallel queries, and the like dwarf the
contribution a handful of C-language functions would add.

I do understand why more trivial features are scrutinized the way they are
but I still hold the opinion that features such as this should generally be
given the benefit of inclusion unless there are concrete technical concerns.

David J.

Re: [HACKERS] PREPARE dynamic SQL in plpgsql

[David G. Johnston]

On Fri, Mar 11, 2016 at 4:45 PM, Koichi Suzuki  wrote:

> Hi,
>
> Does someone know how to prepare a synamic SQL statement in plpgsql?
>
> All the examples, PG documents describe only about preparing static SQL
> statement.
>
>
You might want to rephrase the question.  From the pl/pgsql documentation:

"Note: The PL/pgSQL EXECUTE statement is not related to the EXECUTE SQL
statement supported by the PostgreSQL server. The server's EXECUTE
statement cannot be used directly within PL/pgSQL functions (and is not
needed)."

http://www.postgresql.org/docs/9.5/interactive/plpgsql-statements.html#PLPGSQL-STATEMENTS-EXECUTING-DYN

So even if you could SQL-PREPARE you wouldn't be able to execute it.

The linked section describes how to construct and execute dynamic queries
in pl/pgsql.  The short answer is you create variable of type text and then
EXECUTE it.

stmt := format('SELECT * FROM %i', in_tbl_name);
EXECUTE stmt [...]

David J.

Re: [HACKERS] PREPARE dynamic SQL in plpgsql

[David G. Johnston]

On Fri, Mar 11, 2016 at 4:45 PM, Koichi Suzuki  wrote:

> Hi,
>
> Does someone know how to prepare a synamic SQL statement in plpgsql?
>
> All the examples, PG documents describe only about preparing static SQL
> statement.
>
>
This is not an appropriate question for the -hackers list.  It is better
asked on either -general or, more correctly, -novice.

It is also somewhat lacking in detail - including an example of what you've
tried or read would be a nice touch.
​
David J.

Re: [HACKERS] Performance improvement for joins where outer side is unique

[David G. Johnston]

On Saturday, March 12, 2016, David Rowley 
wrote:

> On 12 March 2016 at 11:43, Tom Lane >
> wrote:
> > I wrote:
> >> I wondered why, instead of inventing an extra semantics-modifying flag,
> >> we couldn't just change the jointype to *be* JOIN_SEMI when we've
> >> discovered that the inner side is unique.
> >
> > BTW, to clarify: I'm not imagining that we'd make this change in the
> > query jointree, as for example prepjointree.c might do.  That would
> appear
> > to add join order constraints, which we don't want.  But I'm thinking
> that
> > at the instant where we form a join Path, we could change the Path's
> > jointype to be JOIN_SEMI or JOIN_SEMI_OUTER if we're able to prove the
> > inner side unique, rather than annotating the Path with a separate flag.
> > Then that representation is what propagates forward.
>
> Thanks for looking at this.
>
> Yes that might work, since we'd just be changing the jointype in the
> JoinPath, if that path was discarded if favour of, say the commutative
> variant, which was not "unique inner", then it shouldn't matter, as
> the join type for that path would be the original one.
>
> The thing that might matter is that, this;
>
>
> explain (costs off) select * from t1 inner join t2 on t1.id=t2.id
>  QUERY PLAN
> --
>  Hash Join
>Hash Cond: (t1.id = t2.id)
>->  Seq Scan on t1
>->  Hash
>  ->  Seq Scan on t2
>
> could become;
>
>   QUERY PLAN
> --
>  Hash Semi Join
>Hash Cond: (t1.id = t2.id)
>->  Seq Scan on t1
>->  Hash
>  ->  Seq Scan on t2
>
> Wouldn't that cause quite a bit of confusion? People browsing EXPLAIN
> output might be a bit confused at the lack of EXISTS/IN clause in a
> query which is showing a Semi Join. Now, we could get around that by
> adding JOIN_SEMI_INNER I guess, and just displaying that as a normal
> inner join, yet it'll behave exactly like JOIN_SEMI!
>
>
Don't the semantics of a SEMI JOIN also state that the output columns only
come from the outer relation? i.e., the inner relation doesn't contribute
either rows or columns to the final result?  Or is that simply
an implementation artifact of the fact that the only current way to perform
a semi-join explicitly is via exists/in?

David J.

Re: [HACKERS] Performance improvement for joins where outer side is unique

[David G. Johnston]

On Saturday, March 12, 2016, Tom Lane  wrote:

> "David G. Johnston" > writes:
> > Don't the semantics of a SEMI JOIN also state that the output columns
> only
> > come from the outer relation? i.e., the inner relation doesn't contribute
> > either rows or columns to the final result?  Or is that simply
> > an implementation artifact of the fact that the only current way to
> perform
> > a semi-join explicitly is via exists/in?
>
> I think it's an artifact.  What nodes.h actually says about it is you get
> the values of one randomly-selected matching inner row, which seems like
> a fine definition for the purposes we plan to put it to.
>
>
But is it a definition that actually materializes anywhere presently?

I'm not sure what we consider an authoritative source but relational
algebra does define the results of semi and anti joins as only containing
rows from main relation.

https://en.m.wikipedia.org/wiki/Relational_algebra

Given that this is largely internals (aside from the plan explanations
themselves) I guess we can punt for now but calling an inner or outer join
a semijoin in this case relies on a non-standard definition of semijoin -
namely that it is an optimized variation of the other joins instead of a
join type in its own right.  This is complicated further in that we
do implement a true semijoin (using exists) while we allow for an anti join
to be non-standard if expressed using "left join ... is null" instead of
via "not exists".

Calling these optimizations outer/inner+semi/anti preserves the ability to
distinguish these versions from the standard definitions.  I do like ithe
idea of it being exposed and encapsulated as a distinct join type instead
of being an attribute.

David J.

Re: [HACKERS] Performance improvement for joins where outer side is unique

[David G. Johnston]

On Saturday, March 12, 2016, David G. Johnston 
wrote:

> On Saturday, March 12, 2016, Tom Lane  > wrote:
>
>> "David G. Johnston"  writes:
>> > Don't the semantics of a SEMI JOIN also state that the output columns
>> only
>> > come from the outer relation? i.e., the inner relation doesn't
>> contribute
>> > either rows or columns to the final result?  Or is that simply
>> > an implementation artifact of the fact that the only current way to
>> perform
>> > a semi-join explicitly is via exists/in?
>>
>> I think it's an artifact.  What nodes.h actually says about it is you get
>> the values of one randomly-selected matching inner row, which seems like
>> a fine definition for the purposes we plan to put it to.
>>
>>
> But is it a definition that actually materializes anywhere presently?
>
> I'm not sure what we consider an authoritative source but relational
> algebra does define the results of semi and anti joins as only containing
> rows from main relation.
>
> https://en.m.wikipedia.org/wiki/Relational_algebra
>

Pondering it more calling these optimizations "semi" joins further
distances us from the meaning of "semi" as used in relational algebra.  The
half that semi refers to IS that only one half of the tables are returned.
That you only get a single row of output regardless of multiple potential
matches is simply a consequence of this and general set theory.

In short "semi" communicates a semantic meaning as to the intended output
of the query irrespective of the data upon which it is executed.  We now
are hijacking the and calling something "semi" if by some chance the data
the query is operating against happens to be accommodating to some
particular optimization.

This seems wrong on definitional and cleanliness grounds.

So while I'm still liking the idea of introducing specializations of outer
and inner joins I think calling them "semi" joins adds a definitional
inconsistency we are better off avoiding.

This came about because calling something "outer semi join" struck me as
odd.

Something like "outer only join" and "inner only join" comes to mind.
Consider the parallel between this and "index only scan".  Learning that
"only" means "join the outer row to the (at most for outer) one and only
row in the inner relation" doesn't seem to much of a challenge.

David J.

Re: [HACKERS] Re: Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Thu, Mar 17, 2016 at 8:41 AM, David Steele  wrote:

> On 3/17/16 11:30 AM, David G. Johnston wrote:
> > On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker  > <mailto:corey.huin...@gmail.com>>wrote:
> >
> > On Thu, Mar 17, 2016 at 10:00 AM, David Steele  > <mailto:da...@pgmasters.net>> wrote:
> >
> > On 3/17/16 4:49 AM, Dean Rasheed wrote:
> >
> > > On 16 March 2016 at 23:32, David Steele  <mailto:da...@pgmasters.net>> wrote:
> > >
> > >>
> > >> I think in this case it comes down to a committer's judgement
> so I have
> > >> marked this "ready for committer" and passed the buck on to
> Álvaro.
> > >
> > > So I was pretty much "meh" on this patch too, because I'm not
> > > convinced it actually saves much typing, if any.
> > >
> > > However, I now realise that it introduces a
> backwards-compatibility
> > > breakage. Today it is possible to type
> > >
> > > SELECT * FROM generate_series('01-01-2000'::date,
> '01-04-2000', '7 days');
> >
> > It can also be broken as below and this is even scarier to me:
> >
> >
> > Above and below are the same query​...
>
> Not sure I agree.  My point was that even if developers were pretty
> careful with their casting (or are using two actual dates) then there's
> still possibility for breakage.
>

With the first argument casted to date it doesn't matter whether you cast
the second argument as the pseudo-type anyelement will take its value from
the first argument and force the second to date.

The main problem is that the system tries to cast unknown to integer based
upon finding gs(date, date, integer) and fails without ever considering
that gs(ts, ts, interval) would succeed.


> > postgres=# SELECT * FROM generate_series('01-01-2000'::date,
> > '01-04-2000'::date, '7 days');
> > ERROR:  invalid input syntax for integer: "7 days"
> > LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7
> > days');
> > <...>
> >
> > I see two ways around this:
> >
> > 1. Drop the step parameter entirely. My own use cases only ever
> > require the step values 1 and -1, and which one the user wants can
> > be inferred from (start < end). This would still change the output
> > type where a person wanted timestamps, but instead input two dates.
> >
> > ​Undesirable.​
>
> Very undesirable.  Week intervals are a very valid use case and I don't
> like the automagic interval idea.
>
> >
> > 2. Rename the function date_series() or generate_series_date()
> >
> > I still think this is an important function because at the last
> > several places I've worked, I've found myself manufacturing a query
> > where some event data is likely to have date gaps, but we want to
> > see the date gaps or at least have the 0 values contribute to a
> > later average aggregate.
> >
> >
> > ​I'd call it "generate_dates(...)" and be done with it.
> >
> > We would then have:
> >
> > generate_series()
> > generate_subscripts()
> > generate_dates()
>
> To me this completely negates the idea of this "just working" which is
> why it got a +1 from me in the first place.  If I have to remember to
> use a different function name then I'd prefer to just cast on the
> timestamp version of generate_series().
>
>
​So let the user decide whether to trade-off learning a new function name
but getting dates instead of timestamps or going through the hassle of
casting.​

For me, it would have been a nice bonus if the generate_series() could be
used directly but I feel that the desired functionality is desirable and
the name itself is of only minor consideration.

I can see that newbies might ponder why two functions exist but they should
understand "backward compatibility".

I suspect that most people will simply think: "use generate_series for
numbers and use generate_dates for, well, dates".  The ones left out in the
cold are those wondering why they use "generate_series" for timestamp
series with a finer precision than date.  I'd be willing to offer a
"generate_timestamps" to placate them.  And might as well toss in
"generate_numbers" for completeness - though now I likely doom the
proposal...so I'll stick with just add generate_dates so the behavior is
possible without superfluous casting or the need to write a custom
function.  Dates are too common a unit of measure to leave working with
them this cumbersome.

David J.

Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

Figured out it had to be added to 2016-09...done

On Wed, Mar 9, 2016 at 11:40 PM, David G. Johnston <
david.g.johns...@gmail.com> wrote:

> Adding -hackers for consideration in the Commitfest.
>
> Thanks!
>
> David J.
>
> >>>Original request by me
>
>
> http://www.postgresql.org/message-id/CAKFQuwZqjz-je3Z=8jdodym3jm-n2ul4cuqy5vh8n75e5v1...@mail.gmail.com
>
> When executing a query using \watch in psql the first execution of the
> query includes "Title is [...]" when \pset title is in use.  Subsequent
> executions do not.  Once that first display goes off-screen the information
> in the title is no longer readily accessible.  If using \watch for a
> long-running monitoring query it can be helpful to incorporate some context
> information into the title.
>
> -- Forwarded message --
> From: Michael Paquier 
> Date: Thu, Jan 28, 2016 at 6:01 AM
> Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
> interations
> To: "David G. Johnston" 
> Cc: Tom Lane , "pgsql-gene...@postgresql.org" <
> pgsql-gene...@postgresql.org>
>
>
> On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
>  wrote:
> > On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <
> michael.paqu...@gmail.com>
> > wrote:
> >>
> >> On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
> >>  wrote:
> >> > So how about:
> >> >
> >> > + snprintf(title, strlen(myopt.title) + 50,
> >> > + _("Watch every %lds\t%s\t%s"),
> >> > +  sleep, head_title, asctime(localtime(&timer)));
> >>
> >> I would just keep the timestamp and the title separated so what do you
> >> think about that instead?
> >> Watch every Xs   $timestamp
> >> $head_title
> >
> >
> > That works.  I like having the title immediately above the table.
> >
> > The other option that came to mind would be to place the time information
> > after the table display while leaving the title before it.  On an output
> > that requires more vertical space than is available in the terminal one
> > would no longer have to scroll up to confirm last execution time.  If
> doing
> > this I'd probably get rid of any logic that attempts to center the time
> > information on the table and simply leave it left-aligned.
>
> ​And the example:
> ​
> OK, attached is an updated patch. How does that look?
>
>Watch every 5sFri Jan 29 13:06:31 2016
>
> This is a medium length title
> repeat
>
> 
> --
>  
> 
> (1 row)
> ​
>
>
>

Re: [HACKERS] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Thursday, March 17, 2016, Michael Paquier 
wrote:

> On Fri, Mar 18, 2016 at 8:16 AM, Tom Lane  > wrote:
> > David Steele > writes:
> >> On 3/17/16 7:00 PM, Tom Lane wrote:
> >>> The message I saw was post-1-March.  If it was in fact submitted in
> >>> time for 2016-03, then we owe it a review.
> >
> >> I meant to add the CF record and forgot:
> >> https://commitfest.postgresql.org/9/480
> >> It was added 2016-01-13 by Michael Paquier.
> >
> > OK, so it looks like David's 10-Mar patch was actually just a repost of
> > Michael's 28-Jan patch, which was already in the queue to be reviewed in
> > 2016-03 (and hasn't yet been).  So the addition to 2016-09 was simply
> > erroneous and should be deleted.
>
> My mistake I guess. I should have mentioned as well on this thread
> that I registered it.
>
>
And I didn't look hard enough on the CF site...sorry 'bout that.  All good
now and I've learned for next time.

David J.

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

[David G. Johnston]

On Wed, Mar 16, 2016 at 4:39 PM, Tom Lane  wrote:

> Jim Nasby  writes:
> > On 3/3/16 4:51 AM, Pavel Stehule wrote:
> >> CREATE TABLE a(a int);
> >> CREATE TABLE b(a a.a%TYPE)
> >>
> >> And the people expecting the living relation between table a and table
> >> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
> >> a.a or drop a?
> >>
> >> So this is reason, why I don't would this feature in SQL side.
>
> > I don't buy that. plpgsql doesn't work that way, so why would this?
> > *especially* with the %TYPE decorator.
>
> Yeah.  The %TYPE decorator doesn't work like that in the core parser
> either: when you use it, the referenced type is determined immediately
> and then it's just as if you'd written that type name to begin with.
>

I'm missing something here...%TYPE ends up getting parsed repeatedly and so
appears to be change if the variable upon which it is based changes - even
if once parsed it remains constant for the lifetime of the function's
evaluation.​

I guess what is being said is that the "constant" behavior in SQL ends up
being permanent because a given statement is only ever conceptually parsed
and executed a single time - unlike a function body.  The nature of any
solution would still have the same characteristics within a function
because the inherent re-parsing nature and not because of any direct
capability of %TYPE itself.

I do not see a reason for any of these "type operators" to work
> differently.
>
> Another analogy that might help make the point is
>
> set search_path = a;
> create table myschema.tab(f1 mytype);
> set search_path = b;
>
> If there are types "mytype" in both schemas a and b, is myschema.tab.f1
> now of type b.mytype?  No.  The meaning of the type reference is
> determined when the command executes, and then you're done.
>
​
And its no different than our treatment of "*"

CREATE VIEW test_view
SELECT *
FROM temp_table;

Adding columns to temp_table doesn't impact which columns the view returns.

David J.​

Re: [HACKERS] Make primnodes.h gender neutral

[David G. Johnston]

On Thu, Mar 17, 2016 at 2:17 PM, Gavin Flower  wrote:

> On 18/03/16 09:41, Joshua D. Drake wrote:
>
>> On 03/17/2016 01:36 PM, Alvaro Herrera wrote:
>>
>> [...]
>
>>
>>
>>> (*) I'm probably going to be expelled from the project for saying this,
>>> but I very much doubt that female coders stay away from PostgreSQL just
>>> because some files say "he" in comments rather than "she" or "he or she"
>>> or "one" or "they".  They probably have different reasons for staying
>>> away from the project.
>>>
>>
>> Wanna bet? There is a very loud movement about this. We can either:
>>
>> A. Start fighting about it
>>
>> B. Just fix it, it doesn't matter anyway and it doesn't hurt the quality
>> of the code or the documentation
>>
>> JD
>>
>
> I strongly think that gender should not be mentioned unless it is relevant
> - as constructs like 'he or she' are clumsy and distract from what is being
> commented on, not to mention that some rare people are: neither, both, or
> ambiguous (from research I did when I read a rather curious article).
>
> Other than 'one', 'their', 'they', &' them' - there are role specific
> references like 'user', 'developer', & 'DBA', ... that can be used where
> appropriate.
>
> I tend to prefer the term 'Gender Appropriate' rather than 'Gender
> Neutral', as sometimes mentioning gender IS very relevant!
>

​That's only half the issue.  If some people want to review every new patch
for gender appropriateness and point out any problems before those patches
get committed I'm doubting anyone is going to complain.

The question here is whether we should fix the wording of a comment that
was added in, and exists unchanged since, 7.2

IMO we can be more sensitive to these issues in the present without having
to apologize for (and fix) this project's history and the writing of people
who may no longer even be around.  Hopefully this compromise position is
sufficiently accommodating - I am personally fine if the people with real
control decide to operate under this premise.

​If we are going to make a concerted effort to change historical writing we
might as well just take the hit and "s/he/one/g"​.  Later, if someone
reading the revised wording finds it distasteful they can fix those
instances that are so egregious or presently-relevant to warrant the effort.

If we do this how many new developers are we expecting to subscribe to the
-hackers list and make serious contributions - say, by reviewing the large
backlog of patches we presently have?

David J.

Re: [HACKERS] Re: Add generate_series(date,date) and generate_series(date,date,integer)

[David G. Johnston]

On Thu, Mar 17, 2016 at 7:57 AM, Corey Huinker 
wrote:

> On Thu, Mar 17, 2016 at 10:00 AM, David Steele 
> wrote:
>
>> On 3/17/16 4:49 AM, Dean Rasheed wrote:
>>
>> > On 16 March 2016 at 23:32, David Steele  wrote:
>> >
>> >>
>> >> I think in this case it comes down to a committer's judgement so I have
>> >> marked this "ready for committer" and passed the buck on to Álvaro.
>> >
>> > So I was pretty much "meh" on this patch too, because I'm not
>> > convinced it actually saves much typing, if any.
>> >
>> > However, I now realise that it introduces a backwards-compatibility
>> > breakage. Today it is possible to type
>> >
>> > SELECT * FROM generate_series('01-01-2000'::date, '01-04-2000', '7
>> days');
>>
>> It can also be broken as below and this is even scarier to me:
>>
>
Above and below are the same query​...


>
>> postgres=# SELECT * FROM generate_series('01-01-2000'::date,
>> '01-04-2000'::date, '7 days');
>> ERROR:  invalid input syntax for integer: "7 days"
>> LINE 1: ...te_series('01-01-2000'::date, '01-04-2000'::date, '7 days');
>>
>> And only works when:
>>
>> postgres=# SELECT * FROM generate_series('01-01-2000'::date,
>> '01-04-2000'::date, '7 days'::interval);
>> generate_series
>> 
>>  2000-01-01 00:00:00+00
>> (1 row)
>> --
>> -David
>> da...@pgmasters.net
>>
>
> Not sure what's wrong with the patch, but I get a clean one to you pending
> the outcome of the design discussion. v4 just ripped out the infinity
> tests, so v3 is valid for the issue you found..
>
> So the function clobbers the situation where the user meant a timestamp
> range but instead gave two dates, and meant an interval but gave a string.
> I'm curious how generate_series() for numeric didn't have the same issue
> with floats.
>
>
​The numeric forms use anyelement for all three arguments but the timestamp
version uses an explicit interval for the third.​


I see two ways around this:
>
> 1. Drop the step parameter entirely. My own use cases only ever require
> the step values 1 and -1, and which one the user wants can be inferred from
> (start < end). This would still change the output type where a person
> wanted timestamps, but instead input two dates.
>

​Undesirable.​

2. Rename the function date_series() or generate_series_date()
>
> I still think this is an important function because at the last several
> places I've worked, I've found myself manufacturing a query where some
> event data is likely to have date gaps, but we want to see the date gaps or
> at least have the 0 values contribute to a later average aggregate.
>
>
​I'd call it "generate_dates(...)" and be done with it.

We would then have:

generate_series()
generate_subscripts()
generate_dates()

David J.
​

Re: [HACKERS] Make primnodes.h gender neutral

[David G. Johnston]

On Thursday, March 17, 2016, Robert Haas  wrote:

> On Thu, Mar 17, 2016 at 6:34 PM, Chapman Flack  > wrote:
> > For those of us who are outside of the twitterverse sort of on purpose,
> > are there a few representative links you could post? Maybe this is such
> > fresh breaking news Google hasn't spidered it yet, but I didn't find
> > any reference to the primnodes language when I looked, and I really am
> > curious to see just exactly what kind of issue is being made around
> it
>
> Similarly here.  The comment implies that the user is male.  It
> shouldn't.  Let's fix it in whatever way is most expedient and move
> on.  If at some point we are overwhelmed with a slough of patches
> making similar changes, we can at that time ask for them to be
> consolidated, just as we would do for typo fixes, grammar fixes, or
> warning fixes.  It is not necessary to insist on that that now because
> we are not faced with any such issue at this time.  If we gain a
> reputation as a community that is not willing to make reasonable
> efforts to use gender-neutral language, it will hurt us far more than
> the comment itself.  Let's not go there.


>
And if someone had just posted the patch and not prefaced it "we need to
check all of our comments for gender usage" it probably would have slipped
thorough without comment.

But that isn't what happened and since, based upon previous comments, we
expected many other commits of this sort we rightly discussed how to do
it.  The original author indeed figured one patch per file was probably a
good way to do things but I can others would seem to prefer one targeted
patch.

Beyond that it's the usual bike shedding when it comes to writing...which
was the original contra-vote.

David J.

Re: [HACKERS] pgbench - allow backslash-continuations in custom scripts

[David G. Johnston]

On Sunday, March 20, 2016, Tom Lane  wrote:

>
> * Allow backslash commands to span lines, probably by adopting the
> rule that backslash immediately followed by newline is to be ignored
> within a backslash command.  This would not be compatible with psql,
> though, at least not unless we wanted to change psql too.
>
>
This would be appreciated.  The main case I find wanting this is writing
out long \copy expressions.  Solving really complex ones using temporary
tables works but being able to spread it out over multiple lines would be a
welcomed addition.

David J.

Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Mon, Mar 21, 2016 at 8:03 AM, Robert Haas  wrote:

> On Sun, Mar 20, 2016 at 9:31 AM, Michael Paquier
>  wrote:
> > And the patch attached gives the following output:
> > With title:
> > =# \watch 1
> > Watch every 1sSun Mar 20 22:28:38 2016
> > popo
> >  a
> > ---
> >  1
> > (1 row)
>

​This doesn't show the blank line above "popo" that the prior example
had...​

>
> > And without title:
> > Watch every 1sSun Mar 20 22:29:31 2016
> >
> >  a
> > ---
> >  1
> > (1 row)
>
>
​Unchanged from present behavior - but its not obvious that the watch line
is center-aligned​

And does everybody agree that this is a desirable change?
>

​Adding the title is desirable.  While I'm inclined to bike-shed this
anything that gets it in I can live with and so I'm content letting the
author/committer decide where exactly things (including whitespace) appear.

​It is a bit odd that the "Watch every %s" gets centered if the result
is wide but that the title remains left-aligned.

The minimally invasive change would be the following:

>optional title<
>watch<
>(blank line)
>headers
>head-body divider
>body
>optional footer

Though I like the idea of:

>optional title
>(blank line - if Title present)
>headers
>head-body divider
>body
>optional footer
>watch

​David J.​

Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Mon, Mar 21, 2016 at 10:14 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > Well, the title isn't normally centered, but yeah, that is odd.  Yeah,
> > that is odd.  Come to think of it, I think I might have expected the
> > title to appear *above* "Watch every %s", not below it.  That might
> > decrease the oddness.
>
> AFAICS, it appears *beside* it with this patch.  It's only below if the
> terminal is narrow enough that it wraps to there.
>
> > As for letting the committer decide, I don't care about this
> > personally at all, so I'm only looking at it to be nice to the people
> > who do.  Whatever is the consensus is OK with me.  I just don't want
> > to get yelled at later for committing something here, so it would be
> > nice to see a few votes for whatever we're gonna do here.
>
> I'm still of the opinion that what would make the most sense is to replace
> the "Watch every Ns" text with the user-given title, if there is one.
> I ran that up the flagpole already and didn't get a lot of salutes, but
> it seems to respond to your concern that the user title ought to be first.
>
> Regardless of that, I concur with your complaints about coding style, in
> particular with the need to repeat the magic constant 50 in several
> places.  Also, I think the patch makes do_watch return the wrong result
> code for the (typical) case where we exit because of query cancel not
> PSQLexecWatch failure.
>
> So on the whole, I'd do it as attached.
>
​
I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:

if (user_title)​
​snprintf(title, title_len, "%s\t%s (%ld​s)", user_title,
asctime(localtime(&timer)), sleep)

"""
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

col1
-
1
"""

David J.

Re: [HACKERS] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Monday, March 21, 2016, Tom Lane  wrote:

> "David G. Johnston" > writes:
> > I'd rather not omit sleep but removing "Watch every" is fine (preferred
> > actually), so:
> > Title Is Here Mon Mar 21 15:05:06 2016 (5s)
>
> Meh ... seems a bit awkward to me.  Couldn't you include " (5s)" in the
> title, if you want that info?  If it's variable, you could still
> accommodate that:


Actually, only if it's a variable that you setup and repeat and you show.
A bit cumbersome and mixes the parts that are title and those that are
present only because you are watching.


> regression=# \set delay 5
> regression=# \pset title 'My Title (':delay' s)'
> Title is "My Title (5 s)".
> regression=# select repeat('xyzzy',12) \watch :delay
>

David J.

Re: [HACKERS] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Monday, March 21, 2016, Robert Haas  wrote:

> On Mon, Mar 21, 2016 at 2:09 PM, David G. Johnston
> > wrote:
> > On Monday, March 21, 2016, Tom Lane >
> wrote:
> >> "David G. Johnston" > writes:
> >> > I'd rather not omit sleep but removing "Watch every" is fine
> (preferred
> >> > actually), so:
> >> > Title Is Here Mon Mar 21 15:05:06 2016 (5s)
> >>
> >> Meh ... seems a bit awkward to me.  Couldn't you include " (5s)" in the
> >> title, if you want that info?  If it's variable, you could still
> >> accommodate that:
> >
> > Actually, only if it's a variable that you setup and repeat and you
> show.  A
> > bit cumbersome and mixes the parts that are title and those that are
> present
> > only because you are watching.
>
> Ah, come on.  This doesn't really seem like an issue we should spend
> more time quibbling about.  I think Tom's version is fine.
>
>
Tom doesn't care enough to veto and you don't really care...

I'll admit it's awkward because it's abbreviated but if someone enters
\watch 5 and then sees (5s) in the title I think they can put two and two
together.

If the watched query takes a long to run, or there is a disruption, knowing
when the last one ran and how often it is supposed to run is useful info to
have at ones fingertips.  I have done this myself occasionally so I'm not
speaking from theory.  But I won't complain if its removed.

David J.

Re: [HACKERS] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Monday, March 21, 2016, Tom Lane  wrote:

> "David G. Johnston" > writes:
> > I'll admit it's awkward because it's abbreviated but if someone enters
> > \watch 5 and then sees (5s) in the title I think they can put two and two
> > together.
>
> Where I find this to be awkward is that the format is randomly different
> between the user-title and no-user-title cases.
>
> What about just discarding the old format entirely, and printing one of
> these two things:
>
> Timestamp (every Ns)
>
> User Given Title  Timestamp (every Ns)
>
>
This works for me.

David J.

Re: [HACKERS] Request - repeat value of \pset title during \watch interations

[David G. Johnston]

On Monday, March 21, 2016, Tom Lane  wrote:

> "David G. Johnston" > writes:
> > On Monday, March 21, 2016, Tom Lane >
> wrote:
> >> What about just discarding the old format entirely, and printing one of
> >> these two things:
> >>
> >> Timestamp (every Ns)
> >>
> >> User Given Title  Timestamp (every Ns)
>
> > This works for me.
>
> If I don't hear objections PDQ, I'm going to update the docs and commit
> it like that.
>
>
Saw it go in.  Thank You.

David J.

Re: [HACKERS] problem with precendence order in JSONB merge operator

[David G. Johnston]

On Tue, Mar 22, 2016 at 1:52 PM, Peter Krauss  wrote:

> Seems that parser not using precedence ideal order, and that casting
> obligation losts performance.
>
> The first problem is self-evident in this example:
>
> SELECT '{"x":1}'::jsonb || (('{"A":{"y":2}}'::jsonb)->'A')
>   -- it is ok, expected result with (x,y)
> SELECT '{"x":1}'::jsonb || '{"A":{"y":2}}'::jsonb)->'A'
>   -- non-expected result (y).
>
> Higher precedence  most
> be for -> operator, that is like an object-oriented *path* operator,
> always higher than algebric ones.
>
​There is presently no formal concept of "path operator" in PostgreSQL.
 "->" is a user-defined operator, as is "||"​ and thus have equal
precedence and left associativity.

http://www.postgresql.org/docs/current/static/sql-syntax-lexical.html

Regardless, "||" is not an "algebric" [sic] operator...I'm curious what
source you are using to back your claim of operator precedence between
different so-called "operator types".

Its highly undesirable to make changes to operator precedence.

Operators are simply symbols to the parser - there is no context involved
that would allow making their precedence dynamic.  So all PostgreSQL sees
is "||", not a "JSONB merge operator".

Other problem is using this operation as SQL function
>
>   CREATE FUNCTION term_lib.junpack(jsonb,text) RETURNS JSONB AS $f$
> SELECT ($1-$2)::JSONB || ($1->>$2)::JSONB;
>   $f$ LANGUAGE SQL IMMUTABLE;
>
> without casting produce error. Perhaps will be "more friendly" without
> cast obligation,
>
> and it is a performance problem, the abusive use of castings losts
> performance.
>
I cannot make this work...

version
PostgreSQL 9.5.1 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
4.8.2-19ubuntu1) 4.8.2, 64-bit

SELECT ('{"a":1,"b":2}'::jsonb - 'b'::text)::jsonb ||
('{"a":1,"b":2}'::jsonb #> 'b'::text)::jsonb

> ​SQL Error: ERROR: invalid concatenation of jsonb objects
​
This seems like user error but without a self-contained test case
exercising the query (the use of a function in this context should be
immaterial) I'm finding it hard to explain why.  My simple case returns a
non-object with rightly cannot be appended to an object.

In isolatoin you can avoid casting the RHS of the || operator by using the
"#>(jsonb,text[])" operator

SELECT pg_typeof('{"a":1,"b":{"c":2}}'::jsonb #> array['b']::text[]) --jsonb

JSON, IME, still needs some fleshing out.  Efficient usage might require
additional features but for now one needs to get very familiar with all the
various operator variants that allow the user to choose whether to return
json or text and to pick the correct one for their needs.

​David J.
​

Re: [HACKERS] problem with precendence order in JSONB merge operator

[David G. Johnston]

Please don't top-post.

On Tuesday, March 22, 2016, Peter Krauss  wrote:

> Subjective notes to contextualize (try to explain on bad-English) my
> "precedence order" and JSONB visions:
>
> JSON datatype is perfect as workaround, and for many simple and less
> exigent applications.
> JSONB is the  "first class" datatype for user community, we expected years
> (!) for it ... Need some "first class" and friendly behaviour.
>
> In this context JSONB is not "any other" datatype, it is the bridge
> between relational data and flexible data...
> It is the Holy Grail and the Rosetta Stone :-)
>
> I think JSONB operators need some more attention, in semantic and
> usability contexts.   If you want to add  some friendliness and
> orthogonality in JSONB operators, will be natural to see -> operator as a
> kind of object-oriented *path* operator...
> By other hand, of course, you can do the simplest to implement JSONB...
> But you do a lot
>  (!), it
> was not easy to arrive here, and need only a little bit more to  reach
> perfection ;-)
>
>
You are welcome to supply a patch for this particular "little bit" - but I
suspect you will find it quite disruptive to backward compatibility and
general system internals if you attempt to do so.  But maybe not.

Any change you make in this area will effect every usage of that operator
whether part of core or end-user defined.  We have baggage that limits
our ability to be perfect.

So while I'll agree with your premise I don't see (really, don't care to
look for) a way to make your desire a reality.  But you and smarter people
than I are welcome to dive into the code and see if you can come up with
something that works.

David J.

Re: [HACKERS] PostgreSQL 9.6 behavior change with set returning (funct).*

[David G. Johnston]

On Wednesday, March 23, 2016, Tom Lane  wrote:

> "Regina Obe" > writes:
> > In the past couple of weeks our PostGIS tests against PostgreSQL 9.6 dev
> > started failing.  I traced the issue down to a behavior change in 9.6
> when
> > dealing with output of set returning functions when used with (func).*
> > syntax.
>
> > CREATE OR REPLACE FUNCTION dumpset(param_num integer, param_text text)
> > RETURNS TABLE(id integer, junk1 text, junk2 text)
> > ...
> > -- Get 16 rows in 9.6, Get 8 rows in 9.5
> > SELECT (dumpset(f.test, 'hello world' || f.test)).*
> > FROM generate_series(1,4) As f(test)
> > ORDER BY junk2;
>
> I think this is a side-effect of 9118d03a8 ("When appropriate, postpone
> SELECT output expressions till after ORDER BY").  Previously, although you
> got N evaluations of the SRF which is pretty horrid, they were all in the
> same targetlist and hence ran in sync and produced only the expected
> number of rows (thanks to the otherwise-indefensible ExecTargetList
> behavior by which multiple SRF tlist expressions produce a number of rows
> equal to the least common multiple of their periods, not the product).
> That commit causes the evaluation of dumpset(...).junk1 to be postponed
> till after the Sort step, but the evaluation of dumpset(...).junk2
> necessarily can't be.  So now you get dumpset(...).junk2 inflating
> the original rowcount 2X, and then dumpset(...).junk1 inflating it
> another 2X after the Sort.
>
> We could remain bug-compatible with the old behavior by adding some
> restriction to keep all the tlist SRFs getting evaluated at the same
> plan step, at least to the extent that we can.  I think you could get
> similar strange behaviors in prior releases if you used GROUP BY or
> another syntax that might result in early evaluation of the sort column,
> and we're not going to be able to fix that.  But we could prevent this
> particular optimization from introducing new strangeness.
>
> But I'm not really sure that we should.  The way that you *should*
> write this query is
>
> SELECT ds.*
> FROM generate_series(1,4) AS f(test),
>  dumpset(f.test, 'hello world' || f.test) AS ds
> ORDER BY junk2;
>
> which is both SQL-standard semantics and much more efficient than
> SRF-in-tlist.  We've more or less deprecated SRF-in-tlist since we
> introduced LATERAL in 9.3.  How much are we willing to do to stay
> bug-compatible with old behaviors here?
>
>
My gut reaction is that this is an unnecessary regression for the sake of a
performance optimization that is likely drowned out in the usage presented
anyway.

The pivot for me would be how hard would it be to maintain the old behavior
in this "more or less deprecated" scenario.  I have no way to judge that.

David J.

Re: [HACKERS] PostgreSQL 9.6 behavior change with set returning (funct).*

[David G. Johnston]

On Wed, Mar 23, 2016 at 2:11 PM, Tom Lane  wrote:

>
> ​​
> In the meantime I suppose there's a case to be made for preserving
> bug compatibility as much as possible.
>
> So anyway the question is whether to commit the attached or not.


​+1 for commit - I'll trust Tom on the quality of the patch :)

David J.

Re: [HACKERS] Alter or rename enum value

[David G. Johnston]

On Friday, March 25, 2016, Andrew Dunstan  wrote:

>
> On 03/25/2016 04:13 AM, Matthias Kurz wrote:
>
>>
>> Hopefully at the commitfest at least the transaction limitation
>> will/could be tackled - that would help us a lot already.
>>
>>
> I don't believe anyone knows how to do that safely. Enums pose special
> problems here exactly because unlike all other types the set of valid
> values is mutable.
>

Yeah, I'm not sure there is much blue sky here as long as the definition of
an enum is considered system data.  It probably needs to be altered so that
a user can create a table of class enum with a known layout that PostgreSQL
can rely upon to perform optimizations and provide useful behaviors - at
least internally.  The most visible behavior being displaying the label
while ordering using its position.

The system, seeing a data type of that class, would have an implicit
reference between columns of that type and the source table.
You have to use normal cascade update/delete/do-nothing while performing
DML on the source table.

In some ways it would be a specialized composite type, and we could
leverage that to you all the syntax available for those - but without
having a different function for each differently named enum classed table
since they all would share a common structure, differing only in name.  But
the tables would be in user space and not a preordained relation in
pg_catalog.  Maybe require they all inherit from some template but empty
table...

David J.

Re: [HACKERS] Draft release notes for next week's releases

[David G. Johnston]

On Sun, Mar 27, 2016 at 8:43 PM, Peter Geoghegan  wrote:

> On Sat, Mar 26, 2016 at 4:34 PM, Tom Lane  wrote:
> > Probably the most discussion-worthy item is whether we can say
> > anything more about the strxfrm mess.  Should we make a wiki
> > page about that and have the release note item link to it?
>
> I think that there is an argument against doing so, which is that
> right now, all we have to offer on that are weasel words. However, I'm
> still in favor of a Wiki page, because I would not be at all surprised
> if our understanding of this problem evolved, and we were able to
> offer better answers in several weeks. Realistically, it will probably
> take at least that long before affected users even start to think
> about this.
>

​One question to debate is whether placing a list of "known" (collated from
the program runs lots of people performed) would do more harm than good.
Personally I'd rather see a list of known failures and evaluate my
situation objectively (i.e., large index but no reported problem on my
combination of locale and platform).  I understand that a lack of evidence
is not proof that I am unaffected at this stage in the game.  Having
something I can execute on my server to try and verify behavior -
irrespective of the correctness of the indexes themselves - would be
welcomed.

David J.

Re: [HACKERS] Nested funtion

[David G. Johnston]

On Sun, Mar 27, 2016 at 9:14 PM, Sridhar N Bamandlapally <
sridhar@gmail.com> wrote:

> Hi
>
> Is there any way to create nested function?
>
> oracle to postgres migration required super function variable reference
> into nested function without nested function parameter
>
> Oracle sample:
> ---
> create or replace function f1(n number) return number
> is
> vs number:=1;
> function nf1(m number) return number is
> begin
> return vs + m + n;
> end;
> begin
> return nf1(2);
> end;
> /
>
> run:
> 
> SQL> select f1(9) from dual;
>
>  F1(9)
> --
> 12
>
>
PostgreSQL's ​pl/pgsql langauge doesn't grok closures.  You are welcome to
write "f1" in a language that does.  Perl and Python are built-in and many
others are available.  I assume at least one of them can do this.

​David J.
​
​

Re: [HACKERS] SET syntax in INSERT

[David G. Johnston]

On Thu, Jan 14, 2016 at 1:07 PM, Tom Lane  wrote:

> Vitaly Burovoy  writes:
> > On 1/14/16, Tom Lane  wrote:
> >> It's more than syntactic sugar; you are going to have to invent
> semantics,
> >> as well, because it's less than clear what partial-field assignments
> >> should do.
> >>
> >> Assume a table with an int-array column, and consider
> >> INSERT INTO foo SET arraycol[2] = 7, arraycol[4] = 11;
>
> > Right part is a column name, not an expression. Isn't it?


> > You can't now do something like
> > INSERT INTO foo (arraycol[2], arraycol[4]) VALUES(7, 11);
>
> Hm ... actually, you might want to try that before opining
>

​So what's the problem, then?  It seems like a decision has already been
made.

David J.
​

Re: [HACKERS] SET syntax in INSERT

[David G. Johnston]

On Thu, Jan 14, 2016 at 1:25 PM, Tom Lane  wrote:

> "David G. Johnston"  writes:
> > On Thu, Jan 14, 2016 at 1:07 PM, Tom Lane  wrote:
> >> Vitaly Burovoy  writes:
> >>> You can't now do something like
> >>> INSERT INTO foo (arraycol[2], arraycol[4]) VALUES(7, 11);
>
> >> Hm ... actually, you might want to try that before opining
>
> > So what's the problem, then?  It seems like a decision has already been
> > made.
>
> Yeah, but is it a decision that we might later find to be at odds
> with a future SQL standard?  The more places we embed that behavior,
> the more risk we take.
>

While I agree with the sentiment I'm not seeing the added risk introduced
as being a major blocker if the syntactic sugar is indeed popular and
otherwise desirable from a code maintenance standpoint.  If the standard
introduces a contradictory concept that we need to address we can do so.
As we've already defined this specific behavior any conflict will likely
result in the already defined behavior changing since having the same
overall concept implemented differently for "VALUES" compared to "SET"
would be undesirable​.  If we end up changing that whether we
"doubled-down" by implementing "SET" seems immaterial.

The question, then, is whether there is any behavior that needs to be
uniquely defined for SET that doesn't already come into play when using
VALUES or SELECT?  I cannot think of any but given the somewhat clandestine
nature of your first example maybe you know of others.

David J.


David J.

Re: [HACKERS] dealing with extension dependencies that aren't quite 'e'

[David G. Johnston]

On Fri, Jan 15, 2016 at 7:49 AM, Tom Lane  wrote:

> Abhijit Menon-Sen  writes:
> > I'm looking at an extension that creates some triggers (on user tables)
> > dynamically (i.e., not during CREATE EXTENSION, but afterwards). The
> > author has two problems with it:
>
>
How do these triggers come to be?


> > * «DROP EXTENSION ext» won't work without adding CASCADE, which is an
> >   (admittedly relatively minor) inconvenience to users.
>
> I am not sure why that's a bad thing.
>

​Agreed.  The triggers the extension creates are not part of the extension
itself and thus should not be removed even if the extension itself is
removed.​


> > * More importantly, pg_dump will dump all those trigger definitions,
> >   which is inappropriate in this case because the extension will try
> >   to create them.
>
> Or that.  Shouldn't pg_dump be expected to restore the same state
> that was there before?  IOW, what is the argument that this doesn't
> just represent poorly-thought-through extension behavior?
>

​Also agreed - pending an answer to my question.  Restoration involves
recreating the state of the database without "executing" things.  It is
assumed that those things not directly created as part of executing "CREATE
EXTENSION" are instead created by "executing" things located in the
extension (e.g., functions) and thus direct re-creation has to occur since
there is no mechanism to execute during restoration.

If there is some sort of catalog-driven user-space population going on the
selection criteria should omit from selection any objects already affected.

This is a bunch of hand-waving, though.  It would help to have a concrete
use-case to discuss explicitly rather than espouse theory.

I am not familiar enough with the dependency and extension internals to
comment on the merit of a new kind of dependency type behaving as
described.  It sounds like it would allow for a more accurate description
of the internal dependencies of the database - which is good absent any
kind of cost consideration.

David J.

Re: [HACKERS] Set search_path + server-prepared statements = cached plan must not change result type

[David G. Johnston]

On Monday, January 25, 2016, Vladimir Sitnikov 
wrote:

> I want to treat 'prepare' operation as an optimization step, so it is
> functionally equivalent to sending a query text.
>
> In other words, I would like backend to track search_path and other
> parameters if necessary transparently‎, creating (caching) different
> execution plans if different plans are required.
> ‎
> Does that make sense?‎
> ‎
>

Prepare creates a plan and a plan has a known output structure.  What you
want is an ability to give a name to a parsed but unplanned query.  This is
not something that prepare should do as it is not a natural extension of
its present responsibility.

Maybe call the new command "PARSE name AS query".

Subsequent prepare commands could refer to named parsed commands to
generate an execution plan in the current context.  If the current context
matches a previously existing plan the command would effectively become a
no-op.  Otherwise a new plan would be generated.  Or, more simply, using
execute and a named parsed query would implicitly perform prepare per the
description above.

I'm not sure how different this is from writing views...though it can be
used for stuff like updates and deletes as well.  You can, I think, already
get something similar by using set from current with a function...

David J.

Re: [HACKERS] Set search_path + server-prepared statements = cached plan must not change result type

[David G. Johnston]

On Thu, Jan 28, 2016 at 7:48 AM, Vladimir Sitnikov <
sitnikov.vladi...@gmail.com> wrote:

> Robert>Hmm, so in your example, you actually want replanning to be able to
> Robert>change the cached plan's result type?
>
> I want backend to cache _several_ plans behind a single "statement name".
> I want to treat "prepare...exec...deallocate" dance as an optimization
> step for a simple "exec...exec...exec" sequence.
> I do not want to care if "previously prepared query is still valid or
> not". For instance, I do not want to check if search_path is still the
> same.
>
> Current backend implementation does not report changes to
> "search_path", thus clients have no solid way to detect "search_path
> changes".
>
> David>Maybe call the new command "PARSE name AS query".
>
> From JDBC perspective, there is no need in "prepare vs parse" distinction:
> 1) Explicit "prepare...execute" are not used in typical application code
> 2) That means, in 99.9% cases, "prepare" would be used by the jdbc driver
> itself
> 3) Thus just a single "protocol command" is sufficient.
>
> What I am saying is there are lots of consumers that want to avoid
> parsing overhead: plpgsql, pgjdbc, pgjdbc-ng, postgresql-async,
> 8kdata/phoebe, etc, etc.
>
> All of them will have to deal with search_path vs prepare issue.
> If you suggest to deprecate "prepare" in favor of "parse", then all of
> the above clients would have to switch to that "parse".
> It does not look like a good solution, since lots of existing clients
> assume "prepare just works".
>
> If "prepare" command gets deprecated, why "parse" would be better?
> What would be the use of "prepare" if all the clients would have to
> use "parse" in order to be search_path-compatible?
>
>
​Further pondering on this topic reveals that I need a more solid
understanding of the underlying layers...I'm not really sure at this point
whether further redefining the behavior of PREPARE is as undesirable as it
first seemed to be.  It does impose some constraints and makes assumptions
in order to provides its capability and so instead of trying to add yet
more complexity to it in order to fulfill this different use case it can at
least be considered that a different module be provided as a solution.  I
guess if it got to the point where the new facility could supersede PREPARE
you would just modify PREPARE but if they end up performing two different
things then no deprecation would be involved.

David J.

Re: [HACKERS] "using previous checkpoint record at" maybe not the greatest idea?

[David G. Johnston]

On Mon, Feb 1, 2016 at 4:58 PM, Andres Freund  wrote:

> Hi,
>
> currently if, when not in standby mode, we can't read a checkpoint
> record, we automatically fall back to the previous checkpoint, and start
> replay from there.
>
> Doing so without user intervention doesn't actually seem like a good
> idea. While not super likely, it's entirely possible that doing so can
> wreck a cluster, that'd otherwise easily recoverable. Imagine e.g. a
> tablespace being dropped - going back to the previous checkpoint very
> well could lead to replay not finishing, as the directory to create
> files in doesn't even exist.


> As there's, afaics, really no "legitimate" reasons for needing to go
> back to the previous checkpoint I don't think we should do so in an
> automated fashion.
>
> All the cases where I could find logs containing "using previous
> checkpoint record at" were when something else had already gone pretty
> badly wrong. Now that obviously doesn't have a very large significance,
> because in the situations where it "just worked" are unlikely to be
> reported...
>
> Am I missing a reason for doing this by default?
>

​Learning by reading here...

http://www.postgresql.org/docs/current/static/wal-internals.html
"""
​After a checkpoint has been made and the log flushed, the checkpoint's
position is saved in the file pg_control. Therefore, at the start of
recovery, the server first reads pg_control and then the checkpoint record;
then it performs the REDO operation by scanning forward from the log
position indicated in the checkpoint record. Because the entire content of
data pages is saved in the log on the first page modification after a
checkpoint (assuming full_page_writes is not disabled), all pages changed
since the checkpoint will be restored to a consistent state.

To deal with the case where pg_control is corrupt, we should support the
possibility of scanning existing log segments in reverse order — newest to
oldest — in order to find the latest checkpoint. This has not been
implemented yet. pg_control is small enough (less than one disk page) that
it is not subject to partial-write problems, and as of this writing there
have been no reports of database failures due solely to the inability to
read pg_control itself. So while it is theoretically a weak spot,
pg_control does not seem to be a problem in practice.
​"""​

​The above comment appears out-of-date if this post describes what
presently happens.

Also, I was​ under the impression that tablespace commands resulted in
checkpoints so that the state of the file system could be presumed
current...

I don't know enough internals but its seems like we'd need to distinguish
between an interrupted checkpoint (pull the plug during checkpoint) and one
that supposedly completed without interruption but then was somehow
corrupted (solar flares).  The former seem legitimate for auto-skip while
the later do not.

David J.

Re: [HACKERS] "using previous checkpoint record at" maybe not the greatest idea?

[David G. Johnston]

On Mon, Feb 1, 2016 at 5:48 PM, Andres Freund  wrote:

> On 2016-02-01 17:29:39 -0700, David G. Johnston wrote:
> > ​Learning by reading here...
> >
> > http://www.postgresql.org/docs/current/static/wal-internals.html
> > """
> > ​After a checkpoint has been made and the log flushed, the checkpoint's
> > position is saved in the file pg_control. Therefore, at the start of
> > recovery, the server first reads pg_control and then the checkpoint
> record;
> > then it performs the REDO operation by scanning forward from the log
> > position indicated in the checkpoint record. Because the entire content
> of
> > data pages is saved in the log on the first page modification after a
> > checkpoint (assuming full_page_writes is not disabled), all pages changed
> > since the checkpoint will be restored to a consistent state.
>
> > ​The above comment appears out-of-date if this post describes what
> > presently happens.
>
> Where do you see a conflict with what I wrote about? We store both the
> last and the previous checkpoint's location in pg_control. Or are you
> talking about:
>

​Mainly the following...but the word I used was "out-of-date" and not
"conflict".​  The present state seems to do the above, and then some.


> > To deal with the case where pg_control is corrupt, we should support the
> > possibility of scanning existing log segments in reverse order — newest
> to
> > oldest — in order to find the latest checkpoint. This has not been
> > implemented yet. pg_control is small enough (less than one disk page)
> that
> > it is not subject to partial-write problems, and as of this writing there
> > have been no reports of database failures due solely to the inability to
> > read pg_control itself. So while it is theoretically a weak spot,
> > pg_control does not seem to be a problem in practice.
>
> if so, no, that's not a out-of-date, as we simply store two checkpoint
> locations:
>  $ pg_controldata /srv/dev/pgdev-dev/|grep 'checkpoint location'
> Latest checkpoint location:   B3/2A730028
> Prior checkpoint location:B3/2A72FFA0
>
>
​The quote implies that only a single checkpoint​ is noted and that no
"searching" is performed - whether by scanning or by being told the
position of a previous one so that it can jump there immediately without
scanning backwards.  It isn't strictly the fact that we do not "scan"
backwards but the implications that arise in making that statement.  Maybe
this is being picky but if you cannot trust the value of "Latest checkpoint
location" then pg_control is arguably corrupt.  Corruption is not strictly
limited to "unable to be read" but does include "contains invalid data".

> Also, I was​ under the impression that tablespace commands resulted in
> > checkpoints so that the state of the file system could be presumed
> > current...
>
> That actually doesn't really make it any better - it forces the *latest*
> checkpoint, but if we can't read that, we'll start with the previous
> one...


> > I don't know enough internals but its seems like we'd need to distinguish
> > between an interrupted checkpoint (pull the plug during checkpoint) and
> one
> > that supposedly completed without interruption but then was somehow
> > corrupted (solar flares).  The former seem legitimate for auto-skip while
> > the later do not.
>
> I don't think such a distinction is really possible (or necessary). If
> pg_control is corrupted we won't even start, and if WAL is corrupted
> that badly we won't finish replay...
>

My takeaway from the above is that we should only record what we think is a
usable/readable/valid checkpoint location to "Latest checkpoint location"​
​
​ (LCL) and if the system is not able to use that information to perform a
successful recovery it should be allowed to die without using the value in
"Previous checkpoint location" - which becomes effectively ignored during
master recovery.

​David J.
​

Re: [HACKERS] PostgreSQL Auditing

[David G. Johnston]

​So, Noah's excellent response has been ignored (from what my threaded
Gmail view tells me) at this point...​

On Tue, Feb 2, 2016 at 6:25 PM, Curtis Ruck <
curtis.ruck+pgsql.hack...@gmail.com> wrote:

> Robert,
>
> This isn't wrong.  I don't see anyone else trying to submit anything in
> reference to auditing.  Just because there have been multiple
> implementations in the past, based on commit histories, there have only
> been a few that tried getting into core or contrib (that i can find in
> mailing list history).
>

​And so if pgaudit is such an improvement then by this logic a poor
implementation would have been committed in the past just because someone
wrote something and proposed it for commit.​  Noah summarized his quite
weighty point-of-view on the outcome of the patch that has been put forth.
One dynamic of which is whether it would be easier - and overall more
productive - to make installing auditing as an extension easier compared to
getting it into core.


> I think if there was a valid commit path laid out for getting auditing
> into core, or into contrib at least, most users would probably find that
> sufficient.  But it seems that every time someone tries to incorporate
> auditing, there are countless and varied reasons to deny its inclusion.
>

​You pre-suppose that people are willing and able to spend their time
trying to predict the future when in reality they would rather be convinced
that what has been put in front of them is worth committing.  For what I
presume are myriad of both technical and political reasons the people who
have the final say have not been so convinced.


> It just seems after reading the mailing list history, that there is a lack
> of interest by people with commit authority, even though there is a decent
> interest in it from the community, and honestly, no one really likes
> auditing, but its one of those damned if you do (in performance) and damned
> if you don't (in legal) things.
>

​So either through persuasion or compensation you need to increase
interest...​



> Additionally Robert, given your professional status, you are by no means
> an unbiased contributor in this discussion.  Your stance on this matter
> shows that you don't necessarily want the open source solution to succeed
> in the commercial/compliance required space.  Instead of arguing blankly
> against inclusion can you at least provide actionable based feedback that
> if met would allow patches of this magnitude in?
>

​Even if that were to be true that would not prevent others involved from
moving things forward - an objection like "I don't want this in -core
because it will eat into my employer's profits" won't persuade many
people.  Nit-picking patches might get a bit further but if there is
sufficient buy-in from the community as a whole it won't last long.​  So
maybe Robert doesn't actively help as much as he could but a lack of
helping is not the same as hindering.


> I'm personally fine with fiscally rewarding organizations that assist my
> customer in succeeding, but its hard to convince my customer to fund open
> source, even though they wouldn't be able to do 75% of what they do without
> it.  Based on past experience this is the same most open source
> organizations face, especially when they don't have the marketing engine
> that the large commercial players have.
>

​Which is why the model seems to be for the service provides who leverage
PostgreSQL to charge their customers enough so that part of the profit can
flow back into the community that they have based their livelihood upon.
Otherwise, maybe features such as this should end up in commercial
offerings and those customers who won't charitably contribute will instead
have an easier time seeing that at least they are spending less for
PostgreSQL licensing than they would for similar features in competitors
offerings.


​In my estimation this ​whole thread started poorly and thus likely will
not garner much if any forward progress on the issue.  Noah's response was
spot-on despite that fact and a proper response to it would likely move
things forward in a more positive manner.  A separate thread could be
started to discuss what can be done to improve the scenario where excellent
patches and/or extensions are available but are not in core.  These two
dynamics are separate and trying to hit both in one thread is just going to
dilute things so that likely neither topic gets progressed.

David J.

Re: [HACKERS] PostgreSQL Audit Extension

[David G. Johnston]

On Wed, Feb 3, 2016 at 9:36 AM, Robert Haas  wrote:

> On Wed, Feb 3, 2016 at 10:37 AM, David Steele  wrote:
> > On 2/1/16 11:23 PM, Robert Haas wrote:
>
> >> In
> >> saying that it's arbitrary, I'm not saying it isn't *useful*.  I'm
> >> saying there could be five extensions like this that make equally
> >> arbitrary decisions about what to do and how to do it, and they could
> >> all be useful to different people.
> >
> > There *could* be five extensions but there are not.  To my knowledge
> > there are two and one is just a more evolved version of the other.
>
> Right now that may be true, although it wouldn't surprise me very much
> to find out that other people have written such extensions and they
> just didn't get as much press.  Also, consider the future.  It is
> *possible* that your version of pgaudit will turn out to be the be-all
> and the end-all, but it's equally possible that somebody will fork
> your version in turn and evolve it some more.  I don't see how you can
> look at the pgaudit facilities you've got here and say that this is
> the last word on auditing and all PostgreSQL users should be content
> with exactly that facility.  I find that ridiculous.  Look me in the
> eye and tell me that nobody's going to fork your version and evolve it
> a bunch more.
>

​This rings hollow to me.  JSON got included with an admittedly weak
feature set and then got significantly improved upon in subsequent
releases.  Those who would be incline to fork pgaudit, seeing it already
being in core, would more likely and to the benefit of the community put
that work into improving the existing work.​  My off-the-cuff understanding
is that some current big features (namely the parallel-related stuff) are
also taking this "lets commit smaller but still useful pieces" into core to
build up to this super-feature we want working two years from now.  I don't
see any fundamental reason auditing couldn't be given the same opportunity
to improve inside core.

The other major downside of having it in core is that now the feature
release cycle is tied to core.  Telling PostGIS they can only release new
features when new versions of PostgreSQL come out would be an unacceptable
situation.

The best of both worlds would be for core to have its own implementation
written as an extension and to readily allow for other implementations to
be plugged in as well.  As your alluded to above there are likely a number
of things core really needs to enable such functionality without providing
the entire UI - leaving that for extensions.


> > People who are interested in audit are also understandably leery of
> > downloading code from an untrusted source.  Both PGXN and GitHub are The
> > Wild West as far as conservative auditors are concerned.
>
> I hate to be rude here, but that's not my problem.  You can put it on
> your corporate web site and let people download it from there.  I'm
> sure that auditors are familiar with the idea of downloading software
> from for-profit companies.  Do they really not use any software from
> Microsoft or Apple, for example?  If the problem is that they will
> trust the PostgreSQL open source project but not YOUR company, then I
> respectfully suggest that you need to establish the necessary
> credibility, not try to piggyback on someone else's.
>

​A bit short-sighted maybe.  Endorsing and including such a feature could
open PostgreSQL up to a​ new market being supported by people who right now
are not readily able to join the PostgreSQL community because they cannot
invest the necessary resources to get the horses put before the cart.
Those people, if they could get their clients to more easily use
PostgreSQL, may just find it worth their while to then contribute back to
this new frontier that has been opened up to them.  This would ideally
increase the number of contributors and reviewers within the community
which is the main thing that is presently needed.


> > I'll be the first to admit that the design is not the prettiest.  Trying
> > to figure out what Postgres is doing internally through a couple of
> > hooks is like trying to replicate the script of a play when all you have
> > is the program.  However, so far it has been performed well and been
> > reliable in field tests.
>
> That's good to hear, but again, it's not enough for a core submission.
> Code that goes into our main git repository needs to be "the
> prettiest".  I mean it's not all perfect of course, but it should be
> pretty darn good.
>
> Also, understand this: when you get a core submission accepted, the
> core project is then responsible for maintaining that code even if you
> disappear.  It's entirely reasonable for the project to demand that
> this isn't going to be too much work.  It's entirely reasonable for
> the community to want the design to be very good and the code quality
> to be high.


​So far so good...​

It's entirely reasonable for the community NOT to want to
> privilege one implementation over another.

Re: [HACKERS] "using previous checkpoint record at" maybe not the greatest idea?

[David G. Johnston]

On Thu, Feb 4, 2016 at 3:57 PM, Alvaro Herrera 
wrote:

> David G. Johnston wrote:
>
> > ​Learning by reading here...
> >
> > http://www.postgresql.org/docs/current/static/wal-internals.html
> > """
> > ​After a checkpoint has been made and the log flushed, the checkpoint's
> > position is saved in the file pg_control. Therefore, at the start of
> > recovery, the server first reads pg_control and then the checkpoint
> record;
> > then it performs the REDO operation by scanning forward from the log
> > position indicated in the checkpoint record. Because the entire content
> of
> > data pages is saved in the log on the first page modification after a
> > checkpoint (assuming full_page_writes is not disabled), all pages changed
> > since the checkpoint will be restored to a consistent state.
> >
> > To deal with the case where pg_control is corrupt, we should support the
> > possibility of scanning existing log segments in reverse order — newest
> to
> > oldest — in order to find the latest checkpoint. This has not been
> > implemented yet. pg_control is small enough (less than one disk page)
> that
> > it is not subject to partial-write problems, and as of this writing there
> > have been no reports of database failures due solely to the inability to
> > read pg_control itself. So while it is theoretically a weak spot,
> > pg_control does not seem to be a problem in practice.
> > ​"""​
> >
> > ​The above comment appears out-of-date if this post describes what
> > presently happens.
>
> I think you're misinterpreting Andres, or the docs, or both.
>
> What Andres says is that the control file (pg_control) stores two
> checkpoint locations: the latest one, and the one before that.  When
> recovery occurs, it starts by looking up the latest checkpoint record;
> if it cannot find that for whatever reason, it falls back to reading the
> previous one.  (He further claims that falling back to the previous one
> is a bad idea.)
>
> What the 2nd para in the documentation is saying is something different:
> it is talking about reading all the pg_xlog files (in reverse order),
> which is not pg_control, and see what checkpoint records are there, then
> figure out which one to use.
>

Yes, I inferred something that obviously isn't true - that the system
doesn't go hunting for a valid checkpoint to begin recovery from.  While it
does not do so in the case of a corrupted pg_control file I further assumed
it never did.  That would be because the documentation doesn't make the
point of stating that two checkpoint positions exist and that PostgreSQL
will try the second one if the first one proves unusable.  Given the topic
of this thread that omission makes the documentation out-of-date.  Maybe
its covered elsewhere but since this section addresses locating a starting
point I would expect any such description ​to be here as well.

David J.

Re: [HACKERS] enable parallel query by default?

[David G. Johnston]

On Monday, February 8, 2016, Andres Freund  wrote:

> Hi,
>
> On 2016-02-08 16:07:05 -0500, Robert Haas wrote:
> > One of the questions I have about parallel query is whether it should
> > be enabled by default.  That is, should we make the default value of
> > max_parallel_degree to a value higher than 0?  Perhaps 1, say?
> >
> > There are some good reasons why this might be a bad idea, such as:
> >
> > - As discussed on a nearby thread, there is every possibility of nasty
> > bugs.
>
> I think that's an argument to enable it till at least beta1. Let's
> change the default, and add an item to the open items list to reconsider
> then.
>
>
I'd rather phrase that as: I cannot think of any reason to not make it on
by default so let's do so until experience tells us differently.  If
experience says it is too buggy then I'd be concerned about allowing it be
to enabled at all let alone by default.  If there are usage concerns then
ideally the postmaster could detect them and configure itself rather than
provide yet another knob for people to research.  I don't know enough about
the specifics on that end myself.  IOW I could not convince myself that the
end of beta was somehow important to this decision - but maybe the hash
join bug has me soured...

As a user I'd like it to just work within the confines of what system
resources I've told PostgreSQL as a whole it can use and that it detects it
has available to it by asking the O/S.

So, for me the quality of the feature is a on/off decision and not a
"defaults" one.  the argument that our default configuration is geared
toward low resource setups and since this is resource intensive it should
be disabled resonates though I'd rather it work reasonably (of self toggle
off) in both low and large resource environments.  Is that possible?

David J.

Re: [HACKERS] proposal: schema PL session variables

[David G. Johnston]

On Tue, Feb 9, 2016 at 11:32 AM, Corey Huinker 
wrote:

>
> Oh, and I suggest we call them SESSION variables rather than SCHEMA
> variables, to reinforce the idea of how long the values in the variables
> live. A session variable is in a sense a 1x1 temp table, whose definition
> persists across sessions but whose value does not.
>
> Of course, if they do persist across sessions, then yeah, SCHEMA makes
> more sense. But every package variable in Oracle PL/SQL was initialized
> when the package was first loaded into the session.
>
>
​The key distinction for SCHEMA was that all functions in the schema would
be able to see them (and only those in the schema).

I am a bit partial, with little deep thought, to the IMPORT mechanic.
Changing them to actual session variables would be doable and you could
allow for the IMPORT specification to use search_path or explicit means to
locate said variables regardless of which schema​

​they exist in.

However, part of the goal is to blend into the broader database community
and thus increase porting capabilities.  I'm not sure how well this would
help fulfill that goal.

David J.
​

Re: [HACKERS] Add schema-qualified relnames in constraint error messages.

[David G. Johnston]

On Thu, Feb 11, 2016 at 10:53 AM, Pavel Stehule 
wrote:

> > most recent error in verbose mode, without making a permanent session
>
>> > state change.  Something like
>> >
>> > regression=# insert into bar values(1);
>> > ERROR:  insert or update on table "bar" violates foreign key constraint
>> "bar_f1_fkey"
>> > DETAIL:  Key (f1)=(1) is not present in table "foo".
>>
>

> > regression=# \saywhat
>
>
​Maybe its too cute but I'll give a +1 for this alone.

David J.

Re: [HACKERS] planstats.sgml

[David G. Johnston]

On Mon, Feb 15, 2016 at 4:23 PM, Tatsuo Ishii  wrote:

> While reading planstats.sgml, I encounted a sentence which I don't
> understand.
>
> These numbers are current as of the last VACUUM or
> ANALYZE on the table.  The planner then fetches the
> actual current number of pages in the table (this is a cheap operation,
> not requiring a table scan).  If that is different from
> relpages then
> reltuples is scaled accordingly to
> arrive at a current number-of-rows estimate.  In this case the value of
> relpages is up-to-date so the rows estimate
> is
> the same as reltuples.
>
> I don't understand the last sentence (In this case...). For me it
> seems it is talking about the case when replages is not different from
> what the planner fetches from the table. If so, why "In this case"?
> Isn't "In this case" referrers to the previous sentence (If that is
> different from...)? Maybe "In this case" should be "Otherwise" or some
> such?
>
>
​The whole sentence is awkward but you are correct in your reading - and
"otherwise" would be a solid choice.

Though iIt seems the whole thing could be simplified to:

These numbers are current as of the last VACUUM or ANALYZE on the table.
To account for subsequent changes the planner obtains the actual page count
for the table and scales pg_class.reltuples by a factor of "actual page
count" over pg_class.relpages.

The "cheap operation" comment seems gratuitous though could still be
injected if desired.

David J.

Re: [HACKERS] The plan for FDW-based sharding

[David G. Johnston]

On Tue, Feb 23, 2016 at 9:43 AM, Bruce Momjian  wrote:

> 4. Cross-node read-write queries:
>
> This will require a global snapshot manager and global snapshot manager.
>

Probably meant "global transaction manager"

​David J.​

Re: [HACKERS] temporary table vs array performance

[David G. Johnston]

Its considered bad form to post to multiple lists.  Please pick the most
relevant one - in this case I'd suggest -general.

On Mon, Sep 26, 2016 at 8:39 AM, dby...@163.com  wrote:

>
> Array is not convenient to use in function, whether
> there are other methods can be replaced temp table in function
>
>
​I have no difficulty using arrays in functions.

As for "other methods" - you can use CTE (WITH) to create a truly local
table - updating the catalogs by using a temp table is indeed quite
expensive.

WITH vals AS  ( VALUES (1, 'lw'), (2, 'lw2') )
SELECT * FROM vals;

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

[David G. Johnston]

On Thu, Oct 5, 2017 at 3:05 PM, Joshua D. Drake 
wrote:

> On 10/05/2017 02:54 PM, David G. Johnston wrote:
>
>> On Thu, Oct 5, 2017 at 2:37 PM, Joshua D. Drake > <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.

[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] 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.

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 
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 2:30 PM, Nico Williams 
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.
​

[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] 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.

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] 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] [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] bit|varbit #, xor operator

[David G. Johnston]

On Mon, Oct 17, 2016 at 1:39 PM, Jim Nasby  wrote:

> On 10/17/16 11:29 AM, Tom Lane wrote:
>
>> Jim Nasby  writes:
>>
>>> On 10/16/16 3:13 PM, Tom Lane wrote:
>>>
 Related to this I'd also like to add a boolean XOR operator as that's a
> relatively common request/question.
>

>> We have boolean XOR; it's spelled "<>".

>>>
>> I always forget about that...
>>>
>>
>> Maybe it should be mentioned explicitly in the docs.
>>
>
> Hrm, went to go add it and it appears we don't have a section for boolean
> type operators. I guess I should add it to 9.1?
>
>
​There are no symbolic operators, just the standard SQL keywords: AND, OR,
NOT.

https://www.postgresql.org/docs/current/static/functions-logical.html

​Adding a note there pertaining to XOR should be sufficient.​

though, it doesn't work for boolean arrays.
>>>
>>
>> Define doesn't work?
>>
>
> I would expect array[true, false] XOR array[true, true] to return
> array[false, true], but <> just returns a single true (since the arrays are
> !=).
>
> Though, I guess what would make the most sense there is a map function
> that would apply an operator or function to every element of a set of
> arrays. But I don't see any way to do that without major changes to how
> anyarray works.


​Yeah, your expectations seem off here given that:

​SELECT array[true, false]::boolean[] AND array[true, true]::boolean[]

is invalid...

David J.

Re: [HACKERS] Multiple psql history files

[David G. Johnston]

On Tue, Oct 18, 2016 at 9:26 AM, Jonathan Jacobson  wrote:

> The .psql_history file is naturally used by different DB connections
> (distinguished by a different combination of host + port + database + user).
> At least in my multi-database working environment, this leads sometimes to
> frustration because there are commands that cannot or should not be used by
> different connections.
> To solve this, psql could keep a separate command history file for each
> connection.
> I will be happy to make this my first contribution to PostgreSQL's code.
> What do you say?
>

​I would say that users needing such fine grained control of the history
file should avail themselves of HISTFILE ​and other related psql variables
and/or the PSQL_HISTORY environment variable.

David J.

Re: [HACKERS] Multiple psql history files

[David G. Johnston]

On Tue, Oct 18, 2016 at 9:26 AM, Jonathan Jacobson  wrote:

> The .psql_history file is naturally used by different DB connections
> (distinguished by a different combination of host + port + database + user).
> At least in my multi-database working environment, this leads sometimes to
> frustration because there are commands that cannot or should not be used by
> different connections.
> To solve this, psql could keep a separate command history file for each
> connection.
> I will be happy to make this my first contribution to PostgreSQL's code.
> What do you say?
>
>
​Existing capabilities not withstanding the feature itself would probably
be considered but I'm doubtful it would replace the default behavior.

The user would, at minimum, have to designate​

​a directory into which the separate history files would be created -
polluting the home directory is unappealing.

If you want to propose the specifics of a feature that meets those two
criteria specific comments and opinions on include-ability can be made.

David J.
​

Re: [HACKERS] Multiple psql history files

[David G. Johnston]

On Tue, Oct 18, 2016 at 9:45 AM, Tom Lane  wrote:

> Jonathan Jacobson  writes:
> > The .psql_history file is naturally used by different DB connections
> > (distinguished by a different combination of host + port + database +
> user).
> > At least in my multi-database working environment, this leads sometimes
> to
> > frustration because there are commands that cannot or should not be used
> by
> > different connections.
> > To solve this, psql could keep a separate command history file for each
> > connection.
> > I will be happy to make this my first contribution to PostgreSQL's code.
> > What do you say?
>
> One interesting point, if you wish to consider history as being
> connection-specific, is what happens during a \c command.  Right
> now the answer is "nothing" but you might wish it were different.
>

​Just to clarify/confirm a point inferred from the docs...

If you place "\set HISTFILE​ ~/.psql_history- :DBNAME
"
​
​
​into your .psqlrc file then when you perform a "\c" the .psqlrc file is
re-read and the ​new value for DBNAME is used to generate a new history
file name.

​As Julien pointed out cross-thread this really does seem to be the best
place to implement such logic - though shell functions and aliases can be
used to some good effect as well.

David J.

Re: [HACKERS] Multiple psql history files

[David G. Johnston]

On Tue, Oct 18, 2016 at 10:21 AM, Tom Lane  wrote:

> "David G. Johnston"  writes:
> > On Tue, Oct 18, 2016 at 9:45 AM, Tom Lane  wrote:
> >> One interesting point, if you wish to consider history as being
> >> connection-specific, is what happens during a \c command.  Right
> >> now the answer is "nothing" but you might wish it were different.
>
> > ​Just to clarify/confirm a point inferred from the docs...
> > If you place "\set HISTFILE​ ~/.psql_history- :DBNAME"
> > ​into your .psqlrc file then when you perform a "\c" the .psqlrc file is
> > re-read and the ​new value for DBNAME is used to generate a new history
> > file name.
>
> Um, no, I see no indication of that in the code.  Where did you read that
> in the docs?
>
>
​
​
 DBNAME
The name of the database you are currently connected to. This is set every
time you connect to a database (including program start-up), but can be
unset.

​​HISTFILE
The file name that will be used to store the history list. The default
value is ~/.psql_history. For example, putting:
\set HISTFILE ~/.psql_history- :DBNAME
in ~/.psqlrc will cause psql to maintain a separate history for each
database.

​The "including program start-up" aspect to DBNAME means that it is changed
upon using "\c"

I inferred the part about .psqlrc being re-read and thus taking on the new
value of :DBNAME in the example.

psqlrc is later defined to be "the [system|user] startup file" so it was
wrong to conclude that it was re-read upon issuing "\c" - though
"connection startup" isn't a totally unreasonable interpretation of the
timing at which this file is read.  Not everyone is going to associate the
"rc" suffix with the file only being read during program startup.
​


> If we wanted the history file to change at \c, I think the best way would
> be to invent some escape-patterns that could be placed in the value of
> HISTFILE, say along the lines of "\set HISTFILE ~/.psql_history-%h-%p",
> and then specify that if the value contains any such patterns we'll dump
> and reload the history file when reconnecting.  But TBH I don't think
> it's worth the trouble.  I'd sure like to have seen multiple requests
> for such functionality before we go to the trouble.
>

A slightly less invasive approach would be to have a "connection startup
script" file..."psql-conn-rc"...that is re-read immediately after a
successful connection is made to a database.

David J.

Re: [HACKERS] Move pg_largeobject to a different tablespace *without* turning on system_table_mods.

[David G. Johnston]

On Wed, Oct 19, 2016 at 9:29 AM, Bruce Momjian  wrote:

> On Tue, Oct 18, 2016 at 04:51:54PM +0200, Andreas Joseph Krogh wrote:
> > > 2. Being able to move pg_largeobject to a different tablespace
> > >*without* turning on system_table_mods. This is important for
> > >people storing LOTS of large-objects on separate
> > >disks (non-SSD) and hence in a different tablespace.
>
> I think an open question is why you would not want to move the other
> system tables at the same time you move pg_largeobject.
>
>
​I think the theory of having all system tables except LO on SSD storage,
and having LO on a less performant device, makes sense.

David J.​

Re: [HACKERS] Renaming of pg_xlog and pg_clog

[David G. Johnston]

On Thu, Oct 20, 2016 at 6:03 PM, Robert Haas  wrote:

> On Thu, Oct 20, 2016 at 3:46 PM, Tom Lane  wrote:
> > I'm mostly with Stephen on this.  As the names stand, they encourage
> > people to go look at the documentation,
> > https://www.postgresql.org/docs/devel/static/storage-file-layout.html
> > which will provide more information than you'd ever get out of any
> > reasonable directory name.
>
> Well, we could change them all to pg_a, pg_b, pg_c, pg_d, ... which
> would encourage that even more strongly.  But I don't think that
> proposal can be taken seriously.  Giving things meaningful names is a
> good practice in almost every case.
>

Those don't have the virtue of being at least somewhat ​
m
nemonic
​ like pg_xact.

I'll toss my lot in with Steven's and Tom's on this.

​I have no problem continuing keeping with historical precedent ​and
allowing mnemonic abbreviations in our directory and file names at this
point.

David J.

​

Re: [HACKERS] Mention column name in error messages

[David G. Johnston]

On Fri, Nov 4, 2016 at 12:15 PM, Tom Lane  wrote:

> Michael Paquier  writes:
> > I am passing that down to a committer for review. The patch looks
> > large, but at 95% it involves diffs in the regression tests,
> > alternative outputs taking a large role in the bloat.
>
> This is kind of cute, but it doesn't seem to cover very much territory,
> because it only catches errors that are found in the parse stage.
> For instance, it fails to cover Franck's original example:
> ​[...]
>
>
> Maybe it'd be all right to commit this anyway, but I'm afraid the most
> common reaction would be "why's it give me this info some of the time,
> but not when I really need it?"  I'm inclined to think that an acceptable
> patch will need to provide context for the plan-time and run-time cases
> too, and I'm not very sure where would be a sane place to plug in for
> those cases.
>

​Agreed.
​

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] 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] 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] 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] 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] 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] 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 Sat, Feb 25, 2017 at 11:11 AM, Greg Stark  wrote:

> On 24 February 2017 at 14:57, David G. Johnston
>  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] 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] 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] Should we eliminate or reduce HUP from docs?

[David G. Johnston]

On Fri, Mar 10, 2017 at 3:28 PM, Alvaro Herrera 
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 3:51 PM, Alvaro Herrera 
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] \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] \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] 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] 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] /proc/self/oom_adj is deprecated in newer Linux kernels

[David G Johnston]

In short:

I can accept the idea that picking reasonable specific values is impossible
so let's just ensure that children are always killed before the parent
(basically the default behavior). If you then say that any system that is
capable of implementing that rule should have it set then leaving it
unexposed makes sense.  That line of thought does not require the abstract
cost of a GUC to factor into the equation.

However, in considering system configuration and concurrently running
processes


Tom Lane-2 wrote
> Extra GUCs do not have zero cost, especially not ones that are as
> complicated-to-explain as this would be.

The explain factor does little for me since if given a reasonable default
the GUC can be ignored until a problem manifests.  At that point not having
a GUC makes resolving the problem require someone to stop using packaging
and instead compile their own source.  This results in a poor user
experience.

So, if someone where to provide a complete patch that introduced a GUC to
enable/disable as well as set priorities - to include documentation and
reasonable postgresql.conf defaults - what specific ongoing GUC costs would
prevent applying said patch?

You mention a security hazard but that is not a cost of GUCs generally but
this one specifically; and one that seems to have been deemed no riskier
than other DBA capabilities.

Help me understand the cost side of the equation since the benefit side
seems clear-cut enough to me.  The OOM problem is real and making PostgreSQL
fit within the overall memory management architecture of a given server is
something that is the responsibility of the system admin in conjunction with
the DBA - not us or the packager.  I'll buy that because this is a linux
issue that the implementation could be packager selectable but given the
dynamics of Linux centralizing a reasonable default implementation into core
makes sense.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/proc-self-oom-adj-is-deprecated-in-newer-Linux-kernels-tp4810971p5806697.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] /proc/self/oom_adj is deprecated in newer Linux kernels

[David G Johnston]

Gurjeet Singh-4 wrote
> Even if the clueless DBA tries to set the oom_score_adj below that of
> Postmaster, Linux kernel prevents that from being done. I demonstrate
> that in the below screen share. I used GUC as well as plain command
> line to try and set a child's badness (oom_score_adj) to be lower than
> that of Postmaster's, and Linux disallows doing that, unless I use
> root's powers.
> 
> So the argument that this GUC is a security concern, can be ignored.
> Root user (one with control of start script) still controls the lowest
> badness setting of all Postgres processes. If done at fork_process
> time, the child process simply inherits parent's badness setting.

The counter point here is that the postmaster can be set to "no kill" and
the >= condition allows for children to achieve the same while it is our
explicit intent that the children be strictly > parent.

To that end, should the adjustment value be provided as an offset to the
postmasters instead of an absolute value - and disallow <= zero offset
values in the process?

I get and generally agree with the environment variable proposal and it's
stated goal to restrict whom can makes changes. But how much less cost does
an environment variable have than a GUC if one GUC argument is still its
maintenance overhead?

David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/proc-self-oom-adj-is-deprecated-in-newer-Linux-kernels-tp4810971p5806700.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] PL/pgSQL support to define multi variables once

[David G Johnston]

Tom Lane-2 wrote
> Andres Freund <

> andres@

> > writes:
>> On 2014-06-13 16:12:36 +0200, Pavel Stehule wrote:
>>> Quan' example is 100% valid in SQL/PSM and what I read about ADA then in
>>> ADA too.
> 
>> So what? plpgsql is neither language and this doesn't seem to be the way
>> to make them actually closer (which I doubt would be a good idea in the
>> first place).
> 
> What plpgsql actually tries to model is Oracle's PL/SQL, in which this
> syntax is specifically *not* allowed (at least according to the 2008-or-so
> manual I have handy).
> 
> At the very least I think we should stay away from this syntax until
> the SQL committee understand it better than they evidently do today.
> I don't want to implement it and then get caught by a future
> clarification that resolves the issue differently than we did.

Haven't read the patch so, conceptually...

Its not quite as unclear as you make it out to be:

local_a, local_b, local_c text := 'a1'; 

The "text" type declaration MUST apply to all three variables, so extending
that to include the default assignment would be the internally consistent
decision.

I'm not sure the following would be useful but:

var_1, var_2, var_3 integer := generate_series(1,3)

If the expression results in either a 3x1 or a 1x3 (in the three var
example) we could do an expansion.  If it results in a 1x1 that value would
be copied without re-executing the function.

Though I suppose someone might want to do the following:

random_1, random_2, random_3 float := random(1234);

The decision to copy, not re-execute, is safer to use as the behavior and
force explicitness in the re-execute situation.


Until then suggest that the friend do:

DECLARE 
local_a text := 'a1';
local_b text := local_a;
local_c text := local_a;
BEGIN 



--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/PL-pgSQL-support-to-define-multi-variables-once-tp5807168p5807215.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] Audit of logout

[David G Johnston]

Tom Lane-2 wrote
> Another answer is to make both variables PGC_SIGHUP, on the grounds
> that it doesn't make much sense for them not to be applied system-wide;
> except that I think there was some idea that logging might be enabled
> per-user or per-database using ALTER ROLE/DATABASE.

>From a trouble-shooting standpoint if I know that client software in
question is focused on particular users/databases being able to only enable
connection logging for those would make sense.  Whether that is a true
production concern is another matter but the possibility does exist.

I personally do not get how a logoff is a risk auditing event.  Logons and
actual queries I can understand.

For the same reason keeping them separate has merit since for imaginable
circumstances the logoffs are noise but the logons are important - and
forcing them to be on/off in tandem disallows the option to remove the noise
from the logs.

David J.






--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Audit-of-logout-tp5806985p5807224.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] comparison operators

[David G Johnston]

Andrew Dunstan wrote
> On 06/17/2014 07:25 PM, Andres Freund wrote:
>> On 2014-06-17 19:22:07 -0400, Tom Lane wrote:
>>> Andrew Dunstan <

> andrew@

> > writes:
 I went to have a look at documenting the jsonb comparison operators,
 and
 found that the docs on comparison operators contain this:
  Comparison operators are available for all relevant data types.
 They neglect to specify further, however. This doesn't seem very
 satisfactory. How is a user to know which are relevant? I know they are
 not available for xml and json, but are for jsonb. Just talking about
 "all relevant types" seems rather hand-wavy.
>>> Well, there are 38 default btree opclasses in the standard system ATM.
>>> Are we worried enough about this to list them all explicitly?  Given the
>>> lack of complaints to date, I'm not.
> 
> I think I'd rather just say "for many data types" or something along 
> those lines, rather than imply that there is some obvious rule that 
> users should be able to intuit.

Ideal world for me: we'd list the data types that do not provide comparison
operators (or not a full set) by default with links to the section in the
documentation where the reasoning for said omission is explained and/or
affirmed.

My other reaction is that referring to data types at all in this section is
unnecessary - other than maybe to state (which it does not currently) that
both sides of the comparison must be of the same (or binary equivalent, like
text/varchar) type or there must exist an implicit cast for one of the
operands.  Much of that knowledge is implied and well understood though, as
is the fact that operators are closely associated with data types.  IOW - I
would be fine with removing "Comparison operators are available for all
relevant data types" and not replacing it with anything.  Though "for many
data types" is my preferred equivalent phrase for the same reasons Andrew
noted.

David J.



--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/comparison-operators-tp5807654p5807757.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Wed, Jun 18, 2014 at 8:01 PM, Josh Berkus [via PostgreSQL] <
ml-node+s1045698n5807868...@n5.nabble.com> wrote:

> On 06/18/2014 04:54 PM, Marko Tiikkaja wrote:
> > On 2014-06-19 1:46 AM, Josh Berkus wrote:
> >> Robert's right, not killing the "BEGIN;" only transactions is liable to
> >> result in user confusion unless we label those sessions differently in
> >> pg_stat_activity.
> >
> > Wouldn't they be labeled differently already?  i.e. the last query would
> > be "BEGIN".  Unless your app tries to unsuccessfully use nested
> > transactions, you would know why it hasn't been killed.
>
> That's pretty darned obscure for a casual user.  *you* would know, and
> *I* would know, but 99.5% of our users would be very confused.
>
> Plus, if a session which has only issued a "BEGIN;" doesn't have a
> snapshot and isn't holding any locks, then I'd argue we shouldn't lable
> it IIT in the first place because it's not doing any of the bad stuff we
> want to resolve by killing IITs.  Effectively, it's just "idle".
>
>
​+1

Since the BEGIN is not meaningfully interpreted until the first subsequent
command (SET excepting apparently - are there others?) is issued no
transaction has begun at this point so "idle" and not "IIT" should be the
reported state on pg_stat_activity​.

Though I can understand some level of surprise if someone sees "idle" with
a "BEGIN" (or SET TIMEZONE) as the last executed command - so maybe "idle
before transaction" instead of "idle in transaction" - which hopefully will
not be assumed to be controlled by the "idle_in_transaction_timeout" GUC.
 I presume that we have some way to distinguish this particular case and
can report it accordingly.  Even if that state endures after a SET TIMEZONE
or similar session configuration command explaining that all those are
"pre-transaction" shouldn't be too hard - though as a third option "idle
initialized transaction" could be the state indicator.

Depending on how "idle in transaction (aborted)" gets resolved we could
also consider "idle in transaction (initializing)" - but since the former,
IMO, should be dropped (and thus matches the "in" specification of the GUC)
naming the later - which should not be dropped (I'll go with the stated
opinion here for now) - with "in" is not advisable.

The current behavior is transparent to the casual user so sticking with
"idle" does seem like the best choice to maintain the undocumented nature
of the behavior.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5807874.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Thu, Jun 19, 2014 at 12:40 PM, Abhijit Menon-Sen-2 [via PostgreSQL] <
ml-node+s1045698n5808016...@n5.nabble.com> wrote:

> At 2014-06-19 17:53:17 +0200, [hidden email]
>  wrote:
> >
> > I much prefer with "in" but it doesn't much matter.
>
> If you look at similar settings like statement_timeout, lock_timeout,
> etc., what comes before the _timeout is a concrete *thing* that can
> timeout, or that a timeout can be applied to (e.g. wal_receiver).
>
> "What's timing out?" "A statement."
>
> But in "idle_in_transaction_timeout", "idle_in_transaction" is not a
> thing. It's a description of the state of a thing (the thing being a
> session in the FATAL variant of your patch, or a transaction in the
> ERROR variant).
>

> "What's timing out?" "An idle in transaction." "Huh?"
>
> Strictly speaking, by this logic, the consistent name for the setting in
> the FATAL variant would be "idle_in_transaction_session_timeout",


​+1; I even suggested something similar (I omitted the "in") - though we
hadn't come to a firm conclusion on what behavior we were going to
implement at the time.  Adding session reasonably precludes us from easily
changing our mind later (or giving the user an option) but that doesn't
seem likely anyway.​

Advocating for the Devil (or a more robust, if probably complicated,
solution):
"idle_in_transaction_timeout=10s"
"idle_in_transaction_target=session|transaction"

Would be a valid pair since the first intentionally would not want to
specify a target - and thus does not fit within the pattern you defined.

"idle_transaction_timeout" is bad since idle is a valid state that is not
being affected by this patch; whereas pgbouncer indeed drops truly idle
connections.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808024.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Sunday, June 22, 2014, Kevin Grittner-5 [via PostgreSQL] <
ml-node+s1045698n580830...@n5.nabble.com> wrote:

> Andres Freund <[hidden email]
> > wrote:
>
> > I think we'll want a version of this that just fails the
> > transaction once we have the infrastructure. So we should choose
> > a name that allows for a complimentary GUC.
>
> If we stick with the rule that what is to the left of _timeout is
> what is being cancelled, the a GUC to cancel a transaction which
> remains idle for too long could be called idle_transaction_timeout.
>
> Do you disagree with the general idea of following that pattern?
>
>
If we ever do give the user an option the non-specific name with separate
type GUC could be used and this session specific variable deprecated.  And
disallow both to be active at the same time.  Or something else.  I agree
that idle_in_transaction_transaction would be proper but troublesome for
the alternative but crossing that bridge if we ever get there seems
reasonable in light of picking the best single name for this specific
feature.

Idle_transaction_timeout has already been discarded since truly idle
transactions are not being affected, only those that are in transaction.
 The first quote above is limited to that subset as well.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808311.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

>
>
> >>>
> >>> I think that'd be rather confusing. For one it'd need to be
> >>> idle_in_transaction_timeout
>
> Why?  We're cancelling an idle transaction, not an "idle in
> transaction", whatever that is.
>
>
​The confusion derives from the fact we are affecting a session whose state
is "idle in transaction", not one that is idle.  We are then, for this
discussion, choosing to either kill the entire session or just the
currently active transaction.  After "idle_in_transaction" there is an
unstated "session" being mentally placed by myself and probably others.
 Following that is then either "session" or "transaction" to denote what is
being affected should the timeout interval come to pass.

Discarding that, probably flawed, mental model makes
"idle_transaction_timeout" seem fine.
 "idle_in_transaction_session_timeout" would indeed be a natural complement
to this.​

I do not expect this concept, should it come to pass, to be that difficult
to document or for someone to learn.

Along with other I still see no reason to avoid "IIT_session_timeout" at
this point.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808471.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

>
> > A long idle in transaction state pretty much always indicates a
> > problematic interaction with postgres.
>
> True.  Which makes me wonder whether we shouldn't default this to
> something non-zero -- even if it is 5 or 10 days.


​I guess it depends on how parental we want to be.  But if we go that route
wouldn't a more harsh/in-your-face default make more sense?  Something in
Minutes, not Days​...say '5min'...

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808473.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Tue, Jun 24, 2014 at 9:20 AM, Vik Fearing [via PostgreSQL] <
ml-node+s1045698n5808882...@n5.nabble.com> wrote:

> On 06/22/2014 05:11 PM, Kevin Grittner wrote:
> > I found one substantive issue that had been missed in discussion,
> > though.  The patch modifies the postgres_fdw extension to make it
> > automatically exempt from an attempt to set a limit like this on
> > the server to which it connects.  I'm not sure that's a good idea.
> > Why should this type of connection be allowed to sit indefinitely
> > with an idle open transaction?  I'm inclined to omit this part of
> > the patch
>
> My reasoning for doing it the way I did is that if a transaction touches
> a foreign table and then goes bumbling along with other things the
> transaction is active but the connection to the remote server remains
> idle in transaction.  If it hits the timeout, when the local transaction
> goes to commit it errors out and you lose all your work.
>
> If the local transaction is actually idle in transaction and the local
> server doesn't have a timeout, we're no worse off than before this patch.
>

​Going off of this reading alone wouldn't we have to allow the client to
set the timeout on the fdw_server - to zero - to ensure reasonable
operation?  If the client has a process that requires ​10 minutes to
complete, and the foreign server has a default 5 minute timeout, if the
client does not disable the timeout on the server wouldn't the foreign
server always cause the process to abort?

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808883.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Tue, Jun 24, 2014 at 10:05 AM, Robert Haas [via PostgreSQL] <
ml-node+s1045698n580889...@n5.nabble.com> wrote:

> On Tue, Jun 24, 2014 at 9:18 AM, Vik Fearing <[hidden email]
> > wrote:
>
> > On 06/22/2014 05:11 PM, Kevin Grittner wrote:
> >> I found one substantive issue that had been missed in discussion,
> >> though.  The patch modifies the postgres_fdw extension to make it
> >> automatically exempt from an attempt to set a limit like this on
> >> the server to which it connects.  I'm not sure that's a good idea.
> >> Why should this type of connection be allowed to sit indefinitely
> >> with an idle open transaction?  I'm inclined to omit this part of
> >> the patch
> >
> > My reasoning for doing it the way I did is that if a transaction touches
> > a foreign table and then goes bumbling along with other things the
> > transaction is active but the connection to the remote server remains
> > idle in transaction.  If it hits the timeout, when the local transaction
> > goes to commit it errors out and you lose all your work.
> >
> > If the local transaction is actually idle in transaction and the local
> > server doesn't have a timeout, we're no worse off than before this
> patch.
>
> I think we are.  First, the correct timeout is a matter of
> remote-server-policy, not local-server-policy.  If the remote server
> wants to boot people with long-running idle transactions, it's
> entitled to do that, and postgres_fdw shouldn't assume that it's
> "special".  The local server policy may be different, and may not even
> have been configured by the same person.  Second, setting another GUC
> at every session start adds overhead for all users of postgres_fdw.
>
> Now, it might be that postgres_fdw should have a facility to allow
> arbitrary options to be set on the foreign side at each connection
> startup.  Then that could be used here if someone wants this behavior.
> But I don't think we should hard-code it, because it could also be NOT
> what someone wants.
>
>
The missing ability is that while the user only cares about the one logical
session we are dealing with two physical sessions in a parent-child
relationship where the child session state does not match that of its
parent.  For me, this whole line of thought is based upon the logical
"idle_in_transaction" - did the application really mean to leave this
hanging?

Say that 90% of the time disabling the timeout will be the correct course
of action; making the user do this explicitly does not seem reasonable.
 And if "doesn't matter" is the current state when the foreign server is
configured no setting will be passed.  Then if the remote server does
institute a timeout all the relevant configurations will need to be changed.

ISTM that the additional overhead in this case would be very small in
percentage terms; at least enough so that usability would be my default
choice.

I have no problem allowing for user-specified behavior but the default of
disabling the timeout seems reasonable.  I am doubting that actually
synchronizing the parent and child sessions, so that the child reports the
same status as the parent, is a valid option - though it would be the
"best" solution since the child would only report IIT if the parent was IIT.

For me, a meaningful default and usability are trumping the unknown
performance degradation.  I can go either way on allowing the local
definition to specify its own non-zero timeout but it probably isn't worth
the effort.  The foreign server administrator ultimately will have to be
aware of which users are connecting via FDW and address his "long-running
transaction" concerns in a more nuanced way than this parameter allows.  In
effect this becomes an 80% solution because it is not (all that) useful on
the remote end of a FDW connection; though at least the local end can make
proper use of it to protect both servers.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808905.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] idle_in_transaction_timeout

[David G Johnston]

On Tue, Jun 24, 2014 at 11:11 AM, Robert Haas [via PostgreSQL] <
ml-node+s1045698n5808915...@n5.nabble.com> wrote:

> On Tue, Jun 24, 2014 at 10:50 AM, Vik Fearing <[hidden email]
> > wrote:
>
> > On 06/24/2014 04:04 PM, Robert Haas wrote:
> >>> If the local transaction is actually idle in transaction and the local
> >>> > server doesn't have a timeout, we're no worse off than before this
> patch.
> >>
> >> I think we are.  First, the correct timeout is a matter of
> >> remote-server-policy, not local-server-policy.  If the remote server
> >> wants to boot people with long-running idle transactions, it's
> >> entitled to do that, and postgres_fdw shouldn't assume that it's
> >> "special".
> >
> > So how would the local transaction ever get its work done?  What option
> > does it have to tell the remote server that it isn't actually idling, it
> > just doesn't need to use the remote connection for a while?
>
> It *is* idling.  You're going to get bloat, and lock contention, and
> so on, just as you would for any other idle session.
>
>
If an application is making use of the foreign server directly then there
is the option to commit after using the foreign server, while saving the
relevant data for the main transaction.  But if you make use of API
functions there can only be a single transaction encompassing both the
local and foreign servers.  But even then, if the user needs a logical
super-transaction across both servers - even though the bulk of the work
occurs locally - that option to commit is then removed regardless of client
usage.

IMO this tool is too blunt to properly allow servers to self-manage
fdw-initiated transactions/sessions; and allowing it to be used is asking
for end-user confusion and frustration.

OTOH, requiring the administrator of the foreign server to issue an ALTER
ROLE fdw_user SET idle_in_transaction_session_timeout = 0; would be fairly
easy to justify.  Allowing them to distinguish between known long-running
and problematic transactions and those that are expected to execute quickly
has value as well.

Ultimately you give the users power and then just need to make sure we
provide sufficient documentation suggestions on how best to configure the
two servers in various typical usage scenarios.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/idle-in-transaction-timeout-tp5805859p5808920.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

[HACKERS] Re: how can i prevent materialized views from refreshing during pg_restore

[David G Johnston]

bithead wrote
> I asked a question over on StackOverflow, and Craig Ringer told me to
> report it here.
> 
> http://stackoverflow.com/questions/24413161/how-can-i-prevent-materialized-views-from-refreshing-during-pg-restore
> 
> I have created a dump of the database using pg_dump in "custom" format
> (-Fc). This format allows for pg_restore to be invoked with the "jobs"
> option (-j8). The jobs options starts 8 processes, and restores the vast
> majority of relations in my database within 10 minutes.
> 
> I'm left with 4 processes. One of them is the refresh of a materialized
> view, and the other 3 are indexes to be applied to 3 tables that the
> materialized view uses as data sources. The indexes are "waiting"
> according to pg_stat_activity, presumably because the REFRESH of the
> materialized view is still accessing the source tables.
> 
> When the indexes are in place, the refresh of the view only takes a couple
> of minutes. Because the indexes are not in place during the REFRESH, I cut
> the REFRESH process off at 17 hours, which made pg_restore fail.
> 
> How can I
> 
> Force the order of items so the indexes get created first
> Turn off the refresh of the materialized view and do it manually later
> Manipulate the dump file in custom format to say "WITH NO DATA"
> Intercept the REFRESH MATERIALIZED VIEW statement and throw it in the
> trash
> 
> Or any other solution that gets the job done?
> 
> I have a dump file that I'm willing to send to somebody that seems to
> reproduce the problem pretty consistently.

Have/can you try the "-l (el) & -L" options to pg_restore?

http://www.postgresql.org/docs/9.3/static/app-pgrestore.html

(example of usage is toward the bottom of the page)

Basically re-order the command sequence so that the materialize runs as late
as possible, or just disable it altogether.

pg_dump/pg_restore should be taught to handle this better, which is the main
reason why Craig had you post here ASAP, but to get it functional for now
manual intervention will be necessary.  In theory the "listing" capabilities
should allow you to do what you need.

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/how-can-i-prevent-materialized-views-from-refreshing-during-pg-restore-tp5809364p5809367.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] Can simplify 'limit 1' with slow function?

[David G Johnston]

Merlin Moncure-2 wrote
> On Tue, Jul 1, 2014 at 2:16 PM, Martijn van Oosterhout
> <

> kleptog@

> > wrote:
>> On Sun, Jun 29, 2014 at 10:05:50PM +0800, gotoschool6g wrote:
>>> The simplified scene:
>>> select slowfunction(s) from a order by b limit 1;
>>> is slow than
>>> select slowfunction(s) from (select s from a order by b limit 1) as z;
>>> if there are many records in table 'a'.
>>>
>>>
>>> The real scene. Function  ST_Distance_Sphere is slow, the query:
>>> SELECT ST_Distance_Sphere(s, ST_GeomFromText('POINT(1 1)')) from road
>>> order by c limit 1;
>>> is slow than:
>>> select ST_Distance_Sphere(s, ST_GeomFromText('POINT(1 1)')) from (SELECT
>>> s from road order by c limit 1) as a;
>>> There are about 7000 records in 'road'.
>>
>> I think to help here I think we need the EXPLAIN ANALYSE output for
>> both queries.
> 
> Well, I think the problem is a well understood one: there is no
> guarantee that functions-in-select-list are called exactly once per
> output row.  This is documented -- for example see here:
> http://www.postgresql.org/docs/9.1/static/explicit-locking.html#ADVISORY-LOCKS.
> In short, if you want very precise control of function evaluation use
> a subquery, or, if you're really paranoid, a CTE.
> 
> merlin

I would have to disagree on the "this is documented" comment - the linked
section on advisory locks does not constitute documentation of the fact that
limit can be applied after expressions in the select-list are evaluated.

http://www.postgresql.org/docs/9.3/static/sql-select.html

In the select command documentation item 5 covers select-list evaluation
while item 9 covers limit thus implying what we are saying - though keep in
mind each select statement gets processed independently and possibly in a
correlated fashion (i.e. potentially multiple times).

David J.




--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Can-simplify-limit-1-with-slow-function-tp5809997p5810061.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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