Re: [HACKERS] [Patch] Log SSL certificate verification errors

[Michael Paquier]

On Sat, Nov 11, 2017 at 3:34 AM, Graham Leggett  wrote:
> Currently neither the server side nor the client side SSL certificate verify 
> callback does anything, leading to potential hair-tearing-out moments.
>
> The following patch to master implements logging of all certificate 
> verification failures, as well as (crucially) which certificates failed to 
> verify, and at what depth, so the admin can zoom in straight onto the problem 
> without any guessing.

Could you attach as a file to this thread a patch that can be easily
applied? Using git --format-patch or simply diff is just fine.

Here are also some community guidelines on the matter:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

And if you are looking for feedback, you should register it to the
next commit fest:
https://commitfest.postgresql.org/16/
-- 
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_upgrade to clusters with a different WAL segment size

[Michael Paquier]

On Sat, Nov 11, 2017 at 12:46 AM, Bossart, Nathan  wrote:
> Allowing changes to the WAL segment size during pg_upgrade seems like a
> nice way to avoid needing a dump and load, so I would like to propose
> adding support for this.  I'd be happy to submit patches for this in the
> next commitfest.

That's a worthy goal.
-- 
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 Sat, Nov 11, 2017 at 12:50 AM, Robert Haas  wrote:
> On Fri, Nov 10, 2017 at 10:34 AM, Tom Lane  wrote:
>> I think as far as that goes, we can just change to "Therefore, by default
>> their use is restricted ...".  Then I suggest adding a  para
>> after that, with wording along the lines of
>>
>> It is possible to GRANT use of server-side lo_import and lo_export to
>> non-superusers, but careful consideration of the security implications
>> is required.  A malicious user of such privileges could easily parlay
>> them into becoming superuser (for example by rewriting server
>> configuration files), or could attack the rest of the server's file
>> system without bothering to obtain database superuser privileges as
>> such.  Access to roles having such privilege must therefore be guarded
>> just as carefully as access to superuser roles.  Nonetheless, if use
>> of server-side lo_import or lo_export is needed for some routine task,
>> it's safer to use a role of this sort than full superuser privilege,
>> as that helps to reduce the risk of damage from accidental errors.
>
> +1.  That seems like great language to me.

+1. Not convinced that mentioning wrappers is worth the complication.
Experienced admins likely already know such matters.
-- 
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

[Stephen Frost]

Michael, Tom,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
> On Fri, Nov 10, 2017 at 10:00 AM, Tom Lane  wrote:
> > Stephen Frost  writes:
> >> I'm guessing no, which essentially means that *we* consider access to
> >> lo_import/lo_export to be equivilant to superuser and therefore we're
> >> not going to implement anything to try and prevent the user who has
> >> access to those functions from becoming superuser.  If we aren't willing
> >> to do that, then how can we really say that there's some difference
> >> between access to these functions and being a superuser?
> >
> > We seem to be talking past each other.  Yes, if a user has malicious
> > intentions, it's possibly to parlay lo_export into obtaining a superuser
> > login (I'm less sure that that's necessarily true for lo_import).
> > That does NOT make it "equivalent", except perhaps in the view of someone
> > who is only considering blocking malevolent actors.  It does not mean that
> > there's no value in preventing a task that needs to run lo_export from
> > being able to accidentally destroy any data in the database.  There's a
> > range of situations where you are concerned about accidents and errors,
> > not malicious intent; but your argument ignores those use-cases.
> 
> That will not sound much as a surprise as I spawned the original
> thread, but like Robert I understand that getting rid of all superuser
> checks is a goal that we are trying to reach to allow admins to have
> more flexibility in handling permissions to a subset of objects.
> Forcing an admin to give full superuser rights to one user willing to
> work only on LOs import and export is a wrong concept.

The problem I have with this is that 'full superuser rights' actually
are being granted by this.  You're able to read any file and write any
file on the filesystem that the PG OS user can.  How is that not 'full
superuser rights'?  The concerns about a mistake being made are just as
serious as they are with someone who has superuser rights as someone
could pretty easily end up overwriting the control file or various other
extremely important bits of the system.  This isn't just about what
'blackhats' can do with this.

As I mentioned up-thread, if we want to make it so that non-superusers
can use lo_import/lo_export, then we should do that in a way that
allows the administrator to specify exactly the rights they really want
to give, based on a use-case for them.  There's no use-case for allowing
someone to use lo_export() to overwrite pg_control.  The use-case would
be to allow them to export to a specific directory (or perhaps a set of
directories), or to read from same.  The same is true of COPY.  Further,
I wonder what would happen if we allow this and then someone comes along
and re-proposes the 'CREATE DIRECTORY' concept that I had once proposed
(ideally with things cleaned up and tightened up to avoid there being
issues using it) to address the actual use-case for these functions to
be available to a non-superuser.  We wouldn't be able to change these
functions to start checking the 'directory' rights or we would break
existing non-superuser usage of them.  I imagine we could create
additional functions which check the 'directory' privileges, but that's
hardly ideal, imv.

I'm disappointed to apparently be in the minority on this, but that
seems to be the case unless someone else wishes to comment.  While I
appreciate the discussion, I'm certainly no more convinced today than I
was when I first saw this go in that allowing these functions to be
granted to non-superusers does anything but effectively make them into
superusers who aren't explicitly identified as superusers.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal: Improve bitmap costing for lossy pages

[Robert Haas]

On Thu, Nov 9, 2017 at 3:55 AM, amul sul  wrote:
> It took me a little while to understand this calculation.  You have moved this
> code from tbm_create(), but I think you should move the following
> comment as well:

I made an adjustment that I hope will address your concern here, made
a few other adjustments, and committed this.

One point of concern that wasn't entirely addressed in the above
discussion is the accuracy of this formula:

+   lossy_pages = Max(0, heap_pages - maxentries / 2);

When I first looked at Dilip's test results, I thought maybe this
formula was way off.  But on closer study, the formula does a decent
(not fantastic) job of estimating the number of exact pages.  The fact
that the number of lossy pages is off is just because the Mackert and
Lohman formula is overestimating how many pages are fetched.  Now, in
Dilip's results, this formula more often than not - but not invariably
- predicted more exact pages than we actually got.  So possibly
instead of maxentries / 2 we could subtract maxentries or some other
multiple of maxentries.  I don't know what's actually best here, but I
think there's a strong argument that this is an improvement as it
stands, and we can adjust it later if it becomes clear what would be
better.

-- 
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] LDAP URI decoding bugs

[Thomas Munro]

On Sat, Nov 11, 2017 at 8:37 AM, Peter Eisentraut
 wrote:
> On 11/6/17 23:30, Michael Paquier wrote:
>> On Fri, Nov 3, 2017 at 12:57 PM, Thomas Munro
>>  wrote:
>>> 1.  If you set up a pg_hba.conf with a URL that lacks a base DN or
>>> hostname, hba.c will segfault on startup when it tries to pstrdup a
>>> null pointer.  Examples: ldapurl="ldap://localhost; and
>>> ldapurl="ldap://;.
>>>
>>> 2.  If we fail to bind but have no binddn configured, we'll pass NULL
>>> to ereport (snprint?) for %s, which segfaults on some libc
>>> implementations.  That crash requires more effort to reproduce but you
>>> can see pretty clearly a few lines above in auth.c that it can be
>>> NULL.  (I'm surprised Coverity didn't complain about that.  Maybe it
>>> can't see this code due to macros.)
>
> committed and backpatched

Thanks!

I suppose someone might eventually want to go further and teach it to
understand such bare URLs or missing options (ie leaving out any bits
you want and falling back to the ldap library's defaults, which come
from places like env variables, .ldaprc and /etc/ldap.conf, the way
that "ldapsearch" and other tools manage to work with reasonable
defaults, or at least only need to be set up in one place for all your
LDAP-client software).  I'm not planning to work on that.

-- 
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] parallelize queries containing initplans

