Re: [HACKERS] plpgsql: open for execute - add USING clause
2010/1/12 Takahiro Itagaki : > Hi, I'm reviewing OPEN FOR EXECUTE USING patch and have a couple of > trivial comments. > > Pavel Stehule wrote: > >> this small patch add missing USING clause to OPEN FOR EXECUTE statement >> + cleaning part of exec_stmt_open function > > - Can we use read_sql_expression2() instead of read_sql_construct() > in gram.y? It could simplify the code a bit. > > - I'd prefer to change the new argument for exec_dynquery_with_params() > from "char *portalname" to "const char *curname". > ok, I accept all comments. revised version are attached. Thank you, Pavel Stehule > Other than the above minor issues, this patch is ready to commit. > > Regards, > --- > Takahiro Itagaki > NTT Open Source Software Center > > > openexecusing-after-rev.diff 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] ECPG patch causes warning
Michael Meskes írta: > On Sun, Jan 10, 2010 at 07:16:59PM +0100, Boszormenyi Zoltan wrote: > >> As would ecpg_dynamic_type(), then. :-( >> > > My guess is that this function is fine when returning InvalidOid = 0. AFAICT > it > is supposed to fill an integer with the SQL3 type code which seems to start > with 1 too. So I will change this one to return 0. > > >> Perhaps InvalidOid wouldn't be the best choice to return, >> because this function returns int, not Oid. InvalidOid equals >> to ECPGt_char. Hm... it would be hiding the failure in >> > > No, ECPGt_char is set to 1. > You're right. >> a good way, as the type returned couldn't be mapped to >> any ECPGt_* type, and the value would be returned in >> a string anyway. We can use ECPGt_char for the unhandled case. >> > > The question is, do we want to catch the unhandled case or shall we assume a > string? Just tell me and I'll commit. > I think it's best to assume a string. ecpg_set_{compat,native}_sqlda() uses the defailt case in that meaning anyway. > Looking at the usage of sqlda_dynamic_type again we would run into this > situation even earlier as the return value then is stort in a short int > because > that's what the other sqlda deffinitions use too. Therefore we have to make > sure we do not cross the short max. I'm glad at least one compiler caught > this. > > Michael > > Thanks, Zoltán Böszörményi -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ -- 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] Streaming replication status
Stefan Kaltenbrunner wrote: so is there an actually concrete proposal of _what_ interals to expose? ' The pieces are coming together...summary: -Status quo: really bad, but could probably ship anyway because existing PITR is no better and people manage to use it -Add slave pg_current_xlog_location() and something like pg_standby_received_xlog_location(): Much better, gets rid of the worst issues here. -Also add pg_standbys_xlog_location() on the master: while they could live without it, this really helps out the "alert/monitor" script writer whose use cases keep popping up here. Details...the original idea from Fujii was: "I'm thinking something like pg_standbys_xlog_location() [on the primary] which returns one row per standby servers, showing pid of walsender, host name/ port number/user OID of the standby, the location where the standby has written/flushed WAL. DBA can measure the gap from the combination of pg_current_xlog_location() and pg_standbys_xlog_location() via one query on the primary." After some naming quibbles and questions about what direction that should happen in, Tom suggested the initial step here is: "It seems to me that we should have at least two functions available on the slave: latest xlog location received and synced to disk by walreceiver (ie, we are guaranteed to be able to replay up to here); and latest xlog location actually replayed (ie, the state visible to queries on the slave). The latter perhaps could be pg_current_xlog_location()." So there's the first two of them: on the slave, pg_current_xlog_location() giving the latest location replayed, and a new one named something like pg_standby_received_xlog_location(). If you take the position that an unreachable standby does provide answers to these questions too (you just won't like them), this pair might be sufficient to ship. To help a lot at dealing with all the error situations where the standby isn't reachable and segments are piling up (possibly leading to full disk), the next figure that seems to answer the most questions is asking the primary "what's the location of the last WAL segment file in the pile of ones to be archived/distributed that has been requested (or processed if that's the easier thing to note) by the standby?". That's what is named pg_standbys_xlog_location() in the first paragraph I quoted. If you know enough to identify that segment file on disk, you can always look at its timestamp (and the ones on the rest of the files in that directory) in a monitoring script to turn that information into segments or a time measurement instead--xlog segments are nicely ordered after all. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.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] Streaming replication and non-blocking I/O
Thanks for your advice! On Wed, Jan 13, 2010 at 3:37 AM, Magnus Hagander wrote: >> This change which moves walreceiver process into a dynamically loaded >> module caused the following compile error on my MinGW environment. > > That sounds strange - it should pick those up from the -lpostgres. Any > chance you have an old postgres binary around from a non-syncrep build > or something? No, there is no old postgres binary. > Do you have an environment to try to build it under msvc? No, unfortunately. > in my > experience, that gives you easier-to-understand error messages in a > lot of cases like this - it removets the mingw black magic. OK. I'll try to build it under msvc. But since there seems to be a long way to go before doing that, I would appreciate if someone could give me some advice. Regards, -- Fujii Masao 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] lock_timeout GUC patch
Jaime Casanova írta: > 2010/1/13 Boszormenyi Zoltan : > >> Tom Lane írta: >> >>> If this patch is touching those parts of relcache.c, it probably needs >>> rethinking. >>> >>> >> What I did there is to check the return value of LockRelationOid() >> > > the hunk was because a diference in the position (i guess patch accept > a hunk of reasonable size, assuming there is something like a > reasonable size for that) > > and is not touching the same as your refactor (sorry if i explain myself bad) > > >> and also elog(PANIC) if the lock wasn't available. >> Does it need rethinking? >> >> > > well, i actually think that PANIC is too high for this... > Well, it tries to lock and then open a critical system index. Failure to open it has PANIC, it seemed appropriate to use the same error level if the lock failure case. Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Deadlock in vacuum (check fails)
I found following strange error on gothic moth: == pgsql.22658/src/test/regress/regression.diffs === *** /zfs_data/home/postgres/buildfarm/gothic_moth/HEAD/pgsql.22658/src/test/regress/expected/vacuum.out Tue Jan 12 22:06:13 2010 --- /zfs_data/home/postgres/buildfarm/gothic_moth/HEAD/pgsql.22658/src/test/regress/results/vacuum.out Tue Jan 12 22:35:25 2010 *** *** 94,99 --- 94,103 -- only non-system tables should be changed VACUUM FULL pg_am; VACUUM FULL pg_class; + ERROR: deadlock detected + DETAIL: Process 5913 waits for AccessExclusiveLock on relation 2662 of database 16384; blocked by process 5915. + Process 5915 waits for AccessShareLock on relation 1259 of database 16384; blocked by process 5913. + HINT: See server log for query details. VACUUM FULL pg_database; VACUUM FULL vaccluster; VACUUM FULL vacid; Detail here: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=gothic_moth&dt=2010-01-12%2021:06:01 Zdenek -- 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] Streaming replication and non-blocking I/O
Fujii Masao wrote: > Done. Currently there is no new libpq function for replication. The > walreceiver uses only existing functions like PQconnectdb, PQexec, > PQgetCopyData, etc. > > git://git.postgresql.org/git/users/fujii/postgres.git > branch: replication Thanks! I'm afraid we haven't quite nailed the select/poll issue yet. You copied pq_wait() from the libpq pqSocketCheck(), but there's one big difference between the backend and the frontend: the frontend always puts the connection to non-blocking mode, while the backend uses blocking mode. At least with SSL, I think it's possible for pq_wait() to return false positives, if the SSL layer decides to renegotiate the connection causing data to flow in the other direction in the underlying TCP connection. A false positive would lead cause walsender to block indefinitely on the pq_getbyte() call. I don't even want to think about the changes required to put the backend socket to non-blocking mode, I don't know that code well enough. Maybe we could temporarily put it to non-blocking mode, read to see if there's any data available, and put it back to blocking mode. But even then I think we'd need to modify at least secure_read() to work correctly with SSL in non-blocking mode. Another idea is to use poll() to check for POLLHUP, on those platforms that have poll(). AFAICS there is no equivalent for that in select(), so for platforms that don't have poll() we would have to simply ignore the issue or write some other platform-specific work-around (Windows WSAEventSelect() seems to have a FD_CLOSE event for that). That would be a quite localized change. -- Heikki Linnakangas 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] Feature patch 1 for plperl [PATCH]
On Tue, Jan 12, 2010 at 07:06:59PM -0500, Robert Haas wrote: > On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas wrote: > >>> I would strongly suggest to Tim that he rip the portions of this patch > >>> that are related to this feature out and submit them separately so > >>> that we can commit the uncontroversial portions first. > >> > >> See my previous email. I suggested that Tim send three patches: one for > >> this > >> controversial stuff, one for the new utility functions for plperl, and one > >> for the remainder. He and I have discussed it and I believe he is agreeable > >> to that. > > > > OK, well then just +1 for that. > > I believe we have agreement on this course of action, so I'm going to > mark the current patch as Returned with Feedback. Hopefully Tim will > submit separate patches for each of these three areas in the next day > or two before start-of-CommitFest That's my plan. Plus, hopefully at least one more for inter-sp calling. > personally, I think they should > each get their own thread and their own entry in the CommitFest app, > for ease of tracking and reviewing. YMMV, of course. Yes, that was also my intent. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Incrementally Updated Backups and restartpoints
Our documentation suggests that you can take a base backup of a warm standby server while it's running: > If we take a backup of the standby server's data directory while it is > processing logs shipped from the primary, we will be able to reload that data > and restart the standby's recovery process from the last restart point. We no > longer need to keep WAL files from before the restart point. If we need to > recover, it will be faster to recover from the incrementally updated backup > than from the original base backup. That doesn't seem safe. If the server makes a new restartpoint while the backup is running, and pg_control is backed up after the new restartpoint is made, recovery will restart from the new restartpoint. That is wrong; recovery needs to restart at the restartpoint that was most recent when the backup started. This is basically the same issue we have solved in master with the backup_label file. I wonder if it would be enough to document that pg_control must be backed up first? -- Heikki Linnakangas 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] Incrementally Updated Backups and restartpoints
On Wed, Jan 13, 2010 at 8:36 PM, Heikki Linnakangas wrote: > Our documentation suggests that you can take a base backup of a warm > standby server while it's running: > >> If we take a backup of the standby server's data directory while it is >> processing logs shipped from the primary, we will be able to reload that >> data and restart the standby's recovery process from the last restart point. >> We no longer need to keep WAL files from before the restart point. If we >> need to recover, it will be faster to recover from the incrementally updated >> backup than from the original base backup. > > That doesn't seem safe. If the server makes a new restartpoint while the > backup is running, and pg_control is backed up after the new > restartpoint is made, recovery will restart from the new restartpoint. > That is wrong; recovery needs to restart at the restartpoint that was > most recent when the backup started. This is basically the same issue we > have solved in master with the backup_label file. Right. > I wonder if it would be enough to document that pg_control must be > backed up first? Probably No. The archive recovery from such base backup would always fail at the end of recovery because there is no backup-end record, i.e., pg_stop_backup() is not executed in that case. Regards, -- Fujii Masao 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] lock_timeout GUC patch
Jaime Casanova írta: > 2010/1/13 Boszormenyi Zoltan : > >> Tom Lane írta: >> >>> If this patch is touching those parts of relcache.c, it probably needs >>> rethinking. >>> >>> >> What I did there is to check the return value of LockRelationOid() >> > > the hunk was because a diference in the position (i guess patch accept > a hunk of reasonable size, assuming there is something like a > reasonable size for that) > Actually the reject was not because of the position difference, Tom's refactor changed one line in load_critical_index(): -ird = RelationBuildDesc(indexoid, NULL); +ird = RelationBuildDesc(indexoid, true); -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ -- 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] Incrementally Updated Backups and restartpoints
Fujii Masao wrote: > On Wed, Jan 13, 2010 at 8:36 PM, Heikki Linnakangas > wrote: >> I wonder if it would be enough to document that pg_control must be >> backed up first? > > Probably No. The archive recovery from such base backup would always > fail at the end of recovery because there is no backup-end record, > i.e., pg_stop_backup() is not executed in that case. No, that's not an issue. We only wait for the backup-end record if we haven't seen yet since we started recovery from the base backup. Assuming the standby had reached that point already before the new backup from the standby started, backupStartLoc is zero in the control file. -- Heikki Linnakangas 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] More frame options in window functions
2010/1/5 Hitoshi Harada : > 2009/12/31 Hitoshi Harada : >> Attached is the fix pointed out in the previous CommitFest plus RANGE >> offset support. > > Improved version attached. In this revision I fixed type mismatch case > like "ORDER BY int4_data RANGE BETWEEN int8_data PRECEDING ...". > > Update of comments and fix typos in documents are also included. Fix some trivial things and synced with HEAD. I've came up with using "upper" and "lower" instead of "start" and "end" for window frame bounds. The upper/lower is more beautiful since two have same length but start/end is used since it was introduced. Comments? Regards, -- Hitoshi Harada more_frame_options.20100113.patch.gz Description: GNU Zip compressed 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] patch to implement ECPG side tracing / tracking ...
Hans, nce Jaime already asked for a use case, just a few small comments from me. > @@ -4,6 +4,7 @@ > #include "postgres_fe.h" > > #include > +#include This is not portable. You don't want to include this header. Did I see this right that you use the statement cache for auto-prepared statements even if the statement is not auto prepared? Some statements are not profiled, how did you decide which one to do? There is no test case. Before looking into it in detail I think we should first figure out if this feature really has a benefit. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org VfL Borussia! Forca Barca! 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] ECPG patch causes warning
On Wed, Jan 13, 2010 at 09:22:28AM +0100, Boszormenyi Zoltan wrote: > I think it's best to assume a string. ecpg_set_{compat,native}_sqlda() > uses the defailt case in that meaning anyway. Okay, applied as you send it. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org VfL Borussia! Forca Barca! 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] Cancelling idle in transaction state
On Sun, 2010-01-03 at 11:55 +0100, Martijn van Oosterhout wrote: > On Fri, Jan 01, 2010 at 03:31:58PM -0500, Kris Jurka wrote: > > The JDBC driver does want "cancel if active" behavior. The JDBC API > > specifies Statement.cancel() where Statement is running one particular > > backend query. So it really does want to cancel just that one query. > > Already this is tough because of the asynchronous nature of the cancel > > protocol and the inability to say exactly what should be cancelled. > > I've looked in the JDBC documentation but I don't quickly see how they > expect this to work with transactions. What is being proposed seems to > me to be: > > If statement active: >put transaction in aborted state > If no statement active: >do nothing > > However, I see that the documentation wants to be able to abort a > *specific* statement, which is not being proposed here. Can that be > implemented on top of the current proposal? That would require Statement-level abort, which we don't have. -- Simon Riggs www.2ndQuadrant.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] Bloom index
README.bloom: contrib/bloom provides signature file based index. Authors: Teodor Sigaev (teo...@sigaev.ru) and Oleg Bartunov (o...@sai.msu.su) Notes: This is a *working* prototype, which can be finishing up to production in case of interest. Particularly, it misses wal support, because of common limitation of contrib modules. This index is useful if table has many attributes and queries can include their arbitary combinations. Traditional Btree index is faster than bloom index , but it'd require too many indexes to support all possible queries, while one need only one bloom index. Bloom index supports only equality comparison. Since it's a signature file, not a tree, it always should be readed fully, but sequentially, so search performance is constant and doesn't depends on a query. Implementation of Bloom filter (http://en.wikipedia.org/wiki/Bloom_filter) allows fast exclusion of non-candidate tuples. Since signature is a lossy representation of all indexed attributes, search results should be rechecked using heap information. User can specify signature length (in uint16, default is 5) and the number of bits, which can be setted, per attribute ( 1 < colN < 2048 ). For example: CREATE INDEX bloomidx ON tbloom(i1,i2,i3) WITH (length=5, col1=2, col2=2, col3=4); Here, we create bloom index with signature length 80 bits and attributes i1, i2 mapped to 2 bits, attribute i3 - to 4 bits. Todo: * add more opclasses * better configurability * add support of arrays with operations contains and intersection Example of usage: select (generate_series(1,1000)*random())::int as i1, (generate_series(1,1000)*random())::int as i2, (generate_series(1,1000)*random())::int as i3, (generate_series(1,1000)*random())::int as i4, (generate_series(1,1000)*random())::int as i5, (generate_series(1,1000)*random())::int as i6, (generate_series(1,1000)*random())::int as i7, (generate_series(1,1000)*random())::int as i8, (generate_series(1,1000)*random())::int as i9, (generate_series(1,1000)*random())::int as i10, (generate_series(1,1000)*random())::int as i11, (generate_series(1,1000)*random())::int as i12, (generate_series(1,1000)*random())::int as i13 into tbloom; create index bloomidx on tbloom using bloom(i1,i2,i3,i4,i5,i6,i7,i8,i9,i10,i11,i12); select pg_relation_size('bloomidx'); create index btree_idx on tbloom(i1,i2,i3,i4,i5,i6,i7,i8,i9,i10,i11,i12); select pg_relation_size('btree_idx'); =# explain analyze select * from tbloom where i2=20 and i10=15; QUERY PLAN - Bitmap Heap Scan on tbloom (cost=1.50..5.52 rows=1 width=52) (actual time=0.057..0.057 rows=0 loops=1) Recheck Cond: ((i2 = 20) AND (i10 = 15)) -> Bitmap Index Scan on bloomidx (cost=0.00..1.50 rows=1 width=0) (actual time=0.041..0.041 rows=9 loops=1) Index Cond: ((i2 = 20) AND (i10 = 15)) Total runtime: 0.081 ms (5 rows) Seqscan is slow. =# set enable_bitmapscan = off; =# set enable_indexscan = off; =# explain analyze select * from tbloom where i2=20 and i10=15; QUERY PLAN -- Seq Scan on tbloom (cost=0.00..25.00 rows=1 width=52) (actual time=0.162..0.162 rows=0 loops=1) Filter: ((i2 = 20) AND (i10 = 15)) Total runtime: 0.181 ms (3 rows) Btree index will be not used. =# drop index bloomidx; =# create index btree_idx on tbloom(i1,i2,i3,i4,i5,i6,i7,i8,i9,i10,i11,i12); =# explain analyze select * from tbloom where i2=20 and i10=15; QUERY PLAN -- Seq Scan on tbloom (cost=0.00..25.00 rows=1 width=52) (actual time=0.210..0.210 rows=0 loops=1) Filter: ((i2 = 20) AND (i10 = 15)) Total runtime: 0.250 ms (3 rows) -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ bloom-0.2.tar.gz Description: Unix tar archive -- 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] lock_timeout GUC patch
2010/1/13 Boszormenyi Zoltan : >> >> well, i actually think that PANIC is too high for this... >> > > Well, it tries to lock and then open a critical system index. > Failure to open it has PANIC, it seemed appropriate to use > the same error level if the lock failure case. > if you try to open a critical system index and it doesn't exist is clearly a signal of corruption, if you can't lock it it's just a concurrency issue... don't see why they both should have the same level of message -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] lock_timeout GUC patch
Boszormenyi Zoltan writes: > Tom Lane írta: >> If this patch is touching those parts of relcache.c, it probably needs >> rethinking. > What I did there is to check the return value of LockRelationOid() > and also elog(PANIC) if the lock wasn't available. > Does it need rethinking? Yes. What you have done is to change all the LockSomething primitives from return void to return bool and thereby require all call sites to check their results. This is a bad idea. There is no way that you can ensure that all third-party modules will make the same change, meaning that accepting this patch will certainly introduce nasty, hard to reproduce bugs. And what's the advantage? The callers are all going to throw errors anyway, so you might as well do that within the Lock function and avoid the system-wide API change. I think this is a big patch with a small patch struggling to get out. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bloom index
2010/1/13 Teodor Sigaev : > CREATE INDEX bloomidx ON tbloom(i1,i2,i3) > WITH (length=5, col1=2, col2=2, col3=4); > > Here, we create bloom index with signature length 80 bits and attributes > i1, i2 mapped to 2 bits, attribute i3 - to 4 bits. This is pretty darn slick. I haven't looked at the code yet, but the functionality sounds very cool, and I hope this is something we'll be able to have as part of PG in the future (though more than likely not for 8.5, I suspect). ...Robert -- 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] Bloom index
This is pretty darn slick. I haven't looked at the code yet, but the It's just a prototype and/or proof of concept functionality sounds very cool, and I hope this is something we'll be able to have as part of PG in the future (though more than likely not for 8.5, I suspect). Of course -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ -- 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] xml2 still essential for us
> (4) In conclusion, I hope that PostgreSQL will keep xml2 > or something similar even when the XSLT and speculative parsing > issues have been addressed. Given your interest in XML2, would you like to be come a maintainer of the module? --Josh Berkus -- 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] plpython3
On Tue, 2010-01-12 at 20:06 -0800, Josh Berkus wrote: > > So it seems to me that the threshold question for this patch is - do > > we think it's a good idea to maintain two implementations of PL/python > > in core? > > Not really, no. This is why we need PGAN ;-) > > If the new implementation is *better* that the existing PL/python, I > could see eventually replacing it. It wouldn't be the first time that a > rewrite exceeded the original tool. I think it is important to remember that the current version of PL/python is pretty weak compared to its counter parts (Specifically PL/Perl). If the new version, is adequately written to community standards and increases PL/Python's capabilities we need to seriously consider it. If we can address any issues with this module, let's commit it as Pl/pythonng3 or something. Anyway, I am +1 on reviewing this patch for viability. I would love to never touch plPerl for advanced procedures again. Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- 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] plpython3
On ons, 2010-01-13 at 09:47 -0800, Joshua D. Drake wrote: > I think it is important to remember that the current version of > PL/python is pretty weak compared to its counter parts (Specifically > PL/Perl). How so? -- 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] Hot standby documentation
On Thu, 2010-01-07 at 18:34 +0900, Fujii Masao wrote: > On Thu, Jan 7, 2010 at 6:09 PM, Joshua Tolley wrote: > > Having concluded I really need to start playing with hot standby, I started > > looking for documentation on the subject. I found what I was looking for; I > > also found this page[1], which, it seems, ought to mention hot standby. > > Comments? > > > > [1] http://developer.postgresql.org/pgdocs/postgres/high-availability.html > > +1 > > At least, it should be mentioned that the slave can answer > read-only queries in "Warm Standby Using Point-In-Time Recovery". > And so "Table 25-1" should be changed. OK, will add. -- Simon Riggs www.2ndQuadrant.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] plpython3
On Wed, 2010-01-13 at 19:53 +0200, Peter Eisentraut wrote: > On ons, 2010-01-13 at 09:47 -0800, Joshua D. Drake wrote: > > I think it is important to remember that the current version of > > PL/python is pretty weak compared to its counter parts (Specifically > > PL/Perl). > > How so? O.k. you may have just called me on an unintentional bluff. My knowledge of plpython is dated. I just tested some of the things (like in/out) and they appear to work at least on 8.4). My argument would be now, what is the benefit of the James Pye version over our version. James can you illustrate succinctly why we should be supporting a new version? If there is, I am still all for it, but I am a python bigot. Sincerely, Joshua D. Drake > > -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- 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] plpython3
> My argument would be now, what is the benefit of the James Pye version > over our version. James can you illustrate succinctly why we should be > supporting a new version? > > If there is, I am still all for it, but I am a python bigot. Yeah, it's just my viewpoint that we don't want 2 python procedural languages in core. One should be in core, and one should go on pgFoundry/PGAN. Which ever one is "better" by some clear definition should go in core. As I said, I have no opinion about whether Pye's PLpython should replace the existing because I'm in no position to judge. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump sort order for functions
I wrote: > Peter Eisentraut writes: >> On mån, 2010-01-11 at 12:54 -0500, Tom Lane wrote: >>> -- Name: binary_coercible(oid, oid); Type: FUNCTION; Schema: public; Owner: >>> postgres >> Um, that tag is the "name", and if you change that, the name in CREATE >> FUNCTION also changes. > So? Actually, we're talking at cross-purposes here. The tag I'm talking about is the one generated via format_function_signature, and the problem with what I had in mind is that it isn't done yet at the point where the sort runs. >> In the mean time, hacking it into the sort function itself as a special >> case works out fine, per attached patch. One might frown upon such an >> exception, but then again, function overloading is an exception to the >> one-name-per-object rule all over the place anyway. ;-) > No, that's a completely bogus solution, because it depends on type > OIDs. It won't be stable across dump/reload, which defeats the purpose > AFAICS. You could probably make it work more safely if you applied getFormattedTypeName() and then compared the string names. That would be rather expensive :-( but in most databases this should happen few enough times so it wouldn't be a problem. [ thinks for a bit ... ] Although getFormattedTypeName depends on the current search_path, so that might be a bit of an issue for stability as well. I guess we could force a standardized path, perhaps pg_catalog only, before sorting. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
On Mon, Jan 11, 2010 at 8:27 PM, Bruce Momjian wrote: > Robert Haas wrote: >> On Sun, Jan 10, 2010 at 4:54 PM, Tom Lane wrote: >> > Robert Haas writes: >> >> I have looked this over a little bit and I guess I don't see why the >> >> lack of a grand plan for how to organize all of our permissions checks >> >> ought to keep us from removing this one on the grounds of redundancy. >> >> We have to attack this problem in small pieces if we're going to make >> >> any progress, and the pieces aren't going to get any smaller than >> >> this. >> > >> > I would turn that argument around: given the lack of a grand plan, >> > why should we remove this particular check at all? Nobody has argued >> > that there would be a significant, or even measurable, performance gain. >> > When and if we do have a plan, we might find ourselves putting this >> > check back. >> >> You're arguing against a straw man - there's clearly no argument here >> from performance. We generally do not choose to litter the code with >> redundant or irrelevant checks because it makes the code difficult to >> maintain and understand. Sometimes it also hurts performance, but >> that's not a necessary criterion for removal. Nor are we generally in >> the habit of keeping redundant code around because a hypothetical >> future refactoring might by chance end up putting exactly the same >> code back. > > I agree. Why are arbitrary restrictions being placed on code > improvements? If code has no purpose, why not remove it, or at least > mark it as NOT_USED. So, where do we go from here? Any other opinions? I'm not sure how much it's really worth fighting over a six line patch, but there's something in me that rails against the idea of telling someone who took the trouble to write a patch "no" when the only argument against it is that we might change our mind at some point in the future. Of course, I will accept the consensus of the community whatever it is, but the only people who have expressed a clear opinion on this version of the patch are Tom, Bruce, and myself, and 2-1 is not a consensus. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Hot Standby and query cancel
We've been chewing around query cancel on Hot Standby and I think things have got fairly confusing, hence a new thread. I enclose a patch that includes all the things that we all agree on so far, in my understanding * Recovery conflict processing uses SIGUSR1 rather than shmem per Tom, while holding ProcArrayLock per Andres * CONFLICT_MODE_ERROR throws ERROR when in a transaction, not idle and not in subtransaction, otherwise becomes CONFLICT_MODE_FATAL per Tom and other discussion * Recovery abort message has additional detail, per Heikki It doesn't include anything still under discussion, though is intended as a base upon which further patches can progress independently. * Infrastructure for supercancel, by Joachim Wieland * Any of the many further ideas by Andres Freund Please review this so we can move onto taking other issues one by one. This is also a base for other HS work I need to complete. I am still testing patch, so should be confident to commit tomorrow barring issues. -- Simon Riggs www.2ndQuadrant.com *** a/src/backend/access/transam/xact.c --- b/src/backend/access/transam/xact.c *** *** 313,320 IsTransactionState(void) /* * IsAbortedTransactionBlockState * ! * This returns true if we are currently running a query ! * within an aborted transaction block. */ bool IsAbortedTransactionBlockState(void) --- 313,319 /* * IsAbortedTransactionBlockState * ! * This returns true if we are within an aborted transaction block. */ bool IsAbortedTransactionBlockState(void) *** a/src/backend/storage/ipc/procarray.c --- b/src/backend/storage/ipc/procarray.c *** *** 324,329 ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) --- 324,330 /* must be cleared with xid/xmin: */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->inCommit = false; /* be sure this is cleared in abort */ + proc->recoveryConflictPending = false; /* Clear the subtransaction-XID cache too while holding the lock */ proc->subxids.nxids = 0; *** *** 350,355 ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) --- 351,357 /* must be cleared with xid/xmin: */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->inCommit = false; /* be sure this is cleared in abort */ + proc->recoveryConflictPending = false; Assert(proc->subxids.nxids == 0); Assert(proc->subxids.overflowed == false); *** *** 377,383 ProcArrayClearTransaction(PGPROC *proc) proc->xid = InvalidTransactionId; proc->lxid = InvalidLocalTransactionId; proc->xmin = InvalidTransactionId; ! proc->recoveryConflictMode = 0; /* redundant, but just in case */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; --- 379,385 proc->xid = InvalidTransactionId; proc->lxid = InvalidLocalTransactionId; proc->xmin = InvalidTransactionId; ! proc->recoveryConflictPending = false; /* redundant, but just in case */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; *** *** 1665,1671 GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid, if (proc->pid == 0) continue; ! if (skipExistingConflicts && proc->recoveryConflictMode > 0) continue; if (!OidIsValid(dbOid) || --- 1667,1673 if (proc->pid == 0) continue; ! if (skipExistingConflicts && proc->recoveryConflictPending) continue; if (!OidIsValid(dbOid) || *** *** 1704,1710 GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid, * Returns pid of the process signaled, or 0 if not found. */ pid_t ! CancelVirtualTransaction(VirtualTransactionId vxid, int cancel_mode) { ProcArrayStruct *arrayP = procArray; int index; --- 1706,1712 * Returns pid of the process signaled, or 0 if not found. */ pid_t ! CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode) { ProcArrayStruct *arrayP = procArray; int index; *** *** 1722,1749 CancelVirtualTransaction(VirtualTransactionId vxid, int cancel_mode) if (procvxid.backendId == vxid.backendId && procvxid.localTransactionId == vxid.localTransactionId) { ! /* ! * Issue orders for the proc to read next time it receives SIGINT ! */ ! if (proc->recoveryConflictMode < cancel_mode) ! proc->recoveryConflictMode = cancel_mode; ! pid = proc->pid; break; } } LWLockRelease(ProcArrayLock); - if (pid != 0) - { - /* - * Kill the pid if it's still here. If not, that's what we wanted - * so ignore any errors. - */ - kill(pid, SIGINT); - } - return pid; } --- 1724,1745 if (procvxid.backendId == vxid.backendId && procvxid.localTransactionId == vxid.localTransactionId) { ! proc->recoveryConflictPending = true; pid = proc->pid; + if (pid != 0) + { + /* + * Kill the pid if it's still here. If not, that's wh
Re: [HACKERS] [PATCH] remove redundant ownership checks
Robert Haas writes: > On Mon, Jan 11, 2010 at 8:27 PM, Bruce Momjian wrote: >> I agree. Why are arbitrary restrictions being placed on code >> improvements? If code has no purpose, why not remove it, or at least >> mark it as NOT_USED. > So, where do we go from here? Any other opinions? I'm not sure how > much it's really worth fighting over a six line patch, but there's > something in me that rails against the idea of telling someone who > took the trouble to write a patch "no" when the only argument against > it is that we might change our mind at some point in the future. Of > course, I will accept the consensus of the community whatever it is, > but the only people who have expressed a clear opinion on this version > of the patch are Tom, Bruce, and myself, and 2-1 is not a consensus. I still say that this patch is putting the cart before the horse. What we need before we do any significant amount of rearrangement of security checks is to have a plan for where they should go. Making a change here and a change there without a plan isn't an improvement, it just risks creating bugs. If I thought this patch represented incremental movement in the direction of a better security-check factorization, I'd be fine with it, but that's not clear either. The argument for it is that these checks are redundant with some other ones, but why should we remove these and not the other ones instead? And there would still be still some checks left in this file, so it doesn't seem to me that we'd have actually made any progress towards clarity of where the checks should be. Plan first, then code, please. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
Robert Haas wrote: > So, where do we go from here? Any other opinions? It seems that we often have people cleaning up redundant or unreachable code to improve code clarity. I'm not in a position right now to confirm that this code is redundant, but if that has been firmly established, I haven't heard any good reason for not fixing it. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] NEED HELP !
Hello all, I am a student-magister and I'm writting my magister work. I realized gist index TPR tree - it is like a simple R tree but moving :) Everything is compiling ok, I creating table and index, but after whese code - DB is restarting :((( * * *set enable_seqscan = false select * from table_of_moving_objects where mov_obj ~ box '(0,0) (1,1)'* I can not understand why DB is restarting ? It is restarting when SQL begins use index :((( Can anybody help me , I would by appretiate every people who downloads my sources, try to deploy index and maybe solve where is the problem. Best regards, Sergej Galkin moving_object.rar 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] [PATCH] remove redundant ownership checks
On Wed, Jan 13, 2010 at 1:15 PM, Robert Haas wrote: > > So, where do we go from here? Any other opinions? if it's not broken, then it doesn't need a fix... if that code is causing a hole in security then i said remove it, if not... what's the problem in let it be until we know exactly what the plan is? -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- 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] NEED HELP !
I want to add than I have a piece of my code that looks very strange Datum gist_mov_penalty(PG_FUNCTION_ARGS) { GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0); GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(1); float * result = (float *) PG_GETARG_POINTER(2); * moving_object *orig = &(*origentry).key; make_now(orig); moving_object *new = &(*origentry).key; make_now(new);* Is everything ok in this code ?? On Wed, Jan 13, 2010 at 8:42 PM, Sergej Galkin wrote: > Hello all, > > I am a student-magister and I'm writting my magister work. I realized gist > index TPR tree - it is like a simple R tree but moving :) > Everything is compiling ok, I creating table and index, but after whese > code - DB is restarting :((( > * > * > *set enable_seqscan = false > select * from table_of_moving_objects where mov_obj ~ box '(0,0) (1,1)'* > > I can not understand why DB is restarting ? It is restarting when SQL > begins use index :((( > Can anybody help me , I would by appretiate every people who downloads my > sources, try to deploy index and maybe solve where is the problem. > > Best regards, > Sergej Galkin >
Re: [HACKERS] Hot Standby and query cancel
Hi Simon, On Wednesday 13 January 2010 19:24:22 Simon Riggs wrote: > We've been chewing around query cancel on Hot Standby and I think things > have got fairly confusing, hence a new thread. Good idea. > I enclose a patch that includes all the things that we all agree on so > far, in my understanding cool. > * Recovery conflict processing uses SIGUSR1 rather than shmem per Tom, > while holding ProcArrayLock per Andres > > * CONFLICT_MODE_ERROR throws ERROR when in a transaction, not idle and > not in subtransaction, otherwise becomes CONFLICT_MODE_FATAL per Tom and > other discussion > > * Recovery abort message has additional detail, per Heikki > > It doesn't include anything still under discussion, though is intended > as a base upon which further patches can progress independently. > I am still testing patch, so should be confident to commit tomorrow > barring issues. I have only looked at briefly because right now I dont have the time (going to eat at a friends place...) but I think I spotted an issue: The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt is not correct right now because that returns true for TBLOCK_SUBABORT as well. Wouldnt that mess with the case where were in a failed subxact and then rollback only that subxact? Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PgEast CFP (second call)
January 13, 2010 PostgreSQL Conference East, The PostgreSQL Conference for Decision Makers, End Users and Developers, is being held at the Radisson Plaza, Warwick Hotel in Philadelphia on March 25th through 28th. This is the second call for papers for this conference. You can review the skeletal agenda here: http://www.postgresqlconference.org/2010/east/agenda You can review content from the recent West conference here: http://www.postgresqlconference.org/2009/west/ You can submit your talk here: http://www.postgresqlconference.org/talksubmission Time line: December 14th: Talk submission opens January 30th: Talk submission closes February 15th: Speaker notification This year we will be continuing our trend of covering the entire PostgreSQL ecosystem. We would like to see talks and tutorials on the following topics: * General PostgreSQL: * Administration * Performance * High Availability * Migration * GIS * Integration * Solutions and White Papers * The Stack: * Python/Django/Pylons/TurboGears/Custom * Perl5/Catalyst/Bricolage * Ruby/Rails * Java (PLJava would be great)/Groovy/Grails * Operating System optimization (Linux/FBSD/Solaris/Windows) * Solutions and White Papers Submit Session: http://www.postgresqlconference.org/talksubmission Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering -- 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] TPR-tree crash WAS: NEED HELP !
Sergej, > I can not understand why DB is restarting ? It is restarting when SQL > begins use index :((( > Can anybody help me , I would by appretiate every people who downloads > my sources, try to deploy index and maybe solve where is the problem. It would help if you gave people a link. Also, if you are Russian, there is a siginficant community of PostgreSQL russian-speakers who might be able to help you. --Josh Berkus -- 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] lock_timeout GUC patch
Tom Lane írta: > Boszormenyi Zoltan writes: > >> Tom Lane írta: >> >>> If this patch is touching those parts of relcache.c, it probably needs >>> rethinking. >>> > > >> What I did there is to check the return value of LockRelationOid() >> and also elog(PANIC) if the lock wasn't available. >> Does it need rethinking? >> > > Yes. What you have done is to change all the LockSomething primitives > from return void to return bool and thereby require all call sites to > check their results. This is a bad idea. Okay, can you tell me how can I get the relation name out of the xid in XactLockTableWait()? There are several call site of this function, and your idea about putting the error code into the LockSomething() functions to preserve the API results strange error messages, like ERROR: could not obtain lock on transaction with ID 658 when I want to UPDATE a tuple in a session when this and another session have a FOR SHARE lock on said tuple. > There is no way that you can > ensure that all third-party modules will make the same change, meaning > that accepting this patch will certainly introduce nasty, hard to > reproduce bugs. And what's the advantage? The callers are all going > to throw errors anyway, so you might as well do that within the Lock > function and avoid the system-wide API change. > > I think this is a big patch with a small patch struggling to get out. > Your smaller patch is attached, with the above strangeness. :-) Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ diff -dcrpN pgsql.orig/doc/src/sgml/config.sgml pgsql.5/doc/src/sgml/config.sgml *** pgsql.orig/doc/src/sgml/config.sgml 2010-01-06 08:43:22.0 +0100 --- pgsql.5/doc/src/sgml/config.sgml 2010-01-13 18:59:19.0 +0100 *** COPY postgres_log FROM '/full/path/to/lo *** 4191,4196 --- 4191,4220 + + lock_timeout (integer) + +lock_timeout configuration parameter + + + + Abort any statement that tries to acquire a heavy-weight lock (e.g. rows, + pages, tables, indices or other objects) and the lock has to wait more + than the specified number of milliseconds, starting from the time the + command arrives at the server from the client. + If log_min_error_statement is set to ERROR or lower, + the statement that timed out will also be logged. A value of zero + (the default) turns off the limitation. + + + + Setting lock_timeout in + postgresql.conf is not recommended because it + affects all sessions. + + + + vacuum_freeze_table_age (integer) diff -dcrpN pgsql.orig/doc/src/sgml/ref/lock.sgml pgsql.5/doc/src/sgml/ref/lock.sgml *** pgsql.orig/doc/src/sgml/ref/lock.sgml 2009-09-18 08:26:40.0 +0200 --- pgsql.5/doc/src/sgml/ref/lock.sgml 2010-01-13 18:59:19.0 +0100 *** LOCK [ TABLE ] [ ONLY ] NOWAIT is specified, LOCK TABLE does not wait to acquire the desired lock: if it cannot be acquired immediately, the command is aborted and an !error is emitted. Once obtained, the lock is held for the !remainder of the current transaction. (There is no UNLOCK TABLE command; locks are always released at transaction end.) --- 39,49 NOWAIT is specified, LOCK TABLE does not wait to acquire the desired lock: if it cannot be acquired immediately, the command is aborted and an !error is emitted. If lock_timeout is set to a value !higher than 0, and the lock cannot be acquired under the specified !timeout value in milliseconds, the command is aborted and an error !is emitted. Once obtained, the lock is held for the remainder of !the current transaction. (There is no UNLOCK TABLE command; locks are always released at transaction end.) diff -dcrpN pgsql.orig/doc/src/sgml/ref/select.sgml pgsql.5/doc/src/sgml/ref/select.sgml *** pgsql.orig/doc/src/sgml/ref/select.sgml 2009-10-29 15:23:52.0 +0100 --- pgsql.5/doc/src/sgml/ref/select.sgml 2010-01-13 18:59:19.0 +0100 *** FOR SHARE [ OF semNum; + + /* + * Note: if errStatus is -1 and errno == EINTR then it means we returned + * from the operation prematurely because we were sent a signal. So we + * try and lock the semaphore again. + * + * Each time around the loop, we check for a cancel/die interrupt. On + * some platforms, if such an interrupt comes in while we are waiting, it + * will cause the semop() call to e
Re: [HACKERS] plpython3
On Wed, Jan 13, 2010 at 1:16 PM, Josh Berkus wrote: > >> My argument would be now, what is the benefit of the James Pye version >> over our version. James can you illustrate succinctly why we should be >> supporting a new version? >> >> If there is, I am still all for it, but I am a python bigot. > > Yeah, it's just my viewpoint that we don't want 2 python procedural > languages in core. One should be in core, and one should go on > pgFoundry/PGAN. Which ever one is "better" by some clear definition > should go in core. That was my thinking also. There are a couple of reasons to think that throwing over the current implementation for an new one may not be the right thing to do. 1. It's not just a rewrite, it's an incompatible rewrite that will present significant user-visible behavioral differences. So replacing the current implementation wholesale would produce massive breakage for anyone actually using PL/python in production. 2. Peter Eisentraut, who has put significant time into improving PL/python for this release (see the commit logs), and who is the ONLY committer to work on improving PL/python for this release, has made it clear that he prefers the current implementation. Given these two facts, it's hard for me to see how we could decide to REMOVE the current implementation and replace it with the new one. So the most we could do is maintain them side by side, and then you have to ask, why? It will actually be much easier for James Pye to maintain his code outside core, and if it turns out to be popular and people come back to us and say "hey, why isn't this the default?" then we can revisit the issue. Sure, his code won't get as much exposure that way, but it's been posted to the mailing list several times now over a period of 8 months and nobody has said "oh, wow, this is great". The most we've gotten is several versions of this exchange: Somebody: We should really consider this code, the current code isn't very good. Peter: What's wrong with it? Somebody: Peter: Why can't we fix that in the existing code base? I'm not saying a complete rewrite isn't the way to go - I'm just saying that there isn't much evidence at this point that it's really necessary, and I don't think we want to maintain two copies of PL/python unless we're really sure that the new implementation is an improvement. ...Robert -- 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] NEED HELP !
On Wed, Jan 13, 2010 at 1:42 PM, Sergej Galkin wrote: > I can not understand why DB is restarting ? It is restarting when SQL begins > use index :((( > Can anybody help me , I would by appretiate every people who downloads my > sources, try to deploy index and maybe solve where is the problem. Your code is probably crashing. Before trying to use the index, run "select pg_backend_pid()" in the backend that you're going to try to use it from. Then use "gdb -p " to attach gdb to the backend. Type "c" for continue. Now run the query that crashes it. gdb will trap the signal and you can use "bt" to get a backtrace. I don't think it would be appropriate for us to help you debug your class project, but learning to use gdb should help you a lot. Also see: http://wiki.postgresql.org/wiki/Guide_to_reporting_problems ...Robert -- 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] Hot Standby and query cancel
On Wed, 2010-01-13 at 19:58 +0100, Andres Freund wrote: > > I am still testing patch, so should be confident to commit tomorrow > > barring issues. > I have only looked at briefly because right now I dont have the time (going > to > eat at a friends place...) but I think I spotted an issue: > The IsAbortedTransactionBlockState() check in RecoveryConflictInterrupt is > not > correct right now because that returns true for TBLOCK_SUBABORT as well. > Wouldnt that mess with the case where were in a failed subxact and then > rollback only that subxact? Well spotted, yes. -- Simon Riggs www.2ndQuadrant.com *** a/src/backend/access/transam/xact.c --- b/src/backend/access/transam/xact.c *** *** 313,320 IsTransactionState(void) /* * IsAbortedTransactionBlockState * ! * This returns true if we are currently running a query ! * within an aborted transaction block. */ bool IsAbortedTransactionBlockState(void) --- 313,319 /* * IsAbortedTransactionBlockState * ! * This returns true if we are within an aborted transaction block. */ bool IsAbortedTransactionBlockState(void) *** a/src/backend/storage/ipc/procarray.c --- b/src/backend/storage/ipc/procarray.c *** *** 324,329 ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) --- 324,330 /* must be cleared with xid/xmin: */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->inCommit = false; /* be sure this is cleared in abort */ + proc->recoveryConflictPending = false; /* Clear the subtransaction-XID cache too while holding the lock */ proc->subxids.nxids = 0; *** *** 350,355 ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) --- 351,357 /* must be cleared with xid/xmin: */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->inCommit = false; /* be sure this is cleared in abort */ + proc->recoveryConflictPending = false; Assert(proc->subxids.nxids == 0); Assert(proc->subxids.overflowed == false); *** *** 377,383 ProcArrayClearTransaction(PGPROC *proc) proc->xid = InvalidTransactionId; proc->lxid = InvalidLocalTransactionId; proc->xmin = InvalidTransactionId; ! proc->recoveryConflictMode = 0; /* redundant, but just in case */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; --- 379,385 proc->xid = InvalidTransactionId; proc->lxid = InvalidLocalTransactionId; proc->xmin = InvalidTransactionId; ! proc->recoveryConflictPending = false; /* redundant, but just in case */ proc->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; *** *** 1665,1671 GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid, if (proc->pid == 0) continue; ! if (skipExistingConflicts && proc->recoveryConflictMode > 0) continue; if (!OidIsValid(dbOid) || --- 1667,1673 if (proc->pid == 0) continue; ! if (skipExistingConflicts && proc->recoveryConflictPending) continue; if (!OidIsValid(dbOid) || *** *** 1704,1710 GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid, * Returns pid of the process signaled, or 0 if not found. */ pid_t ! CancelVirtualTransaction(VirtualTransactionId vxid, int cancel_mode) { ProcArrayStruct *arrayP = procArray; int index; --- 1706,1712 * Returns pid of the process signaled, or 0 if not found. */ pid_t ! CancelVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode) { ProcArrayStruct *arrayP = procArray; int index; *** *** 1722,1749 CancelVirtualTransaction(VirtualTransactionId vxid, int cancel_mode) if (procvxid.backendId == vxid.backendId && procvxid.localTransactionId == vxid.localTransactionId) { ! /* ! * Issue orders for the proc to read next time it receives SIGINT ! */ ! if (proc->recoveryConflictMode < cancel_mode) ! proc->recoveryConflictMode = cancel_mode; ! pid = proc->pid; break; } } LWLockRelease(ProcArrayLock); - if (pid != 0) - { - /* - * Kill the pid if it's still here. If not, that's what we wanted - * so ignore any errors. - */ - kill(pid, SIGINT); - } - return pid; } --- 1724,1745 if (procvxid.backendId == vxid.backendId && procvxid.localTransactionId == vxid.localTransactionId) { ! proc->recoveryConflictPending = true; pid = proc->pid; + if (pid != 0) + { + /* + * Kill the pid if it's still here. If not, that's what we wanted + * so ignore any errors. + */ + (void) SendProcSignal(pid, sigmode, vxid.backendId); + } break; } } LWLockRelease(ProcArrayLock); return pid; } *** *** 1834,1839 CancelDBBackends(Oid databaseid) --- 1830,1836 { ProcArrayStruct *arrayP = procArray; int index; + pid_t pid = 0; /* tell all backends to die */ LWLockAcquire(Pr
Re: [HACKERS] [PATCH] remove redundant ownership checks
* Jaime Casanova (jcasa...@systemguards.com.ec) wrote: > if it's not broken, then it doesn't need a fix... > if that code is causing a hole in security then i said remove it, if > not... what's the problem in let it be until we know exactly what the > plan is? The chances of getting concensus on an overarching big plan are slim to none, which essentially means it's not going to get changed. I've suggested an approach and had no feedback on it. What's probably needed, to get attention anyway, is a patch which starts to move us in the right direction. That's going to be more than a 6-line patch, but doesn't have to be the whole thing either. For my 2c, I think a patch which attacks 'AT_PrepCmd' and rips out all of the owner checks from it and moves them to appropriate places (that is to say, per my proposal, very shortly before the 'work' is actually done, which is also often where the *other* permission checks are done, for those pieces which require more than just a simple owner check) would probably be the first step. Of course, the code between AT_PrepCmd and where the permissions checks are moved to would need to be reviewed and vetted to make sure there isn't anything being done that shouldn't be done without permission, but, honestly, I don't recall seeing much of that. We're actually pretty good about having a "gather info" stage followed by a "implement change" stage. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Deadlock in vacuum (check fails)
Zdenek Kotala writes: > I found following strange error on gothic moth: >VACUUM FULL pg_class; > + ERROR: deadlock detected > + DETAIL: Process 5913 waits for AccessExclusiveLock on relation 2662 > of database 16384; blocked by process 5915. > + Process 5915 waits for AccessShareLock on relation 1259 of database > 16384; blocked by process 5913. > + HINT: See server log for query details. The server log shows that 5913 was trying to VACUUM FULL pg_class, and 5915 was in the midst of backend startup. I believe that what is happening is that 5915 was doing a full startup without relcache init file (which had likely been deleted by a previous vacuum) and it was in the middle of load_critical_index() for index 2662 = pg_class_oid_index. It would have needed to read pg_class for that. Meanwhile the VACUUM FULL had ex-lock on pg_class and needed to lock its indexes. So basically what this boils down to is that load_critical_index is locking an index before locking its underlying relation, which generally speaking is against our coding rules. The relcache code has been like that since 8.2, so I'm a bit surprised that we have not seen this reported before. It could only happen when the relcache init file is missing, which isn't the normal state, but it's not exactly unusual either. Probably the appropriate fix is to make load_critical_index get AccessShare lock on the underlying catalog not only the index it's after. This would slow things down a tad, but since it's not in the normal startup path I don't think that matters. Should we back-patch this? The bug appears to be real back to 8.2, but if we've not noticed before, I'm not sure how important it is. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] NEED HELP !
I debugged index with gdb and found that it is segmentation fault in my procedure named *gist_mov_consistent* it is only 7 lines. So I think it is worth to publish it to public, * * *Datum gist_mov_consistent(PG_FUNCTION_ARGS)* *{* *GISTENTRY *entry = (GISTENTRY *)PG_GETARG_POINTER(0);* *BOX *query = PG_GETARG_BOX_P(1);* *StrategyNumber strategy = (StrategyNumber)PG_GETARG_UINT16(2);* ** *if (DatumGetMovP(entry->key) == NULL || query == NULL)* *PG_RETURN_BOOL(FALSE);* ** *PG_RETURN_BOOL(obj_contains(DatumGetMovP(entry->key), query));* *}* Do you have any ideas where is the problem ? :) Best regards, Sergej Galkin On Wed, Jan 13, 2010 at 8:42 PM, Sergej Galkin wrote: > Hello all, > > I am a student-magister and I'm writting my magister work. I realized gist > index TPR tree - it is like a simple R tree but moving :) > Everything is compiling ok, I creating table and index, but after whese > code - DB is restarting :((( > * > * > *set enable_seqscan = false > select * from table_of_moving_objects where mov_obj ~ box '(0,0) (1,1)'* > > I can not understand why DB is restarting ? It is restarting when SQL > begins use index :((( > Can anybody help me , I would by appretiate every people who downloads my > sources, try to deploy index and maybe solve where is the problem. > > Best regards, > Sergej Galkin >
[HACKERS] Add utility functions to plperl [PATCH]
This is the first of the patches to be split out from the former 'plperl feature patch 1'. Changes in this patch: - Added utility functions: quote_literal, quote_nullable, quote_ident, encode_bytea, decode_bytea, looks_like_number, encode_array_literal, encode_array_constructor. - Stored procedure subs are now given names ($name__$oid). This is invisible to PL/Perl stored procedures but makes tools like Devel::NYTProf and Devel::Cover _much_ more useful. - Warnings no longer have an extra newline in the NOTICE text. - Corresponding documentation changes in plperl.sgml, plus: Removed some trailing whitespace. Made some examples use more idiomatic perl. Added the missing ', arguments' to docs of spi_exec_prepared(). - Assorted minor changes Various minor optimizations like pre-growing data structures. Makes proper use of the recently updated ppport.h. Tim. diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 37114bd..94db722 100644 *** a/doc/src/sgml/plperl.sgml --- b/doc/src/sgml/plperl.sgml *** *** 13,19 PL/Perl is a loadable procedural language that enables you to write !PostgreSQL functions in the http://www.perl.org";>Perl programming language. --- 13,19 PL/Perl is a loadable procedural language that enables you to write !PostgreSQL functions in the http://www.perl.org";>Perl programming language. *** $$ LANGUAGE plperl; *** 101,107 linkend="sql-syntax-dollar-quoting">) for the string constant. If you choose to use escape string syntax E'', you must double the single quote marks (') and backslashes !(\) used in the body of the function (see ). --- 101,107 linkend="sql-syntax-dollar-quoting">) for the string constant. If you choose to use escape string syntax E'', you must double the single quote marks (') and backslashes !(\) used in the body of the function (see ). *** $$ LANGUAGE plperl; *** 141,153 CREATE FUNCTION perl_max (integer, integer) RETURNS integer AS $$ ! my ($x,$y) = @_; ! if (! defined $x) { ! if (! defined $y) { return undef; } return $y; } ! if (! defined $y) { return $x; } ! if ($x > $y) { return $x; } return $y; $$ LANGUAGE plperl; --- 141,153 CREATE FUNCTION perl_max (integer, integer) RETURNS integer AS $$ ! my ($x, $y) = @_; ! if (not defined $x) { ! return undef if not defined $y; return $y; } ! return $x if not defined $y; ! return $x if $x > $y; return $y; $$ LANGUAGE plperl; *** $$ LANGUAGE plperl; *** 158,189 Anything in a function argument that is not a reference is !a string, which is in the standard PostgreSQL external text representation for the relevant data type. In the case of ordinary numeric or text types, Perl will just do the right thing and the programmer will normally not have to worry about it. However, in !other cases the argument will need to be converted into a form that is !more usable in Perl. For example, here is how to convert an argument of !type bytea into unescaped binary !data: ! ! ! my $arg = shift; ! $arg =~ s!\\(?:\\|(\d{3}))!$1 ? chr(oct($1)) : "\\"!ge; ! ! !Similarly, values passed back to PostgreSQL !must be in the external text representation format. For example, here !is how to escape binary data for a return value of type bytea: ! ! ! $retval =~ s!(\\|[^ -~])!sprintf("\\%03o",ord($1))!ge; ! return $retval; ! ! --- 158,178 Anything in a function argument that is not a reference is !a string, which is in the standard PostgreSQL external text representation for the relevant data type. In the case of ordinary numeric or text types, Perl will just do the right thing and the programmer will normally not have to worry about it. However, in !other cases the argument will need to be converted into a form that is !more usable in Perl. For example, the decode_bytea !function can be used to convert an argument of !type bytea into unescaped binary. !Similarly, values passed back to PostgreSQL !must be in the external text representation format. For example, the !encode_bytea function can be used to !to escape binary data for a return value of type bytea. *** BEGIN { strict->import(); } *** 322,328 ! Database Access from PL/Perl --- 311,320 ! ! Built-in Functions ! ! Database Access from PL/Perl *** BEGIN { strict->import(); } *** 340,346 spi_query(command) spi_fetchrow(cursor) spi_prepare(command, argument types) ! spi_exec_prepared(plan) spi
Re: [HACKERS] NEED HELP !
And *int obj_contains(moving_object *a, BOX *b) { if (b->low.x > a->x_low) return 0; if (b->low.y > a->y_low) return 0; if (b->high.x < a->x_high) return 0; if (b->high.y < a->y_high) return 0; return 1; }* this is the procedure obj contains On Wed, Jan 13, 2010 at 8:56 PM, Sergej Galkin wrote: > I want to add than I have a piece of my code that looks very strange > > Datum gist_mov_penalty(PG_FUNCTION_ARGS) > { > GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0); > GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(1); > float * result = (float *) PG_GETARG_POINTER(2); > * moving_object *orig = &(*origentry).key; > make_now(orig); > moving_object *new = &(*origentry).key; > make_now(new);* > > > > Is everything ok in this code ?? > > > On Wed, Jan 13, 2010 at 8:42 PM, Sergej Galkin wrote: > >> Hello all, >> >> I am a student-magister and I'm writting my magister work. I realized gist >> index TPR tree - it is like a simple R tree but moving :) >> Everything is compiling ok, I creating table and index, but after whese >> code - DB is restarting :((( >> * >> * >> *set enable_seqscan = false >> select * from table_of_moving_objects where mov_obj ~ box '(0,0) (1,1)'* >> >> I can not understand why DB is restarting ? It is restarting when SQL >> begins use index :((( >> Can anybody help me , I would by appretiate every people who downloads my >> sources, try to deploy index and maybe solve where is the problem. >> >> Best regards, >> Sergej Galkin >> > >
Re: [HACKERS] [PATCH] remove redundant ownership checks
On Wed, Jan 13, 2010 at 1:34 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jan 11, 2010 at 8:27 PM, Bruce Momjian wrote: >>> I agree. Why are arbitrary restrictions being placed on code >>> improvements? If code has no purpose, why not remove it, or at least >>> mark it as NOT_USED. > >> So, where do we go from here? Any other opinions? I'm not sure how >> much it's really worth fighting over a six line patch, but there's >> something in me that rails against the idea of telling someone who >> took the trouble to write a patch "no" when the only argument against >> it is that we might change our mind at some point in the future. Of >> course, I will accept the consensus of the community whatever it is, >> but the only people who have expressed a clear opinion on this version >> of the patch are Tom, Bruce, and myself, and 2-1 is not a consensus. > > I still say that this patch is putting the cart before the horse. > What we need before we do any significant amount of rearrangement > of security checks is to have a plan for where they should go. > Making a change here and a change there without a plan isn't an > improvement, it just risks creating bugs. > > If I thought this patch represented incremental movement in the > direction of a better security-check factorization, I'd be fine with it, > but that's not clear either. The argument for it is that these checks > are redundant with some other ones, but why should we remove these and > not the other ones instead? That's a good question, and I have an answer. Most of the ALTER TABLE operations use ATSimplePermissions() or ATSimplePermissionsRelationOrIndex() to check permissions. The exceptions are AT_SetDistinct (and I'm wondering if we shouldn't remove that exception, see my recent message on attoptions) and AT_ChangeOwner (see comments for why). ATSimplePermissions() checks (1) that we are operating on the right sort of relkind, (2) that the current user owns the object, and (3) that we are operating on a system catalog only if allowSystemTableMods. When we are enabling or disabling a rule (but not in other cases), we then recheck only the second of those conditions. Removing the "other check" (the call to ATSimplePermissions) would require copying the other two bits of logic into EnableDisableRule - so we'd have duplicate code floating around, and the EnableDisableRule operations would have bespoke code instead of sharing the common mechanism used by the remaining ALTER TABLE commands. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] segmentation fault in function
I am realizing gist index and get a bug, that crashes DB. I' debugged my program as Robert(thanks !) advised me and I know which procedure crashed. *Datum gist_mov_consistent(PG_FUNCTION_ARGS)* *{* *GISTENTRY *entry = (GISTENTRY *)PG_GETARG_POINTER(0);* *BOX *query = PG_GETARG_BOX_P(1);* *StrategyNumber strategy = (StrategyNumber)PG_GETARG_UINT16(2);* ** *if (DatumGetMovP(entry->key) == NULL || query == NULL)* *PG_RETURN_BOOL(FALSE);* ** *PG_RETURN_BOOL(obj_contains(DatumGetMovP(entry->key), query));* *}* *int obj_contains(moving_object *a, BOX *b)* *{* ** *if (b->low.x > a->x_low)* *return 0;* *if (b->low.y > a->y_low)* *return 0;* *if (b->high.x < a->x_high)* *return 0;* *if (b->high.y < a->y_high)* *return 0;* *return 1;* *} ** Do you have any ideas ?** Best regards, Sergej Galkin *
Re: [HACKERS] [PATCH] remove redundant ownership checks
Robert Haas writes: > On Wed, Jan 13, 2010 at 1:34 PM, Tom Lane wrote: >> If I thought this patch represented incremental movement in the >> direction of a better security-check factorization, I'd be fine with it, >> but that's not clear either. The argument for it is that these checks >> are redundant with some other ones, but why should we remove these and >> not the other ones instead? > That's a good question, and I have an answer [ namely that ALTER TABLE > is the right place ]. But note Stephen Frost's concurrent reply suggesting that he wants to move the checks *out* of ALTER TABLE. With his plan, these checks are probably in the right place already. I'm a little worried by Stephen's plan, mainly because I'm concerned that it would lead to ALTER TABLE taking exclusive lock on a table long before it gets around to checking permissions. Still, that's just extending a window that exists now. Anyway, this is the sort of thing that should be hashed out before starting to write code. Adding or removing security checks is not the hard part, the hard part is deciding where they should be. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython3
On Jan 13, 2010, at 11:08 AM, Joshua D. Drake wrote: > My argument would be now, what is the benefit of the James Pye version > over our version. James can you illustrate succinctly why we should be > supporting a new version? Doing so, succinctly, is unfortunately difficult. It is primarily a matter of comparing features, AFAICT. And, furthermore, some features may not be useful to some users. It exposes additional functionality that should *not* be incrementally developed in plpython as it would break applications. This was the point of trying to move forward with it for Python 3. Function Modules: - Does away with the need for GD/SD (more natural Python environment). - Allows tracebacks (tracebacks are useful, right?) to implemented easily. - Does *not* expose a bastardized variant of the language by pretending that "modules/script files" can return and yield. - Helps to promote the Python tenet of being explicit. Native Typing: - Provides PG type introspection not available in any other PL, AFAIK. - Improves efficiency in some cases (conversion must be _explicitly_ called for) - MD Array support. - Composites are a sequence and a mapping. Other features: http://wiki.postgresql.org/wiki/WIP:plpython3 Aside from function modules and native typing, many of plpython3's features could be implemented incrementally. However, I had a chance to sprint and they are available now in a new implementation. I did so, rather than improving plpython, because I believe that native typing and function modules are very useful. I'm not sure this fulfills your request, but, hopefully, it's a start. -- 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] plpython3
On Wed, 2010-01-13 at 13:06 -0700, James William Pye wrote: > On Jan 13, 2010, at 11:08 AM, Joshua D. Drake wrote: > > My argument would be now, what is the benefit of the James Pye version > > over our version. James can you illustrate succinctly why we should be > > supporting a new version? > > > Doing so, succinctly, is unfortunately difficult. > It is primarily a matter of comparing features, AFAICT. And, furthermore, > some features may not be useful to some users. > > It exposes additional functionality that should *not* be incrementally > developed in plpython as it would break applications. This was the point of > trying to move forward with it for Python 3. > > Function Modules: > - Does away with the need for GD/SD (more natural Python environment). > - Allows tracebacks (tracebacks are useful, right?) to implemented easily. > - Does *not* expose a bastardized variant of the language by pretending that > "modules/script files" can return and yield. > - Helps to promote the Python tenet of being explicit. > > Native Typing: > - Provides PG type introspection not available in any other PL, AFAIK. > - Improves efficiency in some cases (conversion must be _explicitly_ called > for) > - MD Array support. > - Composites are a sequence and a mapping. > > Other features: http://wiki.postgresql.org/wiki/WIP:plpython3 > > > Aside from function modules and native typing, many of plpython3's features > could be implemented incrementally. However, I had a chance to sprint and > they are available now in a new implementation. I did so, rather than > improving plpython, because I believe that native typing and function modules > are very useful. > > I'm not sure this fulfills your request, but, hopefully, it's a start. It does actually. Now, hackers... as a Python guy I can say these things are truly useful, to a Python programmer trying to use Python as a procedural language with PostgreSQL. What do we think? Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- 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] Bloom index
On Wed, 13 Jan 2010, Robert Haas wrote: 2010/1/13 Teodor Sigaev : CREATE INDEX bloomidx ON tbloom(i1,i2,i3) WITH (length=5, col1=2, col2=2, col3=4); Here, we create bloom index with signature length 80 bits and attributes i1, i2 mapped to 2 bits, attribute i3 - to 4 bits. This is pretty darn slick. I haven't looked at the code yet, but the functionality sounds very cool, and I hope this is something we'll be able to have as part of PG in the future (though more than likely not for 8.5, I suspect). Yes, one of the goal of our announcement is to let potential users to know what we have done and what benefits it (index) provides. In other words, sponsor(s), you are welcome ! Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] RFC: PostgreSQL Add-On Network
robertmh...@gmail.com (Robert Haas) writes: > On Fri, Jan 8, 2010 at 10:12 AM, Dave Page wrote: >>> I have long spoken against making Windows a second class citizen. But I >>> don't think David is going to do that (and I'll hound him if he does). But >>> that doesn't mean it has to be fully supported from day one. >> >> I'm not saying it should be supported from day 1, but I think the >> initial plan will make it very difficult to add Windows support later >> without a great deal of rewriting/redesign. It's lack of forward >> planning I was objecting to. > > I personally suspect that the client is not the most important part of > this project. I think the value of CPAN is for searching, more than > auto-installing. Personally, I never use the auto-install feature > because I always want more control than you get that way. I just use > the site to find possible modules and browse the docs, and then if I > find something I like I check with I can pull it from the Red Hat > repos with rpm, and if not I download it and look it over to see if it > DWIW, and then if so I usually make a private SRPM for it and install > from that. I'd be happy if we just had a good search-and-download > site. If "PGAN" leads to us having: a) A database containing a useful set of metadata about a large set of extensions, and b) A way for PostgreSQL developers and binary distribution makers (who *do* have GCC / XCode / MingW / Visual Studio / ... available to them) to easily: - build - test - try out - think about how to package that large set of extensions then we've got a Big Win of the same sort as CPAN, Ruby Gems, and PyPI. It does NOT need to include installers for every known kind of computer; that is a *second* problem, which actually requires a series of solutions for: a) Fedora b) Debian (hence derivatives like Ubuntu) c) BSD Ports d) Yes, Windows e) I think Solaris has something new for packaging... If David gets it to the point where it's easy to build and install extensions into a PostgreSQL installation, then turning that into packages for specific targets should be a not-insurmountable problem that may be treated separately. > That having been said, we should consider our filesystem layout > carefully however to make sure that if we want to provide things like > Windows installers in the future, we have a clean way to do that. If the extensions get installed in a way that is "scalable" in the sense that it's not a particularly big deal to write a script that pulls 250 extensions and installs them on a particular host for a particular PG installation, then I'd think that the exercise has been a successful one. That leads, naturally enough, to an "Extension BuildFarm" :-). I'd be somewhat surprised if the use of Windows was a material factor in the matter. -- wm(X,Y):-write(X),write('@'),write(Y). wm('cbbrowne','acm.org'). http://www3.sympatico.ca/cbbrowne/postgresql.html "Laugh-a while you can, Monkey Boy." -- Dr. Lizardo - Buckaroo Banzai -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
On Wed, Jan 13, 2010 at 12:54, Tom Lane wrote: > I'm a little worried by Stephen's plan, mainly because I'm concerned > that it would lead to ALTER TABLE taking exclusive lock on a table long > before it gets around to checking permissions. Still, that's just > extending a window that exists now. Im of the opinion if we are going to be meddling with the permission checks in this area one of the goals should be close or at least tighten up that window. So you cant lock a table you dont have permission to (either via LOCK or ALTER TABLE). (Ignoring the issues of concurrent permission changes of course...) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
On Wed, Jan 13, 2010 at 13:46, Alex Hunsaker wrote: > Im of the opinion if we are going to be meddling with the permission > checks [...] Specifically: http://archives.postgresql.org/pgsql-hackers/2009-05/msg00566.php Sounds like most solutions would put us back to exactly what you were trying to fix. :( Maybe its a moot point. But I figured while we are talking about ALTER permissions -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
* Alex Hunsaker (bada...@gmail.com) wrote: > On Wed, Jan 13, 2010 at 12:54, Tom Lane wrote: > > I'm a little worried by Stephen's plan, mainly because I'm concerned > > that it would lead to ALTER TABLE taking exclusive lock on a table long > > before it gets around to checking permissions. Still, that's just > > extending a window that exists now. > > Im of the opinion if we are going to be meddling with the permission > checks in this area one of the goals should be close or at least > tighten up that window. So you cant lock a table you dont have > permission to (either via LOCK or ALTER TABLE). (Ignoring the issues > of concurrent permission changes of course...) Trying to minimize that makes the permissions checking a royal mess by making it have to happen all over the place, after every little bit of information is gathered. I'm not a fan of that because of both concerns about making sure it's correct and actually matches our documention, as well as any possibility of making it a pluggable framework. At the moment, we're doing permissions checks on the main table before we even know if the other tables referenced in the command exist. I don't think we're talking about a serious difference in time here either, to be honest. Not to mention that if you don't have access to the schema, you wouldn't be able to take a lock on the table at all, so I'm really not sure how big a deal this is.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] plpython3
On ons, 2010-01-13 at 12:12 -0800, Joshua D. Drake wrote: > On Wed, 2010-01-13 at 13:06 -0700, James William Pye wrote: > > Function Modules: > > - Does away with the need for GD/SD (more natural Python environment). > > - Allows tracebacks (tracebacks are useful, right?) to implemented easily. > > - Does *not* expose a bastardized variant of the language by pretending > > that "modules/script files" can return and yield. > > - Helps to promote the Python tenet of being explicit. > > > > Native Typing: > > - Provides PG type introspection not available in any other PL, AFAIK. > > - Improves efficiency in some cases (conversion must be _explicitly_ > > called for) > > - MD Array support. > > - Composites are a sequence and a mapping. > > > > Other features: http://wiki.postgresql.org/wiki/WIP:plpython3 > > > > > > Aside from function modules and native typing, many of plpython3's features > > could be implemented incrementally. However, I had a chance to sprint and > > they are available now in a new implementation. I did so, rather than > > improving plpython, because I believe that native typing and function > > modules are very useful. > > > > I'm not sure this fulfills your request, but, hopefully, it's a start. > > It does actually. Now, hackers... as a Python guy I can say these things > are truly useful, to a Python programmer trying to use Python as a > procedural language with PostgreSQL. The problem I'm having with this discussion is that every time someone asks what the supposed advantages of this new Python PL are, a feature list like the above is dumped, 75% of which is subjective and tends to use semi-buzzwords, such that then someone else who by his own admission isn't completely up to date on things says, sure, that sounds great. Who wouldn't like a "more natural Python environment", "native typing" and "efficiency", and maybe even "explicitness"? The current PL/Python also has, arguably, a more natural Python environment, native typing, efficiency, and explicitness. So there you go. Now what? -- 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] lock_timeout GUC patch
Boszormenyi Zoltan írta: > Tom Lane írta: > >> Boszormenyi Zoltan writes: >> >> >>> Tom Lane írta: >>> >>> If this patch is touching those parts of relcache.c, it probably needs rethinking. >> >> >>> What I did there is to check the return value of LockRelationOid() >>> and also elog(PANIC) if the lock wasn't available. >>> Does it need rethinking? >>> >>> >> Yes. What you have done is to change all the LockSomething primitives >> from return void to return bool and thereby require all call sites to >> check their results. This is a bad idea. >> > > Okay, can you tell me how can I get the relation name > out of the xid in XactLockTableWait()? There are several > call site of this function, and your idea about putting the error > code into the LockSomething() functions to preserve the API > results strange error messages, like > > ERROR: could not obtain lock on transaction with ID 658 > > when I want to UPDATE a tuple in a session when > this and another session have a FOR SHARE lock > on said tuple. > > >> There is no way that you can >> ensure that all third-party modules will make the same change, meaning >> that accepting this patch will certainly introduce nasty, hard to >> reproduce bugs. And what's the advantage? The callers are all going >> to throw errors anyway, so you might as well do that within the Lock >> function and avoid the system-wide API change. >> May I change the interface of XactLockTableWait() and MultiXactIdWait()? Not the return value, only the number of parameters. E.g. with the relation name, like in the attached patch. This solves the problem of bad error messages... What do you think? >> I think this is a big patch with a small patch struggling to get out. >> >> > > Your smaller patch is attached, with the above strangeness. :-) > > Best regards, > Zoltán Böszörményi > > > > > -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics -- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ diff -dcrpN pgsql.orig/doc/src/sgml/config.sgml pgsql.5/doc/src/sgml/config.sgml *** pgsql.orig/doc/src/sgml/config.sgml 2010-01-06 08:43:22.0 +0100 --- pgsql.5/doc/src/sgml/config.sgml 2010-01-13 18:59:19.0 +0100 *** COPY postgres_log FROM '/full/path/to/lo *** 4191,4196 --- 4191,4220 + + lock_timeout (integer) + +lock_timeout configuration parameter + + + + Abort any statement that tries to acquire a heavy-weight lock (e.g. rows, + pages, tables, indices or other objects) and the lock has to wait more + than the specified number of milliseconds, starting from the time the + command arrives at the server from the client. + If log_min_error_statement is set to ERROR or lower, + the statement that timed out will also be logged. A value of zero + (the default) turns off the limitation. + + + + Setting lock_timeout in + postgresql.conf is not recommended because it + affects all sessions. + + + + vacuum_freeze_table_age (integer) diff -dcrpN pgsql.orig/doc/src/sgml/ref/lock.sgml pgsql.5/doc/src/sgml/ref/lock.sgml *** pgsql.orig/doc/src/sgml/ref/lock.sgml 2009-09-18 08:26:40.0 +0200 --- pgsql.5/doc/src/sgml/ref/lock.sgml 2010-01-13 18:59:19.0 +0100 *** LOCK [ TABLE ] [ ONLY ] NOWAIT is specified, LOCK TABLE does not wait to acquire the desired lock: if it cannot be acquired immediately, the command is aborted and an !error is emitted. Once obtained, the lock is held for the !remainder of the current transaction. (There is no UNLOCK TABLE command; locks are always released at transaction end.) --- 39,49 NOWAIT is specified, LOCK TABLE does not wait to acquire the desired lock: if it cannot be acquired immediately, the command is aborted and an !error is emitted. If lock_timeout is set to a value !higher than 0, and the lock cannot be acquired under the specified !timeout value in milliseconds, the command is aborted and an error !is emitted. Once obtained, the lock is held for the remainder of !the current transaction. (There is no UNLOCK TABLE command; locks are always released at transaction end.) diff -dcrpN pgsql.orig/doc/src/sgml/ref/select.sgml pgsql.5/doc/src/sgml/ref/select.sgml *** pgsql.orig/doc/src/sgml/ref/select.sgml 2009-10-29 15:23:52.0 +0100 --- pgsql.5/doc
Re: [HACKERS] [PATCH] remove redundant ownership checks
* Alex Hunsaker (bada...@gmail.com) wrote: > On Wed, Jan 13, 2010 at 13:46, Alex Hunsaker wrote: > > Im of the opinion if we are going to be meddling with the permission > > checks [...] > > Specifically: > http://archives.postgresql.org/pgsql-hackers/2009-05/msg00566.php > > Sounds like most solutions would put us back to exactly what you were > trying to fix. :( Maybe its a moot point. But I figured while we are > talking about ALTER permissions Maybe I missed it, but I don't see anything that said ALTER TABLE was changed or fixed to address this concern. It might make the timing increase some, and it'd be interesting in any case to see just what the timing change looked like, but I don't know that it's really all that important.. At least if the timing didn't change much we could claim that this didn't affect this use-case, but I don't believe it shouldn't be done if it does. I don't see a way to actually *fix* the problem of not allowing someone who doesn't have all the right permissions to not lock the table at all. Taking a share lock and then trying to upgrade it isn't a good idea either. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] remove redundant ownership checks
Alex Hunsaker writes: > Im of the opinion if we are going to be meddling with the permission > checks in this area one of the goals should be close or at least > tighten up that window. So you cant lock a table you dont have > permission to (either via LOCK or ALTER TABLE). (Ignoring the issues > of concurrent permission changes of course...) Well, that's exactly the problem: it's not very sane to do permissions checking on a table you have no lock whatsoever on, because the table could be dropped, renamed, or have its permissions altered underneath you. We could imagine taking a weak lock that forbids those operations and then upgrading once we're sure we have the right to take a stronger lock, but lock upgrade is a certain ticket to deadlocks. So yeah, it'd be nice, but it's not apparent how to do it. The best thing I can see how to do is keep the window between taking the lock and verifying permissions narrow. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch to implement ECPG side tracing / tracking ...
Michael Meskes wrote: Hans, nce Jaime already asked for a use case, just a few small comments from me. @@ -4,6 +4,7 @@ #include "postgres_fe.h" #include +#include This is not portable. You don't want to include this header. Did I see this right that you use the statement cache for auto-prepared statements even if the statement is not auto prepared? Some statements are not profiled, how did you decide which one to do? There is no test case. Before looking into it in detail I think we should first figure out if this feature really has a benefit. Michael hello ... the use cases for this thing are quite simple: we are currently porting hundreds (!) of complex Informix terminal applications to PostgreSQL. these are basically terminal applications used to perform a certain tasks. given the vast amount of code, we simply cannot change a single program because if we have to dig into the actual application code, we are dead before actually starting (business logic is a nightmare). so, to get around the problem we are basically adding all extensions to ECPG we need to make this work. this is why we did all this SQLDA stuff and so on you have seen recently. the current problems are a bit more delicate: we have this vast number of programs and some of them perform better than Informix and some simply don't. Informix has some sort of "explain mode" (I forgot the exact name) which allows you to see which query is executed how by the system. effectively, you can use it to performance tune your precompiler application. in PostgreSQL it is currently a little hard to get from the log what is executed how often by which application in which speed and so on. so, we came up with the idea of adding a flag to the precompiler which essential keep stats for us and display it on exit (could be sent to a file then or so without anybody's notice). this would give excellent data to start with and it would make checking the database part of the application easily. why for prepared queries: we found out that Informix is heavily using prepared queries internally. we already fixed something in this area (patch sent some time ago) and we were finally able to catch up with Informix performance-wise in this area (mostly cursor work). before this auto_prepare fix, we were sometimes 2-3 times slower than Informix. saving on network time solved the job. now we are left with many many programs performing somehow strange and we need to check for every program why. a decent summary on exit would be gold here. it seems we will also come up with a server-side extension soon which basically compares and logs planner / executor starts the way we do it for stored procedures now (thanks to martin pilhak). we simply need it so that we can figure out which of our XXX programs did what then. testing one after the other is not so easy, some of them depend on each. to make it short: it is impossible to port hundreds of applications to PostgreSQL without having the chance to trace what the precompiler is doing how often in which program via which connection. it is simply impossible. so, we really and desparately need this patch in. many thanks, hans -- Cybertec Schoenig & Schoenig GmbH Reyergasse 9 / 2 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython3
On Wed, 2010-01-13 at 23:27 +0200, Peter Eisentraut wrote: > The problem I'm having with this discussion is that every time someone > asks what the supposed advantages of this new Python PL are, a feature > list like the above is dumped, 75% of which is subjective and tends to > use semi-buzzwords, such that then someone else who by his own admission > isn't completely up to date on things says, sure, that sounds great. > Who wouldn't like a "more natural Python environment", "native typing" > and "efficiency", and maybe even "explicitness"? The current PL/Python > also has, arguably, a more natural Python environment, native typing, > efficiency, and explicitness. So there you go. Now what? Peter, are you interested in the benefit of the project, or your own ego? Take it easy and let's see if we can be productive here. I was just stating that James's explanation was good and looking for more information from a reviewer. Secondly nobody (at least I am not) is suggesting we dump the current PL/Python, so your hard work is not going to be lost. The only thing I am currently looking for is an objective review of the patch based on the benefits it provides. I can tell you that if the Pye patch provides stack trace capability and the current PL does not. That right there is enough for me to push a +1 for review and possible inclusion. If you feel what James is saying is actually trite and more buzzword compliant I welcome the opportunity to see a comparative of the current status of PL/Python versus the Pye patch from you. If nothing else it is a learning opportunity for all involved. Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564 Consulting, Training, Support, Custom Development, Engineering Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] primary key display in psql
When you look at a table definition with psql \d, one of the arguably most important pieces of information -- the primary key -- is hidden somewhere below under "indexes": Table "public.test2" Column | Type | Modifiers +-+--- a | integer | not null b | integer | not null Indexes: "test2_pkey" PRIMARY KEY, btree (a, b) I think we could easily improve that by having it look something like this instead: Table "public.test2" Column | Type | Modifiers +-+--- a | integer | PK b | integer | PK Indexes: "test2_pkey" PRIMARY KEY, btree (a, b) Since there can only be one primary key, this should be unambiguous. I don't have time to code this up right now, but maybe someone feels inspired. What do you think? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch to implement ECPG side tracing / tracking ...
Hans-Juergen Schoenig writes: > Michael Meskes wrote: >> Before looking into it in detail I think we should first figure out if this >> feature really has a benefit. > the use cases for this thing are quite simple: we are currently porting > hundreds (!) of complex Informix terminal applications to PostgreSQL. > [ and need to optimize them ] What you didn't explain is why you need client-side tracing rather than using the rather extensive facilities that already exist server-side. In particular, have you looked at CVS tip contrib/auto_explain? It seems like you are duplicating a lot of what that can do. If that needs some additional features, you could work on that. From the big picture standpoint I think it makes a lot more sense to add instrumentation server-side than client-side. Any features you add client-side are only available to ecpg users, and you have to cope with ensuring there's a way to collect the data out of the application (which may be running in an environment where that's hard). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython3
On ons, 2010-01-13 at 13:33 -0800, Joshua D. Drake wrote: > The only thing I am currently looking for is an objective review of the > patch based on the benefits it provides. Right, but I was opining that such a vague feature listing is not adequate for that. > I can tell you that if the Pye > patch provides stack trace capability and the current PL does not. That > right there is enough for me to push a +1 for review and possible > inclusion. That is the case. > If you feel what James is saying is actually trite and more buzzword > compliant I welcome the opportunity to see a comparative of the current > status of PL/Python versus the Pye patch from you. If nothing else it is > a learning opportunity for all involved. There was extensive discussion about some of the design decisions upthread, so any reviewer should review those. It may end up being a agree-to-disagree situation. -- 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] primary key display in psql
Peter Eisentraut writes: > I think we could easily improve that by having it look something like > this instead: > Table "public.test2" > Column | Type | Modifiers > +-+--- > a | integer | PK > b | integer | PK > Indexes: > "test2_pkey" PRIMARY KEY, btree (a, b) Spelling out "primary key" would seem to be more in keeping with existing entries in that column, eg we have "not null" not "NN". I think this is a sensible proposal for a single-column PK, but am less sure that it makes sense for multi-col. The modifiers column is intended to describe column constraints; which a multi-col PK is not, by definition. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] per-user pg_service.conf
I was surprised/annoyed to find out that there is no way to have per-user pg_service.conf, something like ~/.pg_service.conf (well, except by export PGSYSCONFDIR). That would be easy to add. Comments? -- 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] Serializable Isolation without blocking
Robert Haas wrote: > Nope, you're on target. Although - if I were you - I would post > the ACCESS EXCLUSIVE lock version of the patch for feedback. I > can't speak for anyone else, but I'll read it. Here you go! :-) This is the milestone of having full serializable behavior, albeit with horrible performance, using the simplest implementation possible. I didn't use ACCESS EXCLUSIVE locks, because on review it seemed to me that a SHARE lock would be strong enough. It compiles and passes the regression tests, and I've been testing some of the scenarios previously used to show the snapshot anomalies; I now get correct behavior through blocking. I identified the points to insert predicate locking by looking for places where ExecStoreTuple was called with a valid heap buffer; if there is anywhere that obtains tuples from the heap without going through that method, I have more work to do. If anyone knows of such locations, I'd be grateful for a "heads up". If I've done anything horribly wrong in organizing the code, that'd be nice to hear about before I go too much farther, too. I'm definitely not looking for this to be committed, but should I add it to the CF page just for a "feedback" review? (I'm OK with keeping it more ad hoc, especially if it's going to hold up the beta at all.) -Kevin *** a/src/backend/catalog/index.c --- b/src/backend/catalog/index.c *** *** 2133,2139 IndexCheckExclusion(Relation heapRelation, * * After completing validate_index(), we wait until all transactions that * were alive at the time of the reference snapshot are gone; this is ! * necessary to be sure there are none left with a serializable snapshot * older than the reference (and hence possibly able to see tuples we did * not index).Then we mark the index "indisvalid" and commit. Subsequent * transactions will be able to use it for queries. --- 2133,2139 * * After completing validate_index(), we wait until all transactions that * were alive at the time of the reference snapshot are gone; this is ! * necessary to be sure there are none left with a transaction-based snapshot * older than the reference (and hence possibly able to see tuples we did * not index).Then we mark the index "indisvalid" and commit. Subsequent * transactions will be able to use it for queries. *** a/src/backend/commands/trigger.c --- b/src/backend/commands/trigger.c *** *** 2319,2325 ltrmark:; case HeapTupleUpdated: ReleaseBuffer(buffer); ! if (IsXactIsoLevelSerializable) ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); --- 2319,2325 case HeapTupleUpdated: ReleaseBuffer(buffer); ! if (IsXactIsoLevelXactSnapshotBased) ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); *** a/src/backend/executor/execMain.c --- b/src/backend/executor/execMain.c *** *** 1538,1544 EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, case HeapTupleUpdated: ReleaseBuffer(buffer); ! if (IsXactIsoLevelSerializable) ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); --- 1538,1544 case HeapTupleUpdated: ReleaseBuffer(buffer); ! if (IsXactIsoLevelXactSnapshotBased) ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); *** a/src/backend/executor/nodeBitmapHeapscan.c --- b/src/backend/executor/nodeBitmapHeapscan.c *** *** 42,47 --- 42,48 #include "executor/nodeBitmapHeapscan.h" #include "pgstat.h" #include "storage/bufmgr.h" + #include "storage/predicate.h" #include "utils/memutils.h" #include "utils/snapmgr.h" #include "utils/tqual.h" *** *** 11
Re: [HACKERS] primary key display in psql
On Wed, Jan 13, 2010 at 4:47 PM, Tom Lane wrote: > Peter Eisentraut writes: >> I think we could easily improve that by having it look something like >> this instead: > >> Table "public.test2" >> Column | Type | Modifiers >> +-+--- >> a | integer | PK >> b | integer | PK >> Indexes: >> "test2_pkey" PRIMARY KEY, btree (a, b) > > Spelling out "primary key" would seem to be more in keeping with existing > entries in that column, eg we have "not null" not "NN". > > I think this is a sensible proposal for a single-column PK, but am less > sure that it makes sense for multi-col. The modifiers column is > intended to describe column constraints; which a multi-col PK is not, > by definition. Yeah, IIRC, MySQL shows PRI for each column of a multi-column primary key, and I think it's horribly confusing. I wouldn't even be in favor of doing this just for the single-column case, on the grounds that it makes the single and multiple column cases asymmetrical. IMO, the \d output has too many bells and whistles already; the last thing we should do is add more. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch to implement ECPG side tracing / tracking ...
On Wed, Jan 13, 2010 at 4:42 PM, Tom Lane wrote: > Hans-Juergen Schoenig writes: >> Michael Meskes wrote: >>> Before looking into it in detail I think we should first figure out if this >>> feature really has a benefit. > >> the use cases for this thing are quite simple: we are currently porting >> hundreds (!) of complex Informix terminal applications to PostgreSQL. >> [ and need to optimize them ] > > What you didn't explain is why you need client-side tracing rather than > using the rather extensive facilities that already exist server-side. > In particular, have you looked at CVS tip contrib/auto_explain? It > seems like you are duplicating a lot of what that can do. If that needs > some additional features, you could work on that. From the big picture > standpoint I think it makes a lot more sense to add instrumentation > server-side than client-side. Any features you add client-side are only > available to ecpg users, and you have to cope with ensuring there's a > way to collect the data out of the application (which may be running in > an environment where that's hard). The OP might even want to think about just turning on log_min_duration_statement for all queries. auto_explain might even be more than is needed. ...Robert -- 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] Incrementally Updated Backups and restartpoints
On Wed, Jan 13, 2010 at 9:34 PM, Heikki Linnakangas wrote: > No, that's not an issue. We only wait for the backup-end record if we > haven't seen yet since we started recovery from the base backup. > Assuming the standby had reached that point already before the new > backup from the standby started, backupStartLoc is zero in the control file. OK. That assumption should be documented? And, when we start an archive recovery from the backup from the standby, we seem to reach a safe starting point before database has actually become consistent. It's because backupStartLoc is zero. Isn't this an issue? Regards, -- Fujii Masao 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] Serializable Isolation without blocking
"Kevin Grittner" wrote: > Here you go! :-) Hmmm... I just got a write skew example to show a snapshot isolation anomaly, so I've got something wrong still. :-( Will continue to work on it. Sorry. -Kevin -- 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] Serializable Isolation without blocking
"Kevin Grittner" wrote: > This is the milestone of having full serializable behavior, albeit > with horrible performance, using the simplest implementation > possible. A tad too simple, as it turns out. It did it's main job of providing a simple milestone to identify code organization and lock points, but I'd have to jigger some things to make S2PL work with snapshot isolation which aren't needed for SSI. So, for those keeping score, I'm moving on down the checklist to get to the SSI implementation, rather than futzing with code which would just be thrown away. No need for anyone to test for full serializable behavior. Comments on technique welcome. -Kevin -- 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] per-user pg_service.conf
Peter Eisentraut writes: > I was surprised/annoyed to find out that there is no way to have > per-user pg_service.conf, something like ~/.pg_service.conf (well, > except by export PGSYSCONFDIR). That would be easy to add. Comments? +1. I'll use it the day it exists. -- dim -- 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] per-user pg_service.conf
On Wed, Jan 13, 2010 at 11:49:59PM +0200, Peter Eisentraut wrote: > I was surprised/annoyed to find out that there is no way to have > per-user pg_service.conf, something like ~/.pg_service.conf (well, > except by export PGSYSCONFDIR). That would be easy to add. Comments? +1 from me. I was similarly surprised to learn the same thing recently, but admit I didn't take the time see how easily it could be changed. -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com signature.asc Description: Digital signature
[HACKERS] last CommitFest coming up in just under 24 hours
Patch authors, please make sure your patches are listed on commitfest.postgresql.org. https://commitfest.postgresql.org/action/commitfest_view/open All, we still need reviewers for the following patches. New XLOG record indicating WAL-skipping Fix large object support in pg_dump knngist (WIP) plpython3 Add utility functions to plperl At least two people have expressed an interested in the perl patch, so I am pretty sure it will get covered, if not in the first round then later on, but feel free to sign up now anyway if you're interested. Thanks, ...Robert -- 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] plpgsql: open for execute - add USING clause
Pavel Stehule wrote: > ok, I accept all comments. > revised version are attached. Good. This patch is ready to commit. I'll do it soon if no objections. BTW, I found inconsistent parameter dumps in the codes. Some of them add '$', but others does not. Are they intentional? Or, should we adjust them to use one of the formats? [pl_funcs.c] dump_dynexecute() dump_raise() printf("parameter %d: ", i++); dump_dynfors() dump_open() dump_return_query() printf("parameter $%d: ", i++); Regards, --- Takahiro Itagaki 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] [PATCH] remove redundant ownership checks
(2010/01/14 4:54), Tom Lane wrote: Robert Haas writes: On Wed, Jan 13, 2010 at 1:34 PM, Tom Lane wrote: If I thought this patch represented incremental movement in the direction of a better security-check factorization, I'd be fine with it, but that's not clear either. �The argument for it is that these checks are redundant with some other ones, but why should we remove these and not the other ones instead? That's a good question, and I have an answer [ namely that ALTER TABLE is the right place ]. But note Stephen Frost's concurrent reply suggesting that he wants to move the checks *out* of ALTER TABLE. With his plan, these checks are probably in the right place already. Note that this patch tries to remove redundant checks in this code path. If ATPrepCmd() would not be a right place to apply permission checks, we should remove invocation of the ATSimplePermissions() for AT_EnableRule and so on. (Of course, we need to copy two other sanity check in the ATSimplePermissions() also) However, in my opinion, ATPrepCmd() is more appropriate to apply permission checks than EnableDisableRule(), because we deal with rewrite rule (that does not have individual ownership and acls) as properties of a relation, not an independent database object, although it is stored in its own system catalog. It is quite natural to check privileges to alter properties of a relaion in tablecmd.c, rather than rewriteDefine.c. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
(2010/01/14 4:27), Stephen Frost wrote: > * Jaime Casanova (jcasa...@systemguards.com.ec) wrote: >> if it's not broken, then it doesn't need a fix... >> if that code is causing a hole in security then i said remove it, if >> not... what's the problem in let it be until we know exactly what the >> plan is? > > The chances of getting concensus on an overarching big plan are slim > to none, which essentially means it's not going to get changed. I've > suggested an approach and had no feedback on it. What's probably > needed, to get attention anyway, is a patch which starts to move us in > the right direction. That's going to be more than a 6-line patch, but > doesn't have to be the whole thing either. > > For my 2c, I think a patch which attacks 'AT_PrepCmd' and rips out all > of the owner checks from it and moves them to appropriate places (that > is to say, per my proposal, very shortly before the 'work' is actually > done, which is also often where the *other* permission checks are done, > for those pieces which require more than just a simple owner check) > would probably be the first step. > > Of course, the code between AT_PrepCmd and where the permissions checks > are moved to would need to be reviewed and vetted to make sure there > isn't anything being done that shouldn't be done without permission, > but, honestly, I don't recall seeing much of that. We're actually > pretty good about having a "gather info" stage followed by a "implement > change" stage. Some of ALTER TABLE operations take multiple permission checks, not only ownership of the relation to be altered. For example, ALTER TABLE with SET TABLESPACE option also need ACL_CREATE permission on the new tablespace, not only ownership of the relation. It means we need to gather two information before whole of the permission checks. (1) OID of the relation to be altered. (2) OID of the tablespace to be set. In my understanding, Stephen suggests that we should, ideally, rip out permission logic from the code closely-combined with the steps of implementation. Of course, it does not mean all the checks should be moved just before simple_heap_update(). However, it is a separate topic for this patch. This patch just tries to remove redundant checks, and integrate them into a common place where most of ALTER TABLE option applies its permission checks on. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Odd cruft in .psql_history in HEAD
I noticed odd stuff showing up when I fired up an 8.3 psql after using psql in HEAD. It shows up in .psql_history as well: deci...@platter.1[20:32]~:5%tail -n 2 .psql_history \134df+\040tools.raise_exception \df+ tools.raise_exception deci...@platter.1[20:35]~:6% (last entry is from the 8.3 psql) -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] improving log management
Hi, We support several methods for logging server messages. The "native" methods (stderr, csvlogs) has poor management. We can't compress logs, send them to another location/server, or just remove old ones. Another problem is that we can't remove (automatically) old logs based on the number of existing log files (of course we can do it using log_truncate_on_rotation and a log_filename that matches the number of files -- but it *isn't* flexible). The other log management softwares have a way to do that so why our logger doesn't have such capability? I want to propose a new command to be execute after the log file is rotated. A GUC parameter log_after_rotation_command that takes a (set of) command(s) that will be executed after a log file is rotated. It should be set only by superuser. For example: # compress and store the log file in another location log_after_rotation_command = 'gzip %f && mv %p.gz /a/mylogs' Depending on the comments, I will clean up the patch and send it later today. -- Euler Taveira de Oliveira http://www.timbira.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] Odd cruft in .psql_history in HEAD
Jim Nasby writes: > I noticed odd stuff showing up when I fired up an 8.3 psql after using psql > in HEAD. It shows up in .psql_history as well: Platform? readline version? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving log management
Euler Taveira de Oliveira writes: > I want to propose a new command to be execute after the log file is > rotated. (1) Windows compatibility? (2) What happens if the command takes a significant amount of time to execute? We can't afford to have the log collector blocked. (3) What do you intend those %p and %f symbols to signify? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
The similar matter can be reproduced with ALTER TABLE ... TYPE statement, not only RENAME TO option. postgres=# CREATE TABLE t1 (a int); CREATE TABLE postgres=# CREATE TABLE s1 (a int); CREATE TABLE postgres=# CREATE TABLE ts (b int) inherits (t1, s1); NOTICE: merging multiple inherited definitions of column "a" CREATE TABLE postgres=# ALTER TABLE t1 ALTER a TYPE text; ALTER TABLE postgres=# insert into t1 values ('aaa'); INSERT 0 1 postgres=# insert into ts values ('bbb', 2); INSERT 0 1 postgres=# SELECT * FROM t1; a - aaa bbb (2 rows) postgres=# SELECT * FROM s1; ERROR: attribute "a" of relation "ts" does not match parent's type In the renameatt(), we can count an expected inhcount of the column to be renamed on find_all_inheritors() at the top-level recursion. But it does not work well for the new one, because it is handled within the common ATPrepCmd() scheme. I reconsidered that we need a function to check whether the given column is inherited from multiple root parents, or not, for each levels. Then, it can be called from both of renameatt() and ATPrepAlterColumnType(). (2010/01/04 18:55), KaiGai Kohei wrote: >>> The method I suggested would allow the >>> necessary information to be extracted during the initial search for >>> child tables, which we have to do anyway. >> >> find_all_inheritors() returns a clean list which does not contain >> duplicated OID of the inherited relation, so it seems to me we need >> to change the function prototype but it affects other parts, or to add >> a new function which also returns number of duplications, not only OIDs. >> >> Or, we can call find_inheritance_children() in renameatt() as if >> find_all_inheritors() doing except for list_concat_unique_oid(). > > The attached patch modified the condition to prevent renaming. > > It computes an expected inhcount for each child relations on the initial > call of the renameatt() for the parent relation. > The find_all_inheritors_with_inhcount() returns OID of the inherited > relations and the expected inhcoundt. If a child relation has diamond > inheritance tree, it has its expected inhcount larger than 1. > > This patch raises an error, if pg_attribute.inhcount is larger than > the expected inhcount. It can be happen when the attribute to be > renamed is merged from any other unrelated relations in the child > relations. > > See the example: > >postgres=# CREATE TABLE t1 (a int); >CREATE TABLE >postgres=# CREATE TABLE t2 (b int) inherits (t1); >CREATE TABLE >postgres=# CREATE TABLE t3 (c int) inherits (t1); >CREATE TABLE >postgres=# CREATE TABLE t4 (d int) inherits (t2, t3); >NOTICE: merging multiple inherited definitions of column "a" >CREATE TABLE > >postgres=# ALTER TABLE t1 RENAME a TO x; >ALTER TABLE >postgres=# \d t4 > Table "public.t4" > Column | Type | Modifiers >+-+--- > x | integer | > b | integer | > c | integer | > d | integer | >Inherits: t2, > t3 > > We can rename a of t1, t2, t3 and t4 correctly, although t4.a has inherited > from > multiple relations. > >postgres=# CREATE TABLE s1 (x int); >CREATE TABLE >postgres=# CREATE TABLE t5 (e int) inherits (t2, t3, s1); >NOTICE: merging multiple inherited definitions of column "x" >NOTICE: merging multiple inherited definitions of column "x" >CREATE TABLE >postgres=# ALTER TABLE t1 RENAME x TO y; >ERROR: cannot rename multiple inherited column "x" > > But, the new t5 prevent to rename "x" to "y", because t5.x is also inherited > from > the s1 and merged. So, its inhcount is 3 larger than expected inhcount (=2). > >postgres=# SELECT attname, attinhcount FROM pg_attribute where attname='x' > and attrelid='t5'::regclass; > attname | attinhcount >-+- > x | 3 >(1 row) > > Thanks, > > > > -- OSS Platform Development Division, NEC KaiGai Kohei -- 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] improving log management
Euler Taveira de Oliveira wrote: > other log management softwares have a way to do that so why our logger doesn't > have such capability? > > I want to propose a new command to be execute after the log file is rotated. A > GUC parameter log_after_rotation_command that takes a (set of) command(s) that > will be executed after a log file is rotated. If you have better loggers already, why don't you use them? In another word, should we cooperate with them instead of re-inventing alternative loggers? We have "Logging Brainstorm" topic in out wiki. It might help you. http://wiki.postgresql.org/wiki/Logging_Brainstorm Regards, --- Takahiro Itagaki 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] Pretty printed trigger in psql
Takahiro Itagaki writes: > The attached patch eliminates unneeded parentheses by using > pg_get_triggerdef(pretty = true) in psql. Is this patch reversed? It seems so but the listed file timestamps don't match that idea ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython3
On Jan 13, 2010, at 2:27 PM, Peter Eisentraut wrote: > The problem I'm having with this discussion is that every time someone > asks what the supposed advantages of this new Python PL are, a feature > list like the above is dumped, I agree that this is unfortunate, but how else can we to discuss the advantages? It boils down to comparing a couple feature lists, and *maybe* some implementation details. No? > 75% of which is subjective and tends to use semi-buzzwords, You say "semi-buzzwords", I say "names". I have to call "it" something. > such that then someone else who by his own admission > isn't completely up to date on things says, sure, that sounds great. Which is why we need to get some more experienced Python users involved in this. Well, even the mileage of inexperienced users is quite useful for detecting what level of obviousness has been achieved by the features, so I'm not trying to exclude anyone. > The current PL/Python also has, arguably, a more natural Python environment, No, it doesn't. GD/SD are contrived in order to compensate for the very absence of that. Additionally, "modules / script files" don't return or yield. > native typing, Okay, that's arguably subjective, but when I write "native typing", it's wrt PG, not Python's built-in types. If that wasn't the case, I wouldn't call it anything--save "[data] conversion" when necessary--as it wouldn't be much of a feature to clamor about. > efficiency, Yes, as discussed in the thread before there are trade-offs here wrt how PG data is handled. It depends pretty heavily on how the parameters / query results are used. Although, as stated before, the difference in efficiency can be rather significant in situations where conversion to built-in Python types is *not* desired. > and explicitness. Hrm? What is explicit about munging the source code of the procedure to make it into a function body? Perhaps you could give some examples where plpython helps promote explicitness? And sure, I understand that other PLs do this. That may be fine for those languages, but for Python it's not, IMO. -- 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] plpython3
On Jan 13, 2010, at 12:15 PM, Robert Haas wrote: > 1. It's not just a rewrite, it's an incompatible rewrite that will > present significant user-visible behavioral differences. So replacing > the current implementation wholesale would produce massive breakage > for anyone actually using PL/python in production. Right. That was the point of trying to leverage Python 3 to make the distinction. Most people will need to update their functions if they are moving to Python 3. And for the larger chunks of code, the hard stuff, the amount of change required is likely significant already. > Given these two facts, it's hard for me to see how we could decide to > REMOVE the current implementation and replace it with the new one. So > the most we could do is maintain them side by side, and then you have > to ask, why? My original hope was that plpython would be maintained for several years to come and when Python 3 started picking up steam, we would deprecate plpython. If people still wanted to use it, they could continue using the older version of PG and/or someone could continue to maintain plpython out of core for legacy support. > Sure, his code won't get as much exposure that way, =) Try next to none. The existence of core's implementation makes competing *very* difficult, IMO. Thinking of something along the lines: "Why would I use/contribute to your implementation when core has one?" And, all I can say in response is, "Check out my features." Subsequently, they will probably weigh the added risk of choosing the loner's implementation and come to the conclusion that using core's would be safer in the long term. I can't fault that line of reasoning, so ISTM that it would be difficult to "sell". > but it's been posted to the mailing list several times now > over a period of 8 months and nobody has said "oh, wow, this is > great". Yeah. :( In the past, one person showed interest in function modules(Stuart, see the first WIP message), and two others showed interest in native typing(Nathan and Tino). Mr. Drake has also shown some interest in this thread. But, yes, you are correct. There has been no "wow, this is great" message. -- 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] Pretty printed trigger in psql
Tom Lane wrote: > Takahiro Itagaki writes: > > The attached patch eliminates unneeded parentheses by using > > pg_get_triggerdef(pretty = true) in psql. > > Is this patch reversed? It seems so but the listed file timestamps > don't match that idea ... Sorry, I cannot understand what you mean... My English might be broken. May I explain what I did again? 1. psql has been used pg_get_triggerdef(oid). 2. I added pg_get_triggerdef(oid, pretty = false) at the last commit fest for pg_dump to support dumping triggeres with WHEN cluase. In that time, PRETTYFLAG_PAREN and PRETTYFLAG_INDENT are used when pretty = true. 3 psql still uses pg_get_triggerdef(oid [, pretty=false] ). Also, pg_dump should use (pretty=false) for safer migration. No programs use pg_get_triggerdef(pretty=true) is for now. 4. psql will be better to use pg_get_triggerdef(oid, true) to display trigger definitions cleanly, but it also should print them in one line. For the purpose, the patch changes two things: - Modify psql to use pg_get_triggerdef(oid, true) when server version >= 8.5. - Remove PRETTYFLAG_INDENT from pg_get_triggerdef(). It will partially revert the changes in 2. Regards, --- Takahiro Itagaki 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] Pretty printed trigger in psql
Takahiro Itagaki writes: > Tom Lane wrote: >> Is this patch reversed? It seems so but the listed file timestamps >> don't match that idea ... > Sorry, I cannot understand what you mean... The patch looks like it removes, rather than adds, your intended changes. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] remove redundant ownership checks
KaiGai Kohei wrote: (2010/01/14 4:54), Tom Lane wrote: Robert Haas writes: On Wed, Jan 13, 2010 at 1:34 PM, Tom Lane wrote: If I thought this patch represented incremental movement in the direction of a better security-check factorization, I'd be fine with it, but that's not clear either. �The argument for it is that these checks are redundant with some other ones, but why should we remove these and not the other ones instead? That's a good question, and I have an answer [ namely that ALTER TABLE is the right place ]. But note Stephen Frost's concurrent reply suggesting that he wants to move the checks *out* of ALTER TABLE. With his plan, these checks are probably in the right place already. Note that this patch tries to remove redundant checks in this code path. If ATPrepCmd() would not be a right place to apply permission checks we should remove invocation of the ATSimplePermissions()... I'm glad to see this discussion thread, because I think it's really getting to the heart of the core issues that have made development in this area (like SEPostgreSQL) more complicated than it needs to be. I just talked with Stephen for a bit tonight to try and get my head wrapped around what you're all trying to do, and from what I gather the plan here that's taking shape looks like this: 1) Reach an agreement of the preferred way to handle permissions checks in these situations where there is more than one such check going on, and therefore a direction to consolidate all of them toward 2) Update the messy ALTER TABLE code to use that preferred method 3) Find another messy chunk of code and confirm the same style of refactoring can be applied to it as well (Stephen suggested ALTER TYPE as a candidate here) 4) Finish up converting any other ALTER commands that have split checks in them, since ALTER seems to be operation that's most prone to this type of issue 5) Confirm that the permissions checks in the major operations with simpler permissions related code paths (CREATE etc.) are also free of split checks 6) Add an extended alternate permissions checker to the now consolidated code, such as considering security labels. If the previous steps were done right, this should have a smaller logical and diff footprint than previous such efforts because it will touch many less places. Tom's objection to this patch is that without at least a general idea what form (2) through (4) (or similar incremental steps) intend to take, you don't want to touch just (2) lest you do something that only make sense for it--but not the later work. The patch being discussed here is a first step of the work needed for (2). However, it seems pretty clear to me that there's not even close to an agreement about step (1) here yet. Let me quote a few bits out of context to highlight: KaiGai: "...it is quite natural to check permission to modify properties of relations in ATPrepCmd" Robert: "Most of the ALTER TABLE operations use ATSimplePermissions() or ATSimplePermissionsRelationOrIndex() to check permissions...what I have in mind is to modify ATPrepCmd...". Stephen: "I think a patch which attacks ATPrepCmd and rips out all of the owner checks from it and moves them to appropriate places...would probably be the first step...At the moment we do a 'simple' check in ATPrepCmd (essentially, ownership of the relation) and then any more complicated checks have to be done by the function...this patch isn't doing that because it was intended to make the existing code consistant, not institute a new policy for how permissions checking should be done." (Apologies if a fragment or two of the above aren't in the archives, I think I grabbed a bit from one of the off-list messages in my mailbox while assembling). I've looked at this for a little bit, and I sure can't tell who's right here. What I am sure of though is that even a majority here isn't going to fly. If we don't even have all three of you guys lined up in the same direction on something this small, there's little hope of getting the whole community sold on this already controversial issue. Tom said back on 12/17 that "we need a very well-defined notion of where permissions checks should be made", the thing I pulled out as (1) above. The discussion around that topic has been going on here quite regularly now for almost a month, and these little quoted bits highlight that opinion is still quite split. Please keep hammering away at this little piece; I think it's really important to set a good example here. But the preference of the last CF is to not apply any patch which doesn't have a very clear justification to be committed. Given that whether this patch is applied or not to 8.5 really doesn't make any functional difference, I don't see anywhere for this to go right now except for "Returned with Feedback". It's extremely valuable to have had this patch submitted. I don't believe an exact spot of contention
Re: [HACKERS] [BUG?] strange behavior in ALTER TABLE ... RENAME TO on inherited columns
The attached patch fixes bugs when we try to rename (and change type) on a column inherited from the different origin and merged. This patch adds: List *find_column_origin(Oid relOid, const char *colName) It returns the list of relation OIDs which originally defines the given column. In most cases, it returns a list with an element. But, if the column is inherited from multiple parent relations and merged during the inheritance tree, the returned list contains multiple OIDs. In this case, we have to forbid changing type and renaming to keep correctness of the table definition. Example) postgres=# CREATE TABLE t1 (a int, b int); CREATE TABLE postgres=# CREATE TABLE s1 (b int, c int); CREATE TABLE postgres=# CREATE TABLE ts (d int) INHERITS (t1, s1); NOTICE: merging multiple inherited definitions of column "b" CREATE TABLE postgres=# ALTER TABLE t1 ALTER a TYPE text; ALTER TABLE postgres=# ALTER TABLE t1 ALTER b TYPE text; ERROR: cannot alter multiple inherited column "b" < (*) (*) ts.b is also inherited from s1, not only t1, so forbid changing type postgres=# ALTER TABLE s1 RENAME c TO cc; ALTER TABLE postgres=# ALTER TABLE s1 RENAME b TO bb; ERROR: cannot rename multiple inherited column "b" < (*) (*) ts.b is also inherited from s1, not only t1, so forbid renaming postgres=# \d+ ts Table "public.ts" Column | Type | Modifiers | Storage | Description +-+---+--+- a | text| | extended | b | integer | | plain| cc | integer | | plain| d | integer | | plain| Inherits: t1, s1 Has OIDs: no In the case when a column is inherited from multiple relations but it eventually has same origin (diamond inheritance tree case), we don't need to forbid these operations. In this case, find_column_origin() does not return duplicated OIDs, all the caller has to do is to check whether length of the returned list is larger than 1, or no. Thanks, (2010/01/14 12:43), KaiGai Kohei wrote: > The similar matter can be reproduced with ALTER TABLE ... TYPE statement, > not only RENAME TO option. > >postgres=# CREATE TABLE t1 (a int); >CREATE TABLE >postgres=# CREATE TABLE s1 (a int); >CREATE TABLE > >postgres=# CREATE TABLE ts (b int) inherits (t1, s1); >NOTICE: merging multiple inherited definitions of column "a" >CREATE TABLE >postgres=# ALTER TABLE t1 ALTER a TYPE text; >ALTER TABLE > >postgres=# insert into t1 values ('aaa'); >INSERT 0 1 >postgres=# insert into ts values ('bbb', 2); >INSERT 0 1 >postgres=# SELECT * FROM t1; > a >- > aaa > bbb >(2 rows) > >postgres=# SELECT * FROM s1; >ERROR: attribute "a" of relation "ts" does not match parent's type > > In the renameatt(), we can count an expected inhcount of the column to be > renamed on find_all_inheritors() at the top-level recursion. > But it does not work well for the new one, because it is handled within > the common ATPrepCmd() scheme. > > I reconsidered that we need a function to check whether the given column > is inherited from multiple root parents, or not, for each levels. > Then, it can be called from both of renameatt() and ATPrepAlterColumnType(). > > > (2010/01/04 18:55), KaiGai Kohei wrote: The method I suggested would allow the necessary information to be extracted during the initial search for child tables, which we have to do anyway. >>> >>> find_all_inheritors() returns a clean list which does not contain >>> duplicated OID of the inherited relation, so it seems to me we need >>> to change the function prototype but it affects other parts, or to add >>> a new function which also returns number of duplications, not only OIDs. >>> >>> Or, we can call find_inheritance_children() in renameatt() as if >>> find_all_inheritors() doing except for list_concat_unique_oid(). >> >> The attached patch modified the condition to prevent renaming. >> >> It computes an expected inhcount for each child relations on the initial >> call of the renameatt() for the parent relation. >> The find_all_inheritors_with_inhcount() returns OID of the inherited >> relations and the expected inhcoundt. If a child relation has diamond >> inheritance tree, it has its expected inhcount larger than 1. >> >> This patch raises an error, if pg_attribute.inhcount is larger than >> the expected inhcount. It can be happen when the attribute to be >> renamed is merged from any other unrelated relations in the child >> relations. >> >> See the example: >> >> postgres=# CREATE TABLE t1 (a int); >> CREATE TABLE >> postgres=# CREATE TABLE t2 (b int) inherits (t1); >> CREATE TABLE >> postgres=# CREATE TABLE t3 (c int) inherits (t1); >> CREATE TABLE >> postgres=# CREATE TABLE t4 (d int) inherits (t2, t3); >> NOTICE: merging multip
Re: [HACKERS] plpython3
James William Pye wrote: On Jan 13, 2010, at 2:27 PM, Peter Eisentraut wrote: The problem I'm having with this discussion is that every time someone asks what the supposed advantages of this new Python PL are, a feature list like the above is dumped, I agree that this is unfortunate, but how else can we to discuss the advantages? It boils down to comparing a couple feature lists, and *maybe* some implementation details. No? Code samples. You're trying to unseat a well established incumbent here, and you're not ever going to do that with documentation. Maybe your plpython3 has some compelling features to it. I don't know, because even with several thousand lines of basic Python code to my credit I cannot understand a single one of the arguments you presented for why your implementation is better--except agreeing that, yes, tracebacks are useful And even on that one, I'm not going to take your word on the superiority of your implementation. You're writing way over people's heads here. (Doesn't help that your docs link at the bottom of http://wiki.postgresql.org/wiki/WIP:plpython3 is broken either). If one has to be a Python expert to understand your position, you've already lost. Python code is easy to read though. If you'd said "here's a great example of how Function Modules are an improvement over what you can do with the current pl/python," that would be infinitely more useful than the list of language trivia related to them. You should be aiming to put Peter on the spot to respond to claims you make like "you can't do this easily with the current implementation" after showing an elegant bit of code. One of the things I'm increasingly frustrated by (and don't take this personally, this is a general comment coming more from the last CF rather than something I mean to single you out for) is how many patch submissions we get that don't have *compelling* examples showing their value. Have a better programming approach to something? Show me the old way and how the new way is better. Performance improvement? Provide a complete, self-contained example showing how to demonstrate it. New type of feature? Cut and paste a whole session showing how it's used, with every single command you typed after initdb. Basically, if a reviewer can't confirm your patch is doing something useful in five minutes and be excited that they've watched something interesting happen, you're decreasing the chances that your patch will ever go anywhere dramatically. I hope that everyone submitting patches reads http://www.depesz.com/ at least once in a while. One of the things I really enjoy about his blog is how he shows complete working examples of so many patches. To pick a standout recent entry, http://www.depesz.com/index.php/2010/01/03/waiting-for-8-5-exclusion-constraints/ takes "exclusion constraints"--a feature I didn't follow a bit of the discussion about--and works through the whole feature with a series of examples that, while still complicated, are completely self-contained and possible to follow along until you understand how it all fits together. Patch submitters should consider it a goal to make life that easy for the reviewer stuck with checking their patch out. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com
Re: [HACKERS] plpgsql: open for execute - add USING clause
2010/1/14 Takahiro Itagaki : > > Pavel Stehule wrote: > >> ok, I accept all comments. >> revised version are attached. > > Good. This patch is ready to commit. I'll do it soon if no objections. > > BTW, I found inconsistent parameter dumps in the codes. Some of them > add '$', but others does not. Are they intentional? Or, should we > adjust them to use one of the formats? > > [pl_funcs.c] > dump_dynexecute() > dump_raise() > printf(" parameter %d: ", i++); > dump_dynfors() > dump_open() > dump_return_query() > printf(" parameter $%d: ", i++); > isn't parameter of raise statement different than query parameter? I thing so $x convention respects parameter holder syntax. Regards Pavel > > Regards, > --- > Takahiro Itagaki > 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] mailing list archiver chewing patches
Hi, 3) A nice set of SQL queries to return message, parts, threads, folders based on $criteria (search, id, folder, etc) I guess Matteo's working on that… Right, but this is where I want to see the AOX schema "imporove"... In ways like adding persistant tables for threading, which are updated by triggers as new messages are delivered, etc. Documented queries that show how to use CTEs, ltree, etc to get threaded views, good FTS support (with indexes and triggers managing them), etc. +1. I just didn't understand how much your proposal fit into current work :) I'm looking into it. The link I've previously sent will most likely return a 500 error for the time being. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers