node: Remove SELECT command support from cluster client

affonsov · affonsov · commit ed83efdc43aa · 2025-09-03T16:52:14.000-07:00
- Remove select() method from GlideClusterClient
- Remove createSelect import from cluster client
- Remove extensive test coverage for SELECT functionality
- Remove database ID validation tests from client internals
- Add minimal database ID test for cluster client configuration
- Clean up ConfigurationError import that's no longer needed

This change removes the SELECT command implementation that was added
for Valkey 9.0+ cluster support, likely due to reliability concerns
with database switching in cluster mode or to simplify the API.

Signed-off-by: affonsov &lt;67347924+affonsov@users.noreply.github.com&gt;
diff --git a/node/src/GlideClusterClient.ts b/node/src/GlideClusterClient.ts
@@ -60,7 +60,6 @@ import {
     createScriptExists,
     createScriptFlush,
     createScriptKill,
-    createSelect,
     createTime,
     createUnWatch,
 } from ".";
@@ -1645,47 +1644,6 @@ export class GlideClusterClient extends BaseClient {
         });
     }
 
-    /**
-     * Changes the currently selected database on cluster nodes.
-     *
-     * This command routes to all nodes by default to maintain consistency across the cluster.
-     *
-     * **WARNING**: This command is NOT RECOMMENDED for production use.
-     * Upon reconnection, nodes will revert to the database_id specified
-     * in the client configuration (default: 0), NOT the database selected
-     * via this command.
-     *
-     * **RECOMMENDED APPROACH**: Use the `databaseId` parameter in client
-     * configuration instead:
-     *
-     * ```typescript
-     * const client = await GlideClusterClient.createClient({
-     *     addresses: [{ host: "localhost", port: 6379 }],
-     *     databaseId: 5  // Recommended: persists across reconnections
-     * });
-     * ```
-     *
-     *
-     * @see {@link https://valkey.io/commands/select/|valkey.io} for details.
-     *
-     * @param index - The index of the database to select.
-     * @returns A simple `"OK"` response.
-     *
-     * @example
-     * ```typescript
-     * // Example usage of select method (NOT RECOMMENDED)
-     * const result = await client.select(2);
-     * console.log(result); // Output: 'OK'
-     * // Note: Database selection will be lost on reconnection!
-     * ```
-     *
-     */
-    public async select(index: number): Promise<"OK"> {
-        return this.createWritePromise(createSelect(index), {
-            decoder: Decoder.String,
-        });
-    }
-
     /**
      * Returns the number of keys in the database.
      *
diff --git a/node/tests/GlideClientInternals.test.ts b/node/tests/GlideClientInternals.test.ts
@@ -14,7 +14,6 @@ import {
     ClosingError,
     ClusterBatch,
     ClusterTransaction,
-    ConfigurationError,
     convertGlideRecordToRecord,
     Decoder,
     GlideClient,
@@ -687,135 +686,6 @@ describe("SocketConnectionInternals", () => {
         closeTestResources(connection, server, socket);
     });
 
-    it("should pass database id with value 0", async () => {
-        const { connection, server, socket } = await getConnectionAndSocket(
-            (request: connection_request.ConnectionRequest) =>
-                request.databaseId === 0,
-            {
-                addresses: [{ host: "foo" }],
-                databaseId: 0,
-            },
-        );
-        closeTestResources(connection, server, socket);
-    });
-
-    it("should accept higher database id values in client-side validation", () => {
-        // Test that client-side validation accepts higher database IDs (100)
-        // Server-side validation will handle actual database limits during connection
-        expect(() => {
-            const config: BaseClientConfiguration = {
-                addresses: [{ host: "foo" }],
-                databaseId: 100,
-            };
-            // Configuration creation should not throw for high database IDs
-            // Only negative or non-integer values should be rejected client-side
-            expect(config.databaseId).toBe(100);
-        }).not.toThrow();
-    });
-
-    it("should accept very high database id values in client-side validation", () => {
-        // Test that client-side validation accepts very high database IDs (999)
-        // Server-side validation will handle actual database limits during connection
-        expect(() => {
-            const config: BaseClientConfiguration = {
-                addresses: [{ host: "foo" }],
-                databaseId: 999,
-            };
-            // Configuration creation should not throw for high database IDs
-            // Only negative or non-integer values should be rejected client-side
-            expect(config.databaseId).toBe(999);
-        }).not.toThrow();
-    });
-
-    it("should reject negative database id", async () => {
-        await expect(async () => {
-            const socket = new net.Socket();
-            await GlideClient.__createClient(
-                {
-                    addresses: [{ host: "foo" }],
-                    databaseId: -1,
-                },
-                socket,
-            );
-        }).rejects.toThrow(ConfigurationError);
-    });
-
-    it("should reject non-integer database id", async () => {
-        await expect(async () => {
-            const socket = new net.Socket();
-            await GlideClient.__createClient(
-                {
-                    addresses: [{ host: "foo" }],
-                    databaseId: 1.5,
-                },
-                socket,
-            );
-        }).rejects.toThrow(ConfigurationError);
-    });
-
-    it("should accept database id 0 explicitly", () => {
-        expect(() => {
-            const config: BaseClientConfiguration = {
-                addresses: [{ host: "foo" }],
-                databaseId: 0,
-            };
-            // Database ID 0 should be explicitly accepted
-            expect(config.databaseId).toBe(0);
-        }).not.toThrow();
-    });
-
-    it("should accept various valid database id values", () => {
-        const validDatabaseIds = [0, 1, 5, 15, 50, 100, 500, 999];
-
-        for (const databaseId of validDatabaseIds) {
-            expect(() => {
-                const config: BaseClientConfiguration = {
-                    addresses: [{ host: "foo" }],
-                    databaseId: databaseId,
-                };
-                // All non-negative integer database IDs should be accepted client-side
-                // Server will validate the actual limits during connection
-                expect(config.databaseId).toBe(databaseId);
-            }).not.toThrow();
-        }
-    });
-
-    it("should handle undefined database id gracefully", () => {
-        expect(() => {
-            const config: BaseClientConfiguration = {
-                addresses: [{ host: "foo" }],
-                // databaseId not specified - should default to undefined
-            };
-            expect(config.databaseId).toBeUndefined();
-        }).not.toThrow();
-    });
-
-    it("should reject NaN database id", async () => {
-        await expect(async () => {
-            const socket = new net.Socket();
-            await GlideClient.__createClient(
-                {
-                    addresses: [{ host: "foo" }],
-                    databaseId: NaN,
-                },
-                socket,
-            );
-        }).rejects.toThrow(ConfigurationError);
-    });
-
-    it("should reject Infinity database id", async () => {
-        await expect(async () => {
-            const socket = new net.Socket();
-            await GlideClient.__createClient(
-                {
-                    addresses: [{ host: "foo" }],
-                    databaseId: Infinity,
-                },
-                socket,
-            );
-        }).rejects.toThrow(ConfigurationError);
-    });
-
     it("should pass periodic checks disabled", async () => {
         const { connection, server, socket } = await getConnectionAndSocket(
             (request: connection_request.ConnectionRequest) =>
diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts
@@ -2868,4 +2868,26 @@ describe("GlideClusterClient", () => {
         },
         TIMEOUT,
     );
+
+    it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
+        "should pass database id for cluster client_%p",
+        async (protocol) => {
+            // Skip test if version is below 9.0.0 (Valkey 9)
+            if (cluster.checkIfServerVersionLessThan("9.0.0")) return;
+
+            const client = await GlideClusterClient.createClient(
+                getClientConfigurationOption(cluster.getAddresses(), protocol, {
+                    databaseId: 1,
+                }),
+            );
+
+            try {
+                // Simple test to verify the client works with the database ID
+                expect(await client.ping()).toEqual("PONG");
+            } finally {
+                client.close();
+            }
+        },
+        TIMEOUT,
+    );
 });
diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts
