Re: Missing grammar production for WITH TIES

[Tom Lane]

Michael Paquier  writes:
> On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
>> Done.  Thanks!

> This has been committed just after beta1 has been stamped.  So it
> means that it won't be included in it, right?

Right.

regards, tom lane

Re: [HACKERS] Restricting maximum keep segments by repslots

[Alvaro Herrera]

On 2020-May-19, Michael Paquier wrote:

> On Mon, May 18, 2020 at 07:44:59PM -0400, Alvaro Herrera wrote:
> > BTW while you're messing with checkpointer, I propose this patch to
> > simplify things.
> 
> It seems to me that this would have a benefit if we begin to have a
> code path in CreateCheckpoint() where where it makes sense to let the
> checkpointer know that no checkpoint has happened, and now we assume
> that a skipped checkpoint is a performed one.

Well, my first attempt at this was returning false in that case, until I
realized that it would break the scheduling algorithm.

> As that's not the case now, I would vote for keeping the code as-is.

The presented patch doesn't have any functional impact; it just writes
the same code in a more concise way.  Like you, I wouldn't change this
if we didn't have a reason to rewrite this section of code.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Missing grammar production for WITH TIES

[Alvaro Herrera]

On 2020-May-19, Vik Fearing wrote:

> On 5/19/20 4:36 AM, Michael Paquier wrote:
>
> > This has been committed just after beta1 has been stamped.  So it
> > means that it won't be included in it, right?
> 
> Correct.

Right.

> I don't know why there was a delay, but it also doesn't bother me.

I didn't want to risk breaking the buildfarm at the last minute.  It'll
be there in beta2.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Missing grammar production for WITH TIES

[Vik Fearing]

On 5/19/20 4:36 AM, Michael Paquier wrote:
> On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
>> Done.  Thanks!
> 
> This has been committed just after beta1 has been stamped.  So it
> means that it won't be included in it, right?

Correct.

I don't know why there was a delay, but it also doesn't bother me.
-- 
Vik Fearing

Re: [HACKERS] Restricting maximum keep segments by repslots

[Michael Paquier]

On Mon, May 18, 2020 at 07:44:59PM -0400, Alvaro Herrera wrote:
> BTW while you're messing with checkpointer, I propose this patch to
> simplify things.

It seems to me that this would have a benefit if we begin to have a
code path in CreateCheckpoint() where where it makes sense to let the
checkpointer know that no checkpoint has happened, and now we assume
that a skipped checkpoint is a performed one.  As that's not the case
now, I would vote for keeping the code as-is.
--
Michael


signature.asc
Description: PGP signature

Re: SyncRepLock acquired exclusively in default configuration

[Masahiko Sawada]

On Sat, 11 Apr 2020 at 09:30, Masahiko Sawada
 wrote:
>
> On Fri, 10 Apr 2020 at 21:52, Fujii Masao  wrote:
> >
> >
> >
> > On 2020/04/10 20:56, Masahiko Sawada wrote:
> > > On Fri, 10 Apr 2020 at 18:57, Fujii Masao  
> > > wrote:
> > >>
> > >>
> > >>
> > >> On 2020/04/10 14:11, Masahiko Sawada wrote:
> > >>> On Fri, 10 Apr 2020 at 13:20, Fujii Masao  
> > >>> wrote:
> > 
> > 
> > 
> >  On 2020/04/08 3:01, Ashwin Agrawal wrote:
> > >
> > > On Mon, Apr 6, 2020 at 2:14 PM Andres Freund  > > > wrote:
> > >
> > >> How about we change it to this ?
> > >
> > >   Hm. Better. But I think it might need at least a compiler 
> > > barrier /
> > >   volatile memory load?  Unlikely here, but otherwise the 
> > > compiler could
> > >   theoretically just stash the variable somewhere locally (it's 
> > > not likely
> > >   to be a problem because it'd not be long ago that we acquired 
> > > an lwlock,
> > >   which is a full barrier).
> > >
> > >
> > > That's the part, I am not fully sure about. But reading the comment 
> > > above SyncRepUpdateSyncStandbysDefined(), it seems fine.
> > >
> > >> Bring back the check which existed based on GUC but instead 
> > > of just blindly
> > >> returning based on just GUC not being set, check
> > >> WalSndCtl->sync_standbys_defined. Thoughts?
> > >
> > >   Hm. Is there any reason not to just check
> > >   WalSndCtl->sync_standbys_defined? rather than both 
> > > !SyncStandbysDefined()
> > >   and WalSndCtl->sync_standbys_defined?
> > >
> > >
> > > Agree, just checking for WalSndCtl->sync_standbys_defined seems fine.
> > 
> >  So the consensus is something like the following? Patch attached.
> > 
> > /*
> >  -* Fast exit if user has not requested sync replication.
> >  +* Fast exit if user has not requested sync replication, or 
> >  there are no
> >  +* sync replication standby names defined.
> >  */
> >  -   if (!SyncRepRequested())
> >  +   if (!SyncRepRequested() ||
> >  +   !((volatile WalSndCtlData *) 
> >  WalSndCtl)->sync_standbys_defined)
> > return;
> > 
> > >>>
> > >>> I think we need more comments describing why checking
> > >>> sync_standby_defined without SyncRepLock is safe here. For example:
> > >>
> > >> Yep, agreed!
> > >>
> > >>> This routine gets called every commit time. So, to check if the
> > >>> synchronous standbys is defined as quick as possible we check
> > >>> WalSndCtl->sync_standbys_defined without acquiring SyncRepLock. Since
> > >>> we make this test unlocked, there's a change we might fail to notice
> > >>> that it has been turned off and continue processing.
> > >>
> > >> Does this really happen? I was thinking that the problem by not taking
> > >> the lock here is that SyncRepWaitForLSN() can see that shared flag after
> > >> SyncRepUpdateSyncStandbysDefined() wakes up all the waiters and
> > >> before it sets the flag to false. Then if SyncRepWaitForLSN() adds  
> > >> itself
> > >> into the wait queue becaues the flag was true, without lock, it may keep
> > >> sleeping infinitely.
> > >
> > > I think that because a backend does the following check after
> > > acquiring SyncRepLock, in that case, once the backend has taken
> > > SyncRepLock it can see that sync_standbys_defined is false and return.
> >
> > Yes, but the backend can see that sync_standby_defined indicates false
> > whether holding SyncRepLock or not, after the checkpointer sets it to false.
> >
> > > But you meant that we do both checks without SyncRepLock?
> >
> > Maybe No. The change that the latest patch provides should be applied, I 
> > think.
> > That is, sync_standbys_defined should be check without lock at first, then
> > only if it's true, it should be checked again with lock.
>
> Yes. My understanding is the same.
>
> After applying your patch, SyncRepWaitForLSN() is going to become
> something like:
>
> /*
>  * Fast exit if user has not requested sync replication, or there are no
>  * sync replication standby names defined.
>  */
> if (!SyncRepRequested() ||
> !((volatile WalSndCtlData *) WalSndCtl)->sync_standbys_defined)
> return;
>
> Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks)));
> Assert(WalSndCtl != NULL);
>
> LWLockAcquire(SyncRepLock, LW_EXCLUSIVE);
> Assert(MyProc->syncRepState == SYNC_REP_NOT_WAITING);
>
> /*
>  * We don't wait for sync rep if WalSndCtl->sync_standbys_defined is not
>  * set.  See SyncRepUpdateSyncStandbysDefined.
>  *
>  * Also check that the standby hasn't already replied. Unlikely race
>  * condition but we'll be fetching that cache line anyway so it's likely
>  * to be a low cost 

Re: Missing grammar production for WITH TIES

[Michael Paquier]

On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
> Done.  Thanks!

This has been committed just after beta1 has been stamped.  So it
means that it won't be included in it, right?
--
Michael


signature.asc
Description: PGP signature

Re: xid wraparound danger due to INDEX_CLEANUP false

[Masahiko Sawada]

On Thu, 7 May 2020 at 16:26, Masahiko Sawada
 wrote:
>
> On Thu, 7 May 2020 at 15:40, Masahiko Sawada
>  wrote:
> >
> > On Thu, 7 May 2020 at 03:28, Peter Geoghegan  wrote:
> > >
> > > On Wed, May 6, 2020 at 2:28 AM Masahiko Sawada
> > >  wrote:
> > > > I've attached the patch fixes this issue.
> > > >
> > > > With this patch, we don't skip only index cleanup phase when
> > > > performing an aggressive vacuum. The reason why I don't skip only
> > > > index cleanup phase is that index vacuum phase can be called multiple
> > > > times, which takes a very long time. Since the purpose of this index
> > > > cleanup is to process recyclable pages it's enough to do only index
> > > > cleanup phase.
> > >
> > > That's only true in nbtree, though. The way that I imagined we'd want
> > > to fix this is by putting control in each index access method. So,
> > > we'd revise the way that amvacuumcleanup() worked -- the
> > > amvacuumcleanup routine for each index AM would sometimes be called in
> > > a mode that means "I don't really want you to do any cleanup, but if
> > > you absolutely have to for your own reasons then you can". This could
> > > be represented using something similar to
> > > IndexVacuumInfo.analyze_only.
> > >
> > > This approach has an obvious disadvantage: the patch really has to
> > > teach *every* index AM to do something with that state (most will
> > > simply do no work). It seems logical to have the index AM control what
> > > happens, though. This allows the logic to live inside
> > > _bt_vacuum_needs_cleanup() in the case of nbtree, so there is only one
> > > place where we make decisions like this.
> > >
> > > Most other AMs don't have this problem. GiST has a similar issue with
> > > recyclable pages, except that it doesn't use 32-bit XIDs so it doesn't
> > > need to care about this stuff at all. Besides, it seems like it might
> > > be a good idea to do other basic maintenance of the index when we're
> > > "skipping" index cleanup. We probably should always do things that
> > > require only a small, fixed amount of work. Things like maintaining
> > > metadata in the metapage.
> > >
> > > There may be practical reasons why this approach isn't suitable for
> > > backpatch even if it is a superior approach. What do you think?
> >
> > I agree this idea is better. I was thinking the same approach but I
> > was concerned about backpatching. Especially since I was thinking to
> > add one or two fields to IndexVacuumInfo existing index AM might not
> > work with the new VacuumInfo structure.
>
> It would be ok if we added these fields at the end of VacuumInfo structure?
>

I've attached WIP patch for HEAD. With this patch, the core pass
index_cleanup to bulkdelete and vacuumcleanup callbacks so that they
can make decision whether run vacuum or not.

If the direction of this patch seems good, I'll change the patch so
that we pass something information to these callbacks indicating
whether this vacuum is anti-wraparound vacuum. This is necessary
because it's enough to invoke index cleanup before XID wraparound as
per discussion so far.

Regards,

-- 
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


fix_index_cleanup_v2_WIP.patch
Description: Binary data

Re: factorial function/phase out postfix operators?

[Vik Fearing]

On 5/19/20 4:03 AM, Tom Lane wrote:
> Peter Eisentraut  writes:
>> What are the thoughts about then marking the postfix operator deprecated 
>> and eventually removing it?
> 
> If we do this it'd require a plan.  We'd have to also warn about the
> feature deprecation in (at least) the CREATE OPERATOR man page, and
> we'd have to decide how many release cycles the deprecation notices
> need to stand for.

I have never come across any custom postfix operators in the wild, and
I've never even seen ! used in practice.

So I would suggest a very short deprecation period.  Deprecate now in
13, let 14 go by, and rip it all out for 15.  That should give us enough
time to extend the deprecation period if we need to, or go back on it
entirely (like I seem to remember we did with VACUUM FREEZE).

> If that's the intention, though, it'd be good to get those deprecation
> notices published in v13 not v14.

+1
-- 
Vik Fearing

PostgreSQL 13 Beta 1 Release Announcement Draft

[Jonathan S. Katz]

Hi,

Attached is a draft of the release announcement for the PostgreSQL 13
Beta 1 release this week.

The goal of this release announcement is to make people aware of the new
features that are introduced in PostgreSQL 13 and, importantly, get them
to start testing. I have tried to include a broad array of features that
can noticeably impact people's usage of PostgreSQL. Note that the order
of the features in the announcement are not in any particular ranking
(though I do call out VACUUM as being one of the "most anticipated
features"), but are my efforts to try and tell a story about the release.

Please let me know your thoughts, comments, corrections, etc. and also
if there are any glaring omissions. I know this is a bit longer than a
typical release announcement, but please let me know your feedback
before the end of Wed. May 20 AOE (i.e. before the release ships).

Thanks for your review!

Jonathan
PostgreSQL 13 Beta 1 Released
=

The PostgreSQL Global Development Group announces that the first beta release of
PostgreSQL 13 is now available for download. This release contains previews of
all features that will be available in the final release of PostgreSQL 13,
though some details of the release could change before then.

You can find information about all of the features and changes found in
PostgreSQL 13 in the [release 
notes](https://www.postgresql.org/docs/13/release-13.html):

  
[https://www.postgresql.org/docs/13/release-13.html](https://www.postgresql.org/docs/13/release-13.html)

In the spirit of the open source PostgreSQL community, we strongly encourage you
to test the new features of PostgreSQL 13 in your database systems to help us
eliminate any bugs or other issues that may exist. While we do not advise you to
run PostgreSQL 13 Beta 1 in your production environments, we encourage you to
find ways to run your typical application workloads against this beta release.

Your testing and feedback will help the community ensure that the PostgreSQL 13
release upholds our standards of providing a stable, reliable release of the
world's most advanced open source relational database. You can read more about
the [beta testing process](https://www.postgresql.org/developer/beta/) and how
you can contribute here:

  
[https://www.postgresql.org/developer/beta/](https://www.postgresql.org/developer/beta/)

PostgreSQL 13 Feature Highlights


PostgreSQL 13 introduces many features and improvements to the world's most
advanced open source database. The following is just a small sample of new
features introduced into this latest version of PostgreSQL.

Our goal is to make you aware of the upcoming features so you can help with the
[testing effort](https://www.postgresql.org/developer/beta/). We strongly
encourage you provide feedback as well as any bug reports so we can ensure we
can continue to deliver stable, reliable database software.


### Functionality

There are many new features that improve the overall performance of
PostgreSQL 13 and make it easier to work with simple and complex workloads.

On the performance front, B-tree indexes, the standard index of PostgreSQL,
received improvements to how duplicate data is handled in the index. This helps
to shrink the index size and improve the speed of lookups, particularly for
indexes that have significant duplicates.

PostgreSQL 13 adds incremental sorting, which accelerates sorting data when
existing data in a query is already sorted. Queries with OR clauses or IN/ANY
constant lists can now make use of extended statistics, created with
`CREATE STATISTICS`, which can lead to better planning and performance gains.
PostgreSQL 13 can now use disk storage for hash aggregation (used as part of
aggregate queries) with large aggregation sets.

There are more improvements added to PostgreSQL's partitioning functionality in
this release, including an increased number of cases where a
"partitionwise join" (a join between matching partitions) can be used, which
can improve overall query execution time. Partitioned tables now support
`BEFORE` row-level triggers, and a partitioned table can now be fully replicated
via logical replication without having to publish individual partitions.

PostgreSQL 13 brings more convenience to writing queries and introduces features
such as `FETCH FIRST WITH TIES` that will return any additional rows that match
the last row. There is also the addition of the ".datetime" function for
jsonpath queries, which will automatically convert a date-like or time-like
string to the appropriate PostgreSQL date/time datatype. It is also even easier
now to generate random UUIDs, as the `gen_random_uuid()` can be used without
having to enable any extensions.

### Administration

One of the most anticipated features of PostgreSQL 13 is the ability for the
`VACUUM` command to process indexes in parallel. This functionality can be
accessed with the new `PARALLEL` option on the `VACUUM` 

Re: Planning counters in pg_stat_statements (using pgss_store)

[Andy Fan]

Thanks for the excellent extension. I want to add 5 more fields to satisfy
the
following requirements.

int   subplan; /* No. of subplan in this query */
int   subquery; /* No. of subquery */
int   joincnt; /* How many relations are joined */
bool hasagg; /* if we have agg function in this query */
bool hasgroup; /* has group clause */


