Re: [HACKERS] Multiple psql -c / -f options
IMHO the current behavior is broken: decibel@decina:[17:46]~/pgsql/HEAD/i$bin/psql -c 'select 1' -c 'select 2' ?column? -- 2 (1 row) Another try with one -c but with similar results: sh> psql -c "SELECT 1; SELECT 'hello';" ?column? -- hello (1 row) sh> psql -V psql (PostgreSQL) 9.3.1 -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql tab completion for updatable foreign tables
On 17 October 2013 03:29, Peter Eisentraut wrote: > On Mon, 2013-07-08 at 16:04 +, Dean Rasheed wrote: >> There was concern that pg_relation_is_updatable() would end up opening >> every relation in the database, hammering performance. I now realise >> that these tab-complete queries have a limit (1000 by default) so I >> don't think this is such an issue. I tested it by creating 10,000 >> randomly named auto-updatable views on top of a table, and didn't see >> any performance problems. > > Even if performance isn't a problem, do we really want tab completion > interfering with table locking? That might be a step too far. > That's a good point. Currently it's calling pg_table_is_visible(), which is doing similar work opening each relation, but it isn't locking any of them. > Personally, I think this is too fancy anyway. I'd just complete all > views and foreign tables and be done with it. We don't inspect > permissions either, for example. This might be too confusing for users. > Yeah, I think you're probably right. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR : 'tuple concurrently updated'
On Wed, Oct 16, 2013 at 5:55 PM, Stéphan BEUZE
wrote:
> The following query is performed concurrently by two threads logged in with
> two different users:
>
> WITH raw_stat AS (
> SELECT
>host(client_addr) as client_addr,
>pid ,
>usename
> FROM
>pg_stat_activity
> WHERE
>usename = current_user
> )
> INSERT INTO my_stat(id, client_addr, pid, usename)
> SELECT
> nextval('mystat_sequence'), t.client_addr, t.pid, t.usename
> FROM (
> SELECT
> client_addr, pid, usename
> FROM
> raw_stat s
> WHERE
> NOT EXISTS (
>SELECT
> NULL
>FROM
> my_stat u
>WHERE
> current_date = u.creation
>AND
> s.pid = u.pid
>AND
> s.client_addr = u.client_addr
>AND
> s.usename = u.usename
> )
> ) t;
>
> From time to time, I get the following error: "tuple concurrently updated"
>
> I can't figure out what throw this error and why this error is thrown. Can
> you shed a light ?
I have tried by using this query in a loop of 5000 and run the loop
in 2 different connections with different users, but could not get the
error.
What I understood from sql statement is that it will insert new
rows when there are new/different connections, so simply running this
sql statement
from 2 connections might not insert any new rows.
a. Are there any new connections happening, how this table is
getting populated?
b. How did you concluded that above sql statement leads to error,
because this error doesn't seem to occur in path of above sql
statement.
c. Are there any other sql statements in connection where you see this error?
Can you explain a bit more about your scenario, so that this error
can be reproduced easily.
> ---
> Here is the sql definition of the table mystat.
>
> **mystats.sql**
>
> CREATE TABLE mystat
> (
> id bigint NOT NULL,
> creation date NOT NULL DEFAULT current_date,
>
> client_addr text NOT NULL,
> pid integer NOT NULL,
> usename name NOT NULL,
> CONSTRAINT statistiques_connexions_pkey PRIMARY KEY (id)
> )
> WITH (
> OIDS=FALSE
> );
Some comments about SQL statements:
a. table name provided as part of schema (mystat) is different
from one used in sql statement(my_stat)
b. definition of sequence mystat_sequence is missing, although it
doesn't seem to be necessary, but if you can provide the definition
you are using
then it will be better.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning recovery.conf into GUCs
On Wed, Oct 16, 2013 at 1:34 PM, Josh Berkus wrote:
> Jaime,
>> = Building =
>>
>> In pg_basebackup we have now 2 unused functions:
>> escapeConnectionParameter and escape_quotes. the only use of these
>> functions was when that tool created the recovery.conf file so they
>> aren't needed anymore.
>
> Except that we'll want 9.4's -R to do something, probably create a file
> called conf.d/replication.conf. Mind you, it won't need the same wonky
> quoting stuff.
>
Currently the patch uses -R to create the recovery trigger file
>> 1) the file to trigger recovery is now called standby.enabled. I know
>> this is because we use the file to also make the node a standby.
>> Now, reality is that the file doesn't make the node a standby but the
>> combination of this file (which starts recovery) + standby_mode=on.
>> so, i agree with the suggestion of naming the file: recovery.trigger
>>
>> 2) This patch removes the dual existence of recovery.conf: as a state
>> file and as a configuration file
>>
>> - the former is accomplished by changing the name of the file that
>> triggers recovery (from recovery.conf to standby.enabled)
>> - the latter is done by moving all parameters to postgresql.conf and
>> *not reading* recovery.conf anymore
>>
>> so, after applying this patch postgres don't use recovery.conf for
>> anything... except for complaining.
>> it's completely fair to say we are no longer using that file and
>> ignoring its existence, but why we should care if users want to use a
>> file with that name for inclusion in postgresql.conf or where they put
>> that hypotetic file?
>
> So this is a bit of a catch-22. If we allow the user to use a file
> named "recovery.conf" in $PGDATA, then the user may be unaware that the
> *meaning* of the file has changed, which can result in unplanned
> promotion of replicas after upgrade.
>
well, after upgrade you should do checks. and even if it happens,
after it happens once people will be aware of the change.
now, some suggestions were made to avoid the problem. 1) read the file
if exists last in the process of postgresql.conf, 2) add a GUC
indicating if it should read it and include it (not using it as a
trigger file). another one, 3) include in this release an
include_if_exists directive and give a warning if it sees the file
then include it, on next release remove the include_if_exists (at
least that way people will be warned before breaking compatibility)
please, keep in mind none of these suggestions include make the
recovery.conf file act as a trigger for recovery
> *on the other hand*, if we prevent creation of a configuration file
> named "recovery.conf", then we block efforts to write
> backwards-compatible management utilities.
>
and all tools and procedures that currently exists.
> AFAIK, there is no good solution for this, which is why it's taken so
> long for us to resolve the general issue. Given that, I think it's
> better to go for the breakage and all the warnings. If a user wants a
> file called recovery.conf, put it in the conf.d directory.
>
well, there should be good solutions... maybe we haven't thought them yet.
anyway, we can't postpone the decision forever. we need to make a
decision and stick with it otherwise this patch will be stalled N
releases for no good reason
> I don't remember, though: how does this patch handle PITR recovery?
>
exactly as it is now, if it sees the recovery trigger file, then it
starts ArchiveRecovery and after it finish delete the file (the only
difference) and increment the timeline
>> = Code & functionality =
>
>> + {"restore_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
>> + {"archive_cleanup_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
>> + {"recovery_end_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
>> + {"recovery_target_xid", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
>> + {"recovery_target_name", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
>> + {"recovery_target_time", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
>> + {"trigger_file", PGC_POSTMASTER, REPLICATION_STANDBY,
>>
>> Not sure about these ones
>>
>> + {"recovery_target_timeline", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
>> + {"primary_conninfo", PGC_POSTMASTER, REPLICATION_STANDBY,
>
> It would be really nice to change these on the fly; it would help a lot
> of issues with minor changes to replication config. I can understand,
> though, that the replication code might not be prepared for that.
>
well, archive_command can be changed right now with a SIGHUP so at
least that one shouldn't change... and i don't think most of these are
too different. even if we are not sure we can do this now and change
them as SIGHUP later
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mai
[HACKERS] Multiple psql -c / -f options
IMHO the current behavior is broken:
decibel@decina:[17:46]~/pgsql/HEAD/i$bin/psql -c 'select 1' -c 'select 2'
?column?
--
2
(1 row)
I would expect psql to either run both commands or throw an error.
What I'd personally prefer is that psql execute -c and -f (and arguably -v) in
the order they're encountered, within the same session. I realize you can get
the same behavior by creating a .sql file, but for simple needs that's
sometimes more hassle than it's worth.
If we don't want to support that, we should throw an error if we encounter more
than one -c|-f. Not doing so allows for subtle, silent breakage.
Related to this, there's a bunch of other options that should only be allowed
once (ie: -d).
BTW, why do we special-case -? and -V at the top of main?
if (argc > 1)
{
if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") ==
0)
{
usage();
exit(EXIT_SUCCESS);
}
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V")
== 0)
{
showVersion();
exit(EXIT_SUCCESS);
}
}
--
Jim C. Nasby, Data Architect j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net
--
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] signed vs. unsigned in TYPEALIGN (was Re: space reserved for WAL record does not match what was written: panic on windows)
On 2013-10-17 18:04:34 -0400, Noah Misch wrote: > On Thu, Oct 17, 2013 at 08:27:01PM +0200, Andres Freund wrote: > > On 2013-10-17 12:33:45 -0400, Noah Misch wrote: > > > > 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? > > > > (Note that Size is just a typedef for size_t, in c.h) > > > > > > C99 doesn't require it, but I have never heard of a platform where it is > > > false. sizeof(intptr_t) > sizeof(size_t) systems have existed. > > > > Either way, both have to be at least 4byte on 32bit platforms and 8byte > > on 64bit ones. So I as well think we're good. > > C99 does not have concepts like "32bit platform" and "64bit platform", so it > cannot make such a constraint. Nonetheless, I agree we're good with respect > to implementations actually worth anticipating. But afaik we indirectly require either 4 or 8 byte pointers or in configure. And we have a requirement for non-segmented memory afaics. So both size_t and intptr_t have to be big enough to store a pointer. Which in turn implies that they have to be at least 4/8 bytes. > Having said that, changing the ancient macros to use uintptr_t does have the > advantage you mention, and I'm failing to think of a disadvantage. +1 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
[HACKERS] signed vs. unsigned in TYPEALIGN (was Re: space reserved for WAL record does not match what was written: panic on windows)
On Thu, Oct 17, 2013 at 08:27:01PM +0200, Andres Freund wrote: > On 2013-10-17 12:33:45 -0400, Noah Misch wrote: > > > 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? > > > (Note that Size is just a typedef for size_t, in c.h) > > > > C99 doesn't require it, but I have never heard of a platform where it is > > false. sizeof(intptr_t) > sizeof(size_t) systems have existed. > > Either way, both have to be at least 4byte on 32bit platforms and 8byte > on 64bit ones. So I as well think we're good. C99 does not have concepts like "32bit platform" and "64bit platform", so it cannot make such a constraint. Nonetheless, I agree we're good with respect to implementations actually worth anticipating. > > > 2. If intptr_t is a signed type, as it appears to be, and size_t is an > > > unsigned type, as I believe it to be, then is it safe to use the > > > macros written for the signed type with a value of the unsigned type? > > > Off-hand I can't see a problem there, but I'm not certain I'm not > > > missing something. > > > > Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc(). > > Looping back to my question above, I think it doesn't matter (on a two's > > complement system) whether the macro uses a signed type or an unsigned type. > > It changes the type of the result; that's all. Nonetheless, we should be > > consistent about signedness between the regular and 64-bit macro variants. > > I am not all that comfortable on relying on 2s complement here. Maybe we > want to compile without -fwrapv one day which would make signed overflow > undefined again. I don't think there are too many situations where the > compiler would have the required knowledge to optimize stuff away, > but... Here is my position statement on that issue: http://www.postgresql.org/message-id/20130220025819.gb6...@tornado.leadboat.com > So I vote for only using unsigned arithmetic. I don't see anything > preventing that? TYPEALIGN has used signed math since the dawn of history, and TYPEALIGN64 departed from that precedent without comment. That led me to think of the situation as prompting a fix for the oversight in TYPEALIGN64: --- a/src/include/c.h +++ b/src/include/c.h @@ -560,3 +560,3 @@ typedef NameData *Name; #define TYPEALIGN64(ALIGNVAL,LEN) \ - (((uint64) (LEN) + ((ALIGNVAL) - 1)) & ~((uint64) ((ALIGNVAL) - 1))) + (((int64) (LEN) + ((ALIGNVAL) - 1)) & ~((int64) ((ALIGNVAL) - 1))) Having said that, changing the ancient macros to use uintptr_t does have the advantage you mention, and I'm failing to think of a disadvantage. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing old ports and architectures
On 10/17/13 12:45 PM, Robert Haas wrote: > The attached patch, which I propose to apply relatively soon if nobody > objects, removes the IRIX port. +1 -- 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] pg_sleep(interval)
On 10/17/2013 06:59 PM, Josh Berkus wrote: > Our project has a serious, chronic problem with giving new > patch-submitters a bad experience, and this patch is a good example of > that. The ultimate result is that people go off to contribute to other > projects where submissions are easier and the rules for what gets > accepted are relatively transparent. That may be true, but it depends on the contributor. I would much rather be told that my contribution is not up to snuff than what happened on another project I recently tried to contribute to for the first time. A parser refactoring broke my code. I reported it and it was promptly fixed. When the fix came up for review, I said it needed a regression test to prevent it from happening again and I was told by the author that such a test would be "flimsy" and it went on to be committed (by that same guy) without one. I'm undecided whether I'll be contributing there any further. The rigor here makes me want to try and try again. -- Vik -- 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] pg_sleep(interval)
On 10/17/2013 08:36 PM, Kevin Grittner wrote:
> Josh Berkus wrote:
>
>> Our project has a serious, chronic problem with giving new
>> patch-submitters a bad experience, and this patch is a good
>> example of that.
> Perhaps; but it has also been an example of the benefits of having
> tight review.
FWIW, I agree. I have been impressed by the rigorous review process of
this project ever since I started following it.
> IMO, pg_sleep_for() and pg_sleep_until() are better
> than the initial proposal.
I agree here, as well. I was quite pleased with myself when I thought
of it, and I would not have thought of it had it not been for all the
pushback I received. Whether it goes in or not (I hope it does), I am
happy with the process.
> For one thing, since each accepts a
> specific type, it allows for cleaner syntax. These are not only
> unambiguous, they are easier to code and read than what was
> originally proposed:
>
> select pg_sleep_for('10 minutes');
> select pg_sleep_until('tomorrow 05:00');
These are pretty much exactly the examples I put in my documentation.
--
Vik
--
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] Auto-tuning work_mem and maintenance_work_mem
On Thu, Oct 17, 2013 at 7:22 AM, Robert Haas wrote: > On Wed, Oct 16, 2013 at 5:14 PM, Josh Berkus wrote: >> On 10/16/2013 01:25 PM, Andrew Dunstan wrote: >>> Andres has just been politely pointing out to me that my knowledge of >>> memory allocators is a little out of date (i.e. by a decade or two), and >>> that this memory is not in fact likely to be held for a long time, at >>> least on most modern systems. That undermines completely my reasoning >>> above. >> >> Except that Opensolaris and FreeBSD still have the old memory allocation >> behavior, as do older Linux kernels, many of which will remain in >> production for years. I have no idea what Windows' memory management >> behavior is. >> >> So this is a case of needing to know considerably more than the >> available RAM to determine a good setting. > > I agree, but I still think my previous proposal of increasing the > defaults for work_mem and maintenance_work_mem by 4X would serve many > more people well than it would serve poorly. I haven't heard anyone > disagree with that notion. Does anyone disagree? Should we do it? One source of hesitation for me is that I have a hunch that the stock assumptions that go into shared_buffers will probably change, possibly by a lot. For a lot of (I think very good-) reasons current policy is to set s_b to max 2GB. After we nail some of the outstanding contention issues and make other optimizations I expect that the optimal setting may be 50% of ram or higher. Point being: I like the idea of an initdb time setting that takes in the estimated amount of RAM that is going to be dedicated to the database. From there, we then estimate best settings for the various configurations (perhaps taking in extra hypothetical parameters to help that job along). So the anchor should not be s_b, but user specified. 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] Auto-tuning work_mem and maintenance_work_mem
On 10/17/2013 10:33 AM, Jeff Janes wrote: A lot. A whole lot, more than what most people have in production with more than that. You are forgetting a very large segment of the population who run... VMs. Why don't we just have 3 default config files: 2GB memory 4GB memory 8GB memory But what would go in each of those files? Once we agree on what would be in them, why not just have a continuous knob that does that same thing? Because we should set defaults, not optimized parameters. Workloads vary and we can reasonably say this is what we want BY DEFAULT for something but we can not reasonably say, "this is what will suit your needs". Once you get above 8GB of memory you are dealing with workloads that vary widely and will almost always need some kind of indvidual attention. However, 8GB and below, we can set reasonable defaults that allow a user to likely but possibly not worry about changing the conf. Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats -- 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] pg_sleep(interval)
Josh Berkus wrote:
> Our project has a serious, chronic problem with giving new
> patch-submitters a bad experience, and this patch is a good
> example of that.
Perhaps; but it has also been an example of the benefits of having
tight review. IMO, pg_sleep_for() and pg_sleep_until() are better
than the initial proposal. For one thing, since each accepts a
specific type, it allows for cleaner syntax. These are not only
unambiguous, they are easier to code and read than what was
originally proposed:
select pg_sleep_for('10 minutes');
select pg_sleep_until('tomorrow 05:00');
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: space reserved for WAL record does not match what was written: panic on windows
On 2013-10-17 12:33:45 -0400, Noah Misch wrote: > > But if we're bent on minimizing the use of 64-bit arithmetic on 32-bit > > systems, then presumably I should instead go back and retrofit that > > patch to use Size rather than uint64 to represent the size of a > > segment. But then I have two concerns: > > I'm not bent on _minimizing_ use of 64-bit arithmetic on 32-bit systems, but I > disfavor an addition of such usage rippling through various hot paths of the > system indiscriminately. Making a design choice to use *ALIGN64 in a > particular module doesn't bother me that way. I am fine with that as well. It seems unlikely that parallel sort or whatever will be as hot as tuple deforming code on systems where 32bit arithmetic is slow. > > 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? > > (Note that Size is just a typedef for size_t, in c.h) > > C99 doesn't require it, but I have never heard of a platform where it is > false. sizeof(intptr_t) > sizeof(size_t) systems have existed. Either way, both have to be at least 4byte on 32bit platforms and 8byte on 64bit ones. So I as well think we're good. > > 2. If intptr_t is a signed type, as it appears to be, and size_t is an > > unsigned type, as I believe it to be, then is it safe to use the > > macros written for the signed type with a value of the unsigned type? > > Off-hand I can't see a problem there, but I'm not certain I'm not > > missing something. > > Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc(). > Looping back to my question above, I think it doesn't matter (on a two's > complement system) whether the macro uses a signed type or an unsigned type. > It changes the type of the result; that's all. Nonetheless, we should be > consistent about signedness between the regular and 64-bit macro variants. I am not all that comfortable on relying on 2s complement here. Maybe we want to compile without -fwrapv one day which would make signed overflow undefined again. I don't think there are too many situations where the compiler would have the required knowledge to optimize stuff away, but... So I vote for only using unsigned arithmetic. I don't see anything preventing that? 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] Re: space reserved for WAL record does not match what was written: panic on windows
On Thu, Oct 17, 2013 at 12:33 PM, Noah Misch wrote: >> But if we're bent on minimizing the use of 64-bit arithmetic on 32-bit >> systems, then presumably I should instead go back and retrofit that >> patch to use Size rather than uint64 to represent the size of a >> segment. But then I have two concerns: > > I'm not bent on _minimizing_ use of 64-bit arithmetic on 32-bit systems, but I > disfavor an addition of such usage rippling through various hot paths of the > system indiscriminately. Making a design choice to use *ALIGN64 in a > particular module doesn't bother me that way. OK. Well that'd probably be simpler from my point of view but I'm not 100% sure. If we're going to allow that, then I think we need 64-bit versions of all of the alignment macros. Anyone think that's a bad idea? >> 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? >> (Note that Size is just a typedef for size_t, in c.h) > > C99 doesn't require it, but I have never heard of a platform where it is > false. sizeof(intptr_t) > sizeof(size_t) systems have existed. That's good, I think. Because if it weren't true then we'd potentially need three versions of this macro: one for intptr_t, another for size_t, and a third for uint64. >> 2. If intptr_t is a signed type, as it appears to be, and size_t is an >> unsigned type, as I believe it to be, then is it safe to use the >> macros written for the signed type with a value of the unsigned type? >> Off-hand I can't see a problem there, but I'm not certain I'm not >> missing something. > > Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc(). > Looping back to my question above, I think it doesn't matter (on a two's > complement system) whether the macro uses a signed type or an unsigned type. > It changes the type of the result; that's all. Nonetheless, we should be > consistent about signedness between the regular and 64-bit macro variants. So, are you proposing using uintptr_t there instead of intptr_t? Or what? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 1:10 PM, Josh Berkus wrote: > On 10/17/2013 10:01 AM, Robert Haas wrote: >> But if you're asking my opinion, I think doing it on the function >> level is a whole lot better and easier to get right. A flag like the >> one I mentioned here can be set for one particular function with the >> absolute certainty that behavior will not change for any function with >> some other name. That type of surety is pretty much impossible to get >> with casts. > > The other argument for doing it at the function level is that we could > then expose it to users, who could use it to manage their own overloaded > functions. We would NOT want to encourage users to mess with cast > precedence, because it would be impossible for them to achieve their > desired result that way. > > On the other hand, prioritization at the function level likely wouldn't > help us with operators at all, because there the cast has to be chosen > before we choose a function. So if we pursued the function route, then > we'd eventually want to add a "preferred" flag for operators too. Which > would be a lot more trouble, because it would affect the planner, but at > least that would be a seperate step. Actually the operator resolution code is very much parallel to the function resolution code. I am quite sure in Advanced Server we only needed to add proisweak to handle both cases; unless I'm quite mistaken, we did not add oprisweak. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 12:59 PM, Josh Berkus wrote:
> Now, I do think the argument of "we don't really need pg_sleep(interval)
> because it's trivial to do yourself" has some merit, and that would be a
> good reason to argue acceptance or not. However, to date that has not
> been the topic of discussion.
I've made that exact argument several times on this thread. For example:
http://www.postgresql.org/message-id/CA+TgmobKneq=f9e8tzywg6haotzxozpvjqh14mpb9f+xlv6...@mail.gmail.com
I've been focusing on the backward compatibility issue mostly BECAUSE
I don't think the feature has much incremental value. If logical
replication or parallel query required breaking pg_sleep('42'), I
wouldn't be objecting. I'm sorry if that wasn't clear, and I further
apologize if you think I'm being too hard on a new patch submitter.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
On Thu, Oct 17, 2013 at 9:03 AM, Joshua D. Drake wrote: > > On 10/17/2013 08:55 AM, Kevin Grittner wrote: > >> >> Robert Haas wrote: >> >> I still think my previous proposal of increasing the defaults for >>> work_mem and maintenance_work_mem by 4X would serve many more >>> people well than it would serve poorly. I haven't heard anyone >>> disagree with that notion. Does anyone disagree? Should we do >>> it? >>> >> >> I think that it makes sense to do that. Those are still reasonable >> defaults for a machine with 2GB of RAM, maybe even with less. >> We're talking about putting this only in a release that will come >> out in 2014. How many machines used for a database server that new >> will have less than that? >> > > A lot. A whole lot, more than what most people have in production with > more than that. You are forgetting a very large segment of the population > who run... VMs. > > Why don't we just have 3 default config files: > > 2GB memory > 4GB memory > 8GB memory > But what would go in each of those files? Once we agree on what would be in them, why not just have a continuous knob that does that same thing? Would your suggestion for the 2GB file have work_mem be at least 4 MB? Cheers, Jeff
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 10:01 AM, Robert Haas wrote: > But if you're asking my opinion, I think doing it on the function > level is a whole lot better and easier to get right. A flag like the > one I mentioned here can be set for one particular function with the > absolute certainty that behavior will not change for any function with > some other name. That type of surety is pretty much impossible to get > with casts. The other argument for doing it at the function level is that we could then expose it to users, who could use it to manage their own overloaded functions. We would NOT want to encourage users to mess with cast precedence, because it would be impossible for them to achieve their desired result that way. On the other hand, prioritization at the function level likely wouldn't help us with operators at all, because there the cast has to be chosen before we choose a function. So if we pursued the function route, then we'd eventually want to add a "preferred" flag for operators too. Which would be a lot more trouble, because it would affect the planner, but at least that would be a seperate step. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
On 10/17/2013 09:49 AM, Robert Haas wrote: A lot. A whole lot, more than what most people have in production with more than that. You are forgetting a very large segment of the population who run... VMs. That's true, but are you actually arguing for keeping work_mem at 1MB? Even on a VM with only 1GB of RAM, work_mem=4MB is not going to cause any problems unless you're also trying to service a large number of simultaneous connections. And if you're doing that, you probably need to rethink something anyway. No. I am arguing for the multiple config file option. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats -- 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] pg_sleep(interval)
On Thu, Oct 17, 2013 at 12:45 PM, Josh Berkus wrote: >> Obviously, the implicit casts are not for PostgreSQL and would be >> rightly rejected here, but I am not sure that the ability to prefer >> one function or operator over others in an overloading situation is >> such a bad idea. So far, our internal testing seems to show that it >> works well and doesn't break things. > > Hmmm. Is this better to do on the cast level or the function level? > For the case discussed, it would be sufficient to have a way to mark a > particular function signature as "preferred" in cases of ambiguity, and > that would be a lot less likely to have side effects. Mind you, fixing > the cast in general would fix far more annoying cases, but I also see it > as something which would be very hard to get correct ... This is of course to some degree a matter of opinion. The ankle bone is connected to the leg bone, and the leg bone is connected to the knee bone, and all that. It's very legitimate to think that we can change the system in a variety of places to fix any given problem. But if you're asking my opinion, I think doing it on the function level is a whole lot better and easier to get right. A flag like the one I mentioned here can be set for one particular function with the absolute certainty that behavior will not change for any function with some other name. That type of surety is pretty much impossible to get with casts. It's also not clear to me that the rules are logically related to the input data type. We might prefer to choose pg_sleep(double) over pg_sleep(interval) on backwards-compatibility grounds, but some other function might be in the opposite situation. You can't really get a lot of fine-grained control with casting here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Fabien, > My experience at trying to pass minor patches shows that the basic > behavior is conservatism. Maybe this is necessary to the stability of > the project, but this is really annoying for pretty small changes on > side tools, and I think that it tends to over-conservatism and ampers > good will. You have to argue a lot about trivial things. My ratio for > passing very minor patches on pgbench for instance is 1:3 or worst, 1 > unit programming and testing versus 3 units writing mails to argue about > this and that. Maybe the ratio is better for big important patches. Your > patch is quite representative, 1 line of trivial code, a few lines of > tests and docs, and how many lines and time at writing mails? This is, personally, the *entire* reason I've been arguing for this patch. I see the arguments against it as being a case of unnecessary conservatism, and cynically realize that if a well-known major contributor had proposed it, the patch would be committed already. Our project has a serious, chronic problem with giving new patch-submitters a bad experience, and this patch is a good example of that. The ultimate result is that people go off to contribute to other projects where submissions are easier and the rules for what gets accepted are relatively transparent. Personally, I don't care about pg_sleep(interval) really. But I do care that a minor contributor be able to submit and have accepted a patch of clear general usability, and not get it rejected on the basis of "it might break something for someone even though we don't have a clear idea who". Now, I do think the argument of "we don't really need pg_sleep(interval) because it's trivial to do yourself" has some merit, and that would be a good reason to argue acceptance or not. However, to date that has not been the topic of discussion. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
On Thu, Oct 17, 2013 at 12:03 PM, Joshua D. Drake wrote: > On 10/17/2013 08:55 AM, Kevin Grittner wrote: >> Robert Haas wrote: >> >>> I still think my previous proposal of increasing the defaults for >>> work_mem and maintenance_work_mem by 4X would serve many more >>> people well than it would serve poorly. I haven't heard anyone >>> disagree with that notion. Does anyone disagree? Should we do >>> it? >> >> >> I think that it makes sense to do that. Those are still reasonable >> defaults for a machine with 2GB of RAM, maybe even with less. >> We're talking about putting this only in a release that will come >> out in 2014. How many machines used for a database server that new >> will have less than that? > > A lot. A whole lot, more than what most people have in production with more > than that. You are forgetting a very large segment of the population who > run... VMs. That's true, but are you actually arguing for keeping work_mem at 1MB? Even on a VM with only 1GB of RAM, work_mem=4MB is not going to cause any problems unless you're also trying to service a large number of simultaneous connections. And if you're doing that, you probably need to rethink something anyway. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
JD, > A lot. A whole lot, more than what most people have in production with > more than that. You are forgetting a very large segment of the > population who run... VMs. Actually, even a "mini" AWS instance has 1GB of RAM. And nobody who uses a "micro" is going to expect it to perform well under load. I think it's completely reasonable to tell people running on < 1GB of ram to tune PostgreSQL "down". 4MB work_mem / 64MB maint_work_mem still works fine at 1GB. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert, > Obviously, the implicit casts are not for PostgreSQL and would be > rightly rejected here, but I am not sure that the ability to prefer > one function or operator over others in an overloading situation is > such a bad idea. So far, our internal testing seems to show that it > works well and doesn't break things. Hmmm. Is this better to do on the cast level or the function level? For the case discussed, it would be sufficient to have a way to mark a particular function signature as "preferred" in cases of ambiguity, and that would be a lot less likely to have side effects. Mind you, fixing the cast in general would fix far more annoying cases, but I also see it as something which would be very hard to get correct ... -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing old ports and architectures
On Wed, Oct 16, 2013 at 1:25 PM, Stefan Kaltenbrunner wrote: > On 10/16/2013 07:04 PM, Robert Haas wrote: >> On Sat, Oct 12, 2013 at 8:46 PM, Andres Freund >> wrote: >>> I think we should remove support the following ports: >>> - IRIX >>> - UnixWare >>> - Tru64 >> >> According to http://en.wikipedia.org/wiki/IRIX, IRIX has been >> officially retired. The last release of IRIX was in 2006 and support >> will end in December of 2013. Therefore, it will be unsupported by >> the time PostgreSQL 9.4 is released. >> >> According to http://en.wikipedia.org/wiki/UnixWare, UnixWare is not >> dead, although there have been no new releases in 5 years. >> >> According to http://en.wikipedia.org/wiki/Tru64_UNIX, Tru64 has been >> officially retired. Support ended in December, 2012. This seems safe >> to remove. >> >> So I vote for removing IRIX and Tru64 immediately, but I'm a little >> more hesitant about shooting UnixWare, since it's technically still >> supported. >> >>> Neither of those are relevant. > > agreed The attached patch, which I propose to apply relatively soon if nobody objects, removes the IRIX port. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company remove-irix-port.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: space reserved for WAL record does not match what was written: panic on windows
On Thu, Oct 17, 2013 at 08:39:56AM -0400, Robert Haas wrote: > On Fri, Oct 11, 2013 at 1:14 AM, Noah Misch wrote: > > On Thu, Oct 10, 2013 at 03:23:30PM +0200, Andres Freund wrote: > >> On 2013-10-10 08:59:47 -0400, Robert Haas wrote: > >> > I'd be inclined to make the computation unconditionally 64-bit. I > >> > doubt the speed penalty is enough to worry about, and I think we're > >> > going to have more and more cases where optimizing for 32-bit > >> > platforms is just not the right decision. > >> > >> MAXALIGN is used in several of PG's hottest functions in many > >> scenarios. att_align_nominal is used in slot_deform_tuple, > >> heap_deform_tuple, nocachegetattr, etc. So I don't think that's viable > >> yet. At least not with much more benefit than this... > > > > Agreed. Besides performance, aligning a wider-than-pointer value is an > > unusual need; authors should think thrice before doing that. I might have > > even defined the MAXALIGN64 macro in xlog.c rather than a core header. > > > > Incidentally, why does MAXALIGN64 use unsigned math while MAXALIGN uses > > signed > > math? > > Well, if this is the consensus, then I think the dynamic shared memory > patch may need some revision. In that patch, I used uint64 to > represent the size of the dynamic shared memory segment, sort of on > the theory that we were going to use this to be allocating big chunks > of dynamic shared memory for stuff like parallel sort. In follow-on > patches I'm currently developing to actually do stuff with dynamic > shared memory, this results in extensive use of MAXALIGN64, and it > really kind of looks like it wants the whole set of alignment macros, > not just that one. So option one is to leave the dsm code alone and > add the rest of the macros. > > But if we're bent on minimizing the use of 64-bit arithmetic on 32-bit > systems, then presumably I should instead go back and retrofit that > patch to use Size rather than uint64 to represent the size of a > segment. But then I have two concerns: I'm not bent on _minimizing_ use of 64-bit arithmetic on 32-bit systems, but I disfavor an addition of such usage rippling through various hot paths of the system indiscriminately. Making a design choice to use *ALIGN64 in a particular module doesn't bother me that way. > 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? > (Note that Size is just a typedef for size_t, in c.h) C99 doesn't require it, but I have never heard of a platform where it is false. sizeof(intptr_t) > sizeof(size_t) systems have existed. > 2. If intptr_t is a signed type, as it appears to be, and size_t is an > unsigned type, as I believe it to be, then is it safe to use the > macros written for the signed type with a value of the unsigned type? > Off-hand I can't see a problem there, but I'm not certain I'm not > missing something. Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc(). Looping back to my question above, I think it doesn't matter (on a two's complement system) whether the macro uses a signed type or an unsigned type. It changes the type of the result; that's all. Nonetheless, we should be consistent about signedness between the regular and 64-bit macro variants. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
On 10/17/2013 08:55 AM, Kevin Grittner wrote: Robert Haas wrote: I still think my previous proposal of increasing the defaults for work_mem and maintenance_work_mem by 4X would serve many more people well than it would serve poorly. I haven't heard anyone disagree with that notion. Does anyone disagree? Should we do it? I think that it makes sense to do that. Those are still reasonable defaults for a machine with 2GB of RAM, maybe even with less. We're talking about putting this only in a release that will come out in 2014. How many machines used for a database server that new will have less than that? A lot. A whole lot, more than what most people have in production with more than that. You are forgetting a very large segment of the population who run... VMs. Why don't we just have 3 default config files: 2GB memory 4GB memory 8GB memory Have initdb detect how much memory is available on the machine in TOTAL and pick the most appropriate. Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats -- 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] Auto-tuning work_mem and maintenance_work_mem
Robert Haas wrote: > I still think my previous proposal of increasing the defaults for > work_mem and maintenance_work_mem by 4X would serve many more > people well than it would serve poorly. I haven't heard anyone > disagree with that notion. Does anyone disagree? Should we do > it? I think that it makes sense to do that. Those are still reasonable defaults for a machine with 2GB of RAM, maybe even with less. We're talking about putting this only in a release that will come out in 2014. How many machines used for a database server that new will have less than that? -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Rework SSL renegotiation code
Vik Fearing wrote: > On 09/23/2013 10:51 PM, Alvaro Herrera wrote: > > + /* are we in the middle of a renegotiation? */ > > + static bool in_ssl_renegotiation = false; > > Since this was committed, I'm getting the following warning: > > be-secure.c:105:13: warning: ‘in_ssl_renegotiation’ defined but not used > [-Wunused-variable] Jaime Casanova wrote: > Shouldn't this new variable be declared inside the #ifdef USE_SSL block? > My compiler is giving me a warning for the unused variable Yep, thanks guys. Just pushed a fix. -- Á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: [HACKERS] [PATCH] hstore_to_json: empty hstores must return empty json objects
On 10/17/2013 10:23 AM, Alvaro Herrera wrote:
Oskari Saarenmaa wrote:
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
Yeah. I'm going to fix this this morning.
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] [PATCH] hstore_to_json: empty hstores must return empty json objects
On 17/10/13 17:23, Alvaro Herrera wrote:
Oskari Saarenmaa wrote:
@@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
if (count == 0)
{
- out = palloc(1);
- *out = '\0';
+ out = palloc(3);
+ memcpy(out, "{}", 3);
PG_RETURN_TEXT_P(cstring_to_text(out));
}
Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
I'm not too familiar with PG internals and just modified this to work
like it did before, but it looks like the extra allocation is indeed
unnecessary.
I can post a new patch to make this use cstring_to_text_with_len("{}",
2) (to avoid an unnecessary strlen call) or you can just make the change
and push the modified version.
Thanks,
Oskari
--
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] hstore_to_json: empty hstores must return empty json objects
Oskari Saarenmaa wrote:
> @@ -1241,8 +1241,8 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
>
> if (count == 0)
> {
> - out = palloc(1);
> - *out = '\0';
> + out = palloc(3);
> + memcpy(out, "{}", 3);
> PG_RETURN_TEXT_P(cstring_to_text(out));
> }
Can't you just PG_RETURN_TEXT_P(cstring_to_text("{}")) ?
--
Á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: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/13 8:06 AM, Robert Haas wrote: > Actually, this could be fixed by having a way to declare one of the > overloaded functions as the preferred option and resolving ambiguous > calls in favor of the highest-priority function. In fact, > EnterpriseDB has added just such an option to Advanced Server 9.3, and > it fixes several longstanding difficult choices between being > Oracle-compatible and being PostgreSQL-compatible; we're now more > compatible with both. If we had that option in PostgreSQL, we could > declare the existing function as the preferred option, add the new > one, and be pretty much assured of not breaking anything. That might be worth discussing. I'd prefer somehow getting rid of the unknown literals/type, but that's a different discussion. > However, I've learned from experience that submitting patches to > improve PostgreSQL's only-marginally-usable ambiguous function > resolution code is an exercise in futility. My last attempt ended > with a clear majority of people telling me that they liked failing to > call *the only function called foo* when confronted with a call that > was clearly intended to reference that function. As nearly as I can > tell, people like the idea of such unnecessary failures only in > theory. As soon as it comes to any practical case (like this one), > people start looking for workarounds. Right now there aren't any good > ones, and the reason for that is simple: we refuse to entertain adding > them. Well, that was a proposal to make things more loose, and it's reasonable to have issues with that. On the other hand, making things more strict is obviously prone to break existing code. So it's indeed difficult to make any changes either way. -- 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] pg_sleep(interval)
On Thu, Oct 17, 2013 at 9:51 AM, Alvaro Herrera wrote: > Robert Haas escribió: >> Actually, this could be fixed by having a way to declare one of the >> overloaded functions as the preferred option and resolving ambiguous >> calls in favor of the highest-priority function. In fact, >> EnterpriseDB has added just such an option to Advanced Server 9.3, and >> it fixes several longstanding difficult choices between being >> Oracle-compatible and being PostgreSQL-compatible; we're now more >> compatible with both. > > How does this work? In Advanced Server, we've added implicit casts between some data types between which PostgreSQL does not have implicit casts. We do this because Oracle implicitly casts between those data types, and having similar casts allows function calls that would succeed on Oracle to also succeed on Advanced Server. Unfortunately, it also renders some operators, particularly textanycat and anytextcat, ambiguous. In existing releases of Advanced Server, we handled that by removing those operators. This creates some breakage in edge cases: concatenation with text still works fine for the data types for which we added implicit casts to text, but for other data types it fails where it would have succeeded in PostgreSQL. In Advanced Server 9.3, we added a new pg_proc column called proisweak. When a function or operator call would otherwise be rejected as ambiguous, we check whether all but one of the remaining candidates are marked proisweak; if so, we select the non-weak candidate. Advanced Server 9.3 now marks the textanycat and anytextcat operators as weak rather than removing them; this allows type-with-an-implicit-cast-to-text || text to work, because we now have a way to prefer implicit cast + textcat to anytextcat. Obviously, the implicit casts are not for PostgreSQL and would be rightly rejected here, but I am not sure that the ability to prefer one function or operator over others in an overloading situation is such a bad idea. So far, our internal testing seems to show that it works well and doesn't break things. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Robert Haas escribió: > Actually, this could be fixed by having a way to declare one of the > overloaded functions as the preferred option and resolving ambiguous > calls in favor of the highest-priority function. In fact, > EnterpriseDB has added just such an option to Advanced Server 9.3, and > it fixes several longstanding difficult choices between being > Oracle-compatible and being PostgreSQL-compatible; we're now more > compatible with both. How does this work? -- Á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: [HACKERS] [PATCH] pg_sleep(interval)
Hello Vik,
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
Opinions?
I liked overloading...
Anyway, I have not looked at the patch in details, but both functions
seems useful at least if you need to sleep, and the solution is
reasonable.
I also like "pg_sleep_until('tomorrow');" that I guess would work, but I'm
not sure of any practical use case for this one. Do you have an example
where it makes sense?
--
Fabien.
--
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] hstore_to_json: empty hstores must return empty json objects
On 10/17/2013 08:20 AM, Robert Haas wrote: On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa wrote: hstore_to_json used to return an empty string for empty hstores, but that is not correct as an empty string is not valid json and calling row_to_json() on rows which have empty hstores will generate invalid json for the entire row. The right thing to do is to return an empty json object. Hmm. This sure looks like a bug to me. Anyone think otherwise? No, you're right. It was a thinko on my part. :-( 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] Unitialized Values in record_image_cmp
On 2013-10-17 08:16:58 -0500, Merlin Moncure wrote: > On Thu, Oct 17, 2013 at 8:01 AM, Andres Freund wrote: > > Hi, > > > > Valgrind tells me: > > ==442828== Conditional jump or move depends on uninitialised value(s) > > ==442828==at 0x80084F: record_image_cmp (rowtypes.c:1521) > > ==442828==by 0x801522: btrecordimagecmp (rowtypes.c:1839) > > ==442828==by 0x8C6604: comparison_shim (sortsupport.c:53) > > ==442828==by 0x64284D: ApplySortComparator (sortsupport.h:143) > > ==442828==by 0x642F3B: MJCompare (nodeMergejoin.c:412) > > ==442828==by 0x643A29: ExecMergeJoin (nodeMergejoin.c:1211) > > ==442828==by 0x62305A: ExecProcNode (execProcnode.c:453) > > ==442828==by 0x640800: ExecLimit (nodeLimit.c:91) > > ==442828==by 0x623135: ExecProcNode (execProcnode.c:500) > > ==442828==by 0x620E15: ExecutePlan (execMain.c:1472) > > ==442828==by 0x61EF94: standard_ExecutorRun (execMain.c:307) > > ==442828==by 0x61EE0B: ExecutorRun (execMain.c:255) > > > > I think Kevin got to it already. see: > http://postgresql.1045698.n5.nabble.com/Record-comparison-compiler-warning-td5774794.html At least that patch isn't applicable - there was an unitialized value at runtime, even using a cassert enabled build. 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] hstore_to_json: empty hstores must return empty json objects
On Thu, Oct 17, 2013 at 7:20 AM, Robert Haas wrote: > On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa wrote: >> hstore_to_json used to return an empty string for empty hstores, but >> that is not correct as an empty string is not valid json and calling >> row_to_json() on rows which have empty hstores will generate invalid >> json for the entire row. The right thing to do is to return an empty >> json object. > > Hmm. This sure looks like a bug to me. > > Anyone think otherwise? It is a bug. 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] Unitialized Values in record_image_cmp
On Thu, Oct 17, 2013 at 8:01 AM, Andres Freund wrote: > Hi, > > Valgrind tells me: > ==442828== Conditional jump or move depends on uninitialised value(s) > ==442828==at 0x80084F: record_image_cmp (rowtypes.c:1521) > ==442828==by 0x801522: btrecordimagecmp (rowtypes.c:1839) > ==442828==by 0x8C6604: comparison_shim (sortsupport.c:53) > ==442828==by 0x64284D: ApplySortComparator (sortsupport.h:143) > ==442828==by 0x642F3B: MJCompare (nodeMergejoin.c:412) > ==442828==by 0x643A29: ExecMergeJoin (nodeMergejoin.c:1211) > ==442828==by 0x62305A: ExecProcNode (execProcnode.c:453) > ==442828==by 0x640800: ExecLimit (nodeLimit.c:91) > ==442828==by 0x623135: ExecProcNode (execProcnode.c:500) > ==442828==by 0x620E15: ExecutePlan (execMain.c:1472) > ==442828==by 0x61EF94: standard_ExecutorRun (execMain.c:307) > ==442828==by 0x61EE0B: ExecutorRun (execMain.c:255) > I think Kevin got to it already. see: http://postgresql.1045698.n5.nabble.com/Record-comparison-compiler-warning-td5774794.html 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] [PATCH] pg_sleep(interval)
On 10/17/2013 02:42 PM, Robert Haas wrote:
> On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing wrote:
>> On 10/17/2013 10:03 AM, Fabien COELHO wrote:
>>> My guess is that it won't be committed if there is a single "but it
>>> might break one code or surprise one user somewhere in the universe",
>>> but I wish I'll be proven wrong. IMO, "returned with feedback" on a 1
>>> liner is really akin to "rejected".
>> I have attached here an entirely new patch (new documentation and
>> everything) that should please everyone. It no longer overloads
>> pg_sleep(double precision) but instead add two new functions:
>>
>> * pg_sleep_for(interval)
>> * pg_sleep_until(timestamp with time zone)
>>
>> Because it's no longer overloading the original pg_sleep, Robert's
>> ambiguity objection is no more.
>>
>> Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
>>
>> If people like this, I'll reject the current patch and add this one to
>> the next commitfest.
> I find that naming relatively elegant. However, you've got to
> schema-qualify every function and operator used in the definitions, or
> you're creating a search-path security vulnerability.
>
Good catch. Updated patch attached.
--
Vik
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 7586,7605 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT
! The following function is available to delay execution of the server
process:
pg_sleep(seconds)
pg_sleep makes the current session's process
sleep until seconds seconds have
elapsed. seconds is a value of type
double precision, so fractional-second delays can be specified.
For example:
SELECT pg_sleep(1.5);
--- 7586,7613
! The following functions are available to delay execution of the server
process:
pg_sleep(seconds)
+ pg_sleep_for(interval)
+ pg_sleep_until(timestamp with time zone)
pg_sleep makes the current session's process
sleep until seconds seconds have
elapsed. seconds is a value of type
double precision, so fractional-second delays can be specified.
+ pg_sleep_for is a convenience function for larger
+ sleep times specified as an interval.
+ pg_sleep_until is a convenience function for when
+ a specific wake-up time is desired.
For example:
SELECT pg_sleep(1.5);
+ SELECT pg_sleep_for('5 minutes');
+ SELECT pg_sleep_until('tomorrow 03:00');
***
*** 7608,7622 SELECT pg_sleep(1.5);
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
Make sure that your session does not hold more locks than necessary
! when calling pg_sleep. Otherwise other sessions
! might have to wait for your sleeping process, slowing down the entire
! system.
--- 7616,7632
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
+ In particular, pg_sleep_until is not guaranteed to
+ wake up exactly at the specified time, but it will not wake up any earlier.
Make sure that your session does not hold more locks than necessary
! when calling pg_sleep or its variants. Otherwise
! other sessions might have to wait for your sleeping process, slowing down
! the entire system.
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0
--- 3017,3026
DESCR("list all files in a directory");
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "701" _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ ));
DESCR("sleep for the specified time in seconds");
+ DATA(insert OID = 3179 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 "1186" _null_ _null_ _null_ _null_ "select pg_catalog.pg_sleep(extract(epoch from pg_catalog.now() operator(pg_catalog.+) $1) operator(pg_catalog.-) extract(epoch from pg_catalog.now()))" _null_ _null_ _null_ ));
+ DESCR("sleep for the specified interval");
+ DATA(insert OID = 3180 ( pg_sleep_until PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 "1184" _null_ _null_ _null_ _null_ "select pg_catalog.pg_sleep(extract(epoch from $1) operator(pg_catalog.-) extract(epoch from pg_catalog.now()))" _null_ _null_ _null_ ));
+ DESCR("sleep until the specified time");
DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 25 "16" _null_
[HACKERS] Unitialized Values in record_image_cmp
Hi, Valgrind tells me: ==442828== Conditional jump or move depends on uninitialised value(s) ==442828==at 0x80084F: record_image_cmp (rowtypes.c:1521) ==442828==by 0x801522: btrecordimagecmp (rowtypes.c:1839) ==442828==by 0x8C6604: comparison_shim (sortsupport.c:53) ==442828==by 0x64284D: ApplySortComparator (sortsupport.h:143) ==442828==by 0x642F3B: MJCompare (nodeMergejoin.c:412) ==442828==by 0x643A29: ExecMergeJoin (nodeMergejoin.c:1211) ==442828==by 0x62305A: ExecProcNode (execProcnode.c:453) ==442828==by 0x640800: ExecLimit (nodeLimit.c:91) ==442828==by 0x623135: ExecProcNode (execProcnode.c:500) ==442828==by 0x620E15: ExecutePlan (execMain.c:1472) ==442828==by 0x61EF94: standard_ExecutorRun (execMain.c:307) ==442828==by 0x61EE0B: ExecutorRun (execMain.c:255) And a quick look indeed seems to confirm that that's possible. 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] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Oct 15, 2013 at 1:19 PM, Peter Geoghegan wrote: >> There could be >> other ways of avoiding that problem, though. Here's an example: >> >> UPSERT table (keycol1, ..., keycoln) = (keyval1, ..., keyvaln) SET >> (nonkeycol1, ..., nonkeycoln) = (nonkeyval1, ..., nonkeyvaln) >> >> That's pretty ugly on multiple levels, and I'm definitely not >> proposing that exact thing, but the idea is: look for a record that >> matches on the key columns/values; if found, update the non-key >> columns with the corresponding values; if not found, construct a new >> row with both the key and nonkey column sets and insert it. If no >> matching unique index exists we'll have to fail, but we stop short of >> having to mention the name of that index. > > What if you want to update the key columns - either the potential > conflict-causing one, or another? I'm not sure what that means in the context of an UPSERT operation. If the update case is, when a = 1 then make a = 2, then which value goes in column a when we insert, 1 or 2? But I suppose if you can work that out it's just a matter of mentioning the column as both a key column and a non-key column. > What about composite unique > constraints? MySQL certainly supports all that, for example. That's why it allows you to specify N key columns rather than restricting you to just one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] buildfarm failures on smew and anole
On 2013-10-16 09:35:46 -0400, Robert Haas wrote: > Gah. I fixed one instance of that problem in test_config_settings(), > but missed the other. Maybe it'd be better to default to none, just as max_connections defaults to 1 and shared_buffers to 16? As we write out the value in the config file, everything should still continue to work. 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] Adding new syntax in postgre sql
I am new to postgre sql .And i want to add some new feature to postgresql
As a startup i have taken the project to add syntax for table partitioning
CREATE TABLE is modified to accept a PARTITION BY clause. This clause
contains one or more partition declarations. The syntax is as follows:
PARTITION BY {partition_type} (column_name[, column_name...])
[PARTITIONS number]
(
partition_declaration[, partition_declaration...]
)
The partition type can be one of HASH, RANGE or LIST.
so can anyone tell me from where to start .
I have read all the basics
but i dont know the control flow for adding new syntax.
so please tell me how to add new syntax
from parser to executor..
--
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] FDW API / flow charts for the docs?
On 17 Říjen 2013, 5:32, Stephen Frost wrote: > Alvaro, > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: >> Tomas Vondra wrote: >> > Attached is the set of flow charts, showing the sequence of callbacks >> > for all the supported commands (i.e. SELECT, INSERT, UPDATE, DELETE, >> > ANALYZE). Wouldn't it be useful to put something like this into the >> > docs? I mean, the FDW API is not going to get any simpler, and for me >> > this was a significant help. >> >> Please see this thread >> http://www.postgresql.org/message-id/4bb9e69f.9080...@usit.uio.no > > The conclusion of that thread appears to be "use dia", which was done > here.. Am I missing something there (full disclosure- I haven't looked > at the dia yet)? My impression from that thread was that one of the requirements is reasonable versioning / diff support, and AFAIK that's not a good match for any GUI-based product. So while I like dia and I used it for drawing the charts I submitted today, I don't think it works with this (quite reasonable) requirement. The only tool that might be a good match is graphviz (also mentioned in that thread). It's text-based, widely available and quite customizable. I'm not a graphviz expert, but attached is a result of 5-minute work with graphviz. Not perfect, but I'm pretty sure we could get much better / nicer results in very short time. It's easy to edit by hand, do versioning and /or diff on that, etc. I doubt there's a better option available. > Also, for my part, I'd suggest putting it on the wiki initially anyway, > as then it can be seen directly (load it as a png or what-have-you) and > it becomes immediately available to users. The .dia should also be on > the wiki, of course, and then included in the PG tree eventually if it's > added as part of the official docs. No problem with that, but I'd like to know in advance if we're willing to put that into the docs / under what requirements etc. Otherwise it might result in a major effort just to get it from wiki into docs later. Tomas fdw.dot Description: application/msword-template fdw.ps Description: PostScript document -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR : 'tuple concurrently updated'
> What PostgreSQL version is this? I'm using "Postgresql 9.2.4, compiled by Visual C++ build 1600, 64-bit" > Are there any triggers on any of these tables? There are no triggers. > Any noteworthy extensions installed? Here is the results returned by "select * from pg_available_extensions" name;default_version;installed_version adminpack ; 1.0 ; autoinc; 1.0 ; btree_gin ; 1.0 ; btree_gist ; 1.0 ; chkpass; 1.0 ; citext ; 1.0 ; cube ; 1.0 ; dblink ; 1.0 ; dict_int ; 1.0 ; dict_xsyn ; 1.0 ; earthdistance ; 1.0 ; file_fdw ; 1.0 ; fuzzystrmatch ; 1.0 ; hstore ; 1.1 ; insert_username; 1.0 ; intagg ; 1.0 ; intarray ; 1.0 ; isn; 1.0 ; lo ; 1.0 ; ltree ; 1.0 ; moddatetime; 1.0 ; pageinspect; 1.0 ; pgcrypto ; 1.0 ; pgrowlocks ; 1.0 ; pgstattuple; 1.0 ; pg_buffercache ; 1.0 ; pg_freespacemap; 1.0 ; pg_stat_statements ; 1.1 ; pg_trgm; 1.0 ; pldbgapi ; 1.0 ; plperl ; 1.0 ; plperlu; 1.0 ; plpgsql; 1.0 ; 1.0 plpython2u ; 1.0 ; plpython3u ; 1.0 ; plpythonu ; 1.0 ; pltcl ; 1.0 ; pltclu ; 1.0 ; refint ; 1.0 ; seg; 1.0 ; sslinfo; 1.0 ; tablefunc ; 1.0 ; tcn; 1.0 ; test_parser; 1.0 ; timetravel ; 1.0 ; tsearch2 ; 1.0 ; unaccent ; 1.0 ; uuid-ossp ; 1.0 ; xml2 ; 1.0 ; Le 17/10/2013 14:18, Robert Haas a écrit : Hmm. That error isn't supposed to happen; it's denoted in the source code by elog() rather than ereport(), which means that it's just there as a backstop, and never really intended to be become user-visible. So I'd say you've found a bug. What PostgreSQL version is this? There are actually two places where that error can happen: simple_heap_update and simple_heap_delete. If you set the error verbosity to verbose, you should be able to see which function is at fault. The thing is, I don't see anything in that query which would update or delete any tuples, so there must be more to the story. If you have the ability to build from source, you could try setting a long sleep just before that error is thrown. Then run your test case until it hangs at that spot and get a stack backtrace. But that may be more troubleshooting than you want to get into. Are there any triggers on any of these tables? Any noteworthy extensions installed? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Oct 15, 2013 at 9:56 AM, Robert Haas wrote: > Well, I don't know that any of us can claim to have a lock on what the > syntax should look like. Sure. But it's not just syntax. We're talking about functional differences too, since you're talking about mandating an update, which is a not the same as an "update locked row only conditionally", or a delete. I get that it's a little verbose, but then this is ORM plumbing for many of those that would prefer a more succinct syntax. Those people would also benefit from having their ORM do something much more powerful for them when needed. > I think we need to hear some proposals. Agreed. > You've heard my gripe about the current syntax (which Andres appears > to share), but I shan't attempt to prejudice you in favor of my > preferred alternative, because I don't have one yet. FWIW, I sincerely see very real advantages to what I've proposed here. To me, the fact that it's convenient to implement is beside the point. > There could be > other ways of avoiding that problem, though. Here's an example: > > UPSERT table (keycol1, ..., keycoln) = (keyval1, ..., keyvaln) SET > (nonkeycol1, ..., nonkeycoln) = (nonkeyval1, ..., nonkeyvaln) > > That's pretty ugly on multiple levels, and I'm definitely not > proposing that exact thing, but the idea is: look for a record that > matches on the key columns/values; if found, update the non-key > columns with the corresponding values; if not found, construct a new > row with both the key and nonkey column sets and insert it. If no > matching unique index exists we'll have to fail, but we stop short of > having to mention the name of that index. What if you want to update the key columns - either the potential conflict-causing one, or another? What about composite unique constraints? MySQL certainly supports all that, for example. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Oct 15, 2013 at 11:34 AM, Peter Geoghegan wrote: >> Again, other people can have different opinions on this, and that's >> fine. I'm just giving you mine. > > I will defer to the majority opinion here. But you also expressed > concern about surprising results due to the wrong unique constraint > violation being the source of a conflict. Couldn't this syntax (with > the wCTE upsert pattern) help with that, by naming the constant > inserted in the update too? It would be pretty simple to expose that, > and far less grotty than naming a unique index in DML. Well, I don't know that any of us can claim to have a lock on what the syntax should look like. I think we need to hear some proposals. You've heard my gripe about the current syntax (which Andres appears to share), but I shan't attempt to prejudice you in favor of my preferred alternative, because I don't have one yet. There could be other ways of avoiding that problem, though. Here's an example: UPSERT table (keycol1, ..., keycoln) = (keyval1, ..., keyvaln) SET (nonkeycol1, ..., nonkeycoln) = (nonkeyval1, ..., nonkeyvaln) That's pretty ugly on multiple levels, and I'm definitely not proposing that exact thing, but the idea is: look for a record that matches on the key columns/values; if found, update the non-key columns with the corresponding values; if not found, construct a new row with both the key and nonkey column sets and insert it. If no matching unique index exists we'll have to fail, but we stop short of having to mention the name of that index. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Oct 15, 2013 at 8:11 AM, Robert Haas wrote: > I'm not saying "go implement MERGE". I'm saying, make the > insert-or-update operation a single statement, using some syntax TBD, > instead of requiring the use of a new insert statement that makes > invisible rows visible as a side effect, so that you can wrap that in > a CTE and feed it to an update statement. That's complex and, AFAICS, > unlike how any other database product handles this. Well, lots of other databases have their own unique way of doing this - apart from MySQL's INSERT...ON DUPLICATE KEY UPDATE, there is a variant within Teradata, Sybase and SQLite. They're all different. And in the case of Teradata, it was an interim feature towards MERGE which came in a much later release, which is how I see this. No other database system even has writeable CTEs, of course. It's a fairly recent idea. > Again, other people can have different opinions on this, and that's > fine. I'm just giving you mine. I will defer to the majority opinion here. But you also expressed concern about surprising results due to the wrong unique constraint violation being the source of a conflict. Couldn't this syntax (with the wCTE upsert pattern) help with that, by naming the constant inserted in the update too? It would be pretty simple to expose that, and far less grotty than naming a unique index in DML. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing wrote:
> On 10/17/2013 10:03 AM, Fabien COELHO wrote:
>> My guess is that it won't be committed if there is a single "but it
>> might break one code or surprise one user somewhere in the universe",
>> but I wish I'll be proven wrong. IMO, "returned with feedback" on a 1
>> liner is really akin to "rejected".
>
> I have attached here an entirely new patch (new documentation and
> everything) that should please everyone. It no longer overloads
> pg_sleep(double precision) but instead add two new functions:
>
> * pg_sleep_for(interval)
> * pg_sleep_until(timestamp with time zone)
>
> Because it's no longer overloading the original pg_sleep, Robert's
> ambiguity objection is no more.
>
> Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
>
> If people like this, I'll reject the current patch and add this one to
> the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: space reserved for WAL record does not match what was written: panic on windows
On Fri, Oct 11, 2013 at 1:14 AM, Noah Misch wrote: > On Thu, Oct 10, 2013 at 03:23:30PM +0200, Andres Freund wrote: >> On 2013-10-10 08:59:47 -0400, Robert Haas wrote: >> > On Tue, Oct 8, 2013 at 6:24 PM, Andres Freund >> > wrote: >> > > Do you have a better alternative? Making the computation unconditionally >> > > 64bit will have a runtime overhead and adding a StaticAssert in the >> > > existing macro doesn't work because we use it in array sizes where gcc >> > > balks. >> > > We could try using inline functions, but that's not going to be pretty >> > > either. >> > > >> > > I don't really see that many further usecases that will align 64bit >> > > values on 32bit platforms, so I think we're ok for now. >> > >> > I'd be inclined to make the computation unconditionally 64-bit. I >> > doubt the speed penalty is enough to worry about, and I think we're >> > going to have more and more cases where optimizing for 32-bit >> > platforms is just not the right decision. >> >> MAXALIGN is used in several of PG's hottest functions in many >> scenarios. att_align_nominal is used in slot_deform_tuple, >> heap_deform_tuple, nocachegetattr, etc. So I don't think that's viable >> yet. At least not with much more benefit than this... > > Agreed. Besides performance, aligning a wider-than-pointer value is an > unusual need; authors should think thrice before doing that. I might have > even defined the MAXALIGN64 macro in xlog.c rather than a core header. > > Incidentally, why does MAXALIGN64 use unsigned math while MAXALIGN uses signed > math? Well, if this is the consensus, then I think the dynamic shared memory patch may need some revision. In that patch, I used uint64 to represent the size of the dynamic shared memory segment, sort of on the theory that we were going to use this to be allocating big chunks of dynamic shared memory for stuff like parallel sort. In follow-on patches I'm currently developing to actually do stuff with dynamic shared memory, this results in extensive use of MAXALIGN64, and it really kind of looks like it wants the whole set of alignment macros, not just that one. So option one is to leave the dsm code alone and add the rest of the macros. But if we're bent on minimizing the use of 64-bit arithmetic on 32-bit systems, then presumably I should instead go back and retrofit that patch to use Size rather than uint64 to represent the size of a segment. But then I have two concerns: 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)? (Note that Size is just a typedef for size_t, in c.h) 2. If intptr_t is a signed type, as it appears to be, and size_t is an unsigned type, as I believe it to be, then is it safe to use the macros written for the signed type with a value of the unsigned type? Off-hand I can't see a problem there, but I'm not certain I'm not missing something. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
> My guess is that it won't be committed if there is a single "but it
> might break one code or surprise one user somewhere in the universe",
> but I wish I'll be proven wrong. IMO, "returned with feedback" on a 1
> liner is really akin to "rejected".
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
Opinions?
--
Vik
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 7586,7605 SELECT TIMESTAMP 'now'; -- incorrect for use with DEFAULT
! The following function is available to delay execution of the server
process:
pg_sleep(seconds)
pg_sleep makes the current session's process
sleep until seconds seconds have
elapsed. seconds is a value of type
double precision, so fractional-second delays can be specified.
For example:
SELECT pg_sleep(1.5);
--- 7586,7613
! The following functions are available to delay execution of the server
process:
pg_sleep(seconds)
+ pg_sleep_for(interval)
+ pg_sleep_until(timestamp with time zone)
pg_sleep makes the current session's process
sleep until seconds seconds have
elapsed. seconds is a value of type
double precision, so fractional-second delays can be specified.
+ pg_sleep_for is a convenience function for larger
+ sleep times specified as an interval.
+ pg_sleep_until is a convenience function for when
+ a specific wake-up time is desired.
For example:
SELECT pg_sleep(1.5);
+ SELECT pg_sleep_for('5 minutes');
+ SELECT pg_sleep_until('tomorrow 03:00');
***
*** 7608,7622 SELECT pg_sleep(1.5);
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
Make sure that your session does not hold more locks than necessary
! when calling pg_sleep. Otherwise other sessions
! might have to wait for your sleeping process, slowing down the entire
! system.
--- 7616,7632
The effective resolution of the sleep interval is platform-specific;
0.01 seconds is a common value. The sleep delay will be at least as long
as specified. It might be longer depending on factors such as server load.
+ In particular, pg_sleep_until is not guaranteed to
+ wake up exactly at the specified time, but it will not wake up any earlier.
Make sure that your session does not hold more locks than necessary
! when calling pg_sleep or its variants. Otherwise
! other sessions might have to wait for your sleeping process, slowing down
! the entire system.
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 3017,3022 DATA(insert OID = 2625 ( pg_ls_dir PGNSP PGUID 12 1 1000 0 0 f f f f t t v 1 0
--- 3017,3026
DESCR("list all files in a directory");
DATA(insert OID = 2626 ( pg_sleep PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "701" _null_ _null_ _null_ _null_ pg_sleep _null_ _null_ _null_ ));
DESCR("sleep for the specified time in seconds");
+ DATA(insert OID = 3179 ( pg_sleep_for PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 "1186" _null_ _null_ _null_ _null_ "select pg_sleep(extract(epoch from now() + $1) - extract(epoch from now()))" _null_ _null_ _null_ ));
+ DESCR("sleep for the specified interval");
+ DATA(insert OID = 3180 ( pg_sleep_until PGNSP PGUID 14 1 0 0 0 f f f f t f v 1 0 2278 "1184" _null_ _null_ _null_ _null_ "select pg_sleep(extract(epoch from $1) - extract(epoch from now()))" _null_ _null_ _null_ ));
+ DESCR("sleep until the specified time");
DATA(insert OID = 2971 ( textPGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 25 "16" _null_ _null_ _null_ _null_ booltext _null_ _null_ _null_ ));
DESCR("convert boolean to text");
*** a/src/test/regress/expected/stats.out
--- b/src/test/regress/expected/stats.out
***
*** 18,26 SET enable_indexscan TO on;
SET enable_indexonlyscan TO off;
-- wait to let any prior tests finish dumping out stats;
-- else our messages might get lost due to contention
! SELECT pg_sleep(2.0);
! pg_sleep
! --
(1 row)
--- 18,26
SET enable_indexon
Re: [HACKERS] Auto-tuning work_mem and maintenance_work_mem
On Wed, Oct 16, 2013 at 5:14 PM, Josh Berkus wrote: > On 10/16/2013 01:25 PM, Andrew Dunstan wrote: >> Andres has just been politely pointing out to me that my knowledge of >> memory allocators is a little out of date (i.e. by a decade or two), and >> that this memory is not in fact likely to be held for a long time, at >> least on most modern systems. That undermines completely my reasoning >> above. > > Except that Opensolaris and FreeBSD still have the old memory allocation > behavior, as do older Linux kernels, many of which will remain in > production for years. I have no idea what Windows' memory management > behavior is. > > So this is a case of needing to know considerably more than the > available RAM to determine a good setting. I agree, but I still think my previous proposal of increasing the defaults for work_mem and maintenance_work_mem by 4X would serve many more people well than it would serve poorly. I haven't heard anyone disagree with that notion. Does anyone disagree? Should we do it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] hstore_to_json: empty hstores must return empty json objects
On Wed, Oct 16, 2013 at 12:02 PM, Oskari Saarenmaa wrote: > hstore_to_json used to return an empty string for empty hstores, but > that is not correct as an empty string is not valid json and calling > row_to_json() on rows which have empty hstores will generate invalid > json for the entire row. The right thing to do is to return an empty > json object. Hmm. This sure looks like a bug to me. Anyone think otherwise? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR : 'tuple concurrently updated'
On Wed, Oct 16, 2013 at 8:25 AM, Stéphan BEUZE
wrote:
> The following query is performed concurrently by two threads logged in with
> two different users:
>
> WITH raw_stat AS (
> SELECT
>host(client_addr) as client_addr,
>pid ,
>usename
> FROM
>pg_stat_activity
> WHERE
>usename = current_user
> )
> INSERT INTO my_stat(id, client_addr, pid, usename)
> SELECT
> nextval('mystat_sequence'), t.client_addr, t.pid, t.usename
> FROM (
> SELECT
> client_addr, pid, usename
> FROM
> raw_stat s
> WHERE
> NOT EXISTS (
>SELECT
> NULL
>FROM
> my_stat u
>WHERE
> current_date = u.creation
>AND
> s.pid = u.pid
>AND
> s.client_addr = u.client_addr
>AND
> s.usename = u.usename
> )
> ) t;
>
> From time to time, I get the following error: "tuple concurrently updated"
>
> I can't figure out what throw this error and why this error is thrown. Can
> you shed a light ?
Hmm. That error isn't supposed to happen; it's denoted in the source
code by elog() rather than ereport(), which means that it's just there
as a backstop, and never really intended to be become user-visible.
So I'd say you've found a bug. What PostgreSQL version is this?
There are actually two places where that error can happen:
simple_heap_update and simple_heap_delete. If you set the error
verbosity to verbose, you should be able to see which function is at
fault. The thing is, I don't see anything in that query which would
update or delete any tuples, so there must be more to the story. If
you have the ability to build from source, you could try setting a
long sleep just before that error is thrown. Then run your test case
until it hangs at that spot and get a stack backtrace. But that may
be more troubleshooting than you want to get into. Are there any
triggers on any of these tables? Any noteworthy extensions installed?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] removing old ports and architectures
On Thu, Oct 17, 2013 at 12:22 AM, Noah Misch wrote: > Removing support for alpha is a different animal compared to removing support > for non-gcc MIPS and most of the others in your list. A hacker wishing to > restore support for another MIPS compiler would fill in the assembly code > blanks, probably using code right out of an architecture manual. A hacker > wishing to restore support for alpha would find himself auditing every > lock-impoverished algorithm in the backend. I had much the same thought last night. So I reverse my vote on Alpha: let's drop it. I had thought that perhaps there'd be some value in keeping it to force ourselves to consider what will happen under the weakest generally-understood memory model, but in fact that's probably a doomed effort without having the hardware available to test the code. As you say, any future atomics support for such a platform will be a major undertaking. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
On Wed, Oct 16, 2013 at 5:16 PM, Peter Eisentraut wrote:
> On 10/16/13 2:40 PM, Robert Haas wrote:
>> On Wed, Oct 16, 2013 at 1:26 PM, Josh Berkus wrote:
>>> Also, as Tom pointed out, at some point we have to either say we really
>>> support overloading or we don't.
>>
>> We clearly do support overloading. I don't think that's at issue.
>> But as we all know, using it can cause formerly unambiguous queries to
>> become ambiguous and stop working.
>
> But that's not really what this is. It's one thing to be wary about
> adding foo(bigint, int, smallint) when there are already three other
> permutations in the system. (At least in other languages, compilers
> will give you warnings or errors when this creates an ambiguity, so
> there's no guessing.) But the problem here is that if there already is a
>
> foo(type1)
>
> then the proposal to add
>
> foo(type2)
>
> can always be shot down by
>
> "But this will break foo('type1val')."
>
> That can't be in the spirit of overloading.
>
> The only way to fix this is that at the time when you add foo(type1) you
> need to prevent people from being able to call foo('type1val') and
> instead require the full syntax foo(type1 'type1val'). The only way to
> do that, I think, is to add some other foo(type3) at the same time.
> There is just something wrong with that.
Actually, this could be fixed by having a way to declare one of the
overloaded functions as the preferred option and resolving ambiguous
calls in favor of the highest-priority function. In fact,
EnterpriseDB has added just such an option to Advanced Server 9.3, and
it fixes several longstanding difficult choices between being
Oracle-compatible and being PostgreSQL-compatible; we're now more
compatible with both. If we had that option in PostgreSQL, we could
declare the existing function as the preferred option, add the new
one, and be pretty much assured of not breaking anything.
However, I've learned from experience that submitting patches to
improve PostgreSQL's only-marginally-usable ambiguous function
resolution code is an exercise in futility. My last attempt ended
with a clear majority of people telling me that they liked failing to
call *the only function called foo* when confronted with a call that
was clearly intended to reference that function. As nearly as I can
tell, people like the idea of such unnecessary failures only in
theory. As soon as it comes to any practical case (like this one),
people start looking for workarounds. Right now there aren't any good
ones, and the reason for that is simple: we refuse to entertain adding
them.
Just sayin'.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_sleep(interval)
Hello Vik, For: Josh, Stephen, me Against: Robert Neutral: Tom, you For the record, I'm not neutral, I'm *FOR*. I reviewed it and said that I think it is fine. But I'm still nobody here:-) My experience at trying to pass minor patches shows that the basic behavior is conservatism. Maybe this is necessary to the stability of the project, but this is really annoying for pretty small changes on side tools, and I think that it tends to over-conservatism and ampers good will. You have to argue a lot about trivial things. My ratio for passing very minor patches on pgbench for instance is 1:3 or worst, 1 unit programming and testing versus 3 units writing mails to argue about this and that. Maybe the ratio is better for big important patches. Your patch is quite representative, 1 line of trivial code, a few lines of tests and docs, and how many lines and time at writing mails? I don't know if that's enough of a consensus to commit it, but I do think it's not nearly enough of a consensus to reject it. My guess is that it won't be committed if there is a single "but it might break one code or surprise one user somewhere in the universe", but I wish I'll be proven wrong. IMO, "returned with feedback" on a 1 liner is really akin to "rejected". -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
