Re: [HACKERS] Update comments in nodeModifyTable.c

2017-06-06 Thread Robert Haas
On Mon, Jun 5, 2017 at 4:45 AM, Etsuro Fujita
 wrote:
> While working on [1], I noticed that the comment in ExecModifyTable:
>
>  * Foreign table updates have a wholerow attribute when the
>  * relation has an AFTER ROW trigger.
>
> is not 100% correct because a foreign table has a wholerow attrubute when
> the relation has an AFTER ROW or BEFORE ROW trigger (see
> rewriteTargetListUD).  So I'd propose s/an AFTER ROW trigger/a row-level
> trigger/.  Attached is a patch for that.

That seems better, but looking at rewriteTargetListUD, it seems that
the actual rule is that this happens when there is a row-level on
either UPDATE or DELETE.  If there is only a row-level trigger on
INSERT, then it is not done.  Perhaps we should try to include that
detail in the comment as well.

-- 
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] Minor fix for EventCacheLookup()

2017-06-06 Thread Robert Haas
On Tue, Jun 6, 2017 at 12:24 AM, Amit Langote
 wrote:
> It should return NIL when no entry is found in the cache, not NULL.
>
> Attached patch fixes that.

I'm not sure how much value neatnik-ism has in cases like this, but committed.

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


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


Re: [HACKERS] sketchy partcollation handling

2017-06-06 Thread Robert Haas
On Sun, Jun 4, 2017 at 10:18 PM, Amit Langote
 wrote:
> I think we can call it a bug of StorePartitionKey().  I looked at the
> similar code in index_create() (which actually I had originally looked at
> for reference when writing the partitioning code in question) and looks
> like it doesn't store the dependency for collation 0 and for the default
> collation of the database.  I think the partitioning code should do the
> same.  Attached find a patch for the same (which also updates the
> documentation as mentioned above); with the patch:

Thanks.  Committed.

> BTW, the places which check whether the collation to store a dependency
> for is the database default collation don't need to do that.  I mean the
> following code block in all of these places:
>
>/* The default collation is pinned, so don't bother recording it */
>if (OidIsValid(attr->attcollation) &&
>attr->attcollation != DEFAULT_COLLATION_OID)
>{
>referenced.classId = CollationRelationId;
>referenced.objectId = attr->attcollation;
>referenced.objectSubId = 0;
>recordDependencyOn(, , DEPENDENCY_NORMAL);
>}
>
> That's because the default collation is pinned and the dependency code
> checks isObjectPinned() and does not create pg_depend entries if so.
> Those places are:
>
> AddNewAttributeTuples
> StorePartitionKey
> index_create
> GenerateTypeDependencies
> add_column_collation_dependency

We could go change them all, but I guess I don't particularly see the point.

-- 
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] Extra Vietnamese unaccent rules

2017-06-06 Thread Dang Minh Huong
On Jun 4, 29 Heisei, at 00:48, Bruce Momjian  wrote:On Sun, Jun  4, 2017 at 12:43:17AM +0900, Dang Minh Huong wrote:On May 30, 29 Heisei, at 00:22, Dang Minh Huong  wrote:On May 29, 29 Heisei, at 10:47, Thomas Munro > wrote:On Sun, May 28, 2017 at 7:55 PM, Dang Minh Huong > wrote:Thanks for reporting and lecture about unicode.I attached a patch as the instruction from Thomas. Could you confirm it.-   is_plain_letter(table[codepoint.combining_ids[0]]) and \+   (is_plain_letter(table[codepoint.combining_ids[0]]) or\+    len(table[codepoint.combining_ids[0]].combining_ids) > 1) and \Shouldn't you use "or is_letter_with_marks()", instead of "or len(...)1"?  Your test might catch something that isn't based on a 'letter'(according to is_plain_letter).  Otherwise this looks pretty good tome.  Please add it to the next commitfest.Thanks for confirm, sir.I will add it to the next CF soon.Sorry for lately response. I attach the update patch.Uh, there is no patch attached.Sorry sir, reattach the patch.I also added it to the next CF and set reviewers to Thomas Munro. Could you confirm for me.

unaccent.patch
Description: Binary data
---Thanks and best regards,Dang Minh Huong

Re: [HACKERS] question about replication docs

2017-06-06 Thread Peter Eisentraut
On 5/29/17 09:11, Dave Cramer wrote:
> The following makes an explicit reference to the simple query protocol
> being the only protocol allowed in walsender mode. It is my
> understanding this is true for logical replication as well ??

It's the same thing (for this purpose).

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


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


Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

2017-06-06 Thread Sven R. Kunze

On 06.06.2017 14:31, Bruce Momjian wrote:

On Tue, Jun  6, 2017 at 04:39:50AM -0300, Alvaro Herrera wrote:

I was thinking that you could create the init fork for each unlogged
table in a permanent tablespace (probably the default one for the
database).

FWIW I don't think calling these tablespaces "temporary" is the right
word.  It's not the tablespaces that are temporary.  Maybe "evanescent".

I was thinking "transient".  Amazon uses "ephemeral".



Just dropping a translator result here:

[1] results of https://www.dict.cc/deutsch-englisch/fl%C3%BCchtig.html

