[#25482] YSQL: improve logging style/linter

Summary:
- Add and update linter rules related to logging

  - Combine missing_linebreak_before_errmsg,
    missing_linebreak_before_errdetail, missing_linebreak_before_errhint
    to a single rule missing_linebreak_before_err

    - Additionally handle errcode in this rule
    - Avoid false positives with comments containing text such as
      "errmsg()"
    - Blacklist some files where upstream postgres breaks this rule

  - Add new rule missing_linebreak_before_paren_err

    - Make it a warning instead of error because upstream postgres
      breaks this rule several times

  - Add new rule missing_paren_before_err
  - Update likely_bad_capitalization_in_errmsg,
    likely_bad_lowercase_in_errdetail, likely_bad_lowercase_in_errhint

    - Capture more cases where "err" is preceded by opening parenthesis
    - Make likely_bad_lowercase_in_errhint handle errhint_plural as
      well, just like how likely_bad_lowercase_in_errdetail does it
      (this must have been missed previously)
    - Fix the regex to properly find bad casing (this must have been an
      oversight previously)

- Fix likely_bad_.*err.* rules manually, avoiding false positives, which
  were found in the following files:

  - verify_nbtree.c
  - cube.c
  - jsonb_plpython.c
  - passwordcheck.c
  - pgcrypto.c
  - heap_surgery.c
  - pqcomm.c
  - postgres.c
  - spell.c
  - jsonbsubs.c
  - varlena.c
  - plperl.c
  - ybgin.c
  - pg_yb_utils.c

  While at it, fix punctuation as well (errmsg should not have
  sentence-ending punctuation; errdetail/errhint should), fix
  grammar/spelling, and break long errmsg to errdetail/errhint where
  suitable.
Jira: DB-14732

Test Plan:
On Almalinux 8, zsh with EXTENDEDGLOB option:

    [ -z "$(arc lint src/postgres/**/*.{c,h} | grep missing_linebreak_before_err)" ]

Close: #25482
Jenkins: all tests

Reviewers: rbarigidad, vpatibandla, devansh.saxena

Reviewed By: devansh.saxena

Subscribers: ybase, yql, ycdcxcluster

Differential Revision: https://phorge.dev.yugabyte.com/D40958

java/yb-pgsql/src/test/java/org/yb/pgsql/TestBatchCopyFrom.java

@@ -40,7 +40,7 @@ public class TestBatchCopyFrom extends BasePgSQLTest {
   private static final Logger LOG = LoggerFactory.getLogger(TestBatchCopyFrom.class);
 
   private static final String INVALID_BATCH_OPTION_USAGE_WARNING_MSG =
-      "Batched COPY is not supported";
+      "batched COPY is not supported";
   private static final String INVALID_BATCH_SIZE_ERROR_MSG =
       "argument to option \"rows_per_transaction\" must be a positive integer";
   private static final String INVALID_NUM_SKIPPED_ROWS_ERROR_MSG =
@@ -724,8 +724,7 @@ public void testEnableUpsertModeSessionVariableTrigger() throws Exception {
         statement,
         String.format("COPY %s FROM \'%s\' WITH (FORMAT CSV, HEADER)", tableName, absFilePath),
         Arrays.asList(
-          "Batched COPY is not supported on table with non RI trigger. " +
-            "Defaulting to using one transaction for the entire copy.",
+          "batched COPY is not supported on table with non RI trigger",
           "trigger_func(before_ins_stmt) called: action = INSERT, when = BEFORE, level = STATEMENT",
           "trigger_func(after_ins_stmt) called: action = INSERT, when = AFTER, level = STATEMENT"));
 

java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgReplicationSlot.java

@@ -3038,8 +3038,8 @@ public void testDynamicTableWithReplicaIdentityChangeWithPgoutput() throws Excep
     try {
       result.addAll(receiveMessage(stream, 12));
     } catch (PSQLException e) {
-      assertTrue(e.getMessage().contains("Replica identity CHANGE is not supported for output"
-        + " plugin pgoutput. Consider using output plugin yboutput instead."));
+      assertTrue(e.getMessage().contains("replica identity CHANGE is not supported for output"
+        + " plugin pgoutput"));
     }
   }
 

src/postgres/contrib/pg_stat_statements/pg_stat_statements.c

@@ -3506,7 +3506,7 @@ yb_get_histogram_jsonb_args(uint64 queryid, Oid userid, Oid dbid, bool top_level
 	if (!is_allowed_role && userid != GetUserId())
 	{
 		ereport(ERROR,
-				(errmsg("Insufficient privilege to read this query detail.")));
+				(errmsg("insufficient privilege to read this query detail")));
 		PG_RETURN_DATUM(0);
 	}
 
@@ -3515,8 +3515,8 @@ yb_get_histogram_jsonb_args(uint64 queryid, Oid userid, Oid dbid, bool top_level
 	if (!entry)
 	{
 		ereport(ERROR,
-				(errmsg("Invalid combination of queryid, userid, dbid and top_level.\n"
-						"Please refer to pg_stat_statements for the correct details.")));
+				(errmsg("invalid combination of queryid, userid, dbid, and top_level"),
+				 errhint("Please refer to pg_stat_statements for the correct details.")));
 		PG_RETURN_DATUM(0);
 	}
 

src/postgres/src/backend/access/heap/heapam.c

@@ -2073,7 +2073,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	if (IsYBRelation(relation))
 		ereport(ERROR,
 				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("Operation not allowed in YugaByte mode %s",
+				 errmsg("operation not allowed in Yugabyte mode %s",
 						__func__)));
 
 	/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
@@ -2327,7 +2327,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	if (IsYBRelation(relation))
 		ereport(ERROR,
 				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("Operation not allowed in YugaByte mode")));
