Re: [HACKERS] pg_execute_from_file review

2010-12-08 Thread Dimitri Fontaine
Tom Lane writes: > Er ... what good is that? A non-relocatable extension doesn't *need* > any such substitution, because it knows perfectly well what schema it's > putting its stuff into. Only the relocatable case has use for it. So > you might as well drop the substitution mechanism entirely.

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Tom Lane
Dimitri Fontaine writes: > Tom Lane writes: >> Anyway, in a less blue-sky vein: we could fix some of these problems by >> having an explicit relocatable-or-not property for extensions. If it is >> relocatable, it's required to keep all its owned objects in the target >> schema, and ALTER EXTENSI

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread David E. Wheeler
On Dec 7, 2010, at 1:17 PM, Dimitri Fontaine wrote: >> Anyway, in a less blue-sky vein: we could fix some of these problems by >> having an explicit relocatable-or-not property for extensions. If it is >> relocatable, it's required to keep all its owned objects in the target >> schema, and ALTER

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Dimitri Fontaine
Tom Lane writes: > I confess to not having paid a whole lot of attention to the threads > about this feature, so maybe this point has been addressed already, but: > what of the schema itself? That is, if an extension has some/all of its > objects in a given schema, is that schema itself one of th

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Tom Lane
Robert Haas writes: > I think you've gotten to the heart of the matter here. Extensions > need to either be schema objects, or not. If they are, let's go all > the way and compel everything in the extension to live in the schema > that owns it, and make the extension itself live in that schema t

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Robert Haas
On Tue, Dec 7, 2010 at 11:38 AM, Tom Lane wrote: > Anyway the main problem at the moment is that the proposed design is > meant to allow "relocatable" extensions, but it doesn't behave > pleasantly in the case where somebody tries to relocate a > non-relocatable extension. > > It also strikes me t

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Tom Lane
Andrew Dunstan writes: > On 12/07/2010 11:13 AM, Robert Haas wrote: >> Why not? This feature seems to be pretty heavily designed around the >> assumption that everything's going to live in one schema, so is there >> any harm in making that explicit? > In previous discussions IIRC the consensus w

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Andrew Dunstan
On 12/07/2010 11:13 AM, Robert Haas wrote: On Mon, Dec 6, 2010 at 2:36 PM, Tom Lane wrote: There's a difference between whether an extension as such is considered to belong to a schema and whether its contained objects do. We can't really avoid the fact that functions, operators, etc must be

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Robert Haas
On Mon, Dec 6, 2010 at 2:36 PM, Tom Lane wrote: > There's a difference between whether an extension as such is considered > to belong to a schema and whether its contained objects do.  We can't > really avoid the fact that functions, operators, etc must be assigned to > some particular schema.  It

Re: [HACKERS] pg_execute_from_file review

