Skip to content

2.25.1.0-b171

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
Assets 2
Loading