Обсуждение: Another swing at JSON
Attached is a patch that adds a 'json' contrib module. Although we may want a built-in JSON data type in the near future, making it a module (for the time being) has a couple advantages: * Installable on current and previous versions of PostgreSQL. The json module supports PostgreSQL 8.4.0 and up. * Easier to maintain. json.sql.in is easy to work on, src/include/catalog/pg_proc.h is not. Currently, there are no functions for converting to/from PostgreSQL values or getting/setting sub-values (e.g. JSONPath). However, I did adapt the json_stringify function written by Itagaki Takahiro in his patch ( http://archives.postgresql.org/pgsql-hackers/2010-09/msg01200.php ). JSON datums are stored internally as condensed JSON text. By "condensed", I mean that whitespace is removed, and escapes are converted to characters when it's possible and efficient to do so. Although a binary internal format would be more size-efficient in theory, condensed JSON text is pretty efficient, too. Internally, the implementation does not construct any parse trees, which should provide a major performance advantage. Almost all of the code has been rewritten since my original patch ( http://archives.postgresql.org/pgsql-hackers/2010-07/msg01215.php ). Some will be happy to know that the new code doesn't have any gotos :-) Joey Adams
Вложения
On Mon, Mar 28, 2011 at 1:21 PM, Joseph Adams <joeyadams3.14159@gmail.com> wrote: > Attached is a patch that adds a 'json' contrib module. Although we > may want a built-in JSON data type in the near future, making it a > module (for the time being) has a couple advantages: Is this something you'd hope to get committed at some point, or do you plan to maintain it as an independent project? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Mar 28, 2011 at 1:48 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Mon, Mar 28, 2011 at 1:21 PM, Joseph Adams > <joeyadams3.14159@gmail.com> wrote: >> Attached is a patch that adds a 'json' contrib module. Although we >> may want a built-in JSON data type in the near future, making it a >> module (for the time being) has a couple advantages: > > Is this something you'd hope to get committed at some point, or do you > plan to maintain it as an independent project? I'm hoping to get it committed at some point, perhaps as a module soon, and a built-in later. Plenty of people are still waiting for JSON data type support, for example: http://stackoverflow.com/questions/4995945/optimize-escape-json-in-postgresql-9-0
On Mon, Mar 28, 2011 at 2:03 PM, Joseph Adams <joeyadams3.14159@gmail.com> wrote: > On Mon, Mar 28, 2011 at 1:48 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Mon, Mar 28, 2011 at 1:21 PM, Joseph Adams >> <joeyadams3.14159@gmail.com> wrote: >>> Attached is a patch that adds a 'json' contrib module. Although we >>> may want a built-in JSON data type in the near future, making it a >>> module (for the time being) has a couple advantages: >> >> Is this something you'd hope to get committed at some point, or do you >> plan to maintain it as an independent project? > > I'm hoping to get it committed at some point, perhaps as a module > soon, and a built-in later. Plenty of people are still waiting for > JSON data type support, for example: > > http://stackoverflow.com/questions/4995945/optimize-escape-json-in-postgresql-9-0 Well, one thing you'll need to do is rework it for the new 9.1 extensions interface. Once you're reasonably happy with it, I think it'd be good to add this to the next CommitFest: https://commitfest.postgresql.org/action/commitfest_view/open I'd like to review it more, but it's more than I can tackle at the moment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 3/28/11 10:21 AM, Joseph Adams wrote: > Currently, there are no functions for converting to/from PostgreSQL > values or getting/setting sub-values (e.g. JSONPath). However, I did > adapt the json_stringify function written by Itagaki Takahiro in his > patch ( http://archives.postgresql.org/pgsql-hackers/2010-09/msg01200.php > ). Would it be possible for you to add a TODO list for JSON support to the wiki? We have some potential GSOC students who are interested in working on JSON support. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Mon, Mar 28, 2011 at 2:17 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Well, one thing you'll need to do is rework it for the new 9.1 > extensions interface. Done. The new extension interface isn't exactly compatible with the old, so I dropped support for PostgreSQL 8.4 from the module. I suppose I could maintain a back-ported json module separately. On Mon, Mar 28, 2011 at 7:44 PM, Josh Berkus <josh@agliodbs.com> wrote: > On 3/28/11 10:21 AM, Joseph Adams wrote: >> Currently, there are no functions for converting to/from PostgreSQL >> values or getting/setting sub-values (e.g. JSONPath). However, I did >> adapt the json_stringify function written by Itagaki Takahiro in his >> patch ( http://archives.postgresql.org/pgsql-hackers/2010-09/msg01200.php >> ). > > Would it be possible for you to add a TODO list for JSON support to the > wiki? We have some potential GSOC students who are interested in > working on JSON support. What exactly should go on the TODO list? Adding more features to this JSON data type implementation (and eventually merging some/all of it into core)? Or doing a comparative analysis (benchmarking, etc.) of the ~seven JSON data type implementations floating around? Since I'm not sure what was decided regarding an efficient binary internal representation, I don't know what we should write on the TODO list. In my humble (and biased) opinion, we should review and commit my JSON data type code as a starting point. Then, a GSoC student could work on features (e.g. value conversion, JSONPath), integration (e.g. moving to core, EXPLAIN FORMAT JSON, PL/Js, etc.), and improvements (e.g. a binary internal representation). Thoughts? Joey Adams
Вложения
Joseph Adams <joeyadams3.14159@gmail.com> writes: > Done. The new extension interface isn't exactly compatible with the > old, so I dropped support for PostgreSQL 8.4 from the module. I > suppose I could maintain a back-ported json module separately. In fact it is, but there's some history hiding the fact. I'm overdue to another doc patch on the matter, but it's quite simple. You don't need to use MODULE_PATHNAME in recent enough versions of PostgreSQL, meaning any version that's not currently EOL'ed. Just use $libdir and the backend code will be happy with it. That means you don't need the .sql.in file either. You don't need to use the control file property module_pathname either in most cases, that's only useful if you are building more than one extension from the same Makefile. So just use $libdir/json in your json.sql file and be done with it. Your extension is now compatible with both pre-9.1 and 9.1. I'm not sure how to spell that in the docs though, any help here would be welcome. Also, do we want to adapt contrib to be better examples, or do we want contrib to remain full of its history? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Mar 29, 2011 at 10:26 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Joseph Adams <joeyadams3.14159@gmail.com> writes: >> Done. The new extension interface isn't exactly compatible with the >> old, so I dropped support for PostgreSQL 8.4 from the module. I >> suppose I could maintain a back-ported json module separately. > > In fact it is, but there's some history hiding the fact. I'm overdue to > another doc patch on the matter, but it's quite simple. Cool, thanks! Attached is an updated patch for the module. Backward compatibility has been brought back, and the module has been tested on PostgreSQL versions 8.4.0 and 9.1devel. However, there are a couple minor caveats: * The last test, relocatable, fails (and should fail) on pre-9.1 because it's a test related to the new extension interface. * init.sql is rather hacky in how it caters to both the old extension system and the new one: \set ECHO none SET client_min_messages = fatal; CREATE EXTENSION json; \i json--0.1.sql SET client_min_messages = warning; ... RESET client_min_messages; \set ECHO all It would be nice if I could make a Makefile conditional that skips the relocatable test and loads init-pre9.1.sql if the new extension interface isn't available. Is there a Makefile variable or something I can use to do this? Also, should uninstall_json.sql be named something else, like json--uninstall--0.1.sql ? Thanks, Joey Adams
Вложения
Joseph Adams <joeyadams3.14159@gmail.com> writes: > It would be nice if I could make a Makefile conditional that skips the > relocatable test and loads init-pre9.1.sql if the new extension > interface isn't available. Is there a Makefile variable or something > I can use to do this? You can use VERSION and MAJORVERSION variables, those are defined in Makefile.global and available as soon as you did include the PGXS Makefile. > Also, should uninstall_json.sql be named something else, like > json--uninstall--0.1.sql ? You don't need no uninstall script no more, try DROP EXTENSION json; and DROP EXTENSION json CASCADE; Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Mar 29, 2011 at 2:42 PM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >> Also, should uninstall_json.sql be named something else, like >> json--uninstall--0.1.sql ? > > You don't need no uninstall script no more, try DROP EXTENSION json; and > DROP EXTENSION json CASCADE; It's there for pre-9.1, where DROP EXTENSION is not available.
On Tue, Mar 29, 2011 at 2:42 PM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Joseph Adams <joeyadams3.14159@gmail.com> writes: >> It would be nice if I could make a Makefile conditional that skips the >> relocatable test and loads init-pre9.1.sql if the new extension >> interface isn't available. Is there a Makefile variable or something >> I can use to do this? > > You can use VERSION and MAJORVERSION variables, those are defined in > Makefile.global and available as soon as you did include the PGXS > Makefile. The problem is, I'd have to include the PGXS Makefile before defining a parameter used by the PGXS Makefile. I could include the PGXS Makefile twice, once at the top, and again at the bottom, but that produces a bunch of ugly warnings like: ../../src/Makefile.global:418: warning: overriding commands for target `submake-libpq' Not only that, but MAJORVERSION is formatted as a decimal (like 9.1 or 8.4). Although I could use some hacky string manipulation, or compare MAJORVERSION against all prior supported versions, either approach would be needlessly error-prone. I'm thinking the pg_config utility should either make PG_VERSION_NUM (e.g. 90100) available, or it should define something indicating the presence of the new extension system. Joey Adams
Joseph Adams <joeyadams3.14159@gmail.com> writes: > would be needlessly error-prone. I'm thinking the pg_config utility > should either make PG_VERSION_NUM (e.g. 90100) available, or it should > define something indicating the presence of the new extension system. Here's the ugly trick from ip4r, that's used by more extension: PREFIX_PGVER = $(shell echo $(VERSION) | awk -F. '{ print $$1*100+$$2 }') Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Mar 29, 2011 at 02:56:52PM -0400, Joseph Adams wrote: > On Tue, Mar 29, 2011 at 2:42 PM, Dimitri Fontaine > <dimitri@2ndquadrant.fr> wrote: > >> Also, should uninstall_json.sql be named something else, like > >> json--uninstall--0.1.sql ? > > > > You don't need no uninstall script no more, try DROP EXTENSION json; and > > DROP EXTENSION json CASCADE; > > It's there for pre-9.1, where DROP EXTENSION is not available. Anything going into the PostgreSQL code base will be for 9.2, so anything else would be a separate (if somewhat related) project. I suspect the code will be a good deal cleaner if you do just the 9.2+ version and see who wants it back-patched, if anyone does :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@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
On Tue, Mar 29, 2011 at 4:02 PM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Here's the ugly trick from ip4r, that's used by more extension: > > PREFIX_PGVER = $(shell echo $(VERSION) | awk -F. '{ print $$1*100+$$2 }') Thanks. I applied a minor variation of this trick to the JSON module, so now it builds/installs/tests cleanly on both REL8_4_0 and HEAD (though it won't work if you copy contrib/json into a pre-9.1 PostgreSQL source directory and type `make` without USE_PGXS=1). I also went ahead and renamed uninstall_json.sql to json--uninstall--0.1.sql (again, it's for pre-9.1 users) and removed unnecessary trailing spaces. > Anything going into the PostgreSQL code base will be for 9.2, so > anything else would be a separate (if somewhat related) project. I > suspect the code will be a good deal cleaner if you do just the 9.2+ > version and see who wants it back-patched, if anyone does :) It's a trivial matter to remove backward compatibility from contrib/json, if anybody wants me to do it. I can just remove compat.[ch], */init-pre9.1.* , remove the PREFIX_PGVER trick from the Makefile, remove a few lines in the source code, and maintain the backported json module elsewhere. It's just a matter of whether or not explicit backward-compatibility is desirable in modules shipped with releases. Joey Adams
Вложения
Excerpts from Joseph Adams's message of mar mar 29 22:15:11 -0300 2011: > On Tue, Mar 29, 2011 at 4:02 PM, Dimitri Fontaine > <dimitri@2ndquadrant.fr> wrote: > > Here's the ugly trick from ip4r, that's used by more extension: > > > > PREFIX_PGVER = $(shell echo $(VERSION) | awk -F. '{ print $$1*100+$$2 }') > > Thanks. I applied a minor variation of this trick to the JSON module, > so now it builds/installs/tests cleanly on both REL8_4_0 and HEAD > (though it won't work if you copy contrib/json into a pre-9.1 > PostgreSQL source directory and type `make` without USE_PGXS=1). Why are you worrying with the non-PGXS build chain anyway? Just assume that the module is going to be built with PGXS and things should just work. We've gone over this a dozen times in the past. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Why are you worrying with the non-PGXS build chain anyway? Just assume > that the module is going to be built with PGXS and things should just > work. > > We've gone over this a dozen times in the past. +1 I'm not sure why we still support the pre-PGXS build recipe in the contrib Makefiles, and didn't want to change that as part as the extension patch series, but I think we should reconsider. This and removing module_pathname in the control files to just use $libdir/contrib in the .sql files. That would set a better example to people who want to make their own extensions, as the general case is that those will not get into contrib. I think we should lower the differences between contrib and external extensions, so that contrib is only about who maintains the code and distribute the extension. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Why are you worrying with the non-PGXS build chain anyway? Just assume > > that the module is going to be built with PGXS and things should just > > work. > > > > We've gone over this a dozen times in the past. > > +1 > > I'm not sure why we still support the pre-PGXS build recipe in the > contrib Makefiles, and didn't want to change that as part as the > extension patch series, but I think we should reconsider. This is in the archives somewhere. I think it's something to do with the fact that "make check" doesn't work under PGXS; you have to use "make installcheck". Or maybe it was something else. I don't really remember the details. I also pushed for this, a long time ago. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Mar 30, 2011 at 10:27:07AM +0200, Dimitri Fontaine wrote: > I think we should lower the differences between contrib and external > extensions, so that contrib is only about who maintains the code and > distribute the extension. +10 :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@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
Excerpts from Alvaro Herrera's message of mié mar 30 10:27:39 -0300 2011: > Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011: > > I'm not sure why we still support the pre-PGXS build recipe in the > > contrib Makefiles, and didn't want to change that as part as the > > extension patch series, but I think we should reconsider. > > This is in the archives somewhere. I think it's something to do with > the fact that "make check" doesn't work under PGXS; you have to use > "make installcheck". Or maybe it was something else. I don't really > remember the details. I also pushed for this, a long time ago. In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php Tom writes: > The main reason contrib still has the alternate method is that PGXS > doesn't really work until after you've installed the core build. Maybe we could have a look and try to fix that problem. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Mar 30, 2011 at 10:32:55AM -0300, Alvaro Herrera wrote: > Excerpts from Alvaro Herrera's message of mié mar 30 10:27:39 -0300 2011: > > Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011: > > > > I'm not sure why we still support the pre-PGXS build recipe in the > > > contrib Makefiles, and didn't want to change that as part as the > > > extension patch series, but I think we should reconsider. > > > > This is in the archives somewhere. I think it's something to do with > > the fact that "make check" doesn't work under PGXS; you have to use > > "make installcheck". Or maybe it was something else. I don't really > > remember the details. I also pushed for this, a long time ago. > > In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php > Tom writes: > > > The main reason contrib still has the alternate method is that PGXS > > doesn't really work until after you've installed the core build. > > Maybe we could have a look and try to fix that problem. +1 :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@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
On 03/30/2011 09:42 AM, David Fetter wrote: >> >> In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php >> Tom writes: >> >>> The main reason contrib still has the alternate method is that PGXS >>> doesn't really work until after you've installed the core build. >> Maybe we could have a look and try to fix that problem. > +1 :) > > Maybe we could teach pg_config to report appropriate settings for uninstalled source via an environment setting. Without changing pg_config it looks like we have a chicken and egg problem. (If my suggestion is right, this would probably be a good beginner TODO.) cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > Maybe we could teach pg_config to report appropriate settings for > uninstalled source via an environment setting. Without changing pg_config it > looks like we have a chicken and egg problem. > > (If my suggestion is right, this would probably be a good beginner TODO.) Is it still 9.1 material? I think it should be considered UI improvements on what we already have (contribs and extensions), and allowed to be fixed while in beta. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 03/30/2011 11:37 AM, Dimitri Fontaine wrote: > Andrew Dunstan<andrew@dunslane.net> writes: >> Maybe we could teach pg_config to report appropriate settings for >> uninstalled source via an environment setting. Without changing pg_config it >> looks like we have a chicken and egg problem. >> >> (If my suggestion is right, this would probably be a good beginner TODO.) > Is it still 9.1 material? I think it should be considered UI > improvements on what we already have (contribs and extensions), and > allowed to be fixed while in beta. I think we're pretty much down to only fixing bugs now, for 9.1, and this isn't a bug, however inconvenient it might be. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > I think we're pretty much down to only fixing bugs now, for 9.1, and this > isn't a bug, however inconvenient it might be. It's not just inconvenient, it's setting a bad example for people to work on their own extensions. It's more than unfortunate. I will prepare a doc patch, but copying from contrib is the usual way to go creating your own extension, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 03/30/2011 12:29 PM, Dimitri Fontaine wrote: > Andrew Dunstan<andrew@dunslane.net> writes: >> I think we're pretty much down to only fixing bugs now, for 9.1, and this >> isn't a bug, however inconvenient it might be. > It's not just inconvenient, it's setting a bad example for people to > work on their own extensions. It's more than unfortunate. I will > prepare a doc patch, but copying from contrib is the usual way to go > creating your own extension, right? > > None of that makes it a bug. I don't have any objection to putting some comments in the contrib Makefiles telling people to use PGXS, but I don't think that at this stage of the cycle we can start work on something that so far is just an idea from the top of my head. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > I don't have any objection to putting some comments in the contrib Makefiles > telling people to use PGXS, but I don't think that at this stage of the > cycle we can start work on something that so far is just an idea from the > top of my head. I might be mistaken on how much work is hidden there, ok, you have the point. I will try to clarify this in the doc patch I intend to submit soon'ish: we still accept those while getting to beta, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Excerpts from Dimitri Fontaine's message of mié mar 30 15:05:36 -0300 2011: > Andrew Dunstan <andrew@dunslane.net> writes: > > I don't have any objection to putting some comments in the contrib Makefiles > > telling people to use PGXS, but I don't think that at this stage of the > > cycle we can start work on something that so far is just an idea from the > > top of my head. > > I might be mistaken on how much work is hidden there, ok, you have the > point. I will try to clarify this in the doc patch I intend to submit > soon'ish: we still accept those while getting to beta, right? Sure. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Andrew Dunstan <andrew@dunslane.net> writes: > On 03/30/2011 12:29 PM, Dimitri Fontaine wrote: >> Andrew Dunstan<andrew@dunslane.net> writes: >>> I think we're pretty much down to only fixing bugs now, for 9.1, and this >>> isn't a bug, however inconvenient it might be. >> It's not just inconvenient, it's setting a bad example for people to >> work on their own extensions. It's more than unfortunate. I will >> prepare a doc patch, but copying from contrib is the usual way to go >> creating your own extension, right? > None of that makes it a bug. Possibly more to the point, we don't even have a design sketch for a better solution; and we are *certainly* past the point where blue-sky stuff ought to be going into 9.1. The reason it seems (to me) nontrivial to change is this: the PGXS build method assumes that the correct pg_config can be found in your PATH. That is pretty much guaranteed to not be the case during a in-tree build. Even if we modified the PATH to include wherever pg_config is hiding, the information it puts out about where to look for include and library files would be wrong. Another small problem with depending on pg_config during an in-tree build is that it would completely break cross-compile builds. (Maybe those are in bad shape already, I'm not sure. But configure for example is still going out of its way to support them.) So, I'm interested in trying to improve this, but it looks like a research project from here. regards, tom lane
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > This and removing module_pathname in the control files to just use > $libdir/contrib in the .sql files. That would set a better example to > people who want to make their own extensions, as the general case is > that those will not get into contrib. I'm not sure it's a better example. We considered doing that before, and decided not to on the grounds that using MODULE_PATHNAME avoids hard-wiring the name of the shared library into the SQL scripts. Also, IIRC, there are a couple of contrib modules where there's an actual problem in doing it like that; though I'm unable to recall details as I'm fighting a head cold at the moment. My original intention when I started working with the extensions patch had in fact been to do what you suggest above, but I was convinced not to. I don't think we should reverse that decision at the last minute. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: >> This and removing module_pathname in the control files to just use >> $libdir/contrib in the .sql files. That would set a better example to >> people who want to make their own extensions, as the general case is >> that those will not get into contrib. > > My original intention when I started working with the extensions patch > had in fact been to do what you suggest above, but I was convinced not > to. I don't think we should reverse that decision at the last minute. Ok, I'll trust you on that. So the other way to ease compatibility would be to keep the .sql.in to .sql Makefile rule in pgxs.mk but make it just to a copy in 9.1. Then the same old script will just continue working as soon as you edit the module_pathname in the control file. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Tom Lane <tgl@sss.pgh.pa.us> writes: > So, I'm interested in trying to improve this, but it looks like a > research project from here. True: I don't have a baked solution that we would just need to apply. The simplest idea I can think of is forcing make install before to build contribs so that PGXS works “normally”. We would have to clarify the point somewhere visible in the docs though, so that we can say to people building extensions to mimic contrib/ except for the in-tree old-style Makefile support. In fact, adding some comments about that in all the contrib extension Makefiles might be our best shot at it, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--On 29. März 2011 21:15:11 -0400 Joseph Adams <joeyadams3.14159@gmail.com> wrote: > Thanks. I applied a minor variation of this trick to the JSON module, > so now it builds/installs/tests cleanly on both REL8_4_0 and HEAD > (though it won't work if you copy contrib/json into a pre-9.1 > PostgreSQL source directory and type `make` without USE_PGXS=1). > > I also went ahead and renamed uninstall_json.sql to > json--uninstall--0.1.sql (again, it's for pre-9.1 users) and removed > unnecessary trailing spaces. > >> Anything going into the PostgreSQL code base will be for 9.2, so >> anything else would be a separate (if somewhat related) project. I >> suspect the code will be a good deal cleaner if you do just the 9.2+ >> version and see who wants it back-patched, if anyone does :) > > It's a trivial matter to remove backward compatibility from > contrib/json, if anybody wants me to do it. I can just remove > compat.[ch], */init-pre9.1.* , remove the PREFIX_PGVER trick from the > Makefile, remove a few lines in the source code, and maintain the > backported json module elsewhere. It's just a matter of whether or > not explicit backward-compatibility is desirable in modules shipped > with releases. I started looking into this. A very minor adjusted patch to filelist.sgml was required to apply the patch cleanly to current -HEAD (attached). After reading Joseph's comment upthread, I don't see any consensus wether the existing pre-9.1 support is required or even desired. Maybe i missed it, but do we really expect an extension (or contrib module) to be backwards compatible to earlier major releases, when shipped in contrib/ ? -- Thanks Bernd
Вложения
Bernd Helmle <mailings@oopsware.de> writes: > After reading Joseph's comment upthread, I don't see any consensus > wether the existing pre-9.1 support is required or even desired. Maybe > i missed it, but do we really expect an extension (or contrib module) > to be backwards compatible to earlier major releases, when shipped in > contrib/ ? No, we don't. You won't find any attempt in any contrib module to build against prior releases. There's not much point, since they're shipped with a specific release of the core. regards, tom lane
--On 16. Juni 2011 17:38:07 -0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: >> After reading Joseph's comment upthread, I don't see any consensus >> wether the existing pre-9.1 support is required or even desired. Maybe >> i missed it, but do we really expect an extension (or contrib module) >> to be backwards compatible to earlier major releases, when shipped in >> contrib/ ? > > No, we don't. You won't find any attempt in any contrib module to build > against prior releases. There's not much point, since they're shipped > with a specific release of the core. Okay, then we should remove this code. It doesn't do any complicated, but it seems a waste of code in this case (and from a maintenance point of view). Joseph, are you able to remove the compatibility code for this CF? -- Thanks Bernd
On Fri, Jun 17, 2011 at 2:29 AM, Bernd Helmle <mailings@oopsware.de> wrote: > Joseph, are you able to remove the compatibility code for this CF? Done. Note that this module builds, tests, and installs successfully with USE_PGXS=1. However, building without USE_PGXS=1 produces the following: CREATE EXTENSION json; ERROR: incompatible library "/usr/lib/postgresql/json.so": version mismatch DETAIL: Server is version 9.1, library is version 9.2. Similar problems occur with a couple other modules I tried (hstore, intarray). Joey
Вложения
--On 17. Juni 2011 18:06:58 -0400 Joseph Adams <joeyadams3.14159@gmail.com> wrote: > Done. Note that this module builds, tests, and installs successfully > with USE_PGXS=1. However, building without USE_PGXS=1 produces the > following: > > CREATE EXTENSION json; > ERROR: incompatible library "/usr/lib/postgresql/json.so": version > mismatch DETAIL: Server is version 9.1, library is version 9.2. > > Similar problems occur with a couple other modules I tried (hstore, intarray). Hmm, works for me. Seems you have messed up your installation in some way (build against current -HEAD but running against a 9.1?). I'm going to review in the next couple of days again. -- Thanks Bernd