+				 errmsg("operation not allowed in Yugabyte mode")));
 
 	/* currently not needed (thus unsupported) for heap_multi_insert() */
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));

src/postgres/src/backend/access/yb_access/yb_lsm.c

@@ -72,7 +72,7 @@ doBindsForIdxWrite(YBCPgStatement stmt,
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("Missing base table ybctid in index write request")));
+				 errmsg("missing base table ybctid in index write request")));
 	}
 
 	bool has_null_attr = false;
@@ -150,7 +150,7 @@ doAssignForIdxUpdate(YBCPgStatement stmt,
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("Missing base table ybctid in index write request")));
+				 errmsg("missing base table ybctid in index write request")));
 	}
 
 	bool has_null_attr = false;

src/postgres/src/backend/catalog/aclchk.c

@@ -5010,7 +5010,7 @@ pg_tablegroup_aclmask(Oid grp_oid, Oid roleid,
 	if (!YbTablegroupCatalogExists) {
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("Tablegroup system catalog does not exist.")));
+				 errmsg("tablegroup system catalog does not exist")));
 	}
 
 	/* Superusers bypass all permission checking. */
@@ -5838,7 +5838,7 @@ pg_tablegroup_ownercheck(Oid grp_oid, Oid roleid)
 	if (!YbTablegroupCatalogExists) {
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("Tablegroup system catalog does not exist.")));
+				 errmsg("tablegroup system catalog does not exist")));
 	}
 
 	/* Superusers and yb_db_admin role bypass all permission checking. */

src/postgres/src/backend/catalog/pg_shdepend.c

@@ -456,7 +456,7 @@ recordDependencyOnTablespace(Oid classId, Oid objectId, Oid tablespace)
 		if (tablespace == InvalidOid)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("Tablespace dependencies cannot be recorded on InvalidOid")));
