Re: [HACKERS] Multi column range partition table

[Robert Haas]

On Sun, Jul 9, 2017 at 2:42 AM, Dean Rasheed  wrote:
> On 6 July 2017 at 22:43, Joe Conway  wrote:
>> I agree we should get this right the first time and I also agree with
>> Dean's proposal, so I guess I'm a +2
>
> On 7 July 2017 at 03:21, Amit Langote  wrote:
>> +1 to releasing this syntax in PG 10.
>
> So, that's 3 votes in favour of replacing UNBOUNDED with
> MINVALUE/MAXVALUE for range partition bounds in PG 10. Not a huge
> consensus, but no objections either. Any one else have an opinion?
>
> Robert, have you been following this thread?

Uh, no.  Sorry.  I agree that it's a big problem that (10, UNBOUNDED)
interpreted as a maximum value means first_column <= 10 and when
interpreted as a minimum value means first_column >= 10, because those
things aren't opposites of each other.  I guess the proposal here
would make (10, MAXVALUE) as a maximum value mean first_column <= 10
and as a minimum would mean first_column > 10, and contrariwise for
MINVALUE.  That seems to restore the intended design principle of the
system, which is good, but...

...originally, Amit proposed to attach a postfix INCLUSIVE or
EXCLUSIVE to each bound specification, and this does feel like a bit
of a back door to the same place, kinda.  A partition defined to run
from (10, MAXVALUE) TO (11, MAXVALUE) is a lot like a partition
defined to run from (10) EXCLUSIVE to (11) EXCLUSIVE.  And if we
eventually decide to allow that, then what will be the difference
between a partition which starts at (10, MAXVALUE) EXCLUSIVE and one
which starts from (10, MAXVALUE) INCLUSIVE?

I haven't thought through this well enough to be sure that there's any
problem with what is being proposed, and I definitely don't have a
better solution off the top of my head, but I feel slightly nervous.

Apologies again for the slow response - will update again by Monday.

-- 
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] Subscription code improvements

[Masahiko Sawada]

On Wed, Jul 12, 2017 at 11:19 AM, Masahiko Sawada  wrote:
> On Sat, Jul 8, 2017 at 5:19 AM, Petr Jelinek
>  wrote:
>> Hi,
>>
>> I have done some review of subscription handling (well self-review) and
>> here is the result of that (It's slightly improved version from another
>> thread [1]).
>
> Thank you for working on this and patches!
>
>> I split it into several patches:
>>
>> 0001 - Makes subscription worker exit nicely when there is subscription
>> missing (ie was removed while it was starting) and also makes disabled
>> message look same as the message when disabled state was detected while
>> worker is running. This is mostly just nicer behavior for user (ie no
>> unexpected errors in log when you just disabled the subscription).
>>
>> 0002 - This is bugfix - the sync worker should exit when waiting for
>> apply and apply dies otherwise there is possibility of not being
>> correctly synchronized.
>>
>> 0003 - Splits the schizophrenic SetSubscriptionRelState function into
>> two which don't try to do broken upsert and report proper errors instead.
>>
>> 0004 - Solves the issue which Masahiko Sawada reported [2] about ALTER
>> SUBSCRIPTION handling of workers not being transactional - we now do
>> killing of workers transactionally (and we do the same when possible in
>> DROP SUBSCRIPTION).
>>
>> 0005 - Bugfix to allow syscache access to subscription without being
>> connected to database - this should work since subscription is pinned
>> catalog, it wasn't caught because nothing in core is using it (yet).
>>
>> 0006 - Makes the locking of subscriptions more granular (this depends on
>> 0005). This removes the ugly AccessExclusiveLock on the pg_subscription
>> catalog from DROP SUBSCRIPTION and also solves some potential race
>> conditions between launcher, ALTER SUBSCRIPTION and
>> process_syncing_tables_for_apply(). Also simplifies memory handling in
>> launcher as well as logicalrep_worker_stop() function. This basically
>> makes subscriptions behave like every other object in the database in
>> terms of locking.
>>
>> Only the 0002, 0004 and 0005 are actual bug fixes, but I'd still like to
>> get it all into PG10 as especially the locking now behaves really
>> differently than everything else and that does not seem like a good idea.
>>
>
> I'm now planing to review 0002, 0004 and 0005 patches first as they
> are bug fixes.

I've reviewed 0002, 0004 and 0005 patches briefly. Here are some comments.

--
0002-Exit-in-sync-worker-if-relation-was-removed-during-s.patch

As Petr mentioned, if the table subscription is removed before the
table sync worker gets the subscription relation entry,
GetSubscriptionRelState could returns SUBREL_STATE_UNKNOWN and this
issue happens.
Actually we now can handle this case properly by commit
8dc7c338129d22a52d4afcf2f83a73041119efda. So this seems to be an
improvement and not be a release blocker. Therefore for this patch, I
think it's better to do ereport in the
switch(MyLogicalRepWorker->relstate) block.

BTW, since missing_ok of GetSubscriptionRelState() is always set to
true I think that we can remove it.
Also because the reason I mention at later part this fix might not be necessary.

--
0004-Only-kill-sync-workers-at-commit-time-in-SUBSCRIPTIO.patch

+   /*
+* If we are dropping slot, stop all the subscription workers
immediately
+* so that the slot is accessible, otherwise just shedule the
stop at the
+* end of the transaction.
+*
+* New workers won't be started because we hold exclusive lock on the
+* subscription till the end of transaction.
+*/
+   LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
+   subworkers = logicalrep_sub_workers_find(subid, false);
+   LWLockRelease(LogicalRepWorkerLock);
+   foreach (lc, subworkers)
+   {
+   LogicalRepWorker *w = (LogicalRepWorker *) lfirst(lc);
+   if (sub->slotname)
+   logicalrep_worker_stop(w->subid, w->relid);
+   else
+   logicalrep_worker_stop_at_commit(w->subid, w->relid);
+   }
+   list_free(subworkers);

I think if we kill the all workers including table sync workers then
the fix by 0002 patch is not necessary actually, because the table
sync worker will not see that the subscription relation state has been
removed.
Also, logicalrep_sub_workers_find() function is defined in 0006 patch
but it would be better to move it to 0004 patch.

--
0005-Allow-syscache-access-to-subscriptions-in-database-l.patch

Do we need to update the comment at the top of IndexScanOK()?

To summary, I think we now have only one issue; ALTER SUBSCRIPTION is
not transactional, 0004 patch is addressing this issue . 0002 patch
seems an improvement patch to me, and it might be resolved by 0004
patch. 0005 patch is required by 0006 patch which is an improvement
patch. Am I missing something?

Regards,

--
Masahiko Sawada

Re: [HACKERS] SCRAM auth and Pgpool-II

[Chapman Flack]

On 07/13/17 21:54, Tatsuo Ishii wrote:
>>> The comment in pg_hba.conf.sample seem to prefer md5 over clear text
>>> password.
>>>
>>> # Note that "password" sends passwords in clear text; "md5" or
>>> # "scram-sha-256" are preferred since they send encrypted passwords.
>>
>> Should that be reworded to eliminate "md5"? I'd consider "scram-sha-256"
>> suitable over a clear channel, but I've never recommended "md5" for that.
> 
> I don't think so unless clear text password is superior than md5.

Neither is suitable on an unencrypted channel (as has been repeatedly
observed back to 2005 at least [1], so I guess I'm not spilling the beans).
At last, scram-sha-256 is an option that is believable for that use.

So, allowing that neither "password" nor "md5" should ever be used on
an unencrypted channel, as long as the channel is encrypted they are both
protected (by the channel encryption) from eavesdropping, so they score
a tie on that dimension. For a tiebreaker, you could look at the
consequences of revealing rolpassword from pg_authid. On that dimension,
with "md5" you have revealed a password-equivalent, while with "password"
you have not [2], so on that dimension "password" indeed is superior to
"md5".

-Chap

[1]: https://www.postgresql.org/message-id/8764ygc7i6.fsf%40stark.xeocode.com
[2]:
https://www.postgresql.org/message-id/20050421190637.GF29028%40ns.snowman.net


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

[HACKERS] Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

[Haribabu Kommi]

On Fri, Jul 14, 2017 at 2:54 AM, Heikki Linnakangas  wrote:

> On 05/03/2017 07:32 AM, Haribabu Kommi wrote:
>
>> [Adding -hackers mailing list]
>>
>> On Fri, Apr 28, 2017 at 6:28 PM,  wrote:
>>
>> The following bug has been logged on the website:
>>>
>>> Bug reference:  14634
>>> Logged by:  Henry Boehlert
>>> Email address:  henry_boehl...@agilent.com
>>> PostgreSQL version: 9.6.2
>>> Operating system:   Windows Server 2012 R2 6.3.9600
>>> Description:
>>>
>>> Executing command pg_basebackup -D -F t writes its output to stdout,
>>> which
>>> is open in text mode, causing LF to be converted to CR LF thus corrupting
>>> the resulting archive.
>>>
>>> To write the tar to stdout, on Windows stdout's mode should be
>>> temporarily
>>> switched to binary.
>>>
>>> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
>>>
>>>
>> Thanks for reporting the issue.
>> With the attached patch, I was able to extract the tar file that gets
>> generated when the tar file is written into stdout. I tested the
>> the compressed tar also.
>>
>> This bug needs to be fixed in back branches also.
>>
>
> Seems reasonable. One question:
>
> In the patch, you used "_setmode" function, while the calls in
> src/bin/pg_dump/pg_backup_archiver.c use "setmode". There are a few
> places in the backend that also use "_setmode". What's the difference?
> Should we change some of them to be consistent?
>

Actually there is no functional difference between these two functions.
one is a POSIX variant and another one is ISO C++ variant [1]. The support
of POSIX variant is deprecated in windows, because of this reason we should
use the _setmode instead of setmode.

I attached the patch to change the pg_dump code to use _setmode function
instead of _setmode to be consistent with other functions.

[1] -
https://docs.microsoft.com/en-gb/cpp/c-runtime-library/reference/posix-setmode


Regards,
Hari Babu
Fujitsu Australia


0002-Replace-setmode-with-_setmode-function.patch
Description: Binary data


0001-pg_basebackup-windows-tar-mode-to-stdout-fix.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] SCRAM auth and Pgpool-II

[Tatsuo Ishii]

>> The comment in pg_hba.conf.sample seem to prefer md5 over clear text
>> password.
>> 
>> # Note that "password" sends passwords in clear text; "md5" or
>> # "scram-sha-256" are preferred since they send encrypted passwords.
> 
> Should that be reworded to eliminate "md5"? I'd consider "scram-sha-256"
> suitable over a clear channel, but I've never recommended "md5" for that.

I don't think so unless clear text password is superior than md5.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] Inadequate infrastructure for NextValueExpr

[Thomas Munro]

On Fri, Jul 14, 2017 at 9:34 AM, Tom Lane  wrote:
> Somebody decided they could add a new primnode type without bothering to
> build out very much infrastructure for it.  Thus:
>
> regression=# create table foo (f1 int, f2 int generated always as identity);
> CREATE TABLE
> regression=# insert into foo values(1);
> INSERT 0 1
> regression=# explain verbose insert into foo values(1);
> ERROR:  unrecognized node type: 146
>
> because ruleutils.c has never heard of NextValueExpr.  The lack of
> outfuncs/readfuncs support for it is rather distressing as well.
> That doesn't break parallel queries today, because (I think)
> you can't get one of these nodes in a parallelizable query, but it
> is going to cause problems for debugging.  It will also break
> (more or less) pg_stat_statements.  I also wonder whether costsize.c
> oughtn't be charging some estimated execution cost for it.

I wrote a script to cross-check various node handling functions and it told me:

T_NextValueExpr not handled in outfuncs.c
T_ObjectWithArgs not handled in outfuncs.c
T_AccessPriv not handled in outfuncs.c
T_CreateOpClassItem not handled in outfuncs.c
T_FunctionParameter not handled in outfuncs.c
T_InferClause not handled in outfuncs.c
T_OnConflictClause not handled in outfuncs.c
T_RoleSpec not handled in outfuncs.c
T_PartitionCmd not handled in outfuncs.c
T_NamedTuplestoreScan can be produced by outfuncs.c with tagname
NAMEDTUPLESTORESCAN but that tagname is not recognized by readfuncs.c
T_Alias not handled in ruleutils.c
T_RangeVar not handled in ruleutils.c
T_Expr not handled in ruleutils.c
T_CaseWhen not handled in ruleutils.c
T_TargetEntry not handled in ruleutils.c
T_RangeTblRef not handled in ruleutils.c
T_JoinExpr not handled in ruleutils.c
T_FromExpr not handled in ruleutils.c
T_OnConflictExpr not handled in ruleutils.c
T_IntoClause not handled in ruleutils.c
T_NextValueExpr not handled in ruleutils.c

It classifies all node types into categories PLAN NODES, PRIMITIVE
NODES, ... using the per-group comments in nodes.h, and then checks
some rules that I inferred (obviously incorrectly) about which of
those categories should be handled by copy, out, equal, read and
get_rule_expr functions and also checks that readfuncs.c and
outfuncs.c agree on the name string.  It needs some work though:
anyone got any ideas on how to improve the categories and rules to
make it right?  To use this approach, it would need to be the case
that each checked function exhaustively handles a subset of node tags
that can be described by listing those categories.

That revealed a defect in commit
18ce3a4ab22d2984f8540ab480979c851dae5338 which I think should be
corrected with something like the attached, though I'm not sure if
it's possible to reach it.  It would be nice to do something more
mechanised for this type of code...

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


read-namedtuplestorescan.patch
Description: Binary data
#!/usr/bin/env python
#
# Cross-check node handling code in PostgreSQL looking for bugs.

import re

def scan_node_categories(path):
  """Find all type tags and put them into categories."""
  results = {}
  with open(path) as file:
current_list = []
results["INVALID"] = current_list
for line in file:
  matches = re.search(r"TAGS FOR ([A-Z].*(NODES|STUFF))", line)
  if matches:
