Update increment logic, do not prefetch (yugabyte#9)

* remove unnecessary comment, only prefetch when necessary

* cleanup relcache comment, do not count empty password

* do not inc past cap

* do not close unopened heap

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

@@ -280,9 +280,12 @@ public void testLogin() throws Exception {
     }
     assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 1, false);
 
-    /* Now even the correct password will not let us in */
+    /*
+     * Now even the correct password will not let us in.
+     * Failed attempts above the limit + 1 are not counted.
+     */
     attemptLogin(USERNAME, PASSWORD);
     attemptLogin(USERNAME, "wrong");
-    assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 3, false);
+    assertProfileStateForUser(USERNAME, PRF_1_FAILED_ATTEMPTS + 1, false);
   }
 }

src/postgres/src/backend/commands/ybc_profile.c

@@ -152,7 +152,7 @@ get_profile_oid(const char *prfname, bool missing_ok)
 	heap_close(rel, AccessShareLock);
 
 	if (!OidIsValid(result) && !missing_ok)
-		ereport(ERROR, 
+		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_OBJECT),
 				 errmsg("profile \"%s\" does not exist", prfname)));
 
@@ -252,7 +252,7 @@ RemoveProfileById(Oid prfid)
 	if (!HeapTupleIsValid(tuple))
 	{
 		ereport(ERROR,
-				(errcode(ERRCODE_UNDEFINED_OBJECT), 
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
 				 errmsg("profile with oid %u does not exist", prfid)));
 	}
 
@@ -373,12 +373,6 @@ get_role_oid_from_role_profile(Oid roleprfid)
 	return roleid;
 }
 
-/*
- * This function does not check that the profile tables exist. It is either
- * called before the database is initalized, or as a helper for another
- * function that should do this verification. In either case, it is up to the
- * caller to verify that this function can do the right thing.
- */
 HeapTuple
 get_role_profile_tuple(Oid roleid)
 {
@@ -411,7 +405,7 @@ get_role_profile_tuple(Oid roleid)
 
 
 /*
- * get_role_profile_oid - given a role oid, return the oid of the row in 
+ * get_role_profile_oid - given a role oid, return the oid of the row in
  * pg_yb_role_profile for that role.
  *
  * If missing_ok is false, throw an error if role profile is not found.
@@ -501,7 +495,6 @@ update_role_profile(Oid roleid, const char *rolename, Datum *new_record,
  * rolename: Name of the role. Required for error messages
  * prfname: Name of the profile.
  */
-
 void
 CreateRoleProfile(Oid roleid, const char *rolename, const char *prfname)
 {
@@ -578,7 +571,7 @@ CreateRoleProfile(Oid roleid, const char *rolename, const char *prfname)
  *
  * roleid - the oid of the role
  * rolename - Name of the role. Used in the error message
- * isEenabled - bool value
+ * isEnabled - bool value
  */
 void
 EnableRoleProfile(Oid roleid, const char *rolename, bool is_enabled)
@@ -690,9 +683,6 @@ IncFailedAttemptsAndMaybeDisableProfile(Oid roleid, const char *rolename)
 /*
  * YBCIncFailedAttemptsAndMaybeDisableProfile - increment failed_attempts
  * counter and disable if it exceeds limit
- * This function does not check that the table exists. Since it is called
- * before the database is initialized, it expects its caller to verify that
- * the profile tables exist.
  *
  * roleid - the oid of the role
  */
@@ -703,8 +693,9 @@ YBCIncFailedAttemptsAndMaybeDisableProfile(Oid roleid)
 	Form_pg_yb_role_profile rolprfform;
 	Form_pg_yb_profile 		prfform;
 	HeapTuple 				prftuple;
-	int 					failed_attempts;
 	int 					failed_attempts_limit;
+	int						current_failed_attempts;
+	int 					new_failed_attempts;
 	bool 					rolisenabled;
 
 	rolprftuple = get_role_profile_tuple(roleid);
@@ -723,14 +714,18 @@ YBCIncFailedAttemptsAndMaybeDisableProfile(Oid roleid)
 
 	prfform = (Form_pg_yb_profile) GETSTRUCT(prftuple);
 
-	failed_attempts = DatumGetInt16(rolprfform->rolfailedloginattempts) + 1;
+	current_failed_attempts = DatumGetInt16(rolprfform->rolfailedloginattempts);
 	failed_attempts_limit = DatumGetInt16(prfform->prffailedloginattempts);
 
+	new_failed_attempts = current_failed_attempts < failed_attempts_limit
+						? current_failed_attempts + 1
+						: failed_attempts_limit + 1;
+
 	// Keep role enabled IFF role is enabled AND failed attempts < limit
 	rolisenabled = rolprfform->rolisenabled &&
-						(failed_attempts <= failed_attempts_limit);
+						(new_failed_attempts <= failed_attempts_limit);
 
-	YBCExecuteUpdateLoginAttempts(roleid, failed_attempts, rolisenabled);
+	YBCExecuteUpdateLoginAttempts(roleid, new_failed_attempts, rolisenabled);
 	CommitTransactionCommand();
 }
 

src/postgres/src/backend/libpq/auth.c

@@ -654,8 +654,8 @@ ClientAuthentication(Port *port)
 					profileisdisabled = true;
 				}
 			}
