Re: [HACKERS] pg_class.relpages/allvisible probably shouldn't be a int4
On Fri, May 9, 2014 at 1:50 PM, Andres Freund and...@2ndquadrant.com wrote: And adding a proper unsigned type doesn't sound like a small amount of work. Perhaps not, but it's overdue. We ought to have one. -- Peter Geoghegan -- 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] Compression of full-page-writes
Hello, What kind of error did you get at the server crash? Assertion error? If yes, it might be because of the conflict with 4a170ee9e0ebd7021cb1190fabd5b0cbe2effb8e. This commit forbids palloc from being called within a critical section, but the patch does that and then the assertion error happens. That's a bug of the patch. seems to be that STATEMENT: create table test (id integer); TRAP: FailedAssertion(!(CritSectionCount == 0 || (CurrentMemoryContext) == ErrorContext || (MyAuxProcType == CheckpointerProcess)), File: mcxt.c, Line: 670) LOG: server process (PID 29721) was terminated by signal 6: Aborted DETAIL: Failed process was running: drop table test; LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. How do i resolve this? Thank you, Sameer -- 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] [v9.5] Custom Plan API
On 8 May 2014 22:55, Tom Lane t...@sss.pgh.pa.us wrote: We're past the prototyping stage and into productionising what we know works, AFAIK. If that point is not clear, then we need to discuss that first. OK, I'll bite: what here do we know works? Not a damn thing AFAICS; it's all speculation that certain hooks might be useful, and speculation that's not supported by a lot of evidence. If you think this isn't prototyping, I wonder what you think *is* prototyping. My research contacts advise me of this recent work http://www.ntu.edu.sg/home/bshe/hashjoinonapu_vldb13.pdf and also that they expect a prototype to be ready by October, which I have been told will be open source. So there are at least two groups looking at this as a serious option for Postgres (not including the above paper's authors). That isn't *now*, but it is at least a time scale that fits with acting on this in the next release, if we can separate out the various ideas and agree we wish to proceed. I'll submerge again... -- Simon Riggs 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On 11 May 2014 07:37, Amit Kapila amit.kapil...@gmail.com wrote: Tom Lane has explained these problems in a very clear manner in his below mail and shared his opinion about this feature as well. http://www.postgresql.org/message-id/26819.1291133...@sss.pgh.pa.us I don't have Tom's wonderfully articulate way of saying things, so I'll say it my way: If you want to do this, you already can already write a query that has the same effect. But supporting the syntax directly to execute a statement with an undefinable outcome is a pretty bad idea, and worse than that, there's a ton of useful things that we *do* want that would be a much higher priority for work than this. If you support Postgres, prioritise, please. -- Simon Riggs 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] pg_class.relpages/allvisible probably shouldn't be a int4
On 2014-05-10 23:21:34 -0700, Peter Geoghegan wrote: On Fri, May 9, 2014 at 1:50 PM, Andres Freund and...@2ndquadrant.com wrote: And adding a proper unsigned type doesn't sound like a small amount of work. Perhaps not, but it's overdue. We ought to have one. Maybe. But there's so many things to decide around it that I don't think it's a good prerequisite for not showing essentially corrupted values in a supported scenario. Greetings, Andres Freund -- 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On 2014-05-11 10:33:10 +0200, Simon Riggs wrote: On 11 May 2014 07:37, Amit Kapila amit.kapil...@gmail.com wrote: Tom Lane has explained these problems in a very clear manner in his below mail and shared his opinion about this feature as well. http://www.postgresql.org/message-id/26819.1291133...@sss.pgh.pa.us I don't have Tom's wonderfully articulate way of saying things, so I'll say it my way: If you want to do this, you already can already write a query that has the same effect. But supporting the syntax directly to execute a statement with an undefinable outcome is a pretty bad idea, and worse than that, there's a ton of useful things that we *do* want that would be a much higher priority for work than this. If you support Postgres, prioritise, please. I don't know. I'd find UPDATE/DELETE ORDER BY something rather useful. It's required to avoid deadlocks in many scenarios and it's not that obvious how to write the queries in a correct manner. LIMIT would be a nice bonus for queues, especially if we can get SKIP LOCKED. Greetings, Andres Freund -- 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On Sun, May 11, 2014 at 10:33 AM, Simon Riggs si...@2ndquadrant.com wrote: On 11 May 2014 07:37, Amit Kapila amit.kapil...@gmail.com wrote: Tom Lane has explained these problems in a very clear manner in his below mail and shared his opinion about this feature as well. http://www.postgresql.org/message-id/26819.1291133...@sss.pgh.pa.us I don't have Tom's wonderfully articulate way of saying things, so I'll say it my way: If you want to do this, you already can already write a query that has the same effect. But supporting the syntax directly to execute a statement with an undefinable outcome is a pretty bad idea, and worse than that, there's a ton of useful things that we *do* want that would be a much higher priority for work than this. If you support Postgres, prioritise, please. Yes, you can already achieve what this feature can do by other means, but this feature makes these cases 1) more efficient in terms of how much work has to be done 2) simpler and more elegant to write. But frankly I find it a bit insulting that I shouldn't work on this based on other people's priorities. This is a high priority item for me. I'll look at the problem reported by Amit and come back with a solution and my rationale for adding this feature. ♜ -- 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On 11 May 2014 11:18, Andres Freund and...@2ndquadrant.com wrote: On 2014-05-11 10:33:10 +0200, Simon Riggs wrote: On 11 May 2014 07:37, Amit Kapila amit.kapil...@gmail.com wrote: Tom Lane has explained these problems in a very clear manner in his below mail and shared his opinion about this feature as well. http://www.postgresql.org/message-id/26819.1291133...@sss.pgh.pa.us I don't have Tom's wonderfully articulate way of saying things, so I'll say it my way: If you want to do this, you already can already write a query that has the same effect. But supporting the syntax directly to execute a statement with an undefinable outcome is a pretty bad idea, and worse than that, there's a ton of useful things that we *do* want that would be a much higher priority for work than this. If you support Postgres, prioritise, please. I don't know. I'd find UPDATE/DELETE ORDER BY something rather useful. Perhaps if an index exists to provide an ordering that makes it clear what this means, then yes. Forcing Rukh to implement ORDER BY when an index doesn't exist sounds like useless work though, so if we were to accept that this statement gives an ERROR in the absence of such an index, it would make sense all round. It's required to avoid deadlocks in many scenarios and it's not that obvious how to write the queries in a correct manner. LIMIT would be a nice bonus for queues, especially if we can get SKIP LOCKED. With SKIP LOCKED it begins to have some use. Thanks for the nudge. -- Simon Riggs 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] Compression of full-page-writes
On 30 August 2013 04:55, Fujii Masao masao.fu...@gmail.com wrote: My idea is very simple, just compress FPW because FPW is a big part of WAL. I used pglz_compress() as a compression method, but you might think that other method is better. We can add something like FPW-compression-hook for that later. The patch adds new GUC parameter, but I'm thinking to merge it to full_page_writes parameter to avoid increasing the number of GUC. That is, I'm thinking to change full_page_writes so that it can accept new value 'compress'. * Result [tps] 1386.8 (compress_backup_block = off) 1627.7 (compress_backup_block = on) [the amount of WAL generated during running pgbench] 4302 MB (compress_backup_block = off) 1521 MB (compress_backup_block = on) Compressing FPWs definitely makes sense for bulk actions. I'm worried that the loss of performance occurs by greatly elongating transaction response times immediately after a checkpoint, which were already a problem. I'd be interested to look at the response time curves there. Maybe it makes sense to compress FPWs if we do, say, N FPW writes in a transaction. Just ideas. I was thinking about this and about our previous thoughts about double buffering. FPWs are made in foreground, so will always slow down transaction rates. If we could move to double buffering we could avoid FPWs altogether. Thoughts? -- Simon Riggs 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] pg_class.relpages/allvisible probably shouldn't be a int4
Andres Freund and...@2ndquadrant.com writes: On 2014-05-10 23:21:34 -0700, Peter Geoghegan wrote: On Fri, May 9, 2014 at 1:50 PM, Andres Freund and...@2ndquadrant.com wrote: And adding a proper unsigned type doesn't sound like a small amount of work. Perhaps not, but it's overdue. We ought to have one. Maybe. But there's so many things to decide around it that I don't think it's a good prerequisite for not showing essentially corrupted values in a supported scenario. It's a lot harder than it sounds at first; see past discussions about how we could shoehorn one into the numeric type hierarchy. And think about how C expressions that mix signed and unsigned inputs tend to give surprising results :-( The bigger picture though is that it's hard to get excited about this particular scenario, because if you are up to the point where your table size overflows int32, you are less than a factor of 2 away from the hard limit of BlockNumber, and will therefore soon have much bigger problems to worry about than whether pg_class.relpages prints confusingly. My advice to anyone who reported this from the field would certainly be time to think about partitioning that table. So if I were to take Andres' complaint seriously at all, I'd be thinking in terms of do we need to widen BlockNumber to int64?, not how do we make this print as unsigned?. But I doubt such a proposal would fly, because of the negative impact on index sizes. A possible cosmetic fix is to just clamp the value vacuum/analyze will store into relpages at INT_MAX, while scaling reltuples so that the all-important reltuples/relpages ratio stays at reality. But that might be harder than it sounds too, because of the moving average tracking behavior for reltuples. And it'd be a code path that gets no meaningful testing :-( 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
Simon Riggs si...@2ndquadrant.com writes: On 11 May 2014 11:18, Andres Freund and...@2ndquadrant.com wrote: I don't know. I'd find UPDATE/DELETE ORDER BY something rather useful. Perhaps if an index exists to provide an ordering that makes it clear what this means, then yes. The $64 question is whether we'd accept an implementation that fails if the target table has children (ie, is partitioned). That seems to me to not be up to the project's usual quality expectations, but maybe if there's enough demand for a partial solution we should do so. It strikes me that a big part of the problem here is that the current support for this case assumes that the children don't all have the same rowtype. Which is important if you think of child table as meaning subclass in the OO sense. But for ordinary partitioning cases it's just useless complexity, and ModifyTable isn't the only thing that suffers from that useless complexity. If we had a notion of partitioned table that involved a restriction that all the child tables have the exact same rowtype, we could implement UPDATE/DELETE in a much saner fashion --- just one plan tree, not one per child table --- and it would be possible to support UPDATE/DELETE ORDER BY LIMIT with no more work than for the single-table case. So that might shift the calculation as to whether we're willing to accept a partial implementation. Another idea is that the main reason we do things like this is the assumption that for UPDATE, ModifyTable receives complete new rows that only need to be pushed back into the table (and hence have to already match the rowtype of the specific child table). What if we got rid of that and had the incoming tuples just have the target row identifier (tableoid+TID) and the values for the updated columns? ModifyTable then would have to visit the old row (something it must do anyway, NB), pull out the values for the not-to-be-updated columns, form the final tuple and store it. It could implement this separately for each child table, with a different mapping of which columns receive the updates. This eliminates the whole multiple-plan-tree business at a stroke ... and TBH, it's not immediately obvious that this would not be as efficient or more so than the way we do UPDATEs today, even in the single-target-table case. Pumping all those not-updated column values through the plan tree isn't free. The more I think about it, the more promising this sounds --- though I confess to being badly undercaffeinated at the moment, so maybe there's some fatal problem I'm missing. 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] postgresql.auto.conf read from wrong directory
Amit Kapila amit.kapil...@gmail.com writes: In above scenario, I think you are expecting it should use /data2/postgresql.auto.conf and that is what you have mentioned up-thread. The way to handle it by server is just to forbid setting this parameter by Alter System or the user himself should not perform such an action. Here if we want user to be careful of performing such an action, then may be it's better to have such an indication in ALTER SYSTEM documentation. I think it's clearly *necessary* to forbid setting data_directory in postgresql.auto.conf. The file is defined to be found in the data directory, so any such setting is circular logic by definition; no good can come of not rejecting it. We already have a GUC flag bit about disallowing certain variables in the config file (though I don't remember if it's enforced or just advisory). It seems to me that we'd better invent one for disallowing in ALTER SYSTEM, as well. 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] postgresql.auto.conf read from wrong directory
Amit Kapila amit.kapil...@gmail.com writes: This problem occurs because we don't have the value of data_directory set in postgresql.conf by the time we want to parse .auto.conf file during server start. The value of data_directory is only available after processing of config files. To fix it, we need to store the value of data_directory during parse of postgresql.conf file so that we can use it till data_directory is actually set. Attached patch fixes the problem. Aside from being about as ugly as it could possibly be, this is wrong, because it only covers one of the possible sources of a data_directory that's different from the config file location. In particular, it's possible to set --config_file=something on the postmaster command line and also set a data directory different than that via -D or a PGDATA environment variable, rather than an entry in postgresql.conf (see the initial logic in SelectConfigFiles). While it's possible that you could deal with that with some even uglier variant on this patch (perhaps involving making SelectConfigFiles export its internal value of configdir), I think that having ProcessConfigFile understand exactly what SelectConfigFiles is going to do to select the final value of data_directory would be a clear modularity violation, and very fragile as well. I think what probably has to happen is that ProcessConfigFile shouldn't be internally responsible for reading the auto file at all, but that we do that via two separate calls to ProcessConfigFile, one for the main file and then one for the auto file; and during initial startup, SelectConfigFiles doesn't make the call for the auto file until after it's established the final value of data_directory. (And by the by, I think that DataDir not data_directory is what to be using to compute the auto file's path.) It's distressing that this wasn't noticed till now; it shows that nobody tested ALTER SYSTEM with a config file outside the data directory, which seems like a rather fundamental omission for any work with GUC files. 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] postgresql.auto.conf read from wrong directory
I wrote: I think what probably has to happen is that ProcessConfigFile shouldn't be internally responsible for reading the auto file at all, but that we do that via two separate calls to ProcessConfigFile, one for the main file and then one for the auto file; and during initial startup, SelectConfigFiles doesn't make the call for the auto file until after it's established the final value of data_directory. Since this bug would block testing of ALTER SYSTEM by a nontrivial population of users, I felt it was important to get it fixed before beta, so I went to try and fix it as above. It turns out that reading the two config files separately doesn't work because ProcessConfigFile will think all the settings got removed from the file. What we can do for the moment, though, is to just run ProcessConfigFile twice during startup, skipping the auto file the first time. It might be worth refactoring ProcessConfigFile to avoid the overhead of reading postgresql.conf twice, but it would be a lot of work and I think the gain would be marginal; in any case there's not time to do that today. But I've got the main problem fixed in time for beta. 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] imprecise pg_basebackup documentation about excluded files
On Sat, May 10, 2014 at 3:33 AM, Peter Eisentraut pete...@gmx.net wrote: The pg_basebackup documentation says that only regular files and directories are allowed in the data directory. But it is more correct that any other files are skipped. Attached is a patch to correct that. I also added a link to the protocol documentation and added more details there, also about pg_replslot handling. Not sure exactly how much detail we want to document, but it seems reasonable to make it complete if we provide a list at all. +1. Seems good to me. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pg_class.relpages/allvisible probably shouldn't be a int4
On 2014-05-11 12:24:30 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-05-10 23:21:34 -0700, Peter Geoghegan wrote: On Fri, May 9, 2014 at 1:50 PM, Andres Freund and...@2ndquadrant.com wrote: And adding a proper unsigned type doesn't sound like a small amount of work. Perhaps not, but it's overdue. We ought to have one. Maybe. But there's so many things to decide around it that I don't think it's a good prerequisite for not showing essentially corrupted values in a supported scenario. It's a lot harder than it sounds at first; see past discussions about how we could shoehorn one into the numeric type hierarchy. And think about how C expressions that mix signed and unsigned inputs tend to give surprising results :-( Yea. I really don't like to take on such a major project to solve a minor problem. What I am thinking about right now is to expose a 'pg_blocknumber' type. That only does very basic operations and implicitly casts to int64. That's probably a much more handleable problem and it also might give us some more experience with unsigned types. Comments? The bigger picture though is that it's hard to get excited about this particular scenario, because if you are up to the point where your table size overflows int32, you are less than a factor of 2 away from the hard limit of BlockNumber, and will therefore soon have much bigger problems to worry about than whether pg_class.relpages prints confusingly. Well. It takes several years to get to 16TB, so properly supporting 16TB 32TB is a pretty good step. It'll allow a couple of years continued operation. My advice to anyone who reported this from the field would certainly be time to think about partitioning that table. That definitely got to be the mid to longterm plan. The problem is that our partitioning sucks. Majorly. My hope is that by allowing that large tables to work more properly we have time to improve partitioning to the level that it doesn't basically requires to remove all nontrivial constraints. So if I were to take Andres' complaint seriously at all, I'd be thinking in terms of do we need to widen BlockNumber to int64?, not how do we make this print as unsigned?. But I doubt such a proposal would fly, because of the negative impact on index sizes. Yea, I am not wild for that either. I guess migrating to a postgres with a larger blocksize is the next step. A possible cosmetic fix is to just clamp the value vacuum/analyze will store into relpages at INT_MAX, while scaling reltuples so that the all-important reltuples/relpages ratio stays at reality. But that might be harder than it sounds too, because of the moving average tracking behavior for reltuples. And it'd be a code path that gets no meaningful testing :-( I am absolutely not a fan of that either :(. Greetings, Andres Freund -- 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On 2014-05-11 12:47:21 -0400, Tom Lane wrote: Another idea is that the main reason we do things like this is the assumption that for UPDATE, ModifyTable receives complete new rows that only need to be pushed back into the table (and hence have to already match the rowtype of the specific child table). What if we got rid of that and had the incoming tuples just have the target row identifier (tableoid+TID) and the values for the updated columns? ModifyTable then would have to visit the old row (something it must do anyway, NB), pull out the values for the not-to-be-updated columns, form the final tuple and store it. It could implement this separately for each child table, with a different mapping of which columns receive the updates. This eliminates the whole multiple-plan-tree business at a stroke ... and TBH, it's not immediately obvious that this would not be as efficient or more so than the way we do UPDATEs today, even in the single-target-table case. Pumping all those not-updated column values through the plan tree isn't free. The more I think about it, the more promising this sounds --- though I confess to being badly undercaffeinated at the moment, so maybe there's some fatal problem I'm missing. Yes, that sounds like a rather good plan. There's probably some fun keeping the executor state straight when switching more frequently than now and we'd probably need some (implicitly?) added type coercions? I also agree, while there probably are some cases where'd be slower, that the majority of cases will be faster. Greetings, Andres Freund -- 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] postgresql.auto.conf read from wrong directory
On Sun, May 11, 2014 at 3:20 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: I think what probably has to happen is that ProcessConfigFile shouldn't be internally responsible for reading the auto file at all, but that we do that via two separate calls to ProcessConfigFile, one for the main file and then one for the auto file; and during initial startup, SelectConfigFiles doesn't make the call for the auto file until after it's established the final value of data_directory. Since this bug would block testing of ALTER SYSTEM by a nontrivial population of users, I felt it was important to get it fixed before beta, so I went to try and fix it as above. It turns out that reading the two config files separately doesn't work because ProcessConfigFile will think all the settings got removed from the file. What we can do for the moment, though, is to just run ProcessConfigFile twice during startup, skipping the auto file the first time. It might be worth refactoring ProcessConfigFile to avoid the overhead of reading postgresql.conf twice, but it would be a lot of work and I think the gain would be marginal; in any case there's not time to do that today. But I've got the main problem fixed in time for beta. Thanks for dealing with this. I was skeptical of the proposed patch, but didn't have time to think hard enough about it to say something intelligent. I'm glad you did. -- 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] [PATCH] empty xml values
While looking into this report
http://www.postgresql.org/message-id/cf48ccfb.65a9d%tim.k...@gmail.com I
noticed that we don't accept empty values as xml content values, even
though this should apparently be allowed by the spec. Attached is a
patch to fix it (which needs updates to xml_1.out, which I omit here for
simplicity).
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 422be69..7abe215 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -1400,6 +1400,8 @@ static void SPI_sql_row_to_xmlelement(int rownum, StringInfo result,
doc-encoding = xmlStrdup((const xmlChar *) UTF-8);
doc-standalone = standalone;
+ if (*(utf8string + count))
+ {
res_code = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
utf8string + count, NULL);
if (res_code != 0 || xmlerrcxt-err_occurred)
@@ -1407,6 +1409,7 @@ static void SPI_sql_row_to_xmlelement(int rownum, StringInfo result,
invalid XML content);
}
}
+ }
PG_CATCH();
{
if (doc != NULL)
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 382f9df..6e6c673 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -194,6 +194,18 @@ SELECT xmlelement(name foo, xmlattributes( as funny, xml 'ba/r' as fun
foo funny=lt;gt;amp;quot;' funnier=blt;a/gt;r/
(1 row)
+SELECT xmlparse(content '');
+ xmlparse
+--
+
+(1 row)
+
+SELECT xmlparse(content ' ');
+ xmlparse
+--
+
+(1 row)
+
SELECT xmlparse(content 'abc');
xmlparse
--
@@ -251,6 +263,22 @@ SELECT xmlparse(content 'nosuchprefix:tag/');
nosuchprefix:tag/
(1 row)
+SELECT xmlparse(document '');
+ERROR: invalid XML document
+DETAIL: line 1: switching encoding : no input
+
+^
+line 1: Document is empty
+
+^
+line 1: Start tag expected, '' not found
+
+^
+SELECT xmlparse(document ' ');
+ERROR: invalid XML document
+DETAIL: line 1: Start tag expected, '' not found
+
+ ^
SELECT xmlparse(document 'abc');
ERROR: invalid XML document
DETAIL: line 1: Start tag expected, '' not found
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 90d4d67..922ab7a 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -60,6 +60,8 @@ CREATE TABLE xmltest (
SELECT xmlelement(name foo, xmlattributes( as funny, xml 'ba/r' as funnier));
+SELECT xmlparse(content '');
+SELECT xmlparse(content ' ');
SELECT xmlparse(content 'abc');
SELECT xmlparse(content 'abcx/abc');
SELECT xmlparse(content 'invalidentity/invalidentity');
@@ -69,6 +71,8 @@ CREATE TABLE xmltest (
SELECT xmlparse(content 'twoerrorsidontexist;/unbalanced');
SELECT xmlparse(content 'nosuchprefix:tag/');
+SELECT xmlparse(document '');
+SELECT xmlparse(document ' ');
SELECT xmlparse(document 'abc');
SELECT xmlparse(document 'abcx/abc');
SELECT xmlparse(document 'invalidentity/abc');
--
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] pgaudit - an auditing extension for PostgreSQL
On Sun, May 4, 2014 at 11:12:57AM -0400, Tom Lane wrote: Stephen Frost sfr...@snowman.net writes: * Abhijit Menon-Sen (a...@2ndquadrant.com) wrote: 1. I wish it were possible to prevent even the superuser from disabling audit logging once it's enabled, so that if someone gained superuser access without authorisation, their actions would still be logged. But I don't think there's any way to do this. Their actions should be logged up until they disable auditing and hopefully those logs would be sent somewhere that they're unable to destroy (eg: syslog). Of course, we make that difficult by not supporting log targets based on criteria (logging EVERYTHING to syslog would suck). I don't see a way to fix this, except to minimize the amount of things requiring superuser to reduce the chances of it being compromised, which is something I've been hoping to see happen for a long time. Prohibiting actions to the superuser is a fundamentally flawed concept. If you do that, you just end up having to invent a new more super kind of superuser who *can* do whatever it is that needs to be done. We did create a replication role that could only read data, right? Is that similar? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] [v9.5] Custom Plan API
On 8 May 2014 22:55, Tom Lane t...@sss.pgh.pa.us wrote: We're past the prototyping stage and into productionising what we know works, AFAIK. If that point is not clear, then we need to discuss that first. OK, I'll bite: what here do we know works? Not a damn thing AFAICS; it's all speculation that certain hooks might be useful, and speculation that's not supported by a lot of evidence. If you think this isn't prototyping, I wonder what you think *is* prototyping. My research contacts advise me of this recent work http://www.ntu.edu.sg/home/bshe/hashjoinonapu_vldb13.pdf and also that they expect a prototype to be ready by October, which I have been told will be open source. So there are at least two groups looking at this as a serious option for Postgres (not including the above paper's authors). That isn't *now*, but it is at least a time scale that fits with acting on this in the next release, if we can separate out the various ideas and agree we wish to proceed. I'll submerge again... Through the discussion last week, our minimum consensus are: 1. Deregulated enhancement of FDW is not a way to go 2. Custom-path that can replace built-in scan makes sense as a first step towards the future enhancement. Its planner integration is enough simple to do. 3. Custom-path that can replace built-in join takes investigation how to integrate existing planner structure, to avoid (3a) reinvention of whole of join handling in extension side, and (3b) unnecessary extension calls towards the case obviously unsupported. So, I'd like to start the (2) portion towards the upcoming 1st commit-fest on the v9.5 development cycle. Also, we will be able to have discussion for the (3) portion concurrently, probably, towards 2nd commit-fest. Unfortunately, I cannot participate PGcon/Ottawa this year. Please share us the face-to-face discussion here. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.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] postgresql.auto.conf read from wrong directory
On Mon, May 12, 2014 at 12:50 AM, Tom Lane t...@sss.pgh.pa.us wrote: Since this bug would block testing of ALTER SYSTEM by a nontrivial population of users, I felt it was important to get it fixed before beta, so I went to try and fix it as above. It turns out that reading the two config files separately doesn't work because ProcessConfigFile will think all the settings got removed from the file. What we can do for the moment, though, is to just run ProcessConfigFile twice during startup, skipping the auto file the first time. It might be worth refactoring ProcessConfigFile to avoid the overhead of reading postgresql.conf twice, but it would be a lot of work and I think the gain would be marginal; in any case there's not time to do that today. But I've got the main problem fixed in time for beta. Thanks for fixing it. I will provide a fix to forbid data_directory via Alter System as suggested by you upthread. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
