Re: [HACKERS] Pg V10: Patch for bug in bonjour support

[Thomas Munro]

On Thu, Nov 9, 2017 at 6:27 PM, Tom Lane  wrote:
> Thomas Munro  writes:
>> On Thu, Nov 9, 2017 at 5:03 PM, Tom Lane  wrote:
>>> Is the AC_SEARCH_LIBS configure call needed to make PG build with the
>>> FreeBSD package?
>
>> Yes.  My take is that the commit was correct: the library is needed
>> for --with-bonjour to work on non-macOS systems, and apparently it can
>> work (though I didn't personally try to assess that beyond seeing that
>> it could start up and connect to mdnsd).  Perhaps Avahi doesn't
>> qualify as a suitable Bonjour implementation any more though, and
>> someone out there might like to consider writing a --with-avahi option
>> that uses the native API it's shouting about.
>
> I'm not sure what to do at this point.  I concur that the AC_SEARCH_LIBS
> call is helpful if you're using mDNSResponder on FreeBSD (or wherever
> else that may be available) ... but I'm worried that it will enable
> people to create broken builds on Linux without trying very hard.
> We might be wise to deem that putting that call in is just creating
> an attractive nuisance.
>
> This would certainly be easier if we had a certifiably-working interface
> to the avahi library.  But we don't, and I don't plan to write one,
> and I doubt anyone else will come out of the woodwork to do it either.
>
> Is there really much interest in Bonjour support on non-macOS platforms?
> I hadn't heard that anybody but Apple was invested in it.

Not from me.  My only interest here was to pipe up because I knew that
what was originally proposed would have broken stuff on macOS, and
after that piqued curiosity.  I won't mind at all if you revert the
commit to prevent confusion.  If the intersection of FreeBSD,
PostgreSQL and Bonjour users is a non-empty set, [s]he might at least
find this archived discussion useful...

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Thomas Munro  writes:
> On Thu, Nov 9, 2017 at 5:03 PM, Tom Lane  wrote:
>> Is the AC_SEARCH_LIBS configure call needed to make PG build with the
>> FreeBSD package?

> Yes.  My take is that the commit was correct: the library is needed
> for --with-bonjour to work on non-macOS systems, and apparently it can
> work (though I didn't personally try to assess that beyond seeing that
> it could start up and connect to mdnsd).  Perhaps Avahi doesn't
> qualify as a suitable Bonjour implementation any more though, and
> someone out there might like to consider writing a --with-avahi option
> that uses the native API it's shouting about.

I'm not sure what to do at this point.  I concur that the AC_SEARCH_LIBS
call is helpful if you're using mDNSResponder on FreeBSD (or wherever
else that may be available) ... but I'm worried that it will enable
people to create broken builds on Linux without trying very hard.
We might be wise to deem that putting that call in is just creating
an attractive nuisance.

This would certainly be easier if we had a certifiably-working interface
to the avahi library.  But we don't, and I don't plan to write one,
and I doubt anyone else will come out of the woodwork to do it either.

Is there really much interest in Bonjour support on non-macOS platforms?
I hadn't heard that anybody but Apple was invested in it.

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] Pg V10: Patch for bug in bonjour support

[Thomas Munro]

On Thu, Nov 9, 2017 at 5:03 PM, Tom Lane  wrote:
> Is the AC_SEARCH_LIBS configure call needed to make PG build with the
> FreeBSD package?

Yes.  My take is that the commit was correct: the library is needed
for --with-bonjour to work on non-macOS systems, and apparently it can
work (though I didn't personally try to assess that beyond seeing that
it could start up and connect to mdnsd).  Perhaps Avahi doesn't
qualify as a suitable Bonjour implementation any more though, and
someone out there might like to consider writing a --with-avahi option
that uses the native API it's shouting about.

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] [POC] Faster processing at Gather node

[Amit Kapila]

On Wed, Nov 8, 2017 at 1:02 AM, Andres Freund  wrote:
> Hi,
>
> On 2017-11-06 10:56:43 +0530, Amit Kapila wrote:
>> On Sun, Nov 5, 2017 at 6:54 AM, Andres Freund  wrote
>> > On 2017-11-05 01:05:59 +0100, Robert Haas wrote:
>> >> skip-gather-project-v1.patch does what it says on the tin.  I still
>> >> don't have a test case for this, and I didn't find that it helped very
>> >> much,
>>
>> I am also wondering in which case it can help and I can't think of the
>> case.
>
> I'm confused?  Isn't it fairly obvious that unnecessarily projecting
> at the gather node is wasteful? Obviously depending on the query you'll
> see smaller / bigger gains, but that there's beenfdits should be fairly 
> obvious?
>
>

I agree that there could be benefits depending on the statement.  I
initially thought that we are kind of re-evaluating the expressions in
target list as part of projection even if worker backend has already
done that, but that was not the case and instead, we are deforming the
tuples sent by workers.  Now, I think as a general principle it is a
good idea to delay the deforming as much as possible.

About the patch,


  /*
- * Initialize result tuple type and projection info.
- */
- ExecAssignResultTypeFromTL(>ps);
- ExecAssignProjectionInfo(>ps, NULL);
-

- /*
  * Initialize funnel slot to same tuple descriptor as outer plan.
  */
  if (!ExecContextForcesOids(>ps, ))
@@ -115,6 +109,12 @@ ExecInitGather(Gather *node, EState *estate, int eflags)
  tupDesc = ExecTypeFromTL(outerNode->targetlist, hasoid);
  ExecSetSlotDescriptor(gatherstate->funnel_slot, tupDesc);

+ /*
+ * Initialize result tuple type and projection info.
+ */
+ ExecAssignResultTypeFromTL(>ps);
+ ExecConditionalAssignProjectionInfo(>ps, tupDesc, OUTER_VAR);
+

This change looks suspicious to me.  I think here we can't use the
tupDesc constructed from targetlist.  One problem, I could see is that
the check for hasOid setting in tlist_matches_tupdesc won't give the
correct answer.   In case of the scan, we use the tuple descriptor
stored in relation descriptor which will allow us to take the right
decision in tlist_matches_tupdesc.  If you try the statement CREATE
TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind = 'r'; in
force_parallel_mode=regress, then you can reproduce the problem I am
trying to highlight.


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


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

Re: [HACKERS] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Thomas Munro  writes:
> On Thu, Nov 9, 2017 at 1:39 PM, Tom Lane  wrote:
>> Hm, the library on F25 is also avahi's.  Digging in the archives, I find
>> this old thread reporting the same behavior:
>> https://www.postgresql.org/message-id/flat/17824.1252293423%40sss.pgh.pa.us

> FWIW it builds and starts up fine on FreeBSD with
> mDNSResponder-878.1.1 installed (Apache-licensed Apple Bonjour code)
> and the mdnsd daemon started.  If I don't start mdnsd it shows an
> error at startup.  When built against the (conflicting)
> avahi-libdns-0.6.31_2 package it shows the WARNING you reported and
> "DNSServiceRegister() failed: error code -65537", which might just
> mean it wants to talk to some daemon I'm not running.

Interesting.  Fedora doesn't seem to package mDNSResponder as such ---
"dnf search mDNSResponder" just returns more pointers to avahi:

avahi-compat-libdns_sd.x86_64 : Libraries for Apple Bonjour mDNSResponder
  : compatibility
avahi-compat-libdns_sd.i686 : Libraries for Apple Bonjour mDNSResponder
: compatibility
avahi-compat-libdns_sd-devel.x86_64 : Header files for the Apple Bonjour
: mDNSResponder compatibility libraries
avahi-compat-libdns_sd-devel.i686 : Header files for the Apple Bonjour
  : mDNSResponder compatibility libraries

Is the AC_SEARCH_LIBS configure call needed to make PG build with the
FreeBSD package?

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] UPDATE of partition key

[Thomas Munro]

On Wed, Nov 8, 2017 at 5:57 PM, Amit Khandekar  wrote:
> On 8 November 2017 at 07:55, Thomas Munro  
> wrote:
>> On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas  wrote:
>>> The changes to trigger.c still make me super-nervous.  Hey THOMAS
>>> MUNRO, any chance you could review that part?

At first, it seemed quite strange to me that row triggers and
statement triggers fire different events for the same modification.
Row triggers see DELETE +  INSERT (necessarily because different
tables are involved), but this fact is hidden from the target table's
statement triggers.

The alternative would be for all triggers to see consistent events and
transitions.  Instead of having your special case code in ExecInsert
and ExecDelete that creates the two halves of a 'synthetic' UPDATE for
the transition tables, you'd just let the existing ExecInsert and
ExecDelete code do its thing, and you'd need a flag to record that you
should also fire INSERT/DELETE after statement triggers if any rows
moved.

After sleeping on this question, I am coming around to the view that
the way you have it is right.  The distinction isn't really between
row triggers and statement triggers, it's between triggers at
different levels in the hierarchy.  It just so happens that we
currently only fire target table statement triggers and leaf table row
triggers.  Future development ideas that seem consistent with your
choice:

1.  If we ever allow row triggers with transition tables on child
tables, then I think *their* transition tables should certainly see
the deletes and inserts, otherwise OLD TABLE and NEW TABLE would be
inconsistent with the OLD and NEW variables in a single trigger
invocation.  (These were prohibited mainly due to lack of time and
(AFAIK) limited usefulness; I think they would need probably need
their own separate tuplestores, or possibly some kind of filtering.)

2.  If we ever allow row triggers on partitioned tables (ie that fire
when its children are modified), then I think their UPDATE trigger
should probably fire when a row moves between any two (grand-)*child
tables, just as you have it for target table statement triggers.  It
doesn't matter that the view from parent tables' triggers is
inconsistent with the view from leaf table triggers: it's a feature
that we 'hide' partitioning from the user to the extent we can so that
you can treat the partitioned table just like a table.

Any other views?

As for the code, I haven't figured out how to break it yet, and I'm
wondering if there is some way to refactor so that ExecInsert and
ExecDelete don't have to record pseudo-UPDATE trigger events.

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] [PATCH] Overestimated filter cost and its mitigation

[Ashutosh Bapat]

On Wed, Nov 8, 2017 at 3:18 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> I think it would be a good idea, as Thomas says, to order the qual
>> clauses at an earlier stage and then remember our decision.  However,
>> we have to think about whether that's going to increase planning time
>> in a noticeable way.  I wonder why we currently postpone this until
>> such a late phase of planning.
>
> Because (1) up to now there's been no need to consider the qual ordering
> till later, and (2) re-doing that sort for each path seemed unduly
> expensive.  If we're to try to estimate whether later quals will be
> reached, then sure the ordering becomes important.  I'm still concerned
> about (2) though.  If there were a way to pre-sort the quals once for
> all paths, the problem goes away, but I don't think that works ---
> indexscans may segregate some quals, and in join cases different paths
> will actually have different sets of quals they need to check depending
> on the join order.
>

Looking at order_qual_clauses(), we can say that a set of quals q1
 qn are ordered the same irrespective of the set of clauses they
are subset of. E.g. if {q1 .. qn} is subset of Q (ordered as Qo) and
also Q' (ordered as Q'o) the order in which they appear in Qo and Q'o
is same. So, even if different paths segregate the clauses in
different set, within each set the order is same. FWIW, we can order
all clauses in largest set once and use that order every time. Albeit
we will have to remember the order somewhere OR make the separator
routine retain the order in the larger set, which I guess is true
about all separator functions.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


-- 
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] Hang in pldebugger after git commit : 98a64d0

[Andres Freund]