What about VOLATILE like computer memory (RAM)?

Sven

[1]
volatile {adj}chem.
briefly {adv}
fleeting {adj}
fugitive {adj} [temporary, fleeing]
brief {adj}
cursory {adj}
transient {adj}
vaguely {adv} [look, interested]
transitory {adj}
perfunctory {adj}
hasty {adj}
ephemeral {adj}
fleetingly {adv}
momentary {adj}
evanescent {adj}
passing {adj}
absconded {adj}
casual {adj} [glance, acquaintance etc.]
desultory {adj} [reading]
tangential {adj}
ethereal {adj}chem.
etherial {adj}chem.
flighty {adj}
sketchy {adj} [not detailed]
quick {adj}
perfunctorily {adv}
sketchily {adv}
fugitively {adv}
volatilely {adv}
hit and run {adj}
ephemerally {adv}
non-permanent {adj}
fugacious {adj} [literary]



Re: [HACKERS] Index created in BEFORE trigger not updated during INSERT

2017-06-06 Thread Albe Laurenz
Tom Lane wrote:
>> Andres Freund  writes:
>>> Hm, strategically sprinkled CheckTableNotInUse() might do the trick?
> 
> Attached is a proposed patch that closes off this problem.  I've tested
> it to the extent that it blocks Albe's example and passes check-world.

I tested it, and it works fine.  Thanks!

> I'm unsure whether to back-patch or not; the main argument for not doing
> so is that if any extensions are calling DefineIndex() directly, this
> would be an API break for them.  Given what a weird case this is, I'm not
> sure it's worth that.
> 
> A possible end-run around the API objection would be to not add an extra
> argument to DefineIndex() in the back branches, but to use !is_alter_table
> as the control condition.  That would work for the core callers, although
> we might need a special case for bootstrap mode.  On the other hand,
> thinking again about hypothetical third-party callers, it's possible that
> that's not the right answer for them, in which case they'd be really in
> trouble.  So I'm not that much in love with that answer.

It causes a slight bellyache to leave an unfixed data corruption bug
in the back branches (if only index corruption), but I agree that it is
such a weird case to create an index in a BEFORE trigger that we probably
can live without a back-patch.

Yours,
Laurenz Albe

-- 
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] Challenges preventing us moving to 64 bit transaction id (XID)?

2017-06-06 Thread Bruce Momjian
On Tue, Jun  6, 2017 at 09:05:03AM -0400, Peter Eisentraut wrote:
> On 6/6/17 08:29, Bruce Momjian wrote:
> > On Tue, Jun  6, 2017 at 06:00:54PM +0800, Craig Ringer wrote:
> >> Tom's point is, I think, that we'll want to stay pg_upgrade
> >> compatible. So when we see a pg10 tuple and want to add a new page
> >> with a new page header that has an epoch, but the whole page is full
> >> so there isn't 32 bits left to move tuples "down" the page, what do we
> >> do?
> > 
> > I guess I am missing something.  If you see an old page version number,
> > you know none of the tuples are from running transactions so you can
> > just freeze them all, after consulting the pg_clog.  What am I missing?
> > If the page is full, why are you trying to add to the page?
> 
> The problem is if you want to delete from such a page.  Then you need to
> update the tuple's xmax and stick the new xid epoch somewhere.
> 
> We had an unconference session at PGCon about this.  These issues were
> all discussed and some ideas were thrown around.  We can expect a patch
> to appear soon, I think.

Sorry I missed the unconference session.

OK, crazy idea.  Since we know the creation is frozen can we put the
epoch in the xmin and set some tuple bit that only has meaning on old
page versions?  Yeah, I said crazy.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient 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] inconsistent application_name use in logical workers

2017-06-06 Thread Peter Eisentraut
On 6/6/17 06:51, Petr Jelinek wrote:
> On 06/06/17 04:19, Peter Eisentraut wrote:
>> The logical replication code is supposed to use the subscription name as
>> the fallback_application_name, but in some cases it uses the slot name,
>> which could be different.  See attached patch to correct this.
> 
> Hmm, well the differentiation has a reason though. The application_name
> is used for sync rep and having synchronization connection using same
> application_name might have adverse effects there because
> synchronization connection can be in-front of main apply one, so sync
> rep will think something is consumed while it's not.

True, we should use a different name for tablesync.c.  But the one in
worker.c appears to be a mistake then?

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


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


Re: [HACKERS] Challenges preventing us moving to 64 bit transaction id (XID)?

2017-06-06 Thread Peter Eisentraut
On 6/6/17 08:29, Bruce Momjian wrote:
> On Tue, Jun  6, 2017 at 06:00:54PM +0800, Craig Ringer wrote:
>> Tom's point is, I think, that we'll want to stay pg_upgrade
>> compatible. So when we see a pg10 tuple and want to add a new page
>> with a new page header that has an epoch, but the whole page is full
>> so there isn't 32 bits left to move tuples "down" the page, what do we
>> do?
> 
> I guess I am missing something.  If you see an old page version number,
> you know none of the tuples are from running transactions so you can
> just freeze them all, after consulting the pg_clog.  What am I missing?
> If the page is full, why are you trying to add to the page?

