Re: [HACKERS] pg_ctl idempotent option
On Mon, Jan 14, 2013 at 4:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: Here is a patch to add an option -I/--idempotent to pg_ctl, the result of which is that pg_ctl doesn't error on start or stop if the server is already running or already stopped. Idempotent is a ten-dollar word. Can we find something that average people wouldn't need to consult a dictionary to understand? I disagree that we should dumb things down when the word means exactly what we want and based on the rest of this thread is the only word or word cluster that carries the desired meaning. I vote to keep --idempotent. Vik
Re: [HACKERS] proposal: fix corner use case of variadic fuctions usage
On Sat, Dec 1, 2012 at 1:14 PM, Pavel Stehule pavel.steh...@gmail.comwrote: Hello Hi Pavel. I am trying to review this patch and on my work computer everything compiles and tests perfectly. However, on my laptop, the regression tests don't pass with cache lookup failed for type XYZ where XYZ is some number that does not appear to be any type oid. I don't really know where to go from here. I am asking that other people try this patch to see if they get errors as well. yes, I checked it on .x86_64 and I had a same problems probably there was more than one issue - I had to fix a creating a unpacked params and I had a issue with gcc optimalization when I used a stack variable for fcinfo. Now I fixed these issues and I hope so it will work on all platforms It appears to work a lot better, yes. I played around with it a little bit and wasn't able to break it, so I'm marking it as ready for committer. Some wordsmithing will need to be done on the code comments.
Re: [HACKERS] DEALLOCATE IF EXISTS
On Tue, Nov 27, 2012 at 3:15 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I fail to see the point of DEALLOCATE IF EXISTS. Do you have real use case for this, or was this just a case of adding IF EXISTS to all commands for the sake of completeness? Usually the client knows what statements have been prepared, but perhaps you want to make sure everything is deallocated in some error handling case or similar. But in that case, you might as well just issue a regular DEALLOCATE and ignore errors. Or even more likely, you'll want to use DEALLOCATE ALL. Hmm. The test case I had for it, which was very annoying in an I want to be lazy sort of way, I am unable to reproduce now. So I guess this becomes a make it like the others and the community can decide whether that's desirable. In my personal case, which again I can't reproduce because it's been a while since I've done it, DEALLOCATE ALL would have worked. I was basically preparing a query to work on it in the same conditions that it would be executed in a function, and I was only working on one of these at a time so ALL would have been fine.
Re: [HACKERS] proposal: fix corner use case of variadic fuctions usage
On Sun, Nov 4, 2012 at 12:49 PM, Pavel Stehule pavel.steh...@gmail.comwrote:
Hello
here is patch, that enables using a variadic parameter modifier for
variadic any function.
Motivation for this patch is consistent behave of format function,
but it fixes behave of all this class variadic functions.
postgres= -- check pass variadic argument
postgres= select format('%s, %s', variadic array['Hello','World']);
format
──
Hello, World
(1 row)
postgres= -- multidimensional array is supported
postgres= select format('%s, %s', variadic
array[['Nazdar','Svete'],['Hello','World']]);
format
───
{Nazdar,Svete}, {Hello,World}
(1 row)
It respect Tom's comments - it is based on short-lived FmgrInfo
structures, that are created immediately before function invocation.
Note: there is unsolved issue - reusing transformed arguments - so it
is little bit suboptimal for VARIADIC RETURNING MultiFuncCall
functions, where we don't need to repeat a unpacking of array value.
Regards
Pavel
Hi Pavel.
I am trying to review this patch and on my work computer everything
compiles and tests perfectly. However, on my laptop, the regression tests
don't pass with cache lookup failed for type XYZ where XYZ is some number
that does not appear to be any type oid.
I don't really know where to go from here. I am asking that other people
try this patch to see if they get errors as well.
Vik
PS: I won't be able to answer this thread until Tuesday.
Re: [HACKERS] DEALLOCATE IF EXISTS
On Tue, Oct 9, 2012 at 4:44 PM, Vik Reykja vikrey...@gmail.com wrote: On Tue, Oct 9, 2012 at 4:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: =?ISO-8859-1?Q?S=E9bastien_Lardi=E8re?= slardi...@hi-media.com writes: Indeed, brackets was not correct, it's better now (I think), and correct some comments. Still wrong ... at the very least you missed copyfuncs/equalfuncs. In general, when adding a field to a struct, it's good practice to grep for all uses of that struct. I don't see Sébastien's message, but I made the same mistake in my patch. Another one is attached with copyfuncs and equalfuncs. I did a grep for DeallocateStmt and I don't believe I have missed anything else. Also, I'm changing the subject so as not to hijack this thread any further. I am taking no comments to mean no objections and have added this to the next commitfest.
Re: [HACKERS] Re: [PATCH] Enforce that INSERT...RETURNING preserves the order of multi rows
On Sun, Oct 21, 2012 at 6:20 PM, Abhijit Menon-Sen a...@2ndquadrant.comwrote: At 2012-10-21 11:49:26 -0400, cbbro...@gmail.com wrote: If there is a natural sequence (e.g. - a value assigned by nextval()), that offers a natural place to apply the usual order-imposing ORDER BY that we are expected to use elsewhere. Note: INSERT … RETURNING doesn't accept an ORDER BY clause. Would anyone be opposed to somebody - say, me - writing a patch to allow that? It would take me a lot longer than an experienced hacker to do it, but I'm willing to try.
Re: [HACKERS] Truncate if exists
On Tue, Oct 9, 2012 at 11:09 AM, Simon Riggs si...@2ndquadrant.com wrote: Anyone want to check for any other missing IF EXISTS capability in other DDL? Yes, DEALLOCATE.
Re: [HACKERS] Truncate if exists
On Tue, Oct 9, 2012 at 11:51 AM, Vik Reykja vikrey...@gmail.com wrote: On Tue, Oct 9, 2012 at 11:09 AM, Simon Riggs si...@2ndquadrant.comwrote: Anyone want to check for any other missing IF EXISTS capability in other DDL? Yes, DEALLOCATE. Patch attached. deallocate_if_exists.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.2rc1 produces incorrect results
On Wed, Sep 5, 2012 at 6:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: I think probably the best fix is to rejigger things so that Params assigned by different executions of SS_replace_correlation_vars and createplan.c can't share PARAM_EXEC numbers. This will result in rather larger ecxt_param_exec_vals arrays at runtime, but the array entries aren't very large, so I don't think it'll matter. Attached is a draft patch against HEAD for this. I think it makes the planner's handling of outer-level Params far less squishy than it's ever been, but it is rather a large change. Not sure whether to risk pushing it into 9.2 right now, or wait till after we cut 9.2.0 ... thoughts? I am not in a position to know what's best for the project but my company can't upgrade (from 9.0) until this is fixed. We'll wait for 9.2.1 if we have to. After all, we skipped 9.1.
[HACKERS] 9.2rc1 produces incorrect results
Hello. It took me a while to get a version of this that was independent of my data, but here it is. I don't understand what's going wrong but if you change any part of this query (or at least any part I tried), the correct result is returned. This script will reproduce it: = create table t1 (id integer primary key); create table t2 (id integer primary key references t1 (id)); insert into t1 (id) select generate_series(1, 10); -- size matters insert into t2 (id) values (1); -- get a known value in the table insert into t2 (id) select g from generate_series(2, 10) g where random() 0.01; -- size matters again analyze t1; analyze t2; with A as ( select t2.id, t2.id = 1 as is_something from t2 join t1 on t1.id = t2.id left join pg_class pg_c on pg_c.relname = t2.id::text -- I haven't tried on a user table where pg_c.oid is null ), B as ( select A.id, row_number() over (partition by A.id) as order -- this seems to be important, too from A ) select A.id, array(select B.id from B where B.id = A.id) from A where A.is_something union all select A.id, array(select B.id from B where B.id = A.id) from A where A.is_something; = As you can (hopefully) see, the two UNIONed queries are identical but do not return the same values. I wish I had the skills to attach a patch to this message, but alas I do not.
Re: [HACKERS] Covering Indexes
On Tue, Jul 17, 2012 at 6:08 PM, Simon Riggs si...@2ndquadrant.com wrote: On 17 July 2012 16:54, David E. Wheeler da...@justatheory.com wrote: Yeah, but that index is unnecessarily big if one will never use c or d in the search. The nice thing about covering indexes as described for SQLite 4 and implemented in MSSQL is that you can specify additional columns that just come along for the ride, but are not part of the indexed data: CREATE INDEX cover1 ON table1(a,b) COVERING(c,d); Yes, you can do that by also indexing c and d as of 9.2, but it might be nice to be able to include them in the index as additional row data without actually indexing them. Can you explain what you mean by without actually indexing them? ISTM that it is a non-feature if the index is already non-unique, and the difference is simply down to the amount of snake oil applied to the descriptive text on the release notes. It would be useful in non-unique indexes to store data without ordering operators (like xml).
Re: [HACKERS] Schema version management
On Thu, Jul 5, 2012 at 3:32 PM, Michael Glaesemann g...@seespotcode.netwrote: On Jul 5, 2012, at 9:21, Andrew Dunstan wrote: No they are not necessarily one logical unit. You could have a bunch of functions called, say, equal which have pretty much nothing to do with each other, since they refer to different types. +1 from me for putting one function definition per file. +1. It might make sense to include some sort of argument type information. The function signature is really its identifier. The function name is only part of it. I'll go against the flow here. I would prefer to have all overloaded functions in the same file.
Re: [HACKERS] REVIEW: Optimize referential integrity checks (todo item)
On Wed, Jun 20, 2012 at 2:19 AM, Tom Lane t...@sss.pgh.pa.us wrote: I've marked this patch committed, although in the end there was nothing left of it ;-) Thank you, Dean and Tom! I'm sorry for not participating in this thread, I've been away for the past five weeks and have much catching up to do.
Re: [HACKERS] New Postgres committer: Kevin Grittner
On Thu, Jun 7, 2012 at 6:15 PM, Tom Lane t...@sss.pgh.pa.us wrote: I am pleased to announce that Kevin Grittner has accepted the core committee's invitation to become our newest committer. As you all know, Kevin's done a good deal of work on the project over the past couple of years. We judged that he has the requisite skills, dedication to the project, and a suitable degree of caution to be a good committer. Please join me in welcoming him aboard. Congrats, Kevin! I think you'll make a wonderful addition to the core team.
Re: [HACKERS] Add primary key/unique constraint using prefix columns of an index
On Tue, May 22, 2012 at 1:36 PM, Simon Riggs si...@2ndquadrant.com wrote: On 22 May 2012 18:24, Jeff Janes jeff.ja...@gmail.com wrote: Now that there are index only scans, there is a use case for having a composite index which has the primary key or a unique key as the prefix column(s) but with extra columns after that. Currently you would also need another index with exactly the primary/unique key, which seems like a waste of storage and maintenance. Should there be a way to declare a unique index with the unique property applying to a prefix of the indexed columns/expression? And having that, a way to turn that prefix into a primary key constraint? Of course this is easier said then done, but is there some reason for it not to be a to-do item? +1 Very useful Semi-private note to Simon: isn't this pretty much what I was advocating at the London meetup last month?
Re: [HACKERS] Draft release notes complete
On Thu, May 10, 2012 at 2:24 PM, Andrew Dunstan and...@dunslane.net wrote: On 05/10/2012 08:11 AM, Peter Geoghegan wrote: I'm not really sure why you've listed Daniel Farina as a co-author of the pg_stat_statements normalisation feature. He did a good job of reviewing it, but he didn't actually contribute any code. It looks like reviewers have been given credit throughout. Which could be good incentive to become more involved in reviewing for some people.
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 2:31 PM, Simon Riggs si...@2ndquadrant.com wrote: Would we consider adding such a switch (it should be easy enough to do), or do we want to push this off to the mythical let's improve the logging subsystem project that might eventually materialize if we're lucky? Meaning - would people object to such a switch? Yes, if the new parameter allows a generic filter on multiple user-specified message types. Are you answering the Would we consider or the would people object?
Re: [HACKERS] Future of our regular expression code
On Sat, Feb 18, 2012 at 21:16, Vik Reykja vikrey...@gmail.com wrote: I would be willing to have a go at translating test cases. I do not (yet) have the C knowledge to maintain the regex code, though. I got suddenly swamped and forgot I had signed up for this. I'm still pretty swamped and I would like these regression tests to be in for 9.2 so if someone else would like to pick them up, I would be grateful. If they're still not done by the time I resurface, I will attack them again.
Re: [HACKERS] foreign key locks, 2nd attempt
On Thu, Feb 23, 2012 at 19:44, Kevin Grittner kevin.gritt...@wicourts.govwrote: One of the problems that Florian was trying to address is that people often have a need to enforce something with a lot of similarity to a foreign key, but with more subtle logic than declarative foreign keys support. One example would be the case Robert has used in some presentations, where the manager column in each row in a project table must contain the id of a row in a person table *which has the project_manager boolean column set to TRUE*. Short of using the new serializable transaction isolation level in all related transactions, hand-coding enforcement of this useful invariant through trigger code (or application code enforced through some framework) is very tricky. The change to SELECT FOR UPDATE that Florian was working on would make it pretty straightforward. I'm not sure what Florian's patch does, but I've been trying to advocate syntax like the following for this exact scenario: foreign key (manager_id, true) references person (id, is_manager) Basically, allow us to use constants instead of field names as part of foreign keys. I have no idea what the implementation aspect of this is, but I need the user aspect of it and don't know the best way to get it.
Re: [HACKERS] Future of our regular expression code
On Sat, Feb 18, 2012 at 21:04, Simon Riggs si...@2ndquadrant.com wrote: On Sat, Feb 18, 2012 at 7:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: One immediate consequence of deciding that we are lead maintainers and not just consumers is that we should put in some regression tests, instead of taking the attitude that the Tcl guys are in charge of that. I have a head cold today and am not firing on enough cylinders to do anything actually complicated, so I was thinking of spending the afternoon transliterating the Tcl regex test cases into SQL as a starting point. Having just had that brand of virus, I'd skip it and take the time off, like I should have. Translating the test cases is a great way in for a volunteer, so please leave a few easy things to get people started on the road to maintaining that. I would be willing to have a go at translating test cases. I do not (yet) have the C knowledge to maintain the regex code, though.
Re: [HACKERS] Notes about fixing regexes and UTF-8 (yet again)
On Sun, Feb 19, 2012 at 04:33, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 18, 2012 at 7:29 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah, it's conceivable that we could implement something whereby characters with codes above some cutoff point are handled via runtime calls to iswalpha() and friends, rather than being included in the statically-constructed DFA maps. The cutoff point could likely be a lot less than U+, too, thereby saving storage and map build time all round. In the meantime, I still think the caching logic is worth having, and we could at least make some people happy if we selected a cutoff point somewhere between U+FF and U+. I don't have any strong ideas about what a good compromise cutoff would be. One possibility is U+7FF, which corresponds to the limit of what fits in 2-byte UTF8; but I don't know if that corresponds to any significant dropoff in frequency of usage. The problem, of course, is that this probably depends quite a bit on what language you happen to be using. For some languages, it won't matter whether you cut it off at U+FF or U+7FF; while for others even U+ might not be enough. So I think this is one of those cases where it's somewhat meaningless to talk about frequency of usage. Does it make sense for regexps to have collations?
Re: [HACKERS] Notes about fixing regexes and UTF-8 (yet again)
On Sun, Feb 19, 2012 at 05:03, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 18, 2012 at 10:38 PM, Vik Reykja vikrey...@gmail.com wrote: Does it make sense for regexps to have collations? As I understand it, collations determine the sort-ordering of strings. Regular expressions don't care about that. Why do you ask? Perhaps I used the wrong term, but I was thinking the locale could tell us what alphabet we're dealing with. So a regexp using en_US would give different word-boundary results from one using zh_CN.
Re: [HACKERS] Optimize referential integrity checks (todo item)
On Mon, Feb 13, 2012 at 15:25, Robert Haas robertmh...@gmail.com wrote: On Sat, Feb 11, 2012 at 9:06 PM, Vik Reykja vikrey...@gmail.com wrote: I decided to take a crack at the todo item created from the following post: http://archives.postgresql.org/pgsql-performance/2005-10/msg00458.php The attached patch makes the desired changes in both code and function naming. It seemed quite easy to do but wasn't marked as easy on the todo, so I'm wondering if I've missed something. It's kind of hard to say whether you've missed something, because you haven't really explained what problem this is solving; the thread you linked too isn't very clear about that either. At first blush, it seems like you've renamed a bunch of stuff without making very much change to what actually happens. Changing lots of copies of equal to unchanged doesn't seem to me to be accomplishing anything. It's very simple really, and most of it is indeed renaming the functions. The problem this solves is that foreign key constraints are sometimes checked when they don't need to be. See my example below. All regression tests pass. You should add some new ones showing how this patch improves the behavior relative to the previous code. Or if you can't, then you should provide a complete, self-contained test case that a reviewer can use to see how your proposed changes improve things. I have no idea how a regression test would be able to see this change, so here's a test case that you can follow with the debugger. /* initial setup */ create table a (x int, y int, primary key (x, y)); create table b (x int, y int, z int, foreign key (x, y) references a); insert into a values (1, 2); insert into b values (1, null, 3); /* seeing the difference */ update b set z=0; When that update is run, it will check if the FK (x, y) has changed to know if it needs to verify that the values are present in the other table. The equality functions that do that don't consider two nulls to be equal (per sql logic) and so reverified the constraint. Tom noticed that it didn't need to because it hadn't really changed. In the above example, the current code will recheck the constraint and the new code won't. It's not really testing equality anymore (because null does not equal null), so I renamed them causing a lot of noise in the diff. We're in the middle of a CommitFest right now, Yes, I wasn't expecting this to be committed, I just didn't want to lose track of it. so please add this patch to the next one if you would like it reviewed: https://commitfest.postgresql.org/action/commitfest_view/open Will do.
Re: [HACKERS] Optimize referential integrity checks (todo item)
On Mon, Feb 13, 2012 at 11:02, Chetan Suttraway chetan.suttra...@enterprisedb.com wrote: The patch was not getting applied. Was seeing below message: postgresql$ git apply /Downloads/unchanged.patch error: src/backend/utils/adt/ri_triggers.c: already exists in working directory Have come up with attached patch which hopefully should not have missed any of your changes. Thank you for doing that. What command did you use? I followed the procedure on the wiki [1] but I must be doing something wrong. [1] http://wiki.postgresql.org/wiki/Working_with_Git#Context_diffs_with_Git Please verify the changes. They look good. Thanks again.
[HACKERS] Optimize referential integrity checks (todo item)
I decided to take a crack at the todo item created from the following post:
http://archives.postgresql.org/pgsql-performance/2005-10/msg00458.php
The attached patch makes the desired changes in both code and function
naming.
It seemed quite easy to do but wasn't marked as easy on the todo, so I'm
wondering if I've missed something. All regression tests pass.
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
new file mode 100644
index 03a974a..107408f
*** a/src/backend/utils/adt/ri_triggers.c
--- b/src/backend/utils/adt/ri_triggers.c
*** static void ri_BuildQueryKeyFull(RI_Quer
*** 205,215
static void ri_BuildQueryKeyPkCheck(RI_QueryKey *key,
const RI_ConstraintInfo *riinfo,
int32 constr_queryno);
! static bool ri_KeysEqual(Relation rel, HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
! static bool ri_AllKeysUnequal(Relation rel, HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
! static bool ri_OneKeyEqual(Relation rel, int column,
HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
static bool ri_AttributesEqual(Oid eq_opr, Oid typeid,
--- 205,215
static void ri_BuildQueryKeyPkCheck(RI_QueryKey *key,
const RI_ConstraintInfo *riinfo,
int32 constr_queryno);
! static bool ri_KeysUnchanged(Relation rel, HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
! static bool ri_AllKeysChanged(Relation rel, HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
! static bool ri_OneKeyUnchanged(Relation rel, int column,
HeapTuple oldtup, HeapTuple newtup,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
static bool ri_AttributesEqual(Oid eq_opr, Oid typeid,
*** RI_FKey_noaction_upd(PG_FUNCTION_ARGS)
*** 932,940
}
/*
! * No need to check anything if old and new keys are equal
*/
! if (ri_KeysEqual(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowShareLock);
return PointerGetDatum(NULL);
--- 932,940
}
/*
! * No need to check anything if old and new keys are unchanged
*/
! if (ri_KeysUnchanged(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowShareLock);
return PointerGetDatum(NULL);
*** RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
*** 1281,1289
}
/*
! * No need to do anything if old and new keys are equal
*/
! if (ri_KeysEqual(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowExclusiveLock);
return PointerGetDatum(NULL);
--- 1281,1289
}
/*
! * No need to do anything if old and new keys are unchanged
*/
! if (ri_KeysUnchanged(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowExclusiveLock);
return PointerGetDatum(NULL);
*** RI_FKey_restrict_upd(PG_FUNCTION_ARGS)
*** 1646,1654
}
/*
! * No need to check anything if old and new keys are equal
*/
! if (ri_KeysEqual(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowShareLock);
return PointerGetDatum(NULL);
--- 1646,1654
}
/*
! * No need to check anything if old and new keys are unchanged
*/
! if (ri_KeysUnchanged(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowShareLock);
return PointerGetDatum(NULL);
*** RI_FKey_setnull_upd(PG_FUNCTION_ARGS)
*** 1993,2001
}
/*
! * No need to do anything if old and new keys are equal
*/
! if (ri_KeysEqual(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowExclusiveLock);
return PointerGetDatum(NULL);
--- 1993,2001
}
/*
! * No need to do anything if old and new keys are unchanged
*/
! if (ri_KeysUnchanged(pk_rel, old_row, new_row, riinfo, true))
{
heap_close(fk_rel, RowExclusiveLock);
return PointerGetDatum(NULL);
*** RI_FKey_setnull_upd(PG_FUNCTION_ARGS)
*** 2012,2024
* our cached plan, unless the update happens to change all
* columns in the key. Fortunately, for the most common case of a
* single-column foreign key, this will be true.
- *
- * In case you're wondering, the inequality check works because we
- * know that the old key value has no NULLs (see above).
*/
use_cached_query = (riinfo.confmatchtype == FKCONSTR_MATCH_FULL) ||
! ri_AllKeysUnequal(pk_rel, old_row, new_row,
riinfo, true);
/*
--- 2012,2021
* our cached plan, unless the update happens to change all
* columns in the key. Fortunately, for the most common case of a
* single-column foreign key, this will be true.
Re: [HACKERS] Different error messages executing CREATE TABLE or ALTER TABLE to create a column xmin
On Thu, Jan 26, 2012 at 17:58, Robert Haas robertmh...@gmail.com wrote: On Thu, Jan 26, 2012 at 11:29 AM, Tom Lane t...@sss.pgh.pa.us wrote: This looks reasonable to me, except that possibly the new error message text could do with a bit more thought. It seems randomly unlike the normal message, and I also have a bit of logical difficulty with the wording equating a column with a column name. The wording that is in use in the existing CREATE TABLE case is column name \%s\ conflicts with a system column name We could do worse than to use that verbatim, so as to avoid introducing a new translatable string. Another possibility is column \%s\ of relation \%s\ already exists as a system column Or we could keep the primary errmsg the same as it is for a normal column and instead add a DETAIL explaining that this is a system column. I intended for the message to match the CREATE TABLE case. I think it does, except I see now that Vik's patch left out the word name where the original string has it. So I'll vote in favor of your first option, above, since that's what I intended anyway. My intention was to replicate the CREATE TABLE message; I'm not sure how I failed on that particular point. Thank you guys for noticing and fixing it (along with all the other corrections).
Re: [HACKERS] Different error messages executing CREATE TABLE or ALTER TABLE to create a column xmin
On Thu, Jan 26, 2012 at 18:47, Robert Haas robertmh...@gmail.com wrote: OK, committed with that further change. Thank you, Robert! My first real contribution, even if tiny :-) Just a small nit to pick, though: Giuseppe Sucameli contributed to this patch but was not credited in the commit log.
[HACKERS] Different error messages executing CREATE TABLE or ALTER TABLE to create a column xmin
I took my first stab at hacking the sources to fix the bug reported here:
http://archives.postgresql.org/pgsql-bugs/2012-01/msg00152.php
It's such a simple patch but it took me several hours with Google and IRC
and I'm sure I did many things wrong. It seems to work as advertised,
though, so I'm submitting it.
I suppose since I have now submitted a patch, it is my moral obligation to
review at least one. I'm not sure how helpful I'll be, but I'll go bite
the bullet and sign myself up anyway.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
new file mode 100644
index cb8ac67..7555d19
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*** ATExecAddColumn(List **wqueue, AlteredTa
*** 4235,4240
--- 4235,4241
List *children;
ListCell *child;
AclResult aclresult;
+ HeapTuple attTuple;
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
*** ATExecAddColumn(List **wqueue, AlteredTa
*** 4314,4326
* this test is deliberately not attisdropped-aware, since if one tries to
* add a column matching a dropped column name, it's gonna fail anyway.
*/
! if (SearchSysCacheExists2(ATTNAME,
! ObjectIdGetDatum(myrelid),
! PointerGetDatum(colDef-colname)))
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_COLUMN),
! errmsg(column \%s\ of relation \%s\ already exists,
! colDef-colname, RelationGetRelationName(rel;
/* Determine the new attribute's number */
if (isOid)
--- 4315,4341
* this test is deliberately not attisdropped-aware, since if one tries to
* add a column matching a dropped column name, it's gonna fail anyway.
*/
! attTuple = SearchSysCache2(ATTNAME,
! ObjectIdGetDatum(myrelid),
! PointerGetDatum(colDef-colname));
!
! if (HeapTupleIsValid(attTuple))
! {
! int attnum;
! attnum = ((Form_pg_attribute) GETSTRUCT(attTuple))-attnum;
! if (attnum = 0)
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_COLUMN),
! errmsg(column \%s\ conflicts with a system column name,
! colDef-colname)));
! else
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_COLUMN),
! errmsg(column \%s\ of relation \%s\ already exists,
! colDef-colname, RelationGetRelationName(rel;
!
! ReleaseSysCache(attTuple);
! }
/* Determine the new attribute's number */
if (isOid)
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
new file mode 100644
index e992549..8385afb
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*** COMMENT ON TABLE tmp_wrong IS 'table com
*** 7,12
--- 7,14
ERROR: relation tmp_wrong does not exist
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
+ ERROR: column xmin conflicts with a system column name
ALTER TABLE tmp ADD COLUMN a int4 default 3;
ALTER TABLE tmp ADD COLUMN b name;
ALTER TABLE tmp ADD COLUMN c text;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
new file mode 100644
index d9bf08d..d4e4c49
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*** COMMENT ON TABLE tmp_wrong IS 'table com
*** 9,14
--- 9,16
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
+
ALTER TABLE tmp ADD COLUMN a int4 default 3;
ALTER TABLE tmp ADD COLUMN b name;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Documentation mistake
in Section 13.2.3 of the 9.1 docs [1], the follow sentence fragment can be found: using Serializable transactions will allow one transaction to commit and and will roll the other back Note the double and towards the end. (Is this the right list for this kind of report?) [1] http://www.postgresql.org/docs/9.1/static/transaction-iso.html#XACT-SERIALIZABLE
Re: [HACKERS] Inputting relative datetimes
On Thu, Aug 25, 2011 at 11:39, Dean Rasheed dean.a.rash...@gmail.comwrote: My first thought was to have some general way of adding or subtracting an interval at the end of an input timestamp, eg. by adding another couple of special values - plus interval and minus interval. This would allow things like: TIMESTAMPTZ 'today minus 5 days' TIMESTAMPTZ 'now plus 2 hours' Funny you should mention intervals... timestamptz 'today' - interval '5 days' timestamptz 'now' + interval '2 hours'
Re: [HACKERS] Regression tests versus the buildfarm environment
On Wed, Aug 11, 2010 at 06:42, Tom Lane t...@sss.pgh.pa.us wrote: I am not sure if there's anything very good we can do about the problem of pg_regress misidentifying the postmaster it's managed to connect to. A real solution would probably be much more trouble than it's worth, anyway. However, it does seem like we ought to be able to do something about two buildfarm critters defaulting to the same choice of port number. The buildfarm infrastructure goes to great lengths to pick nonconflicting port numbers for the installed postmasters it runs; but we're ignoring all that effort and just using a hardwired port number for make check. This is dumb. pg_regress does have a --port argument that can be used to override that default. I don't know whether the buildfarm script calls pg_regress directly or does make check. If the latter, we'd need to twiddle the Makefiles to allow a port number to get passed in. But this seems well worthwhile to me. Comments? We just put in the possibility to name the client connections. Would it be interesting to be able to name the server installation itself?
