Thanks for taking a look.
On 3/3/18, Tom Lane <t...@sss.pgh.pa.us> wrote:
> John Naylor <jcnay...@gmail.com> writes:
>> Version 8, rebased against 76b6aa41f41d.
>
> I took a preliminary look through this, without yet attempting to execute
> the script against HEAD. I have a few thoughts:
>
> * I'm inclined not to commit the conversion scripts to the repo. I doubt
> there are third parties out there with a need for them, and if they do
> need them they can get 'em out of this thread in the mailing list
> archives. (If anyone has a different opinion about that, speak up!)
If no one feels strongly otherwise, I'll just attach the conversion
script along with the other patches for next version. To be clear, the
rewrite script is intended be committed, both to enforce formatting
and as a springboard for bulk editing. Now, whether that belongs in
/src/include/catalog or /src/tools is debatable.
> * I'm a little disturbed by the fact that 0002 has to "re-doublequote
> values that are macros expanded by initdb.c". I see that there are only
> a small number of affected places, so maybe it's not really worth working
> harder, but I worry that something might get missed. Is there any way to
> include this consideration in the automated conversion, or at least to
> verify that we found all the places to quote? Or, seeing that 0004 seems
> to be introducing some quoting-related hacks to genbki.pl anyway, maybe
> we could take care of the issue there?
The quoting hacks are really to keep the postgres.bki diff as small as
possible (attached). The simplest and most air-tight way to address
your concern would be to double-quote everything when writing the bki
file. That could be done last as a follow-up.
> * I notice you have a few "preliminary cleanup" changes here and there
> in the series, such as fixing the inconsistent spelling of
> Anum_pg_init_privs_initprivs. These could be applied before we start
> the main conversion process, and I'm inclined to do that since we could
> get 'em out of the way early. Ideally, the main conversion would only
> touch the header files and related scripts/makefiles.
...
> * In 0003, I'd recommend leaving the re-indentation to happen in the next
> perltidy run (assuming perltidy would fix that, which I think is true but
> I might be wrong). It's just creating more review work to do it here.
> In any case, the patch summary line is pretty misleading since it's
> *not* just reindenting, but also refactoring genbki.pl. (BTW, if that
> refactoring would work on the script as it is, maybe that's another
> thing we could do early? The more we can do before "flag day", the
> better IMO.)
I tested perltidy 20090616 and it handles it fine. I'll submit a
preliminary patch soon to get some of those items out of the way.
> * In 0006, I'm not very pleased with the introduction of
> "Makefile.headers". I'd keep those macros where they are in
> catalog/Makefile. backend/Makefile doesn't need to know about that,
> especially since it's doing an unconditional invocation of
> catalog/Makefile anyway. It could just do something like
>
> submake-schemapg:
> $(MAKE) -C catalog generated-headers
>
> and leave it to catalog/Makefile to know what needs to happen for
> both schemapg.h and the other generated files.
I wasn't happy with it either, but I couldn't get it to build
otherwise. The sticking point was the symlinks in
$(builddir)/src/include/catalog. $(MAKE) -C catalog doesn't handle
that. The makefile in /src/common relies on the backend makefile to
know what to invoke for a given header. IIRC, relpath.c includes
pg_tablespace.h, which now requires pg_tablespace_d.h to be built.
Perhaps /src/common/Makefile could invoke the catalog makefile
directly, and the pg_*_d.h headers could be written to
$(builddir)/src/include/catalog directly? I'll hack on it some more.
> Overall, though, this is looking pretty promising.
>
> regards, tom lane
>
Glad to hear it.
-John Naylor
--- postgres.bki.orig 2018-02-25 15:58:50.082261557 +0700
+++ postgres.bki.noquotes 2018-02-25 16:12:39.967498748 +0700
@@ -5651,9 +5651,9 @@
lanacl = aclitem[]
)
open pg_language
-insert OID = 12 ( "internal" 10 f f 0 0 2246 _null_ )
-insert OID = 13 ( "c" 10 f f 0 0 2247 _null_ )
-insert OID = 14 ( "sql" 10 f t 0 0 2248 _null_ )
+insert OID = 12 ( internal 10 f f 0 0 2246 _null_ )
+insert OID = 13 ( c 10 f f 0 0 2247 _null_ )
+insert OID = 14 ( sql 10 f t 0 0 2248 _null_ )
close pg_language
create pg_largeobject_metadata 2995
(
@@ -5753,8 +5753,8 @@
insert ( 2798 n 0 2796 - 2796 - - - - - f f r r 2799 27 0 0 0 _null_ _null_ )
insert ( 3527 n 0 3524 - 3524 - - - - - f f r r 3518 3500 0 0 0 _null_ _null_ )
insert ( 3565 n 0 3563 - 3563 - - - - - f f r r 1203 869 0 0 0 _null_ _null_ )
-insert ( 2147 n 0 2804 - 463 - - 2804 3547 - f f r r 0 20 0 20 0 "0" "0" )
-insert ( 2803 n 0 1219 - 463 - - 1219 3546 - f f r r 0 20 0 20 0 "0" "0" )
+insert ( 2147 n 0 2804 - 463 - - 2804 3547 - f f r r 0 20 0 20 0 0 0 )
+insert ( 2803 n 0 1219 - 463 - - 1219 3546 - f f r r 0 20 0 20 0 0 0 )
insert ( 2718 n 0 1836 2514 3341 3335 3336 1836 3569 2514 f f r r 0 2281 128
2281 128 _null_ _null_ )
insert ( 2719 n 0 1835 3390 3338 3339 3340 1835 3568 3390 f f r r 0 2281 48
2281 48 _null_ _null_ )
insert ( 2720 n 0 1834 3390 3338 3339 3340 1834 3567 3390 f f r r 0 2281 48
2281 48 _null_ _null_ )
@@ -5791,7 +5791,7 @@
insert ( 2157 n 0 208 1832 276 - - - - - f f r r 0 1022 0 0 0 "{0,0,0}" _null_
)
insert ( 2158 n 0 222 1832 276 - - - - - f f r r 0 1022 0 0 0 "{0,0,0}" _null_
)
insert ( 2159 n 0 1833 1839 3341 3335 3336 1833 3548 1839 f f r r 0 2281 128
2281 128 _null_ _null_ )
-insert ( 2818 n 0 2805 - 463 - - - - - f f r r 0 20 0 0 0 "0" _null_ )
+insert ( 2818 n 0 2805 - 463 - - - - - f f r r 0 20 0 0 0 0 _null_ )
insert ( 2819 n 0 2806 2807 3342 - - - - - f f r r 0 1022 0 0 0
"{0,0,0,0,0,0}" _null_ )
insert ( 2820 n 0 2806 2808 3342 - - - - - f f r r 0 1022 0 0 0
"{0,0,0,0,0,0}" _null_ )
insert ( 2821 n 0 2806 2809 3342 - - - - - f f r r 0 1022 0 0 0
"{0,0,0,0,0,0}" _null_ )
@@ -6177,9 +6177,9 @@
nspacl = aclitem[]
)
open pg_namespace
-insert OID = 11 ( "pg_catalog" 10 _null_ )
-insert OID = 99 ( "pg_toast" 10 _null_ )
-insert OID = 2200 ( "public" 10 _null_ )
+insert OID = 11 ( pg_catalog 10 _null_ )
+insert OID = 99 ( pg_toast 10 _null_ )
+insert OID = 2200 ( public 10 _null_ )
close pg_namespace
create pg_conversion 2607
(
@@ -6255,14 +6255,14 @@
tmplacl = aclitem[]
)
open pg_pltemplate
-insert ( "plpgsql" t t "plpgsql_call_handler" "plpgsql_inline_handler"
"plpgsql_validator" "$libdir/plpgsql" _null_ )
-insert ( "pltcl" t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_
)
-insert ( "pltclu" f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl"
_null_ )
-insert ( "plperl" t t "plperl_call_handler" "plperl_inline_handler"
"plperl_validator" "$libdir/plperl" _null_ )
-insert ( "plperlu" f f "plperlu_call_handler" "plperlu_inline_handler"
"plperlu_validator" "$libdir/plperl" _null_ )
-insert ( "plpythonu" f f "plpython_call_handler" "plpython_inline_handler"
"plpython_validator" "$libdir/plpython2" _null_ )
-insert ( "plpython2u" f f "plpython2_call_handler" "plpython2_inline_handler"
"plpython2_validator" "$libdir/plpython2" _null_ )
-insert ( "plpython3u" f f "plpython3_call_handler" "plpython3_inline_handler"
"plpython3_validator" "$libdir/plpython3" _null_ )
+insert ( plpgsql t t plpgsql_call_handler plpgsql_inline_handler
plpgsql_validator "$libdir/plpgsql" _null_ )
+insert ( pltcl t t pltcl_call_handler _null_ _null_ "$libdir/pltcl" _null_ )
+insert ( pltclu f f pltclu_call_handler _null_ _null_ "$libdir/pltcl" _null_ )
+insert ( plperl t t plperl_call_handler plperl_inline_handler plperl_validator
"$libdir/plperl" _null_ )
+insert ( plperlu f f plperlu_call_handler plperlu_inline_handler
plperlu_validator "$libdir/plperl" _null_ )
+insert ( plpythonu f f plpython_call_handler plpython_inline_handler
plpython_validator "$libdir/plpython2" _null_ )
+insert ( plpython2u f f plpython2_call_handler plpython2_inline_handler
plpython2_validator "$libdir/plpython2" _null_ )
+insert ( plpython3u f f plpython3_call_handler plpython3_inline_handler
plpython3_validator "$libdir/plpython3" _null_ )
close pg_pltemplate
create pg_authid 1260 shared_relation rowtype_oid 2842
(
@@ -6280,11 +6280,11 @@
)
open pg_authid
insert OID = 10 ( "POSTGRES" t t t t t t t -1 _null_ _null_ )
-insert OID = 3373 ( "pg_monitor" f t f f f f f -1 _null_ _null_ )
-insert OID = 3374 ( "pg_read_all_settings" f t f f f f f -1 _null_ _null_ )
-insert OID = 3375 ( "pg_read_all_stats" f t f f f f f -1 _null_ _null_ )
-insert OID = 3377 ( "pg_stat_scan_tables" f t f f f f f -1 _null_ _null_ )
-insert OID = 4200 ( "pg_signal_backend" f t f f f f f -1 _null_ _null_ )
+insert OID = 3373 ( pg_monitor f t f f f f f -1 _null_ _null_ )
+insert OID = 3374 ( pg_read_all_settings f t f f f f f -1 _null_ _null_ )
+insert OID = 3375 ( pg_read_all_stats f t f f f f f -1 _null_ _null_ )
+insert OID = 3377 ( pg_stat_scan_tables f t f f f f f -1 _null_ _null_ )
+insert OID = 4200 ( pg_signal_backend f t f f f f f -1 _null_ _null_ )
close pg_authid
create pg_auth_members 1261 shared_relation without_oids rowtype_oid 2843
(
@@ -6323,7 +6323,7 @@
cfgparser = oid
)
open pg_ts_config
-insert OID = 3748 ( "simple" 11 10 3722 )
+insert OID = 3748 ( simple 11 10 3722 )
close pg_ts_config
create pg_ts_config_map 3603 without_oids
(
@@ -6362,7 +6362,7 @@
dictinitoption = text
)
open pg_ts_dict
-insert OID = 3765 ( "simple" 11 10 3727 _null_ )
+insert OID = 3765 ( simple 11 10 3727 _null_ )
close pg_ts_dict
create pg_ts_parser 3601
(
@@ -6375,7 +6375,7 @@
prslextype = regproc
)
open pg_ts_parser
-insert OID = 3722 ( "default" 11 3717 3718 3719 3720 3721 )
+insert OID = 3722 ( default 11 3717 3718 3719 3720 3721 )
close pg_ts_parser
create pg_ts_template 3764
(
@@ -6385,10 +6385,10 @@
tmpllexize = regproc
)
open pg_ts_template
-insert OID = 3727 ( "simple" 11 3725 3726 )
-insert OID = 3730 ( "synonym" 11 3728 3729 )
-insert OID = 3733 ( "ispell" 11 3731 3732 )
-insert OID = 3742 ( "thesaurus" 11 3740 3741 )
+insert OID = 3727 ( simple 11 3725 3726 )
+insert OID = 3730 ( synonym 11 3728 3729 )
+insert OID = 3733 ( ispell 11 3731 3732 )
+insert OID = 3742 ( thesaurus 11 3740 3741 )
close pg_ts_template
create pg_extension 3079
(
@@ -6511,8 +6511,8 @@
)
open pg_collation
insert OID = 100 ( default 11 10 d -1 "" "" _null_ )
-insert OID = 950 ( C 11 10 c -1 "C" "C" _null_ )
-insert OID = 951 ( POSIX 11 10 c -1 "POSIX" "POSIX" _null_ )
+insert OID = 950 ( C 11 10 c -1 C C _null_ )
+insert OID = 951 ( POSIX 11 10 c -1 POSIX POSIX _null_ )
close pg_collation
create pg_partitioned_table 3350 without_oids
(