The problem is if you want to delete from such a page.  Then you need to
update the tuple's xmax and stick the new xid epoch somewhere.

We had an unconference session at PGCon about this.  These issues were
all discussed and some ideas were thrown around.  We can expect a patch
to appear soon, I think.

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


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


Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

2017-06-06 Thread Peter Eisentraut
On 6/6/17 03:39, Alvaro Herrera wrote:
> FWIW I don't think calling these tablespaces "temporary" is the right
> word.  It's not the tablespaces that are temporary.

The SQL standard meaning of temporary is that the content goes away.  It
is only in PostgreSQL that it also means that the object goes away.  I
think it's OK that it can encompass both meanings.

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


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


Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

2017-06-06 Thread Bruce Momjian
On Tue, Jun  6, 2017 at 04:39:50AM -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> 
> > FWIW, allowing UNLOGGED tables, rather than just TEMPORARY ones,
> > increases the complexity of that project noticeably.  For TEMPORARY you
> > basically don't need to do much but to recreate the structure inside the
> > tablespace at start - fairly simple.  But for UNLOGGED you need to find
> > a way to recreate the relevant file and init forks - otherwise we might
> > not notice what needs to be reset at a crash restart, and we might error
> > out when executing selects etc. and then the table's not there.
> > Presumably recreating files & init forks that at first table access is
> > doable, but it's not entirely trivial to do locking wise.
> 
> I was thinking that you could create the init fork for each unlogged
> table in a permanent tablespace (probably the default one for the
> database).
> 
> FWIW I don't think calling these tablespaces "temporary" is the right
> word.  It's not the tablespaces that are temporary.  Maybe "evanescent".

I was thinking "transient".  Amazon uses "ephemeral".

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient 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: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256

2017-06-06 Thread Stephen Frost
Michael,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
> On Fri, Jun 2, 2017 at 10:25 AM, Robert Haas  wrote:
> > On Thu, Jun 1, 2017 at 9:13 PM, Michael Paquier
> >  wrote:
> >> It seems to me that any testing in this area won't fly high as long as
> >> there is no way to enforce the list of TLS implementations that a
> >> server allows. There have been discussions about being able to control
> >> that after the OpenSSL vulnerabilities that were protocol-specific and
> >> there were even patches adding GUCs for this purpose. At the end,
> >> everything has been rejected as Postgres enforces the use of the
> >> newest one when doing the SSL handshake.
> >
> > TLS implementations, or TLS versions?  What does the TLS version have
> > to do with this issue?
> 
> I really mean *version* here. Unlike OpenSSL, the Windows TLS
> implementation does not offer an API to choose the latest TLS version
> available:
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa380513(v=vs.85).aspx
> It is up to the server and the client to negotiate that, so it seems
> to me that we want some control in this area, which would be important
> for testing as well because the TLS finish message differs a bit
> across versions, in length mainly. On top of that per the aggressive
> updates that Windows does from time to time they may as well forcibly
> expose users to a broken TLS implementation...
> MacOS has something similar to OpenSSL, with
> SSLGetProtocolVersionMax(), which is nice.

We mainly need to know what version was used, right..?  Isn't that
available?

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Challenges preventing us moving to 64 bit transaction id (XID)?

2017-06-06 Thread Bruce Momjian
On Tue, Jun  6, 2017 at 06:00:54PM +0800, Craig Ringer wrote:
> On 6 June 2017 at 12:38, Ashutosh Bapat  
> wrote:
> > On Tue, Jun 6, 2017 at 10:00 AM, Tom Lane  wrote:
> >> In my mind the harder problem is where to find another 32 bits for the
> >> new page header field.  You could convert the header format on-the-fly
> >> if there's free space in the page, but what if there isn't?
> >
> > I guess, we will have to reserve 32 bits in the header. That's much
> > better than increasing tuple header by 32 bits.
> 
> Tom's point is, I think, that we'll want to stay pg_upgrade
> compatible. So when we see a pg10 tuple and want to add a new page
> with a new page header that has an epoch, but the whole page is full
> so there isn't 32 bits left to move tuples "down" the page, what do we
> do?

I guess I am missing something.  If you see an old page version number,
you know none of the tuples are from running transactions so you can
just freeze them all, after consulting the pg_clog.  What am I missing?
If the page is full, why are you trying to add to the page?

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient 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] inconsistent application_name use in logical workers

2017-06-06 Thread Petr Jelinek
On 06/06/17 04:19, Peter Eisentraut wrote:
> The logical replication code is supposed to use the subscription name as
> the fallback_application_name, but in some cases it uses the slot name,
> which could be different.  See attached patch to correct this.
> 

Hmm, well the differentiation has a reason though. The application_name
is used for sync rep and having synchronization connection using same
application_name might have adverse effects there because
synchronization connection can be in-front of main apply one, so sync
rep will think something is consumed while it's not.

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


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


Re: [HACKERS] Proposal : For Auto-Prewarm.

