Re: [HACKERS] INSERT ... ON CONFLICT syntax issues
On 05/05/2015 12:16 AM, Peter Geoghegan wrote: On Sun, Apr 26, 2015 at 2:22 AM, Heikki Linnakangas hlinn...@iki.fi wrote: The ability to specify a constraint by name hasn't been implemented, but that would read quite naturally as: INSERT INTO mytable ON CONFLICT ON CONSTRAINT my_constraint UPDATE ... For the record, I have made this change on Github ... Great, thanks. I'm a bit late to the party as I haven't paid much attention to the syntax before, but let me give some comments on this arbiter index inference thingie. To recap, there are three variants: A. INSERT ... ON CONFLICT DO NOTHING No arbiter is specified. This means that a conflict on any unique or exclusion constraint is not allowed (and will do nothing instead). This variant is only accepted for DO NOTHING. B. INSERT ... ON CONFLICT ON constraint name DO NOTHING/UPDATE In this variant, you explicitly specify the constraint by name. C. INSERT ... ON CONFLICT (index params) [WHERE expression] DO NOTHING/UPDATE This specifies an index (or indexes, in the corner case that there are several identical ones), by listing the columns/expressions and the predicate for a partial index. The list of columns and WHERE match the syntax for CREATE INDEX. That's pretty good overall. A few questions: 1. Why is the variant without specifying an index or constraint not allowed with DO UPDATE? I agree it might not make much sense, but then again, it might. If we're afraid that it's too unsafe to be the default if you don't specify any constraint, how about allowing it with a more verbose ON CONFLICT ON ANY CONSTRAINT syntax? 2. Why can't you specify multiple constraints, even though we implicitly allow any with the first variant? Finally, a couple of suggestions. It would be pretty handy to allow: INSERT ... ON CONFLICT ON PRIMARY KEY DO NOTHING/UPDATE Also, I wonder if we should change the B syntax to be: INSERT ... ON CONFLICT ON *CONSTRAINT* constraint name DO NOTHING/UPDATE That would allow the syntax can be expanded in the future to specify conflicts on other kind of things. The ON PRIMARY KEY syntax should be unambiguous with out, because PRIMARY is a reserved keyword, but for example, we might want to add ON UNIQUE INDEX index name later. - Heikki -- 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] initdb -S and tablespaces
On Tue, May 5, 2015 at 6:26 AM, David Rowley dgrowle...@gmail.com wrote: On 5 May 2015 at 06:23, Robert Haas robertmh...@gmail.com wrote: OK, committed and back-patched. There's a couple of problems with this that the windows buildfarm members are not happy with. The attached patch seems to work locally. Thanks. I think the open() stuff should be fixed by using BasicOpenFile() rather than introducing support for the two-argument form of open(). I'll push a fix shortly. -- 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] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident
On Fri, May 1, 2015 at 1:06 PM, Andres Freund and...@anarazel.de wrote: On 2015-05-01 13:03:39 -0400, Bruce Momjian wrote: Applied and backpatched to 9.4. Ah. Forgot about that. Thanks! Andres .oO(bugtracker?) I don't really see how that would help. People can forget about a ticket in a bugtracker at least as easily as they can forget about a mailing list thread or a CF entry. There's no substitute for human nagging, and I'm glad Bruce has taken that on. -- 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] Disabling trust/ident authentication configure option
Hello, I am the one who suggested the patch to Credativ, so let me explain my reasoning. It is clear that it is basically impossible to get perfect security but this patch would help to solve one small but for our use case quite dangerous issue. Changing the password to something simple is immediately obvious as a security flaw for most people who may come across database configurations, but for the TRUST mode you actually need to know some background on why this is dangerous and when. Especially as it is well documented it might seem legit to some people who are not that experienced with postgres setup. I think this case is sufficiently generic to be applicable to many others and to justify to at least provide the option to work around it. Preventing such configuration errors is a usual topic in most hardening guidelines and most commercial databases take steps to offer more secure mechanisms to implement passwordless access or reset passwords. Regarding passwords what you are saying is basically a People can leave their windows open anyways, so why bother installing door locks kind of argument. In my opinion the secure way to go would be to allow exactly one defined way to access the DB without authentication / reset authentication tokens and make this way easily recognizable / auditable. A nice addition would be to be able to force password policies (e.g. using passwordcheck + something like cracklib)... So please consider to include this patch as it does not change the default behavior but implement a simple way to comply with security policies and actually increase security for some specific use cases. BR, Volker Aßmann On Thu, Apr 30, 2015 at 2:00 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 16, 2015 at 9:55 AM, Bernd Helmle maili...@oopsware.de wrote: PostgreSQL is deployed as part of a larger technical solution (e.g. a Telecommunication system) and a field engineer has to install/upgrade this solution. The engineer is a specialist in the Telco domain and has only little knowledge of databases and especially PostgreSQL setup. We now want to provide these kinds of users with pre-hardened packages that make it very hard to accidentally expose their database. For this purpose the patch allows to optionally disable the trust and ident authentication methods. Especially the trust mechanism is very critical as it might actually provide useful functionality for our user. Think of an engineer who has to do a night shift upgrade with a customer breathing down his neck to get the system back online. Circumventing all these authentication configuration issues by just enabling trust is very easy and looks well supported and documented. But... the user could use password authentication with the password set to x and that would be insecure, too, yet not prevented by any of this. I think it's pretty hard to prevent someone who has filesystem-level access to the database server from configuring it insecurely. Of course, it's fine for people to make changes like this in their own copies of PostgreSQL, but I'm not in favor of incorporating those changes into core. I don't think there's enough general utility to this to justify that, and more to the point, I think different people will want different things. We haven't, for example, ever had a request for this specific thing before. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Patch for bug #12845 (GB18030 encoding)
Hi, Can someone look at this patch. It should fix bug #12845. The current tests for conversions are very minimal. I expanded them a bit for this bug. I think the binary search in the .map files should be removed but I leave that for another patch. 0001-Have-GB18030-handle-more-than-2-byte-Unicode-code-po.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] feature freeze and beta schedule
On Fri, May 1, 2015 at 1:16 PM, Stephen Frost sfr...@snowman.net wrote: We really need to segregate the two.. By that what I mean is this: I want an always-open bugfix CF, which allows us to keep track of bugfix patches. Having something about applies to versions X, Y, Z would be nice too... /me prods Magnus I don't really see that as a step forward. Right now there's one place to look at for patches that somebody wants me (or someone) to pay attention to, and, when time permits, I look at it. Dividing that up into two places is not going to make me look at them more frequently. -- 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] extend pgbench expressions with functions
v4, rebase (after pgbench moved to src) and use of the recently added
syntax_error function.
v5 which adds a forgotten header declaration for a new function, so as to
avoid a warning.
--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index a808546..d09b2bf 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -762,7 +762,7 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/
references to variables literal:/replaceablevariablename/,
and expressions composed of unary (literal-/) or binary operators
(literal+/, literal-/, literal*/, literal//, literal%/)
- with their usual associativity, and parentheses.
+ with their usual associativity, function literalabs/ and parentheses.
/para
para
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index e68631e..f5708e2 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -20,6 +20,7 @@ static PgBenchExpr *make_integer_constant(int64 ival);
static PgBenchExpr *make_variable(char *varname);
static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
PgBenchExpr *rexpr);
+static PgBenchExpr *make_func(const char *fname, PgBenchExpr *arg1);
%}
@@ -35,9 +36,9 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
%type expr expr
%type ival INTEGER
-%type str VARIABLE
+%type str VARIABLE FUNCTION
-%token INTEGER VARIABLE
+%token INTEGER VARIABLE FUNCTION
%token CHAR_ERROR /* never used, will raise a syntax error */
/* Precedence: lowest to highest */
@@ -59,6 +60,7 @@ expr: '(' expr ')' { $$ = $2; }
| expr '%' expr { $$ = make_op('%', $1, $3); }
| INTEGER{ $$ = make_integer_constant($1); }
| VARIABLE { $$ = make_variable($1); }
+ | FUNCTION '(' expr ')' { $$ = make_func($1, $3); pg_free($1); }
;
%%
@@ -95,4 +97,33 @@ make_op(char operator, PgBenchExpr *lexpr, PgBenchExpr *rexpr)
return expr;
}
+static struct {
+ char * fname;
+ int nargs;
+ PgBenchFunction tag;
+} PGBENCH_FUNCTIONS[] = {
+ { abs, 1, PGBENCH_ABS }
+};
+
+static PgBenchExpr *
+make_func(const char * fname, PgBenchExpr *arg1)
+{
+ PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+ int nfunctions = sizeof(PGBENCH_FUNCTIONS) / sizeof(PGBENCH_FUNCTIONS[0]);
+ int i;
+
+ expr-etype = ENODE_FUNCTION;
+ expr-u.function.function = PGBENCH_NONE;
+
+ for (i = 0; i nfunctions; i++)
+ if (pg_strcasecmp(fname, PGBENCH_FUNCTIONS[i].fname) == 0)
+ expr-u.function.function = PGBENCH_FUNCTIONS[i].tag;
+
+ if (expr-u.function.function == PGBENCH_NONE)
+ expr_yyerror_more(unexpected function name, fname);
+
+ expr-u.function.arg1 = arg1;
+ return expr;
+}
+
#include exprscan.c
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5331ab7..0e07bfe 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -57,6 +57,11 @@ space [ \t\r\f]
yylval.ival = strtoint64(yytext);
return INTEGER;
}
+[a-zA-Z]+ {
+ yycol += yyleng;
+ yylval.str = pg_strdup(yytext);
+ return FUNCTION;
+}
[\n] { yycol = 0; yyline++; }
{space}+ { yycol += yyleng; /* ignore */ }
@@ -71,10 +76,16 @@ space [ \t\r\f]
%%
void
-yyerror(const char *message)
+expr_yyerror_more(const char *message, const char *more)
{
syntax_error(expr_source, expr_lineno, expr_full_line, expr_command,
- message, NULL, expr_col + yycol);
+ message, more, expr_col + yycol);
+}
+
+void
+yyerror(const char *message)
+{
+ expr_yyerror_more(message, NULL);
}
/*
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 06a4dfd..5084381 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -956,6 +956,26 @@ evaluateExpr(CState *st, PgBenchExpr *expr, int64 *retval)
return false;
}
+ case ENODE_FUNCTION:
+ {
+switch (expr-u.function.function)
+{
+ case PGBENCH_ABS:
+ {
+ int64 arg1;
+ if (!evaluateExpr(st, expr-u.function.arg1, arg1))
+ return false;
+
+ *retval = arg1 0? arg1: -arg1;
+ return true;
+ }
+ default:
+ fprintf(stderr, bad function (%d)\n,
+expr-u.function.function);
+ return false;
+}
+ }
+
default:
break;
}
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index a3db6b9..5062ef5 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -15,11 +15,18 @@ typedef enum PgBenchExprType
{
ENODE_INTEGER_CONSTANT,
ENODE_VARIABLE,
- ENODE_OPERATOR
+ ENODE_OPERATOR,
+ ENODE_FUNCTION
} PgBenchExprType;
typedef struct PgBenchExpr PgBenchExpr;
+typedef enum PgBenchFunction
+{
+ PGBENCH_NONE,
+ PGBENCH_ABS
+} PgBenchFunction;
+
struct PgBenchExpr
{
PgBenchExprType etype;
@@ -39,6 +46,11 @@ struct PgBenchExpr
PgBenchExpr *lexpr;
PgBenchExpr *rexpr;
} operator;
+ struct
+ {
+ PgBenchFunction function;
+ PgBenchExpr *arg1;
+ } function;
} u;
};
@@
Re: [HACKERS] psql :: support for \ev viewname and \sv viewname
On Mon, May 4, 2015 at 5:21 AM, Petr Korobeinikov pkorobeini...@gmail.com wrote: I'm proposing to add two new subcommands in psql: 1. \ev viewname - edit view definition with external editor (like \ef for function) 2. \sv viewname - show view definition (like \sf for function, for consistency) Sounds nice. Make sure to add your patch to the open CommitFest so we don't forget about it. https://commitfest.postgresql.org/action/commitfest_view/open -- 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] BRIN range operator class
On 05/05/2015 01:10 PM, Emre Hasegeli wrote: I already replied his email [1]. Which issues do you mean? Sorry, my bad please ignore the previous email. -- Andreas Karlsson -- 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] replication slot restart_lsn initialization
Hi, On 2015-05-06 00:42:14 +, Duran, Danilo wrote: I am looking to better understand the thought behind a replication slot's restart_lsn initialization. Currently in 9.4 and master, a replication slot's restart_lsn is set to InvalidXLogRecPtr and will only start tracking restart_lsn once a walreceiver has confirmed receipt of an lsn. Right, for physical slots that's true. Was there any consideration for initializing restart_lsn to the latest WAL write pointer when a slot is created? Or for allowing an optional parameter in pg_create_(physical|logical)_replication_slot() for specifying the restart_lsn at slot creation? I've been wondering about allowing for the latter alternative. I could have used it a couple times. The former doesn't make much sense to me, it could be too far *ahead* in many cases actually. A patch for this would be fairly trivial. It doesn't make sense for logical slots (as the computation of the initial lsn is more complicated; it's also also already set when you create one. I believe there are valid usage patterns where the user would like to start holding transaction logs from being removed/recycled during the time that the standby is being restored from base backup. Currently this can be worked around by using pg_receivexlog immediately after creating the replication slot but it feels kind of hacky. Yea, that's what I've done as well. I'd much rather have a proper option for it. It is also strange that the return type for pg_create_(physical|logical)_replication_slot includes xlog_position but as far as I can tell, it will never contain a value. Is this intended for something in the future? It'll contain something for logical slots. I wanted the physical version to be analogous, with the thought of adding a different signature at some point. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Implementing SQL ASSERTION
On 4/30/15 6:36 PM, Joe Wildish wrote: I’m wondering if there are other people out there working on implementing SQL ASSERTION functionality? I was the last one, probably: http://www.postgresql.org/message-id/1384486216.5008.17.ca...@vanquo.pezone.net. I intend to pick up that work sometime, but feel free to review the thread for a start. The main question was how to manage transaction isolation. -- 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] [COMMITTERS] pgsql: Add transforms feature
On 5/3/15 2:27 PM, Tom Lane wrote: 1. Preventing undefined-symbol errors at link time might be standard according to some platforms, but it is not the universal default, and I think there is no very good reason to assume that it is possible everywhere. So I'm afraid that prairiedog is just the canary in the coal mine, warning us about portability problems we're going to have with other non-mainstream platforms not represented in the buildfarm. I don't think this is an issue, for the following reasons: Any dlopenable module will have undefined symbols, namely those that it uses to call back into the executable that loads the module. The only way it can know to not complain about those symbols is if we tell it about that executable at build time. Our current shared library build setup makes reference to the postgres executable only on three platforms: Windows, OS X, AIX. All other platforms necessarily accept all undefined symbols at build time. We already know that it can also be made to work on Windows and OS X. I expect that we might need some tweaking on AIX, but I'm quite sure it can be made to work there also, because ... More generally, the way to build a dlopenable module with GNU libtool is libtool --mode=link gcc -module -o hello.la foo.lo hello.lo This works on all platforms, for a very large value of all. There isn't even an option to make a reference to the loading executable. As a side note, Perl and Python themselves don't even build without this option, so even if such platforms existed, they couldn't install Perl or Python as we know it, and so wouldn't be able to use these features. 2. Preventing undefined-symbol errors at link time will hide actual coding errors, not only the intended cross-plugin reference cases. We have repeatedly seen the buildfarm members that complain about this find actual bugs that were missed on laxer platforms. Therefore I think that this approach is going to lead to masking bugs that we'll find only by much more expensive/painful means than noticing a buildfarm failure. I appreciate this issue, and I have actually done quite a bit of research in the hope of being able to provide similar functionality on other platforms. But on platforms such as Linux, there is no equivalent option at all. That is, you cannot build a dlopenable module with the provision to error on all undefined symbols except those found, say, in the postgres binary. So there is no way to make all platforms reasonably similar here. Which leads to a social problem. This is a feature intended for extension authors. Extension authors will just build their code on Linux, and if it runs, they ship it. Not everyone (hardly anyone) has the option of doing platform testing for extensions. So if some non-mainstream platform is more picky than others, then the inevitable result is that extensions are more likely to be broken on non-mainstream platforms. This is already frequently the case for other reasons, with non-mainstream effectively meaning anything other than the author's own machine in some cases, it appears. We're not going to make this better by maintaining platform-specific traps. This issue already exists independent of this feature. If I want to create an extension that adds some functionality to hstore or hll or postgis, I'll just call their symbols, it works, I'll ship it. We don't have any way to prevent this, because many mainstream platforms don't have the required fine-grained undefined symbol controls. And the extension authors' path of least resistance when faced with a build failure report on OS X, say, is to just add an option to the link command in their extension, it works, it ships. Moreover, I'm not sure this error checking actually buys us much in practice. A typoed symbol will be flagged by a compiler warning, and any undefined symbol will be caught be the test suite as soon as the module is loaded. So I can't imagine what those buildfarm complaints are, although I'd be interested look into them the analyze the circumstances if someone could point me to some concrete cases. Nonetheless, if there is so much attachment to this behavior, there might be a simple compromise. We keep the error checking on by default, but allow a pgxs-level variable setting like SHLIB_ALLOW_UNDEFINED. This would essentially be equivalent to the I'll-just-hack-my-extension's-makefile approach mentioned above that will inevitably happen, but that way we can document it and maintain some level of control over it. Surely there are other ways to deal with this requirement that don't require forcing undefined symbols to be treated as valid. For instance, our existing find_rendezvous_variable() infrastructure might offer a usable solution for passing a function pointer from plugin A to plugin B. Indeed, that's pretty much what it was invented for. The rendezvous variables were invented for a different problem, namely that you can load modules in
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Fri, May 1, 2015 at 9:04 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 30, 2015 at 11:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote: VACUUM has both syntax: with parentheses and without parentheses. I think we should have both syntax for REINDEX like VACUUM does because it would be pain to put parentheses whenever we want to do REINDEX. Are the parentheses optional in REINDEX command? No. The unparenthesized VACUUM syntax was added back before we realized that that kind of syntax is a terrible idea. It requires every option to be a keyword, and those keywords have to be in a fixed order. I believe the intention is to keep the old VACUUM syntax around for backward-compatibility, but not to extend it. Same for EXPLAIN and COPY. REINDEX will have only one option VERBOSE for now. Even we're in a situation like that it's not clear to be added newly additional option to REINDEX now, we should need to put parenthesis? Also I'm not sure that both implementation and documentation regarding VERBOSE option should be optional. Regards, --- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] feature freeze and beta schedule
On Tue, May 5, 2015 at 3:47 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, May 1, 2015 at 1:16 PM, Stephen Frost sfr...@snowman.net wrote: We really need to segregate the two.. By that what I mean is this: I want an always-open bugfix CF, which allows us to keep track of bugfix patches. Having something about applies to versions X, Y, Z would be nice too... /me prods Magnus I don't really see that as a step forward. Right now there's one place to look at for patches that somebody wants me (or someone) to pay attention to, and, when time permits, I look at it. Dividing that up into two places is not going to make me look at them more frequently. Thanks for highlighting that somebody was actually trying to get my attention in that thread. I missed it :) Which brings the point - this should be discussed in a separate thread, really. It's not part of feature freeze or beta schedule... It's more about process... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0
On Tue, May 5, 2015 at 8:40 AM, Andres Freund and...@anarazel.de wrote: One additional thing I'm wondering about is the following: Right now INSERT ... ON CONFLICT NOTHING does not acquire a row level lock on the 'target' tuple. Are we really ok with that? Because in this form ON CONFLICT NOTHING really doesn't guarantee much, the conflicting tuple could just be deleted directly after the check. ISTM we should just acquire the lock in the same way ExecOnConflictUpdate does. In the majority of the cases that'll be what users actually expect behaviourally. Locking the row is not nothing, though. If you want to lock the row, use an UPSERT with a tautologically false WHERE clause (like WHERE false). This is how other similar ignore features work in other systems, including MySQL, SQLite, and Oracle (which surprisingly has a hint that does this - surprising only because hints don't usually change the meaning of statements). It could make a big difference with a large bulk loading session, because just locking the rows will generate WAL and dirty pages. ETL is really the use-case for DO NOTHING. -- 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
[HACKERS] Fixing busted citext function declarations
In http://www.postgresql.org/message-id/bn1pr04mb37467aa1d412223b3d4a595df...@bn1pr04mb374.namprd04.prod.outlook.com it's revealed that the citext extension misdeclares its versions of regexp_matches(): they should return SETOF text[] but they're marked as returning just text[]. We know generally how to fix this sort of thing: create a new version of the citext extension and provide an upgrade script that repairs the error. However there are a couple of points that deserve discussion. * We can't use CREATE OR REPLACE FUNCTION in the upgrade script because that intentionally doesn't let you change the result type of an existing function. I considered doing a manual UPDATE of the pg_proc entry, but then remembered why CREATE OR REPLACE FUNCTION is picky about this: the result type, including set-ness, is embedded in the parse tree of any view referencing the function. So AFAICS we need to actually drop and recreate the citext regexp_matches() functions in the upgrade script. That means ALTER EXTENSION citext UPDATE will fail if these functions are being used in any views. That's annoying but I see no way around it. (We could have the upgrade script do DROP CASCADE, but that seems way too destructive.) * Is anyone concerned about back-patching this fix? I suppose you could make an argument that some app somewhere might be relying on the current behavior of citext regexp_matches(), which effectively is to return only the first match even if you said 'g'. One answer would be to keep on supplying the citext 1.0 script in the back branches, so that anyone who really needed it could still install 1.0 not 1.1. That's not our usual practice though, so unless there's serious concern that such a problem really exists, I'd rather not do that. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] parallel mode and parallel contexts
On Sat, May 2, 2015 at 12:35 PM, Noah Misch n...@leadboat.com wrote: Perhaps, rather than model it as master M waiting on worker list W1|W2|W3, model it with queue-nonempty and queue-nonfull events, one pair per queue. Each individual queue has only a single reader and a single writer. In your example, there would be three queues, not just one. Of course, one could design a new shared memory data structure representing a collection of related queues, but I still don't see exactly how we get around the problem of that requiring O(MaxBackends^2) storage space. M subscribes to queue0-nonempty, and each of W1,W2,W3 publish to it. M publishes to queue0-nonfull, and the workers subscribe. An edge forms in the deadlock detector's graph when all of an event's subscribers wait on it. (One can approximate that in userspace with a pair of advisory locks.) An edge between which two processes? After thinking about it a bit more, I realized that even if we settle on some solution to that problem, there's another issues: the wait-edges created by this system don't really have the same semantics as regular lock waits. Suppose A is waiting on a lock held by B and B is waiting for a lock held by A; that's a deadlock. But if A is waiting for B to write to a tuple queue and B is waiting for A to read from a tuple queue, that's not a deadlock if the queues in question are the same. I do see a deadlock. B wants to block until A reads from the queue, so it advertises that and sleeps. Waking up deadlock_timeout later, it notices that A is waiting for B to write something. B will not spontaneously suspend waiting to write to the queue, nor will A suspend waiting to read from the queue. Thus, the deadlock is valid. This assumes the deadlock detector reasons from an authoritative picture of pending waits and that we reliably wake up a process when the condition it sought has arrived. We have that for heavyweight lock acquisitions. The assumption may be incompatible with Andres's hope, quoted above, of avoiding the full lock acquisition procedure. I don't understand. What's typically going to happen here is that the queue will initially be empty, and A will block. Suppose B has a large message to write to the queue, let's say 100x the queue size. It will write the first 1% of the message into the queue, set A's latch, and go to sleep. A will now wake up, drain the queue, set B's latch, and go to sleep. B will then wake up, write the next chunk of the message, set A's latch again, and go to sleep. They'll go back and forth like this until the entire message has been transmitted. There's no deadlock here: everything is working as designed. But there may be instants where both A and B are waiting, because (for example) A may set B's latch and finish going to sleep before B gets around to waking up. That's probably something that we could patch around, but I think it's missing the larger point. When you're dealing with locks, there is one operation that causes blocking (acquiring a lock) and another operation that unblocks other processes (releasing a lock). With message queues, there are still two operations (reading and writing), but either of them can either cause you to block yourself, or to unblock another process. To put that another way, locks enforce mutual exclusion; message queues force mutual *inclusion*. Locks cause blocking when two processes try to operate on the same object at the same time; message queues cause blocking *unless* two processes operate on the same object at the same time. That difference seems to me to be quite fundamental. So then you have to wonder why we're not solving problem #1, because the deadlock was just as certain before we generated the maximum possible number of tuples locally as it was afterwards. The 1. above reads like a benefit, not a problem. What might you solve about it? Sorry, that wasn't very clear. Normally, it is a benefit, but in some cases, it could result in a long delay in reporting an inevitable deadlock. What I mean is: suppose we construct a working mechanism where the deadlock detector knows about tuple queue waits. Suppose further that we have a parallel leader and two workers cooperating to do a task that takes 300 s and can be parallelized with perfect efficiency so that, working together, those three processes can get the job done in just 100 s. If the leader tries to take a lock held in a conflicting mode by one of the workers, the worker will fill up the tuple queue - probably quite quickly - and wait. We now detect a deadlock. Our detection is timely, and life is good. On the other hand, suppose that one of the *workers* tries to take a lock held in a conflicting mode by the other worker or by the leader. There is no immediate deadlock, because the leader is not waiting. Instead, we'll finish the computation with the remaining worker and the leader, which will take 150 seconds since we only have
Re: [HACKERS] Fixing busted citext function declarations
Tom Lane wrote: * We can't use CREATE OR REPLACE FUNCTION in the upgrade script because that intentionally doesn't let you change the result type of an existing function. I considered doing a manual UPDATE of the pg_proc entry, but then remembered why CREATE OR REPLACE FUNCTION is picky about this: the result type, including set-ness, is embedded in the parse tree of any view referencing the function. So AFAICS we need to actually drop and recreate the citext regexp_matches() functions in the upgrade script. That means ALTER EXTENSION citext UPDATE will fail if these functions are being used in any views. That's annoying but I see no way around it. (We could have the upgrade script do DROP CASCADE, but that seems way too destructive.) I think we do need to have the upgrade script drop/recreate without cascade. Then, users can alter extension upgrade, note the problematic views (which should be part of the error message), drop them, then retry the extension update and re-create their views. This is necessarily a manual procedure -- I don't think we can re-create views using the function automatically. CASCADE seems pretty dangerous. (I think it is possible that the behavior change is actually problematic as opposed to just behaving differently. For instance, if the function is used in a subselect that's expected to return only one row, and it suddenly starts returning more, the query would raise an error. Seems better to err on the side of caution.) * Is anyone concerned about back-patching this fix? I suppose you could make an argument that some app somewhere might be relying on the current behavior of citext regexp_matches(), which effectively is to return only the first match even if you said 'g'. One answer would be to keep on supplying the citext 1.0 script in the back branches, so that anyone who really needed it could still install 1.0 not 1.1. That's not our usual practice though, so unless there's serious concern that such a problem really exists, I'd rather not do that. I think we should keep the 1.0 version this time, in back branches. This can be invoked manually only (i.e. the default version would still be 1.1), and it wouldn't be provided in the master branch, so it would not cause long-term maintenance pain. But it should be possible for people to re-create their current databases, in case they need to upgrade for unrelated reasons and have views dependant on this behavior. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] INSERT ... ON CONFLICT error messages
I've read through all the error messages in the patch. Some comments: postgres=# create table foo (id int4); CREATE TABLE postgres=# create unique index foo_y on foo (id) where id 0; CREATE INDEX postgres=# insert into foo values (-1) on conflict (id) where id 0 do nothing; ERROR: inferred arbiter partial unique index's predicate does not cover tuple proposed for insertion DETAIL: ON CONFLICT inference clause implies that the tuple proposed for insertion must be covered by the predicate of partial index foo_y. I'm surprised. If the inserted value doesn't match the WHERE clause of the constraint, there is clearly no conflict, so I would assume the above to work without error. postgres=# create table foo (id int4 primary key, col2 int4, constraint col2_unique unique (col2) deferrable); CREATE TABLE postgres=# insert into foo values (2) on conflict on foo_pkey do nothing; ERROR: ON CONFLICT is not supported on relations with deferred unique constraints/exclusion constraints Why not? I would understand if the specified arbiter constraint is deferred, but why does it matter if one of the other constraints is? postgres=# create table foo (id int4 primary key); CREATE TABLE postgres=# begin isolation level serializable; BEGIN postgres=# select 1; -- to get the snapshot ?column? -- 1 (1 row) in another session, insert 1 postgres=# insert into foo values (1) on conflict on foo_pkey do nothing; ERROR: could not serialize access due to concurrent insert or update directing alternative ON CONFLICT path What about a delete? Don't you get a serialization error, if another backend deletes the conflicting tuple, and you perform an INSERT based on the effects of the deleting transaction, which is not supposed to be visible to you? The error message is quite long. How about just giving the default could not serialize access due to concurrent update message? I don't think the directing alternative ON CONFLICT path really helps the user. postgres=# insert into foo values (1), (2) on conflict on foo_pkey do update set id = 2; ERROR: ON CONFLICT DO UPDATE command could not lock/update self-inserted tuple HINT: Ensure that no rows proposed for insertion within the same command have duplicate constrained values. lock/update doesn't sound good. If the system knows which it is, should say so. But in this case, locking the row just an implementation detail. We're performing the UPDATE when that error happens, not locking. How about: ERROR: could not update tuple that was inserted in the same command HINT: Ensure that the rows being inserted do not conflict with each other. BTW, the tuple might be an updated version of an existing tuple, i.e. the ON CONFLICT DO UPDATE ... was fired on an earlier row. Not necessarily a row that was inserted in the same command. The message is technically correct, but you need to understand the difference between tuple and a row. Actually, should we avoid using the term tuple altogether in user-facing errors? postgres=# insert into foo values (1) on conflict (xmin) do nothing; ERROR: system columns may not appear in unique index inference specification may is usually not the right word to use in error messages, as it implies permission to do something (see style guide). It's not totally inappropriate in this case, but how about: ERROR: system columns cannot be used in an ON CONFLICT clause postgres=# create table foo (id int4, constraint xcheck CHECK (id 0));CREATE TABLE postgres=# insert into foo values (1) on conflict on xcheck do nothing; ERROR: constraint in ON CONFLICT clause has no associated index How about: ERROR: xcheck is a CHECK constraint DETAIL: Only unique or exclusion constraints can be used in ON CONFLICT ON clause. That's assuming that xcheck actually is a CHECK constraint. Similarly for a foreign key constraint. postgres=# create table foo (id int4, constraint foo_x exclude using gist (id with =) ); CREATE TABLE postgres=# insert into foo values (1) on conflict on foo_x do update set id=-1; ERROR: foo_x is not a unique index DETAIL: ON CONFLICT DO UPDATE requires unique index as arbiter. I think it would be better to refer to the constraint here, rather than the index. The user specified the constraint, the fact that it's backed by an index is just an implementation detail. Actually, the user specified an exclusion constraint, and expected DO UPDATE to work with that. But we don't support that. So we should say: ERROR: ON CONFLICT ... DO UPDATE not supported with exclusion constraints postgres=# create table foo (id int4 primary key, t text); CREATE TABLE postgres=# insert into foo values (1) on conflict (t) do nothing; ERROR: could not infer which unique index to use from expressions/columns and predicate provided for ON CONFLICT I think we should assume that the user listed the columns correctly, but there
Re: [HACKERS] Fixing busted citext function declarations
On May 5, 2015, at 10:07 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: So AFAICS we need to actually drop and recreate the citext regexp_matches() functions in the upgrade script. That means ALTER EXTENSION citext UPDATE will fail if these functions are being used in any views. That's annoying but I see no way around it. (We could have the upgrade script do DROP CASCADE, but that seems way too destructive.) I think we do need to have the upgrade script drop/recreate without cascade. Then, users can alter extension upgrade, note the problematic views (which should be part of the error message), drop them, then retry the extension update and re-create their views. This is necessarily a manual procedure -- I don't think we can re-create views using the function automatically. CASCADE seems pretty dangerous. FWIW, this is a challenge inherent in all extension upgrade scripts. It’d be great if there was a way to defer such dependency errors to COMMIT time, so if a function is replaced with a new one that’s compatible with the old, the dependency tree could be updated automatically. Best, David smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] Fixing busted citext function declarations
Alvaro Herrera alvhe...@2ndquadrant.com writes: (I think it is possible that the behavior change is actually problematic as opposed to just behaving differently. For instance, if the function is used in a subselect that's expected to return only one row, and it suddenly starts returning more, the query would raise an error. Seems better to err on the side of caution.) Yeah. Also, I realized from the citext regression tests that there's a behavioral change even if you *don't* use the 'g' flag: the previous behavior was to return a null on no match, but now you get zero rows out instead. That's a fairly significant change. I think we should keep the 1.0 version this time, in back branches. Agreed. Maybe we shouldn't even make 1.1 the default in the back branches. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixing busted citext function declarations
David G. Johnston david.g.johns...@gmail.com writes: On Tue, May 5, 2015 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: I think we should keep the 1.0 version this time, in back branches. Agreed. Maybe we shouldn't even make 1.1 the default in the back branches. âDoes 9.0 get different treatment?â Given the lack of previous complaints, I'm inclined to not touch 9.0 at all. We don't have any mechanism like multiple extension versions to let users control what happens in 9.0, and this seems like rather a large behavior change to set loose in such an old branch without that. If (I'm not sure this is the case - or must be...) a pg_dump/pg_restore sequence against a back-branch database installs the default version of the extension for that PostgreSQL version I would agree; and, to clarify, we would still provide the ability to upgrade to citext-1.1 in back-branches. Right. Alvaro and it (1.0) wouldn't be provided in the master branch Why wouldn't it? The current behavior is without question broken, so I don't see a good argument for preserving it forever. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] add -s to vacuumdb
Hey folks, Just had your standard... our pg_ tables are all bloated out, what is a good way to take care of that. We have -s for reindexdb but not vacuumdb. Thoughts? JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. Announcing I'm offended is basically telling the world you can't control your own emotions, so everyone else should do it for you. -- 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] Auditing extension for PostgreSQL (Take 2)
On 5/4/15 8:37 PM, Stephen Frost wrote: I don't follow this logic. The concerns raised above are about changing our in-core logging. We haven't got in-core auditing and so I don't see how they apply to it. How is session auditing substantially different from statement logging? I think it is not, and we could tweak the logging facilities a bit to satisfy the audit trail case while making often-requested enhancement to the traditional logging use case as well at the same time. At least no one has disputed that yet. The only argument against has been that they don't want to touch the logging. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add -s to vacuumdb
On 05/05/2015 03:06 PM, Joshua D. Drake wrote: Hey folks, Just had your standard... our pg_ tables are all bloated out, what is a good way to take care of that. We have -s for reindexdb but not vacuumdb. Thoughts? What command will it run? VACUUM doesn't have a SYSTEM flag, unlike REINDEX cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add -s to vacuumdb
On 05/05/2015 12:22 PM, Andrew Dunstan wrote: On 05/05/2015 03:06 PM, Joshua D. Drake wrote: Hey folks, Just had your standard... our pg_ tables are all bloated out, what is a good way to take care of that. We have -s for reindexdb but not vacuumdb. Thoughts? What command will it run? VACUUM doesn't have a SYSTEM flag, unlike REINDEX The equivalent of: vacuum (FULL/VERBOSE/ANALYZE/FREEZE) pg_* Just like REINDEX/reindexdb JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. Announcing I'm offended is basically telling the world you can't control your own emotions, so everyone else should do it for you. -- 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] initdb start server recommendation
On 5/1/15 10:55 AM, Euler Taveira wrote: On 01-05-2015 11:14, Bruce Momjian wrote: Currently initdb outputs suggested text on starting the server: Success. You can now start the database server using: /u/pgsql/bin/postgres -D /u/pgsql/data or /u/pgsql/bin/pg_ctl -D /u/pgsql/data -l logfile start I am now thinking pg_ctl should be recommended first. At the time this text was written pg_ctl was new. +1. BTW, why are we advocating postgres binary use at all? AFAICS the main postgres (or postmaster) uses are (i) startup script (which also advocate for 'pg_ctl -w') and (ii) disaster/debugging purposes. None of those use cases are intended for general users. Let's make it simple and drop 'postgres' line. 1. I like copying and pasting the postgres line during development. That's not a reason to keep it, necessarily. 2. If you're saying, most people shouldn't run postgres directly, then most people also shouldn't run initdb directly. This message will mainly be seen either by developers or testers or tutorial users or do-it-yourselfers. In which case knowing the functionality of the postgres program is valid. 3. It's not clear that pg_ctl is necessarily the best way to start the server. With things like systemd, launchd, supervisord that like to manage the daemons directly, using postgres directly might be the preferable choice. -- 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] BRIN range operator class
Looking at patch 04, it seems to me that it would be better to have
the OpcInfo struct carry the typecache struct rather than the type OID,
so that we can avoid repeated typecache lookups in brin_deform_tuple;
something like
/* struct returned by OpcInfo amproc */
typedef struct BrinOpcInfo
{
/* Number of columns stored in an index column of this opclass */
uint16 oi_nstored;
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
/* Typecache entries of the stored columns */
TypeCacheEntry oi_typcache[FLEXIBLE_ARRAY_MEMBER];
} BrinOpcInfo;
Looking into it now.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] BRIN range operator class
Alvaro Herrera wrote:
Looking at patch 04, it seems to me that it would be better to have
the OpcInfo struct carry the typecache struct rather than the type OID,
so that we can avoid repeated typecache lookups in brin_deform_tuple;
Here's the patch.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
commit 53f9b1ac9a4b73eddcb7acc99aeacff34336f7ad
Author: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Tue May 5 16:19:53 2015 -0300
use typcache rather than oid
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 1b15a7b..bd3191d 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -181,7 +181,7 @@ brin_page_items(PG_FUNCTION_ARGS)
column-nstored = opcinfo-oi_nstored;
for (i = 0; i opcinfo-oi_nstored; i++)
{
-getTypeOutputInfo(opcinfo-oi_typids[i], output, isVarlena);
+getTypeOutputInfo(opcinfo-oi_typcache[i]-type_id, output, isVarlena);
fmgr_info(output, column-outputFn[i]);
}
diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index 1ac282c..92dac7c 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -428,8 +428,8 @@ typedef struct BrinOpcInfo
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
-/* Type IDs of the stored columns */
-Oid oi_typids[FLEXIBLE_ARRAY_MEMBER];
+/* Type cache entries of the stored columns */
+TypeCacheEntry *oi_typcache[FLEXIBLE_ARRAY_MEMBER];
} BrinOpcInfo;
/programlisting
structnameBrinOpcInfo/.structfieldoi_opaque/ can be used by the
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 299d6f7..ce8652d 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -62,8 +62,8 @@ brin_minmax_opcinfo(PG_FUNCTION_ARGS)
result-oi_nstored = 2;
result-oi_opaque = (MinmaxOpaque *)
MAXALIGN((char *) result + SizeofBrinOpcInfo(2));
- result-oi_typids[0] = typoid;
- result-oi_typids[1] = typoid;
+ result-oi_typcache[0] = result-oi_typcache[1] =
+ lookup_type_cache(typoid, 0);
PG_RETURN_POINTER(result);
}
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 08fa998..22ce74a 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -68,7 +68,7 @@ brtuple_disk_tupdesc(BrinDesc *brdesc)
{
for (j = 0; j brdesc-bd_info[i]-oi_nstored; j++)
TupleDescInitEntry(tupdesc, attno++, NULL,
- brdesc-bd_info[i]-oi_typids[j],
+ brdesc-bd_info[i]-oi_typcache[j]-type_id,
-1, 0);
}
@@ -444,8 +444,8 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple)
for (i = 0; i brdesc-bd_info[keyno]-oi_nstored; i++)
dtup-bt_columns[keyno].bv_values[i] =
datumCopy(values[valueno++],
- brdesc-bd_tupdesc-attrs[keyno]-attbyval,
- brdesc-bd_tupdesc-attrs[keyno]-attlen);
+ brdesc-bd_info[keyno]-oi_typcache[i]-typbyval,
+ brdesc-bd_info[keyno]-oi_typcache[i]-typlen);
dtup-bt_columns[keyno].bv_hasnulls = hasnulls[keyno];
dtup-bt_columns[keyno].bv_allnulls = false;
diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index 84eed61..1486d04 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -16,6 +16,7 @@
#include storage/bufpage.h
#include storage/off.h
#include utils/relcache.h
+#include utils/typcache.h
/*
@@ -32,13 +33,13 @@ typedef struct BrinOpcInfo
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
- /* Type IDs of the stored columns */
- Oid oi_typids[FLEXIBLE_ARRAY_MEMBER];
+ /* Type cache entries of the stored columns */
+ TypeCacheEntry *oi_typcache[FLEXIBLE_ARRAY_MEMBER];
} BrinOpcInfo;
/* the size of a BrinOpcInfo for the given number of columns */
#define SizeofBrinOpcInfo(ncols) \
- (offsetof(BrinOpcInfo, oi_typids) + sizeof(Oid) * ncols)
+ (offsetof(BrinOpcInfo, oi_typcache) + sizeof(TypeCacheEntry *) * ncols)
typedef struct BrinDesc
{
--
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] Unexpected speed PLAIN vs. MAIN
On Mon, May 04, 2015 at 01:50:45PM -0400, Tom Lane wrote: Sandro Santilli s...@keybit.net writes: I'm comparing speed of some queries against tables having the same data but different storage, and got an unexpected behavior. The tables have 2 integer fields and a PcPatch field (p, custom type from pgPointCloud). There are no TOASTs involved (the toast table associated with the table with MAIN storage is empty, the table with PLAIN storage has no toast table). Running a SELECT count(p) takes 6261.699 ms on the table with MAIN storage and 18488.713 ms on the table with PLAIN storage. The number of buffer reads are about the same. Why would reading presence/absence of a value be faster from MAIN than from PLAIN storage ? Hm ... MAIN allows in-line compression while PLAIN doesn't. But for count(), that would only make a difference if it resulted in a smaller physical table size, which it evidently didn't. My best guess is that the OS had many of the pages from rtlidar_dim_main sitting in OS disk cache, so that those buffer reads didn't all translate to physical I/O. Try flushing the OS cache immediately before each trial to get more-reproducible results. Bingo, it was the OS disk cache. Thanks for the tip ! That cache (Linux) acts in mysterious ways, btw. After a new boot, with no explicit flushing, I obtained slow times in both tables (~18 secs) with queries in this order: PLAIN,MAIN,PLAIN,MAIN. Then 3 queries in a row against MAIN brought down its timing to 2, but after that no number of consecutive queries against PLAIN could do that. It took a disk flush (echo 3/proc/sys/vm/drop_caches; sync was not enough) to get the 18 seconds back on reading MAIN and allowing me to force caching PLAIN via consecutive calls... I'll play a bit with pgfincore to learn more. (http://git.postgresql.org/gitweb/?p=pgfincore.git;a=summary) --strk; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add -s to vacuumdb
On Tue, May 5, 2015 at 4:26 PM, Joshua D. Drake j...@commandprompt.com wrote: On 05/05/2015 12:22 PM, Andrew Dunstan wrote: On 05/05/2015 03:06 PM, Joshua D. Drake wrote: Hey folks, Just had your standard... our pg_ tables are all bloated out, what is a good way to take care of that. We have -s for reindexdb but not vacuumdb. Thoughts? What command will it run? VACUUM doesn't have a SYSTEM flag, unlike REINDEX The equivalent of: vacuum (FULL/VERBOSE/ANALYZE/FREEZE) pg_* Just like REINDEX/reindexdb IMHO isn't difficult to add a --system option to vacuumdb and would be nice to have a --schema option too. Some time ago I proposed to add SCHEMA option to VACUUM command [1] but it was not very well accepted and the better way is add more options to vacuumdb. I started a patch to add more powers to --tables option to vacuumdb but unfortunately I had no time to finish it. I know it's another thing but the internal refactoring in vacuumdb to support it enable us to add --system and --schema options also. Regards, [1] http://www.postgresql.org/message-id/CAFcNs+pHJLmLM9n=b6aofv3xppwhq+znnejv9ukvpnszyeg...@mail.gmail.com -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0
On 2015-04-26 18:02:06 -0700, Peter Geoghegan wrote: Remaining challenges = Another thing I'm wondering about is dealing with deferrable constraints/deferred indexes. a) Why does ExecCheckIndexConstraints() check for indisimmediate for *all* indexes and not just when it's an arbiter index? That seems needlessly restrictive. b) There's a doc addition to set_constraints.sgml + constraints are affected by this setting. Note that constraints + that are literalDEFERRED/literal cannot be used as arbiters by + the literalON CONFLICT/ clause that commandINSERT/ + supports. which I don't think makes sense: SET CONSTRAINTS will never change anything relevant for ON CONFLICT, right? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0
Hi, On 2015-04-26 18:02:06 -0700, Peter Geoghegan wrote: Remaining challenges = One additional thing I'm wondering about is the following: Right now INSERT ... ON CONFLICT NOTHING does not acquire a row level lock on the 'target' tuple. Are we really ok with that? Because in this form ON CONFLICT NOTHING really doesn't guarantee much, the conflicting tuple could just be deleted directly after the check. ISTM we should just acquire the lock in the same way ExecOnConflictUpdate does. In the majority of the cases that'll be what users actually expect behaviourally. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Disabling trust/ident authentication configure option
On Tue, May 5, 2015 at 8:05 AM, Volker Aßmann volker.assm...@gmail.com wrote: Changing the password to something simple is immediately obvious as a security flaw for most people who may come across database configurations, but for the TRUST mode you actually need to know some background on why this is dangerous and when. I frankly find that a bit difficult to swallow. You think that everyone knows that bad passwords are a problem, but some people might not realize that an authentication method called trust might not be secure? I suppose that's possible, but I really think that if you install an *authentication method* whose name means *just trust the other guy to be telling the truth* without thinking about the consequences of that, it's hard to have a lot of sympathy for whatever happens afterwards. Besides, your patch doesn't just disable trust. It also disables ident authentication, which in some network configurations could conceivably be more secure than password authentication. When applied to the local machine, ident actually means peer, which has an *excellent* chance of being more secure than password authentication. For that matter, even trust might be better than password. Anybody who can sniff the network traffic can read the password right off of the wire. So either way, your PostgreSQL server is gonna get hacked, but if you use password authentication, you might reveal a password that is also used to protect access to something else that used to be secure. Personally, if I were going to start disabling authentication methods at compile time, I'd start with password and md5. If you are not using SSL, and you use password or md5 authentication, you're basically saying, well, I'm OK with somebody reading all of the data that I'm sending and receiving over the wire, and I'm willing to take the risk that my passwords are easily crackable or can be read straight off the wire using wireshark, but to send your own queries you will have to make at least some minimal effort. If you need real security, that is not nearly good enough. If you don't need real security, why bother making people hassle with a password that's not providing any real protection? There are some valid answers to that question - e.g. if you are on a corporate WAN, you probably can't fire somebody for blundering into an unprotected resource, but if somebody goes to the trouble of cracking the password, even if it's weak, then you can probably nail them. For most users, though, I think password and md5 authentication serve mostly to give people the illusion that they've secured the server. The real security, if there is any, comes primarily from restricting incoming connections via listen_addresses and/or operating system mechanisms such as iptables and/or pg_hba.conf, and from requiring the use of SSL. Passwords are weak sauce. A final point to consider is: what happens when you lock yourself out of the server, like by forgetting that password? Normally you can recover by logging in as the postgres operating system user and then connecting to the server using trust or peer authentication locally; or you can make some temporary modification to pg_hba.conf to open up trust access over the network in a carefully considered and temporary way. But your proposal would remove that safety hatch. I guess you could shut down the server and start up a single-user session to change the password, but that means taking an outage. Again, if you like that trade-off, you can patch your own copy of the source code however you like, but that doesn't sound like something I'd want to recommend in general. -- 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] Proposal : REINDEX xxx VERBOSE
On Tue, May 5, 2015 at 11:10 AM, Sawada Masahiko sawada.m...@gmail.com wrote: On Fri, May 1, 2015 at 9:04 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 30, 2015 at 11:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote: VACUUM has both syntax: with parentheses and without parentheses. I think we should have both syntax for REINDEX like VACUUM does because it would be pain to put parentheses whenever we want to do REINDEX. Are the parentheses optional in REINDEX command? No. The unparenthesized VACUUM syntax was added back before we realized that that kind of syntax is a terrible idea. It requires every option to be a keyword, and those keywords have to be in a fixed order. I believe the intention is to keep the old VACUUM syntax around for backward-compatibility, but not to extend it. Same for EXPLAIN and COPY. REINDEX will have only one option VERBOSE for now. Even we're in a situation like that it's not clear to be added newly additional option to REINDEX now, we should need to put parenthesis? In my opinion, yes. The whole point of a flexible options syntax is that we can add new options without changing the grammar. That involves some compromise on the syntax, which doesn't bother me a bit. Our previous experiments with this for EXPLAIN and COPY and VACUUM have worked out quite well, and I see no reason for pessimism here. Also I'm not sure that both implementation and documentation regarding VERBOSE option should be optional. I don't know what this means. -- 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 CONFLICT syntax issues
On 2015-05-04 14:16:42 -0700, Peter Geoghegan wrote: On Sun, Apr 26, 2015 at 2:22 AM, Heikki Linnakangas hlinn...@iki.fi wrote: The ability to specify a constraint by name hasn't been implemented, but that would read quite naturally as: INSERT INTO mytable ON CONFLICT ON CONSTRAINT my_constraint UPDATE ... For the record, I have made this change on Github Theoretically this changes the pictures for FDWs, right? Right now there's +para + commandINSERT/ with an literalON CONFLICT/ clause is not + supported with a unique index inference specification, since a + conflict arbitrating unique index cannot meaningfully be inferred + on a foreign table (this implies that literalON CONFLICT DO + UPDATE/ is never supported, since the specification is + mandatory there). +/para but theoretically the constraint name could be meaningful on the other side... I don't think this is anyting for 9.5, but it might be interesting for later. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixing busted citext function declarations
On Tue, May 5, 2015 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: (I think it is possible that the behavior change is actually problematic as opposed to just behaving differently. For instance, if the function is used in a subselect that's expected to return only one row, and it suddenly starts returning more, the query would raise an error. Seems better to err on the side of caution.) Yeah. Also, I realized from the citext regression tests that there's a behavioral change even if you *don't* use the 'g' flag: the previous behavior was to return a null on no match, but now you get zero rows out instead. That's a fairly significant change. I think we should keep the 1.0 version this time, in back branches. Agreed. Maybe we shouldn't even make 1.1 the default in the back branches. Does 9.0 get different treatment? If (I'm not sure this is the case - or must be...) a pg_dump/pg_restore sequence against a back-branch database installs the default version of the extension for that PostgreSQL version I would agree; and, to clarify, we would still provide the ability to upgrade to citext-1.1 in back-branches. Alvaro and it (1.0) wouldn't be provided in the master branch Why wouldn't it? The whole point of versioning is to mark a release as stable/immutable and therefore eliminate any maintenance burden by simply refusing to maintain it. There is no maintainability reason to avoid shipping the previous versions that I can think of if the extension infrastructure works as advertised. Is there anywhere besides the source code that a user can read how extensions and pg_dump/pg_restore inter-operate in this manner? Neither pg_dump/restore nor CREATE EXTENSION cover the topic and there isn't a chapter called upgrading that would be a nice place to put such information... David J
Re: [HACKERS] BRIN range operator class
After looking at 05 again, I don't like the same as % business. Creating a whole new class of exceptions is not my thing, particularly not in a regression test whose sole purpose is to look for exceptional (a.k.a. wrong) cases. I would much rather define the opclasses for those two datatypes using the existing @ operators rather than create operators for this purpose. We can add a note to the docs, for historical reasons the brin opclass for datatype box/point uses the @ operator instead of , or something like that. AFAICS this is just some pretty small changes to patches 05 and 06. Will you please resubmit? I just pushed patch 01, and I'm looking at 04 next. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] BRIN range operator class
On 05/05/2015 11:57 AM, Emre Hasegeli wrote: From my point of view as a reviewer this patch set is very close to being committable. Thank you. The new versions are attached. Nice, I think it is ready now other than the issues Alvaro raised in his review[1]. Have you given those any thought? Notes 1. http://www.postgresql.org/message-id/20150406211724.gh4...@alvh.no-ip.org Andreas -- 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] BRIN range operator class
On 05/05/2015 04:24 AM, Alvaro Herrera wrote: Stefan Keller wrote: I'm keen to see if a PostGIS specialist jumps in and adds PostGIS geometry support. Did you test the patch proposed here already? It could be a very good contribution. Indeed, I have done some testing of the patch but more people testing would be nice. Andreas -- 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] BRIN range operator class
Indeed, I have done some testing of the patch but more people testing would be nice. The inclusion opclass should work for other data types as long required operators and SQL level support functions are supplied. Maybe it would work for PostGIS, too. -- 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] initdb -S and tablespaces
On 5 May 2015 at 06:23, Robert Haas robertmh...@gmail.com wrote: OK, committed and back-patched. There's a couple of problems with this that the windows buildfarm members are not happy with. The attached patch seems to work locally. Regards David Rowley fsync_win32_fix.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] BRIN range operator class
Can you please explain what is the purpose of patch 07? I'm not sure I understand; are we trying to avoid having to add pg_amproc entries for these operators and instead piggy-back on btree opclass definitions? Not too much in love with that idea; I see that there is less tedium in that the brin opclass definition is simpler. One disadvantage is a 3x increase in the number of syscache lookups to get the function you need, unless I'm reading things wrong. Maybe this is not performance critical. Anyway I tried applying it on isolation, and found that it fails the assertion that tests the union support proc in brininsert. That doesn't seem okay. I mean, it's okay not to run the test for the inclusion opclasses, but why does it now fail in minmax which was previously passing? Couldn't figure it out. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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 CONFLICT syntax issues
On Tue, May 5, 2015 at 9:36 AM, Andres Freund and...@anarazel.de wrote: Theoretically this changes the pictures for FDWs, right? Right now there's +para + commandINSERT/ with an literalON CONFLICT/ clause is not + supported with a unique index inference specification, since a + conflict arbitrating unique index cannot meaningfully be inferred + on a foreign table (this implies that literalON CONFLICT DO + UPDATE/ is never supported, since the specification is + mandatory there). +/para but theoretically the constraint name could be meaningful on the other side... Well, the inference clause could be too -- in that sense, the constraint name isn't special at all. But you need to invent a way of making the optimizer infer an index on the foreign side (and even with a named constraint, we go from constraint Oid in the parser to pg_index Oid in the optimizer, so it's a similar process to regular inference). Of course, teaching the optimizer about foreign indexes is a whole new infrastructure. Note that this really is the explanation for why postgres_fdw only has limited support. Sure, I haven't added deparsing logic for ON CONFLICT UPDATE, but it would be pretty easy to do so, and that isn't the blocker at all. I don't think this is anyting for 9.5, but it might be interesting for later. Sure. -- 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] BRIN range operator class
Nice, I think it is ready now other than the issues Alvaro raised in his review[1]. Have you given those any thought? I already replied his email [1]. Which issues do you mean? [1] http://www.postgresql.org/message-id/CAE2gYzxQ-Gk3q3jYWT=1enlebsgcgu28+1axml4omcwjbkp...@mail.gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
