Re: generating fmgr prototypes automatically

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: generating fmgr prototypes automatically
Date: 2017-01-03 19:16:08
Message-ID: CAFj8pRCj_aqxQGdc4c5_9XvCs8YuBCGySb-Cm-Asdbgn55Lp0g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

2016-12-31 6:46 GMT+01:00 Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com
>:

> During some recent patch reviews, there was some back and forth about
> where to put prototypes for fmgr-callable functions. We don't actually
> need these prototypes except to satisfy the compiler, so we end up
> sprinkling them around random header files.
>
> I figured we could generate these prototypes automatically using the
> existing Gen_fmgrtab.pl script. That ends up saving more than 2000
> lines of manual code, and it helps detect when a function exists in code
> but is not registered in pg_proc.h.
>
> ... which this actually found in cash.c. I ended up adding the missing
> entries in pg_proc and pg_operator, but we could also opt to just remove
> the unused code.
>
> There are long-standing symbol clashes from the lo_* functions between
> the backend and libpq. So far this hasn't bothered anyone, but because
> this patch rearranges some header files, the libpqwalreceiver code gets
> upset. So I renamed the C symbols for the backends functions in a
> separate patch. The user-visible function names remain the same.
>

I am sending a review of these patches

1. there was not any problem with patching, compiling, all test passed
2. the doc and regress tests are not necessary ??

patch 0001 .. trivial cleaning
patch 0002 .. renaming lo_* to be_lo_* -- the prefix "be" is not what I
expect - maybe "pg" instead. More because the be-fsstubs.h will be holds
only lo_read, lo_write on end
patch 0003 .. trivial, but doesn't mean so we have not regress tests for
these functions?
patch 0004 .. bigger part - I miss a comments when there are a exceptions:

extern Datum numeric_float8_no_overflow(PG_FUNCTION_ARGS);
extern Datum nextval(PG_FUNCTION_ARGS);
extern Datum fmgr_sql(PG_FUNCTION_ARGS);
extern Datum aggregate_dummy(PG_FUNCTION_ARGS);

I found some obsolete definitions in c files

pgstatfuncs.c

/* bogus ... these externs should be in a header file */
extern Datum pg_stat_get_numscans(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_returned(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_fetched(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_inserted(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_updated(PG_FUNCTION_ARGS);

namespace.c
Datum><-->pg_table_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_type_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_function_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_operator_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_opclass_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_opfamily_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_collation_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_conversion_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_ts_parser_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_ts_dict_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_ts_template_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_ts_config_is_visible(PG_FUNCTION_ARGS);
Datum<-><-->pg_my_temp_schema(PG_FUNCTION_ARGS);
Datum<-><-->pg_is_other_temp_schema(PG_FUNCTION_ARGS);

Regards

Pavel

>
> --
> Peter Eisentraut http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2017-01-03 19:17:18 Re: emergency outage requiring database restart
Previous Message Peter Eisentraut 2017-01-03 19:05:21 Re: emergency outage requiring database restart