SQLachemy error overriding

Test Lint changelog util moved error_message sqlglot conflicts tests postgres Error changed import added detail in connection added detail in connection test fix Error msg modified More tests changelog merge conflicts Review cmts Exceptions usageerror Doc note changelog printing ploomber link printing ploomber link err msg changed Review comments Integration test Error msg postgres test runtimeerror Empty commit sqlalchemy version ci fix test ci fix test moved modify_exceptions skipping failing tests Disabled modify_exception fixed tests revert changes Revert xfail ipython usageerror comment comment Empty commit Fix removed skip win fix ipython removed setup changed exception drop table error msg sq brackets format fixed test Empty-Commit rebase func naming Revert xfail moved strings duckdb tests changelog File renamed Integration tests Lint Lint err msg assert Generic db Revert test revert space integration revert sqlparse added original msg tests original in connection tests modified test fix redundant str
ploomber · May 15, 2023 · 536cadc · 536cadc
1 parent 87dce34
commit 536cadc
Show file tree

Hide file tree

Showing 11 changed files with 354 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,11 +1,12 @@
 # CHANGELOG
 
 ## 0.7.5dev
-* [Feature] Using native DuckDB `.df()` method when using `autopandas` 
-
+* [Feature] Using native DuckDB `.df()` method when using `autopandas`
 * [Doc] documenting `%sqlcmd tables`/`%sqlcmd columns`
 * [Feature] Better error messages when function used in plotting API unsupported by DB driver (#159)
 * [Fix] Fix the default value of %config SqlMagic.displaylimit to 10 (#462)
+* [Feature] Detailed error messages when syntax error in SQL query, postgres connection password missing or inaccessible, invalid DuckDB connection string (#229)
+
 
 ## 0.7.4 (2023-04-28)
 No changes
@@ -299,4 +300,4 @@ Converted from an IPython Plugin to an Extension for 1.0 compatibility
 
 *Release date: 21-Mar-2013*
 
-* Initial release
+* Initial release
diff --git a/doc/community/developer-guide.md b/doc/community/developer-guide.md
@@ -61,6 +61,16 @@ The internal implementation of `sql.exceptions` is a workaround due to some IPyt
 
 +++
 
+### Handling common errors
+
+Currently, these common errors are handled by providing more meaningful error messages:
+
+* Syntax error in SQL query - The SQL query is parsed using `sqlglot` and possible syntax issues or query suggestions are provided to the user. This raises a `UsageError` with the message.
+* Missing password when connecting to PostgreSQL - Displays guide on DB connections
+* Invalid connection strings when connecting to DuckDB.
+
++++
+
 ## Unit testing
 
 ### Running tests

diff --git a/src/sql/__init__.py b/src/sql/__init__.py
@@ -1,4 +1,6 @@
 from .magic import RenderMagic, SqlMagic, load_ipython_extension
+from .error_message import SYNTAX_ERROR
+from .connection import PLOOMBER_DOCS_LINK_STR
 
 __version__ = "0.7.5dev"
 
@@ -7,4 +9,6 @@
     "RenderMagic",
     "SqlMagic",
     "load_ipython_extension",
+    "SYNTAX_ERROR",
+    "PLOOMBER_DOCS_LINK_STR",
 ]
diff --git a/src/sql/connection.py b/src/sql/connection.py
@@ -3,18 +3,19 @@
 
 import sqlalchemy
 from sqlalchemy.engine import Engine
-from sqlalchemy.exc import NoSuchModuleError
+from sqlalchemy.exc import NoSuchModuleError, OperationalError
 from IPython.core.error import UsageError
 import difflib
 import sqlglot
 
 from sql.store import store
 from sql.telemetry import telemetry
 from sql import exceptions
+from sql.error_message import detail
+from ploomber_core.exceptions import modify_exceptions
 
-PLOOMBER_SUPPORT_LINK_STR = (
-    "For technical support: https://ploomber.io/community"
-    "\nDocumentation: https://jupysql.ploomber.io/en/latest/connecting.html"
+PLOOMBER_DOCS_LINK_STR = (
+    "Documentation: https://jupysql.ploomber.io/en/latest/connecting.html"
 )
 IS_SQLALCHEMY_ONE = int(sqlalchemy.__version__.split(".")[0]) == 1
 
@@ -129,19 +130,19 @@ def __init__(self, engine, alias=None):
         self.name = self.assign_name(engine)
         self.dialect = self.url.get_dialect()
         self.engine = engine
-        self.session = engine.connect()
 
         if IS_SQLALCHEMY_ONE:
             self.metadata = sqlalchemy.MetaData(bind=engine)
 
-        self.connections[
-            alias
-            or (
-                repr(sqlalchemy.MetaData(bind=engine).bind.url)
-                if IS_SQLALCHEMY_ONE
-                else repr(engine.url)
-            )
-        ] = self
+        url = (
+            repr(sqlalchemy.MetaData(bind=engine).bind.url)
+            if IS_SQLALCHEMY_ONE
+            else repr(engine.url)
+        )
+
+        self.session = self._create_session(engine, url)
+
+        self.connections[alias or url] = self
 
         self.connect_args = None
         self.alias = alias
@@ -157,6 +158,21 @@ def _suggest_fix_no_module_found(module_name):
         options = [f"{prefix}{suffix}", suggest_str]
         return "\n\n".join(options)
 
+    @classmethod
+    @modify_exceptions
+    def _create_session(cls, engine, connect_str):
+        try:
+            session = engine.connect()
+            return session
+        except OperationalError as e:
+            detailed_msg = detail(e)
+            if detailed_msg is not None:
+                raise exceptions.UsageError(detailed_msg)
+            else:
+                print(e)
+        except Exception as e:
+            raise cls._error_invalid_connection_info(e, connect_str) from e
+
     @classmethod
     def _suggest_fix(cls, env_var, connect_str=None):
         """
@@ -203,21 +219,25 @@ def _suggest_fix(cls, env_var, connect_str=None):
         if len(options) >= 3:
             options.insert(-1, "OR")
 
-        options.append(PLOOMBER_SUPPORT_LINK_STR)
+        options.append(PLOOMBER_DOCS_LINK_STR)
 
         return "\n\n".join(options)
 
     @classmethod
     def _error_no_connection(cls):
         """Error when there isn't any connection"""
-        return UsageError("No active connection." + cls._suggest_fix(env_var=True))
+        err = UsageError("No active connection." + cls._suggest_fix(env_var=True))
+        err.modify_exception = True
+        return err
 
     @classmethod
     def _error_invalid_connection_info(cls, e, connect_str):
-        return UsageError(
+        err = UsageError(
             "An error happened while creating the connection: "
             f"{e}.{cls._suggest_fix(env_var=False, connect_str=connect_str)}"
         )
+        err.modify_exception = True
+        return err
 
     @classmethod
     def from_connect_str(
@@ -245,7 +265,7 @@ def from_connect_str(
                     [
                         str(e),
                         suggestion_str,
-                        PLOOMBER_SUPPORT_LINK_STR,
+                        PLOOMBER_DOCS_LINK_STR,
                     ]
                 )
             ) from e

diff --git a/src/sql/error_message.py b/src/sql/error_message.py
@@ -0,0 +1,53 @@
+import sqlglot
+import sqlparse
+
+SYNTAX_ERROR = "\nLooks like there is a syntax error in your query."
+ORIGINAL_ERROR = "\nOriginal error message from DB driver:\n"
+
+
+def detail(original_error, query=None):
+    original_error = str(original_error)
+    return_msg = SYNTAX_ERROR
+    if "syntax error" in original_error:
+        query_list = sqlparse.split(query)
+        for q in query_list:
+            try:
+                q = q.strip()
+                q = q[:-1] if q.endswith(";") else q
+                parse = sqlglot.transpile(q)
+                suggestions = ""
+                if q.upper() not in [suggestion.upper() for suggestion in parse]:
+                    suggestions += f"Did you mean : {parse}\n"
+                return_msg = (
+                    return_msg + "Possible reason: \n" + suggestions
+                    if suggestions
+                    else return_msg
+                )
+
+            except sqlglot.errors.ParseError as e:
+                err = e.errors
+                position = ""
+                for item in err:
+                    position += (
+                        f"Syntax Error in {q}: {item['description']} at "
+                        f"Line {item['line']}, Column {item['col']}\n"
+                    )
+                return_msg = (
+                    return_msg + "Possible reason: \n" + position
+                    if position
+                    else return_msg
+                )
+
+        return return_msg + "\n" + ORIGINAL_ERROR + original_error + "\n"
+
+    if "fe_sendauth: no password supplied" in original_error:
+        return (
+            "\nLooks like you have run into some issues. "
+            "Review our DB connection via URL strings guide: "
+            "https://jupysql.ploomber.io/en/latest/connecting.html ."
+            " Using Ubuntu? Check out this guide: "
+            "https://help.ubuntu.com/community/PostgreSQL#fe_sendauth:_"
+            "no_password_supplied\n" + ORIGINAL_ERROR + original_error + "\n"
+        )
+
+    return None
diff --git a/src/sql/magic.py b/src/sql/magic.py
@@ -29,6 +29,7 @@
 from sql._patch import patch_ipython_usage_error
 from ploomber_core.dependencies import check_installed
 
+from sql.error_message import detail
 from traitlets.config.configurable import Configurable
 from traitlets import Bool, Int, TraitError, Unicode, Dict, observe, validate
 
@@ -281,6 +282,7 @@ def execute(self, line="", cell="", local_ns=None):
         )
 
     @telemetry.log_call("execute", payload=True)
+    @modify_exceptions
     def _execute(self, payload, line, cell, local_ns, is_interactive_mode=False):
         def interactive_execute_wrapper(**kwargs):
             for key, value in kwargs.items():
@@ -429,11 +431,18 @@ def interactive_execute_wrapper(**kwargs):
         # JA: added DatabaseError for MySQL
         except (ProgrammingError, OperationalError, DatabaseError) as e:
             # Sqlite apparently return all errors as OperationalError :/
-
+            detailed_msg = detail(e, command.sql)
             if self.short_errors:
-                print(e)
+                if detailed_msg is not None:
+                    err = exceptions.UsageError(detailed_msg)
+                    raise err
+                else:
+                    print(e)
             else:
-                raise
+                if detailed_msg is not None:
+                    print(detailed_msg)
+                e.modify_exception = True
+                raise e
 
     legal_sql_identifier = re.compile(r"^[A-Za-z0-9#_$]+")
 

diff --git a/src/tests/conftest.py b/src/tests/conftest.py
@@ -69,7 +69,7 @@ def ip(ip_empty):
             "CREATE TABLE test (n INT, name TEXT)",
             "INSERT INTO test VALUES (1, 'foo')",
             "INSERT INTO test VALUES (2, 'bar')",
-            'CREATE TABLE "table with spaces" (first INT, second TEXT)',
+            "CREATE TABLE [table with spaces] (first INT, second TEXT)",
             "CREATE TABLE author (first_name, last_name, year_of_death)",
             "INSERT INTO author VALUES ('William', 'Shakespeare', 1616)",
             "INSERT INTO author VALUES ('Bertold', 'Brecht', 1956)",

diff --git a/src/tests/integration/conftest.py b/src/tests/integration/conftest.py
@@ -131,6 +131,15 @@ def ip_with_postgreSQL(ip_empty, setup_postgreSQL):
     ip_empty.run_cell("%sql -x " + alias)
 
 
+@pytest.fixture
+def postgreSQL_config_incorrect_pwd(ip_empty, setup_postgreSQL):
+    configKey = "postgreSQL"
+    alias = _testing.DatabaseConfigHelper.get_database_config(configKey)["alias"]
+    url = _testing.DatabaseConfigHelper.get_database_url(configKey)
+    url = url.replace(":ploomber_app_password", "")
+    return alias, url
+
+
 @pytest.fixture(scope="session")
 def setup_mySQL(test_table_name_dict, skip_on_live_mode):
     with _testing.mysql():

diff --git a/src/tests/integration/test_postgreSQL.py b/src/tests/integration/test_postgreSQL.py
@@ -17,3 +17,16 @@ def test_auto_commit_mode_on(ip_with_postgreSQL, capsys):
     assert out_after_creating.error_in_exec is None
     assert any(row[0] == "new_db" for row in out_all_dbs)
     assert "CREATE DATABASE cannot run inside a transaction block" not in out
+
+
+def test_postgres_error(ip_empty, postgreSQL_config_incorrect_pwd):
+    alias, url = postgreSQL_config_incorrect_pwd
+
+    # Select database engine
+    out = ip_empty.run_cell("%sql " + url + " --alias " + alias)
+    assert "Review our DB connection via URL strings guide" in str(out.error_in_exec)
+    assert "Original error message from DB driver" in str(out.error_in_exec)
+    assert (
+        "If you need help solving this issue, "
+        "send us a message: https://ploomber.io/community" in str(out.error_in_exec)
+    )