category = matches.group(1)
current_list = []
results[category] = current_list
continue
  matches = re.search(r"(T_[a-zA-Z0-9_]+)", line)
  if matches:
assert current_list != None
tag = matches.group(1)
current_list.append(tag)
  return results

def scan_switch(path, function):
  """Find all cases handled by "function" in translation unit "path"."""
  cases = []
  in_function = False
  with open(path) as file:
for line in file:
  if line.startswith(function + "("):
in_function = True
  elif line.startswith("}"):
in_function = False
  elif in_function:
matches = re.search(r"case (T_.+):", line)
if matches:
  cases.append(matches.group(1))
  return cases

def scan_read_tagnames(path):
  """Find all tagnames that readfuncs.c recognizes."""
  results = []
  with open(path) as file:
for line in file:
  matches = re.search("MATCH\(\"([A-Z]+)\", ([0-9]+)", line)
  if matches:
tagname = matches.group(1)
length = int(matches.group(2))
results.append(tagname)
if length != len(tagname):
  print "Unexpected length:", line
  return results

def scan_out_tagnames(path):
  """Learn what tagname outfuncs.c uses to output each type."""
  results = {}
  current_tag = None
  with open(path) as file:
for line in file:
  matches = re.match(r"_out.*, const ([^ ]+) \*", line)
  if matches:
# see makeNode macro definition: typedef Foo must have enum T_Foo
current_tag = "T_" + matches.group(1)
  elif line.startswith("}"):

Re: [HACKERS] SCRAM auth and Pgpool-II

[Chapman Flack]

On 07/13/17 20:09, Tatsuo Ishii wrote:

> The comment in pg_hba.conf.sample seem to prefer md5 over clear text
> password.
> 
> # Note that "password" sends passwords in clear text; "md5" or
> # "scram-sha-256" are preferred since they send encrypted passwords.

Should that be reworded to eliminate "md5"? I'd consider "scram-sha-256"
suitable over a clear channel, but I've never recommended "md5" for that.

-Chap


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

Re: Fwd: [HACKERS] Vacuum: allow usage of more than 1GB of work mem

[Claudio Freire]

On Wed, Jul 12, 2017 at 1:29 PM, Claudio Freire  wrote:
> On Wed, Jul 12, 2017 at 1:08 PM, Claudio Freire  
> wrote:
>> On Wed, Jul 12, 2017 at 11:48 AM, Alexey Chernyshov
>>  wrote:
>>> Thank you for the patch and benchmark results, I have a couple remarks.
>>> Firstly, padding in DeadTuplesSegment
>>>
>>> typedef struct DeadTuplesSegment
>>>
>>> {
>>>
>>> ItemPointerData last_dead_tuple;/* Copy of the last dead tuple
>>> (unset
>>>
>>>  * until the segment is fully
>>>
>>>  * populated). Keep it first to
>>> simplify
>>>
>>>  * binary searches */
>>>
>>> unsigned short padding;/* Align dt_tids to 32-bits,
>>>
>>>  * sizeof(ItemPointerData) is aligned to
>>>
>>>  * short, so add a padding short, to make
>>> the
>>>
>>>  * size of DeadTuplesSegment a multiple of
>>>
>>>  * 32-bits and align integer components for
>>>
>>>  * better performance during lookups into
>>> the
>>>
>>>  * multiarray */
>>>
>>> intnum_dead_tuples;/* # of entries in the segment */
>>>
>>> intmax_dead_tuples;/* # of entries allocated in the
>>> segment */
>>>
>>> ItemPointer dt_tids;/* Array of dead tuples */
>>>
>>> }DeadTuplesSegment;
>>>
>>> In the comments to ItemPointerData is written that it is 6 bytes long, but
>>> can be padded to 8 bytes by some compilers, so if we add padding in a
>>> current way, there is no guaranty that it will be done as it is expected.
>>> The other way to do it with pg_attribute_alligned. But in my opinion, there
>>> is no need to do it manually, because the compiler will do this optimization
>>> itself.
>>
>> I'll look into it. But my experience is that compilers won't align
>> struct size like this, only attributes, and this attribute is composed
>> of 16-bit attributes so it doesn't get aligned by default.
>
> Doing sizeof(DeadTuplesSegment) suggests you were indeed right, at
> least in GCC. I'll remove the padding.
>
> Seems I just got the wrong impression at some point.

Updated versions of the patches attached.

A few runs of the benchmark show no significant difference, as it
should (being all cosmetic changes).

The bigger benchmark will take longer.
From b2bf9a671c2c4d517d59628f12a7f564ccf152f6 Mon Sep 17 00:00:00 2001
From: Claudio Freire 
Date: Mon, 12 Sep 2016 23:36:42 -0300
Subject: [PATCH 1/2] Vacuum: allow using more than 1GB work mem

Turn the dead_tuples array into a structure composed of several
exponentially bigger arrays, to enable usage of more than 1GB
of work mem during vacuum and thus reduce the number of full
index scans necessary to remove all dead tids when the memory is
available.

Improve test ability to spot vacuum errors
---
 src/backend/commands/vacuumlazy.c| 402 ---
 src/test/regress/expected/vacuum.out |  26 +++
 src/test/regress/sql/vacuum.sql  |  19 ++
 3 files changed, 373 insertions(+), 74 deletions(-)

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 30a0050..287accd 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -11,11 +11,24 @@
  *
  * We are willing to use at most maintenance_work_mem (or perhaps
  * autovacuum_work_mem) memory space to keep track of dead tuples.  We
- * initially allocate an array of TIDs of that size, with an upper limit that
+ * initially allocate an array of TIDs of 128MB, or an upper limit that
  * depends on table size (this limit ensures we don't allocate a huge area
- * uselessly for vacuuming small tables).  If the array threatens to overflow,
+ * uselessly for vacuuming small tables). Additional arrays of increasingly
+ * large sizes are allocated as they become necessary.
+ *
+ * The TID array is thus represented as a list of multiple segments of
+ * varying size, beginning with the initial size of up to 128MB, and growing
+ * exponentially until the whole budget of (autovacuum_)maintenance_work_mem
+ * is used up.
+ *
+ * Lookup in that structure happens with a binary search of segments, and then
+ * with a binary search within each segment. Since segment's size grows
+ * exponentially, this retains O(log N) lookup complexity.
+ *
+ * If the multiarray's total size threatens to grow beyond maintenance_work_mem,
  * we suspend the heap scan phase and perform a pass of index cleanup and page
- * compaction, then resume the heap scan with an empty TID array.
+ * compaction, then resume the heap scan with an array of logically empty but
+ * already preallocated TID segments to be refilled with more dead tuple TIDs.
  *
  * If we're 

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

[Amit Langote]

On 2017/07/13 19:57, Ashutosh Bapat wrote:
> On Thu, Jul 13, 2017 at 12:01 PM, Amit Langote
>  wrote:
>> The description of \d[S+] currently does not mention that it will list
>> materialized views and foreign tables.  Attached fixes that.
>>
> 
> I guess the same change is applicable to the description of \d[S+] NAME as 
> well.

Thanks for the review.  Fixed in the attached.

Thanks,
Amit
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index b3dbb5946e..3fad56109b 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -219,8 +219,8 @@ slashUsage(unsigned short int pager)
 