[Tom Lane]

Robert Haas  writes:
> On Fri, Nov 10, 2017 at 2:24 PM, Tom Lane  wrote:
>> Also, I wonder whether the InvalidOid hack in SS_assign_special_param
>> requires commentary.  It might be safer to use a valid type OID there,
>> perhaps VOIDOID or INTERNALOID.

> There is existing precedent for using InvalidOid to denote the absence
> of a parameter -- cf. copyParamList, SerializeParamList.

OK, fair enough.  I was concerned that this was adding a corner case
not otherwise seen with Params, but evidently not.

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] Planning counters in pg_stat_statements

[Thomas Munro]

On Tue, Nov 7, 2017 at 6:39 PM, Tsunakawa, Takayuki
 wrote:
> From: pgsql-hackers-ow...@postgresql.org
>> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Thomas Munro
>> I have often wanted $SUBJECT and was happy to find that Fujii-san had posted
>> a patch five years ago[1].  The reception then seemed positive.
>> So here is a refurbished and (hopefully) improved version of his patch with
>> a new column for the replan count.  Thoughts?
>
> That's a timely proposal.  I sometimes faced performance problems where the 
> time pg_stat_statements shows is much shorter than the application perceives. 
>  The latest experience was that the execution time of a transaction, which 
> consists of dozens of DMLs and COMMIT, was about 200ms from the application's 
> perspective, while pg_stat_statements showed only about 10ms in total.  The 
> network should not be the cause because the application ran on the same host 
> as the database server.  I wanted to know how long the parsing and planning 
> time was.

Note that this patch doesn't include the parse or parse analysis
times.  I guess they would be less interesting?  But perhaps someone
would want to have the complete query production line measured.

BTW the reason I was looking into this was because an Oracle user
asked me how to see "hard parse" times on Postgres, and I've talked to
others who seem strangely concerned with "parsing" time.  On Oracle I
believe that term covers (among other things) actually planning, and I
guess planning is the most interesting component.  Planning is the
thing I've wanted to measure myself, to diagnose problems relating to
partition/inheritance planning and join explosions and to figure out
which things should be changed to PREPARE/EXECUTE.  Perhaps a separate
parse/analysis counter might become more interesting for us if we ever
add automatic plan cache so you could assess how often you're getting
an implicit prepared statement (something like Oracle's "soft parse")?

> BTW, the current pg_stat_statement shows unexpected time for COMMIT.  I 
> expect it to include the whole COMMIT processing, including the long WAL 
> flush and sync rep wait.  However, it only shows the time for the transaction 
> state change in memory.

That's an interesting point.  You could install a transaction hook to
measure that easily enough, but I'm not sure how useful it'd be: you'd
be grouping together COMMIT timing data from transactions that are
doing very different things (including nothing).  Would that tell you
anything actionable?  If you include commit time for COMMIT statements
then you'd also have to decide whether to include it for DML
statements that run in an implicit transaction.  The trouble with that
is that the same statement inside an explicit transaction wouldn't
have any commit time, so you'd be mixing oranges and apples.  I guess
you could fix that by putting adding "commits" and  "commit_time"
columns (= counters for this statement run as implicit transaction),
but I wonder if commit time monitoring really belongs somewhere else.

For sync rep waits, that's what the pg_stat_replication.XXX_lag
columns tell you.

-- 
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] parallelize queries containing initplans

[Robert Haas]

On Fri, Nov 10, 2017 at 2:24 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I decided to try instead teaching the planner to keep track of the
>> types of PARAM_EXEC parameters as they were created, and that seems to
>> work fine.  See 0001, attached.
>
> I did not look at the other part, but 0001 looks reasonable to me.

Thanks for looking.

> I might quibble with the grammar in the generate_new_param comment:
>
> - * need to record the PARAM_EXEC slot number as being allocated.
> + * need to make sure we have record the type in paramExecTypes (otherwise,
> + * there won't be a slot allocated for it).
>   */
>
> I'd just go with "need to record the type in ..."

Noted.

> Also, I wonder whether the InvalidOid hack in SS_assign_special_param
> requires commentary.  It might be safer to use a valid type OID there,
> perhaps VOIDOID or INTERNALOID.

I think it's pretty straightforward -- if, as the existing comments
say, no Param node will be present and no value will be stored, then
we do not and cannot record the type of the value that we're not
storing.

There is existing precedent for using InvalidOid to denote the absence
of a parameter -- cf. copyParamList, SerializeParamList.  That
convention was not invented by me or the parallel query stuff,
although it has become more widespread for that reason.  I am
disinclined to have this patch invent a New Way To Do It.

-- 
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] LDAP URI decoding bugs

[Peter Eisentraut]

On 11/6/17 23:30, Michael Paquier wrote:
> On Fri, Nov 3, 2017 at 12:57 PM, Thomas Munro
>  wrote:
>> 1.  If you set up a pg_hba.conf with a URL that lacks a base DN or
>> hostname, hba.c will segfault on startup when it tries to pstrdup a
>> null pointer.  Examples: ldapurl="ldap://localhost; and
>> ldapurl="ldap://;.
>>
>> 2.  If we fail to bind but have no binddn configured, we'll pass NULL
>> to ereport (snprint?) for %s, which segfaults on some libc
>> implementations.  That crash requires more effort to reproduce but you
>> can see pretty clearly a few lines above in auth.c that it can be
>> NULL.  (I'm surprised Coverity didn't complain about that.  Maybe it
>> can't see this code due to macros.)

committed and backpatched

-- 
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] Restrict concurrent update/delete with UPDATE of partition key

[Robert Haas]

On Wed, Sep 27, 2017 at 7:07 AM, amul sul  wrote:
> Attaching POC patch that throws an error in the case of a concurrent update
> to an already deleted tuple due to UPDATE of partition key[1].
>
> In a normal update new tuple is linked to the old one via ctid forming
> a chain of tuple versions but UPDATE of partition key[1] move tuple
> from one partition to an another partition table which breaks that chain.

This patch needs a rebase.  It has one whitespace-only hunk that
should possibly be excluded.

I think the basic idea of this is sound.  Either you or Amit need to
document the behavior in the user-facing documentation, and it needs
tests that hit every single one of the new error checks you've added
(obviously, the tests will only work in combination with Amit's
patch).  The isolation should be sufficient to write such tests.

It needs some more extensive comments as well.  The fact that we're
assigning a meaning to ip_blkid -> InvalidBlockNumber is a big deal,
and should at least be documented in itemptr.h in the comments for the
ItemPointerData structure.

I suspect ExecOnConflictUpdate needs an update for the
HeapTupleUpdated case similar to what you've done elsewhere.

-- 
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] parallelize queries containing initplans

[Tom Lane]

Robert Haas  writes:
> I decided to try instead teaching the planner to keep track of the
> types of PARAM_EXEC parameters as they were created, and that seems to
> work fine.  See 0001, attached.

I did not look at the other part, but 0001 looks reasonable to me.
I might quibble with the grammar in the generate_new_param comment:

- * need to record the PARAM_EXEC slot number as being allocated.
+ * need to make sure we have record the type in paramExecTypes (otherwise,
+ * there won't be a slot allocated for it).
  */

I'd just go with "need to record the type in ..."

Also, I wonder whether the InvalidOid hack in SS_assign_special_param
requires commentary.  It might be safer to use a valid type OID there,
perhaps VOIDOID or INTERNALOID.

regards, tom lane


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

Re: [HACKERS] Add some const decorations to prototypes

[Peter Eisentraut]

On 11/10/17 10:29, Fabien COELHO wrote:
> Or basically all is fine, I'm just nitpicking for nothing, shame on me.
> 
> As I said, I rather like more precise declarations.

committed

-- 
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] parallelize queries containing initplans

[Robert Haas]

On Tue, Nov 7, 2017 at 4:45 AM, Amit Kapila  wrote:
> As mentioned, changed the status of the patch in CF app.

