[GitHub] [incubator-superset] etr2460 commented on a change in pull request #11099: feat: more specific presto error messages

[GitBox <gi...@apache.org>]

etr2460 commented on a change in pull request #11099:
URL: https://github.com/apache/incubator-superset/pull/11099#discussion_r496904619



##########
File path: docs/src/pages/docs/Miscellaneous/issue_codes.mdx
##########
@@ -43,3 +43,33 @@ This may be due to a syntax error, a bug in your query, or some other
 internal failure within the database. This is usually not an
 issue within Superset, but instead a problem with the underlying
 database that serves your query.
+
+## Issue 1003

Review comment:
       My hope was this this could actually be db agnostic, and we could use the same issue number for any db

##########
File path: docs/src/pages/docs/Miscellaneous/issue_codes.mdx
##########
@@ -43,3 +43,33 @@ This may be due to a syntax error, a bug in your query, or some other
 internal failure within the database. This is usually not an
 issue within Superset, but instead a problem with the underlying
 database that serves your query.
+
+## Issue 1003
+
+```
+There is a syntax error in the SQL query. Perhaps there was a misspelling or a typo.
+```
+
+Your query failed because of a syntax error within the underlying query. Please

Review comment:
       no clue, but i was matching the line breaks from earlier in the file. @pkdotson could you comment?

##########
File path: docs/src/pages/docs/Miscellaneous/issue_codes.mdx
##########
@@ -43,3 +43,33 @@ This may be due to a syntax error, a bug in your query, or some other
 internal failure within the database. This is usually not an
 issue within Superset, but instead a problem with the underlying
 database that serves your query.
+
+## Issue 1003
+
+```
+There is a syntax error in the SQL query. Perhaps there was a misspelling or a typo.
+```
+
+Your query failed because of a syntax error within the underlying query. Please
+validate that all columns or tables referenced within the query are spelled
+correctly.
+
+## Issue 1004
+
+```
+The column was changed in the underlying database structure.

Review comment:
       This was from the designs that @Steejay provided. What do you think about the wording here Stephen?

##########
File path: superset-frontend/src/components/ErrorMessage/types.ts
##########
@@ -26,6 +26,8 @@ export const ErrorTypeEnum = {
 
   // DB Engine errors
   GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR',
+  PRESTO_CANNOT_RESOLVE_COLUMN_ERROR: 'PRESTO_CANNOT_RESOLVE_COLUMN_ERROR',

Review comment:
       yeah, this was matching the wording from the presto error message. as mentioned before though, i wonder if this should be db engine agnostic and should just be `COLUMN_DOES_NOT_EXIST_ERROR` and `TABLE_DOES_NOT_EXIST_ERROR`?

##########
File path: superset/db_engine_specs/presto.py
##########
@@ -1034,3 +1039,53 @@ def get_function_names(cls, database: "Database") -> List[str]:
         :return: A list of function names useable in the database
         """
         return database.get_df("SHOW FUNCTIONS")["Function"].tolist()
+
+    @classmethod
+    def extract_errors(cls, ex: Exception) -> List[Dict[str, Any]]:
+        raw_message = cls._extract_error_message(ex)
+
+        column_match = re.search(COLUMN_NOT_RESOLVED_ERROR_REGEX, raw_message)
+        if column_match:
+            return [
+                dataclasses.asdict(
+                    SupersetError(
+                        error_type=SupersetErrorType.PRESTO_CANNOT_RESOLVE_COLUMN_ERROR,
+                        message=__(
+                            'We can\'t seem to resolve the column "%(column_name)s" at '

Review comment:
       this is how flask_babel says to do string replacement in translation functions. I'm not sure this works with f strings by default (and am uncertain how to test): https://flask-babel.tkte.ch/#using-translations




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org