Summary:
src/yb/yql/pggate/ybc_pg_typedefs.h names types in an inconsistent way.
Here are some examples:
- typedef struct PgGFlagsAccessor { ... } YBCPgGFlagsAccessor;
- typedef struct YbCloneInfo { ... } YbCloneInfo;
- typedef struct PgYCQLStatementStats { ... } YCQLStatementStats;
- typedef struct YbInsertOnConflictKeyInfo { ... }
YBCInsertOnConflictKeyInfo;
- typedef struct { ... } YBAdvisoryLockId;
Make it all consistent as follows:
- typedef struct { ... } YbcPgGFlagsAccessor;
- typedef struct { ... } YbcCloneInfo;
- typedef struct { ... } YbcYCQLStatementStats;
- typedef struct { ... } YbcInsertOnConflictKeyInfo;
- typedef struct { ... } YbcAdvisoryLockId;
Previously, both names could be used to reference the same
enum/struct/union in C++ code, so it resulted in inconsistencies on
which name is used. By having a single name, this inconsistency
disappears. "Ybc" prefix is chosen to signify that the object is
defined in extern "C". The ones that affect postgres are all defined in
src/yb/yql/**/ybc_*.h (with the exception of src/yb/util/status.h, which
only has a small section extern "C" and is manually renamed with "Ybc"
prefix). This is after renaming some filenames to have "ybc_" prefix.
The files are as follows:
- ybc_pg_typedefs.h
- ybc_pggate.cc
- ybc_pggate.h
- ysql_bench_metrics_handler.h -> ybc_ysql_bench_metrics_handler.h
- pgsql_webserver_wrapper.h -> ybc_pg_webserver_wrapper.h
- yb_guc.h -> ybc_guc.h
In case a struct definition has #ifdef __cplusplus, a name after
"struct" is necessary to appease -Wnon-c-typedef-for-linkage which
causes failure on clang compilation. So we have the following cases:
- typedef struct YbcPgExecParameters { ... } YbcPgExecParameters;
- typedef struct YbcPgExecOutParamValue { ... } YbcPgExecOutParamValue;
In case a struct is not defined but still involved in typedef, both the
struct name and typedef name should match. There are currently no cases
of this.
Add lint rules:
- missing_ybc_prefix: make sure there is a "Ybc" prefix for all typedefs
in these files. This can flag some false positives such as
UnorderedSliceSet, but for simplicitly of the lint rule, rename those
anyway.
- mismatching_typename: in case name is required after
enum/struct/union, make sure the two names are the same.
- missing_ybc_in_filename: make sure files have "ybc_" prefix if they
have extern "C".
- bad_ybc_in_filename: inverse of above.
Move existing src/postgres/ybsimplelint.sh to src/lint/postgres.sh, and
add the new lint rules to new file src/lint/pggate_ybc.sh.
A followup will change the remaining type names in src/postgres to have
"yb" (case insensitive), and typedefs.list will be updated with YB
types, which are easily identifiable by the presence of "yb". This is a
step towards making pgindent usable.
Function names and global variable names also ideally should have proper
"yb" prefixing, but they are a currently lower priority. The priority
right now is to get pgindent working.
Jira: DB-14842
Test Plan:
On Almalinux 8, ctags version "Exuberant Ctags 5.8", vi version "VIM -
Vi IMproved 8.0",
arc lint src/yb/yql/***/ybc_*
Close: #25587
Reviewers: xCluster, hsunder, patnaik.balivada
Reviewed By: patnaik.balivada
Subscribers: patnaik.balivada, ybase, yql, ycdcxcluster
Differential Revision: https://phorge.dev.yugabyte.com/D41070