Re: [HACKERS] Disabling trust/ident authentication configure option
--On 30. April 2015 08:00:23 -0400 Robert Haas robertmh...@gmail.com wrote: 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. Sure. But I think the point is to make their engineers to think about what they're doing. Typing in a password gives you at least a hint, that you are probably should use something safe. I agree that you couldn't really make that bullet proof from just this excluded functionality, but i could imagine that this makes sense in a more system-wide context. 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. Well, i found at least one of such a proposal here: http://www.postgresql.org/message-id/CAN2Y=umt7cpkxzhaufw7szeckdwcwsuulmh4xphuxkqbtdu...@mail.gmail.com -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] psql :: support for \ev viewname and \sv viewname
Hackers!
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)
What's inside:
1. review-ready implementation of \ev and \sv psql subcommands for editing
and viewing view's definition.
2. psql's doc update with new subcommands description.
3. a bit of extracting common source code parts into separate functions.
4. psql internal help update.
5. tab completion update.
There is one narrow place in this patch: current implementation doesn't
support create statement formatting for recursive views.
Any comments? Suggestions?
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 62a3b21..d668777 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1651,6 +1651,28 @@ Tue Oct 26 21:40:57 CEST 1999
varlistentry
+termliteral\ev optional replaceable
class=parameterviewname/ /optional /literal/term
+
+listitem
+para
+ This command fetches and edits the definition of the named view,
+ in the form of a commandCREATE OR REPLACE VIEW/ command.
+ Editing is done in the same way as for literal\edit/.
+ After the editor exits, the updated command waits in the query buffer;
+ type semicolon or literal\g/ to send it, or literal\r/
+ to cancel.
+/para
+
+para
+ If no view is specified, a blank commandCREATE VEIW/
+ template is presented for editing.
+/para
+
+/listitem
+ /varlistentry
+
+
+ varlistentry
termliteral\encoding [ replaceable
class=parameterencoding/replaceable ]/literal/term
listitem
@@ -2522,6 +2544,25 @@ testdb=gt; userinput\setenv LESS -imx4F/userinput
varlistentry
+termliteral\sv[+] replaceable class=parameterviewname/
/literal/term
+
+listitem
+para
+ This command fetches and shows the definition of the named view,
+ in the form of a commandCREATE OR REPLACE VIEW/ command.
+ The definition is printed to the current query output channel,
+ as set by command\o/command.
+/para
+
+para
+ If literal+/literal is appended to the command name, then the
+ output lines are numbered, with the first line of the view definition
+ being line 1.
+/para
+ /varlistentry
+
+
+ varlistentry
termliteral\t/literal/term
listitem
para
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 70b7d3b..948e381 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -60,9 +60,17 @@ static bool do_connect(char *dbname, char *user, char *host,
char *port);
static bool do_shell(const char *command);
static bool do_watch(PQExpBuffer query_buf, long sleep);
static bool lookup_function_oid(const char *desc, Oid *foid);
+static bool lookup_view_oid(const char *desc, Oid *view_oid);
static bool get_create_function_cmd(Oid oid, PQExpBuffer buf);
-static int strip_lineno_from_funcdesc(char *func);
+static bool get_create_view_cmd(Oid oid, PQExpBuffer buf);
+static void format_create_view_cmd(char *view, PQExpBuffer buf);
+static int strip_lineno_from_objdesc(char *func);
static void minimal_error_message(PGresult *res);
+static int count_lines_in_buf(PQExpBuffer buf);
+static void print_with_linenumbers(FILE *output,
+ char *lines,
+ const char
*header_cmp_keyword,
+ size_t
header_cmp_sz);
static void printSSLInfo(void);
static bool printPsetInfo(const char *param, struct printQueryOpt *popt);
@@ -612,7 +620,7 @@ exec_command(const char *cmd,
func = psql_scan_slash_option(scan_state,
OT_WHOLE_LINE, NULL, true);
- lineno = strip_lineno_from_funcdesc(func);
+ lineno = strip_lineno_from_objdesc(func);
if (lineno == 0)
{
/* error already reported */
@@ -682,6 +690,77 @@ exec_command(const char *cmd,
}
}
+ /*
+* \ev -- edit the named view, or present a blank CREATE VIEW viewname
AS
+* template if no argument is given
+*/
+ else if (strcmp(cmd, ev) == 0)
+ {
+ int lineno = -1;
+
+ if (pset.sversion 70100)
+ {
+ psql_error(The server (version %d.%d) does not support
editing view definition.\n,
+ pset.sversion / 1,
(pset.sversion / 100) % 100);
+
Re: [HACKERS] cost_index() and path row estimate.
--On 1. Mai 2015 11:30:51 -0700 Tom Lane t...@sss.pgh.pa.us wrote: No. The non-parameterized paths for a given relation must all have the same rowcount estimates; otherwise the path comparison logic fails fundamentally. Another way to look at it is that every correct path will yield the same number of rows in reality; so it would be wrong to give a path that makes use of a partial index a rowcount advantage over a path that is not using the partial index but nonetheless is enforcing exactly the same set of scan restriction clauses. Thanks for the explanation. -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] JSON data type and schema validation
On 04 May 2015, at 06:20, Dmitry Shirokov deadr...@gmail.com wrote: Hi all, Are there any plans to introduce in next versions of Postgres a schema validation for JSON field type? It would be very nice to have a support of something like json-schema spec, see http://json-schema.org/documentation.html. Right now there's the only way to do it via individual constraints, which is not very convenient in most cases. Please correct me if I'm wrong. Take a look at https://github.com/akorotkov/jsquery You can find schema validation example in docs. Cheers, Dmitry -- 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] psql :: support for \ev viewname and \sv viewname
2015-05-04 11:21 GMT+02:00 Petr Korobeinikov pkorobeini...@gmail.com: Hackers! 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) +1 Pavel What's inside: 1. review-ready implementation of \ev and \sv psql subcommands for editing and viewing view's definition. 2. psql's doc update with new subcommands description. 3. a bit of extracting common source code parts into separate functions. 4. psql internal help update. 5. tab completion update. There is one narrow place in this patch: current implementation doesn't support create statement formatting for recursive views. Any comments? Suggestions? -- 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] two-arg current_setting() with fallback
Hello, Well, speaking of the two-arg form vs alternate name, here's a version of the patch which includes the new behavior Thought i will attempt a review. The patch applies cleanly to latest HEAD. patch -p1 /home/Sameer/Downloads/0001-Add-two-arg-form-of-current_setting-to-optionally-su.patch patching file doc/src/sgml/func.sgml Hunk #1 succeeded at 16216 (offset 1 line). Hunk #2 succeeded at 16255 (offset 1 line). patching file src/backend/utils/misc/guc.c Hunk #1 succeeded at 7693 (offset -3 lines). Hunk #2 succeeded at 8012 (offset -3 lines). patching file src/include/catalog/pg_proc.h Hunk #1 succeeded at 3044 (offset 4 lines). patching file src/include/utils/builtins.h patching file src/include/utils/guc.h patching file src/test/regress/expected/guc.out patching file src/test/regress/sql/guc.sql But i do get error at make make -C catalog schemapg.h make[3]: Entering directory `/home/Sameer/git/latest_postgres/postgres/src/backend/catalog' cd ../../../src/include/catalog '/usr/bin/perl' ./duplicate_oids 3280 make[3]: *** [postgres.bki] Error 1 make[3]: Leaving directory `/home/Sameer/git/latest_postgres/postgres/src/backend/catalog' make[2]: *** [submake-schemapg] Error 2 make[2]: Leaving directory `/home/Sameer/git/latest_postgres/postgres/src/backend' make[1]: *** [all-backend-recurse] Error 2 make[1]: Leaving directory `/home/Sameer/git/latest_postgres/postgres/src' make: *** [all-src-recurse] Error 2 regards Sameer -- View this message in context: http://postgresql.nabble.com/PATCH-two-arg-current-setting-with-fallback-tp5842654p5847904.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] json_populate_record issue - TupleDesc reference leak
On 04/30/2015 08:36 AM, Pavel Stehule wrote:
Still issue is not fixed still
create type pt as (a int, b int);
postgres=# select json_populate_record('(10,20)'::pt, '{}');
WARNING: TupleDesc reference leak: TupleDesc 0x7f413ca325b0
(16560,-1) still referenced
Looking at it now. (Please remember not to top-post.)
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] cache invalidation for PL/pgsql functions
On Fri, May 1, 2015 at 12:29 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, May 1, 2015 at 9:09 AM, Marko Tiikkaja ma...@joh.to wrote: We recently hit a similar case in our production environment. What was annoying about it is that there didn't seem to be a way for the application to fix the issue by itself, short of reconnecting; even DISCARD ALL doesn't help. If we can't fix the underlying issue, can we at least provide a way for apps to invalidate these caches themselves, for example in the form of a DISCARD option? It's been discussed before and I am in favor of it. I'm not. We should fix the problem not expect applications to band-aid around it. This would be particularly ill-advised because there are so many applications that just blindly do DISCARD ALL when changing contexts. The most common real world manifestation of this problem (having to DISCARD to invalidate plans) is when using schema partitioned data with pl/pgsql functions. Attempting to hide everything under DISCARD is trading one problem for another: it's going to be hard for users of tools like pgbouncer (the main users of DISCARD) to correctly judge the performance trade-offs and this statement's workings is already pretty arcane. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] BUG in XLogRecordAssemble
hi,
I found the code in 'backend/access/transam/xloginsert.c' as that:
XLogRecordAssemble:
if (prev_regbuf RelFileNodeEquals(regbuf-rnode, prev_regbuf-rnode))
{
samerel = true;
bkpb.fork_flags |= BKPBLOCK_SAME_REL;
prev_regbuf = regbuf;
}
else
samerel = false;
There is the only place that prev_regbuf is assigned, so prev_regbuf will never
be assigned.
The patch will fix it, Thanks.
Zhang Zq
XLogRecordAssemble.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] Unexpected speed PLAIN vs. MAIN
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 ?
The explain output:
=# explain (analyze, verbose, buffers) select count(pa) from rtlidar_dim_main;
Aggregate (cost=1202627.85..1202627.86 rows=1 width=32) (actual
time=6261.644..6261.644 rows=1 loops=1)
Output: count(pa)
Buffers: shared hit=32 read=1187659
- Seq Scan on public.rtlidar_dim_main (cost=0.00..1199640.48 rows=1194948
width=32) (actual time=0.060..6105.566 rows=1194948 loops=1)
Output: id, source, pa
Buffers: shared hit=32 read=1187659
Total runtime: 6261.699 ms
=# explain (analyze, verbose, buffers) select count(pa) from rtlidar_dim_plain;
Aggregate (cost=1202627.85..1202627.86 rows=1 width=32) (actual
time=18473.973..18473.973 rows=1 loops=1)
Output: count(pa)
Buffers: shared hit=37 read=1187654
- Seq Scan on public.rtlidar_dim_plain (cost=0.00..1199640.48
rows=1194948 width=32) (actual time=0.058..18247.974 rows=1194948 loops=1)
Output: id, source, pa
Buffers: shared hit=37 read=1187654
Total runtime: 18474.028 ms
The relation sizes:
=# select pg_total_relation_size('rtlidar_dim_plain');
9756426240
=# select pg_total_relation_size('rtlidar_dim_main');
9756434432
--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] BUG in XLogRecordAssemble
On Tue, May 5, 2015 at 1:04 AM, Zhang Zq zqzhangm...@163.com wrote: There is the only place that prev_regbuf is assigned, so prev_regbuf will never be assigned. The patch will fix it, Thanks. Indeed. I think you are right. We never set prev_regbuf with the current code. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Loss of some parts of the function definition
About view. I found where I saw it was a discussion solve some problems with view https://wiki.postgresql.org/wiki/Todo#Views_and_Rules, ie it is in the list of TODO, so there is a chance that it will be implemented. 03 Май 2015 г. 12:15 пользователь Sergey Grinko sergey.gri...@gmail.com написал: Thank you Jim! Views, they also have the problem. In my practice I use them very little, so do not just remember them. Somewhere I read that already are going to introduce their storage source. If I find this source, then I write the link here. I am a supporter of conservation of the source code. I hope that the PostgreSQL developers still implement the storage of the full DDL and PostgreSQL then receive another plus in competition with commercial databases. 01 Май 2015 г. 23:03 пользователь Jim Nasby jim.na...@bluetreble.com написал: On 4/30/15 6:44 AM, Sergey Grinko wrote: Now create a script in the application of its function parameters and return values can be declared using %TYPE. However, when you save the script is stored inside the server only what is considered his body. Thus, we obtain: ... We actually mung things a lot worse when it comes to views, so I'm curious why you're only worried about the problems with stored functions? FWIW, I think the best 'solution' to this right now is to actually keep your original definitions as files in your VCS and use something like sqitch for deployment. Taken to it's logical extreme, that means that the only thing you ever 'patch' is an actual table (via ALTER TABLE), or indexes. Everything else essentially gets treated like regular code. That's still not terribly satisfying since unlike other forms of software you now have all that definition both in your VCS and the database itself, but ISTM that's a much bigger problem than the small amount of info we lose from stored functions... -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Re: [HACKERS] psql :: support for \ev viewname and \sv viewname
On Mon, May 4, 2015 at 6:26 AM, Pavel Stehule pavel.steh...@gmail.com wrote: 2015-05-04 11:21 GMT+02:00 Petr Korobeinikov pkorobeini...@gmail.com: Hackers! 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) +1 +1 During the FISL13 [1] (year 2012) me and other friends (Leonardo César, Dickson Guedes and Fernando Ike) implemented a very WIP patch [2] to support the \ev subcommand in psql. Unfortunately we didn't go ahead with this work. :-( I'll do some reviews. Regards, [1] http://softwarelivre.org/fisl13?lang=en [2] https://github.com/lhcezar/postgres/commit/b4bfc3b17b4a32850d6035165209b2b82746190a -- 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] Unexpected speed PLAIN vs. MAIN
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. 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] BUG in XLogRecordAssemble
On 05/04/2015 07:04 PM, Zhang Zq wrote:
hi,
I found the code in 'backend/access/transam/xloginsert.c' as that:
XLogRecordAssemble:
if (prev_regbuf RelFileNodeEquals(regbuf-rnode, prev_regbuf-rnode))
{
samerel = true;
bkpb.fork_flags |= BKPBLOCK_SAME_REL;
prev_regbuf = regbuf;
}
else
samerel = false;
There is the only place that prev_regbuf is assigned, so prev_regbuf will never
be assigned.
The patch will fix it, Thanks.
Thanks, good catch! Committed.
- 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 Fri, May 1, 2015 at 10:41 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: At 2015-05-01 09:57:28 -0400, robertmh...@gmail.com wrote: If you don't object to this version, I'll commit it. Looks fine to me, thank you. OK, committed and back-patched. As for the non-backpatchable 0002, I agree with Andres that it should be included in 9.5; but I take it you're still not convinced? No, I'm not convinced. That patch will protect you in one extremely specific scenario: you turn off fsync, do some stuff, shut down, turn fsync back on again, and start up. But it won't protect you if you crash while fsync is off, or after you shut down with fsync=off and before you restart with fsync=on. And there's no documentation change here that would help anyone distinguish between the situations in which they are protected and the situations in which they are not protected. Without that, a lot of people are going to get this wrong. As an alternative, how about fsync=shutdown parameter? This could be documented to fsync the data directory at shutdown. It could document that there is a risk of corruption if the server crashes, but that the database is OK if shut down cleanly. fsync=off could document that you must run initdb --sync-only after shutting down, else you are unsafe. I'm not wedded to any particular solution, but an undocumented hack that some people will manage to use safely some of the time doesn't seem good enough to me. -- 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] deparsing utility commands
Here's a cleaned up version of this patch; I threw together a very quick
test module, and updated a conflicting OID. As far as I can tell, I'm
only missing the documentation updates before this is push-able.
One change to note is that the AlterTable support used to ignore
commands that didn't match the OID as set by
EventTriggerAlterTableRelid(); the comment there said that the point was
to avoid collecting the same commands to child tables as recursion
occured in execution. I think that would be imposing such a decision;
perhaps some users of this infrastructure want to know about the
operations as they happen on child tables too. With this definition it
is up to the user module to ignore the duplicates.
Thanks for your review. In reply to your comments:
Amit Kapila wrote:
Few Comments/Questions regrading first 2 patches:
Regarding Patch 0001-deparse-infrastructure-needed-for-command-deparsing
1.
+ * Currently, sql_drop, table_rewrite, ddL_command_end events are the only
/ddL_command_end/ddl_command_end
'L' should be in lower case.
True. Fixed.
2.
+ * FIXME this API isn't considering the possibility that a xact/subxact is
+ * aborted partway through. Probably it's best to add an
+ * AtEOSubXact_EventTriggers() to fix this.
+ */
+void
+EventTriggerAlterTableEnd(void)
{
..
}
Wouldn't the same fix be required for RollbackToSavePoint case
as well? Do you intend to fix this in separate patch?
Acrually, I figured that this is not at issue. When a subxact is rolled
back, the whole currentEventTriggerState thing is reverted to a previous
item in the stack; if an event trigger is fired by ALTER TABLE, and the
resulting function invokes ALTER TABLE again, they collect their
commands in separate state elements, so there is never a conflict.
I can't think of any situation in which one event trigger function adds
elements to the list of the calling command.
3.
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
Copyright notice years should be same.
Yeah, fixed.
4.
+ /*
+ * copying the node is moderately challenging ... should we consider
+ * changing InternalGrant into a full-fledged node instead?
+ */
+ icopy = palloc(sizeof(InternalGrant));
+ memcpy(icopy, istmt, sizeof(InternalGrant));
+ icopy-objects = list_copy(istmt-objects);
Don't we need to copy (AclMode privileges;)?
AFAICT that's copied by memcpy.
5.
-static void AlterOpFamilyAdd(List *opfamilyname, Oid amoid, Oid
opfamilyoid,
+static void AlterOpFamilyAdd(AlterOpFamilyStmt *stmt,
+ List *opfamilyname, Oid amoid, Oid opfamilyoid,
int maxOpNumber, int maxProcNumber,
List *items);
-static void AlterOpFamilyDrop(List *opfamilyname, Oid amoid, Oid
opfamilyoid,
+static void AlterOpFamilyDrop(AlterOpFamilyStmt *stmt,
+ List *opfamilyname, Oid amoid, Oid opfamilyoid,
int maxOpNumber, int maxProcNumber,
List *items);
Now that both the above functions have parameter AlterOpFamilyStmt *stmt,
so can't we get rid of second parameter List *opfamilyname as that
is part of structure AlterOpFamilyStmt?
Yeah, I considered that as I wrote the code but dropped it out of
laziness. I have done so now.
6.
@@ -1175,204 +1229,258 @@ ProcessUtilitySlow(Node *parsetree,
..
+ EventTriggerAlterTableStart(parsetree);
+ address =
+ DefineIndex(relid, /* OID of heap relation */
+ stmt,
+ InvalidOid, /* no predefined OID */
+ false, /* is_alter_table */
+ true, /* check_rights */
+ false, /* skip_build */
+ false); /* quiet */
+ /*
+ * Add the CREATE INDEX node itself to stash right away; if
+ * there were any commands stashed in the ALTER TABLE code,
+ * we need them to appear after this one.
+ */
+ EventTriggerCollectSimpleCommand(address, secondaryObject,
+ parsetree);
+ commandCollected = true;
+ EventTriggerAlterTableEnd();
Is there a reason why EventTriggerAlterTableRelid() is not called
after EventTriggerAlterTableStart() in this flow?
All paths that go through AlterTableInternal() have the Oid set by that
function.
7.
+void
+EventTriggerInhibitCommandCollection(void)
+void
+EventTriggerUndoInhibitCommandCollection(void)
These function names are understandable, some alternative names could be
InhibitEventTriggerCommandCollection(),
PreventEventTriggerCommandCollection(),
or ProhibitEventTriggerCommandCollection() if you prefer anyone of these
over others.
Hmm, most of the reason I picked these names is the EventTrigger prefix.
8.
case T_CreateOpClassStmt:
- DefineOpClass((CreateOpClassStmt *) parsetree);
+ address = DefineOpClass((CreateOpClassStmt *) parsetree);
+ /* command is stashed in DefineOpClass */
+ commandCollected = true;
Is there a need to remember address if command is already
collected?
None. Removed that.
Regarding Patch 0002-changes-to-core-to-support-the-deparse-extension:
I decided against committing 0002 patch for
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
* Peter Eisentraut (pete...@gmx.net) wrote: On 4/30/15 6:05 AM, Fujii Masao wrote: The specification of session audit logging seems to be still unclear to me. As I had mentioned previously, I would prefer session audit logging to be integrated with the normal statement logging configuration. I'd certainly love to see the logging in core be improved, but that's also rather tangential to this extension and we could certainly have both quite happily. Further, being able to configure and have consistent information from the extension is valuable even if options are added to the in-core logging to make it more flexible. One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), but it seems unlikely to seriously reduce the usefulness of session logging being in the extension and producing consistent output with the object logging. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
* Peter Eisentraut (pete...@gmx.net) wrote: On 5/4/15 3:00 PM, Stephen Frost wrote: One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), Well yeah, from my perspective, the reason not much has happened in the area of logging is that you and Magnus have repeatedly said you were planning some things. If that was holding anyone back from working on it, then I'm truely sorry. I would encourage anyone interesting in any particular feature to speak up and ask what the status is and if others are working on something, especially if they have time to spend advancing it. I was certainly quite happy when Abhijit posted the initial version of this nearly a year ago as it showed that there were individuals able to spend substantial time on it, as well as a use-case which would be solved through such an extension. I don't wish to lay claim to any particular feature nor to make any guarantees, but I will say that I'm happy to have moved into a position in the past year where I can devote a great deal more in time and resources towards PostgreSQL than I've ever been able to in the past. The reasons given above against changing logging just as easily apply to auditing. 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. This implementation is only a starting point. We don't know whether it will fulfill anyone's requirements. The requirements for logging are it feels good enough for an admin. The requirements for auditing are it satisfies this checklist. We need to be prepared to aggressively evolve this as we gather more information about requirements. Otherwise this will become something like contrib/isn, where we know it doesn't satisfy real-world uses anymore, but we're afraid to touch it because someone might be using it and we don't have the domain knowledge to tell them to stop. I agree that this is a starting point. From the discussions which I've had with PostgreSQL users (both our clients and others), this does fulfill a set of requirements for them. That isn't to say that it's a complete and total solution, nor that we will stop here (we certainly won't!), but I'm confident it *is* solving a real use-case for our users. I don't mean to speak for them, but my understanding is that the work which was done by Abhijit and 2ndQ was sponsored by an EU grant which had a specific set of requirements which this is intended to satisfy. Further, we are absolutely prepared to aggressively evolve this as we learn and understand how it's being used out in the field- but I don't believe we're ever going to really understand that until we put something out there. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] BRIN range operator class
Hi, 2015-05-05 2:51 GMT+02:00 Andreas Karlsson andr...@proxel.se: From my point of view as a reviewer this patch set is very close to being committable. I'd like to thank already now to all committers and reviewers and hope BRIN makes it into PG 9.5. As a database instructor, conference organisator and geospatial specialist I'm looking forward for this clever new index. I'm keen to see if a PostGIS specialist jumps in and adds PostGIS geometry support. Yours, S. 2015-05-05 2:51 GMT+02:00 Andreas Karlsson andr...@proxel.se: From my point of view as a reviewer this patch set is very close to being committable. = brin-inclusion-v06-01-sql-level-support-functions.patch This patch looks good. = brin-inclusion-v06-02-strategy-numbers.patch This patch looks good, but shouldn't it be merged with 07? = brin-inclusion-v06-03-remove-assert-checking.patch As you wrote earlier this is needed because the new range indexes would violate the asserts. I think it is fine to remove the assertion. = brin-inclusion-v06-04-fix-brin-deform-tuple.patch This patch looks good and can be committed separately. = brin-inclusion-v06-05-box-vs-point-operators.patch This patch looks good and can be committed separately. = brin-inclusion-v06-06-inclusion-opclasses.patch - operator classes store the union of the values in the indexed column is not technically true. It stores something which covers all of the values. - Missing space in except box and point*/. - Otherwise looks good. = brin-inclusion-v06-07-remove-minmax-amprocs.patch Shouldn't this be merged with 02? Otherwise it looks good. -- 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 -- 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
From my point of view as a reviewer this patch set is very close to being committable. = brin-inclusion-v06-01-sql-level-support-functions.patch This patch looks good. = brin-inclusion-v06-02-strategy-numbers.patch This patch looks good, but shouldn't it be merged with 07? = brin-inclusion-v06-03-remove-assert-checking.patch As you wrote earlier this is needed because the new range indexes would violate the asserts. I think it is fine to remove the assertion. = brin-inclusion-v06-04-fix-brin-deform-tuple.patch This patch looks good and can be committed separately. = brin-inclusion-v06-05-box-vs-point-operators.patch This patch looks good and can be committed separately. = brin-inclusion-v06-06-inclusion-opclasses.patch - operator classes store the union of the values in the indexed column is not technically true. It stores something which covers all of the values. - Missing space in except box and point*/. - Otherwise looks good. = brin-inclusion-v06-07-remove-minmax-amprocs.patch Shouldn't this be merged with 02? Otherwise it looks good. -- 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] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0
On Fri, May 1, 2015 at 7:49 AM, Andres Freund and...@anarazel.de wrote: seems weird for both the BEFORE INSERT and BEFORE UPDATE triggers to get a crack at the same tuple, so your way might be better after all. But on the other hand, the BEFORE INSERT trigger might have had side effects, so we can't just pretend it didn't happen. Well, I think it's pretty unavoidable to fire both. On that part I think were pretty lengthy discussions a year or so back. One idea is to decide that an INSERT with an ON CONFLICT UPDATE handler is still an INSERT. Period. So the INSERT triggers run, the UPDATE triggers don't, and that's it. I think that'd be much worse. A question has come up about RTEs, column-level privileges and BEFORE triggers. This commit message gives a summary: https://github.com/petergeoghegan/postgres/commit/87b9f27055e81d1396db3d10a5e9d01c52603783 I'm pretty sure that I'll have to require SELECT privileges of the EXCLUDED.* RTE too (i.e. revert that commit, and probably document the new behavior of requiring that). I think it's worth getting feedback on this point, though, since it is a somewhat subtle issue. -- 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
Stefan Keller wrote: Hi, 2015-05-05 2:51 GMT+02:00 Andreas Karlsson andr...@proxel.se: From my point of view as a reviewer this patch set is very close to being committable. I'd like to thank already now to all committers and reviewers and hope BRIN makes it into PG 9.5. As a database instructor, conference organisator and geospatial specialist I'm looking forward for this clever new index. Appreciated. The base BRIN code is already in 9.5, so barring significant issues you should see it in the next major release. Support for geometry types and the like is still pending, but I hope to get to it shortly. 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. -- Á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 UPDATE/IGNORE 4.0
On 2015-05-04 19:13:27 -0700, Peter Geoghegan wrote: A question has come up about RTEs, column-level privileges and BEFORE triggers. This commit message gives a summary: https://github.com/petergeoghegan/postgres/commit/87b9f27055e81d1396db3d10a5e9d01c52603783 I'm pretty sure that I'll have to require SELECT privileges of the EXCLUDED.* RTE too (i.e. revert that commit, and probably document the new behavior of requiring that). I think it's worth getting feedback on this point, though, since it is a somewhat subtle issue. I think it's pretty clear that we'll have to require that. 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 syntax issues
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 (Andres and I have been doing a lot of clean-up. I point this change in particular out because it's a behavioral change). The INSERT documentation has been updated to reflect this, and includes an example. This copy of the documentation is current: http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html -- 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] PGCon 2015
In 6 weeks, people start arriving in Ottawa for PGCon 2015. Have you registered? There's still time. Get in today. We have a great list of talks: http://www.pgcon.org/2015/schedule/events.en.html Given by great speakers: http://www.pgcon.org/2015/schedule/speakers.en.html You'll want to be there. Don't leave it much longer. — Dan Langille http://langille.org/ signature.asc Description: Message signed with OpenPGP using GPGMail
[HACKERS] tzdata and 9.4.2, etc.
Folks, We are now three tzdata changes behind. There are bugs in pg_dump which create real restore errors for people using PostGIS, one of our most popular extensions. Can we please wrap and ship 9.4.2, etc., and do it soon? 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 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] Auditing extension for PostgreSQL (Take 2)
On 5/4/15 3:00 PM, Stephen Frost wrote: One particular advantage of having the extension is that having it doesn't impact existing users of the in-core logging system. I don't recall any specific proposals for improving the in-core logging system to add the capabilities for session logging that the extension provides, but it seems likely to require changes to at least the CSV format, new log_line_prefix escape codes (which we're using quite a few of already...), new GUCs, and potentially behavior changes to make it work. As I say above, I'm happy to have those discussions (and I've been party to them quite a few times in the past...), Well yeah, from my perspective, the reason not much has happened in the area of logging is that you and Magnus have repeatedly said you were planning some things. The reasons given above against changing logging just as easily apply to auditing. This implementation is only a starting point. We don't know whether it will fulfill anyone's requirements. The requirements for logging are it feels good enough for an admin. The requirements for auditing are it satisfies this checklist. We need to be prepared to aggressively evolve this as we gather more information about requirements. Otherwise this will become something like contrib/isn, where we know it doesn't satisfy real-world uses anymore, but we're afraid to touch it because someone might be using it and we don't have the domain knowledge to tell them to stop. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
Did this every go any further? I wrote some data transformation script at work, and after seeing with count -2017657667 (and similar) in my scripts log I got a bit worried. seeing else where were folks just run a full on count(*) later to check counts but that is even MORE time and I was thinking it was a psycopg2 problem, but seems there are issues with the internal counters in pg as well for tracking large changes. thanks, Mark On Sun, Feb 2, 2014 at 9:12 AM, Tom Lane t...@sss.pgh.pa.us wrote: Vik Fearing vik.fear...@dalibo.com writes: Without re-doing the work, my IRC logs show that I was bothered by this in src/backend/tcop/postgres.c: max_rows = pq_getmsgint(input_message, 4); I needed to change max_rows to int64 which meant I had to change pq_getmsgint to pq_getmsgint64 which made me a little worried. As well you should be, because we are *not* doing that. That would be a guaranteed-incompatible protocol change. Fortunately, I don't see any functional need for widening the row-limit field in execute messages; how likely is it that someone wants to fetch exactly 3 billion rows? The practical use-cases for nonzero row limits generally involve fetching a bufferload worth of data at a time, so that the restriction to getting no more than INT_MAX rows at once is several orders of magnitude away from being a problem. The same goes for internal uses of row limits, which makes it questionable whether it's worth changing the width of ExecutorRun's count parameter, which is what I assume you were on about here. But in any case, if we did that we'd not try to reflect it as far as here, because the message format specs can't change. 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] INSERT ... ON CONFLICT syntax issues
On Wed, Apr 29, 2015 at 12:09 PM, Robert Haas robertmh...@gmail.com wrote: * Don't change the names of the pseudo-alias EXCLUDED.* (or the alias TARGET.*). Those seem fine to me as well. There seem to be a few votes for NEW and OLD. That's what I proposed originally, and (surprise, surprise) I still like that better too. That makes the following valid: INSERT INTO distributors (did, dname) VALUES (5, 'Gizmo transglobal') ON CONFLICT (did) DO UPDATE SET dname = NEW.dname RETURNING OLD.dname; So you're projecting OLD.dname from RETURNING, here -- so OLD refers to the row added back to the relation on update (or perhaps the row simply inserted). That's pretty bad. I really don't want to add a kludge to make the target relation have an alias in one context but not in the other. -- 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] Providing catalog view to pg_hba.conf file - Patch submission
On 5/1/15 12:33 PM, Andres Freund wrote: On 2015-04-08 19:19:29 +0100, Greg Stark wrote: I'm not sure what the best way to handle the hand-off from patch contribution to reviewer/committer. If I start tweaking things then you send in a new version it's actually more work to resolve the conflicts. I think at this point it's easiest if I just take it from here. Are you intending to commit this? It still looks quite dubious to me. The more I test this, the more fond I grow of the idea of having this information available in SQL. But I'm also growing more perplexed by how this the file is mapped to a table. It just isn't a good match. For instance: What is keyword_databases? Why is it an array? Same for keyword_users. How can I know whether a given database or user matches a keyword? What is compare_method? (Should perhaps be keyword_address?) Why is compare method set to mask when a hostname is set? (Column order is also a bit confusing here.) I'd also like options to be jsonb instead of a text array. Ultimately, I don't see how this is better than just showing the raw file. I don't see a sensible way to compute answers to questions such as What is the authentication method for user X from IP address Y. If I can't do that, what's the point of having a processed version? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use outerPlanState() consistently in executor code
On Fri, May 1, 2015 at 1:05 PM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: On Thu, Apr 30, 2015 at 5:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: I think I'd have done many of these as + PlanState *outerPlan = outerPlanState(node); rather than finding assorted random places to initialize the variables. Agreed. Attached patch is revision along this line. Except for a few that delayed assignments does not look a random kludge, I moved most of others together with the declaration. I fixed several whitespace errors, reverted the permissions changes you included, adjusted the remaining call site to be the way Tom wants (and I think he's right), and committed this. -- 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 UPDATE/IGNORE 4.0
On Mon, May 4, 2015 at 9:00 PM, Andres Freund and...@anarazel.de wrote: I think it's pretty clear that we'll have to require that. Okay, then. I'll push out revised testing of column-level privileges later. (Andres rebased, and we're now pushing code to: https://github.com/petergeoghegan/postgres/commits/insert_conflict_5). -- 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
