Re: [HACKERS] newline conversion in SQL command strings
On 20.09.2012 05:56, Peter Eisentraut wrote: I have received a number of bug reports about plsh choking on Windows-style line endings. The problem is that the user uses some Windows-based tool or other to execute an SQL command line this: CREATE FUNCTION foo() RETURNS somethingCRLF LANGUAGE plshCRLF AS $$CRLF #!/bin/shCRLF CRLF do somethingCRLF do somethingCRLF $$;CRLF which (apparently, I don't have Windows handy) creates a function with the prosrc body of 'CRLF #!/bin/shCRLF CRLF do somethingCRLF do somethingCRLF ' But executing this fails because Unix shells rejectCR characters in inappropriate places as syntax errors. I don't know how to handle that. It would be unfortunate to have the behavior of a function depend on the kind of client used to create or modify it. Could you strip the CRs? Either at CREATE FUNCTION time, or when the function is executed. - 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] Fwd: PATCH: psql boolean display
It doesn't look like this patch is going anywhere. I agree with Tom's
comments that we need to think how this works for all datatypes, not
just booleans. And a simple substitution of values isn't enough; an
application might want to output all integers in hex, for example. A
custom domain in the server is one way to implement that, or you can
check the datatype in the application and act accordingly. It doesn't
belong in psql, so I'll mark this as rejected in the commitfest app.
On 02.09.2012 19:47, Pavel Stehule wrote:
2012/9/2 Tom Lanet...@sss.pgh.pa.us:
I wrote:
Phil Sorberp...@omniti.com writes:
What my patch was intended to do was let the end user set boolean
output to any arbitrary values. While foo/bar is pretty useless, it
was meant to reinforce that it was capable of any arbitrary value. I
can think of a decent list of other output an end user might want,
such as:
true/false
yes/no
y/n
on/off
1/0
enabled/disabled
Plus the different capitalized forms.
I can readily see that people might want boolean columns displayed in
such ways in custom applications. I'm less convinced that there is much
use for it in psql, though.
BTW, another point that your list brings to mind is that somebody who
wants something like this would very possibly want different displays
for different columns. The proposed feature, being one-size-fits-all
for boolean, couldn't handle that.
I proposed just more cleaner and more conventional boolean output in
psql - nothing more. We can write formatting functions, CASE, we can
use enums.
What would make a lot more sense in my mind would be to label columns
*in the database* to show how they ought to be displayed.
One conceivable method is to make a collection of domains over bool,
and drive the display off the particular domain used. However we lack
some infrastructure that would be needed for this (in particular, I'm
pretty sure the PQftype data delivered to the client identifies the
underlying type and not the domain).
Another approach is to make a collection of enum types, in which case
you don't need any client-side support at all. I experimented with
this method a bit, and it seems workable:
regression=# create type mybool as enum ('no', 'yes');
CREATE TYPE
regression=# create function bool(mybool) returns bool as
$$ select $1 = 'yes'::mybool $$ language sql immutable;
CREATE FUNCTION
regression=# create cast (mybool as bool) with function bool(mybool) as
assignment;
CREATE CAST
regression=# create table mb(f1 mybool);
CREATE TABLE
regression=# insert into mb values('no'),('yes');
INSERT 0 2
regression=# select * from mb where f1;
f1
-
yes
(1 row)
regression=# select * from mb where f1 = 'yes';
f1
-
yes
(1 row)
I tried making the cast be implicit, but that caused problems with
ambiguous operators, so assignment seems to be the best you can do here.
A variant of this is to build casts in the other direction
(bool::mybool), declare columns in the database as regular bool,
and apply the casts in queries when you want columns displayed in a
particular way. This might be the best solution if the desired
display is at all context-dependent.
When I worked on PSM I required possibility to simple specification
expected datatype out of SQL statement - some like enhancing
parametrised queries - with fourth parameter - expected types.
Then somebody can set expected type for some column simply - out of
query - and transformation can be fast. It should be used for
unsupported date formats and similar tasks.
Regards
Pavel
regards, tom lane
--
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Make pg_basebackup configure and start standby [Review]
On Sun, 01 Jul 2012 13:02:17 +0200 Boszormenyi Zoltan wrote:
attached is a patch that does $SUBJECT.
It's a usability enhancement, to take a backup, write
a minimalistic recovery.conf and start the streaming
standby in one go.
Comments?
[Review of Patch]
Basic stuff:
--
- Patch applies OK
- Compiles cleanly with no warnings
What it does:
-
The pg_basebackup tool does the backup of Cluster from server to the
specified location.
This new functionality will also writes the recovery.conf in the database
directory and start the standby server based on options passed to
pg_basebackup.
Usability
For usability aspect, I am not very sure how many users would like to start
the standby server using basebackup.
According to me it can be useful for users who have automated scripts to
start server after backup can use this feature.
Feature Testing:
-
1. Test pg_basebackup with option -R to check that the recovery.conf file is
written to data directory.
--recovery.conf file is created in data directory.
2. Test pg_basebackup with option -R to check that the recovery.conf file is
not able to create because of disk full.
--Error is given as recovery.conf file is not able to create.
3. Test pg_basebackup with option -S to check the standby server start on
the same/different machine.
--Starting standby server is success in if pg_basebackup is taken in
different machine.
4. Test pg_basebackup with both options -S and -R to check the standby
server start on same/different machine.
--Starting standby server is success in if pg_basebackup is taken in
different machine.
5. Test pg_basebackup with option -S including -h, -U, -p, -w and -W to
check the standy server start
and verify the recovery.conf which is created in data directory.
--Except password, rest of the primary connection info parameters are
working fine.
6. Test pg_basebackup with conflict options (-x or -X and -R or -S).
--Error is given when the conflict options are provided to
pg_basebackup.
7. Test pg_basebackup with option -S where pg_ctl/postmaster binaries are
not present in the path.
--Error is given as not able to execute.
8. Test pg_basebackup with option -S by connecting to a standby server.
--standby server is started successfully when pg_basebackup is made from
a standby server also.
Code Review:
1. In function WriteRecoveryConf(), un-initialized filename is used.
due to which it can print junk for below line in code
printf(add password to primary_conninfo in %s if needed\n, filename);
2. In function WriteRecoveryConf(), in below code if fopen fails (due to
disk full or any other file related error) it will print the error and
exits.
So now it can be confusing to user, in respect to can he consider
backup as successfull and proceed. IMO, either error meesage or
documentation
can suggest the for such error user can proceed with backup to write
his own recovery.conf and start the standby.
+cf = fopen(filename, w);
+if (cf == NULL)
+{
+fprintf(stderr, _(cannot create %s), filename);
+exit(1);
+}
3. In function main,
instead of the following code it can be changed in two different ways,
if (startstandby)
writerecoveryconf = true;
change1:
case 'R':
writerecoveryconf = true;
break;
case 'S':
startstandby = true;
writerecoveryconf = true;
break;
change2:
case 'S':
startstandby = true;
case 'R':
writerecoveryconf = true;
break;
4. The password is not written to primary_conninfo even if the dbpassword is
present because of this reason
connecting to the primary is failing because of authentication failure.
5. write the function header for the newly added functions.
6. execvp function is deprecated beginning in Visual C++ 2005. which is used
to fork the pg_ctl process.
http://msdn.microsoft.com/en-us/library/ms235414.aspx
7. In StartStandby function, it is better to free the memory allocated for
path (path = xstrdup(command);)
Defects:
-
1. If the pg_basebackup is used in the same machine with the option of -S,
the standby server start
will fail as the port already in use because of using the same
postgresql.conf.
2. If the hot_standby=off in master conf file, the same is copied to
subscriber and starts the server. with that
no client connections are allowed to the server.
Documentation issues:
1. For -R option,
[HACKERS] Doubt Regarding changes to disable keepalives in walsender
In commit da4efa13d801ccc179f1d2c24d8a60c4a2f8ede9, sending of keepalive messages have been disabled in walsender. Commit message is Turn off WalSender keepalives by default, users can enable if desired. But I am not able to see how users can enable it, can you please point me if I am missing something? This doubt came as I am implementing replication timeout for walreceiver and for that walsender has to send some form of heartbeat message at predefined interval. I wanted to use keepalive to achieve the same, but then noticed the above and got stuck. With Regards, Amit Kapila.
Re: [HACKERS] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
On 16.08.2012 14:43, Pavel Stehule wrote: Hello here is updated patch [Review of Patch] Basic stuff: - Patch applies OK. but offset difference in line numbers. - Compiles with errors in contrib [pg_stat_statements, sepgsql] modules - Regression failed; one test-case in COPY due to incomplete test-case attached patch. same as reported by Heikki Details of compilation errors and regression failure: -- PATH : postgres/src/contrib/pg_stat_statements gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_stat_statements.o pg_stat_statements.c pg_stat_statements.c: In function â_PG_initâ: pg_stat_statements.c:363: warning: assignment from incompatible pointer type pg_stat_statements.c: In function âpgss_ProcessUtilityâ: pg_stat_statements.c:823: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:826: error: too few arguments to function âstandard_ProcessUtilityâ pg_stat_statements.c:884: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:887: error: too few arguments to function âstandard_ProcessUtilityâ make: *** [pg_stat_statements.o] Error 1 PATH : postgres/src/contrib/sepgsql gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o hooks.o hooks.c In file included from hooks.c:26: sepgsql.h:17:29: error: selinux/selinux.h: No such file or directory sepgsql.h:18:25: error: selinux/avc.h: No such file or directory In file included from hooks.c:26: sepgsql.h:239: warning: âstruct av_decisionâ declared inside parameter list sepgsql.h:239: warning: its scope is only this definition or declaration, which is probably not what you want hooks.c: In function âsepgsql_utility_commandâ: hooks.c:331: error: too few arguments to function ânext_ProcessUtility_hookâ hooks.c:334: error: too few arguments to function âstandard_ProcessUtilityâ hooks.c: In function â_PG_initâ: hooks.c:365: warning: implicit declaration of function âis_selinux_enabledâ hooks.c:426: warning: assignment from incompatible pointer type make: *** [hooks.o] Error 1 REGRESSION TEST: -- make installcheck test copy ... FAILED 1 of 132 tests failed. ~/postgres/src/test/regress cat regression.diffs *** /home/postgres/code/src/test/regress/expected/copy.out 2012-09-18 19:57:02.0 +0530 --- /home/postgres/code/src/test/regress/results/copy.out 2012-09-18 19:57:18.0 +0530 *** *** 71,73 --- 71,80 c1,col with , comma,col with quote 1,a,1 2,b,2 + -- copy should to return processed rows + do $$ + + ERROR: unterminated dollar-quoted string at or near $$ + + LINE 1: do $$ +^ What it does: -- Modification to get the number of processed rows evaluated via SPI. The changes are to add extra parameter in ProcessUtility to get the number of rows processed by COPY command. Code Review Comments: - 1. New parameter is added to ProcessUtility_hook_type function but the functions which get assigned to these functions like sepgsql_utility_command, pgss_ProcessUtility, prototype definition is not modified. 2. Why to add the new parameter if completionTag hold the number of processed tuple information; can be extracted from it as follows: _SPI_current-processed = strtoul(completionTag + 7, NULL, 10); 3. Why new variable added in portal [portal-processed] this is not used in code ? Testing: The following test is carried out on the patch. 1. Normal SQL command COPY FROM / TO is tested. 2. SQL command COPY FROM / TO is tested from plpgsql. Test cases are attached in Testcases_Copy_Processed.txt With Regards, Amit Kapila. --1) Normal SQL command COPY FROM and TO functionalities. CREATE TABLE tbl (a int); INSERT INTO tbl VALUES(generate_series(1,1000)); COPY tbl TO '/home/kiran/tbl.txt'; CREATE TABLE tbl1 (a int); COPY tbl1 FROM '/home/kiran/tbl.txt'; DELETE FROM tbl1; DROP TABLE tbl; DROP TABLE tbl1; --2) In side SPI [plpgsql function], SQL command COPY FROM and TO functionalities. CREATE TABLE tbl (a int); INSERT INTO tbl VALUES(generate_series(1,1000)); CREATE OR REPLACE FUNCTION public.copy_to_tbl() RETURNS integer LANGUAGE plpgsql AS $function$ DECLARE r int; BEGIN COPY tbl TO '/home/kiran/tbl.txt'; get diagnostics r = row_count; RETURN r; end; $function$; SELECT copy_to_tbl(); CREATE TABLE tbl1 (a int); CREATE OR REPLACE FUNCTION public.copy_from_tbl() RETURNS
Re: [HACKERS] newline conversion in SQL command strings
On 9/20/12 2:01 AM, Heikki Linnakangas wrote: Could you strip the CRs? Either at CREATE FUNCTION time, or when the function is executed. It has been proposed that the plsh handler should strip the CRs before execution. But I don't think that is a correct solution, because that is user data which could be relevant. It could be the case, for example, that plsh is run on a Windows host with a particular shell that requires CRLF line endings. -- 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] Invalid optimization of VOLATILE function in WHERE clause?
On Wed, Sep 19, 2012 at 2:39 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: It still seems like awfully weird behavior. Why? The WHERE condition relates only to the output of the _stats subquery, so why shouldn't it be evaluated there, rather than after the join? In another thread, Tom Lane t...@sss.pgh.pa.us wrote: It's easier to understand why this is if you realize that SQL has a very clear model of a pipeline of query execution. Conceptually, what happens is: 1. Form the cartesian product of the tables listed in FROM (ie, all combinations of rows). 2. Apply the WHERE condition to each row from 1, and drop rows that don't pass it. People expect that the results will be consistent with this model, even if the implementation is optimized under the covers. I think correct semantics should trump performance here. Hm, I bet it's possible (although probably not easy) to deduce volatility from the function body...maybe through the validator. If you could do that (perhaps warning in cases where you can't), then the performance regression-inducing-argument (which I agree with) becomes greatly ameliorated. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [v9.3] Extra Daemons (Re: [HACKERS] elegant and effective way for running jobs inside a database)
Excerpts from Amit Kapila's message of jue sep 20 02:10:23 -0300 2012: Why can't worker tasks be also permanent, which can be controlled through configuration. What I mean to say is that if user has need for parallel operations he can configure max_worker_tasks and those many worker tasks will get created. Otherwise without having such parameter, we might not be sure whether such deamons will be of use to database users who don't need any background ops. The dynamism will come in to scene when we need to allocate such daemons for particular ops(query), because might be operation need certain number of worker tasks, but no such task is available, at that time it need to be decided whether to spawn a new task or change the parallelism in operation such that it can be executed with available number of worker tasks. Well, there is a difficulty here which is that the number of processes connected to databases must be configured during postmaster start (because it determines the size of certain shared memory structs). So you cannot just spawn more tasks if all max_worker_tasks are busy. (This is a problem only for those workers that want to be connected as backends. Those that want libpq connections do not need this and are easier to handle.) The design we're currently discussing actually does not require a new GUC parameter at all. This is why: since the workers must be registered before postmaster start anyway (in the _PG_init function of a module that's listed in shared_preload_libraries) then we have to run a registering function during postmaster start. So postmaster can simply count how many it needs and size those structs from there. Workers that do not need a backend-like connection don't have a shmem sizing requirement so are not important for this. Configuration is thus simplified. BTW I am working on this patch and I think I have a workable design in place; I just couldn't get the code done before the start of this commitfest. (I am missing handling the EXEC_BACKEND case though, but I will not even look into that until the basic Unix case is working). One thing I am not going to look into is how is this new capability be used for parallel query. I feel we have enough use cases without it, that we can develop a fairly powerful feature. After that is done and proven (and committed) we can look into how we can use this to implement these short-lived workers for stuff such as parallel query. -- Álvaro Herrerahttp://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: [v9.3] Extra Daemons (Re: [HACKERS] elegant and effective way for running jobs inside a database)
2012/9/20 Amit Kapila amit.kap...@huawei.com: On Thursday, September 20, 2012 1:44 AM Simon Riggs wrote: On 12 September 2012 04:30, Amit Kapila amit.kap...@huawei.com wrote: On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote: Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012: We have some use cases for this patch, when can you post a new version? I would test and review it. What use cases do you have in mind? Wouldn't it be helpful for some features like parallel query in future? Trying to solve that is what delayed this patch, so the scope of this needs to be permanent daemons rather than dynamically spawned worker tasks. Why can't worker tasks be also permanent, which can be controlled through configuration. What I mean to say is that if user has need for parallel operations he can configure max_worker_tasks and those many worker tasks will get created. Otherwise without having such parameter, we might not be sure whether such deamons will be of use to database users who don't need any background ops. The dynamism will come in to scene when we need to allocate such daemons for particular ops(query), because might be operation need certain number of worker tasks, but no such task is available, at that time it need to be decided whether to spawn a new task or change the parallelism in operation such that it can be executed with available number of worker tasks. Although I understood and agree that such permanent daemons will be useful for usecases other than parallel operations. However my thinking is that having permanent daemons can also be useful for parallel ops. So even currently it is getting developed for certain usecases but the overall idea can be enhanced to have them for parallel ops as well. I'm also not sure why permanent daemons is more difficult than dynamically spawned daemons, because I guess all the jobs needed for permanent daemons are quite similar to what we're now doing to implement bgwriter or others in postmaster.c, except for it is implemented with loadable modules. 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
[HACKERS] XLogInsert scaling, revisited
I've been slowly continuing to work that I started last winder to make XLogInsert scale better. I have tried quite a few different approaches since then, and have settled on the attached. This is similar but not exactly the same as what I did in the patches I posted earlier. The basic idea, like before, is to split WAL insertion into two phases: 1. Reserve the right amount of WAL. This is done while holding just a spinlock. Thanks to the changes I made earlier to the WAL format, the space calculations are now much simpler and the critical section boils down to almost just CurBytePos += size_of_wal_record. See ReserveXLogInsertLocation() function. 2. Copy the WAL record to the right location in the WAL buffers. This slower part can be done mostly in parallel. The difficult part is tracking which insertions are currently in progress, and being able to wait for an insertion to finish copying the record data in place. I'm using a small number (7 at the moment) of WAL insertion slots for that. The first thing that XLogInsert does is to grab one of the slots. Each slot is protected by a LWLock, and XLogInsert reserves a slot by acquiring its lock. It holds the lock until it has completely finished copying the WAL record in place. In each slot, there's an XLogRecPtr that indicates how far the current inserter has progressed with its insertion. Typically, for a short record that fits on a single page, it is updated after the insertion is finished, but if the insertion needs to wait for a WAL buffer to become available, it updates the XLogRecPtr before sleeping. To wait for all insertions up to a point to finish, you scan all the insertion slots, and observe that the XLogRecPtrs in them are = the point you're interested in. The number of slots is a tradeoff: more slots allow more concurrency in inserting records, but makes it slower to determine how far it can be safely flushed. I did some performance tests with this, on an 8-core HP Proliant server, in a VM running under VMware vSphere 5.1. The tests were performed with Greg Smith's pgbench-tools kit, with one of two custom workload scripts: 1. Insert 1000 rows in each transaction. This is exactly the sort of workload where WALInsertLock currently becomes a bottleneck. Without the the patch, the test scales very badly, with about 420 TPS with a single client, peaking only at 520 TPS with two clients. With the patch, it scales up to about 1200 TPS, with 7 clients. I believe the test becomes I/O limited at that point; looking at iostat output while the test is running shows about 200MB/s of writes, and that is roughly what the I/O subsystem of this machine can do, according to a simple test with 'dd ...; sync. Or perhaps having more insertion slots would allow it to go higher - the patch uses exactly 7 slots at the moment. http://hlinnaka.iki.fi/xloginsert-scaling/results-1k/ 2. Insert only 10 rows in each transaction. This simulates an OLTP workload with fairly small transactions. The patch doesn't make a huge difference with that workload. It performs somewhat worse with 4-16 clients, but then somewhat better with 16 clients. The patch adds some overhead to flushing the WAL, I believe that's what's causing the slowdown with 4-16 clients. But with more clients, the WALInsertLock bottleneck becomes more significant, and you start to see a benefit again. http://hlinnaka.iki.fi/xloginsert-scaling/results-10/ Overall, the results look pretty good. I'm going to take a closer look at the slowdown in the second test. I think it might be fixable with some changes to how WaitInsertionsToFinish() and WALWriteLock work together, although I'm not sure how exactly it ought to work. Comments, ideas? - Heikki xloginsert-scale-20.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Invalid optimization of VOLATILE function in WHERE clause?
On Thu, Sep 20, 2012 at 9:24 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Merlin Moncure mmonc...@gmail.com wrote: Hm, I bet it's possible (although probably not easy) to deduce volatility from the function body...maybe through the validator. If you could do that (perhaps warning in cases where you can't), then the performance regression-inducing-argument (which I agree with) becomes greatly ameliorated. For about the last 10 years the Wisconsin Courts have been parsing SQL code to generate Java query classes, including stored procedures, and determining information like this. For example, we set a readOnly property for the query class by examining the statements in the procedure and the readOnly status of each called procedure. It wasn't that hard for us, and I'm not sure what would make much it harder in PostgreSQL, if we can do it where a parse tree for the function is handy. hm, what do you do about 'after the fact' changes to things the procedure body is pointing to? what would the server have to do? merlin -- 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] XLogInsert scaling, revisited
On Thursday, September 20, 2012 05:37:42 PM Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: I've been slowly continuing to work that I started last winder to make XLogInsert scale better. I have tried quite a few different approaches since then, and have settled on the attached. This is similar but not exactly the same as what I did in the patches I posted earlier. Sounds pretty cool from a quick read. This sounds pretty good. I'm a bit bothered by the fact that you've settled on 7 parallel-insertion slots after testing on an 8-core machine. I suspect that it's not a coincidence that you're seeing a sweet spot for #slots ~= #CPUs. If that is what's happening, we're going to want to be able to configure the #slots at postmaster start. Not sure how we'd go about it exactly - is there any reasonably portable way to find out how many CPUs the machine has? Or do we have to use a GUC for that? Several platforms support sysconf(_SC_NPROCESSORS_CONF) although after a quick look it doesn't seem to be standardized. A guc initialized to that or falling back to 4 or so? Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XLogInsert scaling, revisited
On 20.09.2012 18:37, Tom Lane wrote: I suspect that it's not a coincidence that you're seeing a sweet spot for #slots ~= #CPUs. Yeah, quite possible. I did not test with any different number of slots, so I don't know if that's the sweet spot, but I wouldn't be surprised if it is. If that is what's happening, we're going to want to be able to configure the #slots at postmaster start. Not sure how we'd go about it exactly - is there any reasonably portable way to find out how many CPUs the machine has? Or do we have to use a GUC for that? Detecting the number of CPUs and using that might not be optimal. Even with a machine with a lot of CPUs, a workload might not be limited by WAL insertion speed. Perhaps we could have a counter of how often you have to wait for a slot, and adjust the number of slots on the fly based on that. Similar to the way the spinlock delay is adjusted. At the moment, I'm grabbing the lock on a slot before determining which blocks need to be backed up because of full_page_writes, and before calculating the CRCs. I can try to move that so that the lock is grabbed later, more like the WALInsertLock currently works. That would make the duration the slot locks are held much shorter, which probably would make the number of slots less important. - 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] XLogInsert scaling, revisited
Heikki Linnakangas hlinnakan...@vmware.com writes: I've been slowly continuing to work that I started last winder to make XLogInsert scale better. I have tried quite a few different approaches since then, and have settled on the attached. This is similar but not exactly the same as what I did in the patches I posted earlier. This sounds pretty good. I'm a bit bothered by the fact that you've settled on 7 parallel-insertion slots after testing on an 8-core machine. I suspect that it's not a coincidence that you're seeing a sweet spot for #slots ~= #CPUs. If that is what's happening, we're going to want to be able to configure the #slots at postmaster start. Not sure how we'd go about it exactly - is there any reasonably portable way to find out how many CPUs the machine has? Or do we have to use a GUC for that? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Suggestion for --truncate-tables to pg_restore
Hi, I've had problems using pg_restore --data-only when restoring individual schemas (which contain data which has had bad things done to it). --clean does not work well because of dependent objects in other schemas. Patch to the docs attached (before I go and do any real coding.) Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index b276da6..94d359d 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -539,6 +539,26 @@ /varlistentry varlistentry + termoption-u//term + termoption--truncate-tables//term + listitem + para +This option is only relevant when performing a data-only +restore. It instructs applicationpg_restore/application +to execute commands to truncate the target tables while the +data is reloaded. Use this when restoring tables or schemas +and option--clean/clean cannot be used because dependent +objects would be destroyed. + /para + + para + The option--disable-triggers/option will almost always + always need to be used in conjunction with this option to + disable check constraints on foreign keys. + /para + /varlistentry + + varlistentry termoption--use-set-session-authorization/option/term listitem para -- 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] XLogInsert scaling, revisited
On 20 September 2012 16:29, Heikki Linnakangas hlinnakan...@vmware.com wrote: 1. Insert 1000 rows in each transaction. This is exactly the sort of workload where WALInsertLock currently becomes a bottleneck. Without the the patch, the test scales very badly, with about 420 TPS with a single client, peaking only at 520 TPS with two clients. With the patch, it scales up to about 1200 TPS, with 7 clients. I believe the test becomes I/O limited at that point; looking at iostat output while the test is running shows about 200MB/s of writes, and that is roughly what the I/O subsystem of this machine can do, according to a simple test with 'dd ...; sync. Or perhaps having more insertion slots would allow it to go higher - the patch uses exactly 7 slots at the moment. http://hlinnaka.iki.fi/xloginsert-scaling/results-1k/ 2. Insert only 10 rows in each transaction. This simulates an OLTP workload with fairly small transactions. The patch doesn't make a huge difference with that workload. It performs somewhat worse with 4-16 clients, but then somewhat better with 16 clients. The patch adds some overhead to flushing the WAL, I believe that's what's causing the slowdown with 4-16 clients. But with more clients, the WALInsertLock bottleneck becomes more significant, and you start to see a benefit again. http://hlinnaka.iki.fi/xloginsert-scaling/results-10/ Overall, the results look pretty good. Yes, excellent work. The results seem sensitive to the use case, so my thoughts immediately switch to auto-tuning or at least appropriate usage. I'm a bit worried that its a narrow use case, since the problem quickly moves from lock contention to I/O limiting. It sounds like the use case where this is a win would be parallel data loading into a high I/O bandwidth server. Could we do some more tests/discuss to see how wide the use case is? I'm also wondering about this from a different perspective. I was looking to rate-limit WAL inserts from certain operations - would rate-limiting remove the contention problem, or is that just a different feature. -- Simon Riggs 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] Move postgresql_fdw_validator into dblink
Hanada-san, I checked your patch. It can be applied to the latest master branch without any conflicts, and regression tests were fine. Unlike the original postgresql_fdw_validator(), the new dblink_fdw_validator() has wise idea; that pulls list of connection options from libpq, instead of self-defined static table. I basically agree not to have multiple source of option list. + /* +* Get list of valid libpq options. It contains default values too, but we +* don't care the values. Obtained list is allocated with malloc, so we +* must free it before leaving this function. +*/ + options = PQconndefaults(); + if (options == NULL) + ereport(ERROR, + (errcode(ERRCODE_FDW_OUT_OF_MEMORY), +errmsg(out of memory), +errdetail(could not get libpq default connection options))); I doubt whether PQconndefaults needs to be invoked for each validator calls. At least, supported option list of libpq should be never changed during run-time. So, I think PQconndefaults() should be called only once at first invocation, then option list can be stored in static variables or somewhere. As source code comments says, it is allocated with malloc, thus, we don't worry about unintentional release. :-) I don't think other part of this patch is arguable. Thanks, 2012/9/13 Shigeru HANADA shigeru.han...@gmail.com: Kaigai-san, (2012/09/13 16:56), Kohei KaiGai wrote: What about your plan to upstream contrib/pgsql_fdw module on the upcoming commit-fest? I will post pgsql_fdw patch (though it will be renamed to postgresql_fdw) in opening CF (2012 Sep), as soon as I resolve a bug in ANALYZE support, maybe on tomorrow. :-) Even though I understand the point I noticed (miss-synchronization of sub- transaction block between local and remote side) is never easy problem to solve, it is worth to get the patch on the table of discussion. In my opinion, it is an idea to split-off the transaction control portion as a limitation of current version. For example, just raise an error when the foreign-table being referenced in sub-transaction block; with explicit description in the document. I agree not to support synchronize TX block between remote and local, at least in next CF (I mean keeping remote TX open until local COMMIT or ABORT). It would require 2PC and many issues to be solved, so I'd like to focus fundamental part first. OTOH, using foreign tables in sub-transaction seems essential to me. Anyway, let me pick up your patch for reviewing. And, I hope you to prepare contrib/pgsql_fdw patch based on this patch. Thanks for your volunteer :-) Regards, -- Shigeru HANADA -- 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] newline conversion in SQL command strings
On 09/20/2012 09:12 AM, Peter Eisentraut wrote: On 9/20/12 2:01 AM, Heikki Linnakangas wrote: Could you strip the CRs? Either at CREATE FUNCTION time, or when the function is executed. It has been proposed that the plsh handler should strip the CRs before execution. But I don't think that is a correct solution, because that is user data which could be relevant. It could be the case, for example, that plsh is run on a Windows host with a particular shell that requires CRLF line endings. I confess I find it hard to take plsh terribly seriously, it just seems like a bad solution for practically any problem, when compared with, say, plperlu, and it seems so Unixish that the very idea of using it at all on Windows strikes me as crazy. Having said that, and devoted all of 5 minutes to looking at the code, I suggest that you make two changes: if (sourcecode[0] == '\n') sourcecode++; would become: if (sourcecode[0] == '\n' || (sourcecode[0] == '\r' sourcecode[1] == '\n')) sourcecode++; and len = strcspn(rest, \n); would become: len = strcspn(rest, \r\n); Also, there is probably a pretty good case for opening the temp file using PG_BINARY_W rather than w so that Windows doesn't produce extra CRs for you (see recent discussion of this w.r.t. pg_upgrade). cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] newline conversion in SQL command strings
Andrew Dunstan and...@dunslane.net writes: On 09/20/2012 09:12 AM, Peter Eisentraut wrote: It has been proposed that the plsh handler should strip the CRs before execution. But I don't think that is a correct solution, because that is user data which could be relevant. It could be the case, for example, that plsh is run on a Windows host with a particular shell that requires CRLF line endings. I confess I find it hard to take plsh terribly seriously, Perhaps, but the general problem remains, for any language whose interpreter is unforgiving about line endings. However, I think the problem is insoluble as Peter has stated it. His original complaint was ... It would be unfortunate to have the behavior of a function depend on the kind of client used to create or modify it. but I don't see a way to reconcile that with the notion that CRs might be user data. Either you think they're significant, in which case you should retain the function body as presented, or you think they aren't, in which case you might as well strip them. I lean to the position that you're best off stripping them, given that the lines are to be fed to a Unix shell. The argument about some other shell possibly requiring them seems pretty tenuous. (IOW, I think that the PL-specific code is exactly the place to be making such decisions. I don't want Postgres' generic function support making any decisions about whether CRs in function bodies are significant, because that decision could be language-specific.) 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] ToDo: allow to get a number of processed rows by COPY statement [Review of Patch]
Hello Basic stuff: - Patch applies OK. but offset difference in line numbers. - Compiles with errors in contrib [pg_stat_statements, sepgsql] modules - Regression failed; one test-case in COPY due to incomplete test-case attached patch. – same as reported by Heikki fixed patch is in attachment Details of compilation errors and regression failure: -- PATH : postgres/src/contrib/pg_stat_statements gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o pg_stat_statements.o pg_stat_statements.c pg_stat_statements.c: In function â_PG_initâ: pg_stat_statements.c:363: warning: assignment from incompatible pointer type pg_stat_statements.c: In function âpgss_ProcessUtilityâ: pg_stat_statements.c:823: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:826: error: too few arguments to function âstandard_ProcessUtilityâ pg_stat_statements.c:884: error: too few arguments to function âprev_ProcessUtilityâ pg_stat_statements.c:887: error: too few arguments to function âstandard_ProcessUtilityâ make: *** [pg_stat_statements.o] Error 1 PATH : postgres/src/contrib/sepgsql gcc -g -ggdb3 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -c -o hooks.o hooks.c In file included from hooks.c:26: sepgsql.h:17:29: error: selinux/selinux.h: No such file or directory sepgsql.h:18:25: error: selinux/avc.h: No such file or directory In file included from hooks.c:26: sepgsql.h:239: warning: âstruct av_decisionâ declared inside parameter list sepgsql.h:239: warning: its scope is only this definition or declaration, which is probably not what you want hooks.c: In function âsepgsql_utility_commandâ: hooks.c:331: error: too few arguments to function ânext_ProcessUtility_hookâ hooks.c:334: error: too few arguments to function âstandard_ProcessUtilityâ hooks.c: In function â_PG_initâ: hooks.c:365: warning: implicit declaration of function âis_selinux_enabledâ hooks.c:426: warning: assignment from incompatible pointer type make: *** [hooks.o] Error 1 REGRESSION TEST: -- make installcheck test copy ... FAILED 1 of 132 tests failed. ~/postgres/src/test/regress cat regression.diffs *** /home/postgres/code/src/test/regress/expected/copy.out 2012-09-18 19:57:02.0 +0530 --- /home/postgres/code/src/test/regress/results/copy.out 2012-09-18 19:57:18.0 +0530 *** *** 71,73 --- 71,80 c1,col with , comma,col with quote 1,a,1 2,b,2 + -- copy should to return processed rows + do $$ + + ERROR: unterminated dollar-quoted string at or near $$ + + LINE 1: do $$ +^ fixed What it does: -- Modification to get the number of processed rows evaluated via SPI. The changes are to add extra parameter in ProcessUtility to get the number of rows processed by COPY command. Code Review Comments: - 1. New parameter is added to ProcessUtility_hook_type function but the functions which get assigned to these functions like sepgsql_utility_command, pgss_ProcessUtility, prototype definition is not modified. 2. Why to add the new parameter if completionTag hold the number of processed tuple information; can be extracted from it as follows: _SPI_current-processed = strtoul(completionTag + 7, NULL, 10); this is basic question. I prefer a natural type for counter - uint64 instead text. And there are no simply way to get offset (7 in this case) 3. Why new variable added in portal [portal-processed] this is not used in code ? This value can be used anywhere instead parsing completionTag to get returned numbers Testing: The following test is carried out on the patch. 1. Normal SQL command COPY FROM / TO is tested. 2. SQL command COPY FROM / TO is tested from plpgsql. Test cases are attached in Testcases_Copy_Processed.txt Regards Pavel With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers copy-processed-rows.diff 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] newline conversion in SQL command strings
On 09/20/2012 03:34 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 09/20/2012 09:12 AM, Peter Eisentraut wrote: It has been proposed that the plsh handler should strip the CRs before execution. But I don't think that is a correct solution, because that is user data which could be relevant. It could be the case, for example, that plsh is run on a Windows host with a particular shell that requires CRLF line endings. I confess I find it hard to take plsh terribly seriously, Perhaps, but the general problem remains, for any language whose interpreter is unforgiving about line endings. plsh is the only one I know of that writes the script out to a temp file, and it opens that file on Windows in text mode (if I'm looking at the right source), which will convert LF into CRLF. Hence my suggestion that he change that. Of course, if the function body actually has embedded CRs then there is a different set of problems. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal and patch : support INSERT INTO...RETURNING with partitioned table using rule
John Lumby johnlu...@hotmail.com writes:
On Fri, 22 Jun 2012 09:55:13, Robert Haas wrote:
I do notice that the RETURNING clause of the INSERT can't reference
NEW, which seems like a restriction that we probably ought to lift,
but it doesn't seem to have much to do with your patch.
The main use of my proposal is to be able to return the value of the
sequence assigned to the NEW.id column, so yes that is a serious
restriction.
I think both of you are confused. What the RETURNING clause can see is
the inserted row's actual values. You can certainly get the assigned
sequence ID out of that. I would argue that being able to see the NEW.*
expressions is at best secondary, because that data doesn't necessarily
have anything to do with what went into the table (consider the
possibility that a BEFORE trigger changed it).
However, even if that restriction is lifted, it will not help with the
case where the rule is an invocation of a function, which is the case
I need.
What you're requesting seems pretty much nonsensical to me. The point
of being able to write a RETURNING clause in a rule is to emulate what
would happen with RETURNING on a regular table. As an example, suppose
that I have
create table t (id serial, data1 text, data2 text);
and for whatever reason I write
insert into t(data1, data2) values('foo', 'bar') returning id, data2;
I should get back the generated sequence value and the data2 value, but
*not* the data1 value. Anything else is just wrong. Now, if t has a
rule ON INSERT DO INSTEAD SELECT somefunction(), how is that going to
happen? The function doesn't know what the RETURNING clause looks like.
If we had a notional inserted-row-value then the executor could do the
RETURNING computation based on that, but there's no way to make a
connection between whatever the function does internally and the data
for RETURNING to chew on.
The whole concept of ON INSERT DO [INSTEAD/ALSO] SELECT seems pretty
shaky to me, as it *necessarily* involves a command substitution that
causes an INSERT to act in a strange fashion that the client application
will need special code to cope with. I won't argue to take the feature
out, because people do use it in custom applications --- but it doesn't
play nice with RETURNING, and I don't think it can be made to. It's
pretty much a legacy method of doing business IMO.
It seems to me that instead of lobbying to throw another kluge on top
of that pile, you'd be better off looking for alternative solutions.
Have you tried implementing this as an INSTEAD OF trigger, and not using
rules at all? That mechanism works just fine with RETURNING, and it
seems to me that it would let you do whatever you could do inside a
custom function. It would certainly be enough for the
dynamic-partition-redirection problem.
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] PATCH: pgbench - aggregation of info written into log
On 3.9.2012 01:28, Jeff Janes wrote:
On Sun, Sep 2, 2012 at 3:46 PM, Tomas Vondra t...@fuzzy.cz wrote:
Fixed. I've kept use_log_agg only and I've removed the default. Also
I've added one more check (that the total duration is a multiple of
the aggregation interval).
Hi Tomas,
In the docs, -l is an option, not an application:
application-l/application
Also, the docs still refer to option-A/option, rather than to
option--aggregate-interval/option, in a few places.
Fixed.
I think a section in the docs that points out that transactions run by
specifying mulitple -f will be mingled together into an interval would
be a good idea, as that could easily be overlooked (as I did earlier).
Fixed, added a paragraph mentioning this.
The docs do not mention anywhere what the units for the latencies are.
I think the format of the logfile should somehow make it unmistakably
different from the regular -l logfile, for example by prefixing every
line with Aggregate: . Or maybe there is some other solution, but
I think that having two log formats, both of which consist of nothing
but 6 integers, but yet mean very different things, is a recipe for
confusion.
Not sure how to do that, I'd say it's a responsibility of the user.
Is it unavoidable that the interval be an integral number of seconds?
I found the units in the original code confusing, so maybe there is
some reason it needs to be like that that I don't understand yet.
I'll look into it some more.
Not really, but I don't see a real use case for such intervals, so I'm
keeping the integers for now.
Also, I've realized the last interval may not be logged at all - I'll
take a look into this in the next version of the patch.
Tomas
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index f5ac3b1..19c3b18 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -145,6 +145,7 @@ char *index_tablespace = NULL;
#define naccounts 10
bool use_log;/* log transaction latencies to
a file */
+intuse_log_agg;/* log aggregates instead of
individual transactions */
bool is_connect; /* establish connection for
each transaction */
bool is_latencies; /* report per-command latencies */
intmain_pid; /* main process id used
in log filename */
@@ -240,6 +241,18 @@ typedef struct
char *argv[MAX_ARGS]; /* command word list */
} Command;
+typedef struct
+{
+
+ longstart_time; /* when does the interval start
*/
+ int cnt;/* number of transactions */
+ double min_duration; /* min/max durations */
+ double max_duration;
+ double sum;/* sum(duration),
sum(duration^2) - for estimates */
+ double sum2;
+
+} AggVals;
+
static Command **sql_files[MAX_FILES]; /* SQL script files */
static int num_files; /* number of script files */
static int num_commands = 0; /* total number of Command structs */
@@ -364,6 +377,8 @@ usage(void)
-f FILENAME read transaction script from FILENAME\n
-j NUM number of threads (default: 1)\n
-l write transaction times to log file\n
+--aggregate-interval NUM\n
+ aggregate data over NUM seconds\n
-M simple|extended|prepared\n
protocol for submitting queries to server
(default: simple)\n
-n do not run VACUUM before tests\n
@@ -817,9 +832,22 @@ clientDone(CState *st, bool ok)
return false; /* always false */
}
+static
+void agg_vals_init(AggVals * aggs, instr_time start)
+{
+ aggs-cnt = 0;
+ aggs-sum = 0;
+ aggs-sum2 = 0;
+
+ aggs-min_duration = 3600 * 100.0; /* one hour */
+ aggs-max_duration = 0;
+
+ aggs-start_time = INSTR_TIME_GET_DOUBLE(start);
+}
+
/* return false iff client should be disconnected */
static bool
-doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile)
+doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile,
AggVals * agg)
{
PGresult *res;
Command **commands;
@@ -881,17 +909,70 @@ top:
diff = now;
INSTR_TIME_SUBTRACT(diff, st-txn_begin);
usec = (double) INSTR_TIME_GET_MICROSEC(diff);
-
+
+ /* should we aggregate the results or not? */
+ if (use_log_agg)
+ {
+
+ /* are we still in the same interval? if yes,
accumulate the
+
[HACKERS] Assigning NULL to a record variable
Related to bug #6123, Wisconsin Courts are now using triggers with the workaround to be safe with the patch put forward by Tom, even though they are still running with the earlier patch proposal by me (which tolerates an UPDATE or DELETE of a row being deleted). The general structure of the BEFORE DELETE triggers with cascading logic was like this: DECLARE return_value parenttable; BEGIN return_value := OLD; DELETE FROM childtable1 WHERE child of parent; IF FOUND THEN return_value := NULL; END IF; DELETE FROM childtablen WHERE child of parent; IF FOUND THEN return_value := NULL; END IF; IF return_value IS NULL THEN DELETE FROM parent WHERE primary key matches OLD; END IF; RETURN return_value; END; This did not work for cases where the AFTER DELETE trigger performed an action which was not idempotent because, while return_value was NULL enough to enter that last IF clause, it was not NULL enough to prevent the DELETE attempt and fire subsequent triggers. The assignment of NULL to a variable with a record type doesn't assign a simple NULL, but a record with NULL in each element. It seems like a POLA violation that: return_value := NULL; RETURN return_value; behaves differently than: RETURN NULL; We've fixed the afflicted trigger functions by adding a RETURN NULL; inside that last IF block, but: - Is this behavior required by standard? - If not, do we want to keep this behavior? - If we keep this behavior, should the triggering operation be suppressed when (NOT return_value IS NOT NULL) instead of when (return_value IS NOT DISTINCT FROM NULL)? -Kevin -- 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] Configuration include directory
Hello! I've spent a little time with this patch and have attached revision 6. Thanks, Noah, for a fantastically detailed review. The only thing I didn't do that Noah suggested was run pgindent on guc-file.l. A cursory search did not reveal source compatible with my operating system for 'indent'. If someone points me to it, I'd happily also comply with the request to reindent. And document how to do that on my platform(s). :) I did just remove the references to the Apache project etc. I agree that providing best practices is good, but I'm skeptical about including best practices piecemeal. Adding it to earlier tutorial sections would probably be a bit more visible IMO. I also added examples to postgresql.conf.sample, per a suggestion from Dave Page. -selena -- http://chesnok.com config-directory-v6.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] Assigning NULL to a record variable
Kevin Grittner kgri...@mail.com writes: ... This did not work for cases where the AFTER DELETE trigger performed an action which was not idempotent because, while return_value was NULL enough to enter that last IF clause, it was not NULL enough to prevent the DELETE attempt and fire subsequent triggers. The assignment of NULL to a variable with a record type doesn't assign a simple NULL, but a record with NULL in each element. I believe that this is forced by plpgsql's implementation. IIRC, a declared variable of a named composite type (not RECORD) is implemented as a row structure, meaning it actually consists of a separate plpgsql variable for each column. So there's no physical way for it to represent a simple NULL composite value. I've suggested in the past that we might want to go over to treating such variables more like RECORD, ie the representation is always a HeapTuple. I'm not sure what the performance tradeoffs would be --- some things would get faster and others slower, probably, since field access would be more work but conversion to/from HeapTuple would get far cheaper. - If we keep this behavior, should the triggering operation be suppressed when (NOT return_value IS NOT NULL) instead of when (return_value IS NOT DISTINCT FROM NULL)? Can't do that, because it would break the perfectly-legitimate case where the trigger is trying to process a row of all nulls. 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] Assigning NULL to a record variable
2012/9/20 Tom Lane t...@sss.pgh.pa.us: Kevin Grittner kgri...@mail.com writes: ... This did not work for cases where the AFTER DELETE trigger performed an action which was not idempotent because, while return_value was NULL enough to enter that last IF clause, it was not NULL enough to prevent the DELETE attempt and fire subsequent triggers. The assignment of NULL to a variable with a record type doesn't assign a simple NULL, but a record with NULL in each element. I believe that this is forced by plpgsql's implementation. IIRC, a declared variable of a named composite type (not RECORD) is implemented as a row structure, meaning it actually consists of a separate plpgsql variable for each column. So there's no physical way for it to represent a simple NULL composite value. I've suggested in the past that we might want to go over to treating such variables more like RECORD, ie the representation is always a HeapTuple. I had same idea when I worked on SQL/PSM - but there is significant difference in performance (probably less in real tasks) I'm not sure what the performance tradeoffs would be --- some things would get faster and others slower, probably, since field access would be more work but conversion to/from HeapTuple would get far cheaper. when fields are fix length, then field's update is significantly faster then when RECORD is used - If we keep this behavior, should the triggering operation be suppressed when (NOT return_value IS NOT NULL) instead of when (return_value IS NOT DISTINCT FROM NULL)? Can't do that, because it would break the perfectly-legitimate case where the trigger is trying to process a row of all nulls. 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Properly set relpersistence for fake relcache entries.
On Monday, September 17, 2012 03:58:37 PM Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Btw, I played with this some more on Saturday and I think, while definitely a bad bug, the actual consequences aren't as bad as at least I initially feared. Fake relcache entries are currently set in 3 scenarios during recovery: 1. removal of ALL_VISIBLE in heapam.c 2. incomplete splits and incomplete deletions in nbtxlog.c 3. incomplete splits in ginxlog.c [ #1 doesn't really hurt in 9.1, and the others are low probability ] OK, that explains why we've not seen a blizzard of trouble reports. Still seems like a good idea to fix it ASAP, though. Btw, I think RhodiumToad/Andrew Gierth and I some time ago helped a user in the IRC Channel that had symptoms matching this bug. Situation was that he started to get very high IO and xid wraparound shutdown warnings due to never finishing and not canceleable autovacuums. After some investigation it turned out that btree indexes were processed at that time. We found they had cyclic btpo_next pointers leading to an endless loop in _bt_pagedel. We solved the issue by forcing leftsib = P_NONE inside the while (P_ISDELETED(opaque) || opaque-btpo_next != target) which let a queue DROP INDEX get the necessary locks. Unfortuantely this was on a busy production system with a nearing shutdown, so not much was kept for further diagnosis. After this bug was discovered I asked the user and indeed they previously shutdown the database twice in quick succession during heavy activity with -m immediate which could exactly lead to such a problem due to incompletely processed page splits. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assigning NULL to a record variable
Pavel Stehule pavel.steh...@gmail.com writes: 2012/9/20 Tom Lane t...@sss.pgh.pa.us: I'm not sure what the performance tradeoffs would be --- some things would get faster and others slower, probably, since field access would be more work but conversion to/from HeapTuple would get far cheaper. when fields are fix length, then field's update is significantly faster then when RECORD is used [ shrug... ] Maybe not if we put a little effort into it. It would be interesting to consider using a TupleTableSlot instead of a bare HeapTuple for instance. In any case you're ignoring the point that other things will get faster. 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] alter enum add value if not exists
Andrew Dunstan and...@dunslane.net writes: On 08/23/2012 07:39 AM, Magnus Hagander wrote: It doesn't break, of course ,since it's protected by the unique index. But aren't you at risk of getting the very error message you're trying to avoid? Yeah, looking further this was probably a thinko on my part. Thanks for noticing. I've moved the test down so it's done right after the lock is acquired. Revised patch attached. This patch looks sane as far as it goes. It strikes me though that if we're going to invent an opt_if_not_exists production in the grammar, there are a lot of other places where it should be used too, for consistency if nothing else. However, it would be reasonable to do that mop-up as a separate commit. If you prefer, commit what you've got and then I'll see about the other thing. 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] 64-bit API for large object
3) Backend inv_api.c functions(Nozomi Anzai)
No need to add new functions. Just extend them to handle 64-bit data.
BTW , what will happen if older 32-bit libpq accesses large objects
over 2GB?
lo_read and lo_write: they can read or write lobjs using 32-bit API as
long as requested read/write data length is smaller than 2GB. So I
think we can safely allow them to access over 2GB lobjs.
lo_lseek: again as long as requested offset is smaller than 2GB, there
would be no problem.
lo_tell:if current seek position is beyond 2GB, returns an error.
Even though iteration of lo_lseek() may move the offset to 4TB, it also
makes unavailable to use lo_tell() to obtain the current offset, so I think
it is reasonable behavior.
However, error code is not an appropriate one.
+ if (INT_MAX offset)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+errmsg(invalid large-object
descriptor: %d, fd)));
+ PG_RETURN_INT32(-1);
+ }
According to the manpage of lseek(2)
EOVERFLOW
The resulting file offset cannot be represented in an off_t.
Please add a new error code such as ERRCODE_BLOB_OFFSET_OVERFLOW.
Agreed.
--
Nozomi Anzai
SRA OSS, Inc. Japan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_reorg in core?
Hi all, During the last PGCon, I heard that some community members would be interested in having pg_reorg directly in core. Just to recall, pg_reorg is a functionality developped by NTT that allows to redistribute a table without taking locks on it. The technique it uses to reorganize the table is to create a temporary copy of the table to be redistributed with a CREATE TABLE AS whose definition changes if table is redistributed with a VACUUM FULL or CLUSTER. Then it follows this mechanism: - triggers are created to redirect all the DMLs that occur on the table to an intermediate log table. - creation of indexes on the temporary table based on what the user wishes - Apply the logs registered during the index creation - Swap the names of freshly created table and old table - Drop the useless objects The code is hosted by pg_foundry here: http://pgfoundry.org/projects/reorg/. I am also maintaining a fork in github in sync with pgfoundry here: https://github.com/michaelpq/pg_reorg. Just, do you guys think it is worth adding a functionality like pg_reorg in core or not? If yes, well I think the code of pg_reorg is going to need some modifications to make it more compatible with contrib modules using only EXTENSION. For the time being pg_reorg is divided into 2 parts, binary and library. The library part is the SQL portion of pg_reorg, containing a set of C functions that are called by the binary part. This has been extended to support CREATE EXTENSION recently. The binary part creates a command pg_reorg in charge of calling the set of functions created by the lib part, being just a wrapper of the library part to control the creation and deletion of the objects. It is also in charge of deleting the temporary objects by callback if an error occurs. By using the binary command, it is possible to reorganize a single table or a database, in this case reorganizing a database launches only a loop on each table of this database. My idea is to remove the binary part and to rely only on the library part to make pg_reorg a single extension with only system functions like other contrib modules. In order to do that what is missing is a function that could be used as an entry point for table reorganization, a function of the type pg_reorg_table(tableoid) and pg_reorg_table(tableoid, text). All the functionalities of pg_reorg could be reproducible: - pg_reorg_table(tableoid) for a VACUUM FULL reorganization - pg_reorg_table(tableoid, NULL) for a CLUSTER reorganization if table has a CLUSTER key - pg_reorg_table(tableoid, columnname) for a CLUSTER reorganization based on a wanted column. Is it worth the shot? Regards, -- Michael Paquier http://michael.otacoo.com
Re: [HACKERS] pg_reorg in core?
On Thu, Sep 20, 2012 at 7:05 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hi all, During the last PGCon, I heard that some community members would be interested in having pg_reorg directly in core. I'm actually not crazy about this idea, at least not given the current state of pg_reorg. Right now, there are a quite a few fixes and features which remain to be merged in to cvs head, but at least we can develop pg_reorg on a schedule independent of Postgres itself, i.e. we can release new features more often than once a year. Perhaps when pg_reorg is more stable, and the known bugs and missing features have been ironed out, we could think about integrating into core. Granted, a nice thing about integrating with core is we'd probably have more of an early warning when reshuffling of PG breaks pg_reorg (e.g. the recent splitting of the htup headers), but such changes have been quick and easy to fix so far. Just to recall, pg_reorg is a functionality developped by NTT that allows to redistribute a table without taking locks on it. The technique it uses to reorganize the table is to create a temporary copy of the table to be redistributed with a CREATE TABLE AS whose definition changes if table is redistributed with a VACUUM FULL or CLUSTER. Then it follows this mechanism: - triggers are created to redirect all the DMLs that occur on the table to an intermediate log table. N.B. CREATE TRIGGER takes an AccessExclusiveLock on the table, see below. - creation of indexes on the temporary table based on what the user wishes - Apply the logs registered during the index creation - Swap the names of freshly created table and old table - Drop the useless objects The code is hosted by pg_foundry here: http://pgfoundry.org/projects/reorg/. I am also maintaining a fork in github in sync with pgfoundry here: https://github.com/michaelpq/pg_reorg. Just, do you guys think it is worth adding a functionality like pg_reorg in core or not? If yes, well I think the code of pg_reorg is going to need some modifications to make it more compatible with contrib modules using only EXTENSION. For the time being pg_reorg is divided into 2 parts, binary and library. The library part is the SQL portion of pg_reorg, containing a set of C functions that are called by the binary part. This has been extended to support CREATE EXTENSION recently. The binary part creates a command pg_reorg in charge of calling the set of functions created by the lib part, being just a wrapper of the library part to control the creation and deletion of the objects. It is also in charge of deleting the temporary objects by callback if an error occurs. By using the binary command, it is possible to reorganize a single table or a database, in this case reorganizing a database launches only a loop on each table of this database. My idea is to remove the binary part and to rely only on the library part to make pg_reorg a single extension with only system functions like other contrib modules. In order to do that what is missing is a function that could be used as an entry point for table reorganization, a function of the type pg_reorg_table(tableoid) and pg_reorg_table(tableoid, text). All the functionalities of pg_reorg could be reproducible: - pg_reorg_table(tableoid) for a VACUUM FULL reorganization - pg_reorg_table(tableoid, NULL) for a CLUSTER reorganization if table has a CLUSTER key - pg_reorg_table(tableoid, columnname) for a CLUSTER reorganization based on a wanted column. Is it worth the shot? I haven't seen this documented as such, but AFAICT the reason that pg_reorg is split into a binary and set of backend functions which are called by the binary is that pg_reorg needs to be able to control its steps in several transactions so as to avoid holding locks excessively. The reorg_one_table() function uses four or five transactions per table, in fact. If all the logic currently in the pg_reorg binary were moved into backend functions, calling pg_reorg_table() would have to be a single transaction, and there would be no advantage to using such a function vs. CLUSTER or VACUUM FULL. Also, having a separate binary we should be able to perform some neat tricks such as parallel index builds using multiple connections (I'm messing around with this idea now). AFAIK this would also not be possible if pg_reorg were contained solely in the library functions. Josh -- 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] 64-bit API for large object
To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
is a pointer to 64-bit integer and actual data is placed somewhere
else. There might be other way: add new member to union u to store
64-bit integer:
typedef struct
{
int len;
int isint;
union
{
int*ptr;/* can't use void
(dec compiler barfs) */
int integer;
int64 bigint; /* 64-bit integer */
} u;
} PQArgBlock;
I'm a little bit worried about this way because PQArgBlock is a public
interface.
I'm inclined to add a new field for the union; that seems to me straight
forward approach.
For example, the manner in lo_seek64() seems to me confusable.
It set 1 on isint field even though pointer is delivered actually.
+ argv[1].isint = 1;
+ argv[1].len = 8;
+ argv[1].u.ptr = (int *) len;
I have to admit that this is confusing. However I'm worring about
changing sizeof(PQArgBlock) from compatibility's point of view. Maybe
I'm just a paranoia though.
Also we add new type pg_int64:
#ifndef NO_PG_INT64
#define HAVE_PG_INT64 1
typedef long long int pg_int64;
#endif
in postgres_ext.h per suggestion from Tom Lane:
http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php
I'm uncertain about context of this discussion.
Does it make matter if we include stdint.h and use int64_t instead
of the self defined data type?
I think Tom's point is, there are tons of applications which define
their own int64_t (at least in 2005).
Also pg_config.h has:
#define HAVE_STDINT_H 1
and this suggests that PostgreSQL adopts to platforms which does not
have stdint.h. If so, we need to take care of such platforms anyway.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.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] 64-bit API for large object
Tatsuo Ishii is...@postgresql.org writes: To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr is a pointer to 64-bit integer and actual data is placed somewhere else. Yeah, I think we have to do it like that. Changing the size of PQArgBlock would be a libpq ABI break, which IMO is sufficiently painful to kill this whole proposal. Much better a little localized ugliness in fe-lobj.c. 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] pg_reorg in core?
On Fri, Sep 21, 2012 at 12:07 PM, Josh Kupershmidt schmi...@gmail.comwrote: On Thu, Sep 20, 2012 at 7:05 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hi all, During the last PGCon, I heard that some community members would be interested in having pg_reorg directly in core. I'm actually not crazy about this idea, at least not given the current state of pg_reorg. Right now, there are a quite a few fixes and features which remain to be merged in to cvs head, but at least we can develop pg_reorg on a schedule independent of Postgres itself, i.e. we can release new features more often than once a year. Perhaps when pg_reorg is more stable, and the known bugs and missing features have been ironed out, we could think about integrating into core. What could be also great is to move the project directly into github to facilitate its maintenance and development. My own copy is based and synced on what is in pgfoundry as I don't own any admin access to on pgfoundry (honestly don't think I can get one either), even if I am from NTT. Hey, some people with admin rights here? Granted, a nice thing about integrating with core is we'd probably have more of an early warning when reshuffling of PG breaks pg_reorg (e.g. the recent splitting of the htup headers), but such changes have been quick and easy to fix so far. Yes, that is also why I am proposing to integrate it into core. Its maintenance pace would be faster and easier than it is now in pgfoundry. However, if hackers do not think that it is worth adding it to core... Well separate development as done now would be fine but slower... Also, just by watching the extension modules in contrib, I haven't seen one using both the library and binary at the same time like pg_reorg does. - creation of indexes on the temporary table based on what the user wishes - Apply the logs registered during the index creation - Swap the names of freshly created table and old table - Drop the useless objects The code is hosted by pg_foundry here: http://pgfoundry.org/projects/reorg/. I am also maintaining a fork in github in sync with pgfoundry here: https://github.com/michaelpq/pg_reorg. Just, do you guys think it is worth adding a functionality like pg_reorg in core or not? If yes, well I think the code of pg_reorg is going to need some modifications to make it more compatible with contrib modules using only EXTENSION. For the time being pg_reorg is divided into 2 parts, binary and library. The library part is the SQL portion of pg_reorg, containing a set of C functions that are called by the binary part. This has been extended to support CREATE EXTENSION recently. The binary part creates a command pg_reorg in charge of calling the set of functions created by the lib part, being just a wrapper of the library part to control the creation and deletion of the objects. It is also in charge of deleting the temporary objects by callback if an error occurs. By using the binary command, it is possible to reorganize a single table or a database, in this case reorganizing a database launches only a loop on each table of this database. My idea is to remove the binary part and to rely only on the library part to make pg_reorg a single extension with only system functions like other contrib modules. In order to do that what is missing is a function that could be used as an entry point for table reorganization, a function of the type pg_reorg_table(tableoid) and pg_reorg_table(tableoid, text). All the functionalities of pg_reorg could be reproducible: - pg_reorg_table(tableoid) for a VACUUM FULL reorganization - pg_reorg_table(tableoid, NULL) for a CLUSTER reorganization if table has a CLUSTER key - pg_reorg_table(tableoid, columnname) for a CLUSTER reorganization based on a wanted column. Is it worth the shot? I haven't seen this documented as such, but AFAICT the reason that pg_reorg is split into a binary and set of backend functions which are called by the binary is that pg_reorg needs to be able to control its steps in several transactions so as to avoid holding locks excessively. The reorg_one_table() function uses four or five transactions per table, in fact. If all the logic currently in the pg_reorg binary were moved into backend functions, calling pg_reorg_table() would have to be a single transaction, and there would be no advantage to using such a function vs. CLUSTER or VACUUM FULL. Of course, but functionalities like CREATE INDEX CONCURRENTLY use multiple transactions. Couldn't it be possible to use something similar to make the modifications visible to other backends? Also, having a separate binary we should be able to perform some neat tricks such as parallel index builds using multiple connections (I'm messing around with this idea now). AFAIK this would also not be possible if pg_reorg were contained solely in the library functions.
Re: [HACKERS] pg_reorg in core?
On Thu, Sep 20, 2012 at 7:05 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hi all, During the last PGCon, I heard that some community members would be interested in having pg_reorg directly in core. Just to recall, pg_reorg is a functionality developped by NTT that allows to redistribute a table without taking locks on it. The technique it uses to reorganize the table is to create a temporary copy of the table to be redistributed with a CREATE TABLE AS whose definition changes if table is redistributed with a VACUUM FULL or CLUSTER. Then it follows this mechanism: - triggers are created to redirect all the DMLs that occur on the table to an intermediate log table. - creation of indexes on the temporary table based on what the user wishes - Apply the logs registered during the index creation - Swap the names of freshly created table and old table - Drop the useless objects I'm not familiar with pg_reorg, but I wonder why we need a separate program for this task. I know pg_reorg is ok as an external program per se, but if we could optimize CLUSTER (or VACUUM which I'm a little pessimistic about) in the same way, it's much nicer than having additional binary + extension. Isn't it possible to do the same thing above within the CLUSTER command? Maybe CLUSTER .. CONCURRENTLY? Thanks, -- Hitoshi Harada -- 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_reorg in core?
On Thu, Sep 20, 2012 at 8:33 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Sep 21, 2012 at 12:07 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Thu, Sep 20, 2012 at 7:05 PM, Michael Paquier michael.paqu...@gmail.com wrote: What could be also great is to move the project directly into github to facilitate its maintenance and development. No argument from me there, especially as I have my own fork in github, but that's up to the current maintainers. Granted, a nice thing about integrating with core is we'd probably have more of an early warning when reshuffling of PG breaks pg_reorg (e.g. the recent splitting of the htup headers), but such changes have been quick and easy to fix so far. Yes, that is also why I am proposing to integrate it into core. Its maintenance pace would be faster and easier than it is now in pgfoundry. If the argument for moving pg_reorg into core is faster and easier development, well I don't really buy that. Yes, there would presumably be more eyeballs on the project, but you could make the same argument about any auxiliary Postgres project which wants more attention, and we can't have everything in core. And I fail to see how being in-core makes development easier; I think everyone here would agree that the bar to commit things to core is pretty darn high. If you're concerned about the [lack of] development on pg_reorg, there are plenty of things to fix without moving the project. I recently posted an issues roundup to the reorg list, if you are interested in pitching in. Josh -- 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] Assigning NULL to a record variable
2012/9/20 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2012/9/20 Tom Lane t...@sss.pgh.pa.us: I'm not sure what the performance tradeoffs would be --- some things would get faster and others slower, probably, since field access would be more work but conversion to/from HeapTuple would get far cheaper. when fields are fix length, then field's update is significantly faster then when RECORD is used [ shrug... ] Maybe not if we put a little effort into it. It would be interesting to consider using a TupleTableSlot instead of a bare HeapTuple for instance. In any case you're ignoring the point that other things will get faster. I didn't try to optimize this. But these tests showed important impact. postgres=# create table foo(a int, b int, c int); CREATE TABLE postgres=# insert into foo select 1,2,3 from generate_series(1,10); INSERT 0 10 postgres=# select count(*) from foo; count 10 (1 row) postgres=# do $$ declare r record; begin for r in select * from foo loop perform r.a + r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 712.404 ms postgres=# do $$ declare r foo; begin for r in select * from foo loop perform r.a + r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 1617.847 ms In this case, record is faster - it was used in PL/PSM0 too postgres=# do $$ declare r record; begin for r in select * from foo loop r.a := r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 170.701 ms postgres=# do $$ declare r foo; begin for r in select * from foo loop r.a := r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 96.467 ms or postgres=# do $$ declare r record; t int; begin for r in select * from foo loop t := r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 127.760 ms postgres=# do $$ declare r foo; t int; begin for r in select * from foo loop t := r.b + r.c; end loop; end; $$ language plpgsql; DO Time: 87.003 ms typed composite variable is significantly faster in simple queries (expressions) But I invite any reduction in this area regards, Pavel 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: [v9.3] Extra Daemons (Re: [HACKERS] elegant and effective way for running jobs inside a database)
On Thursday, September 20, 2012 7:13 PM Alvaro Herrera wrote: Excerpts from Amit Kapila's message of jue sep 20 02:10:23 -0300 2012: Why can't worker tasks be also permanent, which can be controlled through configuration. What I mean to say is that if user has need for parallel operations he can configure max_worker_tasks and those many worker tasks will get created. Otherwise without having such parameter, we might not be sure whether such deamons will be of use to database users who don't need any background ops. The dynamism will come in to scene when we need to allocate such daemons for particular ops(query), because might be operation need certain number of worker tasks, but no such task is available, at that time it need to be decided whether to spawn a new task or change the parallelism in operation such that it can be executed with available number of worker tasks. Well, there is a difficulty here which is that the number of processes connected to databases must be configured during postmaster start (because it determines the size of certain shared memory structs). So you cannot just spawn more tasks if all max_worker_tasks are busy. (This is a problem only for those workers that want to be connected as backends. Those that want libpq connections do not need this and are easier to handle.) Are you telling about shared memory structs that need to be allocated for each worker task? I am not sure if they can be shared across multiple slaves or will be required for each slave. However even if that is not possible, other mechanism can be used to get the work done by existing slaves. If not above then where there is a need of dynamic worker tasks as mentioned by Simon? The design we're currently discussing actually does not require a new GUC parameter at all. This is why: since the workers must be registered before postmaster start anyway (in the _PG_init function of a module that's listed in shared_preload_libraries) then we have to run a registering function during postmaster start. So postmaster can simply count how many it needs and size those structs from there. Workers that do not need a backend-like connection don't have a shmem sizing requirement so are not important for this. Configuration is thus simplified. BTW I am working on this patch and I think I have a workable design in place; I just couldn't get the code done before the start of this commitfest. (I am missing handling the EXEC_BACKEND case though, but I will not even look into that until the basic Unix case is working). One thing I am not going to look into is how is this new capability be used for parallel query. I feel we have enough use cases without it, that we can develop a fairly powerful feature. After that is done and proven (and committed) we can look into how we can use this to implement these short-lived workers for stuff such as parallel query. Agreed and I also meant to say the same as you are saying. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [v9.3] Extra Daemons (Re: [HACKERS] elegant and effective way for running jobs inside a database)
On Thursday, September 20, 2012 7:35 PM Kohei KaiGai wrote: 2012/9/20 Amit Kapila amit.kap...@huawei.com: On Thursday, September 20, 2012 1:44 AM Simon Riggs wrote: On 12 September 2012 04:30, Amit Kapila amit.kap...@huawei.com wrote: On Tuesday, September 11, 2012 9:09 PM Alvaro Herrera wrote: Excerpts from Boszormenyi Zoltan's message of vie jun 29 09:11:23 -0400 2012: We have some use cases for this patch, when can you post a new version? I would test and review it. What use cases do you have in mind? Wouldn't it be helpful for some features like parallel query in future? Trying to solve that is what delayed this patch, so the scope of this needs to be permanent daemons rather than dynamically spawned worker tasks. Why can't worker tasks be also permanent, which can be controlled through configuration. What I mean to say is that if user has need for parallel operations he can configure max_worker_tasks and those many worker tasks will get created. Otherwise without having such parameter, we might not be sure whether such deamons will be of use to database users who don't need any background ops. The dynamism will come in to scene when we need to allocate such daemons for particular ops(query), because might be operation need certain number of worker tasks, but no such task is available, at that time it need to be decided whether to spawn a new task or change the parallelism in operation such that it can be executed with available number of worker tasks. I'm also not sure why permanent daemons is more difficult than dynamically spawned daemons, I think Alvaro and Simon also felt permanent daemons is not difficult and is the right way to go, thats why the feature is getting developed on those lines. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
