Re: [HACKERS] custom function for converting human readable sizes to bytes

[Pavel Stehule]

Hi


>>> -1 from me.  I don't think we should muck with the way pg_size_pretty
>>> works.
>>>
>>
>> new update - I reverted changes in pg_size_pretty
>>
>
> Hi,
>
> I didn't check out earlier versions of this patch, but the latest one
> still changes pg_size_pretty() to emit PB suffix.
>
> I don't think it is worth it to throw a number of changes together like
> that.  We should focus on adding pg_size_bytes() first and make it
> compatible with both pg_size_pretty() and existing GUC units: that is
> support suffixes up to TB and make sure they have the meaning of powers of
> 2^10, not 10^3.  Re-using the table present in guc.c would be a plus.
>
> Next, we could think about adding handling of PB suffix on input and
> output, but I don't see a big problem if that is emitted as 1024TB or the
> user has to specify it as 1024TB in a GUC or argument to pg_size_bytes():
> an minor inconvenience only.
>

Last version still support BP in pg_size_pretty. It isn't big change. PB
isn't issue.

We have to do significant decision - should to support SI units in
pg_size_bytes? We cannot to change it later. There is disagreement for SI
units in pg_size_pretty, so SI units in pg_size_bytes can be inconsistent.

Regards

Pavel


>
> --
> Alex
>
>

Re: [HACKERS] proposal: multiple psql option -c

[Bruce Momjian]

On Thu, Dec 10, 2015 at 11:10:55AM -0500, Robert Haas wrote:
> On Wed, Dec 9, 2015 at 12:15 AM, Pavel Stehule  
> wrote:
> >> On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule 
> >> wrote:
> >> > should be noted, recorded somewhere so this introduce possible
> >> > compatibility
> >> > issue - due default processing .psqlrc.
> >>
> >> That's written in the commit log, so I guess that's fine.
> >
> > ook
> 
> Bruce uses the commit log to prepare the release notes, so I guess
> he'll make mention of this.

Yes, I will certainly see it.

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

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


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

[HACKERS] Better detail logging for password auth failures

[Tom Lane]

We often tell people to look in the postmaster log for more information
about authentication problems; but an off-list question prompted me to
notice that many of the common authentication failure cases don't actually
get any useful commentary in the log.  The attached proposed patch
remedies this by adding specific log detail messages for all the
non-out-of-memory cases processed by md5_crypt_verify().  Essentially,
this is just covering cases that I omitted to cover in commit 64e43c59,
for reasons that no longer seem very good to me.

I did not bother with going through the other auth methods in similar
detail.  It seems like only password authentication is in use by people
who are in need of this kind of help.  (But if someone else wants to do
something similar for other auth methods, feel free.)

In passing, the patch gets rid of a vestigial CHECK_FOR_INTERRUPTS()
call; it was added by e710b65c and IMO should have been removed again
by 6647248e.  There's certainly no very good reason to have one right
at that spot anymore.

Any objections?

regards, tom lane

diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 97be944..64c25e8 100644
*** a/src/backend/libpq/crypt.c
--- b/src/backend/libpq/crypt.c
*** md5_crypt_verify(const Port *port, const
*** 50,56 
--- 50,60 
  	/* Get role info from pg_authid */
  	roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(role));
  	if (!HeapTupleIsValid(roleTup))
+ 	{
+ 		*logdetail = psprintf(_("Role \"%s\" does not exist."),
+ 			  role);
  		return STATUS_ERROR;	/* no such user */
+ 	}
  
  	datum = SysCacheGetAttr(AUTHNAME, roleTup,
  			Anum_pg_authid_rolpassword, );