I spent some time reviewing this patch today and found myself still
quite uncomfortable with the fact that it was adding execution-time
work to track the types of parameters - types that would usually not
even be used.  I found the changes to nodeNestLoop.c to be
particularly objectionable, because we could end up doing the work
over and over when it is actually not needed at all, or at most once.
I decided to try instead teaching the planner to keep track of the
types of PARAM_EXEC parameters as they were created, and that seems to
work fine.  See 0001, attached.

0002, attached, is my worked-over version of the rest of the patch.  I
moved the code that serializes and deserializes PARAM_EXEC from
nodeSubplan.c -- which seemed like a strange choice - to
execParallel.c.  I removed the type OID from the serialization format
because there's no reason to do that any more; the worker already
knows the types from the plan.  I did some renaming of the functions
involved and some adjustment of the comments to refer to "PARAM_EXEC
parameters" instead of initPlan parameters, because there's no reason
that I can see why this can only work for initPlans.  A Gather node on
the inner side of a nested loop doesn't sound like a great idea, but I
think this infrastructure could handle it (though it would need some
more planner work).  I broke a lot of long lines in your version of
the patch into multiple lines; please try to be attentive to this
issue when writing patches in general, as it is a bit tedious to go
through and insert line breaks in many places.

Please let me know your thoughts on the attached patches.

Thanks,

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


0001-param-exec-types-v1.patch
Description: Binary data


0002-pq-pushdown-initplan-rebased.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

[HACKERS] [Patch] Log SSL certificate verification errors

[Graham Leggett]

Hi all,

Currently neither the server side nor the client side SSL certificate verify 
callback does anything, leading to potential hair-tearing-out moments.

The following patch to master implements logging of all certificate 
verification failures, as well as (crucially) which certificates failed to 
verify, and at what depth, so the admin can zoom in straight onto the problem 
without any guessing.

The SSL test suite runs without regression:

Little-Net:ssl minfrin$ make check
rm -rf '/Users/minfrin/src/postgres/postgres-trunk'/tmp_install
/bin/sh ../../../config/install-sh -c -d 
'/Users/minfrin/src/postgres/postgres-trunk'/tmp_install/log
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C '../../..' 
DESTDIR='/Users/minfrin/src/postgres/postgres-trunk'/tmp_install install 
>'/Users/minfrin/src/postgres/postgres-trunk'/tmp_install/log/install.log 2>&1
rm -rf '/Users/minfrin/src/postgres/postgres-trunk/src/test/ssl'/tmp_check
/bin/sh ../../../config/install-sh -c -d 
'/Users/minfrin/src/postgres/postgres-trunk/src/test/ssl'/tmp_check
cd . && TESTDIR='/Users/minfrin/src/postgres/postgres-trunk/src/test/ssl' 
PATH="/Users/minfrin/src/postgres/postgres-trunk/tmp_install/tmp/postgresql/bin:$PATH"
 
DYLD_LIBRARY_PATH="/Users/minfrin/src/postgres/postgres-trunk/tmp_install/tmp/postgresql/lib"
 PGPORT='65432' 
PG_REGRESS='/Users/minfrin/src/postgres/postgres-trunk/src/test/ssl/../../../src/test/regress/pg_regress'
 /opt/local/bin/prove -I ../../../src/test/perl/ -I .  t/*.pl
t/001_ssltests.pl .. ok 
All tests successful.
Files=1, Tests=40,  6 wallclock secs ( 0.04 usr  0.01 sys +  2.01 cusr  1.21 
csys =  3.27 CPU)
Result: PASS


Index: src/backend/libpq/be-secure-openssl.c
===
--- src/backend/libpq/be-secure-openssl.c   (revision 61109)
+++ src/backend/libpq/be-secure-openssl.c   (working copy)
@@ -999,9 +999,9 @@
 /*
  * Certificate verification callback
  *
- * This callback allows us to log intermediate problems during
- * verification, but for now we'll see if the final error message
- * contains enough information.
+ * There are 50 ways to leave your lover, and 67 ways to fail
+ * certificate verification. Log details of all failed certificate
+ * verification results.
  *
  * This callback also allows us to override the default acceptance
  * criteria (e.g., accepting self-signed or expired certs), but
@@ -1010,6 +1010,28 @@
 static int
 verify_cb(int ok, X509_STORE_CTX *ctx)
 {
+   char *subject, *issuer;
+   X509 *cert;
+   int err, depth;
+
+   if (!ok)
+   {
+   cert = X509_STORE_CTX_get_current_cert(ctx);
+   err = X509_STORE_CTX_get_error(ctx);
+   depth = X509_STORE_CTX_get_error_depth(ctx);
+
+   subject = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 
0);
+   issuer = X509_NAME_oneline(X509_get_issuer_name(cert), NULL, 0);
+
+   ereport(COMMERROR,
+   (errcode(ERRCODE_PROTOCOL_VIOLATION),
+   errmsg("SSL certificate verification result: %s 
(depth %d, subject '%s', issuer '%s')",
+   X509_verify_cert_error_string(err), 
depth, subject, issuer)));
+
+   OPENSSL_free(subject);
+   OPENSSL_free(issuer);
+   }
+
return ok;
 }
 
Index: src/interfaces/libpq/fe-secure-openssl.c
===
--- src/interfaces/libpq/fe-secure-openssl.c(revision 61109)
+++ src/interfaces/libpq/fe-secure-openssl.c(working copy)
@@ -82,6 +82,8 @@
 
 static bool ssl_lib_initialized = false;
 
+static int ssl_ex_data_index;
+
 #ifdef ENABLE_THREAD_SAFETY
 static long ssl_open_connections = 0;
 
@@ -182,6 +184,7 @@
 */
