Tom Lane t...@sss.pgh.pa.us writes:
Attached is an updated patch that incorporates all of the review I've
And that looks great, thanks. I've only had time to read the patch,
will play with it later on today, hopefully.
I've spotted a comment that I think you missed updating. The schema
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
I've spotted a comment that I think you missed updating. The schema
given in the control file is now created in all cases rather than only
when the extension is not relocatable, right?
Hm, no, that logic is the same as before no?
I also note
Tom Lane t...@sss.pgh.pa.us writes:
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
I've spotted a comment that I think you missed updating. The schema
given in the control file is now created in all cases rather than only
when the extension is not relocatable, right?
Hm, no, that logic is
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us writes:
Hm, no, that logic is the same as before no?
Well I had
if (!control-relocatable control-schema != NULL)
And you have
+ else if (control-schema != NULL)
Yeah, I deleted that
Tom Lane t...@sss.pgh.pa.us writes:
Yeah, I deleted that relocatable test because it's redundant:
control-schema cannot be set for a relocatable extension,
cf the test in read_extension_control_file.
I missed that you kept this test in your version of the patch. Sorry
for noise.
Regardsm
--
I have gone ahead and committed the core and documentation parts of this
patch, but not as yet any of the contrib changes; that is, all the
contrib modules are still building old-style. I had intended to do it
in two steps all along, so as to get some buildfarm proof for the thesis
that we
Tom Lane t...@sss.pgh.pa.us writes:
I have gone ahead and committed the core and documentation parts of this
Thank you!
And I'd like to take the time to thank all of you who helped me reach
this goal, but that ranges to about everyone here who I talked to at any
conference those last two or
Attached is an updated patch that incorporates all of the review I've
done so far on the core code. This omits the contrib changes, which
I've not looked at in any detail, and the docs changes since I've not
yet updated the docs to match today's code changes. User-visible
changes are that WITH
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us writes:
Actually, I was about to pick up and start working on the whole
extensions patch series, but now I'm confused as to what and where is
the latest version.
Ok, here's what I have, attached, as patch v30. What
Tom Lane t...@sss.pgh.pa.us writes:
What are the guc.c changes in this patch? They appear completely
unrelated to the topic.
Right. Didn't spot them. Sorry for the noise in the patch, it must be
a merge problem in my repository. Do you want me to clean that up or is
it already to late?
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us writes:
What are the guc.c changes in this patch? They appear completely
unrelated to the topic.
Right. Didn't spot them. Sorry for the noise in the patch, it must be
a merge problem in my repository. Do you want
While I'm looking at this ... what is the rationale for treating rewrite
rules as members of extensions, ie, why does the patch touch
rewriteDefine.c? ISTM a rule is a property of a table and could not
sensibly be an independent member of an extension. If there is a use
for that, why are table
Tom Lane t...@sss.pgh.pa.us writes:
While I'm looking at this ... what is the rationale for treating rewrite
rules as members of extensions, ie, why does the patch touch
rewriteDefine.c? ISTM a rule is a property of a table and could not
sensibly be an independent member of an extension. If
Itagaki Takahiro itagaki.takah...@gmail.com writes:
Hi, the attached is a further cleanup of the latest commit
(1db20cdd36cb1c2cc5ef2210a23b3c09f5058690).
Thanks! Given that the patch contains some merging from master's
branch, I'm not sure if I should apply it to my repository then handle
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Itagaki Takahiro itagaki.takah...@gmail.com writes:
Hi, the attached is a further cleanup of the latest commit
(1db20cdd36cb1c2cc5ef2210a23b3c09f5058690).
Thanks! Given that the patch contains some merging from master's
branch, I'm not sure if
Itagaki Takahiro itagaki.takah...@gmail.com writes:
* relocatable and schema seems to be duplicated options.
They are not, really. If you have a relocatable extension, then there's
no schema option in the control file (setting it is an ERROR). If you
have a non-relocatable extension, then you
On Fri, Jan 28, 2011 at 18:03, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
After review, I included all your proposed changes, thanks a lot!
Please find attached a new version of the patch, v29,
Additional questions and discussions:
* relocatable and schema seems to be duplicated options.
On Thu, Jan 27, 2011 at 20:01, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
Ok, done. Of course, it solves the whole problem Itagaki had with
adminpack because we stop relying on dependencies to get it right now.
I found pg_restore with -c option fails when an extension is created
in
Itagaki Takahiro itagaki.takah...@gmail.com writes:
I found pg_restore with -c option fails when an extension is created
in pg_catalog. Since pg_catalog is an implicit search target, so we
might need the catalog to search_path explicitly.
Note that I can restore adminpack with not errors
On Thu, Jan 27, 2011 at 22:48, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
Itagaki Takahiro itagaki.takah...@gmail.com writes:
I found pg_restore with -c option fails when an extension is created
in pg_catalog.
Nice catch, thank you very much (again) for finding those :)
Seems good.
Itagaki Takahiro itagaki.takah...@gmail.com writes:
Since pg_dump won't dump user objects in pg_catalog, adminpack can
avoid the above errors by installing functions in pg_catalog.
CREATE EXTENSION might have the same issue -- Can EXTENSION work
without errors when we install extensions in
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
The missing entry in pg_depend is the reason why the extension is not
part of the dump. We could fix that using a LEFT JOIN here and COALESCE
to force the namespace as pg_catalog. Is that not a kludge?
Yes, it is. Why is the pg_depend entry
Tom Lane t...@sss.pgh.pa.us writes:
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
The missing entry in pg_depend is the reason why the extension is not
part of the dump. We could fix that using a LEFT JOIN here and COALESCE
to force the namespace as pg_catalog. Is that not a kludge?
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us writes:
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
The missing entry in pg_depend is the reason why the extension is not
part of the dump. We could fix that using a LEFT JOIN here and COALESCE
to force the
Tom Lane t...@sss.pgh.pa.us writes:
OK, so I guess I'm missing why the extension code is looking for stuff
dependent on the pg_catalog schema. That schema certainly doesn't
belong to any extension.
Exactly. We're on the same page here, full agreement. So the extension
patch is not
Dimitri Fontaine dimi...@2ndquadrant.fr 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
Dimitri Fontaine dimi...@2ndquadrant.fr 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
Tom Lane t...@sss.pgh.pa.us 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
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us 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
Tom Lane t...@sss.pgh.pa.us 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
On Mon, Jan 24, 2011 at 18:22, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
Following recent commit of the hstore Makefile cleaning, that I included
into my extension patch too, I have a nice occasion to push version 27
of the patch. Please find it enclosed.
Hi, I read the patch and test it
Hi,
Itagaki Takahiro itagaki.takah...@gmail.com writes:
Hi, I read the patch and test it in some degree. It works as expected
generally, but still needs a bit more development in edge case.
Thanks for the review!
* commands/extension.h needs cleanup.
- Missing extern declarations for
Itagaki Takahiro itagaki.takah...@gmail.com writes:
* Extensions installed in pg_catalog:
If we install an extension into pg_catalog,
CREATE EXTENSION xxx WITH SCHEMA pg_catalog
pg_dump dumps nothing about the extension. We might need special care
for modules that install functions only in
On Tue, Jan 25, 2011 at 04:23:41PM +0100, Dimitri Fontaine wrote:
Itagaki Takahiro itagaki.takah...@gmail.com writes:
* Extensions installed in pg_catalog:
If we install an extension into pg_catalog,
CREATE EXTENSION xxx WITH SCHEMA pg_catalog
pg_dump dumps nothing about the extension.
On Jan 25, 2011, at 7:27 AM, David Fetter wrote:
Other than adminpack, I think it makes sense to say that extensions
are not going into pg_catalog…
Given this, we should maybe see about either making adminpack part of
PostgreSQL's core distribution (probably a good idea) or moving it out
On Wed, Jan 26, 2011 at 02:57, David E. Wheeler da...@kineticode.com wrote:
Other than adminpack, I think it makes sense to say that extensions
are not going into pg_catalog…
Given this, we should maybe see about either making adminpack part of
PostgreSQL's core distribution (probably a good
36 matches
Mail list logo
