Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Query Optimizer + Parallel Operators
This is kinda scary . Oracle guy asking for PostgreSQL documentation and internals of the optimizer. On Thu, Jan 27, 2011 at 12:14 AM, Josh Berkus wrote: > Felix, > > > I'm interested in the query optimizer of PostgreSQL DB. Where could I > > find useful documentation or could you send me a pointer in the source > code? > > > > What kind of parallelism does PostgreSQL use for operators, like > > selection or join? > > Normally we're very helpful with this kind of information ... it's all > public after all ... but I have to say that the domain you're e-mailing > from makes it a little hard to give you a direct answer. > > Could you maybe give us a little information about what you want this > information for? > > -- > -- Josh Berkus > PostgreSQL Experts Inc. > http://www.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] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 09:35:24AM -0500, Robert Haas wrote: > On Mon, Jan 24, 2011 at 11:13 PM, Noah Misch wrote: > >> > This helps on conversions like varchar(4)->varchar(8) and text->xml. > >> > >> I've read through this patch somewhat. ?As I believe Tom also > >> commented previously, exemptor is a pretty bad choice of name. > > > > I spent awhile with a thesaurus before coining that. ?Since Tom's comment, > > I've > > been hoping the next person to complain would at least suggest a better > > name. > > Shooting in the dark on a nomenclature issue isn't fun, and I'm not picky. > CREATE CAST (source_type AS target_type) > WITH FUNCTION function_name (argument_type, [, ...]) > [ ANALYZE USING function_name ] > [ AS ASSIGNMENT | AS IMPLICIT ] Thanks for writing about it. Seems the rest of the thread converged pretty well on a set of viable name candidates. > >>?I think what this patch is trying to do is tag the > >> call to the cast function with the information that we might not need > >> to call it, but ISTM it would be better to just notice that the > >> function call is unnecessary and insert a RelabelType node instead. > > > > This patch does exactly that for varchar(4) -> varchar(8) and similar cases. > > Oh, uh, err... OK, I obviously haven't understood it well enough. > I'll look at it some more, but if there's anything you can write up to > explain what you changed and why, that would be helpful. Based on downthread discussion, I figure this will all change a good deal. I'll still briefly explain the patch as written. Most of the patch is plumbing to support the new syntax, catalog entries, and FuncExpr field. The important changes are in parse_coerce.c. I modified find_coercion_pathway() and find_typmod_coercion_function() to retrieve pg_cast.castexemptor alongside pg_cast.castfunc. Their callers (coerce_type() and coerce_type_typmod(), respectively) then call the new apply_exemptor_function(), which calls the exemptor function, if any, returns the value to place in FuncExpr.funcexempt, and possibly updates the CoercionPathType the caller is about to use. build_coercion_expression(), unchanged except to populate FuncExpr.funcexempt, remains responsible for creating the appropriate node (RelabelType, FuncExpr). Finally, I change GetCoerceExemptions to use FuncExpr.funcexempt. > I think I'm > also having trouble with the fact that these patches don't apply > cleanly against the master branch, because they're stacked up on each > other. Maybe this will be easier once we get more of the ALTER TYPE 2 > stuff in. It's certainly tricky to review a bunch of patches in parallel when they have a serial dependency chain. I'll merge the at0 and at2 bits per your request to see what we can do there. > > As for performance, coerce_to_target_type() is certainly in wide use, but > > it's > > not exactly a hot path. ?I prepared and ran the attached > > bench-coerce-worst.sql > > and bench-coerce-best.sql. ?Both are quite artificial. ?The first one > > basically > > asks "Can we measure the new overhead at all?" ?The second one asks "Would > > any > > ordinary query benefit from removing a superfluous cast from an expression > > tree?" ?The worst case had an 4% _speedup_, and the best case had a 27% > > speedup, > > suggesting answers of "no" and "yes (but not dramatically)". ?The > > "worst-case" > > speedup doesn't make sense, so it's probably an artifact of some recent > > change > > on master creating a larger performance dip in this pathological test case. > > ?I > > could rebase the patch and rerun the benchmark, but I doubt an exact figure > > would be much more meaningful. ?Unfortunately, I can't think of any > > more-natural > > tests (help welcome) that would still illustrate any performance difference. > > That certainly seems promising, but I don't understand how the new > code can be faster on your constructed worst case. Thoughts? I hadn't merged master into my at3 branch in awhile; my best guess is that some recent change in master slowed that test case down by about 4%. I could merge up to confirm that theory. Again, though, it's an abjectly silly test case. Even if the test showed a 50% slowdown, we wouldn't have much cause for concern. Perhaps a 300% slowdown would have been notable. > I'm more concerned about modularity than performance. Sticking a > field into every FuncExpr so that if it happens to get passed to an > ALTER TABLE statement we can pull the flag out seems really ugly to > me. Understood. I agree that it's awfully specialized to be hanging around in FuncExpr. Doing it this way seemed to minimize the global quantity of ugly code, but you're right -- better to have somewhat-ugly code in tablecmds.c than to expose this in FuncExpr. Most of the benefit of the current approach will be gone when the optimization moves to eval_const_expressions(), anyway. One way or another, the next version will not do this. Thanks, nm -- Sent via
Re: [HACKERS] sepgsql contrib module
(2011/01/27 0:25), Robert Haas wrote: > 2011/1/25 KaiGai Kohei: >> (2011/01/26 12:23), KaiGai Kohei wrote: > Yikes. On further examination, exec_object_restorecon() is pretty > bogus. Surely you need some calls to quote_literal_cstr() in there > someplace. >>> Are you concerning about the object name being supplied to >>> selabel_lookup_raw() in exec_object_restorecon()? >>> I also think this quoting you suggested is reasonable. >>> >> How about the case when the object name only contains alphabet and >> numerical characters? > > Oh, quote_literal_cstr() is the wrong function - these are > identifiers, not literals. So we should use quote_identifier(). > OK, I did with quote_identifier(). The attached patch fixes up several stuffs in sepgsql module. - The object names being supplied to selabel_lookup_raw() to lookup initial labels become qualified by quote_identifier(), if necessary. - On access violation, sepgsql_check_perms() records audit logs. It contains object name being referenced. It became generated using getObjectDescription(). - Also, sepgsql_audit_log() becomes to quote the supplied object name, because it may contains white-space. - Error messages become obtaining "%m", when the error was originated from the libselinux interfaces. It will provides DBA a hint why interactions with SELinux does not work well. - Documentation was updated to suggest users to install libselinux v2.0.93 or later, because it used newer features than ones provided in v2.0.80. - Regression Test was updated, because of error message updates. Thanks, -- KaiGai Kohei sepgsql-v9.1-fixup.1.patch Description: application/octect-stream -- 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] Include WAL in base backup
On Wed, Jan 26, 2011 at 5:17 AM, Magnus Hagander wrote: > We should, and the easiest way is to actually use XLogRead() since the > code is already there. How about the way in this patch? Thanks for the update. I reread the patch. + MemSet(&statbuf, 0, sizeof(statbuf)); + statbuf.st_mode=S_IRUSR | S_IWUSR; +#ifndef WIN32 + statbuf.st_uid=getuid(); + statbuf.st_gid=getgid(); +#endif + statbuf.st_size=XLogSegSize; + statbuf.st_mtime=time(NULL); Can you put a space around "="? In the multiple-backups patch, Heikki uses geteuid and getegid for the same purpose instead of getuid and getgid, as follows. ! /* Windows doesn't have the concept of uid and gid */ ! #ifdef WIN32 ! statbuf.st_uid = 0; ! statbuf.st_gid = 0; ! #else ! statbuf.st_uid = geteuid(); ! statbuf.st_gid = getegid(); ! #endif ! statbuf.st_mtime = time(NULL); ! statbuf.st_mode = S_IRUSR | S_IWUSR; ! statbuf.st_size = len; Which is correct? Since we cannot start the PostgreSQL when getuid != geteuid, I don't think that there is really difference between getuid and geteuid. But other code always uses geteuid, so I think that geteuid should be used here instead of getuid for the sake of consistency. OTOH, I'm not sure if there is really difference between getgid and getegid in the backend (though I guess getgid == getegid because getuid == geteuid), and which should be used here. What is your opinion? + XLogFileName(xlogname, ThisTimeLineID, logid, logseg); + + sprintf(fn, "./pg_xlog/%s", xlogname); + _tarWriteHeader(fn, NULL, &statbuf); Can we use XLogFilePath? instead? Because sendFile has not been used. + XLByteToSeg(endptr, endlogid, endlogseg); + /* Have we reached our stop position yet? */ + if (logid > endlogid || + (logid == endlogid && logseg >= endlogseg)) + break; What I said in upthread is wrong. We should use XLByteToPrevSeg for endptr and check "logseg > endlogseg". Otherwise, if endptr is not a boundary byte, endlogid/endlogseg indicates the last necessary WAL file, but it's not sent. + if (percent > 100) + percent = 100; I'm not sure if this is good idea because the total received can go further than the estimated size though the percentage stops at 100. This looks more confusing than the previous behavior. Anyway, I think that we need to document about the combination of -P and -x in pg_basebackup. In pg_basebackup.sgml --checkpoint fast|spread Though this is not the problem of the patch, I found the inconsistency of the descriptions about options of pg_basebackup. We should s/--checkpoint/--checkpoint= Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ERROR: unexpected data beyond EOF ... on NFS mounted PGDATA (SOLVED)
I've been working closely with Black Duck Software, and their customer, to get to the bottom of $subject, and we have just declared success. Here is a summary of the problem and solution for the archives. The end-customer has a fairly beefy server, lots of RAM and CPUs, and is running an I/O intensive application that takes hours to complete. PGDATA resides on an NFS mounted NetApp. The OS is x86_64 RHEL, and was up to date with the latest kernel. They had been experiencing the "data beyond EOF" ERROR fairly often, but sporadically, for some time. We installed an instrumented version of Postgres that pointed toward a kernel bug -- specifically lseek(...,SEEK_END) returning the wrong result, consistent with postgres source code comments (although those were written in reference to a much older kernel). Fortunately for us, the kernel NFS client maintainer, Trond Myklebust, happened to be involved in this investigation, and he was able to find and fix the kernel bugs at the root of this problem. We have now finished sufficient testing to convince ourselves that Trond's patch indeed solves the problem at hand, at least in the form we have been experiencing. Part of his patch was a backport from newer kernels, but part was completely new. He has submitted (or will) the new part upstream for inclusion in future kernels, and submitted a bug report to Red Hat so that hopefully the patch will be included in updated kernel RPMs from Red Hat. For reference, the bug report can be found here: https://bugzilla.redhat.com/show_bug.cgi?id=672981 Trond was also kind enough to provide a patched version of the current RHEL kernel. An x86_64 and source RPM are available here in case someone has an immediate need: http://www.joeconway.com/rpms/kernel-2.6.18-238.el5.nfslseekfixv2.x86_64.rpm http://www.joeconway.com/rpms/kernel-2.6.18-238.el5.nfslseekfixv2.src.rpm HTH, Joe -- Joe Conway credativ LLC: http://www.credativ.us Linux, PostgreSQL, and general Open Source Training, Service, Consulting, & 24x7 Support signature.asc Description: OpenPGP digital signature
Re: [HACKERS] log_checkpoints and restartpoint
On Wed, Jan 26, 2011 at 7:59 PM, Simon Riggs wrote: > On Wed, 2011-01-26 at 13:14 +0900, Fujii Masao wrote: > >> When log_checkpoints is enabled, checkpoint logs the number of >> WAL files created/deleted/recycled, but restartpoint doesn't. >> This is OK before 9.0 because restartpoint had never created/ >> deleted/recycled WAL files. But, in 9.0 or later, restartpoint might >> do that while walreceiver is running. So I think that it's useful to >> log those numbers even in restartpoint in the now. >> >> The attached patch changes LogCheckpointEnd so that it logs >> the number of WAL files created/deleted/recycled even in >> restartpoint. >> >> And I found the problem about the initialization of CheckpointStats >> struct. The attached patch also fixes this. > > Thanks. > > Can you add to CF app so we can track this as well? It's easier to track > everything in one place. Sure. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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 TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 07:52:10PM -0500, Tom Lane wrote: > Noah Misch writes: > > text -> xml > > BTW, that reminds me of something that I think was mentioned way back > when, but absolutely fails to fit into any of the frameworks discussed > here: the mere fact that a type is binary-compatible (with or without > checking) doesn't make it compatible enough to not have to recreate > indexes. Where and how are we going to have a wart to determine if > that's necessary? Design (section 3): http://archives.postgresql.org/message-id/20101229125625.ga27...@tornado.gateway.2wire.net Implementation: http://archives.postgresql.org/message-id/20110113230124.ga18...@tornado.gateway.2wire.net [disclaimer: I've yet to post an updated version fixing a localized bug in this patch] I ended up making no attempt to optimize indexes that have predicates or expression columns; we'll just rebuild them every time. Aside from that, I failed to find an index on built-in types that requires a rebuild after a type change optimized by this patch stack. So, the entire wart is really for the benefit of third-party types that might need it. > And if the answer is "rebuild indexes whenever the > data type changes", isn't that a further big dent in the argument that > it's worth avoiding a table rewrite? No. Rewriting the table means rebuilding *all* indexes, but the worst case for a non-rewrite type change is to rebuild all indexes that depend on the changed column. That's a large win by itself, but we'll usually do even better. > A text->xml replacement is going > to be far from cheap anyway. It's tough to generalize. You can certainly construct a pathological case with minimal win, but you can just as well construct the opposite. Consider a wide table with a narrow XML column. Consider a usually-NULL XML column. nm -- 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] new compiler warnings
Tom Lane wrote: > Bruce Momjian writes: > > I can remove this warning by casting the pointer to (void *), rather > > than (const void *) because that is what the prototype uses on my system > > uses (libz.so.1.1.4): > > > ZEXTERN int ZEXPORTgzwrite OF((gzFile file, > >const voidp buf, unsigned len)); > > BTW, I don't understand why that fixes it for you either. As you can > see, gzwrite *is* declared with const. The reason why you're getting a > warning is that zconf.h #define's const as nothing unless it thinks > you're on an ANSI compiler (and the difference between 1.1.4 and 1.2.3 > is mostly that the former's test for ANSI-ness is brain dead). But if > you're compiling that #define then const or lack of it should mean > nothing to you. Let's wait and see if anyone else complains; I have adjusted things here. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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 TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 07:44:43PM -0500, Tom Lane wrote: > > numeric(8,2) -> numeric(7,2) > > varbit(8) -> varbit(7) > > text -> xml > > But how often do those really come up? I'll speak from my own experience, having little idea of the larger community experience on this one. I usually don't even contemplate changes like this on nontrivial tables, because the pain of the downtime usually won't make up for getting the schema just like I want it. Cases that I can't discard on those grounds are fairly rare. As an order-of-magnitude estimate, I'd throw out one instance per DBA-annum among the DBAs whose work I witness. > And do you really save that > much? The table still has to be locked against other users, so you're > still down, and you're still doing all the reads and computation. I > don't deny that saving the writes is worth something; I just don't agree > that it's worth the development and maintenance effort that such a wart > is going to cost us. User-exposed features are *expensive*. If you have no indexes, you still save 50-75% of the cost by just reading and computing, not rewriting. Each index you add, even if it doesn't involve the column, pushes that advantage even further. With a typical handful of indexes, a 95+% cost savings is common enough. If we implemented ALTER TABLE ... SET DATA TYPE ... IMPLICIT, I'd agree that the marginal value of automatically detecting the above three cases would not justify the cost. -- 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 TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 7:44 PM, Tom Lane wrote: > But how often do those really come up? And do you really save that > much? The table still has to be locked against other users, so you're > still down, and you're still doing all the reads and computation. I > don't deny that saving the writes is worth something; I just don't agree > that it's worth the development and maintenance effort that such a wart > is going to cost us. User-exposed features are *expensive*. I would think that text -> [something that's still a varlena but with tighter validation] would be quite common. -- 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] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane wrote: > Robert Haas writes: >> Well, I guess my thought was that we what we were doing is extending >> the coercion system to be able to make decisions based on both type >> OID and typemod. > > Well, that's an interesting thought, but the proposal at hand is far > more limited than that --- it's only an optimization that can be applied > in limited situations. If we want to do something like what you're > suggesting here, it's not going to get done for 9.1. Eh, why is this not entirely straightforward? *scratches head* -- 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] new compiler warnings
Bruce Momjian writes: > I can remove this warning by casting the pointer to (void *), rather > than (const void *) because that is what the prototype uses on my system > uses (libz.so.1.1.4): > ZEXTERN int ZEXPORTgzwrite OF((gzFile file, > const voidp buf, unsigned len)); BTW, I don't understand why that fixes it for you either. As you can see, gzwrite *is* declared with const. The reason why you're getting a warning is that zconf.h #define's const as nothing unless it thinks you're on an ANSI compiler (and the difference between 1.1.4 and 1.2.3 is mostly that the former's test for ANSI-ness is brain dead). But if you're compiling that #define then const or lack of it should mean nothing to you. 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] new compiler warnings
Bruce Momjian wrote: > Bruce Momjian wrote: > > Robert Haas wrote: > > > I recently started getting these: > > > > > > plpython.c: In function ?PLy_output?: > > > plpython.c:3468: warning: format not a string literal and no format > > > arguments > > > plpython.c: In function ?PLy_elog?: > > > plpython.c:3620: warning: format not a string literal and no format > > > arguments > > > plpython.c:3627: warning: format not a string literal and no format > > > arguments > > > > And I see this warning: > > > > compress_io.c:597: warning: passing arg 2 of `gzwrite' discards > > qualifiers from pointer target type > > I can remove this warning by casting the pointer to (void *), rather > than (const void *) because that is what the prototype uses on my system > uses (libz.so.1.1.4): > > ZEXTERN int ZEXPORTgzwrite OF((gzFile file, > const voidp buf, unsigned len)); I was just suggesting that others might also see this warning for older libs. You don't need to change it for me. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] new compiler warnings
Bruce Momjian writes: >> And I see this warning: >> >> compress_io.c:597: warning: passing arg 2 of `gzwrite' discards >> qualifiers from pointer target type > I can remove this warning by casting the pointer to (void *), rather > than (const void *) because that is what the prototype uses on my system > uses (libz.so.1.1.4): Casting away const manually isn't much of an improvement, and will more than likely provoke warnings of its own on other compilers. Aren't you overdue for a zlib update? I'm pretty sure there are known security bugs in 1.1.4 (which dates from 2002). I see no such warning with zlib 1.2.3, which itself isn't exactly wet behind the ears (2005). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
Noah Misch writes: > text -> xml BTW, that reminds me of something that I think was mentioned way back when, but absolutely fails to fit into any of the frameworks discussed here: the mere fact that a type is binary-compatible (with or without checking) doesn't make it compatible enough to not have to recreate indexes. Where and how are we going to have a wart to determine if that's necessary? And if the answer is "rebuild indexes whenever the data type changes", isn't that a further big dent in the argument that it's worth avoiding a table rewrite? A text->xml replacement is going to be far from cheap anyway. 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, 2011-01-26 at 17:39 -0500, Robert Haas wrote: > On Wed, Jan 26, 2011 at 5:30 PM, Richard Broersma > wrote: > >> I'm not sure what you mean by this. > > > > Now that I read it, I not sure what I meant either. :) How about this: the > > selection, management, and oversight of grants and mentees should be opaque > > to the community so as to prevent distraction. There should be no > > appearance of community endorsement of such programs. > > I think there's no need to announce who you've made grants too, if > that's what you mean. But it shouldn't be hidden from the community > either. We ought to know who to praise/blame if something good/bad > happens. We are a 501c3. I don't think we are allowed to hide it even if we wanted to :P JD -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- 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 TYPE 3: add facility to identify further no-work cases
Noah Misch writes: > On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote: >> I remain completely unexcited about optimizing that case, especially if >> it doesn't fit into a general framework. The bang for the buck ratio >> is not good: too much work, too much uglification, too little return. > The return looks attractive when you actually save six hours of downtime. If > I'm the only one that sees such a savings for one of his databases, though, I > suppose it's not worthwhile. We'd miss optimizing these cases: > numeric(8,2) -> numeric(7,2) > varbit(8) -> varbit(7) > text -> xml But how often do those really come up? And do you really save that much? The table still has to be locked against other users, so you're still down, and you're still doing all the reads and computation. I don't deny that saving the writes is worth something; I just don't agree that it's worth the development and maintenance effort that such a wart is going to cost us. User-exposed features are *expensive*. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote: > Noah Misch writes: > > If we hook this into eval_const_expressions, it definitely seems > > cleaner to attach the auxiliary function to the pg_proc. Otherwise, > > we'd reconstruct which cast led to each function call -- is there even > > enough information available to do so unambiguously? > > As far as that goes, yes there is --- otherwise ruleutils would be > unable to reverse-list cast constructs. IIRC the function call is > marked as being a cast, and you get the source and dest type IDs > from ordinary exprType() inspection. Good point. So it is, FuncExpr.funcformat conveys that. > > For the syntax, then, would a new common_func_opt_item of "WHEN func" fit? > > WHEN is appropriate for the restricted CAST case, but it doesn't seem > like le mot juste for a general function option, unless we restrict > the helper function to return boolean which is not what I had in mind. True. Uhh, "PARSER MAPPING func"? "PLANS CONVERSION func"? > > That covers fully-removable casts, but ALTER TABLE still needs to identify > > casts > > that may throw errors but never change the value's binary representation. > > I remain completely unexcited about optimizing that case, especially if > it doesn't fit into a general framework. The bang for the buck ratio > is not good: too much work, too much uglification, too little return. The return looks attractive when you actually save six hours of downtime. If I'm the only one that sees such a savings for one of his databases, though, I suppose it's not worthwhile. We'd miss optimizing these cases: numeric(8,2) -> numeric(7,2) varbit(8) -> varbit(7) text -> xml The xml one is the most interesting to me. In the design thread, Robert also expressed interest in that one. Jim Nasby mentioned numeric generally; Jim, what kind of numeric conversions are important to you? If anyone else will miss one of those greatly, please speak up. I found that many interesting cases in this area, most notably varchar(8) -> varchar(4), are safe a good deal of the time, but not provably so. So perhaps a system for automatically detecting them would be overkill, but an addition to the ALTER TABLE syntax[1] to request them would be worthwhile. nm [1] http://archives.postgresql.org/message-id/20110106042626.ga28...@tornado.leadboat.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] new compiler warnings
Bruce Momjian wrote:
> Robert Haas wrote:
> > I recently started getting these:
> >
> > plpython.c: In function ?PLy_output?:
> > plpython.c:3468: warning: format not a string literal and no format
> > arguments
> > plpython.c: In function ?PLy_elog?:
> > plpython.c:3620: warning: format not a string literal and no format
> > arguments
> > plpython.c:3627: warning: format not a string literal and no format
> > arguments
>
> And I see this warning:
>
> compress_io.c:597: warning: passing arg 2 of `gzwrite' discards
> qualifiers from pointer target type
I can remove this warning by casting the pointer to (void *), rather
than (const void *) because that is what the prototype uses on my system
uses (libz.so.1.1.4):
ZEXTERN int ZEXPORTgzwrite OF((gzFile file,
const voidp buf, unsigned len));
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index fb280ab..a00bb54 100644
*** a/src/bin/pg_dump/compress_io.c
--- b/src/bin/pg_dump/compress_io.c
*** cfwrite(const void *ptr, int size, cfp *
*** 594,600
{
#ifdef HAVE_LIBZ
if (fp->compressedfp)
! return gzwrite(fp->compressedfp, ptr, size);
else
#endif
return fwrite(ptr, 1, size, fp->uncompressedfp);
--- 594,600
{
#ifdef HAVE_LIBZ
if (fp->compressedfp)
! return gzwrite(fp->compressedfp, (void *)ptr, size);
else
#endif
return fwrite(ptr, 1, size, fp->uncompressedfp);
--
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] [NOVICE] systable_getnext_ordered
I wrote: > y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) writes: >> after systable_getnext_ordered returned NULL, is it ok to call it again? > I wouldn't rely on it working. >> i'm wondering because inv_truncate seems to do it and expecting NULL. > Hmm, that may well be a bug. Have you tested it? I looked at this a bit more closely, and basically the answer is that inv_truncate is accidentally failing to fail. What will actually happen if systable_getnext_ordered is called another time, at least when using a btree index, is that it will start the same requested scan over again, ie deliver all the tuples it did the first time (which is none, in this case). That's an implementation artifact, but since the behavior is undefined in the first place, it's not wrong. Now, if inv_truncate's initial call on systable_getnext_ordered returns NULL (ie, the truncation point is past the current EOF page), it will fall through to the "Write a brand new page" code, which will create and insert a partial page at the truncation point. It then goes to the delete-all-remaining-pages loop. Because that starts a fresh scan with the very same scan key conditions, you might expect that it would find and delete the page it just inserted --- causing the apparent EOF of the blob to be wrong afterwards. It accidentally fails to do that because the new tuple postdates the snapshot it's scanning with. So the loop terminates having found no matching tuples, and all is well. So this code is confusing, inefficient (performing a useless search of the index), only works because of an obscure consideration not explained in the comments, and sets a bad precedent for people to follow. I'm going to go change it to explicitly not do the final loop if the initial search failed. It's not a bug, exactly, but it's sure lousy coding. Thanks for pointing it out. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: In pg_test_fsync, use K(1024) rather than k(1000) for write size units.
Bruce Momjian wrote: > Peter Eisentraut wrote: > > We use small "k" in postgresql.conf, so pg_test_fsync should use the > > same. Using "kB" would be more accurate in any case. > > OK, done with the attached applied patch. FYI, I had used 'k' because this page suggests that k is 1000 and K is 1024, at least by the JEDEC memory standards: http://en.wikipedia.org/wiki/Kilo -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pl/python refactoring
On 27/01/11 00:40, Peter Eisentraut wrote: > On tor, 2011-01-20 at 03:16 +0100, Jan Urbański wrote: >> Here's an updated patch series for PL/Python refactoring. It was 16 >> patches at first, 8 are committed, 1 got dropped, so we're down to 7. > > Everything(*) is now committed. Great, thanks. I'll be posting updated versions of the remaining patches to their corresponding threads (or their review threads, if they already exist). Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= writes: > _2 is only Python 2.2, but I tried: with Python 2.2 there's a whole lot > of regression tests that fail. The last release of 2.2 is April 2003, > maybe it's time to forget about that particular dinosaur? Well, there's little point in carrying an incorrect expected-file, so unless someone is prepared to test the case regularly, I'd agree with deleting that file. We could just add a note that the expected outputs are intended for 2.3 and up and you'll get some error-message-wording discrepancies with older python versions. The differences between _2 and _3 seem small enough for people to handle by visual inspection if they really want to test the case. 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: PL/Python table functions
On 24/01/11 05:42, Hitoshi Harada wrote:
> 2011/1/23 Jan Urbański :
>> On 22/01/11 11:15, Hitoshi Harada wrote:
> I tested the new incremental patch and the previous example works
> fine. I don't know if this can be handled properly but another example
> is:
>
> regression=# create function func1(t text) returns record as $$ return
> {'name':t, 'value':0}; $$ language plpythonu ;
> CREATE FUNCTION
> regression=# select * from func1('jan') as (name text, value bigint);
> name | value
> --+---
> jan | 0
> (1 row)
>
> regression=# select * from func1('jan') as (value text, name bigint);
> value | name
> ---+--
> jan |0
> (1 row)
>
> where value and name don't point to the correct data. Your
> data-type-check logic might not be enough.
I have to admit that I don't 100% understand what's happening when you
have a function that returns RECORD and has no OUT parameters, but I did
a quick check with PL/pgSQL and it behaves the same:
create or replace function rec(t text) returns record as $$
declare
r record;
begin
select t as name, 0 as value into r;
return r;
end;
$$ language plpgsql;
pl_regression=# select * from rec('jan') as (value text, name integer);
value | name
---+--
jan |0
(1 row)
pl_regression=# select * from rec('jan') as (name text, value integer);
name | value
--+---
jan | 0
(1 row)
So PL/pgSQL seems to work positionally, whereas PL/Python uses the
variable names when fetching them from the mapping Python returned. All
in all, it works the same as in other PLs, so at least it's consistent.
I'm also attaching an updated version that should apply on top of my
github refactor branch (or incrementally over the new set of refactor
patches that I will post shortly to the refactor thread).
Cheers,
Jan
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 16d78ae..167393e 100644
*** a/src/pl/plpython/Makefile
--- b/src/pl/plpython/Makefile
*** REGRESS = \
*** 79,84
--- 79,85
plpython_types \
plpython_error \
plpython_unicode \
+ plpython_composite \
plpython_drop
# where to find psql for running the tests
PSQLDIR = $(bindir)
diff --git a/src/pl/plpython/expected/plpython_composite.out b/src/pl/plpython/expected/plpython_composite.out
index ...1576588 .
*** a/src/pl/plpython/expected/plpython_composite.out
--- b/src/pl/plpython/expected/plpython_composite.out
***
*** 0
--- 1,309
+ CREATE FUNCTION multiout_simple(OUT i integer, OUT j integer) AS $$
+ return (1, 2)
+ $$ LANGUAGE plpythonu;
+ SELECT multiout_simple();
+ multiout_simple
+ -
+ (1,2)
+ (1 row)
+
+ SELECT * FROM multiout_simple();
+ i | j
+ ---+---
+ 1 | 2
+ (1 row)
+
+ SELECT i, j + 2 FROM multiout_simple();
+ i | ?column?
+ ---+--
+ 1 |4
+ (1 row)
+
+ SELECT (multiout_simple()).j + 3;
+ ?column?
+ --
+ 5
+ (1 row)
+
+ CREATE FUNCTION multiout_simple_setof(n integer = 1, OUT integer, OUT integer) RETURNS SETOF record AS $$
+ return [(1, 2)] * n
+ $$ LANGUAGE plpythonu;
+ SELECT multiout_simple_setof();
+ multiout_simple_setof
+ ---
+ (1,2)
+ (1 row)
+
+ SELECT * FROM multiout_simple_setof();
+ column1 | column2
+ -+-
+1 | 2
+ (1 row)
+
+ SELECT * FROM multiout_simple_setof(3);
+ column1 | column2
+ -+-
+1 | 2
+1 | 2
+1 | 2
+ (3 rows)
+
+ CREATE FUNCTION multiout_record_as(typ text,
+first text, OUT first text,
+second integer, OUT second integer,
+retnull boolean) RETURNS record AS $$
+ if retnull:
+ return None
+ if typ == 'dict':
+ return { 'first': first, 'second': second, 'additionalfield': 'must not cause trouble' }
+ elif typ == 'tuple':
+ return ( first, second )
+ elif typ == 'list':
+ return [ first, second ]
+ elif typ == 'obj':
+ class type_record: pass
+ type_record.first = first
+ type_record.second = second
+ return type_record
+ $$ LANGUAGE plpythonu;
+ SELECT * from multiout_record_as('dict', 'foo', 1, 'f');
+ first | second
+ ---+
+ foo | 1
+ (1 row)
+
+ SELECT multiout_record_as('dict', 'foo', 1, 'f');
+ multiout_record_as
+
+ (foo,1)
+ (1 row)
+
+ SELECT *, s IS NULL as snull from multiout_record_as('tuple', 'xxx', NULL, 'f') AS f(f, s);
+ f | s | snull
+ -+---+---
+ xxx | | t
+ (1 row)
+
+ SELECT *, f IS NULL as fnull, s IS NULL as snull from multiout_record_as('tuple', 'xxx', 1, 't') AS f(f, s);
+ f | s | fnull | snull
+ ---+---+---+---
+| | t | t
+ (1 row)
+
+ SELECT * from multiout_record_as('obj', NULL, 10, 'f');
+ first | second
+ ---+
+| 10
+ (1 row)
+
+ CREATE FUNCTION multiout_setof(n integer,
+OUT power_of_2
Re: [HACKERS] pl/python refactoring
On tor, 2011-01-20 at 03:16 +0100, Jan Urbański wrote: > Here's an updated patch series for PL/Python refactoring. It was 16 > patches at first, 8 are committed, 1 got dropped, so we're down to 7. Everything(*) is now committed. In 0006-Improve-exception-usage-in-PL-Python.patch I went for TypeError instead of ValueError because that matched better with the behavior of some Python built-ins. Same idea, though. (*) In 0007-Do-not-prefix-error-messages-with-the-string-PL-Pyth.patch, I did not commit the bit that moved pg_verifymbstr outside the TRY block. This is debatable. I observe that there are other uses of pg_verifymbstr in TRY blocks. Also, we document that strings in Python code must be in the server encoding, so I would argue that this error could be considered a Python error and thus the current code would be correct. -- 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 TYPE 3: add facility to identify further no-work cases
Noah Misch writes: > If we hook this into eval_const_expressions, it definitely seems > cleaner to attach the auxiliary function to the pg_proc. Otherwise, > we'd reconstruct which cast led to each function call -- is there even > enough information available to do so unambiguously? As far as that goes, yes there is --- otherwise ruleutils would be unable to reverse-list cast constructs. IIRC the function call is marked as being a cast, and you get the source and dest type IDs from ordinary exprType() inspection. > For the syntax, then, would a new common_func_opt_item of "WHEN func" fit? WHEN is appropriate for the restricted CAST case, but it doesn't seem like le mot juste for a general function option, unless we restrict the helper function to return boolean which is not what I had in mind. > That covers fully-removable casts, but ALTER TABLE still needs to identify > casts > that may throw errors but never change the value's binary representation. I remain completely unexcited about optimizing that case, especially if it doesn't fit into a general framework. The bang for the buck ratio is not good: too much work, too much uglification, too little return. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
On 27/01/11 00:15, Tom Lane wrote: > Peter Eisentraut writes: >> On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote: >>> I was a bit disturbed by the fact that fixing only one of the four >>> variant files was enough to turn the whole buildfarm green. Are the >>> other three cases even needed anymore? If so, how can we get some >>> coverage on them? > >> This is explained in plpython/expected/README. As you can see there, it >> would need some careful attention from buildfarm code and participants >> to cover all that. > > Hmmm ... well, the fact that we have zero coverage on the first two > variants definitely seems surprising and scary to me. Why aren't those > getting hit by the non-C-locale buildfarm runs? Looking at the README, you get the basic output file if you have server encoding != SQL_ASCII and client encoding UTF8, which is what I was testing with. _0 is when you have server encoding != SQL_ASCII and client encoding != UTF8, which I'm not sure how popular of a setup is in the buildfarm (or maybe by sheer luck it didn't break, dunno). _2 is only Python 2.2, but I tried: with Python 2.2 there's a whole lot of regression tests that fail. The last release of 2.2 is April 2003, maybe it's time to forget about that particular dinosaur? When coding I was running tests with Pythons 2.3 to 3.1 and trying to keep the stuff working with these versions, as the last 2.3 release was in March 2008. _3 is the variant file you get if your server is SQL_ASCII and you have a non-ancient Python, which I guess is the config quote a few buildfarm animals has. So three things: * I should test with SQL_ASCII * we might want to check if the the _0 variant file needs updates * maybe it's time to stop supporting Python 2.2 Cheers, Jan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
Peter Eisentraut writes: > On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote: >> I was a bit disturbed by the fact that fixing only one of the four >> variant files was enough to turn the whole buildfarm green. Are the >> other three cases even needed anymore? If so, how can we get some >> coverage on them? > This is explained in plpython/expected/README. As you can see there, it > would need some careful attention from buildfarm code and participants > to cover all that. Hmmm ... well, the fact that we have zero coverage on the first two variants definitely seems surprising and scary to me. Why aren't those getting hit by the non-C-locale buildfarm runs? 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] Query Optimizer + Parallel Operators
Felix, > I'm interested in the query optimizer of PostgreSQL DB. Where could I > find useful documentation or could you send me a pointer in the source code? > > What kind of parallelism does PostgreSQL use for operators, like > selection or join? Normally we're very helpful with this kind of information ... it's all public after all ... but I have to say that the domain you're e-mailing from makes it a little hard to give you a direct answer. Could you maybe give us a little information about what you want this information for? -- -- Josh Berkus PostgreSQL Experts Inc. http://www.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] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 05:32:00PM -0500, Tom Lane wrote: > Robert Haas writes: > > Well, if you're positive we're eventually going to want this in > > pg_proc, we may as well add it now. But I'm not too convinced it's > > the right general API. The number of people writing exactly x + 0 or > > x * 0 in a query has got to be vanishingly small; I'm not eager to add > > additional parse analysis time to every SQL statement that has a > > function in it just to detect those cases. > > Actually, you've got that backwards: the facility I've got in mind would > cost next to nothing when not used. The place where we'd want to insert > this in eval_const_expressions has already got its hands on the relevant > pg_proc row, so checking for a nonzero hook-function reference would be > a matter of a couple of instructions. If we go with a pg_cast entry > then we're going to have to add a pg_cast lookup for every cast, whether > it turns out to be optimizable or not; which is going to cost quite a > lot more. The intermediate hook function I was sketching might be > worthwhile from a performance standpoint even if we don't expose the > more general feature to users, just because it would be possible to > avoid useless pg_cast lookups (by not installing the hook except on > pg_proc entries for which there's a relevant CAST WHEN function to call). If we hook this into eval_const_expressions, it definitely seems cleaner to attach the auxiliary function to the pg_proc. Otherwise, we'd reconstruct which cast led to each function call -- is there even enough information available to do so unambiguously? Unlike something typmod-specific, these functions would effectively need to be written in C. Seems like a perfectly acceptable constraint, though. For the syntax, then, would a new common_func_opt_item of "WHEN func" fit? That covers fully-removable casts, but ALTER TABLE still needs to identify casts that may throw errors but never change the value's binary representation. Where does that fit? Another pg_proc column for a function called to answer that question, called only from an ALTER TABLE-specific code path? Thanks for the feedback/analysis. nm -- 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 TYPE 3: add facility to identify further no-work cases
Noah Misch writes: > On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote: >> Actually, I can construct a concrete example where applying this >> optimization in the parser will do the wrong thing: >> >> CREATE TABLE base (f1 varchar(4)); >> CREATE VIEW vv AS SELECT f1::varchar(8) FROM base; >> ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16); >> >> If the parser is taught to throw away "useless" length coercions, then >> the stored form of vv will contain no cast, and the results will be >> wrong after base's column is widened. > We reject that: > [local] test=# ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16); > ERROR: cannot alter type of a column used by a view or rule > DETAIL: rule _RETURN on view vv depends on column "f1" Nonetheless, the stored form of vv will contain no cast, which can hardly be thought safe here. Relaxing the restriction you cite is (or should be) on the TODO list, but we'll never be able to do it if the parser throws away relevant information. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote: > Peter Eisentraut writes: > > On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote: > >> Peter Eisentraut writes: > >>> Get rid of the global variable holding the error state > > >> The buildfarm doesn't like this patch one bit. > > > I have verified your adjustments and fixed up the rest. > > I was a bit disturbed by the fact that fixing only one of the four > variant files was enough to turn the whole buildfarm green. Are the > other three cases even needed anymore? If so, how can we get some > coverage on them? This is explained in plpython/expected/README. As you can see there, it would need some careful attention from buildfarm code and participants to cover all that. -- 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 TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > Well, I guess my thought was that we what we were doing is extending > the coercion system to be able to make decisions based on both type > OID and typemod. Well, that's an interesting thought, but the proposal at hand is far more limited than that --- it's only an optimization that can be applied in limited situations. If we want to do something like what you're suggesting here, it's not going to get done for 9.1. > A further problem is that I don't think it'd play well at all with the > richer-typemod concept we keep bandying about. If, for example, we > represented all arrays with the same OID (getting rid of the > double-entries in pg_type) and used some kind of augmented-typemod to > store the number of dimensions and the base type, this would > completely fall over. Your proposal would completely fall over, you mean. An Expr->Expr hook API wouldn't be affected at all. I'm not sure how important that concern is though, because it's hard to see how any such change wouldn't break existing cast implementation functions anyway. If the API for length-coercing cast functions changes, breaking their helper functions too hardly seems like an issue. Or are you saying you want to punt this whole proposal till after that happens? I had muttered something of the sort way upthread, but I didn't think anyone else thought that way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote: > I wrote: > > ... Another issue is that premature > > optimization in the parser creates headaches if conditions change such > > that a previous optimization is no longer valid --- you may have stored > > rules wherein the optimization was already applied. (Not sure that > > specific issue applies to casting, since we have no ALTER CAST commmand; > > but in general you want expression optimizations applied downstream from > > the rule rewriter not upstream.) > > Actually, I can construct a concrete example where applying this > optimization in the parser will do the wrong thing: > > CREATE TABLE base (f1 varchar(4)); > CREATE VIEW vv AS SELECT f1::varchar(8) FROM base; > ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16); > > If the parser is taught to throw away "useless" length coercions, then > the stored form of vv will contain no cast, and the results will be > wrong after base's column is widened. We reject that: [local] test=# ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16); ERROR: cannot alter type of a column used by a view or rule DETAIL: rule _RETURN on view vv depends on column "f1" -- 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 TYPE 3: add facility to identify further no-work cases
I wrote: > ... Another issue is that premature > optimization in the parser creates headaches if conditions change such > that a previous optimization is no longer valid --- you may have stored > rules wherein the optimization was already applied. (Not sure that > specific issue applies to casting, since we have no ALTER CAST commmand; > but in general you want expression optimizations applied downstream from > the rule rewriter not upstream.) Actually, I can construct a concrete example where applying this optimization in the parser will do the wrong thing: CREATE TABLE base (f1 varchar(4)); CREATE VIEW vv AS SELECT f1::varchar(8) FROM base; ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16); If the parser is taught to throw away "useless" length coercions, then the stored form of vv will contain no cast, and the results will be wrong after base's column is widened. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 5:41 PM, Tom Lane wrote: > Robert Haas writes: >> Oh, really? I was thinking the logic should go into find_coercion_pathway(). > > Well, I've been saying right along that it should be in > eval_const_expressions. Putting this sort of optimization in the parser > is invariably the wrong thing, because it fails to catch all the > possibilities. As an example, inlining a SQL function could expose > opportunities of this type. Another issue is that premature > optimization in the parser creates headaches if conditions change such > that a previous optimization is no longer valid --- you may have stored > rules wherein the optimization was already applied. (Not sure that > specific issue applies to casting, since we have no ALTER CAST commmand; > but in general you want expression optimizations applied downstream from > the rule rewriter not upstream.) Well, I guess my thought was that we what we were doing is extending the coercion system to be able to make decisions based on both type OID and typemod. It seems very odd to make an initial decision based on type OID in one place and then have a completely different system for incorporating the typemod; and it also seems quite a bit more expensive, since you'd have to consider it for every function, not just casts. A further problem is that I don't think it'd play well at all with the richer-typemod concept we keep bandying about. If, for example, we represented all arrays with the same OID (getting rid of the double-entries in pg_type) and used some kind of augmented-typemod to store the number of dimensions and the base type, this would completely fall over. -- 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] Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
Peter Eisentraut writes: > On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote: >> Peter Eisentraut writes: >>> Get rid of the global variable holding the error state >> The buildfarm doesn't like this patch one bit. > I have verified your adjustments and fixed up the rest. I was a bit disturbed by the fact that fixing only one of the four variant files was enough to turn the whole buildfarm green. Are the other three cases even needed anymore? If so, how can we get some coverage on them? 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 13:35, Alexey Klyukin wrote: > > On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: >>> (it's obviously reversed comparing with the original one), but it still >>> segfaults after I fixed that. Ahh yep, the reason reversing the check did not fix it is order of operations. I had this fixed, but I had some unrelated changes in my tree. So I manually git add(ed) the plperl files so I could use git diff --cached to make the diff. Then I fixed this issue, but forgot to git-add the changes :(. That explains why make installcheck worked for me, but the diff I made was broken. If you add some parens around ref it should work: if ref($arg) !~ /ARRAY/; btw the next version I plan on posting will check more explicitly: if ref($arg) !~ /^(?:ARRAY|PostgreSQL::InServer::ARRAY)$/; -- 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: [COMMITTERS] pgsql: Get rid of the global variable holding the error state
On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote: > Peter Eisentraut writes: > > Get rid of the global variable holding the error state > > The buildfarm doesn't like this patch one bit. I have verified your adjustments and fixed up the rest. -- 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] .gitignore patch for coverage builds
Excerpts from Tom Lane's message of mié ene 26 19:20:52 -0300 2011: > Robert Haas writes: > > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane wrote: > >> Ick. That's an awful lot of stuff to have global ignores for. > > > The "coverage" directory ignore seems a little icky, but the rest > > seems unlikely to pick up anything incidental. > > Tying /coverage to the root as in his V2 makes that better, Hmm, I don't think that works, because you can run "make coverage" in any subdir and it will create a "coverage" subdir there. > but I'm > still unexcited about the thesis that we should auto-ignore the results > of any random tool somebody wants to run in their source tree. Well, in this case it's not any random tool, because it's integrated into our makefiles. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > Oh, really? I was thinking the logic should go into find_coercion_pathway(). Well, I've been saying right along that it should be in eval_const_expressions. Putting this sort of optimization in the parser is invariably the wrong thing, because it fails to catch all the possibilities. As an example, inlining a SQL function could expose opportunities of this type. Another issue is that premature optimization in the parser creates headaches if conditions change such that a previous optimization is no longer valid --- you may have stored rules wherein the optimization was already applied. (Not sure that specific issue applies to casting, since we have no ALTER CAST commmand; but in general you want expression optimizations applied downstream from the rule rewriter not upstream.) 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] new compiler warnings
On Wed, Jan 26, 2011 at 5:20 PM, Peter Eisentraut wrote: > On ons, 2011-01-26 at 17:00 -0500, Robert Haas wrote: >> More to the point, regardless of whether the warning is reasonable or >> not, there's tangible value in a warning-free build, which we have had >> on most of the systems I use until recently. > > I don't disagree that the warnings are valid. I'd just like to see them > as well. > > It turns out you need -Wformat-security with newer GCC versions. We > might want to add that to the standard options set. > > Anyway: Fixed. Thanks! -- 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 5:30 PM, Richard Broersma wrote: >> I'm not sure what you mean by this. > > Now that I read it, I not sure what I meant either. :) How about this: the > selection, management, and oversight of grants and mentees should be opaque > to the community so as to prevent distraction. There should be no > appearance of community endorsement of such programs. I think there's no need to announce who you've made grants too, if that's what you mean. But it shouldn't be hidden from the community either. We ought to know who to praise/blame if something good/bad happens. -- 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] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 5:32 PM, Tom Lane wrote: > Robert Haas writes: >> Well, if you're positive we're eventually going to want this in >> pg_proc, we may as well add it now. But I'm not too convinced it's >> the right general API. The number of people writing exactly x + 0 or >> x * 0 in a query has got to be vanishingly small; I'm not eager to add >> additional parse analysis time to every SQL statement that has a >> function in it just to detect those cases. > > Actually, you've got that backwards: the facility I've got in mind would > cost next to nothing when not used. The place where we'd want to insert > this in eval_const_expressions has already got its hands on the relevant > pg_proc row, so checking for a nonzero hook-function reference would be > a matter of a couple of instructions. If we go with a pg_cast entry > then we're going to have to add a pg_cast lookup for every cast, whether > it turns out to be optimizable or not; which is going to cost quite a > lot more. The intermediate hook function I was sketching might be > worthwhile from a performance standpoint even if we don't expose the > more general feature to users, just because it would be possible to > avoid useless pg_cast lookups (by not installing the hook except on > pg_proc entries for which there's a relevant CAST WHEN function to call). Oh, really? I was thinking the logic should go into find_coercion_pathway(). >> Even slightly more >> complicated problems seem intractable - e.g. (x + 1) = x can be >> simplified to constant false, and NOT ((x + 1) = x) can be simplified >> to x IS NOT NULL, but under the proposed API those would have to hang >> off of =(int4,int4), which seems pretty darn ugly. > > True, but where else are you going to hang them off of? Not that I was > particularly thinking of doing either one of those. Beats me, just thinking out loud. -- 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] new compiler warnings
Peter Eisentraut writes: > It turns out you need -Wformat-security with newer GCC versions. Ah-hah. > We might want to add that to the standard options set. +1. Probably this will require an extra configure test, but even so it's well worthwhile. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > Well, if you're positive we're eventually going to want this in > pg_proc, we may as well add it now. But I'm not too convinced it's > the right general API. The number of people writing exactly x + 0 or > x * 0 in a query has got to be vanishingly small; I'm not eager to add > additional parse analysis time to every SQL statement that has a > function in it just to detect those cases. Actually, you've got that backwards: the facility I've got in mind would cost next to nothing when not used. The place where we'd want to insert this in eval_const_expressions has already got its hands on the relevant pg_proc row, so checking for a nonzero hook-function reference would be a matter of a couple of instructions. If we go with a pg_cast entry then we're going to have to add a pg_cast lookup for every cast, whether it turns out to be optimizable or not; which is going to cost quite a lot more. The intermediate hook function I was sketching might be worthwhile from a performance standpoint even if we don't expose the more general feature to users, just because it would be possible to avoid useless pg_cast lookups (by not installing the hook except on pg_proc entries for which there's a relevant CAST WHEN function to call). > Even slightly more > complicated problems seem intractable - e.g. (x + 1) = x can be > simplified to constant false, and NOT ((x + 1) = x) can be simplified > to x IS NOT NULL, but under the proposed API those would have to hang > off of =(int4,int4), which seems pretty darn ugly. True, but where else are you going to hang them off of? Not that I was particularly thinking of doing either one of those. 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 1:55 PM, Robert Haas wrote: > I don't think that's it exactly. Basically, if you fund reviewers, > and we get lots more people doing reviews and they're all great, I'll > be happy. If you fund reviewers, and we get lots more people doing > reviews and they're all terrible, I'll be unhappy. And likewise if > you do or don't fund mentors. The results matter a lot, and none of > us know that for sure yet. This makes sense. I should clarify that this point in time were talking about one maybe two people can awarded grants. Over the course of a year I wouldn't expect more that four grants issued (at least for now.) With these numbers, there is too much to be gained or lost from the perceptive of the community in my opinion. > I think all I (and others) are asking you > do is think about it carefully before you decide what to do; I at > least am not trying to push you down any particular path. > Fair enough. > > So any person regardless of association or funding is free to approach to > > community for assistance. > > I strongly agree with that statement. Of course, all such help is on > a best-effort, volunteer basis. If you need more than that, you can > try (a) begging, (b) T-shirts, or (c) money. What's not clear to me > is whether you do in fact need more than that, and which of (a)-(c) is > the best way to get it. > > > In addition, third party organizations should > > maintain a healthy disconnection from the community. > > I'm not sure what you mean by this. > Now that I read it, I not sure what I meant either. :) How about this: the selection, management, and oversight of grants and mentees should be opaque to the community so as to prevent distraction. There should be no appearance of community endorsement of such programs. -- Regards, Richard Broersma Jr.
Re: [HACKERS] .gitignore patch for coverage builds
Tom Lane wrote: > I'm still unexcited about the thesis that we should auto-ignore > the results of any random tool somebody wants to run in their > source tree. Hos about just the tools supported by our documentation, configure file and make file? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] .gitignore patch for coverage builds
Robert Haas writes: > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane wrote: >> Ick. That's an awful lot of stuff to have global ignores for. > The "coverage" directory ignore seems a little icky, but the rest > seems unlikely to pick up anything incidental. Tying /coverage to the root as in his V2 makes that better, but I'm still unexcited about the thesis that we should auto-ignore the results of any random tool somebody wants to run in their source tree. 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] new compiler warnings
On ons, 2011-01-26 at 17:00 -0500, Robert Haas wrote: > More to the point, regardless of whether the warning is reasonable or > not, there's tangible value in a warning-free build, which we have had > on most of the systems I use until recently. I don't disagree that the warnings are valid. I'd just like to see them as well. It turns out you need -Wformat-security with newer GCC versions. We might want to add that to the standard options set. Anyway: Fixed. -- 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] new compiler warnings
Robert Haas writes: > But I think I did get it on a recently-updated Fedora 13 box also, I > can check if it's important. F-13 doesn't show it for me. I get the impression from these results that maybe gcc versions >= about 4.4 have been tweaked to not show it ... which doesn't really seem like a step forward. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 5:01 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane wrote: >>> I don't mind confining the feature to casts to start with, but it might >>> be a good idea to specify the check-function API in a way that would let >>> it be plugged into a more generally available call-simplification hook >>> later. > >> Got any suggestions? My thought was that it should just get (type, >> typemod, type, typemod) and return a boolean. We could do something >> more complicated, like Expr -> Expr, but I'm pretty unconvinced >> there's any value there. > > Well, (type, typemod, type, typemod) would be fine as long as the only > case you're interested in optimizing is typemod-lengthening coercions. > I think we're going to want the more general Expr-to-Expr capability > eventually. > > I guess we could do the restricted case for now, with the idea that > there could be a standard Expr-to-Expr interface function created later > and installed into the generic hook facility for functions that are cast > functions. That could look into pg_cast and make the more restricted > call when appropriate. (The meat of this function would be the same > code you'd have to add to eval_const_expressions anyway for the > hard-wired version...) Well, if you're positive we're eventually going to want this in pg_proc, we may as well add it now. But I'm not too convinced it's the right general API. The number of people writing exactly x + 0 or x * 0 in a query has got to be vanishingly small; I'm not eager to add additional parse analysis time to every SQL statement that has a function in it just to detect those cases. Even slightly more complicated problems seem intractable - e.g. (x + 1) = x can be simplified to constant false, and NOT ((x + 1) = x) can be simplified to x IS NOT NULL, but under the proposed API those would have to hang off of =(int4,int4), which seems pretty darn ugly. -- 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] [COMMITTERS] pgsql: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
Robert Haas writes: > Yeah, I wasn't aware of that. I'll go revert, but I think I'll also > add a big fat comment, because this is entirely non-obvious, What I think would actually be helpful would be to improve the error message. I'm not sure if it's practical to pass down the specific reason(s) why we need to throw error, but if not, we could consider a wishy-washy HINT along the lines of "This could be because of X, Y, or Z". 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] .gitignore patch for coverage builds
On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane wrote: > "Kevin Grittner" writes: >> Building for coverage and running the reports littered my tree with >> files which should probably be in .gitignore for just such a >> contingency. Patch attached. > > Ick. That's an awful lot of stuff to have global ignores for. The "coverage" directory ignore seems a little icky, but the rest seems unlikely to pick up anything incidental. -- 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] ALTER TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane wrote: >> I don't mind confining the feature to casts to start with, but it might >> be a good idea to specify the check-function API in a way that would let >> it be plugged into a more generally available call-simplification hook >> later. > Got any suggestions? My thought was that it should just get (type, > typemod, type, typemod) and return a boolean. We could do something > more complicated, like Expr -> Expr, but I'm pretty unconvinced > there's any value there. Well, (type, typemod, type, typemod) would be fine as long as the only case you're interested in optimizing is typemod-lengthening coercions. I think we're going to want the more general Expr-to-Expr capability eventually. I guess we could do the restricted case for now, with the idea that there could be a standard Expr-to-Expr interface function created later and installed into the generic hook facility for functions that are cast functions. That could look into pg_cast and make the more restricted call when appropriate. (The meat of this function would be the same code you'd have to add to eval_const_expressions anyway for the hard-wired version...) > I'd like to see some kind of appropriate > hook for interjecting selectivity estimates for cases that we > currently can't recognize, but my gut feeling is that that's > insufficiently related at the problem at hand to worry about it. Agreed, that seems a bit off-topic. There are ancient comments in the source code complaining about the lack of selectivity hooks for function (as opposed to operator) calls, but that's not what Noah signed up to fix. In any case I'm not sure that expression simplification is closely connected to selectivity estimation --- to my mind it's an independent process that is good to run first. As an example, the ALTER TABLE case has a use for the former but not the latter. 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] new compiler warnings
On Wed, Jan 26, 2011 at 4:50 PM, Tom Lane wrote: > Peter Eisentraut writes: >> On ons, 2011-01-26 at 06:33 -0500, Robert Haas wrote: >>> I recently started getting these: >>> >>> plpython.c: In function ‘PLy_output’: >>> plpython.c:3468: warning: format not a string literal and no format >>> arguments >>> plpython.c: In function ‘PLy_elog’: >>> plpython.c:3620: warning: format not a string literal and no format >>> arguments >>> plpython.c:3627: warning: format not a string literal and no format >>> arguments >>> >>> Please fix. > >> Which version of which compiler is showing this? > > I've been seeing that for some time with gcc 2.95.3, so it's not exactly > a new issue. I've not seen it with modern versions, but I'm not sure > why not. What it's unhappy about is the "errhint(hint)" calls, which > I agree with it are dangerous on their face. Maybe you're 100% sure the > hint strings will never contain percent marks, but I'm not. More to the point, regardless of whether the warning is reasonable or not, there's tangible value in a warning-free build, which we have had on most of the systems I use until recently. My Ubuntu system is complaining about something unrelated and stupid, but I haven't taken time to look into what's required to fix it yet, and I don't think it's a new problem. (Why use Ubuntu instead of Red Hat, you ask? Because the last Fedora I put on there had bugs in the X driver that made it crash several times after every reboot, and occasionally at other times. The year of the Linux desktop is apparently NOT 2010, and I'm not holding my breath for 2011 either.) -- 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 4:39 PM, Richard Broersma wrote: > So I take it that the concern is not how reviews are funded, but over the > perceived connection between the organic community and third party > organizations. This makes sense. I don't think that's it exactly. Basically, if you fund reviewers, and we get lots more people doing reviews and they're all great, I'll be happy. If you fund reviewers, and we get lots more people doing reviews and they're all terrible, I'll be unhappy. And likewise if you do or don't fund mentors. The results matter a lot, and none of us know that for sure yet. I think all I (and others) are asking you do is think about it carefully before you decide what to do; I at least am not trying to push you down any particular path. > So any person regardless of association or funding is free to approach to > community for assistance. I strongly agree with that statement. Of course, all such help is on a best-effort, volunteer basis. If you need more than that, you can try (a) begging, (b) T-shirts, or (c) money. What's not clear to me is whether you do in fact need more than that, and which of (a)-(c) is the best way to get it. > In addition, third party organizations should > maintain a healthy disconnection from the community. I'm not sure what you mean by this. -- 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] new compiler warnings
Peter Eisentraut writes: > On ons, 2011-01-26 at 06:33 -0500, Robert Haas wrote: >> I recently started getting these: >> >> plpython.c: In function âPLy_outputâ: >> plpython.c:3468: warning: format not a string literal and no format arguments >> plpython.c: In function âPLy_elogâ: >> plpython.c:3620: warning: format not a string literal and no format arguments >> plpython.c:3627: warning: format not a string literal and no format arguments >> >> Please fix. > Which version of which compiler is showing this? I've been seeing that for some time with gcc 2.95.3, so it's not exactly a new issue. I've not seen it with modern versions, but I'm not sure why not. What it's unhappy about is the "errhint(hint)" calls, which I agree with it are dangerous on their face. Maybe you're 100% sure the hint strings will never contain percent marks, but I'm not. 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 12:29:23PM -0800, Joshua D. Drake wrote: > On Wed, 2011-01-26 at 14:15 -0500, Robert Haas wrote: > > On Wed, Jan 26, 2011 at 1:32 PM, Richard Broersma > > wrote: > > > On Wed, Jan 26, 2011 at 3:12 AM, Simon Riggs > > > wrote: > > >> You're paying the reviewers; are you paying the mentors? > > > > > > The answer to this question is that we can fund mentor (teacher). > > > However, > > > the amount to fund a mentor would be significantly less that the amount to > > > fund a reviewer (student). The mentors are part of the educational > > > process. > > > > Usually, in an educational process, it's the teachers who get paid, > > and the students who have to pay to get educated. I realize this is > > somewhat different because we want to encourage people to get involved > > in the project, but it still seems weird. > > Not somewhat, completely. Most of the "teachers" we have are already > getting paid to work on PostgreSQL. There are some exceptions of course > but if you look at the list of people that are qualified to actually > review code, they are getting paid *for PostgreSQL*. > > Now, that isn't to say you don't bring up a good point, you do. I think > it may be worthwhile to find a way to also compensate mentors but as you > say the goal here is encourage people to get involved. However there is > the underlying goal of educating future PostgreSQL contributors, and > let's face it --- reviewing code sucks and money is a great motivator > (especially in today's economy or if you are a starving student). I admire your motives, and agree with them. We just differ on how best to do this. One situation I want to avoid is one where the mere offer of money, even if money never changes hands, totally changes the situation, and much for the worse. I'll be happy to describe such a situation off line if anyone's interested. Another is a system of perverse incentives, as others have described, and perverse incentives are harder to avoid up front than they first appear, as money is often itself an incentive to game systems in novel and terrible ways. One thing I've thought of that could help and would be a good use of money would be an extension to the pgbuildfarm code that detects and acts on bit rot. I don't have time to build it right now, but I'd be happy to iron out the spec and help someone else, that person being paid to develop it. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] .gitignore patch for coverage builds
"Kevin Grittner" writes: > Building for coverage and running the reports littered my tree with > files which should probably be in .gitignore for just such a > contingency. Patch attached. Ick. That's an awful lot of stuff to have global ignores for. Perhaps we should recommend people do coverage tests in separate build trees, instead. 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] new compiler warnings
On Wed, Jan 26, 2011 at 4:20 PM, Peter Eisentraut wrote: > On ons, 2011-01-26 at 06:33 -0500, Robert Haas wrote: >> I recently started getting these: >> >> plpython.c: In function ‘PLy_output’: >> plpython.c:3468: warning: format not a string literal and no format arguments >> plpython.c: In function ‘PLy_elog’: >> plpython.c:3620: warning: format not a string literal and no format arguments >> plpython.c:3627: warning: format not a string literal and no format arguments >> >> Please fix. > > Which version of which compiler is showing this? I got it on gcc version 4.2.1 (Apple Inc. build 5664) I did not get it on Fedora 12, gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC). But I think I did get it on a recently-updated Fedora 13 box also, I can check if it's important. -- 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 1:19 PM, Robert Haas wrote: > It's just that > I require both income and sleep. That's probably not an issue for > people who are just getting started in the community. > > Another question is whether you really need assigned mentors at all. ... > Very few emails on -hackers go unanswered. > So I take it that the concern is not how reviews are funded, but over the perceived connection between the organic community and third party organizations. This makes sense. So any person regardless of association or funding is free to approach to community for assistance. In addition, third party organizations should maintain a healthy disconnection from the community. Is this correct? -- Regards, Richard Broersma Jr.
Re: [HACKERS] .gitignore patch for coverage builds
"Kevin Grittner" wrote: > Patch attached. The coverage directory belongs under "Local excludes in root directory". Version 2. -Kevin *** a/.gitignore --- b/.gitignore *** *** 12,19 --- 12,28 *.mo objfiles.txt .deps/ + *.h.gcov + *.c.gcov + *.y.gcov + *.l.gcov + *.c.gcov.out + *.gcno + *.gcda + lcov.info # Local excludes in root directory /GNUmakefile /config.log /config.status + /coverage/ -- 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: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
On Wed, Jan 26, 2011 at 4:20 PM, Tom Lane wrote: > Robert Haas writes: >> Considering the number of OTHER places we'd have to break backward >> compatibility, one more wouldn't bother me any, but apparently that's >> just me. > > Well, again, it'd be all right with me if we were going to get any > meaningful increment in functionality out of it, but we aren't. If you > add the column and the default in separate steps, you get the same > result and all the behavior is spec-compliant. > > There's some history here that you may not be familiar with --- we used > to have exactly this bug in regards to the mainline ALTER TABLE case. > Observe the results in PG 7.1: > > regression=# create table foo(f1 int); > CREATE > regression=# insert into foo values(1); > INSERT 3151259 1 > regression=# insert into foo values(2); > INSERT 3151260 1 > regression=# alter table foo add column f2 int default 2; > ALTER > regression=# select * from foo; > f1 | f2 > + > 1 | > 2 | > (2 rows) > > In 7.2 through 7.4 the ALTER failed instead: > regression=# alter table foo add column f2 int default 2; > ERROR: Adding columns with defaults is not implemented. > Add the column, then use ALTER TABLE SET DEFAULT. > > and by 8.0 we'd finally made it work per spec. But we got lots of flak > meanwhile from people who'd gotten used to the old behavior. So those > of us who were around then aren't eager to repeat that. The code you're > complaining about now was put in only a month after we got that problem > fixed, so the issue was plenty fresh in mind at the time. Yeah, I wasn't aware of that. I'll go revert, but I think I'll also add a big fat comment, because this is entirely non-obvious, especially because we don't disallow the cases SET NOT NULL and ADD table_constraint, which have the same darn problem. Aren't people who are used to those cases going to be pretty surprised too? -- 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] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane wrote: >>> Robert Haas writes: It's not obvious to me that it has a use case outside of casts, but it's certainly possible I'm missing something. > >>> A possible example is simplifying X + 0 to X, or X * 0 to 0. > >> Oh, I see. The times I've seen an issue with those kinds of >> expressions have always been related to selectivity estimation. > > Yeah, helping the planner recognize equivalent cases is at least as > large a reason for wanting this as any direct savings of execution time. Agreed. > I don't mind confining the feature to casts to start with, but it might > be a good idea to specify the check-function API in a way that would let > it be plugged into a more generally available call-simplification hook > later. Got any suggestions? My thought was that it should just get (type, typemod, type, typemod) and return a boolean. We could do something more complicated, like Expr -> Expr, but I'm pretty unconvinced there's any value there. I'd like to see some kind of appropriate hook for interjecting selectivity estimates for cases that we currently can't recognize, but my gut feeling is that that's insufficiently related at the problem at hand to worry about 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
[HACKERS] Re: In pg_test_fsync, use K(1024) rather than k(1000) for write size units.
Peter Eisentraut wrote:
> We use small "k" in postgresql.conf, so pg_test_fsync should use the
> same. Using "kB" would be more accurate in any case.
OK, done with the attached applied patch.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_test_fsync/pg_test_fsync.c b/contrib/pg_test_fsync/pg_test_fsync.c
index 23800d6..d075483 100644
*** a/contrib/pg_test_fsync/pg_test_fsync.c
--- b/contrib/pg_test_fsync/pg_test_fsync.c
*** test_sync(int writes_per_op)
*** 175,183
bool fs_warning = false;
if (writes_per_op == 1)
! printf("\nCompare file sync methods using one %dK write:\n", XLOG_BLCKSZ_K);
else
! printf("\nCompare file sync methods using two %dK writes:\n", XLOG_BLCKSZ_K);
printf("(in wal_sync_method preference order, except fdatasync\n");
printf("is Linux's default)\n");
--- 175,183
bool fs_warning = false;
if (writes_per_op == 1)
! printf("\nCompare file sync methods using one %dkB write:\n", XLOG_BLCKSZ_K);
else
! printf("\nCompare file sync methods using two %dkB writes:\n", XLOG_BLCKSZ_K);
printf("(in wal_sync_method preference order, except fdatasync\n");
printf("is Linux's default)\n");
*** static void
*** 391,404
test_open_syncs(void)
{
printf("\nCompare open_sync with different write sizes:\n");
! printf("(This is designed to compare the cost of writing 16K\n");
printf("in different write open_sync sizes.)\n");
! test_open_sync("16K open_sync write", 16);
! test_open_sync(" 8K open_sync writes", 8);
! test_open_sync(" 4K open_sync writes", 4);
! test_open_sync(" 2K open_sync writes", 2);
! test_open_sync(" 1K open_sync writes", 1);
}
/*
--- 391,404
test_open_syncs(void)
{
printf("\nCompare open_sync with different write sizes:\n");
! printf("(This is designed to compare the cost of writing 16kB\n");
printf("in different write open_sync sizes.)\n");
! test_open_sync("16kB open_sync write", 16);
! test_open_sync(" 8kB open_sync writes", 8);
! test_open_sync(" 4kB open_sync writes", 4);
! test_open_sync(" 2kB open_sync writes", 2);
! test_open_sync(" 1kB open_sync writes", 1);
}
/*
*** test_non_sync(void)
*** 517,523
/*
* Test a simple write without fsync
*/
! printf("\nNon-Sync'ed %dK writes:\n", XLOG_BLCKSZ_K);
printf(LABEL_FORMAT, "write");
fflush(stdout);
--- 517,523
/*
* Test a simple write without fsync
*/
! printf("\nNon-Sync'ed %dkB writes:\n", XLOG_BLCKSZ_K);
printf(LABEL_FORMAT, "write");
fflush(stdout);
--
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: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
Robert Haas writes: > Considering the number of OTHER places we'd have to break backward > compatibility, one more wouldn't bother me any, but apparently that's > just me. Well, again, it'd be all right with me if we were going to get any meaningful increment in functionality out of it, but we aren't. If you add the column and the default in separate steps, you get the same result and all the behavior is spec-compliant. There's some history here that you may not be familiar with --- we used to have exactly this bug in regards to the mainline ALTER TABLE case. Observe the results in PG 7.1: regression=# create table foo(f1 int); CREATE regression=# insert into foo values(1); INSERT 3151259 1 regression=# insert into foo values(2); INSERT 3151260 1 regression=# alter table foo add column f2 int default 2; ALTER regression=# select * from foo; f1 | f2 + 1 | 2 | (2 rows) In 7.2 through 7.4 the ALTER failed instead: regression=# alter table foo add column f2 int default 2; ERROR: Adding columns with defaults is not implemented. Add the column, then use ALTER TABLE SET DEFAULT. and by 8.0 we'd finally made it work per spec. But we got lots of flak meanwhile from people who'd gotten used to the old behavior. So those of us who were around then aren't eager to repeat that. The code you're complaining about now was put in only a month after we got that problem fixed, so the issue was plenty fresh in mind at the time. 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] new compiler warnings
On ons, 2011-01-26 at 06:33 -0500, Robert Haas wrote: > I recently started getting these: > > plpython.c: In function ‘PLy_output’: > plpython.c:3468: warning: format not a string literal and no format arguments > plpython.c: In function ‘PLy_elog’: > plpython.c:3620: warning: format not a string literal and no format arguments > plpython.c:3627: warning: format not a string literal and no format arguments > > Please fix. Which version of which compiler is showing this? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 3:29 PM, Joshua D. Drake wrote: > Not somewhat, completely. Most of the "teachers" we have are already > getting paid to work on PostgreSQL. There are some exceptions of course > but if you look at the list of people that are qualified to actually > review code, they are getting paid *for PostgreSQL*. Most people who are making their living from PostgreSQL are getting paid for work they do for customers. The work they do in the community is sponsored, incidental, something that they do for the PR value, and/or on their own time. There are only a very, very small number of people who get paid to spend a significant portion of their time hacking on PG just for the heck of it. Now, if you can get enough qualified people to volunteer to review without paying them, by all means, don't pay them - anything else would be silly. But I think that in general people who are earning their living off of PG are *more* likely to need to be paid, not less. My ability to increase the amount of PG stuff I'm doing for free is zero, if not negative. It's not that I don't want to. It's just that I require both income and sleep. That's probably not an issue for people who are just getting started in the community. Another question is whether you really need assigned mentors at all. Perhaps if newcomer Alice is assigned to mentor Bob, experienced PG hacker Charlie will feel he doesn't need to offer advice, because Bob's got it. But what if Bob (who isn't getting paid, after all) has to fly to Tajikistan that week to help somebody who IS paying him? Then Alice is left hanging. Or alternatively, what if Alice (knowing that Bob is her mentor) emails him repeatedly for advice off-list, but it turns out that Bob is out of step with the community on that particular issue[1]? Better to have Alice asking on the list and getting advice in public. Very few emails on -hackers go unanswered. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company [1] This has been known to happen. Even to people who might be referred to as Bob. Even today. I'm just saying. And don't call me Bob. -- 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: In pg_test_fsync, use K(1024) rather than k(1000) for write size units.
We use small "k" in postgresql.conf, so pg_test_fsync should use the same. Using "kB" would be more accurate in any case. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] .gitignore patch for coverage builds
Building for coverage and running the reports littered my tree with files which should probably be in .gitignore for just such a contingency. Patch attached. -Kevin *** a/.gitignore --- b/.gitignore *** *** 12,17 --- 12,26 *.mo objfiles.txt .deps/ + *.h.gcov + *.c.gcov + *.y.gcov + *.l.gcov + *.c.gcov.out + *.gcno + *.gcda + lcov.info + coverage/ # Local excludes in root directory /GNUmakefile -- 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] Extensions support for pg_dump, patch v27
Tom Lane writes: > Oh: then you're doing it wrong. If you want to remember that WITH > SCHEMA was specified, you need to explicitly store that as another > column in pg_extension. You should not be depending on the dependency > mechanism to remember that for you, any more than we'd use pg_depend to > remember a table's relnamespace. The dependency mechanism is there > to figure out the consequences of a DROP command, it's not there to > remember arbitrary facts. (And yes, I know that we've cheated on that > principle a few times before; but you can't do it here.) The thinking is that we need to have the dependency registered too, so that DROP SCHEMA will cascade to the extension. So while at it, I also used the dependency for tracking the schema. Even if I get to use a column to track the schema, I will have to maintain registering the dependency. Should I do that? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane wrote: >> Robert Haas writes: >>> It's not obvious to me that it has a use case outside of casts, but >>> it's certainly possible I'm missing something. >> A possible example is simplifying X + 0 to X, or X * 0 to 0. > Oh, I see. The times I've seen an issue with those kinds of > expressions have always been related to selectivity estimation. Yeah, helping the planner recognize equivalent cases is at least as large a reason for wanting this as any direct savings of execution time. I don't mind confining the feature to casts to start with, but it might be a good idea to specify the check-function API in a way that would let it be plugged into a more generally available call-simplification hook later. 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] [COMMITTERS] pgsql: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
On Wed, Jan 26, 2011 at 3:48 PM, Tom Lane wrote: >> Well, actually, what I thought was that the rowtype *should* act >> exactly like a separately-declared composite rowtype. Which is to >> say, it shouldn't have a default, because a separately-declared >> composite rowtype *can't have a default*. If that's not the consensus >> position, so be it, but it made sense to me. > > The fact that a separately-declared composite type can't have defaults > is an implementation restriction. It is a feature required by SQL spec, > so we ought to plan on doing it someday, IMO. OK. Well, I think we need to document that somewhere, then, at least in a comment; maybe in the documentation. > It's conceivable that once we have that implemented, we will decide that > table rowtypes should act differently from separately-declared composite > types to the extent of not honoring defaults inherited from their table. > That would be a terrible violation of POLA in my opinion, but we might > have to do it for backwards compatibility's sake. No, I wouldn't support that at all. I was simply assuming that there was no intention for composite types to ever support defaults or constraints, and like I say, we don't seem to worry about it anywhere else (INSERT, SET NOT NULL, etc). I maintain it's pretty inconsistent to do it only in this case, regardless of whether it's technically a standards-compliance regression. The patch may make us less consistent with the SQL spec, and it sounds like there are a couple other votes for not doing it on that basis, but it makes us more consistent with ourselves. If we ever support defaults and constraints on composite types generally, then the behavior you're imagining would seems like it would be the right thing to do. Considering the number of OTHER places we'd have to break backward compatibility, one more wouldn't bother me any, but apparently that's just me. -- 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: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
Just a small comment: If someone offered me $15 to mentor a reviewer, I would tell him to kindly go away. If the same person were to offer me a $15 t-shirt saying I mentored the review (or whatever), I would consider it. Yes, I know I could buy the t-shirt with the money. People are strange that way. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
Robert Haas writes: > On Wed, Jan 26, 2011 at 1:32 PM, Tom Lane wrote: >> I will agree that a language lawyer could argue that a table rowtype >> doesn't have to act like a separately-declared composite type, but >> that surely doesn't meet the POLA. > Well, actually, what I thought was that the rowtype *should* act > exactly like a separately-declared composite rowtype. Which is to > say, it shouldn't have a default, because a separately-declared > composite rowtype *can't have a default*. If that's not the consensus > position, so be it, but it made sense to me. The fact that a separately-declared composite type can't have defaults is an implementation restriction. It is a feature required by SQL spec, so we ought to plan on doing it someday, IMO. It's conceivable that once we have that implemented, we will decide that table rowtypes should act differently from separately-declared composite types to the extent of not honoring defaults inherited from their table. That would be a terrible violation of POLA in my opinion, but we might have to do it for backwards compatibility's sake. What I *don't* want to do is introduce another not-per-spec behavior that we will have to make such hard choices about when the time comes, when we aren't getting any meaningful functionality increment out of allowing the not-per-spec behavior. And that's exactly the situation with this ALTER case. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane wrote: >>> If you didn't mind inverting the sense of the result >>> then we could use "EXECUTE IF function_name". > >> What about borrowing from the trigger syntax? > >> WITH FUNCTION function_name (argument_type, [...]) WHEN >> function_that_returns_true_when_the_call_is_needed > > That's a good thought. Or we could use WHEN NOT check_function if you > want to keep to the negative-result semantics. Seems a bit contorted; I don't see any particular reason to prefer positive vs negative semantics in this case. >>> One point worth making here is that eval_const_expressions() does not >>> currently care very much whether a function call came from cast syntax >>> or explicitly. It might be worth thinking about whether we want to have >>> a generic this-function-call-is-a-no-op simplifier hook available for >>> *all* functions not just those that are casts. I'm not sure we want to >>> pay the overhead of another pg_proc column, but it's something to think >>> about. > >> It's not obvious to me that it has a use case outside of casts, but >> it's certainly possible I'm missing something. > > A possible example is simplifying X + 0 to X, or X * 0 to 0. I've never > wanted to inject such datatype-specific knowledge directly into the > planner, but if we viewed it as function-specific knowledge supplied by > a per-function helper routine, maybe it would fly. Knowing that a cast > function does nothing useful for particular cases could be seen as a > special case of this type of simplification. Oh, I see. The times I've seen an issue with those kinds of expressions have always been related to selectivity estimation. For example, you'd like to get a selectivity estimate of 1-nullfrac for (x+0)=x and 0 for (x+1)=x, and maybe (1-nullfrac)/2 for (x%2)=0. This would only handle the first of those cases, so I'm inclined to think it's too weak to have much general utility. The casting cases can, I think, have a much larger impact - they occur more often in practice, and if you can (e.g.) avoid an entire table rewrite, that's a pretty big deal. Another semi-related problem case I've run across is that CASE WHEN ... THEN 1 WHEN ... THEN 2 END ought to be known to only ever emit 1 and 2, and the selectivity of comparing that to some other value ought to be computed on that basis. But now I'm really wandering off into the weeds. -- 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] SSI patch version 14
On Wed, Jan 26, 2011 at 02:36:25PM -0600, Kevin Grittner wrote: > Same benefit in terms of exercising more lines of code, but > *without* exposing the uninitialized structure to other threads. Won't this cause a deadlock because locks are being acquired out of order? Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] SSI patch version 14
I wrote: > You're right. How about this?: That's even worse. I'm putting back to where you had it and taking a break before I do anything else that dumb. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions support for pg_dump, patch v27
Dimitri Fontaine writes: > Tom Lane writes: >> Mph. Although such an extension should certainly carry a dependency on >> the schema it's using, I'm not sure that it makes sense to consider that >> the extension (as opposed to its contained objects) belongs to the >> schema. > Well yes, extension are not living in a schema, we just offer users a > way to influence the script as far as where the extension's objects are > to be found and register a dependency so that it's easy to remember what > the users asked. So that for example we are able to act the same way on > pg_restore. Oh: then you're doing it wrong. If you want to remember that WITH SCHEMA was specified, you need to explicitly store that as another column in pg_extension. You should not be depending on the dependency mechanism to remember that for you, any more than we'd use pg_depend to remember a table's relnamespace. The dependency mechanism is there to figure out the consequences of a DROP command, it's not there to remember arbitrary facts. (And yes, I know that we've cheated on that principle a few times before; but you can't do it here.) 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] arrays as pl/perl input arguments [PATCH]
On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: > On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin wrote: >> Hi, >> >> On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: >> >>> On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin wrote: > > On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: > >> On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: >> >>> You mean packing both a string representation and a reference to a >>> single SV * value? >> >> Dunno, I'm not a guts guy. > > Well, neither me (I haven't used much of the guts api there). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). >>> [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? >>> >>> So its been over a week with no comments. ISTM there were more people >>> against adding yet another GUC. Barring objection ill finish the >>> missing parts of the POC patch I posted and submit that. >> >> I've played with that patch just today. I found a problem with it, when I >> tried to use the array in a string context the backend segfaulted with: >> "WARNING: Deep recursion on subroutine "main::encode_array_literal" at -e >> line 74" just before the segfault. I think the problem is in the regexp >> check in 'encode_array_literal' (it's obviously reversed comparing with the >> original one), > > Yeah, I noticed that after I sent it out :(. > >> but it still segfaults after I fixed that. > > I seem to recall fixing this post email as well... Can you provide the > function that broke so I can double check? (Or was it part of the > regression test?) Sure, it's really simple (and the plperl_array regressions tests exposes this problem as well): CREATE OR REPLACE FUNCTION test_array(INTEGER[]) RETURNS TEXT AS $$ my $array = shift; print "$array"."\n"; $$ LANGUAGE plperl; /A I will look into this problem tomorrow unless you'll be lucky to fix it before that. Thank you for the review and your patch. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] SSI patch version 14
Dan Ports wrote: > Isn't this placement the same as the version we had before that > didn't work? You're right. How about this?: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=86b839291e2588e59875fb87d05432f8049575f6 Same benefit in terms of exercising more lines of code, but *without* exposing the uninitialized structure to other threads. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, 2011-01-26 at 14:15 -0500, Robert Haas wrote: > On Wed, Jan 26, 2011 at 1:32 PM, Richard Broersma > wrote: > > On Wed, Jan 26, 2011 at 3:12 AM, Simon Riggs wrote: > >> You're paying the reviewers; are you paying the mentors? > > > > The answer to this question is that we can fund mentor (teacher). However, > > the amount to fund a mentor would be significantly less that the amount to > > fund a reviewer (student). The mentors are part of the educational process. > > Usually, in an educational process, it's the teachers who get paid, > and the students who have to pay to get educated. I realize this is > somewhat different because we want to encourage people to get involved > in the project, but it still seems weird. Not somewhat, completely. Most of the "teachers" we have are already getting paid to work on PostgreSQL. There are some exceptions of course but if you look at the list of people that are qualified to actually review code, they are getting paid *for PostgreSQL*. Now, that isn't to say you don't bring up a good point, you do. I think it may be worthwhile to find a way to also compensate mentors but as you say the goal here is encourage people to get involved. However there is the underlying goal of educating future PostgreSQL contributors, and let's face it --- reviewing code sucks and money is a great motivator (especially in today's economy or if you are a starving student). > And I actually kind of > agree with David Fetter. Aside from the scenario he mentioned (people > who don't get paid stop volunteering, a phenomenon that has been > documented to occur in other contexts), You have people that are in it for the money. There is nothing wrong with that. Hopefully through this grant they will gain enough skill and public notice to pick up a job where they might be able to give back to the community on a paid basis (probably not, but maybe). If people stop volunteering cause there is no money, then we care why? They are likely not vested in the community anyway. Either way, the mission has been accomplished. They were paid to be educated and learn the review/commitfest process, they did so. If they wish to move on, that's up to them. Do we want them to stay? Of course! However, I fail how to see the concern has anything to do with the grant process. > there's also the problem that > people might sign up to get the money but then do a lousy job. Well that is the risk we all face and if the mentor feedback was that the person did a lousy job (let's assume they were just lazy, not that they tried really hard but weren't up to the task), then they would risk ever receiving future grants. > People > sometimes do a lousy job now too, but at least we can count on the > fact that everyone who signs up to do it has some intrinsic > motivation. I think anyone who is going to make it through a grant process specifically for this purpose is going to have some intrinsic motivation beyond money. We aren't talking about shelling out 50k here. Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- 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] SSI patch version 14
On Wed, Jan 26, 2011 at 01:42:23PM -0600, Kevin Grittner wrote: > Dan, do you still have access to that machine you were using for the > DBT-2 runs? Could we get a coverage run with and without > TEST_OLDSERXID defined? Sure, I'll give it a shot (once I figure out how to enable coverage...) Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] SSI patch version 14
On Wed, Jan 26, 2011 at 10:01:28AM -0600, Kevin Grittner wrote: > In looking at it just now, I noticed that after trying it in a > couple different places what was left in the repository was not the > optimal version for code coverage. I've put this back to the > version which did a better job, for reasons described in the commit > comment: Isn't this placement the same as the version we had before that didn't work? Specifically, aren't we going to have problems running with TEST_OLDSERXID enabled because CreatePredTran succeeds and links a new SerializableXact into the active list, but we don't initialize it before we drop SerializableXactHashLock to call SummarizeOldestCommittedSxact? I seem to recall SummarizeOldestCommittedSxact failing before because of the uninitialized entry, but more generally since we drop the lock something else might scan the list. (This isn't a problem in the non-test case because we'd only do that if CreatePredTran fails.) Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] [COMMITTERS] pgsql: Remove arbitrary ALTER TABLE .. ADD COLUMN restriction.
On Wed, Jan 26, 2011 at 1:32 PM, Tom Lane wrote: >> I think you're conflating the table with its row type, and I'd like to >> see some prior writing indicating otherwise. > > I will agree that a language lawyer could argue that a table rowtype > doesn't have to act like a separately-declared composite type, but > that surely doesn't meet the POLA. Well, actually, what I thought was that the rowtype *should* act exactly like a separately-declared composite rowtype. Which is to say, it shouldn't have a default, because a separately-declared composite rowtype *can't have a default*. If that's not the consensus position, so be it, but it made sense to me. -- 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] SSI patch version 14
"Kevin Grittner" wrote: > Alvaro Herrera wrote: > >> BTW did you try "make coverage" and friends? See >> http://www.postgresql.org/docs/9.0/static/regress-coverage.html >> and >> http://developer.postgresql.org/~petere/coverage/ > > I had missed that; thanks for pointing it out! > > I'm doing a coverage build now, to see what coverage we get from > `make check` (probably not much) and `make dcheck`. Well, that was a bit better than I expected. While the overall code coverage for PostgreSQL source code is: Overall coverage rate: lines..: 64.8% (130296 of 201140 lines) functions..: 72.0% (7997 of 11109 functions) The coverage for predicate.c, after running both check and dcheck, was (formatted to match above): lines..: 69.8% (925 of 1325 lines) functions..: 85.7% (48 of 56 functions) Most of what was missed was in the SLRU or 2PC code, which is expected. I'll bet that the DBT-2 runs, between the "normal" and TEST_OLDSERXID flavors, would get us about 2/3 of the way from those numbers toward 100%, with almost all the residual being in 2PC. Does anyone have suggestions for automated 2PC tests? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
Robert Haas writes: > On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane wrote: >> If you didn't mind inverting the sense of the result >> then we could use "EXECUTE IF function_name". > What about borrowing from the trigger syntax? > WITH FUNCTION function_name (argument_type, [...]) WHEN > function_that_returns_true_when_the_call_is_needed That's a good thought. Or we could use WHEN NOT check_function if you want to keep to the negative-result semantics. >> One point worth making here is that eval_const_expressions() does not >> currently care very much whether a function call came from cast syntax >> or explicitly. It might be worth thinking about whether we want to have >> a generic this-function-call-is-a-no-op simplifier hook available for >> *all* functions not just those that are casts. I'm not sure we want to >> pay the overhead of another pg_proc column, but it's something to think >> about. > It's not obvious to me that it has a use case outside of casts, but > it's certainly possible I'm missing something. A possible example is simplifying X + 0 to X, or X * 0 to 0. I've never wanted to inject such datatype-specific knowledge directly into the planner, but if we viewed it as function-specific knowledge supplied by a per-function helper routine, maybe it would fly. Knowing that a cast function does nothing useful for particular cases could be seen as a special case of this type of simplification. 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin wrote: > Hi, > > On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: > >> On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker wrote: >>> On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin >>> wrote: On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: > On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: > >> You mean packing both a string representation and a reference to a >> single SV * value? > > Dunno, I'm not a guts guy. Well, neither me (I haven't used much of the guts api there). >>> >>> Find attached a proof of concept that modifies Alexey's patch to do >>> the above (using the overload example I and others posted). >> [ ... ] >>> Thoughts? Should I polish this a bit more? Or do we like the GUC better? >> >> So its been over a week with no comments. ISTM there were more people >> against adding yet another GUC. Barring objection ill finish the >> missing parts of the POC patch I posted and submit that. > > I've played with that patch just today. I found a problem with it, when I > tried to use the array in a string context the backend segfaulted with: > "WARNING: Deep recursion on subroutine "main::encode_array_literal" at -e > line 74" just before the segfault. I think the problem is in the regexp check > in 'encode_array_literal' (it's obviously reversed comparing with the > original one), Yeah, I noticed that after I sent it out :(. > but it still segfaults after I fixed that. I seem to recall fixing this post email as well... Can you provide the function that broke so I can double check? (Or was it part of the regression test?) Thanks for taking the time to play with it. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
On Wed, Jan 26, 2011 at 11:15 AM, Robert Haas wrote: > Usually, in an educational process, it's the teachers who get paid, > and the students who have to pay to get educated. I realize this is > somewhat different because we want to encourage people to get involved > in the project, but it still seems weird. This is probably a good point. I've never mentored, taught, authored a patch or review, so I can say what is similar or different. > People > sometimes do a lousy job now too, but at least we can count on the > fact that everyone who signs up to do it has some intrinsic > motivation. > > http://www.nytimes.com/2005/05/15/books/chapters/0515-1st-levitt.html > Interesting. -- Regards, Richard Broersma Jr.
Re: [HACKERS] Extensions support for pg_dump, patch v27
Tom Lane writes: > Mph. Although such an extension should certainly carry a dependency on > the schema it's using, I'm not sure that it makes sense to consider that > the extension (as opposed to its contained objects) belongs to the > schema. If we think that extensions live inside schemas then it's > logically difficult for an extension to own objects scattered across > multiple schemas, which is surely a restriction we do not want. So I'd > have expected that extensions use unqualified names, like languages or > tablespaces. That being the case, there is no reason why pg_dump ought > to be making any such test. Well yes, extension are not living in a schema, we just offer users a way to influence the script as far as where the extension's objects are to be found and register a dependency so that it's easy to remember what the users asked. So that for example we are able to act the same way on pg_restore. Now, pg_dump makes no such test already, but a query is using a JOIN to list extensions and their target schema, where it's possible that the target has not been recorded by recordDependencyOn(): in this case the whole extension's is filtered out of the resultset. Quick and dirty fix, I proposed to LEFT JOIN in the pg_dump query… > There are places where pg_dump refuses to dump objects contained in > pg_catalog on the grounds that they're system objects, but that > heuristic ought not apply to extensions IMO, even if you don't agree > with the conclusion of the preceding paragraph. Any extension is, by > definition, not built-in. Agreed. The problem we're having here is how to implement all that yet fully support adminpack. The design we're talking about is the same. >> Well I proposed is nothing more than a crock. What I'd like us to do >> instead is ERRORing out when you want to use pg_catalog for an >> extension. > > Right offhand I'm not seeing the need for such a restriction, though > certainly I'd not cry a lot if we had to impose it. ISTM what we've got > here is some bogus what-to-dump rules in pg_dump. Extensions should > always be dumped. Agreed. Trying to solve an implementation detail, and that's the easier way to prevent users from shooting themselves in the foot here. >> What do we want to do with adminpack? Including its functions into >> core, or have it use another schema? I don't think an extension >> installing its objects into pg_catalog is that good an idea > > I'm trying to avoid having an opinion on that. The reasons for it might > or might not be very good, but I'd rather that the extension mechanism > didn't break the ability for people to make such decisions. You still can do one of the following commands, where you're not having a say on the real schema that adminpack will use because it's not relocatable and not paying attention to @extschema@, but apart from this lie everything will just work. I'd be happy to ship with such a lie, but I can see why people want something different. CREATE EXTENSION adminpack; CREATE EXTENSION adminpack WITH SCHEMA utils; Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RRR] [HACKERS] Seeking Mentors for Funded Reviewers
"David E. Wheeler" writes: > I think M. Fetter is completely wrong. If people are rethinking > whether they should volunteer based on whether other people are being > funded for their time to review patches, we don't want such people > around anyway. Let them leave. I can see his concern though: we have to be very careful to avoid establishing perverse incentives. The larger picture is that quite a few people are paid to work on Postgres already --- me, for instance. That doesn't seem to have discouraged other people from working on it on their own time. But I'm not paid according to how many bugs I find, and wouldn't want to be. I don't have a problem with funding people to work on Postgres. We just have to be careful that the grants aren't set up in a way that might encourage people to game the system. I'm also not sure that "review a patch" is a well-chosen specific goal to have here, especially not for people who've not been around the project at all. It's hard enough for people who *do* have a lot of context to do useful reviews. 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] Extensions support for pg_dump, patch v27
Dimitri Fontaine writes: > We could use get_extension_namespace() just after recoding the > dependency and error out if we don't find the arguments we gave to > recordDependencyOn() so that we're not duplicating code. That will > cover any pinned schema. I'm preparing a patch to do that. Kids are falling asleep and the patch there: http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=5d0834a8de54a52c601e4fd04aee2d19d1eef4c6 > What do we want to do with adminpack? Including its functions into > core, or have it use another schema? I don't think an extension > installing its objects into pg_catalog is that good an idea… As we're still waiting on some decision here, and some others in previous mails of this same thread, I'm waiting some more before to produce the next patch in the series. See http://archives.postgresql.org/pgsql-hackers/2011-01/msg02385.php http://archives.postgresql.org/pgsql-hackers/2011-01/msg02392.php Of course the git repository is uptodate should you want to try the newest code without waiting on me for the next patch release. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: > On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker wrote: >> On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin >> wrote: >>> >>> On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote: >>> On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote: > You mean packing both a string representation and a reference to a single > SV * value? Dunno, I'm not a guts guy. >>> >>> Well, neither me (I haven't used much of the guts api there). >> >> Find attached a proof of concept that modifies Alexey's patch to do >> the above (using the overload example I and others posted). > [ ... ] >> Thoughts? Should I polish this a bit more? Or do we like the GUC better? > > So its been over a week with no comments. ISTM there were more people > against adding yet another GUC. Barring objection ill finish the > missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: "WARNING: Deep recursion on subroutine "main::encode_array_literal" at -e line 74" just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), but it still segfaults after I fixed that. Other than that, the approach looks good to me, I'm for eliminating the GUC setting in favor of it. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] SSI patch version 14
Alvaro Herrera wrote: > BTW did you try "make coverage" and friends? See > http://www.postgresql.org/docs/9.0/static/regress-coverage.html > and > http://developer.postgresql.org/~petere/coverage/ I had missed that; thanks for pointing it out! I'm doing a coverage build now, to see what coverage we get from `make check` (probably not much) and `make dcheck`. Dan, do you still have access to that machine you were using for the DBT-2 runs? Could we get a coverage run with and without TEST_OLDSERXID defined? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Explain with schema
Cristiano Duarte writes: > I was thinking about an old 2007 topic, where schema > qualification was proposed on the EXPLAIN output > (http://archives.postgresql.org/pgsql- > hackers/2007-06/msg00473.php). > Besides my need for this "feature" for my own PgFoundry > project (that need to parse the explain output), at that time, > it seemed that a XML output maybe fit this requirement. > For some time, postgresql has XML output. So, is it possible > to place, at least on the XML output, the schema name of the > objects involved? See EXPLAIN (VERBOSE, FORMAT XML) ... 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] Extensions support for pg_dump, patch v27
Dimitri Fontaine writes: > So in his tests, Itagaki was tempted to issue the following statement: > CREATE EXTENSION adminpack WITH SCHEMA pg_catalog; > That's supposed to register a dependency from the extension to its > declared hosting schema (adminpack is not relocatable so that entirely > depends on its script - which forces pg_catalog). > Then you get the same problems as with any other object that lives into > this schema, pg_dump will avoid dumping its definition ... Mph. Although such an extension should certainly carry a dependency on the schema it's using, I'm not sure that it makes sense to consider that the extension (as opposed to its contained objects) belongs to the schema. If we think that extensions live inside schemas then it's logically difficult for an extension to own objects scattered across multiple schemas, which is surely a restriction we do not want. So I'd have expected that extensions use unqualified names, like languages or tablespaces. That being the case, there is no reason why pg_dump ought to be making any such test. There are places where pg_dump refuses to dump objects contained in pg_catalog on the grounds that they're system objects, but that heuristic ought not apply to extensions IMO, even if you don't agree with the conclusion of the preceding paragraph. Any extension is, by definition, not built-in. > Well I proposed is nothing more than a crock. What I'd like us to do > instead is ERRORing out when you want to use pg_catalog for an > extension. Right offhand I'm not seeing the need for such a restriction, though certainly I'd not cry a lot if we had to impose it. ISTM what we've got here is some bogus what-to-dump rules in pg_dump. Extensions should always be dumped. > What do we want to do with adminpack? Including its functions into > core, or have it use another schema? I don't think an extension > installing its objects into pg_catalog is that good an idea I'm trying to avoid having an opinion on that. The reasons for it might or might not be very good, but I'd rather that the extension mechanism didn't break the ability for people to make such decisions. 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] SSI patch version 14
Excerpts from Kevin Grittner's message of mié ene 26 14:07:18 -0300 2011: > Simon Riggs wrote: > > Pounding for hours on 16 CPU box sounds good. What diagnostics or > > instrumentation are included with the patch? How will we know > > whether pounding for hours is actually touching all relevant parts > > of code? I've done such things myself only to later realise I > > wasn't actually testing the right piece of code. > > We've looked at distributions of failed transactions by ereport > statement. This confirms that we are indeed exercising the vast > majority of the code. See separate post for how we pushed execution > into the summarization path to test code related to that. BTW did you try "make coverage" and friends? See http://www.postgresql.org/docs/9.0/static/regress-coverage.html and http://developer.postgresql.org/~petere/coverage/ -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 3: add facility to identify further no-work cases
On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane wrote: > Robert Haas writes: >> ... A side issue is that I really >> want to avoid adding a new parser keyword for this. It doesn't bother >> me too much to add keywords for really important and user-visible >> features, but when we're adding stuff that's only going to be used by >> 0.1% of our users it's really better if we can avoid it, because it >> slows down the parser. Maybe we could do something like: > >> CREATE CAST (source_type AS target_type) >> WITH FUNCTION function_name (argument_type, [, ...]) >> [ ANALYZE USING function_name ] >> [ AS ASSIGNMENT | AS IMPLICIT ] > > I'm not terribly thrilled with the suggestion of "ANALYZE" here, because > given the way we use that word elsewhere, people are likely to think it > means something to do with statistics collection; or at least that it > implies some deep and complicated analysis of the cast. > > I suggest using a phrase based on the idea that this function tells you > whether you can skip the cast, or (if the sense is inverted) whether the > cast has to be executed. "SKIP IF function_name" would be nice but SKIP > isn't an extant keyword either. The best variant that I can come up > with after a minute's contemplation of kwlist.h is "NO WORK IF > function_name". If you didn't mind inverting the sense of the result > then we could use "EXECUTE IF function_name". What about borrowing from the trigger syntax? WITH FUNCTION function_name (argument_type, [...]) WHEN function_that_returns_true_when_the_call_is_needed > One point worth making here is that eval_const_expressions() does not > currently care very much whether a function call came from cast syntax > or explicitly. It might be worth thinking about whether we want to have > a generic this-function-call-is-a-no-op simplifier hook available for > *all* functions not just those that are casts. I'm not sure we want to > pay the overhead of another pg_proc column, but it's something to think > about. It's not obvious to me that it has a use case outside of casts, but it's certainly possible I'm 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
