From 9cd15c7755759d94e86ee2e7e3b57d90d7386fa0 Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:13:28 -0500
Subject: [PATCH 1/6] Move var_expand

---
 src/sql/command.py        | 36 +++++++++++-------------------------
 src/tests/test_command.py | 18 +++++++++---------
 2 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/src/sql/command.py b/src/sql/command.py
index 11af09195..c5c5c0537 100644
--- a/src/sql/command.py
+++ b/src/sql/command.py
@@ -23,7 +23,7 @@ class SQLCommand:
 
     def __init__(self, magic, user_ns, line, cell) -> None:
         # Support for the variable substition in the SQL clause
-        line, cell = self._var_expand(magic, user_ns, line, cell)
+        # line, cell = self._var_expand(magic, user_ns, line, cell)
         self.args = parse.magic_args(magic.execute, line)
         # self.args.line (everything that appears after %sql/%%sql in the first line)
         # is splited in tokens (delimited by spaces), this checks if we have one arg
@@ -52,8 +52,9 @@ def __init__(self, magic, user_ns, line, cell) -> None:
                 self.command_text = infile.read() + "\n" + self.command_text
 
         self.parsed = parse.parse(self.command_text, magic)
-
-        self.parsed["sql_original"] = self.parsed["sql"]
+        self.parsed["sql_original"] = self._var_expand(
+            self.parsed["sql"], user_ns, magic
+        )
 
         if add_conn:
             self.parsed["connection"] = user_ns[self.args.line[0]]
@@ -89,32 +90,17 @@ def result_var(self):
         """Returns the result_var"""
         return self.parsed["result_var"]
 
-    def _var_expand(self, magic, user_ns, line, cell):
-        """
-        Support for the variable substition in the SQL clause
-        For now, we have enabled two ways:
-        1. Latest format, {{a}}, we use jinja2 to parse the string with {{a}} format
-        2. Legacy format, {a}, $a, and :a format.
+    def _var_expand(self, sql, user_ns, magic):
+        sql = Template(sql).render(user_ns)
+        parsed_sql = magic.shell.var_expand(sql, depth=2)
 
-        We will deprecate the legacy format feature in next major version
-        """
-        self.is_legacy_var_expand_parsed = False
-        # Latest format parsing
-        # TODO: support --param and --use-global logic here
-        # Ref: https://github.com/ploomber/jupysql/issues/93
-        line = Template(line).render(user_ns)
-        cell = Template(cell).render(user_ns)
-        # Legacy format parsing
-        parsed_cell = magic.shell.var_expand(cell, depth=2)
-        parsed_line = magic.shell.var_expand(line, depth=2)
-        # Exclusive the string with "://", but has :variable
-        has_SQLAlchemy_var_expand = re.search("(?<!://):[^/]+", line) or ":" in cell
-        if parsed_line != line or parsed_cell != cell or has_SQLAlchemy_var_expand:
-            self.is_legacy_var_expand_parsed = True
+        has_SQLAlchemy_var_expand = re.search("(?<!://):[^/]+", sql)
+        if parsed_sql != sql or has_SQLAlchemy_var_expand:
             warnings.warn(
                 "Variable substitution with $var and {var} has been "
                 "deprecated and will be removed in a future version. "
                 "Use {{var}} instead.",
                 FutureWarning,
             )