fprintf(output, _("Informational\n"));
fprintf(output, _("  (options: S = show system objects, + = additional 
detail)\n"));
-   fprintf(output, _("  \\d[S+] list tables, views, and 
sequences\n"));
-   fprintf(output, _("  \\d[S+]  NAME   describe table, view, 
sequence, or index\n"));
+   fprintf(output, _("  \\d[S+] list tables, views, 
materialized views, sequences, and foreign tables\n"));
+   fprintf(output, _("  \\d[S+]  NAME   describe table, view, 
materialized view, sequence, index, or foreign table\n"));
fprintf(output, _("  \\da[S]  [PATTERN]  list aggregates\n"));
fprintf(output, _("  \\dA[+]  [PATTERN]  list access methods\n"));
fprintf(output, _("  \\db[+]  [PATTERN]  list tablespaces\n"));

-- 
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] SCRAM auth and Pgpool-II

[Tatsuo Ishii]

>> Using a clear text password would not be acceptable for users even
>> through an encrypted connection, I think.
> 
> Really, I don't think users who are concerned with security should be
> using the md5 method either.

The comment in pg_hba.conf.sample seem to prefer md5 over clear text
password.

# Note that "password" sends passwords in clear text; "md5" or
# "scram-sha-256" are preferred since they send encrypted passwords.

> What would be really nice for such cases is support for Kerberos and
> delegated Kerberos credentials.  Having pgpool support that would remove
> the need to deal with passwords at all.
> 
> Ditto for having postgres_fdw support same.  More often than not,
> Kerberos deployments (via AD, generally) is what I find in the
> enterprises that I work with and they're happy to see we have Kerberos
> but it's disappointing when they can't use Kerberos with either
> connection poolers or with FDWs.

I would add supporting Kerberos to the Pgpool-II todo list.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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_stop_backup(wait_for_archive := true) on standby server

[Stephen Frost]

Michael, all,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
> On Thu, Jul 13, 2017 at 7:13 AM, Masahiko Sawada  
> wrote:
> > Sorry, I missed lots of typo in the last patch. All comments from you
> > are incorporated into the attached latest patch and I've checked it
> > whether there is other typos. Please review it.
> 
> Thanks for providing a new version of the patch very quickly. I have
> spent some time looking at it again and testing it, and this version
> looks correct to me. Stephen, as a potential committer, would you look
> at it to address this open item?

Yes, this weekend.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] SCRAM auth and Pgpool-II

[Stephen Frost]

Greetings Tatsuo,

* Tatsuo Ishii (is...@sraoss.co.jp) wrote:
> > What I am suggesting here is that in order to handle properly SCRAM
> > with channel binding, pgpool has to provide a different handling for
> > client <-> pgpool and pgpool <-> Postgres. In short, I don't have a
> > better answer than having pgpool impersonate the server and request
> > for a password in cleartext through an encrypted connection between
> > pgpool and the client if pgpool does not know about it, and then let
> > pgpool do by itself the SCRAM authentication on a per-connection basis
> > with each Postgres instances. When using channel binding, what would
> > matter is the TLS finish (for tls-unique) or the hash server
> > certificate between Postgres and pgpool, not between the client and
> > pgpool. But that's actually the point you are raising here:
> 
> Using a clear text password would not be acceptable for users even
> through an encrypted connection, I think.

Really, I don't think users who are concerned with security should be
using the md5 method either.

What would be really nice for such cases is support for Kerberos and
delegated Kerberos credentials.  Having pgpool support that would remove
the need to deal with passwords at all.

Ditto for having postgres_fdw support same.  More often than not,
Kerberos deployments (via AD, generally) is what I find in the
enterprises that I work with and they're happy to see we have Kerberos
but it's disappointing when they can't use Kerberos with either
connection poolers or with FDWs.

Thanks!

Stephen


signature.asc
Description: Digital signature

[HACKERS] Inadequate infrastructure for NextValueExpr

[Tom Lane]

Somebody decided they could add a new primnode type without bothering to
build out very much infrastructure for it.  Thus:

regression=# create table foo (f1 int, f2 int generated always as identity);
CREATE TABLE
regression=# insert into foo values(1);
INSERT 0 1
regression=# explain verbose insert into foo values(1);
ERROR:  unrecognized node type: 146

because ruleutils.c has never heard of NextValueExpr.  The lack of
outfuncs/readfuncs support for it is rather distressing as well.
That doesn't break parallel queries today, because (I think)
you can't get one of these nodes in a parallelizable query, but it
is going to cause problems for debugging.  It will also break
(more or less) pg_stat_statements.  I also wonder whether costsize.c
oughtn't be charging some estimated execution cost for 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] CAST vs ::

[Tom Lane]

"David G. Johnston"  writes:
> On Thursday, July 13, 2017, Tom Lane  wrote:
>> Maybe we can hack ruleutils to use
>> the CAST syntax only in this specific context.

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

Hm, it's worse than I thought: the argument of the CAST expression
needn't be a function call at all, and on top of that, the parser
will throw away a no-op cast altogether.  So for example:

regression=# create view vvc as select * from cast(1+2 as int) c(x);
CREATE VIEW
regression=# \d+ vvc
 View "public.vvc"
 Column |  Type   | Collation | Nullable | Default | Storage | Description 
+-+---+--+-+-+-
 x  | integer |   |  | | plain   | 
View definition:
 SELECT c.x
   FROM 1 + 2 c(x);

To make the world safe for this behavior by extending the grammar,
we'd have to be prepared to accept an arbitrary a_expr, without even
surrounding parentheses, as a FROM item.  I don't think there's much
chance of making that work without grammar conflicts, and even if we
managed, the SQL committee would probably find a way to break it with
some future feature addition.

So what I'm now thinking is to make ruleutils.c look at the expression in
an RTE_FUNCTION FROM item and see if it will decompile as something with
the syntax of a function call.  If not, or if there's any doubt, emit a
dummy CAST(... AS sametype) around it.  That would cause the above example
to come out the way it went in.

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] [WIP] Zipfian distribution in pgbench

[Peter Geoghegan]

On Thu, Jul 13, 2017 at 12:49 PM, Peter Geoghegan  wrote:
> To reiterate what I say above:
>
> The number of leaf pages with dead items is 20 with this most recent
> run (128 clients, patched + unpatched). The leftmost internal page one
> level up from the leaf level contains 289 items. Whereas last time it
> was 353 items.
>
> That's a difference between having 20 hot/bloated leaf pages, and
> probably 84 hot/bloated pages, which I infer must have been the total
> number of bloated leaf pages within "result.txt". I think that
> something about all the "pgbench_index_*txt" tests are very different
> to what we see within "result.txt". It's as if there was a problem
> when "result.txt" ran, but that problem somehow didn't come up when
> you did new tests.

I just figured out that "result.txt" is only a 60 second pgbench run.
Is the same true for other tests?

It would be much more interesting to see runs that lasted 10 minutes
or more. Maybe with pgbench-tools. I would expect that a decline in
performance due to bloating the index could happen only after a
certain threshold was crossed. Things like HOT pruning and LP_DEAD
cleanup could be pretty effective, until finally a tipping point is
reached and they're much less effective.

-- 
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] PostgreSQL - Weak DH group

[Heikki Linnakangas]

On 07/13/2017 10:13 PM, Robert Haas wrote:

On Thu, Jul 13, 2017 at 1:30 PM, Tom Lane  wrote:

Heikki Linnakangas  writes:

I don't think this can be backpatched. It changes the default DH
parameters from 1024 bits to 2048 bits. That's a good thing for
security, but older clients might not support it, and would refuse to
connect or would fall back to something less secure.


Do we have any hard information about which versions of which clients
might not support that?  (In particular I'm wondering if any still exist
in the wild.)


Yeah.  If we break clients for v10 two months from release, some
drivers won't be updated by release time, and that sounds pretty
unfriendly to me.  On the other hand, if there is only a theoretical
risk of breakage and no clients that we actually know about will have
a problem with it, then the argument for waiting is weaker.  I'm not
generally very excited about changing things after beta2, which is
where are, but if this is a security issue then we might need to hold
our nose and go ahead.  I'm against it if it's likely to cause
real-world connectivity problems, though.


Googling around, I believe Java 6 is the only straggler [1]. So we would 
be breaking that. Java 7 also doesn't support DH parameters > 1024 bits, 
but it supports ECDHE, which is prioritized over DH ciphers, so it 
doesn't matter.


Java 6 was released back in 2006. The last public release was in 2013. 
It wouldn't surprise me to still see it bundled with random proprietary 
software packages, though. The official PostgreSQL JDBC driver still 
supports it, but there has been discussion recently on dropping support 
for it, and even for Java 7. [2]


I would be OK with breaking DH with Java 6 in PostgreSQL 10, especially 
since there's a simple workaround (generate a 1024-bit DH parameters 
file). I would be less enthusiastic about doing that in a minor release, 
although maybe that wouldn't be too bad either, if we put a prominent 
notice with the workaround in the release notes.


[1] https://wiki.mozilla.org/Security/Server_Side_TLS#DHE_and_ECDHE_support

[2] 
https://www.postgresql.org/message-id/69ae857b-15cc-36dd-f380-6620ef1effb9%408kdata.com


- Heikki


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

Re: [HACKERS] pl/perl extension fails on Windows

[Christoph Berg]

Re: Dave Page 2017-07-12 

> > Well, we have various buildfarm machines running perls newer than that,
> > eg, crake, with 5.24.1.  So I'd say there is something busted about your
> > perl installation.  Perhaps leftover bits of an older version somewhere?
> >
> 
> Well crake is a Fedora box - and we have no problems on Linux, only on
> Windows.

The plperl segfault on Debian's kfreebsd port I reported back in 2013
is also still present:

https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de

https://buildd.debian.org/status/fetch.php?pkg=postgresql-10=kfreebsd-amd64=10~beta2-1=1499947011=0

(Arguably, this is a toy architecture, so we can just leave it unfixed
there without any harm...)

Christoph


-- 
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] [WIP] Zipfian distribution in pgbench

[Peter Geoghegan]

On Thu, Jul 13, 2017 at 10:02 AM, Peter Geoghegan  wrote:
> The number of leaf pages at the left hand side of the leaf level seems
> to be ~50 less than the unpatched 128 client case was the first time
> around, which seems like a significant difference. I wonder why. Maybe
> autovacuum ran at the right/wrong time this time around?

To reiterate what I say above:

The number of leaf pages with dead items is 20 with this most recent
run (128 clients, patched + unpatched). The leftmost internal page one
level up from the leaf level contains 289 items. Whereas last time it
was 353 items.

That's a difference between having 20 hot/bloated leaf pages, and
probably 84 hot/bloated pages, which I infer must have been the total
number of bloated leaf pages within "result.txt". I think that
something about all the "pgbench_index_*txt" tests are very different
to what we see within "result.txt". It's as if there was a problem
when "result.txt" ran, but that problem somehow didn't come up when
you did new tests.


-- 
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] Domains and arrays and composites, oh my

[Tom Lane]

"David G. Johnston"  writes:
> On Thursday, July 13, 2017, Tom Lane  wrote:
>> regression=# select * from fdc();
>> fdc
>> ---
>> (1,2)
>> (1 row)

> Select (fdc).* from fdc();  is considerably more intuitive that the cast.
> Does that give the expected multi-column result?

Yeah, it does, although I'm not sure how intuitive it is that the
parentheses are significant ...

regression=#  select * from fdc();
  fdc  
---
 (1,2)
(1 row)

regression=# select fdc from fdc();
  fdc  
---
 (1,2)
(1 row)

regression=# select fdc.* from fdc();
  fdc  
---
 (1,2)
(1 row)

regression=# select (fdc).* from fdc();
 r | i 
---+---
 1 | 2
(1 row)

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] Domains and arrays and composites, oh my

[David G. Johnston]

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

David J.

Re: [HACKERS] CAST vs ::

[David G. Johnston]

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

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

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

David J.

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

[Tom Lane]

I wrote:
> I started to look into allowing domains over composite types, which is
> another never-implemented case that there's no very good reason not to
> allow.  Well, other than the argument that the SQL standard only allows
> domains over "predefined" (built-in) types ... but we blew past that
> restriction ages ago.

Attached is a draft patch that allows domains over composite types.
I think it's probably complete on its own terms, but there are some
questions around behavior of functions returning domain-over-composite
that could use discussion, and some of the PLs need some follow-on work.

The core principle here was to not modify execution-time behavior by
adding domain checks to existing code paths.  Rather, I wanted the
parser to insert CoerceToDomain nodes wherever checks would be needed.
Row-returning node types such as RowExpr and FieldStore only return
regular composite types, as before; to generate a value of a composite
domain, we construct a value of the base type and then CoerceToDomain.
(This is pretty analogous to what happens for domains over arrays.)
Whole-row Vars can only have regular composite types as vartype,
TupleDescs can only have regular composite types as tdtypeid, composite
Datums only have regular composite type OIDs in datum_typeid.  (The last
would be expected anyway, since the physical representation of a domain
value is never different from that of its base type.)

Doing it that way led to a nicely small patch, only about 160 net new
lines of code.  However, not touching the executor meant not touching
the behavior of functions that return domains, even if the domain is
domain-over-composite.  In code terms this means that 
get_call_result_type() and sibling functions will return TYPEFUNC_SCALAR
not TYPEFUNC_COMPOSITE for such a result type.  The difficulty with
changing that is that if these functions look through the domain, then
the calling code (in, usually, a PL) will simply build and return a result
of the underlying composite type, failing to apply any domain constraints.
Trying to get out-of-core PLs on board with a change in those requirements
seems like a risky proposition.

Concretely, consider

create type complex as (r float8, i float8);
create domain dcomplex as complex;

You can make a SQL-language function to return complex in either of two
ways:

create function fc() returns complex language sql
as 'select 1.0::float8, 2.0::float8';

create function fc() returns complex language sql
as 'select row(1.0::float8, 2.0::float8)::complex';

As the patch stands, though, only the second way works for domains over
composite:

regression=# create function fdc() returns dcomplex language sql
as 'select 1.0::float8, 2.0::float8';
ERROR:  return type mismatch in function declared to return dcomplex
DETAIL:  Final statement must return exactly one column.
CONTEXT:  SQL function "fdc"
regression=# create function fdc() returns dcomplex language sql
as 'select row(1.0::float8, 2.0)::dcomplex';
CREATE FUNCTION

Now, maybe that's fine.  SQL-language functions have never been very
willing to insert implicit casts to get to the function result type,
and certainly the only way that the first definition could be legal
is if there were an implicit up-cast to the domain type.  It might be
OK to just leave it like this, though some documentation about it
would be a good idea.

plpgsql functions seem generally okay as far as composite domain return
types go, because they don't have anything corresponding to the row
return convention of SQL functions.  And plpgsql's greater willingness
to do implicit coercions reduces the notational burden, too.  But
there's some work yet to be done to get plpgsql to realize that
composite domain local variables have substructure.  For example,
this works:

declare x complex;
...
x.r := 1;

but it fails if x is dcomplex.  But ISTM that that would be better
handled as a followon feature patch.  I suspect that the other PLs may
have similar issues where it'd be nice to allow domain-over-composite
to act like a plain composite for specific purposes; but I've not looked.

Another issue related to function result types is that the parser
considers a function-in-FROM returning a composite domain to be
producing a scalar result not a rowtype.  Thus you get this for a
function returning complex:

regression=# select * from fc();
 r | i 
---+---
 1 | 2
(1 row)

but this for a function returning dcomplex:

regression=# select * from fdc();
  fdc  
---
 (1,2)
(1 row)

I think that that could be changed with only local changes in parse
analysis, but do we want to change it?  Arguably, making fdc() act the
same as fc() here would amount to implicitly downcasting the domain to
its base type.  But doing so here is optional, not necessary in order to
make the statement sane at all, and it's arguable that we shouldn't do
that if the user didn't tell us to.  A user who does want that to happen
can downcast explicitly:

regression=# 

Re: [HACKERS] PostgreSQL - Weak DH group

[Robert Haas]

On Thu, Jul 13, 2017 at 1:30 PM, Tom Lane  wrote:
> Heikki Linnakangas  writes:
>> I don't think this can be backpatched. It changes the default DH
>> parameters from 1024 bits to 2048 bits. That's a good thing for
>> security, but older clients might not support it, and would refuse to
>> connect or would fall back to something less secure.
>
> Do we have any hard information about which versions of which clients
> might not support that?  (In particular I'm wondering if any still exist
> in the wild.)

Yeah.  If we break clients for v10 two months from release, some
drivers won't be updated by release time, and that sounds pretty
unfriendly to me.  On the other hand, if there is only a theoretical
risk of breakage and no clients that we actually know about will have
a problem with it, then the argument for waiting is weaker.  I'm not
generally very excited about changing things after beta2, which is
where are, but if this is a security issue then we might need to hold
our nose and go ahead.  I'm against it if it's likely to cause
real-world connectivity problems, though.

-- 
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] PostgreSQL - Weak DH group

[Tom Lane]

Heikki Linnakangas  writes:
> I don't think this can be backpatched. It changes the default DH 
> parameters from 1024 bits to 2048 bits. That's a good thing for 
> security, but older clients might not support it, and would refuse to 
> connect or would fall back to something less secure.

Do we have any hard information about which versions of which clients
might not support that?  (In particular I'm wondering if any still exist
in the wild.)

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] CAST vs ::

[Tom Lane]

In most places, you can write CAST(x AS t) and x::t interchangeably.
But that doesn't work for function-in-FROM.  This is OK: 

select * from cast(fdc() as complex);

but this is not:

select * from fdc()::complex;
ERROR:  syntax error at or near "::"

I just realized that this is a problem for ruleutils.c, which thinks
it can always use the short form:

regression=# create view vv as select * from cast(fdc() as complex);
CREATE VIEW
regression=# \d+ vv
  View "public.vv"
 Column |   Type   | Collation | Nullable | Default | Storage | Descript
ion 
+--+---+--+-+-+-

 r  | double precision |   |  | | plain   | 
 i  | double precision |   |  | | plain   | 
View definition:
 SELECT fdc.r,
fdc.i
   FROM fdc()::complex fdc(r, i);

That view definition will not reload.

Not sure about the most reasonable fix.  It might be possible to tweak
the grammar to allow this case, but I'm not at all sure about that.
An easy fix would be to make ruleutils print casts as CAST() all the
time, but that would probably annoy a lot of people (it'd certainly
break a lot of regression tests).  Maybe we can hack ruleutils to use
the CAST syntax only in this specific context.

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] Fixup some misusage of appendStringInfo and friends

[Heikki Linnakangas]

On 04/27/2017 03:14 AM, David Rowley wrote:

On 27 April 2017 at 06:41, Peter Eisentraut
 wrote:

On 4/19/17 08:42, Ashutosh Bapat wrote:

I reviewed the patch. It compiles clean, make check-world passes. I do
not see any issue with it.


Looks reasonable.  Let's keep it for the next commit fest.


Thank you to both of you for looking. I'd thought that maybe the new
stuff in PG10 should be fixed before the release. If we waited, and
fix in PG11 then backpatching is more of a pain.

However, I wasn't careful in the patch to touch only new to PG10 code.

I'll defer to your better judgment and add to the next 'fest.


I think that's a very good argument. Cleaning up code that's new in this 
version seems like a fair game, and a good idea. The places that are not 
new in PostgreSQL 10 are more questionable, but seems harmless enough 
anyway.


Did you have an outright objection to this, Peter? The patch looks good 
to me at a quick glance, I think we should commit this now.


- Heikki



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

Re: [HACKERS] PostgreSQL - Weak DH group

[Joe Conway]

On 07/13/2017 01:07 PM, Simon Riggs wrote:
> On 13 July 2017 at 16:32, Heikki Linnakangas  wrote:
>> (We dropped the ball back in October, continuing the discussion now)
>>
>> On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:
>>>
>>> On 10/06/2016 10:26 PM, Christoph Berg wrote:

 Re: Heikki Linnakangas 2016-10-06
 
