Re: [HACKERS] [GENERAL] [SQL] pg_multixact issues
On 2014-09-18 22:52:57 +0530, Dev Kumkar wrote: On Thu, Sep 18, 2014 at 6:20 PM, Dev Kumkar devdas.kum...@gmail.com wrote: On Thu, Sep 18, 2014 at 4:03 PM, Andres Freund and...@2ndquadrant.com wrote: I don't think that's relevant for you. Did you upgrade the database using pg_upgrade? That's correct! No, there is no upgrade here. Can you show pg_controldata output and the output of 'SELECT oid, datname, relfrozenxid, age(relfrozenxid), relminmxid FROM pg_database;'? Here are the details: oid datname datfrozenxidage(datfrozenxid)datminmxid 16384 myDB1673 10872259 1 Additionally wanted to mention couple more points here: When I try to run vacuum full on this machine then facing following issue: INFO: vacuuming myDB.mytable ERROR: MultiXactId 3622035 has not been created yet -- apparent wraparound No Select statements are working on this table, is the table corrupt? Any inputs/hints/tips here? Yes: Learning some patience. You'd given the previous answer two hours before this one. Nobody is paid to work on this list... 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] [GENERAL] [SQL] pg_multixact issues
On Fri, Sep 19, 2014 at 1:03 PM, Andres Freund and...@2ndquadrant.com wrote: Yes: Learning some patience. You'd given the previous answer two hours before this one. Nobody is paid to work on this list... Apologies for the delay, was working/troubleshooting same issue and was away from my emails. :( Regards...
Re: [HACKERS] pg_xlogdump --stats
Hi. I've attached two patches here: 0001-Make-pg_xlogdump-record-stats-display-summary-statis.patch is my earlier patch to pg_xlogdump, rebased to master. It introduces the new rm_identify callback, but doesn't touch rm_desc. Other than rebasing to master after the INT64_FORMAT changes, I haven't changed anything. 0002-Clearly-separate-rm_identify-and-rm_desc-outputs.patch then makes the change you (Heikki) wanted to see: rm_identify returns a name, and rm_desc can be used to obtain optional additional detail, and xlog.c just glues the two together in a new xlog_outdesc function. rm_desc is changed largely only to (a) remove the prefix: , and (b) not append UNKNOWN for unidentified records. (I've done a little cleaning-up in the second patch, e.g. nbtdesc.c had a bunch of repeated cases that I've unified, which seems a pretty nice readability improvement overall.) Good enough? -- Abhijit From a85a5906733ca1324f7fceb6c4ff5b968a70532e Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen a...@2ndquadrant.com Date: Wed, 4 Jun 2014 14:22:33 +0530 Subject: Make 'pg_xlogdump --[record-]stats' display summary statistics --- configure | 2 +- configure.in | 2 +- contrib/pg_xlogdump/pg_xlogdump.c | 207 +- contrib/pg_xlogdump/rmgrdesc.c| 4 +- contrib/pg_xlogdump/rmgrdesc.h| 1 + doc/src/sgml/pg_xlogdump.sgml | 22 src/backend/access/rmgrdesc/clogdesc.c| 18 +++ src/backend/access/rmgrdesc/dbasedesc.c | 18 +++ src/backend/access/rmgrdesc/gindesc.c | 42 ++ src/backend/access/rmgrdesc/gistdesc.c| 21 +++ src/backend/access/rmgrdesc/hashdesc.c| 6 + src/backend/access/rmgrdesc/heapdesc.c| 83 src/backend/access/rmgrdesc/mxactdesc.c | 21 +++ src/backend/access/rmgrdesc/nbtdesc.c | 54 src/backend/access/rmgrdesc/relmapdesc.c | 15 +++ src/backend/access/rmgrdesc/seqdesc.c | 15 +++ src/backend/access/rmgrdesc/smgrdesc.c| 18 +++ src/backend/access/rmgrdesc/spgdesc.c | 39 ++ src/backend/access/rmgrdesc/standbydesc.c | 18 +++ src/backend/access/rmgrdesc/tblspcdesc.c | 18 +++ src/backend/access/rmgrdesc/xactdesc.c| 33 + src/backend/access/rmgrdesc/xlogdesc.c| 45 +++ src/backend/access/transam/rmgr.c | 4 +- src/include/access/clog.h | 1 + src/include/access/gin.h | 1 + src/include/access/gist_private.h | 1 + src/include/access/hash.h | 1 + src/include/access/heapam_xlog.h | 2 + src/include/access/multixact.h| 1 + src/include/access/nbtree.h | 1 + src/include/access/rmgr.h | 2 +- src/include/access/rmgrlist.h | 34 ++--- src/include/access/spgist.h | 1 + src/include/access/xact.h | 1 + src/include/access/xlog.h | 1 + src/include/access/xlog_internal.h| 1 + src/include/c.h | 3 + src/include/catalog/storage_xlog.h| 1 + src/include/commands/dbcommands.h | 1 + src/include/commands/sequence.h | 1 + src/include/commands/tablespace.h | 1 + src/include/storage/standby.h | 1 + src/include/utils/relmapper.h | 1 + 43 files changed, 736 insertions(+), 27 deletions(-) diff --git a/configure b/configure index dac8e49..22eb857 100755 --- a/configure +++ b/configure @@ -13091,7 +13091,7 @@ fi # If we found long int is 64 bits, assume snprintf handles it. If # we found we need to use long long int, better check. We cope with # snprintfs that use %lld, %qd, or %I64d as the format. If none of these -# work, fall back to our own snprintf emulation (which we know uses %lld). +# works, fall back to our own snprintf emulation (which we know uses %lld). if test $HAVE_LONG_LONG_INT_64 = yes ; then if test $pgac_need_repl_snprintf = no; then diff --git a/configure.in b/configure.in index 1392277..c3c458c 100644 --- a/configure.in +++ b/configure.in @@ -1637,7 +1637,7 @@ fi # If we found long int is 64 bits, assume snprintf handles it. If # we found we need to use long long int, better check. We cope with # snprintfs that use %lld, %qd, or %I64d as the format. If none of these -# work, fall back to our own snprintf emulation (which we know uses %lld). +# works, fall back to our own snprintf emulation (which we know uses %lld). if test $HAVE_LONG_LONG_INT_64 = yes ; then if test $pgac_need_repl_snprintf = no; then diff --git a/contrib/pg_xlogdump/pg_xlogdump.c b/contrib/pg_xlogdump/pg_xlogdump.c index c555786..11da5a8 100644 --- a/contrib/pg_xlogdump/pg_xlogdump.c +++ b/contrib/pg_xlogdump/pg_xlogdump.c @@ -15,9 +15,10 @@ #include dirent.h #include unistd.h -#include access/xlog.h #include access/xlogreader.h #include access/transam.h
Re: [HACKERS] pg_xlogdump --stats
On 2014-09-19 13:24:11 +0530, Abhijit Menon-Sen wrote:
diff --git a/configure.in b/configure.in
index 1392277..c3c458c 100644
--- a/configure.in
+++ b/configure.in
@@ -1637,7 +1637,7 @@ fi
# If we found long int is 64 bits, assume snprintf handles it. If
# we found we need to use long long int, better check. We cope with
# snprintfs that use %lld, %qd, or %I64d as the format. If none of these
-# work, fall back to our own snprintf emulation (which we know uses %lld).
+# works, fall back to our own snprintf emulation (which we know uses %lld).
spurious independent change? Also I actually think the original version
is correct?
+typedef struct XLogDumpStats
+{
+ uint64 count;
+ Stats rmgr_stats[RM_NEXT_ID];
+ Stats record_stats[RM_NEXT_ID][16];
+} XLogDumpStats;
I dislike the literal 16 here and someplace later. A define for the max
number of records would make it clearer.
/*
+ * Store per-rmgr and per-record statistics for a given record.
+ */
+static void
+XLogDumpCountRecord(XLogDumpConfig *config, XLogDumpStats *stats, XLogRecPtr
ReadRecPtr, XLogRecord *record)
+{
+ RmgrId rmid;
+ uint8 recid;
+
+ if (config-filter_by_rmgr != -1
+ config-filter_by_rmgr != record-xl_rmid)
+ return;
+
+ if (config-filter_by_xid_enabled
+ config-filter_by_xid != record-xl_xid)
+ return;
Perhaps we should move these kind of checks outside? So
XLogDumpDisplayRecord and this don't have to repeat them. I sure hope
we'll get some more. I e.g. really, really would like to have a
relfilenode check once Heikki's changes to make that possible are in.
+ stats-count++;
+
+ /* Update per-rmgr statistics */
+
+ rmid = record-xl_rmid;
+
+ stats-rmgr_stats[rmid].count++;
+ stats-rmgr_stats[rmid].rec_len +=
+ record-xl_len + SizeOfXLogRecord;
+ stats-rmgr_stats[rmid].fpi_len +=
+ record-xl_tot_len - (record-xl_len + SizeOfXLogRecord);
a) Whoever introduced the notion of rec_len vs tot_len in regards to
including/excluding SizeOfXLogRecord ...
b) I'm not against it, but I wonder if the best way to add the
SizeOfXLogRecord to the record size. It's just as much part of the
FPI. And this means that the record length will be 0 even if all
the record data has been removed due to the FPI.
static void
usage(void)
{
@@ -401,6 +581,8 @@ usage(void)
printf( (default: 1 or the value used in
STARTSEG)\n);
printf( -V, --version output version information, then
exit\n);
printf( -x, --xid=XID only show records with TransactionId
XID\n);
+ printf( -z, --statsshow per-rmgr statistics\n);
+ printf( -Z, --record-stats show per-record statistics\n);
printf( -?, --help show this help, then exit\n);
}
What was the reason you moved away from --stats=record/rmgr? I think we
possibly will add further ones, so that seems more
extensible?
diff --git a/contrib/pg_xlogdump/rmgrdesc.c b/contrib/pg_xlogdump/rmgrdesc.c
index cbcaaa6..dc27fd1 100644
--- a/contrib/pg_xlogdump/rmgrdesc.c
+++ b/contrib/pg_xlogdump/rmgrdesc.c
It's trivial to separate in this case, but I'd much rather have patches
like this rm_identity stuff split up in the future.
diff --git a/src/backend/access/rmgrdesc/heapdesc.c
b/src/backend/access/rmgrdesc/heapdesc.c
index 24b6f92..91a8e72 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -193,3 +193,86 @@ heap2_desc(StringInfo buf, XLogRecord *record)
else
appendStringInfoString(buf, UNKNOWN);
}
+
+static const char *
+append_init(const char *str)
+{
+ static char x[32];
+
+ strcpy(x, str);
+ strcat(x, +INIT);
+
+ return x;
+}
+
+const char *
+heap_identify(uint8 info)
+{
+ const char *id = NULL;
+
+ switch (info XLOG_HEAP_OPMASK)
+ {
+ case XLOG_HEAP_INSERT:
+ id = INSERT;
+ break;
+ case XLOG_HEAP_DELETE:
+ id = DELETE;
+ break;
+ case XLOG_HEAP_UPDATE:
+ id = UPDATE;
+ break;
+ case XLOG_HEAP_HOT_UPDATE:
+ id = HOT_UPDATE;
+ break;
+ case XLOG_HEAP_LOCK:
+ id = LOCK;
+ break;
+ case XLOG_HEAP_INPLACE:
+ id = INPLACE;
+ break;
+ }
+
+ if (info XLOG_HEAP_INIT_PAGE)
+ id = append_init(id);
+
+ return id;
+}
Hm. I'm a bit doubtful about the static buffer used in
append_init(). That means the returned value from heap_identity() is
only valid until the next call. That at the very least needs to
Re: [HACKERS] pg_xlogdump --stats
At 2014-09-19 13:24:11 +0530, a...@2ndquadrant.com wrote:
Good enough?
Not quite. I've attached a small additional patch that shifts the
responsibility of adding rm_name to the output from xlog_outrec to
xlog_outdesc. Now we get WAL_DEBUG output like this:
LOG: INSERT @ 0/16C51D0: prev 0/16C5160; xid 692; len 31: Heap/INSERT: rel
1663/16384/16385; tid 0/5
…which is consistent with pg_xlogdump --stats output too.
-- Abhijit
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1265eca..b9bf206 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1287,7 +1287,7 @@ begin:;
for (; rdata != NULL; rdata = rdata-next)
appendBinaryStringInfo(recordbuf, rdata-data, rdata-len);
- appendStringInfoString(buf, - );
+ appendStringInfoString(buf, : );
xlog_outdesc(buf, rechdr-xl_rmid, (XLogRecord *) recordbuf.data);
}
elog(LOG, %s, buf.data);
@@ -6710,7 +6710,7 @@ StartupXLOG(void)
(uint32) (ReadRecPtr 32), (uint32) ReadRecPtr,
(uint32) (EndRecPtr 32), (uint32) EndRecPtr);
xlog_outrec(buf, record);
- appendStringInfoString(buf, - );
+ appendStringInfoString(buf, : );
xlog_outdesc(buf, record-xl_rmid, record);
elog(LOG, %s, buf.data);
pfree(buf.data);
@@ -9625,8 +9625,6 @@ xlog_outrec(StringInfo buf, XLogRecord *record)
if (record-xl_info XLR_BKP_BLOCK(i))
appendStringInfo(buf, ; bkpb%d, i);
}
-
- appendStringInfo(buf, : %s, RmgrTable[record-xl_rmid].rm_name);
}
#endif /* WAL_DEBUG */
@@ -9639,6 +9637,9 @@ xlog_outdesc(StringInfo buf, RmgrId rmid, XLogRecord *record)
{
const char *id;
+ appendStringInfoString(buf, RmgrTable[rmid].rm_name);
+ appendStringInfoChar(buf, '/');
+
id = RmgrTable[rmid].rm_identify(record-xl_info);
if (id == NULL)
appendStringInfo(buf, %X, record-xl_info);
--
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_xlogdump --stats
At 2014-09-19 10:44:48 +0200, and...@2ndquadrant.com wrote: # snprintfs that use %lld, %qd, or %I64d as the format. If none of these -# work, fall back to our own snprintf emulation (which we know uses %lld). +# works, fall back to our own snprintf emulation (which we know uses %lld). spurious independent change? It was part of the original patch, but I guess Heikki didn't commit it, so it was left over in the rebase. Also I actually think the original version is correct? It is not. I suspect it will begin to sound wrong to you if you replace none with not one in the sentence. But I don't care enough to argue about it. It's a very common error. + Stats record_stats[RM_NEXT_ID][16]; +} XLogDumpStats; I dislike the literal 16 here and someplace later. A define for the max number of records would make it clearer. OK, will change. Perhaps we should move these kind of checks outside? OK, will change. a) Whoever introduced the notion of rec_len vs tot_len in regards to including/excluding SizeOfXLogRecord ... (It wasn't me, honest!) b) I'm not against it, but I wonder if the best way to add the SizeOfXLogRecord to the record size. It's just as much part of the FPI. And this means that the record length will be 0 even if all the record data has been removed due to the FPI. I'm not sure I understand what you are proposing here. What was the reason you moved away from --stats=record/rmgr? I think we possibly will add further ones, so that seems more extensible? It was because I wanted --stats to default to =rmgr, so I tried to make the argument optional, but getopt in Windows didn't like that. Here's an excerpt from the earlier discussion: 3. Some compilation error in windows .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2065: 'optional_argument' : undeclared identifier .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2099: initializer is not a constant optional_argument should be added to getopt_long.h file for windows. Hmm. I have no idea what to do about this. I did notice when I wrote the code that nothing else used optional_argument, but I didn't realise that it wouldn't work on Windows. It may be that the best thing to do would be to avoid using optional_argument altogether, and have separate --stats and --stats-per-record options. Thoughts? I have no objection to doing it differently if someone tells me how to make Windows happy (preferably without making me unhappy). It's trivial to separate in this case, but I'd much rather have patches like this rm_identity stuff split up in the future. Sorry. I'd split it up that way in the past, but forgot to do it again in this round. Will do when I resend with the changes above. That means the returned value from heap_identity() is only valid until the next call. That at the very least needs to be written down explicitly somewhere. Where? In the comment in xlog_internal.h, perhaps? That's already in there afaics: Will fix (rebase cruft). Given that you've removed the UNKNOWNs from the rm_descs, this really should add it here. You would prefer to see HEAP/UNKNOWN rather than HEAP/32 for an unidentifiable xl_info? -- Abhijit -- 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_xlogdump --stats
Hi, On 2014-09-19 14:38:29 +0530, Abhijit Menon-Sen wrote: b) I'm not against it, but I wonder if the best way to add the SizeOfXLogRecord to the record size. It's just as much part of the FPI. And this means that the record length will be 0 even if all the record data has been removed due to the FPI. I'm not sure I understand what you are proposing here. I'm not really proposing anything. I'm just stating that it's notationally not clear and might be improved. Doesn't need to be now. What was the reason you moved away from --stats=record/rmgr? I think we possibly will add further ones, so that seems more extensible? It was because I wanted --stats to default to =rmgr, so I tried to make the argument optional, but getopt in Windows didn't like that. Here's an excerpt from the earlier discussion: 3. Some compilation error in windows .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2065: 'optional_argument' : undeclared identifier .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2099: initializer is not a constant optional_argument should be added to getopt_long.h file for windows. Hmm. I have no idea what to do about this. I did notice when I wrote the code that nothing else used optional_argument, but I didn't realise that it wouldn't work on Windows. It may be that the best thing to do would be to avoid using optional_argument altogether, and have separate --stats and --stats-per-record options. Thoughts? Oh, I've since implemented optional arguments for windows/replacement getopt_long. It's used by psql since a week or so ago. I have no objection to doing it differently if someone tells me how to make Windows happy (preferably without making me unhappy). [x] Done Given that you've removed the UNKNOWNs from the rm_descs, this really should add it here. You would prefer to see HEAP/UNKNOWN rather than HEAP/32 for an unidentifiable xl_info? UNKNOWN (32)? It should visually stand out more than just the number. Somebody borked something if that happens. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] GCC memory barriers are missing cc clobbers
Hi, barrier.h defines memory barriers for x86 as: 32bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%esp) : : : memory) 64bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%rsp) : : : memory) But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? 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] pg_xlogdump --stats
I've attached the revised patch, split up differently:
1. Introduce rm_identify, change rm_desc, glue the two together in
xlog.c
2. Introduce pg_xlogdump --stats[=record].
The requested changes (16, filter, z:, UNKNOWN) are included. The
grammar nitpicking and rebase cruft is^Ware^Wain't included.
I ran Postgres with WAL_DEBUG for a while, and I ran pg_xlogdump with
--stats (and --stats=rmgr) and --stats=record with and without a few
different variants of -r heap. Everything looked OK to me.
I hope I didn't miss anything this time.
-- Abhijit
From da6df2f24bb8ea768d6e7671474b0ad7d0b798d1 Mon Sep 17 00:00:00 2001
From: Abhijit Menon-Sen a...@2ndquadrant.com
Date: Fri, 19 Sep 2014 15:08:45 +0530
Subject: Introduce an RmgrDescData.rm_identify callback to name records
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
For example, rm_identify turns XLOG_BTREE_VACUUM into VACUUM based on
xl_info. rm_desc now omits the prefix that (unsystematically) duplicated
the rm_identity output, and only provides extra detail if available:
LOG: INSERT @ 0/16C50F0: prev 0/16C5080; xid 690; len 31: Heap/INSERT: rel 1663/16384/16385; tid 0/3
In the above, Heap comes from rm_name, INSERT comes from
rm_identify, and rel 1663/… comes from rm_desc.
The three are glued together by a new xlog_outdesc function in xlog.c.
---
contrib/pg_xlogdump/rmgrdesc.c| 2 +-
src/backend/access/rmgrdesc/clogdesc.c| 27 ---
src/backend/access/rmgrdesc/dbasedesc.c | 24 +-
src/backend/access/rmgrdesc/gindesc.c | 54 ++---
src/backend/access/rmgrdesc/gistdesc.c| 25 +-
src/backend/access/rmgrdesc/hashdesc.c| 6 ++
src/backend/access/rmgrdesc/heapdesc.c| 123 +
src/backend/access/rmgrdesc/mxactdesc.c | 37 ++---
src/backend/access/rmgrdesc/nbtdesc.c | 124 +++---
src/backend/access/rmgrdesc/relmapdesc.c | 19 -
src/backend/access/rmgrdesc/seqdesc.c | 21 +++--
src/backend/access/rmgrdesc/smgrdesc.c| 25 --
src/backend/access/rmgrdesc/spgdesc.c | 59 +++---
src/backend/access/rmgrdesc/standbydesc.c | 25 --
src/backend/access/rmgrdesc/tblspcdesc.c | 25 --
src/backend/access/rmgrdesc/xactdesc.c| 48 +---
src/backend/access/rmgrdesc/xlogdesc.c| 72 -
src/backend/access/transam/rmgr.c | 4 +-
src/backend/access/transam/xlog.c | 45 +--
src/include/access/clog.h | 1 +
src/include/access/gin.h | 1 +
src/include/access/gist_private.h | 1 +
src/include/access/hash.h | 1 +
src/include/access/heapam_xlog.h | 2 +
src/include/access/multixact.h| 1 +
src/include/access/nbtree.h | 1 +
src/include/access/rmgr.h | 2 +-
src/include/access/rmgrlist.h | 34
src/include/access/spgist.h | 1 +
src/include/access/xact.h | 1 +
src/include/access/xlog.h | 1 +
src/include/access/xlog_internal.h| 11 +++
src/include/catalog/storage_xlog.h| 1 +
src/include/commands/dbcommands.h | 1 +
src/include/commands/sequence.h | 1 +
src/include/commands/tablespace.h | 1 +
src/include/storage/standby.h | 1 +
src/include/utils/relmapper.h | 1 +
38 files changed, 597 insertions(+), 232 deletions(-)
diff --git a/contrib/pg_xlogdump/rmgrdesc.c b/contrib/pg_xlogdump/rmgrdesc.c
index cbcaaa6..1064d34 100644
--- a/contrib/pg_xlogdump/rmgrdesc.c
+++ b/contrib/pg_xlogdump/rmgrdesc.c
@@ -27,7 +27,7 @@
#include storage/standby.h
#include utils/relmapper.h
-#define PG_RMGR(symname,name,redo,desc,startup,cleanup) \
+#define PG_RMGR(symname,name,redo,desc,identify,startup,cleanup) \
{ name, desc, },
const RmgrDescData RmgrDescTable[RM_MAX_ID + 1] = {
diff --git a/src/backend/access/rmgrdesc/clogdesc.c b/src/backend/access/rmgrdesc/clogdesc.c
index e82baa8..8beb6d0 100644
--- a/src/backend/access/rmgrdesc/clogdesc.c
+++ b/src/backend/access/rmgrdesc/clogdesc.c
@@ -23,20 +23,29 @@ clog_desc(StringInfo buf, XLogRecord *record)
char *rec = XLogRecGetData(record);
uint8 info = record-xl_info ~XLR_INFO_MASK;
- if (info == CLOG_ZEROPAGE)
+ if (info == CLOG_ZEROPAGE || info == CLOG_TRUNCATE)
{
int pageno;
memcpy(pageno, rec, sizeof(int));
- appendStringInfo(buf, zeropage: %d, pageno);
+ appendStringInfo(buf, %d, pageno);
}
- else if (info == CLOG_TRUNCATE)
- {
- int pageno;
+}
- memcpy(pageno, rec, sizeof(int));
- appendStringInfo(buf, truncate before: %d, pageno);
+const char *
+clog_identify(uint8 info)
+{
+ const char *id = NULL;
+
+ switch (info)
+ {
+ case CLOG_ZEROPAGE:
+ id = ZEROPAGE;
+ break;
+ case CLOG_TRUNCATE:
+ id = TRUNCATE;
+ break;
}
- else
- appendStringInfoString(buf,
Re: [HACKERS] pg_xlogdump --stats
At 2014-09-19 15:39:37 +0530, a...@2ndquadrant.com wrote: I hope I didn't miss anything this time. But of course I did. The attached fixup makes the output of pg_xlogdump match that of xlog_outdesc for unidentifiable records (UNKNOWN (%x)). Sorry for the inconvenience. -- Abhijit diff --git a/contrib/pg_xlogdump/pg_xlogdump.c b/contrib/pg_xlogdump/pg_xlogdump.c index 0a176bb..7686a4f 100644 --- a/contrib/pg_xlogdump/pg_xlogdump.c +++ b/contrib/pg_xlogdump/pg_xlogdump.c @@ -515,7 +515,7 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogDumpStats *stats) id = desc-rm_identify(rj 4); if (id == NULL) - id = psprintf(0x%x, rj 4); + id = psprintf(UNKNOWN (%x), rj 4); XLogDumpStatsRow(psprintf(%s/%s, desc-rm_name, id), count, 100 * (double)count / total_count, -- 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] GCC memory barriers are missing cc clobbers
Hi, On 2014-09-19 12:00:16 +0200, Andres Freund wrote: barrier.h defines memory barriers for x86 as: 32bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%esp) : : : memory) 64bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%rsp) : : : memory) But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? What I missed is that x86 has an implied cc clobber for every inline assembly statement. So forget that. 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] Scaling shared buffer eviction
On Tue, Sep 16, 2014 at 10:21 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Sep 16, 2014 at 8:18 AM, Amit Kapila amit.kapil...@gmail.com wrote: In most cases performance with patch is slightly less as compare to HEAD and the difference is generally less than 1% and in a case or 2 close to 2%. I think the main reason for slight difference is that when the size of shared buffers is almost same as data size, the number of buffers it needs from clock sweep are very less, as an example in first case (when size of shared buffers is 12286MB), it actually needs at most 256 additional buffers (2MB) via clock sweep, where as bgreclaimer will put 2000 (high water mark) additional buffers (0.5% of shared buffers is greater than 2000 ) in free list, so bgreclaimer does some extra work when it is not required and it also leads to condition you mentioned down (freelist will contain buffers that have already been touched since we added them). Now for case 2 (12166MB), we need buffers more than 2000 additional buffers, but not too many, so it can also have similar effect. So there are two suboptimal things that can happen and they pull in opposite directions. I think you should instrument the server how often each is happening. #1 is that we can pop a buffer from the freelist and find that it's been touched. That means we wasted the effort of putting it on the freelist in the first place. #2 is that we can want to pop a buffer from the freelist and find it empty and thus be forced to run the clock sweep ourselves. If we're having problem #1, we could improve things by reducing the water marks. If we're having problem #2, we could improve things by increasing the water marks. If we're having both problems, then I dunno. But let's get some numbers on the frequency of these specific things, rather than just overall tps numbers. Specific numbers of both the configurations for which I have posted data in previous mail are as follows: Scale Factor - 800 Shared_Buffers - 12286MB (Total db size is 12288MB) Client and Thread Count = 64 buffers_touched_freelist - count of buffers that backends found touched after popping from freelist. buffers_backend_clocksweep - count of buffer allocations not satisfied from freelist buffers_alloc 1531023 buffers_backend_clocksweep 0 buffers_touched_freelist 0 Scale Factor - 800 Shared_Buffers - 12166MB (Total db size is 12288MB) Client and Thread Count = 64 buffers_alloc 1531010 buffers_backend_clocksweep 0 buffers_touched_freelist 0 In both the above cases, I have taken data multiple times to ensure correctness. From the above data, it is evident that in both the above configurations all the requests are satisfied from the initial freelist. Basically the amount of shared buffers configured (12286MB = 1572608 buffers and 12166MB = 1557248 buffers) are sufficient to contain all the work load for pgbench run. So now the question is why we are seeing small variation (1%) in data in case all the data fits in shared buffers and the reason could be that we have added few extra instructions (due to increase in StrategyControl structure size, additional function call, one or two new assignments) in the Buffer Allocation path (the extra instructions will also be only till all the data pages gets associated with buffers, after that the control won't even reach StrategyGetBuffer()) or it may be due to variation across different runs with different binaries. I have went ahead to take the data in cases shared buffers are tiny bit (0.1% and .05%) less than workload (based on buffer allocations done in above cases). Performance Data --- Scale Factor - 800 Shared_Buffers - 11950MB Client_Count/Patch_Ver 8 16 32 64 128 HEAD 68424 132540 195496 279511 283280 sbe_v9 68565 132709 194631 284351 289333 Scale Factor - 800 Shared_Buffers - 11955MB Client_Count/Patch_Ver 8 16 32 64 128 HEAD 68331 127752 196385 274387 281753 sbe_v9 68922 131314 194452 284292 287221 The above data indicates that performance is better with patch in almost all cases and especially at high concurrency (64 and 128 client count). The overall conclusion is that with patch a. when the data can fit in RAM and not completely in shared buffers, the performance/scalability is quite good even if shared buffers are just tiny bit less that all the data. b. when shared buffers are sufficient to contain all the data, then there is a slight difference (1%) in performance. d. Lets not do anything as if user does such a configuration, he should be educated to configure shared buffers in a better way and or the performance hit doesn't seem to be justified to do any further work. At least worth entertaining. Based on further analysis, I think this is the way to go. Attached find the patch for new stat (buffers_touched_freelist) just in case you want to run the patch with it and detailed (individual run) performance data. With Regards,
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
Hello, Maybe. Let's get the basic patch done first; then we can argue about that Please find attached patch to compress FPW using pglz compression. All backup blocks in WAL record are compressed at once before inserting it into WAL buffers . Full_page_writes GUC has been modified to accept three values 1. On 2. Compress 3. Off FPW are compressed when full_page_writes is set to compress. FPW generated forcibly during online backup even when full_page_writes is off are also compressed. When full_page_writes is set on FPW are not compressed. Benckmark: Server Specification: Processors:Intel® Xeon ® Processor E5-2650 (2 GHz, 8C/16T, 20 MB) * 2 nos RAM: 32GB Disk : HDD 450GB 10K Hot Plug 2.5-inch SAS HDD * 8 nos 1 x 450 GB SAS HDD, 2.5-inch, 6Gb/s, 10,000 rpm Checkpoint segments: 1024 Checkpoint timeout: 5 mins pgbench -c 64 -j 64 -r -T 900 -M prepared Scale factor: 1000 WAL generated (MB) Throughput (tps) Latency(ms) On 9235.43 979.03 65.36 Compress(pglz) 6518.68 1072.34 59.66 Off501.04 1135.17 56.34 The results show around 30 percent decrease in WAL volume due to compression of FPW. compress_fpw_v1.patch http://postgresql.1045698.n5.nabble.com/file/n5819645/compress_fpw_v1.patch -- View this message in context: http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5819645.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] jsonb format is pessimal for toast compression
On 09/18/2014 09:27 PM, Heikki Linnakangas wrote:
On 09/18/2014 07:53 PM, Josh Berkus wrote:
On 09/16/2014 08:45 PM, Tom Lane wrote:
We're somewhat comparing apples and oranges here, in that I pushed my
approach to something that I think is of committable quality (and which,
not incidentally, fixes some existing bugs that we'd need to fix in any
case); while Heikki's patch was just proof-of-concept. It would be worth
pushing Heikki's patch to committable quality so that we had a more
complete understanding of just what the complexity difference really is.
Is anyone actually working on this?
If not, I'm voting for the all-lengths patch so that we can get 9.4 out
the door.
I'll try to write a more polished patch tomorrow. We'll then see what it
looks like, and can decide if we want it.
Ok, here are two patches. One is a refined version of my earlier patch,
and the other implements the separate offsets array approach. They are
both based on Tom's jsonb-lengths-merged.patch, so they include all the
whitespace fixes etc. he mentioned.
There is no big difference in terms of code complexity between the
patches. IMHO the separate offsets array is easier to understand, but it
makes for more complicated accessor macros to find the beginning of the
variable-length data.
Unlike Tom's patch, these patches don't cache any offsets when doing a
binary search. Doesn't seem worth it, when the access time is O(1) anyway.
Both of these patches have a #define JB_OFFSET_STRIDE for the stride
size. For the separate offsets array, the offsets array has one element
for every JB_OFFSET_STRIDE children. For the other patch, every
JB_OFFSET_STRIDE child stores the end offset, while others store the
length. A smaller value makes random access faster, at the cost of
compressibility / on-disk size. I haven't done any measurements to find
the optimal value, the values in the patches are arbitrary.
I think we should bite the bullet and break compatibility with 9.4beta2
format, even if we go with my patch. In a jsonb object, it makes sense
to store all the keys first, like Tom did, because of cache benefits,
and the future possibility to do smart EXTERNAL access. Also, even if we
can make the on-disk format compatible, it's weird that you can get
different runtime behavior with datums created with a beta version.
Seems more clear to just require a pg_dump + restore.
Tom: You mentioned earlier that your patch fixes some existing bugs.
What were they? There were a bunch of whitespace and comment fixes that
we should apply in any case, but I couldn't see any actual bugs. I think
we should apply those fixes separately, to make sure we don't forget
about them, and to make it easier to review these patches.
- Heikki
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 2fd87fc..456011a 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -196,12 +196,12 @@ jsonb_from_cstring(char *json, int len)
static size_t
checkStringLen(size_t len)
{
- if (len JENTRY_POSMASK)
+ if (len JENTRY_LENMASK)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg(string too long to represent as jsonb string),
errdetail(Due to an implementation restriction, jsonb strings cannot exceed %d bytes.,
- JENTRY_POSMASK)));
+ JENTRY_LENMASK)));
return len;
}
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 04f35bf..7f7ed4f 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -26,15 +26,16 @@
* in MaxAllocSize, and the number of elements (or pairs) must fit in the bits
* reserved for that in the JsonbContainer.header field.
*
- * (the total size of an array's elements is also limited by JENTRY_POSMASK,
- * but we're not concerned about that here)
+ * (The total size of an array's or object's elements is also limited by
+ * JENTRY_LENMASK, but we're not concerned about that here.)
*/
#define JSONB_MAX_ELEMS (Min(MaxAllocSize / sizeof(JsonbValue), JB_CMASK))
#define JSONB_MAX_PAIRS (Min(MaxAllocSize / sizeof(JsonbPair), JB_CMASK))
-static void fillJsonbValue(JEntry *array, int index, char *base_addr,
+static void fillJsonbValue(JEntry *children, int index,
+ char *base_addr, uint32 offset,
JsonbValue *result);
-static bool equalsJsonbScalarValue(JsonbValue *a, JsonbValue *b);
+static bool equalsJsonbScalarValue(JsonbValue *a, JsonbValue *b);
static int compareJsonbScalarValue(JsonbValue *a, JsonbValue *b);
static Jsonb *convertToJsonb(JsonbValue *val);
static void convertJsonbValue(StringInfo buffer, JEntry *header, JsonbValue *val, int level);
@@ -42,7 +43,7 @@ static void convertJsonbArray(StringInfo buffer, JEntry *header, JsonbValue *val
static void convertJsonbObject(StringInfo buffer, JEntry *header, JsonbValue *val, int level);
static void convertJsonbScalar(StringInfo buffer, JEntry *header, JsonbValue
Re: [HACKERS] GCC memory barriers are missing cc clobbers
Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 12:00:16 +0200, Andres Freund wrote: But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? What I missed is that x86 has an implied cc clobber for every inline assembly statement. So forget that. While it might not be buggy as it stands, I think we should add the cc rather than rely on it being implicit. One reason is that people will look at the x86 cases when developing code for other architectures, and they could easily forget to add cc on machines where it does matter. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
Please find attached patch to compress FPW using pglz compression. Please refer the updated patch attached. The earlier patch added few duplicate lines of code in guc.c file. compress_fpw_v1.patch http://postgresql.1045698.n5.nabble.com/file/n5819659/compress_fpw_v1.patch Thank you, Rahila Syed -- View this message in context: http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5819659.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] jsonb format is pessimal for toast compression
Heikki Linnakangas hlinnakan...@vmware.com writes: Tom: You mentioned earlier that your patch fixes some existing bugs. What were they? What I remember at the moment (sans caffeine) is that the routines for assembling jsonb values out of field data were lacking some necessary tests for overflow of the size/offset fields. If you like I can apply those fixes separately, but I think they were sufficiently integrated with other changes in the logic that it wouldn't really help much for patch reviewability. 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] Anonymous code block with parameters
On Thu, Sep 18, 2014 at 5:22 PM, Hannu Krosing ha...@2ndquadrant.com wrote: Though it would be even nicer to have fully in-line type definition SELECT (tup).* FROM ( SELECT CASE WHEN .. THEN ROW(1,2,3)::(a int, b text, c int2) WHEN .. THEN ROW(2,3,4) ELSE ROW (3,4,5) END AS tup FROM .. ) ss +1. Workaround at present (which I mostly use during json serialization) is: SELECT (tup).* FROM ( SELECT CASE WHEN .. THEN (SELECT q FROM (SELECT 1, 2, 3) q) WHEN .. THEN (SELECT q FROM (SELECT 2, 3, 4) q) ELSE (SELECT q FROM (SELECT 3, 4, 5) q) END AS tup FROM .. ) ss If you're talking in line type definitions (which is kinda off topic) though, it'd be nice to consider: * nested type definition: create type foo_t as ( a text, b int, bars bar_t[] as ( c int, d text ), baz baz_t as ( e text, f text ) ); * ...and recursive type references (not being able to recursively serialize json is a major headache) create type foo_t as ( path text, children foo_t[] ); merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
Rahila Syed rahilasyed...@gmail.com writes: Please find attached patch to compress FPW using pglz compression. Patch not actually attached AFAICS (no, a link is not good enough). 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] Final Patch for GROUPING SETS
On Fri, Sep 19, 2014 at 4:45 AM, Andrew Gierth and...@tao11.riddles.org.uk wrote: GroupAggregate (cost=1122.39..1197.48 rows=9 width=8) Group Key: two, four Group Key: two Group Key: () Grouping Sets: [ [two, four], [two], [] +1 looks good to me. (yaml format) Grouping Sets: - - two - four - - two - Now this is weird. But is anyone actually using YAML output format, or was it implemented simply because we can? Marti Do you think it would be reasonable to normalize single-set Marti grouping sets into a normal GROUP BY? It's certainly possible, though it would seem somewhat odd to write queries that way. The reason I bring this up is that queries are frequently dynamically generated by programs. Coders are unlikely to special-case SQL generation when there's just a single grouping set. And that's the power of relational databases: the optimization work is done in the database pretty much transparently to the coder (when it works, that is). would you want the original syntax preserved in views Doesn't matter IMO. Marti I'd expect GROUP BY () to be fully equivalent to having no Marti GROUP BY clause, but there's a difference in explain Marti output. The former displays Grouping Sets: () which is odd, Marti since none of the grouping set keywords were used. That's an implementation artifact, in the sense that we preserve the fact that GROUP BY () was used by using an empty grouping set. Is it a problem, really, that it shows up that way in explain? No, not really a problem. :) Regards, Marti -- 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_xlogdump --stats
Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. I've pushed these after some fixing up. As we previously discussed, I think we might want to adjust the stats further. But this is already really useful. Thanks! 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] GCC memory barriers are missing cc clobbers
On 2014-09-19 09:58:01 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 12:00:16 +0200, Andres Freund wrote: But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? What I missed is that x86 has an implied cc clobber for every inline assembly statement. So forget that. While it might not be buggy as it stands, I think we should add the cc rather than rely on it being implicit. One reason is that people will look at the x86 cases when developing code for other architectures, and they could easily forget to add cc on machines where it does matter. Fair point. It's also extremly poorly documented - my answer is from a gcc dev, I haven't found an official document stating it. I don't really see any need to backpatch though, do you? 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] GCC memory barriers are missing cc clobbers
Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 09:58:01 -0400, Tom Lane wrote: While it might not be buggy as it stands, I think we should add the cc rather than rely on it being implicit. One reason is that people will look at the x86 cases when developing code for other architectures, and they could easily forget to add cc on machines where it does matter. Fair point. It's also extremly poorly documented - my answer is from a gcc dev, I haven't found an official document stating it. I don't really see any need to backpatch though, do you? Well, I'd make it the same in all branches which have that code, which is not very far back is it? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GCC memory barriers are missing cc clobbers
On 2014-09-19 10:58:56 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I don't really see any need to backpatch though, do you? Well, I'd make it the same in all branches which have that code, which is not very far back is it? It was already introduced in 9.2 - no idea whether that's far back or not ;). Done. 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
[HACKERS] identify_locking_dependencies is broken for schema-only dumps
This can lead to deadlocks during parallel restore. Test case: create table bar (a int primary key, b int); create table baz (z int, a int references bar); create view foo as select a, b, sum(1) from bar group by a union all select z, a, 0 from baz; I dumped this with: pg_dump -Fc -s -f test.dmp Then: while (dropdb rhaas; createdb; pg_restore -O -d rhaas -j3 test.dmp); do true; done This quickly fails for me with: pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 2155; 2606 47822 FK CONSTRAINT baz_a_fkey rhaas pg_restore: [archiver (db)] could not execute query: ERROR: deadlock detected DETAIL: Process 81791 waits for AccessExclusiveLock on relation 47862 of database 47861; blocked by process 81789. Process 81789 waits for AccessShareLock on relation 47865 of database 47861; blocked by process 81791. HINT: See server log for query details. Command was: ALTER TABLE ONLY baz ADD CONSTRAINT baz_a_fkey FOREIGN KEY (a) REFERENCES bar(a); WARNING: errors ignored on restore: 2 The attached patch seems to fix it for me. Comments? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index ded9135..0393153 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -4140,11 +4140,10 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te) return; /* - * We assume the item requires exclusive lock on each TABLE DATA item - * listed among its dependencies. (This was originally a dependency on - * the TABLE, but fix_dependencies repointed it to the data item. Note - * that all the entry types we are interested in here are POST_DATA, so - * they will all have been changed this way.) + * We assume the item requires exclusive lock on each TABLE or TABLE DATA + * item listed among its dependencies. (fix_dependencies may have + * repointed TABLE dependencies at TABLE DATA items, but not in a + * schema-only dump.) */ lockids = (DumpId *) pg_malloc(te-nDeps * sizeof(DumpId)); nlockids = 0; @@ -4153,7 +4152,8 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te) DumpId depid = te-dependencies[i]; if (depid = AH-maxDumpId AH-tocsByDumpId[depid] != NULL - strcmp(AH-tocsByDumpId[depid]-desc, TABLE DATA) == 0) + ((strcmp(AH-tocsByDumpId[depid]-desc, TABLE DATA) == 0) || + strcmp(AH-tocsByDumpId[depid]-desc, TABLE) == 0)) lockids[nlockids++] = depid; } -- 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] Final Patch for GROUPING SETS
Marti == Marti Raudsepp ma...@juffo.org writes: (yaml format) Grouping Sets: - - two - four - - two - Marti Now this is weird. You're telling me. Also, feeding it to an online yaml-to-json converter gives the result as [[two,four],[two],null] which is not quite the same as the json version. An alternative would be: Grouping Sets: - - two - four - - two - [] or Grouping Sets: - - two - four - - two - [] though I haven't managed to get that second one to work yet. Marti But is anyone actually using YAML output format, or was it Marti implemented simply because we can? Until someone decides to dike it out, I think we are obligated to make it produce something resembling correct output. Marti The reason I bring this up is that queries are frequently Marti dynamically generated by programs. Good point. would you want the original syntax preserved in views Marti Doesn't matter IMO. I think it's fairly consistent for the parser to do this, since we do a number of other normalization steps there (removing excess nesting and so on). This turns out to be quite trivial. -- 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] Final Patch for GROUPING SETS
On 2014-09-19 16:35:52 +0100, Andrew Gierth wrote: Marti But is anyone actually using YAML output format, or was it Marti implemented simply because we can? Until someone decides to dike it out, I think we are obligated to make it produce something resembling correct output. I vote for ripping it out. There really isn't any justification for it and it broke more than once. Greg: Did you actually ever end up using the yaml output? 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] Final Patch for GROUPING SETS
Andrew == Andrew Gierth and...@tao11.riddles.org.uk writes: Andrew You're telling me. Also, feeding it to an online yaml-to-json Andrew converter gives the result as [[two,four],[two],null] Andrew which is not quite the same as the json version. An Andrew alternative would be: Oh, another YAML alternative would be: Grouping Sets: - [two,four] - [two] - [] Would that be better? (It's not consistent with other YAML outputs like sort/group keys, but it's equally legal as far as I can tell and seems more readable.) -- 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] RLS Design
On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. I wonder if I am equally free to commit my own patches without properly observing the CommitFest process, because it would be a whole lot faster. My pg_background patches have been pending since before the start of the August CommitFest and I accepted that I would have to wait an extra two months to commit those because of a *clerical error*, namely my failure to actually add them to the CommitFest. This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I'm really disappointed by that. I feel I'm essentially getting punished for trying to follow what I understand to the process, which has involved me doing huge amounts of review of other people's patches and waiting a very long time to get my own stuff committed, while you bull ahead with your own patches. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
On Fri, Sep 19, 2014 at 11:05 PM, Rahila Syed rahilasyed...@gmail.com wrote: Please find attached patch to compress FPW using pglz compression. Please refer the updated patch attached. The earlier patch added few duplicate lines of code in guc.c file. compress_fpw_v1.patch http://postgresql.1045698.n5.nabble.com/file/n5819659/compress_fpw_v1.patch I got patching failed to HEAD. Detail is following. Hunk #3 FAILED at 142. 1 out of 3 hunks FAILED -- saving rejects to file src/backend/access/rmgrdesc/xlogdesc.c.rej Regards, --- Sawada Masahiko -- 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] Final Patch for GROUPING SETS
On 19/09/14 17:52, Andres Freund wrote: On 2014-09-19 16:35:52 +0100, Andrew Gierth wrote: Marti But is anyone actually using YAML output format, or was it Marti implemented simply because we can? Until someone decides to dike it out, I think we are obligated to make it produce something resembling correct output. I vote for ripping it out. There really isn't any justification for it and it broke more than once. Even though I really like YAML I say +1, mainly because any YAML 1.2 parser should be able to parse JSON output without problem... -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] RLS Design
On 2014-09-19 11:53:06 -0400, Robert Haas wrote: On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net wrote: If we want to be able to disable RLS w/o dropping the policies, then I think we have to completely de-couple the two and users would then have both add policies AND turn on RLS to have RLS actually be enabled for a given table. I'm on the fence about that. Thoughts? A strong +1 for doing just that. Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. I was also rather surprised by the push. I wanted to write something about it, but: This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. says it better. I think that's generally the case, but doubly so with sensitive stuff like this. I wonder if I am equally free to commit my own patches without properly observing the CommitFest process, because it would be a whole lot faster. My pg_background patches have been pending since before the start of the August CommitFest and I accepted that I would have to wait an extra two months to commit those because of a *clerical error*, namely my failure to actually add them to the CommitFest. FWIW, I think if a patch has been sent in time and has gotten a decent amount of review *and* agreement it's fair for a committer to push forward. That doesn't apply to this thread, but sometimes does for others. 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
[HACKERS] CreateEventTrigStmt copy fix
Hi hackers,
I was trying to create event trigger inside DO statement inside an
extension SQL script and noticed that the new event trigger has empty
evtevent field.
After some tinkering with gdb I found out that the memory context
switches sometimes clear the eventname in CreateEventTrigStmt struct.
The reason for this is that _copyCreateEventTrigStmt uses
COPY_SCALAR_FIELD on eventname instead of COPY_STRING_FIELD.
Attached patch fixes this and also the same issue in
_equalCreateEventTrigStmt.
This should be back-patched to 9.3 where event triggers were introduced.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index aa053a0..24addfb 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3610,7 +3610,7 @@ _copyCreateEventTrigStmt(const CreateEventTrigStmt *from)
CreateEventTrigStmt *newnode = makeNode(CreateEventTrigStmt);
COPY_STRING_FIELD(trigname);
- COPY_SCALAR_FIELD(eventname);
+ COPY_STRING_FIELD(eventname);
COPY_NODE_FIELD(whenclause);
COPY_NODE_FIELD(funcname);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 719923e..9c19f44 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1805,7 +1805,7 @@ static bool
_equalCreateEventTrigStmt(const CreateEventTrigStmt *a, const CreateEventTrigStmt *b)
{
COMPARE_STRING_FIELD(trigname);
- COMPARE_SCALAR_FIELD(eventname);
+ COMPARE_STRING_FIELD(eventname);
COMPARE_NODE_FIELD(funcname);
COMPARE_NODE_FIELD(whenclause);
--
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] RLS Design
On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
On Thu, Sep 11, 2014 at 3:08 PM, Stephen Frost sfr...@snowman.net
wrote:
If we want to be able to disable RLS w/o dropping the policies, then I
think we have to completely de-couple the two and users would then have
both add policies AND turn on RLS to have RLS actually be enabled for a
given table. I'm on the fence about that.
Thoughts?
A strong +1 for doing just that.
Alright, updated patch attached which does just that (thanks to Adam
for the updates for this and testing pg_dump- I just reviewed it and
added some documentation updates and other minor improvements), and
rebased to master. Also removed the catversion bump, so it should apply
cleanly for people, for a while anyway.
This is testing what has been committed:
# create table colours (id serial, name text, visible boolean);
CREATE TABLE
# insert into colours (name, visible) values
('blue',true),('yellow',true),('ultraviolet',false),('green',true),('infrared',false);
INSERT 0 5
# create policy visible_colours on colours for all to joe using (visible =
true);
CREATE POLICY
# grant all on colours to public;
GRANT
# grant all on sequence colours_id_seq to public;
GRANT
# alter table colours enable row level security ;
ALTER TABLE
\c - joe
select * from colours;
id | name | visible
++-
1 | blue | t
2 | yellow | t
4 | green | t
(3 rows)
insert into colours (name, visible) values ('purple',true);
INSERT 0 1
insert into colours (name, visible) values ('transparent',false);
ERROR: new row violates WITH CHECK OPTION for colours
DETAIL: Failing row contains (7, transparent, f).
select * from pg_policies ;
policyname| tablename | roles | cmd | qual | with_check
-+---+---+-+--+
visible_colours | colours | {joe} | ALL | (visible = true) |
(1 row)
There was no WITH CHECK OPTION.
--
Thom
Re: [HACKERS] RLS Design
Thom,
Thanks!
* Thom Brown (t...@linux.com) wrote:
On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote:
# create policy visible_colours on colours for all to joe using (visible =
true);
CREATE POLICY
[...]
insert into colours (name, visible) values ('transparent',false);
ERROR: new row violates WITH CHECK OPTION for colours
DETAIL: Failing row contains (7, transparent, f).
select * from pg_policies ;
policyname| tablename | roles | cmd | qual | with_check
-+---+---+-+--+
visible_colours | colours | {joe} | ALL | (visible = true) |
(1 row)
There was no WITH CHECK OPTION.
As I hope is clear if you look at the documentation- if the WITH CHECK
clause is omitted, then the USING clause is used for both filtering and
checking new records, otherwise you'd be able to add records which
aren't visible to you.
Thanks!
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] RLS Design
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Sun, Sep 14, 2014 at 11:38 AM, Stephen Frost sfr...@snowman.net wrote: Alright, updated patch attached which does just that (thanks to Adam for the updates for this and testing pg_dump- I just reviewed it and added some documentation updates and other minor improvements), and rebased to master. Also removed the catversion bump, so it should apply cleanly for people, for a while anyway. I specifically asked you to hold off on committing this until there was adequate opportunity for review, and explained my reasoning. You committed it anyway. Hum- my apologies, I honestly don't recall you specifically asking for it to be held off indefinitely. :( There was discussion back and forth, quite a bit of it with you, and I thank you for your help with that and certainly welcome any additional comments. This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. I'm really disappointed by that. I feel I'm essentially getting punished for trying to follow what I understand to the process, which has involved me doing huge amounts of review of other people's patches and waiting a very long time to get my own stuff committed, while you bull ahead with your own patches. While I wasn't public about it, I actually specifically discussed this question with others, a few times even, to try and make sure that I wasn't stepping out of line by moving forward. That said, I do see that Andres feels similairly. It certainly wasn't my intent to surprise anyone by it but simply to continue to move forward- in part, to allow me to properly break from it and work on other things, including reviewing other patches in the commitfest. I fear I've simply been overly focused on it these past few weeks, for a variety of reasons that would likely best be discussed at the pub. All-in-all, I feel appropriately chastised and certainly don't wish to be surprising fellow committers. Perhaps we can discuss at the dev meeting. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] RLS Design
On 19 September 2014 17:32, Stephen Frost sfr...@snowman.net wrote:
Thom,
Thanks!
* Thom Brown (t...@linux.com) wrote:
On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote:
# create policy visible_colours on colours for all to joe using (visible
=
true);
CREATE POLICY
[...]
insert into colours (name, visible) values ('transparent',false);
ERROR: new row violates WITH CHECK OPTION for colours
DETAIL: Failing row contains (7, transparent, f).
select * from pg_policies ;
policyname| tablename | roles | cmd | qual |
with_check
-+---+---+-+--+
visible_colours | colours | {joe} | ALL | (visible = true) |
(1 row)
There was no WITH CHECK OPTION.
As I hope is clear if you look at the documentation- if the WITH CHECK
clause is omitted, then the USING clause is used for both filtering and
checking new records, otherwise you'd be able to add records which
aren't visible to you.
I can see that now, although I do find the error message somewhat
confusing. Firstly, it looks like OPTION is part of the parameter name,
which it isn't.
Also, I seem to get an error message with the following:
# create policy nice_colours ON colours for all to joe using (visible =
true) with check (name in ('blue','green','yellow'));
CREATE POLICY
\c - joe
insert into colours (name, visible) values ('blue',false);
ERROR: function with OID 0 does not exist
And if this did work, but I only violated the USING clause, would this
still say the WITH CHECK clause was the cause?
Thom
Re: [HACKERS] RLS Design
On 2014-09-19 12:38:39 -0400, Stephen Frost wrote: I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. I had been hoping that there would be another review coming from somewhere, but there is always a trade-off between waiting longer to get a review ahead of a commit and having it committed and then available more easily for others to work with, review, and generally moving forward. Sure, there is such a tradeoff. But others have to wait months to get enough review. The first revision of the patch in the form you committed was sent 2014-08-19, the first marked *ready for review* (not my words) is from 2014-08-30. 19 days really isn't very far along the tradeoff from waiting for a review to uselessly waiting. 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] RLS Design
On Fri, Sep 19, 2014 at 12:38 PM, Stephen Frost sfr...@snowman.net wrote: This patch, on the other hand, was massively revised after the start of the CommitFest after many months of inactivity and committed with no thorough review by anyone who was truly independent of the development effort. It was then committed with no warning over a specific request, from another committer, that more time be allowed for review. I would not (nor do I feel that I did..) have committed it over a specific request to not do so from another committer. Well, you're wrong. How could this email possibly have been any more clear? http://www.postgresql.org/message-id/CA+TgmoYA=uixxmn390sfgfqgvmll-as5bjal0om7yrppvwn...@mail.gmail.com You can hardly tell me you didn't see that email when you incorporated the technical content into the next patch version. While I wasn't public about it, I actually specifically discussed this question with others, a few times even, to try and make sure that I wasn't stepping out of line by moving forward. And yet you completely ignored the only public commentary on the issue, which was from me. I *should not have had* to object to this patch going in. It was clearly untimely for the August CommitFest, and as a long-time community member, you ought to know full well that any such patch should be resubmitted to a later CommitFest. This patch sat on the shelf for 4 months because you were too busy to work on it, and was committed 5 days from the last posted version, which version had zero review comments. If you didn't have time to work on it for 4 months, you can hardly expect everyone else who has an opinion to comment within 5 days. But, you know, because I could tell that you were fixated on pushing this patch through to commit quickly, I took the time to send you a message on that specific point, even though you should have known full well. In fact I took the time to send TWO. Here's the other one: http://www.postgresql.org/message-id/ca+tgmobqo0z87eivfdewjcac1dc4ahh5wcvoqoxrsateu1t...@mail.gmail.com All-in-all, I feel appropriately chastised and certainly don't wish to be surprising fellow committers. Perhaps we can discuss at the dev meeting. No, I think we should discuss it right now, not nine months from now when the issue has faded from everyone's mind. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] RLS Design
Thom,
* Thom Brown (t...@linux.com) wrote:
On 19 September 2014 17:32, Stephen Frost sfr...@snowman.net wrote:
* Thom Brown (t...@linux.com) wrote:
On 14 September 2014 16:38, Stephen Frost sfr...@snowman.net wrote:
# create policy visible_colours on colours for all to joe using (visible
=
true);
CREATE POLICY
[...]
insert into colours (name, visible) values ('transparent',false);
ERROR: new row violates WITH CHECK OPTION for colours
DETAIL: Failing row contains (7, transparent, f).
select * from pg_policies ;
policyname| tablename | roles | cmd | qual |
with_check
-+---+---+-+--+
visible_colours | colours | {joe} | ALL | (visible = true) |
(1 row)
There was no WITH CHECK OPTION.
As I hope is clear if you look at the documentation- if the WITH CHECK
clause is omitted, then the USING clause is used for both filtering and
checking new records, otherwise you'd be able to add records which
aren't visible to you.
I can see that now, although I do find the error message somewhat
confusing. Firstly, it looks like OPTION is part of the parameter name,
which it isn't.
Hmm, the notion of 'with check option' is from the SQL standard, which
is why I felt the error message was appropriate as-is..
Also, I seem to get an error message with the following:
# create policy nice_colours ON colours for all to joe using (visible =
true) with check (name in ('blue','green','yellow'));
CREATE POLICY
\c - joe
insert into colours (name, visible) values ('blue',false);
ERROR: function with OID 0 does not exist
Now *that* one is interesting and I'll definitely go take a look at it.
We added quite a few regression tests to try and make sure these things
work.
And if this did work, but I only violated the USING clause, would this
still say the WITH CHECK clause was the cause?
WITH CHECK applies for INSERT and UPDATE for the new records going into
the table. You can't actually violate the USING clause for an INSERT
as USING is for filtering records, not checking that records being added
to the table are valid.
To try and clarify- by explicitly setting both USING and WITH CHECK, you
*are* able to INSERT records which are not visible to you. We felt that
was an important capability to support.
Thanks for taking a look at it!
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] B-Tree support function number 3 (strxfrm() optimization)
On Tue, Sep 16, 2014 at 4:55 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Sep 16, 2014 at 1:45 PM, Robert Haas robertmh...@gmail.com wrote: Even though our testing seems to indicate that the memcmp() is basically free, I think it would be good to make the effort to avoid doing memcmp() and then strcoll() and then strncmp(). Seems like it shouldn't be too hard. Really? The tie-breaker for the benefit of locales like hu_HU uses strcmp(), not memcmp(). It operates on the now-terminated copies of strings. There is no reason to think that the strings must be the same size for that strcmp(). I'd rather only do the new opportunistic memcmp() == 0 thing when len1 == len2. And I wouldn't like to have to also figure out that it's safe to use the earlier result, because as it happens len1 == len2, or any other such trickery. OK, good point. So committed as-is, then, except that I rewrote the comments, which I felt were excessively long for the amount of code. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor improvement in lock.sgml
On Tue, Sep 16, 2014 at 7:20 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Here is a patch to a bit improve the reference page for the LOCK command. I think it'd be better for the isolation level to be in capitals and wrapped in the literal tags. It's done that way elsewhere in the same page, so committed. Overall, there is some ambiguity about this. Most places follow your proposed style if literalREAD COMMITTED/literal, literalREPEATABLE READ/literal, and literalSERIALIZABLE/literal, but mvcc.sgml, which discusses isolation levels extensively, just writes them as Read Committed, Repeatable Read, and Serializable. I'm not sure whether more consistency overall would be a good thing - there are some things to recommend the current mix of styles - but mixing styles within a page doesn't seem smart, so this much at least seems uncontroversial. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Support for N synchronous standby servers
On Tue, Sep 16, 2014 at 2:19 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Sep 16, 2014 at 5:25 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Sep 15, 2014 at 3:00 PM, Michael Paquier michael.paqu...@gmail.com wrote: At least a set of hooks has the merit to say: do what you like with your synchronous node policy. Sure. I dunno if people will find that terribly user-friendly, so we might not want that to be the ONLY thing we offer. Well, user-friendly interface is actually the reason why a simple GUC integer was used in the first series of patches present on this thread to set as sync the N-nodes with the lowest priority. I could not come up with something more simple. Hence what about doing the following: - A patch refactoring code for pg_stat_get_wal_senders and SyncRepReleaseWaiters as there is in either case duplicated code in this area to select the synchronous node as the one connected with lowest priority A strong +1 for this idea. I have never liked that, and cleaning it up seems eminently sensible. - A patch defining the hooks necessary, I suspect that two of them are necessary as mentioned upthread. - A patch for a contrib module implementing an example of simple policy. It can be a fancy thing with a custom language or even a more simple thing. I'm less convinced about this part. There's a big usability gap between a GUC and a hook, and I think Heikki's comments upthread were meant to suggest that even in GUC-land we can probably satisfy more use cases that what this patch does now. I think that's right. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Final Patch for GROUPING SETS
On 09/19/2014 08:52 AM, Andres Freund wrote: Until someone decides to dike it out, I think we are obligated to make it produce something resembling correct output. I vote for ripping it out. There really isn't any justification for it and it broke more than once. (a) I personally use it all the time to produce human-readable output, sometimes also working via markdown. It's easier to read than the standard format or JSON, especially when combined with grep or other selective filtering. Note that this use would not at all preclude having the YAML output look wierd as long as it was readable. (b) If we're going to discuss ripping out YAML format, please let's do that as a *separate* patch and discussion, and not as a side effect of Grouping Sets. Otherwise this will be one of those things where people pitch a fit during beta because the people who care about YAML aren't necessarily reading this thread. On 09/19/2014 08:52 AM, Andrew Gierth wrote: Oh, another YAML alternative would be: Grouping Sets: - [two,four] - [two] - [] Would that be better? (It's not consistent with other YAML outputs like sort/group keys, but it's equally legal as far as I can tell and seems more readable.) That works for me. -- Josh Berkus PostgreSQL Experts Inc. http://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] CreateEventTrigStmt copy fix
On Fri, Sep 19, 2014 at 11:09 AM, Petr Jelinek p...@2ndquadrant.com wrote: Hi hackers, I was trying to create event trigger inside DO statement inside an extension SQL script and noticed that the new event trigger has empty evtevent field. After some tinkering with gdb I found out that the memory context switches sometimes clear the eventname in CreateEventTrigStmt struct. The reason for this is that _copyCreateEventTrigStmt uses COPY_SCALAR_FIELD on eventname instead of COPY_STRING_FIELD. Attached patch fixes this and also the same issue in _equalCreateEventTrigStmt. This should be back-patched to 9.3 where event triggers were introduced. Nice catch! And no need to care much about outfuncs.c for this Node type. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing volatile qualifiers from lwlock.c
On Wed, Sep 17, 2014 at 7:45 AM, Andres Freund and...@2ndquadrant.com wrote: I just tried this on my normal x86 workstation. I applied your lwlock patch and ontop I removed most volatiles (there's a couple still required) from xlog.c. Works for 100 seconds. Then I reverted the above commits. Breaks within seconds: master: LOG: request to flush past end of generated WAL; request 2/E5EC3DE0, currpos 2/E5EC1E60 standby: LOG: record with incorrect prev-link 4/684C3108 at 4/684C3108 and similar. So at least for x86 the compiler barriers are obviously required and seemingly working. Oh, that's great. In that case I think I should go ahead and apply that patch in the hopes of turning up any systems where the barriers aren't working properly yet. Although it would be nice to know whether it breaks with *only* the lwlock.c patch. I've attached the very quickly written xlog.c de-volatizing patch. I don't have time to go through this in detail, but I don't object to you applying it if you're confident you've done it carefully enough. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Final Patch for GROUPING SETS
Josh == Josh Berkus j...@agliodbs.com writes: Josh (b) If we're going to discuss ripping out YAML format, please Josh let's do that as a *separate* patch and discussion, +infinity Grouping Sets: - [two,four] - [two] - [] Would that be better? (It's not consistent with other YAML outputs like sort/group keys, but it's equally legal as far as I can tell and seems more readable.) Josh That works for me. I prefer that one to any of the others I've come up with, so unless anyone has a major objection, I'll go with it. -- 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] Turning off HOT/Cleanup sometimes
On Mon, Sep 15, 2014 at 5:13 PM, Simon Riggs si...@2ndquadrant.com wrote: On 15 September 2014 17:09, Robert Haas robertmh...@gmail.com wrote: Do we really want to disable HOT for all catalog scans? The intention of the patch is that catalog scans are treated identically to non-catalog scans. The idea here is that HOT cleanup only occurs on scans on target relations, so only INSERT, UPDATE and DELETE do HOT cleanup. It's possible that many catalog scans don't follow the normal target relation logic, so we might argue we should use HOT every time. OTOH, since we now have separate catalog xmins we may find that using HOT on catalogs is no longer effective. So I could go either way on how to proceed; its an easy change either way. What I'm thinking about is that the smarts to enable pruning is all in the executor nodes. So anything that updates the catalog without going through the executor will never be subject to pruning. That includes nearly all catalog-modifying code throughout the backend. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] B-Tree support function number 3 (strxfrm() optimization)
On Fri, Sep 19, 2014 at 9:59 AM, Robert Haas robertmh...@gmail.com wrote: OK, good point. So committed as-is, then, except that I rewrote the comments, which I felt were excessively long for the amount of code. Thanks! I look forward to hearing your thoughts on the open issues with the patch as a whole. -- 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] removing volatile qualifiers from lwlock.c
On 2014-09-19 13:58:17 -0400, Robert Haas wrote: On Wed, Sep 17, 2014 at 7:45 AM, Andres Freund and...@2ndquadrant.com wrote: I just tried this on my normal x86 workstation. I applied your lwlock patch and ontop I removed most volatiles (there's a couple still required) from xlog.c. Works for 100 seconds. Then I reverted the above commits. Breaks within seconds: master: LOG: request to flush past end of generated WAL; request 2/E5EC3DE0, currpos 2/E5EC1E60 standby: LOG: record with incorrect prev-link 4/684C3108 at 4/684C3108 and similar. So at least for x86 the compiler barriers are obviously required and seemingly working. Oh, that's great. In that case I think I should go ahead and apply that patch in the hopes of turning up any systems where the barriers aren't working properly yet. Agreed. Although it would be nice to know whether it breaks with *only* the lwlock.c patch. It didn't, at least not visibly within the 1000s I let pgbench run. I've attached the very quickly written xlog.c de-volatizing patch. I don't have time to go through this in detail, but I don't object to you applying it if you're confident you've done it carefully enough. It's definitely not yet carefully enough checked. I wanted to get it break fast and only made one pass through the file. But I think it should be easy enough to get it into shape for that. 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] Anonymous code block with parameters
On Fri, Sep 19, 2014 at 9:26 AM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Sep 18, 2014 at 5:22 PM, Hannu Krosing ha...@2ndquadrant.com wrote: Though it would be even nicer to have fully in-line type definition SELECT (tup).* FROM ( SELECT CASE WHEN .. THEN ROW(1,2,3)::(a int, b text, c int2) WHEN .. THEN ROW(2,3,4) ELSE ROW (3,4,5) END AS tup FROM .. ) ss +1. Workaround at present (which I mostly use during json serialization) is: SELECT (tup).* FROM ( SELECT CASE WHEN .. THEN (SELECT q FROM (SELECT 1, 2, 3) q) WHEN .. THEN (SELECT q FROM (SELECT 2, 3, 4) q) ELSE (SELECT q FROM (SELECT 3, 4, 5) q) END AS tup FROM .. ) ss actually, this trick *only* works during json serialization -- it allows control over the column names that row() masks over. trying to expand (tup).* still gives the dreaded ERROR: record type has not been registered. That's because this works: select (q).* from (select 1 as a, 2 as b) q; but this doesn't: select ((select q from (select a,b) q)).* from (select 1 as a, 2 as b) q; merlin -- 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 block with parameters
On 2014-09-19 8:20 PM, Merlin Moncure wrote: actually, this trick *only* works during json serialization -- it allows control over the column names that row() masks over. trying to expand (tup).* still gives the dreaded ERROR: record type has not been registered. That's because this works: select (q).* from (select 1 as a, 2 as b) q; but this doesn't: select ((select q from (select a,b) q)).* from (select 1 as a, 2 as b) q; Yeah. This is a seriously missing feature and a PITA. :-( .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] Turning off HOT/Cleanup sometimes
On 19 September 2014 13:04, Robert Haas robertmh...@gmail.com wrote: What I'm thinking about is that the smarts to enable pruning is all in the executor nodes. So anything that updates the catalog without going through the executor will never be subject to pruning. That includes nearly all catalog-modifying code throughout the backend. Are you saying this is a problem or a benefit? (and please explain why). -- 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] Turning off HOT/Cleanup sometimes
On September 19, 2014 10:16:35 PM CEST, Simon Riggs si...@2ndquadrant.com wrote: On 19 September 2014 13:04, Robert Haas robertmh...@gmail.com wrote: What I'm thinking about is that the smarts to enable pruning is all in the executor nodes. So anything that updates the catalog without going through the executor will never be subject to pruning. That includes nearly all catalog-modifying code throughout the backend. Are you saying this is a problem or a benefit? (and please explain why). I have no idea what Robert is thinking of, but I'd imagine its horrible for workloads with catalog bloat. Like ones involving temp tables. I generally have serious doubts about disabling it generally for read workloads. I imagine it e.g. will significantly penalize workloads where its likely that a cleanup lock can't be acquired every time... Andres --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] Turning off HOT/Cleanup sometimes
Andres Freund and...@anarazel.de writes: On September 19, 2014 10:16:35 PM CEST, Simon Riggs si...@2ndquadrant.com wrote: Are you saying this is a problem or a benefit? (and please explain why). I have no idea what Robert is thinking of, but I'd imagine its horrible for workloads with catalog bloat. Like ones involving temp tables. Yeah. But it's also the case that we know a good deal more about the access patterns for system-driven catalog updates than we do about user queries. ISTM we could probably suppress HOT pruning during catalog *scans* and instead try to do it when a system-driven heap_update occurs. Having said that, this could reasonably be considered outside the scope of a patch that's trying to improve the behavior for user queries. But if the patch author doesn't want to expand the scope like that, ISTM he ought to ensure that the behavior *doesn't* change for system accesses, rather than trying to convince us that disabling HOT for system updates is a good idea. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On Fri, Sep 19, 2014 at 4:30 PM, Andres Freund and...@anarazel.de wrote: On September 19, 2014 10:16:35 PM CEST, Simon Riggs si...@2ndquadrant.com wrote: On 19 September 2014 13:04, Robert Haas robertmh...@gmail.com wrote: What I'm thinking about is that the smarts to enable pruning is all in the executor nodes. So anything that updates the catalog without going through the executor will never be subject to pruning. That includes nearly all catalog-modifying code throughout the backend. Are you saying this is a problem or a benefit? (and please explain why). I have no idea what Robert is thinking of, but I'd imagine its horrible for workloads with catalog bloat. Like ones involving temp tables. Right, that's what I was going for. I generally have serious doubts about disabling it generally for read workloads. I imagine it e.g. will significantly penalize workloads where its likely that a cleanup lock can't be acquired every time... I share that doubt. But I understand why Simon wants to do something, too, because the current situation is not great either. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] B-Tree support function number 3 (strxfrm() optimization)
On Thu, Sep 11, 2014 at 8:34 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Sep 9, 2014 at 2:25 PM, Robert Haas robertmh...@gmail.com wrote: I like that I don't have to care about every combination, and can treat abbreviation abortion as the special case with the extra step, in line with how I think of the optimization conceptually. Does that make sense? No. comparetup_heap() is hip-deep in this optimization as it stands, and what I proposed - if done correctly - isn't going to make that significantly worse. In fact, it really ought to make things better: you should be able to set things up so that ssup-comparator is always the test that should be applied first, regardless of whether we're aborted or not-aborted or not doing this in the first place; and then ssup-tiebreak_comparator, if not NULL, can be applied after that. I'm not following here. Isn't that at least equally true of what I've proposed? Sure, I'm checking if (!state-abbrevAborted) first, but that's irrelevant to the non-abbreviated case. It has nothing to abort. Also, AFAICT we cannot abort and still call ssup-comparator() indifferently, since sorttuple.datum1 fields are perhaps abbreviated keys in half of all cases (i.e. pre-abort tuples), and uninitialized garbage the other half of the time (i.e. post-abort tuples). You can if you engineer ssup-comparator() to contain the right pointer at the right time. Also, shouldn't you go back and fix up those abbreviated keys to point to datum1 again if you abort? Where is the heap_getattr() stuff supposed to happen for the first attribute to get an authoritative comparison in the event of aborting (if we're calling ssup-comparator() on datum1 indifferently)? We decide that we're going to use abbreviated keys within datum1 fields up-front. When we abort, we cannot use datum1 fields at all (which doesn't appear to matter for text -- the datum1 optimization has historically only benefited pass-by-value types). You always pass datum1 to a function. The code doesn't need to care about whether that function is expecting abbreviated or non-abbreviated datums unless that function returns equality. Then it needs to think about calling a backup comparator if there is one. Is doing all this worth the small saving in memory? Personally, I don't think that it is, but I defer to you. I don't care about the memory; I care about the code complexity and the ease of understanding that code. I am confident that it can be done better than the patch does it today. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On 2014-09-19 17:29:08 -0400, Robert Haas wrote: I generally have serious doubts about disabling it generally for read workloads. I imagine it e.g. will significantly penalize workloads where its likely that a cleanup lock can't be acquired every time... I share that doubt. But I understand why Simon wants to do something, too, because the current situation is not great either. Right, I totally agree. I doubt a simple approach like this will work in the general case, but I think something needs to be done. I think limiting the amount of HOT cleanup for readonly queries is a good idea, but I think it has to be gradual. Say after a single cleaned up page at least another 500 pages need to have been touched till the next hot cleanup. That way a single query won't be penalized with cleaning up everything, but there'll be some progress. The other thing I think might be quite worthwile would be to abort hot cleanup when the gain is only minimal. If e.g. only 1 small tuple is removed from a half full page it's not worth the cost of the wal logging et al. 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] Turning off HOT/Cleanup sometimes
On 19 September 2014 15:35, Tom Lane t...@sss.pgh.pa.us wrote: Having said that, this could reasonably be considered outside the scope of a patch that's trying to improve the behavior for user queries. But if the patch author doesn't want to expand the scope like that, ISTM he ought to ensure that the behavior *doesn't* change for system accesses, rather than trying to convince us that disabling HOT for system updates is a good idea. As I said, I could make an argument to go either way, so I was unsure. I'm happy to avoid changing behaviour for catalog scans in this patch. -- 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] Turning off HOT/Cleanup sometimes
On 2014-09-19 16:35:19 -0400, Tom Lane wrote: Andres Freund and...@anarazel.de writes: On September 19, 2014 10:16:35 PM CEST, Simon Riggs si...@2ndquadrant.com wrote: Are you saying this is a problem or a benefit? (and please explain why). I have no idea what Robert is thinking of, but I'd imagine its horrible for workloads with catalog bloat. Like ones involving temp tables. Yeah. But it's also the case that we know a good deal more about the access patterns for system-driven catalog updates than we do about user queries. ISTM we could probably suppress HOT pruning during catalog *scans* and instead try to do it when a system-driven heap_update occurs. Having said that, this could reasonably be considered outside the scope of a patch that's trying to improve the behavior for user queries. But if the patch author doesn't want to expand the scope like that, ISTM he ought to ensure that the behavior *doesn't* change for system accesses, rather than trying to convince us that disabling HOT for system updates is a good idea. I think it'd have to change for anything not done via the executor. There definitely is user defined code out there doing manual heap_* stuff. I know because i've written some. And I know I'm not the only one. If such paths suddenly stop doing HOT cleanup we'll cause a noticeable amount of pain. 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] B-Tree support function number 3 (strxfrm() optimization)
On Fri, Sep 19, 2014 at 2:35 PM, Robert Haas robertmh...@gmail.com wrote: Also, shouldn't you go back and fix up those abbreviated keys to point to datum1 again if you abort? Probably not - it appears to make very little difference to unoptimized pass-by-reference types whether or not datum1 can be used (see my simulation of Kevin's worst case, for example [1]). Streaming through a not inconsiderable proportion of memtuples again is probably a lot worse. The datum1 optimization (which is not all that old) made a lot of sense when initially introduced, because it avoided chasing through a pointer for pass-by-value types. I think that's its sole justification, though. BTW, I think that if we ever get around to doing this for numeric, it won't ever abort. The abbreviation strategy can be adaptive, to maximize the number of comparisons successfully resolved with abbreviated keys. This would probably use a streaming algorithm like HyperLogLog too. [1] http://www.postgresql.org/message-id/cam3swzqhjxiyrsqbs5w3u-vtj_jt2hp8o02big5wyb4m9lp...@mail.gmail.com -- 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] RLS Design
Thom,
Also, I seem to get an error message with the following:
# create policy nice_colours ON colours for all to joe using (visible =
true) with check (name in ('blue','green','yellow'));
CREATE POLICY
\c - joe
insert into colours (name, visible) values ('blue',false);
ERROR: function with OID 0 does not exist
And if this did work, but I only violated the USING clause, would this
still say the WITH CHECK clause was the cause?
Since RLS is built on top of the same mechanisms used for Security Barrier
Views, I figured I would check this case against that and, for the heck of
it, regular VIEWs as well. The result is the same error in both cases
(below and attached). I also verified that this issue exists for 9.4beta2
and the current REL9_4_STABLE branch. If this isn't the expected behavior
(I can't imagine that it is), I am certainly willing to dive into it
further and see what I can determine for a solution/recommendation. At any
rate, this appears to be a previously existing issue with WITH CHECK
OPTION. Thoughts?
postgres=# DROP TABLE IF EXISTS colors CASCADE;
NOTICE: table colors does not exist, skipping
DROP TABLE
postgres=# DROP ROLE IF EXISTS joe;
DROP ROLE
postgres=# CREATE ROLE joe LOGIN;
CREATE ROLE
postgres=# CREATE TABLE colors (name text, visible bool);
CREATE TABLE
postgres=# CREATE OR REPLACE VIEW v_colors_1 WITH (security_barrier) AS
postgres-# SELECT * FROM colors WHERE (name in ('blue', 'green',
'yellow'))
postgres-# WITH CHECK OPTION;
CREATE VIEW
postgres=# CREATE OR REPLACE VIEW v_colors_2 AS
postgres-# SELECT * FROM colors WHERE (name in ('blue', 'green',
'yellow'))
postgres-# WITH CHECK OPTION;
CREATE VIEW
postgres=# GRANT ALL ON v_colors_1, v_colors_2 TO joe;
GRANT
postgres=# \c - joe
You are now connected to database postgres as user joe.
postgres= INSERT INTO v_colors_1 (name, visible) VALUES ('blue', false);
ERROR: function with OID 0 does not exist
postgres= INSERT INTO v_colors_2 (name, visible) VALUES ('blue', false);
ERROR: function with OID 0 does not exist
Thanks,
Adam
--
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
with_check_error.sql
Description: application/sql
--
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_xlogdump --stats
Andres Freund wrote: Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. I've pushed these after some fixing up. Hm, did you keep the static thingy you mentioned upthread (append_init which is duped unless I read wrong)? There's quite some angst due to pg_dump's fmtId() -- I don't think we should be introducing more such things ... -- Álvaro Herrerahttp://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_xlogdump --stats
On 2014-09-19 19:34:08 -0300, Alvaro Herrera wrote: Andres Freund wrote: Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. I've pushed these after some fixing up. Hm, did you keep the static thingy you mentioned upthread (append_init which is duped unless I read wrong)? Yes, I did. There's quite some angst due to pg_dump's fmtId() -- I don't think we should be introducing more such things ... I don't think these two are comparable. I don't really see many more users of this springing up and printing the identity of two different records in the same printf doesn't seem likely 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] Commitfest status
CF3 is actually over for a couple of days, wouldn't it be better to bounce back patches marked as waiting on author and work on the rest needing review? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add CREATE support to event triggers
On Tue, Aug 26, 2014 at 11:14 PM, Michael Paquier michael.paqu...@gmail.com wrote: And a last one before lunch, closing the review for all the basic things... Patch 13: Could you explain why this is necessary? +extern PGDLLIMPORT bool creating_extension; It may make sense by looking at the core features (then why isn't it with the core features?), but I am trying to review the patches in order. Those patches have been reviewed up to number 14. Some of them could be applied as-is as they are useful taken independently, but most of them need a rebase, hence marking it as returned with feedback. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_shmem_allocations view
On Mon, Aug 18, 2014 at 1:12 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Aug 18, 2014 at 1:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I thought you were printing actual pointer addresses. If you're just printing offsets relative to wherever the segment happens to be mapped, I don't care about that. Well, that just means that it's not an *obvious* security risk. I still like the idea of providing something comparable to MemoryContextStats, rather than creating a SQL interface. The problem with a SQL interface is you can't interrogate it unless (1) you are not already inside a query and (2) the client is interactive and under your control. Something you can call easily from gdb is likely to be much more useful in practice. Since the shared memory segment isn't changing at runtime, I don't see this as being a big problem. It could possibly be an issue for dynamic shared memory segments, though. Patch has been reviewed some time ago, extra ideas as well as potential security risks discussed as well but no new version has been sent, hence marking it as returned with feedback. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Support for N synchronous standby servers
On Fri, Sep 19, 2014 at 12:18 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Sep 16, 2014 at 2:19 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Sep 16, 2014 at 5:25 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Sep 15, 2014 at 3:00 PM, Michael Paquier michael.paqu...@gmail.com wrote: At least a set of hooks has the merit to say: do what you like with your synchronous node policy. Sure. I dunno if people will find that terribly user-friendly, so we might not want that to be the ONLY thing we offer. Well, user-friendly interface is actually the reason why a simple GUC integer was used in the first series of patches present on this thread to set as sync the N-nodes with the lowest priority. I could not come up with something more simple. Hence what about doing the following: - A patch refactoring code for pg_stat_get_wal_senders and SyncRepReleaseWaiters as there is in either case duplicated code in this area to select the synchronous node as the one connected with lowest priority A strong +1 for this idea. I have never liked that, and cleaning it up seems eminently sensible. Interestingly, the syncrep code has in some of its code paths the idea that a synchronous node is unique, while other code paths assume that there can be multiple synchronous nodes. If that's fine I think that it would be better to just make the code multiple-sync node aware, by having a single function call in walsender.c and syncrep.c that returns an integer array of WAL sender positions (WalSndCtl). as that seems more extensible long-term. Well for now the array would have a single element, being the WAL sender with lowest priority 0. Feel free to protest about that approach though :) - A patch defining the hooks necessary, I suspect that two of them are necessary as mentioned upthread. - A patch for a contrib module implementing an example of simple policy. It can be a fancy thing with a custom language or even a more simple thing. I'm less convinced about this part. There's a big usability gap between a GUC and a hook, and I think Heikki's comments upthread were meant to suggest that even in GUC-land we can probably satisfy more use cases that what this patch does now. I think that's right. Hehe. OK then let's see how something with a GUC would go then. There is no parameter now using a custom language as format base, but I guess that it is fine to have a text parameter with a validation callback within. No? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] RLS Design
Adam == Brightwell, Adam adam.brightw...@crunchydatasolutions.com
writes:
Adam At any rate, this appears to be a previously existing issue
Adam with WITH CHECK OPTION. Thoughts?
It's definitely an existing issue; you can reproduce it more simply,
no need to mess with different users.
The issue as far as I can tell is that the withCheckOption exprs are
not being processed anywhere in setrefs.c, so it only works at all by
pure fluke: for most operators, the opfuncid is also filled in by
eval_const_expressions, but for whatever reason SAOPs escape this
treatment. Same goes for other similar cases:
create table colors (name text);
create view vc1 as select * from colors where name is distinct from 'grue' with
check option;
create view vc2 as select * from colors where name in ('red','green','blue')
with check option;
create view vc3 as select * from colors where nullif(name,'grue') is null with
check option;
insert into vc1 values ('red'); -- fails
insert into vc2 values ('red'); -- fails
insert into vc3 values ('red'); -- fails
(Also, commands/policy.c has two instances of const char as a
function return type, which is a compiler warning since the const is
meaningless.)
--
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
