Re: Review: Fix snapshot taking inconsistencies

Excerpts from Tom Lane's message of lun oct 04 10:31:26 -0400 2010:

> In the particular case at hand here, I rather wonder why SQL functions
> are depending on postgres.c at all. It might be better to just
> duplicate a bit of code to make them independent. pg_parse_and_rewrite
> would then be dead code and could be deleted.

This idea doesn't work, unless pushed a lot further. Attached are two
separate patches, extracted from the last patch version posted by Marko
(git commit --interactive helped here). The first one does what you
suggest above: remove pg_parse_and_rewrite and inline it into the
callers. The other patch is the remainder.

Read on for the details of the first patch. As for the second patch,
which is Marko's original intention anyway, I don't see that it needs to
be delayed because of the first one. So while I haven't reviewed it, I
think it should be considered separately.

The problem with this patch (0001) is that the inlined versions of the
code that used to be pg_parse_and_rewrite are still depending on
functions in postgres.c. These are pg_parse_query and
pg_analyze_and_rewrite. pg_parse_query is just a layer on top of
raw_parser. pg_analyze_and_rewrite is a layer on top of parse_analyze
plus pg_rewrite_query (also on postgres.c).

Now, the only difference between those functions and the ones that
underlie them is that they have the bunch of tracing macros and
log_parser_stats reporting. So one solution would be to have SQL
functions (pg_proc.c and executor/functions.c) call the raw parser.c and
analyze.c functions directly, without invoking the tracing/logging code.

The other idea would be to move the whole of those functions out of
postgres.c and into their own modules, i.e. move pg_parse_query into
parser.c and pg_analyze_and_rewrite and pg_rewrite_query into
rewriteHandler.c. (This actually requires a bit more effort because we
should also move pg_analyze_and_rewrite_params out of postgres.c,
following pg_analyze_and_rewrite).

Note that pg_analyze_and_rewrite and its "params" siblings are being
called from copy.c, spi.c, optimizer/util/clauses.c, and plancache.c.
So it does make certain sense to move them out of postgres.c, if we want
to think of postgres.c as a module only concerned with client
interaction.

The only quarrel I have with this code shuffling is that
pg_rewrite_query is being called from exec_parse_message. Since it's
now a static function, it would have to stop being static so that it can
be called from both places (postgres.c and rewriteHandler.c)

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support