>
> I propose the attached patch. It gives up on trying to deal with
> multiple
> key lengths (as noted earlier, OpenSSL just always passed
> keylength=1024, so
> that was useless). Instead of using the callback, it just sets fixed DH
> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The
> DH
> parameters are loaded from a file called "dh_params.pem" (instead of
> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters
> are
> used.


 Shouldn't this be a GUC pointing to a configurable location like
 ssl_cert_file? This way, people reading the security section of the
 default postgresql.conf would notice that there's something (new) to
 configure. (And I wouldn't want to start creating symlinks from PGDATA
 to /etc/ssl/something again...)
>>>
>>>
>>> Perhaps. The DH parameters are not quite like other configuration files,
>>> though. The actual parameters used don't matter, you just want to
>>> generate different parameters for every cluster. The key length of the
>>> parameters can be considered configuration, though.
>>
>>
>> Actually, adding a GUC also solves another grief I had about this. There is
>> currently no easy way to tell if your parameter file is being used, or if
>> the server is failing to read it and is falling back to the hard-coded
>> parameters. With a GUC, if the GUC is set it's a good indication that the
>> admin expects the file to really exist and to contain valid parameters. So
>> if the GUC is set, we should throw an error if it cannot be used. And if
>> it's not set, we use the built-in defaults.
>>
>> I rebased the patch, did some other clean up of error reporting, and added a
>> GUC along those lines, as well as docs. How does this look?
>>
>> It's late in the release cycle, but it would be nice to sneak this into v10.
>> Using weak 1024 bit DH parameters is arguably a security issue; it was
>> originally reported as such. There's a work-around for older versions:
>> generate custom 2048 bit parameters and place them in a file called
>> "dh1024.pem", but that's completely undocumented.
>>
>> Thoughts? Objections to committing this now, instead of waiting for v11?
> 
> +1 to include important open items such as this

I have not looked at the patch itself, but conceptually +1 to include in
pg10.


-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PostgreSQL - Weak DH group

[Heikki Linnakangas]

On 07/13/2017 08:04 PM, Alvaro Herrera wrote:

Michael Paquier wrote:

On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas  wrote:



Objections to committing this now, instead of waiting for v11?


But I am -1 for the sneak part. It is not the time to have a new
feature in 10, the focus is to stabilize.


But if we were treating it as a security issue, would we backpatch it?
If we do, then it definitely makes sense to put something in pg10.  I'm
not sure that this patch is it, though -- perhaps it makes sense to put
a minimal fix in older branches, and let the new feature wait for pg11?


I don't think this can be backpatched. It changes the default DH 
parameters from 1024 bits to 2048 bits. That's a good thing for 
security, but older clients might not support it, and would refuse to 
connect or would fall back to something less secure. I don't think there 
are many such clients around anymore, but it's nevertheless not 
something we want to do in a stable release I think the best we can do 
is to document the issue and the workaround. To recap, to use stronger 
DH parameters in stable versions, you need to do "openssl dhparam -out 
$PGDATA/dh1024.pem 2048".


But I'd like to take the opportunity to change this for new 
installations, with v10, instead of waiting for another year. Of course, 
you could say that for any new feature, too, but that doesn't 
necessarily mean that it's a bad argument :-). It's a judgment call, for 
sure.


- Heikki



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

Re: [HACKERS] PostgreSQL - Weak DH group

[Christoph Berg]

Re: Alvaro Herrera 2017-07-13 <20170713170402.74uuoivrgd3c6tnw@alvherre.pgsql>
> > > Objections to committing this now, instead of waiting for v11?
> > 
> > But I am -1 for the sneak part. It is not the time to have a new
> > feature in 10, the focus is to stabilize.
> 
> But if we were treating it as a security issue, would we backpatch it?
> If we do, then it definitely makes sense to put something in pg10.  I'm
> not sure that this patch is it, though -- perhaps it makes sense to put
> a minimal fix in older branches, and let the new feature wait for pg11?

Making it user-configurable seems pretty minimal to me. Everything
else would probably require lengthy explanations about which file
could hold which contents, and this confusion seems to be part of the
problem.

Fwiw, wouldn't it make sense to recreate the default 2048 DH group as
well, maybe each time a new major is branched?

Christoph


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

[Amit Khandekar]

On 5 July 2017 at 15:12, Amit Khandekar  wrote:
> Like I mentioned upthread... in expand_inherited_rtentry(), if we
> replace find_all_inheritors() with something else that returns oids in
> canonical order, that will change the order in which children tables
> get locked, which increases the chance of deadlock. Because, then the
> callers of find_all_inheritors() will lock them in one order, while
> callers of expand_inherited_rtentry() will lock them in a different
> order. Even in the current code, I think there is a chance of
> deadlocks because RelationGetPartitionDispatchInfo() and
> find_all_inheritors() have different lock ordering.
>
> Now, to get the oids of a partitioned table children sorted by
> canonical ordering, (i.e. using the partition bound values) we need to
> either use the partition bounds to sort the oids like the way it is
> done in RelationBuildPartitionDesc() or, open the parent table and get
> it's Relation->rd_partdesc->oids[] which are already sorted in
> canonical order. So if we generate oids using this way in
> find_all_inheritors() and find_inheritance_children(), that will
> generate consistent ordering everywhere. But this method is quite
> expensive as compared to the way oids are generated and sorted using
> oid values in find_inheritance_children().
>
> In both expand_inherited_rtentry() and
> RelationGetPartitionDispatchInfo(), each of the child tables are
> opened.
>
> So, in both of these functions, what we can do is : call a new
> function partition_tree_walker() which does following :
> 1. Lock the children using the existing order (i.e. sorted by oid
> values) using the same function find_all_inheritors(). Rename
> find_all_inheritors() to lock_all_inheritors(... , bool return_oids)
> which returns the oid list only if requested.
> 2. And then scan through each of the partitions in canonical order, by
> opening the parent table, then opening the partition descriptor oids,
> and then doing whatever needs to be done with that partition rel.
>
> partition_tree_walker() will look something like this :
>
> void partition_tree_walker(Oid parentOid, LOCKMODE lockmode,
>void (*walker_func) (), void *context)
> {
> Relation parentrel;
> List *rels_list;
> ListCell *cell;
>
> (void) lock_all_inheritors(parentOid, lockmode,
>false /* don't generate oids */);
>
> parentrel = heap_open(parentOid, NoLock);
> rels_list = append_rel_partition_oids(NIL, parentrel);
>
> /* Scan through all partitioned rels, and at the
>  * same time append their children. */
> foreach(cell, rels_list)
> {
> /* Open partrel without locking; lock_all_inheritors() has locked it 
> */
> Relationpartrel = heap_open(lfirst_oid(cell), NoLock);
>
> /* Append the children of a partitioned rel to the same list
>  * that we are iterating on */
> if (RelationGetPartitionDesc(partrel))
> rels_list = append_rel_partition_oids(rels_list, partrel);
>
> /*
>  * Do whatever processing needs to be done on this partel.
>  * The walker function is free to either close the partel
>  * or keep it opened, but it needs to make sure the opened
>  * ones are closed later
>  */
> walker_func(partrel, context);
> }
> }
>
> List *append_rel_partition_oids(List *rel_list, Relation rel)
> {
> int i;
> for (i = 0; i < rel->rd_partdesc->nparts; i++)
> rel_list = lappend_oid(rel_list, rel->rd_partdesc->oids[i]);
>
> return rel_list;
> }
>
>
> So, in expand_inherited_rtentry() the foreach(l, inhOIDs) loop will be
> replaced by partition_tree_walker(parentOid, expand_rte_walker_func)
> where expand_rte_walker_func() will do all the work done in the for
> loop for each of the partition rels.
>
> Similarly, in RelationGetPartitionDispatchInfo() the initial part
> where it uses APPEND_REL_PARTITION_OIDS() can be replaced by
> partition_tree_walker(rel, dispatch_info_walkerfunc) where
> dispatch_info_walkerfunc() will generate the oids, or may be populate
> the complete PartitionDispatchData structure. 'pd' variable can be
> passed as context to the partition_tree_walker(..., context)
>
> Generating the resultrels in canonical order by opening the tables
> using the above way wouldn't be more expensive than the existing code,
> because even currently we anyways have to open all the tables in both
> of these functions.
>

Attached is a WIP patch (make_resultrels_ordered.patch) that generates
the result rels in canonical order. This patch is kept separate from
the update-partition-key patch, and can be applied on master branch.

In this patch, rather than partition_tree_walker() called with a
context, I have provided a function partition_walker_next() using
which we iterate over all the partitions in canonical order.
partition_walker_next() will take care of appending oids from
partition 

Re: [HACKERS] PostgreSQL - Weak DH group

[Simon Riggs]

On 13 July 2017 at 16:32, Heikki Linnakangas  wrote:
> (We dropped the ball back in October, continuing the discussion now)
>
> On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:
>>
>> On 10/06/2016 10:26 PM, Christoph Berg wrote:
>>>
>>> Re: Heikki Linnakangas 2016-10-06
>>> 

 I propose the attached patch. It gives up on trying to deal with
 multiple
 key lengths (as noted earlier, OpenSSL just always passed
 keylength=1024, so
 that was useless). Instead of using the callback, it just sets fixed DH
 parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The
 DH
 parameters are loaded from a file called "dh_params.pem" (instead of
 "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters
 are
 used.
>>>
>>>
>>> Shouldn't this be a GUC pointing to a configurable location like
>>> ssl_cert_file? This way, people reading the security section of the
>>> default postgresql.conf would notice that there's something (new) to
>>> configure. (And I wouldn't want to start creating symlinks from PGDATA
>>> to /etc/ssl/something again...)
>>
>>
>> Perhaps. The DH parameters are not quite like other configuration files,
>> though. The actual parameters used don't matter, you just want to
>> generate different parameters for every cluster. The key length of the
>> parameters can be considered configuration, though.
>
>
> Actually, adding a GUC also solves another grief I had about this. There is
> currently no easy way to tell if your parameter file is being used, or if
> the server is failing to read it and is falling back to the hard-coded
> parameters. With a GUC, if the GUC is set it's a good indication that the
> admin expects the file to really exist and to contain valid parameters. So
> if the GUC is set, we should throw an error if it cannot be used. And if
> it's not set, we use the built-in defaults.
>
> I rebased the patch, did some other clean up of error reporting, and added a
> GUC along those lines, as well as docs. How does this look?
>
> It's late in the release cycle, but it would be nice to sneak this into v10.
> Using weak 1024 bit DH parameters is arguably a security issue; it was
> originally reported as such. There's a work-around for older versions:
> generate custom 2048 bit parameters and place them in a file called
> "dh1024.pem", but that's completely undocumented.
>
> Thoughts? Objections to committing this now, instead of waiting for v11?

+1 to include important open items such as this

-- 
Simon Riggshttp://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] 10beta1 sequence regression failure on sparc64

[Christoph Berg]

Re: To Andres Freund 2017-05-24 <20170524170921.7pykzbt54dlfk...@msg.df7cb.de>
> > > If we had a typo or something in that code, the build farm should have
> > > caught it by now.
> > > 
> > > I would try compiling with lower -O and see what happens.
> > 
> > Trying -O0 now.
> 
> Sorry for the late reply, I was hoping to catch you on IRC, but then
> didn't manage to be around at a time that would fit the timezone
> shift.
> 
> The -O0 build passed the regression tests. Not sure what that means
> for our problem, though.

Fwiw, the problem is fixed with beta2, even with -O2:

https://buildd.debian.org/status/logs.php?pkg=postgresql-10=sparc64

Christoph


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

Re: [HACKERS] PostgreSQL - Weak DH group

[Alvaro Herrera]

Michael Paquier wrote:
> On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas  wrote:

> > Objections to committing this now, instead of waiting for v11?
> 
> But I am -1 for the sneak part. It is not the time to have a new
> feature in 10, the focus is to stabilize.

But if we were treating it as a security issue, would we backpatch it?
If we do, then it definitely makes sense to put something in pg10.  I'm
not sure that this patch is it, though -- perhaps it makes sense to put
a minimal fix in older branches, and let the new feature wait for pg11?

-- 
Álvaro Herrerahttps://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] [WIP] Zipfian distribution in pgbench

[Peter Geoghegan]

On Thu, Jul 13, 2017 at 4:38 AM, Alik Khilazhev
 wrote:
> I am attaching results of test for 32 and 128 clients for original and
> patched(_bt_doinsert) variants.

Thanks.

The number of leaf pages at the left hand side of the leaf level seems
to be ~50 less than the unpatched 128 client case was the first time
around, which seems like a significant difference. I wonder why. Maybe
autovacuum ran at the right/wrong time this time around?

Did you happen to record TPS for this most recent set of tests?

I notice one possibly interesting thing from these new numbers: the 32
client case is slightly more bloated when unique index enforcement is
removed.

-- 
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] pl/perl extension fails on Windows

[Tom Lane]

Andrew Dunstan  writes:
> It would be nice to get to the bottom of why we're getting a version
> mismatch on Windows, since we're clearly not getting one on Linux.

Yeah, that's what's bothering me: as long as that remains unexplained,
I don't have any confidence that we're fixing the right thing.

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] Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

[Heikki Linnakangas]

On 05/03/2017 07:32 AM, Haribabu Kommi wrote:

[Adding -hackers mailing list]

On Fri, Apr 28, 2017 at 6:28 PM,  wrote:


The following bug has been logged on the website:

Bug reference:  14634
Logged by:  Henry Boehlert
Email address:  henry_boehl...@agilent.com
PostgreSQL version: 9.6.2
Operating system:   Windows Server 2012 R2 6.3.9600
Description:

Executing command pg_basebackup -D -F t writes its output to stdout, which
is open in text mode, causing LF to be converted to CR LF thus corrupting
the resulting archive.

To write the tar to stdout, on Windows stdout's mode should be temporarily
switched to binary.

https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx



Thanks for reporting the issue.
With the attached patch, I was able to extract the tar file that gets
generated when the tar file is written into stdout. I tested the
the compressed tar also.

This bug needs to be fixed in back branches also.


Seems reasonable. One question:

In the patch, you used "_setmode" function, while the calls in 
src/bin/pg_dump/pg_backup_archiver.c use "setmode". There are a few 
places in the backend that also use "_setmode". What's the difference? 
Should we change some of them to be consistent?


- Heikki



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

Re: [HACKERS] PostgreSQL - Weak DH group

[Michael Paquier]

On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas  wrote:
> I rebased the patch, did some other clean up of error reporting, and added a
> GUC along those lines, as well as docs. How does this look?
>
> It's late in the release cycle, but it would be nice to sneak this into v10.
> Using weak 1024 bit DH parameters is arguably a security issue; it was
> originally reported as such. There's a work-around for older versions:
> generate custom 2048 bit parameters and place them in a file called
> "dh1024.pem", but that's completely undocumented.
>
> Thoughts?

The patch looks in good shape to me.

 #include "utils/memutils.h"

-
 static intmy_sock_read(BIO *h, char *buf, int size);
That's unnecessary noise.

+ *Very uncool. Alternatively, the system could refuse to start
+ *if a DH parameters if not specified, but this would tend to
+ *piss off DBAs.
"is not specified".

> Objections to committing this now, instead of waiting for v11?

But I am -1 for the sneak part. It is not the time to have a new
feature in 10, the focus is to stabilize.
-- 
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] WIP Patch: Pgbench Serialization and deadlock errors

[Fabien COELHO]

I was not convinced by the overall memory management around variables
to begin with, and it is even less so with their new copy management.
Maybe having a clean "Variables" data structure could help improve the
situation.


Ok!


Note that there is something for psql (src/bin/psql/variable.c) which may 
or may not be shared. It should be checked before recoding eventually the 
same thing.


--
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] WIP Patch: Pgbench Serialization and deadlock errors

[Fabien COELHO]

Hello,

[...] I didn't make rollbacks to savepoints after the failure because 
they cannot help for serialization failures at all: after rollback to 
savepoint a new attempt will be always unsuccessful.


Not necessarily? It depends on where the locks triggering the issue are 
set, if they are all set after the savepoint it could work on a second 
attempt.


"SimpleStats attempts": I disagree with using this floating poiunt 
oriented structures to count integers. I would suggest "int64 tries" 
instead, which should be enough for the purpose.


I'm not sure that it is enough. Firstly it may be several transactions in 
script so to count the average attempts number you should know the total 
number of runned transactions. Secondly I think that stddev for attempts 
number can be quite interesting and often it is not close to zero.


I would prefer to have a real motivation to add this complexity in the 
report and in the code. Without that, a simple int seems better for now. 
It can be improved later if the need really arises.



Some variables, such as "int attempt_number", should be in the client
structure, not in the client? Generally, try to use block variables if
possible to keep the state clearly disjoints. If there could be NO new
variable at the doCustom level that would be great, because that would
ensure that there is no machine state mixup hidden in these variables.


Do you mean the code cleanup for doCustom function? Because if I do so there 
will be two code styles for state blocks and their variables in this 
function..


I think that any variable shared between state is a recipee for bugs if it 
is not reset properly, so they should be avoided. Maybe there are already 
too many of them, then too bad, not a reason to add more. The status 
before the automaton was a nightmare.



I wondering whether the RETRY & FAILURE states could/should be merged:


I divided these states because if there's a failed transaction block you 
should end it before retrying.


Hmmm. Maybe I'm wrong. I'll think about it.


I would suggest a more compact one-line report about failures:

  "number of failures: 12 (0.001%, deadlock: 7, serialization: 5)"


I think, there may be a misunderstanding. Because script can contain several 
transactions and get both failures.


I do not understand. Both failures number are on the compact line I 
suggested.


--
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] [WIP] Zipfian distribution in pgbench

[Fabien COELHO]

Hello Alik,

A few comments about the patch v2.

Patch applies and compiles.

Documentation says that the closer theta is from 0 the flatter the distribution
but the implementation requires at least 1, including strange error messages:

  zipfian parameter must be greater than 1.00 (not 1.00)

Could theta be allowed between 0 & 1 ? I've tried forcing with theta = 0.1
and it worked well, so I'm not sure that I understand the restriction.
I also tried with theta=0.001 but it seemed less good.

I have also tried to check the distribution wrt the explanations, with the 
attached scripts, n=100, theta=1.01/1.5/3.0: It does not seem to work, 
there is repeatable +15% bias on i=3 and repeatable -3% to -30% bias for 
values in i=10-100, this for different values of theta (1.01,1.5, 
3.0).


If you try the script, beware to set parameters (theta, n) consistently.

About the code:

Remove spaces and tabs at end of lines or on empty lines.

zipfn: I would suggest to move the pg_erand48 out and pass the result
instead of passing the thread. the erand call would move to getZipfianRand.

I'm wondering whether the "nearest" hack could be removed so as to simplify
the cache functions code...

Avoid editing lines without changes (spacesn/tabs?)
  -  thread->logfile = NULL; /* filled in later */
  +  thread->logfile = NULL; /* filled in later */

The documentation explaining the intuitive distribution talks about a N
but does not provide any hint about its value depending on the parameters.

There is an useless empty lien before "" after that.

--
Fabien.

compte_init.sql
Description: application/sql


compte_bench.sql
Description: application/sql


compte_expected.sql
Description: application/sql


compte_results.sql
Description: application/sql

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

Re: [HACKERS] pl/perl extension fails on Windows

[Andrew Dunstan]

On 07/13/2017 10:36 AM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>>> -dVAR; dXSBOOTARGSAPIVERCHK;
>>> +dVAR; dXSBOOTARGSNOVERCHK;
>> Good job hunting this down!
>> One suggestion I saw in a little googling was that we add this to the XS
>> file after the inclusion of XSUB.h:
>> #undef dXSBOOTARGSAPIVERCHK
>> #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK
> I don't see anything even vaguely like that in the Util.c file generated
> by Perl 5.10.1, which is what I've got on my RHEL machine.




This is all fairly modern, so it's hardly surprising that it doesn't
happen with the ancient perl 5.10.

here's a snippet from the generated Util.c on crake (Fedora 25, perl 5.24):

XS_EXTERNAL(boot_PostgreSQL__InServer__Util); /* prototype to pass
-Wmissing-prototypes */
XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
{
#if PERL_VERSION_LE(5, 21, 5)
dVAR; dXSARGS;
#else
dVAR; dXSBOOTARGSAPIVERCHK;
#endif



>
> What I do notice is this in Util.xs:
>
> VERSIONCHECK: DISABLE
>
> which leads immediately to two questions:
>
> 1. Why is your version of xsubpp apparently ignoring this directive
> and generating a version check anyway?
>
> 2. Why do we have this directive in the first place?  It does not seem
> to me like a terribly great idea to ignore low-level version mismatches.
>
> In the same vein, I'm suspicious of proposals to "fix" this problem
> by removing the version check, which seems to be where Ashutosh
> is headed.  In the long run that seems certain to cause huge headaches.



That is a different version check. It's the equivalent of xsubpp's
--noversioncheck flag. The versions it would check are the object file
and the corresponding pm file.

In fact the usage I suggested seems to be blessed in XSUB.h in this comment:

/* dXSBOOTARGSNOVERCHK has no API in xsubpp to choose it so do
#undef dXSBOOTARGSXSAPIVERCHK
#define dXSBOOTARGSXSAPIVERCHK dXSBOOTARGSNOVERCHK */





It would be nice to get to the bottom of why we're getting a version
mismatch on Windows, since we're clearly not getting one on Linux. But
since we've got on happily all these years without the API version check
we might well survive a few more going on as we are.

cheers

andrew

-- 
Andrew Dunstanhttps://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] Function Volatility and Views Unexpected Behavior

[David Kohn]

Thanks for the reminder about explain verbose, that's helpful.

But optimization does massively change the number of calls of a volatile
function in a naive evaluation of a query:

`explain analyze verbose select data1 from table1_silly_view where id >=10
and id <= 100;`

does an index scan and only runs the volatile function for rows in the view
where id >= 10 and id <=100

Subquery Scan on table1_silly_view  (cost=0.29..33.77 rows=91 width=8)
(actual time=2.552..206.563 rows=91 loops=1)

  Output: table1_silly_view.data1

  ->  Index Scan using table1_pkey on public.table1  (cost=0.29..32.86
rows=91 width=20) (actual time=2.550..206.425 rows=91 loops=1)

Output: NULL::integer, table1.data1, something_silly(table1.id)

Index Cond: ((table1.id >= 10) AND (table1.id <= 100))

Planning time: 0.526 ms

Execution time: 206.724 ms


whereas

`explain analyze verbose select data1 from table1_silly_view where id in (
select id from table1 where id >= 10 and id <=100);`

does a full sequential scan, over the view, producing whatever side effects
the volatile function does for every row in the view even though they
produce the same output and have what should be equivalent quals.

Hash Semi Join  (cost=11.24..2793.50 rows=91 width=8) (actual
time=23.603..22759.297 rows=91 loops=1)

  Output: table1_1.data1

  Hash Cond: (table1_1.id = table1.id)

  ->  Seq Scan on public.table1 table1_1  (cost=0.00..2655.00 rows=1
width=20) (actual time=2.468..22720.942 rows=1 loops=1)

Output: table1_1.id, table1_1.data1, something_silly(table1_1.id)

  ->  Hash  (cost=10.11..10.11 rows=91 width=4) (actual time=0.484..0.484
rows=91 loops=1)

Output: table1.id

Buckets: 1024  Batches: 1  Memory Usage: 12kB

->  Index Only Scan using table1_pkey on public.table1
 (cost=0.29..10.11 rows=91 width=4) (actual time=0.383..0.430 rows=91
loops=1)

  Output: table1.id

  Index Cond: ((table1.id >= 10) AND (table1.id <= 100))

  Heap Fetches: 91

Planning time: 0.877 ms

Execution time: 22759.448 ms


I recognize that it is an anti-pattern to put a volatile function call in a
view, and don't know that there's a better way of dealing with it, as not
using indexes in a view that has a volatile function call in it at all
seems like a very bad choice, but still think it might be something to
document better.

-David



On Wed, Jul 12, 2017 at 3:23 PM Tom Lane  wrote:

> David Kohn  writes:
> > I encountered some unexpected behavior when debugging a query that was
> > taking longer than expected, basically, a volatile function that makes a
> > column in a view is called even when that column is not selected in the
> > query, making it so that the function is called for every row in the
> view,
> > I'm not sure that that would necessarily be the expected behavior, as it
> > was my understanding that columns that are not selected are not
> evaluated,
> > for instance if there was a join in a view that produced some columns and
> > said columns were not selected, I would expect it to be optimized away.
>
> No, this is the expected behavior; we don't like optimization to change
> the number of calls of a volatile function from what would occur in naive
> evaluation of the query.  If that prospect doesn't bother you, it's
> likely because your function isn't really volatile ...
>
> > The other problem is that the function call does not appear in the query
> > plan.
>
> I think "explain verbose" will fix that for you.
>
> regards, tom lane
>

Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c

[Alvaro Herrera]

Ashutosh Bapat wrote:

> Happened to stumble across some instances of lfirst() which could use
> lfirst_node() in planner.c. Here's patch which replaces calls to
> lfirst() extracting node pointers by lfirst_node() in planner.c.

Sounds good.

> Are we carrying out such replacements in master or this will be
> considered for v11?

This is for pg11 definitely; please add to the open commitfest.

-- 
Álvaro Herrerahttps://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] PostgreSQL - Weak DH group

[Heikki Linnakangas]

(We dropped the ball back in October, continuing the discussion now)

On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:

On 10/06/2016 10:26 PM, Christoph Berg wrote:

Re: Heikki Linnakangas 2016-10-06 

I propose the attached patch. It gives up on trying to deal with multiple
key lengths (as noted earlier, OpenSSL just always passed keylength=1024, so
that was useless). Instead of using the callback, it just sets fixed DH
parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The DH
parameters are loaded from a file called "dh_params.pem" (instead of
"dh1024.pem"), if present, otherwise the built-in 2048 bit parameters are
used.


Shouldn't this be a GUC pointing to a configurable location like
ssl_cert_file? This way, people reading the security section of the
default postgresql.conf would notice that there's something (new) to
configure. (And I wouldn't want to start creating symlinks from PGDATA
to /etc/ssl/something again...)


Perhaps. The DH parameters are not quite like other configuration files,
though. The actual parameters used don't matter, you just want to
generate different parameters for every cluster. The key length of the
parameters can be considered configuration, though.


Actually, adding a GUC also solves another grief I had about this. There 
is currently no easy way to tell if your parameter file is being used, 
or if the server is failing to read it and is falling back to the 
hard-coded parameters. With a GUC, if the GUC is set it's a good 
indication that the admin expects the file to really exist and to 
contain valid parameters. So if the GUC is set, we should throw an error 
if it cannot be used. And if it's not set, we use the built-in defaults.


I rebased the patch, did some other clean up of error reporting, and 
added a GUC along those lines, as well as docs. How does this look?


It's late in the release cycle, but it would be nice to sneak this into 
v10. Using weak 1024 bit DH parameters is arguably a security issue; it 
was originally reported as such. There's a work-around for older 
versions: generate custom 2048 bit parameters and place them in a file 
called "dh1024.pem", but that's completely undocumented.


Thoughts? Objections to committing this now, instead of waiting for v11?

- Heikki

>From 481e019441dadc0826e6e9b7d4a56c49e63c654b Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Thu, 13 Jul 2017 18:29:48 +0300
Subject: [PATCH 1/1] Always use 2048 bit DH parameters for OpenSSL ephemeral
 DH ciphers.

1024 bits is considered weak these days, but OpenSSL always passes 1024 as
the key length to the tmp_dh callback. All the code to handle other key
lengths as in fact dead.

To remedy those issues:

* Only include hard-coded 2048-bit parameters.
* Set the parameters directly with SSL_CTX_set_tmp_dh(), without the
  callback
* The name of the file containing the DH parameters is now a GUC. This
  replaces the old hardcoded "dh1024.pem" filename. (The files for other
  key lengths, dh512.pem, dh2048.pem, etc. were never actually used.)

Per report by Nicolas Guini

Discussion: https://www.postgresql.org/message-id/camxbouyjooautvozn6ofzym828anrdjuccotccquxjws-l2...@mail.gmail.com
---
 doc/src/sgml/config.sgml  |  23 +++
 src/backend/libpq/be-secure-openssl.c | 265 +-
 src/backend/libpq/be-secure.c |   1 +
 src/backend/utils/misc/guc.c  |  11 ++
 src/backend/utils/misc/postgresql.conf.sample |   1 +
 src/include/libpq/libpq.h |   1 +
 6 files changed, 132 insertions(+), 170 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 80d1679b14..6c1452a9bc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1203,6 +1203,29 @@ include_dir 'conf.d'
   
  
 
+ 
+  ssl_dh_params_file (string)
+  
+   ssl_ecdh_curve configuration parameter
+  
+  
+  
+   
+Specifies the name of the file containing Diffie-Hellman parameters used for
+so-called Perfect Forward Secrecy SSL ciphers. The default is empty, in
+which case compiled-in default DH parameters used. Using custom DH
+parameters reduces the exposure if an attacker manages to crack the
+well-known compiled-in DH parameters. You can create your own DH parameters
+file with the command openssl dhparam -out dhparams.pem 2048.
+   
+
+   
+This parameter can only be set in the postgresql.conf
+file or on the server command line.
+   
+  
+ 
+
  
   krb_server_keyfile (string)
   
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 67145e9412..ed1779d6b6 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -61,23 +61,22 @@
 #include "libpq/libpq.h"
 #include 

Re: [HACKERS] building libpq.a static library

[Jeroen Ooms]

On Thu, Jul 13, 2017 at 4:58 AM, Craig Ringer  wrote:
> You shouldn't ever need static libraries on Windows, though. Because it
> searches the CWD first on its linker search path, you can just drop
> libpq.dll in the same directory as your binary/library and link to the stub
> libpq.lib .

This is not possible in our case. The R programming language binaries
are installed in the "program files" directory which is only writable
by the sys admin.

There are over 10.000 contributed packages (including one with
postgres drivers) which are installed by the user in the home dir and
the package DLL's need to get dynamically loaded at runtime. We have
been working with this system for a very long time and static linking
external libs to the package DLL is really the only reliable way to
prevent DLL problems across Windows versions/configurations

> I'm not trying to block support for a static libpq, I just don't see the
> point.

This is a bit overgeneralized, there are many use cases where static
linking is the only feasible way. Most libs support --enable static
and many distros ship both static and shared libs and leave it up to
user or developer author how to configure their application.


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

Re: [HACKERS] pl/perl extension fails on Windows

[Tom Lane]

Andrew Dunstan  writes:
> On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>> -dVAR; dXSBOOTARGSAPIVERCHK;
>> +dVAR; dXSBOOTARGSNOVERCHK;

> Good job hunting this down!
> One suggestion I saw in a little googling was that we add this to the XS
> file after the inclusion of XSUB.h:
> #undef dXSBOOTARGSAPIVERCHK
> #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK

I don't see anything even vaguely like that in the Util.c file generated
by Perl 5.10.1, which is what I've got on my RHEL machine.

What I do notice is this in Util.xs:

VERSIONCHECK: DISABLE

which leads immediately to two questions:

1. Why is your version of xsubpp apparently ignoring this directive
and generating a version check anyway?

2. Why do we have this directive in the first place?  It does not seem
to me like a terribly great idea to ignore low-level version mismatches.

In the same vein, I'm suspicious of proposals to "fix" this problem
by removing the version check, which seems to be where Ashutosh
is headed.  In the long run that seems certain to cause huge headaches.

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 10 release notes

[Adrien Nayrat]

Hello hackers,

From: Peter Geoghegan 
> Date: Wed, 5 Jul 2017 15:19:57 -0700
> Subject: Re: [BUGS] BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit 
> overflow
> On pgsql-b...@postgresql.org

On 07/06/2017 12:19 AM, Peter Geoghegan wrote:
> In Postgres 10, tuplesort external sort run merging became much faster
> following commit 24598337c8d. It might be noticeable if such a machine
> were using Postgres 10 [...]

Should-we mention this improvement in release notes?

Regards,

-- 
Adrien NAYRAT

http://dalibo.com - http://dalibo.org



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

[Marina Polyakova]

Another detail I forgot about this point: there may be a memory leak
on variables copies, ISTM that the "variables" array is never freed.

I was not convinced by the overall memory management around variables
to begin with, and it is even less so with their new copy management.
Maybe having a clean "Variables" data structure could help improve the
situation.


Ok!

--
Marina Polyakova
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] WIP Patch: Pgbench Serialization and deadlock errors

