Re: [HACKERS] Audit of logout
On Tue, Aug 5, 2014 at 8:04 PM, Fujii Masao masao.fu...@gmail.com wrote:
Yep, the attached patch introduces PGC_SU_BACKEND and
changes the contexts of log_connections and log_disconnections
to PGC_SU_BACKEND. Review?
1.
! else if (context != PGC_POSTMASTER context != PGC_SU_BACKEND
! context != PGC_SU_BACKEND source != PGC_S_CLIENT)
In the above check for PGC_SU_BACKEND is repeated, here
one of the check should be PGC_SU_BACKEND and other
should be PGC_BACKEND.
2.
+ case PGC_SU_BACKEND:
+ if (context == PGC_BACKEND)
+ {
..
..
+ return 0;
+ }
case PGC_BACKEND:
if (context == PGC_SIGHUP)
Changing PGC_SU_BACKEND parameter (log_connections) is
visible even with a non-super user client due to above code.
Shouldn't it be only visible for super-user logins?
Simple steps to reproduce the problem:
a. start Server (default configuration)
b. connect with superuser
c. change in log_connections to on in postgresql.conf
d. perform select pg_reload_conf();
e. connect with non-super-user
f. show log_connections; --This step shows the value as on,
--whereas I think it should have
been off
This test has been performed on *Windows*.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Sat, Aug 23, 2014 at 3:32 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Great. Pushed. Thanks for the patch. There is a typo in what has been pushed. Patch attached. -- Michael diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d37534e..1bb46ea 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10722,7 +10722,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged) /* * Scan conrelid if changing to permanent, else confrelid. This also - * determines whether an useful index exists. + * determines whether a useful index exists. */ ScanKeyInit(skey[0], toLogged ? Anum_pg_constraint_conrelid : -- 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Sat, Aug 23, 2014 at 8:53 AM, Michael Paquier michael.paqu...@gmail.com
wrote:
On Sat, Aug 23, 2014 at 3:32 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
Great. Pushed. Thanks for the patch.
There is a typo in what has been pushed. Patch attached.
Thanks... I fixed that in my last patch to change 'loggedness' to
'persistence'. Attached.
Regards,
--
FabrÃzio de Royes Mello
Consultoria/Coaching PostgreSQL
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d37534e..1d2fe1f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -152,7 +152,7 @@ typedef struct AlteredTableInfo
bool new_notnull; /* T if we added new NOT NULL constraints */
bool rewrite; /* T if a rewrite is forced */
Oid newTableSpace; /* new tablespace; 0 means no change */
- bool chgLoggedness; /* T if SET LOGGED/UNLOGGED is used */
+ bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
char newrelpersistence; /* if above is true */
/* Objects to rebuild after completing ALTER TYPE operations */
List *changedConstraintOids; /* OIDs of constraints to rebuild */
@@ -388,8 +388,8 @@ static void change_owner_recurse_to_sequences(Oid relationOid,
static void ATExecClusterOn(Relation rel, const char *indexName,
LOCKMODE lockmode);
static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
-static bool ATPrepChangeLoggedness(Relation rel, bool toLogged);
-static void ATChangeIndexesLoggedness(Oid relid, char relpersistence);
+static bool ATPrepChangePersistence(Relation rel, bool toLogged);
+static void ATChangeIndexesPersistence(Oid relid, char relpersistence);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename, LOCKMODE lockmode);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
@@ -3174,19 +3174,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_SetLogged: /* SET LOGGED */
ATSimplePermissions(rel, ATT_TABLE);
- tab-chgLoggedness = ATPrepChangeLoggedness(rel, true);
+ tab-chgPersistence = ATPrepChangePersistence(rel, true);
tab-newrelpersistence = RELPERSISTENCE_PERMANENT;
/* force rewrite if necessary */
- if (tab-chgLoggedness)
+ if (tab-chgPersistence)
tab-rewrite = true;
pass = AT_PASS_MISC;
break;
case AT_SetUnLogged: /* SET UNLOGGED */
ATSimplePermissions(rel, ATT_TABLE);
- tab-chgLoggedness = ATPrepChangeLoggedness(rel, false);
+ tab-chgPersistence = ATPrepChangePersistence(rel, false);
tab-newrelpersistence = RELPERSISTENCE_UNLOGGED;
/* force rewrite if necessary */
- if (tab-chgLoggedness)
+ if (tab-chgPersistence)
tab-rewrite = true;
pass = AT_PASS_MISC;
break;
@@ -3668,7 +3668,7 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
* Select persistence of transient table (same as original unless
* user requested a change)
*/
- persistence = tab-chgLoggedness ?
+ persistence = tab-chgPersistence ?
tab-newrelpersistence : OldHeap-rd_rel-relpersistence;
heap_close(OldHeap, NoLock);
@@ -3705,8 +3705,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
* because the rewrite step might read the indexes, and that would
* cause buffers for them to have the wrong setting.
*/
- if (tab-chgLoggedness)
-ATChangeIndexesLoggedness(tab-relid, tab-newrelpersistence);
+ if (tab-chgPersistence)
+ATChangeIndexesPersistence(tab-relid, tab-newrelpersistence);
/*
* Swap the physical files of the old and new heaps, then rebuild
@@ -4119,7 +4119,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
tab-relkind = rel-rd_rel-relkind;
tab-oldDesc = CreateTupleDescCopy(RelationGetDescr(rel));
tab-newrelpersistence = RELPERSISTENCE_PERMANENT;
- tab-chgLoggedness = false;
+ tab-chgPersistence = false;
*wqueue = lappend(*wqueue, tab);
@@ -10678,7 +10678,7 @@ ATExecGenericOptions(Relation rel, List *options)
* checks are skipped), otherwise true.
*/
static bool
-ATPrepChangeLoggedness(Relation rel, bool toLogged)
+ATPrepChangePersistence(Relation rel, bool toLogged)
{
Relation pg_constraint;
HeapTuple tuple;
@@ -10722,7 +10722,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
/*
* Scan conrelid if changing to permanent, else confrelid. This also
- * determines whether an useful index exists.
+ * determines whether a useful index exists.
*/
ScanKeyInit(skey[0],
toLogged ? Anum_pg_constraint_conrelid :
@@ -10792,7 +10792,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
* given persistence.
*/
static void
-ATChangeIndexesLoggedness(Oid relid, char relpersistence)
+ATChangeIndexesPersistence(Oid relid, char relpersistence)
{
Re: [HACKERS] Parallel Sequence Scan doubts
On 08/21/2014 02:47 PM, Haribabu Kommi wrote: Corrected subject. Hi Hackers, Implementation of Parallel Sequence Scan I think you mean Parallel Sequential Scan. Scanning a sequence in parallel doesn't make much sense. 1.Parallel Sequence Scan can achieved by using the background workers doing the job of actual sequence scan including the qualification check also. Only if the qualifiers are stable/immutable I think. Not even necessarily stable functions - consider use of the fmgr/executor state contexts to carry information over between calls, and what parallel execution would do to that. 3. In the executor Init phase, Try to copy the necessary data required by the workers and start the workers. Copy how? Back-ends can only communicate with each other over shared memory, signals, and using sockets. Presumably you'd use a dynamic shared memory segment, but it's going to be interesting to copy that kind of state over. Some of the work Robert has proposed to add support for data structures that are native to dynamic shmem might help, I guess... 4. In the executor run phase, just get the tuples which are sent by the workers and process them further in the plan node execution. Again, how do you propose to copy these back to the main bgworker? That's been one of the things that's limited parallel scan when it's been looked at before. 1. Data structures that are required to be copied from backend to worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate and etc. That's a big etc. Huge, in fact. Any function can reference any global variable. Even an immutable function might use globals for cache etc - and might, for example, set them up using an executor start hook. You cannot really make any assumptions about what functions access what memory. So it's not possible, as far as I can understand, to define a simple subset of state that must be copied. Nor is it necessarily correct to discard the copied state at the end of the run even if you can copy it. Code may well depend on that state being updated and preserved across calls. I see some problems in copying Estate data structure into the shared memory because it contains so many pointers. There is a need of some infrastructure to copy these data structures into the shared memory. It's not just a matter of copying them into/via shmem. It's about their meaning. Does it even make sense to copy the executor state to another backend? If so, you can't copy it back, so what do you do at the end of the scans? Any suggestions? Before you try to design anything more on this, study the *large* amount of discussion that has happened on this topic on this mailing list over the last years. This is not a simple or easy task, and it's not one you should approach without studying what's already been worked on, built, contemplated, etc. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: rounding up time value less than its unit.
Tomonari Katsumata t.katsumata1...@gmail.com writes: This patch rounds up the value when only it's less than required unit. .. Although my original complaint is fixed, I'm worried about this change will make users confusing. Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Is it better to raise a message(ex. INFO) when a value less than required unit is set? No. Non-error messages are probably completely useless in this area: users will typically never see such messages for settings made in postgresql.conf, because it will not occur to them to look in the postmaster log. So the behavior needs to be self-explanatory without any messages; and that means it had better not be very complicated. 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote Tomonari Katsumata lt; t.katsumata1122@ gt; writes: This patch rounds up the value when only it's less than required unit. .. Although my original complaint is fixed, I'm worried about this change will make users confusing. Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Values failing in the range (0, 1) in the canonical unit must be treated specially otherwise we might as well just leave the current behavior as-is since floor is likely just as good a rule as round-nearest. For fractions greater than one round nearest is probably fine and indeed on average results in the least amount of potential adjustment magnitude. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816007.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: rounding up time value less than its unit.
David G Johnston david.g.johns...@gmail.com writes: Tom Lane-2 wrote Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or we could adopt Peter's idea that zero shouldn't be special (instead using, say, -1 to turn things off). But I'm afraid that would break way too many peoples' configuration choices. 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] Removing dependency to wsock32.lib when compiling code on WIndows
On Fri, Aug 22, 2014 at 04:58:47PM +0900, Michael Paquier wrote: Looking more into that, I am seeing that MinGW-32 is failing to find socket at configure, contrary to MinGW-64. Here is what happens for MinGW-64 at configure: configure:7638: checking for library containing socket [...] configure:7669: x86_64-w64-mingw32-gcc -o conftest.exe -Wall -Wmissing-prototypes -Wpointer-arith \ -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-stri\ ct-aliasing -fwrapv -fexcess-precision=standard -g -I./src/include/port/win32 -DEXEC_BACKEND -Wl\ ,--allow-multiple-definition -Wl,--disable-auto-import conftest.c -lws2_32 -lm 5 configure:7669: $? = 0 configure:7686: result: -lws2_32 And for MinGW-32: configure:7638: checking for library containing socket [...] configure:7669: gcc -o conftest.exe -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after\ -statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv\ -fexcess-precision=standard -g -I./src/include/port/win32 -DEXEC_BACKEND -Wl,--allow-multiple-d\ efinition -Wl,--disable-auto-import conftest.c -lws2_32 -lm 5 C:\Users\ioltas\AppData\Local\Temp\cciNV1Y8.o: In function `main': c:\Users\ioltas\git\postgres/conftest.c:33: undefined reference to `socket' collect2.exe: error: ld returned 1 exit status I see that ws2_32.def for 64-bit MinGW-w64 exports plain socket, while 32-bit MinGW-w64 and 32-bit MinGW export socket@12. In other words, 64-bit MinGW-w64 exports a __cdecl socket function, and the others export a __stdcall socket function. AC_SEARCH_LIBS doesn't work with __stdcall. I am not sure which way is better, do we want HAVE_GETADDRINFO or !HAVE_GETADDRINFO in all Windows builds? If we choose the former, we'll need to understand why -lws2_32 is not added to LIBS for MinGW-32. If we choose the latter, we would need to remove -lws2_32 from LIBS with MinGW-64 for consistency with MinGW-32 I think. HAVE_GETADDRINFO is preferable whenever the OS functions getaddrinfo.h would replace are fully adequate. When PostgreSQL established its longstanding Windows behavior in this area, that was not so. A few generations of Windows have since gone out of support, so perhaps the situation has changed. -- 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] change alter user to be a true alias for alter role
Jov am...@amutu.com writes:
I make the v2 of the patch,use Tom's advice.
But I can't make ROLE and USER in the keyword list,it is hard to solve the
conflict,or rewrite many gram rules.
This patch seems to be trying to make ROLE and USER interchangeable
*everywhere* in the grammar, which is moving the goalposts quite a long
way to little purpose. The idea seems rather misguided to me, since
certainly CREATE USER and CREATE ROLE will never be exact synonyms.
I think it's sufficient to solve the original complaint about them not
being interchangeable in ALTER.
The patch as given is broken anyway: it removes both those keywords
from the keyword lists, which effectively makes them fully reserved,
in fact worse than fully reserved.
What I had in mind was more like the attached, which I've not tested
but bison does compile it without complaint. I've not looked at the
documentation end of it, either.
regards, tom lane
*** src/backend/parser/gram.y~ Sat Aug 23 14:53:23 2014
--- src/backend/parser/gram.y Sat Aug 23 18:10:56 2014
*** static Node *makeRecursiveViewSelect(cha
*** 230,236
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
! AlterCompositeTypeStmt AlterUserStmt AlterUserMappingStmt AlterUserSetStmt
AlterRoleStmt AlterRoleSetStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
--- 230,236
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
! AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
*** stmt :
*** 749,756
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
- | AlterUserSetStmt
- | AlterUserStmt
| AnalyzeStmt
| CheckPointStmt
| ClosePortalStmt
--- 749,754
*** CreateUserStmt:
*** 1020,1029
*
* Alter a postgresql DBMS role
*
*/
AlterRoleStmt:
! ALTER ROLE RoleId opt_with AlterOptRoleList
{
AlterRoleStmt *n = makeNode(AlterRoleStmt);
n-role = $3;
--- 1018,1029
*
* Alter a postgresql DBMS role
*
+ * ALTER USER is accepted interchangeably with ALTER ROLE.
+ *
*/
AlterRoleStmt:
! ALTER role_or_user RoleId opt_with AlterOptRoleList
{
AlterRoleStmt *n = makeNode(AlterRoleStmt);
n-role = $3;
*** AlterRoleStmt:
*** 1033,1045
}
;
opt_in_database:
/* EMPTY */ { $$ = NULL; }
| IN_P DATABASE database_name { $$ = $3; }
;
AlterRoleSetStmt:
! ALTER ROLE RoleId opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n-role = $3;
--- 1033,1050
}
;
+ role_or_user:
+ ROLE
+ | USER
+ ;
+
opt_in_database:
/* EMPTY */ { $$ = NULL; }
| IN_P DATABASE database_name { $$ = $3; }
;
AlterRoleSetStmt:
! ALTER role_or_user RoleId opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n-role = $3;
*** AlterRoleSetStmt:
*** 1047,1053
n-setstmt = $5;
$$ = (Node *)n;
}
! | ALTER ROLE ALL opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n-role = NULL;
--- 1052,1058
n-setstmt = $5;
$$ = (Node *)n;
}
! | ALTER role_or_user ALL opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n-role = NULL;
*** AlterRoleSetStmt:
*** 1060,1095
/*
*
- * Alter a postgresql DBMS user
- *
- */
-
- AlterUserStmt:
- ALTER USER RoleId opt_with AlterOptRoleList
- {
- AlterRoleStmt *n = makeNode(AlterRoleStmt);
- n-role = $3;
- n-action = +1; /* add, if there are members */
- n-options = $5;
- $$ = (Node *)n;
- }
- ;
-
-
- AlterUserSetStmt:
- ALTER USER RoleId SetResetClause
- {
- AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
- n-role = $3;
- n-database = NULL;
- n-setstmt = $4;
- $$ = (Node *)n;
- }
- ;
-
-
- /*
- *
*
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or make it an error to specify a value that rounds to 0 but isn't 0. -- greg -- 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: rounding up time value less than its unit.
Tom Lane-2 wrote David G Johnston lt; david.g.johnston@ gt; writes: Tom Lane-2 wrote Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or we could adopt Peter's idea that zero shouldn't be special (instead using, say, -1 to turn things off). But I'm afraid that would break way too many peoples' configuration choices. Yes, using ceil() is the most acceptable, for me, solution that does not involve throwing an error. Are there any examples of where treating zero specially is a problem or is this concern theoretical? We've already decided to risk enabling disabled features by applying this patch since the likelihood of someone relying on the rounding to keep the feature disabled (or at its default value in some instances) is unlikely. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816018.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel Sequence Scan doubts
On Sun, Aug 24, 2014 at 1:11 AM, Craig Ringer cr...@2ndquadrant.com wrote: On 08/21/2014 02:47 PM, Haribabu Kommi wrote: Implementation of Parallel Sequence Scan I think you mean Parallel Sequential Scan. Sorry for not being clear, Yes it is parallel sequential scan. 1.Parallel Sequence Scan can achieved by using the background workers doing the job of actual sequence scan including the qualification check also. Only if the qualifiers are stable/immutable I think. Not even necessarily stable functions - consider use of the fmgr/executor state contexts to carry information over between calls, and what parallel execution would do to that. Thanks for your input. As of now we are targeting only immutable functions, that can be executed in parallel without any side effects. 3. In the executor Init phase, Try to copy the necessary data required by the workers and start the workers. Copy how? Back-ends can only communicate with each other over shared memory, signals, and using sockets. Sorry for not being clear, copying those data structures into dynamic shared memory only. From there the workers can access. 4. In the executor run phase, just get the tuples which are sent by the workers and process them further in the plan node execution. Again, how do you propose to copy these back to the main bgworker? With the help of message queues that are created in the dynamic shared memory, the workers can send the data to the queue. On other side the main backend receives the tuples from the queue. 1. Data structures that are required to be copied from backend to worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate and etc. That's a big etc. Huge, in fact. Any function can reference any global variable. Even an immutable function might use globals for cache etc - and might, for example, set them up using an executor start hook. You cannot really make any assumptions about what functions access what memory. Yes you are correct. For that reason only I am thinking of Supporting of functions that only dependent on input variables and are not modifying any global data. I see some problems in copying Estate data structure into the shared memory because it contains so many pointers. There is a need of some infrastructure to copy these data structures into the shared memory. It's not just a matter of copying them into/via shmem. It's about their meaning. Does it even make sense to copy the executor state to another backend? If so, you can't copy it back, so what do you do at the end of the scans? If we handle the locking of relation in the backend and avoid doing the parallel sequential scan if any sub query is involved, then there is no need of full estate in the worker. In those cases by sharing less information, I think we can execute the plan in the worker. Any suggestions? Before you try to design anything more on this, study the *large* amount of discussion that has happened on this topic on this mailing list over the last years. This is not a simple or easy task, and it's not one you should approach without studying what's already been worked on, built, contemplated, etc. Thanks for your information. Regards, Hari Babu Fujitsu Australia -- 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 Sequence Scan doubts
On 08/24/2014 09:40 AM, Haribabu Kommi wrote: Any suggestions? Another point I didn't raise first time around, but that's IMO quite significant, is that you haven't addressed why this approach to fully parallel seqscans is useful and solves real problems in effective ways. It might seem obvious - of course they're useful. But I see two things they'd address: - CPU-limited sequential scans, where expensive predicates are filtering the scan; and - I/O limited sequential scans, where the predicates already execute fast enough on one CPU, so most time is spent waiting for more disk I/O. The problem I see with your design is that it's not going to be useful for a large class of CPU-limited scans where the predicate isn't composed entirely of immutable functions an operators. Especially since immutable-only predicates are the best candidates for expression indexes anyway. While it'd likely be useful for I/O limited scans, it's going to increase contention on shared_buffers locking and page management. More importantly, is it the most efficient way to solve the problem with I/O limited scans? I would seriously suggest looking at first adding support for asynchronous I/O across ranges of extents during a sequential scan. You might not need multiple worker backends at all. I'm sure using async I/O to implement effective_io_concurrency for seqscans has been been discussed and explored before, so again I think some time in the list archives might make sense. I don't know if it makes sense to do something as complex and parallel multi-process seqscans without having a path forward for supporting non-immutable functions - probably with fmgr API enhancements, additional function labels (PARALLEL), etc, depending on what you find is needed. Do you have specific workloads where you see this as useful, and where doing async I/O and readahead within a single back-end wouldn't solve the same problem? 3. In the executor Init phase, Try to copy the necessary data required by the workers and start the workers. Copy how? Back-ends can only communicate with each other over shared memory, signals, and using sockets. Sorry for not being clear, copying those data structures into dynamic shared memory only. From there the workers can access. That'll probably work with read-only data, but it's not viable for read/write data unless you use a big lock to protect it, in which case you lose the parallelism you want to achieve. You'd have to classify what may be modified during scan execution carefully and determine if you need to feed any of the resulting modifications back to the original backend - and how to merge modifications by multiple workers, if it's even possible. That's going to involve a detailed structure-by-structure analysis and seems likely to be error prone and buggy. I think you should probably talk to Robert Haas about what he's been doing over the last couple of years on parallel query. 4. In the executor run phase, just get the tuples which are sent by the workers and process them further in the plan node execution. Again, how do you propose to copy these back to the main bgworker? With the help of message queues that are created in the dynamic shared memory, the workers can send the data to the queue. On other side the main backend receives the tuples from the queue. OK, so you plan to implement shmem queues. That'd be a useful starting point, as it'd be something that would be useful in its own right. You'd have to be able to handle individual values that're than the ring buffer or whatever you're using for transfers, in case you're dealing with already-detoasted tuples or in-memory tuples. Again, chatting with Robert and others who've worked on dynamic shmem, parallel query, etc would be wise here. Yes you are correct. For that reason only I am thinking of Supporting of functions that only dependent on input variables and are not modifying any global data. You'll want to be careful with that. Nothing stops an immutable function referencing a cache in a C global that it initializes one and then treats as read only, for example. I suspect you'll need a per-function whitelist. I'd love to be wrong. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max
On Mon, Aug 18, 2014 at 8:12 PM, Asif Naeem anaeem...@gmail.com wrote: Thank you for sharing updated patch. With latest 9.5 source code, patch build is failing with following error message i.e. /Applications/Xcode.app/Contents/Developer/usr/bin/make -C catalog schemapg.h cd ../../../src/include/catalog '/opt/local/bin/perl' ./duplicate_oids 3255 make[3]: *** [postgres.bki] Error 1 make[2]: *** [submake-schemapg] Error 2 make[1]: *** [all-backend-recurse] Error 2 make: *** [all-src-recurse] Error 2 New function is being added by following commit i.e. commit b34e37bfefbed1bf9396dde18f308d8b96fd176c Author: Robert Haas rh...@postgresql.org Date: Thu Aug 14 12:09:52 2014 -0400 Add sortsupport routines for text. This provides a small but worthwhile speedup when sorting text, at least in cases to which the sortsupport machinery applies. Robert Haas and Peter Geoghegan It seems that you need to use another oid. Other than this patch looks good to me, please share updated patch and feel free to assign it to committer. Thanks. Thanks for your review. Please find the rebased patch to latest HEAD. Regards, Hari Babu Fujitsu Australia inet_agg_v8.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Missing comment block at the top of streamutil.h and receivelog.h
Hi all, As mentioned in $subject, the header files in src/bin/pg_basebackup do not have a comment block at the top and do not have any copyright text. Any reason for that? Shouldn't we have something for consistency with the other files like in the patch attached? Regards, -- Michael diff --git a/src/bin/pg_basebackup/receivelog.h b/src/bin/pg_basebackup/receivelog.h index 72f8245..cd02d99 100644 --- a/src/bin/pg_basebackup/receivelog.h +++ b/src/bin/pg_basebackup/receivelog.h @@ -1,3 +1,16 @@ +/*- + * + * receivelog.h + * + * Author: Magnus Hagander mag...@hagander.net + * + * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/bin/pg_basebackup/receivelog.h + *- + */ + #include libpq-fe.h #include access/xlogdefs.h diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h index c36a37b..a37d9cd 100644 --- a/src/bin/pg_basebackup/streamutil.h +++ b/src/bin/pg_basebackup/streamutil.h @@ -1,3 +1,16 @@ +/*- + * + * streamutil.h + * + * Author: Magnus Hagander mag...@hagander.net + * + * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/bin/pg_basebackup/streamutil.h + *- + */ + #include libpq-fe.h extern const char *progname; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
