Cleanup from rebase and fix style

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Author: sfc-gh-npuka

pg_lake_iceberg/include/pg_lake/http/http_client.h

@@ -50,7 +50,7 @@ extern bool HttpClientTraceTraffic;
 #define HTTP_STATUS_SERVICE_UNAVAILABLE	503
 
 /* Callback function to determine if a request should be retried */
-typedef bool (*HttpRetryFn) (long status, int maxRetry, int retryNo, void *context, List *headers);
+typedef bool (*HttpRetryFn) (long status, int maxRetry, int retryNo);
 
 /* plain C API (no PostgreSQL types) */
 extern PGDLLEXPORT HttpResult HttpGet(const char *url, List *headers);
@@ -60,6 +60,5 @@ extern PGDLLEXPORT HttpResult HttpDelete(const char *url, List *headers);
 extern PGDLLEXPORT HttpResult HttpPut(const char *url, const char *body, List *headers);
 extern PGDLLEXPORT HttpResult SendHttpRequest(HttpMethod method, const char *url, const char *body, List *headers);
 extern PGDLLEXPORT HttpResult SendHttpRequestWithRetry(HttpMethod method, const char *url, const char *body,
-													   List *headers, HttpRetryFn retryFn, int maxRetry,
-													   void *retryContext);
+													   List *headers, HttpRetryFn retryFn, int maxRetry);
 extern PGDLLEXPORT int LinearBackoffSleepMs(int baseMs, int retryNo);

pg_lake_iceberg/include/pg_lake/rest_catalog/rest_catalog.h

@@ -44,9 +44,8 @@ extern bool RestCatalogEnableVendedCredentials;
 typedef struct RestCatalogOptions
 {
 	char	   *catalog;		/* catalog name, used for token cache keying;
-							 	 * can be 'rest' or a user-created server name
-							 	 * of TYPE 'rest'
-								 */
+								 * can be 'rest' or a user-created server name
+								 * of TYPE 'rest' */
 	char	   *host;
 	char	   *oauthHostPath;
 	char	   *clientId;

pg_lake_iceberg/src/http/http_client.c

@@ -276,8 +276,7 @@ CurlReturnError(CURL *curl, struct curl_slist *headerList,
  */
 HttpResult
 SendHttpRequestWithRetry(HttpMethod method, const char *url, const char *body,
-						 List *headers, HttpRetryFn retryFn, int maxRetry,
-						 void *retryContext)
+						 List *headers, HttpRetryFn retryFn, int maxRetry)
 {
 	Assert(maxRetry > 0);
 
@@ -287,7 +286,7 @@ SendHttpRequestWithRetry(HttpMethod method, const char *url, const char *body,
 	{
 		result = SendHttpRequest(method, url, body, headers);
 
-		if (retryFn != NULL && retryFn(result.status, maxRetry, retryNo, retryContext, headers))
+		if (retryFn != NULL && retryFn(result.status, maxRetry, retryNo))
 			continue;
 		else
 			break;

pg_lake_iceberg/src/rest_catalog/rest_catalog.c

@@ -299,8 +299,8 @@ GetRestCatalogOptionsFromCatalog(const char *catalog)
 	RestCatalogOptions *opts = palloc0(sizeof(RestCatalogOptions));
 
 	/*
-	 * Normalize built-in catalog name to the canonical constant so that
-	 * case variations (e.g. 'REST', 'rEst') compare equal with strcmp.
+	 * Normalize built-in catalog name to the canonical constant so that case
+	 * variations (e.g. 'REST', 'rEst') compare equal with strcmp.
 	 * User-created server names are case-sensitive and stored as-is.
 	 */
 	if (pg_strcasecmp(catalog, REST_CATALOG_NAME) == 0)
@@ -319,9 +319,8 @@ GetRestCatalogOptionsFromCatalog(const char *catalog)
 	opts->locationPrefix = GetIcebergDefaultLocationPrefix();
 
 	/*
-	 * The built-in 'rest' name uses GUCs exclusively.
-	 * For user-created servers, look up server options and
-	 * override the GUC defaults.
+	 * The built-in 'rest' name uses GUCs exclusively. For user-created
+	 * servers, look up server options and override the GUC defaults.
 	 */
 	if (pg_strcasecmp(catalog, REST_CATALOG_NAME) != 0)
 	{
@@ -360,8 +359,9 @@ GetRestCatalogOptionsFromCatalog(const char *catalog)
 				opts->catalogName = defGetString(def);
 			else if (pg_strcasecmp(def->defname, "location_prefix") == 0)
 			{
-				bool        inPlace = false;
-            opts->locationPrefix = StripTrailingSlash(defGetString(def), inPlace);
+				bool		inPlace = false;
+
+				opts->locationPrefix = StripTrailingSlash(defGetString(def), inPlace);
 			}
 		}
 	}
@@ -1531,9 +1531,6 @@ ClassifyRestCatalogRequestRetry(long status, int maxRetry, int retryNo)
  * so normally we wouldn't want any errors to happen, but then
  * Postgres already prevents post-commit backends to receive signals.
  *
- * The serverName is used by the retry callback to invalidate only the
- * matching token cache entry on a 419 (token expired) response.
- *
  * When opts is non-NULL the retry callback can force-refresh the
  * access token and patch the Authorization header on a 419 response.
  * Pass opts = NULL for the token-fetch request itself to avoid recursion.
@@ -1544,7 +1541,6 @@ SendRequestToRestCatalog(HttpMethod method, const char *url, const char *body,
 {
 	const int	MAX_HTTP_RETRY_FOR_REST_CATALOG = 3;
 
-<<<<<<< HEAD
 	HttpResult	result;
 
 	for (int retryNo = 1; retryNo <= MAX_HTTP_RETRY_FOR_REST_CATALOG; retryNo++)
@@ -1569,7 +1565,7 @@ SendRequestToRestCatalog(HttpMethod method, const char *url, const char *body,
 					 * new token.
 					 */
 					bool		forceRefreshToken = true;
-					char	   *freshToken = GetRestCatalogAccessToken(forceRefreshToken);
+					char	   *freshToken = GetRestCatalogAccessToken(opts, forceRefreshToken);
 
 					UpdateAuthorizationHeader(headers, freshToken);
 					continue;
@@ -1582,76 +1578,3 @@ SendRequestToRestCatalog(HttpMethod method, const char *url, const char *body,
 
 	return result;
 }
-=======
-	return SendHttpRequestWithRetry(method, url, body, headers,
-									ShouldRetryRequestToRestCatalog,
-									MAX_HTTP_RETRY_FOR_REST_CATALOG,
-									opts);
-}
-
-
-/*
- * ShouldRetryRequestToRestCatalog checks if the given HTTP result status is retriable.
- * If it is retriable, it performs necessary actions (like sleeping or refreshing token)
- * and returns true. Otherwise, it returns false.
- */
-bool
-ShouldRetryRequestToRestCatalog(long status, int maxRetry, int retryNo,
-								void *context, List *headers)
-{
-	if (retryNo > maxRetry)
-		return false;
-
-	const int	TOO_MANY_REQUEST_STATUS = 429;
-	const int	SERVER_UNAVAILABLE_STATUS = 503;
-	const int	TOKEN_EXPIRED_STATUS = 419;
-
-	/* too many request, wait some time */
-	if (status == TOO_MANY_REQUEST_STATUS)
-	{
-		int			baseMs = 500;
-
-		/*
-		 * LightSleep reacts to signals, and can easily throw an error (e.g.,
-		 * cancel backend). This function can be called at post-commit hook,
-		 * so normally we wouldn't want any errors to happen, but then
-		 * Postgres already prevents post-commit backends to receive signals.
-		 */
-		LightSleep(LinearBackoffSleepMs(baseMs, retryNo));
-		return true;
-	}
-
-	/* server unavailable, lets wait a bit more */
-	else if (status == SERVER_UNAVAILABLE_STATUS)
-	{
-		int			baseMs = 5000;
-
-		LightSleep(LinearBackoffSleepMs(baseMs, retryNo));
-		return true;
-	}
-
-	/* token expired, retry after refreshing token */
-	else if (status == TOKEN_EXPIRED_STATUS)
-	{
-		RestCatalogOptions *opts = (RestCatalogOptions *) context;
-
-		if (opts == NULL)
-			return false;
-
-		/*
-		 * We normally refresh the token only when it is about to expire
-		 * (forceRefreshToken = false), just 1 minute before the expiration
-		 * for each request. Retry logic makes it safer by ensuring we get a
-		 * fresh token for unforeseen circumstances.
-		 */
-		bool		forceRefreshToken = true;
-		char	   *newToken = GetRestCatalogAccessToken(opts, forceRefreshToken);
-
-		linitial(headers) = psprintf("Authorization: Bearer %s", newToken);
-		return true;
-	}
-
-	/* successful or other error, no retry */
-	return false;
-}
->>>>>>> 5f24f40 (Address Onder's review)

pg_lake_iceberg/src/test/test_http_client.c

@@ -34,7 +34,7 @@ PG_FUNCTION_INFO_V1(test_http_with_retry);
 
 static Datum build_http_result(FunctionCallInfo fcinfo, const HttpResult * r);
 static List *extract_headers(FunctionCallInfo fcinfo, int argno);
-static bool TestShouldRetryRequestToRestCatalog(long status, int maxRetry, int retryNo, void *context, List *headers);
+static bool TestShouldRetryRequestToRestCatalog(long status, int maxRetry, int retryNo);
 
 
 Datum
@@ -193,8 +193,7 @@ build_http_result(FunctionCallInfo fcinfo, const HttpResult * r)
  * retries until maxRetry is reached.
  */
 static bool
-TestShouldRetryRequestToRestCatalog(long status, int maxRetry, int retryNo,
-									void *context, List *headers)
+TestShouldRetryRequestToRestCatalog(long status, int maxRetry, int retryNo)
 {
 	if (retryNo > maxRetry)
 		return false;

pg_lake_table/src/ddl/create_table.c

@@ -789,9 +789,9 @@ ProcessCreateIcebergTableFromForeignTableStmt(ProcessUtilityParams * params)
 			/*
 			 * Writable tables always derive catalog_name, catalog_namespace,
 			 * and catalog_table_name from the database name, schema name, and
-			 * table name.  Explicit catalog options on the table are rejected,
-			 * and the server must not have catalog_name set either, since that
-			 * would conflict with the derived values.
+			 * table name.  Explicit catalog options on the table are
+			 * rejected, and the server must not have catalog_name set either,
+			 * since that would conflict with the derived values.
 			 */
 			if (catalogNamespaceProvided != NULL ||
 				catalogTableNameProvided != NULL ||

pg_lake_table/src/transaction/track_iceberg_metadata_changes.c

@@ -124,7 +124,7 @@ static MemoryContext PgLakeXactCommitContext = NULL;
  * where syscache lookups are forbidden. Only one REST catalog server is allowed
  * per transaction.
  */
-static RestCatalogOptions *PgLakeXactRestCatalogOpts = NULL;
+static RestCatalogOptions * PgLakeXactRestCatalogOpts = NULL;
 
 
 /*