1. Usually I want to check total_exec_time / rows to see if the query is
missing
   index, however aggregation/groupby case makes this rule doesn't work. so
   hasagg/hasgroup should be a good rule to filter out these queries.

2. subplan is also a important clue to find out the query to turning. when
we
   check the slow queries with pg_stat_statements, such information maybe
   helpful as well.

3. As for subquery / joincnt,  actually it is just helpful for optimizer
   developer to understand the query character is running most, it doesn't
help
   much for user.


The attached is a PoC, that is far from perfect since 1). It maintain a
per-backend global variable query_character which is only used in
pg_stat_statements extension.  2). The 5 fields is impossible to change no
matter how many times it runs, so it can't be treat as Counter in nature.
However I don't think the above 2 will cause big issues.

I added the columns to V1_8 rather than adding a new version. this can be
changed at final patch.

Any suggestions?


Best Regards
Andy Fan


v1-0001-Add-query-characters-information-to-pg_stat_state.patch
Description: Binary data

Re: factorial function/phase out postfix operators?

[David Fetter]

On Mon, May 18, 2020 at 10:03:13PM -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > What are the thoughts about then marking the postfix operator deprecated 
> > and eventually removing it?
> 
> If we do this it'd require a plan.  We'd have to also warn about the
> feature deprecation in (at least) the CREATE OPERATOR man page, and
> we'd have to decide how many release cycles the deprecation notices
> need to stand for.
> 
> If that's the intention, though, it'd be good to get those deprecation
> notices published in v13 not v14.

+1 for deprecating in v13.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: BUG #16147: postgresql 12.1 (from homebrew) - pg_restore -h localhost --jobs=2 crashes

[Justin Pryzby]

On Thu, Mar 05, 2020 at 07:53:35PM -0800, David Zhang wrote:
> I can reproduce this pg_restore crash issue (pg_dump crash too when running
> with multiple jobs) on MacOS 10.14 Mojave and MacOS 10.15 Catalina using
> following steps.

Isn't this the same as here?
https://www.postgresql.org/message-id/flat/16041-b44f9931ad91fc3d%40postgresql.org
..concluding that macos library fails after forking.

..I found that via: https://github.com/PostgresApp/PostgresApp/issues/538
=> https://www.postgresql.org/message-id/1575881854624-0.post%40n3.nabble.com

-- 
Justin

Re: factorial function/phase out postfix operators?

[Tom Lane]

Peter Eisentraut  writes:
> What are the thoughts about then marking the postfix operator deprecated 
> and eventually removing it?

If we do this it'd require a plan.  We'd have to also warn about the
feature deprecation in (at least) the CREATE OPERATOR man page, and
we'd have to decide how many release cycles the deprecation notices
need to stand for.

If that's the intention, though, it'd be good to get those deprecation
notices published in v13 not v14.

regards, tom lane

