This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit e6e665ea07ba84b6ee907cd6e31d3e00858c65ae Author: Ashwin Agrawal <[email protected]> AuthorDate: Tue Oct 25 15:47:48 2022 -0700 gp_replica_check: resolve FIXMEs FIXMEs were related to adding support for checking dynamic access methods. Given we are far away from supporting all built-in access methods, dynamic support is not on the radar at all. Plus, having a new access method is not a common act at all. So, we can cross the bridge of converting RelationTypeData to a dynamic array instead of the static array when we get there. Hence, for now, resolving the FIXME by manually updating the static array to reflect all the current access methods. Note: with this change now "ao" gets split into ao_row and ao_column. This is nice as can run more granular checking if desired. Also, converted GPDB_94_MERGE_FIXME to GPDB_FEATURE_NOT_SUPPORTED in Makefile. As we are yet to test/validate/enable this tool for various index access methods - which we should do separately. --- gpcontrib/gp_replica_check/Makefile | 7 ++-- gpcontrib/gp_replica_check/gp_replica_check.c | 58 ++++++++++++--------------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/gpcontrib/gp_replica_check/Makefile b/gpcontrib/gp_replica_check/Makefile index db6fadcc99..4476b2db01 100644 --- a/gpcontrib/gp_replica_check/Makefile +++ b/gpcontrib/gp_replica_check/Makefile @@ -14,7 +14,8 @@ include $(top_builddir)/src/Makefile.global include $(top_srcdir)/contrib/contrib-global.mk endif -# GPDB_94_MERGE_FIXME: Enable btree, gin and gist when physical and logical -# order is equivalent. +# GPDB_FEATURE_NOT_SUPPORTED: Enable btree, gin and gist as physical +# and logical order is equivalent now. Also, enable brin, spgist, +# bitmap. Basically, "all" option should be functional for this tool. installcheck: install - gp_replica_check.py -r="heap, sequence, ao" + gp_replica_check.py -r="heap, ao_row, ao_column" diff --git a/gpcontrib/gp_replica_check/gp_replica_check.c b/gpcontrib/gp_replica_check/gp_replica_check.c index f3ecaece51..defef980a8 100644 --- a/gpcontrib/gp_replica_check/gp_replica_check.c +++ b/gpcontrib/gp_replica_check/gp_replica_check.c @@ -81,28 +81,28 @@ typedef struct RelationTypeData bool include; } RelationTypeData; -#define MAX_INCLUDE_RELATION_TYPES 8 +#define MAX_INCLUDE_RELATION_TYPES 10 /* - * GPDB_12_MERGE_FIXME: new access methods can be defined, which cannot be - * checked using the current way by comparing predefined access method OIDs. - * The AM handler functions need to be looked up and compared instead. - * E.g. to tell if it's an appendoptimized row oriented table, look up the - * handler function for that table's AM in pg_am_handler and compare it with - * F_AO_ROW_TABLEAM_HANDLER. + * This is a static pre-defined array for now as currently, this tool works + * only for pre-defined access methods. As most likely will need masking + * functions to perform proper comparisons for newer access methods. Plus, + * having on the fly access methods is not at all common phenomenon. If and + * when in future new access method is added, can update this array or enhance + * the tool to dynamically create this array based on pg_am table. * - * If the tool is desired to be used against pre-defined access methods only, - * then no change would be needed. */ static RelationTypeData relation_types[MAX_INCLUDE_RELATION_TYPES+1] = { + {"heap", false}, {"btree", false}, {"hash", false}, {"gist", false}, {"gin", false}, + {"ao_row", false}, + {"ao_column", false}, + {"brin", false}, + {"spgist", false}, {"bitmap", false}, - {"heap", false}, - {"sequence", false}, - {"ao", false}, {"unknown relam", false} }; @@ -171,29 +171,28 @@ init_relation_types(char *include_relation_types) static RelationTypeData get_relation_type_data(Oid relam, int relkind) { - /* GPDB_12_MERGE_FIXME: Why doesn't this just look up the AM name from pg_am? */ switch(relam) { - case BTREE_AM_OID: + case HEAP_TABLE_AM_OID: return relation_types[0]; - case HASH_AM_OID: + case BTREE_AM_OID: return relation_types[1]; - case GIST_AM_OID: + case HASH_AM_OID: return relation_types[2]; - case GIN_AM_OID: + case GIST_AM_OID: return relation_types[3]; - case BITMAP_AM_OID: + case GIN_AM_OID: return relation_types[4]; - - case HEAP_TABLE_AM_OID: - if (relkind == RELKIND_SEQUENCE) - return relation_types[6]; - else - return relation_types[5]; case AO_ROW_TABLE_AM_OID: + return relation_types[5]; case AO_COLUMN_TABLE_AM_OID: + return relation_types[6]; + case BRIN_AM_OID: return relation_types[7]; - + case SPGIST_AM_OID: + return relation_types[8]; + case BITMAP_AM_OID: + return relation_types[9]; default: return relation_types[MAX_INCLUDE_RELATION_TYPES]; } @@ -538,15 +537,8 @@ get_relfilenode_map() { Form_pg_class classtuple = (Form_pg_class) GETSTRUCT(tup); - /* GPDB_12_MERGE_FIXME: What was the point of the relstorage test here? */ if ((classtuple->relkind == RELKIND_VIEW - || classtuple->relkind == RELKIND_COMPOSITE_TYPE) - /* || (classtuple->relstorage != RELSTORAGE_HEAP - && !relstorage_is_ao(classtuple->relstorage)) */) - continue; - - /* unlogged tables do not propagate to replica servers */ - if (classtuple->relpersistence == RELPERSISTENCE_UNLOGGED) + || classtuple->relkind == RELKIND_COMPOSITE_TYPE)) continue; RelfilenodeEntry *rentry; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
