Re: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review]
2013-01-13 05:49 keltezéssel, Amit kapila írta: On Sunday, January 13, 2013 12:41 AM Tom Lane wrote: Boszormenyi Zoltan z...@cybertec.at writes: No, I mean the reaper(SIGNAL_ARGS) function in src/backend/postmaster/postmaster.c where your patch has this: *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *** *** 2552,2557 reaper(SIGNAL_ARGS) I have not been following this thread, but I happened to see this bit, and I have to say that I am utterly dismayed that anything like this is even on the table. This screams overdesign. We do not need a global lock file, much less postmaster-side cleanup. All that's needed is a suitable convention about temp file names that can be written and then atomically renamed into place. If we're unlucky enough to crash before a temp file can be renamed into place, it'll just sit there harmlessly. I can think of below ways to generate tmp file 1. Generate session specific tmp file name during operation. This will be similar to what is currently being done in OpenTemporaryFileinTablespace() to generate a tempfilename. 2. Generate global tmp file name. For this we need to maintain global counter. 3. Always generate temp file with same name postgresql.auto.conf.tmp, as at a time only one session is allowed to operate. What's wrong with mkstemp(config_dir/postgresql.auto.conf.XX) that returns a file descriptor for an already created file with a unique name? Note that the working directory of PostgreSQL is $PGDATA so config_dir and files under that can be accessed with a relative path. But a cleanup somewhere is needed to remove the stale temp files. I think it's enough at postmaster startup. A machine that's crashing so often that the presence of the stale temp files causes out of disk errors would surely be noticed much earlier. In any approach, there will be chance that temp files will remain if server crashes during this command execution which can lead to collision of temp file name next time user tries to use SET persistent command. mkstemp() replaces the XX suffix with a unique hexadecimal suffix so there will be no collision. An appropriate error will be raised and user manually needs to delete that file. I just noticed that you are using %m in format strings twice. man 3 printf says: m (Glibc extension.) Print output of strerror(errno). No argument is required. This doesn't work anywhere else, only on GLIBC systems, it means Linux. I also like the brevity of this extension but PostgreSQL is a portable system. You should use %s and strerror(errno) as argument explicitly. %m is used at other places in code as well. As far as that goes, %m is appropriate in elog/ereport (which contain special support for it), but not anywhere else. In the patch, it's used in ereport, so I assume it is safe and patch doesn't need any change for %m. OK. With Regards, Amit Kapila. -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] erroneous restore into pg_catalog schema
If you dump a table with -t schema.table, and in the receiving database that schema does not exist, pg_restore-9.3devel will restore into the pg_catalog schema: HEAD $ cat test.sh #!/bin/sh db=backupbug; dropdb --echo $db; createdb --echo $db; echo drop schema if exists s cascade; | psql -ad $db echo create schema s; | psql -ad $db echo create table s.t as select i from generate_series(1,10) as f(i); | psql -ad $db echo '\dt+ pg_catalog.t' | psql -ad $db pg_dump -F c -t s.t -f st.dump $db echo drop schema if exists s cascade; | psql -ad $db pg_restore -xOv -F c -d $db st.dump echo '\dn' | psql -ad $db echo '\dt+ s.' | psql -ad $db echo '\dt+ pg_catalog.t' | psql -ad $db output: $ ./test.sh DROP DATABASE backupbug; CREATE DATABASE backupbug; drop schema if exists s cascade; DROP SCHEMA create schema s; CREATE SCHEMA create table s.t as select i from generate_series(1,1000) as f(i); SELECT 1000 \dt+ pg_catalog.t No matching relations found. drop schema if exists s cascade; DROP SCHEMA pg_restore: connecting to database for restore pg_restore: creating TABLE t pg_restore: processing data for table t pg_restore: setting owner and privileges for TABLE t pg_restore: setting owner and privileges for TABLE DATA t \dn List of schemas Name | Owner +-- public | aardvark (1 row) \dt+ s. No matching relations found. \dt+ pg_catalog.t List of relations Schema | Name | Type | Owner | Size | Description +--+---+--+---+- pg_catalog | t| table | aardvark | 64 kB | (1 row) #-- And then adds insult to injury: backupbug=# drop table pg_catalog.t; ERROR: permission denied: t is a system catalog Off course the workaround is obvious, but shouldn't this be prevented from happening in the first place? 9.2 refuses to do such a restore, which seems much better. (and yes, I did restore a 65 GB table into the pg_catalog schema of a dev machine; how can I remove it? I could initdb, but it's 200+ GB; I'd rather not have to rebuild it) Thanks, Erik Rijkers -- 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] enhanced error fields
On 13 January 2013 06:13, Pavel Stehule pavel.steh...@gmail.com wrote: Not sure, but I don't think it matters. You can blame the constraint implementation, but that doesn't change my feelings about what we need before we can accept a patch like this. Providing something which works only part of the time and then doesn't work for very unclear reasons isn't a good idea. Perhaps we need to fix the constraint implementation and perhaps we need to fix the error information being returned, or most likely we have to fix both, it doesn't change that we need to do something more than just ignore this problem. so we have to solve this issue first. Please, can you do resume, what is and where is current constraint implementation raise strange/unexpected messages? I felt that this was quite unnecessary because of the limited scope of the patch, and because this raises thorny issues of both semantics and implementation. Tom agreed with this general view - after all, this patch exists for the express purpose of having a well-principled way of obtaining the various fields across lc_messages settings. So I don't see that we have to do anything about making a constraint_schema available. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 01/13/2013 12:28 AM, Noah Misch wrote: [Catching up on old threads.] On Sat, Nov 17, 2012 at 03:40:49PM +0100, Hannu Krosing wrote: On 11/17/2012 03:00 PM, Markus Wanner wrote: On 11/17/2012 02:30 PM, Hannu Krosing wrote: Is it possible to replicate UPDATEs and DELETEs without a primary key in PostgreSQL-R No. There must be some way to logically identify the tuple. It can be done as selecting on _all_ attributes and updating/deleting just the first matching row create cursor ... select from t ... where t.* = () fetch one ... delete where current of ... This is on distant (round 3 or 4) roadmap for this work, just was interested if you had found any better way of doing this :) That only works if every attribute's type has a notion of equality (xml does not). The equality operator may have a name other than =, and an operator named = may exist with semantics other than equality (box is affected). Code attempting this replication strategy should select an equality operator the way typcache.c does so. Does this hint that postgreSQL also needs an sameness operator ( is or === in same languages). Or does IS NOT DISTINCT FROM already work even for types without comparison operator ? -- Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 01/13/2013 10:49 AM, Hannu Krosing wrote: On 01/13/2013 12:28 AM, Noah Misch wrote: [Catching up on old threads.] On Sat, Nov 17, 2012 at 03:40:49PM +0100, Hannu Krosing wrote: On 11/17/2012 03:00 PM, Markus Wanner wrote: On 11/17/2012 02:30 PM, Hannu Krosing wrote: Is it possible to replicate UPDATEs and DELETEs without a primary key in PostgreSQL-R No. There must be some way to logically identify the tuple. It can be done as selecting on _all_ attributes and updating/deleting just the first matching row create cursor ... select from t ... where t.* = () fetch one ... delete where current of ... This is on distant (round 3 or 4) roadmap for this work, just was interested if you had found any better way of doing this :) That only works if every attribute's type has a notion of equality (xml does not). The equality operator may have a name other than =, and an operator named = may exist with semantics other than equality (box is affected). Code attempting this replication strategy should select an equality operator the way typcache.c does so. Does this hint that postgreSQL also needs an sameness operator ( is or === in same languages). Or does IS NOT DISTINCT FROM already work even for types without comparison operator ? Just checked - it does not, it still looks for = operator so it is just equality-with-nulls How do people feel about adding a real sameness operator ? Hannu -- Hannu -- 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: log plans of cancelled queries
Pavel Stehule pavel.steh...@gmail.com writes: My propose is proposed for different dimensions and purpose - for example - we have a limit 20 minutes for almost all queries, and after this limit we killing queries. But we have to know little bit more about these bad queries - and we hope, so execution plan can give this additional info. We have same motivation like people who use auto_explain for slow query - but we can't to wait to query complete. Set auto_explain.log_min_duration to 19 mins or maybe 17 and be done? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] erroneous restore into pg_catalog schema
Erik Rijkers e...@xs4all.nl writes: (and yes, I did restore a 65 GB table into the pg_catalog schema of a dev machine; how can I remove it? I could initdb, but it's 200+ GB; I'd rather not have to rebuild it) See allow_system_table_mods http://www.postgresql.org/docs/9.2/static/runtime-config-developer.html Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] [Pgbuildfarm-members] Version 4.10 of buildfarm client released.
Andrew Dunstan wrote:
Part of the trouble with detecting rogue postmasters it might have left
lying around is that various things like to decide what port to run on,
so it's not always easy for the buildfarm to know what it should be
looking for.
For Linux, perhaps some form of lsof with the +D option? Maybe?:
lsof +D $PGDATA -Fp | grep -E '^p[0-9]{1,5}$' | cut -c1- | xargs kill -9
-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] Porting to Haiku
I understand. I will start working on reapplying my patches to master. Git is still new to me, but I'm making progress. I'm running into a little problem with applying my patches. It seems I made a quick fix in my port that I can't commit and I'm not sure what the best way to fix this is. The problem is that Haiku does have getrusage, but does not have all the fields as used in Postgresql. If I undef HAVE_GETRUSAGE, the code that uses those fields is not compiled (see src/backend/trcop/postgres.c line 4338), but then it will include the rusagestub.h file (see line 38 of the same file). The rusagestub.h file contains all the definitions that already exist in Haiku, so then it trips over that. I commented out the include of the rusagestub.h file in my initial work, but that is obviously not a good idea as a patch. I can try to skip the include if compiling on Haiku, but I'm not sure this is the best way to do this. Kind regards, Mark -- Spangalese for beginners: `Geh nae?' `Do you know the serial number on the bumper of that 1958 Thunderbird?' -- 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] [Pgbuildfarm-members] Version 4.10 of buildfarm client released.
On 01/13/2013 10:58 AM, Kevin Grittner wrote:
Andrew Dunstan wrote:
Part of the trouble with detecting rogue postmasters it might have left
lying around is that various things like to decide what port to run on,
so it's not always easy for the buildfarm to know what it should be
looking for.
For Linux, perhaps some form of lsof with the +D option? Maybe?:
lsof +D $PGDATA -Fp | grep -E '^p[0-9]{1,5}$' | cut -c1- | xargs kill -9
This actually won't help. In most cases the relevant data directory has
long disappeared out from under the rogue postmaster as part of
buildfarm cleanup. Also, lsof is not universally available. We try to
avoid creating new dependencies if possible.
Yesterday I committed a change that will let the buildfarm client ensure
that all the tests it runs are run on the configured build port.
Given that, we can should be able reliably to detect a rogue postmaster
by testing for the existence of a socket at /tmp/.S.PGSQL.$buildport.
Certainly, having something there will cause a failure. I currently have
this test running both before a run starts and after it finishes on the
buildfarm development instance (crake), using perl's -S operator. If it
fails there will be a buildfarm failure on stage Pre-run-port-check or
Post-run-port-check.
For the pre-run check I'm not inclined to do anything. If there's a
pre-existing listener on the required port it's an error and we'll just
abort, before we even try a checkout let alone anything else.
For the post-run check, we could possibly do something like
fuser -k /tmp/.s.PGSQL.$buildport
although that's not portable either ;-( .
None of this helps for msvc or mingw builds where there's no unix
socket, and I'll have to come up with another idea. But it's a start.
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] erroneous restore into pg_catalog schema
Erik Rijkers e...@xs4all.nl writes: If you dump a table with -t schema.table, and in the receiving database that schema does not exist, pg_restore-9.3devel will restore into the pg_catalog schema: ... Off course the workaround is obvious, but shouldn't this be prevented from happening in the first place? 9.2 refuses to do such a restore, which seems much better. I said to myself huh? surely we did not change pg_dump's behavior here. But actually it's true, and the culprit is commit 880bfc328, Silently ignore any nonexistent schemas that are listed in search_path. What pg_dump is emitting is SET search_path = s, pg_catalog; CREATE TABLE t (...); and in HEAD the SET throws no error and instead establishes pg_catalog as the target schema for object creation. Oops. So obviously, 880bfc328 was several bricks shy of a load. The arguments for that change in behavior still seem good for schemas *after* the first one; but the situation is entirely different for the first schema, because that's what the command is attempting to establish as the creation schema. In hindsight it might've been better if we'd used a separate GUC for the object creation schema, but it's much too late to change that. When we're dealing with a client-supplied SET command, I think it'd be okay to just throw an error if the first schema doesn't exist. However, the main issue in the discussion leading up to 880bfc328 was how to behave for search_path values coming from noninteractive sources such as postgresql.conf. In such cases we really don't have much choice about whether to adopt the value in some sense. I think maybe what we should do is have namespace.c retain an explicit notion that the first schema listed in search_path didn't exist, and then throw errors if any attempt is made to create objects without an explicitly specified namespace. If we did that then we'd have a choice about whether to throw error in the interactive-SET case. I'm a bit inclined to let it pass with no error for consistency with the non-interactive case; IIRC such consistency was one of the arguments made in the previous discussion. But certainly there's an argument to be made the other way, too. Thoughts? 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: log plans of cancelled queries
2013/1/13 Dimitri Fontaine dimi...@2ndquadrant.fr: Pavel Stehule pavel.steh...@gmail.com writes: My propose is proposed for different dimensions and purpose - for example - we have a limit 20 minutes for almost all queries, and after this limit we killing queries. But we have to know little bit more about these bad queries - and we hope, so execution plan can give this additional info. We have same motivation like people who use auto_explain for slow query - but we can't to wait to query complete. Set auto_explain.log_min_duration to 19 mins or maybe 17 and be done? We have limit 5 sec - but we have to kill queries after 20 minutes - and we have no plan for these queries. Regards Pavel Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: log plans of cancelled queries
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Pavel Stehule pavel.steh...@gmail.com writes: My propose is proposed for different dimensions and purpose - for example - we have a limit 20 minutes for almost all queries, and after this limit we killing queries. But we have to know little bit more about these bad queries - and we hope, so execution plan can give this additional info. We have same motivation like people who use auto_explain for slow query - but we can't to wait to query complete. Set auto_explain.log_min_duration to 19 mins or maybe 17 and be done? That would only help if he were willing to wait for the long-running command to be done (instead of canceling it). It's not hard to think of commands that will still be running after the heat death of the universe. 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] Porting to Haiku
On 01/13/2013 12:18 PM, Mark Hellegers wrote: I understand. I will start working on reapplying my patches to master. Git is still new to me, but I'm making progress. I'm running into a little problem with applying my patches. It seems I made a quick fix in my port that I can't commit and I'm not sure what the best way to fix this is. git commit -a should be all you need to commit something. All commits in git are local, so committing your changes won't hurt anyone else. The problem is that Haiku does have getrusage, but does not have all the fields as used in Postgresql. What is it missing? If I undef HAVE_GETRUSAGE, the code that uses those fields is not compiled (see src/backend/trcop/postgres.c line 4338), but then it will include the rusagestub.h file (see line 38 of the same file). The rusagestub.h file contains all the definitions that already exist in Haiku, so then it trips over that. I commented out the include of the rusagestub.h file in my initial work, but that is obviously not a good idea as a patch. I can try to skip the include if compiling on Haiku, but I'm not sure this is the best way to do this. I did get a haiku instance running yesterday. But I am kinda busy for a few days - I'll try to take a peek at this later in the week. I suggest you set up a repo at bitbucket or github that you can push your changes to so people can pull them if necessary. 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] enhanced error fields
Peter Geoghegan pe...@2ndquadrant.com writes: I felt that this was quite unnecessary because of the limited scope of the patch, and because this raises thorny issues of both semantics and implementation. Tom agreed with this general view - after all, this patch exists for the express purpose of having a well-principled way of obtaining the various fields across lc_messages settings. So I don't see that we have to do anything about making a constraint_schema available. Or in other words, there are two steps here: first, create infrastructure to expose the fields that we already provide within the regular message text; then two, consider adding new fields. The first part of that is a good deal less controversial than the second, so let's go ahead and get that part committed. 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] Re: logical changeset generation v3 - comparison to Postgres-R change set format
Hannu Krosing ha...@2ndquadrant.com writes: How do people feel about adding a real sameness operator ? Just begs the question of what's sameness? In many places we consider a datatype's default btree equality operator to define sameness, but not all types provide a btree opclass (in particular, anything that hasn't got a sensible one-dimensional sort order will not). And some do but it doesn't represent anything that anyone would want to consider sameness --- IIRC, some of the geometric types provide btree opclasses that sort by area. Even for apparently simple types like float8 there are interesting questions like whether minus zero is the same as plus zero. The messiness here is not just due to lack of a notation. 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] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-12 15:39:16 -0500, Tom Lane wrote:
Heikki Linnakangas hlinnakan...@vmware.com writes:
On 12.01.2013 20:42, Andres Freund wrote:
I don't care for that too much in detail -- if errstart were to return
false (it shouldn't, but if it did) this would be utterly broken,
With the current ereport(), we'll call abort() if errstart returns false
and elevel = ERROR. That's even worse than making an error reporting
calls that elog.c isn't prepared for.
No it isn't: you'd get a clean and easily interpretable abort. I am not
sure exactly what would happen if we plow forward with calling elog.c
functions after errstart returns false, but it wouldn't be good, and
debugging a field report of such behavior wouldn't be easy.
This is actually a disadvantage of the proposal to replace the abort()
calls with __builtin_unreachable(), too. The gcc boys interpret the
semantics of that as if control reaches here, the behavior is
undefined --- and their notion of undefined generally seems to amount
to we will screw you as hard as we can. For example, they have no
problem with using the assumption that control won't reach there to make
deductions while optimizing the rest of the function. If it ever did
happen, I would not want to have to debug it. The same goes for
__attribute__((noreturn)), BTW.
We could make add an Assert() additionally?
Yea, I didn't really like it all that much either - but anyway, I have
yet to find *any* version with a local variable that doesn't lead to
200kb size increase :(.
Hmm, that's strange. Assuming the optimizer optimizes away the paths in
the macro that are never taken when elevel is a constant, I would expect
the resulting binary to be smaller than what we have now.
I was wondering about that too, but haven't tried it locally yet. It
could be that Andres was looking at an optimization level in which a
register still got allocated for the local variable.
Nope, it was O2, albeit with -fno-omit-frame-pointer (for usable
hierarchical profiles).
Here's what I got with and without my patch on my laptop:
-rwxr-xr-x 1 heikki heikki 24767237 tammi 12 21:27 postgres-do-while-mypatch
-rwxr-xr-x 1 heikki heikki 24623081 tammi 12 21:28 postgres-unpatched
But when I build without --enable-debug, the situation reverses:
-rwxr-xr-x 1 heikki heikki 5961309 tammi 12 21:48 postgres-do-while-mypatch
-rwxr-xr-x 1 heikki heikki 5986961 tammi 12 21:49 postgres-unpatched
Yes, I noticed that too: these patches make the debug overhead grow
substantially for some reason. It's better to look at the output of
size rather than the executable's total file size. That way you don't
need to rebuild without debug to see reality. (I guess you could also
But the above is spot on. While I used strip earlier I somehow forgot it
here and so the debug overhead is part of what I saw. But, in comparison
to my baseline (which is replacing the abort() with
pg_unreachable()/__builtin_unreachable()) its still a loss
performancewise which is why I didn't doubt my results...
Pavel's testcase with changing ereport implementations (5 runs, minimal
time):
EREPORT_EXPR_OLD_NO_UNREACHABLE: 9964.015ms,5530296 bytes (stripped)
EREPORT_EXPR_OLD_ABORT: 9765.916ms,5466936 bytes (stripped)
EREPORT_EXPR_OLD_UNREACHABLE:9646.502ms,5462456 bytes (stripped)
EREPORT_STMT_HEIKKI: 10133.968ms, 5435704 bytes (stripped)
EREPORT_STMT_ANDRES: 9548.436ms,5462232 bytes (stripped)
Where my variant is:
#define ereport_domain(elevel, domain, rest)\
do {\
const int elevel_ = elevel; \
if (errstart(elevel_,__FILE__, __LINE__, PG_FUNCNAME_MACRO, domain))\
{ \
errfinish rest; \
} \
if (elevel_= ERROR)\
pg_unreachable(); \
} while(0)
So I suggest using that with an additional Assert(0) in the if(elevel_)
branch.
The assembler generated for my version is exactly the same when elevel
is constant in comparison with Heikki's but doesn't generate any
additional code in the case its not constant and I guess thats where it
gets faster?
Greetings,
Andres Freund
--
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] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-12 19:47:18 -0500, Tom Lane wrote:
Heikki Linnakangas hlinnakan...@vmware.com writes:
When I compile with gcc -O0, I get one warning with this:
datetime.c: In function �DateTimeParseError�:
datetime.c:3575:1: warning: �noreturn� function does return [enabled by
default]
That suggests that the compiler didn't correctly deduce that
ereport(ERROR, ...) doesn't return. With -O1, the warning goes away.
Yeah, I am seeing this too. It appears to be endemic to the
local-variable approach, ie if we have
const int elevel_ = (elevel);
...
(elevel_ = ERROR) ? pg_unreachable() : (void) 0
then we do not get the desired results at -O0, which is not terribly
surprising --- I'd not really expect the compiler to propagate the
value of elevel_ when not optimizing.
If we don't use a local variable, we don't get the warning, which
I take to mean that gcc will fold ERROR = ERROR to true even at -O0,
and that it does this early enough to conclude that unreachability
holds.
I experimented with some variant ways of phrasing the macro, but the
only thing that worked at -O0 required __builtin_constant_p, which
rather defeats the purpose of making this accessible to non-gcc
compilers.
Yea, its rather sad. The only thing I can see is actually doing both
variants like:
#define ereport_domain(elevel, domain, rest)\
do {\
const int elevel_ = elevel; \
if (errstart(elevel_,__FILE__, __LINE__, PG_FUNCNAME_MACRO, domain))\
{ \
errfinish rest; \
} \
if (pg_is_known_constant(elevel) (elevel) = ERROR) \
{ \
pg_unreachable(); \
Assert(0); \
} \
else if (elevel_= ERROR) \
{ \
pg_unreachable(); \
Assert(0); \
} \
} while(0)
but that obviously still relies on __builtin_constant_p giving
reasonable answers.
We should probably put that Assert(0) into pg_unreachable()...
If we go with the local-variable approach, we could probably suppress
this warning by putting an abort() call at the bottom of
DateTimeParseError. It seems a tad ugly though, and what's a bigger
deal is that if the compiler is unable to deduce unreachability at -O0
then we are probably going to be fighting such bogus warnings for all
time to come. Note also that an abort() (much less a pg_unreachable())
would not do anything positive like give us a compile warning if we
mistakenly added a case that could fall through.
On the other hand, if there's only one such case in our tree today,
maybe I'm worrying too much.
I think the only reason there's only one such case (two here actually,
second in renametrig) is that all others already have been silenced
because the abort() didn't use to be there. And I think the danger youre
alluding to above is a real one.
Greetings,
Andres Freund
--
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] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 01/13/2013 12:30 PM, Hannu Krosing wrote: On 01/13/2013 10:49 AM, Hannu Krosing wrote: Does this hint that postgreSQL also needs an sameness operator ( is or === in same languages). How do people feel about adding a real sameness operator ? We'd need to define what sameness means. If this goes toward exact match in binary representation, this gets a thumbs-up from me. As a first step in that direction, I'd see adjusting send() and recv() functions to use a portable binary format. A sameness operator could then be implemented by simply comparing two value's send() outputs. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 2013-01-13 12:44:44 -0500, Tom Lane wrote: Hannu Krosing ha...@2ndquadrant.com writes: How do people feel about adding a real sameness operator ? Just begs the question of what's sameness? In many places we consider a datatype's default btree equality operator to define sameness, but not all types provide a btree opclass (in particular, anything that hasn't got a sensible one-dimensional sort order will not). And some do but it doesn't represent anything that anyone would want to consider sameness --- IIRC, some of the geometric types provide btree opclasses that sort by area. Even for apparently simple types like float8 there are interesting questions like whether minus zero is the same as plus zero. The messiness here is not just due to lack of a notation. FWIW *I* (but others might) don't plan to support that case for now, it just seems to be too messy for far too little benefit. Greetings, Andres Freund -- 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] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-12 18:15:17 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-01-12 13:16:56 -0500, Tom Lane wrote: However, using a do-block with a local variable is definitely something worth considering. I'm getting less enamored of the __builtin_constant_p idea after finding out that the gcc boys seem to have curious ideas about what its semantics ought to be: https://bugzilla.redhat.com/show_bug.cgi?id=894515 I wonder whether __builtin_choose_expr is any better? Right offhand I don't see how that helps us. AFAICS, __builtin_choose_expr is the same as x?y:z except that y and z are not required to have the same datatype, which is okay because they require the value of x to be determinable at compile time. So we couldn't just write __builtin_choose_expr((elevel) = ERROR, ...). That would fail if elevel wasn't compile-time-constant. We could conceivably do __builtin_choose_expr(__builtin_constant_p(elevel) (elevel) = ERROR, ...) with the idea of making real sure that that expression reduces to a compile time constant ... but stacking two nonstandard constructs on each other seems to me to probably increase our exposure to gcc's definitional randomness rather than reduce it. I mean, if __builtin_constant_p can't already be trusted to act like a constant, why should we trust that __builtin_choose_expr doesn't also have a curious definition of constant-ness? I was thinking of something like the above, yes. It seems to me to generate code the compiler would need to really make sure the condition in __builtin_choose_expr() is really constant, so I hoped the checks inside are somewhat strict... Greetings, Andres Freund -- 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
[HACKERS] psql binary output
I whipped this up some months ago and forgot that I hadn't sent in the
patch.
This implements two psql commands: \gb and \gbn
Both fetch and output results in binary mode - \gb uses separators,
while \gbn does not. Examples:
[andrew@emma inst.psql-binout.5705]$ echo select bytea '\\x00010203', bytea
'\\x040506' \\gbn | bin/psql | od -c
000 \0 001 002 003 004 005 006
007
[andrew@emma inst.psql-binout.5705]$ echo select bytea '\\x00010203', bytea
'\\x040506' \\gb | bin/psql | od -c
000 \0 001 002 003 | 004 005 006 \n
011
This is an attempt to deal with the question I originally posed here:
http://people.planetpostgresql.org/andrew/index.php?/archives/196-Clever-trick-challenge.html
and is based on a suggestion Tom later made (although anything wrong
here is of course my fault, not his.
If people are interested I'll try to finish this up and document it.
cheers
andrew
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 59f8b03..434b1d6 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -748,6 +748,40 @@ exec_command(const char *cmd,
status = PSQL_CMD_SEND;
}
+ /* \gb means send query, get/produce results in binary */
+ else if (strcmp(cmd, gb) == 0)
+ {
+ char *fname = psql_scan_slash_option(scan_state,
+ OT_FILEPIPE, NULL, false);
+
+ if (!fname)
+ pset.gfname = NULL;
+ else
+ {
+ expand_tilde(fname);
+ pset.gfname = pg_strdup(fname);
+ }
+ free(fname);
+ status = PSQL_CMD_SEND_BIN;
+ }
+
+ /* \gbn means send query, get/produce results in binary, and no separators */
+ else if (strcmp(cmd, gbn) == 0)
+ {
+ char *fname = psql_scan_slash_option(scan_state,
+ OT_FILEPIPE, NULL, false);
+
+ if (!fname)
+ pset.gfname = NULL;
+ else
+ {
+ expand_tilde(fname);
+ pset.gfname = pg_strdup(fname);
+ }
+ free(fname);
+ status = PSQL_CMD_SEND_BIN_NS;
+ }
+
/* help */
else if (strcmp(cmd, h) == 0 || strcmp(cmd, help) == 0)
{
diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index 1b94e44..09a461b 100644
--- a/src/bin/psql/command.h
+++ b/src/bin/psql/command.h
@@ -16,6 +16,8 @@ typedef enum _backslashResult
{
PSQL_CMD_UNKNOWN = 0, /* not done parsing yet (internal only) */
PSQL_CMD_SEND,/* query complete; send off */
+ PSQL_CMD_SEND_BIN, /* same, but get/produce result in binary */
+ PSQL_CMD_SEND_BIN_NS, /* same, but with no separators*/
PSQL_CMD_SKIP_LINE, /* keep building query */
PSQL_CMD_TERMINATE, /* quit program */
PSQL_CMD_NEWEDIT, /* query buffer was changed (e.g., via \e) */
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 5fb0316..590d268 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -579,7 +579,9 @@ PrintNotifications(void)
* Returns true if successful, false otherwise.
*/
static bool
-PrintQueryTuples(const PGresult *results)
+PrintQueryTuples(const PGresult *results,
+ bool binary,
+ bool noseps)
{
printQueryOpt my_popt = pset.popt;
@@ -600,7 +602,10 @@ PrintQueryTuples(const PGresult *results)
return false;
}
- printQuery(results, my_popt, pset.queryFout, pset.logfile);
+ if (!binary)
+ printQuery(results, my_popt, pset.queryFout, pset.logfile);
+ else
+ printQueryBin(results, my_popt, pset.queryFout, pset.logfile, noseps);
/* close file/pipe, restore old setting */
setQFout(NULL);
@@ -612,7 +617,12 @@ PrintQueryTuples(const PGresult *results)
pset.gfname = NULL;
}
else
- printQuery(results, my_popt, pset.queryFout, pset.logfile);
+ {
+ if (!binary)
+ printQuery(results, my_popt, pset.queryFout, pset.logfile);
+ else
+ printQueryBin(results, my_popt, pset.queryFout, pset.logfile, noseps);
+ }
return true;
}
@@ -762,7 +772,9 @@ PrintQueryStatus(PGresult *results)
* Returns true if the query executed successfully, false otherwise.
*/
static bool
-PrintQueryResults(PGresult *results)
+PrintQueryResults(PGresult *results,
+ bool binary,
+ bool noseps)
{
bool success;
const char *cmdstatus;
@@ -774,7 +786,7 @@ PrintQueryResults(PGresult *results)
{
case PGRES_TUPLES_OK:
/* print the data ... */
- success = PrintQueryTuples(results);
+ success = PrintQueryTuples(results, binary, noseps);
/* if it's INSERT/UPDATE/DELETE RETURNING, also print status */
cmdstatus = PQcmdStatus(results);
if (strncmp(cmdstatus, INSERT, 6) == 0 ||
@@ -830,7 +842,9 @@ PrintQueryResults(PGresult *results)
* Returns true if the query executed successfully, false otherwise.
*/
bool
-SendQuery(const char *query)
+SendQuery(const char *query,
+ bool binary,
+ bool noseps)
{
PGresult *results;
PGTransactionStatusType transaction_status;
@@ -928,7 +942,10 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (! binary)
+ results = PQexec(pset.db, query);
+ else
+ results = PQexecParams(pset.db,
Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Andres Freund and...@2ndquadrant.com writes:
On 2013-01-12 15:39:16 -0500, Tom Lane wrote:
This is actually a disadvantage of the proposal to replace the abort()
calls with __builtin_unreachable(), too. The gcc boys interpret the
semantics of that as if control reaches here, the behavior is
undefined
We could make add an Assert() additionally?
I see little value in that. In a non-assert build it will do nothing at
all, and in an assert build it'd just add code bloat.
I think there might be a good argument for defining pg_unreachable() as
abort() in assert-enabled builds, even if the compiler has
__builtin_unreachable(): that way, if the impossible happens, we'll find
out about it in a predictable and debuggable way. And by definition,
we're not so concerned about either performance or code bloat in assert
builds.
Where my variant is:
#define ereport_domain(elevel, domain, rest)\
do {\
const int elevel_ = elevel; \
if (errstart(elevel_,__FILE__, __LINE__, PG_FUNCNAME_MACRO, domain))\
{ \
errfinish rest; \
} \
if (elevel_= ERROR)\
pg_unreachable(); \
} while(0)
This is the same implementation I'd arrived at after looking at
Heikki's. I think we should probably use this for non-gcc builds.
However, for recent gcc (those with __builtin_constant_p) I think that
my original suggestion is better, ie don't use a local variable but
instead use __builtin_constant_p(elevel) (elevel) = ERROR in the
second test. That way we don't have the problem with unreachability
tests failing at -O0. We may still have some issues with bogus warnings
in other compilers, but I think most PG developers are using recent gcc.
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] Porting to Haiku
On 01/13/2013 12:18 PM, Mark Hellegers wrote: I understand. I will start working on reapplying my patches to master. Git is still new to me, but I'm making progress. I'm running into a little problem with applying my patches. It seems I made a quick fix in my port that I can't commit and I'm not sure what the best way to fix this is. git commit -a should be all you need to commit something. All commits in git are local, so committing your changes won't hurt anyone else. Yes, I got that. It's quite different from subversion, which is what I am used to. The problem is that Haiku does have getrusage, but does not have all the fields as used in Postgresql. What is it missing? It complains that the structure rusage is missing the following fields: - ru_inblock - ru_outblock - ru_majflt - ru_minflt - ru_nswap - ru_nsignals - ru_msgrcv - ru_msgsnd - ru_nvcsw - ru_nivcsw I typed them over from the error output, so I may be missing one or mistyped something. If I undef HAVE_GETRUSAGE, the code that uses those fields is not compiled (see src/backend/trcop/postgres.c line 4338), but then it will include the rusagestub.h file (see line 38 of the same file). The rusagestub.h file contains all the definitions that already exist in Haiku, so then it trips over that. I commented out the include of the rusagestub.h file in my initial work, but that is obviously not a good idea as a patch. I can try to skip the include if compiling on Haiku, but I'm not sure this is the best way to do this. I did get a haiku instance running yesterday. But I am kinda busy for a few days - I'll try to take a peek at this later in the week. I suggest you set up a repo at bitbucket or github that you can push your changes to so people can pull them if necessary. I had a quick look at github and saw that there is a mirror of the postgresl project on there, but I'm not sure how to make everything work. I'm afraid I'm a bit too new to git to have that up and running without some help. If you could help me a bit with that (perhaps off list would be best), I would like to give it a try. Kind regards, Mark -- Spangalese for beginners: `Wiggilo wagel hoggle?' `Where can I scrub my eyeballs?' -- 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: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-12 16:36:39 -0500, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
It does *not* combine elog_start and elog_finish into one function if
varargs are available although that brings a rather measurable
size/performance benefit.
Since you've apparently already done the measurement: how much?
It would be a bit tedious to support two different infrastructures for
elog(), but if it's a big enough win maybe we should.
Imo its pretty definitely a big enough win. So big I have a hard time
believing it can be true without negative effects somewhere else.
Well, actually there's a pretty serious negative effect here, which is
that when it's implemented this way it's impossible to save errno for %m
before the elog argument list is evaluated. So I think this is a no-go.
We've always had the contract that functions in the argument list could
stomp on errno without care.
If we switch to a do-while macro expansion it'd be possible to do
something like
do {
int save_errno = errno;
int elevel = whatever;
elog_internal( save_errno, elevel, fmt, __VA__ARGS__ );
} while (0);
but this would almost certainly result in more code bloat not less,
since call sites would now be responsible for fetching errno.
the numbers are:
old definition: 10393.658ms, 5497912 bytes
old definition + unreachable: 10011.102ms, 5469144 bytes
stmt, two calls, unreachable: 10036.132ms, 5468792 bytes
stmt, one call, unreachable:9443.612ms, 5462232 bytes
stmt, one call, unreachable, save errno:9615.863ms, 5489688 bytes
So while not saving errno is unsurprisingly better its still a win.
Greetings,
Andres Freund
--
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] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Andres Freund and...@2ndquadrant.com writes: the numbers are: old definition: 10393.658ms, 5497912 bytes old definition + unreachable: 10011.102ms, 5469144 bytes stmt, two calls, unreachable: 10036.132ms, 5468792 bytes stmt, one call, unreachable:9443.612ms, 5462232 bytes stmt, one call, unreachable, save errno:9615.863ms, 5489688 bytes I find these numbers pretty hard to credit. Why should replacing two calls by one, in code paths that are not being taken, move the runtime so much? The argument that a net reduction of code size is a win doesn't work, because the last case is more code than any except the first. I think you're measuring some coincidental effect or other, not a reproducible performance improvement. Or there's a bug in the code you're using. 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] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-13 14:17:52 -0500, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
the numbers are:
old definition: 10393.658ms, 5497912 bytes
old definition + unreachable: 10011.102ms, 5469144 bytes
stmt, two calls, unreachable: 10036.132ms, 5468792 bytes
stmt, one call, unreachable:9443.612ms, 5462232 bytes
stmt, one call, unreachable, save errno:9615.863ms, 5489688 bytes
I find these numbers pretty hard to credit. Why should replacing two
calls by one, in code paths that are not being taken, move the runtime
so much? The argument that a net reduction of code size is a win
doesn't work, because the last case is more code than any except the
first.
There are quite some elog(DEBUG*)s in the backend, and those are taken,
so I don't find it unreasonable that doing less work in those cases is
measurable.
And if you look at the disassembly of ERROR codepaths:
saving errno, one function:
Dump of assembler code for function palloc:
639 {
0x00736397 +7: mov%rdi,%rsi
0x007363b0 +32:push %rbp
0x007363b1 +33:mov%rsp,%rbp
0x007363b4 +36:sub$0x20,%rsp
640 /* duplicates MemoryContextAlloc to
avoid increased overhead */
641 AssertArg(MemoryContextIsValid(CurrentMemoryContext));
642
643 if (!AllocSizeIsValid(size))
0x00736390 +0: cmp$0x3fff,%rdi
0x0073639a +10:ja 0x7363b0 palloc+32
644elog(ERROR, invalid memory alloc
request size %lu,
0x007363b8 +40:mov%rdi,-0x8(%rbp)
0x007363bc +44:callq 0x462010 __errno_location@plt
0x007363c1 +49:mov-0x8(%rbp),%rsi
0x007363c5 +53:mov$0x8ace30,%r9d
0x007363cb +59:mov$0x14,%ecx
0x007363d0 +64:mov$0x8acefe,%edx
0x007363d5 +69:mov$0x8ace58,%edi
0x007363da +74:mov%rsi,(%rsp)
0x007363de +78:mov(%rax),%r8d
0x007363e1 +81:mov$0x285,%esi
0x007363e6 +86:xor%eax,%eax
0x007363e8 +88:callq 0x71a0b0 elog_all
0x007363ed: nopl (%rax)
645 (unsigned long) size);
646
647 CurrentMemoryContext-isReset = false;
0x0073639c +12: mov0x25c6e5(%rip),%rdi
# 0x992a88 CurrentMemoryContext
0x007363a7 +23:movb $0x0,0x30(%rdi)
648
649 return (*CurrentMemoryContext-methods-alloc)
(CurrentMemoryContext, size);
0x007363a3 +19:mov0x8(%rdi),%rax
0x007363ab +27:mov(%rax),%rax
0x007363ae +30:jmpq *%rax
End of assembler dump.
vs
Dump of assembler code for function palloc:
639 {
0x007382b0 +0: push %rbp
0x007382b1 +1: mov%rsp,%rbp
0x007382b4 +4: push %rbx
0x007382b5 +5: mov%rdi,%rbx
0x007382b8 +8: sub$0x8,%rsp
640 /* duplicates MemoryContextAlloc to
avoid increased overhead */
641 AssertArg(MemoryContextIsValid(CurrentMemoryContext));
642
643 if (!AllocSizeIsValid(size))
0x007382bc +12:cmp$0x3fff,%rdi
0x007382c3 +19:jbe0x7382ed palloc+61
644elog(ERROR, invalid memory alloc
request size %lu,
0x007382c5 +21:mov$0x8aec9e,%edx
0x007382ca +26:mov$0x284,%esi
0x007382cf +31:mov$0x8aebd0,%edi
0x007382d4 +36:callq 0x71bcb0 elog_start
0x007382d9 +41:mov%rbx,%rdx
0x007382dc +44:mov$0x8aec10,%esi
0x007382e1 +49:mov$0x14,%edi
0x007382e6 +54:xor%eax,%eax
0x007382e8 +56:callq 0x71bac0 elog_finish
645 (unsigned long) size);
646
647 CurrentMemoryContext-isReset = false;
0x007382ed +61: mov0x25bc54(%rip),%rdi
# 0x993f48 CurrentMemoryContext
0x007382fb +75:movb $0x0,0x30(%rdi)
648
649 return (*CurrentMemoryContext-methods-alloc)
(CurrentMemoryContext, size);
0x007382f4 +68:mov%rbx,%rsi
0x007382f7 +71:mov0x8(%rdi),%rax
0x007382ff +79:mov(%rax),%rax
0x00738308 +88:jmpq *%rax
0x0073830a: nopw 0x0(%rax,%rax,1)
650 }
0x00738302 +82:add$0x8,%rsp
0x00738306 +86:pop%rbx
0x00738307 +87:pop%rbp
End of assembler dump.
If other critical paths look like this its not that surprising.
I think you're measuring some coincidental
Re: [HACKERS] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 01/13/2013 06:02 PM, Markus Wanner wrote: On 01/13/2013 12:30 PM, Hannu Krosing wrote: On 01/13/2013 10:49 AM, Hannu Krosing wrote: Does this hint that postgreSQL also needs an sameness operator ( is or === in same languages). How do people feel about adding a real sameness operator ? We'd need to define what sameness means. If this goes toward exact match in binary representation, this gets a thumbs-up from me. As a first step in that direction, I'd see adjusting send() and recv() functions to use a portable binary format. A sameness operator could then be implemented by simply comparing two value's send() outputs. This seems like a good definition of sameness to me - if binary images are bitwise same, then the values are the same. And if both are fields of the same type and NULLs then these are also same And defining a cross-platform binary format also a good direction of movement in implementing this. I'd just start with what send() and recv() on each type produces now using GCC on 64bit Intel and move towards adjusting others to match. For a period anything else would still be allowed, but be non-standard I have no strong opinion on typed NULLs, though I'd like them to also be the same for a sake of simplicity. As this would be non-standard anyway, I'd make a row of all nulls NOT be the same as NULL This would be much easier to explain than losing the IS NULL-ness at nesting level 3 ;) Hannu Regards Markus Wanner -- 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: logical changeset generation v3 - comparison to Postgres-R change set format
Hannu Krosing ha...@2ndquadrant.com writes: Does this hint that postgreSQL also needs an sameness operator ( is or === in same languages). How do people feel about adding a real sameness operator ? Well. I would prefer it if we can bypass the need for it. Then Do we need the full range of eq, eql, equal and equalp predicates, and would all of them allow overriding or just some? http://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node74.html Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: log plans of cancelled queries
Tom Lane t...@sss.pgh.pa.us writes: Set auto_explain.log_min_duration to 19 mins or maybe 17 and be done? That would only help if he were willing to wait for the long-running command to be done (instead of canceling it). It's not hard to think of commands that will still be running after the heat death of the universe. Oh. Brain fart from me. Sorry. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Andres Freund and...@2ndquadrant.com writes: On 2013-01-13 14:17:52 -0500, Tom Lane wrote: I find these numbers pretty hard to credit. There are quite some elog(DEBUG*)s in the backend, and those are taken, so I don't find it unreasonable that doing less work in those cases is measurable. Meh. If there are any elog(DEBUG)s in time-critical places, they should be changed to ereport's anyway, so as to eliminate most of the overhead when they're not firing. And if you look at the disassembly of ERROR codepaths: I think your numbers are being twisted by -fno-omit-frame-pointer. What I get, with the __builtin_unreachable version of the macro, looks more like MemoryContextAlloc: cmpq$1073741823, %rsi pushq %rbx movq%rsi, %rbx ja .L53 movq8(%rdi), %rax movb$0, 48(%rdi) popq%rbx movq(%rax), %rax jmp *%rax .L53: movl$__func__.5262, %edx movl$576, %esi movl$.LC2, %edi callelog_start movq%rbx, %rdx movl$.LC3, %esi movl$20, %edi xorl%eax, %eax callelog_finish With -fno-omit-frame-pointer it's a little worse, but still not what you show --- in particular, for me gcc still pushes the elog calls out of the main code path. I don't think that the main path will get any better with one elog function instead of two. It could easily get worse. On many machines, the single-function version would be worse because of needing to use more parameter registers, which would translate into more save/restore work in the calling function, which is overhead that would likely be paid whether elog actually gets called or not. (As an example, in the above code gcc evidently isn't noticing that it doesn't need to save/restore rbx so far as the main path is concerned.) In any case, results from a single micro-benchmark on a single platform with a single compiler version (and single set of compiler options) don't convince me a whole lot here. Basically, the aspects of this that I think are likely to be reproducible wins across different platforms are (a) teaching the compiler that elog(ERROR) doesn't return, and (b) reducing code size as much as possible. The single-function change isn't going to help on either ground --- maybe it would have helped on (b) without the errno problem, but that's going to destroy any possible code size savings. 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] Re: logical changeset generation v3 - comparison to Postgres-R change set format
On 01/13/2013 12:28 AM, Noah Misch wrote: [Catching up on old threads.] On Sat, Nov 17, 2012 at 03:40:49PM +0100, Hannu Krosing wrote: On 11/17/2012 03:00 PM, Markus Wanner wrote: On 11/17/2012 02:30 PM, Hannu Krosing wrote: Is it possible to replicate UPDATEs and DELETEs without a primary key in PostgreSQL-R No. There must be some way to logically identify the tuple. It can be done as selecting on _all_ attributes and updating/deleting just the first matching row create cursor ... select from t ... where t.* = () fetch one ... delete where current of ... This is on distant (round 3 or 4) roadmap for this work, just was interested if you had found any better way of doing this :) That only works if every attribute's type has a notion of equality (xml does not). The equality operator may have a name other than =, and an operator named = may exist with semantics other than equality (box is affected). Code attempting this replication strategy should select an equality operator the way typcache.c does so. A method for making this work as PostgreSQL works now would be to compare textual representations of tuples create cursor ... select from t ... where t::text = '(image of original row)' fetch one ... delete where current of ... But of course having an operator for sameness without needing to convert to text would be better Hannu -- 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] erroneous restore into pg_catalog schema
Tom Lane t...@sss.pgh.pa.us writes: I think maybe what we should do is have namespace.c retain an explicit notion that the first schema listed in search_path didn't exist, and then throw errors if any attempt is made to create objects without an explicitly specified namespace. I don't much like this. SET search_path TO dontexist, a, b; CREATE TABLE foo(); And the table foo is now in a (provided it exists). Your proposal would break that case, right? The problem is that the search_path could come from too many places: postgresql.conf, ALTER ROLE, ALTER DATABASE etc. And I have seen roles setup with some search_path containing schema that will only exist in some of the database they can connect to… -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-13 15:44:58 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: And if you look at the disassembly of ERROR codepaths: I think your numbers are being twisted by -fno-omit-frame-pointer. What I get, with the __builtin_unreachable version of the macro, looks more like The only difference is that the following two instructions aren't done: 0x007363b0 +32:push %rbp 0x007363b1 +33:mov%rsp,%rbp Given that that function barely uses registers (from an amd64 pov at least), thats notreally surprising. MemoryContextAlloc: cmpq$1073741823, %rsi pushq %rbx movq%rsi, %rbx ja .L53 movq8(%rdi), %rax movb$0, 48(%rdi) popq%rbx movq(%rax), %rax jmp *%rax .L53: movl$__func__.5262, %edx movl$576, %esi movl$.LC2, %edi callelog_start movq%rbx, %rdx movl$.LC3, %esi movl$20, %edi xorl%eax, %eax callelog_finish With -fno-omit-frame-pointer it's a little worse, but still not what you show --- in particular, for me gcc still pushes the elog calls out of the main code path. I don't think that the main path will get any better with one elog function instead of two. It could easily get worse. On many machines, the single-function version would be worse because of needing to use more parameter registers, which would translate into more save/restore work in the calling function, which is overhead that would likely be paid whether elog actually gets called or not. (As an example, in the above code gcc evidently isn't noticing that it doesn't need to save/restore rbx so far as the main path is concerned.) In any case, results from a single micro-benchmark on a single platform with a single compiler version (and single set of compiler options) don't convince me a whole lot here. I am not convinced either, I just got curious whether it would be a win after the __VA_ARGS__ thing made it possible. If the errno problem didn't exist I would be pretty damn sure its just about always a win, but the way its now... We could make elog behave the same as ereport WRT to argument evaluation but that seems a bit dangerous given it would still be different on some platforms. Basically, the aspects of this that I think are likely to be reproducible wins across different platforms are (a) teaching the compiler that elog(ERROR) doesn't return, and (b) reducing code size as much as possible. The single-function change isn't going to help on either ground --- maybe it would have helped on (b) without the errno problem, but that's going to destroy any possible code size savings. Agreed. I am happy to produce an updated patch unless youre already on it? Greetings, Andres Freund -- 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] erroneous restore into pg_catalog schema
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: I think maybe what we should do is have namespace.c retain an explicit notion that the first schema listed in search_path didn't exist, and then throw errors if any attempt is made to create objects without an explicitly specified namespace. I don't much like this. SET search_path TO dontexist, a, b; CREATE TABLE foo(); And the table foo is now in a (provided it exists). Your proposal would break that case, right? Break? You can't possibly think that's a good idea. The problem is that the search_path could come from too many places: postgresql.conf, ALTER ROLE, ALTER DATABASE etc. And I have seen roles setup with some search_path containing schema that will only exist in some of the database they can connect to⦠Right, that is the argument for ignoring missing schemas, and I think it is entirely sensible for *search* activities. But allowing *creation* to occur in an indeterminate schema is a horrid idea. BTW, although Erik claimed this behaved more sanely in 9.2, a closer look at the commit logs says that the bogus commit shipped in 9.2, so AFAICS it's broken there too. But earlier releases would have rejected the SET as expected. I think we should assume that existing code is expecting the pre-9.2 behavior. 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] count(*) of zero rows returns 1
Can somebody explain why a standalone count(*) returns 1? postgres=# select count(*); count --- 1 (1 row) I agree it's an odd thing for someone to query, but I feel it should return 0, and not 1. -- Gurjeet Singh http://gurjeet.singh.im/
Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Andres Freund and...@2ndquadrant.com writes: Basically, the aspects of this that I think are likely to be reproducible wins across different platforms are (a) teaching the compiler that elog(ERROR) doesn't return, and (b) reducing code size as much as possible. The single-function change isn't going to help on either ground --- maybe it would have helped on (b) without the errno problem, but that's going to destroy any possible code size savings. Agreed. I am happy to produce an updated patch unless youre already on it? On it now (busy testing on some old slow boxes, else I'd be done already). 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] erroneous restore into pg_catalog schema
Tom Lane t...@sss.pgh.pa.us writes: Break? You can't possibly think that's a good idea. I don't think it is. It's been used as a hack mainly before we had per-user and per-database settings, from what I've seen. Right, that is the argument for ignoring missing schemas, and I think it is entirely sensible for *search* activities. But allowing *creation* to occur in an indeterminate schema is a horrid idea. It's not so much indeterminate for the user, even if I understand why you say that. Creating new schemas is not done lightly in such cases… But well, the solution is simple enough in that case. Use the newer form ALTER ROLE foo IN DATABASE db1 SET search_path TO some, value; So I'm fine with that change in fact. Is it possible to extend the release notes to include so many details about it, as I don't think anyone will get much excited to report that as a HINT when the conditions are met… (although it might be simple enough thanks to the pg_setting view). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] erroneous restore into pg_catalog schema
On Sun, January 13, 2013 22:09, Tom Lane wrote: BTW, although Erik claimed this behaved more sanely in 9.2, a closer look at the commit logs says that the bogus commit shipped in 9.2, so AFAICS it's broken there too. But earlier releases would have rejected the SET as expected. I think we should assume that existing code is expecting the pre-9.2 behavior. $ psql psql (9.2.2-REL9_2_STABLE-20130113_1054-4ae5ee6c9b4dd7cd7e4471a44d371b228a9621c3) Running the same on 9.2.2 (with latest patches): $ ../../pgsql.HEAD/bug/test.sh DROP DATABASE backupbug; CREATE DATABASE backupbug; drop schema if exists s cascade; DROP SCHEMA create schema s; CREATE SCHEMA create table s.t as select i from generate_series(1,1000) as f(i); SELECT 1000 \dt+ pg_catalog.t No matching relations found. drop schema if exists s cascade; DROP SCHEMA pg_restore: connecting to database for restore pg_restore: creating TABLE t pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 169; 1259 26204 TABLE t aardvark pg_restore: [archiver (db)] could not execute query: ERROR: permission denied to create pg_catalog.t DETAIL: System catalog modifications are currently disallowed. Command was: CREATE TABLE t ( i integer ); pg_restore: restoring data for table t pg_restore: [archiver (db)] Error from TOC entry 2780; 0 26204 TABLE DATA t aardvark pg_restore: [archiver (db)] could not execute query: ERROR: relation t does not exist Command was: COPY t (i) FROM stdin; pg_restore: setting owner and privileges for TABLE t pg_restore: setting owner and privileges for TABLE DATA t WARNING: errors ignored on restore: 2 \dn List of schemas Name | Owner +-- public | aardvark (1 row) \dt+ s. No matching relations found. \dt+ pg_catalog.t No matching relations found. -- 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] erroneous restore into pg_catalog schema
On 2013-01-13 12:29:08 -0500, Tom Lane wrote: Erik Rijkers e...@xs4all.nl writes: If you dump a table with -t schema.table, and in the receiving database that schema does not exist, pg_restore-9.3devel will restore into the pg_catalog schema: ... Off course the workaround is obvious, but shouldn't this be prevented from happening in the first place? 9.2 refuses to do such a restore, which seems much better. I said to myself huh? surely we did not change pg_dump's behavior here. But actually it's true, and the culprit is commit 880bfc328, Silently ignore any nonexistent schemas that are listed in search_path. What pg_dump is emitting is SET search_path = s, pg_catalog; CREATE TABLE t (...); and in HEAD the SET throws no error and instead establishes pg_catalog as the target schema for object creation. Oops. So obviously, 880bfc328 was several bricks shy of a load. The arguments for that change in behavior still seem good for schemas *after* the first one; but the situation is entirely different for the first schema, because that's what the command is attempting to establish as the creation schema. There also is the seemingly independent question of why the heck its suddently allowed to create (but not drop?) tables in pg_catalog. 9.2 does: andres=# CREATE TABLE pg_catalog.c AS SELECT 1; ERROR: permission denied to create pg_catalog.c DETAIL: System catalog modifications are currently disallowed. Time: 54.180 ms HEAD: postgres=# CREATE TABLE pg_catalog.test AS SELECT 1; SELECT 1 Time: 124.112 ms postgres=# DROP TABLE pg_catalog.test; ERROR: permission denied: test is a system catalog Time: 0.461 ms Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] erroneous restore into pg_catalog schema
Erik Rijkers e...@xs4all.nl writes: On Sun, January 13, 2013 22:09, Tom Lane wrote: BTW, although Erik claimed this behaved more sanely in 9.2, a closer look at the commit logs says that the bogus commit shipped in 9.2, so AFAICS it's broken there too. [ not so ] Hm, you are right, there's another problem that's independent of search_path. In 9.2, regression=# create table pg_catalog.t(f1 int); ERROR: permission denied to create pg_catalog.t DETAIL: System catalog modifications are currently disallowed. but HEAD is missing that error check: regression=# create table pg_catalog.t(f1 int); CREATE TABLE I will bet that this is more breakage from the DDL-code refactoring that has been going on. I am getting closer and closer to wanting that reverted. KaiGai-san seems to have been throwing out lots of special cases that were there for good reasons. 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] count(*) of zero rows returns 1
Gurjeet Singh singh.gurj...@gmail.com writes: Can somebody explain why a standalone count(*) returns 1? postgres=# select count(*); count --- 1 (1 row) The Oracle equivalent of that would be SELECT count(*) FROM dual. Does it make more sense to you thought of that way? I agree it's an odd thing for someone to query, but I feel it should return 0, and not 1. For that to return zero, it would also be necessary for SELECT 2+2 to return zero rows. Which would be consistent with some views of the universe, but not particularly useful. Another counterexample is regression=# select sum(42); sum - 42 (1 row) which by your argument would need to return NULL, since that would be SUM's result over zero rows. 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] pgcrypto seeding problem when ssl=on
On Fri, Dec 21, 2012 at 10:27 PM, Noah Misch n...@leadboat.com wrote: How about instead calling RAND_cleanup() after each backend fork? Attached is a patch that adds RAND_cleanup() to fork_process(). That way all forked processes start with fresh state. This should make sure the problem does not happen in any processes forked by postmaster. Please backpatch. ... Alternative is to put RAND_cleanup() to BackendInitialize() so only new backends start with fresh state. Another alternative is to put RAND_cleanup() after SSL_accept(), that way core code sees no change, but other OpenSSL users in backend operate securely. -- marko rand_cleanup.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] pgcrypto seeding problem when ssl=on
Marko Kreen mark...@gmail.com writes: On Fri, Dec 21, 2012 at 10:27 PM, Noah Misch n...@leadboat.com wrote: How about instead calling RAND_cleanup() after each backend fork? Attached is a patch that adds RAND_cleanup() to fork_process(). I remain unconvinced that this is the best solution. Anybody else have an opinion? 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] Wide area replication postgres 9.1.6 slon 2.1.2 large table failure.
Tory M Blue wrote: Postgres 9.1.4 slon 2.1.1 -and- Postgres 9.1.6 slon 2.1.2 If it is possible, it never hurts to rule out bugs for which fixes are already available in production releases: http://www.postgresql.org/support/versioning/ Symptoms, slon immediately dies after transferring the biggest table in the set Dies? What does that mean, exactly? 1224459-2013-01-11 14:21:10 PST CONFIG remoteWorkerThread_1: 5760.913 seconds to copy table cls.listings 1224560-2013-01-11 14:21:10 PST CONFIG remoteWorkerThread_1: copy table cls.customers 1224642-2013-01-11 14:21:10 PST CONFIG remoteWorkerThread_1: Begin COPY of table cls.customers 1224733-2013-01-11 14:21:10 PST ERROR remoteWorkerThread_1: select _admissioncls.copyFields(8); --- this has the proper data 1224827:2013-01-11 14:21:10 PST WARN remoteWorkerThread_1: data copy for set 1 failed 1 times - sleep 15 seconds What, specifically, can cause those that set of messages from Slony? I get no errors in the postgres logs, there is no network disconnect and since I can do a copy over the wire that completes, I'm at a loss. I don't know what to look at, what to look for or what to do. Obviously this is the wrong place to slon issues. It is hard to make much of a guess without more data. I recommend looking over this page for ideas: http://wiki.postgresql.org/wiki/Guide_to_reporting_problems ... but in particular I think grabbing the contents of pg_stat_activity (using a superuser login) and pg_locks as soon as you feel it has died will be helpful. -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] pgcrypto seeding problem when ssl=on
On Sun, Jan 13, 2013 at 05:46:12PM -0500, Tom Lane wrote: Marko Kreen mark...@gmail.com writes: On Fri, Dec 21, 2012 at 10:27 PM, Noah Misch n...@leadboat.com wrote: How about instead calling RAND_cleanup() after each backend fork? Attached is a patch that adds RAND_cleanup() to fork_process(). I remain unconvinced that this is the best solution. Anybody else have an opinion? I'd describe it as the clearly-secure solution. The biggest hazard I see is the drain on system entropy. A system having only a blocking /dev/random could suddenly find itself entropy-exhausted after installing the minor upgrade. Backends could block waiting for system entropy to accumulate. That's not directly a problem on Linux. However, if other programs on the system read from /dev/random, the pressure from PostgreSQL's /dev/urandom usage may make those programs wait longer for entropy. I'm also comfortable with the security of Marko's original proposal, modulo it happening in each backend shortly after fork, not merely in pgcrypto. OpenSSL's ssl module uses a similar method, and one of the papers I cited describes it. (If anything, OpenSSL is less cautious. It uses time(), not gettimeofday().) The performance characteristics of this approach are easier to guess: one system call if we use MyProcPid + gettimeofday(), zero if we use MyProcPid + MyStartTime. You proposed mixing gettimeofday() into the postmaster's entropy pool after each fork. I wouldn't be too concerned if we did it that way, but my quick literature review did not turn up any similar ideas. Given that this is strictly more expensive than the previous method, I don't recommend it. Overall, I'd recommend mixing in MyProcPid and MyStartTime after each fork. nm -- 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] COPY .. COMPRESSED
Greetings,
Attached is a patch to add a 'COMPRESSED' option to COPY which will
cause COPY to expect a gzip'd file on input and which will output a
gzip'd file on output. Included is support for backend COPY, psql's
\copy, regression tests for both, and documentation.
On top of this I plan to submit a trivial patch to add support for
this to file_fdw, allowing creation of FDW tables which operate
directly on compressed files (including CSVs, which is what I need
this patch for).
I've also begun working on a patch to allow this capability to be used
through pg_dump/pg_restore which would reduce the bandwidth used
between the client and the server for backups and restores. Ideally,
one would also be able to use custom format dumps, with compression,
even if the client-side pg_dump/pg_restore wasn't compiled with zlib
support.
Thanks,
Stephen
colordiff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
new file mode 100644
index 6a0fabc..5c58dd2
*** a/doc/src/sgml/ref/copy.sgml
--- b/doc/src/sgml/ref/copy.sgml
*** COPY { replaceable class=parameterta
*** 38,43
--- 38,44
DELIMITER 'replaceable class=parameterdelimiter_character/replaceable'
NULL 'replaceable class=parameternull_string/replaceable'
HEADER [ replaceable class=parameterboolean/replaceable ]
+ COMPRESSED [ replaceable class=parameterboolean/replaceable ]
QUOTE 'replaceable class=parameterquote_character/replaceable'
ESCAPE 'replaceable class=parameterescape_character/replaceable'
FORCE_QUOTE { ( replaceable class=parametercolumn_name/replaceable [, ...] ) | * }
*** COPY { replaceable class=parameterta
*** 254,259
--- 255,271
/para
/listitem
/varlistentry
+
+varlistentry
+ termliteralCOMPRESSED/literal/term
+ listitem
+ para
+ Specifies that the file contents are compressed using zlib. On input,
+ the data should be compressed with zlib (eg: using gzip). On output,
+ the resulting data will be the compressed contents of the table.
+ /para
+ /listitem
+/varlistentry
varlistentry
termliteralQUOTE/literal/term
colordiff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
new file mode 100644
index abd82cf..1f394de
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***
*** 19,24
--- 19,27
#include sys/stat.h
#include netinet/in.h
#include arpa/inet.h
+ #ifdef HAVE_LIBZ
+ #include zlib.h
+ #endif
#include access/heapam.h
#include access/htup_details.h
***
*** 59,66
typedef enum CopyDest
{
COPY_FILE, /* to/from file */
COPY_OLD_FE,/* to/from frontend (2.0 protocol) */
! COPY_NEW_FE /* to/from frontend (3.0 protocol) */
} CopyDest;
/*
--- 62,71
typedef enum CopyDest
{
COPY_FILE, /* to/from file */
+ COPY_GZFILE,/* to/from compressed file */
COPY_OLD_FE,/* to/from frontend (2.0 protocol) */
! COPY_NEW_FE,/* to/from frontend (3.0 protocol) */
! COPY_NEW_FE_COMPRESSED /* FE 3.0 protocol, compressed */
} CopyDest;
/*
*** typedef struct CopyStateData
*** 95,100
--- 100,110
/* low-level state data */
CopyDest copy_dest; /* type of copy source/destination */
FILE *copy_file; /* used if copy_dest == COPY_FILE */
+ #ifdef HAVE_LIBZ
+ gzFile copy_gzfile; /* used if copy_dest == COPY_GZFILE */
+ z_stream *zstrm; /* used if streaming compressed data */
+ char *zstrm_buf; /* used if streaming compressed data */
+ #endif
StringInfo fe_msgbuf; /* used for all dests during COPY TO, only for
* dest == COPY_NEW_FE in COPY FROM */
bool fe_eof; /* true if detected end of copy data */
*** typedef struct CopyStateData
*** 109,114
--- 119,125
List *attnumlist; /* integer list of attnums to copy */
char *filename; /* filename, or NULL for STDIN/STDOUT */
bool binary; /* binary format? */
+ bool compressed; /* compressed file? */
bool oids; /* include OIDs? */
bool freeze; /* freeze rows on loading? */
bool csv_mode; /* Comma Separated Value format? */
*** static bool CopyGetInt32(CopyState cstat
*** 320,325
--- 331,437
static void CopySendInt16(CopyState cstate, int16 val);
static bool CopyGetInt16(CopyState cstate, int16 *val);
+ #ifdef HAVE_LIBZ
+ /* Helper functions for working with zlib */
+ static void zstrm_init(CopyState cstate, bool is_from);
+ void *zstrm_palloc(void *curr_memctx, unsigned int num, unsigned int size);
+ void zstrm_pfree(void *opaque, void *addr);
+
+ /*
+ * Define some useful constants.
+ * We're just using the default windowBits, but we have to combine it
+ * with 16 to turn on gzip encoding, which we want to use.
+ */
+ #define DEF_WINDOW_BITS 15
+ #define GZIP_ENCODING 16
+ #define DEF_MEMLEVEL 8
+
+
Re: [HACKERS] Possible PANIC in PostPrepare_Locks
Heikki Linnakangas hlinnakan...@vmware.com writes: On 11.01.2013 04:16, Tom Lane wrote: Also, it looks like we'll need two code paths in PostPrepare_Locks to deal with the possibility that a conflicting entry already exists? I'm not sure this is possible, but I'm not sure it's not, either. If I understand this correctly, that would mean that someone else is holding a lock that conflicts with the lock the transaction-being-prepared holds. That shouldn't happen. After looking at it again I decided the case was impossible because there can be at most one PROCLOCK for any given lock among those held by our own PGPROC (else there's already duplicate keys in the proclock table); so we will create at most one PROCLOCK per lock for the new dummy PGPROC. Any collision would imply that the new PGPROC already held some of those locks, which it surely should not. So I've committed a patch in which the occurrence of any such collision will result in a PANIC, but out-of-memory failures are not possible. 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] COPY .. COMPRESSED
Stephen Frost sfr...@snowman.net writes: Attached is a patch to add a 'COMPRESSED' option to COPY which will cause COPY to expect a gzip'd file on input and which will output a gzip'd file on output. Included is support for backend COPY, psql's \copy, regression tests for both, and documentation. I don't think it's a very good idea to invent such a specialized option, nor to tie it to gzip, which is widely considered to be old news. There was discussion (and, I think, a patch in the queue) for allowing COPY to pipe into or out of an arbitrary shell pipe. Why would that not be enough to cover this use-case? That is, instead of a hard-wired capability, people would do something like COPY TO '| gzip file.gz'. Or they could use bzip2 or whatever struck their fancy. 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] count(*) of zero rows returns 1
On Sun, Jan 13, 2013 at 4:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: Gurjeet Singh singh.gurj...@gmail.com writes: Can somebody explain why a standalone count(*) returns 1? postgres=# select count(*); count --- 1 (1 row) The Oracle equivalent of that would be SELECT count(*) FROM dual. Does it make more sense to you thought of that way? For a user, Oracle's case makes perfect sense, since the command is querying a single-row table. In Postgres' case, there's nothing being queried, so the result's got to be either 0 or NULL. I agree it's an odd thing for someone to query, but I feel it should return 0, and not 1. For that to return zero, it would also be necessary for SELECT 2+2 to return zero rows. Which would be consistent with some views of the universe, but not particularly useful. Another counterexample is regression=# select sum(42); sum - 42 (1 row) which by your argument would need to return NULL, since that would be SUM's result over zero rows. Hmm.. Now that you put it that way, I agree it's a useful feature, or shall I say, a quirk with useful side effect. -- Gurjeet Singh http://gurjeet.singh.im/
Re: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Sunday, January 13, 2013 2:45 PM Boszormenyi Zoltan wrote: 2013-01-13 05:49 keltezéssel, Amit kapila írta: On Sunday, January 13, 2013 12:41 AM Tom Lane wrote: Boszormenyi Zoltan z...@cybertec.at writes: No, I mean the reaper(SIGNAL_ARGS) function in src/backend/postmaster/postmaster.c where your patch has this: *** a/src/backend/postmaster/postmaster.c --- b/src/backend/postmaster/postmaster.c *** *** 2552,2557 reaper(SIGNAL_ARGS) I have not been following this thread, but I happened to see this bit, and I have to say that I am utterly dismayed that anything like this is even on the table. This screams overdesign. We do not need a global lock file, much less postmaster-side cleanup. All that's needed is a suitable convention about temp file names that can be written and then atomically renamed into place. If we're unlucky enough to crash before a temp file can be renamed into place, it'll just sit there harmlessly. I can think of below ways to generate tmp file 1. Generate session specific tmp file name during operation. This will be similar to what is currently being done in OpenTemporaryFileinTablespace() to generate a tempfilename. 2. Generate global tmp file name. For this we need to maintain global counter. 3. Always generate temp file with same name postgresql.auto.conf.tmp, as at a time only one session is allowed to operate. What's wrong with mkstemp(config_dir/postgresql.auto.conf.XX) that returns a file descriptor for an already created file with a unique name? I think for Windows exactly same behavior API might not exist, we might need to use GetTempFileName. It will generate some different kind of name, which means cleanup also need to take care of different kind of names. So if this is right, I think it might not be very appropriate to use it. Please point me if I am wrong as I have not used it. Note that the working directory of PostgreSQL is $PGDATA so config_dir and files under that can be accessed with a relative path. But a cleanup somewhere is needed to remove the stale temp files. I think it's enough at postmaster startup. A machine that's crashing so often that the presence of the stale temp files causes out of disk errors would surely be noticed much earlier. In PostmasterMain(), we already call RemovePGTempFiles(); So I think we can delete this tmp file at that place in PostmasterMain(). I shall do this, if nobody raise objection about it. 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: [HACKERS] I s this a bug of spgist index in a heavy write condition?
at 2013-01-12 02:21, Tom Lane wrote: =?gb2312?B?wO66o8H6?= hailong...@qunar.commailto:hailong...@qunar.com writes: This time I will give you the contents of the table route_raw, the download link is https://www.box.com/s/yxa4yxo6rcb3dzeaefmz or http://dl.dropbox.com/u/203288/route_raw_spgist.sql.tar.gz . Thanks for the data, but I still can't reproduce the problem :-(. Can you confirm whether loading this dump into an empty database and running your test case (15 instances of that script) fails for you? regards, tom lane Yes,I do confirm that. I consider to try to do further test in order to reproduce it again in some way you can reproduce it simply. Thanks again Best Regards!