2017-06-06 Thread Rafia Sabih
On Mon, Jun 5, 2017 at 8:06 PM, Robert Haas  wrote:
>
> Many of these seem worse, like these ones:
>
> - * Quit if we've reached records for another database. Unless the
> + * Quit if we've reached records of another database. Unless the
>
Why is it worse? I've always encountered using "records of database"
and not "records for database". Anyhow, I tried reframing the sentence
altogether.
> - * When we reach a new relation, close the old one.  Note, however,
> - * that the previous try_relation_open may have failed, in which case
> - * rel will be NULL.
> + * On reaching a new relation, close the old one.  Note, that the
> + * previous try_relation_open may have failed, in which case rel will
> + * be NULL.
>
Reframed the sentence.
> - * Try to open each new relation, but only once, when we first
> - * encounter it.  If it's been dropped, skip the associated blocks.
> + * Each relation is open only once at it's first encounter. If it's
> + * been dropped, skip the associated blocks.
>
Reframed.

> IMHO, there's still a good bit of work needed here to make this sound
> like American English.  For example:
>
> - *It is a bgworker which automatically records information about 
> blocks
> - *which were present in buffer pool before server shutdown and then
> - *prewarm the buffer pool upon server restart with those blocks.
> + *It is a bgworker process that automatically records information 
> about
> + *blocks which were present in buffer pool before server
> shutdown and then
> + *prewarms the buffer pool upon server restart with those blocks.
>
> This construction "It is a..." without a clear referent seems to be
> standard in Indian English, but it looks wrong to English speakers
> from other parts of the world, or at least to me.
>
Agreed, tried reframing the sentence.
> + * Since there could be at max one worker who could do a prewarm, hence,
> + * acquiring locks is not required before setting 
> skip_prewarm_on_restart.
>
> To me, adding a comma before hence looks like a significant
> improvement, but the word hence itself seems out-of-place.  Also, I'd
> change "at max" to "at most" and maybe reword the sentence a little.
> There's a lot of little things like this which I have tended be quite
> strict about changing before commit; I occasionally wonder whether
> it's really worth the effort.  It's not really wrong, it just sounds
> weird to me as an American.
>
Agree, sentence reframed.
I am attaching the patch with the modifications I made on a second look.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


cosmetic_autoprewarm_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] Is ECPG's SET CONNECTION really not thread-aware?

2017-06-06 Thread Michael Meskes
Dear Tsunakawa-san,

sorry for the late reply, I've been traveling all of last week and only
just came back.

>> What does this mean by "not thread-aware?"  Does SET CONNECTION in one
>> thread change the current connection in another thread?
>> It doesn't look so, because the connection management and SQL execution
>> in ECPG uses thread-specific data (TSD) to get and set the current 
>> connection,
>> like this:

I'm pretty sure it is indeed thread-aware, although I didn't provide the
code for this feature myself.

> So the doc seems to need fix.  The patch is attached.

Thanks, applied.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL


-- 
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 : For Auto-Prewarm.

2017-06-06 Thread Neha Sharma
Hi,

I have been testing this feature for a while and below are the observations
for few scenarios.

*Observation:*
scenario 1: If we set pg_prewarm.dump_interval = -1.0,we get an additional
warning message in logfile and instead of ending the task of auto-dump it
executes successfully.
[centos@test-machine bin]$ more logfile
2017-06-06 08:39:53.127 GMT [21905] WARNING:  invalid value for parameter
"pg_prewarm.dump_interval": "-1.0"
2017-06-06 08:39:53.127 GMT [21905] HINT:  Valid units for this parameter
are "ms", "s", "min", "h", and "d".
2017-06-06 08:39:53.127 GMT [21905] LOG:  listening on IPv6 address "::1",
port 5432
2017-06-06 08:39:53.127 GMT [21905] LOG:  listening on IPv4 address
"127.0.0.1", port 5432
2017-06-06 08:39:53.130 GMT [21905] LOG:  listening on Unix socket
"/tmp/.s.PGSQL.5432"
2017-06-06 08:39:53.143 GMT [21906] LOG:  database system was shut down at
2017-06-06 08:38:20 GMT
2017-06-06 08:39:53.155 GMT [21905] LOG:  database system is ready to
accept connections
2017-06-06 08:39:53.155 GMT [21912] LOG:  autoprewarm has started
[centos@test-machine bin]$ ps -ef | grep prewarm
centos   21912 21905  0 08:39 ?00:00:00 postgres: bgworker:
autoprewarm
[centos@test-machine bin]$ ./psql postgres
psql (10beta1)
Type "help" for help.

postgres=# show pg_prewarm.dump_interval;
 pg_prewarm.dump_interval
--
 5min
(1 row)

scenario 2: If we set pg_prewarm.dump_interval = 0.0,we get an additional
warning message in logfile and the message states that the task was started
and the worker thread it is also active,but the dump_interval duration is
set to default 5 min (300 sec) instead of 0.

[centos@test-machine bin]$ ps -ef | grep prewarm
centos   21980 21973  0 08:54 ?00:00:00 postgres: bgworker:
autoprewarm

