From 27bef63280f15486b1089534c1ef804f8b48dfbd Mon Sep 17 00:00:00 2001 From: Neelasha Sen Date: Wed, 12 Jul 2023 05:19:47 +0530 Subject: [PATCH] Adding `--with` for CTE (#705) --- CHANGELOG.md | 1 + doc/compose.md | 48 ++++++++++- doc/plot.md | 7 ++ src/sql/__init__.py | 2 - src/sql/command.py | 4 + src/sql/error_message.py | 85 ++++++------------- src/sql/magic.py | 79 ++++++++++------- src/sql/magic_plot.py | 19 ++--- src/sql/util.py | 15 ++-- .../integration/test_generic_db_operations.py | 42 ++++++++- src/tests/test_command.py | 31 +++++++ src/tests/test_magic.py | 12 +-- src/tests/test_magic_cte.py | 29 +------ src/tests/test_magic_plot.py | 39 +++++---- src/tests/test_syntax_errors.py | 84 +----------------- 15 files changed, 253 insertions(+), 244 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4773429f1..8e911a8c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * [Fix] Refactored `ResultSet` to lazy loading (#470) * [Fix] Removed `WITH` when a snippet does not have a dependency (#657) * [Fix] Used display module when generating CTE (#649) +* [Fix] Adding `--with` back because of issues with sqlglot query parser (#684) ## 0.7.9 (2023-06-19) diff --git a/doc/compose.md b/doc/compose.md index b31c29feb..b80b86bf3 100644 --- a/doc/compose.md +++ b/doc/compose.md @@ -27,7 +27,7 @@ pip install jupysql matplotlib ``` -```{versionchanged} 0.7.8 +```{versionchanged} 0.7.10 ``` ```{note} @@ -158,6 +158,52 @@ We can verify the retrieved query returns the same result: {{final}} ``` +#### `--with` argument + +JupySQL also allows you to specify the snippet name explicitly by passing the `--with` argument. This is particularly useful when our parsing logic is unable to determine the table name due to dialect variations. For example, consider the below example: + +```{code-cell} ipython3 +%sql duckdb:// +``` + +```{code-cell} ipython3 +%%sql --save first_cte --no-execute +SELECT 1 AS column1, 2 AS column2 +``` + +```{code-cell} ipython3 +%%sql --save second_cte --no-execute +SELECT + sum(column1), + sum(column2) FILTER (column2 = 2) +FROM first_cte +``` + +```{code-cell} ipython3 +:tags: [raises-exception] + +%%sql +SELECT * FROM second_cte +``` + +Note that the query fails because the clause `FILTER (column2 = 2)` makes it difficult for the parser to extract the table name. While this syntax works on some dialects like `DuckDB`, the more common usage is to specify `WHERE` clause as well, like `FILTER (WHERE column2 = 2)`. + +Now let's run the same query by specifying `--with` argument. + +```{code-cell} ipython3 +%%sql --with first_cte --save second_cte --no-execute +SELECT + sum(column1), + sum(column2) FILTER (column2 = 2) +FROM first_cte +``` + +```{code-cell} ipython3 +%%sql +SELECT * FROM second_cte +``` + + ## Summary In the given example, we demonstrated JupySQL's usage as a tool for managing large SQL queries in Jupyter Notebooks. It effectively broke down a complex query into smaller, organized parts, simplifying the process of analyzing a record store's sales database. By using JupySQL, users can easily maintain and reuse their queries, enhancing the overall data analysis experience. diff --git a/doc/plot.md b/doc/plot.md index ec5bdaaaf..2f65f107c 100644 --- a/doc/plot.md +++ b/doc/plot.md @@ -131,6 +131,13 @@ We can see the highest value is a bit over 6, that's expected since we set a 6.3 +++ +If you wish to specify the saved snippet explicitly, please use the `--with` argument. +[Click here](../compose) for more details on when to specify `--with` explicitly. + +```{code-cell} ipython3 +%sqlplot boxplot --table short_trips --column trip_distance --with short_trips +``` + ## Histogram To create a histogram, call `%sqlplot histogram`, and pass the name of the table, the column you want to plot, and the number of bins. Similarly to what we did in the [Boxplot](#boxplot) example, JupySQL detects a saved snippet and only plots such data subset. diff --git a/src/sql/__init__.py b/src/sql/__init__.py index f309a2762..8f8a027da 100644 --- a/src/sql/__init__.py +++ b/src/sql/__init__.py @@ -1,5 +1,4 @@ from .magic import RenderMagic, SqlMagic, load_ipython_extension -from .error_message import SYNTAX_ERROR from .connection import PLOOMBER_DOCS_LINK_STR __version__ = "0.7.10dev" @@ -9,6 +8,5 @@ "RenderMagic", "SqlMagic", "load_ipython_extension", - "SYNTAX_ERROR", "PLOOMBER_DOCS_LINK_STR", ] diff --git a/src/sql/command.py b/src/sql/command.py index b73355519..5d920992e 100644 --- a/src/sql/command.py +++ b/src/sql/command.py @@ -71,6 +71,10 @@ def __init__(self, magic, user_ns, line, cell) -> None: if add_alias: self.parsed["connection"] = self.args.line[0] + if self.args.with_: + final = store.render(self.parsed["sql"], with_=self.args.with_) + self.parsed["sql"] = str(final) + @property def sql(self): """ diff --git a/src/sql/error_message.py b/src/sql/error_message.py index 640dc94bf..8f9f70569 100644 --- a/src/sql/error_message.py +++ b/src/sql/error_message.py @@ -1,70 +1,39 @@ -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" +CTE_MSG = ( + "If using snippets, you may pass the --with argument explicitly.\n" + "For more details please refer : " + "https://jupysql.ploomber.io/en/latest/compose.html#with-argument" +) -def parse_sqlglot_error(e, q): +def _is_syntax_error(error): """ - Function to parse the error message from sqlglot - - Parameters - ---------- - e: sqlglot.errors.ParseError, exception - while parsing through sqlglot - q : str, user query - - Returns - ------- - str - Formatted error message containing description - and positions + Function to detect whether error message from DB driver + is related to syntax error in user query. """ - 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" - ) - msg = "Possible reason: \n" + position if position else "" - return msg + error_lower = error.lower() + return ( + "syntax error" in error_lower + or ("catalog error" in error_lower and "does not exist" in error_lower) + or "error in your sql syntax" in error_lower + or "incorrect syntax" in error_lower + or "not found" in error_lower + ) -def detail(original_error, query=None): +def detail(original_error): 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: - parse_msg = parse_sqlglot_error(e, q) - return_msg = return_msg + parse_msg if parse_msg else return_msg - - return return_msg + "\n" + ORIGINAL_ERROR + original_error + "\n" + if _is_syntax_error(original_error): + return f"{CTE_MSG}\n\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" - ) + specific_error = """\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""" + + return f"{specific_error}\n{ORIGINAL_ERROR}{original_error}\n" return None diff --git a/src/sql/magic.py b/src/sql/magic.py index e24fea7b5..2ec092792 100644 --- a/src/sql/magic.py +++ b/src/sql/magic.py @@ -31,7 +31,7 @@ from sql.magic_cmd import SqlCmdMagic from sql._patch import patch_ipython_usage_error from sql import query_util -from sql.util import get_suggestions_message, show_deprecation_warning +from sql.util import get_suggestions_message, pretty_print from ploomber_core.dependencies import check_installed from sql.error_message import detail @@ -214,6 +214,35 @@ def check_random_arguments(self, line="", cell=""): "Unrecognized argument(s): {}".format(check_argument) ) + def _error_handling(self, e, query): + detailed_msg = detail(e) + if self.short_errors: + if detailed_msg is not None: + err = exceptions.UsageError(detailed_msg) + raise err + # TODO: move to error_messages.py + # Added here due to circular dependency issue (#545) + elif "no such table" in str(e): + tables = query_util.extract_tables_from_query(query) + for table in tables: + suggestions = get_close_matches(table, list(self._store)) + err_message = f"There is no table with name {table!r}." + # with_message = "Alternatively, please specify table + # name using --with argument" + if len(suggestions) > 0: + suggestions_message = get_suggestions_message(suggestions) + raise exceptions.TableNotFoundError( + f"{err_message}{suggestions_message}" + ) + display.message(str(e)) + else: + display.message(str(e)) + else: + if detailed_msg is not None: + display.message(detailed_msg) + e.modify_exception = True + raise e + @no_var_expand @needs_local_scope @line_magic("sql") @@ -364,12 +393,17 @@ def interactive_execute_wrapper(**kwargs): args = command.args - with_ = self._store.infer_dependencies(command.sql_original, args.save) - if with_: - command.set_sql_with(with_) - display.message(f"Generating CTE with stored snippets: {', '.join(with_)}") + if args.with_: + with_ = args.with_ else: - with_ = None + with_ = self._store.infer_dependencies(command.sql_original, args.save) + if with_: + command.set_sql_with(with_) + display.message( + f"Generating CTE with stored snippets : {pretty_print(with_)}" + ) + else: + with_ = None # Create the interactive slider if args.interact and not is_interactive_mode: @@ -405,7 +439,7 @@ def interactive_execute_wrapper(**kwargs): raw_args = raw_args[1:-1] args.connection_arguments = json.loads(raw_args) except Exception as e: - print(e) + display.message(str(e)) raise e else: args.connection_arguments = {} @@ -458,8 +492,7 @@ def interactive_execute_wrapper(**kwargs): if not command.sql: return - if args.with_: - show_deprecation_warning() + # store the query if needed if args.save: if "-" in args.save: @@ -514,30 +547,12 @@ 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: - if detailed_msg is not None: - err = exceptions.UsageError(detailed_msg) - raise err - # TODO: move to error_messages.py - # Added here due to circular dependency issue (#545) - elif "no such table" in str(e): - tables = query_util.extract_tables_from_query(command.sql) - for table in tables: - suggestions = get_close_matches(table, list(self._store)) - if len(suggestions) > 0: - err_message = f"There is no table with name {table!r}." - suggestions_message = get_suggestions_message(suggestions) - raise exceptions.TableNotFoundError( - f"{err_message}{suggestions_message}" - ) - print(e) - else: - print(e) + self._error_handling(e, command.sql) + except Exception as e: + # handle DuckDB exceptions + if "Catalog Error" in str(e): + self._error_handling(e, command.sql) else: - 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/sql/magic_plot.py b/src/sql/magic_plot.py index f9554099e..ab33fa3b6 100644 --- a/src/sql/magic_plot.py +++ b/src/sql/magic_plot.py @@ -80,14 +80,21 @@ def execute(self, line="", cell="", local_ns=None): "Example: %sqlplot histogram" ) + if cmd.args.line[0] not in SUPPORTED_PLOTS + ["hist", "box"]: + plot_str = util.pretty_print(SUPPORTED_PLOTS, last_delimiter="or") + raise exceptions.UsageError( + f"Unknown plot {cmd.args.line[0]!r}. Must be any of: " f"{plot_str}" + ) + column = util.sanitize_identifier(column) table = util.sanitize_identifier(cmd.args.table) if cmd.args.with_: - util.show_deprecation_warning() - if cmd.args.line[0] in {"box", "boxplot"}: + with_ = cmd.args.with_ + else: with_ = self._check_table_exists(table) + if cmd.args.line[0] in {"box", "boxplot"}: return plot.boxplot( table=table, column=column, @@ -96,7 +103,6 @@ def execute(self, line="", cell="", local_ns=None): conn=None, ) elif cmd.args.line[0] in {"hist", "histogram"}: - with_ = self._check_table_exists(table) return plot.histogram( table=table, column=column, @@ -105,7 +111,6 @@ def execute(self, line="", cell="", local_ns=None): conn=None, ) elif cmd.args.line[0] in {"bar"}: - with_ = self._check_table_exists(table) return plot.bar( table=table, column=column, @@ -115,7 +120,6 @@ def execute(self, line="", cell="", local_ns=None): conn=None, ) elif cmd.args.line[0] in {"pie"}: - with_ = self._check_table_exists(table) return plot.pie( table=table, column=column, @@ -123,11 +127,6 @@ def execute(self, line="", cell="", local_ns=None): show_num=cmd.args.show_numbers, conn=None, ) - else: - plot_str = util.pretty_print(SUPPORTED_PLOTS, last_delimiter="or") - raise exceptions.UsageError( - f"Unknown plot {cmd.args.line[0]!r}. Must be any of: " f"{plot_str}" - ) @staticmethod def _check_table_exists(table): diff --git a/src/sql/util.py b/src/sql/util.py index 19473ace6..3df5122cf 100644 --- a/src/sql/util.py +++ b/src/sql/util.py @@ -129,12 +129,15 @@ def is_table_exists( f"There is no table with name {table!r} in the default schema" ) - suggestions = difflib.get_close_matches(invalid_input, expected) - suggestions_store = difflib.get_close_matches(invalid_input, list(store)) - suggestions.extend(suggestions_store) - suggestions_message = get_suggestions_message(suggestions) - if suggestions_message: - err_message = f"{err_message}{suggestions_message}" + if table not in list(store): + suggestions = difflib.get_close_matches(invalid_input, expected) + suggestions_store = difflib.get_close_matches( + invalid_input, list(store) + ) + suggestions.extend(suggestions_store) + suggestions_message = get_suggestions_message(suggestions) + if suggestions_message: + err_message = f"{err_message}{suggestions_message}" raise exceptions.TableNotFoundError(err_message) return _is_exist diff --git a/src/tests/integration/test_generic_db_operations.py b/src/tests/integration/test_generic_db_operations.py index 4d23f6064..dbd602588 100644 --- a/src/tests/integration/test_generic_db_operations.py +++ b/src/tests/integration/test_generic_db_operations.py @@ -3,7 +3,9 @@ import pytest import warnings from sql.telemetry import telemetry +from sql.error_message import CTE_MSG from unittest.mock import ANY, Mock +from IPython.core.error import UsageError import math @@ -623,9 +625,9 @@ def test_sqlcmd_tables(ip_with_dynamic_db, request): @pytest.mark.parametrize( "cell", [ - "%%sql\nSELECT * FROM numbers WHERE 0=1", + "%%sql\nSELECT * FROM test_numbers WHERE 0=1", "%%sql --with subset\nSELECT * FROM subset WHERE 0=1", - "%%sql\nSELECT *\n-- %one $another\nFROM numbers WHERE 0=1", + "%%sql\nSELECT *\n-- %one $another\nFROM test_numbers WHERE 0=1", ], ids=[ "simple-query", @@ -636,6 +638,16 @@ def test_sqlcmd_tables(ip_with_dynamic_db, request): @pytest.mark.parametrize("ip_with_dynamic_db", ALL_DATABASES) def test_sql_query(ip_with_dynamic_db, cell, request): ip_with_dynamic_db = request.getfixturevalue(ip_with_dynamic_db) + ip_with_dynamic_db.run_cell( + """ + %%sql sqlite:// + CREATE TABLE test_numbers (value_one, value_two); + INSERT INTO test_numbers VALUES (0, 1); + INSERT INTO test_numbers VALUES (0, 0); + INSERT INTO test_numbers VALUES (5, 2); + INSERT INTO test_numbers VALUES (6, 3); + """ + ) ip_with_dynamic_db.run_cell( """%%sql --save subset --no-execute SELECT * FROM numbers WHERE 1=0 @@ -643,3 +655,29 @@ def test_sql_query(ip_with_dynamic_db, cell, request): ) out = ip_with_dynamic_db.run_cell(cell) assert out.error_in_exec is None + + +@pytest.mark.parametrize("ip_with_dynamic_db", ALL_DATABASES) +def test_sql_query_cte_suggestion(ip_with_dynamic_db, request): + ip_with_dynamic_db = request.getfixturevalue(ip_with_dynamic_db) + ip_with_dynamic_db.run_cell( + """%%sql --save first_cte --no-execute +SELECT 1 AS column1, 2 AS column2 +""" + ) + ip_with_dynamic_db.run_cell( + """ + %%sql --save second_cte --no-execute +SELECT + sum(column1), + sum(column2) FILTER (column2 = 2) +FROM first_cte +""" + ) + out = ip_with_dynamic_db.run_cell( + """ + %%sql +SELECT * FROM second_cte""" + ) + assert isinstance(out.error_in_exec, UsageError) + assert CTE_MSG in str(out.error_in_exec) diff --git a/src/tests/test_command.py b/src/tests/test_command.py index 89a085c26..7b73b14d5 100644 --- a/src/tests/test_command.py +++ b/src/tests/test_command.py @@ -95,6 +95,37 @@ def test_parsed( assert cmd.sql_original == parsed_sql +def test_parsed_sql_when_using_with(ip, sql_magic): + ip.run_cell_magic( + "sql", + "--save author_one", + """ + SELECT * FROM author LIMIT 1 + """, + ) + + cmd = SQLCommand( + sql_magic, ip.user_ns, line="--with author_one", cell="SELECT * FROM author_one" + ) + + sql = "WITH `author_one` AS (\n\n SELECT * FROM author LIMIT 1\n )\n\ +SELECT * FROM author_one" + + sql_original = "\nSELECT * FROM author_one" + + assert cmd.parsed == { + "connection": "", + "result_var": None, + "return_result_var": False, + "sql": sql, + "sql_original": sql_original, + } + + assert cmd.connection == "" + assert cmd.sql == sql + assert cmd.sql_original == sql_original + + def test_parsed_sql_when_using_file(ip, sql_magic, tmp_empty): Path("query.sql").write_text("SELECT * FROM author") cmd = SQLCommand(sql_magic, ip.user_ns, "--file query.sql", "") diff --git a/src/tests/test_magic.py b/src/tests/test_magic.py index 814099e90..03e080dd7 100644 --- a/src/tests/test_magic.py +++ b/src/tests/test_magic.py @@ -1235,16 +1235,10 @@ def test_save_with_number_table( def test_save_with_non_existing_with(ip): - with pytest.warns(FutureWarning) as record: - ip.run_cell( - "%sql --with non_existing_sub_query " "SELECT * FROM non_existing_sub_query" - ) - assert len(record) == 1 - assert ( - "CTE dependencies are now automatically inferred, you can omit the " - "--with arguments. Using --with will raise an exception in the next " - "major release so please remove it." in record[0].message.args[0] + out = ip.run_cell( + "%sql --with non_existing_sub_query " "SELECT * FROM non_existing_sub_query" ) + assert isinstance(out.error_in_exec, UsageError) def test_save_with_non_existing_table(ip, capsys): diff --git a/src/tests/test_magic_cte.py b/src/tests/test_magic_cte.py index 5050fa02b..7e1efc66d 100644 --- a/src/tests/test_magic_cte.py +++ b/src/tests/test_magic_cte.py @@ -1,5 +1,6 @@ import pytest from IPython.core.error import UsageError +from sql.error_message import CTE_MSG def test_trailing_semicolons_removed_from_cte(ip): @@ -56,29 +57,7 @@ def test_infer_dependencies(ip, capsys): ) assert result == expected - assert "Generating CTE with stored snippets: author_sub" in out - - -def test_deprecation_warning(ip): - ip.run_cell_magic( - "sql", - "--save author_sub", - "SELECT last_name FROM author WHERE year_of_death > 1900", - ) - - with pytest.warns(FutureWarning) as record: - ip.run_cell_magic( - "sql", - "--with author_sub --save final", - "SELECT last_name FROM author_sub;", - ) - assert len(record) == 1 - assert ( - "CTE dependencies are now automatically inferred," - " you can omit the --with arguments. Using --with will " - "raise an exception in the next major release so please " - "remove it." in record[0].message.args[0] - ) + assert "Generating CTE with stored snippets : 'author_sub'" in out TABLE_NAME_TYPO_ERR_MSG = """ @@ -148,7 +127,7 @@ def test_snippets_delete(ip, capsys): ) out, _ = capsys.readouterr() - assert "Generating CTE with stored snippets: another_orders" in out + assert "Generating CTE with stored snippets : 'another_orders'" in out result_del = ip.run_cell( "%sqlcmd snippets --delete-force-all another_orders" ).result @@ -198,4 +177,4 @@ def test_query_syntax_error(ip): ) assert excinfo.value.error_type == "UsageError" - assert SYNTAX_ERROR_MESSAGE.strip() in str(excinfo.value) + assert CTE_MSG.strip() in str(excinfo.value) diff --git a/src/tests/test_magic_plot.py b/src/tests/test_magic_plot.py index 83046be81..e215e402d 100644 --- a/src/tests/test_magic_plot.py +++ b/src/tests/test_magic_plot.py @@ -99,13 +99,18 @@ def test_validate_arguments(tmp_empty, ip, cell, error_type, error_message): "%sqlplot box --table data.csv --column x", "%sqlplot boxplot --table data.csv --column x --orient h", "%sqlplot boxplot --table subset --column x", + "%sqlplot boxplot --table subset --column x --with subset", "%sqlplot boxplot -t subset -c x -w subset -o h", "%sqlplot boxplot --table nas.csv --column x", "%sqlplot bar -t data.csv -c x", + "%sqlplot bar --table subset --column x", + "%sqlplot bar --table subset --column x --with subset", "%sqlplot bar -t data.csv -c x -S", "%sqlplot bar -t data.csv -c x -o h", "%sqlplot bar -t data.csv -c x y", "%sqlplot pie -t data.csv -c x", + "%sqlplot pie --table subset --column x", + "%sqlplot pie --table subset --column x --with subset", "%sqlplot pie -t data.csv -c x -S", "%sqlplot pie -t data.csv -c x y", '%sqlplot boxplot --table spaces.csv --column "some column"', @@ -147,16 +152,21 @@ def test_validate_arguments(tmp_empty, ip, cell, error_type, error_message): "histogram-bins", "histogram-nas", "boxplot", + "boxplot-with", "box", "boxplot-horizontal", "boxplot-with", "boxplot-shortcuts", "boxplot-nas", "bar-1-col", + "bar-subset", + "bar-subset-with", "bar-1-col-show_num", "bar-1-col-horizontal", "bar-2-col", "pie-1-col", + "pie-subset", + "pie-subset-with", "pie-1-col-show_num", "pie-2-col", "boxplot-column-name-with-spaces", @@ -416,23 +426,6 @@ def test_hist_cust(load_penguin, ip): _ = ax.grid(True) -def test_sqlplot_deprecation_warning(ip_snippets, capsys): - with pytest.warns(FutureWarning) as record: - res = ip_snippets.run_cell( - "%sqlplot boxplot --table subset --column x --with subset" - ) - assert len(record) == 1 - assert ( - "CTE dependencies are now automatically inferred," - " you can omit the --with arguments. Using --with will " - "raise an exception in the next major release so please " - "remove it." in record[0].message.args[0] - ) - out, err = capsys.readouterr() - assert type(res.result).__name__ in {"Axes", "AxesSubplot"} - assert "Plotting using saved snippet : subset" in out - - @pytest.mark.parametrize( "arg", ["--delete", "-d", "--delete-force-all", "-A", "--delete-force", "-D"] ) @@ -454,3 +447,15 @@ def test_sqlplot_snippet_typo(ip_snippets, capsys): ip_snippets.run_cell("%sqlplot boxplot --table subst --column x") out, err = capsys.readouterr() assert TABLE_NAME_TYPO_MSG.strip() == err.strip() + + +MISSING_TABLE_ERROR_MSG = """ +UsageError: There is no table with name 'missing' in the default schema +If you need help solving this issue, send us a message: https://ploomber.io/community +""" + + +def test_sqlplot_missing_table(ip_snippets, capsys): + ip_snippets.run_cell("%sqlplot boxplot --table missing --column x") + out, err = capsys.readouterr() + assert MISSING_TABLE_ERROR_MSG.strip() == err.strip() diff --git a/src/tests/test_syntax_errors.py b/src/tests/test_syntax_errors.py index 582de08ec..2dc4439c2 100644 --- a/src/tests/test_syntax_errors.py +++ b/src/tests/test_syntax_errors.py @@ -4,25 +4,13 @@ from sqlalchemy.exc import OperationalError from IPython.core.error import UsageError -from sql.error_message import SYNTAX_ERROR, ORIGINAL_ERROR +from sql.error_message import ORIGINAL_ERROR, CTE_MSG from ploomber_core.exceptions import COMMUNITY COMMUNITY = COMMUNITY.strip() -@pytest.fixture -def query_no_suggestion(): - return ( - "sql", - "", - """ - sqlite:// - SELECT FROM author; - """, - ) - - @pytest.fixture def query_incorrect_column_name(): return ( @@ -35,18 +23,6 @@ def query_incorrect_column_name(): ) -@pytest.fixture -def query_suggestion(): - return ( - "sql", - "", - """ - sqlite:// - ALTER TABLE author RENAME new_author; - """, - ) - - @pytest.fixture def query_multiple(): return ( @@ -61,39 +37,15 @@ def query_multiple(): def _common_strings_check(err): - assert SYNTAX_ERROR.strip() in str(err.value) assert ORIGINAL_ERROR.strip() in str(err.value) + assert CTE_MSG.strip() in str(err.value) assert COMMUNITY in str(err.value) assert isinstance(err.value, UsageError) -def test_syntax_error_no_suggestion(ip, query_no_suggestion): - with pytest.raises(UsageError) as err: - ip.run_cell_magic(*query_no_suggestion) - _common_strings_check(err) - - -message_no_suggestion_long = f"""\ -(sqlite3.OperationalError) near "FROM": syntax error -{COMMUNITY} -[SQL: SELECT FROM author;] -""" # noqa - - -def test_syntax_error_no_suggestion_long(ip, capsys, query_no_suggestion): - ip.run_line_magic("config", "SqlMagic.short_errors = False") - with pytest.raises(OperationalError) as err: - ip.run_cell_magic(*query_no_suggestion) - out, _ = capsys.readouterr() - assert message_no_suggestion_long.strip() in str(err.value).strip() - assert SYNTAX_ERROR.strip() in out - assert isinstance(err.value, sqlalchemy.exc.OperationalError) - - def test_syntax_error_incorrect_column_name(ip, query_incorrect_column_name): with pytest.raises(UsageError) as err: ip.run_cell_magic(*query_incorrect_column_name) - assert "Syntax Error in SELECT first_(name FROM author:" in str(err.value) _common_strings_check(err) @@ -112,43 +64,12 @@ def test_syntax_error_incorrect_column_name_long( ip.run_cell_magic(*query_incorrect_column_name) out, _ = capsys.readouterr() assert message_incorrect_column_name_long.strip() in str(err.value).strip() - assert SYNTAX_ERROR.strip() in out - assert isinstance(err.value, sqlalchemy.exc.OperationalError) - - -def test_syntax_error_suggestion(ip, query_suggestion): - with pytest.raises(UsageError) as err: - ip.run_cell_magic(*query_suggestion) - assert "Did you mean : ['ALTER TABLE author RENAME TO new_author']" in str( - err.value - ) - _common_strings_check(err) - - -message_error_suggestion_long = f"""\ -(sqlite3.OperationalError) near ";": syntax error -{COMMUNITY} -[SQL: ALTER TABLE author RENAME new_author;] -""" # noqa - - -def test_syntax_error_suggestion_long(ip, capsys, query_suggestion): - ip.run_line_magic("config", "SqlMagic.short_errors = False") - with pytest.raises(OperationalError) as err: - ip.run_cell_magic(*query_suggestion) - out, _ = capsys.readouterr() - assert message_error_suggestion_long.strip() in str(err.value).strip() - assert SYNTAX_ERROR.strip() in out assert isinstance(err.value, sqlalchemy.exc.OperationalError) def test_syntax_error_multiple_statements(ip, query_multiple): with pytest.raises(UsageError) as err: ip.run_cell_magic(*query_multiple) - assert ( - "Did you mean : ['ALTER TABLE author RENAME TO another_name']" - in str(err.value).strip() - ) _common_strings_check(err) @@ -165,5 +86,4 @@ def test_syntax_error_multiple_statements_long(ip, capsys, query_multiple): ip.run_cell_magic(*query_multiple) out, _ = capsys.readouterr() assert message_multiple_statements_long.strip() in str(err.value).strip() - assert SYNTAX_ERROR.strip() in out assert isinstance(err.value, sqlalchemy.exc.OperationalError)