Re: [HACKERS] Update comments in nodeModifyTable.c
On Mon, Jun 5, 2017 at 4:45 AM, Etsuro Fujitawrote: > 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()
On Tue, Jun 6, 2017 at 12:24 AM, Amit Langotewrote: > 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
On Sun, Jun 4, 2017 at 10:18 PM, Amit Langotewrote: > 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
On Jun 4, 29 Heisei, at 00:48, Bruce Momjianwrote: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
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
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
Tom Lane wrote: >> Andres Freundwrites: >>> 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)?
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 Momjianhttp://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
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)?
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
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
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 Momjianhttp://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
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: > On Fri, Jun 2, 2017 at 10:25 AM, Robert Haaswrote: > > 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)?
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
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.
On Mon, Jun 5, 2017 at 8:06 PM, Robert Haaswrote: > > 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?
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.
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 Haaswrote: > 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)?
On 6 June 2017 at 12:38, Ashutosh Bapatwrote: > 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
On Tue, Jun 6, 2017 at 2:41 PM, Amit Langotewrote: > 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
On 2017/06/06 17:50, Dilip Kumar wrote: > On Tue, Jun 6, 2017 at 1:03 PM, amul sulwrote: >> 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
On Sun, Jun 4, 2017 at 1:53 AM, Peter Eisentrautwrote: > 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
On Tue, Jun 6, 2017 at 1:03 PM, amul sulwrote: > 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
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
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
Hi Dilip, Thanks for review. On Sat, Jun 3, 2017 at 6:54 PM, Dilip Kumarwrote: > 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
On Fri, Jun 2, 2017 at 10:08 AM, Stephen Frostwrote: > 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
Hello Dilip, On Mon, Jun 5, 2017 at 8:44 PM, Dilip Kumarwrote: > 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
On Fri, Jun 2, 2017 at 10:25 AM, Robert Haaswrote: > 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