Re: [HACKERS] emergency outage requiring database restart
26.10.2016, 21:34, Andres Freund kirjoitti: Any chance that plsh or the script it executes does anything with the file descriptors it inherits? That'd certainly one way to get into odd corruption issues. We processor really should use O_CLOEXEC for the majority of it file handles. Attached a patch to always use O_CLOEXEC in BasicOpenFile if we're not using EXEC_BACKEND. It'd be nice to not expose all fds to most pl-languages either, but I guess there's no easy solution to that without forcibly closing all fds whenever any functions are called. / Oskari >From 50d7410b895a1aae26c7001f11bd0d71a200ef96 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Wed, 2 Nov 2016 16:42:36 +0200 Subject: [PATCH] BasicOpenFile: always use O_CLOEXEC if it is available --- src/backend/storage/file/fd.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git src/backend/storage/file/fd.c src/backend/storage/file/fd.c index b7ff5ef..6cbe378 100644 --- src/backend/storage/file/fd.c +++ src/backend/storage/file/fd.c @@ -894,7 +894,19 @@ BasicOpenFile(FileName fileName, int fileFlags, int fileMode) int fd; tryAgain: - fd = open(fileName, fileFlags, fileMode); + fd = open(fileName, fileFlags, fileMode +#if defined(O_CLOEXEC) && !defined(EXEC_BACKEND) + /* + * We don't want exec'd processes to inherit our file handles unless + * EXEC_BACKEND is used. We don't expect execve() calls inside the + * postgres code, but extensions and pl-languages may spawn new + * processes that either don't work due to having no usable file + * descriptors or write garbage in the files previously opened by + * us. + */ + | O_CLOEXEC +#endif + ); if (fd >= 0) return fd;/* success! */ -- 2.5.5 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] emergency outage requiring database restart
27.10.2016, 21:53, Merlin Moncure kirjoitti:
As noted earlier, I was not able to reproduce the issue with
crashme.sh, which was:
NUM_FORKS=16
do_parallel psql -p 5432 -c"select PushMarketSample('1740')" castaging_test
do_parallel psql -p 5432 -c"select PushMarketSample('4400')" castaging_test
do_parallel psql -p 5432 -c"select PushMarketSample('2160')" castaging_test
do_parallel psql -p 5432 -c"select PushMarketSample('6680')" castaging_test
(do_parallel is simple wrapper to executing the command in parallel up
to NUM_FORKS). This is on the same server and cluster as above.
This kind of suggests that either
A) there is some concurrent activity from another process that is
tripping the issue
or
B) there is something particular to the session invoking the function
that is participating in the problem. As the application is
structured, a single threaded node.js app is issuing the query that is
high traffic and long lived. It's still running in fact and I'm kind
of tempted to find some downtime to see if I can still reproduce via
the UI.
Your production system's postgres backends probably have a lot more open
files associated with them than the simple test case does. Since
Postgres likes to keep files open as long as possible and only closes
them when you need to free up fds to open new files, it's possible that
your production backends have almost all allowed fds used when you
execute your pl/sh function.
If that's the case, the sqsh process that's executed may not have enough
fds to do what it wanted to do and because of busted error handling
could end up writing to fds that were opened by Postgres and point to
$PGDATA files.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Renaming of pg_xlog and pg_clog
14.10.2016, 07:38, Peter Eisentraut kirjoitti: On 10/12/16 11:22 AM, Tom Lane wrote: The main problem we're trying to fix here is people thinking that something with "log" in the name contains discardable data. Just relocating the directory without renaming it won't improve that. I think it would help if we moved it to something like "internal/pg_xlog" and "internal/pg_clog". Keep the name but move it out of sight. We have a tool called pg_xlogdump in the standard installation. initdb has an option --xlogdir, pg_basebackup has --xlog and others. Renaming the xlog directory would make this all a bit confusing, unless we're prepared to rename the programs and options as well. pg_receivexlog should probably be renamed, seeing how we have pg_recvlogical perhaps pg_recvwal would work? The --xlog, -x, --xlog-method and -X flags for pg_basebackup are a bit confusing as it is. Perhaps they can be kept around as deprecated aliases for a new --wal stream/fetch switch: I don't think we need new --wal and --wal-method switches. pg_resetxlog should probably be called something different than just plain pg_resetwal to make it obvious that running it will cause data loss. pg_xlogdump is a developer tool, users shouldn't care; it's hard enough to use as it is as it doesn't do anything useful when you try to point it to a recycled wal file. All in all, I think this is a good opportunity to clarify what the tools are actually supposed to do and who should be running them. As an author of a backup tool utilizing some of these tools & options I don't think renaming commands and/or arguments is a big deal -- we have to deal with a bunch of changes for every new major version anyway. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL - Weak DH group
06.10.2016, 16:52, Heikki Linnakangas kirjoitti: I propose the attached patch. It gives up on trying to deal with multiple key lengths (as noted earlier, OpenSSL just always passed keylength=1024, so that was useless). Instead of using the callback, it just sets fixed DH parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The DH parameters are loaded from a file called "dh_params.pem" (instead of "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters are used. We've been using the same built-in parameters for 14 years now, they apparently came from https://web.archive.org/web/20011212141438/http://www.skip-vpn.org/spec/numbers.html (the original page is no longer available) and are shared by countless other systems. While we're not using the most common Oakley groups which are presumed to have been broken by various parties (https://weakdh.org) I think it'd be worthwhile to replace the currently built-in parameters with custom ones. And maybe even regenerate parameters for every minor release. HAProxy made a similar change last year, see https://github.com/haproxy/haproxy/commit/d3a341a96fb6107d2b8e3d7a9c0afa2ff43bb0b6 / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pgpassfile connection option
05.10.2016, 20:06, Robert Haas kirjoitti: You could do something like that, I guess, but I think it might be a good idea to wait and see if anyone else has opinions on (1) the desirability of the basic feature, (2) the severity of the security hazard it creates, and (3) your proposed remediation method. I don't think it's appropriate to compare the proposed pgpassfile option to sslkey: the key is never transmitted over the network anywhere. So far I don't see anybody actually endorsing your proposal here, which might mean that any patch you produce will be rejected on the grounds that nobody has a clear use case for this feature. Hey, everybody: chime in here... The original patch didn't come with a description of the problem it was aiming to solve, but I've wanted something perhaps a bit similar in the past. The pghoard backup & restore suite we maintain needs to be able to spawn pg_basebackup and pg_receivexlog processes and in order to avoid passing passwords in command-line arguments visible to everyone who can get a process listing we currently have pghoard edit pgpass files. Having to edit pgpass files at all is annoying and there isn't really a good way to do it: we can edit the one at $HOME and hope it doesn't clash with the expectations of the user running the software, write it to a custom file somewhere else - copying the password to a location the user might not expect - or create a new temporary file on every invocation. I came across some bad advice about passing passwords to libpq today: there was a recommendation for setting a $PASSWORD environment variable and including that in the connection string, using shell to expand it. It got me thinking that maybe such a feature could be implemented in libpq: have it expand unquoted $variables; for example: $ PASSWORD=xyz psql 'password=$PASSWORD dbname=foo' This does have the hazard of making it very easy to accidentally use double quotes instead of single quotes and have the shell expand the variable making it visible in process listing though. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash Indexes
21.09.2016, 15:29, Robert Haas kirjoitti: For PostgreSQL, I expect the benefits of improving hash indexes to be (1) slightly better raw performance for equality comparisons and (2) better concurrency. There's a third benefit: with large columns a hash index is a lot smaller on disk than a btree index. This is the biggest reason I've seen people want to use hash indexes instead of btrees. hashtext() btrees are a workaround, but they require all queries to be adjusted which is a pain. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use pread and pwrite instead of lseek + write and read
17.08.2016, 22:11, Tom Lane kirjoitti: Robert Haas writes: I don't understand why you think this would create non-trivial portability issues. The patch as submitted breaks entirely on platforms without pread/pwrite. Yes, we can add a configure test and some shim functions to fix that, but the argument that it makes the code shorter will get a lot weaker once we do. I posted an updated patch which just calls lseek + read/write, the code's still a lot shorter. I agree that adding such functions is pretty trivial, but there are reasons to think there are other hazards that are less trivial: First, a self-contained shim function will necessarily do an lseek every time, which means performance will get *worse* not better on non-pread platforms. And yes, the existing logic to avoid lseeks fires often enough to be worthwhile, particularly in seqscans. This will only regress on platforms without pread. The only relevant such platform appears to be Windows which has equivalent APIs. FWIW, I ran the same pgbench benchmarks on my Linux system where I always used lseek() + read/write instead of pread and pwrite - they ran slightly faster than the previous code which saved seek positions, but I suppose a workload with lots of seqscans could be slower. Unfortunately I didn't save the actual numbers anywhere, but I can rerun the benchmarks if you're interested. The numbers were pretty stable across multiple runs. Second, I wonder whether this will break any kernel's readahead detection. I wouldn't be too surprised if successive reads (not preads) without intervening lseeks are needed to trigger readahead on at least some platforms. So there's a potential, both on platforms with pread and those without, for this to completely destroy seqscan performance, with penalties very far exceeding what we might save by avoiding some kernel calls. At least Linux and FreeBSD don't seem to care how and why you read pages, they'll do readahead regardless of the way you read files and extend the readahead once you access previously readahead pages. They disable readahead only if fadvise(POSIX_FADV_RANDOM) has been used. I'd expect any kernel that implements mmap to also implement readahead based on page usage rather than than the seek position. Do you know of a kernel that would actually use the seek position for readahead? I'd be more excited about this if the claimed improvement were more than 1.5%, but you know as well as I do that that's barely above the noise floor for most performance measurements. I'm left wondering why bother, and why take any risk of de-optimizing on some platforms. I think it makes sense to try to optimize for the platforms that people actually use for performance critical workloads, especially if it also allows us to simplify the code and remove more lines than we add. It's nice if the software still works on legacy platforms, but I don't think we should be concerned about a hypothetical performance impact on platforms no one uses in production anymore. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use pread and pwrite instead of lseek + write and read
17.08.2016, 16:40, Tom Lane kirjoitti: Oskari Saarenmaa writes: On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5% performance improvement. I would have hoped for a lot better result before anyone would propose that we should deal with all the portability issues this'll create. AFAICT pread and pwrite are available on pretty much all operating systems released in 2000s; it was added to Linux in 1997. Windows and HP-UX 10.20 don't have it, but we can just simulate it using lseek + read/write there without adding too much code. A 1.5% performance improvement is small but measurable - and IMV more importantly it allows us to drop more than 100 lines of backwards (compatible?) code; maybe we could start targeting more recent platforms in v10? That's basically nonsense: we'll end up adding way more than that to deal with platforms that haven't got these APIs. Attached an updated patch that adds a configure check and uses lseek+read/write instead pread/pwrite when the latter aren't available. The previous code ended up seeking anyway in most of the cases and pgbench shows no performance regression on my Linux box. 8 files changed, 54 insertions(+), 168 deletions(-) / Oskari diff --git configure configure index 45c8eef..4d50b52 100755 --- configure +++ configure @@ -12473,7 +12473,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l +for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pread pstat pthread_is_threaded_np pwrite readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git configure.in configure.in index c878b4e..5eddaca 100644 --- configure.in +++ configure.in @@ -1446,7 +1446,7 @@ PGAC_FUNC_WCSTOMBS_L LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) +AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pread pstat pthread_is_threaded_np pwrite readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) AC_REPLACE_FUNCS(fseeko) case $host_os in diff --git src/backend/access/heap/rewriteheap.c src/backend/access/heap/rewriteheap.c index f9ce986..6dc5df3 100644 --- src/backend/access/heap/rewriteheap.c +++ src/backend/access/heap/rewriteheap.c @@ -918,7 +918,7 @@ logical_heap_rewrite_flush_mappings(RewriteState state) * Note that we deviate from the usual WAL coding practices here, * check the above "Logical rewrite support" comment for reasoning. */ - written = FileWrite(src->vfd, waldata_start, len); + written = FileWriteAt(src->vfd, waldata_start, len, src->off); if (written != len) ereport(ERROR, (errcode_for_file_access(), diff --git src/backend/storage/file/buffile.c src/backend/storage/file/buffile.c index 042be79..39482fa 100644 --- src/backend/storage/file/buffile.c +++ src/backend/storage/file/buffile.c @@ -60,12 +60,6 @@ struct BufFile int numFiles; /* number of physical files in set */ /* all files except the last have length exactly MAX_PHYSICAL_FILESIZE */ File *files; /* palloc'd array with numFiles entries */ - off_t *offsets; /* palloc'd array with numFiles entries */ - - /* - * offsets[i] is the current seek position of files[i]. We use this to - * avoid making redundant FileSeek calls. - */ bool isTemp; /* can only add files if this is TRUE */ bool isInterXact; /* keep open over transactions? */ @@ -108,8 +102,6 @@ makeBufFile(File firstfile) file->numFiles = 1; file->files = (File *) palloc(sizeof(File)); file->files[0] = firstfile; - file->offsets = (off_t *) palloc(sizeof(off_t)); - file->offsets[0] = 0L; file->isTemp = false; file->isInterXact = false; file->dirty = false; @@ -143,10 +135,7 @@ extendBufFile(BufFile *file) file->files = (File *) repalloc(file->files, (file->numFiles + 1) * sizeof(File)); - file->offsets = (off_t *) repalloc(file->offsets, - (file->numFiles + 1) * sizeof(off_t)); file->files[file->numFiles] = pfile; - file->offsets[file
[HACKERS] Use pread and pwrite instead of lseek + write and read
Hi,
The Uber blog post, among other things, pointed out that PG uses lseek +
read instead of pread. I didn't see any discussion around that and my
Google searches didn't find any posts about pread / pwrite for the past
10 years.
With that plus the "C++ port" thread in mind, I was wondering if it's
time to see if we could do better by just utilizing newer C and POSIX
features.
The attached patch replaces FileWrite and FileRead with FileWriteAt and
FileReadAt and removes most FileSeek calls. FileSeek is still around so
we can find the end of a file, but it's not used for anything else.
On my laptop a simple pgbench run (scale 100, 15 minutes) shows a 1.5%
performance improvement. A 1.5% performance improvement is small but
measurable - and IMV more importantly it allows us to drop more than 100
lines of backwards (compatible?) code; maybe we could start targeting
more recent platforms in v10?
Obviously this patch needs some more work before it could be merged, and
we probably still need a fallback for some platforms without pread and
pwrite (afaik Windows doesn't implement them.)
/ Oskari
diff --git src/backend/access/heap/rewriteheap.c src/backend/access/heap/rewriteheap.c
index f9ce986..6dc5df3 100644
--- src/backend/access/heap/rewriteheap.c
+++ src/backend/access/heap/rewriteheap.c
@@ -918,7 +918,7 @@ logical_heap_rewrite_flush_mappings(RewriteState state)
* Note that we deviate from the usual WAL coding practices here,
* check the above "Logical rewrite support" comment for reasoning.
*/
- written = FileWrite(src->vfd, waldata_start, len);
+ written = FileWriteAt(src->vfd, waldata_start, len, src->off);
if (written != len)
ereport(ERROR,
(errcode_for_file_access(),
diff --git src/backend/storage/file/buffile.c src/backend/storage/file/buffile.c
index 042be79..39482fa 100644
--- src/backend/storage/file/buffile.c
+++ src/backend/storage/file/buffile.c
@@ -60,12 +60,6 @@ struct BufFile
int numFiles; /* number of physical files in set */
/* all files except the last have length exactly MAX_PHYSICAL_FILESIZE */
File *files; /* palloc'd array with numFiles entries */
- off_t *offsets; /* palloc'd array with numFiles entries */
-
- /*
- * offsets[i] is the current seek position of files[i]. We use this to
- * avoid making redundant FileSeek calls.
- */
bool isTemp; /* can only add files if this is TRUE */
bool isInterXact; /* keep open over transactions? */
@@ -108,8 +102,6 @@ makeBufFile(File firstfile)
file->numFiles = 1;
file->files = (File *) palloc(sizeof(File));
file->files[0] = firstfile;
- file->offsets = (off_t *) palloc(sizeof(off_t));
- file->offsets[0] = 0L;
file->isTemp = false;
file->isInterXact = false;
file->dirty = false;
@@ -143,10 +135,7 @@ extendBufFile(BufFile *file)
file->files = (File *) repalloc(file->files,
(file->numFiles + 1) * sizeof(File));
- file->offsets = (off_t *) repalloc(file->offsets,
- (file->numFiles + 1) * sizeof(off_t));
file->files[file->numFiles] = pfile;
- file->offsets[file->numFiles] = 0L;
file->numFiles++;
}
@@ -210,7 +199,6 @@ BufFileClose(BufFile *file)
FileClose(file->files[i]);
/* release the buffer space */
pfree(file->files);
- pfree(file->offsets);
pfree(file);
}
@@ -241,23 +229,12 @@ BufFileLoadBuffer(BufFile *file)
}
/*
- * May need to reposition physical file.
- */
- thisfile = file->files[file->curFile];
- if (file->curOffset != file->offsets[file->curFile])
- {
- if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
- return;/* seek failed, read nothing */
- file->offsets[file->curFile] = file->curOffset;
- }
-
- /*
* Read whatever we can get, up to a full bufferload.
*/
- file->nbytes = FileRead(thisfile, file->buffer, sizeof(file->buffer));
+ thisfile = file->files[file->curFile];
+ file->nbytes = FileReadAt(thisfile, file->buffer, sizeof(file->buffer), file->curOffset);
if (file->nbytes < 0)
file->nbytes = 0;
- file->offsets[file->curFile] += file->nbytes;
/* we choose not to advance curOffset here */
pgBufferUsage.temp_blks_read++;
@@ -307,20 +284,10 @@ BufFileDumpBuffer(BufFile *file)
bytestowrite = (int) availbytes;
}
- /*
- * May need to reposition physical file.
- */
thisfile = file->files[file->curFile];
- if (file->curOffset != file->offsets[file->curFile])
- {
- if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
-return; /* seek failed, give up */
- file->offsets[file->curFile] = file->curOffset;
- }
- bytestowrite = FileWrite(thisfile, file->buffer + wpos, bytestowrite);
+ bytestowrite = FileWriteAt(thisfile, file->buffer + wpos, bytestowrite, file->curOffset);
if (bytestowrite <= 0)
return;/* failed to write */
- file->offsets[file->curFile] += bytestowrite;
file->curOffset += bytestowrite;
wpos += bytestowrite;
diff --git src/backend/storage/file/fd.c src/backend/storage/file/fd.
[HACKERS] Don't include MMAP DSM's transient files in basebackup
The files are not useful when restoring a backup and would be
automatically deleted on startup anyway. Also deleted an out-of-date
comment in dsm.c.
/ Oskari
>From f26f06049b5f89ca3140462d6816259268322d78 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Wed, 6 Jul 2016 16:35:39 +0300
Subject: [PATCH] Don't include MMAP DSM's transient files in basebackup
Also drop an out-of-date comment about AllocateDir usage in dsm.c.
---
src/backend/replication/basebackup.c | 6 +++---
src/backend/storage/ipc/dsm.c| 1 -
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index da9b7a6..8867ad2 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -976,10 +976,10 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces,
}
/*
- * Skip pg_replslot, not useful to copy. But include it as an empty
- * directory anyway, so we get permissions right.
+ * Skip pg_replslot and pg_dynshmem, not useful to copy. But include
+ * them as empty directories anyway, so we get permissions right.
*/
- if (strcmp(de->d_name, "pg_replslot") == 0)
+ if (strcmp(de->d_name, "pg_replslot") == 0 || strcmp(de->d_name, "pg_dynshmem") == 0)
{
if (!sizeonly)
_tarWriteHeader(pathbuf + basepathlen + 1, NULL, &statbuf);
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 47f2bea..57fecdb 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -293,7 +293,6 @@ dsm_cleanup_for_mmap(void)
DIR *dir;
struct dirent *dent;
- /* Open the directory; can't use AllocateDir in postmaster. */
if ((dir = AllocateDir(PG_DYNSHMEM_DIR)) == NULL)
ereport(ERROR,
(errcode_for_file_access(),
--
2.5.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Show dropped users' backends in pg_stat_activity
24.03.2016, 18:03, Tom Lane kirjoitti: Robert Haas writes: I am not really in favor of half-fixing this. If we can't conveniently wait until a dropped role is completely out of the system, then I don't see a lot of point in trying to do it in the limited cases where we can. If LEFT JOIN is the way to go, then, blech, but, so be it. I concur. Let's put the left join(s) into those views and call it good. BTW, I think we would need the left joins even if we had interlocking in DROP, just to protect ourselves against race conditions. Remember that what pg_stat_activity shows is a snapshot, which might be more or less out of date compared to the catalog contents. Added my patch to the 2016-09 commitfest (https://commitfest.postgresql.org/10/601/) as a bug fix as I thought not showing all backends in pg_stat_activity is a bug. Any chance to get it in 9.6? -- Oskari Saarenmaa Aiven: managed cloud databases https://aiven.io -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Show dropped users' backends in pg_stat_activity
16.03.2016, 17:48, Tom Lane kirjoitti: Robert Haas writes: Gee, I would have expected the DROP to be blocked until the user disconnected, like we do for DROP DATABASE. Making that race-condition-free would require some notion of a lock on roles, I think. Seems pretty messy compared to the amount of actual value obtained. There are good reasons why you can't have a backend running in a nonexistent database; but a backend with a nonexistent user OID is not really going to be a problem for anything except monitoring queries that fail to use left joins where appropriate. I don't think most people expect dropped users to be able to keep using the database. If it's not feasible to block DROP ROLE until the user has disconnected or to kill dropped users' sessions immediately after they're dropped we should at least show their sessions in pg_stat_activity and add a note about it in DROP ROLE docs. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix typos in comments
Attached a patch to fix a bunch of typos in comments. / Oskari >From 1effab7d75c3ac08257c637d8662b4c8b3fdc750 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Tue, 15 Mar 2016 23:45:26 +0200 Subject: [PATCH] misc typofixes in comments --- contrib/pgcrypto/sha1.h| 2 +- contrib/sepgsql/label.c| 2 +- doc/src/sgml/sources.sgml | 2 +- src/backend/access/transam/twophase.c | 2 +- src/backend/libpq/auth.c | 4 ++-- src/backend/optimizer/util/relnode.c | 2 +- src/backend/parser/parse_target.c | 2 +- src/backend/storage/buffer/bufmgr.c| 2 +- src/backend/storage/ipc/dsm_impl.c | 2 +- src/backend/storage/ipc/procarray.c| 2 +- src/backend/storage/ipc/shm_mq.c | 2 +- src/backend/utils/adt/json.c | 2 +- src/backend/utils/adt/windowfuncs.c| 2 +- src/backend/utils/misc/guc.c | 4 ++-- src/bin/pg_dump/compress_io.c | 2 +- src/bin/pg_dump/parallel.c | 2 +- src/bin/pg_upgrade/option.c| 2 +- src/bin/pgbench/pgbench.c | 2 +- src/include/storage/shm_toc.h | 2 +- src/interfaces/ecpg/ecpglib/execute.c | 2 +- src/interfaces/ecpg/preproc/parse.pl | 2 +- src/interfaces/ecpg/preproc/type.c | 2 +- src/test/regress/expected/inherit.out | 4 ++-- src/test/regress/expected/replica_identity.out | 2 +- src/test/regress/sql/inherit.sql | 4 ++-- src/test/regress/sql/replica_identity.sql | 2 +- 26 files changed, 30 insertions(+), 30 deletions(-) diff --git a/contrib/pgcrypto/sha1.h b/contrib/pgcrypto/sha1.h index 5532ca1..2f61e45 100644 --- a/contrib/pgcrypto/sha1.h +++ b/contrib/pgcrypto/sha1.h @@ -63,7 +63,7 @@ extern void sha1_pad(struct sha1_ctxt *); extern void sha1_loop(struct sha1_ctxt *, const uint8 *, size_t); extern void sha1_result(struct sha1_ctxt *, uint8 *); -/* compatibilty with other SHA1 source codes */ +/* compatibility with other SHA1 source codes */ typedef struct sha1_ctxt SHA1_CTX; #define SHA1Init(x) sha1_init((x)) diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c index 3e32f1b..e12a0e8 100644 --- a/contrib/sepgsql/label.c +++ b/contrib/sepgsql/label.c @@ -161,7 +161,7 @@ sepgsql_set_client_label(const char *new_label) /* * sepgsql_xact_callback * - * A callback routine of transaction commit/abort/prepare. Commmit or abort + * A callback routine of transaction commit/abort/prepare. Commit or abort * changes in the client_label_pending list. */ static void diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index fcb3e40..614defa 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -860,7 +860,7 @@ BETTER: unrecognized node type: 42 Code in PostgreSQL should only rely on language features available in the C89 standard. That means a conforming C89 compiler has to be able to compile postgres, at least aside - from a few platform dependant pieces. Features from later + from a few platform dependent pieces. Features from later revision of the C standard or compiler specific features can be used, if a fallback is provided. diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index c4fd9ef..e7234c8 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1613,7 +1613,7 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon) * transaction manager isn't active. * * It's also possible to move I/O out of the lock, but on - * every error we should check whether somebody commited our + * every error we should check whether somebody committed our * transaction in different backend. Let's leave this optimisation * for future, if somebody will spot that this place cause * bottleneck. diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 57c2f48..7f1ae8c 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -838,7 +838,7 @@ pg_GSS_recvauth(Port *port) /* * Loop through GSSAPI message exchange. This exchange can consist of - * multiple messags sent in both directions. First message is always from + * multiple messages sent in both directions. First message is always from * the client. All messages from client to server are password packets * (type 'p'). */ @@ -1078,7 +1078,7 @@ pg_SSPI_recvauth(Port *port) /* * Loop through SSPI message exchange. This exchange can consist of - * multiple messags sent in both directions. First message is always from + * multiple messages sent in both directions. First message is always from * the client. All messages from client to server are password packets * (type 'p'). */ diff --git a/src/
[HACKERS] Show dropped users' backends in pg_stat_activity
I was looking into some issues we recently had when dropping db users and was surprised to see that dropped users' sessions and transactions continue to work after the role is dropped. Since dropping a role requires dropping all grants it has (using DROP OWNED BY ...) the dropped role can't start new transactions that do a whole lot unless there are objects with access granted to PUBLIC, but any running transactions remain running and can write to the database. They can also hold locks which interfere with other backends without showing up in most activity or lock monitoring tools as they won't appear in pg_stat_activity. IMO any open sessions for a dropped user should be automatically terminated when the role is dropped, but that would probably be a bigger change so attached a proposed patch for using left joins in pg_stat_activity and pg_stat_replication to show activity by dropped roles. / Oskari >From 4632aa09fe82d80e378123ca46fdf8aecdda795f Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Mon, 14 Mar 2016 18:34:24 +0200 Subject: [PATCH] pg_stat_activity: display backends for dropped roles --- src/backend/catalog/system_views.sql | 13 ++--- src/test/regress/expected/rules.out | 14 ++ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 84aa061..e0b583e 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -642,9 +642,9 @@ CREATE VIEW pg_stat_activity AS S.backend_xid, s.backend_xmin, S.query -FROM pg_database D, pg_stat_get_activity(NULL) AS S, pg_authid U -WHERE S.datid = D.oid AND -S.usesysid = U.oid; +FROM pg_stat_get_activity(NULL) AS S +JOIN pg_database AS D ON (S.datid = D.oid) +LEFT JOIN pg_authid AS U ON (S.usesysid = U.oid); CREATE VIEW pg_stat_replication AS SELECT @@ -664,10 +664,9 @@ CREATE VIEW pg_stat_replication AS W.replay_location, W.sync_priority, W.sync_state -FROM pg_stat_get_activity(NULL) AS S, pg_authid U, -pg_stat_get_wal_senders() AS W -WHERE S.usesysid = U.oid AND -S.pid = W.pid; +FROM pg_stat_get_activity(NULL) AS S +JOIN pg_stat_get_wal_senders() AS W ON (S.pid = W.pid) +LEFT JOIN pg_authid AS U ON (S.usesysid = U.oid); CREATE VIEW pg_stat_wal_receiver AS SELECT diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 22ea06c..54d7a7b 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1656,10 +1656,9 @@ pg_stat_activity| SELECT s.datid, s.backend_xid, s.backend_xmin, s.query - FROM pg_database d, -pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn), -pg_authid u - WHERE ((s.datid = d.oid) AND (s.usesysid = u.oid)); + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn) + JOIN pg_database d ON ((s.datid = d.oid))) + LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_all_indexes| SELECT c.oid AS relid, i.oid AS indexrelid, n.nspname AS schemaname, @@ -1763,10 +1762,9 @@ pg_stat_replication| SELECT s.pid, w.replay_location, w.sync_priority, w.sync_state - FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn), -pg_authid u, -pg_stat_get_wal_senders() w(pid, state, sent_location, write_location, flush_location, replay_location, sync_priority, sync_state) - WHERE ((s.usesysid = u.oid) AND (s.pid = w.pid)); + FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, ssl, sslversion, sslcipher, sslbits, sslcompression, sslclientdn) + JOIN pg_stat_get_wal_senders() w(pid, state, sent_location, write_location, flush_location, replay_location, sync_priority, sync_state) ON ((s.pid = w.pid))) + LEFT JOIN pg_authid u ON ((s.usesysid = u.oid))); pg_stat_ssl| SELECT
Re: [HACKERS] [patch] extensions_path GUC
23.10.2015, 13:33, Sandro Santilli kirjoitti: One problem we have with PostGIS is you cannot test an extension unless you have access to the system extension dir. The following patch tries to address that by allowing to specify a per-cluster extension path via an "extensions_path" GUC. It is more a request-for-comments rather than a ready patch, as I hadn't considered all use cases like upgrades of already-loaded extensions and the possibility ot have a list of directories to seek for extensions. Anyway, patch is attached. This would be very useful. I proposed this previously in February, but that implementation (almost identical to yours) was rejected. https://commitfest.postgresql.org/5/170/ http://www.postgresql.org/message-id/54e3c31f.8010...@ohmu.fi Both of these implementations miss a way to override the path for extension .so files. / Oskari -- Sent 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_concat: make sure we always return a non-scalar value
09.09.2015, 18:53, Andrew Dunstan kirjoitti:
On 09/08/2015 09:54 AM, Andrew Dunstan wrote:
On 09/05/2015 02:47 AM, Oskari Saarenmaa wrote:
There was a long thread about concatenating jsonb objects to each
other, but that discussion didn't touch concatenating other types.
Currently jsonb_concat always just returns the other argument if one
of arguments is considered empty. This causes surprising behavior
when concatenating scalar values to empty arrays:
os=# select '[]'::jsonb || '1'::jsonb;
1
os=# select '[]'::jsonb || '[1]'::jsonb;
[1]
os=# select '[]'::jsonb || '1'::jsonb || '2'::jsonb;
[1, 2]
os=# select '0'::jsonb || '1'::jsonb;
[0, 1]
os=# select '{"x": "y"}'::jsonb || '[1]'::jsonb;
[{"x": "y"}, 1]
os=# select '{"x": "y"}'::jsonb || '1'::jsonb;
ERROR: invalid concatenation of jsonb objects
Attached a patch to fix and test this. Also added a test case for
concatenating two scalar values which currently produces an array..
I'm not sure that behavior makes sense, but didn't want to change
that in this patch as I guess someone could consider that feature
useful.
This looks correct. Barring objection I'll apply this shortly.
Actually, I'm not sure the test is sufficient here. It looks to me like
we should only be taking this fast path if one operand is an empty array
and the other is a non-scalar array.
Otherwise we get things like this (second case is wrong, I think):
andrew=# select '[]'::jsonb || '"a"';
?column?
--
["a"]
andrew=# select '[]'::jsonb || '{"a":"b"}';
?column?
{"a": "b"}
andrew=# select '[1]'::jsonb || '{"a":"b"}';
?column?
-
[1, {"a": "b"}]
It looks wrong, but I'm not sure what's right in that case. I think
it'd make sense to just restrict concatenation to object || object,
array || array and array || scalar and document that. I doubt many
people expect their objects to turn into arrays if they accidentally
concatenate an array into it. Alternatively the behavior could depend
on the order of arguments for concatenation, array || anything -> array,
object || array -> error.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] misc typofixes
Attached a patch to fix a number of typos in docs and comments I noticed
over the past few weeks.
/ Oskari
>From dd5cb727d7fc1f1168040f502521a897d01cc127 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Sat, 5 Sep 2015 10:12:04 +0300
Subject: [PATCH] typofixes
---
contrib/btree_gist/btree_ts.c| 2 +-
contrib/btree_gist/btree_utils_var.c | 2 +-
contrib/cube/cube.c | 2 +-
doc/src/sgml/ref/alter_role.sgml | 4 ++--
doc/src/sgml/release-9.5.sgml| 4 ++--
doc/src/sgml/sources.sgml| 2 +-
src/backend/access/brin/brin_revmap.c| 4 ++--
src/backend/access/common/heaptuple.c| 2 +-
src/backend/access/gin/ginfast.c | 4 ++--
src/backend/access/gist/gistproc.c | 4 ++--
src/backend/access/heap/heapam.c | 4 ++--
src/backend/access/heap/rewriteheap.c| 4 ++--
src/backend/access/transam/xact.c| 2 +-
src/backend/optimizer/path/costsize.c| 2 +-
src/backend/replication/logical/origin.c | 2 +-
src/backend/utils/adt/regproc.c | 2 +-
src/include/storage/lwlock.h | 4 ++--
17 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index c746c23..ab22b27 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -369,7 +369,7 @@ gbt_ts_penalty(PG_FUNCTION_ARGS)
newdbl[2];
/*
- * We are allways using "double" timestamps here. Precision should be good
+ * We are always using "double" timestamps here. Precision should be good
* enough.
*/
orgdbl[0] = ((double) origentry->lower);
diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c
index 8105a3b..70b3794 100644
--- a/contrib/btree_gist/btree_utils_var.c
+++ b/contrib/btree_gist/btree_utils_var.c
@@ -52,7 +52,7 @@ gbt_var_decompress(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(entry);
}
-/* Returns a better readable representaion of variable key ( sets pointer ) */
+/* Returns a better readable representation of variable key ( sets pointer ) */
GBT_VARKEY_R
gbt_var_key_readable(const GBT_VARKEY *k)
{
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 113c663..a6be59e 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -814,7 +814,7 @@ cube_inter(PG_FUNCTION_ARGS)
Max(LL_COORD(b, i), UR_COORD(b, i))
);
}
- /* continue on the higher dimemsions only present in 'a' */
+ /* continue on the higher dimensions only present in 'a' */
for (; i < DIM(a); i++)
{
result->x[i] = Max(0,
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index e97bf4c..7638817 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -138,7 +138,7 @@ ALTER ROLE { role_specification | A
CURRENT_USER
-Alter the current user instead of an explicitely identified role.
+Alter the current user instead of an explicitly identified role.
@@ -147,7 +147,7 @@ ALTER ROLE { role_specification | A
SESSION_USER
-Alter the current session user instead of an explicitely identified
+Alter the current session user instead of an explicitly identified
role.
diff --git a/doc/src/sgml/release-9.5.sgml b/doc/src/sgml/release-9.5.sgml
index a535e22..59f10f0 100644
--- a/doc/src/sgml/release-9.5.sgml
+++ b/doc/src/sgml/release-9.5.sgml
@@ -460,7 +460,7 @@ FIXME: Add Andres
-Speed up CREATE INDEX by avoiding unneccessary memory copies (Robert Haas)
+Speed up CREATE INDEX by avoiding unnecessary memory copies (Robert Haas)
@@ -,7 +,7 @@ FIXME: Correct description
-->
Allow changing of the WAL
-logging status of a table ater creation with ALTER TABLE .. SET LOGGED /
UNLOGGED (FabrÃzio de Royes Mello)
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index aa20807..d6461ec 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -251,7 +251,7 @@ ereport(ERROR,
- errdetail_log_plural(const char *fmt_singuar, const char
+ errdetail_log_plural(const char *fmt_singular, const char
*fmt_plural, unsigned long n, ...) is like
errdetail_log, but with support for various plural forms of
the message.
diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c
index 62d440f..8c55f16 100644
--- a/src/backend/access/brin/brin_revmap.c
+++ b/src/backend/access/brin/brin_revmap.c
@@ -127,7 +127,7 @@ brinRevmapExtend(BrinRevmap *revmap, BlockNumber heapBlk)
* it's not long enough.
*
* The returned buffer is also recorded in the revmap struct; finishing that
- * releases the buffer, therefore the caller needn't do it explicitely.
+ * releases the bu
[HACKERS] jsonb_concat: make sure we always return a non-scalar value
There was a long thread about concatenating jsonb objects to each other,
but that discussion didn't touch concatenating other types. Currently
jsonb_concat always just returns the other argument if one of arguments
is considered empty. This causes surprising behavior when concatenating
scalar values to empty arrays:
os=# select '[]'::jsonb || '1'::jsonb;
1
os=# select '[]'::jsonb || '[1]'::jsonb;
[1]
os=# select '[]'::jsonb || '1'::jsonb || '2'::jsonb;
[1, 2]
os=# select '0'::jsonb || '1'::jsonb;
[0, 1]
os=# select '{"x": "y"}'::jsonb || '[1]'::jsonb;
[{"x": "y"}, 1]
os=# select '{"x": "y"}'::jsonb || '1'::jsonb;
ERROR: invalid concatenation of jsonb objects
Attached a patch to fix and test this. Also added a test case for
concatenating two scalar values which currently produces an array.. I'm
not sure that behavior makes sense, but didn't want to change that in
this patch as I guess someone could consider that feature useful.
/ Oskari
>From 299124e63bb26ab07fa8429b7d1c2035b81f15d5 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Sat, 5 Sep 2015 09:33:58 +0300
Subject: [PATCH] jsonb_concat: make sure we always return a non-scalar value
jsonb_concat used to always just return the other argument if one of
arguments was considered empty. This caused surprising behavior when
concatenating scalar values to empty arrays.
Fixed this and added a test case for it. Also added a test case for
concatenating two scalar values which currently produces an array.
---
src/backend/utils/adt/jsonfuncs.c | 8 +---
src/test/regress/expected/jsonb.out | 18 ++
src/test/regress/sql/jsonb.sql | 4
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 3b8d42e..57edb63 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3359,11 +3359,13 @@ jsonb_concat(PG_FUNCTION_ARGS)
*it2;
/*
- * If one of the jsonb is empty, just return other.
+ * If one of the jsonb is empty, just return other if it's not
+ * scalar. If it's a scalar we need to perform concatenation to
+ * make sure we return a non-scalar value.
*/
- if (JB_ROOT_COUNT(jb1) == 0)
+ if (JB_ROOT_COUNT(jb1) == 0 && !JB_ROOT_IS_SCALAR(jb2))
PG_RETURN_JSONB(jb2);
- else if (JB_ROOT_COUNT(jb2) == 0)
+ else if (JB_ROOT_COUNT(jb2) == 0 && !JB_ROOT_IS_SCALAR(jb1))
PG_RETURN_JSONB(jb1);
it1 = JsonbIteratorInit(&jb1->root);
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 17656d4..98a318b 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -2912,6 +2912,24 @@ select '"c"' || '["a", "b"]'::jsonb;
["c", "a", "b"]
(1 row)
+select '[]'::jsonb || '["a"]'::jsonb;
+ ?column?
+--
+ ["a"]
+(1 row)
+
+select '[]'::jsonb || '"a"'::jsonb;
+ ?column?
+--
+ ["a"]
+(1 row)
+
+select '"b"'::jsonb || '"a"'::jsonb;
+ ?column?
+
+ ["b", "a"]
+(1 row)
+
select '"a"'::jsonb || '{"a":1}';
ERROR: invalid concatenation of jsonb objects
select '{"a":1}' || '"a"'::jsonb;
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 83ed4eb..5a2b4a8 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -718,6 +718,10 @@ select '["c"]' || '["a", "b"]'::jsonb;
select '["a", "b"]'::jsonb || '"c"';
select '"c"' || '["a", "b"]'::jsonb;
+select '[]'::jsonb || '["a"]'::jsonb;
+select '[]'::jsonb || '"a"'::jsonb;
+select '"b"'::jsonb || '"a"'::jsonb;
+
select '"a"'::jsonb || '{"a":1}';
select '{"a":1}' || '"a"'::jsonb;
--
2.5.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repeated pg_upgrade buildfarm failures on binturon
07.07.2015, 19:50, Tom Lane kirjoitti: > Oskari Saarenmaa writes: >> I've restricted builds to one at a time on that host to work around this >> issue for now. Also attached a patch to explicitly set PWD=$(CURDIR) in >> the Makefile to make sure test.sh runs with the right directory. > > I've pushed a patch for this issue. Please revert your buildfarm > configuration so that we can verify it works now. Ok, just reverted the configuration change and started two test runs, they're now using correct directories. Thanks! Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Repeated pg_upgrade buildfarm failures on binturon
07.07.2015, 14:21, Andres Freund kirjoitti: > On 2015-07-06 20:00:43 -0400, Tom Lane wrote: >> Andres Freund writes: >>> Binturon has repeatedly failed with errors like: >>> ERROR: could not open file "base/16400/32052": No such file or directory >> >> I agree that binturong seems to have something odd going on; but there are >> a lot of other intermittent pg_upgrade test failures in the buildfarm >> history > > binturong seemed to be clean on HEAD for a while now, and the failures > ~80 days ago seem to have had different symptoms (the src/bin move): > http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=binturong&br=HEAD > > other branches are less nice looking for various reasons, but there's > another recurring error: > FATAL: could not open relation mapping file "global/pg_filenode.map": No > such file or directory > > Those seem to indicate something going seriously wrong to me. Binturong and Dingo run on the same host with a hourly cronjob to trigger the builds. These failures are caused by concurrent test runs on different branches which use the same tmp_check directory for pg_upgrade tests, see http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dingo&dt=2015-07-07%2002%3A58%3A01&stg=check-pg_upgrade It looks like neither make (GNU Make 4.0) nor shell (default Solaris /bin/sh) updates $PWD to point to the current directory where test.sh is executed and test.sh puts the test cluster in the original working directory of the process that launched make. I've restricted builds to one at a time on that host to work around this issue for now. Also attached a patch to explicitly set PWD=$(CURDIR) in the Makefile to make sure test.sh runs with the right directory. / Oskari From 61b18821553aa8193e46ad66904fabacb5a5a50a Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Tue, 7 Jul 2015 16:19:42 +0300 Subject: [PATCH] pg_upgrade: explicitly set PWD=$(CURDIR) to work around solaris issue --- src/bin/pg_upgrade/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index d9c8145..3e338e0 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -35,7 +35,7 @@ clean distclean maintainer-clean: pg_upgrade_dump_*.custom pg_upgrade_*.log check: test.sh all - MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $< --install + MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" PWD=$(CURDIR) $(SHELL) $< --install # disabled because it upsets the build farm #installcheck: test.sh -- 1.8.4.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Configurable location for extension .control files
02.07.2015, 20:31, Heikki Linnakangas kirjoitti: > On 03/04/2015 09:41 AM, Oskari Saarenmaa wrote: >> 18.02.2015, 01:49, Jim Nasby kirjoitti: >>> On 2/17/15 4:39 PM, Oskari Saarenmaa wrote: >>>> Here's a patch to allow overriding extension installation directory. >>>> The patch allows superusers to change it at runtime, but we could also >>>> restrict it to postgresql.conf if needed. I don't really see a >>>> point in >>>> restricting it (or not implementing the option at all) as the superuser >>>> can already load custom C code and sql from anywhere in the filesystem; >>>> not having configurable extension directory just makes it harder to >>>> test >>>> extensions and to create private clusters of PG data in a custom >>>> location without using custom binaries. >>> >>> I'm interested in this because it could potentially make it possible to >>> install SQL extensions without OS access. (My understanding is there's >>> some issue with writing the extension files even if LIBDIR is writable >>> by the OS account). >> >> I'm not sure this patch makes extensions without OS access any easier to >> implement; you still need to write the files somewhere, and someone >> needs to set up the cluster with a nonstandard extension path. > > Hmm. I think you're on to something with this patch, but I couldn't > exactly figure out what. What is the purpose of this patch? > > I can see a few things that you might want to do: > > 1. You might want to create a cluster using existing binaries, and set > it up so that you can install extra extensions from a custom directory. > Handy if you don't have write access to /usr, or you only want to make > an extension available in one cluster but not others. > > 2. Like 1, but you want to replace the set of set of extensions altogether. > > 3. Writing an automated regression test for a custom extension. > > With all of those, you have the problem that you actually want to > override both the lib-dir and the extensions-dir. So you'll need to set > dynamic_library_path too. For 3, fiddling with the configuration file is > a bit tedious. And PGXS doesn't currently have support for setting up a > test cluster anyway. I'm somewhat interested in both #1 & #2 for other projects, but I wrote this patch to address #3, i.e. to simplify the test setup we have in place for pgmemcache (https://github.com/ohmu/pgmemcache/blob/master/localtests.sh) and other extensions. I'd like to be able to set up a local PG cluster in /tmp or some other location and load the extensions I just built in there. Overriding dynamic_library_path is already possible, but there's currently no way to test the actual extension .control files. I don't think "fiddling with a configuration file" is all that bad -- you just run initdb and modify the few lines that you're interested in using sed or any other tool; that's something you'll have to do to set port or socket directory anyway. > Oh, and why are we only worried about extensions? There's other stuff in > 'share'-directory that you might also want to override in some clusters > or as part of regression tests: timezone and tsearch data. I suppose someone might be interested in using custom timezone data with existing binaries, but the number of existing third party extensions must be a lot higher than the number of third party timezone data sets. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Raising our compiler requirements for 9.6
01.07.2015, 23:33, Tom Lane kirjoitti: > Andres Freund writes: >> At the very least I think we should start to rely on 'static inline's >> working. There is not, and hasn't been for a while, any buildfarm animal >> that does not support it > > pademelon doesn't. HP-UX 10.20 was released in 1996, last shipped in July 2002 and unsupported since July 2003. Assuming the new features aren't going to be used in release branches PG 9.5 is probably going to support that platform longer than there's hardware to run it.. > But we should not fool ourselves that this comes at zero > compatibility cost. But compatibility with obsolete platforms doesn't come with zero cost either -- and judging from the fact that no one noticed until now that you couldn't even configure PG 9.0 .. 9.3 on recent Solaris 10 releases (which I believe was the most popular proprietary Unix) suggests that not a whole lot of people care about those platforms anymore. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Solaris testers wanted for strxfrm() behavior
27.06.2015, 19:51, Noah Misch kirjoitti: > convert_string_datum() says: > > /* >* Note: originally we guessed at a suitable output buffer > size, and >* only needed to call strxfrm twice if our guess was too small. >* However, it seems that some versions of Solaris have buggy > strxfrm >* that can write past the specified buffer length in that > scenario. >* So, do it the dumb way for portability. > > That arrived in commit 59d9a37, and I think this is the background: > http://www.postgresql.org/message-id/flat/3224.1020394...@sss.pgh.pa.us > > PostgreSQL 9.5 adds a strxfrm() call in bttext_abbrev_convert(), which does > not account for the Solaris bug. I wish to determine whether that bug is > still relevant today. If you have access to Solaris with the is_IS.ISO8859-1 > locale installed (or root access to install it), please compile and run the > attached test program on each Solaris version you have available. Reply here > with the program's output. I especially need a report from Solaris 10, but > reports from older and newer versions are valuable. Thanks. > > > Here is the output on OmniOS r151006, which does not have the bug: > > SunOS ip-10-152-178-106.ec2.internal 5.11 omnios-b281e50 i86pc i386 i86xpv > locale "is_IS.ISO8859-1": strxfrm returned 212; last modified byte at 58 (0x0) > locale "is_IS.ISO8859-1": strxfrm returned 212; last modified byte at 58 (0x0) > locale "": strxfrm returned 264; last modified byte at 58 (0x0) SunOS larry 5.10 Generic_147147-26 sun4u sparc SUNW,Sun-Fire-V215 locale "is_IS.ISO8859-1": strxfrm returned 216; last modified byte at 58 (0x0) locale "is_IS.ISO8859-1": strxfrm returned 216; last modified byte at 58 (0x0) locale "": strxfrm returned 26; last modified byte at 27 (0x0) / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] thread_test's sched_yield requires -lrt on solaris
I configured the dingo and binturong Solaris 10 animals to build 9.3
some time ago but apparently they always failed the configure phase.
Turns out this is caused by thread_test's usage of sched_yield which is
in librt on Solaris but which is not pulled in by anything on 9.3 and
earlier on my box.
Apparently the other Solaris animal (castoroides) requires librt for
fdatasync, but that's not required on my system. On 9.4 and master
librt is required for shm_open so the check doesn't fail there.
Attached a patch to check for sched_yield in configure, the patch only
applies against 9.0 - 9.3 which are using autoconf 2.63. We should
probably check for sched_yield anyway on all branches even if it's not
strictly required on 9.4+ at the moment.
/ Oskari
From b5a7400bdfad10fcb78a371f29fbde5dff52b40d Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Fri, 26 Jun 2015 09:36:29 +0300
Subject: [PATCH] configure: we need -lrt for sched_yield on solaris
thread_test.c uses sched_yield which is in librt on Solaris.
Previously we did not check for sched_yield in configure and would not pull
in librt in all cases. On some Solaris versions librt was required by
fdatasync, but that's not the case anymore on recent versions.
On 9.4 and master librt is again required for shm_open, but in 9.3 and
earlier there's nothing else using librt causing the thread_test check to
fail.
The configure.in diff in this patch applies against 9.0 - master, the
configure diff only applies against 9.0 - 9.3 which use autoconf 2.63; 9.4
and master require an `autoreconf` run.
---
configure| 88
configure.in | 2 ++
2 files changed, 90 insertions(+)
diff --git a/configure b/configure
index 1e95ab4..170e42a 100755
--- a/configure
+++ b/configure
@@ -8512,6 +8512,94 @@ if test "$ac_res" != no; then
fi
+# Required for thread_test.c on Solaris
+{ $as_echo "$as_me:$LINENO: checking for library containing sched_yield" >&5
+$as_echo_n "checking for library containing sched_yield... " >&6; }
+if test "${ac_cv_search_sched_yield+set}" = set; then
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h. */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char sched_yield ();
+int
+main ()
+{
+return sched_yield ();
+ ;
+ return 0;
+}
+_ACEOF
+for ac_lib in '' rt; do
+ if test -z "$ac_lib"; then
+ac_res="none required"
+ else
+ac_res=-l$ac_lib
+LIBS="-l$ac_lib $ac_func_search_save_LIBS"
+ fi
+ rm -f conftest.$ac_objext conftest$ac_exeext
+if { (ac_try="$ac_link"
+case "(($ac_try" in
+ *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+ *) ac_try_echo=$ac_try;;
+esac
+eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
+$as_echo "$ac_try_echo") >&5
+ (eval "$ac_link") 2>conftest.er1
+ ac_status=$?
+ grep -v '^ *+' conftest.er1 >conftest.err
+ rm -f conftest.er1
+ cat conftest.err >&5
+ $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
+ (exit $ac_status); } && {
+ test -z "$ac_c_werror_flag" ||
+ test ! -s conftest.err
+ } && test -s conftest$ac_exeext && {
+ test "$cross_compiling" = yes ||
+ $as_test_x conftest$ac_exeext
+ }; then
+ ac_cv_search_sched_yield=$ac_res
+else
+ $as_echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+
+fi
+
+rm -rf conftest.dSYM
+rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
+ conftest$ac_exeext
+ if test "${ac_cv_search_sched_yield+set}" = set; then
+ break
+fi
+done
+if test "${ac_cv_search_sched_yield+set}" = set; then
+ :
+else
+ ac_cv_search_sched_yield=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:$LINENO: result: $ac_cv_search_sched_yield" >&5
+$as_echo "$ac_cv_search_sched_yield" >&6; }
+ac_res=$ac_cv_search_sched_yield
+if test "$ac_res" != no; then
+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
# Required for thread_test.c on Solaris 2.5:
# Other ports use it too (HP-UX) so test unconditionally
{ $as_echo "$as_me:$LINENO: checking for library containing gethostbyname_r" >&5
diff --git a/configure.in b/configure.in
index 222e3e0..b964644 100644
--- a/configure.in
+++ b/c
Re: [HACKERS] hstore_plpython regression test does not work on Python 3
29.05.2015, 03:12, Peter Eisentraut kirjoitti: > On 5/26/15 5:19 PM, Oskari Saarenmaa wrote: >>> [1] http://pgbuildfarm.org/cgi-bin/show_history.pl?nm=jaguarundi&br=HEAD >> >> Looks like that animal uses Python 3.4. Python 3.3 and newer versions >> default to using a random seed for hashing objects into dicts which >> makes the order of dict elements random; see >> https://docs.python.org/3/using/cmdline.html#cmdoption-R > > Ah, good catch. That explains the, well, randomness. I can reproduce > the test failures with PYTHONHASHSEED=2. > > But I haven't been successful getting that environment variable set so > that it works in the installcheck case. Instead, I have rewritten the > tests to use asserts instead of textual comparisons. See attached > patch. Comments? Looks good to me. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] hstore_plpython regression test does not work on Python 3
22.05.2015, 09:44, Christian Ullrich kirjoitti:
> * Peter Eisentraut wrote:
>> On 5/16/15 12:06 PM, Tom Lane wrote:
>>> As exhibited for instance here:
>>>
>>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spoonbill&dt=2015-05-16%2011%3A00%3A07
>>>
>>>
>>> I've been able to replicate this on a Fedora 21 box: works fine with
>>> Python 2, fails with Python 3. Seems like we still have an issue
>>> with reliance on a system-provided sort method.
>>
>> Pushed a fix, tested with 2.3 .. 3.4.
>
> There is still a sorting problem (of sorts). jaguarundi [1] keeps
> failing intermittently like this:
>
> *** 47,53
> return len(val)
> $$;
> SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
> ! INFO: [{'aa': 'bb', 'cc': None}, {'dd': 'ee'}]
> CONTEXT: PL/Python function "test1arr"
>test1arr
> --
> --- 47,53
> return len(val)
> $$;
> SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
> ! INFO: [{'cc': None, 'aa': 'bb'}, {'dd': 'ee'}]
> CONTEXT: PL/Python function "test1arr"
>test1arr
> --
>
> I cannot find any other animal that does the same, but I doubt it's due
> to CCA this time.
>
> Should dict tests perhaps output sorted(thedict.items()) instead?
> Testing dict formatting could be done with single-item dicts.
>
> [1] http://pgbuildfarm.org/cgi-bin/show_history.pl?nm=jaguarundi&br=HEAD
Looks like that animal uses Python 3.4. Python 3.3 and newer versions
default to using a random seed for hashing objects into dicts which
makes the order of dict elements random; see
https://docs.python.org/3/using/cmdline.html#cmdoption-R
The test case could be changed to use sorted(dict.items()) always, but
there are multiple places where it would need to be applied. Setting
the PYTHONHASHSEED environment variable to a stable value would probably
be easier.
/ Oskari
--
Sent 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 concatenate operator's semantics seem questionable
18.05.2015, 06:41, Josh Berkus kirjoitti:
> On 05/17/2015 05:46 PM, Robert Haas wrote:
>> On May 17, 2015, at 8:38 PM, Peter Geoghegan wrote:
>>> The current behavior does not seem acceptable for the concatenate
>>> operator ("operator || jsonb").
>>
>> I don't agree. It seems pretty clear to me after reading the new posts that
>> the behavior is not an oversight, and that's enough for me to say that we
>> should leave this alone.
>
> Is there a particular reason why "+" makes more sense as "shallow
> concatination" and "||" makes more sense as "deep concatination"? Like,
> something in JS or other client languages which would make that
> preference make more sense to users?
>
> While I hate last-minute changes in general, once we have this
> functionality as || we won't be able to swap operators later if we
> decide that deep concatination should have been ||. So I want to be
> clear on why users will prefer that to + .
Both operations (shallow and deep merge) are useful and needed in many
applications but I've found the shallow merge to be more useful in the
"generic" use case; the deep merge implementations I've run across are
usually application specific as you need to decide what to do with
arrays, conflicting keys, etc.
I think concatenation is the right operator for shallow merge, it's
basically what would happen if you concatenated text representations of
two json objects and replaced the closing and opening braces between
them with a comma:
(substring(a::text, 1, length(a::text)-1) || ',' || substring(b::text,
2))::jsonb
Deep merge could be a function with flags to say what to do about
conflicts, etc.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Configurable location for extension .control files
18.02.2015, 01:49, Jim Nasby kirjoitti: > On 2/17/15 4:39 PM, Oskari Saarenmaa wrote: >> 10.06.2013, 17:51, Dimitri Fontaine kirjoitti: >>> Andres Freund writes: >>>>> In any case, no packager is going to ship an insecure-by-default >>>>> configuration, which is what Dimitri seems to be fantasizing would >>>>> happen. It would have to be local option to relax the permissions >>>>> on the directory, no matter where it is. >>>> >>>> *I* don't want that at all. All I'd like to have is a postgresql.conf >>>> option specifying additional locations. >>> >>> Same from me. I think I would even take non-plural location. >> >> Here's a patch to allow overriding extension installation directory. >> The patch allows superusers to change it at runtime, but we could also >> restrict it to postgresql.conf if needed. I don't really see a point in >> restricting it (or not implementing the option at all) as the superuser >> can already load custom C code and sql from anywhere in the filesystem; >> not having configurable extension directory just makes it harder to test >> extensions and to create private clusters of PG data in a custom >> location without using custom binaries. > > I'm interested in this because it could potentially make it possible to > install SQL extensions without OS access. (My understanding is there's > some issue with writing the extension files even if LIBDIR is writable > by the OS account). I'm not sure this patch makes extensions without OS access any easier to implement; you still need to write the files somewhere, and someone needs to set up the cluster with a nonstandard extension path. >> I don't think anyone should be packaging and shipping PG with >> extension_directory set, but we really should allow it for superusers >> and postgresql.conf so people can test extensions without hacks like >> this: https://github.com/ohmu/pgmemcache/blob/master/localtests.sh#L23 > > FWIW I'd like to see is breaking the cluster setup/teardown capability > in pg_regress into it's own tool. It wouldn't solve the extension test > problem, but it would eliminate the need for most of what that script is > doing, and it would do it more robustly. It would make it very easy to > unit test things with frameworks other than pg_regress. This would certainly be useful. I can try to do something about it if we get configurable extension path supported. The patch is now in https://commitfest.postgresql.org/5/170/ / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] __attribute__ for non-gcc compilers
23.02.2015, 04:31, Robert Haas kirjoitti:
> On Tue, Feb 17, 2015 at 8:41 AM, Oskari Saarenmaa wrote:
>> 15.01.2015, 21:58, Robert Haas kirjoitti:
>>> On Wed, Jan 14, 2015 at 5:54 PM, Andres Freund
>>> wrote:
>>>> I think I'd for now simply not define pg_attribute_aligned() on
>>>> platforms where it's not supported, instead of defining it empty. If we
>>>> need a softer variant we can name it pg_attribute_aligned_if_possible or
>>>> something.
>>>>
>>>> Sounds sane?
>>>
>>> Yes, that sounds like a much better plan.
>>
>> Attached an updated patch rebased on today's git master that never
>> defines aligned or packed empty.
>>
>> This is also included in the current commitfest,
>> https://commitfest.postgresql.org/4/115/
>
> Is this going to play nicely with pgindent?
I ran pgindent on the tree with this patch applied (with a few changes,
pgindent modified atomics headers a bit) and the changes looked ok to
me, mostly pgindent just rewrapped lines like this:
-extern void quickdie(SIGNAL_ARGS) pg_attribute_noreturn;
+extern void
+quickdie(SIGNAL_ARGS) pg_attribute_noreturn;
but there were two cases where it produced a bit weird indentation:
#ifdef __arm__
-pg_attribute_packed/* Appropriate whack upside the
head for ARM */
+ pg_attribute_packed /* Appropriate whack upside
the head for ARM */
#endif
ItemPointerData;
and
void
-pg_attribute_noreturn
+ pg_attribute_noreturn
plpgsql_yyerror(const char *message)
{
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Configurable location for extension .control files
10.06.2013, 17:51, Dimitri Fontaine kirjoitti:
> Andres Freund writes:
>>> In any case, no packager is going to ship an insecure-by-default
>>> configuration, which is what Dimitri seems to be fantasizing would
>>> happen. It would have to be local option to relax the permissions
>>> on the directory, no matter where it is.
>>
>> *I* don't want that at all. All I'd like to have is a postgresql.conf
>> option specifying additional locations.
>
> Same from me. I think I would even take non-plural location.
Here's a patch to allow overriding extension installation directory.
The patch allows superusers to change it at runtime, but we could also
restrict it to postgresql.conf if needed. I don't really see a point in
restricting it (or not implementing the option at all) as the superuser
can already load custom C code and sql from anywhere in the filesystem;
not having configurable extension directory just makes it harder to test
extensions and to create private clusters of PG data in a custom
location without using custom binaries.
I don't think anyone should be packaging and shipping PG with
extension_directory set, but we really should allow it for superusers
and postgresql.conf so people can test extensions without hacks like
this: https://github.com/ohmu/pgmemcache/blob/master/localtests.sh#L23
/ Oskari
From 35cae53aa5e9cf89b19a3ae276e635b42fbe5313 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Tue, 17 Feb 2015 23:27:59 +0200
Subject: [PATCH] Allow overriding extension_directory
Add a new GUC, 'extension_directory' to override the default directory for
extensions. This allows users running their own PostgreSQL clusters using
the system binaries to install custom extensions and makes testing
extensions easier.
---
doc/src/sgml/config.sgml | 38 +++
src/backend/commands/extension.c | 21 +--
src/backend/utils/misc/guc.c | 12 +
src/backend/utils/misc/postgresql.conf.sample | 2 ++
src/include/commands/extension.h | 2 ++
5 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c669f75..f0c762a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6341,6 +6341,44 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
+
+ extension_directory (string)
+
+ extension_directory configuration parameter
+
+ extensions
+
+
+
+Look up extensions from this path when an extension is created using
+the CREATE EXTENSION.
+
+
+
+The value for extension_directory must be an
+existing directory containing .control files for
+extensions.
+
+
+
+By default this is the empty string, which uses a directory based
+on the compiled-in PostgreSQL package
+share data directory; this is where the extensions provided by the
+standard PostgreSQL distribution are
+installed.
+
+
+
+This parameter can be changed at run time by superusers, but a
+setting done that way will only persist until the end of the
+client connection, so this method should be reserved for
+development purposes. The recommended way to set this parameter
+is in the postgresql.conf configuration
+file.
+
+
+
+
gin_fuzzy_search_limit (integer)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 9a0afa4..365ad59 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -56,6 +56,9 @@
#include "utils/tqual.h"
+/* GUC */
+char *Extension_directory;
+
/* Globally visible state variables */
bool creating_extension = false;
Oid CurrentExtensionObject = InvalidOid;
@@ -348,6 +351,9 @@ get_extension_control_directory(void)
char sharepath[MAXPGPATH];
char *result;
+ if (Extension_directory != NULL)
+ return pstrdup(Extension_directory);
+
get_share_path(my_exec_path, sharepath);
result = (char *) palloc(MAXPGPATH);
snprintf(result, MAXPGPATH, "%s/extension", sharepath);
@@ -358,13 +364,11 @@ get_extension_control_directory(void)
static char *
get_extension_control_filename(const char *extname)
{
- char sharepath[MAXPGPATH];
char *result;
- get_share_path(my_exec_path, sharepath);
result = (char *) palloc(MAXPGPATH);
- snprintf(result, MAXPGPATH, "%s/extension/%s.control",
- sharepath, extname);
+ snprintf(result, MAXPGPATH, "%s/%s.control",
+ get_extension_control_directory(), extname);
return result;
}
@@ -372,12 +376,12 @@ get_extension_control_filename(const char *extname)
static char *
get_extension_script
Re: [HACKERS] __attribute__ for non-gcc compilers
17.02.2015, 15:46, Andres Freund kirjoitti: > On 2015-02-17 15:41:45 +0200, Oskari Saarenmaa wrote: >> 15.01.2015, 21:58, Robert Haas kirjoitti: >>> On Wed, Jan 14, 2015 at 5:54 PM, Andres Freund >>> wrote: >>>> I think I'd for now simply not define pg_attribute_aligned() on >>>> platforms where it's not supported, instead of defining it empty. If we >>>> need a softer variant we can name it pg_attribute_aligned_if_possible or >>>> something. >>>> >>>> Sounds sane? >>> >>> Yes, that sounds like a much better plan. >> >> Attached an updated patch rebased on today's git master that never >> defines aligned or packed empty. > > Cool, that looks good. Besides allowing other compilers to use the > __attribute__ stuff, it also seems like a readability win to > me. Especially the various printf stuff looks much better to me this > way. > > I guess you've tested this on solaris and x86 linux? Yeah, tested on x86-64 Linux using gcc version 4.9.2 20150212 (Red Hat 4.9.2-6) and on Solaris 10 using Sun C 5.12 SunOS_sparc. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] __attribute__ for non-gcc compilers
15.01.2015, 00:54, Andres Freund kirjoitti: > I think I'd for now simply not define pg_attribute_aligned() on > platforms where it's not supported, instead of defining it empty. If we > need a softer variant we can name it pg_attribute_aligned_if_possible or > something. Good point, all attributes that actually change program behavior (aligned and packed for now) need to error out during compilation if they're used and they're not supported by the compiler. Attributes which may improve optimization or just provide more information for the developers (noreturn, unused, format) can be defined empty when they're not supported (or are not supported well enough: GCC < 3 doesn't know about %z in printf format.) / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal "VACUUM SCHEMA"
21.12.2014, 18:48, FabrÃzio de Royes Mello kirjoitti:
> I work with some customer that have databases with a lot of schemas and
> sometimes we need to run manual VACUUM in one schema, and would be nice
> to have a new option to run vacuum in relations from a specific schema.
>
> The new syntax could be:
>
> VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] { [ table_name ] | SCHEMA
> schema_name }
>
> Also I'll add a new option to "vacuumdb" client:
>
> -S, --schema=SCHEMA
>
> I can work on this feature to 2015/02 CF.
>
> Thoughts?
This would be useful for ANALYZE to make it easier to run analyze only
for the interesting schemas after a pg_upgrade. I have a database with
most of the actively used data in the "public" schema and a number of
rarely accessed large logging and archive tables in other schemas. It'd
be useful to prioritize analyzing the main tables before doing anything
about the rarely used schemas to allow the database to be put back into
production as soon as possible.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REINDEX CONCURRENTLY 2.0
13.11.2014, 23:50, Andres Freund kirjoitti: > On November 13, 2014 10:23:41 PM CET, Peter Eisentraut > wrote: >> On 11/12/14 7:31 PM, Andres Freund wrote: >>> Yes, it sucks. But it beats not being able to reindex a relation with >> a >>> primary key (referenced by a fkey) without waiting several hours by a >>> couple magnitudes. And that's the current situation. >> >> That's fine, but we have, for better or worse, defined CONCURRENTLY := >> does not take exclusive locks. Use a different adverb for an >> in-between >> facility. > > I think that's not actually a service to our users. They'll have to adapt > their scripts and knowledge when we get around to the more concurrent > version. What exactly CONCURRENTLY means is already not strictly defined and > differs between the actions. > > I'll note that DROP INDEX CONCURRENTLY actually already internally acquires > an AEL lock. Although it's a bit harder to see the consequences of that. If the short-lived lock is the only blocker for this feature at the moment could we just require an additional qualifier for CONCURRENTLY (FORCE?) until the lock can be removed, something like: tmp# REINDEX INDEX CONCURRENTLY tmp_pkey; ERROR: REINDEX INDEX CONCURRENTLY is not fully concurrent; use REINDEX INDEX CONCURRENTLY FORCE to perform reindex with a short-lived lock. tmp=# REINDEX INDEX CONCURRENTLY FORCE tmp_pkey; REINDEX It's not optimal, but currently there's no way to reindex a primary key anywhere close to concurrently and a short lock would be a huge improvement over the current situation. / Oskari -- Sent 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_basebackup fails with long tablespace paths
23.12.2014, 05:00, Amit Kapila kirjoitti: > On Tue, Dec 23, 2014 at 4:10 AM, Oskari Saarenmaa wrote: >> 08.11.2014, 04:03, Peter Eisentraut kirjoitti: >> > It errors when the file >> > name is too long and adds tests for that. This could be applied to 9.5 >> > and backpatched, if we so choose. It might become obsolete if >> > https://commitfest.postgresql.org/action/patch_view?id=1512 is accepted. >> > If that patch doesn't get accepted, I might add my patch to a future >> > commit fest. >> >> I think we should just use the UStar tar format >> (http://en.wikipedia.org/wiki/Tar_%28computing%29#UStar_format) and >> allow long file names; all actively used tar implementations should be >> able to handle them. I'll try to write a patch for that soonish. >> > > I think even using UStar format won't make it work for Windows where > the standard utilities are not able to understand the symlinks in tar. > There is already a patch [1] in this CF which will handle both cases, so > I am > not sure if it is very good idea to go with a new tar format to handle this > issue. > > [1] : https://commitfest.postgresql.org/action/patch_view?id=1512 That patch makes sense for 9.5, but I don't think it's going to be backpatched to previous releases? I think we should also apply Peter's patch to master and backbranches to avoid creating invalid tar files anywhere. And optionally implement and backpatch long filename support in tar even if 9.5 no longer creates tar files with long names. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On Fri, Nov 14, 2014 at 01:57:16AM +0100, Andreas Karlsson wrote: > *** a/configure.in > --- b/configure.in > *** AC_DEFINE_UNQUOTED(MAXIMUM_ALIGNOF, $MAX > *** 1751,1756 > --- 1751,1759 > AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [], > [#include ]) > > + # Check if platform support gcc style 128-bit integers. > + AC_CHECK_TYPES([__int128_t, __uint128_t], [], [], [#include ]) > + > # We also check for sig_atomic_t, which *should* be defined per ANSI > # C, but is missing on some old platforms. > AC_CHECK_TYPES(sig_atomic_t, [], [], [#include ]) __int128_t and __uint128_t are GCC extensions and are not related to stdint.h. > *** a/src/include/pg_config.h.in > --- b/src/include/pg_config.h.in > *** > *** 198,203 > --- 198,209 > /* Define to 1 if you have __sync_compare_and_swap(int64 *, int64, int64). > */ > #undef HAVE_GCC__SYNC_INT64_CAS > > + /* Define to 1 if you have __int128_t */ > + #undef HAVE___INT128_T > + > + /* Define to 1 if you have __uint128_t */ > + #undef HAVE___UINT128_T > + > /* Define to 1 if you have the `getaddrinfo' function. */ > #undef HAVE_GETADDRINFO These changes don't match what my autoconf does. Not a big deal I guess, but if this is merged as-is the next time someone runs autoreconf it'll write these lines differently to a different location of the file and the change will end up as a part of an unrelated commit. > *** a/src/backend/utils/adt/numeric.c > --- b/src/backend/utils/adt/numeric.c > *** static void apply_typmod(NumericVar *var > *** 402,407 > --- 402,410 > static int32 numericvar_to_int4(NumericVar *var); > static bool numericvar_to_int8(NumericVar *var, int64 *result); > static void int8_to_numericvar(int64 val, NumericVar *var); > + #ifdef HAVE_INT128 > + static void int16_to_numericvar(int128 val, NumericVar *var); > + #endif Existing code uses int4 and int8 to refer to 32 and 64 bit integers as they're also PG datatypes, but int16 isn't one and it looks a lot like int16_t. I think it would make sense to just call it int128 internally everywhere instead of int16 which isn't used anywhere else to refer to 128 bit integers. > new file mode 100755 I guess src/tools/git-external-diff generated these bogus "new file mode" lines? I know the project policy says that context diffs should be used, but it seems to me that most people just use unified diffs these days so I'd just use "git show" or "git format-patch -1" to generate the patches. The ones generated by "git format-patch" can be easily applied by reviewers using "git am". / Oskari -- Sent 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_basebackup fails with long tablespace paths
08.11.2014, 04:03, Peter Eisentraut kirjoitti: > On 11/4/14 3:52 PM, Peter Eisentraut wrote: >> > Here are patches to address that. First, it reports errors when >> > attempting to create a tar header that would truncate file or symlink >> > names. Second, it works around the problem in the tests by creating a >> > symlink from the short-name tempdir that we had set up for the >> > Unix-socket directory case. > I ended up splitting this up differently. I applied to part of the > second patch that works around the length issue in tablespaces. So the > tests now pass in 9.4 and up even in working directories with long > names. This clears up the regression in 9.4. > > The remaining, not applied patch is attached. It errors when the file > name is too long and adds tests for that. This could be applied to 9.5 > and backpatched, if we so choose. It might become obsolete if > https://commitfest.postgresql.org/action/patch_view?id=1512 is accepted. > If that patch doesn't get accepted, I might add my patch to a future > commit fest. I think we should just use the UStar tar format (http://en.wikipedia.org/wiki/Tar_%28computing%29#UStar_format) and allow long file names; all actively used tar implementations should be able to handle them. I'll try to write a patch for that soonish. Until UStar format is used we should raise an error if a filename is being truncated by tar instead of creating invalid archives. Also note that Posix tar format allows 100 byte file names as the name doesn't have to be zero terminated, but we may want to stick to 99 bytes in old type tar anyway as using 100 byte filenames has shown bugs in other tar implementations, for example https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689582 - and truncating at 100 bytes instead of 99 doesn't help us too much anyway. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inefficient barriers on solaris with sun cc
06.10.2014, 17:42, Andres Freund kirjoitti: > I think we can pretty much apply Oskari's patch after replacing > acquire/release with read/write intrinsics. Attached a patch rebased to current master using read & write barriers. / Oskari From a994c0f4feff74050ade183ec26d726397fa14a7 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Thu, 23 Oct 2014 18:36:31 +0300 Subject: [PATCH] =?UTF-8?q?=C2=A0atomics:=20add=20compiler=20and=20memory?= =?UTF-8?q?=20barriers=20for=20solaris=20studio?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- configure | 2 +- configure.in | 2 +- src/include/pg_config.h.in| 3 +++ src/include/port/atomics/generic-sunpro.h | 17 + 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/configure b/configure index b403a04..1248b06 100755 --- a/configure +++ b/configure @@ -9164,7 +9164,7 @@ fi done -for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h +for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" diff --git a/configure.in b/configure.in index df86882..0a3725f 100644 --- a/configure.in +++ b/configure.in @@ -1016,7 +1016,7 @@ AC_SUBST(UUID_LIBS) ## dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES -AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) +AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) # On BSD, test for net/if.h will fail unless sys/socket.h # is included first. diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index ddcf4b0..3e78d65 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -340,6 +340,9 @@ /* Define to 1 if `long long int' works and is 64 bits. */ #undef HAVE_LONG_LONG_INT_64 +/* Define to 1 if you have the header file. */ +#undef HAVE_MBARRIER_H + /* Define to 1 if you have the `mbstowcs_l' function. */ #undef HAVE_MBSTOWCS_L diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h index 77d3ebe..cd84107 100644 --- a/src/include/port/atomics/generic-sunpro.h +++ b/src/include/port/atomics/generic-sunpro.h @@ -19,6 +19,23 @@ #if defined(HAVE_ATOMICS) +#ifdef HAVE_MBARRIER_H +#include + +#define pg_compiler_barrier_impl() __compiler_barrier() + +#ifndef pg_memory_barrier_impl +# define pg_memory_barrier_impl() __machine_rw_barrier() +#endif +#ifndef pg_read_barrier_impl +# define pg_read_barrier_impl() __machine_r_barrier() +#endif +#ifndef pg_write_barrier_impl +# define pg_write_barrier_impl() __machine_w_barrier() +#endif + +#endif /* HAVE_MBARRIER_H */ + /* Older versions of the compiler don't have atomic.h... */ #ifdef HAVE_ATOMIC_H -- 1.8.4.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inefficient barriers on solaris with sun cc
26.09.2014, 17:28, Robert Haas kirjoitti: On Fri, Sep 26, 2014 at 8:55 AM, Oskari Saarenmaa wrote: So you think a read barrier is the same thing as an acquire barrier and a write barrier is the same as a release barrier? That would be surprising. It's certainly not true in general. The above doc describes the difference: read barrier requires loads before the barrier to be completed before loads after the barrier - an acquire barrier is the same, but it also requires loads to be complete before stores after the barrier. Similarly write barrier requires stores before the barrier to be completed before stores after the barrier - a release barrier is the same, but it also requires loads before the barrier to be completed before stores after the barrier. So acquire is read + loads-before-stores and release is write + loads-before-stores. Hmm. My impression was that an acquire barrier means that loads and stores can migrate forward across the barrier but not backward; and that a release barrier means that loads and stores can migrate backward across the barrier but not forward. I'm actually not really sure what this means unless the barrier also does something in and of itself. For example, consider this: [...] With the definition you (and Oracle) propose, this won't work, because there's nothing to keep the modification of things from being reordered before flag = 1. What good is that? Apparently, I don't have any idea! I'm not proposing any definition for acquire or release barriers, I was just proposing to use the things Solaris Studio defines as acquire and release barriers to implement read and write barriers in PostgreSQL because similar barrier names are used with gcc and on Solaris Studio acquire is a stronger read barrier and release is a stronger write barrier. atomics.h's definition of pg_(read|write)_barrier doesn't have any requirements for loads before stores, though, so we could use __machine_r_barrier and __machine_w_barrier instead. But as Andres pointed out all this is probably unnecessary and we could define read and write barrier as __compiler_barrier with Solaris Studio cc. It's only available for Solaris (x86 and Sparc) and Linux (x86). / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inefficient barriers on solaris with sun cc
26.09.2014, 15:39, Robert Haas kirjoitti: On Fri, Sep 26, 2014 at 8:36 AM, Oskari Saarenmaa wrote: 25.09.2014, 16:34, Andres Freund kirjoitti: Binaries compiled on solaris using sun studio cc currently don't have compiler and memory barriers implemented. That means we fall back to relatively slow generic implementations for those. Especially compiler, read, write barriers will be much slower than necessary (since they all just need to prevent compiler reordering as both sparc and x86 are run in TSO mode under solaris). Attached patch implements compiler and memory barriers for Solaris Studio based on documentation at http://docs.oracle.com/cd/E18659_01/html/821-1383/gjzmf.html I defined read and write barriers as acquire and release barriers instead of pure read and write ones as that's what other platforms appear to do. So you think a read barrier is the same thing as an acquire barrier and a write barrier is the same as a release barrier? That would be surprising. It's certainly not true in general. The above doc describes the difference: read barrier requires loads before the barrier to be completed before loads after the barrier - an acquire barrier is the same, but it also requires loads to be complete before stores after the barrier. Similarly write barrier requires stores before the barrier to be completed before stores after the barrier - a release barrier is the same, but it also requires loads before the barrier to be completed before stores after the barrier. So acquire is read + loads-before-stores and release is write + loads-before-stores. The generic gcc atomics also define read barrier to __ATOMIC_ACQUIRE and write barrier to __ATOMIC_RELEASE. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inefficient barriers on solaris with sun cc
25.09.2014, 16:34, Andres Freund kirjoitti: Binaries compiled on solaris using sun studio cc currently don't have compiler and memory barriers implemented. That means we fall back to relatively slow generic implementations for those. Especially compiler, read, write barriers will be much slower than necessary (since they all just need to prevent compiler reordering as both sparc and x86 are run in TSO mode under solaris). Attached patch implements compiler and memory barriers for Solaris Studio based on documentation at http://docs.oracle.com/cd/E18659_01/html/821-1383/gjzmf.html I defined read and write barriers as acquire and release barriers instead of pure read and write ones as that's what other platforms appear to do. / Oskari >From 0d1ee2b1d720a6ff1ae6b4707356e198b763fccf Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Fri, 26 Sep 2014 15:05:34 +0300 Subject: [PATCH] =?UTF-8?q?=C2=A0atomics:=20add=20compiler=20and=20memory?= =?UTF-8?q?=20barriers=20for=20solaris=20studio?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- configure | 2 +- configure.in | 2 +- src/include/pg_config.h.in| 3 +++ src/include/port/atomics/generic-sunpro.h | 17 + 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/configure b/configure index f0580ce..6aa55d1 100755 --- a/configure +++ b/configure @@ -9163,7 +9163,7 @@ fi done -for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h +for ac_header in atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" diff --git a/configure.in b/configure.in index 527b076..6dc9c08 100644 --- a/configure.in +++ b/configure.in @@ -1016,7 +1016,7 @@ AC_SUBST(UUID_LIBS) ## dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES -AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) +AC_CHECK_HEADERS([atomic.h crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h]) # On BSD, test for net/if.h will fail unless sys/socket.h # is included first. diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index ddcf4b0..3e78d65 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -340,6 +340,9 @@ /* Define to 1 if `long long int' works and is 64 bits. */ #undef HAVE_LONG_LONG_INT_64 +/* Define to 1 if you have the header file. */ +#undef HAVE_MBARRIER_H + /* Define to 1 if you have the `mbstowcs_l' function. */ #undef HAVE_MBSTOWCS_L diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h index b71b523..faab3d7 100644 --- a/src/include/port/atomics/generic-sunpro.h +++ b/src/include/port/atomics/generic-sunpro.h @@ -17,6 +17,23 @@ * - */ +#ifdef HAVE_MBARRIER_H +#include + +#define pg_compiler_barrier_impl() __compiler_barrier() + +#ifndef pg_memory_barrier_impl +# define pg_memory_barrier_impl() __machine_rw_barrier() +#endif +#ifndef pg_read_barrier_impl +# define pg_read_barrier_impl() __machine_acq_barrier() +#endif +#ifndef pg_write_barrier_impl +# define pg_write_barrier_impl() __machine_rel_barrier() +#endif + +#endif /* HAVE_MBARRIER_H */ + /* Older versions of the compiler don't have atomic.h... */ #ifdef HAVE_ATOMIC_H -- 2.1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing isinf declaration on solaris
24.09.2014, 23:26, Tom Lane kirjoitti: Peter Eisentraut writes: On 9/24/14 9:21 AM, Tom Lane wrote: Agreed, but what about non-GCC compilers? Stick AC_PROG_CC_C99 into configure.in. I think that's a bad idea, unless you mean to do it only on Solaris. If we do that unconditionally, we will pretty much stop getting any warnings about C99-isms on modern platforms. I am not aware that there has been any agreement to move our portability goalposts up to C99. We don't currently try to select a C89 mode in configure.in, we just use the default mode which may be C89 or C99 or something in between. GCC docs used to say that once C99 support is complete it'll switch defaults from gnu90 to gnu99, now the latest docs say that the default will change to gnu11 at some point (https://gcc.gnu.org/onlinedocs/gcc/Standards.html). Solaris Studio already defaults to C99 and it looks like the latest versions of MSVC also support it. I think we should just enable C99 mode when possible to use the backwards compatible features of it (like isinf). If C89 support is still needed we should set up a new buildfarm animal that really uses a C89 mode compiler and makes sure it compiles without warnings. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing isinf declaration on solaris
24.09.2014, 16:21, Tom Lane kirjoitti: Oskari Saarenmaa writes: ... so to enable XPG6 we'd need to use C99 mode anyway. OK. Could we just use -std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM it would be cleaner to just properly enable c99 mode rather than define an undocumented macro to use a couple of c99 declarations. Agreed, but what about non-GCC compilers? Solaris Studio defaults to "-xc99=all,no_lib" which, according to the man page, enables c99 language features but doesn't use c99 standard library semantics. isinf is defined to be a macro by c99 and doesn't require changing the c99 mode so I'd just keep using the defaults with Solaris Studio for now. For GCC --- a/src/template/solaris +++ b/src/template/solaris @@ -0,0 +1,4 @@ +if test "$GCC" = yes ; then + CPPFLAGS="$CPPFLAGS -std=gnu99" +fi + gets rid of the warnings and passes tests. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing isinf declaration on solaris
24.09.2014, 15:25, Tom Lane kirjoitti: Oskari Saarenmaa writes: GCC 4.9 build on Solaris 10 shows these warnings about isinf: float.c: In function 'is_infinite': float.c:178:2: warning: implicit declaration of function 'isinf' Ugh. isinf declaration is in which is included by , but it's surrounded by #if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >= 600 || defined(__C99FEATURES__). A couple of quick Google searches suggests that some other projects have worked around this by always defining __C99FEATURES__ even if compiling in C89 mode. __C99FEATURES__ is only used by math.h and fenv.h in /usr/include. Should we just add -D__C99FEATURES__ to CPPFLAGS in src/template/solaris, add our own declaration of isinf() or do something else about the warning? I'm worried that __C99FEATURES__ might do other, not-so-C89-compatible things in later Solaris releases. Possibly that risk could be addressed by having src/template/solaris make an OS version check before adding the switch, but it'd be a bit painful probably. Based on the #if you show, I'd be more inclined to think about defining _XOPEN_SOURCE to get the result. There is precedent for that in src/template/hpux which does CPPFLAGS="$CPPFLAGS -D_XOPEN_SOURCE_EXTENDED" I forget what-all _XOPEN_SOURCE_EXTENDED enables exactly, but if memory serves there were a nontrivial number of now-considered-standard features turned on by that in ancient HPUX releases. If you want to pursue this route, you'd need to do a bit of digging to see what else _XOPEN_SOURCE controls in Solaris and if there is some front-end feature macro (like _XOPEN_SOURCE_EXTENDED) that you're supposed to set instead of touching it directly. Looking at standards(5) and /usr/include/sys/feature_tests.h it looks like _XOPEN_SOURCE_EXTENDED enables XPG4v2 environment. _XOPEN_SOURCE=600 enables XPG6, but feature_tests.h also has this bit: /* * It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application * using c99. The same is true for POSIX.1-1990, POSIX.2-1992, POSIX.1b, * and POSIX.1c applications. Likewise, it is invalid to compile an XPG6 * or a POSIX.1-2001 application with anything other than a c99 or later * compiler. Therefore, we force an error in both cases. */ so to enable XPG6 we'd need to use C99 mode anyway. Could we just use -std=gnu99 (with -fgnu89-inline if required) with GCC on Solaris? ISTM it would be cleaner to just properly enable c99 mode rather than define an undocumented macro to use a couple of c99 declarations. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] missing isinf declaration on solaris
GCC 4.9 build on Solaris 10 shows these warnings about isinf: float.c: In function 'is_infinite': float.c:178:2: warning: implicit declaration of function 'isinf' [-Wimplicit-function-declaration] See http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=dingo&dt=2014-09-23%2002%3A52%3A00&stg=make isinf declaration is in which is included by , but it's surrounded by #if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >= 600 || defined(__C99FEATURES__). A couple of quick Google searches suggests that some other projects have worked around this by always defining __C99FEATURES__ even if compiling in C89 mode. __C99FEATURES__ is only used by math.h and fenv.h in /usr/include. Should we just add -D__C99FEATURES__ to CPPFLAGS in src/template/solaris, add our own declaration of isinf() or do something else about the warning? / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] better atomics - v0.6
23.09.2014, 15:18, Andres Freund kirjoitti:
On 2014-09-23 13:50:28 +0300, Oskari Saarenmaa wrote:
23.09.2014, 00:01, Andres Freund kirjoitti:
The patches:
0001: The actual atomics API
I tried building PG on Solaris 10/Sparc using GCC 4.9.0 (buildfarm animal
dingo) with this patch but regression tests failed due to:
Btw, if you could try sun studio it'd be great. I wrote the support for
it blindly, and I'd be surprised if I got it right on the first try.
I just installed Solaris Studio 12.3 and tried compiling this:
"../../../../src/include/port/atomics/generic-sunpro.h", line 54: return
value type mismatch
"../../../../src/include/port/atomics/generic-sunpro.h", line 77: return
value type mismatch
"../../../../src/include/port/atomics/generic-sunpro.h", line 79:
#if-less #endif
"../../../../src/include/port/atomics/generic-sunpro.h", line 81:
#if-less #endif
atomic_add_64 and atomic_add_32 don't return anything (the
atomic_add_*_nv variants return the new value) and there were a few
extra #endifs. Regression tests pass after applying the attached patch
which defines PG_HAS_ATOMIC_ADD_FETCH_U32.
There doesn't seem to be a fallback for defining pg_atomic_fetch_add
based on pg_atomic_add_fetch so pg_atomic_add_fetch now gets implemented
using pg_atomic_compare_exchange.
Also, it's not possible to compile PG with FORCE_ATOMICS_BASED_SPINLOCKS
with these patches:
"../../../../src/include/storage/s_lock.h", line 868: #error: PostgreSQL
does not have native spinlock support on this platform
atomics/generic.h would implement atomic flags using operations exposed
by atomics/generic-sunpro.h, but atomics/fallback.h is included before
it and it defines functions for flag operations which s_lock.h doesn't
want to use.
/ Oskari
>From 42a5bbbab0c8f42c6014ebe12c9963b371168866 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Tue, 23 Sep 2014 23:53:46 +0300
Subject: [PATCH] atomics: fix atomic add for sunpro and drop invalid #endifs
Solaris Studio compiler has an atomic add operation that returns the new
value, the one with no _nv suffix doesn't return anything.
---
src/include/port/atomics/generic-sunpro.h | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h
index 10ac70d..fedc099 100644
--- a/src/include/port/atomics/generic-sunpro.h
+++ b/src/include/port/atomics/generic-sunpro.h
@@ -47,14 +47,12 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
return ret;
}
-#define PG_HAS_ATOMIC_FETCH_ADD_U32
+#define PG_HAS_ATOMIC_ADD_FETCH_U32
STATIC_IF_INLINE uint32
-pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
+pg_atomic_add_fetch_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
{
- return atomic_add_32(&ptr->value, add_);
+ return atomic_add_32_nv(&ptr->value, add_);
}
-#endif
-
#define PG_HAS_ATOMIC_COMPARE_EXCHANGE_U64
static inline bool
@@ -70,12 +68,11 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr,
return ret;
}
-#define PG_HAS_ATOMIC_FETCH_ADD_U64
+#define PG_HAS_ATOMIC_ADD_FETCH_U64
STATIC_IF_INLINE uint64
-pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
+pg_atomic_add_fetch_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
{
- return atomic_add_64(&ptr->value, add_);
+ return atomic_add_64_nv(&ptr->value, add_);
}
-#endif
#endif
--
1.8.4.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] better atomics - v0.6
23.09.2014, 00:01, Andres Freund kirjoitti: I've finally managed to incorporate (all the?) feedback I got for 0.5. Imo the current version looks pretty good. Most notable changes: * Lots of comment improvements * code moved out of storage/ into port/ * separated the s_lock.h changes into its own commit * merged i386/amd64 into one file * fixed lots of the little details Amit noticed * fixed lots of XXX/FIXMEs * rebased to today's master * tested various gcc/msvc versions * extended the regression tests * ... The patches: 0001: The actual atomics API I tried building PG on Solaris 10/Sparc using GCC 4.9.0 (buildfarm animal dingo) with this patch but regression tests failed due to: /export/home/os/postgresql/src/test/regress/regress.so: symbol pg_write_barrier_impl: referenced symbol not found which turns out to be caused by a leftover PG_ prefix in ifdefs for HAVE_GCC__ATOMIC_INT64_CAS. Removing the PG_ prefix fixed the build and regression tests. Attached a patch to strip the invalid prefix. 0002: Implement s_lock.h support ontop the atomics API. Existing implementations continue to be used unless FORCE_ATOMICS_BASED_SPINLOCKS is defined Applied this and built PG with and without FORCE_ATOMICS_BASED_SPINLOCKS - both builds passed regression tests. 0003-0005: Not proposed for review here. Just included because code actually using the atomics make testing them easier. I'll look at these patches later. / Oskari >From 207b02cd7ee6d38b92b51195a951713639f0d738 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Tue, 23 Sep 2014 13:28:51 +0300 Subject: [PATCH] atomics: fix ifdefs for gcc atomics --- src/include/port/atomics/generic-gcc.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h index 9b0c636..f82af01 100644 --- a/src/include/port/atomics/generic-gcc.h +++ b/src/include/port/atomics/generic-gcc.h @@ -40,19 +40,19 @@ * definitions where possible, and use this only as a fallback. */ #if !defined(pg_memory_barrier_impl) -# if defined(HAVE_GCC__PG_ATOMIC_INT64_CAS) +# if defined(HAVE_GCC__ATOMIC_INT64_CAS) # define pg_memory_barrier_impl() __atomic_thread_fence(__ATOMIC_SEQ_CST) # elif (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)) # define pg_memory_barrier_impl() __sync_synchronize() # endif #endif /* !defined(pg_memory_barrier_impl) */ -#if !defined(pg_read_barrier_impl) && defined(HAVE_GCC__PG_ATOMIC_INT64_CAS) +#if !defined(pg_read_barrier_impl) && defined(HAVE_GCC__ATOMIC_INT64_CAS) /* acquire semantics include read barrier semantics */ # define pg_read_barrier_impl() __atomic_thread_fence(__ATOMIC_ACQUIRE) #endif -#if !defined(pg_write_barrier_impl) && defined(HAVE_GCC__PG_ATOMIC_INT64_CAS) +#if !defined(pg_write_barrier_impl) && defined(HAVE_GCC__ATOMIC_INT64_CAS) /* release semantics include write barrier semantics */ # define pg_write_barrier_impl() __atomic_thread_fence(__ATOMIC_RELEASE) #endif @@ -101,7 +101,7 @@ typedef struct pg_atomic_uint64 volatile uint64 value; } pg_atomic_uint64; -#endif /* defined(HAVE_GCC__PG_ATOMIC_INT64_CAS) || defined(HAVE_GCC__SYNC_INT64_CAS) */ +#endif /* defined(HAVE_GCC__ATOMIC_INT64_CAS) || defined(HAVE_GCC__SYNC_INT64_CAS) */ /* * Implementation follows. Inlined or directly included from atomics.c -- 1.8.4.1 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PL/pgSQL 1.2
06.09.2014 19:12, Jan Wieck kirjoitti: On 09/06/2014 04:21 AM, Marko Tiikkaja wrote: We wrap these things into (sometimes) simple-looking function so that none of the application developers ever run any SQL. We define an interface between the application and the database, and that interface is implemented using PL/PgSQL functions. Sure, sometimes one function will just fire off a single UPDATE .. RETURNING, or a SELECT, but that doesn't matter. The trick is to be consistent everywhere. There is precisely your root problem. Instead of educating your application developers on how to properly use a relational database system, you try to make it foolproof. There are also other reasons to wrap everything in functions, for example sharding using pl/proxy which by the way always throws an error if a SELECT didn't match exactly one row and the function wasn't declared returning 'SETOF' (although it currently doesn't set any sqlstate for these errors making it a bit difficult to properly catch them.) Anyway, I think the discussed feature to make select, update and delete throw an error if they returned or modified <> 1 row would be more useful as an extension of the basic sql statements instead of a plpgsql (2) only feature to make it possible to use it from other languages and outside functions. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
17.01.2014 19:07, Tom Lane kirjoitti: Oskari Saarenmaa writes: I don't know about others, but filtering by individual SQLSTATE is exactly what I need - I don't want to suppress all plpgsql errors or constraint violations, but I may want to suppress plpgsql RAISE EXCEPTION and CHECK violations. [...] It hasn't really been proven that SQLSTATE-class filtering would work conveniently, but AFAICS the only way to prove or disprove that is for somebody to code it up and use it in production. [...] Another thought here is that if you're willing to have the filter only able to *prevent* logging, and not to let it *cause* logging of messages that would otherwise be suppressed by log_min_messages, it could be implemented as a loadable module using the emit_log_hook. So this is what we ended up doing: a module with emit_log_hook to allow upgrading "log_min_messages" and "log_min_error_statement" values per sqlstate. I'm now using this in production and it has had a positive impact in reducing the volume of (unwanted) logs being collected and allowing a kludgy rsyslog.conf filter to be removed (which didn't really work that well - it only dropped the first part of a multipart log entry, writing partial pg log entries in syslog). https://github.com/ohmu/pgloggingfilter I'm not super happy about the syntax it uses, but at least it should be obvious that it works just like the core GUCs but is settable per sqlstate. I've been planning to sketch a proposal for a better way to configure log verbosity and details based on sqlstate, statement duration or other variables, but unfortunately haven't had time to do it yet. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] parser: optionally warn about missing AS for column and table aliases
05.09.2014 23:45, Marko Tiikkaja kirjoitti: On 2014-09-05 22:38, Oskari Saarenmaa wrote: I wrote the attached patch to optionally emit warnings when column or table aliases are used without the AS keyword after errors caused by typos in statements turning unintended things into aliases came up twice this week. First in a discussion with a colleague who was surprised by a 1 row result for the query 'SELECT COUNT(*) files' and again in the "pl/pgsql 2" thread as plpgsql currently doesn't throw an error if there are more result columns than output columns (SELECT a b INTO f1, f2). The patch is still missing documentation and it needs another patch to modify all the statements in psql & co to use AS so you can use things like \d and tab-completion without triggering the warnings. I can implement those changes if others think this patch makes sense. I think this is only problematic for column aliases. I wouldn't want to put these two to be put into the same category, as I always omit the AS keyword for tables aliases (and will continue to do so), but never omit it for column aliases. I prefer using AS for both, but I can see the case for requiring AS in table aliases being a lot weaker. Not emitting warnings for table aliases would also reduce the changes required in psql & co as they seem to be using aliases mostly (only?) for tables. What'd be a good name for the GUC controlling this, missing_column_as_warning? / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] parser: optionally warn about missing AS for column and table aliases
I wrote the attached patch to optionally emit warnings when column or table
aliases are used without the AS keyword after errors caused by typos in
statements turning unintended things into aliases came up twice this week.
First in a discussion with a colleague who was surprised by a 1 row result
for the query 'SELECT COUNT(*) files' and again in the "pl/pgsql 2" thread
as plpgsql currently doesn't throw an error if there are more result columns
than output columns (SELECT a b INTO f1, f2).
The patch is still missing documentation and it needs another patch to
modify all the statements in psql & co to use AS so you can use things like
\d and tab-completion without triggering the warnings. I can implement
those changes if others think this patch makes sense.
/ Oskari
>From 478e694e5281a3bf91e44177ce925e6625cb44a5 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Fri, 5 Sep 2014 22:31:22 +0300
Subject: [PATCH] parser: optionally warn about missing AS for column and table
aliases
Add a new GUC "missing_as_warning" (defaults to false, the previous
behavior) to raise a WARNING when a column or table alias is used without
the AS keyword.
This allows catching some types of errors where another keyword or a comma
was missing and a label is being used as an alias instead of something else,
for example cases like:
SELECT COUNT(*) files;
SELECT * FROM files users;
SELECT path size FROM files INTO f_path, f_size;
---
src/backend/parser/gram.y| 24 +
src/backend/utils/misc/guc.c | 11 +++
src/include/parser/parser.h | 2 +
src/test/regress/expected/select.out | 170 +++
src/test/regress/sql/select.sql | 47 ++
5 files changed, 254 insertions(+)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b46dd7b..06a71dd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -65,6 +65,10 @@
#include "utils/xml.h"
+/* GUCs */
+bool missing_as_warning = false;
+
+
/*
* Location tracking support --- simpler than bison's default, since we only
* want to track the start position not the end position of each nonterminal.
@@ -10119,12 +10123,20 @@ alias_clause:
}
| ColId '(' name_list ')'
{
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg("alias
without explicit AS and missing_as_warning enabled"),
+
parser_errposition(@3)));
$$ = makeNode(Alias);
$$->aliasname = $1;
$$->colnames = $3;
}
| ColId
{
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg("alias
without explicit AS and missing_as_warning enabled"),
+
parser_errposition(@1)));
$$ = makeNode(Alias);
$$->aliasname = $1;
}
@@ -10156,6 +10168,10 @@ func_alias_clause:
| ColId '(' TableFuncElementList ')'
{
Alias *a = makeNode(Alias);
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg("alias
without explicit AS and missing_as_warning enabled"),
+
parser_errposition(@1)));
a->aliasname = $1;
$$ = list_make2(a, $3);
}
@@ -10244,6 +10260,10 @@ relation_expr_opt_alias: relation_expr
%prec UMINUS
| relation_expr ColId
{
Alias *alias = makeNode(Alias);
+ if (missing_as_warning)
+ ereport(WARNING,
+ (errmsg("alias
without explicit AS and missing_as_warning enabled"),
+
parser_errposition(@2)));
Re: [HACKERS] Switch pg_basebackup to use -X stream instead of -X fetch by default?
On 25/08/14 14:35, Andres Freund wrote: currently pg_basebackup uses fetch mode when only -x is specified - which imo isn't a very good thing to use due to the increased risk of not fetching everything. How about switching to stream mode for 9.5+? +1. I was just wondering why it's not the default a few days ago. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] log_error_verbosity and unexpected errors
On 02/07/14 22:10, Tom Lane wrote: Greg Stark writes: I think log_error_verbosity is a strange variable. It's useless for expected user-facing errors but essential for unexpected errors that indicate bugs in the code -- and you can only have it on for everything or off for everything. I'm finding I usually want it set to 'verbose' for anything that PANICs or is generated by an elog() but it's just noise for anything generated by an ereport() and is ERROR or below. [...] [ thinks for a bit... ] A slightly cleaner approach is to nominate a specified set of SQLSTATEs, certainly including XX000 and perhaps some others, as being ones that force verbose reporting. That would have the same practical effect as far as elogs go, but wouldn't break the nominal functional equivalence. And that brings up the previous work on SQLSTATE-dependent choices about whether to log at all. I remember a patch was submitted for that but don't remember offhand why it didn't get committed. ISTM we should think about reviving that and making the choice be not just "log or not", but "no log, terse log, normal log, verbose log". I had a patch for making log_min_error_statement configurable per SQLSTATE in https://commitfest.postgresql.org/action/patch_view?id=1360 but you pointed out various issues in it and I didn't have time to update it for 9.4. I'm going to rewrite it based on the comments and submit it again for a 9.5 commitfest. The same mechanism could be used to set verbosity per SQLSTATE. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_basebackup: progress report max once per second
31.01.2014 10:59, Sawada Masahiko kirjoitti: On Tue, Jan 21, 2014 at 7:17 AM, Oskari Saarenmaa wrote: 18.11.2013 07:53, Sawada Masahiko kirjoitti: On 13 Nov 2013, at 20:51, Mika Eloranta wrote: Prevent excessive progress reporting that can grow to gigabytes of output with large databases. I got error with following scenario. $ initdb -D data -E UTF8 --no-locale /* setting the replication parameters */ $ pg_basebackup -D 2data Floating point exception LOG: could not send data to client: Broken pipe ERROR: base backup could not send data, aborting backup FATAL: connection to client lost Attached a rebased patch with a fix for division by zero error plus some code style issues. I also moved the patch to the current commitfest. Thank you for updating the patch! I have reviewed it easily. I didn't get error of compile, and the patch works fine. I have one question. What does it mean the calling progress_report() which you added at end of ReceiveUnpackTarFile() and RecieveTarFile()? I could not understand necessity of this code. And the force argument of progress_report() is also same. If you would like to use 'force' option, I think that you should add the comment to source code about it. I think the idea in the new progress_report() call (with force == true) is to make sure that there is at least one progress_report call that actually writes the progress report. Otherwise the final report may go missing if it gets suppressed by the time-based check. The force argument as used in the new call skips that check. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
30.01.2014 11:37, Jeevan Chalke kirjoitti: Hi Oskari, Are you planning to work on what Tom has suggested ? It make sense to me as well. What are your views on that ? Tom's suggestions make sense to me, but unfortunately I haven't had time to work on this feature recently so I don't think it'll make it to 9.4 unless I can complete it during FOSDEM. I updated https://github.com/saaros/postgres/tree/log-by-sqlstate some time ago based on Tom's first set of comments but the tree is still missing changes suggested in the last email. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] integrate pg_upgrade analyze_new_cluster.sh into vacuumdb
09.01.2014 05:15, Peter Eisentraut kirjoitti:
pg_upgrade creates a script analyze_new_cluster.{sh|bat} that runs
vacuumdb --analyze-only in three stages with different statistics target
settings to get a fresh cluster analyzed faster. I think this behavior
is also useful for clusters or databases freshly created by pg_restore
or any other loading mechanism, so it's suboptimal to have this
constrained to pg_upgrade.
I think the three stage analyze is a wrong solution to the "slow
analyze" problem. In my experience most of the analyze time goes to
reading random blocks from the disk but we usually use only a small
portion of that data (1 row per block.)
If we were able to better utilize the data we read we could get good
statistics with a lot less IO than we currently need. This was
discussed in length at
http://www.postgresql.org/message-id/CAM-w4HOjRbNPMW=shjhw_qfapcuu5ege1tmdr0zqu+kqx8q...@mail.gmail.com
but it hasn't turned into patches so far.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_basebackup: progress report max once per second
18.11.2013 07:53, Sawada Masahiko kirjoitti:
On 13 Nov 2013, at 20:51, Mika Eloranta wrote:
Prevent excessive progress reporting that can grow to gigabytes
of output with large databases.
I got error with following scenario.
$ initdb -D data -E UTF8 --no-locale
/* setting the replication parameters */
$ pg_basebackup -D 2data
Floating point exception
LOG: could not send data to client: Broken pipe
ERROR: base backup could not send data, aborting backup
FATAL: connection to client lost
Attached a rebased patch with a fix for division by zero error plus some
code style issues. I also moved the patch to the current commitfest.
/ Oskari
>From 1c54ffc5006320da1b021c2a07939f948ba9fdb1 Mon Sep 17 00:00:00 2001
From: Mika Eloranta
Date: Tue, 21 Jan 2014 00:15:27 +0200
Subject: [PATCH] pg_basebackup: progress report max once per second
Prevent excessive progress reporting that can grow to gigabytes
of output with large databases.
---
src/bin/pg_basebackup/pg_basebackup.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 9d13d57..cae181c 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -15,6 +15,7 @@
#include "libpq-fe.h"
#include "pqexpbuffer.h"
#include "pgtar.h"
+#include "pgtime.h"
#include
#include
@@ -46,6 +47,7 @@ static bool streamwal = false;
static bool fastcheckpoint = false;
static bool writerecoveryconf = false;
static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
+static pg_time_t last_progress_report = 0;
/* Progress counters */
static uint64 totalsize;
@@ -75,7 +77,7 @@ static PQExpBuffer recoveryconfcontents = NULL;
/* Function headers */
static void usage(void);
static void verify_dir_is_empty_or_create(char *dirname);
-static void progress_report(int tablespacenum, const char *filename);
+static void progress_report(int tablespacenum, const char *filename, bool force);
static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
static void ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum);
@@ -401,11 +403,18 @@ verify_dir_is_empty_or_create(char *dirname)
* is enabled, also print the current file name.
*/
static void
-progress_report(int tablespacenum, const char *filename)
+progress_report(int tablespacenum, const char *filename, bool force)
{
- int percent = (int) ((totaldone / 1024) * 100 / totalsize);
+ int percent;
char totaldone_str[32];
char totalsize_str[32];
+ pg_time_t now = time(NULL);
+
+ if (!showprogress || (now == last_progress_report && !force))
+ return; /* Max once per second */
+
+ last_progress_report = now;
+ percent = totalsize ? (int) ((totaldone / 1024) * 100 / totalsize) : 0;
/*
* Avoid overflowing past 100% or the full size. This may make the total
@@ -852,9 +861,9 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
}
}
totaldone += r;
- if (showprogress)
- progress_report(rownum, filename);
+ progress_report(rownum, filename, false);
} /* while (1) */
+ progress_report(rownum, filename, true);
if (copybuf != NULL)
PQfreemem(copybuf);
@@ -1079,8 +1088,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
disconnect_and_exit(1);
}
totaldone += r;
- if (showprogress)
-progress_report(rownum, filename);
+ progress_report(rownum, filename, false);
current_len_left -= r;
if (current_len_left == 0 && current_padding == 0)
@@ -1096,6 +1104,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
}
} /* continuing data in existing file */
} /* loop over all data blocks */
+ progress_report(rownum, filename, true);
if (file != NULL)
{
@@ -1456,8 +1465,7 @@ BaseBackup(void)
tablespacecount = PQntuples(res);
for (i = 0; i < PQntuples(res); i++)
{
- if (showprogress)
- totalsize += atol(PQgetvalue(res, i, 2));
+ totalsize += atol(PQgetvalue(res, i, 2));
/*
* Verify tablespace directories are empty. Don't bother with the
@@ -1504,7 +1512,7 @@ BaseBackup(void)
if (showprogress)
{
- progress_report(PQntuples(res), NULL);
+ progress_report(PQntuples(res), NULL, true);
fprintf(stderr, "\n"); /* Need to move to next line */
}
PQclear(res);
--
1.8.4.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
17.01.2014 00:13, Tom Lane kirjoitti: Oskari Saarenmaa writes: [ 0001-Filter-error-log-statements-by-sqlstate.patch ] I looked at this patch. It took me some time to understand that what it actually does has got approximately nothing to do with what one might first expect: rather than suppressing the entire log message about some error, it only suppresses printing of the triggering SQL statement (if that's available to begin with). The proposed documentation is certainly not clear enough on that point, and the API which appears to allow changing the error severity level associated with a SQLSTATE isn't exactly helping to clarify things either. Also, the patch claims it allows logging of statements that otherwise wouldn't get logged, but AFAICS that's wrong, because we'll never get to this code at all if errstart decides we're not going to log the message. I agree the documentation (and perhaps the feature itself) are a bit confusing, but the idea is to control SQL statement logging when errors occur. This patch doesn't do anything about normal error logging, it only controls when the statements are printed. Running: set log_min_messages = 'warning'; set log_min_error_statement = 'panic'; set log_sqlstate_error_statement = ''; do 'begin raise exception ''foobar 1''; end'; set log_sqlstate_error_statement = 'P0001:error'; do 'begin raise exception ''foobar 2''; end'; set log_min_error_statement = 'error'; set log_sqlstate_error_statement = 'P0001:panic'; do 'begin raise exception ''foobar 3''; end'; logs 2014-01-17 00:37:12 EET ERROR: foobar 1 2014-01-17 00:37:20 EET ERROR: foobar 2 2014-01-17 00:37:20 EET STATEMENT: do 'begin raise exception ''foobar 2''; end'; 2014-01-17 00:38:34 EET ERROR: foobar 3 I find it hard to follow exactly what the use-case for this is; could you make a defense of why we should carry a feature like this? I usually run PG with log_min_messages = warning and log_min_error_statement = error to log any unexpected errors. But as I have a lot of check constraints in my database as well as a lot of plpgsql and plpython code which raises exceptions on invalid user input I also get tons of logs for statements causing "expected" errors which I have to try to filter out with other tools. In any case, I'm finding it hard to believe that filtering by individual SQLSTATEs is a practical API. When we've discussed providing better log filtering in the past, that approach was invariably dismissed on the grounds that it would be far too unwieldy to use --- any DBA attempting to use it in anger would end up with a huge and ever-incomplete list of SQLSTATEs he'd need to filter. A long time ago I suggested that filtering by SQLSTATE class (the first two characters of SQLSTATE) might be useful, but I'm not sure I still believe that, and anyway it's not what's implemented here. I don't know about others, but filtering by individual SQLSTATE is exactly what I need - I don't want to suppress all plpgsql errors or constraint violations, but I may want to suppress plpgsql RAISE EXCEPTION and CHECK violations. I'm concerned also about the potential performance impact of this patch, if used with a SQLSTATE list as long as I think they're likely to get in practice. Have you done any performance testing? Not yet. As this only applies to statement logging (for now at least) I doubt it's a big deal, formatting and writing the statement somewhere is probably at least as expensive as looking up the configuration. As far as the code goes, bit manipulations on uint64s are a pretty crummy substitute for defining a struct with a couple of fields; and the way you're choosing to sort the array seems mighty inefficient, as well as probably wrong in detail (why is the loop ignoring the first array element?); and rather than make fragile assumptions about the maximum length of an elevel name, why not just leave the user's string as-is? But I wouldn't advise worrying about code style issues until we decide if we're accepting the feature. Right now my inclination is to reject it. I agree, I should've just defined a struct and used the original string length when rewriting user string (it's rewritten to drop any duplicates.) I don't think the sort is a big deal, it's only done when the value is defined; the first array element is the length of the array. I can address these points in a new version of this patch if the feature looks useful. Thanks for the review! Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
On Tue, Jan 14, 2014 at 12:22:30PM +0530, Jeevan Chalke wrote: > On Mon, Jan 13, 2014 at 4:30 PM, Oskari Saarenmaa wrote: > > On 13/01/14 10:26, Jeevan Chalke wrote: > > > > > 1. Documentation is missing and thus becomes difficult to understand > > > what exactly you are trying to do. Or in other words, user will be > > > uncertain about using it more efficiently. > > > > I figured I'd write documentation for this if it looks like a useful > > feature which would be accepted for 9.4, but I guess it would've > > helped to have a bit better description of this for the initial > > submission as well. > > > > > > > 2. Some more comments required. At each new function and > > > specifically at get_sqlstate_error_level(). > > > > Just after I submitted the patch I noticed that I had a placeholder > > for comment about that function but never wrote the actual comment, > > sorry about that. > > > > > 3. Please add test-case if possible. > > > > Sure. > > > > > 4. Some code part does not comply with PostgreSQL indentation style. > > > (Can be ignored as it will pass through pg_indent, but better fix > > > it). > > > > > > > I'll try to fix this for v2. > > > > > 5. You have used ""XX000:warning," string to get maximum possible > > > length of the valid sqlstate:level identifier. It's perfect, but > > > small explanation about that will be good there. Also in future if > > > we have any other error level which exceeds this, we need changes > > > here too. Right ? > > > > Good point, I'll address this in v2. > > > > > I will look into this further. But please have your attention on above > > > points. > > > > Thanks for the review! > > Since you are taking care of most of the points above. I will wait for v2 > patch. Till then marking "Waiting on Author". Attached v2 of the patch which addresses the above points. I couldn't figure out how to test log output, but at least the patch now tests that it can set and show the log level. Thanks, Oskari >From 213f647657f318141e3866087a17a863a0f322d9 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Tue, 14 Jan 2014 15:47:39 +0200 Subject: [PATCH] Filter error log statements by sqlstate Allow the default log_min_error_statement to be overridden per sqlstate to make it possible to filter out some error types while maintaining a low log_min_error_statement or enable logging for some error types when the default is to not log anything. --- doc/src/sgml/config.sgml | 30 ++ src/backend/utils/error/elog.c| 220 +- src/backend/utils/misc/guc.c | 14 ++- src/include/utils/guc.h | 4 + src/include/utils/guc_tables.h| 1 + src/test/regress/expected/guc.out | 24 + src/test/regress/sql/guc.sql | 8 ++ 7 files changed, 298 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0f2f2bf..73a58ad 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3743,6 +3743,36 @@ local0.*/var/log/postgresql + + log_sqlstate_error_statement (string) + + log_sqlstate_error_statement configuration parameter + + + +Controls which error types in SQL statements condition are recorded +in the server log. This overrides the global per error type and can be used to +disable logging for certain error types and/or to enable logging for +other types. + + +The value must be a comma-separated list in the form +'ERRORCODE:LOGLEVEL,...'. For example, a setting +of 'P0001:PANIC,22012:ERROR' would never log the +SQL statements for errors generated by the PL/pgSQL +RAISE statement but would always log the +statements causing division by zero errors. + +See for the list of valid error +codes and for valid log +levels. + +Only superusers can change this setting. + + + + log_min_duration_statement (integer) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 8705586..2e74fd5 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -74,7 +74,9 @@ #include "storage/ipc.h" #include "storage/proc.h" #include "tcop/tcopprot.h" +#include "utils/builtins.h" #include "utils/guc.h" +#include "utils/guc_tables.h" #include "utils/memutils.h" #include "utils/ps_status.h" @@ -111,6 +113,11 @@ char
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
Hi, On 13/01/14 10:26, Jeevan Chalke wrote: 1. Documentation is missing and thus becomes difficult to understand what exactly you are trying to do. Or in other words, user will be uncertain about using it more efficiently. I figured I'd write documentation for this if it looks like a useful feature which would be accepted for 9.4, but I guess it would've helped to have a bit better description of this for the initial submission as well. 2. Some more comments required. At each new function and specifically at get_sqlstate_error_level(). Just after I submitted the patch I noticed that I had a placeholder for comment about that function but never wrote the actual comment, sorry about that. 3. Please add test-case if possible. Sure. 4. Some code part does not comply with PostgreSQL indentation style. (Can be ignored as it will pass through pg_indent, but better fix it). I'll try to fix this for v2. 5. You have used ""XX000:warning," string to get maximum possible length of the valid sqlstate:level identifier. It's perfect, but small explanation about that will be good there. Also in future if we have any other error level which exceeds this, we need changes here too. Right ? Good point, I'll address this in v2. I will look into this further. But please have your attention on above points. Thanks for the review! / Oskari On Fri, Jan 10, 2014 at 12:56 AM, Oskari Saarenmaa wrote: > Allow the default log_min_error_statement to be overridden per > sqlstate to make it possible to filter out some error types while > maintaining a low log_min_error_statement or enable logging for some > error types when the default is to not log anything. > > I've tried to do something like this using rsyslog filters, but > that's pretty awkward and doesn't work at all when the statement is > split to multiple syslog messages. > > https://github.com/saaros/postgres/compare/log-by-sqlstate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgcrypto: implement gen_random_uuid
13.01.2014 04:35, Wim Lewis kirjoitti: One comment, this: /* get 128 random bits */ int err = px_get_random_bytes(buf, 16); might be better to use px_get_pseudo_random_bytes(). UUIDs don't need to be unguessable or have perfect entropy; they just need to be collision-resistant. RFC4122 mentions this I think, and if you look at the ossp-uuid function that this is replacing, it also uses its internal PRNG for v4 UUIDs rather than strong high-entropy randomness. (The downside of requesting strong randomness when you don't need it is that it can potentially cause the server to block while the system gathers entropy.) pgcrypto's px_get_pseudo_random_bytes is just a wrapper for px_get_random_bytes which itself calls system_reseed and fortuna_get_bytes. system_reseed function tries to read from /dev/urandom, and only uses /dev/random if reading urandom fails, so it should never block on systems which have urandom. That said, it may still make sense to use px_get_pseudo_random_bytes instead just in case it ever gets modified to do something lighter than px_get_random_bytes. Thanks for the review, Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] pgcrypto: implement gen_random_uuid
The only useful feature of the uuid-ossp module in my opinion is the
uuid_generate_v4 function and as uuid-ossp is more or less abandonware
people have had trouble building and installing it. This patch
implements an alternative uuid v4 generation function in pgcrypto which
could be moved to core once there's a core PRNG with large enough
internal state.
On my test system it took 3796 msec to generate a million UUIDs with
pgcrypto while uuid-ossp took 20375 msec.
https://github.com/saaros/postgres/compare/pgcrypto-uuid-v4
contrib/pgcrypto/Makefile | 2 +-
contrib/pgcrypto/pgcrypto--1.0--1.1.sql | 8
contrib/pgcrypto/{pgcrypto--1.0.sql => pgcrypto--1.1.sql} | 7 ++-
contrib/pgcrypto/pgcrypto.c | 22
++
contrib/pgcrypto/pgcrypto.control | 2 +-
contrib/pgcrypto/pgcrypto.h | 1 +
doc/src/sgml/pgcrypto.sgml| 11 +++
/ Oskari
>From 522fef9c3739d4c4f3c107e574e84db67a0c07a2 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Thu, 9 Jan 2014 22:24:36 +0200
Subject: [PATCH] pgcrypto: implement gen_random_uuid
---
contrib/pgcrypto/Makefile | 2 +-
contrib/pgcrypto/pgcrypto--1.0--1.1.sql | 8 ++
contrib/pgcrypto/pgcrypto--1.0.sql | 202 ---
contrib/pgcrypto/pgcrypto--1.1.sql | 207
contrib/pgcrypto/pgcrypto.c | 22
contrib/pgcrypto/pgcrypto.control | 2 +-
contrib/pgcrypto/pgcrypto.h | 1 +
doc/src/sgml/pgcrypto.sgml | 11 ++
8 files changed, 251 insertions(+), 204 deletions(-)
create mode 100644 contrib/pgcrypto/pgcrypto--1.0--1.1.sql
delete mode 100644 contrib/pgcrypto/pgcrypto--1.0.sql
create mode 100644 contrib/pgcrypto/pgcrypto--1.1.sql
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index dadec95..1c85c98 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -26,7 +26,7 @@ MODULE_big = pgcrypto
OBJS = $(SRCS:.c=.o)
EXTENSION = pgcrypto
-DATA = pgcrypto--1.0.sql pgcrypto--unpackaged--1.0.sql
+DATA = pgcrypto--1.1.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
$(CF_TESTS) \
diff --git a/contrib/pgcrypto/pgcrypto--1.0--1.1.sql b/contrib/pgcrypto/pgcrypto--1.0--1.1.sql
new file mode 100644
index 000..2601669
--- /dev/null
+++ b/contrib/pgcrypto/pgcrypto--1.0--1.1.sql
@@ -0,0 +1,8 @@
+/* contrib/pgcrypto/pgcrypto--1.0--1.1.sql */
+
+\echo Use "ALTER EXTENSION pgcrypto UPDATE" to load this file. \quit
+
+CREATE FUNCTION gen_random_uuid()
+RETURNS uuid
+AS 'MODULE_PATHNAME', 'pg_random_uuid'
+LANGUAGE C VOLATILE;
diff --git a/contrib/pgcrypto/pgcrypto--1.0.sql b/contrib/pgcrypto/pgcrypto--1.0.sql
deleted file mode 100644
index 347825e..000
--- a/contrib/pgcrypto/pgcrypto--1.0.sql
+++ /dev/null
@@ -1,202 +0,0 @@
-/* contrib/pgcrypto/pgcrypto--1.0.sql */
-
--- complain if script is sourced in psql, rather than via CREATE EXTENSION
-\echo Use "CREATE EXTENSION pgcrypto" to load this file. \quit
-
-CREATE FUNCTION digest(text, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_digest'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION digest(bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_digest'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION hmac(text, text, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_hmac'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION hmac(bytea, bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_hmac'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION crypt(text, text)
-RETURNS text
-AS 'MODULE_PATHNAME', 'pg_crypt'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION gen_salt(text)
-RETURNS text
-AS 'MODULE_PATHNAME', 'pg_gen_salt'
-LANGUAGE C VOLATILE STRICT;
-
-CREATE FUNCTION gen_salt(text, int4)
-RETURNS text
-AS 'MODULE_PATHNAME', 'pg_gen_salt_rounds'
-LANGUAGE C VOLATILE STRICT;
-
-CREATE FUNCTION encrypt(bytea, bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_encrypt'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION decrypt(bytea, bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_decrypt'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION encrypt_iv(bytea, bytea, bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_encrypt_iv'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION decrypt_iv(bytea, bytea, bytea, text)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_decrypt_iv'
-LANGUAGE C IMMUTABLE STRICT;
-
-CREATE FUNCTION gen_random_bytes(int4)
-RETURNS bytea
-AS 'MODULE_PATHNAME', 'pg_random_bytes&
[HACKERS] [PATCH] Filter error log statements by sqlstate
Allow the default log_min_error_statement to be overridden per sqlstate
to make it possible to filter out some error types while maintaining a
low log_min_error_statement or enable logging for some error types when
the default is to not log anything.
I've tried to do something like this using rsyslog filters, but that's
pretty awkward and doesn't work at all when the statement is split to
multiple syslog messages.
https://github.com/saaros/postgres/compare/log-by-sqlstate
src/backend/utils/error/elog.c | 183
-
src/backend/utils/misc/guc.c | 14 +++-
src/include/utils/guc.h| 4 +
src/include/utils/guc_tables.h | 1 +
4 files changed, 199 insertions(+), 3 deletions(-)
/ Oskari
>From 61fe332f35f49c59257e9dcd0b5e2ff80f1f4055 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Thu, 9 Jan 2014 20:49:28 +0200
Subject: [PATCH] Filter error log statements by sqlstate
Allow the default log_min_error_statement to be overridden per sqlstate to
make it possible to filter out some error types while maintaining a low
log_min_error_statement or enable logging for some error types when the
default is to not log anything.
---
src/backend/utils/error/elog.c | 183 -
src/backend/utils/misc/guc.c | 14 +++-
src/include/utils/guc.h| 4 +
src/include/utils/guc_tables.h | 1 +
4 files changed, 199 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 3de162b..c843e1a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -74,7 +74,9 @@
#include "storage/ipc.h"
#include "storage/proc.h"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
#include "utils/guc.h"
+#include "utils/guc_tables.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
@@ -111,6 +113,11 @@ char *Log_line_prefix = NULL; /* format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+static uint64 *log_sqlstate_error_statement = NULL;
+static size_t log_sqlstate_error_statement_len = 0;
+
+static int get_sqlstate_error_level(int sqlstate);
+
#ifdef HAVE_SYSLOG
/*
@@ -2475,6 +2482,7 @@ static void
write_csvlog(ErrorData *edata)
{
StringInfoData buf;
+ int requested_log_level;
bool print_stmt = false;
/* static counter for line numbers */
@@ -2618,7 +2626,10 @@ write_csvlog(ErrorData *edata)
appendStringInfoChar(&buf, ',');
/* user query --- only reported if not disabled by the caller */
- if (is_log_level_output(edata->elevel, log_min_error_statement) &&
+ requested_log_level = get_sqlstate_error_level(edata->sqlerrcode);
+ if (requested_log_level < 0)
+ requested_log_level = log_min_error_statement;
+ if (is_log_level_output(edata->elevel, requested_log_level) &&
debug_query_string != NULL &&
!edata->hide_stmt)
print_stmt = true;
@@ -2691,6 +2702,7 @@ static void
send_message_to_server_log(ErrorData *edata)
{
StringInfoData buf;
+ int requested_log_level;
initStringInfo(&buf);
@@ -2775,7 +2787,10 @@ send_message_to_server_log(ErrorData *edata)
/*
* If the user wants the query that generated this error logged, do it.
*/
- if (is_log_level_output(edata->elevel, log_min_error_statement) &&
+ requested_log_level = get_sqlstate_error_level(edata->sqlerrcode);
+ if (requested_log_level < 0)
+ requested_log_level = log_min_error_statement;
+ if (is_log_level_output(edata->elevel, requested_log_level) &&
debug_query_string != NULL &&
!edata->hide_stmt)
{
@@ -3577,3 +3592,167 @@ trace_recovery(int trace_level)
return trace_level;
}
+
+
+/*
+*/
+static int
+get_sqlstate_error_level(int sqlstate)
+{
+ uint64 left = 0, right = log_sqlstate_error_statement_len;
+ while (left < right)
+ {
+ uint64 middle = left + (right - left) / 2;
+ int m_sqlstate = log_sqlstate_error_statement[middle] >> 32;
+
+ if (m_sqlstate == sqlstate)
+ return log_sqlstate_error_statement[middle] & 0x;
+ else if (m_sqlstate < sqlstate)
+ left = middle + 1;
+ else
+ right = middle;
+ }
+ return -1;
+}
+
+bool
+check_log_sqlstate_error(char **newval, void **extra, GucSource source)
+{
+ const struct config_enum_entry *enum_entry;
+ char *rawstring, *new_newval, *rp;
+ List *elemlist;
+ ListCell *l;
+ uint64 *new_array = NULL;
+ int i, new_array_len = 0;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of identifiers */
+ if (!SplitIdentifierString(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ GUC_check_errdetail("List syntax is invalid.");
+ pfree(rawstring);
+ list_free(elemlist);
+ r
Re: [HACKERS] [PATCH] Make various variables read-only (const)
On Sun, Dec 22, 2013 at 09:43:57PM -0500, Robert Haas wrote:
> On Fri, Dec 20, 2013 at 12:01 PM, Oskari Saarenmaa wrote:
> > This allows the variables to be moved from .data to .rodata section which
> > means that more data can be shared by processes and makes sure that nothing
> > can accidentally overwrite the read-only definitions. On a x86-64 Linux
> > system this moves roughly 9 kilobytes of previously writable data to the
> > read-only data segment in the backend and 4 kilobytes in libpq.
> >
> > https://github.com/saaros/postgres/compare/constify
> >
> > 24 files changed, 108 insertions(+), 137 deletions(-)
>
> This sounds like a broadly good thing, but I've had enough painful
> experiences with const to be a little wary. And how much does this
> really affect data sharing? Doesn't copy-on-write do the same thing
> for writable data? Could we get most of the benefit by const-ifying
> one or two large data structures and forget the rest?
Thanks for the review and sorry for the late reply, I was offline for a
while.
As Wim Lewis pointed out in his mail the const data is most likely
mixed with non-const data and copy-on-write won't help with all of it.
Also, some of the const data includes duplicates and thus .data actually
shrinks more than .rodata grows.
We'd probably get most of the space-saving benefits by just constifying the
biggest variables, but I think applying const to more things will also make
things more correct.
> Other comments:
>
> - The first hunk of the patch mutilates the comment it modifies for no
> apparent reason. Please revert.
>
> - Why change the API of transformRelOptions()?
The comment was changed to reflect the new API, I modified
transformRelOptions to only accept a single valid namespace to make things
simpler in the calling code. Nothing used more than one valid namespace
anyway, and it allows us to just use a constant "toast" without having to
create a 2 char* array with a NULL.
> -#define DEF_ENC2NAME(name, codepage) { #name, PG_##name }
> +/* The extra NUL-terminator will make sure a warning is raised if the
> + * storage space for name is too small, otherwise when strlen(name) ==
> + * sizeof(pg_enc2name.name) the NUL-terminator would be silently dropped.
> + */
> +#define DEF_ENC2NAME(name, codepage) { #name "\0", PG_##name }
>
> - The above hunk is not related to the primary purpose of this patch.
It sort-of is. Without fixed size char-arrays it's not possible to move
everything to .rodata, but fixed size char-arrays come with the drawback of
silently dropping the NUL-terminator when strlen(str) == sizeof(array), by
forcing a NUL-terminator in we always get a warning if it would've been
dropped and the size of the array can then be increased.
Thanks,
Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Make various variables read-only (const)
This allows the variables to be moved from .data to .rodata section which
means that more data can be shared by processes and makes sure that nothing
can accidentally overwrite the read-only definitions. On a x86-64 Linux
system this moves roughly 9 kilobytes of previously writable data to the
read-only data segment in the backend and 4 kilobytes in libpq.
https://github.com/saaros/postgres/compare/constify
24 files changed, 108 insertions(+), 137 deletions(-)
/ Oskari
>From 89f0348747b356eaccf5edc2f85bf47d0a35c4f4 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Fri, 20 Dec 2013 18:38:10 +0200
Subject: [PATCH] Make various variables read-only (const)
This allows the variables to be moved from .data to .rodata section which
means that more data can be shared by processes and makes sure that nothing
can accidentally overwrite the read-only definitions. On a x86-64 Linux
system this moves roughly 9 kilobytes of previously writable data to the
read-only data segment in the backend and 4 kilobytes in libpq.
---
src/backend/access/common/reloptions.c | 31
src/backend/catalog/objectaddress.c| 4 +-
src/backend/commands/conversioncmds.c | 2 +-
src/backend/commands/createas.c| 3 +-
src/backend/commands/tablecmds.c | 8 ++--
src/backend/optimizer/path/costsize.c | 2 +-
src/backend/regex/regc_lex.c | 4 +-
src/backend/regex/regcomp.c| 2 +-
src/backend/regex/regerror.c | 6 +--
src/backend/tcop/utility.c | 3 +-
src/backend/tsearch/wparser_def.c | 4 +-
src/backend/utils/adt/datetime.c | 6 +--
src/backend/utils/adt/formatting.c | 67 +++---
src/backend/utils/adt/tsrank.c | 16
src/backend/utils/mb/encnames.c| 31
src/backend/utils/mb/mbutils.c | 8 ++--
src/backend/utils/mb/wchar.c | 2 +-
src/common/relpath.c | 8 ++--
src/include/access/reloptions.h| 5 +--
src/include/common/relpath.h | 3 +-
src/include/mb/pg_wchar.h | 18 -
src/include/optimizer/cost.h | 2 +-
src/include/utils/datetime.h | 2 +
src/port/chklocale.c | 8 ++--
24 files changed, 108 insertions(+), 137 deletions(-)
diff --git a/src/backend/access/common/reloptions.c
b/src/backend/access/common/reloptions.c
index 31941e9..5ec617b 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -564,18 +564,17 @@ add_string_reloption(bits32 kinds, char *name, char
*desc, char *default_val,
* If ignoreOids is true, then we should ignore any occurrence of "oids"
* in the list (it will be or has been handled by interpretOidsOption()).
*
- * Note that this is not responsible for determining whether the options
- * are valid, but it does check that namespaces for all the options given are
- * listed in validnsps. The NULL namespace is always valid and need not be
- * explicitly listed. Passing a NULL pointer means that only the NULL
- * namespace is valid.
+ * Note that this is not responsible for determining whether the options are
+ * valid, but it does check that namespaces for all the options given
+ * matches validnsp. The NULL namespace is always valid. Passing a NULL
+ * valinsp means that only the NULL namespace is valid.
*
* Both oldOptions and the result are text arrays (or NULL for "default"),
* but we declare them as Datums to avoid including array.h in reloptions.h.
*/
Datum
-transformRelOptions(Datum oldOptions, List *defList, char *namspace,
- char *validnsps[], bool ignoreOids,
bool isReset)
+transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
+ const char *validnsp, bool ignoreOids,
bool isReset)
{
Datum result;
ArrayBuildState *astate;
@@ -667,23 +666,7 @@ transformRelOptions(Datum oldOptions, List *defList, char
*namspace,
*/
if (def->defnamespace != NULL)
{
- boolvalid = false;
- int i;
-
- if (validnsps)
- {
- for (i = 0; validnsps[i]; i++)
- {
- if
(pg_strcasecmp(def->defnamespace,
-
validnsps[i]) == 0)
- {
- valid = true;
-
Re: [HACKERS] Minor patch for the uuid-ossp extension
23.11.2013 14:12, Mario Weilguni kirjoitti: Am 22.11.2013 16:15, schrieb Tom Lane: [ memo to self: never, ever accept another contrib module whose name isn't a plain SQL identifier ] Well, in that case and since this is a rarely used extension (I guess so), maybe it would be the best to simply rename that extension to uuidossp (or whatever) and don't make any special treatment for it? There are a couple of threads about issues with uuid-ossp (AIUI it's abandonware at this point). If PostgreSQL had a proper PRNG with a 128-bit state it could just implement uuid_generate_v4() function in core and most people could probably drop uuid-ossp. I have a branch[1] which implements uuid_generate_v4 in core using pg_lrand48, but since it only has 48 bits of state it's probably not an acceptable replacement for uuid-ossp for now. Is anyone working on a new PRNG for PostgreSQL at the moment? / Oskari [1] https://github.com/saaros/postgres/compare/core-uuid-v4 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Autoconf 2.69 update
20.11.2013 23:38, Robert Haas kirjoitti: On Wed, Nov 20, 2013 at 4:31 AM, Oskari Saarenmaa wrote: ISTM autoconf has been better with backwards compatibility lately. Maybe the fatal error could be changed to a warning and/or the check for version == 2.63 be replaced with a check for version >= 2.63? Without a strict requirement for a certain autoconf version it would make sense to also drop the built autoconf artifacts from the git repository which would make diffs shorter and easier to review when touching configure.in. -1 from me. Dropping configure from the repository would significantly increase the burden to compile and install PostgreSQL from source. Not everyone has autoconf installed. I think it's reasonable to assume that people who build from git have autoconf. The release tarballs should still contain the generated configure, etc; I believe this is how a lot of (most?) open source projects handle autoconf artifacts. -1 also for loosening the version check. I guarantee that if we do that, people will use varying versions when regenerating configure, and we'll have a mess. Even if we standardize the version committers are supposed to use, someone will foul it up at least twice a year. The last thing I need is to have more things that I can accidentally screw up while committing; the list is too long already. I realize that those checks are a bit of a nuisance, but if they bother you you can just whack them out locally and proceed. Or else you can download and compile the right version of autoconf. If you're doing sufficiently serious development that you need to touch configure.in, the 5 minutes it takes you to build a local install of the right autoconf version should be the least of your concerns. It's not hard; I've done it multiple times, and have multiple versions of autoconf installed on those systems where I need to be able to re-run autoconf on any supported branch. As long as the released tarballs contain generated scripts I don't really see this being an issue. While changes to configure.in are pretty rare, they do happen and when you have to modify configure the resulting 'git diff', 'git status' etc become unnecessarily large and require you to hand-edit the patches before sending them to the mailing list, etc. One more option would be to include the built configure in release branches as there shouldn't really be many changes to configure.in after branching and it'd make sure that all build farm builders test the same script generated by a known version. Anyway, I won't mind the strict requirement for autoconf that much if it's for a more recent version than 2.63 :-) / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Autoconf 2.69 update
15.11.2013 05:00, Peter Eisentraut kirjoitti: I'm proposing that we upgrade our Autoconf to 2.69, which is the latest right now (release date 2012-04-24). There are no changes in the source needed, just tweak the version number in configure.in (see below) and run autoreconf. I've compared the configure output before and after on a few boxes, and there were no significant changes. +1. Autoconf 2.63 doesn't seem to be available as a package on recent Linux distributions and would make things easier for me. -m4_if(m4_defn([m4_PACKAGE_VERSION]), [2.63], [], [m4_fatal([Autoconf version 2.63 is required. +m4_if(m4_defn([m4_PACKAGE_VERSION]), [2.69], [], [m4_fatal([Autoconf version 2.69 is required. Untested combinations of 'autoconf' and PostgreSQL versions are not recommended. You can remove the check from 'configure.in' but it is then your responsibility whether the result works or not.])]) ISTM autoconf has been better with backwards compatibility lately. Maybe the fatal error could be changed to a warning and/or the check for version == 2.63 be replaced with a check for version >= 2.63? Without a strict requirement for a certain autoconf version it would make sense to also drop the built autoconf artifacts from the git repository which would make diffs shorter and easier to review when touching configure.in. That said, it looks like autoconf 2.67 (from Debian 6) can't handle = in a cflags test, so maybe not.. / Oskari *** # Generated by GNU Autoconf 2.67 for PostgreSQL 9.4devel. ... checking whether gcc supports -fexcess-precision=standard... ./configure: line 4528: pgac_cv_prog_cc_cflags__fexcess_precision_standard=no: command not found -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] configure: allow adding a custom string to PG_VERSION
On Mon, Nov 18, 2013 at 08:48:13PM -0500, Peter Eisentraut wrote: > On Tue, 2013-11-05 at 18:29 +0200, Oskari Saarenmaa wrote: > > This can be used to tag custom built packages with an extra version string > > such as the git describe id or distribution package release version. > > I think this is a reasonable feature, and the implementation is OK, but > I don't see why the format of the extra version information needs to be > exactly > > PG_VERSION="$PACKAGE_VERSION ($withval)" > > I'd imagine, for example, that someone will want to do -something or > +something. So I'd just make this > > PG_VERSION="$PACKAGE_VERSION$withval" > > Comments? Sounds reasonable. > > +# Allow adding a custom string to PG_VERSION > > +PGAC_ARG_REQ(with, extra-version, [NAME], [extra version information], > > +[PG_VERSION="$PACKAGE_VERSION ($withval)"], > > [PG_VERSION="$PACKAGE_VERSION"]) > > This could be indented better. It was a bit confusing at first. Agreed. Attached an updated patch, or you can grab it from https://github.com/saaros/postgres/compare/extra-version Thanks, Oskari >From 00ca6c31db06edee0a6b5b5417eac71c274d7876 Mon Sep 17 00:00:00 2001 From: Oskari Saarenmaa Date: Wed, 20 Nov 2013 01:01:58 +0200 Subject: [PATCH] configure: allow adding a custom string to PG_VERSION This can be used to tag custom built packages with an extra version string such as the git describe id or distribution package release version. Signed-off-by: Oskari Saarenmaa --- configure.in | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index 304399e..1f2f135 100644 --- a/configure.in +++ b/configure.in @@ -29,11 +29,16 @@ AC_CONFIG_AUX_DIR(config) AC_PREFIX_DEFAULT(/usr/local/pgsql) AC_SUBST(configure_args, [$ac_configure_args]) -AC_DEFINE_UNQUOTED(PG_VERSION, "$PACKAGE_VERSION", [PostgreSQL version as a string]) [PG_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] AC_SUBST(PG_MAJORVERSION) AC_DEFINE_UNQUOTED(PG_MAJORVERSION, "$PG_MAJORVERSION", [PostgreSQL major version as a string]) +# Allow adding a custom string to PG_VERSION +PGAC_ARG_REQ(with, extra-version, [NAME], [extra version information], + [PG_VERSION="$PACKAGE_VERSION$withval"], + [PG_VERSION="$PACKAGE_VERSION"]) +AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string]) + AC_CANONICAL_HOST template= @@ -1920,7 +1925,7 @@ else fi AC_DEFINE_UNQUOTED(PG_VERSION_STR, - ["PostgreSQL $PACKAGE_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], + ["PostgreSQL $PG_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], [A string containing the version number, platform, and C compiler]) # Supply a numeric version string for use by 3rd party add-ons -- 1.8.4.2 -- Sent 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_fallocate
On Thu, Oct 31, 2013 at 01:16:44PM +, Mitsumasa KONDO wrote:
> --- a/src/backend/storage/file/fd.c
> +++ b/src/backend/storage/file/fd.c
> @@ -383,6 +383,21 @@ pg_flush_data(int fd, off_t offset, off_t amount)
> return 0;
> }
>
> +/*
> + * pg_fallocate --- advise OS that the data pre-allocate continus file
> segments
> + * in physical disk.
> + *
> + * Not all platforms have fallocate. Some platforms only have
> posix_fallocate,
> + * but it ped zero fill to get pre-allocate file segmnets. It is not good
> + * peformance when extend new segmnets, so we don't use posix_fallocate.
> + */
> +int
> +pg_fallocate(File file, int flags, off_t offset, off_t nbytes)
> +{
> +#if defined(HAVE_FALLOCATE)
> + return fallocate(VfdCache[file].fd, flags, offset, nbytes);
> +#endif
> +}
You should set errno to ENOSYS and return -1 if HAVE_FALLOCATE isn't
defined.
> --- a/src/backend/storage/smgr/md.c
> +++ b/src/backend/storage/smgr/md.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
This would have to be wrapped in #ifdef HAVE_FALLOCATE or
HAVE_LINUX_FALLOC_H; if you want to create a wrapper around fallocate() you
should add PG defines for the flags, too. Otherwise it's probably easier to
just call fallocate() directly inside an #ifdef block as you did in xlog.c.
> @@ -510,6 +511,10 @@ mdextend(SMgrRelation reln, ForkNumber forknum,
> BlockNumber blocknum,
>* if bufmgr.c had to dump another buffer of the same file to make room
>* for the new page's buffer.
>*/
> +
> + if(forknum == 1)
> + pg_fallocate(v->mdfd_vfd, FALLOC_FL_KEEP_SIZE, 0, RELSEG_SIZE);
> +
Return value should be checked; if it's -1 and errno is something else than
ENOSYS or EOPNOTSUPP the disk space allocation failed and you must return an
error.
/ Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] configure: allow adding a custom string to PG_VERSION
This can be used to tag custom built packages with an extra version string such as the git describe id or distribution package release version. Based on pgsql-hackers discussion: http://www.postgresql.org/message-id/20131105102226.ga26...@saarenmaa.fi Signed-off-by: Oskari Saarenmaa --- configure.in | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index a4baeaf..5459c71 100644 --- a/configure.in +++ b/configure.in @@ -29,11 +29,15 @@ AC_CONFIG_AUX_DIR(config) AC_PREFIX_DEFAULT(/usr/local/pgsql) AC_SUBST(configure_args, [$ac_configure_args]) -AC_DEFINE_UNQUOTED(PG_VERSION, "$PACKAGE_VERSION", [PostgreSQL version as a string]) [PG_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] AC_SUBST(PG_MAJORVERSION) AC_DEFINE_UNQUOTED(PG_MAJORVERSION, "$PG_MAJORVERSION", [PostgreSQL major version as a string]) +# Allow adding a custom string to PG_VERSION +PGAC_ARG_REQ(with, extra-version, [NAME], [extra version information], +[PG_VERSION="$PACKAGE_VERSION ($withval)"], [PG_VERSION="$PACKAGE_VERSION"]) +AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string]) + AC_CANONICAL_HOST template= @@ -1920,7 +1924,7 @@ else fi AC_DEFINE_UNQUOTED(PG_VERSION_STR, - ["PostgreSQL $PACKAGE_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], + ["PostgreSQL $PG_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"], [A string containing the version number, platform, and C compiler]) # Supply a numeric version string for use by 3rd party add-ons -- 1.8.4.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] configure: add git describe output to PG_VERSION when building a git tree
On Tue, Nov 05, 2013 at 02:06:26PM +0200, Heikki Linnakangas wrote: > On 05.11.2013 12:22, Oskari Saarenmaa wrote: > >This makes it easy to see if the binaries were built from a real release > >or if they were built from a custom git tree. > > Hmm, that would mean that a build from "git checkout REL9_3_1" > produces a different binary than one built from > postgresql-9.3.1.tar.gz tarball. Good point - how about adding git describe information only if the checkout doesn't match a tag exactly? So when you build REL9_3_1 there would be no extra information, but when you have any local changes on top of it we'd add the git describe output. > I can see some value in that kind of information, ie. knowing what > patches a binary was built with, but this would only solve the > problem for git checkouts. Even for a git checkout, the binaries > won't be automatically updated unless you run "configure" again, > which makes it pretty unreliable. > > -1 from me. I don't think we can solve the problem of finding local changes for all the things people may do, but I'd guess it's pretty common to build postgresql from a clean local git checkout and with this change at least some portion of users would get some extra information. To solve the "rerun configure" thing we could put git version in a new header file and have a makefile dependency on .git/index for regenerating that file when needed. We could also let users override the extra version with a command line argument for configure so distributions could put the distribution package version there, for example "9.3.1-2.fc20" on my Fedora system. > PS, the git command in the patch doesn't work with my version of git: > > $ git describe --tags --long --dirty HEAD > fatal: --dirty is incompatible with committishes > $ git --version > git version 1.8.4.rc3 I initially used HEAD when looking at the git describe values, but then added --dirty which obviously doesn't make sense when describing a ref. Sorry about the broken patch, I was applying these changes manually from configure.in to configure because I didn't have the proper autoconf version installed (autoconf 2.63 doesn't seem to be available in many distributions anymore, maybe the required version could be upgraded at some point..) How about the patch below to fix the exact tag and --dirty issues? / Oskari diff --git a/configure.in b/configure.in index a4baeaf..d253286 100644 --- a/configure.in +++ b/configure.in @@ -29,7 +29,18 @@ AC_CONFIG_AUX_DIR(config) AC_PREFIX_DEFAULT(/usr/local/pgsql) AC_SUBST(configure_args, [$ac_configure_args]) -AC_DEFINE_UNQUOTED(PG_VERSION, "$PACKAGE_VERSION", [PostgreSQL version as a string]) +# Append git tag based version to PG_VERSION if we're building from a git +# checkout that doesn't match a tag exactly. Don't touch PACKAGE_VERSION +# which is used to create other version variables (major version and numeric +# version.) +PG_VERSION="$PACKAGE_VERSION" +if test -d .git; then + exact="`git describe --tags --exact-match --dirty 2>/dev/null || echo dirty`" + if echo "$exact" | grep -q dirty; then +PG_VERSION="$PG_VERSION (`git describe --tags --long --dirty || echo unknown`)" + fi +fi +AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string]) [PG_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] AC_SUBST(PG_MAJORVERSION) AC_DEFINE_UNQUOTED(PG_MAJORVERSION, "$PG_MAJORVERSION", [PostgreSQL major version as a string]) -- 1.8.4.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] configure: add git describe output to PG_VERSION when building a git tree
This makes it easy to see if the binaries were built from a real release or if they were built from a custom git tree. --- configure.in | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/configure.in b/configure.in index a4baeaf..7c5b3ce 100644 --- a/configure.in +++ b/configure.in @@ -29,7 +29,14 @@ AC_CONFIG_AUX_DIR(config) AC_PREFIX_DEFAULT(/usr/local/pgsql) AC_SUBST(configure_args, [$ac_configure_args]) -AC_DEFINE_UNQUOTED(PG_VERSION, "$PACKAGE_VERSION", [PostgreSQL version as a string]) +# Append git tag based version to PG_VERSION if we're building from a git +# checkout, but don't touch PACKAGE_VERSION which is used to create other +# version variables (major version and numeric version.) +PG_VERSION="$PACKAGE_VERSION" +if test -d .git; then + PG_VERSION="$PG_VERSION (`git describe --tags --long --dirty HEAD || echo unknown`)" +fi +AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string]) [PG_MAJORVERSION=`expr "$PACKAGE_VERSION" : '\([0-9][0-9]*\.[0-9][0-9]*\)'`] AC_SUBST(PG_MAJORVERSION) AC_DEFINE_UNQUOTED(PG_MAJORVERSION, "$PG_MAJORVERSION", [PostgreSQL major version as a string]) -- 1.8.4.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] hstore_to_json: empty hstores must return empty json objects
On 17/10/13 17:23, Alvaro Herrera wrote:
Oskari Saarenmaa wrote:
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
I'm not too familiar with PG internals and just modified this to work
like it did before, but it looks like the extra allocation is indeed
unnecessary.
I can post a new patch to make this use cstring_to_text_with_len("{}",
2) (to avoid an unnecessary strlen call) or you can just make the change
and push the modified version.
Thanks,
Oskari
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] hstore_to_json: empty hstores must return empty json objects
hstore_to_json used to return an empty string for empty hstores, but
that is not correct as an empty string is not valid json and calling
row_to_json() on rows which have empty hstores will generate invalid
json for the entire row. The right thing to do is to return an empty
json object.
Signed-off-by: Oskari Saarenmaa
---
This should probably be applied to 9.3 tree as well as master.
# select row_to_json(r.*) from (select ''::hstore as d) r;
{"d":}
# select hstore_to_json('')::text::json;
ERROR: invalid input syntax for type json
contrib/hstore/expected/hstore.out | 12
contrib/hstore/hstore_io.c | 8
contrib/hstore/sql/hstore.sql | 2 ++
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/contrib/hstore/expected/hstore.out
b/contrib/hstore/expected/hstore.out
index 2114143..3280b5e 100644
--- a/contrib/hstore/expected/hstore.out
+++ b/contrib/hstore/expected/hstore.out
@@ -1472,6 +1472,18 @@ select hstore_to_json_loose('"a key" =>1, b => t, c =>
null, d=> 12345, e => 012
{"b": true, "c": null, "d": 12345, "e": "012345", "f": 1.234, "g": 2.345e+4,
"a key": 1}
(1 row)
+select hstore_to_json('');
+ hstore_to_json
+
+ {}
+(1 row)
+
+select hstore_to_json_loose('');
+ hstore_to_json_loose
+--
+ {}
+(1 row)
+
create table test_json_agg (f1 text, f2 hstore);
insert into test_json_agg values ('rec1','"a key" =>1, b => t, c => null, d=>
12345, e => 012345, f=> 1.234, g=> 2.345e+4'),
('rec2','"a key" =>2, b => f, c => "null", d=> -12345, e => 012345.6,
f=> -1.234, g=> 0.345e-4');
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index d3e67dd..3781a71 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
@@ -1370,8 +1370,8 @@ hstore_to_json(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
diff --git a/contrib/hstore/sql/hstore.sql b/contrib/hstore/sql/hstore.sql
index 68b74bf..47cbfad 100644
--- a/contrib/hstore/sql/hstore.sql
+++ b/contrib/hstore/sql/hstore.sql
@@ -335,6 +335,8 @@ select count(*) from testhstore where h = 'pos=>98,
line=>371, node=>CBA, indexe
select hstore_to_json('"a key" =>1, b => t, c => null, d=> 12345, e => 012345,
f=> 1.234, g=> 2.345e+4');
select cast( hstore '"a key" =>1, b => t, c => null, d=> 12345, e => 012345,
f=> 1.234, g=> 2.345e+4' as json);
select hstore_to_json_loose('"a key" =>1, b => t, c => null, d=> 12345, e =>
012345, f=> 1.234, g=> 2.345e+4');
+select hstore_to_json('');
+select hstore_to_json_loose('');
create table test_json_agg (f1 text, f2 hstore);
insert into test_json_agg values ('rec1','"a key" =>1, b => t, c => null, d=>
12345, e => 012345, f=> 1.234, g=> 2.345e+4'),
--
1.8.3.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_upgrade: support for btrfs copy-on-write clones
05.10.2013 16:38, Bruce Momjian kirjoitti: > On Fri, Oct 4, 2013 at 10:42:46PM +0300, Oskari Saarenmaa wrote: >> Thanks for the offers, but it looks like ZFS doesn't actually implement >> a similar file level clone operation. See >> https://github.com/zfsonlinux/zfs/issues/405 for discussion on a feature >> request for it. >> >> ZFS does support cloning entire datasets which seem to be similar to >> btrfs subvolume snapshots and could be used to set up a new data >> directory for a new $PGDATA. This would require the original $PGDATA >> to be a dataset/subvolume of its own and quite a bit different logic >> (than just another file copy method in pg_upgrade) to initialize the new >> version's $PGDATA as a snapshot/clone of the original. The way this >> would work is that the original $PGDATA dataset/subvolume gets cloned to >> a new location after which we move the files out of the way of the new >> PG installation and run pg_upgrade in link mode. I'm not sure if >> there's a good way to integrate this into pg_upgrade or if it's just >> something that could be documented as a fast way to run pg_upgrade >> without touching original files. >> >> With btrfs tooling the sequence would be something like this: >> >> btrfs subvolume snapshot /srv/pg92 /srv/pg93 >> mv /srv/pg93/data /srv/pg93/data92 >> initdb /data/pg93/data >> pg_upgrade --link \ >> --old-datadir=/data/pg93/data92 \ >> --new-datadir=/data/pg93/data > > Does btrfs support file system snapshots? If so, shouldn't people just > take a snapshot of the old data directory before the ugprade, rather > than using cloning? Yeah, it's possible to clone an existing subvolume, but this requires that $PGDATA is a subvolume of its own and would be a bit difficult to integrate into existing pg_upgrade scripts. The BTRFS_IOC_CLONE ioctl operates on file level and can be used to clone files anywhere in a btrfs filesystem. / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_upgrade: support for btrfs copy-on-write clones
03.10.2013 01:35, Larry Rosenman kirjoitti: > On 2013-10-02 17:32, Josh Berkus wrote: >>> No fundamental reason; I'm hoping ZFS will be supported in addition to >>> btrfs, but I don't have any systems with ZFS filesystems at the moment >>> so I haven't been able to test it or find out the mechanisms ZFS uses >>> for cloning. On btrfs cloning is implemented with a custom >>> btrfs-specific ioctl, ZFS probably has something similar which would be >>> pretty easy to add on top of this patch. >> >> Would you like a VM with ZFS on it? I'm pretty sure I can supply one. >> > I can also supply SSH access to a FreeBSD 10 system that is totally ZFS. Thanks for the offers, but it looks like ZFS doesn't actually implement a similar file level clone operation. See https://github.com/zfsonlinux/zfs/issues/405 for discussion on a feature request for it. ZFS does support cloning entire datasets which seem to be similar to btrfs subvolume snapshots and could be used to set up a new data directory for a new $PGDATA. This would require the original $PGDATA to be a dataset/subvolume of its own and quite a bit different logic (than just another file copy method in pg_upgrade) to initialize the new version's $PGDATA as a snapshot/clone of the original. The way this would work is that the original $PGDATA dataset/subvolume gets cloned to a new location after which we move the files out of the way of the new PG installation and run pg_upgrade in link mode. I'm not sure if there's a good way to integrate this into pg_upgrade or if it's just something that could be documented as a fast way to run pg_upgrade without touching original files. With btrfs tooling the sequence would be something like this: btrfs subvolume snapshot /srv/pg92 /srv/pg93 mv /srv/pg93/data /srv/pg93/data92 initdb /data/pg93/data pg_upgrade --link \ --old-datadir=/data/pg93/data92 \ --new-datadir=/data/pg93/data / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_upgrade: support for btrfs copy-on-write clones
On 02/10/13 17:18, Andrew Dunstan wrote: On 10/01/2013 06:31 PM, Oskari Saarenmaa wrote: Add file cloning as an alternative data transfer method to pg_upgrade. Currently only btrfs is supported, but copy-on-write cloning is also available on at least ZFS. Cloning must be requested explicitly and if it isn't supported by the operating system or filesystem a fatal error is thrown. So, just curious, why isn't ZFS supported? It's what I am more interested in, at least. No fundamental reason; I'm hoping ZFS will be supported in addition to btrfs, but I don't have any systems with ZFS filesystems at the moment so I haven't been able to test it or find out the mechanisms ZFS uses for cloning. On btrfs cloning is implemented with a custom btrfs-specific ioctl, ZFS probably has something similar which would be pretty easy to add on top of this patch. Added this patch to commitfest as suggested, https://commitfest.postgresql.org/action/patch_view?id=1251 / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] pg_upgrade: support for btrfs copy-on-write clones
Add file cloning as an alternative data transfer method to pg_upgrade.
Currently only btrfs is supported, but copy-on-write cloning is also
available on at least ZFS. Cloning must be requested explicitly and if
it isn't supported by the operating system or filesystem a fatal error
is thrown.
This provides upgrade performance similar to link mode while allowing
the old cluster to be used even after the new one has been started.
Signed-off-by: Oskari Saarenmaa
---
configure| 5 +-
configure.in | 7 ++-
contrib/pg_upgrade/check.c | 3 +
contrib/pg_upgrade/file.c| 125 +--
contrib/pg_upgrade/option.c | 7 +++
contrib/pg_upgrade/pg_upgrade.h | 13 ++--
contrib/pg_upgrade/relfilenode.c | 31 --
doc/src/sgml/pgupgrade.sgml | 7 +++
src/include/pg_config.h.in | 3 +
9 files changed, 141 insertions(+), 60 deletions(-)
diff --git a/configure b/configure
index c685ca3..5087463 100755
--- a/configure
+++ b/configure
@@ -10351,7 +10351,10 @@ done
-for ac_header in crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h
langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h
sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h
sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
+for ac_header in crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h
langinfo.h \
+linux/btrfs.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h \
+sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h \
+sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
do
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
if { as_var=$as_ac_Header; eval "test \"\${$as_var+set}\" = set"; }; then
diff --git a/configure.in b/configure.in
index 82771bd..609aa73 100644
--- a/configure.in
+++ b/configure.in
@@ -982,7 +982,12 @@ AC_SUBST(OSSP_UUID_LIBS)
##
dnl sys/socket.h is required by AC_FUNC_ACCEPT_ARGTYPES
-AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h
langinfo.h poll.h pwd.h sys/ioctl.h sys/ipc.h sys/poll.h sys/pstat.h
sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/socket.h sys/sockio.h
sys/tas.h sys/time.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h])
+AC_CHECK_HEADERS([crypt.h dld.h fp_class.h getopt.h ieeefp.h ifaddrs.h \
+ langinfo.h linux/btrfs.h poll.h pwd.h sys/ioctl.h \
+ sys/ipc.h sys/poll.h sys/pstat.h sys/resource.h \
+ sys/select.h sys/sem.h sys/shm.h sys/socket.h \
+ sys/sockio.h sys/tas.h sys/time.h sys/un.h termios.h \
+ ucred.h utime.h wchar.h wctype.h])
# On BSD, test for net/if.h will fail unless sys/socket.h
# is included first.
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 0376fcb..2a52dd8 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -151,6 +151,9 @@ check_new_cluster(void)
if (user_opts.transfer_mode == TRANSFER_MODE_LINK)
check_hard_link();
+ if (user_opts.transfer_mode == TRANSFER_MODE_CLONE)
+ check_clone_file();
+
check_is_super_user(&new_cluster);
/*
diff --git a/contrib/pg_upgrade/file.c b/contrib/pg_upgrade/file.c
index dfeb79f..fc935b7 100644
--- a/contrib/pg_upgrade/file.c
+++ b/contrib/pg_upgrade/file.c
@@ -8,11 +8,16 @@
*/
#include "postgres_fe.h"
+#include "pg_config.h"
#include "pg_upgrade.h"
#include
+#ifdef HAVE_LINUX_BTRFS_H
+# include
+# include
+#endif
#ifndef WIN32
@@ -23,21 +28,42 @@ static int win32_pghardlink(const char *src, const char
*dst);
/*
- * copyAndUpdateFile()
+ * upgradeFile()
*
- * Copies a relation file from src to dst. If pageConverter is non-NULL,
this function
- * uses that pageConverter to do a page-by-page conversion.
+ * Transfer a relation file from src to dst using one of the supported
+ * methods. If the on-disk format of the new cluster is bit-for-bit
+ * compatible with the on-disk format of the old cluster we can simply link
+ * each relation to perform a true in-place upgrade. Otherwise we must copy
+ * (either block-by-block or using a copy-on-write clone) the data from old
+ * cluster to new cluster and then perform the conversion.
*/
const char *
-copyAndUpdateFile(pageCnvCtx *pageConverter,
- const char *src, const char *dst, bool force)
+upgradeFile(transferMode transfer_mode, const char *src,
+ const char *dst, pageCnvCtx *pageConverter)
{
if (pageConverter == NULL)
{
- if (pg_copy_file(src, dst, force) == -1)
- return getErrorText(errno);
- else
- return NULL;
+ int rc = -1;
+
+
[HACKERS] [PATCH] PL/Python: Add spidata to all spiexceptions
PL/Python maps Python SPIError exceptions with 'spidata' attribute into SQL
errors. PL/Python also creates classes in plpy.spiexceptions for all known
errors but does not initialize their spidata, so when a PL/Python function
raises such an exception it is not recognized properly and is always
reported as an internal error.
This allows PL/Python code to raise exceptions that PL/pgSQL can catch and
which are correctly reported in logs instead of always showing up as XX000.
---
src/pl/plpython/expected/plpython_error.out | 12
src/pl/plpython/expected/plpython_error_0.out | 12
src/pl/plpython/plpy_plpymodule.c | 9 +
src/pl/plpython/sql/plpython_error.sql| 14 ++
4 files changed, 47 insertions(+)
diff --git a/src/pl/plpython/expected/plpython_error.out
b/src/pl/plpython/expected/plpython_error.out
index e1ec9c2..c1c36d9 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -400,3 +400,15 @@ CONTEXT: Traceback (most recent call last):
PL/Python function "manual_subxact_prepared", line 4, in
plpy.execute(save)
PL/Python function "manual_subxact_prepared"
+/* raising plpy.spiexception.* from python code should preserve sqlstate
+ */
+CREATE FUNCTION plpy_raise_spiexception() RETURNS void AS $$
+raise plpy.spiexceptions.DivisionByZero()
+$$ LANGUAGE plpythonu;
+DO $$
+BEGIN
+ SELECT plpy_raise_spiexception();
+EXCEPTION WHEN division_by_zero THEN
+ -- NOOP
+END
+$$ LANGUAGE plpgsql;
diff --git a/src/pl/plpython/expected/plpython_error_0.out
b/src/pl/plpython/expected/plpython_error_0.out
index 6f74a50..61d95e3 100644
--- a/src/pl/plpython/expected/plpython_error_0.out
+++ b/src/pl/plpython/expected/plpython_error_0.out
@@ -400,3 +400,15 @@ CONTEXT: Traceback (most recent call last):
PL/Python function "manual_subxact_prepared", line 4, in
plpy.execute(save)
PL/Python function "manual_subxact_prepared"
+/* raising plpy.spiexception.* from python code should preserve sqlstate
+ */
+CREATE FUNCTION plpy_raise_spiexception() RETURNS void AS $$
+raise plpy.spiexceptions.DivisionByZero()
+$$ LANGUAGE plpythonu;
+DO $$
+BEGIN
+ SELECT plpy_raise_spiexception();
+EXCEPTION WHEN division_by_zero THEN
+ -- NOOP
+END
+$$ LANGUAGE plpgsql;
diff --git a/src/pl/plpython/plpy_plpymodule.c
b/src/pl/plpython/plpy_plpymodule.c
index 37ea2a4..4213e83 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -247,6 +247,7 @@ PLy_generate_spi_exceptions(PyObject *mod, PyObject *base)
PyObject *exc;
PLyExceptionEntry *entry;
PyObject *sqlstate;
+ PyObject *spidata;
PyObject *dict = PyDict_New();
if (dict == NULL)
@@ -258,6 +259,14 @@ PLy_generate_spi_exceptions(PyObject *mod, PyObject *base)
PyDict_SetItemString(dict, "sqlstate", sqlstate);
Py_DECREF(sqlstate);
+
+ spidata = Py_BuildValue("izzzi", exception_map[i].sqlstate,
+ NULL, NULL,
NULL, 0);
+ if (spidata == NULL)
+ PLy_elog(ERROR, "could not generate SPI exceptions");
+ PyDict_SetItemString(dict, "spidata", spidata);
+ Py_DECREF(spidata);
+
exc = PyErr_NewException(exception_map[i].name, base, dict);
PyModule_AddObject(mod, exception_map[i].classname, exc);
entry = hash_search(PLy_spi_exceptions,
&exception_map[i].sqlstate,
diff --git a/src/pl/plpython/sql/plpython_error.sql
b/src/pl/plpython/sql/plpython_error.sql
index 502bbec..ec93144 100644
--- a/src/pl/plpython/sql/plpython_error.sql
+++ b/src/pl/plpython/sql/plpython_error.sql
@@ -298,3 +298,17 @@ plpy.execute(rollback)
$$ LANGUAGE plpythonu;
SELECT manual_subxact_prepared();
+
+/* raising plpy.spiexception.* from python code should preserve sqlstate
+ */
+CREATE FUNCTION plpy_raise_spiexception() RETURNS void AS $$
+raise plpy.spiexceptions.DivisionByZero()
+$$ LANGUAGE plpythonu;
+
+DO $$
+BEGIN
+ SELECT plpy_raise_spiexception();
+EXCEPTION WHEN division_by_zero THEN
+ -- NOOP
+END
+$$ LANGUAGE plpgsql;
--
1.7.12.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