-        return parsed_line, parsed_cell
+
+        return parsed_sql
diff --git a/src/tests/test_command.py b/src/tests/test_command.py
index 63ee871f8..b91d3d84b 100644
--- a/src/tests/test_command.py
+++ b/src/tests/test_command.py
@@ -14,11 +14,11 @@ def sql_magic(ip):
 @pytest.mark.parametrize(
     "line, cell, parsed_sql, parsed_connection, parsed_result_var",
     [
-        ("something --no-execute", "", "something\n", "", None),
+        ("something --no-execute", "", "something", "", None),
         ("sqlite://", "", "", "sqlite://", None),
-        ("SELECT * FROM TABLE", "", "SELECT * FROM TABLE\n", "", None),
+        ("SELECT * FROM TABLE", "", "SELECT * FROM TABLE", "", None),
         ("SELECT * FROM", "TABLE", "SELECT * FROM\nTABLE", "", None),
-        ("my_var << SELECT * FROM table", "", "SELECT * FROM table\n", "", "my_var"),
+        ("my_var << SELECT * FROM table", "", "SELECT * FROM table", "", "my_var"),
         ("my_var << SELECT *", "FROM table", "SELECT *\nFROM table", "", "my_var"),
         ("[db]", "", "", "sqlite://", None),
     ],
@@ -104,13 +104,13 @@ def test_parsed_sql_when_using_file(ip, sql_magic, tmp_empty):
     assert cmd.parsed == {
         "connection": "",
         "result_var": None,
-        "sql": "SELECT * FROM author\n\n",
-        "sql_original": "SELECT * FROM author\n\n",
+        "sql": "SELECT * FROM author\n",
+        "sql_original": "SELECT * FROM author\n",
     }
 
     assert cmd.connection == ""
-    assert cmd.sql == "SELECT * FROM author\n\n"
-    assert cmd.sql_original == "SELECT * FROM author\n\n"
+    assert cmd.sql == "SELECT * FROM author\n"
+    assert cmd.sql_original == "SELECT * FROM author\n"
 
 
 def test_args(ip, sql_magic):
@@ -220,7 +220,7 @@ def test_variable_substitution_legacy_dollar_prefix_cell_magic(ip, sql_magic):
         cell="GRANT CONNECT ON DATABASE postgres TO $username;",
     )
 
-    assert cmd.parsed["sql"] == "\nGRANT CONNECT ON DATABASE postgres TO some-user;"
+    assert cmd.parsed["sql"] == "GRANT CONNECT ON DATABASE postgres TO some-user;"
 
 
 def test_variable_substitution_legacy_single_curly_cell_magic(ip, sql_magic):
@@ -261,4 +261,4 @@ def test_variable_substitution_double_curly_line_magic(ip, sql_magic):
     )
 
     # print ("cmd.parsed['sql']", cmd.parsed["sql"])
-    assert cmd.parsed["sql"] == "SELECT first_name FROM author LIMIT 5;\n"
+    assert cmd.parsed["sql"] == "SELECT first_name FROM author LIMIT 5;"

From 0bb89c1da463da85f69234161eddfbb7531026f1 Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:30:38 -0500
Subject: [PATCH 2/6] Add: no warning test case

---
 src/tests/integration/test_generic_db_opeations.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/tests/integration/test_generic_db_opeations.py b/src/tests/integration/test_generic_db_opeations.py
index 6adbb8919..d3503d544 100644
--- a/src/tests/integration/test_generic_db_opeations.py
+++ b/src/tests/integration/test_generic_db_opeations.py
@@ -1,5 +1,6 @@
 import shutil
 import pytest
+import warnings
 from sql.telemetry import telemetry
 from unittest.mock import ANY, Mock
 
