Re: [HACKERS] Missing psql tab-completion for ALTER FOREIGN TABLE
On 2015/04/30 1:59, Robert Haas wrote: On Mon, Apr 27, 2015 at 2:50 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Here is a patch to add missing tab-completion for ALTER FOREIGN TABLE. I'll add this to the next CF. Committed, thanks. Thanks! Best regards, Etsuro Fujita -- 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] [RFC] sepgsql: prohibit users to relabel objects
Oh, I wasn't aware of that. Any hints where to look at? Thanks! PS: sorry for top posting. - Original Message - From: Robert Haas robertmh...@gmail.com To: Denis Kirjanov k...@linux-powerpc.org Cc: pgsql-hackers@postgresql.org, Alexey Zhuchkov ale...@itsirius.su, Denis Kirjanov k...@itsirius.su Sent: Wednesday, April 29, 2015 9:01:36 PM Subject: Re: [HACKERS] [RFC] sepgsql: prohibit users to relabel objects On Wed, Apr 29, 2015 at 9:15 AM, Denis Kirjanov k...@linux-powerpc.org wrote: Enforce access control on security labels defined by admin and prohibit users to relabel the objects Really? Why? I would think it's the policy's job to restrict relabel operations. -- 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] How about to have relnamespace and relrole?
Hello, I fonund that pg_proc.h got modified so rebased and
rearranged the patchset merging the recent fixes.
regards,
I sent the previous mail unfinished.
At Thu, 09 Apr 2015 17:25:10 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote in
20150409.172510.29010318.horiguchi.kyot...@lab.ntt.co.jp
Hello, sorry for the absence. I changed the regnamespace's
behavior as the same as the other reg* types. And I attached a
patch as separate one that fixes regroleout to do the same as the
other reg* types, because I have
because, I doubt that it is appropriate way to do so.
0001-Add-regrole_v6.patch : fix regnamespace to behave as the
same as the other reg* types.
0001-Make-regnamespace-behave-as-the-same-as-other-reg-ty.patch:
Diff from v5 to v6, only for convinience.
0001-Fix-regroleout-s-behavior-following-other-out-functi.patch:
Fixes regroleout so as to behave as the same as other reg*
types, but might be a bit too large.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 2210bc524906ec1c9fdf4649260568b0ba807c30 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp
Date: Wed, 18 Feb 2015 14:38:32 +0900
Subject: [PATCH 1/2] Add regrole-v7
Add new OID aliass type regrole. The new type has the scope of whole
the database cluster so it doesn't behave as the same as the existing
OID alias types which have database scope, concerning object
dependency. To get rid of confusion, inhibit constants of the new type
from appearing where dependencies are made involving it.
Documentation about this new type is added and some existing
descriptions are modified according to the restriction of the
type. Addition to it, put a note about possible MVCC violation and
optimization issues, which are general over the all reg* types.
---
contrib/spi/insert_username.c | 2 +-
contrib/spi/timetravel.c | 2 +-
doc/src/sgml/datatype.sgml| 27 +++--
src/backend/bootstrap/bootstrap.c | 2 +
src/backend/catalog/dependency.c | 10
src/backend/catalog/objectaddress.c | 20 +++
src/backend/utils/adt/acl.c | 2 +-
src/backend/utils/adt/name.c | 4 +-
src/backend/utils/adt/regproc.c | 104 ++
src/backend/utils/adt/selfuncs.c | 2 +
src/backend/utils/cache/catcache.c| 1 +
src/backend/utils/init/miscinit.c | 24 +---
src/include/catalog/pg_cast.h | 7 +++
src/include/catalog/pg_proc.h | 12
src/include/catalog/pg_type.h | 5 ++
src/include/foreign/foreign.h | 2 +-
src/include/miscadmin.h | 2 +-
src/include/utils/builtins.h | 5 ++
src/test/regress/expected/regproc.out | 26 -
src/test/regress/sql/regproc.sql | 7 +++
20 files changed, 235 insertions(+), 31 deletions(-)
diff --git a/contrib/spi/insert_username.c b/contrib/spi/insert_username.c
index 8752078..3812525 100644
--- a/contrib/spi/insert_username.c
+++ b/contrib/spi/insert_username.c
@@ -79,7 +79,7 @@ insert_username(PG_FUNCTION_ARGS)
args[0], relname)));
/* create fields containing name */
- newval = CStringGetTextDatum(GetUserNameFromId(GetUserId()));
+ newval = CStringGetTextDatum(GetUserNameFromId(GetUserId(), false));
/* construct new tuple */
rettuple = SPI_modifytuple(rel, rettuple, 1, attnum, newval, NULL);
diff --git a/contrib/spi/timetravel.c b/contrib/spi/timetravel.c
index 0699438..e125986 100644
--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -174,7 +174,7 @@ timetravel(PG_FUNCTION_ARGS)
}
/* create fields containing name */
- newuser = CStringGetTextDatum(GetUserNameFromId(GetUserId()));
+ newuser = CStringGetTextDatum(GetUserNameFromId(GetUserId(), false));
nulltext = (Datum) NULL;
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index da1f25f..0cac993 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4321,8 +4321,9 @@ SET xmloption TO { DOCUMENT | CONTENT };
an object identifier. There are also several alias types for
typeoid/: typeregproc/, typeregprocedure/,
typeregoper/, typeregoperator/, typeregclass/,
-typeregtype/, typeregconfig/, and typeregdictionary/.
-xref linkend=datatype-oid-table shows an overview.
+typeregtype/, typeregrole/, typeregconfig/, and
+typeregdictionary/. xref linkend=datatype-oid-table shows
+an overview.
/para
para
@@ -4431,6 +4432,13 @@ SELECT * FROM pg_attribute
/row
row
+entrytyperegrole//entry
+entrystructnamepg_authid//entry
+entryrole name/entry
+entryliteralsmithee//entry
+ /row
+
+ row
entrytyperegconfig//entry
entrystructnamepg_ts_config//entry
entrytext search configuration/entry
@@ -4448,7 +4456,8 @@ SELECT * FROM pg_attribute
/table
para
-
Re: [HACKERS] Minor improvement to config.sgml
On 2015/04/30 7:06, Robert Haas wrote: On Thu, Apr 16, 2015 at 3:30 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Attached is a small patch to mark up on with literal in doc/src/sgml/config.sgml. Committed. Thanks for picking this up! Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PL/pgSQL, RAISE and error context
2015-04-30 10:24 GMT+02:00 Marko Tiikkaja ma...@joh.to:
Hi Pavel,
This doesn't seem to be what I thought we had agreed on. For example:
=# create function barf() returns void as $$ begin raise notice without
context 'hello world'; end $$ language plpgsql;
CREATE FUNCTION
=# create function foof() returns void as $$ begin perform barf(); end $$
language plpgsql;
CREATE FUNCTION
=# select foof();
NOTICE: hello world
CONTEXT: SQL statement SELECT barf()
PL/pgSQL function foof() line 1 at PERFORM
It's not only clear that WITHOUT CONTEXT didn't really work here, but it
also had absolutely no effect since the context within barf() is also
displayed.
It doesn't look well - because it should be solve by errhidecontext(true)
yes, there is a issue in send_message_to_frontend - this ignore
edata-hide_ctx field. After fixing, it working as expected - so this is a
bug in implementation of errhidecontext()
should be
if (edata-context !edata-hide_ctx)
{
pq_sendbyte(msgbuf, PG_DIAG_CONTEXT);
err_sendstring(msgbuf, edata-context);
}
and probably getting stack in err_finish should be fixed too:
if (!edata-hide_ctx)
for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext-previous)
(*econtext-callback) (econtext-arg);
Regards
Pavel
I'll look on this issue.
Regards
Pavel
.m
Re: [HACKERS] PL/pgSQL, RAISE and error context
2015-04-28 19:44 GMT+02:00 Jim Nasby jim.na...@bluetreble.com: On 4/28/15 1:16 AM, Pavel Stehule wrote: I think it can't be any clearer than the proposed plpgsql.display_context_min_messages client_min_context. It's doing the same thing as min_messages does, just for context instead of the message. Or does this affect client and log the same way? it affect client and log together maybe min_context +1 third variant with GUC plpgsql.min_context Regards Pavel -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com commit c2f49938f636864234d03994d2f64f8095392d11 Author: Pavel Stehule pavel.steh...@gooddata.com Date: Sat Apr 25 22:09:28 2015 +0200 initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement. initial implementation of plpgsql GUC plpgsql.min_context diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index d36acf6..ffc3eb8 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3406,10 +3406,10 @@ END LOOP optional replaceablelabel/replaceable /optional; raise errors. synopsis -RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; -RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; -RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; -RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional; +RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; +RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; +RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional; +RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional; RAISE ; /synopsis @@ -3431,6 +3431,18 @@ RAISE ; /para para +The options literalWITH CONTEXT/literal or literalWITHOUT CONTEXT/literal +can enforce or suppress context information related to error or notice. This possibility +can be forced by settings of configuration parameter literalplpgsql.min_context/. +This allows same values like replaceable class=parameterlevel/replaceable option plus +value literalnone/literal that is a default. When it is changed, then all errors and notices +with higher than specified severity are raised with context info. +programlisting +RAISE NOTICE WITH CONTEXT 'This message will have a context'; +/programlisting + /para + + para After replaceable class=parameterlevel/replaceable if any, you can write a replaceable class=parameterformat/replaceable (which must be a simple string literal, not an expression). The diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index deefb1f..eaee5a7 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt) char *err_table = NULL; char *err_schema = NULL; ListCell *lc; + bool hide_ctx = true; /* suppress context by default */ /* RAISE with no parameters: re-throw current exception */ if (stmt-condname == NULL stmt-message == NULL @@ -3080,10
Re: [HACKERS] ATSimpleRecursion() and inheritance foreign parents
On 2015/04/29 4:35, Tom Lane wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/04/28 15:17, Amit Langote wrote: The code at the beginning of ATSimpleRecursion() looks like - if (recurse rel-rd_rel-relkind == RELKIND_RELATION) Not sure if it's great idea, but now that foreign tables can also have children, should above be changed to take that into account? Yeah, I think we should now allow the recursion for inheritance parents that are foreign tables as well. Attached is a patch for that. Yeah, this is just an oversight. Fix pushed, and also a similar fix in parse_utilcmd.c. Thanks! Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Missing importing option of postgres_fdw
On 2015/04/30 2:10, Robert Haas wrote: On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier michael.paqu...@gmail.com wrote: Authorizing ALTER FOREIGN TABLE as query string that a FDW can use with IMPORT FOREIGN SCHEMA is a different feature than what is proposed in this patch, aka an option for postgres_fdw and meritates a discussion on its own because it impacts all the FDWs and not only postgres_fdw. Now, related to this patch, we could live without authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does authorize the definition of CHECK constraints. I agree. I don't think there's a huge problem with allowing IMPORT FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it doesn't really seem to be necessary. I don't see why we can't just declare the CHECK constraints in the CREATE FOREIGN TABLE statement instead of adding more DDL. I think that it'd improve the convenience of an FDW developer writing ImportForeignSchema() to allow it to return ALTER FOREIGN TABLE (and perhaps DROP FOREIGN TABLE) as well, but I agree that that needs another discussion. So I'll leave that as is and update the patch as discussed above. Thanks for the comments! Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PL/pgSQL, RAISE and error context
Hi Pavel, This doesn't seem to be what I thought we had agreed on. For example: =# create function barf() returns void as $$ begin raise notice without context 'hello world'; end $$ language plpgsql; CREATE FUNCTION =# create function foof() returns void as $$ begin perform barf(); end $$ language plpgsql; CREATE FUNCTION =# select foof(); NOTICE: hello world CONTEXT: SQL statement SELECT barf() PL/pgSQL function foof() line 1 at PERFORM It's not only clear that WITHOUT CONTEXT didn't really work here, but it also had absolutely no effect since the context within barf() is also displayed. .m -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor typo in doc: replication-origins.sgml
On Thu, Apr 30, 2015 at 7:23 AM, Amit Langote langote_amit...@lab.ntt.co.jp wrote: Hi, Attached does: s/pg_replication_origin_xact-setup/pg_replication_origin_xact_setup/g or, (s/-/_/g) Applied, thanks. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Auditing extension for PostgreSQL (Take 2)
On Thu, Apr 30, 2015 at 12:57 PM, Sawada Masahiko sawada.m...@gmail.com wrote: On Wed, Apr 29, 2015 at 12:17 AM, David Steele da...@pgmasters.net wrote: On 4/28/15 2:14 AM, Sawada Masahiko wrote: On Fri, Apr 24, 2015 at 3:23 AM, David Steele da...@pgmasters.net wrote: I've also added some checking to make sure that if anything looks funny on the stack an error will be generated. Thanks for the feedback! Thank you for updating the patch! I ran the postgres regression test on database which is enabled pg_audit, it works fine. Looks good to me. If someone don't have review comment or bug report, I will mark this as Ready for Committer. Thank you! I appreciate all your work reviewing this patch and of course everyone else who commented on, reviewed or tested the patch along the way. I have changed the status this to Ready for Committer. The specification of session audit logging seems to be still unclear to me. For example, why doesn't session audit logging log queries accessing to a catalog like pg_class? Why doesn't it log any queries executed in aborted transaction state? Since there is no such information in the document, I'm afraid that users would easily get confused with it. Even if we document it, I'm not sure if the current behavior is good for the audit purpose. For example, some users may want to log even queries accessing to the catalogs. I have no idea about when the current CommitFest will end. But probably we don't have that much time left. So I'm thinking that maybe we should pick up small, self-contained and useful part from the patch and focus on that. If we try to commit every features that the patch provides, we might get nothing at least in 9.5, I'm afraid. Regards, -- Fujii Masao -- 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] Freeze avoidance of very large table.
On Fri, Apr 24, 2015 at 11:21 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
On Fri, Apr 24, 2015 at 1:31 AM, Jim Nasby jim.na...@bluetreble.com wrote:
On 4/23/15 11:06 AM, Petr Jelinek wrote:
On 23/04/15 17:45, Bruce Momjian wrote:
On Thu, Apr 23, 2015 at 09:45:38AM -0400, Robert Haas wrote:
Agreed, no extra file, and the same write volume as currently. It would
also match pg_clog, which uses two bits per transaction --- maybe we can
reuse some of that code.
Yeah, this approach seems promising. We probably can't reuse code from
clog because the usage pattern is different (key for clog is xid, while
for visibility/freeze map ctid is used). But visibility map storage
layer is pretty simple so it should be easy to extend it for this use.
Actually, there may be some bit manipulation functions we could reuse;
things like efficiently counting how many things in a byte are set. Probably
doesn't make sense to fully refactor it, but at least CLOG is a good source
for cut/paste/whack.
I agree with adding a bit that indicates corresponding page is
all-frozen into VM, just like CLOG.
I'll change the patch as second version patch.
The second patch is attached.
In second patch, I added a bit that indicates all tuples in page are
completely frozen into visibility map.
The visibility map became a bitmap with two bit per heap page:
all-visible and all-frozen.
The logics around vacuum, insert/update/delete heap are almost same as
previous version.
This patch lack some point: documentation, comment in source code,
etc, so it's WIP patch yet,
but I think that it's enough to discuss about this.
Please feedbacks.
Regards,
---
Sawada Masahiko
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b504ccd..a06e16d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -86,7 +86,8 @@ static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tup,
-bool all_visible_cleared, bool new_all_visible_cleared);
+bool all_visible_cleared, bool new_all_visible_cleared,
+bool all_frozen_cleared, bool new_all_frozen_cleared);
static void HeapSatisfiesHOTandKeyUpdate(Relation relation,
Bitmapset *hot_attrs,
Bitmapset *key_attrs, Bitmapset *id_attrs,
@@ -2068,7 +2069,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
HeapTuple heaptup;
Buffer buffer;
Buffer vmbuffer = InvalidBuffer;
- bool all_visible_cleared = false;
+ bool all_visible_cleared = false,
+all_frozen_cleared = false;
/*
* Fill in tuple header fields, assign an OID, and toast the tuple if
@@ -2092,8 +2094,9 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
CheckForSerializableConflictIn(relation, NULL, InvalidBuffer);
/*
- * Find buffer to insert this tuple into. If the page is all visible,
- * this will also pin the requisite visibility map page.
+ * Find buffer to insert this tuple into. If the page is all visible
+ * of all frozen, this will also pin the requisite visibility map and
+ * frozen map page.
*/
buffer = RelationGetBufferForTuple(relation, heaptup-t_len,
InvalidBuffer, options, bistate,
@@ -2110,7 +2113,16 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
PageClearAllVisible(BufferGetPage(buffer));
visibilitymap_clear(relation,
ItemPointerGetBlockNumber((heaptup-t_self)),
- vmbuffer);
+ vmbuffer, VISIBILITYMAP_ALL_VISIBLE);
+ }
+
+ if (PageIsAllFrozen(BufferGetPage(buffer)))
+ {
+ all_frozen_cleared = true;
+ PageClearAllFrozen(BufferGetPage(buffer));
+ visibilitymap_clear(relation,
+ ItemPointerGetBlockNumber((heaptup-t_self)),
+ vmbuffer, VISIBILITYMAP_ALL_FROZEN);
}
/*
@@ -2157,6 +2169,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
xlrec.offnum = ItemPointerGetOffsetNumber(heaptup-t_self);
xlrec.flags = all_visible_cleared ? XLOG_HEAP_ALL_VISIBLE_CLEARED : 0;
+ if (all_frozen_cleared)
+ xlrec.flags |= XLOG_HEAP_ALL_FROZEN_CLEARED;
Assert(ItemPointerGetBlockNumber(heaptup-t_self) == BufferGetBlockNumber(buffer));
/*
@@ -2350,7 +2364,8 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
{
Buffer buffer;
Buffer vmbuffer = InvalidBuffer;
- bool all_visible_cleared = false;
+ bool all_visible_cleared = false,
+ all_frozen_cleared = false;
int nthispage;
CHECK_FOR_INTERRUPTS();
@@ -2395,7 +2410,16 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
PageClearAllVisible(page);
visibilitymap_clear(relation,
BufferGetBlockNumber(buffer),
-vmbuffer);
+vmbuffer, VISIBILITYMAP_ALL_VISIBLE);
+ }
+
+ if (PageIsAllFrozen(page))
+ {
+ all_frozen_cleared = true;
+ PageClearAllFrozen(page);
+
Re: [HACKERS] PL/pgSQL, RAISE and error context
2015-04-30 10:50 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
2015-04-30 10:24 GMT+02:00 Marko Tiikkaja ma...@joh.to:
Hi Pavel,
This doesn't seem to be what I thought we had agreed on. For example:
=# create function barf() returns void as $$ begin raise notice without
context 'hello world'; end $$ language plpgsql;
CREATE FUNCTION
=# create function foof() returns void as $$ begin perform barf(); end $$
language plpgsql;
CREATE FUNCTION
=# select foof();
NOTICE: hello world
CONTEXT: SQL statement SELECT barf()
PL/pgSQL function foof() line 1 at PERFORM
It's not only clear that WITHOUT CONTEXT didn't really work here, but it
also had absolutely no effect since the context within barf() is also
displayed.
It doesn't look well - because it should be solve by errhidecontext(true)
yes, there is a issue in send_message_to_frontend - this ignore
edata-hide_ctx field. After fixing, it working as expected - so this is a
bug in implementation of errhidecontext()
should be
if (edata-context !edata-hide_ctx)
{
pq_sendbyte(msgbuf, PG_DIAG_CONTEXT);
err_sendstring(msgbuf, edata-context);
}
and probably getting stack in err_finish should be fixed too:
if (!edata-hide_ctx)
for (econtext = error_context_stack;
econtext != NULL;
econtext = econtext-previous)
(*econtext-callback) (econtext-arg);
Regards
Pavel
I am sending patch
I'll look on this issue.
Regards
Pavel
.m
[HACKERS] bugfix: incomplete implementation of errhidecontext
Hi
current implementation of errhidecontext is not complete:
1. it sends context to client
2. it collect context although it will not be displayed
Attached patch fixing it
commit 7ee40ad6e5233f0ca2a5c10d1afcfb5d035164e6
Author: root root@localhost.localdomain
Date: Thu Apr 30 11:59:45 2015 +0200
fix bug in errhidecontext() implementation
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index dfd102a..2a9d0fd 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -426,10 +426,11 @@ errfinish(int dummy,...)
* functions will be treated as recursive errors --- this ensures we will
* avoid infinite recursion (see errstart).
*/
- for (econtext = error_context_stack;
- econtext != NULL;
- econtext = econtext-previous)
- (*econtext-callback) (econtext-arg);
+ if (!edata-hide_ctx)
+ for (econtext = error_context_stack;
+ econtext != NULL;
+ econtext = econtext-previous)
+ (*econtext-callback) (econtext-arg);
/*
* If ERROR (not more nor less) we pass it off to the current handler.
@@ -3137,7 +3138,7 @@ send_message_to_frontend(ErrorData *edata)
err_sendstring(msgbuf, edata-hint);
}
- if (edata-context)
+ if (edata-context !edata-hide_ctx)
{
pq_sendbyte(msgbuf, PG_DIAG_CONTEXT);
err_sendstring(msgbuf, edata-context);
--
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 Fri, Apr 10, 2015 at 2:52 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
On Thu, Apr 9, 2015 at 1:14 PM, Fujii Masao masao.fu...@gmail.com wrote:
On Wed, Apr 8, 2015 at 10:53 PM, Sawada Masahiko sawada.m...@gmail.com
wrote:
On Wed, Apr 8, 2015 at 1:09 PM, Fujii Masao masao.fu...@gmail.com wrote:
On Wed, Apr 8, 2015 at 1:57 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
On Wed, Apr 8, 2015 at 1:11 AM, Fabrízio de Royes Mello
fabriziome...@gmail.com wrote:
On Tue, Apr 7, 2015 at 1:04 PM, Sawada Masahiko sawada.m...@gmail.com
wrote:
On Tue, Apr 7, 2015 at 10:12 PM, Fabrízio de Royes Mello
fabriziome...@gmail.com wrote:
On Tue, Apr 7, 2015 at 7:22 AM, Sawada Masahiko
sawada.m...@gmail.com
wrote:
Thank you for your reviewing.
I modified the patch and attached latest version patch(v7).
Please have a look it.
Looks good to me. Attached patch (v8) just fix a tab indentation in
gram.y.
I had forgotten fix a tab indentation, sorry.
Thank you for reviewing!
It looks good to me too.
Can this patch be marked as Ready for Committer?
+1
Changed status to Ready for Committer.
The patch adds new syntax like REINDEX ... WITH VERBOSE, i.e., () is not
added after WITH clause. Did we reach the consensus about this syntax?
The last email from Robert just makes me think that () should be added
into the syntax.
Thank you for taking time for this patch!
I removed the FORCE option from REINDEX, so you would need to update the
patch.
Thanks.
I will change the patch with this change.
This was quite complicated issue since we already have a lot of style
command currently.
We can think many of style from various perspective: kind of DDL, new
or old command, maintenance command. And each command has each its
story.
I believe we have reached the consensus with this style at least once
(please see previous discussion), but we might needs to discuss
more...
Okay, another question is that; WITH must be required whenever the options
are specified? Or should it be abbreviatable?
In previous discussion, the WITH clause is always required by VERBOSE
option. I don't think to enable us to abbreviate WITH clause for now.
Also, at this time that FORCE option is removed, we could bring back
idea is to put VERBOSE at after object name like CLUSTER. (INDEX,
TABLE, etc.)
Attached v10 patch is latest version patch.
The syntax is,
REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
That is, WITH clause is optional.
Regards,
---
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 998340c..2e8db5a 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
refsynopsisdiv
synopsis
-REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable
+REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ WITH ] [ VERBOSE ]
/synopsis
/refsynopsisdiv
@@ -150,6 +150,15 @@ REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAM
/para
/listitem
/varlistentry
+
+ varlistentry
+termliteralVERBOSE/literal/term
+listitem
+ para
+ Prints a progress report as each index is reindexed.
+ /para
+/listitem
+ /varlistentry
/variablelist
/refsect1
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ac3b785..27364ec 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -63,6 +63,7 @@
#include utils/inval.h
#include utils/lsyscache.h
#include utils/memutils.h
+#include utils/pg_rusage.h
#include utils/syscache.h
#include utils/tuplesort.h
#include utils/snapmgr.h
@@ -3133,13 +3134,18 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* reindex_index - This routine is used to recreate a single index
*/
void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
+reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+bool verbose)
{
Relation iRel,
heapRelation;
Oid heapId;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
+ int elevel = verbose ? INFO : DEBUG2;
+ PGRUsage ru0;
+
+ pg_rusage_init(ru0);
/*
* Open and lock the parent heap relation. ShareLock is sufficient since
@@ -3283,6 +3289,13 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
heap_close(pg_index, RowExclusiveLock);
}
+ /* Log what we did */
+ ereport(elevel,
+ (errmsg(index \%s\ was reindexed.,
+ get_rel_name(indexId)),
+ errdetail(%s.,
+ pg_rusage_show(ru0;
+
/* Close rels, but keep locks */
index_close(iRel, NoLock);
heap_close(heapRelation, NoLock);
@@ -3324,7 +3337,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
* index rebuild.
*/
bool
-reindex_relation(Oid relid, int flags)
+reindex_relation(Oid relid,
Re: [HACKERS] alternative compression algorithms?
On Wed, Apr 29, 2015 at 9:12 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: The whole script (doing a lot of estimates) takes 1:50 with pglz and only 1:25 with lz4. That's ~25-30% improvement. Still pretty good -- 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] CTE optimization fence on the todo list?
On Thu, Apr 30, 2015 at 12:44 AM, Chris Rogers teuk...@gmail.com wrote: Has there been any movement on this in the last couple years? I could really use the ability to optimize across CTE boundaries, and it seems like a lot of other people could too. I'm not aware that anyone is working on it. -- 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 Thu, Apr 30, 2015 at 8:39 PM, Robert Haas robertmh...@gmail.com wrote:
On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
Attached v10 patch is latest version patch.
The syntax is,
REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
That is, WITH clause is optional.
I thought we agreed on moving this earlier in the command:
http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
Oh, I see.
Attached patch is modified syntax as
REINDEX [VERBOSE] { INDEX | ... } name
Thought?
Regards,
---
Sawada Masahiko
000_reindex_verbose_v11.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] Precedence of NOT LIKE, NOT BETWEEN, etc
On Wed, Apr 8, 2015 at 01:14:38PM -0400, Tom Lane wrote: Greg Stark st...@mit.edu writes: On Tue, Feb 24, 2015 at 5:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Also, it strikes me that we could significantly reduce, maybe even fully eliminate, the funny behaviors around the existing base_yylex() substitutions if we made them use the same idea, ie replace the leading token with something special but keep the second token's separate identity. Unless somebody sees a hole in this idea, I'll probably go do that and then come back to the precedence issues. IIRC that's exactly what the earlier patch for this did. Right, see d809fd0008a2e26de463f47b7aba0365264078f3 Where are we on this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Loss of some parts of the function definition
Hi, Dear developers, I have a request to you. 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: 1) loss of the custom formatting. 2) loss of communication parameters and return types with these types of fields to create the function. 3) multidimensional arrays are transformed into one-dimensional: [][] - [] 4) loss of data accuracy: numeric(n,m) - numeric Please - how to save and restore the entire text of the definition to CREATE END; unchanged. -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com
Re: [HACKERS] Re: [BUGS] BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)
On Sat, Mar 21, 2015 at 9:00 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 20, 2015 at 9:48 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Oct 28, 2014 at 07:02:41AM +, krystian.bi...@gmail.com wrote: The following bug has been logged on the website: Bug reference: 11805 Logged by: Krystian Bigaj Email address: krystian.bi...@gmail.com PostgreSQL version: 9.3.5 Operating system: Windows 7 Pro x64 Description: pg_ctl on Windows during service start/shutdown should notify service manager about it's status by increment dwCheckPoint and call to SetServiceStatus/pgwin32_SetServiceStatus. However during shutdown there is a missing call to SetServiceStatus. See src\bin\pg_ctl\pg_ctl.c: [ thread moved to hackers ] Can a Windows person look into this issue? http://www.postgresql.org/message-id/20141028070241.2593.58...@wrigleys.postgresql.org The thread includes a patch. I need a second person to verify its validity. Thanks. FWIW, it looks sane to me to do so, ServiceMain declaration is in charge to start the service, and to wait for the postmaster to stop, and indeed process may increment dwcheckpoint in -w mode, and it expects for process to wait for 12 times but this promise is broken. The extra calls to SetServiceStatus are also welcome to let the SCM know the current status in more details. So, what's next here? -- 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] Use outerPlanState() consistently in executor code
On Thu, Apr 30, 2015 at 08:46:55AM -0400, Robert Haas wrote: On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: In executor context, outerPlanState(node) is the same as node-ss.ps.lefttree. We follow this in most places except a few. This patch clean up the outliers and might save us a few instructions by removing indirection. Most of changes are trivial. Except I take out an outerPlan nullable check in grouping iterator - as a it surely has a left child. I don't see any particular reason not to do this. The patch is weird, though. If I open it with less, I get binary garbage. My Mac's TextEdit app opens it OK though. The patch is encoded as utf-16le, and has MSDOS newlines, ^M. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Reducing tuple overhead
On 25 April 2015 at 01:12, Amit Kapila amit.kapil...@gmail.com wrote: On Sat, Apr 25, 2015 at 1:58 AM, Jim Nasby jim.na...@bluetreble.com wrote: On 4/23/15 10:40 PM, Amit Kapila wrote: I agree with you and what I think one of the major reasons of bloat is that Index segment doesn't have visibility information due to which clearing of Index needs to be tied along with heap. Now if we can move transaction information at page level, then we can even think of having it in Index segment as well and then Index can delete/prune it's tuples on it's own which can reduce the bloat in index significantly and there is a benefit to Vacuum as well. I don't see how putting visibility at the page level helps indexes at all. We could already put XMIN in indexes if we wanted, but it won't help, because... We can do that by putting transaction info at tuple level in index as well but that will be huge increase in size of index unless we devise a way to have variable index tuple header rather than a fixed. Now this has some downsides as well like Delete needs to traverse Index segment as well to Delete mark the tuples, but I think the upsides of reducing bloat can certainly outweigh the downsides. ... which isn't possible. You can not go from a heap tuple to an index tuple. We will have the access to index value during delete, so why do you think that we need linkage between heap and index tuple to perform Delete operation? I think we need to think more to design Delete .. by CTID, but that should be doable. I see some assumptions here that need to be challenged. We can keep xmin and/or xmax on index entries. The above discussion assumes that the information needs to be updated synchronously. We already store visibility information on index entries using the lazily updated killtuple mechanism, so I don't see much problem in setting the xmin in a similar lazy manner. That way when we use the index if xmax is set we use it, if it is not we check the heap. (And then you get to freeze indexes as well ;-( ) Anyway, I have no objection to making index AM pass visibility information to indexes that wish to know the information, as long as it is provided lazily. The second assumption is that if we had visibility information in the index that it would make a difference to bloat. Since as I mention, we already do have visibility information, I don't see that adding xmax or xmin would make any difference at all to bloat. So -1 to adding it **for that reason**. A much better idea is to work out how to avoid index bloat at cause. If we are running an UPDATE and we cannot get a cleanup lock, we give up and do a non-HOT update, causing the index to bloat. It seems better to wait for a short period to see if we can get the cleanup lock. The short period is currently 0, so lets start there and vary the duration of wait upwards proportionally as the index gets more bloated. -- Simon Riggshttp://www.2ndQuadrant.com/ http://www.2ndquadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
Re: [HACKERS] Reducing tuple overhead
On Thu, Apr 30, 2015 at 8:05 AM, Simon Riggs si...@2ndquadrant.com wrote: A much better idea is to work out how to avoid index bloat at cause. If we are running an UPDATE and we cannot get a cleanup lock, we give up and do a non-HOT update, causing the index to bloat. It seems better to wait for a short period to see if we can get the cleanup lock. The short period is currently 0, so lets start there and vary the duration of wait upwards proportionally as the index gets more bloated. What I'd be worried about there is that it would be very hard to tune the wait time, and that the operating system scheduling granularity (10ms?) would be way too long. But I'm in vigorous agreement with you on one point: the solution to index bloat (and probably heap bloat, too) is not to clean it up faster but to create less of it in the first place. Making more updates HOT is one way to do that. -- 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] json_populate_record issue - TupleDesc reference leak
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
2015-04-30 14:32 GMT+02:00 Bruce Momjian br...@momjian.us:
On Thu, Feb 26, 2015 at 05:31:44PM -0500, Tom Lane wrote:
Andrew Dunstan and...@dunslane.net writes:
This doesn't look quite right. Shouldn't we unconditionally release the
Tupledesc before the returns at lines 2118 and 2127, just as we do at
the bottom of the function at line 2285?
I think Pavel's patch is probably OK as-is, because the tupdesc returned
by get_call_result_type isn't reference-counted; but I agree the code
would look cleaner your way. If the main exit isn't bothering to
distinguish this then the early exits should not either.
What I'm wondering about, though, is this bit at line 2125:
/* same logic as for json */
if (!have_record_arg rec)
PG_RETURN_POINTER(rec);
If that's supposed to be the same logic as in the other path, then how
is it that !have_record_arg has anything to do with whether the JSON
object is empty? Either the code is broken, or the comment is.
Where are we on this?
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Re: [HACKERS] Disabling trust/ident authentication configure option
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
Re: [HACKERS] BRIN range operator class
On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thanks for the updated patch; I will at it as soon as time allows. (Not really all that soon, regrettably.) Judging from a quick look, I think patches 1 and 5 can be committed quickly; they imply no changes to other parts of BRIN. (Not sure why 1 and 5 are separate. Any reason for this?) Also patch 2. Patch 4 looks like a simple bugfix (or maybe a generalization) of BRIN framework code; should also be committable right away. Needs a closer look of course. Is this still pending? -- 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] configure can't detect proper pthread flags
On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov jcmvb...@gmail.com wrote: On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov jcmvb...@gmail.com wrote: On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov jcmvb...@gmail.com wrote: Ok, one more attempt: maybe instead of checking that stderr is empty we could check that stderr has changed in the presence of the option that we test? The patch: http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvb...@gmail.com Ping? Are there any issues with that approach and the patch? I think the thing to do is add your patch here so that it doesn't get forgotten about: 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] Reducing tuple overhead
On Thu, Apr 30, 2015 at 12:31 AM, Amit Kapila amit.kapil...@gmail.com wrote: I think I am missing something here, but when this second evaluation is needed. Basically what I understand from index insertion is that it evaluates the value to be inserted in index before calling nbtree module and then nbtree just inserts the value/tuple passed to it. Sure, but what happens if it doesn't evaluate to the same value? Consider a tuple where a = 1 and a function f(a). You insert the tuple, evaluate f(a), and get 17. So you insert an index tuple into the btree with a value of 17, pointing at the tuple. Now you delete the tuple, evaluate f(a) again, and this time you get 42. You search the btree for an index tuple with a value of 42, and you don't find one. But the index tuple is still there. With the current approach, that doesn't happen, because we effectively search the entire index for tuples pointing at the heap tuple we're trying to get rid of. The only problem with that is that it's crushingly expensive when the index is large and the number of tuples we're cleaning out is comparatively small. But that's a big problem. -- 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] [RFC] sepgsql: prohibit users to relabel objects
On Thu, Apr 30, 2015 at 4:13 AM, Denis Kirjanov k...@itsirius.su wrote: Oh, I wasn't aware of that. Any hints where to look at? Unfortunately, I don't really understand in detail how to write selinux policies, so no. -- 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] json_populate_record issue - TupleDesc reference leak
On Thu, Feb 26, 2015 at 05:31:44PM -0500, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: This doesn't look quite right. Shouldn't we unconditionally release the Tupledesc before the returns at lines 2118 and 2127, just as we do at the bottom of the function at line 2285? I think Pavel's patch is probably OK as-is, because the tupdesc returned by get_call_result_type isn't reference-counted; but I agree the code would look cleaner your way. If the main exit isn't bothering to distinguish this then the early exits should not either. What I'm wondering about, though, is this bit at line 2125: /* same logic as for json */ if (!have_record_arg rec) PG_RETURN_POINTER(rec); If that's supposed to be the same logic as in the other path, then how is it that !have_record_arg has anything to do with whether the JSON object is empty? Either the code is broken, or the comment is. Where are we on this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] feature freeze and beta schedule
The schedule https://wiki.postgresql.org/wiki/PgCon_2014_Developer_Meeting#9.5_Schedule calls for beta in June. In light of that, the core team has agreed to call for feature freeze on May 15 That means that all patches that add or change features should be committed by then. If you have spare cycles, there are a number of relevant patches still open in the commit fest. -- 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
Hi 2015-04-30 13:44 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: Hi, Dear developers, I have a request to you. 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: 1) loss of the custom formatting. 2) loss of communication parameters and return types with these types of fields to create the function. 3) multidimensional arrays are transformed into one-dimensional: [][] - [] 4) loss of data accuracy: numeric(n,m) - numeric Please - how to save and restore the entire text of the definition to CREATE END; unchanged. I am afraid, it is not possible Postgres doesn't distinguish between multidimensional and one dimensional arrays - multidimensional is just syntax suger, same is function arguments - Postgres doesn't store precision for parameters. type%TYPE is translated to target type outside plpgsql function. These informations are not saved, so you cannot to take it from PostgreSQL Regards Pavel Stehule -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com
Re: [HACKERS] Use outerPlanState() consistently in executor code
On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: In executor context, outerPlanState(node) is the same as node-ss.ps.lefttree. We follow this in most places except a few. This patch clean up the outliers and might save us a few instructions by removing indirection. Most of changes are trivial. Except I take out an outerPlan nullable check in grouping iterator - as a it surely has a left child. I don't see any particular reason not to do this. The patch is weird, though. If I open it with less, I get binary garbage. My Mac's TextEdit app opens it OK though. -- 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] Re: [BUGS] BUG #11805: Missing SetServiceStatus call during service shutdown in pg_ctl (Windows only)
On Thu, Apr 30, 2015 at 9:53 PM, Robert Haas robertmh...@gmail.com wrote: On Sat, Mar 21, 2015 at 9:00 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 20, 2015 at 9:48 PM, Bruce Momjian br...@momjian.us wrote: On Tue, Oct 28, 2014 at 07:02:41AM +, krystian.bi...@gmail.com wrote: The following bug has been logged on the website: Bug reference: 11805 Logged by: Krystian Bigaj Email address: krystian.bi...@gmail.com PostgreSQL version: 9.3.5 Operating system: Windows 7 Pro x64 Description: pg_ctl on Windows during service start/shutdown should notify service manager about it's status by increment dwCheckPoint and call to SetServiceStatus/pgwin32_SetServiceStatus. However during shutdown there is a missing call to SetServiceStatus. See src\bin\pg_ctl\pg_ctl.c: [ thread moved to hackers ] Can a Windows person look into this issue? http://www.postgresql.org/message-id/20141028070241.2593.58...@wrigleys.postgresql.org The thread includes a patch. I need a second person to verify its validity. Thanks. FWIW, it looks sane to me to do so, ServiceMain declaration is in charge to start the service, and to wait for the postmaster to stop, and indeed process may increment dwcheckpoint in -w mode, and it expects for process to wait for 12 times but this promise is broken. The extra calls to SetServiceStatus are also welcome to let the SCM know the current status in more details. So, what's next here? I guess that a committer opinion would be welcome. IMO the current behavior is a bug. -- 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
I agree that it is better to show what really works. I propose to allow additional option through a source code which is made on the basis of a compilation of metadata. This will solve the problem. 2015-04-30 16:19 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: 2015-04-30 15:08 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: That's what I have to do now. But there is some problem. When you try to build the update script I get to Git code is always different from what I see in the database. It is not right. MSSQL Server, Oracle, ... always saving of the full text DDL. I do not understand why PostgreSQL believe that part of the source function must be removed !? I can understand to problem, but it doesn't help to you. Postgres displays the code, that is really used. So we can speak what is more wrong - displaying original but not used code, or displaying really used code. I am thinking so current solution is better - any other solution mean 2x stored data, that can be partially inconsistent. It cannot be comparable with Oracle - because it is different technology. 2015-04-30 15:59 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: 2015-04-30 14:52 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: Yes, I understand that. So I ask to implement saving of the full text DDL. This will allow developers to be able to save a meaning at the level of the source code. I ask to make sure that the function pg_get_function_def () returns previously stored full text DDL, instead of generating input and output parameters based on metadata. I don't see a sense of this - usually much better is storing code to files and using GIT and other. Surely, you can safe code to any custom table. Regards Pavel 2015-04-30 15:46 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: Hi 2015-04-30 13:44 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: Hi, Dear developers, I have a request to you. 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: 1) loss of the custom formatting. 2) loss of communication parameters and return types with these types of fields to create the function. 3) multidimensional arrays are transformed into one-dimensional: [][] - [] 4) loss of data accuracy: numeric(n,m) - numeric Please - how to save and restore the entire text of the definition to CREATE END; unchanged. I am afraid, it is not possible Postgres doesn't distinguish between multidimensional and one dimensional arrays - multidimensional is just syntax suger, same is function arguments - Postgres doesn't store precision for parameters. type%TYPE is translated to target type outside plpgsql function. These informations are not saved, so you cannot to take it from PostgreSQL Regards Pavel Stehule -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com
Re: [HACKERS] Proposal : REINDEX xxx VERBOSE
On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
Attached v10 patch is latest version patch.
The syntax is,
REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
That is, WITH clause is optional.
I thought we agreed on moving this earlier in the command:
http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
--
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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
On Sun, Apr 26, 2015 at 10:00 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
The attached patch v13 is revised one according to the suggestion
by Robert.
Thanks.
The last hunk in foreign.c is a useless whitespace change.
Sorry, my oversight.
+ /* actually, not shift members */
Change to: shift of 0 is the same as copying
But actually, do we really need all of this? I think you could reduce
the size of this function to three lines of code if you just did this:
x = -1;
while ((x = bms_next_member(inputset, x)) = 0)
outputset = bms_add_member(inputset, x + shift);
It might be very slightly slower, but I think it would be worth it to
reduce the amount of code needed.
OK, I reverted the bms_shift_members().
It seems to me the code block for T_ForeignScan and T_CustomScan in
setrefs.c are a bit large. It may be better to have a separate
function like T_IndexOnlyScan.
How about your opinion?
+* 5. Consider paths added by FDW, in case when both of outer and
+* inner relations are managed by the same driver.
Change to: If both inner and outer relations are managed by the same
FDW, give it a chance to push down joins.
OK,
+* 6. At the last, consider paths added by extension, in addition to
the
+* built-in paths.
Change to: Finally, give extensions a chance to manipulate the path list.
OK,
+* Fetch relation-id, if this foreign-scan node actuall scans on
+* a particular real relation. Elsewhere, InvalidOid shall be
+* informed to the FDW driver.
Change to: If we're scanning a base relation, look up the OID. (We
can skip this if scanning a join relation.)
OK,
+* Sanity check. Pseudo scan tuple-descriptor shall be constructed
+* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
+* ensure all valid TLEs have to locate prior to junk ones.
Is the goal here to make attribute numbers match up? If so, between
where and where? If not, please explain further.
No, its purpose is to reduce unnecessary projection.
The *_ps_tlist is not only used to construct tuple-descriptor of
Foreign/CustomScan with scanrelid==0, but also used to resolve var-
nodes with varno==INDEX_VAR in EXPLAIN command.
For example,
SELECT t1.y, t2.b FROM t1, t2 WHERE t1.x = t2.a;
If t1.x = t2.a is executable on external computing resource (like
remote RDBMS or GPU device, etc), both of t1.x and t2.a don't need
to appear on the targetlist of joinrel.
In this case, the best *_ps_tlist consists of two var-nodes of t1.x
and t2.a because it fits tuple-descriptor of result tuple slot, thus
it can skip per-tuple projection.
On the other hands, we may want to print out expression clause that
shall be executed on the external resource; t1.x = t2.a in this
case. If FDW/CSP keeps this clause in expression form, its var-nodes
shall be rewritten to a pair of INDEX_VAR and resno on *_ps_tlist.
So, deparse_expression() needs to be capable to find out t1.x and
t2.a on the *_ps_tlist. However, it does not make sense to include
these variables on the scan tuple-descriptor.
ExecInitForeignScan() and ExecInitCustomScan() makes its scan tuple-
descriptor using ExecCleanTypeFromTL(), not ExecTypeFromTL(), to omit
these unreferenced variables on the *_ps_tlist. All the var-nodes with
INDEX_VAR shall be identified by offset from head of the list, we cannot
allow any target-entry with resjunk=false after ones with resjunk=true,
to keep the expected varattno.
This sanity checks ensures no target-entry with resjunk=false after
the resjunk=true. It helps to distinct attributes to be included in
the result tuple from the ones for just reference in EXPLAIN.
Did my explain above introduced the reason of this sanity check well?
+ if (splan-scan.scanrelid == 0)
+ {
...
+ }
splan-scan.scanrelid += rtoffset;
Does this need an else? It seems surprising that you would offset
scanrelid even if it's starting out as zero.
(Note that there are two instances of this pattern.)
'break' was put on the tail of if-block, however, it may lead potential
bugs in the future. I'll use if-else manner as usual.
+ * 'found' : indicates whether RelOptInfo is actually constructed.
+ * true, if it was already built and on the cache.
Leftover hunk. Revert this.
Fixed,
+typedef void (*GetForeignJoinPaths_function ) (PlannerInfo *root,
Whitespace is wrong, still.
Fixed,
+ * An optional fdw_ps_tlist is used to map a reference to an attribute of
+ * underlying relation(s) on a pair of INDEX_VAR and alternative varattno.
on - onto
OK,
+ * It looks like a scan on pseudo relation that is usually result of
+ * relations join on remote data source, and FDW driver is responsible to
+ * set expected target list for this.
Change to: When fdw_ps_tlist is used, this
Re: [HACKERS] configure can't detect proper pthread flags
On Thu, Apr 30, 2015 at 3:51 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Apr 8, 2015 at 2:31 AM, Max Filippov jcmvb...@gmail.com wrote: On Sat, Mar 21, 2015 at 2:06 AM, Max Filippov jcmvb...@gmail.com wrote: On Fri, Mar 20, 2015 at 3:43 PM, Max Filippov jcmvb...@gmail.com wrote: Ok, one more attempt: maybe instead of checking that stderr is empty we could check that stderr has changed in the presence of the option that we test? The patch: http://www.postgresql.org/message-id/1426860321-13586-1-git-send-email-jcmvb...@gmail.com Ping? Are there any issues with that approach and the patch? I think the thing to do is add your patch here so that it doesn't get forgotten about: https://commitfest.postgresql.org/action/commitfest_view/open Done: https://commitfest.postgresql.org/5/232/ -- Thanks. -- Max -- 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] Precedence of NOT LIKE, NOT BETWEEN, etc
Bruce Momjian br...@momjian.us writes: On Wed, Apr 8, 2015 at 01:14:38PM -0400, Tom Lane wrote: Greg Stark st...@mit.edu writes: On Tue, Feb 24, 2015 at 5:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: Also, it strikes me that we could significantly reduce, maybe even fully eliminate, the funny behaviors around the existing base_yylex() substitutions if we made them use the same idea, ie replace the leading token with something special but keep the second token's separate identity. Unless somebody sees a hole in this idea, I'll probably go do that and then come back to the precedence issues. IIRC that's exactly what the earlier patch for this did. Right, see d809fd0008a2e26de463f47b7aba0365264078f3 Where are we on this? It's done as far as seemed reasonable to push it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use outerPlanState() consistently in executor code
On Thu, Apr 30, 2015 at 9:02 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Apr 30, 2015 at 08:46:55AM -0400, Robert Haas wrote: On Wed, Apr 15, 2015 at 3:38 PM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: In executor context, outerPlanState(node) is the same as node-ss.ps.lefttree. We follow this in most places except a few. This patch clean up the outliers and might save us a few instructions by removing indirection. Most of changes are trivial. Except I take out an outerPlan nullable check in grouping iterator - as a it surely has a left child. I don't see any particular reason not to do this. The patch is weird, though. If I open it with less, I get binary garbage. My Mac's TextEdit app opens it OK though. The patch is encoded as utf-16le, and has MSDOS newlines, ^M. I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient. UTF-8 would be much better, so I don't have to figure out how to convert. -- 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] Loss of some parts of the function definition
2015-04-30 15:34 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: I agree that it is better to show what really works. I propose to allow additional option through a source code which is made on the basis of a compilation of metadata. This will solve the problem. You can to teach PostgreSQL function to use precision and derived types - it is not plpgsql issue only - it is related to all PL. There was some proposals in this area. Currently it is much better situation than year ago, because plpgsql use binary cast instead IO cast now. Regards Pavel Stehule 2015-04-30 16:19 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: 2015-04-30 15:08 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: That's what I have to do now. But there is some problem. When you try to build the update script I get to Git code is always different from what I see in the database. It is not right. MSSQL Server, Oracle, ... always saving of the full text DDL. I do not understand why PostgreSQL believe that part of the source function must be removed !? I can understand to problem, but it doesn't help to you. Postgres displays the code, that is really used. So we can speak what is more wrong - displaying original but not used code, or displaying really used code. I am thinking so current solution is better - any other solution mean 2x stored data, that can be partially inconsistent. It cannot be comparable with Oracle - because it is different technology. 2015-04-30 15:59 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: 2015-04-30 14:52 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: Yes, I understand that. So I ask to implement saving of the full text DDL. This will allow developers to be able to save a meaning at the level of the source code. I ask to make sure that the function pg_get_function_def () returns previously stored full text DDL, instead of generating input and output parameters based on metadata. I don't see a sense of this - usually much better is storing code to files and using GIT and other. Surely, you can safe code to any custom table. Regards Pavel 2015-04-30 15:46 GMT+03:00 Pavel Stehule pavel.steh...@gmail.com: Hi 2015-04-30 13:44 GMT+02:00 Sergey Grinko sergey.gri...@gmail.com: Hi, Dear developers, I have a request to you. 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: 1) loss of the custom formatting. 2) loss of communication parameters and return types with these types of fields to create the function. 3) multidimensional arrays are transformed into one-dimensional: [][] - [] 4) loss of data accuracy: numeric(n,m) - numeric Please - how to save and restore the entire text of the definition to CREATE END; unchanged. I am afraid, it is not possible Postgres doesn't distinguish between multidimensional and one dimensional arrays - multidimensional is just syntax suger, same is function arguments - Postgres doesn't store precision for parameters. type%TYPE is translated to target type outside plpgsql function. These informations are not saved, so you cannot to take it from PostgreSQL Regards Pavel Stehule -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com -- Yours faithfully, Sergey Grinko Email: sergey.gri...@gmail.com
Re: [HACKERS] Reducing tuple overhead
On Thu, Apr 30, 2015 at 5:03 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 30, 2015 at 12:31 AM, Amit Kapila amit.kapil...@gmail.com wrote: I think I am missing something here, but when this second evaluation is needed. Basically what I understand from index insertion is that it evaluates the value to be inserted in index before calling nbtree module and then nbtree just inserts the value/tuple passed to it. Sure, but what happens if it doesn't evaluate to the same value? Consider a tuple where a = 1 and a function f(a). You insert the tuple, evaluate f(a), and get 17. So you insert an index tuple into the btree with a value of 17, pointing at the tuple. Now you delete the tuple, evaluate f(a) again, and this time you get 42. As the index expression contain table columns and all the functions or operators used in expression must be IMMUTABLE, won't that guarantee to avoid such a situation? If not, then I think for such indexes we might need to search for tupleoffset in the entire index and mark it as Delete or may be for such indexes follow the current mechanism. I think it will still give us lot of benefit in more common cases. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] Reducing tuple overhead
On Thu, Apr 30, 2015 at 9:46 AM, Amit Kapila amit.kapil...@gmail.com wrote: As the index expression contain table columns and all the functions or operators used in expression must be IMMUTABLE, won't that guarantee to avoid such a situation? The concern is that they might be labeled as immutable but not actually behave that way. The other, related problem is that the ordering operator might start to return different results than it did at index creation time. For example, consider a btree index built on a text column. Now consider 'yum update'. glibc gets updated, collation ordering of various strings change, and now you've got tuples that are in the wrong place in the index, because when the index was built, we thought A B, but now we think B A. You would think the glibc maintainers might avoid such changes in minor releases, or that the Red Hat guys would avoid packaging and shipping those changes in minor releases, but you'd be wrong. We've seen cases where the master and the standby were both running RHEL X.Y (same X.Y on both servers) but they didn't agree on the collation definitions, so queries that worked on the master failed on the slave. If not, then I think for such indexes we might need to search for tupleoffset in the entire index and mark it as Delete or may be for such indexes follow the current mechanism. I think that *is* the current mechanism. I think it will still give us lot of benefit in more common cases. It's hard to figure out exactly what can work here. Aside from correctness issues, the biggest problem with refinding the index tuples one-by-one and killing them is that it may suck for bulk operations. When you delete one tuple from the table, refinding the index tuples and killing them immediately is probably the smartest thing you can do. If you postpone it, somebody may be forced to split the page because it's full, whereas if you do it right away, the next guy who wants to put a tuple on that page may be able to make it fit. That's a big win. But if you want to delete 10 million tuples, doing an index scan for each one may induce a tremendous amount of random I/O on the index pages, possibly visiting and dirtying the same pages more than once. Scanning the whole index for tuples to kill avoids that. -- 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] contrib/fuzzystrmatch/dmetaphone.c license
On Wed, Feb 25, 2015 at 08:36:49PM -0500, Andrew Dunstan wrote: I doubt we want to rip it out without some suitable replacement -- do we? That's more than 10 years ago. I remember creating this for my then work at the North Carolina State Highway Patrol and sending it to Joe, but that's about the extent of my recollection. If the Artistic License isn't acceptable. I guess we'd have to try to get the code relicensed, or reimplement the function ourselves. There are numerous implementations out there we could copy from or use as a basis for reimplementation, including several licensed under the Apache 2.0 license - is that compatible with ours? Perhaps a company large enough to have in-house counsel (EnterpriseDB?) could get a quick legal opinion on the license before we start pursuing other things? Perhaps this is just a non-issue... The first para above was written by Dave Page, who works for EDB Where are we on this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Moving on to close the current CF 2015-02
On Wed, Apr 29, 2015 at 1:10 AM, Magnus Hagander mag...@hagander.net wrote: On Fri, Apr 17, 2015 at 4:57 PM, Magnus Hagander mag...@hagander.net wrote: On Fri, Apr 17, 2015 at 9:23 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Apr 17, 2015 at 4:22 PM, Michael Paquier wrote: @Magnus: having the possibility to mark a patch as returned with feedback without bumping it to the next CF automatically would be cool to being moving on. Meh. cool to have to help moving on. Yeah, it's at the top of my list of priorities once I get some time to spend on community stuff. Hopefully I can get around to it next week. There is a small chance I can do it before then, but it is indeed small... My apologies for that being delayed even longe rthan that. I've finally pushed the changes that: * Renames the current returned with feedback to moved to next cf * Adds a new status, returned with feedback, that is the same as rejected in everything except the label (meaning it closes the patch out, but does *not* move it to the next CF). This was at least my understanding of the consensus :) Thanks a lot for this! This looks neat to me. -- 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] Turning recovery.conf into GUCs
On Sat, Feb 21, 2015 at 6:45 AM, Peter Eisentraut pete...@gmx.net wrote: On 2/19/15 4:33 PM, Josh Berkus wrote: On 02/19/2015 12:23 PM, Peter Eisentraut wrote: On 1/6/15 4:22 PM, Peter Eisentraut wrote: That said, there is a much simpler way to achieve that specific functionality: Expose all the recovery settings as fake read-only GUC variables. See attached patch for an example. The commit fest app has this as the patch of record. I don't think there is a real updated patch under consideration here. This item should probably not be in the commit fest at all. What happened to the original patch? Regardless of what we do, keeping recovery.conf the way it is can't be what we go forward with. There was disagreement on many of the details, and no subsequent new proposal. Patch has been marked as Returned with feedback. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] feature freeze and beta schedule
On 4/30/15 12:01 PM, Robert Haas wrote: So generally we have stamped in late April or early May and released in September, but last year we didn't release until December. I assume that if we stamp beta1 in June instead of May, that's going to somewhat delay the final release as well, but I'm not sure how much. The idea when that schedule was cobbled together in the dying minutes of the last developer meeting ;-) was to have a shorter beta and still shoot for a September release. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ERROR: unexpected data beyond EOF
Alright folks, So I have this error: postgres[21118]: [8-1] ERROR: unexpected data beyond EOF in block 9 of relation base/430666195/430666206 Which produces this lovely hint: postgres[21118]: [8-2] HINT: This has been seen to occur with buggy kernels; consider updating your system. However, the problem is, all relevant information we can find is Linux/NFS based. Now it is no secret that Pg does some weird things on NFS or Virtualized volumes but I am not sure where to even start with this. The system is: FreeBSD 10 ZFS iSCSI RAID 50 (don't start, I didn't spec it). fsync on, full_page_writes on The restart of PostgreSQL makes the error go away and things progress normally. We don't experience further errors etc... Any thoughts on this? 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] Use outerPlanState() consistently in executor code
On Thu, Apr 30, 2015 at 8:02 AM, Robert Haas robertmh...@gmail.com wrote:
I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient.
UTF-8 would be much better, so I don't have to figure out how to
convert.
The patch is generated via github windows tool and that's possibly
why. I regenerated it in Linux box and see attached (sending this
email in Windows and I hope no magic happens in-between).
Please let me know if that works.
Thank you,
Qingqing
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 9ff0eff..672825a 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2054,10 +2054,11 @@ ExecReScanAgg(AggState *node)
{
ExprContext *econtext = node-ss.ps.ps_ExprContext;
int aggno;
+ PlanState *outerPlan;
node-agg_done = false;
-
node-ss.ps.ps_TupFromTlist = false;
+ outerPlan = outerPlanState(node);
if (((Agg *) node-ss.ps.plan)-aggstrategy == AGG_HASHED)
{
@@ -2075,7 +2076,7 @@ ExecReScanAgg(AggState *node)
* parameter changes, then we can just rescan the existing hash
table;
* no need to build it again.
*/
- if (node-ss.ps.lefttree-chgParam == NULL)
+ if (outerPlan-chgParam == NULL)
{
ResetTupleHashIterator(node-hashtable,
node-hashiter);
return;
@@ -2133,8 +2134,8 @@ ExecReScanAgg(AggState *node)
* if chgParam of subnode is not null then plan will be re-scanned by
* first ExecProcNode.
*/
- if (node-ss.ps.lefttree-chgParam == NULL)
- ExecReScan(node-ss.ps.lefttree);
+ if (outerPlan-chgParam == NULL)
+ ExecReScan(outerPlan);
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c
b/src/backend/executor/nodeBitmapHeapscan.c
index 8ea8b9f..6502841 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -449,6 +449,8 @@ ExecBitmapHeapScan(BitmapHeapScanState *node)
void
ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
{
+ PlanState *outerPlan;
+
/* rescan to release any page pin */
heap_rescan(node-ss.ss_currentScanDesc, NULL);
@@ -469,8 +471,9 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
* if chgParam of subnode is not null then plan will be re-scanned by
* first ExecProcNode.
*/
- if (node-ss.ps.lefttree-chgParam == NULL)
- ExecReScan(node-ss.ps.lefttree);
+ outerPlan = outerPlanState(node);
+ if (outerPlan-chgParam == NULL)
+ ExecReScan(outerPlan);
}
/*
diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c
index 83d562e..b0d5442 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -280,6 +280,8 @@ ExecEndGroup(GroupState *node)
void
ExecReScanGroup(GroupState *node)
{
+ PlanState *outerPlan;
+
node-grp_done = FALSE;
node-ss.ps.ps_TupFromTlist = false;
/* must clear first tuple */
@@ -289,7 +291,7 @@ ExecReScanGroup(GroupState *node)
* if chgParam of subnode is not null then plan will be re-scanned by
* first ExecProcNode.
*/
- if (node-ss.ps.lefttree
- node-ss.ps.lefttree-chgParam == NULL)
- ExecReScan(node-ss.ps.lefttree);
+ outerPlan = outerPlanState(node);
+ if (outerPlan-chgParam == NULL)
+ ExecReScan(outerPlan);
}
diff --git a/src/backend/executor/nodeMaterial.c
b/src/backend/executor/nodeMaterial.c
index 1158825..41859e0 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -317,6 +317,9 @@ ExecMaterialRestrPos(MaterialState *node)
void
ExecReScanMaterial(MaterialState *node)
{
+ PlanState *outerPlan;
+
+ outerPlan = outerPlanState(node);
ExecClearTuple(node-ss.ps.ps_ResultTupleSlot);
if (node-eflags != 0)
@@ -339,13 +342,13 @@ ExecReScanMaterial(MaterialState *node)
* Otherwise we can just rewind and rescan the stored output.
The
* state of the subnode does not change.
*/
- if (node-ss.ps.lefttree-chgParam != NULL ||
+ if (outerPlan-chgParam != NULL ||
(node-eflags EXEC_FLAG_REWIND) == 0)
{
tuplestore_end(node-tuplestorestate);
node-tuplestorestate = NULL;
- if (node-ss.ps.lefttree-chgParam == NULL)
- ExecReScan(node-ss.ps.lefttree);
+ if (outerPlan-chgParam == NULL)
+ ExecReScan(outerPlan);
node-eof_underlying = false;
Re: [HACKERS] Relation extension scalability
On Fri, Apr 17, 2015 at 11:19 AM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: Most commercial database employs a DMS storage model, where it manages object mapping and freespace itself. So different objects are sharing storage within several files. Surely it has historic reasons, but it has several advantages over current model: - remove fd pressure - remove double buffering (by introducing ADIO) - controlled layout and access pattern (sequential and read-ahead) - better quota management - performance potentially better Considering platforms supported and the stableness period needed, we shall support both current storage model and DMS model. I will stop here to see if this deserves further discussion. Sorry, it might considered double-posting here but I am wondering have we ever discussed this before? If we already have some conclusions on this, could anyone share me a link? Thanks, Qingqing -- 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 Thu, Apr 30, 2015 at 9:15 AM, Sawada Masahiko sawada.m...@gmail.com wrote:
On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas robertmh...@gmail.com wrote:
On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
Attached v10 patch is latest version patch.
The syntax is,
REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
That is, WITH clause is optional.
I thought we agreed on moving this earlier in the command:
http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
Oh, I see.
Attached patch is modified syntax as
REINDEX [VERBOSE] { INDEX | ... } name
Thought?
I thought what we agreed on was:
REINDEX (flexible options) { INDEX | ... } name
The argument wasn't about whether to use flexible options, but where
in the command to put them.
--
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_upgrade: quote directory names in delete_old_cluster script
On Wed, Apr 29, 2015 at 11:59:26PM -0300, Alvaro Herrera wrote: Bruce Momjian wrote: I have developed the attached patch to use platform-specific quoting of path names. Part of me wonders about initdb's existing DIR_SEP and QUOTE_PATH definitions ... seems messy to reinvent these things all over again. I don't think we can reuse QUOTE_PATH as it is used in initdb for displaying potential ways of starting the server, and it assumes Unix doesn't need quoting, while Windows usually does. For an actual script, we always want to use quoting. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] ERROR: unexpected data beyond EOF
Joshua D. Drake wrote: Alright folks, So I have this error: postgres[21118]: [8-1] ERROR: unexpected data beyond EOF in block 9 of relation base/430666195/430666206 Which produces this lovely hint: postgres[21118]: [8-2] HINT: This has been seen to occur with buggy kernels; consider updating your system. However, the problem is, all relevant information we can find is Linux/NFS based. I have vague recollections of seeing this relatively recently on tables that were truncated often. Is that the case here? -- Á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] ERROR: unexpected data beyond EOF
On 04/30/2015 10:28 AM, Alvaro Herrera wrote: Joshua D. Drake wrote: Alright folks, So I have this error: postgres[21118]: [8-1] ERROR: unexpected data beyond EOF in block 9 of relation base/430666195/430666206 Which produces this lovely hint: postgres[21118]: [8-2] HINT: This has been seen to occur with buggy kernels; consider updating your system. However, the problem is, all relevant information we can find is Linux/NFS based. I have vague recollections of seeing this relatively recently on tables that were truncated often. Is that the case here? Just dug into the table a bit and yes, it appears to be a transform table. Further questions/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] feature freeze and beta schedule
On Thu, Apr 30, 2015 at 12:52 PM, Peter Eisentraut pete...@gmx.net wrote: On 4/30/15 12:01 PM, Robert Haas wrote: So generally we have stamped in late April or early May and released in September, but last year we didn't release until December. I assume that if we stamp beta1 in June instead of May, that's going to somewhat delay the final release as well, but I'm not sure how much. The idea when that schedule was cobbled together in the dying minutes of the last developer meeting ;-) was to have a shorter beta and still shoot for a September release. I think that could be doable if we can keep focus, but that's easier said than done. -- 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] cost_index() and path row estimate.
While looking into a customer performance problem, i saw this in
costsize.c, cost_index() (9.3.6, but it looks the same in HEAD):
/* Mark the path with the correct row estimate */
if (path-path.param_info)
{
path-path.rows = path-path.param_info-ppi_rows;
/* also get the set of clauses that should be enforced by the
scan */
allclauses =
list_concat(list_copy(path-path.param_info-ppi_clauses),
baserel-baserestrictinfo);
}
else
{
path-path.rows = baserel-rows;
/* allclauses should just be the rel's restriction clauses */
allclauses = baserel-baserestrictinfo;
}
What i'm wondering is the else branch, where the baserel row estimate is
assigned to the
IndexPath. However, it occurs to me that in conjunction with a partial
index, the overall row estimate will be constrained by the row estimate
from the partial index itself, no? I see that this doesn't have an impact
on the cost estimation of the index scan itself, since cost_index() does
this later:
/* estimate number of main-table tuples fetched */
tuples_fetched = clamp_row_est(indexSelectivity * baserel-tuples);
I stumpled across this, since i see heavy misestimates in the EXPLAIN
output, where the estimated row count from the partial index vs. the real
row count heavily differs. In the customers case, there are two tables,
where one of the relation has many tuples in the JOIN condition which are
NULLs, like:
CREATE TABLE a2(id integer);
CREATE TABLE b2(id integer);
INSERT INTO a2 SELECT NULL FROM generate_series(1, 9800) AS t(id);
INSERT INTO a2 SELECT t.id FROM generate_series(1, 200) AS t(id);
INSERT INTO b2 SELECT t.id FROM generate_series(1, 200) AS t(id);
CREATE INDEX ON a2(id) WHERE id IS NOT NULL;
CREATE INDEX ON b2(id);
Here i get the following plan:
EXPLAIN ANALYZE SELECT * FROM b2 INNER JOIN a2 ON a2.id = b2.id ;
Merge Join (cost=10.79..12.63 rows=4 width=8) (actual time=0.084..0.291
rows=200 loops=1)
Merge Cond: (b2.id = a2.id)
- Sort (cost=10.64..11.14 rows=200 width=4) (actual time=0.069..0.082
rows=200 loops=1)
Sort Key: b2.id
Sort Method: quicksort Memory: 35kB
- Seq Scan on b2 (cost=0.00..3.00 rows=200 width=4) (actual
time=0.010..0.027 rows=200 loops=1)
- Index Only Scan using a2_id_idx on a2 (cost=0.14..15.14 rows=1
width=4) (actual time=0.012..0.074 rows=200 loops=1)
Heap Fetches: 200
Total runtime: 0.350 ms
EXPLAIN ANALYZE SELECT * FROM b2 INNER JOIN a2 ON a2.id = b2.id WHERE a2.id
IS NOT NULL;
Merge Join (cost=10.79..12.12 rows=1 width=8) (actual time=0.080..0.281
rows=200 loops=1)
Merge Cond: (b2.id = a2.id)
- Sort (cost=10.64..11.14 rows=200 width=4) (actual time=0.063..0.070
rows=200 loops=1)
Sort Key: b2.id
Sort Method: quicksort Memory: 35kB
- Seq Scan on b2 (cost=0.00..3.00 rows=200 width=4) (actual
time=0.010..0.034 rows=200 loops=1)
- Index Only Scan using a2_id_idx on a2 (cost=0.14..15.64 rows=200
width=4) (actual time=0.012..0.052 rows=200 loops=1)
Index Cond: (id IS NOT NULL)
Heap Fetches: 200
Total runtime: 0.335 ms
With the partial index predicate explicitly specified the row estimate is
accurate, without the predicate the row estimate of the index scan on
a2_id_idx assumes 1.
It's very likely i miss something really important here, could someone shed
some light on this?
--
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] BRIN range operator class
Robert Haas wrote: On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thanks for the updated patch; I will at it as soon as time allows. (Not really all that soon, regrettably.) Judging from a quick look, I think patches 1 and 5 can be committed quickly; they imply no changes to other parts of BRIN. (Not sure why 1 and 5 are separate. Any reason for this?) Also patch 2. Patch 4 looks like a simple bugfix (or maybe a generalization) of BRIN framework code; should also be committable right away. Needs a closer look of course. Is this still pending? Yeah. -- Á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] ERROR: unexpected data beyond EOF
On 04/30/2015 12:09 PM, Alvaro Herrera wrote: Joshua D. Drake wrote: I take that back, it appears this table is heavily deleted from and also uses the lo_manage() triggers. Well, if it's heavily deleted, then it's probably also heavily vacuumed and from time to time empty pages at the tail are removed by vacuum. It might also be the case I was remembering, rather than regular TRUNCATE. I don't think the vacuumlo stuff would have much to do with it issue; I think it would only scan the table, then delete stuff from pg_largeobject. It doesn't modify the table itself (unless I'm misremembering) Anyway, I don't remember that we reached any useful conclusion. Andres suspected a PG bug, but we didn't find anything. Well it certainly causes an outage unfortunately. Once the error occurs postgresql will stop accepting requests (which is correct but still). 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] pgbench -f and vacuum
Robert Haas robertmh...@gmail.com writes: On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes jeff.ja...@gmail.com wrote: But as far as what has been discussed on the central topic of this thread, I think that doing the vacuum and making the failure for non-existent tables be non-fatal when -f is provided would be an improvement. Or maybe just making it non-fatal at all times--if the table is needed and not present, the session will fail quite soon anyway. I don't see the other changes as being improvements. I would rather just learn to add the -n when I use -f and don't have the default tables in place, than have to learn new methods for saying no really, I left -n off on purpose when I have a custom file which does use the default tables and I want them vacuumed. So, discussion seems to have died off here. I think what Jeff is proposing here is a reasonable compromise. Patch for that attached. +1 as to the basic behavior, but I'm not convinced that this is user-friendly reporting: + if (PQresultStatus(res) != PGRES_COMMAND_OK) + fprintf(stderr, %s, PQerrorMessage(con)); I would be a bit surprised to see pgbench report an ERROR and then continue on anyway; I might think that was a bug, even. I am not sure exactly what it should print instead though. Some perhaps viable proposals: * don't print anything at all, just chug along. * do something like fprintf(stderr, Ignoring: %s, PQerrorMessage(con)); * add something like (Ignoring this error and continuing anyway) on a line after the error message. (I realize this takes us right back into the bikeshedding game, but I do think that what's displayed is important.) 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] ERROR: unexpected data beyond EOF
Joshua D. Drake wrote: I take that back, it appears this table is heavily deleted from and also uses the lo_manage() triggers. Well, if it's heavily deleted, then it's probably also heavily vacuumed and from time to time empty pages at the tail are removed by vacuum. It might also be the case I was remembering, rather than regular TRUNCATE. I don't think the vacuumlo stuff would have much to do with it issue; I think it would only scan the table, then delete stuff from pg_largeobject. It doesn't modify the table itself (unless I'm misremembering) Anyway, I don't remember that we reached any useful conclusion. Andres suspected a PG bug, but we didn't find anything. -- Á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] initdb -S and tablespaces
On Thu, Apr 16, 2015 at 9:24 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: Here's a variation of the earlier patch that follows all links in PGDATA. Does this look more like what you had in mind? I'm really confused by the additional control-file field. It is documented as indicating whether fsync was ever disabled while the server was running. But: 1. It doesn't do that. As soon as we fsync the data directory, we reset the flag. That's not what ever disabled means to me. 2. I don't know why it's part of this patch. Tracking whether fsync was ever disabled could be useful forensically, but isn't related to fsync-ing the data directory after a crash, so I dunno why we'd put that in this patch. Tracking whether fsync was disabled recently, as the patch actually does, doesn't seem to be of any obvious value in preventing corruption either. Also, it seems awfully unfortunate to me that we're duplicating a whole pile of code into xlog.c here. Maybe there's no way to avoid the code duplication, but pre_sync_fname() seems like it'd more naturally go in fd.c than here. I dunno where walkdir should go, but again, not in xlog.c. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] alter user/role CURRENT_USER
Kyotaro HORIGUCHI wrote: Thank you for completing this and very sorry not to respond these days. I understood that it is committed after I noticed that rebasing my code failed.. You'd do well to check your email, I guess :-) Although after committed, I found some issues as I looked on it. Please forgive me to comment it now after all this time. | =# alter role current_user rename to PubLic; | ERROR: CURRENT_USER cannot be used as a role name | LINE 1: alter role current_user rename to PubLic; |^ The error message sounds somewhat different from the intention. I think the following message would be clearer. | ERROR: CURRENT_USER cannot be used as a role name here Okay, changed. The document sql-altergroup.html says | ALTER GROUP role_specification ADD USER user_name [, ... ] But current_user is also usable in user_name list. So the doc should be as following, but it would not be necessary to be fixed because it is an obsolete commnand.. | ALTER GROUP role_specification ADD USER role_specification [, ... ] Yeah, EDONTCARE. ALTER GROUP role_spec ADD/DROP USER role_spec is naturally denied so I think no additional description is needed. +1 sql-alterpolicy.html ALTER POLICY name ON table_name TO also accepts current_user and so as the role to which the policy applies. Changed. # As a different topic, the syntax ALTER POLICY pname ON # tname TO user looks a bit wired, it might be better be to # be ON tname APPLY TO user but I shouldn't try to fix it # since it is a long standing syntax.. Yeah, it's a bit strange. Not a strong opinion. Maybe you should raise it as a separate thread. sql-createtablespace.html sql-drop-owned.html, sql-reassign-owned.html Changed. == sql-grant.html, sql-revoke.html, GRANT roles TO roles and REVOKE roles FROM roles are the modern equivalents of the deprecated syntaxes ALTER roles ADD USER roles and ALTER roles DROP USER roles respectively. But the current parser infrastructure doesn't allow coexistence of the two following syntaxes but I couldn't find the way to their coexistence. I decided to leave this out. I think we should consider it as a new patch for 9.6; these changes aren't as clear-cut as the rest of your patch. I didn't want to have to research the ecpg changes. -- Á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] pgbench -f and vacuum
On Thu, Apr 30, 2015 at 4:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes jeff.ja...@gmail.com wrote: But as far as what has been discussed on the central topic of this thread, I think that doing the vacuum and making the failure for non-existent tables be non-fatal when -f is provided would be an improvement. Or maybe just making it non-fatal at all times--if the table is needed and not present, the session will fail quite soon anyway. I don't see the other changes as being improvements. I would rather just learn to add the -n when I use -f and don't have the default tables in place, than have to learn new methods for saying no really, I left -n off on purpose when I have a custom file which does use the default tables and I want them vacuumed. So, discussion seems to have died off here. I think what Jeff is proposing here is a reasonable compromise. Patch for that attached. +1 as to the basic behavior, but I'm not convinced that this is user-friendly reporting: + if (PQresultStatus(res) != PGRES_COMMAND_OK) + fprintf(stderr, %s, PQerrorMessage(con)); I would be a bit surprised to see pgbench report an ERROR and then continue on anyway; I might think that was a bug, even. I am not sure exactly what it should print instead though. Some perhaps viable proposals: * don't print anything at all, just chug along. * do something like fprintf(stderr, Ignoring: %s, PQerrorMessage(con)); * add something like (Ignoring this error and continuing anyway) on a line after the error message. (I realize this takes us right back into the bikeshedding game, but I do think that what's displayed is important.) I tried it out before sending the patch and it's really not that bad. It's says: starting vacuum ERROR: blah ERROR: blah ERROR: blah done And then continues on. Sure, that's not the greatest error reporting output ever, but what do you expect from pgbench? I think it's clear enough what's going on there. The messages appear in quick succession, because it doesn't take very long to notice that the table isn't there, so it's not like you are sitting there going wait, what?. If we're going to add something, I like your second suggestion (ignoring this error and continuing anyway) more than the first one. Putting ignoring: before the thing you plan to ignore will be confusing, I think. -- 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] PATCH: adaptive ndistinct estimator v4
On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: attached is v4 of the patch implementing adaptive ndistinct estimator. So, I took a look at this today. It's interesting work, but it looks more like a research project than something we can commit to 9.5. As far as I can see, this still computes the estimate the way we do today, but then also computes the estimate using this new method. The estimate computed the new way isn't stored anywhere, so this doesn't really change behavior, except for printing out a WARNING comparing the values produced by the two estimators. IMHO, the comments in this patch are pretty inscrutable. I believe this is because they presume more knowledge of what the patch is doing than I myself possess. For example: + * The AEE estimator is based on solving this equality (for m) + * + * m - f1 - f2 = f1 * (A + A(m)) / (B + B(m)) + * + * where A, B are effectively constants (not depending on m), and A(m) + * and B(m) are functions. This is equal to solving + * + * 0 = f1 * (A + A(m)) / (B + B(m)) - (m - f1 - f2) Perhaps I am just a dummy, but I have no idea what any of that means. I think that needs to be fixed so that someone who knows what n_distinct is but knows nothing about the details of these estimators can get an idea of how they are doing their thing without too much effort. I think a lot of the comments share this problem. Aside from the problems mentioned above, there's the broader question of how to evaluate the quality of the estimates produced by this estimator vs. what we're doing right now. I see that Jeff Janes has pointed out some cases that seem to regress with this patch; those presumably need some investigation, or at least some comment. And I think some testing from other people would be good too, before we commit to this. Leaving that aside, at some point, you'll say, OK, there may be some regressions left but overall I believe this is going to be a win in most cases. It's going to be really hard for anyone, no matter how smart, to figure out through code review whether that is true. So committing this figures to be extremely frightening. It's just not going to be reasonably possible to know what percentage of users are going to be more happy after this change and what percentage are going to be less happy. Therefore, I think that: 1. This should be committed near the beginning of a release cycle, not near the end. That way, if there are problem cases, we'll have a year or so of developer test to shake them out. 2. There should be a compatibility GUC to restore the old behavior. The new behavior should be the default, because if we're not confident that the new behavior will be better for most people, we have no business installing it in the first place (plus few people will try it). But just in case it turns out to suck for some people, we should provide an escape hatch, at least for a few releases. 3. There should be some clear documentation in the comments indicating why we believe that this is a whole lot better than what we do today. Maybe this has been discussed adequately on the thread and maybe it hasn't, but whoever goes to look at the committed code should not have to go root through hackers threads to understand why we replaced the existing estimator. It should be right there in the code. If, hypothetically speaking, I were to commit this, and if, again strictly hypothetically, another distinguished committer were to write back a year or two later, clearly Robert was an idiot to commit this because it's no better than what we had before then I want to be able to say clearly, you have not what got committed very carefully, because the comment for function blat clearly explains that this new technology is teh awesome. -- 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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
On Thu, Apr 30, 2015 at 9:16 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
It seems to me the code block for T_ForeignScan and T_CustomScan in
setrefs.c are a bit large. It may be better to have a separate
function like T_IndexOnlyScan.
How about your opinion?
Either way is OK with me. Please do as you think best.
OK, in setrefs.c, I moved the code block for T_ForeignScan and T_CustomScan
into set_foreignscan_references() and set_customscan_references() for each.
Its nest-level is a bit deep to keep all the stuff within 80-characters row.
It also uses bms_add_member(), instead of bms_shift_members() reverted.
+* Sanity check. Pseudo scan tuple-descriptor shall be constructed
+* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
+* ensure all valid TLEs have to locate prior to junk ones.
Is the goal here to make attribute numbers match up? If so, between
where and where? If not, please explain further.
No, its purpose is to reduce unnecessary projection.
The *_ps_tlist is not only used to construct tuple-descriptor of
Foreign/CustomScan with scanrelid==0, but also used to resolve var-
nodes with varno==INDEX_VAR in EXPLAIN command.
For example,
SELECT t1.y, t2.b FROM t1, t2 WHERE t1.x = t2.a;
If t1.x = t2.a is executable on external computing resource (like
remote RDBMS or GPU device, etc), both of t1.x and t2.a don't need
to appear on the targetlist of joinrel.
In this case, the best *_ps_tlist consists of two var-nodes of t1.x
and t2.a because it fits tuple-descriptor of result tuple slot, thus
it can skip per-tuple projection.
On the other hands, we may want to print out expression clause that
shall be executed on the external resource; t1.x = t2.a in this
case. If FDW/CSP keeps this clause in expression form, its var-nodes
shall be rewritten to a pair of INDEX_VAR and resno on *_ps_tlist.
So, deparse_expression() needs to be capable to find out t1.x and
t2.a on the *_ps_tlist. However, it does not make sense to include
these variables on the scan tuple-descriptor.
ExecInitForeignScan() and ExecInitCustomScan() makes its scan tuple-
descriptor using ExecCleanTypeFromTL(), not ExecTypeFromTL(), to omit
these unreferenced variables on the *_ps_tlist. All the var-nodes with
INDEX_VAR shall be identified by offset from head of the list, we cannot
allow any target-entry with resjunk=false after ones with resjunk=true,
to keep the expected varattno.
This sanity checks ensures no target-entry with resjunk=false after
the resjunk=true. It helps to distinct attributes to be included in
the result tuple from the ones for just reference in EXPLAIN.
Did my explain above introduced the reason of this sanity check well?
Yeah, I think so. So what we want to do in this comment is summarize
all of that briefly. Maybe something like this:
Sanity check. There may be resjunk entries in fdw_ps_tlist that are
included only to help EXPLAIN deparse plans properly. We require that
these are at the end, so that when the executor builds the scan
descriptor based on the non-junk entries, it gets the attribute
numbers correct.
Thanks, I used this sentence as is.
+ if (splan-scan.scanrelid == 0)
+ {
...
+ }
splan-scan.scanrelid += rtoffset;
Does this need an else? It seems surprising that you would offset
scanrelid even if it's starting out as zero.
(Note that there are two instances of this pattern.)
'break' was put on the tail of if-block, however, it may lead potential
bugs in the future. I'll use if-else manner as usual.
Ah, OK, I missed that. Yeah, that's probably a good change.
set_foreignscan_references() and set_customscan_references() are
split by two portions using the manner above; a code block if scanrelid==0
and others.
I assume you realize you did not attach an updated patch?
I wanted to submit the v14 after the above items get clarified.
The attached patch (v14) includes all what you suggested in the previous
message.
Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei kai...@ak.jp.nec.com
pgsql-v9.5-custom-join.v14.patch
Description: pgsql-v9.5-custom-join.v14.patch
--
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] ERROR: unexpected data beyond EOF
I take that back, it appears this table is heavily deleted from and also uses the lo_manage() triggers. -- 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] parallel mode and parallel contexts
On Wed, Apr 29, 2015 at 12:23 PM, Robert Haas robertmh...@gmail.com wrote: So, I think it makes sense to split up this patch in two. There's no real debate, AFAICS, about anything in the patch other than the heavyweight locking stuff. So I'd like to go ahead and commit the rest. That's attached here as parallel-mode-v10.patch. Hearing no objections, done. Still hoping for some input on the rest. -- 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] EvalPlanQual behaves oddly for FDW queries involving system columns
Robert Haas robertmh...@gmail.com writes: Tom, you're listed as the committer for this in the CF app. Are you still planning to take care of this? It seems that time is beginning to run short. Yeah, I will address this (and start looking at GROUPING SETS) next week. I'm out of town right now. 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] pgbench -f and vacuum
On Wed, Feb 11, 2015 at 2:00 PM, Jeff Janes jeff.ja...@gmail.com wrote: But as far as what has been discussed on the central topic of this thread, I think that doing the vacuum and making the failure for non-existent tables be non-fatal when -f is provided would be an improvement. Or maybe just making it non-fatal at all times--if the table is needed and not present, the session will fail quite soon anyway. I don't see the other changes as being improvements. I would rather just learn to add the -n when I use -f and don't have the default tables in place, than have to learn new methods for saying no really, I left -n off on purpose when I have a custom file which does use the default tables and I want them vacuumed. So, discussion seems to have died off here. I think what Jeff is proposing here is a reasonable compromise. Patch for that attached. Objections? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company pgbench-vacuum-failure-not-fatal.patch Description: binary/octet-stream -- 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] Faster setup_param_list() in plpgsql
2015-04-29 9:26 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hi all I am looking on this patch. I can confirm 10-15% speedup - and the idea behind this patch looks well. This patch http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us contains two parts a) relative large refactoring b) support for resettable fields in param_list instead total reset I believe it should be in two separate patches. Refactoring is trivial and there is no any possible objection. I was wrong, there is relative strong dependency between these two parts, so it should be commited as one patch Regards Pavel Regards Pavel
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 12/20/14 12:11 PM, Steve Singer wrote: On 12/19/2014 10:41 AM, Alex Shulgin wrote: I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong, and in your case there would be a message like the following: WARNING: pg_hba.conf not reloaded So an extra hint about file timestamp is unneeded. That makes sense to me. I haven't found any new issues with this patch. I think it is ready for a committer. There were later comments in this thread that disagreed with the extra logging infrastructure, and there were some questions about whether it should only log on failed authentication attempts. Altogether, still some open questions about behavior and implementation approach. So I'm marking this as returned with feedback for now. Personally, I think this could be a useful feature, but it needs more fine-tuning. -- 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] EvalPlanQual behaves oddly for FDW queries involving system columns
On Thu, Apr 16, 2015 at 2:55 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Ah, you are right. FOR NO KEY UPDATE and FOR KEY SHARE would be useful in the Postgres FDW if we assume the user performs those properly based on information about keys for a remote table. Sorry, my explanation was not correct, but I want to make it clear that the proposed patch also allows the FDW to change the behavior of FOR NO KEY UPDATE and/or FOR KEY SHARE row locking so as to match the local semantics exactly. BTW, I revised docs a bit. Attached is an updated version of the patch. Tom, you're listed as the committer for this in the CF app. Are you still planning to take care of this? It seems that time is beginning to run short. -- 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] Faster setup_param_list() in plpgsql
Review: What this patch does - it change a mechanism, how a values of variables are transmitted to SPI. In previous variant values are copied to ParamListInfo before every evaluation of any expression. New mechanism is smarter. It refresh only ROW, REC values when are marked as dirty (when these variables was used). ParamListInfo is dynamically updated when value is assigned to variable. This patch can significantly reduce a overhead related to preparing parameters - more it cleaning code. 1. There are no problem with patching, regress tests 2. The changes are clean and well documented 3. I can confirm 10% of speedup. This patch is ready for commit. Regards Pavel Stehule 2015-04-30 20:50 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: 2015-04-29 9:26 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hi all I am looking on this patch. I can confirm 10-15% speedup - and the idea behind this patch looks well. This patch http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us contains two parts a) relative large refactoring b) support for resettable fields in param_list instead total reset I believe it should be in two separate patches. Refactoring is trivial and there is no any possible objection. I was wrong, there is relative strong dependency between these two parts, so it should be commited as one patch Regards Pavel Regards Pavel
Re: [HACKERS] BuildTupleFromCStrings Memory Documentation?
Jason Petersen ja...@citusdata.com writes: Within the core codebase, BuildTupleFromCStrings is often called within a temporary memory context cleared after the call. In dblink.c, this is justified as being needed to â[clean up] not only the data we have direct access to, but any cruft the I/O functions might leakâ. I wrote a pretty minimal case to call BuildTupleFromCStrings in a loop (attached) and found that I was using 40GB of RAM in a few minutes, though I was not allocating any memory myself and immediately freed the tuple it returned. Is the need to wrap this call in a protective context documented anywhere? Portions of the documentation use BuildTupleFromCStrings in examples without mentioning this precaution. Is it just well-known, or did I miss a README or comment somewhere? Most uses of BuildTupleFromCStrings only do it once per invocation of the calling function, so that an outer-level reset of the calling function's evaluation context is what takes care of any memory leaked by the I/O functions BuildTupleFromCStrings invokes. If you intend to call it many times within the same function call then you should use a context you can reset between calls. This risk is hardly unique to BuildTupleFromCStrings, which is why the documentation doesn't make a big point of it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] feature freeze and beta schedule
On Fri, May 1, 2015 at 1:55 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 30, 2015 at 12:52 PM, Peter Eisentraut pete...@gmx.net wrote: On 4/30/15 12:01 PM, Robert Haas wrote: So generally we have stamped in late April or early May and released in September, but last year we didn't release until December. I assume that if we stamp beta1 in June instead of May, that's going to somewhat delay the final release as well, but I'm not sure how much. The idea when that schedule was cobbled together in the dying minutes of the last developer meeting ;-) was to have a shorter beta and still shoot for a September release. I think that could be doable if we can keep focus, but that's easier said than done. Just to make people aware here that I am not slacking: I am going to be out of town for a couple of weeks more or less until the end of May with limited access to computer so I am not sure I will be able to help much. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_rewind test race condition..?
On 04/29/2015 06:03 AM, Stephen Frost wrote:
* Heikki Linnakangas (hlinn...@iki.fi) wrote:
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7173,7 +7173,10 @@ StartupXLOG(void)
* than is appropriate now that we're not in standby mode anymore.
*/
if (fast_promoted)
+ {
+ sleep(5);
RequestCheckpoint(CHECKPOINT_FORCE);
+ }
}
The simplest fix would be to force a checkpoint in the regression
test, before running pg_rewind. It's a bit of a cop out, since you'd
still get the same issue when you tried to do the same thing in the
real world. It should be rare in practice - you'd not normally run
pg_rewind immediately after promoting the standby - but a better
error message at least would be nice..
Forcing a checkpoint in the regression tests and then providing a better
error message sounds reasonable to me. I agree that it's very unlikely
to happen in the real world, even when you're bouncing between systems
for upgrades, etc, you're unlikely to do it fast enough for this issue
to exhibit itself, and a better error message would help any users who
manage to run into this (perhaps during their own testing).
I've committed this simple fix for now.
- 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
Robert Haas wrote: On Thu, Apr 30, 2015 at 6:18 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Also, it seems awfully unfortunate to me that we're duplicating a whole pile of code into xlog.c here. Maybe there's no way to avoid the code duplication, but pre_sync_fname() seems like it'd more naturally go in fd.c than here. I dunno where walkdir should go, but again, not in xlog.c. Hm, there's an interest in backpatching this as a bugfix, if I understand correctly; hence the duplicated code. We could remove the duplicity later with a refactoring patch in master only. That seems pretty silly. If we going to add pre_sync_fname() to every branch, we should add it to the same (correct) file in all of them, not put it in xlog.c in the back-branches and fd.c in master. Ah, so that's not the duplicate code that I was remembering -- I think it's walkdir() or something like that, which is in initdb IIRC. -- Á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] initdb -S and tablespaces
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 30, 2015 at 6:44 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Ah, so that's not the duplicate code that I was remembering -- I think it's walkdir() or something like that, which is in initdb IIRC. Yeah, walkdir() is there too. But if we're going to add that to the backend, I think it should go in src/backend/storage/file, not src/backend/access/transam. Agreed that .../transam is a pretty horrid choice; but maybe we should be thinking about putting it in src/common, so there's only one copy? As for the notion that this needs to be back-patched, I would say no. 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] procost for to_tsvector
On Wed, Mar 11, 2015 at 02:40:16PM +, Andrew Gierth wrote: An issue that comes up regularly on IRC is that text search queries, especially on relatively modest size tables or for relatively non-selective words, often misplan as a seqscan based on the fact that to_tsvector has procost=1. Clearly this cost number is ludicrous. Getting the right cost estimate would obviously mean taking the cost of detoasting into account, but even without doing that, there's a strong argument that it should be increased to at least the order of 100. (With the default cpu_operator_cost that would make each to_tsvector call cost 0.25.) (The guy I was just helping on IRC was seeing a slowdown of 100x from a seqscan in a query that selected about 50 rows from about 500.) Where are we on setting increasing procost for to_tsvector? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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
At 2015-04-30 15:37:44 -0400, robertmh...@gmail.com wrote: 1. It doesn't do that. As soon as we fsync the data directory, we reset the flag. That's not what ever disabled means to me. Could you suggest an acceptable alternative wording? I can't immediately think of anything better than disabled since the last restart. That is conditional on our resetting the flag, which we will do only if fsync is enabled at startup. So it's true, but not the whole truth. 2. I don't know why it's part of this patch. In 20150115133245.gg5...@awork2.anarazel.de, Andres explained his rationale as follows: «What I am thinking of is that, currently, if you start the server for initial loading with fsync=off, and then restart it, you're open to data loss. So when the current config file setting is changed from off to on, we should fsync the data directory. Even if there was no crash restart.» That's what I tried to implement. Also, it seems awfully unfortunate to me that we're duplicating a whole pile of code into xlog.c here. I have pointed out and discussed the duplication several times. I did it this way only because we were considering backporting the changes, and at the time it seemed better to do this and fix the duplication in a separate patch. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] transforms vs. CLOBBER_CACHE_ALWAYS
* Andrew Dunstan: friarbird is a FreeBSD buildfarm animal running with -DCLOBBER_CACHE_ALWAYS. It usually completes a run in about 6.5 hours. However, it's been stuck since Monday running the plpython regression tests. The only relevant commit seems to be the transforms feature. Here's what it's been doing: query| SELECT cursor_plan(); Same here, on jaguarundi. I actually killed it intentionally this morning, hoping that whatever the problem was might have been fixed already. No such luck. I would suspect that it might have something to do with the OS, if all the other CCA animals weren't lining up nicely behind in the buildfarm status page. I imagine it was in some sort of infinite loop. gdb says it's all in src/backend/utils/cache/plancache.c, although not the same line each time I run it. I ktrace'd it this morning, but cleverly did not keep the dump. It looked much the same to me, though, it was reading the same filenode over and over again. -- Christian -- 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: adaptive ndistinct estimator v4
Hi, On 04/30/15 22:57, Robert Haas wrote: On Tue, Mar 31, 2015 at 3:02 PM, Tomas Vondra tomas.von...@2ndquadrant.com wrote: attached is v4 of the patch implementing adaptive ndistinct estimator. So, I took a look at this today. It's interesting work, but it looks more like a research project than something we can commit to 9.5. As far as I can see, this still computes the estimate the way we do today, but then also computes the estimate using this new method. The estimate computed the new way isn't stored anywhere, so this doesn't really change behavior, except for printing out a WARNING comparing the values produced by the two estimators. I agree that this is not ready for 9.5 - it was meant as an experiment (hence printing the estimate in a WARNING, to make it easier to compare the value to the current estimator). Without that it'd be much more complicated to compare the old/new estimates, but you're right this is not suitable for commit. So far it received only reviews from Jeff Janes (thanks!), but I think a change like this really requires a more thorough review, including the math part. I don't expect that to happen at the very end of the last CF before the freeze. IMHO, the comments in this patch are pretty inscrutable. I believe this is because they presume more knowledge of what the patch is doing than I myself possess. For example: + * The AEE estimator is based on solving this equality (for m) + * + * m - f1 - f2 = f1 * (A + A(m)) / (B + B(m)) + * + * where A, B are effectively constants (not depending on m), and A(m) + * and B(m) are functions. This is equal to solving + * + * 0 = f1 * (A + A(m)) / (B + B(m)) - (m - f1 - f2) Perhaps I am just a dummy, but I have no idea what any of that means. I think that needs to be fixed so that someone who knows what n_distinct is but knows nothing about the details of these estimators can get an idea of how they are doing their thing without too much effort. I think a lot of the comments share this problem. Well, I don't think you're dummy, but this requires reading the paper describing the estimator. Explaining that fully would essentially mean copying a large portion of the paper in the comment, and I suppose that's not a good idea. The explanation might be perhaps a bit more detailed, though - not sure what's the right balance. Aside from the problems mentioned above, there's the broader question of how to evaluate the quality of the estimates produced by this estimator vs. what we're doing right now. I see that Jeff Janes has pointed out some cases that seem to regress with this patch; those presumably need some investigation, or at least some comment. And I think some testing from other people would be good too, before we commit to this. Yeah, evaluating is difficult. I think that Jeff's approach - i.e. testing the estimator on real-world data sets - is the right approach here. Testing on synthetic data sets has it's value too (if only to better understand the failure cases). Leaving that aside, at some point, you'll say, OK, there may be some regressions left but overall I believe this is going to be a win in most cases. It's going to be really hard for anyone, no matter how smart, to figure out through code review whether that is true. So committing this figures to be extremely frightening. It's just not going to be reasonably possible to know what percentage of users are going to be more happy after this change and what percentage are going to be less happy. For every pair of estimators you can find cases where one of them is better than the other one. It's pretty much impossible to find an estimator that beats all other estimators on all possible inputs. There's no way to make this an improvement for everyone - it will produce worse estimates in some cases, and we have to admit that. If we think this is unacceptable, we're effectively stuck with the current estimator forever. Therefore, I think that: 1. This should be committed near the beginning of a release cycle, not near the end. That way, if there are problem cases, we'll have a year or so of developer test to shake them out. +1 2. There should be a compatibility GUC to restore the old behavior. The new behavior should be the default, because if we're not confident that the new behavior will be better for most people, we have no business installing it in the first place (plus few people will try it). But just in case it turns out to suck for some people, we should provide an escape hatch, at least for a few releases. I think a compatibility GUC is a damn poor solution, IMNSHO. For example, GUCs are database-wide, but I do expect the estimator to perform worse only on a few data sets / columns. So making this a column-level settings would be more appropriate, I think. But it might work during the development cycle, as it would make comparing the estimators possible (just run the tests with the GUC set differently).
Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0
On 04/27/2015 11:02 PM, Peter Geoghegan wrote: On Mon, Apr 27, 2015 at 8:31 PM, Heikki Linnakangas hlinn...@iki.fi wrote: I thought we had an ironclad scheme to prevent deadlocks like this, so I'd like to understand why that happens. Okay. I think I know how it happens (I was always skeptical of the idea that this would be 100% reliable), but I'll be able to show you exactly how tomorrow. I'll have pg_xlogdump output then. I was able to reproduce this, using two sessions, so that on session does a regular INSERT, and another does INSERT ON CONFLICT, after adding a sleep(5) to a strategic place. So this was indeed a live bug, reproducible even without the hack you had to allow ON CONFLICT UPDATE with exclusion constraints. Fortunately this is easy to fix. Here's how to reproduce: 1. Insert sleep(5) into ExecInsertIndexTuples, just after the index_insert() call. 2. Create the test table and index: create extension btree_gist; create table foo (id int4, constraint foo_x exclude using gist (id with =) ); 3. Launch two psql sessions, A and B. Do the following: A: set deadlock_timeout='10s'; B: set deadlock_timeout='20s'; A: begin; select txid_current(); B: begin; select txid_current(); A: insert into foo values (1) on conflict do nothing; (the insert will hit the sleep(5) - quickly perform the second insert quickly: ) B: insert into foo values (1); At this point, both transactions have already inserted the tuple to the heap. A has done so speculatively, but B has done a regular insert. B will find A's tuple and wait until A's speculative insertion completes. A will find B's tuple, and wait until B completes, and you get the deadlock. Thanks to the way the deadlock_timeout's are set, A will detect the deadlock first and abort. That's not cool with ON CONFLICT IGNORE. To fix that, we need to fix the livelock insurance check so that A does not wait for B here. Because B is not a speculative insertion, A should cancel its speculative insertion and retry instead. (I pushed the one-line fix for that to your github repository) - 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] shared_libperl, shared_libpython
On 4/28/15 11:48 PM, Tom Lane wrote:
My preference would be to rip all that out and let the compiler or
linker decide when it doesn't want to link something.
Works for me, assuming that we get an understandable failure message and
not, say, a plperl.so that mysteriously doesn't work.
Well, I can't really guarantee that, and I also recall that in some
cases a shared/non-shared mismatch will work but be slow or something
like that.
So I went for the configure detection. For Perl, this is
straightforward. For Python and Tcl, it gets tricky because they look
at the actual file in some cases, which requires knowing DLSUFFIX, which
is not available in configure. (And it's a lie anyway, because on OS X
it does not distinguish between .so and .dylib in a principled way.)
For Tcl, this is only necessary for version before Tcl 8.0 (according to
code comments), which I think we can safely drop. For Python, it's
still necessary, so I hardcoded a few choices in an ugly way.
I think overall this patch is still an improvement in several ways.
Worst case, we could turn some of these configure errors into warnings.
diff --git a/config/python.m4 b/config/python.m4
index 7012c53..c8f784e 100644
--- a/config/python.m4
+++ b/config/python.m4
@@ -93,7 +93,6 @@ AC_MSG_RESULT([${python_libspec} ${python_additional_libs}])
AC_SUBST(python_libdir)[]dnl
AC_SUBST(python_libspec)[]dnl
AC_SUBST(python_additional_libs)[]dnl
-AC_SUBST(python_enable_shared)[]dnl
# threaded python is not supported on OpenBSD
AC_MSG_CHECKING(whether Python is compiled with thread support)
diff --git a/configure b/configure
index 7c0bd0c..cddbbef 100755
--- a/configure
+++ b/configure
@@ -641,7 +641,6 @@ TCL_SHLIB_LD_LIBS
TCL_SHARED_BUILD
TCL_LIB_SPEC
TCL_LIBS
-TCL_LIB_FILE
TCL_INCLUDE_SPEC
TCL_CONFIG_SH
TCLSH
@@ -662,7 +661,6 @@ HAVE_IPV6
LIBOBJS
UUID_LIBS
ZIC
-python_enable_shared
python_additional_libs
python_libspec
python_libdir
@@ -7384,6 +7382,12 @@ perl_useshrplib=`$PERL -MConfig -e 'print $Config{useshrplib}'`
test $PORTNAME = win32 perl_useshrplib=`echo $perl_useshrplib | sed 's,,/,g'`
{ $as_echo $as_me:${as_lineno-$LINENO}: result: $perl_useshrplib 5
$as_echo $perl_useshrplib 6; }
+ if test $perl_useshrplib != yes test $perl_useshrplib != true; then
+as_fn_error $? cannot build PL/Perl because libperl is not a shared library
+You might have to rebuild your Perl installation. Refer to the
+documentation for details. Use --without-perl to disable building
+PL/Perl. $LINENO 5
+ fi
{ $as_echo $as_me:${as_lineno-$LINENO}: checking for flags to link embedded Perl 5
$as_echo_n checking for flags to link embedded Perl... 6; }
@@ -7537,6 +7541,33 @@ $as_echo no 6; }
fi
+
+ # We need libpython as a shared library. In Python =2.5, we asks
+ # Python directly. But because this has been broken in Debian for a
+ # long time (http://bugs.debian.org/695979), and to support older
+ # Python versions, we see if there is a file that is named like a
+ # shared library as a fallback.
+
+ if test $python_enable_shared != 11; then
+# OS X does supply a .dylib even though Py_ENABLE_SHARED does not get set
+if test $PORTNAME = darwinX; then
+ python_enable_shared=1
+else
+ for dlsuffix in .so .dll .sl; do
+if ls $python_libdir/libpython*${dlsuffix}* /dev/null 21; then
+ python_enable_shared=1
+ break
+fi
+ done
+fi
+ fi
+
+ if test $python_enable_shared != 1; then
+as_fn_error $? cannot build PL/Python because libpython is not a shared library
+You might have to rebuild your Python installation. Refer to the
+documentation for details. Use --without-python to disable building
+PL/Python. $LINENO 5
+ fi
fi
if test $cross_compiling = yes test -z $with_system_tzdata; then
@@ -14736,12 +14767,15 @@ fi
. $TCL_CONFIG_SH
eval TCL_INCLUDE_SPEC=\$TCL_INCLUDE_SPEC\
-eval TCL_LIB_FILE=\$TCL_LIB_FILE\
eval TCL_LIBS=\$TCL_LIBS\
eval TCL_LIB_SPEC=\$TCL_LIB_SPEC\
eval TCL_SHARED_BUILD=\$TCL_SHARED_BUILD\
-# now that we have TCL_INCLUDE_SPEC, we can check for tcl.h
+if test $TCL_SHARED_BUILD != 1; then
+ as_fn_error $? cannot build PL/Tcl because Tcl is not a shared library
+Use --without-tcl to disable building PL/Tcl. $LINENO 5
+fi
+# now that we have TCL_INCLUDE_SPEC, we can check for tcl.h
ac_save_CPPFLAGS=$CPPFLAGS
CPPFLAGS=$TCL_INCLUDE_SPEC $CPPFLAGS
ac_fn_c_check_header_mongrel $LINENO tcl.h ac_cv_header_tcl_h $ac_includes_default
diff --git a/configure.in b/configure.in
index 1cd9e1e..a9e2257 100644
--- a/configure.in
+++ b/configure.in
@@ -889,12 +889,45 @@ if test $with_perl = yes; then
AC_MSG_ERROR([Perl not found])
fi
PGAC_CHECK_PERL_CONFIGS([archlibexp,privlibexp,useshrplib])
+ if test $perl_useshrplib != yes test $perl_useshrplib != true; then
+AC_MSG_ERROR([cannot build PL/Perl because libperl is not a shared library
+You might have to rebuild your Perl
Re: [HACKERS] Reducing tuple overhead
On Thu, Apr 30, 2015 at 7:24 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 30, 2015 at 9:46 AM, Amit Kapila amit.kapil...@gmail.com wrote: As the index expression contain table columns and all the functions or operators used in expression must be IMMUTABLE, won't that guarantee to avoid such a situation? The concern is that they might be labeled as immutable but not actually behave that way. The other, related problem is that the ordering operator might start to return different results than it did at index creation time. For example, consider a btree index built on a text column. Now consider 'yum update'. glibc gets updated, collation ordering of various strings change, and now you've got tuples that are in the wrong place in the index, because when the index was built, we thought A B, but now we think B A. You would think the glibc maintainers might avoid such changes in minor releases, or that the Red Hat guys would avoid packaging and shipping those changes in minor releases, but you'd be wrong. We've seen cases where the master and the standby were both running RHEL X.Y (same X.Y on both servers) but they didn't agree on the collation definitions, so queries that worked on the master failed on the slave. This is quite similar to IMMUTABLE function, but for an operator. I think guaranteeing the immutable nature for a function is the job of application/user. Oracle uses something similar (DETERMINISTIC functions) for function based indexes and asks user to ensure the DETERMINISTIC property of function. I think having synchronous delete (delete from index at the same time as from heap) is one of the main differentiating factor for not having bloat in indexes in some of the other databases. If we don't want to change the current property for functions or operators for indexes, then we can have a new type of index which can have visibility information and users are advised to use such an index where they can ensure that functions or operators for that index are IMMUTABLE. Here, there is one argument that users might or might not be able to ensure the same, but I think if it can be ensured by users of other successful databases, then the same should be possible for PostgreSQL users as well, after all this can bring a lot of value on table (avoiding index bloat) for users. I think it will still give us lot of benefit in more common cases. It's hard to figure out exactly what can work here. Aside from correctness issues, the biggest problem with refinding the index tuples one-by-one and killing them is that it may suck for bulk operations. When you delete one tuple from the table, refinding the index tuples and killing them immediately is probably the smartest thing you can do. If you postpone it, somebody may be forced to split the page because it's full, whereas if you do it right away, the next guy who wants to put a tuple on that page may be able to make it fit. That's a big win. Exactly, I have that big win in my mind. But if you want to delete 10 million tuples, doing an index scan for each one may induce a tremendous amount of random I/O on the index pages, possibly visiting and dirtying the same pages more than once. Scanning the whole index for tuples to kill avoids that. Even doing it only from heap might not be cheap. I think for doing BULK delete (as you described), users of other databases have some smart ways like: a. If possible drop the indexes b. instead of deleting the records you don't want, SELECT OUT the records you do -- drop the old table -- rename new table. c. Archive instead of delete Instead of deleting, just set a flag to mark a row as archived, invalid, deleted, etc. This will avoid the immediate overhead of index maintenance. Ignore the rows temporarily and let some other job delete them later. The large downside to this is that it affects any query on the table. ... possibly some other ways. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
[HACKERS] BuildTupleFromCStrings Memory Documentation?
Within the core codebase, BuildTupleFromCStrings is often called within a temporary memory context cleared after the call. In dblink.c, this is justified as being needed to “[clean up] not only the data we have direct access to, but anycruft the I/O functions might leak”.I wrote a pretty minimal case to call BuildTupleFromCStrings in a loop (attached) and found that I was using 40GB of RAM in a few minutes, though I was not allocating any memory myself and immediately freed the tuple it returned.Is the need to wrap this call in a protective context documented anywhere? Portions of the documentation useBuildTupleFromCStrings in examples without mentioning this precaution. Is it just well-known, or did I miss a README or comment somewhere? --Jason PetersenSoftware Engineer | Citus Data303.736.9255ja...@citusdata.com tup_memleak.c Description: Binary data signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
On Tue, Mar 10, 2015 at 02:51:30PM -0400, Robert Haas wrote: On Tue, Mar 10, 2015 at 2:32 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 1. funcname_signature_string 2. get_rule_expr Thanks. Patch attached. I'll commit this if there are no objections. Robert, are you going to apply this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Final Patch for GROUPING SETS
On Thu, Apr 30, 2015 at 05:35:26AM +0100, Andrew Gierth wrote: Andres == Andres Freund and...@anarazel.de writes: + * TODO: AGG_HASHED doesn't support multiple grouping sets yet. Andres Are you intending to resolve this before an eventual commit? Original plan was to tackle AGG_HASHED after a working implementation was committed; +1 for that plan. Andres Possibly after the 'structural' issues are resolved? Or do you Andres think this can safely be put of for another release? I think the feature is useful even without AGG_HASHED, even though that means it can sometimes be beaten on performance by using UNION ALL of many separate GROUP BYs; but I'd defer to the opinions of others on that point. It will be a tough call, and PostgreSQL has gone each way on some recent feature. I recommend considering both GroupAggregate and HashAggregate in all design discussion but continuing to work toward a first commit implementing GroupAggregate alone. With that in the tree, we'll be in a better position to decide whether to release a feature paused at that stage in its development. Critical facts are uncertain, so a discussion today would be unproductive. Andres So, having quickly scanned through the patch, do I understand Andres correctly that the contentious problems are: Andres * Arguably this converts the execution *tree* into a DAG. Tom Andres seems to be rather uncomfortable with that. I am wondering Andres whether this really is a big deal - essentially this only Andres happens in a relatively 'isolated' part of the tree right? Andres I.e. if those chained together nodes were considered one node, Andres there would not be any loops? Additionally, the way Andres parametrized scans works already essentially violates the Andres tree paradigma somewhat. I agree with your assessment. That has been contentious. The major downsides as I see them with the current approach are: 1. It makes plans (and hence explain output) nest very deeply if you have complex grouping sets (especially cubes with high dimensionality). This can make explain very slow in the most extreme cases I'm not worried about that. If anything, the response is to modify explain to more-quickly/compactly present affected plan trees. 2. A union-based approach would have a chance of including AGG_HASHED support without any significant code changes, - CTE x - entire input subplan here - Append - GroupAggregate - Sort - CTE Scan x - GroupAggregate - Sort - CTE Scan x - HashAggregate - CTE Scan x [...] This uses 50-67% more I/O than the current strategy, which makes it a dead end from my standpoint. Details: http://www.postgresql.org/message-id/20141221210005.ga1864...@tornado.leadboat.com Andres Are those the two bigger controversial areas? Or are there Andres others in your respective views? Another controversial item was the introduction of GroupedVar. I know of no additional controversies to add to this list. Thanks, nm -- 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] transforms vs. CLOBBER_CACHE_ALWAYS
On 4/30/15 2:49 PM, Andrew Dunstan wrote: friarbird is a FreeBSD buildfarm animal running with -DCLOBBER_CACHE_ALWAYS. It usually completes a run in about 6.5 hours. However, it's been stuck since Monday running the plpython regression tests. The only relevant commit seems to be the transforms feature. I can reproduce it. I'll look into it. -- 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] One question about security label command
2015-05-01 7:40 GMT+09:00 Alvaro Herrera alvhe...@2ndquadrant.com: Kouhei Kaigai wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: The idea of making the regression test entirely independent of the system's policy would presumably solve this problem, so I'd kind of like to see progress on that front. Apologies, I guess it wasn't clear, but that's what I was intending to advocate. OK, I'll try to design a new regression test policy that is independent from the system's policy assumption, like unconfined domain. Please give me time for this work. Any progress here? Not done. The last version I rebuild had a trouble on user/role transition from unconfined_u/unconfined_r to the self defined user/role... So, I'm trying to keep the user/role field (that is not redefined for several years) but to define self domain/types (that have been redefined multiple times) for the regression test at this moment. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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: adaptive ndistinct estimator v4
On 05/01/15 00:18, Robert Haas wrote: On Thu, Apr 30, 2015 at 5:31 PM, Heikki Linnakangas hlinn...@iki.fi wrote: You can override the ndistinct estimate with ALTER TABLE. I think that's enough for an escape hatch. I'm not saying that isn't nice to have, but I don't think it really helps much here. Setting the value manually requires that you know what value to set, and you might not. If, on some workloads, the old algorithm beats the new one reliably, you want to be able to actually go back to the old algorithm, not manually override every wrong decision it makes. A GUC for this is pretty cheap insurance. IMHO this is exactly the same situation as with the current ndistinct estimator. If we find out we'd have to use this workaround more frequently than before, then clearly the new estimator is rubbish and should not be committed. In other words, I agree with Heikki. -- Tomas Vondra http://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] Proposal : REINDEX xxx VERBOSE
On Fri, May 1, 2015 at 1:38 AM, Robert Haas robertmh...@gmail.com wrote:
On Thu, Apr 30, 2015 at 9:15 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
On Thu, Apr 30, 2015 at 8:39 PM, Robert Haas robertmh...@gmail.com wrote:
On Thu, Apr 30, 2015 at 6:37 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:
Attached v10 patch is latest version patch.
The syntax is,
REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ]
That is, WITH clause is optional.
I thought we agreed on moving this earlier in the command:
http://www.postgresql.org/message-id/18569.1423159...@sss.pgh.pa.us
Oh, I see.
Attached patch is modified syntax as
REINDEX [VERBOSE] { INDEX | ... } name
Thought?
I thought what we agreed on was:
REINDEX (flexible options) { INDEX | ... } name
The argument wasn't about whether to use flexible options, but where
in the command to put them.
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?
And CLUSTER should have syntax like that in future?
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] proposal: disallow operator = and use it for named parameters
It is done Dne 1.5.2015 3:11 napsal uživatel Bruce Momjian br...@momjian.us: On Tue, Mar 10, 2015 at 02:51:30PM -0400, Robert Haas wrote: On Tue, Mar 10, 2015 at 2:32 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 1. funcname_signature_string 2. get_rule_expr Thanks. Patch attached. I'll commit this if there are no objections. Robert, are you going to apply this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Re: [HACKERS] initdb -S and tablespaces
At 2015-04-30 16:56:17 -0700, t...@sss.pgh.pa.us wrote: As for the notion that this needs to be back-patched, I would say no. Not even just the fsync after crash part? I could separate that out from the control file changes and try to eliminate the duplication. I think that would be worth back-patching, at least. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] One question about security label command
Kouhei Kaigai wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: The idea of making the regression test entirely independent of the system's policy would presumably solve this problem, so I'd kind of like to see progress on that front. Apologies, I guess it wasn't clear, but that's what I was intending to advocate. OK, I'll try to design a new regression test policy that is independent from the system's policy assumption, like unconfined domain. Please give me time for this work. Any progress here? -- Á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] collations in shared catalogs?
On Thu, Apr 30, 2015 at 08:16:09AM -0700, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On 2015-02-25 12:08:32 -0500, Tom Lane wrote: The most obvious fix is to change provider to a NAME column. Where are we on this? Not done yet, but we should make a point of making that fix before 9.5. Please add it to the open items page for 9.5. I am not sure there's anything useful to be done about this in the back branches. Done. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Broken handling of NULLs in TG_ARGV
Jim Nasby jim.na...@bluetreble.com writes: plpgsql's handling of NULLs in TG_ARGV turns actual nulls into text 'null'. Hopefully we can all agree that's broken. You apparently have not read the CREATE TRIGGER reference page very carefully: arguments An optional comma-separated list of arguments to be provided to the function when the trigger is executed. The arguments are literal string constants. Simple names and numeric constants can be written here, too, but they will all be converted to strings. There isn't any such thing as a genuine SQL NULL argument; the examples you provided are just text strings, not SQL NULLs. In order to make them be actual nulls, we would have to redefine the arguments as being expressions of some sort, which is problematic for backwards-compatibility reasons. It also seems like rather a lot of new mechanism to add for something with (evidently) near-zero user demand. 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] Use outerPlanState() consistently in executor code
On Thu, Apr 30, 2015 at 1:44 PM, Qingqing Zhou zhouqq.postg...@gmail.com wrote: On Thu, Apr 30, 2015 at 8:02 AM, Robert Haas robertmh...@gmail.com wrote: I don't mind the MSDOS newlines, but the UTF-16le bit is inconvenient. UTF-8 would be much better, so I don't have to figure out how to convert. The patch is generated via github windows tool and that's possibly why. I regenerated it in Linux box and see attached (sending this email in Windows and I hope no magic happens in-between). Please let me know if that works. Yeah, that seems fine. Anyone want to object to 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] One question about security label command
Stephen Frost wrote: Hi, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Could you provide a buildfarm animal that runs the sepgsql test in all branches on a regular basis? Would be great if KaiGai can, of course, but I'm planning to stand one up here soon in any case. I don't think this is done, is it? -- Á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