+					 errmsg("tablespace dependencies cannot be recorded on InvalidOid")));
 
 		/*
 		 * Since the pg_default and pg_global tablespaces cannot be dropped,

src/postgres/src/backend/catalog/yb_catalog/yb_type.c

@@ -125,7 +125,7 @@ YbDataTypeFromOidMod(int attnum, Oid type_id)
 			default:
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("System column not yet supported in YugaByte: %d", attnum)));
+						 errmsg("system column not yet supported in Yugabyte: %d", attnum)));
 				break;
 		}
 	}
@@ -259,7 +259,7 @@ Datum YbBinaryToDatum(const void *data, int64 bytes, const YBCPgTypeAttrs *type_
 	/* PostgreSQL can represent text strings up to 1 GB minus a four-byte header. */
 	if (bytes > kYBCMaxPostgresTextSizeBytes || bytes < 0) {
 		ereport(ERROR, (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-						errmsg("Invalid data size")));
+						errmsg("invalid data size")));
 	}
 	return PointerGetDatum(cstring_to_text_with_len(data, bytes));
 }
@@ -313,7 +313,7 @@ Datum YbBPCharToDatum(const char *data, int64 bytes, const YBCPgTypeAttrs *type_
 	/* PostgreSQL can represent text strings up to 1 GB minus a four-byte header. */
 	if (bytes > kYBCMaxPostgresTextSizeBytes || bytes < 0) {
 		ereport(ERROR, (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-						errmsg("Invalid data size")));
+						errmsg("invalid data size")));
 	}
 
 	LOCAL_FCINFO(fcinfo, 3);
@@ -333,7 +333,7 @@ Datum YbVarcharToDatum(const char *data, int64 bytes, const YBCPgTypeAttrs *type
 	/* PostgreSQL can represent text strings up to 1 GB minus a four-byte header. */
 	if (bytes > kYBCMaxPostgresTextSizeBytes || bytes < 0) {
 		ereport(ERROR, (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-						errmsg("Invalid data size")));
+						errmsg("invalid data size")));
 	}
 
 	LOCAL_FCINFO(fcinfo, 3);
@@ -356,7 +356,7 @@ Datum YbNameToDatum(const char *data, int64 bytes, const YBCPgTypeAttrs *type_at
 	/* PostgreSQL can represent text strings up to 1 GB minus a four-byte header. */
 	if (bytes > kYBCMaxPostgresTextSizeBytes || bytes < 0) {
 		ereport(ERROR, (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-						errmsg("Invalid data size")));
+						errmsg("invalid data size")));
 	}
 
 	/* Truncate oversize input */
