From 1d096df7f4da722330ee41e1918f1374781267b9 Mon Sep 17 00:00:00 2001
From: Onur Tirtir <onurcantirtir@gmail.com>
Date: Wed, 24 Jan 2024 15:58:55 +0300
Subject: [PATCH] Not use hardcoded LOCAL_HOST_NAME but citus.local_hostname to
 distinguish loopback connections (#7436)

Fixes a bug that breaks queries from non-maindbs when
citus.local_hostname is set to a value different than "localhost".

This is a very old bug doesn't cause a problem as long as Citus catalog
is available to FindWorkerNode(). And the catalog is always available
unless we're in non-main database, which might be the case on main but
not on older releases, hence not adding a `DESCRIPTION`. For this
reason, I don't see a reason to backport this.

Maybe we should totally refrain using LOCAL_HOST_NAME in all code-paths,
but not doing that in this PR as the other paths don't seem to be
breaking something that is user-facing.

```c
char *
GetAuthinfo(char *hostname, int32 port, char *user)
{
	char *authinfo = NULL;
	bool isLoopback = (strncmp(LOCAL_HOST_NAME, hostname, MAX_NODE_LENGTH) == 0 &&
					   PostPortNumber == port);

	if (IsTransactionState())
	{
		int64 nodeId = WILDCARD_NODE_ID;

		/* -1 is a special value for loopback connections (task tracker) */
		if (isLoopback)
		{
			nodeId = LOCALHOST_NODE_ID;
		}
		else
		{
			WorkerNode *worker = FindWorkerNode(hostname, port);
			if (worker != NULL)
			{
				nodeId = worker->nodeId;
			}
		}

		authinfo = GetAuthinfoViaCatalog(user, nodeId);
	}

	return (authinfo != NULL) ? authinfo : "";
}
```
---
 .../connection/connection_configuration.c        | 16 +++++++++++++++-
 .../distributed/metadata/metadata_cache.c        |  8 --------
 src/test/regress/expected/other_databases.out    |  6 ++++++
 src/test/regress/sql/other_databases.sql         |  7 +++++++
 4 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/backend/distributed/connection/connection_configuration.c b/src/backend/distributed/connection/connection_configuration.c
index c52254f9c..e07692517 100644
--- a/src/backend/distributed/connection/connection_configuration.c
+++ b/src/backend/distributed/connection/connection_configuration.c
@@ -521,9 +521,23 @@ char *
 GetAuthinfo(char *hostname, int32 port, char *user)
 {
 	char *authinfo = NULL;
-	bool isLoopback = (strncmp(LOCAL_HOST_NAME, hostname, MAX_NODE_LENGTH) == 0 &&
+	bool isLoopback = (strncmp(LocalHostName, hostname, MAX_NODE_LENGTH) == 0 &&
 					   PostPortNumber == port);
 
+	/*
+	 * Citus will not be loaded when we run a global DDL command from a
+	 * Citus non-main database.
+	 */
+	if (!CitusHasBeenLoaded())
+	{
+		/*
+		 * We don't expect non-main databases to connect to a node other than
+		 * the local one.
+		 */
+		Assert(isLoopback);
+		return "";
+	}
+
 	if (IsTransactionState())
 	{
 		int64 nodeId = WILDCARD_NODE_ID;
diff --git a/src/backend/distributed/metadata/metadata_cache.c b/src/backend/distributed/metadata/metadata_cache.c
index 555365e68..402dedb8a 100644
--- a/src/backend/distributed/metadata/metadata_cache.c
+++ b/src/backend/distributed/metadata/metadata_cache.c
@@ -5723,14 +5723,6 @@ GetPoolinfoViaCatalog(int32 nodeId)
 char *
 GetAuthinfoViaCatalog(const char *roleName, int64 nodeId)
 {
-	/*
-	 * Citus will not be loaded when we run a global DDL command from a
-	 * Citus non-main database.
-	 */
-	if (!CitusHasBeenLoaded())
-	{
-		return "";
-	}
 	char *authinfo = "";
 	Datum nodeIdDatumArray[2] = {
 		Int32GetDatum(nodeId),
diff --git a/src/test/regress/expected/other_databases.out b/src/test/regress/expected/other_databases.out
index 9e170861e..15bff66ed 100644
--- a/src/test/regress/expected/other_databases.out
+++ b/src/test/regress/expected/other_databases.out
@@ -71,9 +71,15 @@ SELECT citus_internal.execute_command_on_remote_nodes_as_user($$SELECT 'dangerou
 ERROR:  operation is not allowed
 HINT:  Run the command with a superuser.
 \c other_db1
+SET citus.local_hostname TO '127.0.0.1';
 SET ROLE nonsuperuser;
+-- Make sure that we don't try to access pg_dist_node.
+-- Otherwise, we would get the following error:
+--   ERROR:  cache lookup failed for pg_dist_node, called too early?
 CREATE USER other_db_user9;
 RESET ROLE;
+RESET citus.local_hostname;
+RESET ROLE;
 \c regression
 SELECT usename FROM pg_user WHERE usename LIKE 'other\_db\_user%' ORDER BY 1;
     usename
diff --git a/src/test/regress/sql/other_databases.sql b/src/test/regress/sql/other_databases.sql
index 563793518..8cd54f354 100644
--- a/src/test/regress/sql/other_databases.sql
+++ b/src/test/regress/sql/other_databases.sql
@@ -51,9 +51,16 @@ SET ROLE nonsuperuser;
 SELECT citus_internal.execute_command_on_remote_nodes_as_user($$SELECT 'dangerous query'$$, 'postgres');
 
 \c other_db1
+SET citus.local_hostname TO '127.0.0.1';
 SET ROLE nonsuperuser;
+
+-- Make sure that we don't try to access pg_dist_node.
+-- Otherwise, we would get the following error:
+--   ERROR:  cache lookup failed for pg_dist_node, called too early?
 CREATE USER other_db_user9;
 
+RESET ROLE;
+RESET citus.local_hostname;
 RESET ROLE;
 \c regression
 SELECT usename FROM pg_user WHERE usename LIKE 'other\_db\_user%' ORDER BY 1;