On 2016-12-16 23:04:13 -0500, Robert Haas wrote:
> >>  BTW, I suggest this rewritten comment:
> >>
> >> /*--
> >>  * FD_READ events are edge-triggered on Windows per
> >>  * 
> >> https://msdn.microsoft.com/en-us/library/windows/desktop/ms741576(v=vs.85).aspx
> >>
> >
> > Isn't the statement in above doc. "For FD_READ, FD_OOB, and FD_ACCEPT
> > network events, network event recording and event object signaling are
> > level-triggered." says that FD_READ events are level-triggered which
> > seems to be contradictory with above comment?
> 
> Argh.  I see your point.  Maybe we'd better rephrase that.  The
> document does say that, but the behavior they described is actually a
> weird hybrid of level-triggered and edge-triggered.  We should
> probably just avoid that terminology altogether and explain that
> read-ready won't necessarily be re-signalled unless there is an
> intervening recv().

(just looked at some latch code, reminding me of this)

FWIW, I wonder if the issue here isn't whether WaitForMultipleObjects /
WSAEventSelect is level triggered, but that WSAEnumNetworkEvents()
pretty much explicitly is not. And it seems to have effects over
multiple handles for the same socket.

The relevant line from the docs is: "The WSAEnumNetworkEvents function
discovers occurrences of network events for the indicated socket, clear
internal network event records, and reset event objects (optional)."

Note that it clears the state from the socket, *not* just the handle.


That behaviour of WSAEnumNetworkEvents() also seems to explains why
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f7819baa618c528f60e266874051563ecfe08207
was necessary, and why all the weird workaround in win32/socket.c exist.

Greetings,

Andres Freund


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

Re: [HACKERS] [PATCH] Assert that the correct locks are held when calling PageGetLSN()

[Michael Paquier]

On Thu, Nov 9, 2017 at 8:25 AM, Asim Praveen  wrote:
> Indeed, the assertion tripped during WAL replay on the standby.  This was
> caught by TAP tests under src/test/recovery.  The assertion is now fixed so
> that WAL replay is exempt from the check.  Please find the new patch
> attached.  The tests now pass with the fix.  I also manually verified that
> recovery works with "wal_consistency_checking=all".

I still have a bad feeling about that bit... Still, it does not change
the fact that patch 0001 in
https://www.postgresql.org/message-id/CANXE4TccH_VjdKaHc9=KyH0Y7WORqZN+=mH5f=mp0bw3gzx...@mail.gmail.com
needs a committer per the fact that it visibly fixes incorrect backend
code and API contract. So I am switching the CF entry to ready for
committer, but only for 0001.

The other things could always be taken care of later.
-- 
Michael


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

Re: [HACKERS] [PATCH] A hook for session start

[Michael Paquier]

On Thu, Nov 9, 2017 at 2:42 AM, Fabrízio de Royes Mello
 wrote:
> On Wed, Nov 8, 2017 at 12:47 AM, Michael Paquier 
> wrote:
>> - Let's restrict the logging to a role name instead of a database
>> name, and let's parametrize it with a setting in the temporary
>> configuration file. Let's not bother about multiple role support with
>> a list, for the sake of tests and simplicity only defining one role
>> looks fine to me. Comments in the code should be clear about the
>> dependency.
>
> Makes sense and simplify the test code. Fixed.

+   if (!strcmp(username, "regress_sess_hook_usr2"))
+   {
+   const char *dbname;
[...]
+++ b/src/test/modules/test_session_hooks/session_hooks.conf
@@ -0,0 +1 @@
+shared_preload_libraries = 'test_session_hooks'
Don't you think that this should be a GUC? My previous comment
outlined that. I won't fight hard on that point in any case, don't
worry. I just want to make things clear :)
-- 
Michael


-- 
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] Simplify ACL handling for large objects and removal of superuser() checks

[Michael Paquier]

On Thu, Nov 9, 2017 at 6:05 AM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Tue, Sep 26, 2017 at 11:42 AM, Vaishnavi Prabakaran
>>  wrote:
>>> I moved the cf entry to "ready for committer", and though my vote is for
>>> keeping the existing API behavior with write implying read, I let the
>>> committer decide whether the following behavior change is Ok or not.
>>> "Reading from a large-object descriptor opened with INV_WRITE is NOT
>>> possible"
>
>> Thanks for the review!
>
> After chewing on this some more, I'm inclined to agree that we should
> not change the behavior of the read/write flags.  There's been no
> field requests for a true-write-only mode, so I think we're much more
> likely to get complaints from users whose code we broke than plaudits
> from people who think the change is helpful.

Thanks for the input. Then that's two people favoring no changes.

> I believe it would be easy enough to adjust the patch so that we can
> still have the refactoring benefits; we'd just need the bit that
> translates from external to internal flags to go more like
>
> if (flags & INV_WRITE)
> descflags |= IFS_WRLOCK | IFS_RDLOCK;
> if (flags & INV_READ)
> descflags |= IFS_RDLOCK;
>
> (Preferably with a comment about why it's like this.)

Sure, I have updated the patch with this comment:
+   /*
+* Historically, no difference is made between (INV_WRITE) and
+* (INV_WRITE | INV_READ), the caller being allowed to read the large
+* object descriptor in either case.
+*/
Of course please feel free to reword if you find something better-suited.

> Another idea would be to invent a new external flag bit "INV_WRITE_ONLY",
> so that people who wanted true write-only could get it, without breaking
> backwards-compatible behavior.  But I'm inclined to wait for some field
> demand to show up before adding even that little bit of complication.

Demand that may never show up, and the current behavior on HEAD does
not receive any complains either. I am keeping the patch simple for
now. That's less aspirin needed for everybody.
-- 
Michael


0003-Move-ACL-checks-for-large-objects-when-opening-them.patch
Description: Binary data


0001-Remove-ALLOW_DANGEROUS_LO_FUNCTIONS-for-LO-related-s.patch
Description: Binary data


0002-Replace-superuser-checks-of-large-object-import-expo.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] [PATCH] fix wrong create table statement in documentation

[Amit Langote]

On 2017/11/09 7:21, Tom Lane wrote:
> jotpe  writes:
>> In the current documentation [1] this create table statement is listed:
>> CREATE TABLE measurement_y2008m01 PARTITION OF measurement
>>  FOR VALUES FROM ('2008-01-01') TO ('2008-02-01')
>>  TABLESPACE fasttablespace
>>  WITH (parallel_workers = 4);
> 
> Yup, that's wrong.  Fix pushed, thanks!

Oops!  Thanks Tom for the fix.

Regards,
Amit



-- 
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] Pg V10: Patch for bug in bonjour support

[Thomas Munro]

On Thu, Nov 9, 2017 at 1:39 PM, Tom Lane  wrote:
> Luke Lonergan  writes:
>> On 11/8/17, 3:00 PM, "Tom Lane"  wrote:
>>> BTW, when I try this on Fedora 25, it builds cleanly but the feature
>>> doesn't seem to work --- I get this at postmaster start:
>>> ...
>>> I wonder which libdns_sd you are using.
>
>> libavahi-compat-libdnssd1:amd64: /usr/lib/x86_64-linux-gnu/libdns_sd.so.1.0.0
>
> Hm, the library on F25 is also avahi's.  Digging in the archives, I find
> this old thread reporting the same behavior:
>
> https://www.postgresql.org/message-id/flat/17824.1252293423%40sss.pgh.pa.us
>
> So now I'm wondering if you know something the rest of us don't about
> how to configure the platform for bonjour to work.

FWIW it builds and starts up fine on FreeBSD with
mDNSResponder-878.1.1 installed (Apache-licensed Apple Bonjour code)
and the mdnsd daemon started.  If I don't start mdnsd it shows an
error at startup.  When built against the (conflicting)
avahi-libdns-0.6.31_2 package it shows the WARNING you reported and
"DNSServiceRegister() failed: error code -65537", which might just
mean it wants to talk to some daemon I'm not running.

-- 
Thomas Munro
http://www.enterprisedb.com


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

Re: [HACKERS] Pg V10: Patch for bug in bonjour support

[Luke Lonergan]

Hey Tom,

On 11/8/17, 4:39 PM, "Tom Lane"  wrote:

So now I'm wondering if you know something the rest of us don't about
how to configure the platform for bonjour to work.

Nope – in fact, I hadn’t tried to use Bonjour on this instance, but had only 
enabled it thinking I’d want it within my network to be discoverable via 
Bonjour…

Given these issues I’d recommend disabling the functionality for non-Mac until 
someone can get a workable solution in place – then at least it will avoid 
people like me trying to enable a broken feature and failing…

- Luke




-- 
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] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Luke Lonergan  writes:
> On 11/8/17, 3:00 PM, "Tom Lane"  wrote:
>> BTW, when I try this on Fedora 25, it builds cleanly but the feature
>> doesn't seem to work --- I get this at postmaster start:
>> ...
>> I wonder which libdns_sd you are using.

> libavahi-compat-libdnssd1:amd64: /usr/lib/x86_64-linux-gnu/libdns_sd.so.1.0.0

Hm, the library on F25 is also avahi's.  Digging in the archives, I find
this old thread reporting the same behavior:

https://www.postgresql.org/message-id/flat/17824.1252293423%40sss.pgh.pa.us

So now I'm wondering if you know something the rest of us don't about
how to configure the platform for bonjour to work.

I'm also a bit disturbed about the report that libdns_sd was causing
the postmaster to become multithreaded.  If still true, that's quite
bad, and might be a reason to decide we don't want this change after
all.

regards, tom lane


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

[HACKERS] postgresql v9.5 and SSL: LOG: could not accept SSL connection: tlsv1 alert unknown ca

[Graham Leggett]

Hi all,

I have a working postgresql v9.3 installation running on out-of-the-box Ubuntu 
Trusty, and it works fine. The job at hand: replace the server with postgresql 
v9.5 on out-of-the-box Ubuntu Xenial, but this does not work fine.

I am getting the problem described on this page: http://www.pontifier.com/?p=23

2017-11-08 22:43:39 UTC [2553-1] [unknown]@[unknown] LOG:  could not accept SSL 
connection: tlsv1 alert unknown ca

To start with, the certs on the postgresql server validate without a problem, 
they are signed with SHA265:

root@sql01:/var/lib/postgresql/9.5/main# openssl verify -CAfile root.crt 
server.crt
server.crt: OK

The server.crt contains a cert signed by two intermediates, in turn signed by 
the root.

The postgresql server has an ssl configuration as follows:

ssl = true  # (change requires restart)
ssl_cert_file = '/var/lib/postgresql/9.5/main/server.crt'   # 
(change requires restart)
ssl_key_file = '/var/lib/postgresql/9.5/main/server.key'# 
(change requires restart)
ssl_ca_file = '/var/lib/postgresql/9.5/main/root.crt'
ssl_crl_file = '/var/lib/postgresql/9.5/main/root.crl'

If I place bogus values in ssl_cert_file postgresql complains as expected. If I 
place what I believe to be valid values, postgresql is silent on the issue in 
the log files.

First question - apart from the quoted message in the logfile, the logfile is 
completely silent on the state of SSL. Is there some kind of debug option that 
will tell me a) what certs/keys/ca certs/crls have been picked up, and b) 
whether these have been validated by postgresql as functional? Obviously I can 
(and have) run the certs through openssl, but that tells me openssl is happy, 
not that postgresql is happy.

Digging deeper, I’m trying the pg_isready tool to test if the server is ready. 
Unfortunately this gives inconsistent results:

postgres@sql02:~$ /usr/bin/pg_isready -t 0 -d 
'postgresql://sql01:5432?user=repmgr=verify-ca'
sql01:5432 - no response