Re: [HACKERS] Restricting maximum keep segments by repslots

[Alvaro Herrera]

BTW while you're messing with checkpointer, I propose this patch to
simplify things.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 9148a6defa2e8b3fd81b982de53f73584a8b3d10 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Fri, 8 May 2020 15:28:57 -0400
Subject: [PATCH] CreateCheckPoint return bool

---
 src/backend/access/transam/xlog.c | 13 +++--
 src/backend/postmaster/checkpointer.c |  9 ++---
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ca09d81b08..8990ef7348 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8695,8 +8695,10 @@ UpdateCheckPointDistanceEstimate(uint64 nbytes)
  * All of this mechanism allows us to continue working while we checkpoint.
  * As a result, timing of actions is critical here and be careful to note that
  * this function will likely take minutes to execute on a busy system.
+ *
+ * Return value is true.
  */
-void
+bool
 CreateCheckPoint(int flags)
 {
 	bool		shutdown;
@@ -8815,7 +8817,12 @@ CreateCheckPoint(int flags)
 			END_CRIT_SECTION();
 			ereport(DEBUG1,
 	(errmsg("checkpoint skipped because system is idle")));
-			return;
+
+			/*
+			 * Returns true even if checkpoint is skipped; this is required to
+			 * prevent breaking the checkpoint scheduling algorithm.
+			 */
+			return true;
 		}
 	}
 
@@ -9107,6 +9114,8 @@ CreateCheckPoint(int flags)
 	 CheckpointStats.ckpt_segs_recycled);
 
 	LWLockRelease(CheckpointLock);
+
+	return true;
 }
 
 /*
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 34ed9f7887..380b243547 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -436,13 +436,8 @@ CheckpointerMain(void)
 			/*
 			 * Do the checkpoint.
 			 */
