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:47:20 UTC
(superset) branch redshift-check updated (e12df43345a -> 40376db00ab)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch redshift-check
in repository https://gitbox.apache.org/repos/asf/superset.git
discard e12df43345a feat: prevent Postgres connection to Redshift
new 40376db00ab feat: prevent Postgres connection to Redshift
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (e12df43345a)
\
N -- N -- N refs/heads/redshift-check (40376db00ab)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
superset/db_engine_specs/postgres.py | 29 +++++++-------
tests/unit_tests/db_engine_specs/test_postgres.py | 48 +++++++++++++++++------
2 files changed, 51 insertions(+), 26 deletions(-)
(superset) 01/01: feat: prevent Postgres connection to Redshift
Posted by be...@apache.org.
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 40376db00ab44595add84bc9d4b80e30f9665be7
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 | 82 ++++++++++++++
tests/unit_tests/db_engine_specs/test_postgres.py | 131 +++++++++++++++++++++-
2 files changed, 212 insertions(+), 1 deletion(-)
diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py
index b744b7b440b..3f1903139d9 100644
--- a/superset/db_engine_specs/postgres.py
+++ b/superset/db_engine_specs/postgres.py
@@ -81,6 +81,48 @@ 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:
+ """
+ Event that checks if database is Amazon Redshift.
+
+ 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 str(version).lower():
+ 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 +612,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 +656,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..8d51166c2b4 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,128 @@ 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