postgres@sql02:~$ /usr/bin/psql -d 
'postgresql://sql01:5432?user=repmgr=verify-ca'
psql: SSL error: certificate verify failed

In the pg_isready case, the error is discarded and replaced with the inaccurate 
message “no response”. In the psql case, the error is too vague to be useful - 
it tells us a certificate verification failed, but didn’t tell us what 
specifically failed about the verification.

Sniffing the connection with ssldump gives us the following:

New TCP connection #8: 172.29.231.43(60178) <-> 172.29.228.240(5432)
8 1  0.0039 (0.0039)  C>S  Handshake
  ClientHello
Version 3.3 
cipher suites
Unknown value 0xc030
Unknown value 0xc02c
Unknown value 0xc028
Unknown value 0xc024
Unknown value 0xc014
Unknown value 0xc00a
Unknown value 0xa5
Unknown value 0xa3
Unknown value 0xa1
Unknown value 0x9f
Unknown value 0x6b
Unknown value 0x6a
Unknown value 0x69
Unknown value 0x68
TLS_DHE_RSA_WITH_AES_256_CBC_SHA
TLS_DHE_DSS_WITH_AES_256_CBC_SHA
TLS_DH_RSA_WITH_AES_256_CBC_SHA
TLS_DH_DSS_WITH_AES_256_CBC_SHA
Unknown value 0x88
Unknown value 0x87
Unknown value 0x86
Unknown value 0x85
Unknown value 0xc032
Unknown value 0xc02e
Unknown value 0xc02a
Unknown value 0xc026
Unknown value 0xc00f
Unknown value 0xc005
Unknown value 0x9d
Unknown value 0x3d
TLS_RSA_WITH_AES_256_CBC_SHA
Unknown value 0x84
Unknown value 0xc02f
Unknown value 0xc02b
Unknown value 0xc027
Unknown value 0xc023
Unknown value 0xc013
Unknown value 0xc009
Unknown value 0xa4
Unknown value 0xa2
Unknown value 0xa0
Unknown value 0x9e
TLS_DHE_DSS_WITH_NULL_SHA
Unknown value 0x40
Unknown value 0x3f
Unknown value 0x3e
TLS_DHE_RSA_WITH_AES_128_CBC_SHA
TLS_DHE_DSS_WITH_AES_128_CBC_SHA
TLS_DH_RSA_WITH_AES_128_CBC_SHA
TLS_DH_DSS_WITH_AES_128_CBC_SHA
Unknown value 0x9a
Unknown value 0x99
Unknown value 0x98
Unknown value 0x97
Unknown value 0x45
Unknown value 0x44
Unknown value 0x43
Unknown value 0x42
Unknown value 0xc031
Unknown value 0xc02d
Unknown value 0xc029
Unknown value 0xc025
Unknown value 0xc00e
Unknown value 0xc004
Unknown value 0x9c
Unknown value 0x3c
TLS_RSA_WITH_AES_128_CBC_SHA
Unknown value 0x96
Unknown value 0x41
Unknown value 0xc011
Unknown value 0xc007
Unknown value 0xc00c
Unknown value 0xc002
TLS_RSA_WITH_RC4_128_SHA
TLS_RSA_WITH_RC4_128_MD5
Unknown value 0xc012
Unknown value 0xc008
TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA

Re: [HACKERS] Transaction control in procedures

[Simon Riggs]

On 31 October 2017 at 15:38, Peter Eisentraut
 wrote:
> Here is a patch that implements transaction control in PL/Python
> procedures.  (This patch goes on top of "SQL procedures" patch v1.)

The patch is incredibly short for such a feature, which is probably a
good indication that it is feasible.

Amazing!

> So you can do this:
>
> CREATE PROCEDURE transaction_test1()
> LANGUAGE plpythonu
> AS $$
> for i in range(0, 10):
> plpy.execute("INSERT INTO test1 (a) VALUES (%d)" % i)
> if i % 2 == 0:
>  plpy.commit()
> else:
>  plpy.rollback()
> $$;
>
> CALL transaction_test1();
>
> I started with PL/Python because the internal structures there are more
> manageable.

AFAICS we execute 10 INSERTs and each one runs in a new top-level xid.
The INSERTs succeed in all cases but we then abort odd-numbered ones
and commit even numbered ones.

What would happen if some of the INSERTs failed? Where would control
go to? (Maybe this is just "no change" in this particular proc)

What happens if the procedure is updated during execution? Presumably
it keeps executing the original version as seen in the initial
snapshot?

Does the xmin of this session advance after each transaction, or do we
hold the snapshot used for the procedure body open, causing us to hold
back xmin and prevent vacuuming from being effective?

What would happen if a procedure recursively called itself? And yet it
was updated half-way through? Would that throw an error (I think it
should).

> Obviously, people will want this for PL/pgSQL as well, and
> I have that in the works.  It's not in a usable state, but I have found
> that the work needed is essentially the same as in PL/Python for example.
>
> I have discovered three groups of obstacles that needed addressing to
> make this work.  At this point, the patch is more of a demo of what
> these issues are, and if we come to satisfactory solutions for each of
> them, things should fall into place more easily afterwards.
>
> 1) While calling CommitTransactionCommand() in the middle of a utility
> command works just fine (several utility commands do this, of course),
> calling AbortCurrentTransaction() in a similar way does not.  There are
> a few pieces of code that think that a transaction abort will always
> result in a return to the main control loop, and so they will just clean
> away everything.  This is what the changes in portalmem.c are about.
> Some comments there already hint about the issue.  No doubt this will
> need further refinement.  I think it would be desirable in general to
> separate the control flow concerns from the transaction management
> concerns more cleanly.

+1

> 2) SPI needs some work.  It thinks that it can clean everything away at
> transaction end.  I have found that instead of TopTransactionContext one
> can use PortalContext and get a more suitable life cycle for the memory.
>  I have played with some variants to make this configurable (e.g.,
> argument to SPI_connect()), but that didn't seem very useful.  There are
> some comments indicating that there might not always be a PortalContext,
> but the existing tests don't seem to mind.  (There was a thread recently
> about making a fake PortalContext for autovacuum, so maybe the current
> idea is that we make sure there always is a PortalContext.)  Maybe we
> need another context like StatementContext or ProcedureContext.
>
> There also needs to be a way via SPI to end transactions and allowing
> *some* cleanup to happen but leaving the memory around.  I have done
> that via additional SPI API functions like SPI_commit(), which are then
> available to PL implementations.  I also tried making it possible
> calling transaction statements directly via SPI_exec() or similar, but
> that ended up a total disaster.  So from the API perspective, I like the
> current implementation, but the details will no doubt need refinement.
>
> 3) The PL implementations themselves allocate memory in
> transaction-bound contexts for convenience as well.  This is usually
> easy to fix by switching to PortalContext as well.  As you see, the
> PL/Python code part of the patch is actually very small.  Changes in
> other PLs would be similar.

Is there some kind of interlock to prevent dropping the portal half way thru?

Will the SPI transaction control functions fail if called from a
normal function?

> Two issues have not been addressed yet:
>
> A) It would be desirable to be able to run commands such as VACUUM and
> CREATE INDEX CONCURRENTLY in a procedure.  This needs a bit of thinking
> and standards-lawyering about the semantics, like where exactly do
> transactions begin and end in various combinations.  It will probably
> also need a different entry point into SPI, because SPI_exec cannot
> handle statements ending transactions.  But so far my assessment is that
> this can be added in a mostly independent way later on.

Sounds like a separate commit, but perhaps it influences 

Re: [HACKERS] [PATCH] Assert that the correct locks are held when calling PageGetLSN()

[Asim Praveen]

Hi Michael

On Mon, Nov 6, 2017 at 6:18 PM, Michael Paquier 
wrote:

>
> Did you really test WAL replay? This still ignores that PageGetLSN is
> as well taken in some code paths, like recovery, where actions on the
> page are guaranteed to be serialized, like during recovery, so this
> patch would cause the system to blow up. Note that pageinspect,
> amcheck and wal_consistency_checking also process on page copies. So
> the assertion failure of 0002 would trigger in those cases.
>

Indeed, the assertion tripped during WAL replay on the standby.  This was
caught by TAP tests under src/test/recovery.  The assertion is now fixed so
that WAL replay is exempt from the check.  Please find the new patch
attached.  The tests now pass with the fix.  I also manually verified that
recovery works with "wal_consistency_checking=all".

Asim


0002-PageGetLSN-assert-that-locks-are-properly-held.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] taking stdbool.h into use

[Michael Paquier]

On Thu, Nov 9, 2017 at 1:46 AM, Peter Eisentraut
 wrote:
> On 10/29/17 08:50, Michael Paquier wrote:
>> I spotted a couple of other things while looking at your patches and
>> the code tree.
>>
>> -   return (ginCompareItemPointers(>itemptr, iptr) > 0) ? TRUE : 
>> FALSE;
>> +   return (ginCompareItemPointers(>itemptr, iptr) > 0) ? true : 
>> false;
>> You could simplify that at the same time by removing such things. The
>> "false : true" pattern is less frequent than the "true : false"
>> pattern.
>
> I have found many more instances like that.  It might be worth
> simplifying a bit, but that sounds like a separate undertaking.

Yeah, I just mentioned one for reference. And I can see 66 instances.
It may be not worth changing either to simplify back-patching.

>> Some comments in the code still mention FALSE or TRUE:
>> - hashsearch.c uses FALSE in some comments.
>> - In execExpr.c, ExecCheck mentions TRUE.
>
> That one is an SQL TRUE, so I left it.

Oops. You are right. I tried to be careful with what was referring to SQL and C.
-- 
Michael


-- 
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] Pg V10: Patch for bug in bonjour support

[Luke Lonergan]

# dpkg -S !$

dpkg -S /usr/lib/x86_64-linux-gnu/libdns_sd.so.1.0.0

libavahi-compat-libdnssd1:amd64: /usr/lib/x86_64-linux-gnu/libdns_sd.so.1.0.0

 

Also:
  ii  libavahi-compat-libdnssd1:amd640.6.32-1ubuntu1
amd64Avahi Apple Bonjour compatibility library

 

- Luke

 

On 11/8/17, 3:00 PM, "Tom Lane"  wrote:


BTW, when I try this on Fedora 25, it builds cleanly but the feature

    doesn't seem to work --- I get this at postmaster start:

I wonder which libdns_sd you are using.

 

Re: [HACKERS] Pg V10: Patch for bug in bonjour support

[Luke Lonergan]

Hi Tom – works for me on Linux (Ubuntu)…

- Luke

*** /home/llonergan/archive/configure.in2017-11-08 14:17:56.804891827 
-0800
--- configure.in2017-11-08 14:15:58.961186149 -0800
***
*** 1293,1298 
--- 1293,1299 
  
  if test "$with_bonjour" = yes ; then
AC_CHECK_HEADER(dns_sd.h, [], [AC_MSG_ERROR([header file  is 
required for Bonjour])])
+   AC_SEARCH_LIBS([DNSServiceRefSockFD],[dns_sd])
  fi
  
  # for contrib/uuid-ossp





-- 
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] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Luke Lonergan  writes:
> Hi Tom – works for me on Linux (Ubuntu)…

BTW, when I try this on Fedora 25, it builds cleanly but the feature
doesn't seem to work --- I get this at postmaster start:

*** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility 
layer of Avahi.
*** WARNING *** Please fix your application to use the native API of Avahi!
*** WARNING *** For more information see 

2017-11-08 17:58:42.451 EST [23762] LOG:  DNSServiceRegister() failed: error 
code -65540