-			if (!do_restartpoint)
-			{
-CreateCheckPoint(flags);
-ckpt_performed = true;
-			}
-			else
-ckpt_performed = CreateRestartPoint(flags);
+			ckpt_performed = do_restartpoint ?
+CreateRestartPoint(flags) : CreateCheckPoint(flags);
 
 			/*
 			 * After any checkpoint, close all smgr files.  This is so we
-- 
2.20.1

Re: Missing grammar production for WITH TIES

[Alvaro Herrera]

On 2020-May-18, Alvaro Herrera wrote:

> On 2020-May-18, Vik Fearing wrote:
> 
> > The syntax for FETCH FIRST allows the  to be
> > absent (implying 1).
> > 
> > We implement this correctly for ONLY, but WITH TIES didn't get the memo.
> 
> Oops, yes.  I added a test.  Will get this pushed immediately after I
> see beta1 produced.

Done.  Thanks!

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: POC: rational number type (fractions)

[Tom Lane]

Chapman Flack  writes:
> On 05/18/20 17:33, Peter Eisentraut wrote:
>> The numeric type already stores rational numbers.  How is this different? 
>> What's the use?

> Won't ever quite represent, say, 1/3, no matter how big you let it get.

There surely are use-cases for true rational arithmetic, but I'm
dubious that it belongs in core Postgres.  I don't think that enough
of our users would want it to justify expending core-project maintenance
effort on it.  So I'd be happier to see this as an out-of-core extension.

(That'd also ease dealing with the prospect of having more than one
variant, as was mentioned upthread.)

regards, tom lane

Re: POC: rational number type (fractions)

[Chapman Flack]

On 05/18/20 17:33, Peter Eisentraut wrote:
> The numeric type already stores rational numbers.  How is this different? 
> What's the use?

Seems like numeric is a base-1 representation. Will work ok for
a rational whose denominator factors into 2s and 5s.

Won't ever quite represent, say, 1/3, no matter how big you let it get.

Regards,
-Chap

Re: PostgresSQL project

[Peter Eisentraut]

On 2020-05-18 18:21, Luke Porter wrote:
I am a member of a small UK based team with extensive database 
experience. We are considering a project using PostgresSQL source code 
which only uses the insert data capabilities.


Is there a contact who we could speak with and discuss our project aims 
in principal.


If yours is an open-source project that you eventually want to share 
with the community, then you can discuss it on this mailing list.


If it is a closed-source, proprietary, or in-house project, then the 
community isn't the right place to discuss it, but you could hire 
professional consultants to help you, depending on your needs.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: POC: rational number type (fractions)

[Peter Eisentraut]

On 2020-02-08 05:25, Joe Nelson wrote:

Hi hackers, attached is a proof of concept patch adding a new base type
called "rational" to represent fractions. It includes arithmetic,
simplification, conversion to/from float, finding intermediates with a
stern-brocot tree, custom aggregates, and btree/hash indices.


The numeric type already stores rational numbers.  How is this 
different?  What's the use?


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Another modest proposal for docs formatting: catalog descriptions

[Alvaro Herrera]

On 2020-May-06, Alvaro Herrera wrote:

> ... oh, okay.  I guess I was reporting that the font on the new version
> seems to have got smaller.  Looking at other pages, it appears that the
> font is indeed a lot smaller in all tables, including those Tom has been
> editing.  So maybe this is desirable for some reason.  I'll have to keep
> my magnifying glass handy, I suppose.

I happened to notice that the font used in the tables get smaller if you
make the browser window narrower.  So what was happening is that I was
using a window that didn't cover the entire screen.

If I let the window use my whole screen, the font in the table is the
same size than the text outside the table; if I reduce to ~1239 pixels,
the font becomes somewhat smaller; if I further reduce to ~953 pixels,
it gets really small.  Meanwhile, the non-table text keeps the same size
the whole time.  (The pixel sizes at which changes occur seem to vary
with the zoom percentage I use, but the behavior is always there.)

Is this something that CSS does somehow?  Is this something you expected?

Happens with both Brave (a Chromium derivate) and Firefox.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

PostgresSQL project

[Luke Porter]

Hi

I am a member of a small UK based team with extensive database experience. We 
are considering a project using PostgresSQL source code which only uses the 
insert data capabilities.

Is there a contact who we could speak with and discuss our project aims in 
principal.

Thanks

Luke

Re: Missing grammar production for WITH TIES

[Vik Fearing]

On 5/18/20 7:03 PM, Alvaro Herrera wrote:
> On 2020-May-18, Vik Fearing wrote:
> 
>> The syntax for FETCH FIRST allows the  to be
>> absent (implying 1).
>>
>> We implement this correctly for ONLY, but WITH TIES didn't get the memo.
> 
> Oops, yes.  I added a test.  Will get this pushed immediately after I
> see beta1 produced.

Thanks!
-- 
Vik Fearing

Re: pgindent && weirdness

[Stephen Frost]

Greetings,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Thomas Munro  writes:
> > It seems I cannot.  Please go ahead.
> 
> [ yawn... ]  It's about bedtime here, but I'll take care of it in the
> morning.
> 
> Off the critical path, we oughta figure out why the repo wouldn't
> let you commit.  What I was told was it was set up to be writable
> by all PG committers.

Just happened to see this.  Might be I'm not looking at the right thing,
but from what I can tell, the repo is set up with only you as having
write access.  We also don't currently have updating the pg_bsd_indent
repo on git.postgresql.org as part of our SOP for adding/removing
committers.

All of this is fixable, of course.  I've CC'd this to sysadmins@ to
highlight this issue and possible change to that repo and our SOP.

Barring complaints or concerns, based on Tom's comments above, I'll
adjust that repo to be 'owned' by pginfra, with all committers having
read/write access, and add it to our committer add/remove SOP to
update that repo's access list whenever there are changes.

I'll plan to do that in a couple of days to allow for any concerns or
questions, as this isn't a critical issue at present based on the above
comments.

Thanks,

Stephen


signature.asc
Description: PGP signature

Re: Two fsync related performance issues?

[Tom Lane]

Paul Guo  writes:
> table directories & wal fsync probably dominates the fsync time. Do we
> know any possible real scenario that requires table directory fsync?

Yes, there are filesystems where that's absolutely required.  See
past discussions that led to putting in those fsyncs (we did not
always have them).

regards, tom lane

Re: Why is pq_begintypsend so slow?

[Tom Lane]

Ranier Vilela  writes:
> Again, I see problems with the types declared in Postgres.
> 1. pq_sendint32 (StringInfo buf, uint32 i)
> 2. extern void pq_sendbytes (StringInfo buf, const char * data, int
> datalen);

We could spend the next ten years cleaning up minor discrepancies
like that, and have nothing much to show for the work.

> To avoid converting from (int) to (uint32), even if afterwards there is a
> conversion from (uint32) to (int)?

You do realize that that conversion costs nothing?

regards, tom lane

Re: fill_extraUpdatedCols is done in completely the wrong place

[Tom Lane]

Peter Eisentraut  writes:
> On 2020-05-08 21:05, Tom Lane wrote:
>> I happened to notice $subject while working on the release notes.
>> AFAICS, it is 100% inappropriate for the parser to compute the
>> set of generated columns affected by an UPDATE, because that set
>> could change before execution.  It would be really easy to break
>> this for an UPDATE in a stored rule, for example.

> Do you have a specific situation in mind?  How would a rule change the 
> set of columns updated by a query?  Something involving CTEs?  Having a 
> test case would be good.

broken-update-rule.sql, attached, shows the scenario I had in mind:
the rule UPDATE query knows nothing of the generated column that
gets added after the rule is stored, so the UPDATE fails to update it.

However, on the way to preparing that test case I discovered that
auto-updatable views have the same disease even when the generated column
exists from the get-go; see broken-updatable-views.sql.  In the context
of the existing design, I suppose this means that there needs to be
a fill_extraUpdatedCols call somewhere in the code path that constructs
an auto-update query.  But if we moved the whole thing to the executor
then the problem would go away.

I observe also that the executor doesn't seem to need this bitmap at all
unless (a) there are triggers or (b) there are generated columns.
So in a lot of simpler cases, the cost of doing fill_extraUpdatedCols
at either parse or plan time would be quite wasted.  That might be a good
argument for moving it to executor start, even though we'd then have
to re-do it when re-using a prepared plan.

regards, tom lane

drop view v1;
drop table tab1;

create table tab1 (f1 int, id serial);

insert into tab1 values (42);

table tab1;

create view v1 as select f1, id from tab1;

create rule r1 as on update to v1 do instead
update tab1 set f1 = new.f1 where tab1.id = new.id;

update v1 set f1 = 11;

table tab1;

alter table tab1 add column f2 int generated always as (f1 * 2) stored;

table tab1;

update v1 set f1 = 12;

table tab1;
drop view v1;
drop table tab1;

create table tab1 (f1 int, f2 int generated always as (f1 * 2) stored);

insert into tab1 values (42);

table tab1;

create view v1 as select * from tab1;

update v1 set f1 = 11;

table tab1;

Re: Why is pq_begintypsend so slow?

[Ranier Vilela]

Em seg., 18 de mai. de 2020 às 13:38, Tom Lane  escreveu:

> Andres Freund  writes:
> >> FWIW, I've also observed, in another thread (the node func generation
> >> thing [1]), that inlining enlargeStringInfo() helps a lot, especially
> >> when inlining some of its callers. Moving e.g. appendStringInfo() inline
> >> allows the compiler to sometimes optimize away the strlen. But if
> >> e.g. an inlined appendBinaryStringInfo() still calls enlargeStringInfo()
> >> unconditionally, successive appends cannot optimize away memory accesses
> >> for ->len/->data.
>
> > With a set of patches doing so, int4send itself is not a significant
> > factor for my test benchmark [1] anymore.
>
> This thread seems to have died out, possibly because the last set of
> patches that Andres posted was sufficiently complicated and invasive
> that nobody wanted to review it.  I thought about this again after
> seeing that [1] is mostly about pq_begintypsend overhead, and had
> an epiphany: there isn't really a strong reason for pq_begintypsend
> to be inserting bits into the buffer at all.  The bytes will be
> filled by pq_endtypsend, and nothing in between should be touching
> them.  So I propose 0001 attached.  It's poking into the stringinfo
> abstraction a bit more than I would want to do if there weren't a
> compelling performance reason to do so, but there evidently is.
>
> With 0001, pq_begintypsend drops from being the top single routine
> in a profile of a test case like [1] to being well down the list.
> The next biggest cost compared to text-format output is that
> printtup() itself is noticeably more expensive.  A lot of the extra
> cost there seems to be from pq_sendint32(), which is getting inlined
> into printtup(), and there probably isn't much we can do to make that
> cheaper. But eliminating a common subexpression as in 0002 below does
> help noticeably, at least with the rather old gcc I'm using.
>
Again, I see problems with the types declared in Postgres.
1. pq_sendint32 (StringInfo buf, uint32 i)
2. extern void pq_sendbytes (StringInfo buf, const char * data, int
datalen);

Wouldn't it be better to declare outputlen (0002) as uint32?
To avoid converting from (int) to (uint32), even if afterwards there is a
conversion from (uint32) to (int)?

regards,
Ranier Vilela

Re: Vintage unused variables in pg_dump.c

[Tom Lane]

Daniel Gustafsson  writes:
> Unless I'm missing something, the g_comment_start and g_comment_end variables
> in pg_dump.c seems to have been unused since 30ab5bd43d8f2082659191 (in the 
> 7.2
> cycle) and can probably be safely removed by now.

Indeed.  (Well, I didn't verify your statement about when they were
last used, but they're clearly dead now.)  Pushed.

regards, tom lane

Re: Missing grammar production for WITH TIES

[Alvaro Herrera]

On 2020-May-18, Vik Fearing wrote:

> The syntax for FETCH FIRST allows the  to be
> absent (implying 1).
> 
> We implement this correctly for ONLY, but WITH TIES didn't get the memo.

Oops, yes.  I added a test.  Will get this pushed immediately after I
see beta1 produced.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From bdd724370215a550586e49a2f8ce2f554bdf79f4 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Mon, 18 May 2020 11:57:05 -0400
Subject: [PATCH] WITH TIES: number of rows is optional and default to one

Author: Vik Fearing
---
 src/backend/parser/gram.y   |  8 
 src/test/regress/expected/limit.out | 17 +
 src/test/regress/sql/limit.sql  |  5 +
 3 files changed, 30 insertions(+)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c78f2d1b5..a24b30f06f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11816,6 +11816,14 @@ limit_clause:
 	n->limitOption = LIMIT_OPTION_COUNT;
 	$$ = n;
 }
+			| FETCH first_or_next row_or_rows WITH TIES
+{
+	SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+	n->limitOffset = NULL;
+	n->limitCount = makeIntConst(1, -1);
+	n->limitOption = LIMIT_OPTION_WITH_TIES;
+	$$ = n;
+}
 		;
 
 offset_clause:
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index a4e175855c..e6f6809fbe 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -576,6 +576,23 @@ SELECT  thousand
 0
 (10 rows)
 
+SELECT  thousand
+		FROM onek WHERE thousand < 5
+		ORDER BY thousand FETCH FIRST ROWS WITH TIES;
+ thousand 
+--
+0
+0
+0
+0
+0
+0
+0
+0
+0
+0
+(10 rows)
+
 SELECT  thousand
 		FROM onek WHERE thousand < 5
 		ORDER BY thousand FETCH FIRST 1 ROW WITH TIES;
diff --git a/src/test/regress/sql/limit.sql b/src/test/regress/sql/limit.sql
index afce5019b2..d2d4ef132d 100644
--- a/src/test/regress/sql/limit.sql
+++ b/src/test/regress/sql/limit.sql
@@ -161,6 +161,10 @@ SELECT  thousand
 		FROM onek WHERE thousand < 5
 		ORDER BY thousand FETCH FIRST 2 ROW WITH TIES;
 
+SELECT  thousand
+		FROM onek WHERE thousand < 5
+		ORDER BY thousand FETCH FIRST ROWS WITH TIES;
+
 SELECT  thousand
 		FROM onek WHERE thousand < 5
 		ORDER BY thousand FETCH FIRST 1 ROW WITH TIES;
@@ -168,6 +172,7 @@ SELECT  thousand
 SELECT  thousand
 		FROM onek WHERE thousand < 5
 		ORDER BY thousand FETCH FIRST 2 ROW ONLY;
+
 -- should fail
 SELECT ''::text AS two, unique1, unique2, stringu1
 		FROM onek WHERE unique1 > 50
-- 
2.20.1

Re: Why is pq_begintypsend so slow?

[Tom Lane]

Andres Freund  writes:
>> FWIW, I've also observed, in another thread (the node func generation
>> thing [1]), that inlining enlargeStringInfo() helps a lot, especially
>> when inlining some of its callers. Moving e.g. appendStringInfo() inline
>> allows the compiler to sometimes optimize away the strlen. But if
>> e.g. an inlined appendBinaryStringInfo() still calls enlargeStringInfo()
>> unconditionally, successive appends cannot optimize away memory accesses
>> for ->len/->data.

> With a set of patches doing so, int4send itself is not a significant
> factor for my test benchmark [1] anymore.

This thread seems to have died out, possibly because the last set of
patches that Andres posted was sufficiently complicated and invasive
that nobody wanted to review it.  I thought about this again after
seeing that [1] is mostly about pq_begintypsend overhead, and had
an epiphany: there isn't really a strong reason for pq_begintypsend
to be inserting bits into the buffer at all.  The bytes will be
filled by pq_endtypsend, and nothing in between should be touching
them.  So I propose 0001 attached.  It's poking into the stringinfo
abstraction a bit more than I would want to do if there weren't a
compelling performance reason to do so, but there evidently is.

With 0001, pq_begintypsend drops from being the top single routine
in a profile of a test case like [1] to being well down the list.
The next biggest cost compared to text-format output is that
printtup() itself is noticeably more expensive.  A lot of the extra
cost there seems to be from pq_sendint32(), which is getting inlined
into printtup(), and there probably isn't much we can do to make that
cheaper. But eliminating a common subexpression as in 0002 below does
help noticeably, at least with the rather old gcc I'm using.

For me, the combination of these two eliminates most but not quite
all of the cost penalty of binary over text output as seen in [1].

regards, tom lane

[1] 
https://www.postgresql.org/message-id/CAMovtNoHFod2jMAKQjjxv209PCTJx5Kc66anwWvX0mEiaXwgmA%40mail.gmail.com

diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index a6f990c..03b7404 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -328,11 +328,16 @@ void
 pq_begintypsend(StringInfo buf)
 {
 	initStringInfo(buf);
-	/* Reserve four bytes for the bytea length word */
-	appendStringInfoCharMacro(buf, '\0');
-	appendStringInfoCharMacro(buf, '\0');
-	appendStringInfoCharMacro(buf, '\0');
-	appendStringInfoCharMacro(buf, '\0');
+
+	/*
+	 * Reserve four bytes for the bytea length word.  We don't need to fill
+	 * them with anything (pq_endtypsend will do that), and this function is
+	 * enough of a hot spot that it's worth cheating to save some cycles. Note
+	 * in particular that we don't bother to guarantee that the buffer is
+	 * null-terminated.
+	 */
+	Assert(buf->maxlen > 4);
+	buf->len = 4;
 }
 
 /* 
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index dd1bac0..a9315c6 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -438,11 +438,12 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
 		{
 			/* Binary output */
 			bytea	   *outputbytes;
