Re: Extensions, this time with a patch

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, David Fetter <david(at)fetter(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions, this time with a patch
Date: 2010-10-16 08:58:58
Message-ID: m2zkue1pkd.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Hmm. To be honest I don't like the direction that pg_execute_from_file
> has taken. (Now that I look, it's been like this since inception). I
> have two problems with it: one is that it is #including half the world
> into genfile.c. This already smells trouble in itself. I got worried
> when I saw the CommandDest declaration. Really, I think having the guts
> of postgres.c into that file is not a good idea from a modularisation
> point of view.

Understood. The thinking back when I worked on that part was to minimize
the diff and remain localized, which is known to be a bad idea... I'll
rework that part as soon as we agree on the other one:

> The other problem is that it's slurping the whole file and executing it
> as a single query. This is really two problems: one is that you
> shouldn't be trusting that the file is going to be small enough to be
> read that way. The other one is that I don't think it's a good idea to
> execute it in a fell swoop; seems to be it would be better to split it
> into queries, and rewrite/parse/plan/execute them one by one.
>
> I think a better way to go about this is to have another entry point in
> postgres.c that executes a query the way you want; and somehow read the
> file in small chunks, find where each query ends, and execute them one
> by one. (To be honest, I have no idea how to find out where each query
> ends. psql knows how to do it, but I'm not sure how trustworthy it
> is.)

Well, that's the reason why it's done this way now, relying on a multiple
queries portal. The only trustworthy way to split the queries apart in
the SQL install script would be to rely on gram.y, and I didn't find the
API to explicitly loop over each query parsed.

Given some advice, I'll rework that part too. The good news is that it's
well separated from the rest of the extension's work.

We need a way to do the same as \i in psql, but from the backend, and we
won't be returning anything from there, so we don't need to handle more
than one portal definition in a single fe/be communication.

> As far as #include lines go, please keep them in alphabetical order. As
> a matter of style we always have "postgres.h" alone, then a blank line,
> then system includes, another blank, then the rest of the includes.

Will do.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2010-10-16 09:35:06 Re: Trailing Whitespace Tips (was: Re: starting to review the Extend NOT NULL representation to pg_constraint patch)
Previous Message Dimitri Fontaine 2010-10-16 08:51:15 Re: Extensions, this time with a patch