Re: [HACKERS] enhanced error fields
2012/7/2 Peter Geoghegan pe...@2ndquadrant.com: On 2 July 2012 15:19, Peter Geoghegan pe...@2ndquadrant.com wrote: On 9 May 2012 14:33, Pavel Stehule pavel.steh...@gmail.com wrote: here is patch with enhancing ErrorData structure. Now constraints errors and RI uses these fields So I took a look at the patch eelog-2012-05-09.diff today. All of the following remarks apply to it alone. I decided to follow through and take a look at eelog-plpgsql-2012-05-09.diff today too, while I have all of this swapped into my head. This patch is not an atomic unit - it builds upon the first patch. I successfully merged the local feature branch that I'd created for eelog-2012-05-09.diff without any merge conflicts, and I can build Postgres and get the regression tests to pass (including a couple of new ones, for this added functionality for plggsql - the functionality is testing exclusively using the new (9.2) get stacked diagnostics and raise custom exception 'some_custom_exception' using feature). Since that feature branch had all my revisions committed, my observations about redundancies in the other base patch still stand - the 2 functions mentioned did not exist for the benefit of this further patch either. There is a typo here: + case PLPGSQL_RAISEOPTION_TRIGGER_SCHEMA: + printf( TRIGGER_SCHENA = ); + break; } I'm not sure about this inconsistency within unreserved_keyword: For routines: + | K_DIAG_ROUTINE_NAME + | K_DIAG_ROUTINE_SCHEMA For triggers: + | K_DIAG_TRIGGER_NAME + | K_DIAG_TRIGGER_SCHEMA For tables: + | K_DIAG_SCHEMA_NAME . . **SNIP** . + | K_DIAG_TABLE_NAME The same inconsistency exists within the anonymous enum that contains PLPGSQL_GETDIAG_TABLE_NAME (and other constants), as well as the new token keywords within plpgsql's gram.y . The doc changes need a little work here too. I'm not sure that I agree with the extensive use of the term routine in all of these constants - sure, information_schema has a view called routines. But wouldn't it be more appropriate to use a Postgres-centric term within our own code? So, what about the concern about performance taking a hit when plpgsql exception blocks are entered as a result of this patch? Well, while I think that an effort to reduce the overhead of PL exception handling would be worthwhile, these patches do not appear to alter things appreciable (though the overhead *is* measurable): [peter@peterlaptop eelog]$ ls exceptions.sql test_eelog_outer.sql Patch (eelog-plpgsql): [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n transaction type: Custom query scaling factor: 1 query mode: simple number of clients: 10 number of threads: 1 duration: 300 s number of transactions actually processed: 305756 tps = 1019.026055 (including connections establishing) tps = 1019.090135 (excluding connections establishing) Master: [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n transaction type: Custom query scaling factor: 1 query mode: simple number of clients: 10 number of threads: 1 duration: 300 s number of transactions actually processed: 308376 tps = 1027.908182 (including connections establishing) tps = 1027.977879 (excluding connections establishing) An archive with simple scripts for repeating this are attached, if anyone is interested. yes, I think so slowdown is not significant for usual situations and patterns. I got about 3% slowdown on code like begin insert into tab -- raise exception exception when others .. ... end; and only when all inserts fails. Regards Pavel -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte
OK. So, in that case, I suggest that if the leading byte is non-zero, we emit 0x9d followed by the three available bytes, instead of first testing whether the first byte is = 0xf0. That test seems to serve no purpose but to confuse the issue. Probably the code shoud look like this(see below comment): else if (lb = 0xf0 lb = 0xfe) { if (lb = 0xf4) *to++ = 0x9c; else *to++ = 0x9d; *to++ = lb; *to++ = (*from 8) 0xff; *to++ = *from 0xff; cnt += 4; I further suggest that we improve the comments on the mule functions for both wchar-mb and mb-wchar to make all this more clear. I have added comments about mule internal encoding by refreshing my memory and from old document found on web(http://mibai.tec.u-ryukyu.ac.jp/cgi-bin/info2www?%28mule%29Buffer%20and%20string). Please take a look at. BTW, it seems conversion between multibyte and wchar can be roundtrip in the leading character is LCPRV2 case: If the second byte of wchar (out of 4 bytes of wchar. The first byte is always 0x00) is in range of 0xf0 to 0xf4, then the first byte of multibyte must be 0x9c. If the second byte of wchar is in range of 0xf5 to 0xfe, then the first byte of multibyte must be 0x9d. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index d456309..1148eb5 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -37,6 +37,31 @@ typedef unsigned int pg_wchar; #define ISSJISTAIL(c) (((c) = 0x40 (c) = 0x7e) || ((c) = 0x80 (c) = 0xfc)) /* + * Currently PostgreSQL supports 5 types of mule internal encodings: + * + * 1) 1-byte ASCII characters, each byte is below 0x7f. + * + * 2) Official single byte charsets such as ISO 8859 latin1. Each + *mule character consists of 2 bytes: LC1 + C1, where LC1 is + *corresponds to each charset and in range of 0x81 to 0x8d and C1 + *is in rage of 0xa0 to 0xff(ISO 8859-1 for example, plus each + *high bit is on). + * + * 3) Private single byte charsets such as SISHENG. Each mule + *character consists of 3 bytes: LCPRV1 + LC12 + C1 where LCPRV1 + *is either 0x9a (if LC12 is in range of 0xa0 to 0xdf) or 0x9b (if + *LC12 is in range of 0xe0 to 0xef). + * + * 4) Official multibyte charsets such as JIS X0208. Each mule + *character consists of 3 bytes: LC2 + C1 + C2 where LC2 is + *corresponds to each charset and is in rage of 0x90 to 0x99. C1 + *and C2 is in rage of 0xa0 to 0xff(each high bit is on). + * + * 5) Private multibyte charsets such as CNS 11643-1992 Plane 3. + *Each mule character consists of 4 bytes: LCPRV2 + LC22 + C1 + + *C2. where LCPRV2 is either 0x9c (if LC12 is in range of 0xf0 to + *0xf4) or 0x9d (if LC22 is in range of 0xf5 to 0xfe). + * * Leading byte types or leading prefix byte for MULE internal code. * See http://www.xemacs.org for more details. (there is a doc titled * XEmacs Internals Manual, MULE Character Sets and Encodings -- 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] SP-GiST for ranges based on 2d-mapping and quad-tree
On Thu, 2012-06-14 at 02:56 +0400, Alexander Korotkov wrote: Hackers, attached patch implements quad-tree on ranges. Some performance results in comparison with current GiST indexing. Index creation is slightly slower. Probably, it need some investigation. Search queries on SP-GiST use much more pages. However this comparison can be not really correct, because SP-GiST can pin same buffer several times during one scan. In CPU search queries on SP-GiST seems to be slightly faster. Dramatical difference in column @ const query is thanks to 2d-mapping. More comments: * Minor rebase is required (simple int2 - int16). * Perhaps I'm mistaken, but the following code in getQuadrant() looks wrong to me, shouldn't the 1 and 2 be reversed? if (range_cmp_bounds(typcache, upper, centroidUpper) = 0) return 1; else return 2; * in the choose method, why does in-allTheSame unconditionally match? Why not split? Similarly, why does inner_consistent always match when the nodes are allTheSame? * It's a little confusing having empty prefixes mean that empty range go to node0, and non-empty ranges meaning that empty ranges go to node4 (quadrant 5). Why can't there just always be 5 nodes, and iff all the ranges are empty, then the prefix is NULL? And for that matter, let's let the quadrant equal the node number, and have the empty ranges in node0. I don't see much point in always subtracting 1 from the quadrant number. Regards, Jeff Davis -- 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] SP-GiST for ranges based on 2d-mapping and quad-tree
On Mon, 2012-07-02 at 23:47 -0700, Jeff Davis wrote: On Thu, 2012-06-14 at 02:56 +0400, Alexander Korotkov wrote: Hackers, attached patch implements quad-tree on ranges. Some performance results in comparison with current GiST indexing. Index creation is slightly slower. Probably, it need some investigation. Search queries on SP-GiST use much more pages. However this comparison can be not really correct, because SP-GiST can pin same buffer several times during one scan. In CPU search queries on SP-GiST seems to be slightly faster. Dramatical difference in column @ const query is thanks to 2d-mapping. Also, it would be helpful to add a couple tests to rangetypes.sql. Regards, Jeff Davis -- 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] Proof of concept: auto updatable views
On 2 July 2012 21:34, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jul 1, 2012 at 6:35 PM, Dean Rasheed dean.a.rash...@gmail.com wrote: Basically what it does is this: in the first stage of query rewriting, just after any non-SELECT rules are applied, the new code kicks in - if the target relation is a view, and there were no unqualified INSTEAD rules, and there are no INSTEAD OF triggers, it tests if ... The consensus last time seemed to be that backwards compatibility should be offered through a new GUC variable to allow this feature to be disabled globally, which I've not implemented yet. I think the backward-compatibility concerns with this approach would be much less than with the previously-proposed approach, so I'm not 100% sure we need a backward compatibility knob. If the above description is correct, the behavior is changed only in cases that previously threw errors, so I tend to agree that no backwards compatibility knob is needed. Yeah, I think you're right - the default ACLs will typically give sensible behaviour. So unless users have been cavalier with the use of GRANT ALL, their existing views are only going to start becoming updatable to the owners of the views (and only then if the view owner already has write permission on the underlying table). Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgsql_fdw in contrib
Hanada-san, Regarding to the issue around sub-transaction abort, an ideal solution might be execution of SAVEPOINT command on remote side synchronously. It allows to rollback the active transaction into the savepoint on the remote server when local one get rolled-back. However, I'm not inclined to merge anything ideal within a single patch, and I'd like to recommend to keep your idea simple then revise the functionality on the upcoming three commit fest. How about your opinion? The author is you. 2012/6/26 Kohei KaiGai kai...@kaigai.gr.jp: Harada-san, I checked your patch, and had an impression that includes many improvements from the previous revision that I looked at the last commit fest. However, I noticed several points to be revised, or investigated. * It seems to me expected results of the regression test is not attached, even though test cases were included. Please add it. * cleanup_connection() does not close the connection in case when this callback was invoked due to an error under sub- transaction. It probably makes problematic behavior. See the following steps to reproduce the matter: postgres=# BEGIN; BEGIN postgres=# SELECT * FROM ft3; a | b ---+- 1 | aaa 2 | bbb 3 | ccc 4 | ddd 5 | eee 6 | fff (6 rows) postgres=# SAVEPOINT sv_01; SAVEPOINT postgres=# SELECT * FROM ft3 WHERE 1 / (a - 4) 0; -- should be error on remote transaction ERROR: could not execute remote query DETAIL: ERROR: division by zero HINT: SELECT a, b FROM public.t1 WHERE (((1 OPERATOR(pg_catalog./) (a OPERATOR(pg_catalog.-) 4)) OPERATOR(pg_catalog.) 0)) postgres=# ROLLBACK TO SAVEPOINT sv_01; ROLLBACK postgres=# SELECT * FROM ft3; ERROR: could not execute EXPLAIN for cost estimation DETAIL: ERROR: current transaction is aborted, commands ignored until end of transaction block HINT: SELECT a, b FROM public.t1 Once we got an error under the remote transaction, it eventually raises an error on local side, then cleanup_connection() should be invoked. But it ignores the error due to sub-transaction, thus, the remote transaction being already aborted is kept. I'd like to suggest two remedy. 1. connections are closed, even if errors on sub-transaction. 2. close the connection if PQexecParams() returns an error, on execute_query() prior to raise a local error. * Regarding to deparseSimpleSql(), it pulls attributes being referenced from baserestrictinfo and reltargetlist using pull_var_clause(). Is it unavailable to use local_conds instead of baserestrictinfo? We can optimize references to the variable being consumed at the remote side only. All we need to transfer is variables referenced in targetlist and local-clauses. In addition, is pull_var_clause() reasonable to list up all the attribute referenced at the both expression tree? It seems to be pull_varattnos() is more useful API in this situation. * Regarding to deparseRelation(), it scans simple_rte_array to fetch RangeTblEntry with relation-id of the target foreign table. It does not match correct entry if same foreign table is appeared in this list twice or more, like a case of self-join. I'd like to recommend to use simple_rte_array[baserel-relid] instead. In addition, it checks whether relkind is RELKIND_FOREIGN_TABLE, or not. It seems to me this check should be Assert(), if routines of pgsql_fdw is called towards regular relations. * Regarding to deparseVar(), is it unnecessary to check relkind of the relation being referenced by Var node, isn't it? * Regarding to deparseBoolExpr(), compiler raised a warning because op can be used without initialized. * Even though it is harmless, sortConditions() is a misleading function name. How about categolizeConditions() or screeningConditions()? Thanks for your great jobs. 2012/6/14 Shigeru HANADA shigeru.han...@gmail.com: I'd like to propose pgsql_fdw, FDW for PostgreSQL, as a contrib module in core, again. This patch is basically rebased version of the patches proposed in 9.2 development cycle, and contains some additional changes such as concern about volatile variables for PG_CATCH blocks. In addition, I combined old three patches (pgsql_fdw core functionality, push-down support, and analyze support) into one patch for ease of review. (I think these features are necessary for pgsql_fdw use.) After the development for 9.2 cycle, pgsql_fdw can gather statistics from remote side by providing sampling function to analyze module in backend core, use them to estimate selectivity of WHERE clauses which will be pushed-down, and retrieve query results from remote side with custom row processor which prevents memory exhaustion from huge result set. It would be possible to add some more features, such as ORDER BY push-down with index information support, without changing existing APIs, but at first
Re: [HACKERS] [DOCS] File format for SSL CRL file
On Tuesday, July 3, 2012, Alvaro Herrera wrote: Excerpts from Greg Smith's message of lun jul 02 20:30:07 -0400 2012: A documentation comment came in recently about ssl-tcp.html not specifying what format is expected for the CRL file. Seems like something that could be described better now that I look at it, so I'm passing that along with just wording edits from me; this is from user oneironautics: The root.crl needs to be in PEM (and not DER) format. If a certificate file exists but is the wrong type, you will be told it cannot find the file when it exists, with this sort of error in the log: LOG: SSL certificate revocation list file root.crl not found, skipping: no SSL error reported HEAD is different in this area -- it dies with a FATAL instead of just skipping it. Yes, and if somebody forgot, that was an intentional change :) Also, the error message seems rather poor. Maybe the code should call X509_STORE_CTX_get_error() instead of SSLerrmessage (which calls ERR_get_error; apparently not the right thing to do). I don't see how that would work - X509_STORE_CTX_get_error() takes an X509_STORE_CTX as parameter ,and we don't have one of those. And unfortunately the function we use to load the store seems to be undocumented, so it's hard to know what we're supposed to use.. (I do agree we should try to figure out a better error message, of course..) //Magnus -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
[HACKERS] xlog filename formatting functions in recovery
Hello, I've noticed recently that I can't seem to use the convenient xlog filename formatting functions while I'm in a standby. I don't see an incredibly obvious reason why that is the case, so here's a patch that simply removes the ban on being able to call these formatting functions. Perhaps I'm missing something, but it feels like this is an oversight from the days when there was no useful function to call that formatted a X/X-type record on a standby. -- fdr Make-xlog-filename-formatters-usable-while-in-recovery-v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlog filename formatting functions in recovery
On Tue, Jul 3, 2012 at 1:24 AM, Daniel Farina dan...@heroku.com wrote: Hello, I've noticed recently that I can't seem to use the convenient xlog filename formatting functions while I'm in a standby. I don't see an incredibly obvious reason why that is the case, so here's a patch that simply removes the ban on being able to call these formatting functions. Perhaps I'm missing something, but it feels like this is an oversight from the days when there was no useful function to call that formatted a X/X-type record on a standby. (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) -- fdr -- 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] xlog filename formatting functions in recovery
On 03/07/12 20:24, Daniel Farina wrote: Hello, I've noticed recently that I can't seem to use the convenient xlog filename formatting functions while I'm in a standby. I don't see an incredibly obvious reason why that is the case, so here's a patch that simply removes the ban on being able to call these formatting functions. Perhaps I'm missing something, but it feels like this is an oversight from the days when there was no useful function to call that formatted a X/X-type record on a standby. I must add a +1 to that, gotta be my pet peeve with this otherwise fine functionality. Also it would be really nice to have a single function that gave the current receive or active wal offset so things like Nagios didn't have to know the if db is a standby or not to work out said offset... Regards Mark
Re: [HACKERS] pl/perl and utf-8 in sql_ascii databases
Hello, Here is regression test runs on pg's also built with cygwin-gcc and VC++. The patches attached following, - plperl_sql_ascii-4.patch : fix for pl/perl utf8 vs sql_ascii - plperl_sql_ascii_regress-1.patch : regression test for this patch. I added some tests on encoding to this. I will mark this patch as 'ready for committer' after this. For the continuity of the behavior for sql_ascii and the chars like \x80, It might be better if the main patch is back ported into 9.1 and 9.2. New regression tests seems to have less necessity to do since it has not been there from the first.. This regression test runs for all of Build with gcc3 / Linux(CentOS6.2-64) Built with Cygwin gcc3 / Windows7-64 Built with VC++2008 / ActivePerl5.12 / Windows7-64 == I've been stuck in mud trying to plperl work on windows environment. I saw many messages complaining that plperl wouldn't be built to work. For the convenience of those and myself, I describe the process of building postgresql with plperl on Windows with cygwin and VC++ I've done below. Ok. Since there found to be only two patterns in the regression test. The fancy thing is no more needed. I will unfold them and make sure to work on mingw build environment. And for one more environment, on the one with VC++.. I'll need a bit longer time to make out what vcregress.pl does. I could understand what you meant after I managed to build plperl to run properly. vcregress.pl reads $REGRESS in GNUmakefile so variable substitution of make does not work on Windows' regression. I resolved this problem by copying plperl_lc_*.out files into plperl_lc.out before it runs pg_regress. - The main patch fixes the sql-ascii handling itself shoud ported into 9.2 and 9.1. Someone shoud work for this. (me?) done. - The remainder of the patch whic fixes the easy fixable leakes of palloc'ed memory won't be ported into 9.1. This is only for 9.3dev. - The patch for 9.3dev will be provided with the new regression test. It will be easily ported into 9.1 and 9.2 and there seems to be no problem technically, but a bit unsure from the other points of view... What should I do for this? regards, Addition - Building Windows binary for plperl NOTE: This is ONE example I tried and turned out a success. A. Cygwin Versions: Windows 7 64bit Cygwin 1.7.15 gcc 3.4.4 (cygwin-server is running) 1. Build perl aside system-installed one perl-5.16.0$ export PATH=/usr/local/bin:/bin:/usr/bin:/usr/sbin perl-5.16.0$ ./Configure -- perl-5.16.0$ ./Configure -d perl-5.16.0$ make perl-5.16.0$ make install - The first line needed to avoid the Makefile of perl stops by parens in search path. 2. Build postgresql with --with-perl pg93dev$ ./configure --with-perl pg93dev$ make all 3. Run the regression test for plperl pg93dev/src/pl/plperl$ make check pg93dev/src/pl/plperl$ make check ENCODING=sql-ascii B. VC++ Versions: Windows 7 64bit Microsoft Visual C++ 2008 Express Edition Active Perl v6.12.4 x86 1. Install Active Perl normally. Assuming the install location is c:\Perl and I did all operation on cmd.exe after this. 2. Create config.pl in src/tools/msvc pg cd src\tools\msvc msvc copy config_default.pl config.pl Edit as follows - perl=undef,# --with-perl + perl='c:\Perl',# --with-perl 3. Build it msvc C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\bin\vcvars32.bat msvc build msvc install c:\pgsql 4. Run the regression tests 4.1 Create test database and run postgres for UTF8 test A set PATH=c:\pgsql\bin;c:\pgsql\lib;%PATH% A initdb -D pgdata dir --no-locale --encoding=utf8 A postgres -D pgdata dir ... on another cmd.exe B set PATH=c:\pgsql\bin;c:\pgsql\lib;%PATH% pg cd src\tools\msvc msvc vcregress plcheck 4.2 Run regression test for SQL-ASCII. A (delete pgdata dir and its contents.) A initdb -D pgdata dir --no-locale --encoding=sql_ascii A postgres -D pgdata dir ... same as 4.1 here after... -- Kyotaro Horiguchi NTT Open Source Software Center == My e-mail address has been changed since Apr. 1, 2012. diff --git a/src/pl/plperl/Util.xs b/src/pl/plperl/Util.xs index 7d0102b..4b4b680 100644 --- a/src/pl/plperl/Util.xs +++ b/src/pl/plperl/Util.xs @@ -67,8 +67,11 @@ static text * sv2text(SV *sv) { char *str = sv2cstr(sv); + text *text; - return cstring_to_text(str); + text = cstring_to_text(str); + pfree(str); + return text; } MODULE = PostgreSQL::InServer::Util PREFIX = util_ @@ -113,10 +116,12 @@ util_quote_literal(sv) } else { text *arg = sv2text(sv); -text *ret = DatumGetTextP(DirectFunctionCall1(quote_literal, PointerGetDatum(arg))); - char *str = text_to_cstring(ret); - RETVAL =
Re: [HACKERS] Event Triggers reduced, v1
Robert Haas robertmh...@gmail.com writes: Given what I foresee, simply having another columns in there named evtstags with the exact same content as evttags would be the simplest and most natural implementation, really. That seems a lot less general for no particular gain. The gain is code, docs and usage simplification. I think going general here is going to confuse every one involved and that we should have two targets in mind: classic use cases that we want to address easily enough in SQL and with some PLpgSQL help, and advanced use cases that are possible to implement in PL/C using the parse tree and the soon to come back rewritten command string. IOW, let's make the simple things simple and the complex one possible. The following is quite long an email where I try to give plenty of examples and to detail the logic I'm working with so that you can easily stab at whichever part you're thinking is not going to fly. I don't foresee more generic needs here, unless you can convince me that we need both a. a full stack of arbitrarily nested commands and b. a way to match and target any level of that stack. Well, let's take the example of an ALTER TABLE command. You could want to match on: - the type of object the user mentioned in the command (did he write ALTER TABLE or ALTER VIEW?) - the type of object actually being modified (could differ if he used I don't think it's possible to implement that without shaking all the system, after having a look at the following lines from gram.y: ALTER VIEW qualified_name alter_table_cmds { AlterTableStmt *n = makeNode(AlterTableStmt); So, the way to implement that need from an event trigger is to use the parse tree, and hopefully soon enough the rewritten command string. ALTER TABLE on a view, or visca versa) - the particular ALTER TABLE subcommand in use (e.g. SET STATISTICS) Now we can publish that, we would have some events with tag = 'ALTER TABLE' then some others with toplevel = 'ALTER TABLE' tag = 'SET STATISTICS' The same idea would need to get implemented for serial, where the tag is 'CREATE SEQUENCE' and the toplevel tag is 'CREATE TABLE'. That allows to easily install an event trigger that gets called every time a sequence is created, you can then have a look at the toplevel command tag if you need to. CREATE EVENT TRIGGER snitch_seqs ON command_start WHEN tag IN ('CREATE SEQUENCE') EXECUTE PROCEDURE snitch_seqs(); The idea is that the function snitch_seqs has a magic variable TG_TOPLEVEL that can be tested and will be set to 'CREATE TABLE' when we're dealing with a SERIAL, in that example. If you want your event trigger to only ever deal with SERIAL, you could install it this way: CREATE EVENT TRIGGER my_serial_trigger ON command_start WHEN toplevel IN ('CREATE TABLE') AND tag IN ('CREATE SEQUENCE') EXECUTE PROCEDURE handle_serial_trigger(); Now let's see about getting more generic than that. We also can get tag = 'CREATE INDEX' and toplevel = 'ALTER TABLE' when adding a primary key for example. That's an example that can lead us to more than 2 levels of nested tags, which I would want to avoid. The stack here would look like: 1. ALTER TABLE 2. ADD PRIMARY KEY 3. CREATE INDEX I think only having 1 and 3 is enough, for more details the command string and the parse tree are available. In the main use case of replication, you mostly just want to replicate the command string. You might want to apply some transformation rules to it (table names, cope with a different target schema, etc) but typically those rules are to be run in the subscriber system, not on the provider (picture a federating system where each provider uses the same schema, that gets mapped to a schema per provider on the subscriber). The other problem with the stack of tags is matching them. I don't see that it helps writing event triggers much. In the previous example, if you want an event trigger that fires on each ALTER TABLE, you don't know which level of the stack to target. Either you have to target the current tag or the toplevel tag or something in between. We could easily have the following tag stack: 1. CREATE SCHEMA 2. ALTER TABLE 3. ADD PRIMARY KEY 4. CREATE INDEX So now we need a way to target any entry in the stack and a way to represent the stack in every PL language we have, and an easy way to analyze the stack. For PLpgSQL I guess that means we want to expose this tag stack as a TABLE, and the complexity just went off the table. My view point is that for any practical case I can think about what we care about is the current command being run, and given how PostgreSQL is made today that means handling one level of sub commands. That addresses ALTER TABLE and also DROP CASCADE. I don't think adding-in an ALTER TABLE that never happened in the middle of those two
Re: [HACKERS] xlog filename formatting functions in recovery
-Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Daniel Farina Sent: Tuesday, July 03, 2012 2:02 PM (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) It seems you have added it in current commit fest. Shouldn't it be added for next CF. With Regards, Amit Kapila. -- 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] huge tlb support
On Tuesday, July 03, 2012 05:18:04 AM Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jun 29, 2012 at 3:52 PM, Andres Freund and...@2ndquadrant.com wrote: In a *very* quick patch I tested using huge pages/MAP_HUGETLB for the mmap'ed memory. So, considering that there is required setup, it seems that the obvious thing to do here is add a GUC: huge_tlb_pages (boolean). We also need some logic to figure out how big the huge tlb size is... /sys/kernel/mm/hugepages/* contains a directory for each possible size. A bit unfortunately named though hugepages-2048kB. We need to parse that. The other alternative is to try with MAP_HUGETLB and, if it fails, try again without MAP_HUGETLB. +1 for not making people configure this manually. I don't think thats going to fly that well. You need to specifically allocate hugepages at boot or shortly thereafter. If postgres just grabs some of the available space without asking it very well might cause other applications not to be able to start. Were not allocating half of the system memory without asking either... Also, I was under the impression that recent Linux kernels use hugepages automatically if they can, so I wonder exactly what Andres was testing on ... At the time I was running the test I was running a moderately new kernel: andres@awork2:~$ uname -a Linux awork2 3.4.3-andres #138 SMP Mon Jun 19 12:46:32 CEST 2012 x86_64 GNU/Linux andres@awork2:~$ zcat /proc/config.gz |grep HUGE CONFIG_TRANSPARENT_HUGEPAGE=y CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS=y # CONFIG_TRANSPARENT_HUGEPAGE_MADVISE is not set CONFIG_HUGETLBFS=y CONFIG_HUGETLB_PAGE=y So, transparent hugepages are enabled by default. The problem is that the kernel needs 2MB of adjacent physical memory mapping to 2MB of adjacent virtual memory. In on-demand, cow virtual memory systems that just doesn't happen all the time if youre not doing file mmap while triggering massive readaheads. Especially if the system has been running for some time because the memory just gets too fragmented to have lots of adjacent physical memory around. There was/is talk about moving physical memory around to make room for more huge pages but thats not there yet and the patches I have seen incurred quite some overhead. Btw, the introduction of transparent hugepages advocated that there are still benefits in manual hugepage setups. Btw, should anybody want to test this: After boot you can allocate huge pages with: during runtime: echo 3000 /proc/sys/vm/nr_hugepages or at boot you can add a parameter: hugepages=3000 (allocates 6GB of huge pages on x86-64) The runtime one might take quite a time till it has found enough pages or even fall short. You can see the huge page status with: andres@awork2:~$ cat /proc/meminfo |grep Huge AnonHugePages:591872 kB HugePages_Total:3000 HugePages_Free: 3000 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlog filename formatting functions in recovery
On Tue, Jul 3, 2012 at 6:43 AM, Amit Kapila amit.kap...@huawei.com wrote: (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) It seems you have added it in current commit fest. Shouldn't it be added for next CF. Yep. The current CF has been closed to new submissions for two and a half weeks. On the substance of the patch, I believe the reason why this is currently disallowed is because the TLI is implicitly taken from the running system, and on the standby that might be the wrong value. I might be misremembering. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlog filename formatting functions in recovery
On Tuesday, July 3, 2012, Robert Haas wrote: On Tue, Jul 3, 2012 at 6:43 AM, Amit Kapila amit.kap...@huawei.comjavascript:; wrote: (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) It seems you have added it in current commit fest. Shouldn't it be added for next CF. Yep. The current CF has been closed to new submissions for two and a half weeks. Might this be something to consder for 9.2, though? It could be considered a bugfix. On the substance of the patch, I believe the reason why this is currently disallowed is because the TLI is implicitly taken from the running system, and on the standby that might be the wrong value. I might be misremembering. That is, however, assuming that this part is not true. I don't recall for sure, but I have a feeling it might be correct. In which case a much bigger patch is needed, and definitely not something for 9.2... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Event Triggers reduced, v1
On Mon, Jul 2, 2012 at 11:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 2, 2012 at 6:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: Um, doesn't that require nonrectangular arrays? Doh. You're right: I keep forgetting that arrays have to be rectangular. Any suggestions on a sensible way to represent this? Are there likely to be enough entries that storage efficiency actually matters? If not, we could use a 2xN array of {key,allowed_value} pairs, that is {{thingy,item1},{thingy,item2},{otherthingy,foo},{otherthingy,bar}} Or perhaps push these out into a separate table, along the lines of oid key allowed_value and use an oidvector to list the selected values in a trigger entry? It seems likely that there will fairly commonly be many allowed values per key. However, the universe of legal keys will be quite small, probably a total of 2-4. So maybe the best representation is to have an a separate column for each key and store the array of legal values in it. That's more or less what Dimitri already has in his latest patch, except that after looking it over I'm inclined to think that we'd be better off storing the keys as text and translating to internal ID numbers when we read and cache the table, rather than storing the ID numbers in the table. That will make the catalog contents easier to read, and more importantly will mean that a change to the internal ID numbers need not be initdb-forcing. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] huge tlb support
On Tuesday, July 03, 2012 04:49:10 AM Robert Haas wrote: So, considering that there is required setup, it seems that the obvious thing to do here is add a GUC: huge_tlb_pages (boolean). The other alternative is to try with MAP_HUGETLB and, if it fails, try again without MAP_HUGETLB. What about huge_tlb_pages = off|try|on with try being the default? Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlog filename formatting functions in recovery
On Tue, Jul 3, 2012 at 8:16 AM, Magnus Hagander mag...@hagander.net wrote: On Tuesday, July 3, 2012, Robert Haas wrote: On Tue, Jul 3, 2012 at 6:43 AM, Amit Kapila amit.kap...@huawei.com wrote: (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) It seems you have added it in current commit fest. Shouldn't it be added for next CF. Yep. The current CF has been closed to new submissions for two and a half weeks. Might this be something to consder for 9.2, though? It could be considered a bugfix. On the substance of the patch, I believe the reason why this is currently disallowed is because the TLI is implicitly taken from the running system, and on the standby that might be the wrong value. I might be misremembering. That is, however, assuming that this part is not true. I don't recall for sure, but I have a feeling it might be correct. In which case a much bigger patch is needed, and definitely not something for 9.2... Even if we were to conclude that the argument about TLIs is not valid, I'd be very reluctant to slip something like this into 9.2, because we have no time left to recant if it later turns out that there's some other reason why it's not a good idea. Removing error checks is one of those things that you want to try to get done early in the release cycle, because the consequences are sometimes difficult to foresee, and you may not find out why it was a bad idea until users start complaining. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Oracle porting sample instr function
Greg Smith wrote: A web site doc comment from user skong today points out a small issue around the sample INSTR function given in plpgsql-porting.html that I can't confirm (none of those dirty Oracle instances here today), but it sounds legit. A look at Oracle's documentation on the INSTR function at http://docs.oracle.com/cd/B28359_01/olap.111/b28126/dml_functions_1103.h tm says that the 3rd input, position to start searching, cannot be zero. skong says that Oracle will just return a 0 if you give it that invalid input. The INSTR implementation in the docs will instead search backwards from the end of the string if you tell it to start at 0, same as if you gave it a negative input. I think it's therefore possible to get the plpgsql version to return a value in cases Oracle would instead return 0. Seems like a straightforward thing to confirm and change the sample to do differently; just have to add an explicit test for a 0 value of beg_index. I can confirm that Oracle returns 0 if the third argument to INSTR is 0. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] huge tlb support
On Tue, Jul 3, 2012 at 8:23 AM, Andres Freund and...@2ndquadrant.com wrote: On Tuesday, July 03, 2012 04:49:10 AM Robert Haas wrote: So, considering that there is required setup, it seems that the obvious thing to do here is add a GUC: huge_tlb_pages (boolean). The other alternative is to try with MAP_HUGETLB and, if it fails, try again without MAP_HUGETLB. What about huge_tlb_pages = off|try|on with try being the default? That seems reasonable to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Oracle porting sample instr function
On Tue, Jul 3, 2012 at 8:42 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: I can confirm that Oracle returns 0 if the third argument to INSTR is 0. Can someone provide a suitable doc patch? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Bloom filter for 9.2 ...
hello, some time ago oleg and teodor have posted a PostgreSQL version of bloom filters. as this appears to be a useful thing for many people i have ported this prototype to PostgreSQL 9.2. it seems to work as expected on OS X and Linux. as it is a contrib module it lacks xlog support. maybe some people can make use of this one. many thanks, hans bloom-0.4.tar.gz Description: GNU Zip compressed data -- Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: http://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] Posix Shared Mem patch
On Thu, Jun 28, 2012 at 11:26 AM, Robert Haas robertmh...@gmail.com wrote: Assuming things go well, there are a number of follow-on things that we need to do finish this up: 1. Update the documentation. I skipped this for now, because I think that what we write there is going to be heavily dependent on how portable this turns out to be, which we don't know yet. Also, it's not exactly clear to me what the documentation should say if this does turn out to work everywhere. Much of section 17.4 will become irrelevant to most users, but I doubt we'd just want to remove it; it could still matter for people running EXEC_BACKEND or running many postmasters on the same machine or, of course, people running on platforms where this just doesn't work, if there are any. Here's a patch that attempts to begin the work of adjusting the documentation for this brave new world. I am guessing that there may be other places in the documentation that also require updating, and this page probably needs more work, but it's a start. 2. Update the HINT messages when shared memory allocation fails. Maybe the new most-common-failure mode there will be too many postmasters running on the same machine? We might need to wait for some field reports before adjusting this. I think this is mostly a matter of removing the text that says fix this by reducing shme-related parameters from the relevant hint messages. 3. Consider adjusting the logic inside initdb. If this works everywhere, the code for determining how to set shared_buffers should become pretty much irrelevant. Even if it only works some places, we could add 64MB or 128MB or whatever to the list of values we probe, so that people won't get quite such a sucky configuration out of the box. Of course there's no number here that will be good for everyone. I posted a patch for this one last night. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company shmem-docs.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SP-GiST for ranges based on 2d-mapping and quad-tree
On Mon, 2012-07-02 at 23:47 -0700, Jeff Davis wrote: * Perhaps I'm mistaken, but the following code in getQuadrant() looks wrong to me, shouldn't the 1 and 2 be reversed? if (range_cmp_bounds(typcache, upper, centroidUpper) = 0) return 1; else return 2; Oops, looks like I was mistaken. The code looks fine to me now. Regards, Jeff Davis -- 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] User-Id Tracking when Portal was started
On Mon, Jul 2, 2012 at 10:55 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: The attached patch is delivered from the discussion around row-level access control feature. A problem Florian pointed out is refcursor declared in security definer function. Even though all the permission checks are applied based on privilege of the owner of security-definer function in case when it tries to define a cursor bound to a particular query, it shall be executed under the credential of executor. In the result, current_user or has_table_privilege() will return unexpected result, even if it would be used in as a part of security policy for each row. Why not just save and restore the user ID and security context unconditionally, instead of doing this kind of dance? + if (portal-userId != GetUserId()) + SetUserIdAndSecContext(portal-userId, portal-secCo + else + saveUserId = InvalidOid; -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH 08/16] Introduce the ApplyCache module which can reassemble transactions from a stream of interspersed changes
On Thu, Jun 28, 2012 at 12:04 PM, Andres Freund and...@2ndquadrant.com wrote: On Thursday, June 28, 2012 06:01:10 PM Robert Haas wrote: On Tue, Jun 26, 2012 at 8:13 PM, Andres Freund and...@2ndquadrant.com wrote: It even can be significantly higher than max_connections because subtransactions are only recognizable as part of their parent transaction uppon commit. I've been wondering whether sub-XID assignment was going to end up on the list of things that need to be WAL-logged to enable logical replication. It would be nicer to avoid that if we can, but I have a feeling that we may not be able to. I don't think it needs to. We only need that information during commit and we have it there. If a subtxn aborts a separate abort is logged, so thats no problem. The 'merging' of the transactions would be slightly easier if we had the knowledge from the get go but that would add complications again in the case of rollbacks. What do you think we need it? Well, I don't know for sure that we do. But, ultimately, I think we're going to find that applying the whole transaction at transaction-end is something that we need to optimize - so that we can apply the transaction as it happens and commit it at the end. For small transactions this is no big deal, but for big transactions it could lead to hours and hours of replication delay. I'm not sure whether it's practical to fix that in version 1 - certainly there are a lot of issues there - but ultimately I think it's something that our users are going to demand. And it seems like that might require knowing the transaction tree at some point before the commit record shows up. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] TODO list updated for 9.3
I have removed the completed 9.2 TODO items so people can start updating the TODO list completed items for 9.3. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Incorrect behaviour when using a GiST index on points
On Thu, Jun 21, 2012 at 2:53 PM, Alexander Korotkov aekorot...@gmail.com wrote: On Wed, Feb 22, 2012 at 5:56 PM, Alexander Korotkov aekorot...@gmail.com wrote: Attached patch fixes GiST behaviour without altering operators behaviour. I think we definitely should apply this patch before 9.2 release, because it is a bug fix. Otherwise people will continue produce incorrect GiST indexes with in-core geometrical opclasses until 9.3. Patch is very simple and only changes few lines of code. Any thoughts? Do we need to apply this patch to 9.2? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] User-Id Tracking when Portal was started
2012/7/3 Robert Haas robertmh...@gmail.com: On Mon, Jul 2, 2012 at 10:55 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote: The attached patch is delivered from the discussion around row-level access control feature. A problem Florian pointed out is refcursor declared in security definer function. Even though all the permission checks are applied based on privilege of the owner of security-definer function in case when it tries to define a cursor bound to a particular query, it shall be executed under the credential of executor. In the result, current_user or has_table_privilege() will return unexpected result, even if it would be used in as a part of security policy for each row. Why not just save and restore the user ID and security context unconditionally, instead of doing this kind of dance? + if (portal-userId != GetUserId()) + SetUserIdAndSecContext(portal-userId, portal-secCo + else + saveUserId = InvalidOid; In case when CurrentUserId was updated during the code block (E.g, execution of SET SESSION AUTHORIZATION), it overwrites this update on restoring user-id and security-context. Comparison of user-id is a simple marker to check whether this portal contain an operation to switch user-id, because these operations are prohibited within security restricted context. (Is there some other good criteria?) Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc
On Thu, Jun 28, 2012 at 9:49 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Jun 18, 2012 at 3:29 PM, Amit Kapila amit.kap...@huawei.com wrote: [ review ] Chetan, this patch is waiting for an update from you. If you'd like this to get committed this CommitFest, we'll need an updated patch soon. Hearing no response, I've marked this patch Returned with Feedback. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect behaviour when using a GiST index on points
Robert Haas robertmh...@gmail.com writes: On Thu, Jun 21, 2012 at 2:53 PM, Alexander Korotkov aekorot...@gmail.com wrote: I think we definitely should apply this patch before 9.2 release, because it is a bug fix. Otherwise people will continue produce incorrect GiST indexes with in-core geometrical opclasses until 9.3. Patch is very simple and only changes few lines of code. Any thoughts? Do we need to apply this patch to 9.2? It's been like that all along, no? I'm feeling hesitant to shove it into 9.2 at this late date. But we should review it and get it into 9.3 early if possible. 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] [Review] Prevent the specification of conflicting transaction read/write options
On Mon, Jun 18, 2012 at 3:47 PM, Amit Kapila amit.kap...@huawei.com wrote: So I am marking this as Waiting on Author Since this patch has not been updated, I'm marking it Returned with Feedback. Hopefully it will be resubmitted for a future CommitFest. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] User-Id Tracking when Portal was started
Kohei KaiGai kai...@kaigai.gr.jp writes: 2012/7/3 Robert Haas robertmh...@gmail.com: Why not just save and restore the user ID and security context unconditionally, instead of doing this kind of dance? + if (portal-userId != GetUserId()) + SetUserIdAndSecContext(portal-userId, portal-secCo + else + saveUserId = InvalidOid; In case when CurrentUserId was updated during the code block (E.g, execution of SET SESSION AUTHORIZATION), it overwrites this update on restoring user-id and security-context. Um... what should happen if there was a SET SESSION AUTHORIZATION to the portal's userId? This test will think nothing happened. 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] Posix Shared Mem patch
On Wednesday, June 27, 2012 05:28:14 AM Robert Haas wrote: On Tue, Jun 26, 2012 at 6:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: Josh Berkus j...@agliodbs.com writes: So let's fix the 80% case with something we feel confident in, and then revisit the no-sysv interlock as a separate patch. That way if we can't fix the interlock issues, we still have a reduced-shmem version of Postgres. Yes. Insisting that we have the whole change in one patch is a good way to prevent any forward progress from happening. As Alvaro noted, there are plenty of issues to resolve without trying to change the interlock mechanism at the same time. So, here's a patch. Instead of using POSIX shmem, I just took the expedient of using mmap() to map a block of MAP_SHARED|MAP_ANONYMOUS memory. The sysv shm is still allocated, but it's just a copy of PGShmemHeader; the real shared memory is the anonymous block. This won't work if EXEC_BACKEND is defined so it just falls back on straight sysv shm in that case. There are obviously some portability issues here - this is documented not to work on Linux = 2.4, but it's not clear whether it fails with some suitable error code or just pretends to work and does the wrong thing. I tested that it does compile and work on both Linux 3.2.6 and MacOS X 10.6.8. And the comments probably need work and... who knows what else is wrong. But, thoughts? Btw, RhodiumToad/Andrew Gierth on irc talked about a reason why sysv shared memory might be advantageous on some platforms. E.g. on freebsd there is the kern.ipc.shm_use_phys setting which prevents paging out shared memory and also seems to make tlb translation cheaper. There does not seem to exist an alternative for anonymous mmap. So maybe we should make that a config option? Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Posix Shared Mem patch
Andres Freund and...@2ndquadrant.com writes: Btw, RhodiumToad/Andrew Gierth on irc talked about a reason why sysv shared memory might be advantageous on some platforms. E.g. on freebsd there is the kern.ipc.shm_use_phys setting which prevents paging out shared memory and also seems to make tlb translation cheaper. There does not seem to exist an alternative for anonymous mmap. Isn't that mlock()? So maybe we should make that a config option? I'd really rather not. If we're going to go in this direction, we should just go there. 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] User-Id Tracking when Portal was started
2012/7/3 Tom Lane t...@sss.pgh.pa.us: Kohei KaiGai kai...@kaigai.gr.jp writes: 2012/7/3 Robert Haas robertmh...@gmail.com: Why not just save and restore the user ID and security context unconditionally, instead of doing this kind of dance? + if (portal-userId != GetUserId()) + SetUserIdAndSecContext(portal-userId, portal-secCo + else + saveUserId = InvalidOid; In case when CurrentUserId was updated during the code block (E.g, execution of SET SESSION AUTHORIZATION), it overwrites this update on restoring user-id and security-context. Um... what should happen if there was a SET SESSION AUTHORIZATION to the portal's userId? This test will think nothing happened. In my test, all the jobs by SET SESSION AUTHORIZATION was cleaned-up... It makes nothing happen from viewpoint of users. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Posix Shared Mem patch
On Tue, Jul 3, 2012 at 11:36 AM, Andres Freund and...@2ndquadrant.com wrote: Btw, RhodiumToad/Andrew Gierth on irc talked about a reason why sysv shared memory might be advantageous on some platforms. E.g. on freebsd there is the kern.ipc.shm_use_phys setting which prevents paging out shared memory and also seems to make tlb translation cheaper. There does not seem to exist an alternative for anonymous mmap. So maybe we should make that a config option? Yeah, I was noticing some notes to that effect in the documentation this morning. I think the alternative for anonymous mmap is mlock(). However, that can hit kernel limits of its own. I'm not sure what the best thing to do about this is. I think most users will want mlock... but maybe not all? So we end up with one option for whether to use mlock and another for whether to use more or less System V shm? Sounds confusing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Posix Shared Mem patch
On Tue, Jul 3, 2012 at 5:36 PM, Andres Freund and...@2ndquadrant.com wrote: On Wednesday, June 27, 2012 05:28:14 AM Robert Haas wrote: On Tue, Jun 26, 2012 at 6:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: Josh Berkus j...@agliodbs.com writes: So let's fix the 80% case with something we feel confident in, and then revisit the no-sysv interlock as a separate patch. That way if we can't fix the interlock issues, we still have a reduced-shmem version of Postgres. Yes. Insisting that we have the whole change in one patch is a good way to prevent any forward progress from happening. As Alvaro noted, there are plenty of issues to resolve without trying to change the interlock mechanism at the same time. So, here's a patch. Instead of using POSIX shmem, I just took the expedient of using mmap() to map a block of MAP_SHARED|MAP_ANONYMOUS memory. The sysv shm is still allocated, but it's just a copy of PGShmemHeader; the real shared memory is the anonymous block. This won't work if EXEC_BACKEND is defined so it just falls back on straight sysv shm in that case. There are obviously some portability issues here - this is documented not to work on Linux = 2.4, but it's not clear whether it fails with some suitable error code or just pretends to work and does the wrong thing. I tested that it does compile and work on both Linux 3.2.6 and MacOS X 10.6.8. And the comments probably need work and... who knows what else is wrong. But, thoughts? Btw, RhodiumToad/Andrew Gierth on irc talked about a reason why sysv shared memory might be advantageous on some platforms. E.g. on freebsd there is the kern.ipc.shm_use_phys setting which prevents paging out shared memory and also seems to make tlb translation cheaper. There does not seem to exist an alternative for anonymous mmap. So maybe we should make that a config option? Interesting to see that FreeBSD does this - while at the same time refusing to fix the use of sysv shared memory under their own jails system (afaik, at least). They seem to be quite undecided on if it's a feature to remove or a feature to expand on :O Not sure I'd trust that to stick around... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Incorrect behaviour when using a GiST index on points
On Tue, Jul 3, 2012 at 11:34 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jun 21, 2012 at 2:53 PM, Alexander Korotkov aekorot...@gmail.com wrote: I think we definitely should apply this patch before 9.2 release, because it is a bug fix. Otherwise people will continue produce incorrect GiST indexes with in-core geometrical opclasses until 9.3. Patch is very simple and only changes few lines of code. Any thoughts? Do we need to apply this patch to 9.2? It's been like that all along, no? Yeah, but it seems an awful lot like a bug. In fact... it's hard to imagine how it could be any more of a bug than this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 29/06/12 00:36, Jan Urbański wrote: On 27/06/12 13:57, Jan Urbański wrote: On 27/06/12 11:51, Asif Naeem wrote: Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. So: I'd add code to translate WINxxx into CPxxx when choosing the Python to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII case alone, as there were no complaints and people using SQL_ASCII are asking for it anyway. Since no one commented, I'll produce a patch to that effect. I believe this should go into 9.2 given that otherwise PL/Python will basically crash any database using the CP12xx encoding. Cheers, Jan -- 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] Pruning the TODO list
On Sat, Jun 30, 2012 at 11:46:12PM +0300, Peter Eisentraut wrote: On lör, 2012-06-30 at 11:08 -0400, Tom Lane wrote: It'd be better to put a disclaimer at the front pointing out that some of these items are unfinished because of lack of consensus, not just lack of code. There is a fairly extensive disclaimer at the top of the wiki page. Maybe it was added recently, though. Yes, Simon added it a week ago. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Ability to listen on two unix sockets
I wrote: On the whole I prefer the solution you mention above: let's generalize the postmaster.pid format (and pg_ctl) so that we don't need to assume anything about port numbers matching up. The nearby discussion about allowing listen_addresses to specify port number would break this assumption anyway. If we just add two port numbers into postmaster.pid, one for the Unix socket and one for the TCP port, we could get rid of the problem entirely. After further thought, I think that this approach would make it a good idea to drop support for alternate port numbers from the present patch. Let's just deal with alternate socket directories for now. There could be a follow-on patch that adds support for nondefault port numbers in both listen_addresses and unix_socket_directories, and fixes up the postmaster.pid format to support that. I will admit that part of my desire to do it this way is a narrow Fedora rationale: in the Fedora package, we are going to want to back-patch the alternate-directory feature into 9.2 (and maybe 9.1) so as to fix our problems with systemd's PrivateTmp feature. The alternate-port-number feature is not necessary for that, and leaving it out would make for a significantly smaller back-patch. But in any case, it seems like adding alternate-port-number support for Unix sockets and not doing it for TCP ports at the same time is just weird. So I think it's a separate feature and should be a separate patch. 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] Event Triggers reduced, v1
Robert Haas robertmh...@gmail.com writes: in it. That's more or less what Dimitri already has in his latest patch, except that after looking it over I'm inclined to think that we'd be better off storing the keys as text and translating to internal ID numbers when we read and cache the table, rather than storing the ID numbers in the table. That will make the catalog contents easier to read, and more importantly will mean that a change to the internal ID numbers need not be initdb-forcing. Well that's what I had in previous versions of the patch, where the code was dealing directly with command tags. If we store text in catalogs and given that we already have the command tag as text, I suppose we would get back to only managing tags as text in the cache key and the rest of the code. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Posix Shared Mem patch
On Tuesday, July 03, 2012 05:41:09 PM Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Btw, RhodiumToad/Andrew Gierth on irc talked about a reason why sysv shared memory might be advantageous on some platforms. E.g. on freebsd there is the kern.ipc.shm_use_phys setting which prevents paging out shared memory and also seems to make tlb translation cheaper. There does not seem to exist an alternative for anonymous mmap. Isn't that mlock()? Similar at least yes. I think it might also make the virtual/physical translation more direct but that ist just the impression of a very short search. So maybe we should make that a config option? I'd really rather not. If we're going to go in this direction, we should just go there. I don't really care, just wanted to bring up that at least one experienced user would be disappointed ;). As the old implementation needs to stay around for EXEC_BACKEND anyway, the price doesn't seem to be too high. Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect behaviour when using a GiST index on points
Oleg Bartunov obartu...@gmail.com writes: Yes, it's a bug and it needs to be applied ! Well, it needs to be *reviewed* first, and nobody's done that ... 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] Posix Shared Mem patch
Andres Freund and...@2ndquadrant.com writes: On Tuesday, July 03, 2012 05:41:09 PM Tom Lane wrote: I'd really rather not. If we're going to go in this direction, we should just go there. I don't really care, just wanted to bring up that at least one experienced user would be disappointed ;). As the old implementation needs to stay around for EXEC_BACKEND anyway, the price doesn't seem to be too high. Well, my feeling is that sooner or later, perhaps sooner, we are going to want to be out from under SysV shmem (and semaphores) entirely. The Linux kernel guys keep threatening to drop support for the feature. So I think that exposing any knobs about this, or encouraging people to rely on corner-case properties of SysV on their platform, is just going to create more pain when we have to pull the plug. 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] User-Id Tracking when Portal was started
Kohei KaiGai kai...@kaigai.gr.jp writes: 2012/7/3 Tom Lane t...@sss.pgh.pa.us: Um... what should happen if there was a SET SESSION AUTHORIZATION to the portal's userId? This test will think nothing happened. In my test, all the jobs by SET SESSION AUTHORIZATION was cleaned-up... It makes nothing happen from viewpoint of users. My point is that it seems like a bug that the secContext gets restored in one case and not the other, depending on which user ID was specified in SET SESSION AUTHORIZATION. 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] Event Triggers reduced, v1
On Tue, Jul 3, 2012 at 11:52 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: in it. That's more or less what Dimitri already has in his latest patch, except that after looking it over I'm inclined to think that we'd be better off storing the keys as text and translating to internal ID numbers when we read and cache the table, rather than storing the ID numbers in the table. That will make the catalog contents easier to read, and more importantly will mean that a change to the internal ID numbers need not be initdb-forcing. Well that's what I had in previous versions of the patch, where the code was dealing directly with command tags. If we store text in catalogs and given that we already have the command tag as text, I suppose we would get back to only managing tags as text in the cache key and the rest of the code. Yeah, I'm of two minds on that. I thought that it made sense to use integer identifiers internally for speed, but now I'm worried that the effort to translate back and forth between strings and integers is going to end up being more than any speed we might save. But I'm still not certain which way is best: maybe your original idea of using strings everywhere will prove to be the winner, but on the other hand I'm not convinced that the code as you have it written today is as efficient as it could be. At any rate, whether or not we end up using strings or integers inside the backend-local caches, I'm inclined to think that it's better to store strings in the catalog, so we'd end up with something like this: CATALOG(pg_event_trigger,3466) { NameDataevtname;/* trigger's name */ int16 evtevent; /* trigger's event */ Oid evtfoid;/* OID of function to be charevtenabled; /* trigger's firing configuratio * session_repli #ifdef CATALOG_VARLEN text evttags[1]; /* command TAGs this event trigger targe #endif } If there's no filter on tags, then I think we should let evttags = NULL. If in the future we have filtering that's not based on tags, we'd add, e.g. text evtshushiness[1] if we were going to filter based on smushiness level. We'd set evtsmushiness = NULL if there's no filtering by smushiness, or else an array of the smushiness levels that fire that trigger. Does that work for you? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [WIP] Patch : Change pg_ident.conf parsing to be the same as pg_hba.conf
On Mon, Jul 2, 2012 at 8:08 AM, Amit Kapila amit.kap...@huawei.com wrote: Suggestions? I suggest you add this to the next CommitFest. :-) https://commitfest.postgresql.org/action/commitfest_view?id=14 Meanwhile, we have this CommitFest to get finished with... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Event Triggers reduced, v1
Robert Haas robertmh...@gmail.com writes: Yeah, I'm of two minds on that. I thought that it made sense to use integer identifiers internally for speed, but now I'm worried that the effort to translate back and forth between strings and integers is going to end up being more than any speed we might save. We do that for nodetags in stored query/expression trees, and I've never seen any indication that it costs enough to notice. It's definitely a huge win for anything that changes regularly, which seems like it'll be a pretty good description of event tags for at least the next few years. 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] Event Triggers reduced, v1
On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Yeah, I'm of two minds on that. I thought that it made sense to use integer identifiers internally for speed, but now I'm worried that the effort to translate back and forth between strings and integers is going to end up being more than any speed we might save. We do that for nodetags in stored query/expression trees, and I've never seen any indication that it costs enough to notice. It's definitely a huge win for anything that changes regularly, which seems like it'll be a pretty good description of event tags for at least the next few years. Good analogy. In that case, as in what I'm proposing here, we use integers in memory and text on disk. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] enhanced error fields
Hello Peter, thank you very much for review 2012/7/2 Peter Geoghegan pe...@2ndquadrant.com: On 9 May 2012 14:33, Pavel Stehule pavel.steh...@gmail.com wrote: here is patch with enhancing ErrorData structure. Now constraints errors and RI uses these fields So I took a look at the patch eelog-2012-05-09.diff today. All of the following remarks apply to it alone. The patch has bitrotted some, due to the fact that Tom bashed around ri_triggers.c somewhat in recent weeks. I took the opportunity to resolve merge conflicts, and attach a revised patch for everyone's convenience. The regression tests pass when the patch is applied. The first thing I noticed about the patch was that inline functions are used freely. While I personally don't find this unreasonable, we recently revisited the question of whether or not it is necessary to continue to support compilers that do not support something equivalent to GNU C inline functions (or that cannot be made to support them via macro hacks). The outcome of that was that it remains necessary to use macros to provide non-inline equivalent functions. For a good example of how that was dealt with quite extensively, see the sortsupport commit: using inline keyword is probably premature optimization in this context and better to remove it. This code is not on critical path. http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c6e3ac11b60ac4a8942ab964252d51c1c0bd8845 In particular, take a look at the new file sortsupport.h . However, whatever we might some day allow with inline functions, the use of extern inline, a C99 feature (or, according to some, anti-feature) seems like a nonstarter into the foreseeable future, unfortunately. Perhaps more to the point, are whatever performance benefit is to be had by the use of inline functions here actually going to pay for the maintenance cost? We already have functions declarations like this, in the same header as your proposed new extern inline functions: extern int geterrcode(void); This function's definition is trivial, and it would probably look like a good candidate for inlining to the compiler, if it had the chance (which, incidentally, it *still* may). However, why bother writing code to support (or encourage) this, considering that this is hardly a performance critical code-path, particularly unlikely to make any appreciable difference? Other style gripes: There are various points at which 80 columns are exceeded, contrary to our style guidelines. Sorry to have to mention it, but the comments need to be cleaned up (I am of course aware that English is not your first language, so that's not a big deal, and I don't think the content of the comments is lacking). it is ok. I'll reformat my code. The patch does essentially work as advertised. Sites that use the new infrastructure are limited to integrity constraint violations (which includes referential integrity constraint violations). So, based on your description, I'd have imagined that all of the sites using errcodes under this banner would have been covered: /* Class 23 - Integrity Constraint Violation */ This would be a reasonably well-defined place to require that this new infrastructure be used going forward (emphasis on the well-defined). Note that the authors of third-party database drivers define exception classes whose structure reflects these errcodes.h codes. To be inconsistent here seems unacceptable, since some future client of, say, pqxx (the example that I am personally most familiar with) might reasonably hope to always see some relation name when they call the e.relation_name() of some pqxx::integrity_constraint_violation object. If we were to commit the patch as-is, that would not be possible, because the following such sites that have not been touched: src/backend/commands/typecmds.c 2233: (errcode(ERRCODE_NOT_NULL_VIOLATION), src/backend/commands/tablecmds.c 3809: (errcode(ERRCODE_NOT_NULL_VIOLATION), src/backend/executor/execQual.c 3786: (errcode(ERRCODE_NOT_NULL_VIOLATION), src/backend/utils/adt/domains.c 126: (errcode(ERRCODE_NOT_NULL_VIOLATION), src/backend/utils/sort/tuplesort.c 3088: (errcode(ERRCODE_UNIQUE_VIOLATION), src/backend/commands/typecmds.c 2602: (errcode(ERRCODE_CHECK_VIOLATION), src/backend/commands/tablecmds.c 3823: (errcode(ERRCODE_CHECK_VIOLATION), 6633: (errcode(ERRCODE_CHECK_VIOLATION), src/backend/executor/execQual.c 3815: (errcode(ERRCODE_CHECK_VIOLATION),
Re: [HACKERS] Ability to listen on two unix sockets
On Tue, Jul 3, 2012 at 11:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: On the whole I prefer the solution you mention above: let's generalize the postmaster.pid format (and pg_ctl) so that we don't need to assume anything about port numbers matching up. The nearby discussion about allowing listen_addresses to specify port number would break this assumption anyway. If we just add two port numbers into postmaster.pid, one for the Unix socket and one for the TCP port, we could get rid of the problem entirely. After further thought, I think that this approach would make it a good idea to drop support for alternate port numbers from the present patch. Let's just deal with alternate socket directories for now. There could be a follow-on patch that adds support for nondefault port numbers in both listen_addresses and unix_socket_directories, and fixes up the postmaster.pid format to support that. I will admit that part of my desire to do it this way is a narrow Fedora rationale: in the Fedora package, we are going to want to back-patch the alternate-directory feature into 9.2 (and maybe 9.1) so as to fix our problems with systemd's PrivateTmp feature. The alternate-port-number feature is not necessary for that, and leaving it out would make for a significantly smaller back-patch. But in any case, it seems like adding alternate-port-number support for Unix sockets and not doing it for TCP ports at the same time is just weird. So I think it's a separate feature and should be a separate patch. +1 I still find it difficult to think of a good use case for multiple ports. cheers andrew
Re: [HACKERS] enhanced error fields
2012/7/2 Peter Geoghegan pe...@2ndquadrant.com: On 2 July 2012 15:19, Peter Geoghegan pe...@2ndquadrant.com wrote: On 9 May 2012 14:33, Pavel Stehule pavel.steh...@gmail.com wrote: here is patch with enhancing ErrorData structure. Now constraints errors and RI uses these fields So I took a look at the patch eelog-2012-05-09.diff today. All of the following remarks apply to it alone. I decided to follow through and take a look at eelog-plpgsql-2012-05-09.diff today too, while I have all of this swapped into my head. This patch is not an atomic unit - it builds upon the first patch. I successfully merged the local feature branch that I'd created for eelog-2012-05-09.diff without any merge conflicts, and I can build Postgres and get the regression tests to pass (including a couple of new ones, for this added functionality for plggsql - the functionality is testing exclusively using the new (9.2) get stacked diagnostics and raise custom exception 'some_custom_exception' using feature). Since that feature branch had all my revisions committed, my observations about redundancies in the other base patch still stand - the 2 functions mentioned did not exist for the benefit of this further patch either. There is a typo here: + case PLPGSQL_RAISEOPTION_TRIGGER_SCHEMA: + printf( TRIGGER_SCHENA = ); + break; } I'm not sure about this inconsistency within unreserved_keyword: For routines: + | K_DIAG_ROUTINE_NAME + | K_DIAG_ROUTINE_SCHEMA For triggers: + | K_DIAG_TRIGGER_NAME + | K_DIAG_TRIGGER_SCHEMA For tables: + | K_DIAG_SCHEMA_NAME . . **SNIP** . + | K_DIAG_TABLE_NAME This inconsistency is based on ANSI SQL design - we can use own identifiers, but I prefer identifiers based on keyword strings. The same inconsistency exists within the anonymous enum that contains PLPGSQL_GETDIAG_TABLE_NAME (and other constants), as well as the new token keywords within plpgsql's gram.y . see standard, please :) - it is not consistent. But it use short and clean names, and it is relative widely used. The doc changes need a little work here too. please, if you can enhance documentation, please, do it. I am not native speaker. I'm not sure that I agree with the extensive use of the term routine in all of these constants - sure, information_schema has a view called routines. But wouldn't it be more appropriate to use a Postgres-centric term within our own code? for me - routine is general world for functions and procedures. So using routine in PostgreSQL is ok. And I believe so we will support procedures too some day. So, what about the concern about performance taking a hit when plpgsql exception blocks are entered as a result of this patch? Well, while I think that an effort to reduce the overhead of PL exception handling would be worthwhile, these patches do not appear to alter things appreciable (though the overhead *is* measurable): [peter@peterlaptop eelog]$ ls exceptions.sql test_eelog_outer.sql Patch (eelog-plpgsql): [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n transaction type: Custom query scaling factor: 1 query mode: simple number of clients: 10 number of threads: 1 duration: 300 s number of transactions actually processed: 305756 tps = 1019.026055 (including connections establishing) tps = 1019.090135 (excluding connections establishing) Master: [peter@peterlaptop eelog]$ pgbench -T 300 -f exceptions.sql -c 10 -n transaction type: Custom query scaling factor: 1 query mode: simple number of clients: 10 number of threads: 1 duration: 300 s number of transactions actually processed: 308376 tps = 1027.908182 (including connections establishing) tps = 1027.977879 (excluding connections establishing) An archive with simple scripts for repeating this are attached, if anyone is interested. thank you for performance testing. It verify my own testing. Thank you for review, Regards Pavel -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] enhanced error fields
Hi, On Monday, July 02, 2012 04:19:56 PM Peter Geoghegan wrote: The first thing I noticed about the patch was that inline functions are used freely. While I personally don't find this unreasonable, we recently revisited the question of whether or not it is necessary to continue to support compilers that do not support something equivalent to GNU C inline functions (or that cannot be made to support them via macro hacks). The outcome of that was that it remains necessary to use macros to provide non-inline equivalent functions. For a good example of how that was dealt with quite extensively, see the sortsupport commit: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c6e3ac11b60ac 4a8942ab964252d51c1c0bd8845 In particular, take a look at the new file sortsupport.h . I actually find sortsupport.h not to be a good example in general because it duplicates the code. Unless youre dealing with minor amounts of code playing macro tricks like I did in the ilist stuff seems to be a better idea. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] xlog filename formatting functions in recovery
On Tue, Jul 3, 2012 at 5:30 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 3, 2012 at 8:16 AM, Magnus Hagander mag...@hagander.net wrote: On Tuesday, July 3, 2012, Robert Haas wrote: On Tue, Jul 3, 2012 at 6:43 AM, Amit Kapila amit.kap...@huawei.com wrote: (added to commitfest: https://commitfest.postgresql.org/action/patch_view?id=888) It seems you have added it in current commit fest. Shouldn't it be added for next CF. Yep. The current CF has been closed to new submissions for two and a half weeks. Might this be something to consder for 9.2, though? It could be considered a bugfix. On the substance of the patch, I believe the reason why this is currently disallowed is because the TLI is implicitly taken from the running system, and on the standby that might be the wrong value. I might be misremembering. That is, however, assuming that this part is not true. I don't recall for sure, but I have a feeling it might be correct. In which case a much bigger patch is needed, and definitely not something for 9.2... Even if we were to conclude that the argument about TLIs is not valid, I'd be very reluctant to slip something like this into 9.2, because we have no time left to recant if it later turns out that there's some other reason why it's not a good idea. Removing error checks is one of those things that you want to try to get done early in the release cycle, because the consequences are sometimes difficult to foresee, and you may not find out why it was a bad idea until users start complaining. Ah. The placement in that particular commitfest was an oversight. I'll move it. -- fdr -- 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] User-Id Tracking when Portal was started
2012/7/3 Tom Lane t...@sss.pgh.pa.us: Kohei KaiGai kai...@kaigai.gr.jp writes: 2012/7/3 Tom Lane t...@sss.pgh.pa.us: Um... what should happen if there was a SET SESSION AUTHORIZATION to the portal's userId? This test will think nothing happened. In my test, all the jobs by SET SESSION AUTHORIZATION was cleaned-up... It makes nothing happen from viewpoint of users. My point is that it seems like a bug that the secContext gets restored in one case and not the other, depending on which user ID was specified in SET SESSION AUTHORIZATION. Sorry, the above description mention about a case when it does not use the marker to distinguish a case to switch user-id from a case not to switch. (I though I was asked the behavior if this logic always switches / restores ids.) The patch itself works correctly, no regression test failed even though several tests switches user-id using SET SESSION AUTHORIZATION. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] enhanced error fields
Excerpts from Pavel Stehule's message of mar jul 03 12:26:57 -0400 2012: 2012/7/2 Peter Geoghegan pe...@2ndquadrant.com: * ereport is used so frequently that it occurs to me that it would be nice to build some error-detection code into this expansion of the mechanism, to detect incorrect use (at least on debug configurations). So maybe constraint_relation_error() lives alongside errdetail() within elog.c. Maybe they return values of each function are some magic value, that is later read within errfinish(int dummy,...) via varargs. From there, on debug configurations, raise a warning if each argument is in a less than idiomatic order (so that we formalise the order). I'm not sure if that's worth the hassle of formalising, but it's worth considering, particularly as there are calls to make our error reporting mechanism more sophisticated. It is question. If I move constraint_relation_error to elog.c, then I have to include utils/rel.h to utils/elog.h. It was a issue previous version - criticised by Tom Including rel.h in elog.h is really really bad. Even if it was only relcache.h it would be bad, because elog.h is included *everywhere*. So current logic is - if you use rel.h and related structs, then you can set these fields in ErrorData. Hm. These new functions do not operate on Relations at all, so having them on relcache.c doesn't seem to me very good ... How about putting the functions in elog.c as Peter suggests, and the declarations in a new header (say relationerror.h or something like that)? That new header would #include relcache.h so if you need to set those fields you include the new header and you have everything you need. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] enhanced error fields
2012/7/3 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of mar jul 03 12:26:57 -0400 2012: 2012/7/2 Peter Geoghegan pe...@2ndquadrant.com: * ereport is used so frequently that it occurs to me that it would be nice to build some error-detection code into this expansion of the mechanism, to detect incorrect use (at least on debug configurations). So maybe constraint_relation_error() lives alongside errdetail() within elog.c. Maybe they return values of each function are some magic value, that is later read within errfinish(int dummy,...) via varargs. From there, on debug configurations, raise a warning if each argument is in a less than idiomatic order (so that we formalise the order). I'm not sure if that's worth the hassle of formalising, but it's worth considering, particularly as there are calls to make our error reporting mechanism more sophisticated. It is question. If I move constraint_relation_error to elog.c, then I have to include utils/rel.h to utils/elog.h. It was a issue previous version - criticised by Tom Including rel.h in elog.h is really really bad. Even if it was only relcache.h it would be bad, because elog.h is included *everywhere*. So current logic is - if you use rel.h and related structs, then you can set these fields in ErrorData. Hm. These new functions do not operate on Relations at all, so having them on relcache.c doesn't seem to me very good ... How about putting the functions in elog.c as Peter suggests, and the declarations in a new header (say relationerror.h or something like that)? That new header would #include relcache.h so if you need to set those fields you include the new header and you have everything you need. it could be Pavel -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] enhanced error fields
On 3 July 2012 17:26, Pavel Stehule pavel.steh...@gmail.com wrote: Hello Peter, thank you very much for review No problem. I'll do some copy-editing of comments and doc changes when you produce another revision. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Event Triggers reduced, v1
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Yeah, I'm of two minds on that. I thought that it made sense to use integer identifiers internally for speed, but now I'm worried that the effort to translate back and forth between strings and integers is going to end up being more than any speed we might save. We do that for nodetags in stored query/expression trees, and I've never seen any indication that it costs enough to notice. It's definitely a huge win for anything that changes regularly, which seems like it'll be a pretty good description of event tags for at least the next few years. Good analogy. In that case, as in what I'm proposing here, we use integers in memory and text on disk. New patch for that tomorrow. I assume we agree on having a column per extra variable, I'm not sure about already including full support for the toplevel one. With some luck I'll have news from you about that before sending the next revision of the patch, which then would include the int16 evttoplevel[1] column. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Support for XLogRecPtr in expand_fmt_string?
Hi, I wonder if we just should add a format code like %R or something similar as a replacement for the %X/%X notion. Having to type something like (uint32) (state-curptr 32), (uint32)state-curptr everywhere is somewhat annoying. Opinions? Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Posix Shared Mem patch
On Tue, Jul 3, 2012 at 6:57 AM, Robert Haas robertmh...@gmail.com wrote: Here's a patch that attempts to begin the work of adjusting the documentation for this brave new world. I am guessing that there may be other places in the documentation that also require updating, and this page probably needs more work, but it's a start. I think the boilerplate warnings in config.sgml about needing to raise the SysV parameters can go away; patch attached. Josh config.sgml.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] Schema version management
On ons, 2012-06-27 at 10:02 +0200, Joel Jacobson wrote: Robert, thank you for keeping this thread alive. Hopefully some more will join the discussion. I'm still hopeful the community can manage to agree upon acceptable tradeoffs and work-arounds to make this possible. I think this idea has merit. Prepare a patch and put it into the next commit fest. I think the benefits clearly outweighs the minor issues of filenames, dumping order, etc. I see the problem that since the dump order is in general not deterministic, this will cause random reordering in your master file that includes all the individual files. Then again, making the dump order deterministic is a problem that can be solved (I suppose), so maybe starting there would be a good step. But it will require a small amount of in-depth pg_dump hacking. -- 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] proof concept - access to session variables on client side
2012/7/3 Peter Eisentraut pete...@gmx.net: On tis, 2012-06-26 at 07:06 +0200, Pavel Stehule wrote: A motivation is integration of possibilities of psql console together with stronger language - plpgsql. Second target is enabling possibility to save a result of some server side process in psql. It improve vars feature in psql. I think it would be better if DO could be extended into some kind of lambda, taking parameters and returning a value. Then you can use existing infrastructure for passing values and saving the return. It would also extend better to other languages. I did it http://archives.postgresql.org/pgsql-hackers/2010-07/msg00118.php it is other approach. I think so callback from server to client is more general solution - access to client system variables is possible, but I know so this is very obscure and risk idea. but any form of parametrization of PL block can be nice. Regards Pavel -- 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] proof concept - access to session variables on client side
On tis, 2012-06-26 at 07:06 +0200, Pavel Stehule wrote: A motivation is integration of possibilities of psql console together with stronger language - plpgsql. Second target is enabling possibility to save a result of some server side process in psql. It improve vars feature in psql. I think it would be better if DO could be extended into some kind of lambda, taking parameters and returning a value. Then you can use existing infrastructure for passing values and saving the return. It would also extend better to other languages. -- 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] Support for XLogRecPtr in expand_fmt_string?
Andres Freund and...@2ndquadrant.com writes: I wonder if we just should add a format code like %R or something similar as a replacement for the %X/%X notion. Only if you can explain how to teach gcc what it means for elog argument match checking. %m is a special case in that it matches up with a longstanding glibc-ism that gcc knows about. Adding format codes of our own invention would be problematic. Having to type something like (uint32) (state-curptr 32), (uint32)state-curptr everywhere is somewhat annoying. If we really feel this is worth doing something about, we could invent a formatting subroutine that converts XLogRecPtr to string (and then we just use %s in the messages). 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] Support for XLogRecPtr in expand_fmt_string?
On tis, 2012-07-03 at 19:35 +0200, Andres Freund wrote: I wonder if we just should add a format code like %R or something similar as a replacement for the %X/%X notion. Having to type something like (uint32) (state-curptr 32), (uint32)state-curptr everywhere is somewhat annoying. Maybe just print it as a single 64-bit value from now on. -- 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] Support for XLogRecPtr in expand_fmt_string?
Peter Eisentraut pete...@gmx.net writes: On tis, 2012-07-03 at 19:35 +0200, Andres Freund wrote: I wonder if we just should add a format code like %R or something similar as a replacement for the %X/%X notion. Maybe just print it as a single 64-bit value from now on. That'd be problematic also, because of the lack of standardization of the format code for uint64. We could write things like message... UINT64_FORMAT ...more message but I wonder how well the translation tools would work with that; and anyway it would at least double the translation effort for messages containing such things. 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] Support for XLogRecPtr in expand_fmt_string?
On Tuesday, July 03, 2012 08:09:40 PM Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I wonder if we just should add a format code like %R or something similar as a replacement for the %X/%X notion. Only if you can explain how to teach gcc what it means for elog argument match checking. %m is a special case in that it matches up with a longstanding glibc-ism that gcc knows about. Adding format codes of our own invention would be problematic. Ah. Yes. That kills the idea. Having to type something like (uint32) (state-curptr 32), (uint32)state-curptr everywhere is somewhat annoying. If we really feel this is worth doing something about, we could invent a formatting subroutine that converts XLogRecPtr to string (and then we just use %s in the messages). I think that would make memory management annoying. Using a static buffer isn't going to work very well either because its valid to pass two recptr's to elog/ereport/ I think at that point the current state is not worth the hassle, sorry for the noise. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Support for XLogRecPtr in expand_fmt_string?
Andres Freund and...@2ndquadrant.com writes: On Tuesday, July 03, 2012 08:09:40 PM Tom Lane wrote: If we really feel this is worth doing something about, we could invent a formatting subroutine that converts XLogRecPtr to string (and then we just use %s in the messages). I think that would make memory management annoying. Using a static buffer isn't going to work very well either because its valid to pass two recptr's to elog/ereport/ Hm. I was assuming that we could probably get away with the static-buffer trick, but if you think not ... One possibility is to make call sites that need this pass local-variable buffers to the formatting subroutine: charxrp_buffer[XLOGRECPTR_BUF_LEN]; charxrp_buffer2[XLOGRECPTR_BUF_LEN]; ereport(, format_xlogrecptr(xrp_buffer, xlogval1), format_xlogrecptr(xrp_buffer2, xlogval2)); but it may not be worth the trouble. 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] pgfoundry references in docs
Attached are two patches, one of which I'd like to apply. Open for discussion on which one. The smaller one, pgfoundry_1.diff, removes the suggestion to apply for new projects on pgfoundry. The reason for this being that pgfoundry doesn't *accept* new projects anymore. The second one removes the reference to pgfoundry completely. As a step in the deprecation. I'd prefer to apply the second one, but will settle for the first one if people object ;) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ pgfoundry_1.diff Description: Binary data pgfoundry_2.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] Bug #6593, extensions, and proposed new patch policy
Excerpts from Alvaro Herrera's message of mié abr 18 18:27:27 -0300 2012: Excerpts from Robert Haas's message of mié abr 18 13:05:03 -0300 2012: On Wed, Apr 18, 2012 at 11:41 AM, Alvaro Herrera alvhe...@alvh.no-ip.org wrote: Per bug #6593, REASSIGN OWNED fails when the affected role owns an extension. This would be trivial to fix if extensions had support code for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable fix right now seems to be to throw an error on REASSIGN OWNED when the user owns an extension. (If anyone wants to claim that we ought to work on a real fix that allows changing the owner internally from REASSIGN OWNED, without introducing ALTER EXTENSION support for doing so, let me know and I'll see about it.) I would be OK with the latter. Here's a patch for that. Applied to master, 9.2 and 9.1. Sorry for dropping the ball on it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Solaris docs
Our documentation still refers to PostgreSQL bundled in solaris, and references downloads to the pgfoundry project. We just removed such references from the website download section, and the downloads are actually on the main site and not on the pgfoundry project primarily. Attached patch removes all those references completely, since Solaris packages are now downloaded the same way as other binaries. It leaves all the notes about how to compile on solaris and other platform details, of course. Any objections? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ solaris_docs.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Make pg_basebackup configure and start standby
On mån, 2012-07-02 at 01:10 +0900, Fujii Masao wrote: But I think that part is lacking in functionality: AFAICT it's hardcoded to only handle host, port, user and password. What about other connection parameters, likely passed to pg_basebackup through the environment in that case? isn't that quite likely to break the server later? What about something like PQconninfo which returns the connection string value established at connection? Maybe the proper way around that is to provide the ability for pg_basebackup to take a full connection string, just like we allow psql to do? +1 I think both of these would be necessary to make this work smoothly. You also need to take into account situations like when pg_basebackup found a server with the help of PG* environment variables that might no longer be set when the server tries to start recovery. So you need something like PQconninfo. -- 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] enhanced error fields
Peter Geoghegan pe...@2ndquadrant.com writes: So I took a look at the patch eelog-2012-05-09.diff today. All of the following remarks apply to it alone. I've been trying out this patch for my own interest (I'm very pleased to see work on this feature), and I have a couple of suggestions from a user's point of view. First: if a not null constraint is violated, the error report includes CONSTRAINT NAME 'not_null_violation'. I think I would find it more useful if CONSTRAINT NAME were left unset rather than given a value that doesn't correspond to a real constraint. A client program can tell it's a null constraint violation from the SQLSTATE. Second: in the case where a foreign-key constraint is violated by a change in the primary-key table, the error report gives the following information: TABLE NAME:name of primary-key table SCHEMA NAME: schema of primary-key table CONSTRAINT NAME: name of foreign-key constraint CONSTRAINT SCHEMA: schema of foreign-key table It doesn't include the name of the foreign-key table (except in the human-readable error message). But in principle you need to know that table name to reliably identify the constraint that was violated. I think what's going on is a mismatch between the way the constraint namespace works in the SQL standard and in PostgreSQL: it looks like the standard expects constraint names to be unique within a schema, while PostgreSQL only requires them to be unique within a table. (A similar issue makes information_schema less useful than the pg_ tables for foreign key constraints.) So I think it would be helpful to go beyond the standard in this case and include the foreign-key table name somewhere in the report. Possibly the enhanced-error reports could somehow add the table name to the string in the CONSTRAINT NAME field, so that the interface PostgreSQL provides looks like the one the standard envisages (which ought to make it easier to write cross-database client code). Or it might be simpler to just add a new enhanced-error field; I can imagine cases where that table name would be the main thing I'd be interested in. -M- -- 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] Solaris docs
On 03/07 21.24, Magnus Hagander wrote: Our documentation still refers to PostgreSQL bundled in solaris, and references downloads to the pgfoundry project. Oh! I wasn't aware of that. Attached patch removes all those references completely, since Solaris packages are now downloaded the same way as other binaries. It leaves all the notes about how to compile on solaris and other platform details, of course. Any objections? No objections from here. - Bjorn Munch (who builds those Solaris packages) -- 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] pgfoundry references in docs
On Jul 3, 2012, at 9:20 PM, Magnus Hagander wrote: The smaller one, pgfoundry_1.diff, removes the suggestion to apply for new projects on pgfoundry. The reason for this being that pgfoundry doesn't *accept* new projects anymore. Should you not perhaps recommend that they go somewhere else? David -- 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] pgfoundry references in docs
On 3 July 2012 20:20, Magnus Hagander mag...@hagander.net wrote: The second one removes the reference to pgfoundry completely. As a step in the deprecation. I'd prefer to apply the second one, but will settle for the first one if people object ;) I'd also prefer if you applied the second one. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On fre, 2012-06-08 at 17:14 +, Amit kapila wrote: This patch is to provide support for fallback application name for contrib/pgbench, oid2name, and dblink. vacuumlo should also be treated, I 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] pgfoundry references in docs
On Tuesday, July 3, 2012, Peter Geoghegan wrote: On 3 July 2012 20:20, Magnus Hagander mag...@hagander.net javascript:; wrote: The second one removes the reference to pgfoundry completely. As a step in the deprecation. I'd prefer to apply the second one, but will settle for the first one if people object ;) I'd also prefer if you applied the second one. +1 -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] lock_timeout and common SIGALRM framework
Excerpts from Boszormenyi Zoltan's message of vie jun 29 14:30:28 -0400 2012: Does anyone have a little time to look at the latest timeout framework with the registration interface and the 2nd patch too? I am at work until Friday next week, after that I will be on vacation for two weeks. Just in case there is anything that needs tweaking to make it more acceptable. I cleaned up this a bit more and now I think it's ready to commit -- as soon as somebody tests that the standby bits still work. I still have not looked at the second patch. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support 1-timeout-framework-v14.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] lock_timeout and common SIGALRM framework
I don't understand why PGSemaphoreTimedLock() is not broken. I mean surely you need a bool return to let the caller know whether the acquisition succeeded or failed? AFAICS you are relying on get_timeout_indicator() but this seems to me the wrong thing to do ... (not to mention how ugly it is to percolate through two levels of abstraction) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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: add conversion from pg_wchar to multibyte
On Tue, Jul 3, 2012 at 10:17 AM, Tatsuo Ishii is...@postgresql.org wrote: OK. So, in that case, I suggest that if the leading byte is non-zero, we emit 0x9d followed by the three available bytes, instead of first testing whether the first byte is = 0xf0. That test seems to serve no purpose but to confuse the issue. Probably the code shoud look like this(see below comment): else if (lb = 0xf0 lb = 0xfe) { if (lb = 0xf4) *to++ = 0x9c; else *to++ = 0x9d; *to++ = lb; *to++ = (*from 8) 0xff; *to++ = *from 0xff; cnt += 4; It's likely we also need to assign some names to all these numbers (0xf0, 0xf4, 0xfe, 0x9c, 0x9d). But it's hard for me to invent such names. I further suggest that we improve the comments on the mule functions for both wchar-mb and mb-wchar to make all this more clear. I have added comments about mule internal encoding by refreshing my memory and from old document found on web( http://mibai.tec.u-ryukyu.ac.jp/cgi-bin/info2www?%28mule%29Buffer%20and%20string ). Please take a look at. BTW, it seems conversion between multibyte and wchar can be roundtrip in the leading character is LCPRV2 case: If the second byte of wchar (out of 4 bytes of wchar. The first byte is always 0x00) is in range of 0xf0 to 0xf4, then the first byte of multibyte must be 0x9c. If the second byte of wchar is in range of 0xf5 to 0xfe, then the first byte of multibyte must be 0x9d. Should I intergrate these code changes into my patch? Or we would like to commit them first? -- With best regards, Alexander Korotkov.
Re: [HACKERS] Incorrect behaviour when using a GiST index on points
On Tue, Jul 3, 2012 at 7:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: Oleg Bartunov obartu...@gmail.com writes: Yes, it's a bug and it needs to be applied ! Well, it needs to be *reviewed* first, and nobody's done that ... I've discussed it with Teodor privately and he has verified by thoughts. I think if he'll verify it in this thread, it should be enough for review of few lines bugfix. If we say about bugfix I provided for backpatch, it need more detailed review. I was a miss that I didn't add it to current commitfest, will add it to the next one. -- With best regards, Alexander Korotkov.
Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte
Alexander Korotkov aekorot...@gmail.com writes: It's likely we also need to assign some names to all these numbers (0xf0, 0xf4, 0xfe, 0x9c, 0x9d). But it's hard for me to invent such names. The encoding ID byte values already have names (see pg_wchar.h), but the private prefix bytes don't. I griped about that upthread. I agree this code needs some basic readability cleanup that's independent of your feature addition. It'd likely be reasonable to do that as a separate patch. 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: add conversion from pg_wchar to multibyte
I have added comments about mule internal encoding by refreshing my memory and from old document found on web(http://mibai.tec.u-ryukyu.ac.jp/cgi-bin/info2www?%28mule%29Buffer%20and%20string). Any objection to apply my patch? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SP-GiST for ranges based on 2d-mapping and quad-tree
On Tue, Jul 3, 2012 at 10:51 AM, Jeff Davis pg...@j-davis.com wrote: On Mon, 2012-07-02 at 23:47 -0700, Jeff Davis wrote: On Thu, 2012-06-14 at 02:56 +0400, Alexander Korotkov wrote: Hackers, attached patch implements quad-tree on ranges. Some performance results in comparison with current GiST indexing. Index creation is slightly slower. Probably, it need some investigation. Search queries on SP-GiST use much more pages. However this comparison can be not really correct, because SP-GiST can pin same buffer several times during one scan. In CPU search queries on SP-GiST seems to be slightly faster. Dramatical difference in column @ const query is thanks to 2d-mapping. Also, it would be helpful to add a couple tests to rangetypes.sql. Thank you for review! Now I'm working on detailed performance benchmarks for different opclasses. I hope to finish it in this week. Then we would see which opclasses we're really need and nail down issues you've pointed. -- With best regards, Alexander Korotkov.
Re: [HACKERS] Patch: add conversion from pg_wchar to multibyte
Tatsuo Ishii is...@postgresql.org writes: I have added comments about mule internal encoding by refreshing my memory and from old document found on web(http://mibai.tec.u-ryukyu.ac.jp/cgi-bin/info2www?%28mule%29Buffer%20and%20string). Any objection to apply my patch? It needs a bit of copy-editing, and I think we need to do more than just add comments: the various byte values should have #defines so that you can grep for code usages. I'll see what I can do with it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpython issue with Win64 (PG 9.2)
On 03/07/12 17:45, Jan Urbański wrote: On 29/06/12 00:36, Jan Urbański wrote: On 27/06/12 13:57, Jan Urbański wrote: On 27/06/12 11:51, Asif Naeem wrote: Hi, On Windows 7 64bit, plpython is causing server crash with the following test case i.e. So: I'd add code to translate WINxxx into CPxxx when choosing the Python to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII case alone, as there were no complaints and people using SQL_ASCII are asking for it anyway. Since no one commented, I'll produce a patch to that effect. I believe this should go into 9.2 given that otherwise PL/Python will basically crash any database using the CP12xx encoding. Patch attached. Asif, could you try a few things on a CP1252 database? First verify if your original test case now works and then try this: create function enctest() returns text as $$ return b'tr\xc3\xb3spido'.decode('utf-8') $$ language plpython3u; select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex'); Thanks, Jan diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c new file mode 100644 index 9a4901e..5fafbd1 *** a/src/pl/plpython/plpy_util.c --- b/src/pl/plpython/plpy_util.c *** PLyUnicode_Bytes(PyObject *unicode) *** 66,80 /* * Python understands almost all PostgreSQL encoding names, but it doesn't ! * know SQL_ASCII. */ ! if (GetDatabaseEncoding() == PG_SQL_ASCII) ! serverenc = ascii; ! else ! serverenc = GetDatabaseEncodingName(); rv = PyUnicode_AsEncodedString(unicode, serverenc, strict); ! if (rv == NULL) ! PLy_elog(ERROR, could not convert Python Unicode object to PostgreSQL server encoding); return rv; } --- 66,125 /* * Python understands almost all PostgreSQL encoding names, but it doesn't ! * know SQL_ASCII and calls the Windows encodings differently. */ ! switch (GetDatabaseEncoding()) ! { ! case PG_SQL_ASCII: ! serverenc = ascii; ! break; ! case PG_WIN1250: ! serverenc = cp1250; ! break; ! case PG_WIN1251: ! serverenc = cp1251; ! break; ! case PG_WIN1252: ! serverenc = cp1252; ! break; ! case PG_WIN1253: ! serverenc = cp1253; ! break; ! case PG_WIN1254: ! serverenc = cp1254; ! break; ! case PG_WIN1255: ! serverenc = cp1255; ! break; ! case PG_WIN1256: ! serverenc = cp1256; ! break; ! case PG_WIN1257: ! serverenc = cp1257; ! break; ! case PG_WIN1258: ! serverenc = cp1258; ! break; ! case PG_WIN866: ! serverenc = cp866; ! break; ! case PG_WIN874: ! serverenc = cp874; ! break; ! default: ! serverenc = GetDatabaseEncodingName(); ! break; ! } ! rv = PyUnicode_AsEncodedString(unicode, serverenc, strict); ! if (rv == NULL) { ! /* ! * Use a plan elog instead of PLy_elog here to avoid getting in ! * recursion trouble when the traceback formatting functions try doing ! * unicode to bytes conversion. ! */ ! elog(ERROR, could not convert Python Unicode object to PostgreSQL server encoding); ! } return rv; } -- 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] Event Triggers reduced, v1
On Tue, Jul 3, 2012 at 1:31 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 3, 2012 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Yeah, I'm of two minds on that. I thought that it made sense to use integer identifiers internally for speed, but now I'm worried that the effort to translate back and forth between strings and integers is going to end up being more than any speed we might save. We do that for nodetags in stored query/expression trees, and I've never seen any indication that it costs enough to notice. It's definitely a huge win for anything that changes regularly, which seems like it'll be a pretty good description of event tags for at least the next few years. Good analogy. In that case, as in what I'm proposing here, we use integers in memory and text on disk. New patch for that tomorrow. I assume we agree on having a column per extra variable, Yes. I'm not sure about already including full support for the toplevel one. With some luck I'll have news from you about that before sending the next revision of the patch, which then would include the int16 evttoplevel[1] column. No, I'm not asking you to add any more columns right now (in fact, please do not). But the type of the existing column should change to text[]. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
(2012/06/28 11:16), Robert Haas wrote: If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, and fallback_application_name is only going to apply in cases where the user doesn't care enough to bother setting application_name. Let's not knock ourselves out to solve a problem that may not be that important to begin with. Thanks for clarification. I got the point. The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Regards, -- Shigeru HANADA -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Hi Shigeru/Robert, -Original Message- From: Shigeru HANADA [mailto:shigeru.han...@gmail.com] Sent: Wednesday, July 04, 2012 6:57 AM (2012/06/28 11:16), Robert Haas wrote: If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, and fallback_application_name is only going to apply in cases where the user doesn't care enough to bother setting application_name. Let's not knock ourselves out to solve a problem that may not be that important to begin with. Thanks for clarification. I got the point. The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Shall I consider following 2 points to update the patch: 1. Apply changes similar to pgbench and oid2name for vacuumlo 2. Remove the modifications for dblink. With Regards, Amit Kapila. -- 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: add conversion from pg_wchar to multibyte
I wrote: Tatsuo Ishii is...@postgresql.org writes: I have added comments about mule internal encoding by refreshing my memory and from old document found on web(http://mibai.tec.u-ryukyu.ac.jp/cgi-bin/info2www?%28mule%29Buffer%20and%20string). Any objection to apply my patch? It needs a bit of copy-editing, and I think we need to do more than just add comments: the various byte values should have #defines so that you can grep for code usages. I'll see what I can do with it. I cleaned up the comments in pg_wchar.h some more, added #define symbols for the LCPRVn marker codes, and committed it. So far as I can see, the only LCPRVn marker code that is actually in use right now is 0x9d --- there are no instances of 9a, 9b, or 9c that I can find. I also read in the xemacs internals doc, at http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145 that XEmacs thinks the marker code for private single-byte charsets is 0x9e (only) and that for private multi-byte charsets is 0x9f (only); moreover they think 0x9a-0x9d are potential future official multibyte charset codes. I don't know how we got to the current state of using 0x9a-0x9d as private charset markers, but it seems pretty inconsistent with XEmacs. Since only 0x9d could possibly be on-disk anywhere at the moment (unless I'm missing something), I think we would be well advised to redefine our marker codes thus: LCPRV1 0x9e (only) (matches XEmacs spec) LCPRV2 0x9d (only) (doesn't match XEmacs, but too late to change) This would simplify and speed up the IS_LCPRVn macros, simplify the conversions that Alexander is worried about, and get us out from under the risk that XEmacs will assign their next three official multibyte encoding IDs. We're still in trouble if they ever get to 0x9d, but since that's the last code they have, I bet they will be in no hurry to use it up. 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: add conversion from pg_wchar to multibyte
So far as I can see, the only LCPRVn marker code that is actually in use right now is 0x9d --- there are no instances of 9a, 9b, or 9c that I can find. I also read in the xemacs internals doc, at http://www.xemacs.org/Documentation/21.5/html/internals_26.html#SEC145 that XEmacs thinks the marker code for private single-byte charsets is 0x9e (only) and that for private multi-byte charsets is 0x9f (only); moreover they think 0x9a-0x9d are potential future official multibyte charset codes. I don't know how we got to the current state of using 0x9a-0x9d as private charset markers, but it seems pretty inconsistent with XEmacs. At the time when mule internal code was introduced to PostgreSQL, xemacs did not have multi encoding capabilty and mule (a patch to emacs) was the only implementation allowed to use multi encoding. So I used the specification of mule documented in the URL I wrote. Since only 0x9d could possibly be on-disk anywhere at the moment (unless I'm missing something), I think we would be well advised to redefine our marker codes thus: LCPRV1 0x9e (only) (matches XEmacs spec) LCPRV2 0x9d (only) (doesn't match XEmacs, but too late to change) This would simplify and speed up the IS_LCPRVn macros, simplify the conversions that Alexander is worried about, and get us out from under the risk that XEmacs will assign their next three official multibyte encoding IDs. We're still in trouble if they ever get to 0x9d, but since that's the last code they have, I bet they will be in no hurry to use it up. 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