Tom Lane 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 three years (I still
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 haven't
Tom Lane 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
--
Dimitri Fontai
Dimitri Fontaine writes:
> Tom Lane 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 relocatable test because it's redundan
Tom Lane writes:
> Dimitri Fontaine 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?
Well I
Dimitri Fontaine 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 that the attached ve
Tom Lane 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
given in the control f
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 NO
Tom Lane 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 there is a use
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 co
Dimitri Fontaine writes:
> Tom Lane 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 al
Tom Lane 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?
Regards,
--
Dimitri
Dimitri Fontaine writes:
> Tom Lane 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 Itagaki just sent
> applies on to
Dimitri Fontaine writes:
> Itagaki Takahiro 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 h
Itagaki Takahiro 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
conflicts, or let you manage
Itagaki Takahiro 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 can either setup the s
On Fri, Jan 28, 2011 at 18:03, Dimitri Fontaine 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.
We could treat an
On Thu, Jan 27, 2011 at 22:48, Dimitri Fontaine wrote:
> Itagaki Takahiro 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.
>> extern bool extension_relocatable_p(Oid ext
Itagaki Takahiro 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 because it has
> explicit "p
On Thu, Jan 27, 2011 at 20:01, Dimitri Fontaine 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 pg_catalog. Since pg_catalog
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
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, extensio
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
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 a
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
>
Tom Lane 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 considering pg_catalog in
Dimitri Fontaine writes:
> Tom Lane writes:
>> Dimitri Fontaine 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,
Tom Lane writes:
> Dimitri Fontaine 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 e
Dimitri Fontaine 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 missing?
Itagaki Takahiro 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 template databases?
Well
On Wed, Jan 26, 2011 at 02:57, David E. Wheeler 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
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
On Tue, Jan 25, 2011 at 04:23:41PM +0100, Dimitri Fontaine wrote:
> Itagaki Takahiro 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 spec
Itagaki Takahiro 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 pg_catalog.
In src/ba
Hi,
Itagaki Takahiro 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 create_extension and
> cre
On Mon, Jan 24, 2011 at 18:22, Dimitri Fontaine 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 in some degree. It
36 matches
Mail list logo