[Marina Polyakova]

Here are a round of comments on the current version of the patch:


Thank you very much again!


There is a latent issue about what is a transaction. For pgbench a
transaction is a full script execution.
For postgresql, it is a statement or a BEGIN/END block, several of
which may appear in a script. From a retry
perspective, you may retry from a SAVEPOINT within a BEGIN/END
block... I'm not sure how to make general sense
of all this, so this is just a comment without attached action for now.


Yes it is. That's why I wrote several notes about it in documentation 
where there may be a misunderstanding:


+Transactions with serialization or deadlock failures (or with 
both

+of them if used script contains several transactions; see
++endterm="transactions-and-scripts-title"> for more information) 
are

+marked separately and their time is not reported as for skipped
+transactions.

+ 
+  What is the 
Transaction Actually Performed in 
pgbench?


+If a transaction has serialization and/or deadlock failures, its
+   time will be reported as serialization 
failure,

+   deadlock failure, or
+   serialization and deadlock failures, respectively.
   
+  
+   
+ Transactions can have both serialization and deadlock failures if 
the

+ used script contained several transactions.  See
+  for more information.
+
+  

+  
+   
+The number of transactions attempts within the interval can be 
greater than
+the number of transactions within this interval multiplied by the 
maximum

+attempts number.  See  for more information.
+   
+  

+   
+ The total sum of per-command failures of each type can 
be greater

+ than the number of transactions with reported failures.
+ See + endterm="transactions-and-scripts-title"> for more 
information.

+ 
+   

And I didn't make rollbacks to savepoints after the failure because they 
cannot help for serialization failures at all: after rollback to 
savepoint a new attempt will be always unsuccessful.



I would consider using "try/tries" instead of "attempt/attempts" as it
is shorter. An English native speaker
opinion would be welcome on that point.


Thank you, I'll change it.


I'm fine with renaming "is_latencies" to "report_per_command", which
is more logical & generic.


Glad to hear it!


"max_attempt_number": I'm against typing fields again in their name,
aka "hungarian naming". I'd suggest
"max_tries" or "max_attempts".


Ok!


"SimpleStats attempts": I disagree with using this floating poiunt
oriented structures to count integers.
I would suggest "int64 tries" instead, which should be enough for the 
purpose.


I'm not sure that it is enough. Firstly it may be several transactions 
in script so to count the average attempts number you should know the 
total number of runned transactions. Secondly I think that stddev for 
attempts number can be quite interesting and often it is not close to 
zero.



LastBeginState -> RetryState? I'm not sure why this state is a pointer
in CState. Putting the struct would avoid malloc/free cycles. Index
"-1" may be used to tell it is not set if necessary.


Thanks, I agree that it's better to do in this way.

"CSTATE_RETRY_FAILED_TRANSACTION" -> "CSTATE_RETRY" is simpler and 
clear enough.


Ok!


In CState and some code, a failure is a failure, maybe one boolean
would be enough. It need only be differentiated when counting, and you
have (deadlock_failure || serialization_failure) everywhere.


I agree with you. I'll change it.


Some variables, such as "int attempt_number", should be in the client
structure, not in the client? Generally, try to use block variables if
possible to keep the state clearly disjoints. If there could be NO new
variable at the doCustom level that would be great, because that would
ensure that there is no machine state mixup hidden in these variables.


Do you mean the code cleanup for doCustom function? Because if I do so 
there will be two code styles for state blocks and their variables in 
this function..



I wondering whether the RETRY & FAILURE states could/should be merged:

  on RETRY:
-> count retry
-> actually retry if < max_tries (reset client state, jump to 
command)

-> else count failure and skip to end of script

The start and end of transaction detection seem expensive (malloc,
...) and assume a one statement per command (what about "BEGIN \; ...
\; COMMIT;", which is not necessarily the case, this limitation should
be documented. ISTM that the space normalization should be avoided,
and something simpler/lighter should be devised? Possibly it should
consider handling SAVEPOINT.


I divided these states because if there's a failed transaction block you 
should end it before retrying. It means to go to states 
CSTATE_START_COMMAND -> CSTATE_WAIT_RESULT -> CSTATE_END_COMMAND with 
the appropriate command. How do you propose not to go to these states?


About 

Re: [HACKERS] Race condition in GetOldestActiveTransactionId()

[Heikki Linnakangas]

On 08/22/2016 01:46 PM, Heikki Linnakangas wrote:

While hacking on the CSN patch, I spotted a race condition between
GetOldestActiveTransactionId() and GetNewTransactionId().
GetOldestActiveTransactionId() calculates the oldest XID that's still
running, by doing:

1. Read nextXid, without a lock. This is the upper bound, if there are
no active XIDs in the proc array.
2. Loop through the proc array, making note of the oldest XID.

While GetNewTransactionId() does:

1. Read and Increment nextXid
2. Set MyPgXact->xid.

It seems possible that if you call GetNewTransactionId() concurrently
with GetOldestActiveTransactionId(), GetOldestActiveTransactionId() sees
the new nextXid value that the concurrent GetNewTransactionId() set, but
doesn't see the old XID in the proc array. It will return a value that
doesn't cover the old XID, i.e. it won't consider the just-assigned XID
as in-progress.

Am I missing something? Commit c6d76d7c added a comment to
GetOldestActiveTransactionId() explaining why it's not necessary to
acquire XidGenLock there, but I think it missed the above race condition.

GetOldestActiveTransactionId() is not performance-critical, it's only
called when performing a checkpoint, so I think we should just bite the
bullet and grab the lock. Per attached patch.


I did some testing, and was able to indeed construct a case where 
oldestActiveXid was off-by-one in an online checkpoint record. However, 
looking at how it's used, I think it happened to not have any 
user-visible effect. The oldestActiveXid value of an online checkpoint 
is only used to determine the point where pg_subtrans is initialized, 
and the XID missed in the computation could not be a subtransaction.


Nevertheless, it's clearly a bug and the fix is simple, so I committed a 
fix.


- Heikki



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

Re: [HACKERS] pl/perl extension fails on Windows

[Andrew Dunstan]

On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>
> After doing some study, I could understand that Util.c is generated
> from Util.xs by xsubpp compiler at build time. This is being done in
> Mkvcbuild.pm file in postgres. If I manually replace
> 'dXSBOOTARGSAPIVERCHK' macro with 'dXSBOOTARGSNOVERCHK' macro in
> src/pl/plperl/Util.c, the things work fine. The diff is as follows,
>
> XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
> {
> #if PERL_VERSION_LE(5, 21, 5)
> dVAR; dXSARGS;
> #else
> -dVAR; dXSBOOTARGSAPIVERCHK;
> +dVAR; dXSBOOTARGSNOVERCHK;
>
> I need to further investigate, but let me know if you have any ideas.




Good job hunting this down!


One suggestion I saw in a little googling was that we add this to the XS
file after the inclusion of XSUB.h:

#undef dXSBOOTARGSAPIVERCHK
#define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK

cheers

andrew

-- 
Andrew Dunstanhttps://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] Bug in ExecModifyTable function and trigger issues for foreign tables

[Etsuro Fujita]

On 2017/06/30 18:44, Etsuro Fujita wrote:

On 2017/06/16 21:29, Etsuro Fujita wrote:
I'll have second thought about this, so I'll mark this as waiting on 
author.


I spent quite a bit of time on this and came up with a solution for 
addressing the concern mentioned by Ashutosh [1].  The basic idea is, as 
I said before, to move rewriteTargetListUD from the rewriter to the 
planner (whether the update or delete is inherited or not), except for 
the view case.  In case of inherited UPDATE/DELETE, the planner adds a 
necessary junk column needed for the update or delete for each child 
relation, without the assumption I made before about junk tlist entries, 
so I think this would be more robust for future changes as mentioned in 
[1].  It wouldn't work well that the planner does the same thing for the 
view case (ie, add a whole-row reference to the given target relation), 
unlike other cases, because what we need to do for that case is to add a 
whole-row reference to the view as the source for an INSTEAD OF trigger, 
not the target.  So, ISTM that it's reasonable to handle that case in 
the rewriter as-is, not in the planner, but one thing I'd like to 
propose to simplify the code in rewriteHandler.c is to move the code for 
the view case in rewriteTargetListUD to ApplyRetrieveRule.  By that 
change, we won't add a whole-row reference to the view in RewriteQuery, 
so we don't need this annoying thing in rewriteTargetView any more:


/*
 * For UPDATE/DELETE, rewriteTargetListUD will have added a 
wholerow junk
 * TLE for the view to the end of the targetlist, which we no 
longer need.

 * Remove it to avoid unnecessary work when we process the targetlist.
 * Note that when we recurse through rewriteQuery a new junk TLE 
will be

 * added to allow the executor to find the proper row in the new target
 * relation.  (So, if we failed to do this, we might have multiple junk
 * TLEs with the same name, which would be disastrous.)
 */
if (parsetree->commandType != CMD_INSERT)
{
TargetEntry *tle = (TargetEntry *) llast(parsetree->targetList);

Assert(tle->resjunk);
Assert(IsA(tle->expr, Var) &&
   ((Var *) tle->expr)->varno == parsetree->resultRelation &&
   ((Var *) tle->expr)->varattno == 0);
parsetree->targetList = list_delete_ptr(parsetree->targetList, 
tle);

}

Attached is an updated version of the patch.  Comments are welcome!

Best regards,
Etsuro Fujita

[1] 
https://www.postgresql.org/message-id/CAFjFpRfTpamoUz6fNyk6gPh%3DecfBJjbUHJNKbDxscpyPJ3FfjQ%40mail.gmail.com
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***
*** 6924,6929  update bar set f2 = f2 + 100 returning *;
--- 6924,6988 
7 | 277
  (6 rows)
  
+ -- Test that UPDATE/DELETE with inherited target works with row-level triggers
+ CREATE TRIGGER trig_row_before
+ BEFORE UPDATE OR DELETE ON bar2
+ FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+ CREATE TRIGGER trig_row_after
+ AFTER UPDATE OR DELETE ON bar2
+ FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+ explain (verbose, costs off)
+ update bar set f2 = f2 + 100;
+   QUERY PLAN  

+ 
--
+  Update on public.bar
+Update on public.bar
+Foreign Update on public.bar2
+  Remote SQL: UPDATE public.loct2 SET f2 = $2 WHERE ctid = $1 RETURNING 
f1, f2, f3
+->  Seq Scan on public.bar
+  Output: bar.f1, (bar.f2 + 100), bar.ctid
+->  Foreign Scan on public.bar2
+  Output: bar2.f1, (bar2.f2 + 100), bar2.f3, bar2.ctid, bar2.*
+  Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
+ (9 rows)
+ 
+ update bar set f2 = f2 + 100;
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (3,333,33),NEW: (3,433,33)
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (4,344,44),NEW: (4,444,44)
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,277,77),NEW: (7,377,77)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (3,333,33),NEW: (3,433,33)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (4,344,44),NEW: (4,444,44)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,277,77),NEW: (7,377,77)
+ explain (verbose, costs off)
+ delete from bar where f2 < 400;
+  QUERY PLAN   
   