*** md5_crypt_verify(const Port *port, const
*** 71,83 
  	ReleaseSysCache(roleTup);
  
  	if (*shadow_pass == '\0')
  		return STATUS_ERROR;	/* empty password */
! 
! 	CHECK_FOR_INTERRUPTS();
  
  	/*
  	 * Compare with the encrypted or plain password depending on the
! 	 * authentication method being used for this connection.
  	 */
  	switch (port->hba->auth_method)
  	{
--- 75,92 
  	ReleaseSysCache(roleTup);
  
  	if (*shadow_pass == '\0')
+ 	{
+ 		*logdetail = psprintf(_("User \"%s\" has an empty password."),
+ 			  role);
  		return STATUS_ERROR;	/* empty password */
! 	}
  
  	/*
  	 * Compare with the encrypted or plain password depending on the
! 	 * authentication method being used for this connection.  (We do not
! 	 * bother setting logdetail for pg_md5_encrypt failure: the only possible
! 	 * error is out-of-memory, which is unlikely, and if it did happen adding
! 	 * a psprintf call would only make things worse.)
  	 */
  	switch (port->hba->auth_method)
  	{
*** md5_crypt_verify(const Port *port, const
*** 154,159 
--- 163,171 
  		else
  			retval = STATUS_OK;
  	}
+ 	else
+ 		*logdetail = psprintf(_("Password does not match for user \"%s\"."),
+ 			  role);
  
  	if (port->hba->auth_method == uaMD5)
  		pfree(crypt_pwd);

-- 
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] Making tab-complete.c easier to maintain

[Tom Lane]

Michael Paquier  writes:
>> On Sun, Dec 20, 2015 at 6:24 AM, Tom Lane  wrote:
>>> 1. I think it would be a good idea to convert the matching rules for
>>> backslash commands too.  To do that, we'd need to provide a case-sensitive
>>> equivalent to word_match and the matching macros.  I think we'd also have
>>> to extend word_match to allow a trailing wildcard character, maybe "*".

> I am not really sure I follow much the use of the wildcard, do you
> mean to be able to work with the [S] extensions of the backslash
> commands which are not completed now?

But they are completed:

regression=# \dfS str
string_agg  string_agg_transfn  strip
string_agg_finalfn  string_to_array strpos

This is because of the use of strncmp instead of plain strcmp
in most of the backslash matching rules, eg the above case is
covered by

else if (strncmp(prev_wd, "\\df", strlen("\\df")) == 0)

I was envisioning that we'd want to convert this to something like

else if (TailMatchesCS1("\\df*"))

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] Patch: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

Good news, everyone!

I discovered that NUM_LOCK_OF_PARTITIONS is a bottleneck for a last
patch. Long story short - NUM_LOCK_OF_PARTITIONS = (1 << 7) gives best
performance:

PN = 16, AN =  8, NUM_LOCK_PARTITIONS = (1 << 7): 4782.9
PN = 16, AN =  4, NUM_LOCK_PARTITIONS = (1 << 7): 4089.9
PN = 16, AN =  2, NUM_LOCK_PARTITIONS = (1 << 7): 2822.5

PN =  8, AN =  8, NUM_LOCK_PARTITIONS = (1 << 7): 4391.7
PN =  8, AN =  4, NUM_LOCK_PARTITIONS = (1 << 7): 3665.0
PN =  8, AN =  2, NUM_LOCK_PARTITIONS = (1 << 7): 2205.7

PN =  4, AN =  8, NUM_LOCK_PARTITIONS = (1 << 7): 4031.9
PN =  4, AN =  4, NUM_LOCK_PARTITIONS = (1 << 7): 3378.2
PN =  4, AN =  2, NUM_LOCK_PARTITIONS = (1 << 7): 2142.4

Also I tried to optimize global_free_list_* procedures as I planned.
Optimized version is asymptotically faster but works about 5% slower in
practice because of some additional operations we need to perform.
Patch is attached to this message in case you would like to examine a
code.

Here is a funny thing - benchmark results I shared 22.12.2015 are wrong
because I forgot to run `make clean` after changing lwlock.h (autotools
doesn't rebuild project properly after changing .h files). Here are
correct results:

60-core server,
pgbench -j 64 -c 64 -f pgbench.sql -T 50 -P 1 my_database

NUM_LOCK_  |  master  | no locks |  lwlock  | spinlock | spinlock 
PARTITIONS | (99ccb2) |  |  |  |  array   
---|--|--|--|--|--
 1 << 4|  660.4   |  2296.3  |  1441.8  |  454.4   |  1597.8  
---|--|--|--|--|--
 1 << 5|  536.6   |  3006.5  |  1632.3  |  381.8   |  1896.8  
---|--|--|--|--|--
 1 << 6|  469.9   |  4083.5  |  1693.4  |  427.7   |  2271.5  
---|--|--|--|--|--
 1 << 7|  430.8   |  4653.0  |  2016.3  |  361.1   |  3277.1  

As you may see "spinlock array" version works quite well. It is almost
5 times faster than current dynahash implementation and only 30% slower
than "no locks" version. It also guarantees that we will use all
available memory.

I experimented with various improvements of "spinlock array" as well.
E.g. I tried to borrow elements not one a time, but it didn't cause any
performance improvements.

In case you believe that 5 times faster is not good enough we can also
use sharded-global-free-list.patch with appropriate AN and PN values.
Still this patch doesn't guarantee that all available memory will be
used ("no lock" patch doesn't guarantee it either).

Considering that benchmark above is not for all possible cases, but for
a very specific one, and that "spinlock array" patch has much better
guarantees then "no lock" patch, and that lock-free algorithms are
pretty complicated and error-prone I suggest we call "spinlock array"
solution good enough, at least until someone complain again that
dynahash is a bottleneck. Naturally first I clean a code, write more
comments, then re-check once again that there is no performance
degradation according to usual pgbench, etc.diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 78f15f0..91fcc05 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -265,7 +265,7 @@ InitShmemIndex(void)
  */
 HTAB *
 ShmemInitHash(const char *name, /* table string name for shmem index */
-			  long init_size,	/* initial table size */
+			  long init_size,	/* initial table size */ // AALEKSEEV: is ignored, refactor!
 			  long max_size,	/* max size of the table */
 			  HASHCTL *infoP,	/* info about key and bucket size */
 			  int hash_flags)	/* info about infoP */
@@ -299,7 +299,7 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
 	/* Pass location of hashtable header to hash_create */
 	infoP->hctl = (HASHHDR *) location;
 
-	return hash_create(name, init_size, infoP, hash_flags);
+	return hash_create(name, max_size, infoP, hash_flags);
 }
 
 /*
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index eacffc4..07a46db 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -87,6 +87,8 @@
 #include "access/xact.h"
 #include "storage/shmem.h"
 #include "storage/spin.h"
+#include "storage/lock.h"
+#include "storage/lwlock.h"
 #include "utils/dynahash.h"
 #include "utils/memutils.h"
 
@@ -118,6 +120,13 @@ typedef HASHELEMENT *HASHBUCKET;
 /* A hash segment is an array of bucket headers */
 typedef HASHBUCKET *HASHSEGMENT;
 
+// AALEKSEEV: TODO: comment, should be power of two
+#define GLOBAL_FREE_LIST_PARTITIONS_NUM 4
+#define GLOBAL_FREE_LIST_PARTITIONS_MASK (GLOBAL_FREE_LIST_PARTITIONS_NUM-1)
+
+// AALEKSEEV: TODO: comment
+#define GLOBAL_FREE_LIST_ALLOC_NUM 8
+
 /*
  * Header structure for a hash table --- contains all changeable info
  *
@@ -129,11 +138,24 @@ typedef HASHBUCKET *HASHSEGMENT;
 struct HASHHDR
 {
 	/* In a 

Re: [HACKERS] dynloader.h missing in prebuilt package for Windows?

[Bruce Momjian]

On Sun, Dec  6, 2015 at 07:16:24PM +, Olson, Ken wrote:
> I have downloaded a fresh copy of the Win x64 installer 
> (postgresql-9.4.5-2-windows-x64.exe) from 
> http://www.enterprisedb.com/products-services-training/pgdownload. The output 
> of pg_config and assodicated directory listing of include/server:
...
>  Directory of C:\PROGRA~1\POSTGR~1\9.4\include\server
> 
> 11/19/2015  12:19 AM30,882 c.h
> 11/19/2015  12:19 AM30,626 fmgr.h
> 11/19/2015  12:19 AM10,711 funcapi.h
> 11/19/2015  12:19 AM 3,986 getaddrinfo.h
> 11/19/2015  12:19 AM   660 getopt_long.h
> 11/19/2015  12:19 AM15,482 miscadmin.h
> 11/19/2015  12:45 AM21,702 pg_config.h
> 11/19/2015  12:45 AM   253 pg_config_ext.h
> 11/19/2015  12:19 AM10,875 pg_config_manual.h
> 11/19/2015  12:45 AM13,392 pg_config_os.h
> 11/19/2015  12:19 AM 1,084 pg_getopt.h
> 11/19/2015  12:19 AM   316 pg_trace.h
> 11/19/2015  12:19 AM27,166 pgstat.h
> 11/19/2015  12:19 AM   606 pgtar.h
> 11/19/2015  12:19 AM 2,309 pgtime.h
> 11/19/2015  12:19 AM27,489 plpgsql.h
> 11/19/2015  12:19 AM14,096 port.h
> 11/19/2015  12:19 AM21,398 postgres.h
> 11/19/2015  12:19 AM 2,109 postgres_ext.h
> 11/19/2015  12:19 AM   763 postgres_fe.h
> 11/19/2015  12:19 AM   843 rusagestub.h
> 11/19/2015  12:19 AM 2,379 windowapi.h

Sorry, I am just getting to this.  This list helps.  Looking at the
'configure' run on Debian, I see at the bottom:

  config.status: linking src/backend/port/tas/dummy.s to src/backend/port/tas.s
  config.status: linking src/backend/port/dynloader/linux.c to 
src/backend/port/dynloader.c
  config.status: linking src/backend/port/sysv_sema.c to 
src/backend/port/pg_sema.c
  config.status: linking src/backend/port/sysv_shmem.c to 
src/backend/port/pg_shmem.c
  config.status: linking src/backend/port/unix_latch.c to 
src/backend/port/pg_latch.c
1 config.status: linking src/backend/port/dynloader/linux.h to 
src/include/dynloader.h
2 config.status: linking src/include/port/linux.h to src/include/pg_config_os.h
  config.status: linking src/makefiles/Makefile.linux to src/Makefile.port

Line #2 copies pg_config_os.h to src/include, and later all
src/include/*.h files are copied to the install directory by
src/include/Makefile.  The same happens for dynloader.h on line #1.

In the Windows MSVC build, we handle pg_config_os.h in the Perl scripts,
but not dynloader.h.  The attached patch copies the process used for
pg_config_os.h to handle dynloader.h.  I believe this is the only *.h
file that has this problem.

This should fix the PL/Java Windows build problem.  I don't think I will
get this patch into 9.5.0 but will put it in after 9.5.0 is released and
it will appear in all the next minor version releases, including 9.5.1,
which should happen in the next 60 days.

Thanks.

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

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
new file mode 100644
index 40e06f6..ea9b857
*** a/src/tools/msvc/Install.pm
--- b/src/tools/msvc/Install.pm
*** sub CopyIncludeFiles
*** 582,588 
  		'Public headers', $target . '/include/',
  		'src/include/',   'postgres_ext.h',
  		'pg_config.h','pg_config_ext.h',
! 		'pg_config_os.h', 'pg_config_manual.h');
  	lcopy('src/include/libpq/libpq-fs.h', $target . '/include/libpq/')
  	  || croak 'Could not copy libpq-fs.h';
  
--- 582,588 
  		'Public headers', $target . '/include/',
  		'src/include/',   'postgres_ext.h',
  		'pg_config.h','pg_config_ext.h',
! 		'pg_config_os.h', 'dynloader.h', 'pg_config_manual.h');
  	lcopy('src/include/libpq/libpq-fs.h', $target . '/include/libpq/')
  	  || croak 'Could not copy libpq-fs.h';
  
*** sub CopyIncludeFiles
*** 605,611 
  	CopyFiles(
  		'Server headers',
  		$target . '/include/server/',
! 		'src/include/', 'pg_config.h', 'pg_config_ext.h', 'pg_config_os.h');
  	CopyFiles(
  		'Grammar header',
  		$target . '/include/server/parser/',
--- 605,612 
  	CopyFiles(
  		'Server headers',
  		$target . '/include/server/',
! 		'src/include/', 'pg_config.h', 'pg_config_ext.h', 'pg_config_os.h',
! 		'dynloader.h');
  	CopyFiles(
  		'Grammar header',
  		$target . '/include/server/parser/',
diff --git a/src/tools/msvc/clean.bat b/src/tools/msvc/clean.bat
new file mode 100755
index e3da6aa..feb0fe5
*** a/src/tools/msvc/clean.bat
--- b/src/tools/msvc/clean.bat
*** REM Delete files created with GenerateFi
*** 38,43 
--- 38,44 
  if exist src\include\pg_config.h del /q src\include\pg_config.h
  if exist src\include\pg_config_ext.h 

Re: [HACKERS] pg_controldata/pg_resetxlog "Latest checkpoint's NextXID" format

[Tom Lane]

=?UTF-8?B?Sm9zw6kgTHVpcyBUYWxsw7Nu?=  writes:
> On 12/29/2015 01:18 PM, Heikki Linnakangas wrote:
>> On 29/12/15 07:14, Joe Conway wrote:
>>> Shouldn't it use "%X/%X", same as e.g. "Prior checkpoint location" and
>>> all the other XIDs?

>> No. The "locations" in the output are WAL locations. Those are 
>> customarily printed with %X/%X. But NextXID is a transaction ID, those 
>> are printed in decimal, with %u.

> But Joe has a point here Others could also be confused if he doubted 
> about this.

Yeah.  Use of the same x/y notation with two different bases seems like
a recipe for confusion.  It's probably too late to do anything about
this for 9.5, but I'd be +1 for adopting Jose's suggestion or some
other formatting tweak in HEAD.

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] Testing Postgresql 9.5 RC1 with Alfresco 5.0.d

[Boriss Mejias]

Simon Riggs wrote on 29/12/15 05:31:
[...]

Thanks for testing.

Do you have any comments on performance testing?


Not really, because I didn't have an equivalent environment to compare. These 
tests took exactly the same time as another one with Postgresql 9.3, with a 
major different in the setting of the server:


Postgresql 9.5 RC1
1 GB of RAM
1 vCPU Intel
CentOS 6.5

vs

Postgresql 9.3
8 GB of RAM
4 vCPU LPAR
Redhat 6.5 PowerLinux

So, I can't really compare. So, definitely no performance degradation, and 
possible a performance gain. But of course, this could all be the fault of Alfresco.


Discussing with the Alfresco community I found a benchmarking test which I'm not 
familiar with. I'll make sure to include that test next time I test a 
pre-release version of Postgresql, and I'll make sure to have equivalent 
environment to test.



Are there any opportunities to improve Alfresco using the new features in
PostgreSQL 9.5?


I'm not an Alfresco developer, but I'll try to contact the Alfresco engineers to 
discuss this.



Thanks


You're welcome and I hope next tests will include some performance measurements 
a part from just compliance tests.


Cheers
Boriss



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



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

[HACKERS] COPY (... tab completion

[Andreas Karlsson]

Hi,

Here is a patch which adds tab completion for COPY with a query. 
Currently there is only completion for COPY with a relation.


Andreas
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 98884eb..98023ca 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1857,11 +1857,18 @@ psql_completion(const char *text, int start, int end)
 /* COPY */
 
 	/*
-	 * If we have COPY [BINARY] (which you'd have to type yourself), offer
-	 * list of tables (Also cover the analogous backslash command)
+	 * If we have COPY, offer list of tables or "(" (Also cover the analogous
+	 * backslash command).
 	 */
-	else if (TailMatches1("COPY|\\copy") || TailMatches2("COPY", "BINARY"))
+	else if (TailMatches1("COPY|\\copy"))
+		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+   " UNION ALL SELECT '('");
+	/* If we have COPY BINARY, compelete with list of tables */
+	else if (TailMatches2("COPY", "BINARY"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+	/* If we have COPY (, complete it with legal commands */
+	else if (TailMatches2("COPY|\\copy", "("))
+		COMPLETE_WITH_LIST4("SELECT", "WITH", "UPDATE", "DELETE");
 	/* If we have COPY|BINARY , complete it with "TO" or "FROM" */
 	else if (TailMatches2("COPY|\\copy|BINARY", MatchAny))
 		COMPLETE_WITH_LIST2("FROM", "TO");

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

Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Pavel Stehule]

Hi

2015-12-08 7:06 GMT+01:00 Catalin Iacob :

> On Thu, Dec 3, 2015 at 6:54 PM, Pavel Stehule 
> wrote:
> > Don't understand - if Fatal has same behave as Error, then why it cannot
> be
> > inherited from Error?
> >
> > What can be broken?
>
> Existing code that did "except plpy.Error as exc" will now also be
> called if plpy.Fatal is raised. That wasn't the case so this changes
> meaning of existing code, therefore it introduces an incompatibility.
>

I read some notes about Python naming convention

and we can use different names for new exception classes

* common ancestor - plpy.BaseError

* descendants - plpy.ExecError, plpy.SPIError, plpy.FatalError


It should to decrease compatibility issues to minimum. SPIError was used
mostly for error trapping (see doc).

Is it ok for you?

Regards

Pavel

Re: [HACKERS] Making tab-complete.c easier to maintain

[Michael Paquier]

On Wed, Dec 30, 2015 at 1:21 AM, Tom Lane  wrote:
> This is because of the use of strncmp instead of plain strcmp
> in most of the backslash matching rules, eg the above case is
> covered by
>
> else if (strncmp(prev_wd, "\\df", strlen("\\df")) == 0)

Ah, OK. The length of the name and not the pattern is used in
word_matches, but we had better use something based on the pattern
shape. And the current logic for backslash commands uses the length of
the pattern, and not the word for its checks.

> I was envisioning that we'd want to convert this to something like
>
> else if (TailMatchesCS1("\\df*"))

That's a better macro name...
-- 
Michael


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

Re: [HACKERS] pg_controldata/pg_resetxlog "Latest checkpoint's NextXID" format

[Joe Conway]

On 12/29/2015 07:15 AM, Tom Lane wrote:
> Yeah.  Use of the same x/y notation with two different bases seems like
> a recipe for confusion.  It's probably too late to do anything about
> this for 9.5, but I'd be +1 for adopting Jose's suggestion or some
> other formatting tweak in HEAD.

I made the "%u/%u" -> "%u:%u" change in the controldata patch I just
posted, but I suppose I should commit that separately. Any complaints
about that?

Joe

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] On columnar storage (2)

[Jim Nasby]

On 12/28/15 1:15 PM, Alvaro Herrera wrote:

Currently within the executor
a tuple is a TupleTableSlot which contains one Datum array, which has
all the values coming out of the HeapTuple; but for split storage
tuples, we will need to have a TupleTableSlot that has multiple "Datum
arrays" (in a way --- because, actually, once we get to vectorise as in
the preceding paragraph, we no longer have a Datum array, but some more
complex representation).

I think that trying to make the FDW API address all these concerns,
while at the same time*also*  serving the needs of external data
sources, insanity will ensue.


Are you familiar with DataFrames in Pandas[1]? They're a collection of 
Series[2], which are essentially vectors. (Technically, they're more 
complex than that because you can assign arbitrary indexes). So instead 
of the normal collection of rows, a DataFrame is a collection of 
columns. Series are also sparse (like our tuples), but the sparse value 
can be anything, not just NULL (or NaN in panda-speak). There's also 
DataFrames in R; not sure how equivalent they are.


I mention this because there's a lot being done with dataframes and they 
might be a good basis for a columnstore API, killing 2 birds with one stone.


BTW, the underlying python type for Series is ndarrays[3], which are 
specifically designed to interface to things like C arrays. So a column 
store could potentially be accessed directly.


Aside from potential API inspiration, it might be useful to prototype a 
columnstore using Series (or maybe ndarrays).


[1] 
http://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.html

[2] http://pandas.pydata.org/pandas-docs/stable/api.html#series
[3] http://docs.scipy.org/doc/numpy-1.10.0/reference/internals.html
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.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] Making tab-complete.c easier to maintain

[Michael Paquier]

On Wed, Dec 30, 2015 at 6:26 AM, Thomas Munro
 wrote:
> On Wed, Dec 30, 2015 at 3:14 AM, Michael Paquier
>  wrote:
>> On Sun, Dec 20, 2015 at 8:08 AM, Michael Paquier
>>  wrote:
>>> On Sun, Dec 20, 2015 at 6:24 AM, Tom Lane  wrote:
 2. I believe that a very large fraction of the TailMatches() rules really
 ought to be Matches(), ie, they should not consider matches that don't
 start at the start of the line.  And there's another bunch that could
 be Matches() if the author hadn't been unaccountably lazy about checking
 all words of the expected command.  If we converted as much as we could
 that way, it would make psql_completion faster because many inapplicable
 rules could be discarded after a single integer comparison on
 previous_words_count, and it would greatly reduce the risk of inapplicable
 matches.  We can't do that for rules meant to apply to DML statements,
 since they can be buried in WITH, EXPLAIN, etc ... but an awful lot of
 the DDL rules could be changed.
>>
>> Yep, clearly. We may gain a bit of performance by matching directly
>> with an equal number of words using Matches instead of a lower bound
>> with TailMatches. I have looked at this thing and hacked a patch as
>> attached.
>
> I see that you changed INSERT and DELETE (but not UPDATE) to use
> MatchesN rather than TailMatchesN.  Shouldn't these stay with
> TailMatchesN for the reason Tom gave above?

Er, yeah. They had better be TailMatches, or even COPY DML stuff will be broken.
-- 
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] Additional role attributes && superuser review

[Michael Paquier]

On Tue, Dec 29, 2015 at 11:55 PM, Stephen Frost  wrote:
> I could go either way on that, really.  I don't find namespace to be
> confusing when used in that way, but I'll change it since others do.

It seems to me that the way patch does it is fine..
-- 
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] Multi-tenancy with RLS

[Haribabu Kommi]

On Thu, Dec 17, 2015 at 12:46 PM, Haribabu Kommi
 wrote:
> Rebased patch is attached as it is having an OID conflict with the
> latest set of changes
> in the master branch.

Here I attached new series of patches with a slightly different approach.
Instead of creating the policies on the system catalog tables whenever
the catalog security command is executed, just enable row level security
on the system catalog tables. During the relation build, in
RelationBuildRowSecurity function, if it is a system relation, frame the
policy using the policy query which we earlier used to create by parsing it.

With the above approach, in case of any problems in the policy, to use
the corrected policy, user just needs to replace the binaries. whereas in
earlier approach, either pg_upgrade or disabling and enabling of catalog
security is required.

Currently it is changed only for shared system catalog tables and also the
way of enabling catalog security on shared system catalog tables is through
initdb only. This also can be changed later. I will do similar changes for
remaining catalog tables.

Any comments on the approach?

Regards,
Hari Babu
Fujitsu Australia


3_shared_catalog_tenancy_v2.patch
Description: Binary data


1_any_privilege_option_v2.patch
Description: Binary data


2_view_security_definer_v2.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] exposing pg_controldata and pg_config as functions

[Joe Conway]

On 12/23/2015 04:37 PM, Michael Paquier wrote:
> On Thu, Dec 24, 2015 at 2:08 AM, Joe Conway  wrote:
>> 2) Change the pg_controldata to be a bunch of separate functions as
>>suggested by Josh Berkus rather than one SRF.
> 
> This looks like a plan, thanks!

As discussed, a completely revamped and split off pg_controldata patch.
Below are the details for those interested.

Comments please.

Joe

===
What this patch does:
---
1) Change NextXID output format from "%u/%u" to "%u:%u"
   (see recent hackers thread)
2) Refactor bin/pg_controldata (there should be no visible change to
   pg_controldata output)
3) Adds new functions, more or less in line with previous discussions:
   * pg_checkpoint_state()
   * pg_controldata_state()
   * pg_recovery_state()
   * pg_init_state()

===
Missing (TODO once agreement on the above is reached):
---
a) documentation
b) catversion bump
c) regression tests

===
New function detail and sample output:
---
postgres=# \x
Expanded display is on.

postgres=# \df pg_*_state
List of functions
-[ RECORD 1 ]---+--
Schema  | pg_catalog
Name| pg_checkpoint_state
Result data type| record
Argument data types | OUT checkpoint_location pg_lsn,
| OUT prior_location pg_lsn,
| OUT redo_location pg_lsn,
| OUT redo_wal_file text,
| OUT timeline_id integer,
| OUT prev_timeline_id integer,
| OUT full_page_writes boolean,
| OUT next_xid text,
| OUT next_oid oid,
| OUT next_multixact_id xid,
| OUT next_multi_offset xid,
| OUT oldest_xid xid,
| OUT oldest_xid_dbid oid,
| OUT oldest_active_xid xid,
| OUT oldest_multi_xid xid,
| OUT oldest_multi_dbid oid,
| OUT oldest_commit_ts_xid xid,
| OUT newest_commit_ts_xid xid,
| OUT checkpoint_time timestamp with time zone
Type| normal
-[ RECORD 2 ]---+--
Schema  | pg_catalog
Name| pg_controldata_state
Result data type| record
Argument data types | OUT pg_control_version integer,
| OUT catalog_version_no integer,
| OUT system_identifier bigint,
| OUT pg_control_last_modified
| timestamp with time zone
Type| normal
-[ RECORD 3 ]---+--
Schema  | pg_catalog
Name| pg_init_state
Result data type| record
Argument data types | OUT max_data_alignment integer,
| OUT database_block_size integer,
| OUT blocks_per_segment integer,
| OUT wal_block_size integer,
| OUT bytes_per_wal_segment integer,
| OUT max_identifier_length integer,
| OUT max_index_columns integer,
| OUT max_toast_chunk_size integer,
| OUT large_object_chunk_size integer,
| OUT bigint_timestamps boolean,
| OUT float4_pass_by_value boolean,
| OUT float8_pass_by_value boolean,
| OUT data_page_checksum_version integer
Type| normal
-[ RECORD 4 ]---+--
Schema  | pg_catalog
Name| pg_recovery_state
Result data type| record
Argument data types | OUT min_recovery_end_location pg_lsn,
| OUT min_recovery_end_timeline integer,
| OUT backup_start_location pg_lsn,
| OUT backup_end_location pg_lsn,
| OUT end_of_backup_record_required boolean
Type| normal


postgres=# select * from pg_controldata_state();
-[ RECORD 1 ]+---
pg_control_version   | 942
catalog_version_no   | 201511071
system_identifier| 6233852631805477166
pg_control_last_modified | 2015-12-29 15:32:09-08

postgres=# select * from pg_checkpoint_state();
-[ RECORD 1 ]+-
checkpoint_location  | 0/14E8C38
prior_location   | 0/14D6340
redo_location| 0/14E8C38
redo_wal_file| 00010001
timeline_id  | 1
prev_timeline_id | 1
full_page_writes | t
next_xid | 0:574
next_oid | 12407
next_multixact_id| 1
next_multi_offset| 0
oldest_xid   | 565
oldest_xid_dbid  | 1
oldest_active_xid| 0
oldest_multi_xid | 1
oldest_multi_dbid| 1
oldest_commit_ts_xid | 0

Re: [HACKERS] Combining Aggregates

[David Rowley]

On 25 December 2015 at 14:10, Robert Haas  wrote:

> On Mon, Dec 21, 2015 at 4:53 PM, David Rowley
>  wrote:
> > On 22 December 2015 at 01:30, Robert Haas  wrote:
> >> Can we use Tom's expanded-object stuff instead of introducing
> >> aggserialfn and aggdeserialfn?  In other words, if you have a
> >> aggtranstype = INTERNAL, then what we do is:
> >>
> >> 1. Create a new data type that represents the transition state.
> >> 2. Use expanded-object notation for that data type when we're just
> >> within a single process, and flatten it when we need to send it
> >> between processes.
> >>
> >
> > I'd not seen this before, but on looking at it I'm not sure if using it
> will
> > be practical to use for this. I may have missed something, but it seems
> that
> > after each call of the transition function, I'd need to ensure that the
> > INTERNAL state was in the varlana format.
>
> No, the idea I had in mind was to allow it to continue to exist in the
> expanded format until you really need it in the varlena format, and
> then serialize it at that point.  You'd actually need to do the
> opposite: if you get an input that is not in expanded format, expand
> it.


Admittedly I'm struggling to see how this can be done. I've spent a good
bit of time analysing how the expanded object stuff works.

Hypothetically let's say we can make it work like:

1. During partial aggregation (finalizeAggs = false), in
finalize_aggregates(), where we'd normally call the final function, instead
flatten INTERNAL states and store the flattened Datum instead of the
pointer to the INTERNAL state.
2. During combining aggregation (combineStates = true) have all the combine
functions written in such a ways that the INTERNAL states expand the
flattened states before combining the aggregate states.

Does that sound like what you had in mind?

If so I can't quite seem to wrap my head around 1. As I'm really not quite
sure how, from finalize_aggregates() we'd flatten the INTERNAL pointer. I
mean, how do we know which flatten function to call here? From reading the
expanded-object code I see that its used in expand_array(), In this case we
know we're working with arrays, so it just always uses the EA_methods
globally scoped struct to get the function pointers it requires for
flattening the array. For the case of finalize_aggregates(), the best I can
think of here is to have a bunch of global structs and then have a giant
case statement to select the correct one. That's clearly horrid, and not
commit worthy, and it does nothing to help user defined aggregates which
use INTERNAL types. Am I missing something here?

As of the most recent patch I posted, having the serial and deserial
functions in the catalogs allows user defined aggregates with INTERNAL
states to work just fine. Admittedly I'm not all that happy that I've had
to add 4 new columns to pg_aggregate to support this, but if I could think
of how to make it work without doing that, then I'd likely go and do that
instead.

If your problem with the serialize and deserialize stuff is around the
serialized format, then can see no reason why we couldn't just invent some
composite types for the current INTERNAL aggregate states, and have the
serialfn convert the INTERNAL state into one of those, then have the
deserialfn perform the opposite. Likely this would be neater than what I
have at the moment with just converting the INTERNAL state into text.

Please let me know what I'm missing with the expanded-object code.

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

Re: [HACKERS] [COMMITTERS] pgsql: Document brin_summarize_new_pages

[Tatsuo Ishii]

Great. It would be nice if brin.sgml has a reference to func.sgml.
I mean something like:

   brin_summarize_new_pages(regclass) function,
   or automatically when VACUUM processes the table.
+  See also .

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

From: Alvaro Herrera 
Subject: [COMMITTERS] pgsql: Document brin_summarize_new_pages
Date: Mon, 28 Dec 2015 18:29:15 +
Message-ID: 

> Document brin_summarize_new_pages
> 
> Pointer out by Jeff Janes
> 
> Branch
> --
> REL9_5_STABLE
> 
> Details
> ---
> http://git.postgresql.org/pg/commitdiff/5bf939411ce7a4ae87cee65daca2e4add44b2b46
> 
> Modified Files
> --
> doc/src/sgml/brin.sgml |   20 
> doc/src/sgml/func.sgml |   42 ++
> 2 files changed, 62 insertions(+)
> 
> 
> -- 
> Sent via pgsql-committers mailing list (pgsql-committ...@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers


-- 
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] dynloader.h missing in prebuilt package for Windows?

[Bruce Momjian]

On Tue, Dec 29, 2015 at 03:01:55PM -0500, Chapman Flack wrote:
> On 12/29/15 14:08, Bruce Momjian wrote:
> 
> > This should fix the PL/Java Windows build problem.  I don't think I will
> > get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> > it will appear in all the next minor version releases, including 9.5.1,
> > which should happen in the next 60 days.
> 
> Thanks!  What I ended up doing in PL/Java was just to create a 'fallback'
> directory with a copy of dynloader.h in it, and adding it at the very
> end of the include path when building on Windows, so I think the build
> will now work either way, using the real file if it is present, or the
> frozen-in-time fallback copy if it isn't.  Then the fallback directory
> can be axed as soon as 9.5.1 becomes the oldest thing in PL/Java's
> rearview support mirror.

Yes, that is a good plan.  I will apply this to all supported major
versions, so a simple minor version upgrade will allow it to work
without the fallback.

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

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


-- 
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] Avoid endless futile table locks in vacuuming.

[Jeff Janes]

On Mon, Dec 28, 2015 at 2:12 PM, Tom Lane  wrote:
> I wrote:
>> Jeff Janes  writes:
>>> If a partially-active table develops a slug of stable all-visible,
>>> non-empty pages at the end of it, then every autovacuum of that table
>>> will skip the end pages on the forward scan, think they might be
>>> truncatable, and take the access exclusive lock to do the truncation.
>>> And then immediately fail when it sees the last page is not empty.
>>> This can persist for an indefinite number of autovac rounds.
>>> The simple solution is to always scan the last page of a table, so it
>>> can be noticed that it is not empty and avoid the truncation attempt.
>
>> This seems like a reasonable proposal, but I find your implementation
>> unconvincing: there are two places in lazy_scan_heap() that pay attention
>> to scan_all, and you touched only one of them.
>
> After further investigation, there is another pre-existing bug: the short
> circuit path for pages not requiring freezing doesn't bother to update
> vacrelstats->nonempty_pages, causing the later logic to think that the
> page is potentially truncatable even if we fix the second check of
> scan_all! So this is pretty broken, and I almost think we should treat it
> as a back-patchable bug fix.
>
> In the attached proposed patch, I added another refinement, which is to
> not bother with forcing the last page to be scanned if we already know
> that we're not going to attempt truncation, because we already found a
> nonempty page too close to the end of the rel. I'm not quite sure this
> is worthwhile, but it takes very little added logic, and saving an I/O
> is probably worth the trouble.

If we are not doing a scan_all and we fail to acquire a clean-up lock on
the last block, and the last block reports that it needs freezing, then we
continue on to wait for the clean-up lock. But there is no need, we don't
really need to freeze the block, and we already know whether it has tuples
or not without the clean up lock.  Couldn't we just set the flag based on
hastup, then 'continue'?

Cheers,

Jeff

Re: [HACKERS] Making tab-complete.c easier to maintain

[Thomas Munro]

On Wed, Dec 30, 2015 at 3:14 AM, Michael Paquier
 wrote:
> On Sun, Dec 20, 2015 at 8:08 AM, Michael Paquier
>  wrote:
>> On Sun, Dec 20, 2015 at 6:24 AM, Tom Lane  wrote:
>>> 2. I believe that a very large fraction of the TailMatches() rules really
>>> ought to be Matches(), ie, they should not consider matches that don't
>>> start at the start of the line.  And there's another bunch that could
>>> be Matches() if the author hadn't been unaccountably lazy about checking
>>> all words of the expected command.  If we converted as much as we could
>>> that way, it would make psql_completion faster because many inapplicable
>>> rules could be discarded after a single integer comparison on
>>> previous_words_count, and it would greatly reduce the risk of inapplicable
>>> matches.  We can't do that for rules meant to apply to DML statements,
>>> since they can be buried in WITH, EXPLAIN, etc ... but an awful lot of
>>> the DDL rules could be changed.
>
> Yep, clearly. We may gain a bit of performance by matching directly
> with an equal number of words using Matches instead of a lower bound
> with TailMatches. I have looked at this thing and hacked a patch as
> attached.

I see that you changed INSERT and DELETE (but not UPDATE) to use
MatchesN rather than TailMatchesN.  Shouldn't these stay with
TailMatchesN for the reason Tom gave above?

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


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

Re: [HACKERS] Avoid endless futile table locks in vacuuming.

[Tom Lane]

Jeff Janes  writes:
> If we are not doing a scan_all and we fail to acquire a clean-up lock on
> the last block, and the last block reports that it needs freezing, then we
> continue on to wait for the clean-up lock. But there is no need, we don't
> really need to freeze the block, and we already know whether it has tuples
> or not without the clean up lock.  Couldn't we just set the flag based on
> hastup, then 'continue'?

Uh, isn't that what my patch is doing?

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] dynloader.h missing in prebuilt package for Windows?

[Chapman Flack]

On 12/29/15 14:08, Bruce Momjian wrote:

> This should fix the PL/Java Windows build problem.  I don't think I will
> get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> it will appear in all the next minor version releases, including 9.5.1,
> which should happen in the next 60 days.

Thanks!  What I ended up doing in PL/Java was just to create a 'fallback'
directory with a copy of dynloader.h in it, and adding it at the very
end of the include path when building on Windows, so I think the build
will now work either way, using the real file if it is present, or the
frozen-in-time fallback copy if it isn't.  Then the fallback directory
can be axed as soon as 9.5.1 becomes the oldest thing in PL/Java's
rearview support mirror.

-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: [HACKERS] Some 9.5beta2 backend processes not terminating properly?

[Amit Kapila]

On Tue, Dec 29, 2015 at 7:04 PM, Shay Rojansky  wrote:

> Could you describe the worklad a bit more? Is this rather concurrent? Do
>> you use optimized or debug builds? How long did you wait for the
>> backends to die? Is this all over localhost, external ip but local,
>> remotely?
>>
>
> The workload is a a rather diverse set of integration tests executed with
> Npgsql. There's no concurrency whatsoever - tests are executed serially.
> The backends stay alive indefinitely, until they are killed. All this is
> over localhost with TCP. I can try other scenarios if that'll help.
>
>

What procedure do you use to kill backends?  Normally, if we kill
via task manager using "End Process", it is considered as backend
crash and the server gets restarted and all other backends got
disconnected.


> > Note that the number of backends that stay stuck after the tests is
>> > constant (always 12).
>>
>> Can you increase the number of backends used in the test? And check
>> whether it's still 12?
>>
>
> Well, I ran the testsuite twice in parallel, and got... 23 backends stuck
> at the end.
>
>
>> How are your clients disconnecting? Possibly without properly
>> disconnecting?
>>
>
> That's possible, definitely in some of the test cases.
>
> What I can do is try to isolate things further by playing around with the
> tests and trying to see if a more minimal repro can be done - I'll try
> doing this later today or tomorrow. If anyone has any other specific tests
> or checks I should do let me know.
>

I think first we should try to isolate whether the hanged backends
are due to the reason that they are not disconnected properly or
there is some other factor involved as well, so you can try to kill/
disconnect the sessions connected via psql in the same way as
you are doing for connections with Npgsql and see if you can
reproduce the same behaviour.

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

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Haribabu Kommi]

On Wed, Dec 30, 2015 at 1:07 AM, Shulgin, Oleksandr
 wrote:
>
> This is close enough, but what I actually mean by "a callback" is more or
> less like the attached version.

Thanks for the changes.

> While at it, I've also added some trivial code to preserve keyword quoting
> in database and user fields, as well as added netmask output parameter; also
> documentation is extended a little.

Thanks for the documentation changes and regarding the quoting, in any system
catalog table, the quoted objects are represented without quotes as below.

postgres=> select datname from pg_database;
datname
---
 postgres
 template1
 template0
 test_user2_db
 TEST_USER1_DB
 test_user2_dB
(6 rows)

Adding quotes to pg_hba_lookup function makes it different from others.
The issues regarding the same is already discussed in [1].

select a.database[1], b.datname from pg_hba_lookup('postgres','kommih','::1')
as a, pg_database as b where a.database[1]
= b.datname;

The queries like above are not possible with quoted output. It is very
rare that the
pg_hba_lookup function used in join operations, but still it is better
to provide
data without quotes. so I reverted these changes in the attached latest patch.

> The biggest question for me is the proper handling of memory contexts for
> HBA and ident files data.  I think it makes sense to release them explicitly
> because with the current state of affairs, we have dangling pointers in
> parsed_{hba,ident}_{context,lines} after release of PostmasterContext.  The
> detailed comment in postgres.c around
> MemoryContextDelete(PostmasterContext); suggests that it's not easy already
> to keep track of the all things that might be affected by this cleanup step:
>
> /*
> * If the PostmasterContext is still around, recycle the space; we don't
> * need it anymore after InitPostgres completes.  Note this does not trash
> * *MyProcPort, because ConnCreate() allocated that space with malloc()
> * ... else we'd need to copy the Port data first.  Also, subsidiary data
> * such as the username isn't lost either; see ProcessStartupPacket().
> */
>
> Not sure if we need any advanced machinery here like some sort of cleanup
> hooks list?  For now I've added discard_{hba,ident}() functions and call
> them explicitly where appropriate.

The added functions properly frees the hba and ident contexts once its use is
finished. I removed the discard function calls in PerformAuthentication function
in EXEC_BACKEND mode, as these are called once the PerformAuthenication
function finishes.

The discard hba and ident context patch is separated from
pg_hba_lookup patch for
easier understanding. The pg_hba_lookup patch needs to be applied on top of
discard_hba_and_ident_cxt patch.

[1] 
http://www.postgresql.org/message-id/cafj8prarzdscocmk30gyydogiwutucz7eve-bbg+wv2wg5e...@mail.gmail.com

Regards,
Hari Babu
Fujitsu Australia


pg_hba_lookup_poc_v11.patch
Description: Binary data


discard_hba_and_ident_cxt.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] pg_controldata/pg_resetxlog "Latest checkpoint's NextXID" format

[Simon Riggs]

On 30 December 2015 at 00:17, Joe Conway  wrote:

> On 12/29/2015 07:15 AM, Tom Lane wrote:
> > Yeah.  Use of the same x/y notation with two different bases seems like
> > a recipe for confusion.  It's probably too late to do anything about
> > this for 9.5, but I'd be +1 for adopting Jose's suggestion or some
> > other formatting tweak in HEAD.
>
> I made the "%u/%u" -> "%u:%u" change in the controldata patch I just
> posted, but I suppose I should commit that separately. Any complaints
> about that?


There is already long precedent about how to represent an XID with an
epoch... and it is neither of those two formats.

http://www.postgresql.org/docs/devel/static/functions-info.html
"Table 9-63. Transaction IDs and Snapshots"
"The internal transaction ID type (xid) is 32 bits wide and wraps around
every 4 billion transactions. However, these functions export a 64-bit
format that is extended with an "epoch" counter so it will not wrap around
during the life of an installation."

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Proposal: "Causal reads" mode for load balancing reads without stale data

[Thomas Munro]

On Wed, Nov 18, 2015 at 11:50 PM, Thomas Munro
 wrote:
> Here is a new version of the patch with a few small improvements:
> ...
> [causal-reads-v3.patch]

That didn't apply after 6e7b3359 (which fix a typo in a comment that I
moved).  Here is a new version that does.

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


causal-reads-v4.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] More thorough planning for OLAP queries (was: [PATCH] Equivalence Class Filters)

[David Rowley]

Hi,

On [1] I suggested an idea to make improvements to the planner around the
Equivalence Class code. Later in [2] Tom raised concerns with this adding
too many planning cycles for a perhaps not common enough situation.  I
don't want to discuss that particular patch here, I want to discuss more
generally about the dilemma about adding more smarts to the planner to
allow it to generate a more optimal plan in order to save on execution time.

In the case of the Equivalence Class Filters code, I quoted an example
where pushing these filters down into the joined relation caused a
significant performance improvement to a query. Now, I understand Tom's
concerns with slowing down the planner, as in cases where the query is
short running, or the optimisations don't apply, then we could cause the
query to overall (including planning time) perform worse. Nobody wants
that, but on the other hand, if spending 5-10 extra microseconds during
planning equates to 6 hours shaved off execution time, then nobody would
think to grudge that extra 5-10 microseconds during planning.

