Re: Select parser at runtime - Mailing list pgsql-hackers

Ian Lance Taylor <ian@airs.com> writes:
> I've been experimenting with using a different parser (one which is
> more Oracle compatible).

Hmm.  What we have here is a mechanism to swap out the entire
backend/parser/ subdirectory --- and nothing else.  Somehow this seems
like the wrong granularity.  parser/ is an awful lot of code to replace
to make a few random tweaks that don't affect query semantics.  Since
you aren't changing the querytree representation nor any of the rewrite/
plan/execute pipeline, it's hard to see how you can do more with this
than very marginal syntax hacks.  But if that's all you want to do,
seems like replacing pieces of the parser/semantic analyzer is the right
mechanism, not the whole durn thing.

> Note that you may want to leave yourself an escape
> hatch of some sort to set the parser back to Postgres standard.
> If this patch is accepted, then some further work needs to be done to
> set the parser for SPI calls, so that it is possible for the user to
> change the parser while still using ordinary PL/pgSQL.

I think both of these issues would need to be addressed before, not
after, considering the patch for acceptance.  In particular, how do we
cater for both plpgsql and a true "PL/SQL" language that uses the Oracle
parser?

> +    {
> +        Datum    result;
> +
> +        parser_function_fcache->fcinfo.arg[0] = PointerGetDatum(query_string);
> +        parser_function_fcache->fcinfo.arg[1] = PointerGetDatum(typev);
> +        parser_function_fcache->fcinfo.arg[2] = Int32GetDatum(nargs);

> -    raw_parsetree_list = parser(query_string, typev, nargs);
> +        result = FunctionCallInvoke(&parser_function_fcache->fcinfo);
> +        raw_parsetree_list = (List *) DatumGetPointer(result);
> +    }

Use FunctionCall3() to hide the cruft here.

> +        fcache = init_fcache(oid, 3, TopMemoryContext);

This is a tad odd.  Why don't you just do fmgr_info and store an FmgrInfo
structure?  You have no use for the rest of an executor fcache
structure.

The update_parser_function_fcache business bothers me, too.  I see the
problem: it doesn't work to do this lookup when postgresql.conf is read
in the postmaster.  However, I really don't like the notion of disabling
check_parser and allowing a possibly-bogus value to be assigned on
faith.  (If the function name *is* bogus, your code can never recover;
at the very least you ought to clear update_parser_function_fcache
before failing.)  Given the extent to which the parser is necessarily
tied to the rest of the system, I'm not sure there's any value in
allowing it to be implemented as a dynamic-link function.  I'd be more
than half inclined to go with a lower-tech solution wherein you expect
the alternate parser to be permanently linked in and known to the
check_parser and assign_parser subroutines.  Then the state variable is
just a C function pointer, and assign_parser looks something like

#ifdef ORACLE_PARSER_SUPPORTED
    if (strcasecmp(value, "Oracle") == 0)
        parser_fn = oracle_parser;
    else
#endif

            regards, tom lane