I wonder which libdns_sd you are using.

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] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Luke Lonergan  writes:
> Hi Tom – works for me on Linux (Ubuntu)…
> +   AC_SEARCH_LIBS([DNSServiceRefSockFD],[dns_sd])

Pushed with an error message added.  I also took the trouble to
standardize the syntax of our various AC_SEARCH_LIBS calls ---
they weren't very consistent about quoting.

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] Exclude pg_internal.init from base backup

[Michael Paquier]

On Thu, Nov 9, 2017 at 1:03 AM, Peter Eisentraut
 wrote:
> On 11/7/17 19:58, Michael Paquier wrote:
>> On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi  
>> wrote:
>>> Thanks for the correction. I was not much aware of SGML markup usage.
>>> While building the documentation, it raises an warning message of "empty
>>> end-tag".
>>> So I just added the end tag. Attached the update patch with the suggested
>>> correction.
>>
>> Ah, I can see the warning as well. Using empty tags is forbidden since
>> c29c5789, which is really recent. Sorry for missing it. Simon got
>> trapped by that as well visibly. Your patch looks good to me.
>
> fixed

Thanks.
-- 
Michael


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

Re: [HACKERS] [PATCH] fix wrong create table statement in documentation

[Tom Lane]

jotpe  writes:
> In the current documentation [1] this create table statement is listed:
> CREATE TABLE measurement_y2008m01 PARTITION OF measurement
>  FOR VALUES FROM ('2008-01-01') TO ('2008-02-01')
>  TABLESPACE fasttablespace
>  WITH (parallel_workers = 4);

Yup, that's wrong.  Fix pushed, thanks!

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] Small improvement to compactify_tuples

[Peter Geoghegan]

On Wed, Nov 8, 2017 at 12:59 PM, Andres Freund  wrote:
> I complained about multiple related things, I'm not exactly sure what
> exactly you're referring to here:
> - The fact that HeapTupleHeaderData's are commonly iterated over in
>   reverse order is bad for performance. For shared buffers resident
>   workloads involving seqscans that yields 15-25% slowdowns for me. It's
>   trivial to fix that by just changing iteration order, but that
>   obviously changes results. But we could reorder the page during heap
>   pruning.

FWIW, the classic page layout (the one that appears in Gray's
Transaction Processing Systems, at any rate) has the ItemId array at
the end of the page and the tuples at the start (immediately after a
generic page header) -- it's the other way around.

I think that that has its pros and cons.

> - The layout of items in index pages is suboptimal. We regularly do
>   binary searches over the the linearly ordered items, which is cache
>   inefficient. So instead we should sort items as [1/2, 1/4, 3/4, ...]
>   elements, which will access items in a close-ish to linear manner.

I still think that we can repurpose each ItemId's lp_len as an
abbreviated key in internal index pages [1], and always get IndexTuple
size through the index tuple header. I actual got as far as writing a
very rough prototype of that. That's obviously a significant project,
but it seems doable.

[1] 
https://www.postgresql.org/message-id/CAH2-Wz=mv4dmoapficrsyntv2kinxeotbwuy5r7fxxoc-oe...@mail.gmail.com
-- 
Peter Geoghegan


-- 
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] Early locking option to parallel backup

[Lucas B]

On 11/06/2017 12:30 PM, Stephen Frost wrote:

* Lucas (luca...@gmail.com) wrote:

pg_dump was taking more than 24 hours to complete in one of my databases. I
begin to research alternatives. Parallel backup reduced the backup time to
little less than a hour, but it failed almost every time because of
concurrent queries that generated exclusive locks. It is difficult to
guarantee that my applications will not issue queries such as drop table,
alter table, truncate table, create index or drop index for a hour. And I
prefer not to create controls mechanisms to that end if I can work around
it.

I certainly understand the value of pg_dump-based backups, but have you
considered doing file-based backups?  That would avoid the need to do
any in-database locking at all, and would give you the ability to do
PITR too.  Further, you could actually restore that backup to another
system and then do a pg_dump there to get a logical representation (and
this would test your physical database backup/restore process too...).
Yes, a point in time recovery has the advantage of keeping the backup 
more up-to-date, but has the disadvantage of being more expensive and 
complex. In my case, point in time recovery would require an upgrade of 
10 TB of storage space and my stakeholders did not approved this 
investment yet.


I suspect that there is lots of users that uses pg_dump as primary 
backup tool and that they would benefit of a more reliable parallel backup.









--
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] Pg V10: Patch for bug in bonjour support

[Tom Lane]

Thomas Munro  writes:
> On Thu, Nov 9, 2017 at 10:05 AM, Luke Lonergan  wrote:
>> +   AC_CHECK_LIB(dns_sd, DNSServiceRefSockFD, [], [AC_MSG_ERROR([library
>> 'dns_sd' is required for Bonjour])])

> It lives in libSystem.dylib (implicitly linked) on macOS, so that
> would break the build there.  We'd need something a bit more
> conditional, but I don't know what.