SOCK_ERRNO_SET(0);
ERR_clear_error();
+   resetPQExpBuffer(>errorMessage);
n = SSL_read(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
 
@@ -200,7 +203,7 @@
if (n < 0)
{
/* Not supposed to happen, so we don't 
translate the msg */
-   printfPQExpBuffer(>errorMessage,
+   appendPQExpBuffer(>errorMessage,
  "SSL_read 
failed but did not provide error information\n");
/* assume the connection is broken */
result_errno = ECONNRESET;
@@ -224,13 +227,13 @@
result_errno = SOCK_ERRNO;
if (result_errno == EPIPE ||
result_errno == ECONNRESET)
-   printfPQExpBuffer(>errorMessage,
+   appendPQExpBuffer(>errorMessage,
 

Re: [HACKERS] Add some const decorations to prototypes

[Peter Eisentraut]

On 11/10/17 11:42, Fabien COELHO wrote:
> After your explanation, and on third thoughts, ISTM that the assignment 
> should not include "const" in the explicit cast, i.e., use
> 
>extern void * msg_func(void);
>const char * msg = (char *) msg_func();
> 
> The variable or field is constant, not what the function returns, so
> 
>const char * msg = (const char *) msg_func();
> 
> does not really make full sense to me, and moreover the compiler does not 
> complain without the const.

The compiler knows how to handle the char * -> const char * case, but
not the char ** -> const char ** case.

-- 
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] Fix bloom WAL tap test

[Tom Lane]

I wrote:
> Is there anything we can do to cut the runtime of the TAP test to
> the point where running it by default wouldn't be so painful?

As an experiment, I tried simply cutting the size of the test table 10X:

diff --git a/contrib/bloom/t/001_wal.pl b/contrib/bloom/t/001_wal.pl
index 1b319c9..566abf9 100644
--- a/contrib/bloom/t/001_wal.pl
+++ b/contrib/bloom/t/001_wal.pl
@@ -57,7 +57,7 @@ $node_standby->start;
 $node_master->safe_psql("postgres", "CREATE EXTENSION bloom;");
 $node_master->safe_psql("postgres", "CREATE TABLE tst (i int4, t text);");
 $node_master->safe_psql("postgres",
-"INSERT INTO tst SELECT i%10, substr(md5(i::text), 1, 1) FROM 
generate_series(1,10) i;"
+"INSERT INTO tst SELECT i%10, substr(md5(i::text), 1, 1) FROM 
generate_series(1,1) i;"
 );
 $node_master->safe_psql("postgres",
"CREATE INDEX bloomidx ON tst USING bloom (i, t) WITH (col1 = 3);");
@@ -72,7 +72,7 @@ for my $i (1 .. 10)
test_index_replay("delete $i");
$node_master->safe_psql("postgres", "VACUUM tst;");
test_index_replay("vacuum $i");
-   my ($start, $end) = (11 + ($i - 1) * 1, 10 + $i * 1);
+   my ($start, $end) = (10001 + ($i - 1) * 1000, 1 + $i * 1000);
$node_master->safe_psql("postgres",
 "INSERT INTO tst SELECT i%10, substr(md5(i::text), 1, 1) FROM 
generate_series($start,$end) i;"
);

This about halved the runtime of the TAP test, and it changed the coverage
footprint not at all according to lcov.  (Said coverage is only marginally
better than what we get without running the bloom TAP test, AFAICT.)

It seems like some effort could be put into both shortening this test
and improving the amount of code it exercises.

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] Fix bloom WAL tap test

[Tom Lane]

Michael Paquier  writes:
> On Thu, Nov 9, 2017 at 7:51 PM, Alexander Korotkov
>  wrote:
>> OK, then so be it :)

> Thanks for the new version. This one, as well as the switch to
> psql_safe in 
> https://www.postgresql.org/message-id/CAPpHfduxgEYF_0BTs-mxGC4=w5sw8rnUbq9BSTp1Wq7=nwr...@mail.gmail.com
> are good for a committer lookup IMO.

The safe_psql change is a clear bug fix, so I've pushed it.

However, as far as adding the TAP test to the standard test suite
goes, I've got two complaints about this patch:

1. It doesn't (I think, can't test) do anything to run the test on
Windows.

2. The TAP test is enormously slower than the standard test.  On my
development workstation, "make installcheck" in contrib/bloom takes
about 0.5 sec; this patch increases that to 11.6 sec.  I'm not too
happy about that as a developer, and even less so as an owner of some
fairly slow buildfarm critters.  I'd say that this might be the
reason the TAP test wasn't part of the standard tests to begin with.

Is there anything we can do to cut the runtime of the TAP test to
the point where running it by default wouldn't be so painful?

regards, tom lane


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

Re: [HACKERS] Add some const decorations to prototypes

[Tom Lane]

Fabien COELHO  writes:
>> LWLockTrancheArray = (char **)
>>  MemoryContextAllocZero(TopMemoryContext,
>> LWLockTranchesAllocated * sizeof(char *));

> After your explanation, and on third thoughts, ISTM that the assignment 
> should not include "const" in the explicit cast,

Can't get terribly excited about that one way or the other.  I think
the statement would be OK as-is, and it would also be fine as

LWLockTrancheArray = (const char **)
MemoryContextAllocZero(TopMemoryContext,
   LWLockTranchesAllocated * sizeof(const char *));

The other two possible combinations are not good of course --- not that
they'd generate invalid code, but that they'd require readers to expend
brain cells convincing themselves that the code wasn't wrong.

> ... and moreover the compiler does not 
> complain without the const.

Arguing on the basis of what your compiler does is a pretty shaky basis.
It's not impossible that someone else's compiler would complain if the
casted-to type isn't identical to the variable's type.  I tend to agree
that a compiler *should* allow "char **" to be cast to "const char **"
silently, but that isn't necessarily what happens in the real world.

regards, tom lane


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

Re: [HACKERS] Add some const decorations to prototypes

[Fabien COELHO]

Hello Tom,


ISTM That there is still at least one strange cast:

+static const char **LWLockTrancheArray = NULL;
+   LWLockTrancheArray = (const char **) // twice



These are not cases of "cheating".  This is just the return value of a
memory allocation function being cast from void * to the appropriate
result type.  That is an orthogonal style decision that I have
maintained in these cases.



Would it make sense that the function returns "const void *", i.e. the
cast is not on the const part but on the pointer type part?


Certainly not -- if malloc-like functions returned "const void *" then
you could never write on allocated space without having casted away
const.


Ok. Sure.


   LWLockTrancheArray = (char **)
   MemoryContextAllocZero(TopMemoryContext,
  LWLockTranchesAllocated * sizeof(char *));

and the reader can see by inspection that the calculation of how much
to allocate (so many char-* items) is consistent with the char-**
result.  It is not necessary to go find the declaration of
LWLockTrancheArray and verify that it's char **, because we trust the
compiler to whine if char ** isn't assignment-compatible with that.
But if we left off the cast and just assigned the function result directly
to the variable, then there would be nothing directly tying the variable's
type to this allocation computation.


Thanks for the reflesher course about the intricacies of "const".

After your explanation, and on third thoughts, ISTM that the assignment 
should not include "const" in the explicit cast, i.e., use


  extern void * msg_func(void);
  const char * msg = (char *) msg_func();

The variable or field is constant, not what the function returns, so

  const char * msg = (const char *) msg_func();

does not really make full sense to me, and moreover the compiler does not 
complain without the const.


--
Fabien.


--
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] Incorrect comment for build_child_join_rel

[Robert Haas]

On Fri, Nov 10, 2017 at 4:34 AM, Etsuro Fujita
 wrote:
> Here is a small patch for $Subject.

Good catch.  Committed.

-- 
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] proposal: psql command \graw

[Pavel Stehule]

2017-11-10 16:38 GMT+01:00 Fabien COELHO :

>
> Hello,
>
> Maybe I'm missing something, but it looks that it could be made to work
>>> without adding another boolean.
>>>
>>
>> The tuples only cannot be disabled, because then other parts print number
>> of rows
>>
>> postgres=# \pset format unaligned
>> Output format is unaligned.
>>
>> postgres=# select 10 as a, 20 as b;
>> a|b
>> 10|20
>> (1 row) <
>>
>
> Argh. Too bad.
>
> I'm not at ease with having two bools which nearly mean the opposite one
> of the other but not exactly... however I'm not sure that there is a
> simpler way out of this, some exception handling is needed one way or the
> other, either within the header or within the footer... Maybe the whole
> topt logic should be reviewed, but that is not the point of this patch.
>

I don't think so it is not correct - this mean tuples only + header.
Probably the best implementation is something three state - all, tuples
only, tuples only and header. But it mean much more changes in psql logic -
not adequate to size of this patch


> So I switched the patch to "ready for committer".
>

Thank you very much

Regards

Pavel


>
> --
> Fabien.
>

Re: [HACKERS] Aggregates push-down to partitions

[Konstantin Knizhnik]

On 10.11.2017 12:15, Ashutosh Bapat wrote:

Maybe in this thread[1] your described problem are solved through

introducing Parallel Append node?

1.
https://www.postgresql.org/message-id/CAJ3gD9dy0K_E8r727heqXoBmWZ83HwLFwdcaSSmBQ1%2BS%2BvRuUQ%40mail.gmail.com

You may want to review [2] and [3] as well.

[2] https://www.postgresql.org/message-id/9666.1491295317@localhost
[3] 
https://www.postgresql.org/message-id/CAM2+6=V64_xhstVHie0Rz=kpeqnljmzt_e314p0jat_oj9m...@mail.gmail.com

Thank you very much for this references.
I applied partition-wise-agg-v6 patches and for partitioned tables it 
works perfectly:


shard=# explain select count(*) from orders;
  QUERY PLAN
---
 Finalize Aggregate  (cost=100415.29..100415.30 rows=1 width=8)
   ->  Append  (cost=50207.63..100415.29 rows=2 width=8)
 ->  Partial Aggregate  (cost=50207.63..50207.64 rows=1 width=8)
   ->  Foreign Scan on orders_0 (cost=101.00..50195.13 
rows=5000 width=0)

 ->  Partial Aggregate  (cost=50207.63..50207.64 rows=1 width=8)
   ->  Foreign Scan on orders_1 (cost=101.00..50195.13 
rows=5000 width=0)

(6 rows)

But I wonder why the same optimization is not applied to normal 
inherited table:


shard=# explain select count(*) from base;
    QUERY PLAN
--
 Aggregate  (cost=44087.99..44088.00 rows=1 width=8)
   ->  Append  (cost=0.00..39079.46 rows=2003414 width=0)
 ->  Seq Scan on base  (cost=0.00..0.00 rows=1 width=0)
 ->  Seq Scan on derived1  (cost=0.00..14425.00 rows=100 
width=0)
 ->  Seq Scan on derived2  (cost=0.00..14425.00 rows=100 
width=0)
 ->  Foreign Scan on derived_fdw  (cost=100.00..212.39 
rows=3413 width=0)

(6 rows)

Are there some principle problems?

--
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

Re: [HACKERS] Add some const decorations to prototypes

[Tom Lane]

Fabien COELHO  writes:
>>> ISTM That there is still at least one strange cast:
 +static const char **LWLockTrancheArray = NULL;
 +   LWLockTrancheArray = (const char **) // twice

>> These are not cases of "cheating".  This is just the return value of a
>> memory allocation function being cast from void * to the appropriate
>> result type.  That is an orthogonal style decision that I have
>> maintained in these cases.

> Would it make sense that the function returns "const void *", i.e. the 
> cast is not on the const part but on the pointer type part?

Certainly not -- if malloc-like functions returned "const void *" then
you could never write on allocated space without having casted away
const.

In any case, what's shown above is not involving any funny stuff,
because the type of LWLockTrancheArray is pointer to non-const
array of pointers to const char strings.  So it's correct to be
assigning a non-const pointer to it.  If it were written like
"const char * const *" then the issues would be quite different.

As for your followup --- yes, we could just omit the cast altogether
and the compiler wouldn't complain, but that is not better style IMO.
The value of the cast really is to document what type we're expecting
the expression to produce.  In context, that statement (today) is

LWLockTrancheArray = (char **)
MemoryContextAllocZero(TopMemoryContext,
   LWLockTranchesAllocated * sizeof(char *));

and the reader can see by inspection that the calculation of how much
to allocate (so many char-* items) is consistent with the char-**
result.  It is not necessary to go find the declaration of
LWLockTrancheArray and verify that it's char **, because we trust the
compiler to whine if char ** isn't assignment-compatible with that.
But if we left off the cast and just assigned the function result directly
to the variable, then there would be nothing directly tying the variable's
type to this allocation computation.

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

[Robert Haas]

On Fri, Nov 10, 2017 at 10:34 AM, Tom Lane  wrote:
> I think as far as that goes, we can just change to "Therefore, by default
> their use is restricted ...".  Then I suggest adding a  para
> after that, with wording along the lines of
>
> It is possible to GRANT use of server-side lo_import and lo_export to
> non-superusers, but careful consideration of the security implications
> is required.  A malicious user of such privileges could easily parlay
> them into becoming superuser (for example by rewriting server
> configuration files), or could attack the rest of the server's file
> system without bothering to obtain database superuser privileges as
> such.  Access to roles having such privilege must therefore be guarded
> just as carefully as access to superuser roles.  Nonetheless, if use
> of server-side lo_import or lo_export is needed for some routine task,
> it's safer to use a role of this sort than full superuser privilege,
> as that helps to reduce the risk of damage from accidental errors.

+1.  That seems like great language to me.

-- 
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] [PATCH] A hook for session start

[Fabrízio de Royes Mello]

On Thu, Nov 9, 2017 at 9:08 PM, Michael Paquier 
wrote:
>
> On Fri, Nov 10, 2017 at 2:32 AM, Fabrízio de Royes Mello
>  wrote:
> > On Thu, Nov 9, 2017 at 12:09 AM, Michael Paquier <
michael.paqu...@gmail.com>
> > wrote:
> >> +++ 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 :)
> >
> > Ooops... my fault... fixed!
> >
> > Thanks again!!
>
> +/* GUC variables */
> +static char *session_hook_username = NULL;
> This causes the module to crash if test_session_hooks.username is not
> set. I would recommend just assigning a default value, say "postgres".
>

Fixed.

New version attached.

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 05c5c19..d3156ad 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 session_start_hook_type session_start_hook;
+extern PGDLLIMPORT session_end_hook_type session_end_hook;
+
 /* GUC-configurable parameters */
 
 typedef enum
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index b7ed0af..a3c8c1e 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -11,6 +11,7 @@ SUBDIRS = \
 		  snapshot_too_old \
 		  test_ddl_deparse \
 		  test_extensions \
+		  test_session_hooks \
 		  test_parser \
 		  test_pg_dump \
 		  test_rbtree \
diff --git a/src/test/modules/test_session_hooks/.gitignore b/src/test/modules/test_session_hooks/.gitignore
new file mode 100644
index 000..5dcb3ff
--- /dev/null
+++ b/src/test/modules/test_session_hooks/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_session_hooks/Makefile b/src/test/modules/test_session_hooks/Makefile
new file mode 100644
index 000..c5c3860
--- /dev/null
+++ b/src/test/modules/test_session_hooks/Makefile
@@ -0,0 +1,21 @@
+# src/test/modules/test_session_hooks/Makefile
+
+MODULES = test_session_hooks
+PGFILEDESC = "test_session_hooks - Test session hooks with an extension"
+
+EXTENSION = test_session_hooks
+DATA = test_session_hooks--1.0.sql
+
+REGRESS = test_session_hooks
+REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_session_hooks/session_hooks.conf
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_session_hooks
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git 

[HACKERS] pg_upgrade to clusters with a different WAL segment size

[Bossart, Nathan]

Hi hackers,

The thread for the recent change to allow setting the WAL segment size at
initdb time [0] included a bit of discussion regarding pg_upgrade [1],
where it was suggested that relaxing an error check (presumably in
check_control_data()) might be enough to upgrade servers to a different WAL
segment size.

We've had success with our initial testing of upgrades to larger WAL
segment sizes, including post-upgrade pgbench runs.  Beyond adjusting
check_control_data(), it looks like the 'pg_resetwal -l' call in
copy_xact_xlog_xid() may need to be adjusted to ensure that the WAL
starting address is set to a valid value.

Allowing changes to the WAL segment size during pg_upgrade seems like a
nice way to avoid needing a dump and load, so I would like to propose
adding support for this.  I'd be happy to submit patches for this in the
next commitfest.

Nathan

[0] 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=fc49e24fa69a15efacd5b8958115ed9c43c48f9a
[1] 
https://www.postgresql.org/message-id/20160826043926.pwkz45ksxpmfn4g6%40alap3.anarazel.de


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

Re: [HACKERS] proposal: psql command \graw

[Fabien COELHO]

Hello,

Maybe I'm missing something, but it looks that it could be made to work 
without adding another boolean.


The tuples only cannot be disabled, because then other parts print number
of rows

postgres=# \pset format unaligned
Output format is unaligned.

postgres=# select 10 as a, 20 as b;
a|b
10|20
(1 row) <


Argh. Too bad.

I'm not at ease with having two bools which nearly mean the opposite one 
of the other but not exactly... however I'm not sure that there is a 
simpler way out of this, some exception handling is needed one way or the 
other, either within the header or within the footer... Maybe the whole 
topt logic should be reviewed, but that is not the point of this patch.


So I switched the patch to "ready for committer".

--
Fabien.


--
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

[Tom Lane]

Michael Paquier  writes:
> That will not sound much as a surprise as I spawned the original
> thread, but like Robert I understand that getting rid of all superuser
> checks is a goal that we are trying to reach to allow admins to have
> more flexibility in handling permissions to a subset of objects.
> Forcing an admin to give full superuser rights to one user willing to
> work only on LOs import and export is a wrong concept.