+			int			outputlen;
 
 			outputbytes = SendFunctionCall(>finfo, attr);
-			pq_sendint32(buf, VARSIZE(outputbytes) - VARHDRSZ);
-			pq_sendbytes(buf, VARDATA(outputbytes),
-		 VARSIZE(outputbytes) - VARHDRSZ);
+			outputlen = VARSIZE(outputbytes) - VARHDRSZ;
+			pq_sendint32(buf, outputlen);
+			pq_sendbytes(buf, VARDATA(outputbytes), outputlen);
 		}
 	}
 

Re: Add A Glossary

[Jürgen Purtz]

On 17.05.20 17:28, Alvaro Herrera wrote:

On 2020-May-17, Erik Rijkers wrote:


On 2020-05-17 08:51, Alvaro Herrera wrote:

I don't think that's the general understanding of those terms.  For all
I know, they*are*  synonyms, and there's no specific term for "the
fluctuating objects" as you call them.  The instance is either running
(in which case there are processes and RAM) or it isn't.

For what it's worth, I've also always understood 'instance' as 'a running
database'.  I admit it might be a left-over from my oracle years:

https://docs.oracle.com/cd/E11882_01/server.112/e40540/startup.htm#CNCPT601

There, 'instance' clearly refers to a running database.  When that database
is stopped, it ceases to be an instance.

I've never understood it that way, but I'm open to having my opinion on
it changed.  So let's discuss it and maybe gather opinions from others.

I think the terms under discussion are just

* cluster
* instance
* server

We don't have "host" (I just made it a synonym for server), but perhaps
we can add that too, if it's useful.  It would be good to be consistent
with historical Postgres usage, such as the initdb usage of "cluster"
etc.

Perhaps we should not only define what our use of each term is, but also
explain how each term is used outside PostgreSQL and highlight the
differences.  (This would be particularly useful for "cluster" ISTM.)


In fact, we have reached a point where we don't have a common 
understanding of a group of terms. I'm sure that we will meet some more 
situations like this in the future. Such discussions, subsequent 
decisions, and implementations in the docs are necessary to gain a solid 
foundation - primarily for newcomers (what is my first motivation) as 
well as for more complex discussions among experts. Obviously, each of 
us will include his previous understanding of terms. But we also should 
be open to sometimes revise old terms.


Here are my two cents.

cluster/instance: PG (mainly) consists of a group of processes that 
commonly act on shared buffers. The processes are very closely related 
to each other and with the buffers. They exist altogether or not at all. 
They use a common initialization file and are incarnated by one command. 
Everything exists solely in RAM and therefor has a fluctuating nature. 
In summary: they build a unit and this unit needs to have a name of 
itself. In some pages we used to use the term *instance* - sometimes in 
extended forms: *database instance*, *PG instance*, *standby instance*, 
*standby server instance*, *server instance*, or *remote instance*.  For 
me, the term *instance* makes sense, the extensions *standby instance* 
and *remote instance* in their context too.


The next essential component is the data itself. It is organized as a 
group of databases plus some common management information (global, 
pg_wal, pg_xact, pg_tblspc, ...). The complete data must be treated as a 
whole because the management information concerns all databases. Its 
nature is different from the processes and shared buffers. Of course, 
its content changes, but it has a steady nature. It even survives a 
'power down'. There is one command to instantiate a new incarnation of 
the directory structure and all files. In summary, it's something of its 
own and should have its own name. 'database' is not possible because it 
consists of databases and other things. My favorite is *cluster*; 
*database cluster* is also possible.


server/host: We need a term to describe the underlying hardware 
respectively the virtual machine or container, where PG is running. I 
suggest to use both *server* and *host*. In computer science, both have 
their eligibility and are widely used. Everybody understands 
*client/server architecture* or *host* in TCP/IP configuration. We 
cannot change such matter of course. I suggest to use both depending on 
the context, but with the same meaning: "real hardware, a container, or 
a virtual machine".


--

Jürgen Purtz

(PS: I added the docs mailing list)

Re: factorial function/phase out postfix operators?

[Bruce Momjian]

On Mon, May 18, 2020 at 05:02:34PM +0200, Vik Fearing wrote:
> On 5/18/20 4:42 PM, Peter Eisentraut wrote:
> > There have been occasional discussions about deprecating or phasing out
> > postfix operators, to make various things easier in the parser.
> > 
> > The first step would in any case be to provide alternatives for the
> > existing postfix operators.  There is currently one, namely the numeric
> > factorial operator "!".  A sensible alternative for that would be
> > providing a function factorial(numeric) -- and that already exists but
> > is not documented.  (Note that the operator is mapped to proname
> > "numeric_fac".  The function "factorial" maps to the same prosrc but is
> > otherwise independent of the operator.)
> > 
> > So I suggest that we add that function to the documentation.
> 
> I think this should be done regardless.
> 
> > (Some adjacent cleanup work might also be in order.  The test cases for
> > factorial are currently in int4.sql, but all the factorial functionality
> > was moved to numeric a long time ago.)
> > 
> > What are the thoughts about then marking the postfix operator deprecated
> > and eventually removing it?
> 
> I am greatly in favor of removing postfix operators as soon as possible.

Agreed.

-- 
  Bruce Momjian  https://momjian.us
  EnterpriseDB https://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

proposal - plpgsql - FOR over unbound cursor

[Pavel Stehule]

Hi

Last week I played with dbms_sql extension and some patterns of usage
cursor in PL/SQL and PL/pgSQL. I found fact, so iteration over cursor (FOR
statement) doesn't support unbound cursors. I think so this limit is not
necessary. This statement can open portal for bound cursor or can iterate
over before opened portal. When portal was opened inside FOR statement,
then it is closed inside this statement.

Implementation is simple, usage is simple too:

CREATE OR REPLACE FUNCTION public.forc02()
 RETURNS void
 LANGUAGE plpgsql
AS $function$
declare
  c refcursor;
  r record;
begin
  open c for select * from generate_series(1,20) g(v);

  for r in c
  loop
raise notice 'cycle body one %', r.v;
exit when r.v >= 6;
  end loop;

  for r in c
  loop
raise notice 'cycle body two %', r.v;
  end loop;

   close c;
end
$function$

Comments, notes?

Regards

Pavel
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 71e5400422..34f8c5fea7 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3471,7 +3471,7 @@ FOR recordvar IN bound_cursorvar label ;
 
 
- The cursor variable must have been bound to some query when it was
+ In this case the cursor variable must have been bound to some query when it was
  declared, and it cannot be open already.  The
  FOR statement automatically opens the cursor, and it closes
  the cursor again when the loop exits.  A list of actual argument value
@@ -3481,6 +3481,21 @@ END LOOP  label ;
  linkend="plpgsql-open-bound-cursor"/>).

 