@@ -382,7 +382,7 @@ Datum YbCStrToDatum(const char *data, int64 bytes, const YBCPgTypeAttrs *type_at
 	/* PostgreSQL can represent text strings up to 1 GB minus a four-byte header. */
 	if (bytes > kYBCMaxPostgresTextSizeBytes || bytes < 0) {
 		ereport(ERROR, (errcode(ERRCODE_STRING_DATA_RIGHT_TRUNCATION),
-						errmsg("Invalid data size")));
+						errmsg("invalid data size")));
 	}
 
 	/*
@@ -576,7 +576,7 @@ Datum YbUuidToDatum(const unsigned char *data, int64 bytes, const YBCPgTypeAttrs
 	pg_uuid_t *uuid;
 	if (bytes != UUID_LEN) {
 		ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED),
-						errmsg("Unexpected size for UUID (%ld)", bytes)));
+						errmsg("unexpected size for UUID (%ld)", bytes)));
 	}
 
 	uuid = (pg_uuid_t *)palloc(sizeof(pg_uuid_t));
@@ -623,7 +623,7 @@ Datum YbIntervalToDatum(const void *data, int64 bytes, const YBCPgTypeAttrs *typ
 	const size_t sz = sizeof(Interval);
 	if (bytes != sz) {
 		ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED),
-						errmsg("Unexpected size for Interval (%ld)", bytes)));
+						errmsg("unexpected size for Interval (%ld)", bytes)));
 	}
 	Interval* result = palloc(sz);
 	memcpy(result, data, sz);

src/postgres/src/backend/commands/copyfrom.c

@@ -749,25 +749,25 @@ CopyFrom(CopyFromState cstate)
 					resultRelInfo->ri_RelationDesc->rd_rel->relkind == RELKIND_FOREIGN_TABLE);
 			ereport(WARNING,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("Batched COPY is not supported on %s tables. "
-							"Defaulting to using one transaction for the entire copy.",
+					 errmsg("batched COPY is not supported on %s tables",
 							YbIsTempRelation(resultRelInfo->ri_RelationDesc) ? "temporary" : "foreign"),
+					 errdetail("Defaulting to using one transaction for the entire copy."),
 					 errhint("Either copy onto non-temporary table or set rows_per_transaction "
 							 "option to `0` to disable batching and remove this warning.")));
 		}
 		else if (YBIsDataSent())
 			ereport(WARNING,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("Batched COPY is not supported in transaction blocks. "
-						"Defaulting to using one transaction for the entire copy."),
+				 errmsg("batched COPY is not supported in transaction blocks"),
+				 errdetail("Defaulting to using one transaction for the entire copy."),
 				 errhint("Either run this COPY outside of a transaction block or set "
 						 "rows_per_transaction option to `0` to disable batching and "
 						 "remove this warning.")));
 		else if (HasNonRITrigger(cstate->rel->trigdesc))
 			ereport(WARNING,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("Batched COPY is not supported on table with non RI trigger. "
-						"Defaulting to using one transaction for the entire copy."),
+				 errmsg("batched COPY is not supported on table with non RI trigger"),
+				 errdetail("Defaulting to using one transaction for the entire copy."),
 				 errhint("Set rows_per_transaction option to `0` to disable batching "
 						 "and remove this warning.")));
 		else

src/postgres/src/backend/commands/dbcommands.c

@@ -1027,49 +1027,49 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 			if (option != NULL && option->arg != NULL)
 				ereport(YBUnsupportedFeatureSignalLevel(),
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("Value other than default for %s option is "
+						 errmsg("value other than default for %s option is "
 								"not yet supported", option->defname),
 						 errhint("Please report the issue on "
 								 "https://github.com/YugaByte/yugabyte-db"
-								 "/issues"),
+								 "/issues."),
 						 parser_errposition(pstate, option->location)));
 		}
 
 		if (dbistemplate)
 			ereport(YBUnsupportedFeatureSignalLevel(),
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("Value other than default or false for "
+					 errmsg("value other than default or false for "
 							"is_template option is not yet supported"),
 					 errhint("Please report the issue on "
-							 "https://github.com/YugaByte/yugabyte-db/issues"),
+							 "https://github.com/YugaByte/yugabyte-db/issues."),
 					 parser_errposition(pstate, distemplate->location)));
 
 		if (encoding >= 0 && encoding != PG_UTF8)
 			ereport(YBUnsupportedFeatureSignalLevel(),
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("Value other than unicode or utf8 for encoding "
+					 errmsg("value other than unicode or utf8 for encoding "
 							"option is not yet supported"),
 					 errhint("Please report the issue on "
-							 "https://github.com/yugabyte/yugabyte-db/issues"),
+							 "https://github.com/yugabyte/yugabyte-db/issues."),
 					 parser_errposition(pstate, dencoding->location)));
 
 		if (!(YBIsCollationEnabled() && kTestOnlyUseOSDefaultCollation) && dcollate &&
 			dbcollate && strcmp(dbcollate, "C") != 0)
 			ereport(YBUnsupportedFeatureSignalLevel(),
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("Value other than 'C' for lc_collate "
+					 errmsg("value other than 'C' for lc_collate "
 							"option is not yet supported"),
 					 errhint("Please report the issue on "
-							 "https://github.com/YugaByte/yugabyte-db/issues"),
+							 "https://github.com/YugaByte/yugabyte-db/issues."),
 					 parser_errposition(pstate, dcollate->location)));
 
 		if (dctype && dbctype && strcmp(dbctype, "en_US.UTF-8") != 0)
 			ereport(YBUnsupportedFeatureSignalLevel(),
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("Value other than 'en_US.UTF-8' for lc_ctype "
+					 errmsg("value other than 'en_US.UTF-8' for lc_ctype "
 							"option is not yet supported"),
 					 errhint("Please report the issue on "
-							 "https://github.com/YugaByte/yugabyte-db/issues"),
+							 "https://github.com/YugaByte/yugabyte-db/issues."),
 					 parser_errposition(pstate, dctype->location)));
 	}
 
@@ -1392,11 +1392,11 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		if (!yb_clone_info.src_owner)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_OBJECT),
-					 errmsg("Could not get source database owner name from oid")));
+					 errmsg("could not get source database owner name from oid")));
 		if (!yb_clone_info.tgt_owner)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_OBJECT),
-					 errmsg("Could not get target database owner name from oid")));
+					 errmsg("could not get target database owner name from oid")));
 	}
 
 	/*
@@ -2557,11 +2557,11 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel)
 			if (option != NULL && option->arg != NULL)
 				ereport(YBUnsupportedFeatureSignalLevel(),
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("Altering %s option is not yet supported",
+						 errmsg("altering %s option is not yet supported",
 								option->defname),
 						 errhint("Please report the issue on "
 								 "https://github.com/YugaByte/yugabyte-db"
-								 "/issues"),
+								 "/issues."),
 						 parser_errposition(pstate, option->location)));
 		}
 	}

src/postgres/src/backend/commands/indexcmds.c

@@ -964,9 +964,9 @@ DefineIndex(Oid relationId,
 			if (stmtTablespace != tablespaceId)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-						errmsg("Tablespace for a primary key index must "
+						errmsg("tablespace for a primary key index must "
 							   " always match the tablespace of the "
-							   " indexed table.")));
+							   " indexed table")));
 		}
 	}
 	else if (stmt->tableSpace)
@@ -3278,7 +3278,7 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 	if (IsYugaByteEnabled() && (concurrently || tablespacename))
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("Only REINDEX with VERBOSE option is supported")));
+				 errmsg("only REINDEX with VERBOSE option is supported")));
 
 	if (concurrently)
 		PreventInTransactionBlock(isTopLevel,

src/postgres/src/backend/commands/publicationcmds.c

@@ -799,9 +799,9 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 								 pubactions.pubtruncate))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("Publishing only a subset of DML commands is not yet supported"),
+				 errmsg("publishing only a subset of DML commands is not yet supported"),
 				 errhint("See https://github.com/yugabyte/yugabyte-db/issues/"
-						 "19250. React with thumbs up to raise its priority")));
+						 "19250. React with thumbs up to raise its priority.")));
 
 	puboid = GetNewOidWithIndex(rel, PublicationObjectIndexId,
 								Anum_pg_publication_oid);
@@ -1005,9 +1005,9 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 								 pubactions.pubtruncate))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("Publishing only a subset of DML commands is not yet supported"),
+				 errmsg("publishing only a subset of DML commands is not yet supported"),
 				 errhint("See https://github.com/yugabyte/yugabyte-db/issues/"
-						 "19250. React with thumbs up to raise its priority")));
+						 "19250. React with thumbs up to raise its priority.")));
 
 	/* Everything ok, form a new tuple. */
 	memset(values, 0, sizeof(values));

src/postgres/src/backend/commands/sequence.c

@@ -736,7 +736,7 @@ YBReadSequenceTuple(Relation seqrel)
 	else
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("Should never reach here")));
+				 errmsg("should never reach here")));
 
 	/* Set up the tuple */
 	Datum value[SEQ_COL_LASTCOL];
@@ -1967,7 +1967,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
 	if (cache_value != NULL && cacheOptionOrLastCache < cacheFlag)
 		ereport(NOTICE,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("Overriding cache option with cache flag or previous cache value."),
+				 errmsg("overriding cache option with cache flag or previous cache value"),
 				 errhint("Cache option cannot be set lower than "
 						 "cache flag or previous cache value.")));