A quick look at the Autoconf manual finds:

 `AC_CHECK_LIB' requires some care in usage, and should be avoided
 in some common cases.  Many standard functions like `gethostbyname'
 appear in the standard C library on some hosts, and in special
 libraries like `nsl' on other hosts.  On some hosts the special
 libraries contain variant implementations that you may not want to
 use.  These days it is normally better to use
 `AC_SEARCH_LIBS([gethostbyname], [nsl])' instead of
 `AC_CHECK_LIB([nsl], [gethostbyname])'.

If Luke wants to check that that works for him, I can check it on
macOS.

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] OpenTemporaryFile() vs resowner.c

[Tom Lane]

Thomas Munro  writes:
> Andres, Robert and Peter G rightly complained[1] that my shared
> temporary file patch opens a file, then calls
> ResourceOwnerEnlargeFiles() which can fail due to lack of memory, and
> then registers the file handle to make sure we don't leak it.  Doh.
> The whole point of the separate ResourceOwnerEnlargeXXX() interface is
> to be able to put it before resource acquisition.

> The existing OpenTemporaryFile() coding has the same mistake.  Please
> see attached.

Pushed.  I grepped for related problems and found that IncrBufferRefCount
was also living dangerously, though in a different way: it remembered
a refcount it hadn't actually applied yet.

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] Pg V10: Patch for bug in bonjour support

[Thomas Munro]

On Thu, Nov 9, 2017 at 10:05 AM, Luke Lonergan  wrote:
>   if test "$with_bonjour" = yes ; then
>
> AC_CHECK_HEADER(dns_sd.h, [], [AC_MSG_ERROR([header file  is
> required for Bonjour])])
>
> +   AC_CHECK_LIB(dns_sd, DNSServiceRefSockFD, [], [AC_MSG_ERROR([library
> 'dns_sd' is required for Bonjour])])
>
>   fi

Hi Luke,

It lives in libSystem.dylib (implicitly linked) on macOS, so that
would break the build there.  We'd need something a bit more
conditional, but I don't know what.

-- 
Thomas Munro
http://www.enterprisedb.com


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

[HACKERS] Pg V10: Patch for bug in bonjour support

[Luke Lonergan]

Hi all – I’m doing some geo analysis and was excited to see all the new 
features in V10 – particularly the declarative partitioning support!

 

Found a tiny bug in the build for Bonjour – patch below:

 

*** configure.in  2017-10-02 14:09:15.0 -0700

--- /home/llonergan/archive/configure.in    2017-11-08 12:53:29.522584528 -0800

***

*** 1293,1298 

--- 1293,1299 

  

  if test "$with_bonjour" = yes ; then

AC_CHECK_HEADER(dns_sd.h, [], [AC_MSG_ERROR([header file  is 
required for Bonjour])])

+   AC_CHECK_LIB(dns_sd, DNSServiceRefSockFD, [], [AC_MSG_ERROR([library 
'dns_sd' is required for Bonjour])])

  fi

  

  # for contrib/uuid-ossp

 

- Luke

Re: [HACKERS] Simplify ACL handling for large objects and removal of superuser() checks

[Tom Lane]

Michael Paquier  writes:
> On Tue, Sep 26, 2017 at 11:42 AM, Vaishnavi Prabakaran
>  wrote:
>> I moved the cf entry to "ready for committer", and though my vote is for
>> keeping the existing API behavior with write implying read, I let the
>> committer decide whether the following behavior change is Ok or not.
>> "Reading from a large-object descriptor opened with INV_WRITE is NOT
>> possible"

> Thanks for the review!

After chewing on this some more, I'm inclined to agree that we should
not change the behavior of the read/write flags.  There's been no
field requests for a true-write-only mode, so I think we're much more
likely to get complaints from users whose code we broke than plaudits
from people who think the change is helpful.

I believe it would be easy enough to adjust the patch so that we can
still have the refactoring benefits; we'd just need the bit that
translates from external to internal flags to go more like

if (flags & INV_WRITE)
descflags |= IFS_WRLOCK | IFS_RDLOCK;
if (flags & INV_READ)
descflags |= IFS_RDLOCK;

(Preferably with a comment about why it's like this.)

Another idea would be to invent a new external flag bit "INV_WRITE_ONLY",
so that people who wanted true write-only could get it, without breaking
backwards-compatible behavior.  But I'm inclined to wait for some field
demand to show up before adding even that little bit of complication.

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] Small improvement to compactify_tuples

[Andres Freund]

On 2017-11-08 12:02:40 -0500, Tom Lane wrote:
> BTW, it strikes me that in considering the rebuild-the-page approach,
> we should not have blinders on and just measure the speed of
> PageRepairFragmentation.  Rather, we should take a look at what happens
> subsequently given a physically-ordered set of tuples.  I can recall
> Andres or someone moaning awhile ago about lack of locality of access in
> index page searches --- maybe applying that approach while vacuuming
> indexes will help?

I complained about multiple related things, I'm not exactly sure what
exactly you're referring to here:
- The fact that HeapTupleHeaderData's are commonly iterated over in
  reverse order is bad for performance. For shared buffers resident
  workloads involving seqscans that yields 15-25% slowdowns for me. It's
  trivial to fix that by just changing iteration order, but that
  obviously changes results. But we could reorder the page during heap
  pruning.

  But that's fairly independent of indexes, so I'm not sure whether
  that's what you're referring.

- The layout of items in index pages is suboptimal. We regularly do
  binary searches over the the linearly ordered items, which is cache
  inefficient. So instead we should sort items as [1/2, 1/4, 3/4, ...]
  elements, which will access items in a close-ish to linear manner.

  But that's fairly independent of pruning, so I'm not sure whether
  that's what you're referring to, either.

Greetings,

Andres Freund


-- 
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] Small improvement to compactify_tuples

[Юрий Соколов]

2017-11-08 20:02 GMT+03:00 Tom Lane :
>
> Claudio Freire  writes:
> > What's perhaps not clear is whether there are better ideas. Like
> > rebuilding the page as Tom proposes, which doesn't seem like a bad
> > idea. Bucket sort already is O(bytes), just as memcopy, only it has a
> > lower constant factor (it's bytes/256 in the original patch), which
> > might make copying the whole page an extra time lose against bucket
> > sort in a few cases.
>
> > Deciding that last point does need more benchmarking. That doesn't
> > mean the other improvements can't be pursued in the meanwhile, right?
>
> Well, I doubt we're going to end up committing more than one of these
> ideas.  The question is which way is best.  If people are willing to
> put in the work to test all of them, let's do it.
>
> BTW, it strikes me that in considering the rebuild-the-page approach,
> we should not have blinders on and just measure the speed of
> PageRepairFragmentation.  Rather, we should take a look at what happens
> subsequently given a physically-ordered set of tuples.  I can recall
> Andres or someone moaning awhile ago about lack of locality of access in
> index page searches --- maybe applying that approach while vacuuming
> indexes will help?
>
> regards, tom lane

I'd like to add qsort_template.h as Claudio suggested, ie in a way close to
simplehash.h. With such template header, there will be no need in
qsort_tuple_gen.pl .

With regards,
Sokolov Yura

[HACKERS] [PATCH] fix wrong create table statement in documentation

[jotpe]

In the current documentation [1] this create table statement is listed:

CREATE TABLE measurement_y2008m01 PARTITION OF measurement
FOR VALUES FROM ('2008-01-01') TO ('2008-02-01')
TABLESPACE fasttablespace
WITH (parallel_workers = 4);


But that did not work:

2017-11-06 22:26:11.757 CET [4699] ERROR:  syntax error at or near 
"WITH" at character 130
2017-11-06 22:26:11.757 CET [4699] STATEMENT:  create table 
measurement_y2008m01 partition of measurement for values from 
('2008-01-01') to ('2008-02-01') tablespace fastspace WITH ( 
parallel_workers = 4 );


  ^
The TABLESPACE part should be listed behind the WITH storage parameter.
I attachted the patch.


[1] 
https://www.postgresql.org/docs/10/static/ddl-partitioning.html#ddl-partitioning-declarative
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 03cbaa6..3f41939 100644
*** a/doc/src/sgml/ddl.sgml
--- b/doc/src/sgml/ddl.sgml
***
*** 3095,3102  CREATE TABLE measurement_y2007m12 PARTITION OF measurement
  
  CREATE TABLE measurement_y2008m01 PARTITION OF measurement
  FOR VALUES FROM ('2008-01-01') TO ('2008-02-01')
! TABLESPACE fasttablespace
! WITH (parallel_workers = 4);
  

  
--- 3095,3102 
  
  CREATE TABLE measurement_y2008m01 PARTITION OF measurement
  FOR VALUES FROM ('2008-01-01') TO ('2008-02-01')
! WITH (parallel_workers = 4)
! TABLESPACE fasttablespace;
  

  

-- 
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] Parallel Hash take II

[Andres Freund]

Hi,

@@ -747,7 +747,7 @@ try_hashjoin_path(PlannerInfo *root,
 * never have any output pathkeys, per comments in create_hashjoin_path.
 */
initial_cost_hashjoin(root, , jointype, hashclauses,
- outer_path, inner_path, 
extra);
+ outer_path, inner_path, 
extra, false);
 
if (add_path_precheck(joinrel,
  workspace.startup_cost, 
workspace.total_cost,
@@ -761,6 +761,7 @@ try_hashjoin_path(PlannerInfo *root,
  extra,
  
outer_path,
  
inner_path,
+ 
false,/* parallel_hash */
  
extra->restrictlist,
  
required_outer,
  
hashclauses));
@@ -776,6 +777,10 @@ try_hashjoin_path(PlannerInfo *root,
  * try_partial_hashjoin_path
  *   Consider a partial hashjoin join path; if it appears useful, push it 
into
  *   the joinrel's partial_pathlist via add_partial_path().
+ *   The outer side is partial.  If parallel_hash is true, then the inner 
path
+ *   must be partial and will be run in parallel to create one or more 
shared
+ *   hash tables; otherwise the inner path must be complete and a copy of 
it
+ *   is run in every process to create separate identical private hash 
tables.
  */

When do we have "or more shared hash tables" rather than one? Are you
thinking about subordinate nodes?


@@ -1839,6 +1846,10 @@ hash_inner_and_outer(PlannerInfo *root,
 * able to properly guarantee uniqueness.  Similarly, we can't 
handle
 * JOIN_FULL and JOIN_RIGHT, because they can produce false null
 * extended rows.  Also, the resulting path must not be 
parameterized.
+* We should be able to support JOIN_FULL and JOIN_RIGHT for 
Parallel
+* Hash, since in that case we're back to a single hash table 
with a
+* single set of match bits for each batch, but that will 
require
+* figuring out a deadlock-free way to wait for the probe to 
finish.
 */

s/should be able/would be able/?



index 6a45b68e5df..2d38a5efae8 100644
--- a/src/backend/storage/ipc/barrier.c
+++ b/src/backend/storage/ipc/barrier.c
@@ -451,7 +451,6 @@ BarrierDetachImpl(Barrier *barrier, bool arrive)
release = true;
barrier->arrived = 0;
++barrier->phase;
-   Assert(barrier->selected);
barrier->selected = false;
}

Uh, what?




diff --git a/src/test/regress/expected/join.out 
b/src/test/regress/expected/join.out
index 35523bd8065..40a076d976f 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5821,6 +5821,9 @@ analyze extremely_skewed;
 insert into extremely_skewed
   select 42 as id, 'aa'
   from generate_series(1, 19000);
+-- Make a relation with a couple of enormous tuples.
+create table wide as select generate_series(1, 2) as id, rpad('', 32, 'x') 
as t;
+alter table wide set (parallel_workers = 2);

I'm doubtful this is actually going to be a wide tuple - this'll get
compressed down quite a bit, no?

postgres[26465][1]=# SELECT octet_length(t), pg_column_size(t) FROM wide ;
┌──┬┐
│ octet_length │ pg_column_size │
├──┼┤
│   32 │   3671 │
│   32 │   3671 │
└──┴┘
(2 rows)


(and yes, it's ridiculous that a compressed datum of that size still
takes up 3kb)



+-- parallel with parallel-aware hash join
+set max_parallel_workers_per_gather = 2;
+set work_mem = '128kB';
+set enable_parallel_hash = on;

I think it'd be better if we structured the file so we just sat guc's
with SET LOCAL inside a transaction.


+-- parallel with parallel-aware hash join
+set max_parallel_workers_per_gather = 2;
+set work_mem = '64kB';
+set enable_parallel_hash = on;
+explain (costs off)
+  select count(*) from simple r join extremely_skewed s using (id);
+  QUERY PLAN   
+---
+ Finalize Aggregate
+   ->  Gather
+ Workers Planned: 2
+ ->  Partial Aggregate
+   ->  Parallel Hash Join
+ Hash Cond: (r.id = s.id)
+ ->  Parallel Seq Scan on simple r
+ ->  Parallel Hash
+   

Re: [HACKERS] need info about extensibility in other databases

[Oleg Bartunov]

On Wed, Nov 8, 2017 at 2:37 PM, Li Song  wrote:
> Hi,
>
> When is the English version of "GiST programming tutorial" available?

Well, I wrote it many years ago, so it needs to be updated. For now,
you can use google translate, which seems works fine
https://translate.google.com/translate?sl=auto=en=y=_t=en=UTF-8=http%3A%2F%2Fwww.sai.msu.su%2F~megera%2Fpostgres%2Ftalks%2Fgist_tutorial.html=

Also, we have 7 papers about indexing, also in russian, check
https://habrahabr.ru/company/postgrespro/

>
> Regards,
> Song
>
>
>
> --
> Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


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

Re: [HACKERS] [PATCH] A hook for session start

[Fabrízio de Royes Mello]

On Wed, Nov 8, 2017 at 12:47 AM, Michael Paquier 
wrote:
>
> +   /* Hook just normal backends */
> +   if (session_end_hook && MyBackendId != InvalidBackendId)
> +   (*session_end_hook) ();
> I have been wondering about the necessity of this restriction.
> Couldn't it be useful to just let the people implementing the hook do
> any decision-making? Tracking some non-backend shutdowns may be useful
> as well for logging purposes.
>

Also makes sense... I move down this check to hook code.


> The patch is beginning to take shape (I really like the test module
> you are implementing here!), still needs a bit more work.
>

Thanks... and your review is helping a lot!!!


> +CREATE ROLE session_hook_usr1 LOGIN;
> +CREATE ROLE session_hook_usr2 LOGIN;
> Roles created during regression tests should be prefixed with regress_.
>

Fixed.


> +   if (prev_session_start_hook)
> +   prev_session_start_hook();
> +
> +   if (session_start_hook_enabled)
> +   (void) register_session_hook("START");
> Shouldn't both be reversed?
>

Fixed.


> +/* sample sessoin end hook function */
> Typo here.
>

Fixed.


> +CREATE DATABASE session_hook_db;
> [...]
> +   if (!strcmp(dbname, "session_hook_db"))
> +   {
> Creating a database is usually avoided in sql files as those can be
> run on existing servers using installcheck. I am getting that this
> restriction is in place so as it is possible to create an initial
> connection to the server so as the base table to log the activity is
> created. That's a fine assumption to me. Still, I think that the
> following changes should be done:
> - Let's restrict the logging to a role name instead of a database
> name, and let's parametrize it with a setting in the temporary
> configuration file. Let's not bother about multiple role support with
> a list, for the sake of tests and simplicity only defining one role
> looks fine to me. Comments in the code should be clear about the
> dependency.

Makes sense and simplify the test code. Fixed.


> - The GUCs test_session_hooks.start_enabled and
> test_session_hooks.end_enabled are actually redundant with the
> database restriction (well, role restriction per previous comment), so
> let's remove them. Please let's also avoid ALTER SYSTEM calls in tests
> as it would impact existing installations with installcheck.
>

Also simplify the test code. Fixed.


> Impossible to tell committer's feeling about this test suite, but my
> opinion is to keep it as that's a good template example about what can
> be done with those two hooks.
>

+1

Attached new version.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2c7260e..52a9641 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -169,6 +169,9 @@ static ProcSignalReason RecoveryConflictReason;
 static MemoryContext row_description_context = NULL;
 static StringInfoData row_description_buf;
 
+/* Hook for plugins to get control at start of session */
+session_start_hook_type session_start_hook = NULL;
+
 /* 
  *		decls for routines only used in this file
  * 
@@ -3857,6 +3860,9 @@ PostgresMain(int argc, char *argv[],
 	if (!IsUnderPostmaster)
 		PgStartTime = GetCurrentTimestamp();
 
+	if (session_start_hook)
+		(*session_start_hook) ();
+
 	/*
 	 * POSTGRES main processing loop begins here
 	 *
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 20f1d27..16ec376 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -76,6 +76,8 @@ static bool ThereIsAtLeastOneRole(void);
 static void process_startup_options(Port *port, bool am_superuser);
 static void process_settings(Oid databaseid, Oid roleid);
 
+/* Hook for plugins to get control at end of session */
+session_end_hook_type session_end_hook = NULL;
 
 /*** InitPostgres support ***/
 
@@ -1154,6 +1156,10 @@ ShutdownPostgres(int code, Datum arg)
 	 * them explicitly.
 	 */
 	LockReleaseAll(USER_LOCKMETHOD, true);
+
+	/* Hook at session end */
+	if (session_end_hook)
+		(*session_end_hook) ();
 }
 
 
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index f8c535c..9f05bfb 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -35,6 +35,13 @@ extern PGDLLIMPORT const char *debug_query_string;
 extern int	max_stack_depth;
 extern int	PostAuthDelay;
 
+/* Hook for plugins to get control at start and end of session */
+typedef void (*session_start_hook_type) (void);
+typedef void (*session_end_hook_type) (void);
+
+extern PGDLLIMPORT 

Re: [HACKERS] SQL procedures

[Merlin Moncure]

On Wed, Nov 8, 2017 at 11:03 AM, Peter Eisentraut
 wrote:
> On 11/8/17 11:11, Merlin Moncure wrote:
>> On Wed, Nov 8, 2017 at 9:13 AM, Peter Eisentraut
>>  wrote:
>>> I have already submitted a separate patch that addresses these questions.
>>
>> Maybe I'm obtuse, but I'm not seeing it? In very interested in the
>> general approach to transaction management; if you've described it in
>> the patch I'll read it there.  Thanks for doing this.
>
> https://www.postgresql.org/message-id/178d3380-0fae-2982-00d6-c43100bc8...@2ndquadrant.com

All right, thanks.  So,
*) Are you sure you want to go the SPI route?  'sql' language
(non-spi) procedures might be simpler from implementation standpoint
and do not need any language adjustments?

*) Is it possible to jump into SPI without having a snapshot already
set up. For example? If I wanted to set isolation level in a
procedure, would I get impacted by this error?
ERROR:  SET TRANSACTION ISOLATION LEVEL must be called before any query

merlin


-- 
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] SQL procedures

[Peter Eisentraut]

On 11/8/17 11:11, Merlin Moncure wrote:
> On Wed, Nov 8, 2017 at 9:13 AM, Peter Eisentraut
>  wrote:
>> I have already submitted a separate patch that addresses these questions.
> 
> Maybe I'm obtuse, but I'm not seeing it? In very interested in the
> general approach to transaction management; if you've described it in
> the patch I'll read it there.  Thanks for doing this.

https://www.postgresql.org/message-id/178d3380-0fae-2982-00d6-c43100bc8...@2ndquadrant.com

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


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

Re: [HACKERS] Small improvement to compactify_tuples

[Tom Lane]

Claudio Freire  writes:
> What's perhaps not clear is whether there are better ideas. Like
> rebuilding the page as Tom proposes, which doesn't seem like a bad
> idea. Bucket sort already is O(bytes), just as memcopy, only it has a
> lower constant factor (it's bytes/256 in the original patch), which
> might make copying the whole page an extra time lose against bucket
> sort in a few cases.

> Deciding that last point does need more benchmarking. That doesn't
> mean the other improvements can't be pursued in the meanwhile, right?

Well, I doubt we're going to end up committing more than one of these
ideas.  The question is which way is best.  If people are willing to
put in the work to test all of them, let's do it.

BTW, it strikes me that in considering the rebuild-the-page approach,
we should not have blinders on and just measure the speed of
PageRepairFragmentation.  Rather, we should take a look at what happens
subsequently given a physically-ordered set of tuples.  I can recall
Andres or someone moaning awhile ago about lack of locality of access in
index page searches --- maybe applying that approach while vacuuming
indexes will help?

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] taking stdbool.h into use

[Peter Eisentraut]

On 10/29/17 08:50, Michael Paquier wrote:
> I had a look at this patch series. Patches 1, 2 (macos headers indeed
> show that NSUNLINKMODULE_OPTION_NONE is set to 0x0), 3 to 7 look fine
> to me.

Committed 4 and 5 together.

> I spotted a couple of other things while looking at your patches and
> the code tree.
> 
> -   return (ginCompareItemPointers(>itemptr, iptr) > 0) ? TRUE : FALSE;
> +   return (ginCompareItemPointers(>itemptr, iptr) > 0) ? true : false;
> You could simplify that at the same time by removing such things. The
> "false : true" pattern is less frequent than the "true : false"
> pattern.

I have found many more instances like that.  It might be worth
simplifying a bit, but that sounds like a separate undertaking.

> Some comments in the code still mention FALSE or TRUE:
> - hashsearch.c uses FALSE in some comments.
> - In execExpr.c, ExecCheck mentions TRUE.

That one is an SQL TRUE, so I left it.

> - AggStateIsShared mentions TRUE and FALSE.
> - In arrayfuncs.c, ReadArrayStr, CopyArrayEls and ReadArrayBinary use TRUE.

Fixed the other ones.

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


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

Re: [HACKERS] Small improvement to compactify_tuples

[Claudio Freire]

On Wed, Nov 8, 2017 at 12:33 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Tue, Nov 7, 2017 at 4:39 PM, Tom Lane  wrote:
>>> What I'm getting from the standard pgbench measurements, on both machines,
>>> is that this patch might be a couple percent slower than HEAD, but that is
>>> barely above the noise floor so I'm not too sure about it.
>
>> Hmm.  It seems like slowing down single client performance by a couple
>> of percent is something that we really don't want to do.
>
> I do not think there is any change here that can be proven to always be a
> win.  Certainly the original patch, which proposes to replace an O(n log n)
> sort algorithm with an O(n^2) one, should not be thought to be that.
> The question to focus on is what's the average case, and I'm not sure how
> to decide what the average case is.  But more than two test scenarios
> would be a good start.
>
> regards, tom lane

Doing no change to the overall algorithm and replacing qsort with an
inlineable type-specific one should be a net win in all cases.

Doing bucket sort with a qsort of large buckets (or small tuple
arrays) should also be a net win in all cases.

Using shell sort might not seem clear, but lets not forget the
original patch only uses it in very small arrays and very infrequently
at that.

What's perhaps not clear is whether there are better ideas. Like
rebuilding the page as Tom proposes, which doesn't seem like a bad
idea. Bucket sort already is O(bytes), just as memcopy, only it has a
lower constant factor (it's bytes/256 in the original patch), which
might make copying the whole page an extra time lose against bucket
sort in a few cases.

Deciding that last point does need more benchmarking. That doesn't
mean the other improvements can't be pursued in the meanwhile, right?


-- 
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] Small improvement to compactify_tuples

[Peter Geoghegan]

On Wed, Nov 8, 2017 at 8:19 AM, Robert Haas  wrote:
> I don't remember any more just how much faster qsort_tuple() and
> qsort_ssup() are than plain qsort(), but it was significant enough to
> convince me to commit 337b6f5ecf05b21b5e997986884d097d60e4e3d0...

IIRC, qsort_ssup() was about 20% faster at the time, while
qsort_tuple() was 5% - 10% faster.

-- 
Peter Geoghegan


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

Re: [HACKERS] [PATCH] Generic type subscripting

[Arthur Zakirov]

Thank you for fixing.

On Tue, Nov 07, 2017 at 09:00:43PM +0100, Dmitry Dolgov wrote:
> > > +Datum
> > > +custom_subscripting_parse(PG_FUNCTION_ARGS)
> > > +{
> > > + boolisAssignment = PG_GETARG_BOOL(0);
> >
> > Here isAssignment is unused variable, so it could be removed.
> 
> In this case I disagree - the purpose of these examples is to show
> everything
> you can use. So I just need to come up with some example that involves
> `isAssignment`.

Understood.

> 
> > > + scratch->d.sbsref.eval_finfo = eval_finfo;
> > > + scratch->d.sbsref.nested_finfo = nested_finfo;
> > > +
> > As I mentioned earlier we need assigning eval_finfo and nested_finfo only
> for EEOP_SBSREF_OLD, EEOP_SBSREF_ASSIGN and EEOP_SBSREF_FETCH steps.
> > Also they should be assigned before calling ExprEvalPushStep(), not
> after. Otherwise some bugs may appear in future.
> 
> I'm really confused about this one. Can you tell me the exact line numbers?
> Because if I remove any of these lines "blindly", tests are failing.

Field scratch->d is union. Its fields should be changed only before calling 
ExprEvalPushStep(), which copies 'scratch'. To be more specific I attached the 
patch 0005-Fix-ExprEvalStep.patch, which can be applyed over your patches.

Some other notes are below.

>  class="parameter">type_modifier_output_function and
> -   analyze_function
> +   analyze_function,
> +   subscripting_parse_function
> +   subscripting_assign_function
> +   subscripting_fetch_function

I think you forgot commas and conjunction 'and'.

> +   The optional
> +   subscripting_parse_function,
> +   subscripting_assign_function
> +   subscripting_fetch_function
> +   contains type-specific logic for subscripting of the data type.

Here you forgot comma or 'and'. Also 'contain' should be used instead 
'contains'.

It seems that in the following you switched descriptions:

> + class="parameter">subscripting_assign_function
> +
> + 
> +  The name of a function that contains type-specific subscripting logic 
> for
> +  fetching an element from the data type.
> + 

subscripting_assign_function is used for updating.

> + class="parameter">subscripting_fetch_function
> +
> + 
> +  The name of a function that contains type-specific subscripting logic 
> for
> +  updating an element in the data type.
> + 

subscripting_fetch_function is used for fetching.

I have a little complain about how ExprEvalStep gets resvalue. resvalue is 
assigned in one place (within ExecEvalSubscriptingRefFetch(), 
ExecEvalSubscriptingRefAssign()), resnull is assigned in another place (within 
jsonb_subscript_fetch(), jsonb_subscript_assign()). I'm not sure that it is a 
good idea, but it is not critical, it is just complaint.

After your fixing I think we should wait for opinion of senior community 
members and mark the patch as 'Ready for Commiter'. Maybe I will do more tests 
and try to implement subscripting to another type.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index c275387319..42a0d31c31 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2350,9 +2350,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *aref, PlanState
 		fmgr_info(aref->refnestedfunc, nested_finfo);
 	}
 
-	scratch->d.sbsref.eval_finfo = eval_finfo;
-	scratch->d.sbsref.nested_finfo = nested_finfo;
-
 	/* Fill constant fields of SubscriptingRefState */
 	arefstate->isassignment = isAssignment;
 	arefstate->refelemtype = aref->refelemtype;
@@ -2377,9 +2374,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *aref, PlanState
 		scratch->d.jump.jumpdone = -1;	/* adjust later */
 		ExprEvalPushStep(state, scratch);
 
-		scratch->d.sbsref.eval_finfo = eval_finfo;
-		scratch->d.sbsref.nested_finfo = nested_finfo;
-
 		adjust_jumps = lappend_int(adjust_jumps,
    state->steps_len - 1);
 	}
@@ -2425,9 +2419,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *aref, PlanState
 		scratch->d.sbsref_subscript.jumpdone = -1;	/* adjust later */
 		ExprEvalPushStep(state, scratch);
 
-		scratch->d.sbsref.eval_finfo = eval_finfo;
-		scratch->d.sbsref.nested_finfo = nested_finfo;
-
 		adjust_jumps = lappend_int(adjust_jumps,
    state->steps_len - 1);
 		i++;
@@ -2462,9 +2453,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *aref, PlanState
 		scratch->d.sbsref_subscript.jumpdone = -1;	/* adjust later */
 		ExprEvalPushStep(state, scratch);
 
-		scratch->d.sbsref.eval_finfo = eval_finfo;
-		scratch->d.sbsref.nested_finfo = nested_finfo;
-
 		adjust_jumps = lappend_int(adjust_jumps,
    state->steps_len - 1);
 		i++;
@@ -2499,10 +2487,9 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *aref, PlanState
 		{
 			scratch->opcode = EEOP_SBSREF_OLD;
 			

Re: [HACKERS] Small improvement to compactify_tuples

[Tom Lane]

Robert Haas  writes:
> Just to throw a random idea out here, we currently have
> gen_qsort_tuple.pl producing qsort_tuple() and qsort_ssup().  Maybe it
> could be modified to also produce a specialized qsort_itemids().  That
> might be noticeably faster that our general-purpose qsort() for the
> reasons mentioned in the comments in gen_qsort_tuple.pl, viz:

+1 for somebody trying that (I'm not volunteering, though).

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] Small improvement to compactify_tuples

[Robert Haas]

On Wed, Nov 8, 2017 at 10:33 AM, Tom Lane  wrote:
> I do not think there is any change here that can be proven to always be a
> win.  Certainly the original patch, which proposes to replace an O(n log n)
> sort algorithm with an O(n^2) one, should not be thought to be that.
> The question to focus on is what's the average case, and I'm not sure how
> to decide what the average case is.  But more than two test scenarios
> would be a good start.

I appreciate the difficulties here; I'm just urging caution.  Let's
not change things just to clear this patch off our plate.

Just to throw a random idea out here, we currently have
gen_qsort_tuple.pl producing qsort_tuple() and qsort_ssup().  Maybe it
could be modified to also produce a specialized qsort_itemids().  That
might be noticeably faster that our general-purpose qsort() for the
reasons mentioned in the comments in gen_qsort_tuple.pl, viz:

# The major effects are (1) inlining simple tuple comparators is much faster
# than jumping through a function pointer and (2) swap and vecswap operations
# specialized to the particular data type of interest (in this case, SortTuple)
# are faster than the generic routines.

I don't remember any more just how much faster qsort_tuple() and
qsort_ssup() are than plain qsort(), but it was significant enough to
convince me to commit 337b6f5ecf05b21b5e997986884d097d60e4e3d0...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] SQL procedures

[Merlin Moncure]

On Wed, Nov 8, 2017 at 9:13 AM, Peter Eisentraut
 wrote:
> I have already submitted a separate patch that addresses these questions.

Maybe I'm obtuse, but I'm not seeing it? In very interested in the
general approach to transaction management; if you've described it in
the patch I'll read it there.  Thanks for doing this.

merlin


-- 
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] Exclude pg_internal.init from base backup

[Peter Eisentraut]

On 11/7/17 19:58, Michael Paquier wrote:
> On Wed, Nov 8, 2017 at 9:50 AM, Haribabu Kommi  
> wrote:
>> Thanks for the correction. I was not much aware of SGML markup usage.
>> While building the documentation, it raises an warning message of "empty
>> end-tag".
>> So I just added the end tag. Attached the update patch with the suggested
>> correction.
> 
> Ah, I can see the warning as well. Using empty tags is forbidden since
> c29c5789, which is really recent. Sorry for missing it. Simon got
> trapped by that as well visibly. Your patch looks good to me.

fixed

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


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

Re: [HACKERS] Horrible CREATE DATABASE Performance in High Sierra

[Tom Lane]

Peter Eisentraut  writes:
> On 10/7/17 16:46, Tom Lane wrote:
>> Accordingly I propose the attached patch.  If anyone's interested in
>> experimenting on other platforms, we might be able to refine/complicate
>> the FLUSH_DISTANCE selection further, but I think this is probably good
>> enough for a first cut.

> What is the status of this?  Is performance on High Sierra still bad?

I committed the fix at 643c27e36.  If Apple have done anything about the
underlying problem, you couldn't tell it from their non-response to my
bug report.

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] Small improvement to compactify_tuples

[Tom Lane]

Robert Haas  writes:
> On Tue, Nov 7, 2017 at 4:39 PM, Tom Lane  wrote:
>> What I'm getting from the standard pgbench measurements, on both machines,
>> is that this patch might be a couple percent slower than HEAD, but that is
>> barely above the noise floor so I'm not too sure about it.

> Hmm.  It seems like slowing down single client performance by a couple
> of percent is something that we really don't want to do.

I do not think there is any change here that can be proven to always be a
win.  Certainly the original patch, which proposes to replace an O(n log n)
sort algorithm with an O(n^2) one, should not be thought to be that.
The question to focus on is what's the average case, and I'm not sure how
to decide what the average case is.  But more than two test scenarios
would be a good start.

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] Horrible CREATE DATABASE Performance in High Sierra

[Andres Freund]

On November 8, 2017 7:31:17 AM PST, Peter Eisentraut 
 wrote:
>On 10/7/17 16:46, Tom Lane wrote:
>> I wrote:
>>> Current status is that I've filed a bug report with Apple and am
>waiting
>>> to see their response before deciding what to do next.  If they fix
>the
>>> issue promptly then there's little need for us to do anything.
>
>> Accordingly I propose the attached patch.  If anyone's interested in
>> experimenting on other platforms, we might be able to
>refine/complicate
>> the FLUSH_DISTANCE selection further, but I think this is probably
>good
>> enough for a first cut.
>
>What is the status of this?  Is performance on High Sierra still bad?

Didn't Tom commit a workaround?

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
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] Horrible CREATE DATABASE Performance in High Sierra

[Peter Eisentraut]

On 10/7/17 16:46, Tom Lane wrote:
> I wrote:
>> Current status is that I've filed a bug report with Apple and am waiting
>> to see their response before deciding what to do next.  If they fix the
>> issue promptly then there's little need for us to do anything.

> Accordingly I propose the attached patch.  If anyone's interested in
> experimenting on other platforms, we might be able to refine/complicate
> the FLUSH_DISTANCE selection further, but I think this is probably good
> enough for a first cut.

What is the status of this?  Is performance on High Sierra still bad?

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


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

Re: [HACKERS] SQL procedures

[Peter Eisentraut]

On 11/8/17 09:23, Merlin Moncure wrote:
> I do wonder how transaction control could be added later.
> 
> The last time I (lightly) looked at this, I was starting to think that
> working transaction control into the SPI interface was the wrong
> approach; pl/pgsql would have to adopt a very different set of
> behaviors if it was called in a function or a proc.  If you restricted
> language choice to purely SQL, you could work around this problem; SPI
> languages would be totally abstracted from those sets of
> considerations and you could always call an arbitrary language
> function if you needed to.  SQL has no flow control but I'm not too
> concerned about that.

I have already submitted a separate patch that addresses these questions.

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


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

Re: [HACKERS] SQL procedures

[Peter Eisentraut]

On 11/8/17 09:33, Pavel Stehule wrote:
> We can create auto session variable STATUS. This variable can be 0
> if procedure was returned without explicit RETURN value. Or it can
> hold different value specified by RETURN expr.
> 
> This value can be read by GET DIAGNOSTICS xxx = STATUS
> 
> or some similar.
> 
> The motivation is allow some mechanism cheaper than our exceptions.

I suppose this could be a separately discussed feature.  We'd also want
to consider various things that PL/pgSQL pretends to be compatible with.

One of the main motivations for procedures is to do more complex and
expensive things including transaction control.  So saving exception
overhead is not really on the priority list there.

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


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

Re: [HACKERS] SQL procedures

[Peter Eisentraut]

On 11/6/17 16:27, Simon Riggs wrote:
> You mention PARALLEL SAFE is not used for procedures. Isn't it an
> architectural restriction that procedures would not be able to execute
> in parallel? (At least this year)

I'm not sure what you are referring to here.  I don't think the
functionality I'm proposing does anything in parallel or has any
interaction with it.

> I think we need an explanatory section of the docs, but there doesn't
> seem to be one for Functions, so there is no place to add some text
> that says the above.
> 
> I found it confusing that ALTER and DROP ROUTINE exists but not CREATE
> ROUTINE. At very least we should say somewhere "there is no CREATE
> ROUTINE", so its absence is clearly intentional. I did wonder whether
> we should have it as well, but its just one less thing to review, so
> good.

I'll look for a place to add some documentation around this.

> Was surprised that pg_dump didn't use DROP ROUTINE, when appropriate.

It's not clear to me why that would be preferred.

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


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

Re: [HACKERS] SQL procedures

[Tom Lane]

Peter Eisentraut  writes:
> On 10/31/17 14:23, Tom Lane wrote:
>> Why not use VOIDOID for the prorettype value?

> We need a way to distinguish functions that are callable by SELECT and
> procedures that are callable by CALL.

Do procedures of this ilk belong in pg_proc at all?  It seems like a large
fraction of the attributes tracked in pg_proc are senseless for this
purpose.  A new catalog might be a better approach.

In any case, I buy none of your arguments that 0 is a better choice than a
new pseudotype.

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] SQL procedures

[Konstantin Knizhnik]

On 08.11.2017 17:23, Merlin Moncure wrote:

On Tue, Oct 31, 2017 at 12:23 PM, Peter Eisentraut
 wrote:

- Transaction control in procedure bodies

This feature is really key, since it enables via SQL lots of things
that are not possible without external coding, including:
*) very long running processes in a single routine
*) transaction isolation control inside the procedure (currently
client app has to declare this)
*) certain error handling cases that require client side support
*) simple in-database threading
*) simple construction of daemon scripts (yeah, you can use bgworker
for this, but pure sql daemon with a cron heartbeat hook is hard to
beat for simplicity)

I do wonder how transaction control could be added later.

The last time I (lightly) looked at this, I was starting to think that
working transaction control into the SPI interface was the wrong
approach; pl/pgsql would have to adopt a very different set of
behaviors if it was called in a function or a proc.  If you restricted
language choice to purely SQL, you could work around this problem; SPI
languages would be totally abstracted from those sets of
considerations and you could always call an arbitrary language
function if you needed to.  SQL has no flow control but I'm not too
concerned about that.

merlin


I am also very interested in answer on this question: how you are going 
to implement transaction control inside procedure?
Right now in PostgresPRO EE supports autonomous transactions. Them are 
supported both for SQL and plpgsql/plpython APIs.
Them are implemented by saving/restoring transaction context, so unlike 
most of other ATX implementations, in pgpro autonomous
transaction is executed by the same backend. But it is not so easy to 
do: in Postgres almost any module have its own static variables which 
keeps transaction specific data.
So we have to provide a dozen of suspend/resume functions: 
SuspendSnapshot(),  SuspendPredicate(), SuspendStorage(), 
SuspendInvalidationInfo(), SuspendPgXact(), PgStatSuspend(), 
TriggerSuspend(), SuspendSPI()... and properly handle local cache 
invalidation. Patch consists of more than 5 thousand lines.


So my question is whether you are going to implement something similar 
or use completely different approach?
In first case it will be good to somehow unite our efforts... For 
example we can publish our ATX patch for Postgres 10.
We have not done it yet, because there seems to be no chances to push 
this patch to community.









--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



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

[HACKERS] Proposal: ALTER EXTENSION SET OPTION

[Chris Travers]

Hi all;

One of the annoyances we currently deal with regarding analytics extensions
in a PG environment with mixed versions is there is no facility right now
to conditionally support certain modifications to functions that might be
required to make certain features work properly.

The case that comes to mind right now is in marking some functions parallel
safe for PostgreSQL 9.6 and above while taking no action for 9.5.
Currently we have a few options:

1.  Mark on a best effort basis
2,  Drop support for 9.5 and below
3.  Come up with some much more complicated version graph.

It would be very nice to be able to define some options which could be set
for extensions but don't affect their arguments or return types, such as
marking functions parallel-safe and then have scripts which run to define
these functions.

My thinking is one would have a syntax for a few specified options, such as:

ALTER EXTENSION foo SET OPTION PARALLEL SAFETY;
or if the extension supports the reverse:
ALTER EXTENSION foo UNSET OPTION PARALLEL SAFETY;

Over time more options could be added, but would be mapped to a file
convention.  In this case, we could expect:

foo--[version]--set-parallel-safety.sql and
foo--[version]--unset-parallel-safety.sql

-- 
Best Regards,
Chris Travers
Database Administrator

Tel: +49 162 9037 210 | Skype: einhverfr | www.adjust.com
Saarbrücker Straße 37a, 10405 Berlin

Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager

[Masahiko Sawada]

On Wed, Nov 8, 2017 at 5:41 AM, Robert Haas  wrote:
> On Mon, Nov 6, 2017 at 4:42 AM, Masahiko Sawada  wrote:
 I suggest that a good thing to do more or less immediately, regardless
 of when this patch ends up being ready, would be to insert an
 insertion that LockAcquire() is never called while holding a lock of
 one of these types.  If that assertion ever fails, then the whole
 theory that these lock types don't need deadlock detection is wrong,
 and we'd like to find out about that sooner or later.
>>>
>>> I understood. I'll check that first.
>>
>> I've checked whether LockAcquire is called while holding a lock of one
>> of four types: LOCKTAG_RELATION_EXTENSION, LOCKTAG_PAGE,
>> LOCKTAG_TUPLE, and LOCKTAG_SPECULATIVE_TOKEN. To summary, I think that
>> we cannot move these four lock types together out of heavy-weight
>> lock, but can move only the relation extension lock with tricks.
>>
>> Here is detail of the survey.
>
> Thanks for these details, but I'm not sure I fully understand.
>
>> * LOCKTAG_RELATION_EXTENSION
>> There is a path that LockRelationForExtension() could be called while
>> holding another relation extension lock. In brin_getinsertbuffer(), we
>> acquire a relation extension lock for a index relation and could
>> initialize a new buffer (brin_initailize_empty_new_buffer()). During
>> initializing a new buffer, we call RecordPageWithFreeSpace() which
>> eventually can call fsm_readbuf(rel, addr, true) where the third
>> argument is "extend". We can process this problem by having the list
>> (or local hash) of acquired locks and skip acquiring the lock if
>> already had. For other call paths calling LockRelationForExtension, I
>> don't see any problem.
>
> Does calling fsm_readbuf(rel,addr,true) take some heavyweight lock?

No, I meant fsm_readbuf(rel,addr,true) can acquire a relation
extension lock. So it's not problem.

> Basically, what matters here in the end is whether we can articulate a
> deadlock-proof rule around the order in which these locks are
> acquired.

You're right, my survey was not enough to make a decision.

As far as the acquiring these four lock types goes, there are two call
paths that acquire any type of locks while holding another type of
lock. The one is that acquiring a relation extension lock and then
acquiring a relation extension lock for the same relation again. As
explained before, this can be resolved by remembering the holding lock
(perhaps holding only last one is enough). Another is that acquiring
either a tuple lock, a page lock or a speculative insertion lock and
then acquiring a relation extension lock. In the second case, we try
to acquire these two locks in the same order; acquiring 3 types lock
and then extension lock. So it's not problem if we apply the rule that
is that we disallow to try acquiring these three lock types while
holding any relation extension lock. Also, as far as I surveyed there
is no path to acquire a relation lock while holding other 3 type
locks.

>  The simplest such rule would be "you can only acquire one
> lock of any of these types at a time, and you can't subsequently
> acquire a heavyweight lock".  But a more complicated rule would be OK
> too, e.g. "you can acquire as many heavyweight locks as you want, and
> after that you can optionally acquire one page, tuple, or speculative
> token lock, and after that you can acquire a relation extension lock".
> The latter rule, although more complex, is still deadlock-proof,
> because the heavyweight locks still use the deadlock detector, and the
> rest has a consistent order of lock acquisition that precludes one
> backend taking A then B while another backend takes B then A.  I'm not
> entirely clear whether your survey leads us to a place where we can
> articulate such a deadlock-proof rule.

Speaking of the acquiring these four lock types and heavy weight lock,
there obviously is a call path to acquire any of four lock types while
holding a heavy weight lock. In reverse, there also is a call path
that we acquire a heavy weight lock while holding any of four lock
types. The call path I found is that in heap_delete we acquire a tuple
lock and call XactLockTableWait or MultiXactIdWait which eventually
could acquire LOCKTAG_TRANSACTION in order to wait for the concurrent
transactions finish. But IIUC since these functions acquire the lock
for the concurrent transaction's transaction id, deadlocks doesn't
happen.
However, there might be other similar call paths if I'm missing
something. For example, we do some operations that might acquire any
heavy weight locks other than LOCKTAG_TRANSACTION, while holding a
page lock (in ginInsertCleanup) or holding a specualtive insertion
lock (in nodeModifyTable).

To summary, I think we can put the following rules in order to move
four lock types out of heavy weight lock.

1. Do not acquire either a tuple lock, a page lock or a speculative
insertion lock while holding a 

Re: [HACKERS] SQL procedures

[Pavel Stehule]

2017-11-08 15:31 GMT+01:00 Pavel Stehule :

>
>
> 2017-11-08 15:23 GMT+01:00 Peter Eisentraut  com>:
>
>> On 10/31/17 16:50, Pavel Stehule wrote:
>> > Not sure if disabling RETURN is good idea. I can imagine so optional
>> > returning something like int status can be good idea. Cheaper than
>> > raising a exception.
>>
>> We could allow a RETURN without argument in PL/pgSQL, if you just want
>> to exit early.  That syntax is currently not available, but it should
>> not be hard to add.
>>
>> I don't understand the point about wanting to return an int.  How would
>> you pass that around, since there is no declared return type?
>>
>
> We can create auto session variable STATUS. This variable can be 0 if
> procedure was returned without explicit RETURN value. Or it can hold
> different value specified by RETURN expr.
>
> This value can be read by GET DIAGNOSTICS xxx = STATUS
>
> or some similar.
>

The motivation is allow some mechanism cheaper than our exceptions.

Regards

Pavel

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

Re: [HACKERS] SQL procedures

[Pavel Stehule]

2017-11-08 15:23 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 10/31/17 16:50, Pavel Stehule wrote:
> > Not sure if disabling RETURN is good idea. I can imagine so optional
> > returning something like int status can be good idea. Cheaper than
> > raising a exception.
>
> We could allow a RETURN without argument in PL/pgSQL, if you just want
> to exit early.  That syntax is currently not available, but it should
> not be hard to add.
>
> I don't understand the point about wanting to return an int.  How would
> you pass that around, since there is no declared return type?
>

We can create auto session variable STATUS. This variable can be 0 if
procedure was returned without explicit RETURN value. Or it can hold
different value specified by RETURN expr.

This value can be read by GET DIAGNOSTICS xxx = STATUS

or some similar.



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

Re: [HACKERS] SQL procedures

[Merlin Moncure]

On Tue, Oct 31, 2017 at 12:23 PM, Peter Eisentraut
 wrote:
> - Transaction control in procedure bodies

This feature is really key, since it enables via SQL lots of things
that are not possible without external coding, including:
*) very long running processes in a single routine
*) transaction isolation control inside the procedure (currently
client app has to declare this)
*) certain error handling cases that require client side support
*) simple in-database threading
*) simple construction of daemon scripts (yeah, you can use bgworker
for this, but pure sql daemon with a cron heartbeat hook is hard to
beat for simplicity)

I do wonder how transaction control could be added later.

The last time I (lightly) looked at this, I was starting to think that
working transaction control into the SPI interface was the wrong
approach; pl/pgsql would have to adopt a very different set of
behaviors if it was called in a function or a proc.  If you restricted
language choice to purely SQL, you could work around this problem; SPI
languages would be totally abstracted from those sets of
considerations and you could always call an arbitrary language
function if you needed to.  SQL has no flow control but I'm not too
concerned about that.

merlin


-- 
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] SQL procedures

[Peter Eisentraut]

On 10/31/17 16:50, Pavel Stehule wrote:
> Not sure if disabling RETURN is good idea. I can imagine so optional
> returning something like int status can be good idea. Cheaper than
> raising a exception.

We could allow a RETURN without argument in PL/pgSQL, if you just want
to exit early.  That syntax is currently not available, but it should
not be hard to add.

I don't understand the point about wanting to return an int.  How would
you pass that around, since there is no declared return type?

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


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

Re: [HACKERS] SQL procedures

[Peter Eisentraut]

On 10/31/17 14:23, Tom Lane wrote:
> Putting 0 in prorettype seems like a pretty bad idea.

It seemed like the natural thing to do, since we use a zero OID to
indicate "nothing" in many other places.

> Why not use VOIDOID for the prorettype value?

We need a way to distinguish functions that are callable by SELECT and
procedures that are callable by CALL.

> Or if there is some reason why "void" isn't the
> right pseudotype, maybe you should invent a new one, analogous to the
> "trigger" and "event_trigger" pseudotypes.

I guess that would be doable, but I think it would make things more
complicated without any gain that I can see.  In the case of the
pseudotypes you mention, those are the actual types mentioned in the
CREATE FUNCTION command.  If we invented a new pseudotype, that would
run the risk of existing code creating nonsensical reverse compilations
like CREATE FUNCTION RETURNS PROCEDURE.  Catalog queries using
prorettype == 0 would behave sensibly by default.  For example, an inner
or outer join against pg_type would automatically make sense.

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


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

Re: [HACKERS] why not parallel seq scan for slow functions

[Robert Haas]

On Wed, Nov 8, 2017 at 7:26 AM, Amit Kapila  wrote:
> We do want to generate it later when there isn't inheritance involved,
> but only if there is a single rel involved (simple_rel_array_size
> <=2).  The rule is something like this, we will generate the gather
> paths at this stage only if there are more than two rels involved and
> there isn't inheritance involved.

Why is that the correct rule?

Sorry if I'm being dense here.  I would have thought we'd want to skip
it for the topmost scan/join rel regardless of the presence or absence
of inheritance.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] why not parallel seq scan for slow functions

[Amit Kapila]

On Wed, Nov 8, 2017 at 4:34 PM, Robert Haas  wrote:
> On Tue, Nov 7, 2017 at 9:41 PM, Amit Kapila  wrote:
>> This is required to prohibit generating gather path for top rel in
>> case of inheritence (Append node) at this place (we want to generate
>> it later when scan/join target is available).
>
> OK, but why don't we want to generate it later when there isn't
> inheritance involved?
>

We do want to generate it later when there isn't inheritance involved,
but only if there is a single rel involved (simple_rel_array_size
<=2).  The rule is something like this, we will generate the gather
paths at this stage only if there are more than two rels involved and
there isn't inheritance involved.


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


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

Re: [HACKERS] need info about extensibility in other databases

[Li Song]

Hi,

When is the English version of "GiST programming tutorial" available?

Regards, 
Song



--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html


-- 
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] Small improvement to compactify_tuples

[Robert Haas]

On Tue, Nov 7, 2017 at 4:39 PM, Tom Lane  wrote:
> What I'm getting from the standard pgbench measurements, on both machines,
> is that this patch might be a couple percent slower than HEAD, but that is
> barely above the noise floor so I'm not too sure about it.

Hmm.  It seems like slowing down single client performance by a couple
of percent is something that we really don't want to do.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] why not parallel seq scan for slow functions

[Robert Haas]

On Tue, Nov 7, 2017 at 9:41 PM, Amit Kapila  wrote:
> This is required to prohibit generating gather path for top rel in
> case of inheritence (Append node) at this place (we want to generate
> it later when scan/join target is available).

OK, but why don't we want to generate it later when there isn't
inheritance involved?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] path toward faster partition pruning

[Amit Langote]

Hi Rajkumar,

Thanks for testing.

On 2017/11/08 15:52, Rajkumar Raghuwanshi wrote:
> On Mon, Nov 6, 2017 at 3:31 PM, Amit Langote 
> wrote:
> 
>> Attached updated set of patches, including the fix to make the new pruning
>> code handle Boolean partitioning.
>>
> 
> Hi Amit,
> 
> I have tried pruning for different values of constraint exclusion GUC
> change, not sure exactly how it should behave, but I can see with the
> delete statement pruning is not happening when constraint_exclusion is off,
> but select is working as expected. Is this expected behaviour?

Hmm, the new pruning only works for selects, not DML.  The patch also
changes get_relation_constraints() to not include the internal partition
constraints, but mistakenly does so for all query types, not just select.
Will look into it.

Thanks,
Amit



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