@@ -106,10 +107,12 @@ def test_close_and_connect(
     # Disconnect
     ip_with_dynamic_db.run_cell("%sql -x " + conn_alias)
     assert get_connection_count(ip_with_dynamic_db) == 0
-    # Connect
-    ip_with_dynamic_db.run_cell("%sql " + database_url + " --alias " + conn_alias)
-    assert get_connection_count(ip_with_dynamic_db) == 1
+    # Connect, also check there is no error on re-connecting
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        ip_with_dynamic_db.run_cell("%sql " + database_url + " --alias " + conn_alias)
 
+    assert get_connection_count(ip_with_dynamic_db) == 1
 
 # Telemetry
 # Test - Number of active connection

From bc07c6354865bc37bc50cfc651c1adab29736592 Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:32:44 -0500
Subject: [PATCH 3/6] Cleanup code

---
 src/sql/command.py                                 | 3 +--
 src/tests/integration/test_generic_db_opeations.py | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sql/command.py b/src/sql/command.py
index c5c5c0537..27a6f773d 100644
--- a/src/sql/command.py
+++ b/src/sql/command.py
@@ -22,8 +22,6 @@ class SQLCommand:
     """
 
     def __init__(self, magic, user_ns, line, cell) -> None:
-        # Support for the variable substition in the SQL clause
-        # line, cell = self._var_expand(magic, user_ns, line, cell)
         self.args = parse.magic_args(magic.execute, line)
         # self.args.line (everything that appears after %sql/%%sql in the first line)
         # is splited in tokens (delimited by spaces), this checks if we have one arg
@@ -52,6 +50,7 @@ def __init__(self, magic, user_ns, line, cell) -> None:
                 self.command_text = infile.read() + "\n" + self.command_text
 
         self.parsed = parse.parse(self.command_text, magic)
+        # Support for the variable substition in the SQL clause
         self.parsed["sql_original"] = self._var_expand(
             self.parsed["sql"], user_ns, magic
         )
diff --git a/src/tests/integration/test_generic_db_opeations.py b/src/tests/integration/test_generic_db_opeations.py
index d3503d544..53f37e676 100644
--- a/src/tests/integration/test_generic_db_opeations.py
+++ b/src/tests/integration/test_generic_db_opeations.py
@@ -114,6 +114,7 @@ def test_close_and_connect(
 
     assert get_connection_count(ip_with_dynamic_db) == 1
 
+
 # Telemetry
 # Test - Number of active connection
 @pytest.mark.parametrize(

From b59162fd49078d18dec12f4f2cd0b6a0f03c35fe Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:37:13 -0500
Subject: [PATCH 4/6] Fix: test cases

---
 src/sql/command.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sql/command.py b/src/sql/command.py
index 27a6f773d..128f5330e 100644
--- a/src/sql/command.py
+++ b/src/sql/command.py
@@ -50,8 +50,8 @@ def __init__(self, magic, user_ns, line, cell) -> None:
                 self.command_text = infile.read() + "\n" + self.command_text
 
         self.parsed = parse.parse(self.command_text, magic)
-        # Support for the variable substition in the SQL clause
-        self.parsed["sql_original"] = self._var_expand(
+
+        self.parsed["sql_original"] = self.parsed["sql"] = self._var_expand(
             self.parsed["sql"], user_ns, magic
         )
 

From 56cb21a25834e97ba185d076cb98e3749ce3c1b0 Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:52:50 -0500
Subject: [PATCH 5/6] Add: fix me

---
 CHANGELOG.md       | 2 ++
 src/sql/command.py | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 147613c43..31fdc2155 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## 0.6.2dev
 
+* [Fix] Deprecation warning incorrectly displayed #213
+
 ## 0.6.1 (2023-03-02)
 
 * [Feature] Support new variable substitution using `{{variable}}` format ([#137](https://github.com/ploomber/jupysql/pull/137))
diff --git a/src/sql/command.py b/src/sql/command.py
index 128f5330e..c6dca99c8 100644
--- a/src/sql/command.py
+++ b/src/sql/command.py
@@ -1,4 +1,3 @@
-import re
 import warnings
 from IPython.core.magic_arguments import parse_argstring
 from jinja2 import Template
@@ -93,7 +92,9 @@ def _var_expand(self, sql, user_ns, magic):
         sql = Template(sql).render(user_ns)
         parsed_sql = magic.shell.var_expand(sql, depth=2)
 
-        has_SQLAlchemy_var_expand = re.search("(?<!://):[^/]+", sql)
+        has_SQLAlchemy_var_expand = ":" in sql
+        # parsed_sql != sql: Detect IPython interprets {a} or $a
+        # has_SQLAlchemy_var_expand: Detect using Sqlalchemy interprets :a
         if parsed_sql != sql or has_SQLAlchemy_var_expand:
             warnings.warn(
                 "Variable substitution with $var and {var} has been "

From 53e821bd05de17e78784ce685209f6567c0ddc33 Mon Sep 17 00:00:00 2001
From: Tony Kuo <yen.c@ploomber.io>
Date: Sat, 4 Mar 2023 15:54:58 -0500
Subject: [PATCH 6/6] Fix: comment

---
 src/sql/command.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sql/command.py b/src/sql/command.py
index c6dca99c8..56c73e00e 100644
--- a/src/sql/command.py
+++ b/src/sql/command.py
@@ -93,8 +93,8 @@ def _var_expand(self, sql, user_ns, magic):
         parsed_sql = magic.shell.var_expand(sql, depth=2)
 
         has_SQLAlchemy_var_expand = ":" in sql
-        # parsed_sql != sql: Detect IPython interprets {a} or $a
-        # has_SQLAlchemy_var_expand: Detect using Sqlalchemy interprets :a
+        # parsed_sql != sql: detect if using IPython fashion - {a} or $a
+        # has_SQLAlchemy_var_expand: detect if using Sqlalchemy fashion - :a
         if parsed_sql != sql or has_SQLAlchemy_var_expand:
             warnings.warn(
                 "Variable substitution with $var and {var} has been "