Abdulrehman-PIAIC80387 commented on code in PR #40494:
URL: https://github.com/apache/superset/pull/40494#discussion_r3316992924


##########
superset/daos/dataset.py:
##########
@@ -424,6 +424,18 @@ def get_table_by_name(database_id: int, table_name: str) 
-> SqlaTable | None:
             .one_or_none()
         )
 
+    @staticmethod
+    def get_table_by_schema_and_name(
+        database_id: int, schema: str | None, table_name: str
+    ) -> SqlaTable | None:
+        # Filter by schema as well so callers can disambiguate datasets that
+        # share a ``table_name`` across schemas (#30377).
+        return (
+            db.session.query(SqlaTable)
+            .filter_by(database_id=database_id, schema=schema, 
table_name=table_name)
+            .one_or_none()

Review Comment:
   Addressed in 9ab07e3aff: `get_table_by_schema_and_name` now takes `catalog` 
and the filter uses the full `(database_id, catalog, schema, table_name)` 
uniqueness key. The API derives `catalog` from the request body, falling back 
to `database.get_default_catalog()` — matching `DatasetDAO.validate_uniqueness`.



##########
superset/datasets/api.py:
##########
@@ -1088,7 +1091,14 @@ def get_or_create_dataset(self) -> Response:
             return self.response(400, message=ex.messages)
         table_name = body["table_name"]
         database_id = body["database_id"]
-        if table := DatasetDAO.get_table_by_name(database_id, table_name):
+        # Honour the request's ``schema`` so we don't (a) raise 500 on
+        # ``MultipleResultsFound`` when datasets share a ``table_name`` across
+        # schemas, or (b) silently return an existing dataset from the wrong
+        # schema as a false positive (#30377).
+        schema = body.get("schema")
+        if table := DatasetDAO.get_table_by_schema_and_name(
+            database_id, schema, table_name
+        ):

Review Comment:
   Addressed in 9ab07e3aff: schema is now normalised with `body.get("schema") 
or None` before the lookup, so empty-string and missing schema both resolve 
consistently against datasets stored with `NULL` schema.



-- 
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]

Reply via email to