+			ReleaseSysCache(roleTup);
 		}
-		ReleaseSysCache(roleTup);
 
 		if (status == STATUS_OK && !profileisdisabled)
 		{
@@ -667,7 +667,8 @@ ClientAuthentication(Port *port)
 		}
 		else
 		{
-			if (roleid != InvalidOid)
+			/* Do not increment login attempts if no password was supplied */
+			if (roleid != InvalidOid && status != STATUS_EOF)
 			{
 				YBCIncFailedAttemptsAndMaybeDisableProfile(roleid);
 			}

src/postgres/src/backend/utils/cache/relcache.c

@@ -1942,7 +1942,7 @@ YBPreloadRelCache()
 	YbRegisterSysTableForPrefetching(NamespaceRelationId);             // pg_namespace
 	YbRegisterSysTableForPrefetching(AuthIdRelationId);                // pg_authid
 
-	if (YbLoginProfileCatalogsExist)
+	if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist)
 	{
 		YbRegisterSysTableForPrefetching(YbProfileRelationId);         // yb_pg_profile
 		YbRegisterSysTableForPrefetching(YbRoleProfileRelationId);     // yb_pg_role_profile
@@ -4133,8 +4133,8 @@ RelationBuildLocalRelation(const char *relname,
 	 * to the set of shared relations.
 	 */
 	if (shared_relation != IsSharedRelation(relid) && !yb_test_system_catalogs_creation)
-		elog(ERROR, "shared_relation flag for \"%s\" (%s) does not match IsSharedRelation(%u) (%s)",
-			 relname, shared_relation ? "true" : "false", relid, IsSharedRelation(relid) ? "true" : "false");
+		elog(ERROR, "shared_relation flag for \"%s\" does not match IsSharedRelation(%u)",
+			 relname, relid);
 
 	/* (Non-YB) shared relations had better be mapped, too */
 	Assert(IsYugaByteEnabled() ||
@@ -4519,15 +4519,15 @@ RelationCacheInitializePhase2(void)
 				  false, Natts_pg_shseclabel, Desc_pg_shseclabel);
 		formrdesc("pg_subscription", SubscriptionRelation_Rowtype_Id, true,
 				  true, Natts_pg_subscription, Desc_pg_subscription);
-		if (YbLoginProfileCatalogsExist)
+		if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist)
 		{
 			formrdesc("pg_yb_profile", YbProfileRelation_Rowtype_Id, true,
 					true, Natts_pg_yb_profile, Desc_pg_yb_profile);
 			formrdesc("pg_yb_role_profile", YbRoleProfileRelation_Rowtype_Id, true,
 					true, Natts_pg_yb_role_profile, Desc_pg_yb_role_profile);
 		}
 
-#define NUM_CRITICAL_SHARED_RELS    (YbLoginProfileCatalogsExist ? 7 : 5)   /* fix if you change list above */
+#define NUM_CRITICAL_SHARED_RELS    (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist ? 7 : 5)   /* fix if you change list above */
 	}
 
 	MemoryContextSwitchTo(oldcxt);

src/postgres/src/backend/utils/init/postinit.c

@@ -704,7 +704,7 @@ InitPostgresImpl(const char *in_dbname, Oid dboid, const char *username,
 		YbRegisterSysTableForPrefetching(
 				AuthMemRelationId);       // pg_auth_members
 
-		if (YbLoginProfileCatalogsExist)
+		if (*YBCGetGFlags()->ysql_enable_profile && YbLoginProfileCatalogsExist)
 		{
 			YbRegisterSysTableForPrefetching(
 					YbProfileRelationId); // pg_yb_profile