From 1a44010a76fbfefc3f0730a56557922160dd830e Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Tue, 22 Oct 2024 14:04:56 +0200 Subject: [PATCH 1/7] mraba/underscore_column_id: use `_` as column identifier --- src/snowflake/sqlalchemy/base.py | 3 ++- tests/test_compiler.py | 17 +++++++++++- tests/test_quote.py | 23 ++++++++++++++++ tests/test_quote_identifiers.py | 46 ++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 tests/test_quote_identifiers.py diff --git a/src/snowflake/sqlalchemy/base.py b/src/snowflake/sqlalchemy/base.py index 023f7afb..318f0370 100644 --- a/src/snowflake/sqlalchemy/base.py +++ b/src/snowflake/sqlalchemy/base.py @@ -112,7 +112,7 @@ AUTOCOMMIT_REGEXP = re.compile( r"\s*(?:UPDATE|INSERT|DELETE|MERGE|COPY)", re.I | re.UNICODE ) - +ILLEGAL_INITIAL_CHARACTERS = frozenset({d for d in string.digits}.union({"_", "$"})) """ Overwrite methods to handle Snowflake BCR change: @@ -437,6 +437,7 @@ def _join_left_to_right( class SnowflakeIdentifierPreparer(compiler.IdentifierPreparer): reserved_words = {x.lower() for x in RESERVED_WORDS} + illegal_initial_characters = ILLEGAL_INITIAL_CHARACTERS def __init__(self, dialect, **kw): quote = '"' diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 40207b41..55451c2f 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -2,7 +2,7 @@ # Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. # -from sqlalchemy import Integer, String, and_, func, select +from sqlalchemy import Integer, String, and_, func, insert, select from sqlalchemy.schema import DropColumnComment, DropTableComment from sqlalchemy.sql import column, quoted_name, table from sqlalchemy.testing.assertions import AssertsCompiledSQL @@ -33,6 +33,21 @@ def test_now_func(self): dialect="snowflake", ) + def test_underscore_as_valid_identifier(self): + _table = table( + "table_1745924", + column("ca", Integer), + column("cb", String), + column("_", String), + ) + + stmt = insert(_table).values(ca=1, cb="test", _="test_") + self.assert_compile( + stmt, + 'INSERT INTO table_1745924 (ca, cb, "_") VALUES (%(ca)s, %(cb)s, %(_)s)', + dialect="snowflake", + ) + def test_multi_table_delete(self): statement = table1.delete().where(table1.c.id == table2.c.id) self.assert_compile( diff --git a/tests/test_quote.py b/tests/test_quote.py index ca6f36dd..0dd69059 100644 --- a/tests/test_quote.py +++ b/tests/test_quote.py @@ -38,3 +38,26 @@ def test_table_name_with_reserved_words(engine_testaccount, db_parameters): finally: insert_table.drop(engine_testaccount) return insert_table + + +def test_table_column_as_underscore(engine_testaccount): + metadata = MetaData() + test_table_name = "table_1745924" + insert_table = Table( + test_table_name, + metadata, + Column("ca", Integer), + Column("cb", String), + Column("_", String), + ) + metadata.create_all(engine_testaccount) + try: + inspector = inspect(engine_testaccount) + columns_in_insert = inspector.get_columns(test_table_name) + assert len(columns_in_insert) == 3 + assert columns_in_insert[0]["name"] == "ca" + assert columns_in_insert[1]["name"] == "cb" + assert columns_in_insert[2]["name"] == "_" + finally: + insert_table.drop(engine_testaccount) + return insert_table diff --git a/tests/test_quote_identifiers.py b/tests/test_quote_identifiers.py new file mode 100644 index 00000000..c78dbcaa --- /dev/null +++ b/tests/test_quote_identifiers.py @@ -0,0 +1,46 @@ +# +# Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. + +from sqlalchemy import ( + Column, + Integer, + MetaData, + String, + Table, + create_engine, + insert, + select, +) + +from snowflake.sqlalchemy import URL + +from .parameters import CONNECTION_PARAMETERS + + +def test_insert_with_identifier(): + metadata = MetaData() + table = Table( + "table_1745924", + metadata, + Column("ca", Integer), + Column("cb", String), + Column("_", String), + ) + + engine = create_engine(URL(**CONNECTION_PARAMETERS)) + + try: + metadata.create_all(engine) + + with engine.connect() as connection: + connection.execute(insert(table).values(ca=1, cb="test", _="test_")) + connection.execute( + insert(table).values({"ca": 2, "cb": "test", "_": "test_"}) + ) + result = connection.execute(select(table)).fetchall() + assert result == [ + (1, "test", "test_"), + (2, "test", "test_"), + ] + finally: + metadata.drop_all(engine) From 750c06b0d9f9e70ff2e5ffb0a25d1bdb977f106e Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Tue, 22 Oct 2024 17:05:36 +0200 Subject: [PATCH 2/7] mraba/underscore_column_id: more test cases and description.md update --- DESCRIPTION.md | 1 + src/snowflake/sqlalchemy/base.py | 1 + tests/test_compiler.py | 15 ++++++++++++ tests/test_quote_identifiers.py | 40 +++++++++++++++++++++++++------- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/DESCRIPTION.md b/DESCRIPTION.md index 909d52cf..8e481478 100644 --- a/DESCRIPTION.md +++ b/DESCRIPTION.md @@ -11,6 +11,7 @@ Source code is also available at: - (Unreleased) + - Fixed quoting of `_` as column name - Add support for dynamic tables and required options - Add support for hybrid tables - Fixed SAWarning when registering functions with existing name in default namespace diff --git a/src/snowflake/sqlalchemy/base.py b/src/snowflake/sqlalchemy/base.py index 318f0370..2d8267ae 100644 --- a/src/snowflake/sqlalchemy/base.py +++ b/src/snowflake/sqlalchemy/base.py @@ -112,6 +112,7 @@ AUTOCOMMIT_REGEXP = re.compile( r"\s*(?:UPDATE|INSERT|DELETE|MERGE|COPY)", re.I | re.UNICODE ) +# used for quoting identifiers ie. table names, column names, etc. ILLEGAL_INITIAL_CHARACTERS = frozenset({d for d in string.digits}.union({"_", "$"})) """ diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 55451c2f..7a0735ff 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -33,6 +33,21 @@ def test_now_func(self): dialect="snowflake", ) + def test_dot_as_valid_identifier(self): + _table = table( + "table_1745924", + column("ca", Integer), + column("cb", String), + column(".", String), + ) + + stmt = insert(_table).values({"ca": 1, "cb": "test", ".": "test_"}) + self.assert_compile( + stmt, + 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(_)s)', + dialect="snowflake", + ) + def test_underscore_as_valid_identifier(self): _table = table( "table_1745924", diff --git a/tests/test_quote_identifiers.py b/tests/test_quote_identifiers.py index c78dbcaa..7a584551 100644 --- a/tests/test_quote_identifiers.py +++ b/tests/test_quote_identifiers.py @@ -1,6 +1,6 @@ # # Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. - +import pytest from sqlalchemy import ( Column, Integer, @@ -16,15 +16,35 @@ from .parameters import CONNECTION_PARAMETERS +# https://docs.snowflake.com/en/sql-reference/identifiers-syntax#double-quoted-identifiers +DOUBLE_QUOTE_IDENTIFIERS = {i for i in ".'!@#$%^&*"} + -def test_insert_with_identifier(): +@pytest.mark.parametrize( + "identifier", + ( + pytest.param(".", id="dot"), + pytest.param("'", id="single_quote"), + pytest.param("_", id="underscore"), + pytest.param("!", id="exclamation"), + pytest.param("@", id="at"), + pytest.param("#", id="hash"), + pytest.param("$", id="dollar"), + pytest.param("%", id="percent"), + pytest.param("^", id="caret"), + pytest.param("&", id="ampersand"), + pytest.param("*", id="asterisk"), + ), +) +def test_insert_with_identifier_as_column_name(identifier: str): + expected_identifier = f"test: {identifier}" metadata = MetaData() table = Table( "table_1745924", metadata, Column("ca", Integer), Column("cb", String), - Column("_", String), + Column(identifier, String), ) engine = create_engine(URL(**CONNECTION_PARAMETERS)) @@ -33,14 +53,16 @@ def test_insert_with_identifier(): metadata.create_all(engine) with engine.connect() as connection: - connection.execute(insert(table).values(ca=1, cb="test", _="test_")) connection.execute( - insert(table).values({"ca": 2, "cb": "test", "_": "test_"}) + insert(table).values( + { + "ca": 1, + "cb": "test", + identifier: f"test: {identifier}", + } + ) ) result = connection.execute(select(table)).fetchall() - assert result == [ - (1, "test", "test_"), - (2, "test", "test_"), - ] + assert result == [(1, "test", expected_identifier)] finally: metadata.drop_all(engine) From 1fff590820667aa49b0545250e2d2f54741ec892 Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Tue, 22 Oct 2024 17:21:05 +0200 Subject: [PATCH 3/7] mraba/underscore_column_id: after rebase fix --- src/snowflake/sqlalchemy/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/snowflake/sqlalchemy/base.py b/src/snowflake/sqlalchemy/base.py index 2d8267ae..9c29695a 100644 --- a/src/snowflake/sqlalchemy/base.py +++ b/src/snowflake/sqlalchemy/base.py @@ -5,6 +5,7 @@ import itertools import operator import re +import string from typing import List from sqlalchemy import exc as sa_exc From 1c2d22d46209b8bde0f44978b03cd95ec21501cc Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Tue, 22 Oct 2024 17:46:06 +0200 Subject: [PATCH 4/7] mraba/underscore_column_id: test fix --- tests/test_quote_identifiers.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/test_quote_identifiers.py b/tests/test_quote_identifiers.py index 7a584551..aa5df491 100644 --- a/tests/test_quote_identifiers.py +++ b/tests/test_quote_identifiers.py @@ -16,25 +16,10 @@ from .parameters import CONNECTION_PARAMETERS -# https://docs.snowflake.com/en/sql-reference/identifiers-syntax#double-quoted-identifiers -DOUBLE_QUOTE_IDENTIFIERS = {i for i in ".'!@#$%^&*"} - @pytest.mark.parametrize( "identifier", - ( - pytest.param(".", id="dot"), - pytest.param("'", id="single_quote"), - pytest.param("_", id="underscore"), - pytest.param("!", id="exclamation"), - pytest.param("@", id="at"), - pytest.param("#", id="hash"), - pytest.param("$", id="dollar"), - pytest.param("%", id="percent"), - pytest.param("^", id="caret"), - pytest.param("&", id="ampersand"), - pytest.param("*", id="asterisk"), - ), + (pytest.param("_", id="underscore"),), ) def test_insert_with_identifier_as_column_name(identifier: str): expected_identifier = f"test: {identifier}" From 5c55f1342a59c48a93248a0022eefb06ebe03fb0 Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Tue, 22 Oct 2024 18:04:37 +0200 Subject: [PATCH 5/7] mraba/underscore_column_id: test fix 2 --- tests/test_compiler.py | 2 +- tests/test_quote_identifiers.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 7a0735ff..80077e1f 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -44,7 +44,7 @@ def test_dot_as_valid_identifier(self): stmt = insert(_table).values({"ca": 1, "cb": "test", ".": "test_"}) self.assert_compile( stmt, - 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(_)s)', + 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(.)s)', dialect="snowflake", ) diff --git a/tests/test_quote_identifiers.py b/tests/test_quote_identifiers.py index aa5df491..741741a4 100644 --- a/tests/test_quote_identifiers.py +++ b/tests/test_quote_identifiers.py @@ -19,7 +19,10 @@ @pytest.mark.parametrize( "identifier", - (pytest.param("_", id="underscore"),), + ( + pytest.param("_", id="underscore"), + pytest.param(".", id="dot"), + ), ) def test_insert_with_identifier_as_column_name(identifier: str): expected_identifier = f"test: {identifier}" From 678a1d9688e8f4d6f5cb610d53373aee41fe8b99 Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Wed, 23 Oct 2024 08:29:25 +0200 Subject: [PATCH 6/7] mraba/underscore_column_id: test fix 3 --- tests/test_compiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 80077e1f..7a0735ff 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -44,7 +44,7 @@ def test_dot_as_valid_identifier(self): stmt = insert(_table).values({"ca": 1, "cb": "test", ".": "test_"}) self.assert_compile( stmt, - 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(.)s)', + 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(_)s)', dialect="snowflake", ) From c791afbc2410eeb0fff216f1ffb58384639e7beb Mon Sep 17 00:00:00 2001 From: Marcin Raba Date: Wed, 23 Oct 2024 15:47:32 +0200 Subject: [PATCH 7/7] mraba/underscore_column_id: test fix 3 --- tests/test_compiler.py | 15 --------------- tests/test_quote_identifiers.py | 27 +++++++-------------------- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 7a0735ff..55451c2f 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -33,21 +33,6 @@ def test_now_func(self): dialect="snowflake", ) - def test_dot_as_valid_identifier(self): - _table = table( - "table_1745924", - column("ca", Integer), - column("cb", String), - column(".", String), - ) - - stmt = insert(_table).values({"ca": 1, "cb": "test", ".": "test_"}) - self.assert_compile( - stmt, - 'INSERT INTO table_1745924 (ca, cb, ".") VALUES (%(ca)s, %(cb)s, %(_)s)', - dialect="snowflake", - ) - def test_underscore_as_valid_identifier(self): _table = table( "table_1745924", diff --git a/tests/test_quote_identifiers.py b/tests/test_quote_identifiers.py index 741741a4..58575fad 100644 --- a/tests/test_quote_identifiers.py +++ b/tests/test_quote_identifiers.py @@ -1,20 +1,9 @@ # # Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. -import pytest -from sqlalchemy import ( - Column, - Integer, - MetaData, - String, - Table, - create_engine, - insert, - select, -) - -from snowflake.sqlalchemy import URL -from .parameters import CONNECTION_PARAMETERS +# Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. +import pytest +from sqlalchemy import Column, Integer, MetaData, String, Table, insert, select @pytest.mark.parametrize( @@ -24,7 +13,7 @@ pytest.param(".", id="dot"), ), ) -def test_insert_with_identifier_as_column_name(identifier: str): +def test_insert_with_identifier_as_column_name(identifier: str, engine_testaccount): expected_identifier = f"test: {identifier}" metadata = MetaData() table = Table( @@ -35,12 +24,10 @@ def test_insert_with_identifier_as_column_name(identifier: str): Column(identifier, String), ) - engine = create_engine(URL(**CONNECTION_PARAMETERS)) - try: - metadata.create_all(engine) + metadata.create_all(engine_testaccount) - with engine.connect() as connection: + with engine_testaccount.connect() as connection: connection.execute( insert(table).values( { @@ -53,4 +40,4 @@ def test_insert_with_identifier_as_column_name(identifier: str): result = connection.execute(select(table)).fetchall() assert result == [(1, "test", expected_identifier)] finally: - metadata.drop_all(engine) + metadata.drop_all(engine_testaccount)