Right.  I think the question may boil down to how we document this.
The current para reads

The server-side lo_import and
lo_export functions behave considerably differently
from their client-side analogs.  These two functions read and write files
in the server's file system, using the permissions of the database's
owning user.  Therefore, their use is restricted to superusers.  In
contrast, the client-side import and export functions read and write files
in the client's file system, using the permissions of the client program.
The client-side functions do not require superuser privilege.

I think as far as that goes, we can just change to "Therefore, by default
their use is restricted ...".  Then I suggest adding a  para
after that, with wording along the lines of

It is possible to GRANT use of server-side lo_import and lo_export to
non-superusers, but careful consideration of the security implications
is required.  A malicious user of such privileges could easily parlay
them into becoming superuser (for example by rewriting server
configuration files), or could attack the rest of the server's file
system without bothering to obtain database superuser privileges as
such.  Access to roles having such privilege must therefore be guarded
just as carefully as access to superuser roles.  Nonetheless, if use
of server-side lo_import or lo_export is needed for some routine task,
it's safer to use a role of this sort than full superuser privilege,
as that helps to reduce the risk of damage from accidental errors.

We could expand that by mentioning the possibility of wrapper functions,
but it seems long enough already.

Comments?

regards, tom lane


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

Re: [HACKERS] Add some const decorations to prototypes

[Fabien COELHO]

Would it make sense that the function returns "const void *", i.e. the cast 
is not on the const part but on the pointer type part?


Or maybe you do not really need a cast, the following code does not 
generate any warning when compiled with clang & gcc.


 #include 

 // const void * would be ok as well
 void * msg_fun(void)
 {
   return "hello world";
 }

 int main(void)
 {
   const char * msg = msg_fun();
   printf("message: %s\n", msg);
   return 0;
 }

Or basically all is fine, I'm just nitpicking for nothing, shame on me.

As I said, I rather like more precise declarations.

--
Fabien.


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

Re: [HACKERS] Add some const decorations to prototypes

[Fabien COELHO]

ISTM That there is still at least one strange cast:

  +static const char **LWLockTrancheArray = NULL;
  +   LWLockTrancheArray = (const char **) // twice


These are not cases of "cheating".  This is just the return value of a
memory allocation function being cast from void * to the appropriate
result type.  That is an orthogonal style decision that I have
maintained in these cases.


Ok. I'm at the limit of my C abilities.

Your answer is about void * vs char *, I'm okay with that.

My question was about no const / const in the same operation.

Would it make sense that the function returns "const void *", i.e. the 
cast is not on the const part but on the pointer type part?


--
Fabien.


--
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: multivariate histograms and MCV lists

[Mark Dilger]

> On Sep 12, 2017, at 2:06 PM, Tomas Vondra  
> wrote:
> 
> Attached is an updated version of the patch, dealing with fallout of
> 821fb8cdbf700a8aadbe12d5b46ca4e61be5a8a8 which touched the SGML
> documentation for CREATE STATISTICS.

Your patches need updating.

Tom's commit 471d55859c11b40059aef7dd82f82b3a0dc338b1 changed 
src/bin/psql/describe.c, which breaks your 0001-multivariate-MCV-lists.patch.gz
file.

I reviewed the patch a few months ago, and as I recall, it looked good to me.
I should review it again before approving it, though.

mark



-- 
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

[Robert Haas]

On Fri, Nov 10, 2017 at 5:44 AM, Amit Kapila  wrote:
> I am seeing the assertion failure as below on executing the above
> mentioned Create statement:
>
> TRAP: FailedAssertion("!(!(tup->t_data->t_infomask & 0x0008))", File:
> "heapam.c", Line: 2634)
> server closed the connection unexpectedly
> This probably means the server terminated abnormally

OK, I see it now.  Not sure why I couldn't reproduce this before.

I think the problem is not actually with the code that I just wrote.
What I'm seeing is that the slot descriptor's tdhasoid value is false
for both the funnel slot and the result slot; therefore, we conclude
that no projection is needed to remove the OIDs.  That seems to make
sense: if the funnel slot doesn't have OIDs and the result slot
doesn't have OIDs either, then we don't need to remove them.
Unfortunately, even though the funnel slot descriptor is marked
tdhashoid = false, the tuples being stored there actually do have
OIDs.  And that is because they are coming from the underlying
sequential scan, which *also* has OIDs despite the fact that tdhasoid
for it's slot is false.

This had me really confused until I realized that there are two
processes involved.  The problem is that we don't pass eflags down to
the child process -- so in the user backend, everybody agrees that
there shouldn't be OIDs anywhere, because EXEC_FLAG_WITHOUT_OIDS is
set.  In the parallel worker, however, it's not set, so the worker
feels free to do whatever comes naturally, and in this test case that
happens to be returning tuples with OIDs.  Patch for this attached.

I also noticed that the code that initializes the funnel slot is using
its own PlanState rather than the outer plan's PlanState to call
ExecContextForcesOids.  I think that's formally incorrect, because the
goal is to end up with a slot that is the same as the outer plan's
slot.  It doesn't matter because ExecContextForcesOids doesn't care
which PlanState it gets passed, but the comments in
ExecContextForcesOids imply that somebody it might, so perhaps it's
best to clean that up.  Patch for this attached, too.

And here are the other patches again, too.

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


0001-pass-eflags-to-worker-v1.patch
Description: Binary data


0002-forces-oids-neatnikism-v1.patch
Description: Binary data


0003-skip-gather-project-v2.patch
Description: Binary data


0004-shm-mq-less-spinlocks-v2.patch
Description: Binary data


0005-shm-mq-reduce-receiver-latch-set-v1.patch
Description: Binary data


0006-remove-memory-leak-protection-v1.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] Variable substitution in psql backtick expansion

[Pavel Stehule]

Hi

I am sending a review of last patch psql-server-version-1.patch.gz

This patch is trivial - the most big problem is choosing correct name for
GUC. I am thinking so server_version_raw is acceptable.

I had to fix doc - see attached updated patch

All tests passed.

I'll mark this patch as ready for commiters

Regards

Pavel
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d360fc4d58..924766fce7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7956,8 +7956,22 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
   
   

-Reports the version number of the server as an integer. It is determined
-by the value of PG_VERSION_NUM when building the server.
+Reports the version number of the server as a short string. It is determined
+by the value of PG_VERSION when building the server.
+   
+  
+ 
+
+ 
+  server_version_raw (string)
+  
+   server_version_raw configuration parameter
+  
+  
+  
+   
+Reports the version of the server as a long string. It is determined
+by the value of PG_VERSION_STR when building the server.

   
  
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e520cdf3ba..50d6f0a8fc 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3770,11 +3770,14 @@ bar
   
 
   
+SERVER_VERSION
 SERVER_VERSION_NAME
 SERVER_VERSION_NUM
 
 
-The server's version number as a string, for
+The server's version number as a long string, for
+example PostgreSQL 11devel ...,
+as a short string, for
 example 9.6.2, 10.1 or 11beta1,
 and in numeric form, for
 example 90602 or 11.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c4c1afa084..49ff61246f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -500,6 +500,7 @@ static char *locale_collate;
 static char *locale_ctype;
 static char *server_encoding_string;
 static char *server_version_string;
+static char *server_version_raw_string;
 static int	server_version_num;
 static char *timezone_string;
 static char *log_timezone_string;
