Re: [HACKERS] Triggers on foreign tables
On Tue, Mar 18, 2014 at 09:31:06AM +0100, Ronan Dunklau wrote: > Le mardi 18 mars 2014 03:54:19 Kouhei Kaigai a écrit : > > > (1) To acquire the old tuple for UPDATE/DELETE operations, the patch > > > closely > parallels our handling for INSTEAD OF triggers on views. It > > > adds a wholerow resjunk attribute, from which it constructs a HeapTuple > > > before calling a trigger function. This loses the system columns, an > > > irrelevant > > > consideration for views but meaningful for foreign tables. postgres_fdw > > > maintains the "ctid" system column (t_self), but triggers will always see > > > an invalid t_self for the old tuple. One way to fix this is to pass > > > around > the old tuple data as ROW(ctid, oid, xmin, cmin, xmax, cmax, > > > tableoid, wholerow). That's fairly close to sufficient, but it doesn't > > > cover t_ctid. Frankly, I would like to find a principled excuse to not > > > worry about making foreign table system columns accessible from triggers. > > > Supporting system columns dramatically affects the mechanism, and what > > > trigger is likely to care? Unfortunately, that argument seems too weak. > > > Does anyone have a cleaner idea for keeping track of the system column > > > values or a stronger argument for not bothering? > > > > > > > Regarding to the first suggestion, > > I think, it is better not to care about system columns on foreign tables, > > because it fully depends on driver's implementation whether FDW fetches > > "ctid" from its data source, or not. > > Usually, system columns of foreign table, except for tableoid, are > > nonsense. > Because of implementation reason, postgres_fdw fetches "ctid" of > > remote tables on UPDATE / DELETE, it is not a common nature for all FDW > > drivers. For example, we can assume an implementation that uses primary key > > of remote table to identify the record to be updated or deleted. In this > > case, local "ctid" does not have meaningful value. > > So, fundamentally, we cannot guarantee FDW driver returns meaningful "ctid" > > or other system columns. > > > > I agree, I think it is somewhat clunky to have postgres_fdw use a feature > that > is basically meaningless for other FDWs. Looking at some threads in this > list, > it confused many people. My own reasoning for accepting omission of system columns is more along the lines of Robert's argument. Regardless, three folks voting to do so and none against suffices for me. I documented the system columns limitation, made the returningList change I mentioned, and committed the patch. > This is off-topic, but maybe we could devise an API allowing for local > "system > attributes" on foreign table. This would allow FDWs to carry attributes that > weren't declared as part of the table definition. This could then be used for > postgres_fdw ctid, as well as others foreign data wrappers equivalent of an > implicit "tuple id". We could, but I discourage it. System columns are a legacy feature; I doubt we'd choose that design afresh today. On the off-chance that you need the value of a remote system column, you can already declare an ordinary foreign table column for it. I raised the issue because it's inconsistent for RETURNING to convey system columns while tg_trigtuple/tg_newtuple do not, not because acquiring system columns from foreign tables is notably useful. 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] equalTupleDescs() ignores ccvalid/ccnoinherit
On Fri, Mar 21, 2014 at 06:59:05PM +, Simon Riggs wrote: > On 21 March 2014 18:26, Robert Haas wrote: > > >> Given the minor symptoms in released versions, I lean against a back-patch. > > > > FWIW, I'd lean toward a back-patch. It's probably not a big deal > > either way, but I have a hard time seeing what risk we're avoiding by > > not back-patching, and it seems potentially confusing to leave > > known-wrong logic floating around in older branches. > > Agreed. It could lead to some other bug by not fixing it. Fair enough. I've back-patched it. -- 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: Fwd: [HACKERS] Proposal: variant of regclass
On Sat, Mar 22, 2014 at 1:17 PM, Amit Kapila wrote: > On Wed, Feb 26, 2014 at 11:56 AM, Yugo Nagata wrote: >> Thanks for your a lot of comments. I revised the patch according to >> comments from Robert Haas and Marti Raudsepp. > > I have started looking into this patch and below are my > initial findings: > > 1. Dependency is not recorded when to_regclass is used, > however it is recorded when ::regclass is used. Below is > test to demonstrate this point. This change in behaviour is > neither discussed nor mentioned in docs along with patch. I think this is expected as per current design, because for functions, it will create dependency on function (funcid), but not on it's argument values. So I think for this behaviour, we might need to mention in docs that to_regclass() and other newly added functions will behave differently for creation of dependencies. Anyone has any objection for this behaviour difference between usage of ::regclass and to_regclass()? 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] Useless "Replica Identity: NOTHING" noise from psql \d
On Tue, Dec 17, 2013 at 09:37:09AM -0500, Robert Haas wrote:
> > Patch attached.
>
> I vote for showing it only with +, but regardless of whether the value
> matches the expected default. I'd keep the relkind test, though,
> because I think I noticed that it currently shows up for indexes,
> which is dumb.
Is this the patch you had in mind? I kept the pg_catalog filter. Do we
want to always show the replica identity line for \d+?
test=> \d+ test
Table "public.test"
Column | Type | Modifiers | Storage | Stats target | Description
+-+---+-+--+-
x | integer | | plain | |
Replica Identity: full
Has OIDs: no
I used lower-case for the value, rather than all-caps.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index a194ce7..a75fc82
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*** describeOneTableDetails(const char *sche
*** 2345,2358
printTableAddFooter(&cont, buf.data);
}
! if ((tableinfo.relkind == 'r' || tableinfo.relkind == 'm') &&
! tableinfo.relreplident != 'd' && tableinfo.relreplident != 'i')
{
const char *s = _("Replica Identity");
printfPQExpBuffer(&buf, "%s: %s",
s,
! tableinfo.relreplident == 'n' ? "NOTHING" : "FULL");
printTableAddFooter(&cont, buf.data);
}
--- 2345,2358
printTableAddFooter(&cont, buf.data);
}
! if (verbose && (tableinfo.relkind == 'r' || tableinfo.relkind == 'm') &&
! strcmp(schemaname, "pg_catalog") != 0)
{
const char *s = _("Replica Identity");
printfPQExpBuffer(&buf, "%s: %s",
s,
! tableinfo.relreplident == 'n' ? "nothing" : "full");
printTableAddFooter(&cont, buf.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] jsonb status
On Sat, Mar 22, 2014 at 5:22 PM, Bruce Momjian wrote: > What did you decide about hashing values in indexes vs. putting them in > literally? There are two GIN opclasses supplied. There is a default, which supports more operators (various "existence" operators - see the documentation). There is an alternative called jsonb_hash_ops that only supports containment, and performs considerably better than the default. Containment *is* the compelling operator to support, though - you can do rather a lot with it. This must be what you're referring to, since I recall you blogged about the response it got at pgConf.EU. Both are available. -- 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] jsonb status
On Sat, Mar 22, 2014 at 01:53:06PM -0700, Peter Geoghegan wrote: > Attached is v12. I think I've brought this as far as I can. > > This is mostly just bug fixes, and some additional refactoring. I've > incorporated Andres' feedback. The only points that I think worth > noting are: > > * The documentation has been significantly expanded to discuss > "containment" further, since it's a significant part of the feature. > This could probably use some polishing and general scrutiny, which is > something that Andrew may consider. I didn't have time to go over it > to the extent that I'd prefer. > > * I altered containment semantics slightly. Now, it is not possible > for a "raw scalar" to contain a single-element array of the same > scalar, while the inverse is still possible (raw scalars still contain > themselves too). This made sense to me, and is consistent with the > behavior of the B-Tree operator class, where a raw scalar is not equal > to a single-element array of the same scalar. Rather, array is greater > than the scalar on the basis of its type alone, as at every other > nesting level. The fact that an array can contain a raw scalar is > explicitly presented as an exception to the general principle that > containment needs to be at the same nesting level. > > I'm not going to go into the details of the bugs fixed, since no one > reported them here, and since they're trivial in nature. For full > details, you can review the log of our publicly accessible feature > branch. What did you decide about hashing values in indexes vs. putting them in literally? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Partial index locks
On 22 March 2014 16:28, Jim Nasby wrote: > On 3/21/14, 7:59 PM, Vik Fearing wrote: >> >> On 03/22/2014 01:43 AM, Thom Brown wrote: >>> >>> Hi, >>> >>> I've created a table with 1000 partial indexes. Each one matches >>> exactly one row based on the predicate WHERE id = . >>> >>> However, when I perform an UPDATE of a single row in a transaction, >>> I've noticed that all those partial indexes show up in pg_locks with >>> RowExclusiveLock. >>> >>> Only 2 of those indexes have a reference to the row: the primary key >>> and a single partial index. >>> >>> Is it necessary for a partial index that doesn't include the row to be >>> involved in locking? >> >> >> What if the update puts the row into one of the other indexes? > > > Also, why are you doing this in the first place? I'm guessing you measured > some non-trivial performance improvement from doing this; could you share > that with us? Heh, no. I was just experimenting with various things, and also trying to break stuff. -- Thom -- 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 blows up on BOM character sequence
On 3/21/14, 4:54 PM, Tom Lane wrote: Merlin Moncure writes: There is no way for psql to handle that case though unless you'd strip *all* BOMs encountered. Compounding this problem is that there's no practical way AFAIK to send multiple file to psql via single command line invocation. If you pass multiple -f arguments all but one is ignored. Well, that seems like a solvable but rather independent problem. I guess one issue is how you'd define the meaning of --single ... one transaction per run, or one per file? Well, if you're catting multiple files into psql -1, you'd get all the files in one transaction, right? So I'd say that's what should happen. -- 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] ALTER TABLE lock strength reduction patch is unsafe
On 2/26/14, 9:15 AM, Simon Riggs wrote: On 26 February 2014 13:38, Andres Freund wrote: >Hi, > >On 2014-02-26 07:32:45 +, Simon Riggs wrote: >> >* This definitely should include isolationtester tests actually >> > performing concurrent ALTER TABLEs. All that's currently there is >> > tests that the locklevel isn't too high, but not that it actually works. >> >>There is no concurrent behaviour here, hence no code that would be >>exercised by concurrent tests. > >Huh? There's most definitely new concurrent behaviour. Previously no >other backends could have a relation open (and locked) while it got >altered (which then sends out relcache invalidations). That's something >that should be tested. It has been. High volume concurrent testing has been performed, per Tom's original discussion upthread, but that's not part of the test suite. For other tests I have no guide as to how to write a set of automated regression tests. Anything could cause a failure, so I'd need to write an infinite set of tests to prove there is no bug*somewhere*. How many tests are required? 0, 1, 3, 30? ISTM that we don't want hand-written tests here, but rather generated tests that actually hit all potential cases. Obviously we'd never run that as part of normal reqression, but farm animals certainly could. -- 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] Partial index locks
On 3/21/14, 7:59 PM, Vik Fearing wrote: On 03/22/2014 01:43 AM, Thom Brown wrote: Hi, I've created a table with 1000 partial indexes. Each one matches exactly one row based on the predicate WHERE id = . However, when I perform an UPDATE of a single row in a transaction, I've noticed that all those partial indexes show up in pg_locks with RowExclusiveLock. Only 2 of those indexes have a reference to the row: the primary key and a single partial index. Is it necessary for a partial index that doesn't include the row to be involved in locking? What if the update puts the row into one of the other indexes? Also, why are you doing this in the first place? I'm guessing you measured some non-trivial performance improvement from doing this; could you share that with us? -- 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] psql blows up on BOM character sequence
On 3/21/14, 8:13 PM, David E. Wheeler wrote: On Mar 21, 2014, at 2:16 PM, Andrew Dunstan wrote: Surely if it were really a major annoyance, someone would have sent code to fix it during the last 4 years and more since the above. I suspect it's a minor annoyance :-) But by all means add it to the TODO list if it's not there already. I have cleaned up many a BOM added to files that made psql blow up. I think PGAdmin III was a culprit, though I’m not sure (I don’t use, it, cleaned up after coworkers who do). Yes, my coworker that figured out what the problem was said the culprit here is actually pgAdmin. :( -- 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
On 03/22/2014 04:00 PM, Tom Lane wrote:
On the other side, coding with the explicit cast helps guard against far
more dangerous coding errors, which the compiler will*not* help you with.
What if myextra is actually of type "int64 *"?
Indeed, neither "gcc -Wall -Wextra -std=c89 -pedantic" nor "clang
-Weverything -Wno-shadow -std=c89 -pedantic" issues a warning in such
case. "clang --analyze", however, does. Perhaps TenDRA would, if it ever
worked.
This message is meant to be merely informative, since I've put some
effort into this test. I'm not trying to argue.
#include
typedef long int int64;
int main(void)
{
int *myextra;
/* with explicit casting */
myextra = (int *) malloc(sizeof (int));
free(myextra);
/* with no explicit casting */
myextra = malloc(sizeof (int));
free(myextra);
/* myextra now becomes int64 */
{
int64 *myextra;
/* with explicit casting */
myextra = (int *) malloc(sizeof (int)); /* [1], [2]. and [3] warn here */
free(myextra);
/* with no explicit casting */
myextra = malloc(sizeof (int)); /* Only [3] warns here */
free(myextra);
}
return 0;
}
/*
1: gcc 4.8.2: gcc -Wall -Wextra -std=c89 -pedantic /tmp/test.c
2: clang 3.5.0: clang -Weverything -Wno-shadow -std=c89 -pedantic /tmp/test.c
3: clang 3.5.0: clang --analyze /tmp/test.c
*/
--
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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Piotr Stefaniak writes: > Apart from what the page says, I also think of casting malloc() as bad > style and felt the need to bring this up. Well, that's a value judgement I don't happen to agree with. Yeah, it'd be better if the language design were such that we could avoid explicit casting everywhere, but in this context casting is less risky than not casting. > So perhaps this alternative: >myextra = malloc(sizeof *myextra); [ shrug... ] That's about a wash for this exact use case, but it gets messy as soon as the lefthand side is anything more complicated than a simple variable name. And it doesn't scale to cases where the malloc result isn't directly assigned to anything --- for example, what if you want to pass the result of palloc() directly to some other function, or return it from the current function? The bigger picture though is that the style with the explicit cast is already extremely widely used in Postgres. That being the case, conforming to project style is better than using some inconsistent convention, regardless of your personal views about whether there's a better way to do it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
On 03/22/2014 04:00 PM, Tom Lane wrote: That argument is entirely bogus, as it considers only one possible way in which the call could be wrong; a way that is of very low probability in PG usage, since we include in our core headers. Besides which, as noted in the page itself, most modern compilers would warn anyway if you forgot the inclusion. Apart from what the page says, I also think of casting malloc() as bad style and felt the need to bring this up. But since you pointed out why you don't want to remove the cast, I withdraw my previous suggestion. On the other side, coding with the explicit cast helps guard against far more dangerous coding errors, which the compiler will *not* help you with. What if myextra is actually of type "int64 *"? In that case you probably meant to make enough space for an int64 not an int. But without the cast, you won't be told you did anything wrong. This is a particular hazard if you change your mind later on about the type of myextra. (A colleague at Salesforce got burnt in exactly that way, just a couple days ago.) So perhaps this alternative: myextra = malloc(sizeof *myextra); PS. Coding style matters to me, but I was and still am far from insisting on anything. -- 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] Inheritance of foregn key constraints.
Andrzej Mazurkiewicz writes: >> So in other words, somebody could (accidentally or maliciously) break the >> constraint by dropping one of its implementation triggers. I doubt that's >> acceptable. > The present postgres behavior ALLOWS accidental or malicious break the > constraint by dropping one of its implementation triggers. Please ref. to the > following example. > The following script has been run by the postgres user. Well, right there you lost me, because superusers are exempt from all permissions checks by definition; and in particular, direct manipulations of the system catalogs by superusers are always out of scope for discussions of what the system should try to protect itself against. (Try "delete from pg_proc;" in a scratch database sometime.) My point is that without the internal dependency, a normal user could do standard SQL commands (ie DROP TRIGGER) and break the FK that way. That's the case that's not acceptable. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Partial index locks
On 22 March 2014 15:04, Tom Lane wrote: > Thom Brown writes: >> On 22 March 2014 05:32, Tom Lane wrote: >>> Yes. You can't determine whether the index needs to get a new entry >>> without examining its metadata, and that's what the lock is mainly about. > >> I see. Why does this apply to deletes too? > > The executor doesn't take locks on indexes for a delete. I think the > planner probably does, though, since it wants to look at all the indexes > to see if any can be used to satisfy WHERE searches. > > Possibly it would be worth hacking the planner to only take > AccessShareLock not RowExclusiveLock on target indexes in DELETE. > I can't get very excited about that though; in what circumstances > would it actually make a difference? Well I wasn't looking for things to optimise, so much as trying to understand the logic behind the existing behaviour. But thanks for the explanation. -- Thom -- 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] Partial index locks
Thom Brown writes: > On 22 March 2014 05:32, Tom Lane wrote: >> Yes. You can't determine whether the index needs to get a new entry >> without examining its metadata, and that's what the lock is mainly about. > I see. Why does this apply to deletes too? The executor doesn't take locks on indexes for a delete. I think the planner probably does, though, since it wants to look at all the indexes to see if any can be used to satisfy WHERE searches. Possibly it would be worth hacking the planner to only take AccessShareLock not RowExclusiveLock on target indexes in DELETE. I can't get very excited about that though; in what circumstances would it actually make a difference? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Piotr Stefaniak writes: >>> +myextra = (int *) malloc(sizeof(int)); > Please consider not casting malloc(). See That code is per project style, and should stay that way. > http://c-faq.com/malloc/mallocnocast.html That argument is entirely bogus, as it considers only one possible way in which the call could be wrong; a way that is of very low probability in PG usage, since we include in our core headers. Besides which, as noted in the page itself, most modern compilers would warn anyway if you forgot the inclusion. On the other side, coding with the explicit cast helps guard against far more dangerous coding errors, which the compiler will *not* help you with. What if myextra is actually of type "int64 *"? In that case you probably meant to make enough space for an int64 not an int. But without the cast, you won't be told you did anything wrong. This is a particular hazard if you change your mind later on about the type of myextra. (A colleague at Salesforce got burnt in exactly that way, just a couple days ago.) So, general policy around here is that malloc and palloc calls should look like ptr = (foo *) malloc(n * sizeof(foo)); so that there's a direct, visible connection between the size calculation and the type of the resulting pointer. I'm aware that there are some places in the code that fail to do this, but they are not models to emulate. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inheritance of foregn key constraints.
Good Morning. 1. At the beginning some explanations. I am a lazy person that tries not to reinvent a wheel. So I try to use postgres way of automatic processing, i. e. automatic removing dependent objects (which I consider an elegant solution and I really like it). A a result, I have used the pg_depend table to force to remove dependent entries. 2. At the moment the following behavior is a standard one for postgres. - a child table (inheriting form a parent table(s) no FK) can be dropped; - a referred table (master) can be freely dropped with a CASCADE option (causing dropping of the FK); - a referring table (detail) can be freely dropped (causing automatic dropping of the FK); - a CHECK constraint is inherited and the inheritance can be removed freely although leaving the CHECK constraint (no FK); - an inherited table with CHECK constraint can be freely dropped (no FK); - inheritance can be added for existing tables and it can be removed (no FK). 3. The following decisions should be taken for the FK inheritance (partly common issues, however I try to be precise). - (GENERAL statement) Are modifications of a master side hierarchy (a referred side) allowed without dropping the FK? - (GENERAL statement) Are modifications of a detail side hierarchy (a referred side) allowed without dropping the FK? - Is detaching childs allowed in the master and detail hierarchy without dropping the FK? - Is dropping tables allowed in the master and detail hierarchy without dropping the FK? - Is adding inheritance allowed to the master and detail hierarchies without dropping the FK? - Is creating inheriting tables allowed in the master and detail hierarchies without dropping the FK? It would be good if the decisions were consistent with the existing behavior. The consequences of the decisions are rather far going. For large databases adding the FK constraint might last hours or days or perhaps weeks. For my databases, although such modification would last hours and sometimes I have strange and changing ideas - I can live with those hours. Personally I would vote that the above modifications SHOULD BE ALLOWED. Simply, because we do not drop the whole master or detail hierarchy but modify it and it gives certain flexibility to manipulating the schema. The above flexibility is similar to adding inheritance to the existing tables and removing inheritance for them. We do not need to create another inherited table and to move data into it from the existing table. 3. Perhaps , after making the above decisions, a discussion about an implementing changes should be continued. 4. > > My patch need one change that might be of significance. > > A type of the depencencies (pg_depend) among the FK constraint > > (pg_constraint) and the corresponding "RI_ConstraintTrigger" triggers has > > to be changed from DEPENDENCY_INTERNAL to DEPENDENCY_AUTO. > > So in other words, somebody could (accidentally or maliciously) break the > constraint by dropping one of its implementation triggers. I doubt that's > acceptable. The present postgres behavior ALLOWS accidental or malicious break the constraint by dropping one of its implementation triggers. Please ref. to the following example. The following script has been run by the postgres user. CREATE DATABASE lipa; \c lipa CREATE TABLE master (master_a int, CONSTRAINT pk_master PRIMARY KEY (master_a)); CREATE TABLE detail (master_a int, detail_a int, CONSTRAINT fk0_detail FOREIGN KEY (master_a) REFERENCES master(master_a)); SELECT oid, tgrelid, tgname FROM pg_trigger ; DELETE FROM pg_trigger WHERE oid = (SELECT min(oid) FROM pg_trigger WHERE tgname LIKE 'RI_ConstraintTrigger%' LIMIT 1); SELECT oid, tgrelid, tgname FROM pg_trigger ; DROP TABLE detail; DROP TABLE master; \c postgres DROP DATABASE lipa; The results of the run are as follows. psql -f test-malicious-dropping-FK-triggers.sql postgres CREATE DATABASE You are now connected to database "lipa" as user "postgres". CREATE TABLE CREATE TABLE oid | tgrelid |tgname ---+-+-- 39898 | 39889 | RI_ConstraintTrigger_a_39898 39899 | 39889 | RI_ConstraintTrigger_a_39899 39900 | 39894 | RI_ConstraintTrigger_c_39900 39901 | 39894 | RI_ConstraintTrigger_c_39901 (4 rows) DELETE 1 oid | tgrelid |tgname ---+-+-- 39899 | 39889 | RI_ConstraintTrigger_a_39899 39900 | 39894 | RI_ConstraintTrigger_c_39900 39901 | 39894 | RI_ConstraintTrigger_c_39901 (3 rows) psql:test-malicious-dropping-FK-triggers.sql:8: ERROR: could not find tuple for trigger 39898 psql:test-malicious-dropping-FK-triggers.sql:9: ERROR: could not find tuple for trigger 39898 You are now connected to database "postgres" as user "postgres". DROP DATABASE > > > If this modification is not applied, the detail child table cannot be > > dropped without prevous dropping the whole FK constraint because
Re: [HACKERS] Partial index locks
On 22 March 2014 05:32, Tom Lane wrote: > Thom Brown writes: >> Is it necessary for a partial index that doesn't include the row to be >> involved in locking? > > Yes. You can't determine whether the index needs to get a new entry > without examining its metadata, and that's what the lock is mainly about. I see. Why does this apply to deletes too? > The only possible alternative would be to take the minimum possible > lock (AccessShareLock) on each index so its metadata would hold still, > and then upgrade that to RowExclusiveLock on the one(s) we find need > insertions. This is not better; it means *more* lock management traffic > not less, and lock upgrades increase the potential for deadlocks. Yes, I can see that wouldn't be an improvement. -- Thom -- 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] [RFC] What should we do for reliable WAL archiving?
On Sat, Mar 22, 2014 at 06:22:37AM +0900, MauMau wrote: > From: "Jeff Janes" > >Do people really just copy the files from one directory of local > >storage to > >another directory of local storage? I don't see the point of that. > > It makes sense to archive WAL to a directory of local storage for > media recovery. Here, the local storage is a different disk drive > which is directly attached to the database server or directly > connected through SAN. I'm one of those peope. They are archived into a local directory in preparation for an rsync over ssh. > >The recommendation is to refuse to overwrite an existing file of the same > >name, and exit with failure. Which essentially brings archiving > >to a halt, > >because it keeps trying but it will keep failing. If we make a custom > >version, one thing it should do is determine if the existing archived file > >is just a truncated version of the attempting-to-be archived file, and if > >so overwrite it. Because if the first archival command fails with a > >network glitch, it can leave behind a partial file. > > What I'm trying to address is just an alternative to cp/copy which > fsyncs a file. It just overwrites an existing file. I ran into a related problem with cp, where halfway the copy the disk was full and I was left with half a WAL file. This caused the rsync to copy only half a file and the replication broke. This is clearly a recoverable situation, but it didn't recover in this case. > Yes, you're right, the failed archive attempt leaves behind a > partial file which causes subsequent attempts to fail, if you follow > the PG manual. That's another undesirable point in the current doc. > To overcome this, someone on this ML recommended me to do "cp %p > /archive/dir/%f.tmp && mv /archive/dir/%f.tmp /archive/dir/%f". > Does this solve your problem? This would probably have handled it, but I find it odd that there's program to handle restoring of archives properly, but on the archiving side you have to cobble together your own shell scripts which fail in various corner cases. I'd love a program that just Did The Right Thing. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
> +myextra = (int *) malloc(sizeof(int)); Please consider not casting malloc(). See http://c-faq.com/malloc/mallocnocast.html -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: [HACKERS] Proposal: variant of regclass
On Wed, Feb 26, 2014 at 11:56 AM, Yugo Nagata wrote:
> Thanks for your a lot of comments. I revised the patch according to
> comments from Robert Haas and Marti Raudsepp.
I have started looking into this patch and below are my
initial findings:
1. Dependency is not recorded when to_regclass is used,
however it is recorded when ::regclass is used. Below is
test to demonstrate this point. This change in behaviour is
neither discussed nor mentioned in docs along with patch.
usage of ::regclass
create sequence my_seq;
create table t1 (c1 int, c2 int default 'my_seq'::regclass);
insert into t1 values(1);
insert into t1 values(2);
select * from t1;
c1 | c2
+---
1 | 16390
2 | 16390
(2 rows)
drop sequence my_seq;
ERROR: cannot drop sequence my_seq because other
objects depend on it
DETAIL: default for table t1 column c2 depends on
sequence my_seq
HINT: Use DROP ... CASCADE to drop the dependent
objects too.
Check pg_depend has entry for dependency of default
value of table-column on seq.
postgres=# select * from pg_depend where refobjid = 16390;
classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
-+---+--++--+-+-
1247 | 16391 |0 | 1259 |16390 | 0 | i
2604 | 16395 |0 | 1259 |16390 | 0 | n
(2 rows)
postgres=# select oid,adrelid from pg_attrdef;
oid | adrelid
---+-
16395 | 16392
(1 row)
usage of to_regclass
create sequence my_seq;
create table t1 (c1 int, c2 int default to_regclass('my_seq'));
insert into t1 values(1);
insert into t1 values(2);
select * from t1;
c1 | c2
+---
1 | 16396
2 | 16396
select * from pg_depend where refobjid = 16396;
classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
-+---+--++--+-+-
1247 | 16397 |0 | 1259 |16396 | 0 | i
(1 row)
There is only one entry for pg_type which means the
dependent object on sequence is only its type, so it will
allow to drop sequence.
postgres=# drop sequence my_seq;
DROP SEQUENCE
2.
! if (!((Form_pg_type) GETSTRUCT(tup))->typisdefined)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("type \"%s\" is only a shell",
! TypeNameToString(typeName)),
! parser_errposition(NULL, typeName->location)));
In this case it is not exactly same as object does not exist
so I think we should not avoid error in this case
and infact OID is valid for such a case, but in else part
you have assigned it as InvalidOid which seems to be wrong.
3.
regproc_guts()
! {
! if (!missing_ok)
! ereport(ERROR,
! (errcode(ERRCODE_AMBIGUOUS_FUNCTION),
! errmsg("more than one function named \"%s\"",
! pro_name_or_oid)));
! return false;
! }
In to_regproc(), this patch is trying to supress error other
"object-name-not-found" which as far as I could read the thread
was not the original idea and even the description in docs
says only about "object-name-not-found" case.
Doc Description
+ similar to the regclass, regproc, regoper and regtype casts, except
+ that they return NULL when the object is not found, instead of raising
+ an error.
Even if you want to avoid the other error('s) like
above, then I think it's better to mention the same in docs.
I am bit worried, how is user going to distinguish between
the cases when object-not-found and more-than-one-object.
I think such a problem would not arise if we write a function
for regprocedure rather than for regproc, also manual says regprocedure
is more appropriate for most usecases.
http://www.postgresql.org/docs/devel/static/datatype-oid.html
Also I think one other advantage for doing so is that we
don't need to pass missing_ok paramter to some of the functions
like FuncnameGetCandidates(), OpernameGetCandidates().
I understand that you might be using regproc/regoper or might have
a usecase for it, but is it possible for you to use regprocedure/regoperator
instead of regproc/regoper?
4.
+regclass
+get the table oid
Shouldn't it be better to describe it as "get the relation oid" as
it can give oid for other objects like index as well.
5.
+
+ to_regclass, to_regproc, to_regoper and to_regtype are functions
+ similar to the regclass, regproc, regoper and regtype casts, except
In above description, it is better to use 'object identifier types'
rather than 'casts'.
6.
! * Guts of regoperin and to_regproc.
Here it should be regprocin
7.
* If not found and missing_ok is true, returns false instead of raising
an error.
Above line is more than 80 chars, it should be less than
80 char limit. This needs to be corrected whereever this line is used.
8.
! * Guts of regtypein and to_regtype.
! * If the classname is found, returns true and the OID is stored into
*typid_p.
typo in *If the classname is found*, it should be type is found.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterpr
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe Reply-To:
On 21 March 2014 16:11, Simon Riggs wrote: >>> + * Be careful to ensure this function is called for Tables and Indexes >>> only. >>> + * It is not currently safe to be called for Views because security_barrier >>> + * is listed as an option and so would be allowed to be set at a level >>> lower >>> + * than AccessExclusiveLock, which would not be correct. >> >> This statement is accepted and takes only ShareUpdateExclusiveLock: >> >> alter table information_schema.triggers set (security_barrier = true); >> I suggest adding a LOCKMODE field to relopt_gen and adding a >> reloptions_locklevel() function to determine the required level from an >> options list. That puts control of the lock level near the code that >> understands the implications for each option. You can then revert the >> addition of AlterViewInternal() and some of the utility.c changes. > > Sure, that's how we code it, but I'm not sure we should introduce that > feature. The above weirdness is not itself justification. OK, will follow this path. It's a good idea since it encourages authors of new "options" to consider properly the lock level that will be required. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe Reply-To:
On 21 March 2014 23:36, Tom Lane wrote: > Simon Riggs writes: >> On 21 March 2014 20:58, Noah Misch wrote: >>> It's not the behavior I would choose for a new product, but I can't see >>> benefits sufficient to overturn previous decisions to keep it. > >> Speechless > > The key argument for not "fixing" this is that it would break existing > pg_dump files. That's a pretty hard argument to overcome, unfortunately, > even if you're willing to blow off the possibility that client > applications might contain similar shortcuts. We still do our best to > read dump files from the 7.0 era (see ConvertTriggerToFK() for one example > of going above and beyond for that); and every so often we do hear of > people trying to get data out of such ancient servers. So even if you > went and fixed pg_dump tomorrow, it'd probably be ten or fifteen years > before people would let you stop reading dumps from existing versions. Noah had already convinced me, but thank you for explaining the issue behind that. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
