codeant-ai-for-open-source[bot] commented on code in PR #42598:
URL: https://github.com/apache/superset/pull/42598#discussion_r3694002748
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -229,6 +229,104 @@ def test_superset_joins(
assert list(results) == [(10, "ten"), (20, "twenty")]
[email protected]
+def table1_large(session: Session, database1: "Database") -> Iterator[None]:
+ with database1.get_sqla_engine() as engine:
+ with engine.begin() as conn:
+ conn.execute(
+ text(
+ "CREATE TABLE table1_large (a INTEGER NOT NULL PRIMARY
KEY, "
+ "b INTEGER)"
+ )
+ )
+ conn.execute(
+ text("INSERT INTO table1_large (a, b) VALUES (1, 10), (2, 20),
(3, 30)")
+ )
+ db.session.commit()
+
+ yield
+
+ with engine.begin() as conn:
+ conn.execute(text("DROP TABLE table1_large"))
+ db.session.commit()
+
+
[email protected]
+def table2_late_match(session: Session, database2: "Database") ->
Iterator[None]:
+ with database2.get_sqla_engine() as engine:
+ with engine.begin() as conn:
+ conn.execute(
+ text(
+ "CREATE TABLE table2_late_match (a INTEGER NOT NULL
PRIMARY KEY, "
+ "b TEXT)"
+ )
+ )
+ conn.execute(
+ text("INSERT INTO table2_late_match (a, b) VALUES (3,
'thirty')")
+ )
+ db.session.commit()
+
+ yield
+
+ with engine.begin() as conn:
+ conn.execute(text("DROP TABLE table2_late_match"))
+ db.session.commit()
+
+
+@with_config(
+ {
+ "DB_SQLA_URI_VALIDATOR": None,
+ "SUPERSET_META_DB_LIMIT": 2,
+ "DATABASE_OAUTH2_CLIENTS": {},
+ "SQLALCHEMY_CUSTOM_PASSWORD_STORE": None,
+ }
+)
+@with_feature_flags(ENABLE_SUPERSET_META_DB=True)
+def test_superset_joins_with_limit_drops_matches(
+ mocker: MockerFixture,
+ app_context: None,
+ table1_large: None,
+ table2_late_match: None,
+) -> None:
+ """
+ Regression for #36304: SUPERSET_META_DB_LIMIT is applied to each
+ underlying table independently, before the in-memory join runs. A row
+ that has a genuine match on the other side of the join but falls past
+ the per-table limit is silently dropped from the join result, with no
+ error or truncation warning.
+ """
+ mocker.patch(
+ "superset.extensions.metadb.security_manager.raise_for_access",
+ return_value=None,
+ )
+
+ from flask import g
+
+ g.user = mocker.MagicMock()
+ g.user.is_anonymous = False
+
+ try:
+ engine = create_engine("superset://", future=True)
+ except Exception as e:
+ # Skip test if superset:// dialect can't be loaded (common in Docker)
+ pytest.skip(f"Superset dialect not available: {e}")
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> In SQLAlchemy extension tests, allow broad exception handling around
engine creation when it follows the file's established skip pattern; do not
require special-casing individual exceptions for consistency.
**Applied to:**
- `tests/unit_tests/extensions/**`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
tests/unit_tests/extensions/test_sqlalchemy.py:
##########
@@ -229,6 +230,108 @@ def test_superset_joins(
assert list(results) == [(10, "ten"), (20, "twenty")]
[email protected]
+def table1_large(session: Session, database1: "Database") -> Iterator[None]:
+ with database1.get_sqla_engine() as engine:
+ with engine.begin() as conn:
+ conn.execute(
+ text(
+ "CREATE TABLE table1_large (a INTEGER NOT NULL PRIMARY
KEY, "
+ "b INTEGER)"
+ )
+ )
+ conn.execute(
+ text("INSERT INTO table1_large (a, b) VALUES (1, 10), (2, 20),
(3, 30)")
+ )
+ db.session.commit()
+
+ yield
+
+ with engine.begin() as conn:
+ conn.execute(text("DROP TABLE table1_large"))
+ db.session.commit()
+
+
[email protected]
+def table2_late_match(session: Session, database2: "Database") ->
Iterator[None]:
+ with database2.get_sqla_engine() as engine:
+ with engine.begin() as conn:
+ conn.execute(
+ text(
+ "CREATE TABLE table2_late_match (a INTEGER NOT NULL
PRIMARY KEY, "
+ "b TEXT)"
+ )
+ )
+ conn.execute(
+ text("INSERT INTO table2_late_match (a, b) VALUES (3,
'thirty')")
+ )
+ db.session.commit()
+
+ yield
+
+ with engine.begin() as conn:
+ conn.execute(text("DROP TABLE table2_late_match"))
+ db.session.commit()
+
+
+@with_feature_flags(ENABLE_SUPERSET_META_DB=True)
+def test_superset_joins_with_limit_drops_matches(
+ mocker: MockerFixture,
+ monkeypatch: pytest.MonkeyPatch,
+ app_context: None,
+ table1_large: None,
+ table2_late_match: None,
+) -> None:
+ """
+ Regression for #36304: SUPERSET_META_DB_LIMIT is applied to each
+ underlying table independently, before the in-memory join runs. A row
+ that has a genuine match on the other side of the join but falls past
+ the per-table limit is silently dropped from the join result, with no
+ error or truncation warning.
+ """
+ # Use monkeypatch (rather than the `@with_config` decorator) so the
+ # config overrides are guaranteed to be undone even though this test is
+ # expected to fail its assertion until the underlying bug is fixed.
+ # `@with_config` only restores the original values after the wrapped
+ # test function returns normally, so an assertion failure here would
+ # otherwise leak SUPERSET_META_DB_LIMIT=2 into later tests.
+ monkeypatch.setitem(current_app.config, "DB_SQLA_URI_VALIDATOR", None)
+ monkeypatch.setitem(current_app.config, "SUPERSET_META_DB_LIMIT", 2)
+ monkeypatch.setitem(current_app.config, "DATABASE_OAUTH2_CLIENTS", {})
+ monkeypatch.setitem(current_app.config,
"SQLALCHEMY_CUSTOM_PASSWORD_STORE", None)
+
+ mocker.patch(
+ "superset.extensions.metadb.security_manager.raise_for_access",
+ return_value=None,
+ )
+
+ from flask import g
+
+ g.user = mocker.MagicMock()
+ g.user.is_anonymous = False
+
+ try:
+ engine = create_engine("superset://", future=True)
+ except Exception as e:
+ # Skip test if superset:// dialect can't be loaded (common in Docker)
+ pytest.skip(f"Superset dialect not available: {e}")
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag the broad exception handling used to skip unavailable-dialect
tests when following the established pattern in this file.
**Applied to:**
- `tests/unit_tests/extensions/test_sqlalchemy.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