[centos@test-machine bin]$ more logfile
2017-06-06 09:20:52.436 GMT [3] WARNING:  invalid value for parameter
"pg_prewarm.dump_interval": "0.0"
2017-06-06 09:20:52.436 GMT [3] HINT:  Valid units for this parameter
are "ms", "s", "min", "h", and "d".
2017-06-06 09:20:52.436 GMT [3] LOG:  listening on IPv6 address "::1",
port 5432
2017-06-06 09:20:52.437 GMT [3] LOG:  listening on IPv4 address
"127.0.0.1", port 5432
2017-06-06 09:20:52.439 GMT [3] LOG:  listening on Unix socket
"/tmp/.s.PGSQL.5432"
2017-06-06 09:20:52.452 GMT [4] LOG:  database system was shut down at
2017-06-06 09:19:49 GMT
2017-06-06 09:20:52.455 GMT [3] LOG:  database system is ready to
accept connections
2017-06-06 09:20:52.455 GMT [22230] LOG:  autoprewarm has started

[centos@test-machine bin]$ ./psql postgres
psql (10beta1)
Type "help" for help.

postgres=# show pg_prewarm.dump_interval;
 pg_prewarm.dump_interval
--
 5min
(1 row)


On Mon, Jun 5, 2017 at 8:06 PM, Robert Haas  wrote:

