Re: [HACKERS] Lock Wait Statistics (next commitfest)
Stephen Frost wrote: Mark, Your last email on this patch, from August 9th, indicates that you've still got TODO: redo pg_stat_lock_waits Has you updated this patch since then? Stephen, No - that is still a TODO for me - real life getting in the way :-) Cheers Mark -- 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] numeric_to_number() function skipping some digits
On Tue, Sep 22, 2009 at 10:27:19AM +0530, Jeevan Chalke wrote:
It seems that Oracle reads formatting string from right-to-left. Here are
few results:
('number','format') == Oracle PG
('34,50','999,99') == 3450340
('34,50','99,99') == 34503450
('34,50','99,999') == Invalid Number 3450
('34,50','999,999') == Invalid Number 340
It seems worse to to give a wrong answer silently then to throw an error.
What we do now seems sort of MySqlish.
-dg
--
David Gould da...@sonic.net 510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Looking at the way cache invalidations are handled in two-phase transactions, it would be simpler if we store the shared cache invalidation messages in the twophase state file header, like we store deleted relations and subxids. This allows them to be copied to the COMMIT_PREPARED WAL record, so that we don't need treat twophase commits differently than regular ones in xact_redo_commit. As the patch stands, the new xactGetCommittedInvalidationMessages/MakeSharedInvalidMessagesArray mechanism is duplicated functionality with AtPrepare_Inval/-PersistInvalidationMessage - both materialize the pending shared invalidation messages so that they can be written to disk. I did that in my git branch. I wonder if we might have alignment issues with the SharedInvalidationMessages being stored in WAL records, following the subxids. All the data stored in that record have 4-byte alignment at the moment, but if SharedInvalidationMessage ever needs 8-byte alignment, we would have trouble. Probably not worth changing code, it's highly unlikely that SharedInvalidationMessage will ever need 8-byte alignment, but perhaps a comment somewhere would be in order. I note that we don't emit RunningXacts after a shutdown checkpoint. So if recovery starts at a shutdown checkpoint, we don't let read-only backends in until the first online checkpoint. Could we treat a shutdown checkpoint as a snapshot with no transactions running? Or do prepared transactions screw that up? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
The logic in the lock manager to track the number of held
AccessExclusiveLocks (with ProcArrayIncrementNumHeldLocks and
ProcArrayDecrementNumHeldLocks) seems to be broken. I added an Assertion
into ProcArrayDecrementNumHeldLocks:
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1401,6 +1401,7 @@ ProcArrayIncrementNumHeldLocks(PGPROC *proc)
void
ProcArrayDecrementNumHeldLocks(PGPROC *proc)
{
+ Assert(proc-numHeldLocks 0);
proc-numHeldLocks--;
}
This tripped the assertion:
postgres=# CREATE TABLE foo (id int4 primary key);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index
foo_pkey for table foo
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Making matters worse, the primary server refuses to startup up after
that, tripping the assertion again in crash recovery:
$ bin/postmaster -D data
LOG: database system was interrupted while in recovery at 2009-09-23
11:56:15 EEST
HINT: This probably means that some data is corrupted and you will have
to use the last backup for recovery.
LOG: database system was not properly shut down; automatic recovery in
progress
LOG: redo starts at 0/3270
LOG: REDO @ 0/3270; LSN 0/32AC: prev 0/3220; xid 0; len 32:
Heap2 - clean: rel 1663/11562/1249; blk 32 remxid 4352
LOG: consistent recovery state reached
LOG: REDO @ 0/32AC; LSN 0/32CC: prev 0/3270; xid 0; len 4:
XLOG - nextOid: 24600
LOG: REDO @ 0/32CC; LSN 0/32F4: prev 0/32AC; xid 0; len 12:
Storage - file create: base/11562/16408
LOG: REDO @ 0/32F4; LSN 0/3200011C: prev 0/32CC; xid 4364; len
12: Relation - exclusive relation lock: xid 4364 db 11562 rel 16408
LOG: REDO @ 0/3200011C; LSN 0/320001D8: prev 0/32F4; xid 4364; len
159: Heap - insert: rel 1663/11562/1259; tid 5/4
...
LOG: REDO @ 0/32004754; LSN 0/32004878: prev 0/320046A8; xid 4364; len
264: Transaction - commit: 2009-09-23 11:55:51.888398+03; 15 inval
msgs:catcache id38 catcache id37 catcache id38 catcache id37 catcache
id38 catcache id37 catcache id7 catcache id6 catcache id26 smgr relcache
smgr relcache smgr relcache
TRAP: FailedAssertion(!(proc-numHeldLocks 0), File: procarray.c,
Line: 1404)
LOG: startup process (PID 27430) was terminated by signal 6: Aborted
LOG: aborting startup due to startup process failure
I'm sure that's just a simple bug somewhere, but it highlights that we
need be careful to avoid putting any extra work into the normal recovery
path. Otherwise bugs in hot standby related code can cause crash
recovery to fail.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Unicode UTF-8 table formatting for psql text output
On Fri, Sep 18, 2009 at 11:30:05AM -0700, Selena Deckelmann wrote:
Brad says:
The patched code compiles without any additional warnings.
Lint gripes about a trailing ',' in 'typedef enum printTextRule' in
print.h. Other additional lint seem to be false positives. The
regression tests pass against the new patch.
I've attached a new patch which tidies up those extra commas, plus
a patch showing the changes from the previous patch.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`-GPG Public Key: 0x25BFB848 Please GPG sign your mail.
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 7505cd4..41d508e 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -21,6 +21,9 @@
#endif
#include locale.h
+#ifdef HAVE_LANGINFO_H
+#include langinfo.h
+#endif
#include catalog/pg_type.h
#include pqsignal.h
@@ -356,38 +359,75 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout)
/* Aligned text */
//
+static const printTextFormat asciiformat =
+{
+ {
+ { -, +, +, + },
+ { -, +, +, + },
+ { -, +, +, + },
+ { , |, |, | }
+ },
+ :,
+ ;,
+
+};
+
+static const struct printTextFormat utf8format =
+{
+ {
+ /* ─, ┌, ┬, ┐ */
+ { \342\224\200, \342\224\214, \342\224\254, \342\224\220 },
+ /* ━, ┝, ┿, ┥ */
+ { \342\224\201, \342\224\235, \342\224\277, \342\224\245 },
+ /* ─, └, ┴, ┘ */
+ { \342\224\200, \342\224\224, \342\224\264, \342\224\230 },
+ /* N/A, │, │, │ */
+ { , \342\224\202, \342\224\202, \342\224\202 }
+ },
+ /* ╎ */
+ \342\225\216,
+ /* ┊ */
+ \342\224\212,
+ /* ╷ */
+ \342\225\267
+};
/* draw line */
static void
_print_horizontal_line(const unsigned int ncolumns, const unsigned int *widths,
- unsigned short border, FILE *fout)
+ unsigned short border, printTextRule pos,
+ const printTextFormat *format,
+ FILE *fout)
{
unsigned int i,
j;
+ const printTextLineFormat *lformat = format-lrule[pos];
+
if (border == 1)
- fputc('-', fout);
+ fputs(lformat-hrule, fout);
else if (border == 2)
- fputs(+-, fout);
+ fprintf(fout, %s%s, lformat-leftvrule, lformat-hrule);
for (i = 0; i ncolumns; i++)
{
for (j = 0; j widths[i]; j++)
- fputc('-', fout);
+ fputs(lformat-hrule, fout);
if (i ncolumns - 1)
{
if (border == 0)
fputc(' ', fout);
else
-fputs(-+-, fout);
+fprintf(fout, %s%s%s, lformat-hrule,
+ lformat-midvrule, lformat-hrule);
}
}
if (border == 2)
- fputs(-+, fout);
+ fprintf(fout, %s%s, lformat-hrule, lformat-rightvrule);
else if (border == 1)
- fputc('-', fout);
+ fputs(lformat-hrule, fout);
fputc('\n', fout);
}
@@ -397,7 +437,8 @@ _print_horizontal_line(const unsigned int ncolumns, const unsigned int *widths,
* Print pretty boxes around cells.
*/
static void
-print_aligned_text(const printTableContent *cont, FILE *fout)
+print_aligned_text(const printTableContent *cont, const printTextFormat *format,
+ FILE *fout)
{
bool opt_tuples_only = cont-opt-tuples_only;
bool opt_numeric_locale = cont-opt-numericLocale;
@@ -431,6 +472,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
int *bytes_output; /* Bytes output for column value */
int output_columns = 0; /* Width of interactive console */
bool is_pager = false;
+ const printTextLineFormat *dformat = format-lrule[PRINT_RULE_DATA];
if (cancel_pressed)
return;
@@ -709,7 +751,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
int curr_nl_line;
if (opt_border == 2)
-_print_horizontal_line(col_count, width_wrap, opt_border, fout);
+_print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_TOP, format, fout);
for (i = 0; i col_count; i++)
pg_wcsformat((unsigned char *) cont-headers[i],
@@ -722,7 +764,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
while (more_col_wrapping)
{
if (opt_border == 2)
- fprintf(fout, |%c, curr_nl_line ? '+' : ' ');
+ fprintf(fout, %s%c, dformat-leftvrule, curr_nl_line ? '+' : ' ');
else if (opt_border == 1)
fputc(curr_nl_line ? '+' : ' ', fout);
@@ -753,19 +795,20 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
if (opt_border == 0)
fputc(curr_nl_line ? '+' : ' ', fout);
else
- fprintf(fout, |%c, curr_nl_line ? '+' : ' ');
+ fprintf(fout, %s%c, dformat-midvrule, curr_nl_line ? '+' : ' ');
}
}
curr_nl_line++;
if (opt_border == 2)
- fputs( |, fout);
+ fprintf(fout, %s,
+ dformat-rightvrule);
else if (opt_border == 1)
fputc(' ', fout);
fputc('\n', fout);
}
- _print_horizontal_line(col_count, width_wrap, opt_border, fout);
+ _print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_MIDDLE,
Re: [HACKERS] Hot Standby 0.2.1
On Wed, 2009-09-23 at 11:13 +0300, Heikki Linnakangas wrote: Looking at the way cache invalidations are handled in two-phase transactions, it would be simpler if we store the shared cache invalidation messages in the twophase state file header, like we store deleted relations and subxids. This allows them to be copied to the COMMIT_PREPARED WAL record, so that we don't need treat twophase commits differently than regular ones in xact_redo_commit. As the patch stands, the new xactGetCommittedInvalidationMessages/MakeSharedInvalidMessagesArray mechanism is duplicated functionality with AtPrepare_Inval/-PersistInvalidationMessage - both materialize the pending shared invalidation messages so that they can be written to disk. I did that in my git branch. We could, but the prepared transaction path already contains special case code anyway, so we aren't reducing number of test cases required. This looks like a possible area for refactoring, but I don't see the need for pre-factoring. i.e. don't rework before commit, rework after. I wonder if we might have alignment issues with the SharedInvalidationMessages being stored in WAL records, following the subxids. All the data stored in that record have 4-byte alignment at the moment, but if SharedInvalidationMessage ever needs 8-byte alignment, we would have trouble. Probably not worth changing code, it's highly unlikely that SharedInvalidationMessage will ever need 8-byte alignment, but perhaps a comment somewhere would be in order. It's a possible source of bugs, but there are no issues there AFAICS. The technique of multiple arrays on a WAL record wasn't invented by this patch. I note that we don't emit RunningXacts after a shutdown checkpoint. So if recovery starts at a shutdown checkpoint, we don't let read-only backends in until the first online checkpoint. Could we treat a shutdown checkpoint as a snapshot with no transactions running? Or do prepared transactions screw that up? We could, but I see no requirement for starting HS from a backup taken on a shutdown database. It's just another special case to test and since we already have significant number of important test cases I'd say add this later. That seems to have reflected all of your points on this post, though thanks for the comments. I'm keen to reduce complexity in areas that have caused bugs, but for stuff that is working I am tempted to leave alone on such a big patch. Anything we can do to avoid re-testing sections of code and/or reduce the number of tests required is going to increase stability. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
On Wed, 2009-09-23 at 12:07 +0300, Heikki Linnakangas wrote: it highlights that we need be careful to avoid putting any extra work into the normal recovery path. Otherwise bugs in hot standby related code can cause crash recovery to fail. Excellent point. I will put in additional protective code there. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_hba.conf: samehost and samenet [REVIEW]
On Mon, Sep 21, 2009 at 20:12, Stef Walter stef-l...@memberwebs.com wrote: snip Updated in attached patch. This patch does not build on Windows, the error is: ip.obj : error LNK2019: unresolved external symbol __imp__wsaio...@36 referenced in function _pg_foreach_ifaddr ip.obj : error LNK2019: unresolved external symbol __imp__wsasock...@24 referenc ed in function _pg_foreach_ifaddr .\Release\libpq\libpq.dll : fatal error LNK1120: 2 unresolved externals I don't have time to investigate this further right now, so if somebody else want to dig into why that is happening that would be helpful :) Also, one thought - with samenet we currently from what I can tell enumerate all interfaces. Not just those we bind to based on listen_addresses. Is that intentional, or should we restrict us to subnets reachable through the interfaces we're actually listening on? -- 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] Hot Standby 0.2.1
On Wed, 2009-09-23 at 12:07 +0300, Heikki Linnakangas wrote: seems to be broken Agreed. Patch withdrawn for correction and rework. Nothing serious, but not much point doing further testing to all current issues resolved. Tracking of issues raised and later solved via Wiki. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Simon Riggs wrote: On Wed, 2009-09-23 at 11:13 +0300, Heikki Linnakangas wrote: I note that we don't emit RunningXacts after a shutdown checkpoint. So if recovery starts at a shutdown checkpoint, we don't let read-only backends in until the first online checkpoint. Could we treat a shutdown checkpoint as a snapshot with no transactions running? Or do prepared transactions screw that up? We could, but I see no requirement for starting HS from a backup taken on a shutdown database. It's just another special case to test and since we already have significant number of important test cases I'd say add this later. There's also a related issue that if a backend holding AccessExclusiveLock crashes without writing an abort WAL record, the lock is never released in the standby. We handle the expiration of xids at replay of running-xacts records, but AFAICS we don't do that for locks. It shouldn't be much code to clear those states at shutdown checkpoint, just a few lines to call the right functions methinks. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] operator exclusion constraints [was: generalized index constraints]
On Sun, 2009-09-20 at 19:42 -0400, Tom Lane wrote: Jeff Davis pg...@j-davis.com writes: I suppose I should just allow any index_elem. The only way I was able to make the grammar for that work is by using a reserved keyword. The possibilities that make the most sense to me are: index_elem WITH any_operator index_elem WITH OPERATOR any_operator index_elem CHECK any_operator index_elem CHECK OPERATOR any_operator Do any of these look acceptable? I'd vote for CHECK, out of that list. WITH has no mnemonic value whatever. Using CHECK as part of the syntax of an EXCLUSION constraint will surely confuse the whole thing with CHECK constraints. USING OPERATOR is available, 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] [PATCH] DefaultACLs
I made some more small adjustments - mainly renaming stuff after Tom's comment on anonymous code blocks patch and removed one unused shared dependency. -- Regards Petr Jelinek (PJMODOS) defacl-2009-09-23.diff.gz Description: Unix tar archive -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Getting the red out (of the buildfarm)
We have a number of buildfarm members that have been failing on HEAD consistently for some time. It looks from here that the following actions need to be taken: tapir, cardinal: need a newer version of flex installed wombat, eukaryote, chinchilla: these are all failing with LOG: could not bind IPv4 socket: Address already in use HINT: Is another postmaster already running on port 5678? If not, wait a few seconds and retry. This presumably indicates that a postmaster is hanging around from a previous test and needs to be killed manually. The fact that this started to happen about ten days ago on all three machines suggests a generic failure-to-shut-down problem in the buildfarm script. I wonder how up-to-date their scripts are. comet_moth, gothic_moth: these are failing the new plpython_unicode test in locale cs_CZ.ISO8859-2. Somebody needs to do something about that. If it's left to me I'll probably just remove the test that has multiple results. 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] Hot Standby 0.2.1
Heikki Linnakangas wrote: Simon Riggs wrote: On Wed, 2009-09-23 at 11:13 +0300, Heikki Linnakangas wrote: I note that we don't emit RunningXacts after a shutdown checkpoint. So if recovery starts at a shutdown checkpoint, we don't let read-only backends in until the first online checkpoint. Could we treat a shutdown checkpoint as a snapshot with no transactions running? Or do prepared transactions screw that up? We could, but I see no requirement for starting HS from a backup taken on a shutdown database. It's just another special case to test and since we already have significant number of important test cases I'd say add this later. There's also a related issue that if a backend holding AccessExclusiveLock crashes without writing an abort WAL record, the lock is never released in the standby. We handle the expiration of xids at replay of running-xacts records, but AFAICS we don't do that for locks. Ah, scratch that, I now see that we do call XactClearRecoveryTransactions() when we see a shutdown checkpoint, which clears all recovery locks. But doesn't that prematurely clear all locks belonging to prepared transactions as well? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Getting the red out (of the buildfarm)
On Wed, 2009-09-23 at 10:20 -0400, Tom Lane wrote: comet_moth, gothic_moth: these are failing the new plpython_unicode test in locale cs_CZ.ISO8859-2. Somebody needs to do something about that. If it's left to me I'll probably just remove the test that has multiple results. This is, at first glance, not a valid variant result. It's a genuine failure that needs investigation. I can't reproduce the problem with the equivalent locale on Linux, so Zdenek might need to look into it. -- 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] Getting the red out (of the buildfarm)
* Tom Lane wrote: wombat, eukaryote, chinchilla: these are all failing with ... I wonder how up-to-date their scripts are. chinchilla's was ancient, until five minutes ago. Thanks for the prodding. I'm running a --test HEAD now. -- Christian Ullrich -- 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] operator exclusion constraints [was: generalized index constraints]
On Wed, 2009-09-23 at 15:10 +0300, Peter Eisentraut wrote: Using CHECK as part of the syntax of an EXCLUSION constraint will surely confuse the whole thing with CHECK constraints. USING OPERATOR is available, I think. USING won't work because one of the ways to specify the opclass in an index_elem is something like: CREATE INDEX foo_idx on foo (i USING int4_ops); which appears to be undocumented, and it's not obvious to me why that is useful. The normal way is just: CREATE INDEX foo_idx on foo (i int4_ops); Because I am allowing any index_elem for exclusion constraints, that conflicts with the word USING. We can either eliminate the USING variant from opt_class (unless it's necessary for some reason or I missed it in the documentation), or we can use another word (e.g. WITH or WITH OPERATOR) if you don't like CHECK. 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] Anonymous code blocks
Petr Jelinek pjmo...@pjmodos.net writes: Tom Lane napsal(a): The do.sgml file was missing from both your submissions, so I cooked up a very quick-and-dirty reference page. It could stand to be fleshed out a bit, probably. If there's useful material in your original, please submit a followon patch to add it. Aside from worse wording in my version the only difference is the example. I used (and I am killing GRANT ON ALL patch now): Applied, thanks. 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] Hot Standby 0.2.1
On Tue, Sep 22, 2009 at 10:02 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Heikki Linnakangas escribió: Simon Riggs wrote: On Mon, 2009-09-21 at 19:42 -0700, Jeff Janes wrote: jjanes=# begin; BEGIN jjanes=# lock table pgbench_history in access exclusive mode; LOCK TABLE jjanes=# select count(*) from pgbench_history; count 519104 (1 row) jjanes=# select count(*) from pgbench_history; count 527814 (1 row) Is this the expected behavior? By me, yes. WAL replay does not require a table lock to progress. Any changes are protected with block-level locks. It does acquire a table lock and cancel conflicting queries when it is about to replay something that would cause a query to explode, such as dropping a table, as explained in docs. That is rather surprising. You can't get that result in a normal server, which I think is enough of a reason to disallow it. If you do LOCK TABLE ACCESS EXCLUSIVE, you wouldn't expect the contents to change under your nose. Right. I'd rather be denied the lock than have it granted to me but not do the same thing it does on a primary---prevent all changes to the locked table. I think the fallout from that argument is that WAL replay should hold table-level locks (otherwise the workaround to this problem is too special-casey). I'm not sure about that. If I really wanted to get consistent results, I'd use a serializable transaction. Unfortunately, isolation level serializable is not truly serializable. Usually it is good enough, but when it isn't good enough and you need an explicit table lock (a very rare but not nonexistent situation), I think it should either lock the table in the manner it would do on the primary, or throw an error. I think that silently changing the behavior between primary and standby is not a good thing. Cheers, Jeff -- 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] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
On Mon, Sep 21, 2009 at 3:07 AM, Boszormenyi Zoltan z...@cybertec.at wrote: Jeff Janes írta: Maybe I am biased in this because I am primarily thinking about how I would use such a feature, rather than how Hans-Juergen intends to use it, and maybe those uses differ. Hans-Juergen, could you describe your use case a little bit more? Who do is going to be getting these time-out errors, the queries run by the web-app, or longer running back-office queries? And when they do get an error, what will they do about it? Our use case is to port a huge set of Informix apps, that use SET LOCK MODE TO WAIT N; Apparently Tom Lane was on the opinion that PostgreSQL won't need anything more in that regard. Will statement_timeout not suffice for that use case? I understand that they will do different things, but do not understand why those difference are important. Are there invisible deadlocks that need to be timed out, while long running but not dead-locking queries that need to not be timed out? I guess re-running a long-running query is never going to succeed unless the execution plan is improved, while rerunning a long-blocking query is expected to succeed eventually? Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Jeff Janes jeff.ja...@gmail.com writes: Unfortunately, isolation level serializable is not truly serializable. Usually it is good enough, but when it isn't good enough and you need an explicit table lock (a very rare but not nonexistent situation), I think it should either lock the table in the manner it would do on the primary, or throw an error. I think that silently changing the behavior between primary and standby is not a good thing. +1 --- this proposal made me acutely uncomfortable, too. 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] Hot Standby 0.2.1
Simon Riggs wrote: On Wed, 2009-09-23 at 12:07 +0300, Heikki Linnakangas wrote: seems to be broken Agreed. Looking at the relation lock stuff a bit more... When someone tries to acquire an AccessExclusiveLock, but can't get it immediately, we sleep while holding RecoveryInfoLock. That doesn't affect normal queries, but it will in turn block any attempt to get a running-xacts snapshot, and will thus block checkpoint from finishing. It could take a very long time until you get an AccessExclusiveLock on a busy table, so that seems pretty bad. I think we need to think a bit harder about that locking. The problem becomes a lot easier if we accept that it's OK to have a lock included in the running-xacts snapshot and also appear in a XLOG_RELATION_LOCK record later. The standby should handle that gracefully already. If we just remove RecoveryInfoLock, that can happen, but it still won't be possible for a lock to be missed out which is what we really care about. Rather than keep the numHeldLocks counters per-proc in proc array, I think it would be simpler to have a single (or one per lock partition) counter in shared memory in lock.c. It's just an optimization to make it faster to find out that there is no loggable AccessExclusiveLocks in the system, so it really rather belongs into the lock manager. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Simon, Patch withdrawn for correction and rework. Nothing serious, but not much point doing further testing to all current issues resolved. :-( Good thing we went for 4 CFs. Is there a GIT branch of Simon's current working version up somewhere? -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.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] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Jeff, Will statement_timeout not suffice for that use case? Well, currently statement_timeout doesn't affect waiting for locks. And as a DBA, I don't think I'd want the same timeout for executing queries as for waiting for a lock. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.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] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Josh Berkus j...@agliodbs.com writes: Jeff, Will statement_timeout not suffice for that use case? Well, currently statement_timeout doesn't affect waiting for locks. Sure it does. And as a DBA, I don't think I'd want the same timeout for executing queries as for waiting for a lock. Well, that's exactly what Jeff is questioning. How big is the use-case for that exactly? 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] Hot Standby 0.2.1
Josh Berkus wrote: Patch withdrawn for correction and rework. Nothing serious, but not much point doing further testing to all current issues resolved. :-( Good thing we went for 4 CFs. I think we should try to hammer this in in this commitfest. None of the issues found this far are too serious, nothing that requires major rewrites. Is there a GIT branch of Simon's current working version up somewhere? I have my git repository at git.postgresql.org, branch 'hs-riggs'. I've pushed a number of small changes there over Simon's patch. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby 0.2.1
Heikki, I think we should try to hammer this in in this commitfest. None of the issues found this far are too serious, nothing that requires major rewrites. It would certainly be valuable to get users testing it starting with Alpha2 instead of waiting 2 months. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.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] pg_hba.conf: samehost and samenet [REVIEW]
Magnus Hagander wrote: On Mon, Sep 21, 2009 at 20:12, Stef Walter stef-l...@memberwebs.com wrote: snip Updated in attached patch. This patch does not build on Windows, the error is: ip.obj : error LNK2019: unresolved external symbol __imp__wsaio...@36 referenced in function _pg_foreach_ifaddr ip.obj : error LNK2019: unresolved external symbol __imp__wsasock...@24 referenc ed in function _pg_foreach_ifaddr .\Release\libpq\libpq.dll : fatal error LNK1120: 2 unresolved externals I don't have time to investigate this further right now, so if somebody else want to dig into why that is happening that would be helpful :) My windows VM is giving me problems, but I'll try look into it unless someone else beats me to do it. Also, one thought - with samenet we currently from what I can tell enumerate all interfaces. Not just those we bind to based on listen_addresses. Is that intentional, or should we restrict us to subnets reachable through the interfaces we're actually listening on? This would change the scope of the patch significantly. It seems that adding that limitation is unnecessary. In my opinion, if stricter hba security is required, and limiting to specific subnets are desired, those subnets should be entered directly into the pg_hba.conf file. Currently people are adding 0.0.0.0 to a default pg_hba.conf file in order to allow access from nearby machines, without running into the maintenance problems of hard coding IP addresses. However using 0.0.0.0 is clearly suboptimal from a security perspective. I've seen the samenet feature as a way to avoid the use of 0.0.0.0 in these cases. Obviously people who would like stricter postgres security can configure subnets manually, and would probably not be comfortable with 'automatic' decisions being made about the subnets allowed. Cheers, Stef -- 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_hba.conf: samehost and samenet [REVIEW]
On Wed, Sep 23, 2009 at 18:41, Stef Walter stef-l...@memberwebs.com wrote: Magnus Hagander wrote: On Mon, Sep 21, 2009 at 20:12, Stef Walter stef-l...@memberwebs.com wrote: snip Updated in attached patch. This patch does not build on Windows, the error is: ip.obj : error LNK2019: unresolved external symbol __imp__wsaio...@36 referenced in function _pg_foreach_ifaddr ip.obj : error LNK2019: unresolved external symbol __imp__wsasock...@24 referenc ed in function _pg_foreach_ifaddr .\Release\libpq\libpq.dll : fatal error LNK1120: 2 unresolved externals I don't have time to investigate this further right now, so if somebody else want to dig into why that is happening that would be helpful :) My windows VM is giving me problems, but I'll try look into it unless someone else beats me to do it. If you want a VM that works, look at: http://blog.hagander.net/archives/151-Testing-PostgreSQL-patches-on-Windows-using-Amazon-EC2.html If it's just the VM... :-) Also, one thought - with samenet we currently from what I can tell enumerate all interfaces. Not just those we bind to based on listen_addresses. Is that intentional, or should we restrict us to subnets reachable through the interfaces we're actually listening on? This would change the scope of the patch significantly. It seems that adding that limitation is unnecessary. In my opinion, if stricter hba security is required, and limiting to specific subnets are desired, those subnets should be entered directly into the pg_hba.conf file. Currently people are adding 0.0.0.0 to a default pg_hba.conf file in order to allow access from nearby machines, without running into the maintenance problems of hard coding IP addresses. However using 0.0.0.0 is clearly suboptimal from a security perspective. I've seen the samenet feature as a way to avoid the use of 0.0.0.0 in these cases. Obviously people who would like stricter postgres security can configure subnets manually, and would probably not be comfortable with 'automatic' decisions being made about the subnets allowed. Agreed. In that case, I think we just need to make that clearer in the docs, so people don't make the mistake of thinking it means somehting other than what it does. -- 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] operator exclusion constraints [was: generalized index constraints]
Jeff Davis pg...@j-davis.com writes: We can either eliminate the USING variant from opt_class (unless it's necessary for some reason or I missed it in the documentation), or we can use another word (e.g. WITH or WITH OPERATOR) if you don't like CHECK. Hmm ... we don't seem to have documented the USING noise-word, so it probably would be safe to remove it; but why take a chance? I don't particularly agree with Peter's objection to CHECK. There are plenty of examples in SQL of the same keyword being used for different purposes in nearby places. Indeed you could make about the same argument to object to USING, since it'd still be there in USING access_method elsewhere in the same command. I think that USING is just about as content-free as WITH in this particular example --- it doesn't give you any hint about what the purpose of the operator is. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Tom, Well, that's exactly what Jeff is questioning. How big is the use-case for that exactly? I think that it's not necessary to have a 2nd GUC, but for a different reason than argued. For the applications I work on, I tend to set statement_timeout to something high designed just to catch runaway queries, like 2min or 5min (or 1 hour on data warehouses). Partly this is because statement_timeout is so indiscriminate, and I don't want to terminate queries I actually wanted to complete. If the lock time is included in the statement_timeout counter, even more so. This would mean that I'd want a lock_timeout which was much shorter than the statement_timeout. However, I also stand by my statement that I don't think that a blanket per-server lock_timeout is that useful; you want the lock timeout to be based on how many locks you're waiting for, what the particular operation is, what the user is expecting, etc. And you need so send them a custom error message which explains the lock wait. So, while some people have asserted that a lock_timeout GUC would allow users to retrofit older applications to time out on locks, I just don't see that being the case. You'd have to refactor regardless, and if you're going to, just add the WAIT statement to the lock request. So, -1 from me on having a lock_timeout GUC for now. However, I think this is another one worth taking an informal blog poll to reach users other than hackers, yes? -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Unicode Normalization
Hackers, I just had a discussion on IRC about unicode normalization in PostgreSQL. Apparently there is not support for it, currently. Andrew Gierth points out that it's part of the SQL spec to support it, though: RhodiumToad:e.g. NORMALIZE(foo,NFC,len) justatheory:Oh, just a function then, really. RhodiumToad:where the normal form can be any of NFC, NFD, NFKC, NFKD RhodiumToad:except that the normal form is an identifier, not a string RhodiumToad:also the normal form and length are optional RhodiumToad:so NORMALIZE(foo) is equivalent to NORMALIZE(foo,NFC) I looked around and found the Public Software Group's utf8proc project, which even includes some PostgreSQL support (not, alas, for normalization). It has an MIT-licensed C library that offers these functions: uint8_t utf8proc_NFD(uint8_t str) Returns a pointer to newly allocated memory of a NFD normalized version of the null-terminated stringstr. uint8_t utf8proc_NFC(uint8_t str) Returns a pointer to newly allocated memory of a NFC normalized version of the null-terminated stringstr. uint8_t utf8proc_NFKD(uint8_t str) Returns a pointer to newly allocated memory of a NFKD normalized version of the null-terminated stringstr. uint8_t utf8proc_NFKC(uint8_t str) Returns a pointer to newly allocated memory of a NFKC normalized version of the null-terminated stringstr. Anyone got any interest in porting these functions to PostgreSQL? I guess the parser would need to be updated to support the use of identifiers in the NORMALIZE() function, but otherwise it should be a fairly straight-forward port for an experienced C coder, no? Best, 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] TODO item: Allow more complex user/database default GUC settings
Alvaro Herrera alvhe...@commandprompt.com writes: Robert Haas escribió: So here's the followup question - do you intend to do one of those things for this CommitFest, or should we mark this as Returned with Feedback once Bernd posts the rest of his review? What feedback is it supposed to be returned with? Precisely what I wanted is some feedback on the general idea. Brendan's I like the approach is good, but is it enough to deter a later veto from someone else? FWIW, I looked the patch over quickly, and I think it will be fine once Bernd's comments are addressed. In particular I agree with the objection to the name pg_setting as being confusingly close to pg_settings. But pg_user_setting isn't better. Maybe pg_db_role_settings? As far as the lock issue goes, I don't see any reason why the catalog change creates a reason for new/different locking than we had before. Any attempt to make concurrent updates to the same row will generate an error, and that seems enough to me ... 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] Unicode Normalization
On Sep 23, 2009, at 11:08 AM, David E. Wheeler wrote: I just had a discussion on IRC about unicode normalization in PostgreSQL. Apparently there is not support for it, currently. BTW, the only reference I found on the [to do list](http://wiki.postgresql.org/wiki/Todo ) was: More sensible support for Unicode combining characters, normal forms I think that should probably be changed to talk about the unicode standard support. Best, 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] Unicode Normalization
On Sep 23, 2009, at 11:08 AM, David E. Wheeler wrote: I looked around and found the Public Software Group's utf8proc project, which even includes some PostgreSQL support (not, alas, for normalization). It has an MIT-licensed C library that offers these functions: Sorry, forgot the link: http://www.public-software-group.org/utf8proc Best, 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] pg_hba.conf: samehost and samenet [REVIEW]
On Wed, Sep 23, 2009 at 12:41 PM, Stef Walter stef-l...@memberwebs.com wrote: Currently people are adding 0.0.0.0 to a default pg_hba.conf file in order to allow access from nearby machines, without running into the maintenance problems of hard coding IP addresses. However using 0.0.0.0 is clearly suboptimal from a security perspective. If people aren't willing to take the time (5 minutes?) to create an hba.conf file that implements a reasonable security policy, I'm not sure anything we can do - and certainly not this - is going to help very much. I haven't really looked at this patch, but how confident are we that this is actually portable? It would be a shame to spend a lot of time and energy troubleshooting portability problems with a feature that - IMO - has a fairly marginal use case to begin with. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: Jeff, Will statement_timeout not suffice for that use case? Well, currently statement_timeout doesn't affect waiting for locks. Sure it does. And as a DBA, I don't think I'd want the same timeout for executing queries as for waiting for a lock. this is exactly the point it is simply an additional use case. while statement_timeout is perfect to kick out queries which take too long a lock_timeout serves a totally different purpose because you will get a totally different error message. imagine some old 4GL terminal application: in this case you will hardly reach a statement_timeout because you will simply want to wait until things appear on your screen. however, you definitely don't want to wait forever if somebody keeps working on some product which is on stock and never finishes. btw, this old terminal application i was talking about is exactly the usecase we had - this is why this patch has been made. we are porting roughly 2500 terminal application from informix to postgresql. we are talking about entire factory production lines and so on here (the ECPG patches posted recently are for the same project, btw.). there are countless use-cases where you want to know whether you are locked out or whether you are just taking too long - the message is totally different. the goal of the patch is to have a mechanism to make sure that you don't starve to death. as far is syntax is concerned: there are good reasons for WAIT and good reasons for a GUC. while the WAIT syntax is clearly for a very precise instruction for a very certain place in a program, a GUC is a more overall policy. i don't see a reason why we should not have both anyway. a GUC has the charm that it can be assigned to roles, procedures, etc. nicely a WAIT clause has the charm of being incredibly precise. i can see good arguments for both. the code itself is pretty simplistic - it needs no effort to be up to date and it does not harm anything else - it is pretty isolated. many thanks, hans -- Cybertec Schoenig Schoenig GmbH Reyergasse 9 / 2 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5
Jeff Janes wrote: Will statement_timeout not suffice for that use case? we tried to get around it without actually touching the core but we really need this functionality. patching the core here is not the primary desire we have. it is all about modeling some functionality which was truly missing. many thanks, hans -- Cybertec Schoenig Schoenig GmbH Reyergasse 9 / 2 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TODO item: Allow more complex user/database default GUC settings
--On 23. September 2009 14:10:39 -0400 Tom Lane t...@sss.pgh.pa.us wrote: FWIW, I looked the patch over quickly, and I think it will be fine once Bernd's comments are addressed. In particular I agree with the objection to the name pg_setting as being confusingly close to pg_settings. But pg_user_setting isn't better. Maybe pg_db_role_settings? Jepp, that's better, +1 from me. I'm done with this, too, so i will mark this as Returned with Feedback, if no one objects? -- Thanks Bernd -- 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] new version of PQconnectdb was:(Re: [HACKERS] Determining client_encoding from client locale)
Jaime Casanova jcasa...@systemguards.com.ec writes: i extracted the functions to connect that Heikki put on psql in his patch for determining client_encoding from client locale and put it in libpq so i follow the PQconnectdbParams(* params[]) approach. I got around to looking at the actual patch a bit. I hadn't understood why it was so small, but now I do: it's implemented as a wrapper around PQconnectdb. That is, it takes the given keywords and values, and builds a conninfo string, which PQconnectdb then has to pull apart again. This seems, well, dumb. I'll admit that the wasted cycles are probably not much compared to all the other costs of establishing a connection. But it means that you're still exposed to all the other limitations of PQconnectdb, eg any possible bugs or restrictions in the quoting/escaping logic. It seems to me that a non-bogus patch for this would involve refactoring the code within PQconnectdb so that the keywords and values could be plugged in directly without the escaping and de-escaping logic. It doesn't look that hard to pull apart conninfo_parse into two or three functions that would support this. Another reason why it needs refactoring is that this way doesn't expose all the functionality that ought to be available; in particular not the ability to do an async connection while passing the parameters in this style. You need analogs to PQconnectStart and PQconnectPoll too. (Actually I guess the existing PQconnectPoll would work fine, but you definitely need an analog to PQconnectStart.) 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] TODO item: Allow more complex user/database default GUC settings
On Wed, Sep 23, 2009 at 3:03 PM, Bernd Helmle maili...@oopsware.de wrote: --On 23. September 2009 14:10:39 -0400 Tom Lane t...@sss.pgh.pa.us wrote: FWIW, I looked the patch over quickly, and I think it will be fine once Bernd's comments are addressed. In particular I agree with the objection to the name pg_setting as being confusingly close to pg_settings. But pg_user_setting isn't better. Maybe pg_db_role_settings? Jepp, that's better, +1 from me. I'm done with this, too, so i will mark this as Returned with Feedback, if no one objects? It can be marked Waiting on Author if it's going to be reworked in the next few days. If no plans to rework, or if the rework doesn't materialize, then Returned with Feedback. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [rfc] unicode escapes for extended strings
On 9/23/09, Peter Eisentraut pete...@gmx.net wrote: On Wed, 2009-09-09 at 18:26 +0300, Marko Kreen wrote: Unicode escapes for extended strings. Committed. Thank you for handling the patch. I looked at your code for U and saw that you allow standalone second half of the surrogate pair there, although you error out on first half. Was that deliberate? Standalone surrogate halfs cause headaches for anything that wants to handle data in UTF16. The area 0xD800-0xDFFF is explicitly reserved for UTF16 encoding and does not contain any valid Unicode codepoints. Perhaps pg_verifymbstr() should be made to check for such values, because even if we fix the escaping code, such data can still be inserted via plain utf8 or \x escapes? -- marko -- 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_hba.conf: samehost and samenet [REVIEW]
Robert Haas wrote: On Wed, Sep 23, 2009 at 12:41 PM, Stef Walter stef-l...@memberwebs.com wrote: Currently people are adding 0.0.0.0 to a default pg_hba.conf file in order to allow access from nearby machines, without running into the maintenance problems of hard coding IP addresses. However using 0.0.0.0 is clearly suboptimal from a security perspective. If people aren't willing to take the time (5 minutes?) to create an hba.conf file that implements a reasonable security policy, I'm not sure anything we can do - and certainly not this - is going to help very much. I haven't really looked at this patch, but how confident are we that this is actually portable? It would be a shame to spend a lot of time and energy troubleshooting portability problems with a feature that - IMO - has a fairly marginal use case to begin with. Obviously this isn't the an authentication method. If you're using 'trust' authentication with anything but unix sockets you're pretty screwed anyway. This is used in conjuction with an authentication method. The core problem is with renumbering. Due to IPv4 addresses becoming more and more scarce, ISPs are regularly foisting renumbering on their customers. For example, it's in all the new contracts. Often renumbering takes place on networks where the original developers are long gone. Postgresql has always been very fragile when renumbering due to hard coded IP addresses in the pg_hba.conf file. This patch solves that problem for most of the cases, where hosts nearby on the network can talk to postgresql hosts without putting fragile rules into pg_hba.conf. Allowing host names in pg_hba.conf would also solve this problem, although the last person who tried to implement this it was a topic of contention. I asked if I should focus on reverse DNS host names in pg_hba.conf or portability for this samenet patch, and it was indicated that I should do the latter. If there is clear direction within the community to work on DNS based stuff in pg_hba.conf I'd be willing to contribute effort there. Cheers, Stef -- 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_hba.conf: samehost and samenet [REVIEW]
On Wed, Sep 23, 2009 at 3:53 PM, Stef Walter stef-l...@memberwebs.com wrote: Robert Haas wrote: On Wed, Sep 23, 2009 at 12:41 PM, Stef Walter stef-l...@memberwebs.com wrote: Currently people are adding 0.0.0.0 to a default pg_hba.conf file in order to allow access from nearby machines, without running into the maintenance problems of hard coding IP addresses. However using 0.0.0.0 is clearly suboptimal from a security perspective. If people aren't willing to take the time (5 minutes?) to create an hba.conf file that implements a reasonable security policy, I'm not sure anything we can do - and certainly not this - is going to help very much. I haven't really looked at this patch, but how confident are we that this is actually portable? It would be a shame to spend a lot of time and energy troubleshooting portability problems with a feature that - IMO - has a fairly marginal use case to begin with. Obviously this isn't the an authentication method. If you're using 'trust' authentication with anything but unix sockets you're pretty screwed anyway. This is used in conjuction with an authentication method. The core problem is with renumbering. Due to IPv4 addresses becoming more and more scarce, ISPs are regularly foisting renumbering on their customers. For example, it's in all the new contracts. Often renumbering takes place on networks where the original developers are long gone. Postgresql has always been very fragile when renumbering due to hard coded IP addresses in the pg_hba.conf file. This patch solves that problem for most of the cases, where hosts nearby on the network can talk to postgresql hosts without putting fragile rules into pg_hba.conf. Allowing host names in pg_hba.conf would also solve this problem, although the last person who tried to implement this it was a topic of contention. I asked if I should focus on reverse DNS host names in pg_hba.conf or portability for this samenet patch, and it was indicated that I should do the latter. If there is clear direction within the community to work on DNS based stuff in pg_hba.conf I'd be willing to contribute effort there. Personally, I can't imagine using any of these for anything that I cared very much about. IP renumberings are a pain, but I'd rather take a little extra time to make sure it gets done right. I have other things that would need to be fixed too, besides PostgreSQL: for example, IP tables rules. That having been said, I don't think it's my place to harangue someone else about their feature because it doesn't fit my use case. But if it's going to make PostgreSQL not compile/not work the same way on platforms that we otherwise support, then I think it's a bad idea. Otherwise I have no objection. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_hba.conf: samehost and samenet [REVIEW]
Stef Walter stef-l...@memberwebs.com writes: Allowing host names in pg_hba.conf would also solve this problem, although the last person who tried to implement this it was a topic of contention. I asked if I should focus on reverse DNS host names in pg_hba.conf or portability for this samenet patch, and it was indicated that I should do the latter. Agreed, a DNS-based solution would be a huge pain in the rear to do correctly. However, I think what Robert wanted to know was just how portable you believe this solution is. If it doesn't work, and work pretty much the same, on all our supported platforms then I'm afraid we can't use it. There's nothing worse than a security-critical feature that works differently than you expect it to. In this case what particularly scares me is the idea that 'samenet' might be interpreted to let in a larger subnet than the user expected, eg 10/8 instead of 10.0.0/24. You'd likely not notice the problem until after you'd been broken into ... 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] pg_hba.conf: samehost and samenet [REVIEW]
If looking for representation - I consider the default pg_hba.conf to be problematic. Newbies start with trust access, and then do silly things to open it up. I would use samehost, and if samenet worked the same way it does for Postfix, I would probably use samenet. This information can be pulled from the operating system, and the requirement for it to be hard-coded in pg_hba.conf is inconvenient at best, and problematic at worst. Yes, renumbering requires some thought - but I prefer applications that do the majority of this thought for me over applications that require me to do mundane activities. I would also use DNS in pg_hba.conf if it were available. I can see some of the issues with this (should it be mapped to IP right away, or should it be re-evaluated every time?), but ultimately the feature would be useful, and would be widely used. Especially once we get to IPv6, specification of the addresses will become a horrible chore, and solutions which require the IPv6 address to be spelled out will be painful to use. Both of these are generally one time costs for me. They are a pain, but most of us suck it up and swallow. It hasn't been on my list of itches that I just have to scratch. Remember, though, that the majority of PostgreSQL users are not represented on this list, and my pain here might be acceptable, but a newbie will probably either turn away or do something wrong. Better to give them a sensible configuration from the start from, and allow the experts to specify IP addresses if that is what they want to do. Cheers, mark -- Mark Mielkem...@mielke.cc -- 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_hba.conf: samehost and samenet [REVIEW]
Tom Lane wrote: In this case what particularly scares me is the idea that 'samenet' might be interpreted to let in a larger subnet than the user expected, eg 10/8 instead of 10.0.0/24. You'd likely not notice the problem until after you'd been broken into ... I haven't looked at this feature at all, but I'd be inclined, on the grounds you quite reasonably cite, to require a netmask with samenet, rather than just ask the interface for its netmask. cheers andrew -- 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_hba.conf: samehost and samenet [REVIEW]
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: In this case what particularly scares me is the idea that 'samenet' might be interpreted to let in a larger subnet than the user expected, eg 10/8 instead of 10.0.0/24. You'd likely not notice the problem until after you'd been broken into ... I haven't looked at this feature at all, but I'd be inclined, on the grounds you quite reasonably cite, to require a netmask with samenet, rather than just ask the interface for its netmask. I was just thinking the same thing. Could we then unify samehost and samenet into one thing? sameaddr/24 or something like that, with samehost just being the limiting case of all bits used. I am not sure though if this works nicely for IPv6 as well as IPv4. 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] pg_hba.conf: samehost and samenet [REVIEW]
On 09/23/2009 05:37 PM, Andrew Dunstan wrote: Tom Lane wrote: In this case what particularly scares me is the idea that 'samenet' might be interpreted to let in a larger subnet than the user expected, eg 10/8 instead of 10.0.0/24. You'd likely not notice the problem until after you'd been broken into ... I haven't looked at this feature at all, but I'd be inclined, on the grounds you quite reasonably cite, to require a netmask with samenet, rather than just ask the interface for its netmask. I think requiring a netmask defeats some of the value of samenet. When being assigned a new address can change subnet as well. For example, when we moved one of our machines from one room to another it went from /24 to /26. I think it should be understood that the network will not work properly if the user has the wrong network configuration. If they accidentally use /8 instead of /24 on their interface - it's more likely that some or all of their network will become inaccessible, than somebody breaking into their machine. And, anything is better than 0.0.0.0. There are two questions here I think - one is whether or not samenet is valid and would provide value, which I think it is and it does. A second question is whether it should be enabled in the default pg_hba.conf - I think not. Postfix has this capability and it works fine. I use it to allow relay email from machines I trust, because they are on my network. I think many people would use it, and it would be the right solution for many problems. Worrying about how some person somewhere might screw up, when they have the same opportunity to screw up if things are left unchanged (0.0.0.0) is not a practical way of looking at things. How many Postfix servers have you heard of being open relays as a result of samenet? I haven't heard of it ever happening. I suppose it doesn't mean it hasn't happened - but I think getting the network interface configured properly being a necessity for the machine working properly is a very good encouragement for it to work. Cheers, mark -- Mark Mielkem...@mielke.cc -- 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_hba.conf: samehost and samenet [REVIEW]
Mark Mielke m...@mark.mielke.cc writes: Postfix has this capability and it works fine. Hmm, have we looked at the Postfix code to see exactly how they do it? I'd be a *lot* more comfortable adopting logic that's been proven in the field than something written from scratch. 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] pg_hba.conf: samehost and samenet [REVIEW]
On 09/23/2009 05:40 PM, Tom Lane wrote: I haven't looked at this feature at all, but I'd be inclined, on the grounds you quite reasonably cite, to require a netmask with samenet, rather than just ask the interface for its netmask. I was just thinking the same thing. Could we then unify samehost and samenet into one thing? sameaddr/24 or something like that, with samehost just being the limiting case of all bits used. I am not sure though if this works nicely for IPv6 as well as IPv4. I could see some people wanting this as well - but it's not a replacement for samenet, it would be an additional feature. For example, at my company, I have a cluster of machines on a /26 subnet, but for some accesses, I would prefer to open it up to /8, since our company has a /8, and I may want to allow anybody in the company to connect, regardless of how things are routed. I may still want samenet in the same configuration, to grant additional access if the person happens to be on my switch compared to anywhere in the company. For my switch, having to hard code the subnet is back to being a pain. If we enlarge our subnet to /25, it's one more thing that I would have to remember to change unnecessarily. Cheers, mark -- Mark Mielkem...@mielke.cc -- 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] Adding \ev view editor?
On Mon, Sep 21, 2009 at 02:26:05PM -0400, Andrew Dunstan wrote:
andrew=# select pg_get_viewdef('foo',true);
pg_get_viewdef
--
SELECT 'a'::text AS b,
( SELECT 1
FROM dual) AS x,
random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c,
1 AS d
FROM dual;
(1 row)
+1
-dg
--
David Gould da...@sonic.net 510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.
--
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_hba.conf: samehost and samenet [REVIEW]
Tom Lane wrote: Mark Mielke m...@mark.mielke.cc writes: Postfix has this capability and it works fine. Hmm, have we looked at the Postfix code to see exactly how they do it? I'd be a *lot* more comfortable adopting logic that's been proven in the field than something written from scratch. Good idea. As far as I know postfix doesn't support win32. They use a similar approach with using ioctls on some systems, getifaddrs on others. I can take a look at the postfix code (src/util/inet_addr_local.c), check out licenses, add win32 support and adapt it to postgres uses. Cheers, Stef -- 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_hba.conf: samehost and samenet [REVIEW]
Tom Lane wrote: Stef Walter stef-l...@memberwebs.com writes: Allowing host names in pg_hba.conf would also solve this problem, although the last person who tried to implement this it was a topic of contention. I asked if I should focus on reverse DNS host names in pg_hba.conf or portability for this samenet patch, and it was indicated that I should do the latter. Agreed, a DNS-based solution would be a huge pain in the rear to do correctly. However, I think what Robert wanted to know was just how portable you believe this solution is. If it doesn't work, and work pretty much the same, on all our supported platforms then I'm afraid we can't use it. It does work the same on the platforms noted earlier. After work today, I'll put time into making sure that the winsock build problem noted earlier is sorted out. In this case what particularly scares me is the idea that 'samenet' might be interpreted to let in a larger subnet than the user expected, eg 10/8 instead of 10.0.0/24. You'd likely not notice the problem until after you'd been broken into ... As Mark noted in another email, ones networking wouldn't work at all with such a misconfiguration. But if you like I can add additional defensive checks in the code to ignore those obviously invalid netmasks like /0. Basically the OS would be giving postgres bad information. Does postgres generally try to guard against this? I'll follow the convention of the project. Cheers, Stef -- 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] latest hstore patch
On Sep 22, 2009, at 7:18 PM, Andrew Gierth wrote:
Hstore patch incorporating changes as previously discussed.
In addition the requested new features of conversions to and from
array formats have been added (with docs).
Thanks Andrew.
Just a few thoughts for discussion:
* From my previous posts: Is it time to kill off `...@` and `~`,? Not
necessarily for your patch to handle, just wondering what others think.
* I like the %% operator for converting to arrays. Though I think
maybe I would have liked %@ better, but maybe that's just the Perl
hacker in me.
* I also like the new %# operator to convert to two-dimensional
arrays. But if you adopted %@ for arrays, maybe %@@ better indicates a
2-dimensional array? I'm just thinking out lout here, I'm happy to
have them no matter what they're called.
* More name stuff: Why `hstore_to_list` rather than `hstore_to_array`?
And I'm not sure about `hstore_to_matrix` for the 2-dimensional array.
I guess that's better than `hstore_to_multidimensional_array` would
be. ;-)
For those following along at home, here's what these guys look like:
SELECT %% 'a=foo, b=bar'::hstore as array_op,
hstore_to_list('a=foo, b=bar'::hstore),
%# 'a=foo, b=bar'::hstore as matrix_op,
hstore_to_matrix('a=foo, b=bar'::hstore);
array_op| hstore_to_list | matrix_op |
hstore_to_matrix
---++---
+---
{a,foo,b,bar} | {a,foo,b,bar} | {{a,foo},{b,bar}} | {{a,foo},
{b,bar}}
(1 row)
Pretty cool!
* Thanks for updating the docs with:
+ BTREE and HASH index support
+ A fix for the populate_hash() pasto
+ A link to a discussion of backslashing and SQL standard strings
+ A note on the overhead of reading the old binary format
+ Notes on how to update from the old binary format
In the attached patch, I made a few tweaks to the hstore docs, after
applying your patch. I would have created a new patch with everything,
but ran out of time trying to convince Git to create a context diff.
This is a unified diff, but short, with just these changes:
* Fixed doc pasto for %#.
* Noted in docs that the format is new in 8.5, rather than this
version.
* Eliminated a redundant However, .
* Added an example for creating a HASH index.
In sum: Modulo a discussion of the names of the array casting
operators and functions, I think this patch is ready for committer
review.
Thanks,
David
hstore-doc.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] pg_hba.conf: samehost and samenet [REVIEW]
Stef Walter stef-l...@memberwebs.com writes: But if you like I can add additional defensive checks in the code to ignore those obviously invalid netmasks like /0. Basically the OS would be giving postgres bad information. Does postgres generally try to guard against this? I'll follow the convention of the project. I think we'd only bother with that if it was a known failure mode due to platform bugs or common misconfigurations. 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] latest hstore patch
David == David E Wheeler da...@kineticode.com writes: David Just a few thoughts for discussion: David * From my previous posts: Is it time to kill off `...@` and `~`,? David Not necessarily for your patch to handle, just wondering what David others think. I'll take them out if people think that is appropriate. David * I like the %% operator for converting to arrays. Though I David think maybe I would have liked %@ better, but maybe that's David just the Perl hacker in me. I originally tried just % but something in the grammar stops you using that for a unary op. David * I also like the new %# operator to convert to David two-dimensional arrays. But if you adopted %@ for arrays, David maybe %@@ better indicates a 2-dimensional array? I'm just David thinking out lout here, I'm happy to have them no matter what David they're called. %@@ is a bit on the ugly side for an operator I think. David * More name stuff: Why `hstore_to_list` rather than David `hstore_to_array`? And I'm not sure about `hstore_to_matrix` David for the 2-dimensional array. I guess that's better than David `hstore_to_multidimensional_array` would be. ;-) I intentionally avoided hstore_to_array because it would be unclear which one it meant (the 1-d or 2-d result). -- Andrew (irc:RhodiumToad) -- 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] Largeobject access controls
Jaime, KaiGai Kohei wrote: | ALTER LARGE OBJECT is working, but now that we can change the owner of | a LO we should be able to see who the actual owner is... i mean we | should add an owner column in \dl for psql (maybe \dl+) and maybe an | lo_owner() function. | | I would like to buy your idea at the revised patch. Now we don't have xxx_owner() function for other database objects, such as tables, procedures and so on. I agree to enhance \dl command for psql, however, it seems to me that using SELECT from system catalogs are normal manner in pgsql, instead of lo_owner() function. Jaime Casanova wrote: Do you think the largeobject_compat_acl is a meaningful name, instead? maybe something like largeobject_security_controls? It is important to contain a term of compat which means compatible, because this GUC does not disables all the security checks. The v8.4.x checks superuser privilege on using lo_import()/lo_export(). It is also checked in this patch, even if the GUC is turned on. The purpose of the GUC is to provide compatible behavior, not to provide a stuff to turn on/off all the security features in largeobjects. So, I still prefer the largeobject_compat_acl. Now, I'm revising the patch as follows: - pg_largeobject_meta is renamed to pg_largeobject_metadata - The GUC of largeobject_compat_dac is renamed to largeobject_compat_acl - psql supports \dl to show owner of the largeobject - add documentation for the GUC, and add it to the postgresql.conf.sample Any comments? Thanks, -- OSS Platform Development Division, NEC 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] [PATCH] Largeobject access controls
2009/9/23 KaiGai Kohei kai...@ak.jp.nec.com: Now, I'm revising the patch as follows: - pg_largeobject_meta is renamed to pg_largeobject_metadata - The GUC of largeobject_compat_dac is renamed to largeobject_compat_acl - psql supports \dl to show owner of the largeobject - add documentation for the GUC, and add it to the postgresql.conf.sample I still don't like the idea of having a GUC that turns off a substantial part of the security system. Am I the only one? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_hba.conf: samehost and samenet [REVIEW]
On Wed, Sep 23, 2009 at 7:56 PM, Stef Walter stef-l...@memberwebs.com wrote: Tom Lane wrote: Stef Walter stef-l...@memberwebs.com writes: Allowing host names in pg_hba.conf would also solve this problem, although the last person who tried to implement this it was a topic of contention. I asked if I should focus on reverse DNS host names in pg_hba.conf or portability for this samenet patch, and it was indicated that I should do the latter. Agreed, a DNS-based solution would be a huge pain in the rear to do correctly. However, I think what Robert wanted to know was just how portable you believe this solution is. If it doesn't work, and work pretty much the same, on all our supported platforms then I'm afraid we can't use it. It does work the same on the platforms noted earlier. After work today, I'll put time into making sure that the winsock build problem noted earlier is sorted out. Rereading the thread, it seems that the main question is whether there are any platforms that we support that have neither getifaddrs or SIOCGIFCONF, or where they don't work properly. By the way, in foreach_ifaddr_ifconf, what happens if the number of addresses is too large to fit in the arbitrary-size buffer you've chosen here? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject access controls
Robert Haas wrote: 2009/9/23 KaiGai Kohei kai...@ak.jp.nec.com: Now, I'm revising the patch as follows: - pg_largeobject_meta is renamed to pg_largeobject_metadata - The GUC of largeobject_compat_dac is renamed to largeobject_compat_acl - psql supports \dl to show owner of the largeobject - add documentation for the GUC, and add it to the postgresql.conf.sample I still don't like the idea of having a GUC that turns off a substantial part of the security system. Am I the only one? I also think you are right from the viewpoint of the security. Smaller number of pitfall on configuration is basically better. However, we already released v8.4.x or prior versions without ACL checks on largeobjects, so it is necessary to pay attentions for existing SQLs which expect no ACL checks on largeobject accesses. The purpose of the GUC is to provide users compatible bahaviors on largeobjects. BTW, here is one idea. When the largeobject_compat_acl is turned on, it allows to bypass ACL checks, but it generates warning message for violated accesses. User can notice his SQL should be fixed at the v8.5.x or later. (It is similar to the permissive-mode in SELinux.) Thanks, -- OSS Platform Development Division, NEC 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] Using results from INSERT ... RETURNING
On Tue, Sep 22, 2009 at 11:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Right now, it looks like most of the code is being shared between all three plan types. I'm pretty suspicious of how much code this patch moves around and how little of it is actually changed. I can't really tell if there's an actual design improvement here or if this is all window-dressing. My recollection is that we *told* Marko to set things up so that the first patch was mainly just code refactoring. So your second sentence doesn't surprise me. As to the third, I've not looked at the patch, but perhaps it needs to expend more effort on documentation? Well, part of the problem is that I've not had a lot of luck trying to understand how the executor really works (what's a tuple table slot and why do we need to know in advance how many of them there are?). There's this fine comment, which has been in src/backend/executor/README for 8 years and change: XXX a great deal more documentation needs to be written here... However, that's not the whole problem, either. To your point about documentation, it seems this path doesn't touch the README at all, and it needs to, because some of the statements in that file would certainly become false were this patch to be applied. So I think we should at a minimum ask the patch author to (1) fix the explain bugs I found and (2) update the README, as well as (3) revert needless whitespace changes - there are a couple in execMain.c, from the looks of it. However, before he spends too much more time on this feature, it would probably be good for you to take a quick scan through the patch and see what you think of the general approach. I don't think I'm qualified to judge. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject access controls
Now, I'm revising the patch as follows: - pg_largeobject_meta is renamed to pg_largeobject_metadata - The GUC of largeobject_compat_dac is renamed to largeobject_compat_acl - psql supports \dl to show owner of the largeobject - add documentation for the GUC, and add it to the postgresql.conf.sample Here is the revised patch. The \dl command in psql is enhanced as follows: postgres=# \dl Large objects ID | Owner | Description ---++- 16448 | kaigai | 16449 | kaigai | test large object 1 16450 | ymj| 16451 | ymj| 16452 | ymj| test large object 2 16453 | tak| 16454 | tak| (7 rows) The functionality of largeobject_compat_acl (which was named as largeobject_compat_dac at the previous patch) is changed a bit. Its default is 'off'. When it is turned on, access control features on largeobjects performs with the compatible mode. It also checks access permissions on largeobjects, but its results are ignored with notification messages to inform access violation. It means the v8.5.x provides access control on largeobjects in default, although it also provides compatible mode. However, it should be informed to users their SQL to be revised. Example) postgres=# SET SESSION AUTHORIZATION ymj; SET postgres= SELECT loread(lo_open(16453, x'4'::int), 20); ERROR: permission denied for largeobject 16453 postgres=# SET largeobject_compat_acl = on; enables compatible mode SET (Only superuser can set it) postgres=# SET SESSION AUTHORIZATION ymj; SET postgres= SELECT loread(lo_open(16453, x'4'::int), 20); NOTICE: permission denied for largeobject 16453 dose not prevent it loread \x255044462d312e350d0a25b5b5b5b50d0a312030 (1 row) Thanks, $ diffstat sepgsql-02-blob-8.5devel-r2327.patch.gz doc/src/sgml/config.sgml | 25 + doc/src/sgml/ref/allfiles.sgml|1 doc/src/sgml/ref/alter_large_object.sgml | 75 + doc/src/sgml/ref/grant.sgml |8 doc/src/sgml/ref/revoke.sgml |6 doc/src/sgml/reference.sgml |1 src/backend/catalog/Makefile |6 src/backend/catalog/aclchk.c | 249 ++ src/backend/catalog/dependency.c | 14 + src/backend/catalog/pg_largeobject.c | 354 ++-! src/backend/catalog/pg_shdepend.c |8 src/backend/commands/alter.c |5 src/backend/commands/comment.c| 11 src/backend/commands/tablecmds.c |1 src/backend/libpq/be-fsstubs.c| 49 +-- src/backend/parser/gram.y | 20 + src/backend/storage/large_object/inv_api.c| 115 ++---!!! src/backend/tcop/utility.c|3 src/backend/utils/adt/acl.c |5 src/backend/utils/cache/syscache.c| 13 src/backend/utils/misc/guc.c | 10 src/backend/utils/misc/postgresql.conf.sample |1 src/bin/psql/large_obj.c | 10 src/include/catalog/dependency.h |1 src/include/catalog/indexing.h|3 src/include/catalog/pg_largeobject_metadata.h | 67 src/include/nodes/parsenodes.h|1 src/include/utils/acl.h |6 src/include/utils/syscache.h |1 src/test/regress/expected/privileges.out | 204 ++ src/test/regress/expected/sanity_check.out|3 src/test/regress/sql/privileges.sql | 83 ++ 32 files changed, 966 insertions(+), 73 deletions(-), 320 modifications(!) -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com sepgsql-02-blob-8.5devel-r2327.patch.gz Description: application/gzip -- 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] Largeobject access controls
2009/9/23 KaiGai Kohei kai...@ak.jp.nec.com: Jaime, KaiGai Kohei wrote: | ALTER LARGE OBJECT is working, but now that we can change the owner of | a LO we should be able to see who the actual owner is... i mean we | should add an owner column in \dl for psql (maybe \dl+) and maybe an | lo_owner() function. | | I would like to buy your idea at the revised patch. Now we don't have xxx_owner() function for other database objects, such as tables, procedures and so on. good point, but we have has_xx_privileges() family of functions but i think we can add them later if needed... Jaime Casanova wrote: Do you think the largeobject_compat_acl is a meaningful name, instead? maybe something like largeobject_security_controls? It is important to contain a term of compat which means compatible, because this GUC does not disables all the security checks. The v8.4.x checks superuser privilege on using lo_import()/lo_export(). It is also checked in this patch, even if the GUC is turned on. The purpose of the GUC is to provide compatible behavior, not to provide a stuff to turn on/off all the security features in largeobjects. that's why the section in the postgresql.conf is called VERSION/PLATFORM COMPATIBILITY and the subsection Previous PostgreSQL Versions we have other compatibilty GUC and no ones of those has the term compat in it... So, I still prefer the largeobject_compat_acl. maybe enhanced_largeobjects_checks or enhanced_lo_checks or make the GUC an enum and name it largeobject_control_checks with posible values basic and enhanced -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