+ 
-
+  Delete on public.bar
+Delete on public.bar
+Foreign Delete on public.bar2
+  Remote SQL: DELETE FROM public.loct2 WHERE ctid = $1 RETURNING f1, f2, f3
+->  Seq Scan on 

Re: [HACKERS] pl/perl extension fails on Windows

[Ashutosh Sharma]

On Thu, Jul 13, 2017 at 12:01 AM, Andrew Dunstan
 wrote:
> On 07/12/2017 11:49 AM, Dave Page wrote:
>>
>>
>> Well crake is a Fedora box - and we have no problems on Linux, only on
>> Windows.
>>
>>
>
>
> Yeah, I have this on one of my Windows boxes, and haven't had time to
> get to the bottom of it yet ;-(
>

I could also see this problem in my Windows machine and I have spent
some time to analyse it.  Here are my findings:

The stacktrace where the crash happens is:

  perl524.dll!Perl_xs_handshake(const unsigned long key, void
*v_my_perl, const char * file, ...)  Line 5569 C
  plperl.dll!boot_PostgreSQL__InServer__Util(interpreter * my_perl, cv
* cv)  Line 490 + 0x22 bytes C
  perl524.dll!Perl_pp_entersub(interpreter * my_perl)  Line 3987 + 0xc bytes C
  perl524.dll!Perl_runops_standard(interpreter * my_perl)  Line 41 + 0x6 bytes C
  perl524.dll!S_run_body(interpreter * my_perl, long oldscope)  Line 2485 C
  perl524.dll!perl_run(interpreter * my_perl)  Line 2406 + 0xc bytes C
  plperl.dll!plperl_init_interp()  Line 829 + 0xb bytes C
  plperl.dll!_PG_init()  Line 470 + 0x5 bytes C

I couldn't get much out of above bt, but, one thing i could notice is
that the input key passed to 'Perl_xs_handshake()' function is not
matching with the key being generated inside 'Perl_xs_handshake()',
hence, the handshaking is failing.

Please have a look into the following code snippet from 'perl 5.24'
and 'Util.c' file in plperl respectively,

Perl-5.24:
===
got = INT2PTR(void*, (UV)(key & HSm_KEY_MATCH));
need = (void *)(HS_KEY(FALSE, FALSE, "", "") & HSm_KEY_MATCH);
if (UNLIKELY(got != need))
goto bad_handshake;

Util.c in plperl:
==
485 XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
486 {
487 #if PERL_VERSION_LE(5, 21, 5)
488dVAR; dXSARGS;
489 #else
490dVAR; dXSBOOTARGSAPIVERCHK;

Actually the macro 'dXSBOOTARGSAPIVERCHK' in line #490 gets expanded to,

#define XS_APIVERSION_SETXSUBFN_POPMARK_BOOTCHECK
 \
Perl_xs_handshake(HS_KEY(TRUE, TRUE, "v" PERL_API_VERSION_STRING,
""),  \
HS_CXT, __FILE__, "v" PERL_API_VERSION_STRING)

And the point to be noted is, the way, the key is being generated in
above macro and in Perl_xs_handshake(). As shown above,
'XS_APIVERSION_SETXSUBFN_POPMARK_BOOTCHECK' macro passes
'PERL_API_VERSION_STRING' as input to HS_KEY() to generate the key and
this key is passed to Perl_xs_handshake function whereas inside
Perl_xs_handshake(), there is no such version string being used to
generate the key. Hence, the key mismatch is seen thereby resulting in
a bad_handshake error.

After doing some study, I could understand that Util.c is generated
from Util.xs by xsubpp compiler at build time. This is being done in
Mkvcbuild.pm file in postgres. If I manually replace
'dXSBOOTARGSAPIVERCHK' macro with 'dXSBOOTARGSNOVERCHK' macro in
src/pl/plperl/Util.c, the things work fine. The diff is as follows,

XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
{
#if PERL_VERSION_LE(5, 21, 5)
dVAR; dXSARGS;
#else
-dVAR; dXSBOOTARGSAPIVERCHK;
+dVAR; dXSBOOTARGSNOVERCHK;

I need to further investigate, but let me know if you have any ideas.

>
> Latest versions of ActivePerl don't ship with library descriptor files,
> either, which is unpleasant.
>

I too had similar observation and surprisingly, I am seeing
'libperl*.a' file instead of 'perl*.lib' file in most of  the
ActiverPerl versions for Windows.

--
With Regards,
Ashutosh Sharma
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_stop_backup(wait_for_archive := true) on standby server

[Michael Paquier]

On Thu, Jul 13, 2017 at 7:13 AM, Masahiko Sawada  wrote:
> Sorry, I missed lots of typo in the last patch. All comments from you
> are incorporated into the attached latest patch and I've checked it
> whether there is other typos. Please review it.

Thanks for providing a new version of the patch very quickly. I have
spent some time looking at it again and testing it, and this version
looks correct to me. Stephen, as a potential committer, would you look
at it to address this open item?
-- 
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] [WIP] Zipfian distribution in pgbench

[Alik Khilazhev]

On 13 Jul 2017, at 00:20, Peter Geoghegan  wrote:Actually, I mean that I wonder how much of a difference it would makeif this entire block was commented out within _bt_doinsert():if (checkUnique != UNIQUE_CHECK_NO){    …}I am attaching results of test for 32 and 128 clients for original and patched(_bt_doinsert) variants.— Thanks and Regards,Alik KhilazhevPostgres Professional:http://www.postgrespro.comThe Russian Postgres Company  idx  | level | l_item | blkno | btpo_prev | btpo_next | 
btpo_flags | type | live_items | dead_items | avg_item_size | page_size | 
free_size | highkey 
---+---++---+---+---++--+++---+---+---+-
 pgbench_accounts_pkey | 2 |  1 |   290 | 0 | 0 |   
   2 | r| 10 |  0 |15 |  8192 |  7956 | 
 pgbench_accounts_pkey | 1 |  1 | 3 | 0 |   289 |   
   0 | i|298 |  0 |15 |  8192 |  2196 | 
09 96 01 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  2 |   289 | 3 |   575 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
11 2c 03 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  3 |   575 |   289 |   860 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
19 c2 04 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  4 |   860 |   575 |  1145 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
21 58 06 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  5 |  1145 |   860 |  1430 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
29 ee 07 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  6 |  1430 |  1145 |  1715 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
31 84 09 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  7 |  1715 |  1430 |  2000 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
39 1a 0b 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  8 |  2000 |  1715 |  2285 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
41 b0 0c 00 00 00 00 00
 pgbench_accounts_pkey | 1 |  9 |  2285 |  2000 |  2570 |   
   0 | i|285 |  0 |15 |  8192 |  2456 | 
49 46 0e 00 00 00 00 00
 pgbench_accounts_pkey | 1 | 10 |  2570 |  2285 | 0 |   
   0 | i|177 |  0 |15 |  8192 |  4616 | 
 pgbench_accounts_pkey | 0 |  1 | 1 | 0 |  2751 |   
  65 | l| 21 |225 |16 |  8192 |  3228 | 
14 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  2 |  2751 | 1 |  2746 |   
  65 | l| 40 |182 |16 |  8192 |  3708 | 
3b 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  3 |  2746 |  2751 |  2750 |   
  65 | l| 43 |240 |16 |  8192 |  2488 | 
65 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  4 |  2750 |  2746 |  2745 |   
  65 | l| 51 | 57 |16 |  8192 |  5988 | 
97 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  5 |  2745 |  2750 |  2756 |   
  65 | l| 42 | 47 |16 |  8192 |  6368 | 
c0 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  6 |  2756 |  2745 |  2748 |   
  65 | l| 54 |139 |16 |  8192 |  4288 | 
f5 00 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  7 |  2748 |  2756 |  2755 |   
  65 | l| 57 |333 |16 |  8192 |   348 | 
2d 01 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  8 |  2755 |  2748 | 2 |   
  65 | l| 67 |308 |16 |  8192 |   648 | 
6f 01 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 |  9 | 2 |  2755 |  2753 |   
  65 | l| 75 |280 |16 |  8192 |  1048 | 
b9 01 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 | 10 |  2753 | 2 |  2747 |   
  65 | l| 83 |260 |16 |  8192 |  1288 | 
0b 02 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 | 11 |  2747 |  2753 |  2754 |   
  65 | l| 91 |192 |16 |  8192 |  2488 | 
65 02 00 00 00 00 00 00
 pgbench_accounts_pkey | 0 | 12 |  2754 |  2747 | 4 |   
  65 | l|121 |196 |16 

Re: [HACKERS] PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

[Ashutosh Bapat]

On Thu, Jul 13, 2017 at 2:53 PM, Kyotaro HORIGUCHI
 wrote:
> Hello, moved to pgsql-hackers.
>
> This is the revased and revised version of the previous patch.
>
> At Thu, 13 Jul 2017 13:42:49 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
>  wrote in 
> <20170713.134249.97825982.horiguchi.kyot...@lab.ntt.co.jp>
>> At Tue, 11 Jul 2017 15:39:14 -0400, Tom Lane  wrote in 
>> <6234.1499801...@sss.pgh.pa.us>
>> > Amit Langote  writes:
>> > > Horiguchi-san,
>> > > On 2017/07/11 10:28, Kyotaro HORIGUCHI wrote:
>> > >> I faintly recall such discussion was held aroud that time and
>> > >> maybe we concluded that we don't do that but I haven't find such
>> > >> a thread in pgsql-hackers..
>> >
>> > > I mentioned it in my reply.  Here again:
>> > > https://www.postgresql.org/message-id/20160405.184408.166437663.horiguchi.kyotaro%40lab.ntt.co.jp
>> >
>> > The followup discussion noted that that approach was no good because it
>> > would only close connections in the same session that had done the ALTER
>> > SERVER.  I think the basic idea of marking postgres_fdw connections as
>> > needing to be remade when next possible is OK, but we have to drive it
>> > off catcache invalidation events, the same as we did in c52d37c8b.  An
>> > advantage of that way is we don't need any new hooks in the core code.
>> >
>> > Kyotaro-san, are you planning to update your old patch?
>>
>> I'm pleased to do that. I will reconsider the way shown in a mail
>> in the thread soon.
>
> done.
>
> (As a recap) Changing of some options such as host of a foreign
> server or password of a user mapping make the existing
> connections stale, but postgres_fdw continue using them.
>
> The attached patch does the following things.
>
> - postgres_fdw registers two invalidation callbacks on loading.
>
> - On any change on a foreign server or a user mapping, the
>   callbacks mark the affected connection as 'invalid'
>
> - The invalidated connections will be once disconnected before
>   the next execution if no transaction exists.
>
> As the consequence, changes of options properly affects
> subsequent queries in the next transaction on any session. It
> occurs after whatever option has been modifed.
>
> ==
> create server sv1 foreign data wrapper postgres_fdw options (host '/tmp', 
> port '5432', dbname 'postgres');
> create user mapping for public server sv1;
> create table t (a int);
> create foreign table ft1 (a int) server sv1 options (table_name 't1');
> begin;
> select * from ft1; -- no error
> alter server sv1 options (set host '/tmpe');
> select * from ft1; -- no error because transaction is living.
> commit;
> select * from ft1;
> ERROR:  could not connect to server "sv1"  - NEW
> ==
>
> This patch is postgres_fdw-private but it's annoying that hash
> value of syscache is handled in connection.c. If we allow to add
> something to the core for this feature, I could add a new member
> in FdwRoutine to notify invalidation of mapping and server by
> oid. (Of course it is not back-patcheable, though)
>
> Does anyone have opinitons or suggestions?
>

The patch and the idea looks good to me. I haven't reviewed it
thoroughly though.

I noticed a type "suporious", I think you meant "spurious"? Probably
that comment should be part of the function which marks the connection
as invalid e.g. InvalidateConnectionForMapping().

pgfdw_xact_callback() reports the reason for disconnection while
closing a connection. May be we want to report the reason for
disconnection here as well. Also, may be we want to create a function
to discard connection from an entry so that we consistently do the
same things while discarding a connection.

-- 
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] Update description of \d[S+] in \?

[Ashutosh Bapat]

On Thu, Jul 13, 2017 at 12:01 PM, Amit Langote
 wrote:
> The description of \d[S+] currently does not mention that it will list
> materialized views and foreign tables.  Attached fixes that.
>

I guess the same change is applicable to the description of \d[S+] NAME as well.


-- 
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] Row Level Security Documentation

[Fabien COELHO]

Hello Rod,


This version of the table attempts to stipulate which section of the
process the rule applies to.


A few comments about this patch. It applies cleanly, make html is ok.

It adds a summary table which shows for each case what happens. Although 
the information can be guessed/infered from the text, I think that a 
summary table is a good thing, at least because it breaks an otherwise 
dense presentation.


I would suggest the following changes:

The table should be referenced from the description, something like "Table 
xxx summarizes the ..."


ISTM that it would be clearer to split the Policy column into "FOR xxx 
..." and "USING" or "WITH CHECK", and to merge the rows which have the 
same "FOR xxx ..." contents, something like:


   POLICY |
  ---++-
 | USING  | ...
  FOR ALL ...++-
 | WITH CHECK | ...
  ---++-
  FOR SELECT ... | USING  | ...

So that it is clear that only ALL & UPDATE can get both USING & WITH 
CHECK. This can be done with "morerows=1" on an entry so that it spans 
more rows.


For empty cells, maybe a dash would be clearer. Not sure.

--
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] New partitioning - some feedback

[Amit Langote]

On 2017/07/13 7:23, Dean Rasheed wrote:
> On 12 July 2017 at 15:58, Alvaro Herrera  wrote:
>> Amit Langote wrote:
>>> On 2017/07/11 13:34, Alvaro Herrera wrote:
 However, the "list tables"
 command \dt should definitely IMO not list partitions.
>>>
>>> Do you mean never?  Even if a modifier is specified?  In the patch I
>>> proposed, \d! (or \d+ or \d++, if '!' turns out to be unpopular) will list
>>> partitions, but \d or \dt won't.  That is, partitions are hidden by default.
>>
>> I don't think there is any need for a single list of all partition of
>> all tables -- is there?  I can't think of anything, but then I haven't
>> been exposed very much to this feature yet.  For now, I lean towards "never".
>>
> 
> So just focusing on the listing issue for now...
> 
> I tend to agree with some of the upstream comments that a bare \d
> should list everything, including partitions, because partitions are
> still tables that you might want to do DML or DDL on.
> 
> Also, if you look at what we already have, \d lists all types of
> relations, and then there are 2-letter commands \dE, \di, \dm, \ds,
> \dt and \dv that list just specific kinds of relations, for example
> \dE lists foreign tables, and \dt lists local tables, specifically
> excluding foreign tables.
> 
> So ISTM that the most logical extension of that is:
> 
>   \d - list all relations, including partitions

\d does leave out indexes, but that seems okay.  I think it might be okay
to show partitions after all.  If we do so, do we indicate somehow that
they are partitions of some table?  Maybe an additional column "Partition"
with values "yes" or "no" that occurs right next to the Type column.
Output would look something like below:

\d
 List of relations
 Schema |   Name|   Type| Partition | Owner
+---+---+---+---
 public | foo   | table | no| amit
 public | foo_a_seq | sequence  | no| amit
 public | xyz   | partitioned table | no| amit
 public | xyz1  | table | yes   | amit
 public | xyz2  | table | yes   | amit
 public | xyz3  | partitioned table | yes   | amit
 public | xyz4  | foreign table | yes   | amit
(7 rows)


>   \dt - list only tables that are not foreign tables or partitions
> of other tables

Note that that list will include partitioned tables.

> (that's not quite an exact extension of the existing logic, because of
> course it's partitioned tables that have the different relkind, not
> the partitions, but the above seems like the most useful behaviour)

We allow creating regular tables, partitioned tables, and foreign tables
as partitions.  Being a partition is really independent from the
considerations with which these 2-letter commands are designed, that is,
the second letters map one-to-one with relkinds (again, an exception made
when showing both regular tables and partitioned table with \dt.)

If we establish a rule that each such 2-letter command will only show the
tables of the corresponding relkind that are not partitions, that is, only
those for which relispartition=false will be shown, then we should find an
extension/modifier such that for each command it enables listing
partitions as well.

Perhaps the idea you mentioned at [1] of using letter 'P' for that purpose
could work.  As you described, \dtP or \dPt shows tables (partitioned or
not) including those that are partitions.  Bare \d will mean \dPtvmsE.

> I also agree that there probably isn't much need for a list that
> *only* includes partitions, but if someone comes up with a convincing
> use case, then we could add another 2-letter command for that.

I too can't imagine needing to see only partitions.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CAEZATCWcfFtsbKYcVyqUzoOsxkikQjpi_GdjZ_vL6RcX8iLEsg%40mail.gmail.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] PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING

[Kyotaro HORIGUCHI]

Hello, moved to pgsql-hackers.

This is the revased and revised version of the previous patch.

At Thu, 13 Jul 2017 13:42:49 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20170713.134249.97825982.horiguchi.kyot...@lab.ntt.co.jp>
> At Tue, 11 Jul 2017 15:39:14 -0400, Tom Lane  wrote in 
> <6234.1499801...@sss.pgh.pa.us>
> > Amit Langote  writes:
> > > Horiguchi-san,
> > > On 2017/07/11 10:28, Kyotaro HORIGUCHI wrote:
> > >> I faintly recall such discussion was held aroud that time and
> > >> maybe we concluded that we don't do that but I haven't find such
> > >> a thread in pgsql-hackers..
> > 
> > > I mentioned it in my reply.  Here again:
> > > https://www.postgresql.org/message-id/20160405.184408.166437663.horiguchi.kyotaro%40lab.ntt.co.jp
> > 
> > The followup discussion noted that that approach was no good because it
> > would only close connections in the same session that had done the ALTER
> > SERVER.  I think the basic idea of marking postgres_fdw connections as
> > needing to be remade when next possible is OK, but we have to drive it
> > off catcache invalidation events, the same as we did in c52d37c8b.  An
> > advantage of that way is we don't need any new hooks in the core code.
> > 
> > Kyotaro-san, are you planning to update your old patch?
> 
> I'm pleased to do that. I will reconsider the way shown in a mail
> in the thread soon.

done.

(As a recap) Changing of some options such as host of a foreign
server or password of a user mapping make the existing
connections stale, but postgres_fdw continue using them.

The attached patch does the following things.

- postgres_fdw registers two invalidation callbacks on loading.

- On any change on a foreign server or a user mapping, the
  callbacks mark the affected connection as 'invalid'

- The invalidated connections will be once disconnected before
  the next execution if no transaction exists.

As the consequence, changes of options properly affects
subsequent queries in the next transaction on any session. It
occurs after whatever option has been modifed.

==
create server sv1 foreign data wrapper postgres_fdw options (host '/tmp', port 
'5432', dbname 'postgres');
create user mapping for public server sv1;
create table t (a int);
create foreign table ft1 (a int) server sv1 options (table_name 't1');
begin;
select * from ft1; -- no error
alter server sv1 options (set host '/tmpe');
select * from ft1; -- no error because transaction is living.
commit;
select * from ft1;
ERROR:  could not connect to server "sv1"  - NEW
==

This patch is postgres_fdw-private but it's annoying that hash
value of syscache is handled in connection.c. If we allow to add
something to the core for this feature, I could add a new member
in FdwRoutine to notify invalidation of mapping and server by
oid. (Of course it is not back-patcheable, though)

Does anyone have opinitons or suggestions?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
*** a/contrib/postgres_fdw/connection.c
--- b/contrib/postgres_fdw/connection.c
***
*** 53,58  typedef struct ConnCacheEntry
--- 53,61 
  	bool		have_prep_stmt; /* have we prepared any stmts in this xact? */
  	bool		have_error;		/* have any subxacts aborted in this xact? */
  	bool		changing_xact_state;	/* xact state change in process */
+ 	bool		invalidated;	/* true if reconnect is requried */
+ 	uint32		server_hashvalue;	/* hash value of foreign server oid */
+ 	uint32		mapping_hashvalue;  /* hash value of user mapping oid */
  } ConnCacheEntry;
  
  /*
***
*** 150,160  GetConnection(UserMapping *user, bool will_prep_stmt)
--- 153,180 
  		entry->have_prep_stmt = false;
  		entry->have_error = false;
  		entry->changing_xact_state = false;
+ 		entry->invalidated = false;
+ 		entry->server_hashvalue = 0;
+ 		entry->mapping_hashvalue = 0;
  	}
  
  	/* Reject further use of connections which failed abort cleanup. */
  	pgfdw_reject_incomplete_xact_state_change(entry);
  
+ 
+ 	/*
+ 	 * This connection is no longer valid. Disconnect such connections if no
+ 	 * transaction is running. We could avoid suporious disconnection by
+ 	 * examining individual option values but it would be too-much for the
+ 	 * gain.
+ 	 */
+ 	if (entry->conn != NULL && entry->invalidated && entry->xact_depth == 0)
+ 	{
+ 		PQfinish(entry->conn);
+ 		entry->conn = NULL;
+ 		entry->invalidated = false;
+ 	}
+ 
  	/*
  	 * We don't check the health of cached connection here, because it would
  	 * require some overhead.  Broken connection will be detected when the
***
*** 173,178  GetConnection(UserMapping *user, bool will_prep_stmt)
--- 193,202 
  		entry->xact_depth = 0;	/* just to be sure */
  		entry->have_prep_stmt = false;
  		entry->have_error = false;
+ 		entry->server_hashvalue =
+ 			GetSysCacheHashValue1(FOREIGNSERVEROID, server->serverid);
+ 		entry->mapping_hashvalue 

Re: [HACKERS] SCRAM auth and Pgpool-II

[Tatsuo Ishii]

> What I am suggesting here is that in order to handle properly SCRAM
> with channel binding, pgpool has to provide a different handling for
> client <-> pgpool and pgpool <-> Postgres. In short, I don't have a
> better answer than having pgpool impersonate the server and request
> for a password in cleartext through an encrypted connection between
> pgpool and the client if pgpool does not know about it, and then let
> pgpool do by itself the SCRAM authentication on a per-connection basis
> with each Postgres instances. When using channel binding, what would
> matter is the TLS finish (for tls-unique) or the hash server
> certificate between Postgres and pgpool, not between the client and
> pgpool. But that's actually the point you are raising here:

Using a clear text password would not be acceptable for users even
through an encrypted connection, I think.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] Multi column range partition table

[Dean Rasheed]

On 12 July 2017 at 10:46, Ashutosh Bapat
 wrote:
> On Wed, Jul 12, 2017 at 12:54 AM, Dean Rasheed  
> wrote:
>> On 11 July 2017 at 13:29, Ashutosh Bapat
>>> The description in this paragraph seems to be attaching intuitive
>>> meaning of word "unbounded" to MAXVALUE and MINVALUE, which have
>>> different intuitive meanings of themselves. Not sure if that's how we
>>> should describe MAXVALUE/MINVALUE.
>>>
>> I'm not sure I understand your point. MINVALUE and MAXVALUE do mean
>> unbounded below and above respectively. This paragraph is just making
>> the point that that isn't the same as -/+ infinity.
>>
> What confuses me and probably users is something named min/max"value"
> is not a value but something lesser or greater than any other "value".

Ah OK, I see what you're saying.

It's worth noting though that, after a little looking around, I found
that Oracle, MySQL and DB2 all use MINVALUE/MAXVALUE for unbounded
range partitions (although in the case of Oracle and MySQL, they
currently only support specifying upper bounds, and only use MAXVALUE
at the moment).

So MINVALUE/MAXVALUE are likely to be familiar to at least some people
coming from other databases. Of course, for those other databases, the
surrounding syntax for creating partitioned tables is completely
different, but at least this makes the bounds themselves portable (our
supported set of bounds will be a superset of those supported by
Oracle and MySQL, and I think the same as those supported by DB2).

I also personally quite like those terms, because they're nice and
concise, and it's pretty obvious which is which.

Regards,
Dean


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

[HACKERS] More flexible LDAP auth search filters?

[Thomas Munro]

Hi hackers,

A customer asked how to use pg_hba.conf LDAP search+bind
authentication to restrict logins to users in one of a small number of
groups.  ldapsearchattribute only lets you make filters like
"(foo=username)", so it couldn't be done.  Is there any reason we
should allow a more general kind of search filter constructions?

A post on planet.postgresql.org today reminded me that a colleague had
asked me to post this POC patch here for discussion.  It allows custom
filters with ldapsearchprefix and ldapsearchsuffix.  Another approach
might be to take a filter pattern with "%USERNAME%" or whatever in it.
There's an existing precedent for the prefix and suffix approach, but
on the other hand a pattern approach would allow filters where the
username is inserted more than once.

Motivating example:

  ldapsearchprefix="(&(cn="
  ldapsearchsuffix = ")(|(memberof=cn=Paris DBA
Team)(memberof=cn=Tokyo DBA Team))"

Note that with this patch ldapsearchattribute=cn is equivalent to:

  ldasearchprefix="(cn="
  ldapsearchsuffix=")"

Perhaps there are better ways to organise your LDAP servers so that
this sort of thing isn't necessary.  I don't know.  Thoughts?

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


ldap-search-filters-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

[HACKERS] Fix a typo in pg_upgrade/info.c

[Masahiko Sawada]

Hi,

Attached patch for $subject.

s/reporing/reporting/g

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


fix_typo_in_info_c.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] Update description of \d[S+] in \?

[Amit Langote]

The description of \d[S+] currently does not mention that it will list
materialized views and foreign tables.  Attached fixes that.

Thanks,
Amit
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index b3dbb5946e..66fd8b36f9 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -219,7 +219,7 @@ slashUsage(unsigned short int pager)
 
fprintf(output, _("Informational\n"));
fprintf(output, _("  (options: S = show system objects, + = additional 
detail)\n"));
-   fprintf(output, _("  \\d[S+] list tables, views, and 
sequences\n"));
+   fprintf(output, _("  \\d[S+] list tables, views, 
materialized views, sequences, and foreign tables\n"));
fprintf(output, _("  \\d[S+]  NAME   describe table, view, 
sequence, or index\n"));
fprintf(output, _("  \\da[S]  [PATTERN]  list aggregates\n"));
fprintf(output, _("  \\dA[+]  [PATTERN]  list access methods\n"));

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