You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2026/03/17 15:37:45 UTC
(superset) 01/01: feat: prevent Postgres connection to Redshift
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch redshift-check
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e12df43345a666b468168e8ec4c0370b0f8f5759
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Mar 17 11:21:30 2026 -0400
feat: prevent Postgres connection to Redshift
---
superset/db_engine_specs/postgres.py | 81 ++++++++++++++++
tests/unit_tests/db_engine_specs/test_postgres.py | 107 +++++++++++++++++++++-
2 files changed, 187 insertions(+), 1 deletion(-)
diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py
index b744b7b440b..1b89e7e59bb 100644
--- a/superset/db_engine_specs/postgres.py
+++ b/superset/db_engine_specs/postgres.py
@@ -81,6 +81,47 @@ COLUMN_DOES_NOT_EXIST_REGEX = re.compile(
SYNTAX_ERROR_REGEX = re.compile('syntax error at or near "(?P<syntax_error>.*?)"')
+REDSHIFT_HOST_SUFFIXES = (
+ ".redshift.amazonaws.com",
+ ".redshift-serverless.amazonaws.com",
+)
+
+
+def _check_not_redshift(dbapi_connection: Any, connection_record: Any) -> None:
+ """
+ SQLAlchemy pool ``connect`` event that checks whether the database is actually
+ Amazon Redshift by running ``SELECT version()``.
+
+ Redshift returns a version string containing ``Redshift``, e.g.::
+
+ PostgreSQL 8.0.2 on ... Redshift 1.0.77467
+
+ If detected, a ``ValueError`` is raised so that the user is prompted to
+ switch to the ``redshift+psycopg2://`` driver, which ensures the correct
+ sqlglot dialect is used for SQL transpilation.
+ """
+ try:
+ cursor = dbapi_connection.cursor()
+ try:
+ cursor.execute("SELECT version()")
+ version = cursor.fetchone()[0]
+ finally:
+ cursor.close()
+
+ if "Redshift" in version:
+ raise ValueError(
+ "It looks like you're connecting to Amazon Redshift using the "
+ "PostgreSQL driver. Please use the Redshift driver instead "
+ "(redshift+psycopg2://) to ensure proper SQL dialect support."
+ )
+ except ValueError:
+ raise
+ except Exception:
+ logger.debug(
+ "Failed to check database version for Redshift detection",
+ exc_info=True,
+ )
+
def parse_options(connect_args: dict[str, Any]) -> dict[str, str]:
"""
@@ -570,6 +611,39 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
return uri, connect_args
+ @classmethod
+ def validate_database_uri(cls, sqlalchemy_uri: URL) -> None:
+ """
+ Reject ``postgresql://`` URIs that point at Amazon Redshift.
+
+ When a user connects to Redshift via the PostgreSQL dialect, basic
+ queries work but Superset picks the wrong sqlglot dialect for SQL
+ transpilation, which leads to errors.
+
+ This is a fast, hostname-only check that catches standard AWS
+ endpoints. Custom domains and private endpoints are caught
+ separately by the ``SELECT version()`` check that runs during
+ ``test_connection`` (see ``_check_not_redshift``).
+ """
+ host = sqlalchemy_uri.host or ""
+ if any(host.endswith(suffix) for suffix in REDSHIFT_HOST_SUFFIXES):
+ raise ValueError(
+ "It looks like you're connecting to Amazon Redshift using the "
+ "PostgreSQL driver. Please use the Redshift driver instead "
+ "(redshift+psycopg2://) to ensure proper SQL dialect support."
+ )
+
+ super().validate_database_uri(sqlalchemy_uri)
+
+ @staticmethod
+ def mutate_db_for_connection_test(database: Database) -> None:
+ """
+ Flag the database so that the Redshift ``SELECT version()`` check
+ runs during ``test_connection``. The actual check is injected as a
+ pool ``connect`` event inside ``update_params_from_encrypted_extra``.
+ """
+ database._check_redshift_version = True
+
@staticmethod
def update_params_from_encrypted_extra(
database: Database,
@@ -581,6 +655,13 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
Handles AWS IAM authentication if configured, then merges any
remaining encrypted_extra keys into params (standard behavior).
"""
+ # During test_connection, inject a pool event to detect Redshift by
+ # checking SELECT version(). This catches cases where the hostname
+ # doesn't reveal Redshift (custom domains, private endpoints, etc.).
+ if getattr(database, "_check_redshift_version", False):
+ pool_events = params.setdefault("pool_events", [])
+ pool_events.append((_check_not_redshift, "connect"))
+
if not database.encrypted_extra:
return
diff --git a/tests/unit_tests/db_engine_specs/test_postgres.py b/tests/unit_tests/db_engine_specs/test_postgres.py
index ff11dd9aa42..dad6b233055 100644
--- a/tests/unit_tests/db_engine_specs/test_postgres.py
+++ b/tests/unit_tests/db_engine_specs/test_postgres.py
@@ -17,6 +17,7 @@
from datetime import datetime
from typing import Any, Optional
+from unittest.mock import MagicMock
import pytest
from pytest_mock import MockerFixture
@@ -25,7 +26,10 @@ from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, JSON
from sqlalchemy.engine.interfaces import Dialect
from sqlalchemy.engine.url import make_url
-from superset.db_engine_specs.postgres import PostgresEngineSpec as spec # noqa: N813
+from superset.db_engine_specs.postgres import (
+ _check_not_redshift,
+ PostgresEngineSpec as spec, # noqa: N813
+)
from superset.exceptions import SupersetSecurityException
from superset.sql.parse import Table
from superset.utils.core import GenericDataType
@@ -280,3 +284,104 @@ SELECT * \nFROM my_schema.my_table
LIMIT :param_1
""".strip()
)
+
+
+class TestRedshiftDetection:
+ """Tests for detecting Redshift connections via the PostgreSQL dialect."""
+
+ def test_validate_database_uri_redshift_host(self) -> None:
+ """Standard Redshift endpoint is rejected."""
+ uri = make_url(
+ "postgresql://user:pass@my-cluster.abc123.us-east-1"
+ ".redshift.amazonaws.com:5439/dev"
+ )
+ with pytest.raises(ValueError, match="Redshift"):
+ spec.validate_database_uri(uri)
+
+ def test_validate_database_uri_redshift_serverless_host(self) -> None:
+ """Redshift Serverless endpoint is rejected."""
+ uri = make_url(
+ "postgresql://user:pass@my-wg.abc123.us-west-2"
+ ".redshift-serverless.amazonaws.com:5439/dev"
+ )
+ with pytest.raises(ValueError, match="Redshift"):
+ spec.validate_database_uri(uri)
+
+ def test_validate_database_uri_regular_postgres(self) -> None:
+ """Regular PostgreSQL host is allowed."""
+ uri = make_url("postgresql://user:pass@pg.example.com:5432/mydb")
+ spec.validate_database_uri(uri) # should not raise
+
+ def test_validate_database_uri_rds_postgres(self) -> None:
+ """RDS PostgreSQL host is allowed."""
+ uri = make_url(
+ "postgresql://user:pass@my-instance.abc123.us-east-1"
+ ".rds.amazonaws.com:5432/mydb"
+ )
+ spec.validate_database_uri(uri) # should not raise
+
+ def test_validate_database_uri_localhost(self) -> None:
+ """Localhost is allowed."""
+ uri = make_url("postgresql://user:pass@localhost:5432/mydb")
+ spec.validate_database_uri(uri) # should not raise
+
+ def test_check_not_redshift_detects_redshift(self) -> None:
+ """Pool connect event raises for a Redshift version string."""
+ cursor = MagicMock()
+ cursor.fetchone.return_value = (
+ "PostgreSQL 8.0.2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) "
+ "3.4.2 20041017 (Red Hat 3.4.2-6.fc3), Redshift 1.0.77467",
+ )
+ dbapi_conn = MagicMock()
+ dbapi_conn.cursor.return_value = cursor
+
+ with pytest.raises(ValueError, match="Redshift"):
+ _check_not_redshift(dbapi_conn, None)
+
+ def test_check_not_redshift_allows_postgres(self) -> None:
+ """Pool connect event allows a regular PostgreSQL version string."""
+ cursor = MagicMock()
+ cursor.fetchone.return_value = (
+ "PostgreSQL 15.2 on x86_64-pc-linux-gnu, compiled by gcc",
+ )
+ dbapi_conn = MagicMock()
+ dbapi_conn.cursor.return_value = cursor
+
+ _check_not_redshift(dbapi_conn, None) # should not raise
+
+ def test_check_not_redshift_fails_open(self) -> None:
+ """If SELECT version() errors, the connection is still allowed."""
+ cursor = MagicMock()
+ cursor.execute.side_effect = Exception("permission denied")
+ dbapi_conn = MagicMock()
+ dbapi_conn.cursor.return_value = cursor
+
+ _check_not_redshift(dbapi_conn, None) # should not raise
+
+ def test_mutate_db_sets_flag(self) -> None:
+ """mutate_db_for_connection_test sets the check flag."""
+ database = MagicMock()
+ spec.mutate_db_for_connection_test(database)
+ assert database._check_redshift_version is True
+
+ def test_pool_event_injected_when_flag_set(self, mocker: MockerFixture) -> None:
+ """Pool event is added during test_connection."""
+ database = mocker.MagicMock(
+ encrypted_extra=None,
+ _check_redshift_version=True,
+ )
+ params: dict[str, Any] = {}
+ spec.update_params_from_encrypted_extra(database, params)
+
+ assert "pool_events" in params
+ fns = [fn for fn, _ in params["pool_events"]]
+ assert _check_not_redshift in fns
+
+ def test_pool_event_not_injected_without_flag(self, mocker: MockerFixture) -> None:
+ """Pool event is NOT added during normal operation."""
+ database = mocker.MagicMock(encrypted_extra=None)
+ database._check_redshift_version = False
+ params: dict[str, Any] = {}
+ spec.update_params_from_encrypted_extra(database, params)
+
+ assert "pool_events" not in params