@@ -3295,6 +3296,18 @@ static struct config_string ConfigureNamesString[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		/* Can't be set in postgresql.conf */
+		{"server_version_raw", PGC_INTERNAL, PRESET_OPTIONS,
+			gettext_noop("Shows the server version string."),
+			NULL,
+			GUC_REPORT | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+		},
+		_version_raw_string,
+		PG_VERSION_STR,
+		NULL, NULL, NULL
+	},
+
 	{
 		/* Not for general use --- used by SET ROLE */
 		{"role", PGC_USERSET, UNGROUPED,
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 8cc4de3878..cfac89c8da 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3210,7 +3210,8 @@ void
 SyncVariables(void)
 {
 	char		vbuf[32];
-	const char *server_version;
+	const char *server_version,
+			   *server_version_raw;
 
 	/* get stuff from connection */
 	pset.encoding = PQclientEncoding(pset.db);
@@ -3237,6 +3238,17 @@ SyncVariables(void)
 	snprintf(vbuf, sizeof(vbuf), "%d", pset.sversion);
 	SetVariable(pset.vars, "SERVER_VERSION_NUM", vbuf);
 
+	server_version_raw = PQparameterStatus(pset.db, "server_version_raw");
+	/* fall back again */
+	if (!server_version_raw)
+	{
+		snprintf(vbuf, sizeof(vbuf), "PostgreSQL ");
+		formatPGVersionNumber(pset.sversion, true, vbuf + strlen(vbuf),
+			  sizeof(vbuf) - strlen(vbuf));
+		server_version_raw = vbuf;
+	}
+	SetVariable(pset.vars, "SERVER_VERSION", server_version_raw);
+
 	/* send stuff to it, too */
 	PQsetErrorVerbosity(pset.db, pset.verbosity);
 	PQsetErrorContextVisibility(pset.db, pset.show_context);
@@ -3255,6 +3267,7 @@ UnsyncVariables(void)
 	SetVariable(pset.vars, "HOST", NULL);
 	SetVariable(pset.vars, "PORT", NULL);
 	SetVariable(pset.vars, "ENCODING", NULL);
+	SetVariable(pset.vars, "SERVER_VERSION", NULL);
 	SetVariable(pset.vars, "SERVER_VERSION_NAME", NULL);
 	SetVariable(pset.vars, "SERVER_VERSION_NUM", NULL);
 }
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 5335a91440..0418779f79 100644
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -280,6 +280,10 @@ pqSetenvPoll(PGconn *conn)
 		{
 			char	   *ptr;
 
+			/* keep returned value */
+			pqSaveParameterStatus(conn, "server_version_raw",
+  val);
+
 			/* strip off PostgreSQL part */
 			val += 11;
 
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..eabb990d4e 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,14 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select 

Re: [HACKERS] Add some const decorations to prototypes

[Peter Eisentraut]

On 11/4/17 16:50, Fabien COELHO wrote:
>>> Just leave it as char*.  If you change the endptr argument you're going to
>>> force every call site to change their return variable, and some of them
>>> would end up having to cast away the const on their end.
>>
>> OK, here is an updated patch with the controversial bits removed.
> 
> I'm in general favor in helping compilers, but if you have to cheat.
> 
> ISTM That there is still at least one strange cast:
> 
>   +static const char **LWLockTrancheArray = NULL;
>   +   LWLockTrancheArray = (const char **) // twice

These are not cases of "cheating".  This is just the return value of a
memory allocation function being cast from void * to the appropriate
result type.  That is an orthogonal style decision that I have
maintained in these cases.

-- 
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] Transform for pl/perl

[Pavel Stehule]

Hi

2017-10-24 14:27 GMT+02:00 Anthony Bykov :

> There are some moments I should mention:
> 1. {"1":1}::jsonb is transformed into HV {"1"=>"1"}, while
> ["1","2"]::jsonb is transformed into AV ["1", "2"]
>
> 2. If there is a numeric value appear in jsonb, it will be transformed
> to SVnv through string (Numeric->String->SV->SVnv). Not the best
> solution, but as far as I understand this is usual practise in
> postgresql to serialize Numerics and de-serialize them.
>
> 3. SVnv is transformed into jsonb through string
> (SVnv->String->Numeric).
>
> An example may also be helpful to understand extension. So, as an
> example, function "test" transforms incoming jsonb into perl,
> transforms it back into jsonb and returns it.
>
> create extension jsonb_plperl cascade;
>
> create or replace function test(val jsonb)
> returns jsonb
> transform for type jsonb
> language plperl
> as $$
> return $_[0];
> $$;
>
> select test('{"1":1,"example": null}'::jsonb);
>
>
I am looking to this patch:

1. the patch contains some artefacts - look the word "hstore"

2. I got lot of warnings


make[1]: Vstupuje se do adresáře
„/home/pavel/src/postgresql/contrib/jsonb_plperl“
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-g -ggdb -Og -g3 -fno-omit-frame-pointer -fPIC -I../../src/pl/plperl -I.
-I. -I../../src/include  -D_GNU_SOURCE -I/usr/include/libxml2
-I/usr/lib64/perl5/CORE  -c -o jsonb_plperl.o jsonb_plperl.c
jsonb_plperl.c: In function ‘SV_FromJsonbValue’:
jsonb_plperl.c:83:9: warning: ‘result’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  return (result);
 ^
jsonb_plperl.c: In function ‘SV_FromJsonb’:
jsonb_plperl.c:95:10: warning: ‘object’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  HV *object;
  ^~
In file included from /usr/lib64/perl5/CORE/perl.h:5644:0,
 from ../../src/pl/plperl/plperl.h:52,
 from jsonb_plperl.c:17:
/usr/lib64/perl5/CORE/embed.h:404:19: warning: ‘value’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
 #define newRV(a)  Perl_newRV(aTHX_ a)
   ^~
jsonb_plperl.c:101:10: note: ‘value’ was declared here
  SV *value;
  ^
gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-g -ggdb -Og -g3 -fno-omit-frame-pointer -fPIC -shared -o jsonb_plperl.so
jsonb_plperl.o  -L../../src/port -L../../src/common -Wl,--as-needed
-Wl,-rpath,'/usr/lib64/perl5/CORE',--enable-new-dtags   -Wl,-z,relro
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fstack-protector-strong
-L/usr/local/lib  -L/usr/lib64/perl5/CORE -lperl -lpthread -lresolv -lnsl
-ldl -lm -lcrypt -lutil -lc
make[1]: Opouští se adresář
„/home/pavel/src/postgresql/contrib/jsonb_plperl“

[pavel@nemesis contrib]$ gcc --version
gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2)
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

3. regress tests passed

4. There are not any documentation - probably it should be part of PLPerl

5. The regress tests doesn't coverage other datatypes than numbers. I miss
boolean, binary, object, ... Maybe using data::dumper or some similar can
be interesting

Note - it is great extension, I am pleasured so transformations are used.

Regards

Pavel







>
> --
> 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_basebackup --progress output for batch execution

[Martín Marqués]

Hi,

Thanks for having a look at this patch.

2017-11-09 20:55 GMT-03:00 Jeff Janes :
> On Fri, Sep 29, 2017 at 4:00 PM, Martin Marques
>  wrote:
>>
>> Hi,
>>
>> Some time ago I had to work on a system where I was cloning a standby
>> using pg_basebackup, that didn't have screen or tmux. For that reason I
>> redirected the output to a file and ran it with nohup.
>>
>> I normally (always actually ;) ) run pg_basebackup with --progress and
>> --verbose so I can follow how much has been done. When done on a tty you
>> get a nice progress bar with the percentage that has been cloned.
>>
>> The problem came with the execution and redirection of the output, as
>> the --progress option will write a *very* long line!
>>
>> Back then I thought of writing a patch (actually someone suggested I do
>> so) to add a --batch-mode option which would change the behavior
>> pg_basebackup has when printing the output messages.
>
>
>
> While separate lines in the output file is better than one very long line,
> it still doesn't seem so useful.  If you aren't watching it in real time,
> then you really need to have a timestamp on each line so that you can
> interpret the output.  The lines are about one second apart, but I don't
> know robust that timing is under all conditions.

I kind of disagree with your view here.

If the cloning process takes many hours to complete (in my case, it
was around 12 hours IIRC) you might want to peak at the log every now
and then with tail.

I do agree on adding a timestamp prefix to each line, as it's not
clear from the code how often progress_report() is called.

> I think I agree with Arthur that I'd rather have the decision made by
> inspecting whether output is going to a tty, rather than by adding another
> command line option.  But maybe that is not detected robustly enough across
> all platforms and circumstances?

In this case, Arthur's idea is good, but would make the patch less
generic/flexible for the end user.

That's why I tried to reproduce what top does when executed with -b
(Batch mode operation). There, it's the end user who decides how the
output is formatted (well, saying it decides on formatting a bit of an
overstatement, but you get it ;) )

An example where using isatty() might fail is if you run pg_basebackup
from a tty but redirect the output to a file, I believe that in that
case isatty() will return true, but it's very likely that the user
might want batch mode output.

But maybe we should also add Arthurs idea anyway (when not in batch
mode), as it seems pretty lame to output progress in one line if you
are not in a tty.

Thoughts?

-- 
Martín Marquéshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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] Another oddity in handling of WCO constraints in postgres_fdw

[Etsuro Fujita]

(2017/11/01 11:16), Robert Haas wrote:

On Wed, Oct 4, 2017 at 5:58 PM, Ashutosh Bapat
  wrote:

The view with WCO is local but the modification which violates WCO is
being made on remote server by a trigger on remote table. Trying to
control that doesn't seem to be a good idea, just like we can't
control what rows get inserted on the foreign server when they violate
local constraints.


I think that's a fair point.


For local constraints on foreign tables, it's the user's responsibility 
to ensure that those constraints matches the remote side, so we don't 
need to ensure those constraints locally.  But I'm not sure if the same 
thing applies to WCOs on views defined on foreign tables, because in 
some case it's not possible to impose constraints on the remote side 
that match those WCOs, as I explained before.


Best regards,
Etsuro Fujita


--
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] Runtime Partition Pruning

[amul sul]

On Thu, Nov 9, 2017 at 4:48 PM, Beena Emerson  wrote:
> Hello all,
>
> Here is the updated patch which is rebased over v10 of Amit Langote's
> path towards faster pruning patch [1]. It modifies the PartScanKeyInfo
> struct to hold expressions which is then evaluated by the executor to
> fetch the correct partitions using the function.
>

Hi Beena,

I have started looking into your patch, here few initial comments
for your 0001 patch:

1.
 351 + * Evaluate and store the ooutput of ExecInitExpr for each
of the keys.

Typo: ooutput

2.
 822 +   if (IsA(constexpr, Const) &_runtime)
 823 +   continue;
 824 +
 825 +   if (IsA(constexpr, Param) &&!is_runtime)
 826 +   continue;
 827 +

 Add space after '&&'

 3.
 1095 +* Generally for appendrel we don't fetch the clause from the the

Typo: Double 'the'

4.
 272 
-/*-
 273 + 
/*-

 Unnecessary hunk.

5.
 313 +   Node   *n =
eval_const_expressions_from_list(estate->es_param_list_info, val);
 314 +

Crossing 80 column window.  Same at line # 323 & 325

6.
 315 +   keys->eqkeys_datums[i++] = ((Const *) n)->constvalue;

Don’t we need a check for IsA(n, Const) or assert ?

7.
1011 +   if (prmList)
1012 +   context.boundParams = prmList;  /* bound Params */
1013 +   else
1014 +   context.boundParams = NULL;

No need of prmList null check, context.boundParams = prmList; is enough.

8.  It would be nice if you create a separate patch where you are moving
PartScanKeyInfo and exporting function declaration.

9.  Could you please add few regression tests, that would help in
review & testing.

10. Could you please rebase your patch against latest "path toward faster
partition pruning" patch by Amit.

Regards,
Amul


-- 
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 Fri, Nov 10, 2017 at 9:48 AM, Amit Kapila  wrote:
> On Fri, Nov 10, 2017 at 8:36 AM, Robert Haas  wrote:
>> On Thu, Nov 9, 2017 at 9:31 PM, Amit Kapila  wrote:
>>> Have you set force_parallel_mode=regress; before running the
>>> statement?
>>
>> Yes, I tried that first.
>>
>>> If so, then why you need to tune other parallel query
>>> related parameters?
>>
>> Because I couldn't get it to break the other way, I then tried this.
>>
>> Instead of asking me what I did, can you tell me what I need to do?
>> Maybe a self-contained reproducible test case including exactly what
>> goes wrong on your end?
>>
>
> I think we are missing something very basic because you should see the
> failure by executing that statement in force_parallel_mode=regress
> even in a freshly created database.
>

I am seeing the assertion failure as below on executing the above
mentioned Create statement:

TRAP: FailedAssertion("!(!(tup->t_data->t_infomask & 0x0008))", File:
"heapam.c", Line: 2634)
server closed the connection unexpectedly
This probably means the server terminated abnormally


-- 
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

[HACKERS] No mention of CREATE STATISTICS in event trigger docs

[David Rowley]

A patch to fix this is attached.

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


event_trigger_statistics_doc.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

[HACKERS] Incorrect comment for build_child_join_rel

[Etsuro Fujita]

Hi,

Here is a small patch for $Subject.

Best regards,
Etsuro Fujita
*** a/src/backend/optimizer/util/relnode.c
--- b/src/backend/optimizer/util/relnode.c
***
*** 676,683  build_join_rel(PlannerInfo *root,
   * 'sjinfo': child-join context info
   * 'restrictlist': list of RestrictInfo nodes that apply to this particular
   *pair of joinable relations
!  * 'join_appinfos': list of AppendRelInfo nodes for base child relations
!  *involved in this join
   */
  RelOptInfo *
  build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
--- 676,682 
   * 'sjinfo': child-join context info
   * 'restrictlist': list of RestrictInfo nodes that apply to this particular
   *pair of joinable relations
!  * 'jointype' is the join type (inner, left, full, etc)
   */
  RelOptInfo *
  build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,

-- 
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_basebackup --progress output for batch execution

[Arthur Zakirov]

On Thu, Nov 09, 2017 at 03:55:36PM -0800, Jeff Janes wrote:
> 
> I think I agree with Arthur that I'd rather have the decision made by
> inspecting whether output is going to a tty, rather than by adding another
> command line option.  But maybe that is not detected robustly enough across
> all platforms and circumstances?
> 

isatty() is used within Postgres code already (for example, pg_upgrade/util.c).
However, it seems that on Windows isatty() is deprecated and it is recommended 
to use _isatty(). Moreover, on Windows it can give false positive result [1], 
if I'm not mistaken:

> The _isatty function determines whether fd is associated with a character 
> device (a terminal, console, printer, or serial port).

1 - https://msdn.microsoft.com/en-us/library/f4s0ddew(v=vs.140).aspx

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
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

Re: [HACKERS] Aggregates push-down to partitions

[Ashutosh Bapat]

On Fri, Nov 10, 2017 at 12:20 AM, Maksim Milyutin  wrote:
> Hi Konstantin!

>> I wonder if somebody already investigate this problem or working in this
>> direction.
>> May be there are already some patches proposed?
>> I have searched hackers archive, but didn't find something relevant...
>> Are there any suggestions about the best approach to implement this
>> feature?
>>
>
> Maybe in this thread[1] your described problem are solved through
> introducing Parallel Append node?
>
> 1.
> https://www.postgresql.org/message-id/CAJ3gD9dy0K_E8r727heqXoBmWZ83HwLFwdcaSSmBQ1%2BS%2BvRuUQ%40mail.gmail.com

You may want to review [2] and [3] as well.

[2] https://www.postgresql.org/message-id/9666.1491295317@localhost
[3] 
https://www.postgresql.org/message-id/CAM2+6=V64_xhstVHie0Rz=kpeqnljmzt_e314p0jat_oj9m...@mail.gmail.com
-- 
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