+
+ There is a variant for iteration over unbound cursors.
+
+
+ label 
+FOR recordvar IN unbound_cursorvar LOOP
+statements
+END LOOP  label ;
+
+
+ In this case the cursor have to be assigned to opened cursor portal. The
+ FOR statement in this case doesn't open the cursor and
+ doesn't close the cursor when loop exists.
+   
+

  The variable recordvar is automatically
  defined as type record and exists only inside the loop (any
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 9a87cd70f1..392cf9313a 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2834,6 +2834,45 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 	Portal		portal;
 	int			rc;
 
+	curvar = (PLpgSQL_var *) (estate->datums[stmt->curvar]);
+
+	/* --
+	 * REFTYPE cursor support - in this case cursor should be opened,
+	 * so it should be not null with active portal - same prereq like
+	 * FETCH stmt.
+	 * --
+	 */
+	if (curvar->cursor_explicit_expr == NULL)
+	{
+		MemoryContext oldcontext;
+
+		if (curvar->isnull)
+			ereport(ERROR,
+	(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+	 errmsg("cursor variable \"%s\" is null", curvar->refname)));
+
+		/* Use eval_mcontext for short-lived string */
+		oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
+		curname = TextDatumGetCString(curvar->value);
+		MemoryContextSwitchTo(oldcontext);
+
+		portal = SPI_cursor_find(curname);
+		if (portal == NULL)
+			ereport(ERROR,
+	(errcode(ERRCODE_UNDEFINED_CURSOR),
+	 errmsg("cursor \"%s\" does not exist", curname)));
+
+		rc = exec_for_query(estate, (PLpgSQL_stmt_forq *) stmt, portal, false);
+
+		/*
+		 * Don't close portal here - who did portal, then he should
+		 * to close portal.
+		 */
+
+		return rc;
+	}
+
+
 	/* --
 	 * Get the cursor variable and if it has an assigned name, check
 	 * that it's not in use currently.
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 6778d0e771..e372945a82 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -1405,13 +1405,6 @@ for_control		: for_variable K_IN
 		 errmsg("cursor FOR loop must have only one target variable"),
 		 parser_errposition(@1)));
 
-			/* can't use an unbound cursor this way */
-			if (cursor->cursor_explicit_expr == NULL)
-ereport(ERROR,
-		(errcode(ERRCODE_SYNTAX_ERROR),
-		 errmsg("cursor FOR loop must use a bound cursor variable"),
-		 parser_errposition(tokloc)));
-
 			/* collect cursor's parameters if any */
 			new->argquery = read_cursor_args(cursor,
 			 K_LOOP,
@@ -3785,7 +3778,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
 	bool		any_named = false;
 
 	tok = yylex();
-	if (cursor->cursor_explicit_argrow < 0)
+	if (cursor->cursor_explicit_argrow <= 0)
 	{
 		/* No arguments expected */
 		if (tok == '(')
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index d0a6b630b8..95f6bd72c9 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3607,9 +3607,56 @@ begin
   end loop;
 end;
 $$ language plpgsql;
-ERROR:  cursor FOR loop must use a bound cursor variable
-LINE 5:   for r in c loop
-   ^
+select forc_bad();
+ERROR:  cursor 

Re: Two fsync related performance issues?

[Paul Guo]

Thanks for the replies.

On Tue, May 12, 2020 at 2:04 PM Michael Paquier  wrote:

> On Tue, May 12, 2020 at 12:55:37PM +0900, Fujii Masao wrote:
> > On 2020/05/12 9:42, Paul Guo wrote:
> >> 1. StartupXLOG() does fsync on the whole data directory early in
> >> the crash recovery. I'm wondering if we could skip some
> >> directories (at least the pg_log/, table directories) since wal,
> >> etc could ensure consistency.
> >
> > I agree that we can skip log directory but I'm not sure if skipping
> > table directory is really safe. Also ISTM that we can skip the
> directories
> > that those contents are removed or zeroed during recovery,
> > for example, pg_snapshots, pg_substrans, etc.
>
> Basically excludeDirContents[] as of basebackup.c.
>

table directories & wal fsync probably dominates the fsync time. Do we
know any possible real scenario that requires table directory fsync?

Vintage unused variables in pg_dump.c

[Daniel Gustafsson]

Unless I'm missing something, the g_comment_start and g_comment_end variables
in pg_dump.c seems to have been unused since 30ab5bd43d8f2082659191 (in the 7.2
cycle) and can probably be safely removed by now.  The attached passes make
check.

cheers ./daniel



pg_dump_unused_vars.patch
Description: Binary data

Re: factorial function/phase out postfix operators?

[Vik Fearing]

On 5/18/20 4:42 PM, Peter Eisentraut wrote:
> There have been occasional discussions about deprecating or phasing out
> postfix operators, to make various things easier in the parser.
> 
> The first step would in any case be to provide alternatives for the
> existing postfix operators.  There is currently one, namely the numeric
> factorial operator "!".  A sensible alternative for that would be
> providing a function factorial(numeric) -- and that already exists but
> is not documented.  (Note that the operator is mapped to proname
> "numeric_fac".  The function "factorial" maps to the same prosrc but is
> otherwise independent of the operator.)
> 
> So I suggest that we add that function to the documentation.

I think this should be done regardless.

> (Some adjacent cleanup work might also be in order.  The test cases for
> factorial are currently in int4.sql, but all the factorial functionality
> was moved to numeric a long time ago.)
> 
> What are the thoughts about then marking the postfix operator deprecated
> and eventually removing it?

I am greatly in favor of removing postfix operators as soon as possible.
-- 
Vik Fearing

Re: fill_extraUpdatedCols is done in completely the wrong place

[Peter Eisentraut]

On 2020-05-08 21:05, Tom Lane wrote:

I happened to notice $subject while working on the release notes.
AFAICS, it is 100% inappropriate for the parser to compute the
set of generated columns affected by an UPDATE, because that set
could change before execution.  It would be really easy to break
this for an UPDATE in a stored rule, for example.


Do you have a specific situation in mind?  How would a rule change the 
set of columns updated by a query?  Something involving CTEs?  Having a 
test case would be good.



I think that that processing should be done by the planner, instead.
I don't object too much to keeping the data in RTEs ... but there had
better be a bold annotation that the set is not valid till after
planning.

An alternative solution is to keep the set in some executor data structure
and compute it during executor startup; perhaps near to where the
expressions are prepared for execution, so as to save extra stringToNode
calls.


Yeah, really only the executor ended up needing this, so perhaps it 
should be handled in the executor.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

factorial function/phase out postfix operators?

[Peter Eisentraut]

There have been occasional discussions about deprecating or phasing out 
postfix operators, to make various things easier in the parser.


The first step would in any case be to provide alternatives for the 
existing postfix operators.  There is currently one, namely the numeric 
factorial operator "!".  A sensible alternative for that would be 
providing a function factorial(numeric) -- and that already exists but 
is not documented.  (Note that the operator is mapped to proname 
"numeric_fac".  The function "factorial" maps to the same prosrc but is 
otherwise independent of the operator.)


So I suggest that we add that function to the documentation.

(Some adjacent cleanup work might also be in order.  The test cases for 
factorial are currently in int4.sql, but all the factorial functionality 
was moved to numeric a long time ago.)


What are the thoughts about then marking the postfix operator deprecated 
and eventually removing it?


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Missing grammar production for WITH TIES

[Vik Fearing]

The syntax for FETCH FIRST allows the  to be
absent (implying 1).

We implement this correctly for ONLY, but WITH TIES didn't get the memo.

Patch attached.
-- 
Vik Fearing
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c78f2d1b5..a24b30f06f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11816,6 +11816,14 @@ limit_clause:
 	n->limitOption = LIMIT_OPTION_COUNT;
 	$$ = n;
 }
+			| FETCH first_or_next row_or_rows WITH TIES
+{
+	SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+	n->limitOffset = NULL;
+	n->limitCount = makeIntConst(1, -1);
+	n->limitOption = LIMIT_OPTION_WITH_TIES;
+	$$ = n;
+}
 		;
 
 offset_clause:

Re: [PATCH] hs_standby_disallowed test fix

[Peter Eisentraut]

On 2020-05-12 19:35, Tom Lane wrote:

Fujii Masao  writes:

I just wonder why standbycheck regression test doesn't run by default
in buildfarm. Which caused us not to notice this issue long time. Maybe
because it's difficult to set up hot-standby environment in the
regression test? If so, we might need to merge standbycheck test into
TAP tests for recovery.


It seems likely to me that the standbycheck stuff has been completely
obsoleted by the TAP-based recovery tests.  We should get rid of it,
after adding any missing coverage to the TAP tests.


I have looked into this a few times.  It should definitely be done, but 
there is actually a fair amount of coverage in the standbycheck that is 
not in a TAP test, so it would be a fair amount of careful leg work to 
get this all moved over.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: PG 13 release notes, first draft

[Bruce Momjian]

Thanks, applied.

---

On Mon, May 18, 2020 at 11:18:51AM +0200, Daniel Gustafsson wrote:
> > On 5 May 2020, at 05:16, Bruce Momjian  wrote:
> > 
> > I have committed the first draft of the PG 13 release notes.  You can
> > see them here:
> 
> Spotted a typo we probably should fix: s/PostgresSQL/PostgreSQL/ =)
> 
> cheers ./daniel
> 

> diff --git a/doc/src/sgml/release-13.sgml b/doc/src/sgml/release-13.sgml
> index c39a6ad38e..7f864da162 100644
> --- a/doc/src/sgml/release-13.sgml
> +++ b/doc/src/sgml/release-13.sgml
> @@ -215,7 +215,7 @@ Author: Tom Lane 
>  
>   
>Remove support for defining operator
> -  classes using pre-PostgresSQL
> +  classes using pre-PostgreSQL
>8.0 syntax (Daniel Gustafsson)
>   
>  
> @@ -228,7 +228,7 @@ Author: Tom Lane 
>  
>   
>Remove support for defining foreign key
> -  constraints using pre-PostgresSQL
> +  constraints using pre-PostgreSQL
>7.3 syntax (Daniel Gustafsson)
>   
>  
> @@ -242,7 +242,7 @@ Author: Tom Lane 
>   
>Remove support for "opaque" linkend="sql-createtype">pseudo-types used by
> -  pre-PostgresSQL 7.3 servers (Daniel
> +  pre-PostgreSQL 7.3 servers (Daniel
>Gustafsson)
>   
>  


-- 
  Bruce Momjian  https://momjian.us
  EnterpriseDB https://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Spawned Background Process Knows the Exit of Client Process?

[Shichao Jin]

Hi Ashutosh,

Thank you for your answer.

For the first point, as you suggested, we will migrate to table AM sooner
or later.

For the second point, your description is exactly correct (an independent
process to access the storage engine). We can have multiple threads to
overcome the performance issue.

The problem comes from the ignorance of data types for storage engine,
where the storage engine has to get the comparator function of PG to
compare two keys. Otherwise, the storage engine uses "memcmp". In order to
get the compare func, we have to let the independent process dependent on a
specific database to access the catalog (relcache). Unfortunately, the
process cannot become independent anymore once it changed its property by
calling BackgroundWorkerInitializeConnection. Then our design evolves to
spawn multiple processes for accessing different tables created by the
storage engine. As a result, we have to release these spawned processes
once the backend process switches database or terminate itself. Currently,
we can set a timer for inactivity duration, in order to release the
resource. I am wondering is there any elegant way to achieve this goal?

Best,
Shichao

On Mon, 18 May 2020 at 08:37, Ashutosh Bapat 
wrote:

> On Fri, May 15, 2020 at 11:53 PM Shichao Jin  wrote:
> >
> > Hi Postgres Hackers,
> >
> > I am wondering is there any elegant way for self-spawned background
> process (forked by us) to get notified when the regular client-connected
> process exit from the current database (switch db or even terminate)?
> >
> > The background is that we are integrating a thread-model based storage
> engine into Postgres via foreign data wrapper.
>
> PostgreSQL now support pluggable storage API. Have you considered
> using that instead of FDW?
>
> > The engine is not allowed to have multiple processes to access it. So we
> have to spawn a background process to access the engine, while the client
> process can communicate with the spawned process via shared memory. In
> order to let the engine recognize the data type in Postgres, the spawned
> process has to access catalog such as relcache, and It must connect to the
> target database via BackgroundWorkerInitializeConnection to get the info.
> Unfortunately, it is not possible to switch databases for background
> process. So it has to get notified when client process switches db or
> terminate, then we can correspondingly close the spawned process. Please
> advise us if there are alternative approaches.
>
> There can be multiple backends accessing different database. But from
> your description it looks like there is only one background process
> that will access the storage engine and it will be shared by multiple
> backends which may be connected to different databases. If that's
> correct, you will need to make that background process independent of
> database and just access storage. That looks less performance though.
> May be you can elaborate more about your usecase.
>
> --
> Best Wishes,
> Ashutosh Bapat
>

Re: Spawned Background Process Knows the Exit of Client Process?

[Ashutosh Bapat]

On Fri, May 15, 2020 at 11:53 PM Shichao Jin  wrote:
>
> Hi Postgres Hackers,
>
> I am wondering is there any elegant way for self-spawned background process 
> (forked by us) to get notified when the regular client-connected process exit 
> from the current database (switch db or even terminate)?
>
> The background is that we are integrating a thread-model based storage engine 
> into Postgres via foreign data wrapper.

PostgreSQL now support pluggable storage API. Have you considered
using that instead of FDW?

> The engine is not allowed to have multiple processes to access it. So we have 
> to spawn a background process to access the engine, while the client process 
> can communicate with the spawned process via shared memory. In order to let 
> the engine recognize the data type in Postgres, the spawned process has to 
> access catalog such as relcache, and It must connect to the target database 
> via BackgroundWorkerInitializeConnection to get the info. Unfortunately, it 
> is not possible to switch databases for background process. So it has to get 
> notified when client process switches db or terminate, then we can 
> correspondingly close the spawned process. Please advise us if there are 
> alternative approaches.

There can be multiple backends accessing different database. But from
your description it looks like there is only one background process
that will access the storage engine and it will be shared by multiple
backends which may be connected to different databases. If that's
correct, you will need to make that background process independent of
database and just access storage. That looks less performance though.
May be you can elaborate more about your usecase.

-- 
Best Wishes,
Ashutosh Bapat

Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

[Amit Kapila]

On Mon, May 18, 2020 at 4:10 PM Amit Kapila  wrote:
>
> On Sun, May 17, 2020 at 12:41 PM Dilip Kumar  wrote:
> >

Few comments on v20-0010-Bugfix-handling-of-incomplete-toast-tuple
1.
+ /*
+ * If this is a toast insert then set the corresponding bit.  Otherwise, if
+ * we have toast insert bit set and this is insert/update then clear the
+ * bit.
+ */
+ if (toast_insert)
+ toptxn->txn_flags |= RBTXN_HAS_TOAST_INSERT;
+ else if (rbtxn_has_toast_insert(txn) &&
+ ChangeIsInsertOrUpdate(change->action))
+ {

Here, it might better to add a comment on why we expect only
Insert/Update?  Also, it might be better that we add an assert for
other operations.

2.
@@ -1865,8 +1920,8 @@ ReorderBufferProcessTXN(ReorderBuffer *rb,
ReorderBufferTXN *txn,
  * disk.
  */
  dlist_delete(>node);
- ReorderBufferToastAppendChunk(rb, txn, relation,
-   change);
+ ReorderBufferToastAppendChunk(rb, txn, relation,
+   change);
  }

This seems to be a spurious change.

3.
+ /*
+ * If streaming is enable and we have serialized this transaction because
+ * it had incomplete tuple.  So if now we have got the complete tuple we
+ * can stream it.
+ */
+ if (ReorderBufferCanStream(rb) && can_stream && rbtxn_is_serialized(toptxn)
+ && !(rbtxn_has_toast_insert(txn)) && !(rbtxn_has_spec_insert(txn)))
+ {

This comment is just saying what you are doing in the if-check.  I
think you need to explain the rationale behind it. I don't like the
variable name 'can_stream' because it matches ReorderBufferCanStream
whereas it is for a different purpose, how about naming it as
'change_complete' or something like that.  The check has many
conditions, can we move it to a separate function to make the code
here look clean?

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: Performance penalty when requesting text values in binary format

[Laurenz Albe]

On Sat, 2020-05-16 at 20:12 -0500, Jack Christensen wrote:
> I'm the creator of the PostgreSQL driver pgx (https://github.com/jackc/pgx) 
> for the Go language.
> I have found significant performance advantages to using the extended 
> protocol and binary format
> values -- in particular for types such as timestamptz.
> 
> However, I was recently very surprised to find that it is significantly 
> slower to select a text
> type value in the binary format. For an example case of selecting 1,000 rows 
> each with 5 text
> columns of 16 bytes each the application time from sending the query to 
> having received the
> entire response is approximately 16% slower. Here is a link to the test 
> benchmark:
> https://github.com/jackc/pg_text_binary_bench
> 
> Given that the text and binary formats for the text type are identical I 
> would not have
> expected any performance differences.
> 
>  My C is rusty and my knowledge of the PG server internals is minimal but the 
> performance
> difference appears to be that function textsend creates an extra copy where 
> textout
> simply returns a pointer to the existing data. This seems to be superfluous.
> 
> I can work around this by specifying the format per result column instead of 
> specifying
> binary for all but this performance bug / anomaly seemed worth reporting.

Did you profile your benchmark?
It would be interesting to know where the time is spent.

Yours,
Laurenz Albe

Re: Optimizer docs typos

[Richard Guo]

In this same README doc, another suspicious typo to me, which happens in
section "Optimizer Functions", is in the prefix to query_planner(),
we should have three dashes, rather than two, since query_planner() is
called within grouping_planner().

diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 7dcab9a..bace081 100644
--- a/src/backend/optimizer/README
+++ b/src/backend/optimizer/README
@@ -315,7 +315,7 @@ set up for recursive handling of subqueries
   preprocess target list for non-SELECT queries
   handle UNION/INTERSECT/EXCEPT, GROUP BY, HAVING, aggregates,
ORDER BY, DISTINCT, LIMIT
---query_planner()
+---query_planner()
make list of base relations used in query
split up the qual into restrictions (a=1) and joins (b=c)
find qual clauses that enable merge and hash joins

Thanks
Richard

On Mon, May 18, 2020 at 6:00 PM Etsuro Fujita 
wrote:

> On Mon, May 18, 2020 at 6:56 PM Magnus Hagander 
> wrote:
> > On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson 
> wrote:
> >> Attached diff fixes two small typos in the optimizer README.
>
> > Pushed, thanks.
>
> Thank you!
>
> Best regards,
> Etsuro Fujita
>
>
>

Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

[Amit Kapila]

On Sun, May 17, 2020 at 12:41 PM Dilip Kumar  wrote:
>
> On Fri, May 15, 2020 at 4:04 PM Amit Kapila  wrote:
> >
> >
> > Review comments:
> > --
> > 1.
> > @@ -1762,10 +1952,16 @@ ReorderBufferCommit(ReorderBuffer *rb,
> > TransactionId xid,
> >   }
> >
> >   case REORDER_BUFFER_CHANGE_MESSAGE:
> > - rb->message(rb, txn, change->lsn, true,
> > - change->data.msg.prefix,
> > - change->data.msg.message_size,
> > - change->data.msg.message);
> > + if (streaming)
> > + rb->stream_message(rb, txn, change->lsn, true,
> > +change->data.msg.prefix,
> > +change->data.msg.message_size,
> > +change->data.msg.message);
> > + else
> > + rb->message(rb, txn, change->lsn, true,
> > +change->data.msg.prefix,
> > +change->data.msg.message_size,
> > +change->data.msg.message);
> >
> > Don't we need to set any_data_sent flag while streaming messages as we
> > do for other types of changes?
>
> I think any_data_sent, was added to avoid sending abort to the
> subscriber if we haven't sent any data,  but this is not complete as
> the output plugin can also take the decision not to send.  So I think
> this should not be done as part of this patch and can be done
> separately.  I think there is already a thread for handling the
> same[1]
>

Hmm, but prior to this patch, we never use to send (empty) aborts but
now that will be possible. It is probably okay to deal that with
another patch mentioned by you but I felt at least any_data_sent will
work for some cases.  OTOH, it appears to be half-baked solution, so
we should probably refrain from adding it.  BTW, how do the pgoutput
plugin deal with it? I see that apply_handle_stream_abort will
unconditionally try to unlink the file and it will probably fail.
Have you tested this scenario after your latest changes?

>
> > 4.
> > In ReorderBufferProcessTXN(), the patch is calling stream_stop in both
> > the try and catch block.  If there is an error after calling it in a
> > try block, we might call it again via catch.  I think that will lead
> > to sending a stop message twice.  Won't that be a problem?  See the
> > usage of iterstate in the catch block, we have made it safe from a
> > similar problem.
>
> IMHO, we don't need that, because we only call stream_stop in the
> catch block if the error type is ERRCODE_TRANSACTION_ROLLBACK.  So if
> in TRY block we have already stopped the stream then we should not get
> that error.  I have added the comments for the same.
>

I am still slightly nervous about it as I don't see any solid
guarantee for the same.  You are right as the code stands today but
due to any code that gets added in the future, it might not remain
true. I feel it is better to have an Assert here to ensure that
stream_stop won't be called the second time.  I don't see any good way
of doing it other than by maintaining flag or some state but I think
it will be good to ensure this.

>
> > 6.
> > PG_CATCH();
> >   {
> > + MemoryContext ecxt = MemoryContextSwitchTo(ccxt);
> > + ErrorData  *errdata = CopyErrorData();
> >
> > I don't understand the usage of memory context in this part of the
> > code.  Basically, you are switching to CurrentMemoryContext here, do
> > some error handling and then again reset back to some random context
> > before rethrowing the error.  If there is some purpose for it, then it
> > might be better if you can write a few comments to explain the same.
>
> Basically, the ccxt is the CurrentMemoryContext when we started the
> streaming and ecxt it the context when we catch the error.  So
> ideally, before this change, it will rethrow in the context when we
> catch the error i.e. ecxt.  So what we are trying to do is put it back
> to normal context (ccxt) and copy the error data in the normal
> context.  And, if we are not handling it gracefully then put it back
> to the context it was in, and rethrow.
>

Okay, but when errorcode is *not* ERRCODE_TRANSACTION_ROLLBACK, don't
we need to clean up the reorderbuffer by calling
ReorderBufferCleanupTXN?  If so, then you can try to combine it with
the not-streaming else loop.

>
> > 8.
> > @@ -2295,6 +2677,13 @@ ReorderBufferXidSetCatalogChanges(ReorderBuffer
> > *rb, TransactionId xid,
> >   txn = ReorderBufferTXNByXid(rb, xid, true, NULL, lsn, true);
> >
> >   txn->txn_flags |= RBTXN_HAS_CATALOG_CHANGES;
> > +
> > + /*
> > + * TOCHECK: Mark toplevel transaction as having catalog changes too
> > + * if one of its children has.
> > + */
> > + if (txn->toptxn != NULL)
> > + txn->toptxn->txn_flags |= RBTXN_HAS_CATALOG_CHANGES;
> >  }
> >
> > Why are we marking top transaction here?
>
> We need to mark top transaction to decide whether to build tuplecid
> hash or not.  In non-streaming mode, we are only sending during the
> commit time, and during commit time we know whether the top
> transaction has any catalog changes or not based on the invalidation
> message so we are marking the top transaction there in DecodeCommit.
> Since here we are not waiting till 

Re: [PATCH] Add support to psql for edit-and-execute-command

[Pavel Stehule]

po 18. 5. 2020 v 12:16 odesílatel Joe Wildish  napsal:

> On 18 May 2020, at 11:09, Pavel Stehule wrote:
>
> \e is working with not empty line too.You can check
>
> select 1\e
>
> Your patch just save skip on end line and \e
>
> Personally I think so it is good idea
>
> Thanks. I did not realise that \e at the end of a line would edit that
> line. (although you do need to remove the terminator I notice).
>

it is different method with little bit different usage - both method can be
supported


-Joe
>

Re: [PATCH] Add support to psql for edit-and-execute-command

[Joe Wildish]

On 18 May 2020, at 11:09, Pavel Stehule wrote:



\e is working with not empty line too.You can check

select 1\e

Your patch just save skip on end line and \e

Personally I think so it is good idea


Thanks. I did not realise that \e at the end of a line would edit that 
line. (although you do need to remove the terminator I notice).


-Joe

Re: [PATCH] Add support to psql for edit-and-execute-command

[Pavel Stehule]

po 18. 5. 2020 v 12:05 odesílatel Joe Wildish  napsal:

> On 18 May 2020, at 7:08, Oleksandr Shulgin wrote:
>
> The only difference from \e is that you don't need to jump to the end of
> input first, I guess?
>
> AIUI, \e will edit the last thing in history or a specific line number
> from history, whereas the patch will allow the current line to be edited.
> That is 99% of the time what I want.
>
> My work flow is typically "Run some queries" => "Go back to some recent
> query by searching history, often not the most recent" => "Edit query". To
> do the edit in an editor (without the patch), I've been deliberately
> nobbling the query once found from history. This allows it to execute (and
> fail) but places it as the most recent thing in history. Then I hit "\e".
>

\e is working with not empty line too.You can check

select 1\e

Your patch just save skip on end line and \e

Personally I think so it is good idea

Pavel


-Joe
>

Re: [PATCH] Add support to psql for edit-and-execute-command

[Joe Wildish]

On 18 May 2020, at 7:08, Oleksandr Shulgin wrote:

The only difference from \e is that you don't need to jump to the end 
of

input first, I guess?


AIUI, \e will edit the last thing in history or a specific line number 
from history, whereas the patch will allow the current line to be 
edited. That is 99% of the time what I want.


My work flow is typically "Run some queries" => "Go back to some recent 
query by searching history, often not the most recent" => "Edit query". 
To do the edit in an editor (without the patch), I've been deliberately 
nobbling the query once found from history. This allows it to execute 
(and fail) but places it as the most recent thing in history. Then I hit 
"\e".


-Joe

Re: Optimizer docs typos

[Etsuro Fujita]

On Mon, May 18, 2020 at 6:56 PM Magnus Hagander  wrote:
> On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson  wrote:
>> Attached diff fixes two small typos in the optimizer README.

> Pushed, thanks.

Thank you!

Best regards,
Etsuro Fujita

Re: Optimizer docs typos

[Magnus Hagander]

On Mon, May 18, 2020 at 11:31 AM Daniel Gustafsson  wrote:

> Attached diff fixes two small typos in the optimizer README.
>

Pushed, thanks.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Optimizer docs typos

[Daniel Gustafsson]

Attached diff fixes two small typos in the optimizer README.

cheers ./daniel



optimizer_doc_typos.diff
Description: Binary data

Re: pg_bsd_indent and -Wimplicit-fallthrough

[Julien Rouhaud]

On Sun, May 17, 2020 at 2:32 AM Michael Paquier  wrote:
>
> On Sat, May 16, 2020 at 11:56:28AM -0400, Tom Lane wrote:
> > In the meantime, I went ahead and pushed this to our pg_bsd_indent repo.
>
> Thanks, Tom.

+1, thanks a lot!

Re: PG 13 release notes, first draft

[Daniel Gustafsson]

> On 5 May 2020, at 05:16, Bruce Momjian  wrote:
> 
> I have committed the first draft of the PG 13 release notes.  You can
> see them here:

Spotted a typo we probably should fix: s/PostgresSQL/PostgreSQL/ =)

cheers ./daniel



13relnotes_postgressql.diff
Description: Binary data

Re: Fix a typo in slot.c

[Masahiko Sawada]

On Mon, 18 May 2020 at 13:59, Amit Kapila  wrote:
>
> On Fri, May 15, 2020 at 10:08 AM Masahiko Sawada
>  wrote:
> >
> > On Fri, 15 May 2020 at 13:26, Amit Kapila  wrote:
> > >
> > >
> > >  /*
> > > - * Allocate and initialize walsender-related shared memory.
> > > + * Allocate and initialize replication slots' shared memory.
> > >   */
> > >
> > > How about changing it to "Allocate and initialize shared memory for
> > > replication slots"?
> > >
> >
> > Agreed.
> >
> > Attached the updated patch.
> >
>
> Pushed.

Thank you!

Regards,

-- 
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Find query characters in respect of optimizer for develop purpose

[Andy Fan]

Hello:

Before I want to pay attention to some optimizer features, I want to
estimate how much benefits it can create for customers, at least for our
current
running customer. So I want to have some basic idea what kind of the query
is
running now in respect of optimizer.


My basic is we can track it with the below struct(every backend has one
global
variable to record it).

+typedef struct
+{
+   int subplan_count;
+   int subquery_count;
+   int join_count;
+   bool hasagg;
+   bool hasgroup;
+} QueryCharacters;

it will be reset at the beginning of standard_planner, and the values are
increased at  make_subplan, set_subquery_pathlist, make_one_rel,
create_grouping_paths. later it can be tracked and viewed in
pg_stat_statements.


What do you think about the requirement and the method I am thinking? Any
kind of feedback is welcome.


-- 
Best Regards
Andy Fan

Re: Libpq support to connect to standby server as priority

[Greg Nancarrow]

Hi Hackers,

I'd like to submit a new version of a patch that I'd previously
submitted but was eventually Returned with Feedback (closed in
commitfest 2020-03).
The patch enhances the libpq "target_session_attrs" connection
parameter by supporting primary/standby/prefer-standby, and I've
attempted some sort of alignment with similar PGJDBC driver
functionality by adding a "target_server_type" parameter. Now targets
PG14.
I've merged the original set of 3 patches into one patch and tried to
account for most(?) of the requested changes in the feedback comments;
if nothing else, it should be easier to read and understand.
Previous discussion here:
https://www.postgresql.org/message-id/flat/caf3+xm+8-ztokav9ghij3wfgentq97qcjxqt+rbfq6f7onz...@mail.gmail.com

Regards,
Greg Nancarrow
Fujitsu Australia


v15-0001-Enhance-libpq-target_session_attrs-and-add-target_se.patch
Description: Binary data

Re: [PATCH] Add support to psql for edit-and-execute-command

[Oleksandr Shulgin]

On Mon, May 18, 2020 at 1:30 AM Joe Wildish  wrote:

>
> Attached is a small patch for adding "edit-and-execute-command" readline
> support to psql. Bash has this concept and I miss it when using psql. It
> allows you to amend the current line in an editor by pressing "v" (when
> in vi mode) or "C-x C-e" (when in emacs mode). Those are the default
> bindings from bash although of course they can be amended in inputrc.
>

The only difference from \e is that you don't need to jump to the end of
input first, I guess?

--
Alex