2010-12-07 Thread Dimitri Fontaine
Tom Lane writes: > There's a difference between whether an extension as such is considered > to belong to a schema and whether its contained objects do. We can't > really avoid the fact that functions, operators, etc must be assigned to > some particular schema. It seems not particularly importa

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread David E. Wheeler
On Dec 6, 2010, at 11:36 AM, Tom Lane wrote: > There's a difference between whether an extension as such is considered > to belong to a schema and whether its contained objects do. We can't > really avoid the fact that functions, operators, etc must be assigned to > some particular schema. Rig

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Tom Lane
"David E. Wheeler" writes: > The other question I have, though, is how important is it to have extensions > live in a particular schema since there seems to be no advantage to doing so. > With the current patch, I can put extension "foo" in schema "bar", but I > can't put any other extension na

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread David E. Wheeler
On Dec 6, 2010, at 11:12 AM, Tom Lane wrote: > Well, I don't put any stock in the idea that it's important for existing > module .sql files to be usable as-is as extension definition files. If > it happens to fall out that way, fine, but we shouldn't give up anything > else to get that. I agree,

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Tom Lane
"David E. Wheeler" writes: > On Dec 6, 2010, at 10:43 AM, Tom Lane wrote: >> (On the other hand, if we *could* avoid using any explicit >> substitutions, it would certainly ease testing of extension files no? >> They'd be sourceable into psql then.) > Yes. And extension authors would not have to

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread David E. Wheeler
On Dec 6, 2010, at 10:43 AM, Tom Lane wrote: > That's an interesting idea, but I'm not sure it's wise to design around > the assumption that we won't need substitutions ever. What I was > thinking was that we should try to limit knowledge of the substitution > behavior to the extension definition

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Tom Lane
"David E. Wheeler" writes: > On Dec 6, 2010, at 7:19 AM, Tom Lane wrote: >> On the whole I'd prefer not to have any substitution functionality >> hard-wired into pg_execute_file either, though I can see the argument >> that it's necessary for practical use. Basically I'm concerned that >> replace

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Robert Haas
On Mon, Dec 6, 2010 at 12:41 PM, Tom Lane wrote: > Robert Haas writes: >> On Sun, Dec 5, 2010 at 6:01 PM, Tom Lane wrote: >>> Why is there a variadic replace() in this patch at all?  It seems just >>> about entirely unrelated to the stated purpose of the patch, as well >>> as being of dubious us

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread David E. Wheeler
On Dec 6, 2010, at 7:19 AM, Tom Lane wrote: > On the whole I'd prefer not to have any substitution functionality > hard-wired into pg_execute_file either, though I can see the argument > that it's necessary for practical use. Basically I'm concerned that > replace-equivalent behavior is not going

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Tom Lane
Robert Haas writes: > On Sun, Dec 5, 2010 at 6:01 PM, Tom Lane wrote: >> Why is there a variadic replace() in this patch at all? It seems just >> about entirely unrelated to the stated purpose of the patch, as well >> as being of dubious usefulness. When would it be superior to >>replac

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Tom Lane
Dimitri Fontaine writes: > Tom Lane writes: >> Why is there a variadic replace() in this patch at all? It seems just >> about entirely unrelated to the stated purpose of the patch, as well >> as being of dubious usefulness. > It used not to being exposed at the SQL level, but just an internal l

Re: [HACKERS] pg_execute_from_file review

2010-12-06 Thread Dimitri Fontaine
Tom Lane writes: > Why is there a variadic replace() in this patch at all? It seems just > about entirely unrelated to the stated purpose of the patch, as well > as being of dubious usefulness. It used not to being exposed at the SQL level, but just an internal loop in pg_execute_sql_file() when

Re: [HACKERS] pg_execute_from_file review

2010-12-05 Thread Robert Haas
On Sun, Dec 5, 2010 at 6:01 PM, Tom Lane wrote: > Itagaki Takahiro writes: >> On Fri, Dec 3, 2010 at 18:02, Dimitri Fontaine >> wrote: >>> My understanding is that the variadic form shadows the other one in a >>> way that it's now impossible to call it from SQL level. That's the >>> reason why

Re: [HACKERS] pg_execute_from_file review

2010-12-05 Thread Itagaki Takahiro
On Mon, Dec 6, 2010 at 08:01, Tom Lane wrote: > Why is there a variadic replace() in this patch at all? It seems just > about entirely unrelated to the stated purpose of the patch, as well > as being of dubious usefulness. As I wrote in the previous mail, the most important part of the patch for

Re: [HACKERS] pg_execute_from_file review

2010-12-05 Thread Tom Lane
Itagaki Takahiro writes: > On Fri, Dec 3, 2010 at 18:02, Dimitri Fontaine wrote: >> My understanding is that the variadic form shadows the other one in a >> way that it's now impossible to call it from SQL level. That's the >> reason why I did the (text, text, text, VARIADIC text) version before,

Re: [HACKERS] pg_execute_from_file review

2010-12-04 Thread Dimitri Fontaine
Itagaki Takahiro writes: > The VARIADIC version doesn't hide the 3-args version. I tested the > behavior by printf-debug. The planner seems to think the non VARIADIC > version is the best-matched one when 3 arguments are passed. Nice! All's left is then the commit, right? :) Regards, -- Dimitri

Re: [HACKERS] pg_execute_from_file review

2010-12-03 Thread Itagaki Takahiro
On Fri, Dec 3, 2010 at 18:02, Dimitri Fontaine wrote: >   Schema   |  Name   | Result data type | Argument data types |  Type > +-+--+-+ >  pg_catalog | replace | text             | text, VARIADIC text | normal >  pg_catalog | replac

Re: [HACKERS] pg_execute_from_file review

2010-12-03 Thread Dimitri Fontaine
Itagaki Takahiro writes: > I fixed and cleanup some of codes in it; v9 patch attached. Please check > my modifications, and set the status to "Ready to Committer" if you find > no problems. I think documentation and code comments might need to be > checked by native English speakers. Many thanks,

Re: [HACKERS] pg_execute_from_file review

2010-12-02 Thread Itagaki Takahiro
On Thu, Dec 2, 2010 at 20:00, Dimitri Fontaine wrote: > Please find attached the v8 version of the patch, that fixes the following: I fixed and cleanup some of codes in it; v9 patch attached. Please check my modifications, and set the status to "Ready to Committer" if you find no problems. I thin

Re: [HACKERS] pg_execute_from_file review

2010-12-02 Thread Dimitri Fontaine
Hi, Please find attached the v8 version of the patch, that fixes the following: Itagaki Takahiro writes: > * pg_read_binary_file_internal() should return not only the contents > as char * but also the length, because the file might contain 0x00. > In addition, null-terminations for the conte

Re: [HACKERS] pg_execute_from_file review

2010-12-01 Thread Itagaki Takahiro
On Thu, Dec 2, 2010 at 07:00, Dimitri Fontaine wrote: > Here's the result: > > dim=# \df pg_exe*|replace_*|*binary* >                                     List of functions >   Schema   |         Name          | Result data type |    Argument data > types    |  Type > +---

Re: [HACKERS] pg_execute_from_file review

2010-12-01 Thread Dimitri Fontaine
Dimitri Fontaine writes: > Itagaki Takahiro writes: >> My suggestion is to introduce pg_read_binary_file() function that can >> read any files in the server, and make CREATE EXTENSION to use the >> function. Of course, pg_execute_[sql|from]_file() can simplify queries > > It seems like all you're

Re: [HACKERS] pg_execute_from_file review

2010-12-01 Thread Dimitri Fontaine
Itagaki Takahiro writes: > My suggestion is to introduce pg_read_binary_file() function that can > read any files in the server, and make CREATE EXTENSION to use the > function. Of course, pg_execute_[sql|from]_file() can simplify queries It seems like all you're missing in the current patch is t

Re: [HACKERS] pg_execute_from_file review

2010-12-01 Thread Itagaki Takahiro
On Tue, Nov 30, 2010 at 18:47, Dimitri Fontaine wrote: > Itagaki Takahiro writes: >> There are no discussion yet for 1, but I think we need some restrictions > > Well, as a first level of restrictions, the function is superuser > only. I understand and share your concerns, but as the main use for

Re: [HACKERS] pg_execute_from_file review

2010-11-30 Thread Dimitri Fontaine
Itagaki Takahiro writes: > client_encoding won't work at all because read_sql_queries_from_file() > uses pg_verifymbstr(), that is verify the input with *server_encoding*. > > Even if we replace it with pg_verify_mbstr(client_encoding, ...) and > pg_do_encoding_conversion(from client_encoding to s

Re: [HACKERS] pg_execute_from_file review

2010-11-30 Thread Dimitri Fontaine
Itagaki Takahiro writes: > I think there are two topics here: > 1. Do we need to restrict locations in which sql files are executable? > 2. Which is better, pg_execute_sql_file() or EXECUTE pg_read_file() ? > > There are no discussion yet for 1, but I think we need some restrictions Well, as

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Itagaki Takahiro
On Tue, Nov 30, 2010 at 08:56, Alvaro Herrera wrote: >> > * I hope pg_execute_from_file (and pg_read_file) had an option >> >   to specify an character encoding of the file. Especially, SJIS >> >   is still used widely, but it is not a valid server encoding. >> >> So, what about client_encoding he

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Itagaki Takahiro
On Tue, Nov 30, 2010 at 05:03, Dimitri Fontaine wrote: > I believe v6 fixes it all, please find it attached. > >> Design and Implementation >> * pg_execute_from_file() can execute any files even if they are not >>   in $PGDATA. OTOH, we restrict pg_read_file() to read such files. >>   Wh

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Alvaro Herrera
Excerpts from Dimitri Fontaine's message of lun nov 29 17:03:06 -0300 2010: > Itagaki Takahiro writes: > > * I hope pg_execute_from_file (and pg_read_file) had an option > > to specify an character encoding of the file. Especially, SJIS > > is still used widely, but it is not a valid server e

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Dimitri Fontaine
Itagaki Takahiro writes: > I have some comments and questions about > pg_execute_from_file.v5.patch. I believe v6 fixes it all, please find it attached. > Source code > * OID=3627 is used by another function. Don't you expect 3927? > > * There is a compiler warning: > genfile.c: In f

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 12:21 PM, Dimitri Fontaine wrote: > Robert Haas writes: >> I'd pick pg_execute_from_file() and just plain pg_execute(), myself. > > For the record there's only one name exposed at the SQL level. Or do you > want me to expand the patch to actually include a pg_execute() ver

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Dimitri Fontaine
Robert Haas writes: > I'd pick pg_execute_from_file() and just plain pg_execute(), myself. For the record there's only one name exposed at the SQL level. Or do you want me to expand the patch to actually include a pg_execute() version of the function, that would execute the query in PG_GETARG_TEX

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 11:48 AM, Tom Lane wrote: > Robert Haas writes: >> pg_execute_file() could be read to mean you are going to execute the >> file itself (i.e. it's a program). > > Well, if that's what it suggests to you, I don't see how adding "from" > disambiguates anything.  You could be

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Tom Lane
Robert Haas writes: > pg_execute_file() could be read to mean you are going to execute the > file itself (i.e. it's a program). Well, if that's what it suggests to you, I don't see how adding "from" disambiguates anything. You could be executing machine code "from" a file, too. What did you thi

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 11:12 AM, Tom Lane wrote: > Andrew Dunstan writes: >> I'm not sure why you need either "from". It just seems like a noise >> word. Maybe we could use pg_execute_query_file() and >> pg_execute_query_string(), which would be fairly clear and nicely >> symmetrical. > > +1, bu

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Tom Lane
Andrew Dunstan writes: > On 11/29/2010 11:12 AM, Tom Lane wrote: >> +1, but I think "query" is also a noise word here. >> Why not just "pg_execute_file" and "pg_execute_string"? > Well, I put that in to make it clear that the file/string is expected to > contain SQL and not, say, machine code. B

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Andrew Dunstan
On 11/29/2010 11:12 AM, Tom Lane wrote: Andrew Dunstan writes: I'm not sure why you need either "from". It just seems like a noise word. Maybe we could use pg_execute_query_file() and pg_execute_query_string(), which would be fairly clear and nicely symmetrical. +1, but I think "query" is al

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Joshua Tolley
On Mon, Nov 29, 2010 at 11:12:58AM -0500, Tom Lane wrote: > Andrew Dunstan writes: > > I'm not sure why you need either "from". It just seems like a noise > > word. Maybe we could use pg_execute_query_file() and > > pg_execute_query_string(), which would be fairly clear and nicely > > symmetric

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Andrew Dunstan
On 11/29/2010 10:51 AM, Robert Haas wrote: I'm not sure why you need either "from". It just seems like a noise word. Maybe we could use pg_execute_query_file() and pg_execute_query_string(), which would be fairly clear and nicely symmetrical. Because you execute queries, not files. Or at lea

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Tom Lane
Andrew Dunstan writes: > I'm not sure why you need either "from". It just seems like a noise > word. Maybe we could use pg_execute_query_file() and > pg_execute_query_string(), which would be fairly clear and nicely > symmetrical. +1, but I think "query" is also a noise word here. Why not just

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 10:37 AM, Andrew Dunstan wrote: > > > On 11/29/2010 10:30 AM, Robert Haas wrote: >> >> On Mon, Nov 29, 2010 at 10:27 AM, Robert Haas >>  wrote: >>> >>> On Mon, Nov 29, 2010 at 4:26 AM, Dimitri Fontaine >>>  wrote: > > * I'd like to ask native speakers whether "from

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Dimitri Fontaine
Andrew Dunstan writes: > I'm not sure why you need either "from". It just seems like a noise > word. Maybe we could use pg_execute_query_file() and > pg_execute_query_string(), which would be fairly clear and nicely > symmetrical. I'd go with that but need to tell: only pg_execute_query_file() is

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Andrew Dunstan
On 11/29/2010 10:30 AM, Robert Haas wrote: On Mon, Nov 29, 2010 at 10:27 AM, Robert Haas wrote: On Mon, Nov 29, 2010 at 4:26 AM, Dimitri Fontaine wrote: * I'd like to ask native speakers whether "from" is needed in names of "pg_execute_from_file" and "pg_execute_from_query_string". Fai

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 10:27 AM, Robert Haas wrote: > On Mon, Nov 29, 2010 at 4:26 AM, Dimitri Fontaine > wrote: >>> * I'd like to ask native speakers whether "from" is needed in names >>>   of "pg_execute_from_file" and "pg_execute_from_query_string". >> >> Fair enough, will wait for some comme

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Robert Haas
On Mon, Nov 29, 2010 at 4:26 AM, Dimitri Fontaine wrote: >> * I'd like to ask native speakers whether "from" is needed in names >>   of "pg_execute_from_file" and "pg_execute_from_query_string". > > Fair enough, will wait for some comments before producing a v6. Yes, you need the from there. --

Re: [HACKERS] pg_execute_from_file review

2010-11-29 Thread Dimitri Fontaine
Itagaki Takahiro writes: > I have some comments and questions about pg_execute_from_file.v5.patch. Thanks for reviewing it! > Source code > * OID=3627 is used by another function. Don't you expect 3927? Yes indeed. It took me some time to understand what's happening here, and it seems

Re: [HACKERS] pg_execute_from_file review

2010-11-28 Thread Itagaki Takahiro
On Fri, Nov 26, 2010 at 06:24, Dimitri Fontaine wrote: > Thanks for your review. Please find attached a revised patch where I've > changed the internals of the function so that it's split in two and that > the opr_sanity check passes, per comments from David Wheeler and Tom Lane. I have some comm

Re: [HACKERS] pg_execute_from_file review

2010-11-26 Thread Dimitri Fontaine
Joshua Tolley writes: >> > * I'd like to see the docs slightly expanded, specifically to describe >> > parameter replacement. I wondered for a while if I needed to set of >> > parameters in any specific way, before reading the code and realizing >> > they >> > can be whatever I want. >> >>

Re: [HACKERS] pg_execute_from_file review

2010-11-25 Thread Joshua Tolley
On Thu, Nov 25, 2010 at 10:24:51PM +0100, Dimitri Fontaine wrote: > Joshua Tolley writes: > > I've just looked at pg_execute_from_file[1]. The idea here is to execute all > > the SQL commands in a given file. My comments: > > Thanks for your review. Please find attached a revised patch where I've

Re: [HACKERS] pg_execute_from_file review

2010-11-25 Thread Dimitri Fontaine
Joshua Tolley writes: > I've just looked at pg_execute_from_file[1]. The idea here is to execute all > the SQL commands in a given file. My comments: Thanks for your review. Please find attached a revised patch where I've changed the internals of the function so that it's split in two and that th

[HACKERS] pg_execute_from_file review

2010-11-24 Thread Joshua Tolley
I've just looked at pg_execute_from_file[1]. The idea here is to execute all the SQL commands in a given file. My comments: * It applies well enough, and builds fine * It seems to work, and I've not come up with a way to make it break * It seems useful, and to follow the limited design discussion