Re: [HACKERS] generating fmgr prototypes automatically

[Peter Eisentraut]

On 1/5/17 12:24 PM, Pavel Stehule wrote:
> I checked last set of patches and I didn't find any issue. 
> 
> There are no problems with patching, compilation and all regress tests
> passed.
> 
> I'll mark this patch as ready for commiter

This has been committed.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] generating fmgr prototypes automatically

[Pavel Stehule]

Hi

2017-01-04 21:09 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 1/3/17 2:16 PM, Pavel Stehule wrote:
> > 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
>
> It's the naming that the OpenSSL functions use, e.g., be_tls_init.
>
> > patch 0003 .. trivial, but doesn't mean so we have not regress tests for
> > these functions?
>
> OK, added tests.
>
> > 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);
>
> These functions and their special purpose are commented in the .c files,
> so I think that is covered OK.  I added an additional comment to the
> numeric_float8_no_overflow().
>
> > I found some obsolete definitions in c files
>
> OK, fixed those as well.  I think I initially only looked in .h files.
> That just underlines how inconsistent this is, e.g.,
>
> > pgstatfuncs.c
> >
> > /* bogus ... these externs should be in a header file */
>
> > namespace.c
> >
> > /* These don't really need to appear in any header file */
>
> New patch set attached.
>

I checked last set of patches and I didn't find any issue.

There are no problems with patching, compilation and all regress tests
passed.

I'll mark this patch as ready for commiter

Regards

Pavel





>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: [HACKERS] generating fmgr prototypes automatically

[Pavel Stehule]

2017-01-04 22:17 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 1/4/17 3:35 PM, Pavel Stehule wrote:
> > On 1/3/17 2:16 PM, Pavel Stehule wrote:
> > > 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
> >
> > It's the naming that the OpenSSL functions use, e.g., be_tls_init.
> >
> >
> > lo_* functions are used by OpenSSL ?
>
> There are tls functions that exist in similar ways in the backend and in
> libpq, and the backend versions are called be_*.
>

tls functions are not available via V1 API. It is still strange to me.

Regards

Pavel



>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: [HACKERS] generating fmgr prototypes automatically

[Peter Eisentraut]

On 1/4/17 3:35 PM, Pavel Stehule wrote:
> On 1/3/17 2:16 PM, Pavel Stehule wrote:
> > 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
> 
> It's the naming that the OpenSSL functions use, e.g., be_tls_init.
> 
> 
> lo_* functions are used by OpenSSL ?

There are tls functions that exist in similar ways in the backend and in
libpq, and the backend versions are called be_*.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] generating fmgr prototypes automatically

[Pavel Stehule]

2017-01-04 21:09 GMT+01:00 Peter Eisentraut <
peter.eisentr...@2ndquadrant.com>:

> On 1/3/17 2:16 PM, Pavel Stehule wrote:
> > 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
>
> It's the naming that the OpenSSL functions use, e.g., be_tls_init.
>

lo_* functions are used by OpenSSL ?


>
> > patch 0003 .. trivial, but doesn't mean so we have not regress tests for
> > these functions?
>
> OK, added tests.
>
> > 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);
>
> These functions and their special purpose are commented in the .c files,
> so I think that is covered OK.  I added an additional comment to the
> numeric_float8_no_overflow().
>
> > I found some obsolete definitions in c files
>
> OK, fixed those as well.  I think I initially only looked in .h files.
> That just underlines how inconsistent this is, e.g.,
>
> > pgstatfuncs.c
> >
> > /* bogus ... these externs should be in a header file */
>
> > namespace.c
> >
> > /* These don't really need to appear in any header file */
>
> New patch set attached.
>
>
Thank you

I'll check it tomorrow

Regards

Pavel

> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: [HACKERS] generating fmgr prototypes automatically

[Pavel Stehule]

Hi

2016-12-31 6:46 GMT+01:00 Peter Eisentraut :

> 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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>