> On Mon, Jun 5, 2017 at 7:58 AM, Rafia Sabih
>  wrote:
> > I had a look at the patch from stylistic/formatting point of view,
> > please find the attached patch for the suggested modifications.
>
> Many of these seem worse, like these ones:
>
> - * Quit if we've reached records for another database. Unless the
> + * Quit if we've reached records of another database. Unless the
>
> - * When we reach a new relation, close the old one.  Note,
> however,
> - * that the previous try_relation_open may have failed, in which
> case
> - * rel will be NULL.
> + * On reaching a new relation, close the old one.  Note, that the
> + * previous try_relation_open may have failed, in which case rel
> will
> + * be NULL.
>
> - * Try to open each new relation, but only once, when we first
> - * encounter it.  If it's been dropped, skip the associated
> blocks.
> + * Each relation is open only once at it's first encounter. If
> it's
> + * been dropped, skip the associated blocks.
>
> Others are better, like these:
>
> -(errmsg("could not continue autoprewarm worker is
> already running under PID %d",
> +(errmsg("autoprewarm worker is already running under PID
> %d",
>
> - * Start of prewarm per-database worker. This will try to load blocks of
> one
> + * Start prewarm per-database worker, which will load blocks of one
>
> Others don't really seem better or worse, like:
>
> - * Register a per-database worker to load new database's block.
> And
> - * wait until they finish their job to launch next one.
> + * Register a per-database worker to load new database's block.
> Wait
> + * until they finish their job to launch next one.
>
> IMHO, there's still a good bit of work needed here to make this sound
> like American English.  For example:
>
> - *It is a bgworker which automatically records information about
> blocks
> - *which were present in buffer pool before server shutdown and
> then
> - *prewarm the buffer pool upon server restart with 

Re: [HACKERS] Challenges preventing us moving to 64 bit transaction id (XID)?

2017-06-06 Thread Craig Ringer
On 6 June 2017 at 12:38, Ashutosh Bapat  wrote:
> On Tue, Jun 6, 2017 at 10:00 AM, Tom Lane  wrote:
>> Ashutosh Bapat  writes:
>>> On Tue, Jun 6, 2017 at 9:48 AM, Craig Ringer  wrote:
 Storing an epoch implies that rows can't have (xmin,xmax) different by
 more than one epoch. So if you're updating/deleting an extremely old
 tuple you'll presumably have to set xmin to FrozenTransactionId if it
 isn't already, so you can set a new epoch and xmax.
>>
>>> If the page has multiple such tuples, updating one tuple will mean
>>> updating headers of other tuples as well? This means that those tuples
>>> need to be locked for concurrent scans?
>>
>> Locks for tuple header updates are taken at page level anyway, so in
>> principle you could run around and freeze other tuples on the page
>> anytime you had to change the page's high-order-XID value.  Holding
>> the lock for long enough to do that is slightly annoying, but it
>> should happen so seldom as to not represent a real performance problem.
>>
>> In my mind the harder problem is where to find another 32 bits for the
>> new page header field.  You could convert the header format on-the-fly
>> if there's free space in the page, but what if there isn't?
>
> I guess, we will have to reserve 32 bits in the header. That's much
> better than increasing tuple header by 32 bits.

Tom's point is, I think, that we'll want to stay pg_upgrade
compatible. So when we see a pg10 tuple and want to add a new page
with a new page header that has an epoch, but the whole page is full
so there isn't 32 bits left to move tuples "down" the page, what do we
do?





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


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


Re: [HACKERS] [POC] hash partitioning

2017-06-06 Thread Dilip Kumar
On Tue, Jun 6, 2017 at 2:41 PM, Amit Langote
 wrote:
> Consider an example using the partition hierarchy:
>
> root (a int, b char, c int) partition by range (a)
>
>  -> level1 from (1) to (10) partition by list (b)
>
>  -> level2 in ('a') parition by range (c)
>
>  -> leaf from (1) to (10)
>
> Inserting (1, 'b', 1) into level1 will fail, because tuple can't be routed
> at level1 (no partition defined for b = 'b').
>
> Inserting (1, 'a', 10) into level1 will fail, because tuple can't be
> routed at level2 (no partition defined for c >= 10).
>
> Inserting (10, 'a', 1) into level1 will fail, because, although it was
> able to get through level1 and level2 into leaf, a = 10 falls out of
> level1's defined range.  We don't check that 1 <= a < 10 before starting
> the tuple-routing.
>
> I wonder if we should...  Since we don't allow BR triggers on partitioned
> tables, there should not be any harm in doing it just before calling
> ExecFindPartition().  Perhaps, topic for a new thread.

Yeah, correct.

-- 
Regards,
Dilip Kumar
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] [POC] hash partitioning

2017-06-06 Thread Amit Langote
On 2017/06/06 17:50, Dilip Kumar wrote:
> On Tue, Jun 6, 2017 at 1:03 PM, amul sul  wrote:
>> May I ask you, how you sure about 8 is an unfit value for t1 relation?
>> And what if the value other than 8, for e.g. 7?
> 
> Well, First I created t1 as a leaf relation like below, and I tested
> insert into t1 with value 8 and it was violating the partition
> constraint of t1, however, 7 was fine.
> 
> create table t (a int) partition by hash(a);
> create table t1 partition of t for values with (modulus 2, remainder 1);
> 
> Later I dropped this t1 and created 2 level partition with the leaf as a 
> range.
> 
> drop table t1;
> create table t1 partition of t for values with (modulus 2, remainder
> 1) partition by range(a);
> create table t1_1 partition of t1 for values from (8) to (10);
> 
> So now, I am sure that t1_1 can accept the value 8 and its parent t1 can't.
> 
> So I think this can only happen in the case of partitioned by hash
> that a value is legal for the child but illegal for the parent?  Isn't
> it a good idea that if a user is inserting in the top level relation
> he should know for which partition exactly the constraint got
> violated?

It's how the original partitioning code around ExecInsert/CopyFrom works,
not something that only affects hash partitioning.  So, I think that
Amul's patch is fine and if we want to change something here, it should be
done by an independent patch.  See the explanation below:

If we insert into a partition directly, we must check its partition
constraint.  If the partition happens to be itself a partitioned table,
the constraint will be checked *after* tuple-routing and ExecConstraints()
is passed the leaf partition's ResultRelInfo, so if an error occurs there
we will use the leaf partition's name in the message.  Since we combine
the leaf partition's own constraint with all of the ancestors' into a
single expression that is passed to ExecCheck(), it is hard to say exactly
which ancestor's constraint is violated.  However, if the partition
constraint of some intervening ancestor had been violated, we wouldn't be
in ExecConstraints() at all; tuple-routing itself would have failed.  So
it seems that we need worry (if at all) only about partition constraints
of the table mentioned in the insert statement.

Consider an example using the partition hierarchy:

root (a int, b char, c int) partition by range (a)

 -> level1 from (1) to (10) partition by list (b)

 -> level2 in ('a') parition by range (c)

 -> leaf from (1) to (10)

Inserting (1, 'b', 1) into level1 will fail, because tuple can't be routed
at level1 (no partition defined for b = 'b').

Inserting (1, 'a', 10) into level1 will fail, because tuple can't be
routed at level2 (no partition defined for c >= 10).

Inserting (10, 'a', 1) into level1 will fail, because, although it was
able to get through level1 and level2 into leaf, a = 10 falls out of
level1's defined range.  We don't check that 1 <= a < 10 before starting
the tuple-routing.

I wonder if we should...  Since we don't allow BR triggers on partitioned
tables, there should not be any harm in doing it just before calling
ExecFindPartition().  Perhaps, topic for a new thread.

Thanks,
Amit



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


Re: [HACKERS] Get stuck when dropping a subscription during synchronizing table

2017-06-06 Thread Masahiko Sawada
On Sun, Jun 4, 2017 at 1:53 AM, Peter Eisentraut
 wrote:
> On 6/2/17 14:52, Peter Eisentraut wrote:
>> On 5/24/17 15:14, Petr Jelinek wrote:
>>> All the locking works just fine the way it is in master. The issue with
>>> deadlock with apply comes from the wrong handling of the SIGTERM in the
>>> apply (we didn't set InterruptPending). I changed the SIGTERM handler in
>>> patch 0001 just to die which is actually the correct behavior for apply
>>> workers. I also moved the connection cleanup code to the
>>> before_shmem_exit callback (similar to walreceiver) and now that part
>>> works correctly.
>>
>> I have committed this, in two separate parts.  This should fix the
>> originally reported issue.
>>
>> I will continue to work through your other patches.  I notice there is
>> still a bit of discussion about another patch, so please let me know if
>> there is anything else I should be looking for.
>
> I have committed the remaining two patches.  I believe this fixes the
> originally reported issue.
>

IIUC the issue that sync worker could be orphaned and keep running
inside the long COPY is not fixed yet by commit
3c9bc2157a4f465b3c070d1250597568d2dc285f, and should be fixed. Am I
missing something?

Regards,

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


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


Re: [HACKERS] [POC] hash partitioning

2017-06-06 Thread Dilip Kumar
On Tue, Jun 6, 2017 at 1:03 PM, amul sul  wrote:
> May I ask you, how you sure about 8 is an unfit value for t1 relation?
> And what if the value other than 8, for e.g. 7?

Well, First I created t1 as a leaf relation like below, and I tested
insert into t1 with value 8 and it was violating the partition
constraint of t1, however, 7 was fine.

create table t (a int) partition by hash(a);
create table t1 partition of t for values with (modulus 2, remainder 1);

Later I dropped this t1 and created 2 level partition with the leaf as a range.

drop table t1;
create table t1 partition of t for values with (modulus 2, remainder
1) partition by range(a);
create table t1_1 partition of t1 for values from (8) to (10);

So now, I am sure that t1_1 can accept the value 8 and its parent t1 can't.

So I think this can only happen in the case of partitioned by hash
that a value is legal for the child but illegal for the parent?  Isn't
it a good idea that if a user is inserting in the top level relation
he should know for which partition exactly the constraint got
violated?

>
> Updated patch attached.

Thanks.


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


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


[HACKERS] Fix tab-completion of ALTER SUBSCRIPTION SET PUBLICATION

2017-06-06 Thread Masahiko Sawada
Hi,

Attached patch for $subject.

With this patch, ALTER SUBSCRIPTION  SET PUBLICATION  [TAB]
completes with "REFRESH" and "SKIP REFRESH".
Specifying either REFRESH or SKIP REFRESH is mandatory after ALTER
SUBSCRIPTION SET PUBLICATION, so i think it's good to add this.

Regards,

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


fix_ALTER_SUB_tab_completion.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] Use of non-restart-safe storage by temp_tablespaces

2017-06-06 Thread Alvaro Herrera
Andres Freund wrote:

> FWIW, allowing UNLOGGED tables, rather than just TEMPORARY ones,
> increases the complexity of that project noticeably.  For TEMPORARY you
> basically don't need to do much but to recreate the structure inside the
> tablespace at start - fairly simple.  But for UNLOGGED you need to find
> a way to recreate the relevant file and init forks - otherwise we might
> not notice what needs to be reset at a crash restart, and we might error
> out when executing selects etc. and then the table's not there.
> Presumably recreating files & init forks that at first table access is
> doable, but it's not entirely trivial to do locking wise.

I was thinking that you could create the init fork for each unlogged
table in a permanent tablespace (probably the default one for the
database).

FWIW I don't think calling these tablespaces "temporary" is the right
word.  It's not the tablespaces that are temporary.  Maybe "evanescent".
Also, do we really need it to be a keyword after CREATE?  We could put
the option in the WITH clause of CREATE TABLESPACE instead.

-- 
Á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] [POC] hash partitioning

2017-06-06 Thread amul sul
Hi Dilip,

Thanks for review.

On Sat, Jun 3, 2017 at 6:54 PM, Dilip Kumar  wrote:
> On Thu, May 25, 2017 at 9:59 AM, amul sul  wrote:
>> On Mon, May 22, 2017 at 2:23 PM, amul sul  wrote:
>>>
>>> Updated patch attached. Thanks a lot for review.
>>>
>> Minor fix in the document, PFA.
>
> Patch need rebase
>

Done.

> ---
> Function header is not consistent with other neighbouring functions
> (some function contains function name in the header but others don't)
> +/*
> + * Compute the hash value for given not null partition key values.
> + */
>
Done.

> --
> postgres=# create table t1 partition of t for values with (modulus 2,
> remainder 1) partition by range(a);
> CREATE TABLE
> postgres=# create table t1_1 partition of t1 for values from (8) to (10);
> CREATE TABLE
> postgres=# insert into t1 values(8);
> 2017-06-03 18:41:46.067 IST [5433] ERROR:  new row for relation "t1_1"
> violates partition constraint
> 2017-06-03 18:41:46.067 IST [5433] DETAIL:  Failing row contains (8).
> 2017-06-03 18:41:46.067 IST [5433] STATEMENT:  insert into t1 values(8);
> ERROR:  new row for relation "t1_1" violates partition constraint
> DETAIL:  Failing row contains (8).
>
> The value 8 is violating the partition constraint of the t1 and we are
> trying to insert to value in t1,
> still, the error is coming from the leaf level table t1_1, that may be
> fine but from error, it appears that
> it's violating the constraint of t1_1 whereas it's actually violating
> the constraint of t1.
>
> From Implementation, it appears that based on the key are identifying
> the leaf partition and it's only failing during ExecInsert while
> checking the partition constraint.
>
May I ask you, how you sure about 8 is an unfit value for t1 relation?
And what if the value other than 8, for e.g. 7?

Updated patch attached.

Regards,
Amul Sul


0001-Cleanup_v5.patch
Description: Binary data


0002-hash-partitioning_another_design-v13.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: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256

2017-06-06 Thread Michael Paquier
On Fri, Jun 2, 2017 at 10:08 AM, Stephen Frost  wrote:
> I don't have any issue with asking that Michael, or someone, to go look
> at other OpenSSL-using implementations which support channel binding.

I don't see the implementation of other TLS/SSL as a requirement for
channel binding with OpenSSL, and I have no plans to investigate that.
What is clearly a requirement though is to design a set of API generic
enough for the backend and the frontend to fetch the TLS unique
message, and what's needed for endpoint so as any implementation can
plug in easily with PG's channel binding code.

> What I find somewhat objectionable is the notion that if we don't have 5
> different TLS/SSL implementations supported in PG and that we've tested
> that channel binding works correctly among all combinations of all of
> them, then we can't accept a patch implementing it.  I'm exaggerating
> for effect here, of course, and you've agreed that a full, committable,
> implementation isn't necessary, and I agreed with that, but we still
> seem to have a different level of expectation regarding what needs doing
> here.  I don't know that we'll ever be able to nail down the exact
> location of the line in the sand that needs to be crossed here, or agree
> to it, so I'd suggest that we let them continue their efforts to work
> together and provide a patch which they feel is ready to be commented on
> by committers.  Perhaps we'll find that, regardless of where the line
> was drawn exactly, they've crossed it.

As far as I can see, there are a couple of things that I still need to
work on to make people happy:
- Rework the generic APIs for TLS finish and endpoint so as any
implementation can use channel binding without inducing any extra code
footprint to be-secure.c and fe-secure.c.
- Implement endpoint, as Alvaro is saying for JDBC that would be nicer.
- Have a couple of tests for channel binding to allow people to test
the feature easily. Those will be in src/test/ssl/. It would be nice
as well to be able to enforce the channel binding type on libpq-side,
which is useful at least for testing. So we are going to need an
environment variable for this purpose, and a connection parameter.
-- 
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] Default Partition for Range

2017-06-06 Thread Beena Emerson
Hello Dilip,

On Mon, Jun 5, 2017 at 8:44 PM, Dilip Kumar  wrote:
> On Mon, Jun 5, 2017 at 1:43 AM, Beena Emerson  wrote:
>> The new patch is rebased over default_partition_v18.patch
>> [http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg315831.html]
>
> I have done the initial review of the patch, I have few comments.
>
Thank you.
>
> + if ((lower->content[0] == RANGE_DATUM_DEFAULT))
> + {
> + if (partition_bound_has_default(partdesc->boundinfo))
> + {
> + overlap = true;
> + with = partdesc->boundinfo->default_index;
> + }
>
> I think we can change if ((lower->content[0] == RANGE_DATUM_DEFAULT))
> check to if (spec->is_default) that way it will be consistent with the
> check in the PARTITION_STRATEGY_LIST.  Or we can move this complete
> check outside of the switch.

I have now moved the is_default check for both list and range outside
the switch case.

>
> -
>
> - PartitionRangeDatum *datum = castNode(PartitionRangeDatum, lfirst(lc));
> + Node   *node = lfirst(lc);
> + PartitionRangeDatum *datum;
> +
> + datum = castNode(PartitionRangeDatum, node);
>
> Why do you need to change this?

I forgot to remove it.
It was needed for previous version of patch but no longer needed and
hence reverted this change.

>
> --
>
> + if (!datums)
> + bound->content[i] = RANGE_DATUM_DEFAULT;
> +
>
> Better to check if (datums != NULL) for non boolean types.

done.

>
> -
> + if (content1[i] == RANGE_DATUM_DEFAULT ||
> + content2[i] == RANGE_DATUM_DEFAULT)
> + {
> + if (content1[i] == content2[i])
> + return 0;
> + else if (content1[i] == RANGE_DATUM_DEFAULT)
> + return -1;
>
> I don't see any comments why default partition should be considered
> smallest in the index comparison. For negative infinity, it's pretty
> obvious by the enum name itself.

Default could be lowest or highest, no specific reason for putting it lowest.
I have not added any comments in this version.

--

Beena Emerson

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


default_range_partition_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


Re: [JDBC] [HACKERS] Channel binding support for SCRAM-SHA-256

2017-06-06 Thread Michael Paquier
On Fri, Jun 2, 2017 at 10:25 AM, Robert Haas  wrote:
> On Thu, Jun 1, 2017 at 9:13 PM, Michael Paquier
>  wrote:
>> It seems to me that any testing in this area won't fly high as long as
>> there is no way to enforce the list of TLS implementations that a
>> server allows. There have been discussions about being able to control
>> that after the OpenSSL vulnerabilities that were protocol-specific and
>> there were even patches adding GUCs for this purpose. At the end,
>> everything has been rejected as Postgres enforces the use of the
>> newest one when doing the SSL handshake.
>
> TLS implementations, or TLS versions?  What does the TLS version have
> to do with this issue?

I really mean *version* here. Unlike OpenSSL, the Windows TLS
implementation does not offer an API to choose the latest TLS version
available:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa380513(v=vs.85).aspx
It is up to the server and the client to negotiate that, so it seems
to me that we want some control in this area, which would be important
for testing as well because the TLS finish message differs a bit
across versions, in length mainly. On top of that per the aggressive
updates that Windows does from time to time they may as well forcibly
expose users to a broken TLS implementation...
MacOS has something similar to OpenSSL, with
SSLGetProtocolVersionMax(), which is nice.
-- 
Michael


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


<    1   2