[HACKERS] [PATCH] Regression tests in windows ignore white space
In the following thread I discovered that my new regression tests worked perfectly on windows, but when they were run on linux they failed. http://www.postgresql.org/message-id/CAApHDvo_YCiPYGDz07MpX9o6EGg=3mmyJTb0ysPTwoTg3c=t...@mail.gmail.com After looking at pg_regress to see which options it passes to diff I discovered that it passes -w on windows to ignore ALL white space. The attached simple patch changes this so that it only ignores carriage returns. It does this by passing --strip-trailing-cr to diff instead of -w. This should help us few developers who use windows to get our white space correct in out expected results so that the tests also pass on non windows platforms. Regards David Rowley pg_regress_win32_diff_options.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)
On Sun, Dec 15, 2013 at 2:27 PM, Josh Berkus j...@agliodbs.com wrote: On 12/14/2013 05:00 PM, Tom Lane wrote: This consideration also makes me question whether we should apply the method for NUMERIC. Although in principle numeric addition/subtraction is exact, such a sequence could leave us with a different dscale than is returned by the existing code. I'm not sure if changing the number of trailing zeroes is a big enough behavior change to draw complaints. If we're going to disqualify NUMERIC too, we might as well bounce the feature. Without a fast FLOAT or NUMERIC, you've lost most of the target audience. I think even the FLOAT case deserves some consideration. What's the worst-case drift? In general, folks who do aggregate operations on FLOATs aren't expecting an exact answer, or one which is consistent beyond a certain number of significant digits. And Dave is right: how many bug reports would we get about NUMERIC is fast, but FLOAT is slow? From what I can tell adding an inverse transition function to support AVG for numeric does not affect the number of trailing zeros in the results, so I've attached a patch which now has inverse transition functions to support numeric types in AVG and all of the STDDEV* aggregates. The function numeric_avg_accum_inv is the inverse transition function for AVG. In the attached patch I've set this to be the inverse transition function for SUM too. I know it was mentioned that having extra trailing zeros here is a change of results and could be unwanted, but I've left it in for now and I've included a failing regression test to demonstrate exactly what has changed in the hope that it may seed a discussion on what the best solution is. Here's a quick example of what I'm talking about: With this query I'll include a call to MAX to force the executor to not use inverse transitions. SELECT SUM(n::numeric) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING), MAX(n::numeric) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) FROM (VALUES(1,1.01),(2,2),(3,3)) v(i,n); sum| max --+- 6.01 | 3 5 | 3 3 | 3 (3 rows) Notice lack of trailing zeros for the sum results in rows 2 and 3. Here an inverse transition will be performed... Notice the extra trailing zeros on rows 2 and 3. SELECT SUM(n::numeric) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING) FROM (VALUES(1,1.01),(2,2),(3,3)) v(i,n); sum -- 6.01 5.00 3.00 (3 rows) My ideas so far on what to do about this: 1. Don't allow inverse transition for SUM with numeric, but since numeric_avg_accum_inv is already written for AVG numeric support we could include a note in the docs to tell users how to create a SUM aggregate for numeric that supports inverse transitions. 2. Don't worry about the trailing zeros... After all SELECT AVG(n) FROM (VALUES(2::numeric)) v(n); adds a whole bunch of trailing zeros. Only the above 2 queries shows that this behaviour can be a bit weird. 3. Invent some way to remove trailing zeros, perhaps in numeric_out. Is there a reason they are there? Note that this would affect numeric globally. What I don't currently know is: Are there any rules about trailing zeros on numeric? I see that AVG with a single numeric produces many trailing zeros after the decimal point. Are these meant to be there or is it just a side effect of dividing on numerics? Please note that I'm not trying to push for any of the above points. I just want to get the information I currently have out there as perhaps someone can think of something better or show a good reason for or against any of the 3 points. Regards David Rowley -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com inverse_transition_functions_v1.7.patch.gz Description: GNU Zip compressed data -- 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 memset usage in pgcrypto
http://www.viva64.com/en/b/0227/ reported that on-stack memset()s
might be optimized away by compilers. Fix it.
* Replace memset() with px_memset()
* Add px_memset to copy_crlf()
* ADd px_memset to pgp-s2k.c
--
marko
diff --git a/contrib/pgcrypto/crypt-blowfish.c b/contrib/pgcrypto/crypt-blowfish.c
index b49747d..fbaa3d7 100644
--- a/contrib/pgcrypto/crypt-blowfish.c
+++ b/contrib/pgcrypto/crypt-blowfish.c
@@ -35,6 +35,7 @@
#include postgres.h
#include px-crypt.h
+#include px.h
#ifdef __i386__
#define BF_ASM0 /* 1 */
@@ -616,7 +617,7 @@ _crypt_blowfish_rn(const char *key, const char *setting,
count = (BF_word) 1 ((setting[4] - '0') * 10 + (setting[5] - '0'));
if (count 16 || BF_decode(data.binary.salt, setting[7], 16))
{
- memset(data.binary.salt, 0, sizeof(data.binary.salt));
+ px_memset(data.binary.salt, 0, sizeof(data.binary.salt));
return NULL;
}
BF_swap(data.binary.salt, 4);
@@ -729,7 +730,7 @@ _crypt_blowfish_rn(const char *key, const char *setting,
/* Overwrite the most obvious sensitive data we have on the stack. Note
* that this does not guarantee there's no sensitive data left on the
* stack and/or in registers; I'm not aware of portable code that does. */
- memset(data, 0, sizeof(data));
+ px_memset(data, 0, sizeof(data));
return output;
}
diff --git a/contrib/pgcrypto/crypt-md5.c b/contrib/pgcrypto/crypt-md5.c
index 2a5cd70..6a09d76 100644
--- a/contrib/pgcrypto/crypt-md5.c
+++ b/contrib/pgcrypto/crypt-md5.c
@@ -89,7 +89,7 @@ px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen)
px_md_update(ctx, final, pl MD5_SIZE ? MD5_SIZE : pl);
/* Don't leave anything around in vm they could use. */
- memset(final, 0, sizeof final);
+ px_memset(final, 0, sizeof final);
/* Then something really weird... */
for (i = strlen(pw); i; i = 1)
@@ -154,7 +154,7 @@ px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen)
*p = '\0';
/* Don't leave anything around in vm they could use. */
- memset(final, 0, sizeof final);
+ px_memset(final, 0, sizeof final);
px_md_free(ctx1);
px_md_free(ctx);
diff --git a/contrib/pgcrypto/fortuna.c b/contrib/pgcrypto/fortuna.c
index 1228fb4..47380a8 100644
--- a/contrib/pgcrypto/fortuna.c
+++ b/contrib/pgcrypto/fortuna.c
@@ -34,6 +34,7 @@
#include sys/time.h
#include time.h
+#include px.h
#include rijndael.h
#include sha2.h
#include fortuna.h
@@ -169,7 +170,7 @@ md_result(MD_CTX * ctx, uint8 *dst)
memcpy(tmp, ctx, sizeof(*ctx));
SHA256_Final(dst, tmp);
- memset(tmp, 0, sizeof(tmp));
+ px_memset(tmp, 0, sizeof(tmp));
}
/*
@@ -243,7 +244,7 @@ enough_time_passed(FState *st)
if (ok)
memcpy(last, tv, sizeof(tv));
- memset(tv, 0, sizeof(tv));
+ px_memset(tv, 0, sizeof(tv));
return ok;
}
@@ -290,8 +291,8 @@ reseed(FState *st)
/* use new key */
ciph_init(st-ciph, st-key, BLOCK);
- memset(key_md, 0, sizeof(key_md));
- memset(buf, 0, BLOCK);
+ px_memset(key_md, 0, sizeof(key_md));
+ px_memset(buf, 0, BLOCK);
}
/*
@@ -341,8 +342,8 @@ add_entropy(FState *st, const uint8 *data, unsigned len)
if (pos == 0)
st-pool0_bytes += len;
- memset(hash, 0, BLOCK);
- memset(md, 0, sizeof(md));
+ px_memset(hash, 0, BLOCK);
+ px_memset(md, 0, sizeof(md));
}
/*
@@ -378,7 +379,7 @@ startup_tricks(FState *st)
encrypt_counter(st, buf + CIPH_BLOCK);
md_update(st-pool[i], buf, BLOCK);
}
- memset(buf, 0, BLOCK);
+ px_memset(buf, 0, BLOCK);
/* Hide the key. */
rekey(st);
diff --git a/contrib/pgcrypto/internal-sha2.c b/contrib/pgcrypto/internal-sha2.c
index f86b478..912effb 100644
--- a/contrib/pgcrypto/internal-sha2.c
+++ b/contrib/pgcrypto/internal-sha2.c
@@ -84,7 +84,7 @@ int_sha224_free(PX_MD *h)
{
SHA224_CTX *ctx = (SHA224_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
px_free(h);
}
@@ -132,7 +132,7 @@ int_sha256_free(PX_MD *h)
{
SHA256_CTX *ctx = (SHA256_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
px_free(h);
}
@@ -180,7 +180,7 @@ int_sha384_free(PX_MD *h)
{
SHA384_CTX *ctx = (SHA384_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
px_free(h);
}
@@ -228,7 +228,7 @@ int_sha512_free(PX_MD *h)
{
SHA512_CTX *ctx = (SHA512_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
px_free(h);
}
diff --git a/contrib/pgcrypto/internal.c b/contrib/pgcrypto/internal.c
index a02c943..7b33e49 100644
--- a/contrib/pgcrypto/internal.c
+++ b/contrib/pgcrypto/internal.c
@@ -142,7 +142,7 @@ int_md5_free(PX_MD *h)
{
MD5_CTX*ctx = (MD5_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
px_free(h);
}
@@ -190,7 +190,7 @@ int_sha1_free(PX_MD *h)
{
SHA1_CTX *ctx = (SHA1_CTX *) h-p.ptr;
- memset(ctx, 0, sizeof(*ctx));
+ px_memset(ctx, 0, sizeof(*ctx));
px_free(ctx);
Re: [HACKERS] CREATE TABLESPACE SET
On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: I was recently annoyed that I had to do CREATE TABLESPACE x LOCATION y; ALTER TABLESPACE x SET (random_page_cost = z); The attached patch is a quick n' dirty extension to allow the SET directly on the CREATE statement. CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); That should probably be WITH instead of SET for consistency with other similar DDL. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CREATE TABLESPACE SET
On 12/26/2013 06:10 PM, David Fetter wrote: On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: I was recently annoyed that I had to do CREATE TABLESPACE x LOCATION y; ALTER TABLESPACE x SET (random_page_cost = z); The attached patch is a quick n' dirty extension to allow the SET directly on the CREATE statement. CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); That should probably be WITH instead of SET for consistency with other similar DDL. It would make a simpler patch, too, but I wanted to be consistent with ALTER TABLESPACE. I have no firm opinion on the subject, so I'll switch it to WITH if no one else says anything. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #8676: Bug Money JSON
On 12/17/2013 11:16 AM, Andrew Dunstan wrote:
On 12/17/2013 10:31 AM, Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
On Wed, Dec 11, 2013 at 02:30:04PM +,
em...@andersonloyola.com.br wrote:
postgres=# SELECT to_json(a) FROM (VALUES(1000::money)) a(salario);
to_json
---
{salario:$1,000.00}
(1 row)
Yeah. I'll have a look. In fact this looks like it's possibly a couple
of bugs. The JSON produced by the first query is not valid. It looks
like we might need to force money to text unconditionally.
Isn't this simply failure to quote the string properly? What drives
to_json's choice of whether to quote or not, anyway?
If it's numeric, it only quotes if it sees a non-numeric character,
defined thus:
/* letters appearing in numeric output that aren't valid in a JSON
number */
#define NON_NUMERIC_LETTER NnAaIiFfTtYy
I forgot about money when I did that - some of this dates back to 9.2.
I'm about to test the attached patch which should force money to be
quoted always.
This turned out to be not such a good idea. Quite apart from anything
else it doesn't handle domains over money at all well.
The attached patch abandons the test described above, and instead passes
the string from the output function to the json number lexer to see if
it's a valid json number. A small adjustment to the API of that
function was required to make it suitable for this use. This seems like
a much more robust approach.
cheers
andrew
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 1486eda..af8ddc6 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -50,7 +50,7 @@ typedef enum /* contexts of JSON parser */
static inline void json_lex(JsonLexContext *lex);
static inline void json_lex_string(JsonLexContext *lex);
-static inline void json_lex_number(JsonLexContext *lex, char *s);
+static inline void json_lex_number(JsonLexContext *lex, char *s, bool *num_err);
static inline void parse_scalar(JsonLexContext *lex, JsonSemAction *sem);
static void parse_object_field(JsonLexContext *lex, JsonSemAction *sem);
static void parse_object(JsonLexContext *lex, JsonSemAction *sem);
@@ -147,8 +147,6 @@ lex_expect(JsonParseContext ctx, JsonLexContext *lex, JsonTokenType token)
#define TYPCATEGORY_JSON 'j'
/* fake category for types that have a cast to json */
#define TYPCATEGORY_JSON_CAST 'c'
-/* letters appearing in numeric output that aren't valid in a JSON number */
-#define NON_NUMERIC_LETTER NnAaIiFfTtYy
/* chars to consider as part of an alphanumeric token */
#define JSON_ALPHANUMERIC_CHAR(c) \
(((c) = 'a' (c) = 'z') || \
@@ -567,7 +565,7 @@ json_lex(JsonLexContext *lex)
break;
case '-':
/* Negative number. */
-json_lex_number(lex, s + 1);
+json_lex_number(lex, s + 1, NULL);
lex-token_type = JSON_TOKEN_NUMBER;
break;
case '0':
@@ -581,7 +579,7 @@ json_lex(JsonLexContext *lex)
case '8':
case '9':
/* Positive number. */
-json_lex_number(lex, s);
+json_lex_number(lex, s, NULL);
lex-token_type = JSON_TOKEN_NUMBER;
break;
default:
@@ -903,7 +901,7 @@ json_lex_string(JsonLexContext *lex)
*-
*/
static inline void
-json_lex_number(JsonLexContext *lex, char *s)
+json_lex_number(JsonLexContext *lex, char *s, bool *num_err)
{
bool error = false;
char *p;
@@ -976,10 +974,19 @@ json_lex_number(JsonLexContext *lex, char *s)
*/
for (p = s; len lex-input_length JSON_ALPHANUMERIC_CHAR(*p); p++, len++)
error = true;
- lex-prev_token_terminator = lex-token_terminator;
- lex-token_terminator = p;
- if (error)
- report_invalid_token(lex);
+
+ if (num_err != NULL)
+ {
+ /* let the caller handle the error */
+ *num_err = error;
+ }
+ else
+ {
+ lex-prev_token_terminator = lex-token_terminator;
+ lex-token_terminator = p;
+ if (error)
+ report_invalid_token(lex);
+ }
}
/*
@@ -1214,6 +1221,8 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
{
char *outputstr;
text *jsontext;
+ bool numeric_error;
+ JsonLexContext dummy_lex;
if (is_null)
{
@@ -1237,14 +1246,13 @@ datum_to_json(Datum val, bool is_null, StringInfo result,
break;
case TYPCATEGORY_NUMERIC:
outputstr = OidOutputFunctionCall(typoutputfunc, val);
-
/*
* Don't call escape_json here if it's a valid JSON number.
- * Numeric output should usually be a valid JSON number and JSON
- * numbers shouldn't be quoted. Quote cases like Nan and
- * Infinity, however.
*/
- if (strpbrk(outputstr, NON_NUMERIC_LETTER) == NULL)
+ dummy_lex.input = *outputstr == '-' ? outputstr + 1 : outputstr;
+ dummy_lex.input_length = strlen(dummy_lex.input);
+ json_lex_number(dummy_lex, dummy_lex.input, numeric_error);
+ if (! numeric_error)
appendStringInfoString(result, outputstr);
else
escape_json(result, outputstr);
--
Re: [HACKERS] BDR-project
On 12/25/13, 7:01 AM, Andreas Joseph Krogh wrote: http://wiki.postgresql.org/wiki/BDR_Project Is implementing main BDR features into core Postgres a probable objective to version 9.4? No. No BDR feature has been proposed for core PostgreSQL so far. Also, this feature builds on top of the logical replication features, which are themselves questionable for 9.4. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] stuck spinlock
On 12/12/13, 8:45 PM, Tom Lane wrote: Memo to hackers: I think the SIGSTOP stuff is rather obsolete now that most systems dump core files with process IDs embedded in the names. Which systems are those? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XML Issue with DTDs
On Dec23, 2013, at 03:45 , Robert Haas robertmh...@gmail.com wrote: On Fri, Dec 20, 2013 at 8:16 PM, Florian Pflug f...@phlo.org wrote: On Dec20, 2013, at 18:52 , Robert Haas robertmh...@gmail.com wrote: On Thu, Dec 19, 2013 at 6:40 PM, Florian Pflug f...@phlo.org wrote: Solving this seems a bit messy, unfortunately. First, I think we need to have some XMLOPTION value which is a superset of all the others - otherwise, dump restore won't work reliably. That means either allowing DTDs if XMLOPTION is CONTENT, or inventing a third XMLOPTION, say ANY. Or we can just decide that it was a bug that this was ever allowed, and if you upgrade to $FIXEDVERSION you'll need to sanitize your data. This is roughly what we did with encoding checks. What exactly do you suggest we outlaw? !DOCTYPE anywhere but at the beginning. I think we're talking past one another here. Fixing XMLCONCAT/XMLAGG to not produce XML values which are neither valid DOCUMENTS nor valid CONTENT fixes *one* part of the problem. The other part of the problem is that since not every DOCUMENT is valid CONTENT (because CONTENT forbids DTDs) and not every CONTENT is a valid DOCUMENT (because DOCUMENT forbids multiple root nodes), it's impossible to set XMLOPTION to a value which accepts *all* valid XML values. That breaks pg_dump/pg_restore. To fix this, we must provide a way to insert XML data which accepts both DOCUMENTS and CONTENT, and not only one or the other. Due to the way COPY works, we cannot call a special conversion function, so we must modify the input functions. My initial thought was to simply allow XML values which are CONTENT, not DOCUMENTS, to contain a DTD (at the beginning), thus making CONTENT a superset of DOCUMENT. But I've since then realized that the 2003 standard explicitly constrains CONTENT to *not* contain a DTD. The only other option that I can see is to invert a third, non-standard XMLOPTION value, ANY. ANY would accept anything accepted by either DOCUMENT or CONTENT, but no more than that. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XML Issue with DTDs
On Dec23, 2013, at 18:39 , Peter Eisentraut pete...@gmx.net wrote: On 12/19/13, 6:40 PM, Florian Pflug wrote: The following example fails for XMLOPTION set to DOCUMENT as well as for XMLOPTION set to CONTENT. select xmlconcat( xmlparse(document '!DOCTYPE test [!ELEMENT test EMPTY]test/'), xmlparse(content 'test/') )::text::xml; The SQL standard specifies that DTDs are dropped by xmlconcat. It's just not implemented. OK, cool, I'll try to figure out how to do that with libxml best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] stuck spinlock
On Thu, Dec 26, 2013 at 11:54 AM, Peter Eisentraut pete...@gmx.net wrote: On 12/12/13, 8:45 PM, Tom Lane wrote: Memo to hackers: I think the SIGSTOP stuff is rather obsolete now that most systems dump core files with process IDs embedded in the names. Which systems are those? MacOS X dumps core files into /cores/core.$PID, and at least some Linux systems seem to dump them into ./core.$PID I don't know how universal this is. I think a bigger objection to the SIGSTOP stuff is that a lot of bugs are too real time to ever be meaningfully caught that way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] preserving forensic information when we freeze
On Tue, Dec 24, 2013 at 6:22 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-23 14:15:29 -0500, Robert Haas wrote: On Mon, Dec 23, 2013 at 1:57 PM, Andres Freund and...@2ndquadrant.com wrote: Well, all of the fundamental changes (combocids, the initial multixact introduction) have been quite some time ago. I think backward compat has a much higher value these days (I also don't see much point in looking at cmin/cmax for anything but diagnostic purposes). I don't think any of the usecases I've seen would be broken by either fk-locks (multixacts have been there before, doesn't matter much that they now contain updates) nor by forensic freezing. The latter because they really only checked that xmin/xmax were the same, and we hopefully haven't broken that... But part of my point really was the usability, not only the performance. Requiring LATERAL queries to be usable sensibly causes a Meh from my side ;) I simply can't imagine that we're going to want to add a system column every time we change something, or even enough of them to cover all of the things we've already got. We'd need at least infomask, infomask2, and hoff, and maybe some functions for peeking through mxacts to find the updater and locker XIDs and lock modes. With a couple of functions we can do all that and not look back. If system columns don't have an overhead anymore, I fail to see the advantage that functions have over simply accessing parts of the row in the normal way parts of rows are accessed. The only reasoning I can see is lessening the likelihood of conflicts - but that's essentially only solved via namespaced pg_ prefixes for functions as well. I dunno, I just have an uneasy feeling about it. I guess if everyone's happy to add pg_infomask and pg_infomask2 as new system columns, we could go that route. For my part, I think that functions are a real extensibility mechanism and hidden system columns are a crock I'd rather not propagate. But I just work here, and I'll give way if the consensus is otherwise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER SYSTEM SET command to change postgresql.conf parameters
On Tue, Dec 24, 2013 at 10:08 AM, Fujii Masao masao.fu...@gmail.com wrote: On Tue, Dec 24, 2013 at 2:57 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Sun, Dec 22, 2013 at 8:32 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao masao.fu...@gmail.com wrote: I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce it is here. $ psql =# ALTER SYSTEM SET shared_buffers = '512MB'; ALTER SYSTEM =# \q $ pg_ctl -D data reload server signaled 2013-12-22 18:24:13 JST LOG: received SIGHUP, reloading configuration files 2013-12-22 18:24:13 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-22 18:24:13 JST LOG: configuration file X?? contains errors; unaffected changes were applied The configuration file name in the last log message is broken. This problem was introduced by the ALTER SYSTEM SET patch. FreeConfigVariables(head); snip else if (apply) ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg(configuration file \%s\ contains errors; unaffected changes were applied, ErrorConfFile))); The cause of the problem is that, in ProcessConfigFile(), the log message including the 'ErrorConfFile' is emitted after 'head' is free'd even though 'ErrorConfFile' points to one of entry of the 'head'. Your analysis is absolutely right. The reason for this issue is that we want to display filename to which erroneous parameter belongs and in this case we are freeing the parameter info before actual error. To fix, we need to save the filename value before freeing parameter list. Please find the attached patch to fix this issue. Note - In my m/c, I could not reproduce the issue. I think this is not surprising because we are not setting freed memory to NULL, so it can display anything (sometimes right value as well) If you find that the provided patch doesn't fix the problem or you don't find it appropriate due to some other reason, let me know the feedback. When I read ProcessConfigFile() more carefully, I found another related problem. The procedure to reproduce the problem is here. $ psql =# ALTER SYSTEM SET shared_buffers = '256MB'; =# \q $ echo shared_buffers = '256MB' $PGDATA/postgresql.conf $ pg_ctl -D $PGDATA reload 2013-12-25 03:05:44 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-25 03:05:44 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-25 03:05:44 JST LOG: configuration file /dav/alter-system/data/postgresql.auto.conf contains errors; unaffected changes were applied Both postgresql.conf and postgresql.auto.conf contain the setting of the parameter that cannot be changed without restarting the server. But only postgresql.auto.conf was logged, and which would be confusing, I'm afraid. I think that this problem should be fixed together with the problem that I reported before. Thought? I have run into this problem on many occasions because my benchmark scripts usually append a hunk of stuff to postgresql.conf, and any parameters that were already set generate this warning every time you reload, because postgresql.conf now mentions that parameter twice. I'm not quite sure what other problem you want to fix it together with, and I'm not sure what the right fix is either, but +1 for coming up with some solution that's better than what we have now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix typo in src/backend/utils/mmgr/README
On Tue, Dec 24, 2013 at 5:33 AM, Peter Eisentraut pete...@gmx.net wrote: On 12/24/13, 1:33 AM, Etsuro Fujita wrote: This is a small patch to fix a typo in src/backend/utils/mmgr/README. I don't think that change is correct. Why not? It looks like a correct fix to me. The second version looks correct to me too, but I would have fixed it the first way in a vacuum. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Fri, Dec 20, 2013 at 12:39 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. If I understand the problem correctly, it's that as soon as another backend sees the tuple you've inserted and calls XactLockTableWait(), it will not stop waiting even if we later decide to kill the already-inserted tuple. One approach to fix that would be to release and immediately re-acquire the transaction-lock, when you kill an already-inserted tuple. Then teach the callers of XactLockTableWait() to re-check if the tuple is still alive. I'm just waving hands here, but the general idea is to somehow wake up others when you kill the tuple. While mulling this over further, I had an idea about this: suppose we marked the tuple in some fashion that indicates that it's a promise tuple. I imagine an infomask bit, although the concept makes me wince a bit since we don't exactly have bit space coming out of our ears there. Leaving that aside for the moment, whenever somebody looks at the tuple with a mind to calling XactLockTableWait(), they can see that it's a promise tuple and decide to wait on some other heavyweight lock instead. The simplest thing might be for us to acquire a heavyweight lock on the promise tuple before making index entries for it, and then have callers wait on that instead always instead of transitioning from the tuple lock to the xact lock. Then we don't need to do anything screwy like releasing our transaction lock; if we decide to kill the promise tuple, we have a lock to release that pertains specifically to that tuple. This might be a dumb idea; I'm just thinking out loud. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Thu, Dec 26, 2013 at 5:58 PM, Robert Haas robertmh...@gmail.com wrote: While mulling this over further, I had an idea about this: suppose we marked the tuple in some fashion that indicates that it's a promise tuple. I imagine an infomask bit, although the concept makes me wince a bit since we don't exactly have bit space coming out of our ears there. Leaving that aside for the moment, whenever somebody looks at the tuple with a mind to calling XactLockTableWait(), they can see that it's a promise tuple and decide to wait on some other heavyweight lock instead. The simplest thing might be for us to acquire a heavyweight lock on the promise tuple before making index entries for it, and then have callers wait on that instead always instead of transitioning from the tuple lock to the xact lock. I think the interlocking with buffer locks and heavyweight locks to make that work could be complex. I'm working on a scheme where we always acquire a page heavyweight lock ahead of acquiring an equivalent buffer lock, and without any other buffer locks held (for the critical choke point buffer, to implement value locking). With my scheme, you may have to retry, but only in the event of page splits and only at the choke point. In any case, what you describe here strikes me as an expansion on the already less than ideal modularity violation within the btree AM (i.e. the way it buffer locks the heap with its own index buffers concurrently for uniqueness checking). It might be that the best argument for explicit value locks (implemented as page heavyweight locks or whatever) is that they are completely distinct to row locks, and are an abstraction managed entirely by the AM itself, quite similar to the historic, limited value locking that unique index enforcement has always used. If we take Heikki's POC patch as representative of promise tuple schemes in general, this scheme might not be good enough. Index tuple insertions don't wait on each other there, and immediately report conflict. We need pre-checking to get an actual conflict TID in that patch, with no help from btree available. I'm generally opposed to making value locks of any stripe be held for more than an instant (so we should not hold them indefinitely pending another conflicting xact finishing). It's not just that it's convenient to my implementation; I also happen to think that it makes no sense. Should you really lock a value in an earlier unique index for hours, pending conflicter xact finishing, because you just might happen to want to insert said value, but probably not? -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Show lossy heap block info in EXPLAIN ANALYZE for bitmap heap scan
I wrote: Robert Haas wrote: I'd be wary of showing a desired value unless it's highly likely to be accurate. The desired value is accurately estimated based on (a) the total number of exact/lossy pages stored in the TIDBitmap and (b) the following equation in tbm_create(), except for the GIN case where lossy pages are added to the TIDBitmap by tbm_add_page(). I've found there is another risk of overestimating the desired memory space for a BitmapAnded TIDBitmap. I'm inclined to get rid of the estimation functionality from the patch completely, and leave it for future work. Attached is a new version of the patch, which shows only fetch block information and memory usage information. I'll add this to the upcoming CF. Thanks, Best regards, Etsuro Fujita explain-bitmapscan-20131227.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] A GIN index internals question
Hi, Can a posting item / ItemPointer belonging to posting list/tree of some entry stored in a GIN index be lossy? If yes, under what circumstances would such a lossy ItemPointer be included for the entry? I got an impression that there may be some lossy ItemPointers stored in a GIN index while reading keyGetItem() code in src/backend/access/gin/ginget.c -- Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] stuck spinlock
On Thu, Dec 26, 2013 at 03:18:23PM -0800, Robert Haas wrote: On Thu, Dec 26, 2013 at 11:54 AM, Peter Eisentraut pete...@gmx.net wrote: On 12/12/13, 8:45 PM, Tom Lane wrote: Memo to hackers: I think the SIGSTOP stuff is rather obsolete now that most systems dump core files with process IDs embedded in the names. Which systems are those? MacOS X dumps core files into /cores/core.$PID, and at least some Linux systems seem to dump them into ./core.$PID On Linux it's configurable and at least on Ubuntu you get this: $ cat /proc/sys/kernel/core_pattern |/usr/share/apport/apport %p %s %c But yes, it can be configured to icnclude the PID in the filename. Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] stuck spinlock
On 2013-12-12 20:45:17 -0500, Tom Lane wrote: Memo to hackers: I think the SIGSTOP stuff is rather obsolete now that most systems dump core files with process IDs embedded in the names. What would be more useful today is an option to send SIGABRT, or some other signal that would force core dumps. Thoughts? Although I didn't know of the option, I thought having it would be useful before. It allows you to inspect the memory of the individual backends while they are still alive - which allows gdb to call functions. Which surely is helpful when debugging some issues. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On 2013-12-26 21:11:27 -0800, Peter Geoghegan wrote: I'm generally opposed to making value locks of any stripe be held for more than an instant (so we should not hold them indefinitely pending another conflicting xact finishing). It's not just that it's convenient to my implementation; I also happen to think that it makes no sense. Should you really lock a value in an earlier unique index for hours, pending conflicter xact finishing, because you just might happen to want to insert said value, but probably not? There are some advantages: For one, it allows you to guarantee forward progress if you do it right, which surely isn't a bad propert to have. For another, it's much more in line with the way normal uniqueness checks works. Possibly the disadvantages outweigh the advantages, but that's a far cry from making no sense. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] BDR-project
Hi, On 2013-12-26 14:26:15 -0500, Peter Eisentraut wrote: On 12/25/13, 7:01 AM, Andreas Joseph Krogh wrote: http://wiki.postgresql.org/wiki/BDR_Project Is implementing main BDR features into core Postgres a probable objective to version 9.4? Not fully. We hope to integrate some of the building blocks into postgres soon, so we can reduce the size of the diff. Then the plan is to iteratively (continue) to submit features required. No BDR feature has been proposed for core PostgreSQL so far. Also, this feature builds on top of the logical replication features, which are themselves questionable for 9.4. Well, that's not really true. For one, changeset extraction actually is part of the effort, which surely has been submitted. There also have been some further submissions which help build robust replication solutions. One or two of those might be possible to get into 9.4 (they have been in the last CF...) Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