What I'd like to discuss here is what was touched on on that other thread
on ways to get around this problem:

A number of ideas were suggested on the other thread about how we might go
about solving this problem. In [3] Simon talked about perhaps enabling
extra optimisations when the planner sees that the plan will cost more than
some given threshold. That's perhaps an option, but may not work well for
optimisations which must take place very early in planning, for example [4].
Another idea which came up was from Evgeniy [5], which was more of a
request not to do it this way, but never-the-less, the idea was basically
to add lots of GUCs to enable/disable each extra planner feature.

Another option which I've thought about previously was a planner_strength
GUC, at which various additional optimisations are enabled at various
predefined strength levels, so that databases which tend to spend a great
deal more execution time compared to planning time can turn this up a bit
to see if that helps change that ratio a bit.  This idea is far from
perfect though, as who's to say that planner feature X should "kick in"
before planner feature Y? I've also often thought that it might be nice to
have it so the planner does not modify the Parse object, so that the
planner has the option to throw away what it's done so far and start
planning all over again with the "planner_strength" knob turned up to the
maximum, if the cost happened to indicate that the query was going to take
a long time to execute.

In reality we already have some planner features which are possible
candidates for non essential optimisations. For example join removals
likely don't apply in all that many cases, but when they do, this feature
is a great win. So by having some sort of ability to enable/disable planner
features we also stand to actually speed the planner up for fast simple
queries.

I do strongly believe that we need to come up with something to solve this
problem. I already summarised my thoughts on the other thread.

I wrote:
> I believe that with parallel query on the horizon for 9.6 that we're now
> aiming to support bigger OLAP type database than ever before. So if we
> ignore patches like this one then it appears that we have some conflicting
> goals in the community as it seems that we're willing to add the brawn,
but
> we're not willing to add the brain. If this is the case then it's a shame,
> as I think we can have both. So I very much agree on the fact that we must
> find a way to maintain support and high performance of small OLTP
databases
> too.

So here I'd very much like to kick off discussion on an acceptable way to
solve this problem, in a realistic way which we're all happy with.

Comments are of course welcome.

[1]
http://www.postgresql.org/message-id/cakjs1f9fk_x_5hkcpcseimy16owe3empmmgsgwlckkj_rw9...@mail.gmail.com
[2] http://www.postgresql.org/message-id/30810.1449335...@sss.pgh.pa.us
[3]
http://www.postgresql.org/message-id/canp8+jlrprn4ynmsrkoqhyi-dw5jrodmot05qejhrayrsex...@mail.gmail.com
[4]
http://www.postgresql.org/message-id/cakjs1f_uz_mxtpot6epxsghsujoucrkuxyhlh06h072rdxs...@mail.gmail.com
[5]
http://www.postgresql.org/message-id/2f30ba8b-dab9-4907-9e4e-102d24256...@gmail.com

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

Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

[Robert Haas]

On Mon, Dec 28, 2015 at 3:17 AM, Amit Kapila  wrote:
> 2.
> @@ -213,6 +213,7 @@ typedef enum BuiltinTrancheIds
>   LWTRANCHE_WAL_INSERT,
>   LWTRANCHE_BUFFER_CONTENT,
>   LWTRANCHE_BUFFER_IO_IN_PROGRESS,
> + LWTRANCHE_PROC,
>   LWTRANCHE_FIRST_USER_DEFINED
>  } BuiltinTrancheIds;
>
> Other trancheids are based on the name of their corresponding
> LWLock, don't you think it is better to name it as
> LWTRANCHE_BACKEND for the sake of consistency?  Also consider
> changing name at other places in patch for this tranche.

Hmm, don't think I agree with this.  I think LWTRANCHE_PROC is better.
  Remember, backendLock is intended to distinguish that object from
everything else in the PGPROC; but here we're trying to distinguish
stuff in the PGPROC from stuff in other data structures altogether.
That's an important distinction.

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


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

Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

[Amit Kapila]

On Wed, Dec 16, 2015 at 12:26 AM, Robert Haas  wrote:
>
>
> In terms of this project overall, NumLWLocks() now knows about only
> four categories of stuff: fixed lwlocks, backend locks (proc.c),
> replication slot locks, and locks needed by extensions.  I think it'd
> probably be fine to move the backend locks into PGPROC directly, and
> the replication slot locks into ReplicationSlot.
>

IIdus has written a patch to move backend locks into PGPROC which
I am reviewing and will do performance tests once he responds to
my review comments and I have written a patch to move replication
slot locks into ReplicationSlot which is attached with this mail.



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


replslot_iolwlock_tranche_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] commit 5c45d2f need to be back-patch on branch 9.2 & before

[amul sul]

Hi,


Uninitialized variable 'dtype' warning in date_in() is fix[1] on branch 9.3 & 
above, but not for 9.2 & older branches. I guess this should be back-patch. 
[1] commit link=> 
http://git.postgresql.org/pg/commitdiff/5c45d2f87835ccd3ffac338845ec79cab1b31a11


Regards,
Amul Sul


-- 
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_controldata/pg_resetxlog "Latest checkpoint's NextXID" format

[José Luis Tallón]

On 12/29/2015 01:18 PM, Heikki Linnakangas wrote:

On 29/12/15 07:14, Joe Conway wrote:

I wonder why "Latest checkpoint's NextXID" is formated like this:

8<-
printf(_("Latest checkpoint's NextXID:  %u/%u\n"),
   ControlFile.checkPointCopy.nextXidEpoch,
   ControlFile.checkPointCopy.nextXid);
8<-

Shouldn't it use "%X/%X", same as e.g. "Prior checkpoint location" and
all the other XIDs?


No. The "locations" in the output are WAL locations. Those are 
customarily printed with %X/%X. But NextXID is a transaction ID, those 
are printed in decimal, with %u.


But Joe has a point here Others could also be confused if he doubted 
about this.


"Latest checkpoint's NextXID:  %u:%u\n", maybe ??
(slash becomes colon)

Worst part: fuzzies the translations unnecesarily


/ J.L.



--
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_controldata/pg_resetxlog "Latest checkpoint's NextXID" format

[Heikki Linnakangas]

On 29/12/15 07:14, Joe Conway wrote:

I wonder why "Latest checkpoint's NextXID" is formated like this:

8<-
printf(_("Latest checkpoint's NextXID:  %u/%u\n"),
   ControlFile.checkPointCopy.nextXidEpoch,
   ControlFile.checkPointCopy.nextXid);
8<-

Shouldn't it use "%X/%X", same as e.g. "Prior checkpoint location" and
all the other XIDs?


No. The "locations" in the output are WAL locations. Those are 
customarily printed with %X/%X. But NextXID is a transaction ID, those 
are printed in decimal, with %u.


- 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] Additional role attributes && superuser review

[Stephen Frost]

Amit,

* Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> On 2015/12/23 7:23, Stephen Frost wrote:
> > Updated patch attached.  I'll give it another good look and then commit
> > it, barring objections.
> 
> Just a minor nitpick about a code comment -
> 
>  /*
> + * Check that the user is not trying to create a role in the reserved
> + * "pg_" namespace.
> + */
> +if (IsReservedName(stmt->role))
> 
> The wording may be slightly confusing, especially saying "... in ...
> namespace". ISTM, "namespace" is fairly extensively used around the code
> to mean something like "a schema's namespace".
> 
> Could perhaps be reworded as:
> 
>  /*
> + * Check that the user is not trying to create a role with reserved
> + * name, ie, one starting with "pg_".
> 
> If OK, there seems to be one more place further down in the patch with
> similar wording.

I could go either way on that, really.  I don't find namespace to be
confusing when used in that way, but I'll change it since others do.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Some 9.5beta2 backend processes not terminating properly?

[Andres Freund]

On 2015-12-29 12:41:40 +0200, Shay Rojansky wrote:
> >
> > > The tests run for a couple minutes, open and close some connection. With
> > my
> > > pre-9.5 backends, the moment the test runner exits I can see that all
> > > backend processes exit immediately, and pg_activity_stat has no rows
> > > (except the querying one). With 9.5beta2, however, some backend processes
> > > continue to stay alive beyond the test runner, and pg_activity_stat
> > > contains extra rows (state idle, waiting false). This situation persists
> > > until I restart PostgreSQL.

Could you describe the worklad a bit more? Is this rather concurrent? Do
you use optimized or debug builds? How long did you wait for the
backends to die? Is this all over localhost, external ip but local,
remotely?

> Note that the number of backends that stay stuck after the tests is
> constant (always 12).

Can you increase the number of backends used in the test? And check
whether it's still 12?


> Here's are stack dumps of the same process taken with both VS2015 Community
> and Process Explorer, I went over 4 processes and saw the same thing. Let
> me know what I else I can provide to help.
> 
> From VS2015 Community:
> 
> Main Thread
> > ntdll.dll!NtWaitForMultipleObjects() Unknown
>   KernelBase.dll!WaitForMultipleObjectsEx() Unknown
>   KernelBase.dll!WaitForMultipleObjects() Unknown
>   postgres.exe!WaitLatchOrSocket(volatile Latch * latch, int wakeEvents,
> unsigned __int64 sock, long timeout) Line 202 C
>   postgres.exe!secure_read(Port * port, void * ptr, unsigned __int64 len)
> Line 151 C
>   postgres.exe!pq_getbyte() Line 926 C
>   postgres.exe!SocketBackend(StringInfoData * inBuf) Line 345 C
>   postgres.exe!PostgresMain(int argc, char * * argv, const char * dbname,
> const char * username) Line 3984 C
>   postgres.exe!BackendRun(Port * port) Line 4236 C
>   postgres.exe!SubPostmasterMain(int argc, char * * argv) Line 4727 C
>   postgres.exe!main(int argc, char * * argv) Line 211 C
>   postgres.exe!__tmainCRTStartup() Line 626 C
>   kernel32.dll!BaseThreadInitThunk() Unknown
>   ntdll.dll!RtlUserThreadStart() Unknown

Hm. So we're waiting for the latch, and expecting to get a FD_CLOSE
error back because the socket is actually closed. Which should happen
always in that path - a read through win32_latch.c doesn't show any
obvious problems. But then I really have not too much clue about windows
development.

How are your clients disconnecting? Possibly without properly
disconnecting?

Regards,

Andres


-- 
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] Additional role attributes && superuser review

[Stephen Frost]

Noah,

* Noah Misch (n...@leadboat.com) wrote:
> > Updated patch attached.  I'll give it another good look and then commit
> > it, barring objections.
> 
> This thread and its satellite[1] have worked their way through a few designs.
> At first, it was adding role attributes, alongside existing attributes like
> REPLICATION and BYPASSRLS.  It switched[2] to making pg_dump preserve ACLs on
> system objects.  Built-in roles joined[3] the pg_dump work to offer predefined
> collections of ACL grants.  Finally, it dropped[4] the pg_dump side and
> hard-coded the roles into the features they govern.

Correct, after quite a bit of discussion and the conclusion that, while
pg_dump support for dumping ACLs might be interesting, it was quite a
bit more complex an approach than this use-case justified.  Further,
adding support to pg_dump for dumping ACLs could be done independently
of default roles.

The one argument which you've put forth for adding the complexity of
dumping catalog ACLs is that we might reduce the number of default
roles provided to the user.  I disagree that we would.  Having a single
set of default roles which provide a sensible breakdown of permissions
is a better approach than asking every administrator and application
developer who is building tools on top of PG to try and work through
what makes sense themselves, even if that means we have a default role
with a small, or even only an individual, capability.

I also disagree that we won't be able to adjust the privileges granted
to a role in the future.  We have certainly made adjustments to what a
'replication' role is able to do, which has largely been in the 'more
capabilities' direction that you opine concern over.

> To summarize, I think the right next step is to resume designing pg_dump
> support for system object ACLs.  I looked over your other two patches and will
> unshelve those reviews when their time comes.

To be clear, I don't believe the two patches are particularly involved
with each other and don't feel that one needs to wait for the other.

Further, I'm not convinced that adding support for dumping ACLs or, in
general, encouraging users to define their own ACLs on catalog objects
is a good idea.  We certainly have no mechanism in place today for those
ACLs to be respected by SysCache and encouraging their use when we won't
actually respect them is likely to be confusing.  I had thought
differently at one point but my position changed during the discussion
when I realized the complexity and potential confusion it could cause
and considered that against the simplicity and relatively low cost of
having default roles.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Some 9.5beta2 backend processes not terminating properly?

[Shay Rojansky]

>
> > The tests run for a couple minutes, open and close some connection. With
> my
> > pre-9.5 backends, the moment the test runner exits I can see that all
> > backend processes exit immediately, and pg_activity_stat has no rows
> > (except the querying one). With 9.5beta2, however, some backend processes
> > continue to stay alive beyond the test runner, and pg_activity_stat
> > contains extra rows (state idle, waiting false). This situation persists
> > until I restart PostgreSQL.
>
> No idea what's happening, but a couple of questions:
>
> * Are you using SSL connections?
>
> * Can you get stack traces from the seemingly-stuck backends?
>

Most of my tests don't use SSL but some do. Looking at the query field in
pg_stat_activity I can see queries that don't seem to originate from SSL
tests.

Note that the number of backends that stay stuck after the tests is
constant (always 12).

Here's are stack dumps of the same process taken with both VS2015 Community
and Process Explorer, I went over 4 processes and saw the same thing. Let
me know what I else I can provide to help.

>From VS2015 Community:

Main Thread
> ntdll.dll!NtWaitForMultipleObjects() Unknown
  KernelBase.dll!WaitForMultipleObjectsEx() Unknown
  KernelBase.dll!WaitForMultipleObjects() Unknown
  postgres.exe!WaitLatchOrSocket(volatile Latch * latch, int wakeEvents,
unsigned __int64 sock, long timeout) Line 202 C
  postgres.exe!secure_read(Port * port, void * ptr, unsigned __int64 len)
Line 151 C
  postgres.exe!pq_getbyte() Line 926 C
  postgres.exe!SocketBackend(StringInfoData * inBuf) Line 345 C
  postgres.exe!PostgresMain(int argc, char * * argv, const char * dbname,
const char * username) Line 3984 C
  postgres.exe!BackendRun(Port * port) Line 4236 C
  postgres.exe!SubPostmasterMain(int argc, char * * argv) Line 4727 C
  postgres.exe!main(int argc, char * * argv) Line 211 C
  postgres.exe!__tmainCRTStartup() Line 626 C
  kernel32.dll!BaseThreadInitThunk() Unknown
  ntdll.dll!RtlUserThreadStart() Unknown

Worker Thread
> ntdll.dll!NtWaitForWorkViaWorkerFactory() Unknown
  ntdll.dll!TppWorkerThread() Unknown
  kernel32.dll!BaseThreadInitThunk() Unknown
  ntdll.dll!RtlUserThreadStart() Unknown

Worker Thread
> ntdll.dll!NtFsControlFile() Unknown
  KernelBase.dll!ConnectNamedPipe() Unknown
  postgres.exe!pg_signal_thread(void * param) Line 279 C
  kernel32.dll!BaseThreadInitThunk() Unknown
  ntdll.dll!RtlUserThreadStart() Unknown

Worker Thread
> ntdll.dll!NtWaitForSingleObject() Unknown
  KernelBase.dll!WaitForSingleObjectEx() Unknown
  postgres.exe!pg_timer_thread(void * param) Line 49 C
  kernel32.dll!BaseThreadInitThunk() Unknown
  ntdll.dll!RtlUserThreadStart() Unknown

>From Process Explorer (slightly different):

ntoskrnl.exe!KeSynchronizeExecution+0x3de6
ntoskrnl.exe!KeWaitForSingleObject+0xc7a
ntoskrnl.exe!KeWaitForSingleObject+0x709
ntoskrnl.exe!KeWaitForSingleObject+0x375
ntoskrnl.exe!IoQueueWorkItem+0x370
ntoskrnl.exe!KeRemoveQueueEx+0x16ba
ntoskrnl.exe!KeWaitForSingleObject+0xe8e
ntoskrnl.exe!KeWaitForSingleObject+0x709
ntoskrnl.exe!KeWaitForMultipleObjects+0x24e
ntoskrnl.exe!ObWaitForMultipleObjects+0x2bd
ntoskrnl.exe!IoWMIRegistrationControl+0x2402
ntoskrnl.exe!setjmpex+0x3943
ntdll.dll!NtWaitForMultipleObjects+0x14
KERNELBASE.dll!WaitForMultipleObjectsEx+0xef
KERNELBASE.dll!WaitForMultipleObjects+0xe
postgres.exe!WaitLatchOrSocket+0x243
postgres.exe!secure_read+0xb0
postgres.exe!pq_getbyte+0xec
postgres.exe!get_stats_option_name+0x392
postgres.exe!PostgresMain+0x537
postgres.exe!ShmemBackendArrayAllocation+0x2a6a
postgres.exe!SubPostmasterMain+0x273
postgres.exe!main+0x480
postgres.exe!pgwin32_popen+0x130b
KERNEL32.DLL!BaseThreadInitThunk+0x22
ntdll.dll!RtlUserThreadStart+0x34

Re: [HACKERS] Some 9.5beta2 backend processes not terminating properly?

[Shay Rojansky]

>
> Could you describe the worklad a bit more? Is this rather concurrent? Do
> you use optimized or debug builds? How long did you wait for the
> backends to die? Is this all over localhost, external ip but local,
> remotely?
>

The workload is a a rather diverse set of integration tests executed with
Npgsql. There's no concurrency whatsoever - tests are executed serially.
The backends stay alive indefinitely, until they are killed. All this is
over localhost with TCP. I can try other scenarios if that'll help.


> > Note that the number of backends that stay stuck after the tests is
> > constant (always 12).
>
> Can you increase the number of backends used in the test? And check
> whether it's still 12?
>

Well, I ran the testsuite twice in parallel, and got... 23 backends stuck
at the end.


> How are your clients disconnecting? Possibly without properly
> disconnecting?
>

That's possible, definitely in some of the test cases.

What I can do is try to isolate things further by playing around with the
tests and trying to see if a more minimal repro can be done - I'll try
doing this later today or tomorrow. If anyone has any other specific tests
or checks I should do let me know.

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Shulgin, Oleksandr]

On Tue, Dec 29, 2015 at 4:15 AM, Haribabu Kommi 
wrote:

> On Mon, Dec 28, 2015 at 9:09 PM, Shulgin, Oleksandr
>  wrote:
> >
> > Still this requires a revert of the memory context handling commit for
> > load_hba() and load_ident().  I think you can get around the problem by
> > changing these functions to work with CurrentMemoryContext and set it
> > explicitly to the newly allocated PostmasterContext in
> > PerformAuthentication().  In your function you could then create a
> temporary
> > context to be discarded before leaving the function.
>
> Thanks for the review. I didn't understand your point clearly.
>
> In the attached patch, load_hba uses PostmasterContext if it is present,
> otherwise CurretMemoryContext. PostmasterContext is present only
> in the backend start phase.
>
> > I still think you should not try to re-implement check_hba(), but extend
> > this function with means to report line skip reasons as per your
> > requirements.  Having an optional callback function might be a good fit
> (a
> > possible use case is logging the reasons line by line).
>
> check_hba function is enhanced to fill the hba line details with
> reason for mismatch.
> In check_hba function whenever a mismatch is found, the fill_hbaline
> function is
> called to frame the tuple and inserted into tuple store.
>

This is close enough, but what I actually mean by "a callback" is more or
less like the attached version.

While at it, I've also added some trivial code to preserve keyword quoting
in database and user fields, as well as added netmask output parameter;
also documentation is extended a little.

The biggest question for me is the proper handling of memory contexts for
HBA and ident files data.  I think it makes sense to release them
explicitly because with the current state of affairs, we have dangling
pointers in parsed_{hba,ident}_{context,lines} after release of
PostmasterContext.  The detailed comment in postgres.c
around MemoryContextDelete(PostmasterContext); suggests that it's not easy
already to keep track of the all things that might be affected by this
cleanup step:

/*
* If the PostmasterContext is still around, recycle the space; we don't
* need it anymore after InitPostgres completes.  Note this does not trash
* *MyProcPort, because ConnCreate() allocated that space with malloc()
* ... else we'd need to copy the Port data first.  Also, subsidiary data
* such as the username isn't lost either; see ProcessStartupPacket().
*/

Not sure if we need any advanced machinery here like some sort of cleanup
hooks list?  For now I've added discard_{hba,ident}() functions and call
them explicitly where appropriate.

--
Alex
From 56b584c25b952d6855d2a3d77eb40755d982efb9 Mon Sep 17 00:00:00 2001
From: Oleksandr Shulgin 
Date: Tue, 29 Dec 2015 14:51:27 +0100
Subject: [PATCH] WIP: pg_hba_lookup()

---
 doc/src/sgml/func.sgml   |  39 +++
 src/backend/catalog/system_views.sql |   9 +
 src/backend/libpq/hba.c  | 663 ++-
 src/backend/utils/init/postinit.c|  26 +-
 src/include/catalog/pg_proc.h|   2 +
 src/include/libpq/hba.h  |   4 +
 src/include/utils/builtins.h |   3 +
 7 files changed, 720 insertions(+), 26 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index e08bf60..2594dd5
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SELECT collation for ('foo' COLLATE "de_
*** 16576,16581 
--- 16576,16594 
 text
 set parameter and return new value

+   
+
+ 
+  pg_hba_lookup
+ 
+ pg_hba_lookup(database text,
+ user_name text
+ [, address text]
+ [, ssl_inuse text)
+
+record
+scan pg_hba.conf and return examined entries up to a matching one
+   
   
  
 
*** SELECT set_config('log_statement_stats',
*** 16633,16638 
--- 16646,16677 
  
 
  
+
+ pg_hba_lookup returns a set of records containing the
+ line number, mode, type, database, user_name, address, netmask, hostname,
+ method, options and skip reason. For example, to debug problems with user
+ kommih trying to connect to a database postgres
+ from IPv6-address ::1, one can issue a following query:
+ 
+ postgres=# SELECT * FROM pg_hba_lookup('postgres', 'kommih', '::1');
+  line_number |  mode   | type  | database | user_name |  address  | netmask | hostname | method | options |  reason  
+ -+-+---+--+---+---+-+--++-+--
+   84 | skipped | local | {all}| {all} |   |  

Re: [HACKERS] commit 5c45d2f need to be back-patch on branch 9.2 & before

[Andres Freund]

Hi,

On 2015-12-29 11:22:52 +, amul sul wrote:
> Uninitialized variable 'dtype' warning in date_in() is fix[1] on branch 9.3 & 
> above, but not for 9.2 & older branches. I guess this should be back-patch. 
> [1] commit link=> 
> http://git.postgresql.org/pg/commitdiff/5c45d2f87835ccd3ffac338845ec79cab1b31a11

We don't routinely backpatch warnings. Often enough the benefit is not
worth the risk. In this case I'd possibly gone the other way, but given
it's a low frequency warning in old branches I'm not inclined to do so
today.

Andres


-- 
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] Making tab-complete.c easier to maintain

[Michael Paquier]

On Sun, Dec 20, 2015 at 8:08 AM, Michael Paquier
 wrote:
> On Sun, Dec 20, 2015 at 6:24 AM, Tom Lane  wrote:
>> 1. I think it would be a good idea to convert the matching rules for
>> backslash commands too.  To do that, we'd need to provide a case-sensitive
>> equivalent to word_match and the matching macros.  I think we'd also have
>> to extend word_match to allow a trailing wildcard character, maybe "*".

I am not really sure I follow much the use of the wildcard, do you
mean to be able to work with the [S] extensions of the backslash
commands which are not completed now? I am attaching a patch that adds
support for a case-sensitive comparison facility without this wildcard
system, simplifying the backslash commands.

>> 2. I believe that a very large fraction of the TailMatches() rules really
>> ought to be Matches(), ie, they should not consider matches that don't
>> start at the start of the line.  And there's another bunch that could
>> be Matches() if the author hadn't been unaccountably lazy about checking
>> all words of the expected command.  If we converted as much as we could
>> that way, it would make psql_completion faster because many inapplicable
>> rules could be discarded after a single integer comparison on
>> previous_words_count, and it would greatly reduce the risk of inapplicable
>> matches.  We can't do that for rules meant to apply to DML statements,
>> since they can be buried in WITH, EXPLAIN, etc ... but an awful lot of
>> the DDL rules could be changed.

Yep, clearly. We may gain a bit of performance by matching directly
with an equal number of words using Matches instead of a lower bound
with TailMatches. I have looked at this thing and hacked a patch as
attached.

>> 3. The HeadMatches macros are pretty iffy because they can only look back
>> nine words.  I'm tempted to redesign get_previous_words so it just
>> tokenizes the whole line rather than having an arbitrary limitation.
>> (For that matter, it's long overdue for it to be able to deal with
>> multiline input...)
>>
>> I might go look at #3, but I can't currently summon the energy to tackle
>> #1 or #2 --- any volunteers?

#3 has been already done in the mean time...

> I could have a look at both of them and submit patch for next CF, both
> things do not seem that much complicated.

Those things are as well added to the next CF.
-- 
Michael
From 351894c975e72d62b6c49e8ea203fc194ccc59ee Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Tue, 29 Dec 2015 22:13:27 +0900
Subject: [PATCH 1/2] Improve performance of psql tab completion

TailMatches are based on a lower-bound check and Matches uses a direct
match for the number of words. It happens that the former is used in many
places where the latter could be used. Doing the switch improve the
performance of tab completion by having to match only a number of words
for many commands.
---
 src/bin/psql/tab-complete.c | 567 +++-
 1 file changed, 291 insertions(+), 276 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4c93ae9..c920353 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1137,6 +1137,21 @@ psql_completion(const char *text, int start, int end)
 #define Matches4(p1, p2, p3, p4) \
 	(previous_words_count == 4 && \
 	 TailMatches4(p1, p2, p3, p4))
+#define Matches5(p1, p2, p3, p4, p5) \
+	(previous_words_count == 5 && \
+	 TailMatches5(p1, p2, p3, p4, p5))
+#define Matches6(p1, p2, p3, p4, p5, p6) \
+	(previous_words_count == 6 && \
+	 TailMatches6(p1, p2, p3, p4, p5, p6))
+#define Matches7(p1, p2, p3, p4, p5, p6, p7) \
+	(previous_words_count == 7 && \
+	 TailMatches7(p1, p2, p3, p4, p5, p6, p7))
+#define Matches8(p1, p2, p3, p4, p5, p6, p7, p8) \
+	(previous_words_count == 8 && \
+	 TailMatches8(p1, p2, p3, p4, p5, p6, p7, p8))
+#define Matches9(p1, p2, p3, p4, p5, p6, p7, p8, p9) \
+	(previous_words_count == 9 && \
+	 TailMatches9(p1, p2, p3, p4, p5, p6, p7, p8, p9))
 
 	/*
 	 * Macros for matching N words at the start of the line, regardless of
@@ -1266,10 +1281,10 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches7("ALL", "IN", "TABLESPACE", MatchAny, "OWNED", "BY", MatchAny))
 		COMPLETE_WITH_CONST("SET TABLESPACE");
 	/* ALTER AGGREGATE,FUNCTION  */
-	else if (TailMatches3("ALTER", "AGGREGATE|FUNCTION", MatchAny))
+	else if (Matches3("ALTER", "AGGREGATE|FUNCTION", MatchAny))
 		COMPLETE_WITH_CONST("(");
 	/* ALTER AGGREGATE,FUNCTION  (...) */
-	else if (TailMatches4("ALTER", "AGGREGATE|FUNCTION", MatchAny, MatchAny))
+	else if (Matches4("ALTER", "AGGREGATE|FUNCTION", MatchAny, MatchAny))
 	{
 		if (ends_with(prev_wd, ')'))
 			COMPLETE_WITH_LIST3("OWNER TO", "RENAME TO", "SET SCHEMA");
@@ -1278,49 +1293,49 @@ psql_completion(const char *text, int start, int end)
 	}
 
 	/* ALTER SCHEMA  */
-	else if (TailMatches3("ALTER", "SCHEMA", MatchAny))
+	else if