change type of logerrors in pg_exttable from char to boolean

[hyongtao-db]

pg_exttable on gpdb7 is not consistent with gpdb6

This is the result of \d pg_exttable in gpdb6:

[gpadmin@localhost ~]$ psql
psql (9.4.26)
Type "help" for help.
  
gpadmin=# \d pg_exttable
    Table "pg_catalog.pg_exttable"
     Column      |  Type   | Modifiers
-----------------+---------+-----------
 reloid          | oid     | not null
 urilocation     | text[]  |
 execlocation    | text[]  |
 fmttype         | "char"  |
 fmtopts         | text    |
 options         | text[]  |
 command         | text    |
 rejectlimit     | integer |
 rejectlimittype | "char"  |
 logerrors       | boolean |
 encoding        | integer |
 writable        | boolean |
Indexes:
    "pg_exttable_reloid_index" UNIQUE, btree (reloid)

This is the result of \d pg_exttable in gpdb7:

[gpadmin@localhost ~]$ psql
psql (12beta2)
Type "help" for help.
  
gpadmin=# \d pg_exttable
               View "pg_catalog.pg_exttable"
     Column      |  Type   | Collation | Nullable | Default
-----------------+---------+-----------+----------+---------
 reloid          | oid     |           |          |
 urilocation     | text[]  |           |          |
 execlocation    | text[]  |           |          |
 fmttype         | "char"  |           |          |
 fmtopts         | text    |           |          |
 options         | text[]  |           |          |
 command         | text    |           |          |
 rejectlimit     | integer |           |          |
 rejectlimittype | "char"  |           |          |
 logerrors       | "char"  |           |          |
 encoding        | integer |           |          |
 writable        | boolean |           |          |

The type of logerrors is boolean and char in gpdb6 and gpdb7, respectively. As best I know, there are 2 reasons we need this change:

• for easier backup & restore between different versions
• some GUI faces error because pg_exttable on gpdb7 is not consistent with gpdb6

We want that pg_exttable result in gpdb7 should be consistent with gpdb6, the type of logerrors should be revised. BTW, this pr still dosen't affetct the normal funciton of LOG_ERRORS_PERSISTENTLY, only the display result has been changed. Take a example as followed:

SELECT logerrors from pg_exttable a, pg_class b where a.reloid = b.oid and b.relname = 'ext_persistently';
 logerrors 
-----------
 t
(1 row)

the pg_foreign_table still keeps the info log_errors=p:

gpadmin=# select * from pg_catalog.pg_foreign_table;
 ftrelid | ftserver | ftoptions
 16403 |    13212 | {format=text,delimiter=|,"null=\\N","escape=\\",format_type=t,location_uris=file://
@hostname@@abs_srcdir@/data/ext_persistently.tbl,execute_on=ALL_SEGMENTS,reject_limit=100,reject_limit_ty
pe=r,log_errors=p,encoding=6,is_writable=false}

[kainwen]

Code change seems fine.

Lazy question on some history: do you know why it is changed to char ?

[hyongtao-db]

Code change seems fine.

Lazy question on some history: do you know why it is changed to char ?

@kainwen Thanks. The metadata of gpdb7 had better to be consistent with gpdb6 especially when gpbackup or some other data copy/transfer happens between 2 different versions of gpdb.

[adam8157]

This commit changed it to char, and it was by design.

commit 04fdd0a6f45112512cd743b9fc8114f1374b2107
Author: (Jerome)Junfeng Yang <[[email protected]](mailto:[email protected])>
Date:   Fri Mar 20 15:38:04 2020 +0800

   Enable external table's error log to be persistent for ETL. (#9757)

[hyongtao-db]

-1. Backup & restore on Greenplum 7 will lose information, won't it?

@adam8157 Hi Adam, the pr doesn't affect any code logic of LOG ERRORS PERSISTENTLY. As the result as below shows, the pg_foreign_table still keeps the info log_errors=p.

gpadmin=# select * from pg_catalog.pg_foreign_table;
 ftrelid | ftserver | ftoptions
 16403 |    13212 | {format=text,delimiter=|,"null=\\N","escape=\\",format_type=t,location_uris=file://
@hostname@@abs_srcdir@/data/ext_persistently.tbl,execute_on=ALL_SEGMENTS,reject_limit=100,reject_limit_ty
pe=r,log_errors=p,encoding=6,is_writable=false}

As best I know, there are 2 reasons we need this change:

1. for easier backup & restore between different versions
2. some GUI faces error because pg_exttable on gpdb7 is not consistent with gpdb6

Thanks for your comment. If I miss sth, please kindly remind me.

[adam8157]

Oh, the full meta is stored in the foreign table catalog, is it? I'm fine if backup & restore would work in that way.

[hyongtao-db]

Oh, the full meta is stored in the foreign table catalog, is it? I'm fine if backup & restore would work in that way.

Thanks. Yes, all the infos we rely on is saved in pg_foreign_table. Of course we dare not change catalog crudely. Only the display result changes.