Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execute_stream does not remove empty lines with comment only resulting in error: Empty SQL statement. #2132

Open
sfc-gh-melnacouzi opened this issue Jan 13, 2025 · 4 comments
Assignees
Labels
bug status-triage_done Initial triage done, will be further handled by the driver team

Comments

@sfc-gh-melnacouzi
Copy link

sfc-gh-melnacouzi commented Jan 13, 2025

Python version

Python 3.10.0 (default, Mar 3 2022, 03:57:21) [Clang 12.0.0 ]

Operating system and processor architecture

macOS-10.16-x86_64-i386-64bit

Installed packages

annotated-types==0.7.0
anyio==4.8.0
asn1crypto==1.5.1
atpublic==5.0
backports.tarfile==1.2.0
black==24.10.0
certifi==2024.12.14
cffi==1.17.1
cfgv==3.4.0
chardet==5.2.0
charset-normalizer==3.4.1
click==8.1.7
cloudpickle==2.2.1
coverage==7.6.10
cryptography==44.0.0
diff_cover==9.2.1
distlib==0.3.9
exceptiongroup==1.2.2
factory_boy==3.3.1
Faker==33.1.0
filelock==3.16.1
gitdb==4.0.12
GitPython==3.1.43
h11==0.14.0
hatch==1.14.0
hatchling==1.27.0
httpcore==1.0.7
httpx==0.28.1
hyperlink==21.0.0
identify==2.6.5
idna==3.10
importlib_metadata==8.5.0
iniconfig==2.0.0
jaraco.classes==3.4.0
jaraco.context==6.0.1
jaraco.functools==4.1.0
Jinja2==3.1.5
jinja2schema==0.1.4
keyring==25.6.0
markdown-it-py==3.0.0
MarkupSafe==3.0.2
mdurl==0.1.2
more-itertools==10.5.0
mypy==1.14.1
mypy-extensions==1.0.0
nodeenv==1.9.1
packaging==24.2
pathspec==0.12.1
pexpect==4.9.0
platformdirs==4.3.6
pluggy==1.5.0
pre_commit==4.0.1
protobuf==5.29.2
ptyprocess==0.7.0
pycparser==2.22
pydantic==2.9.2
pydantic_core==2.23.4
Pygments==2.19.1
PyJWT==2.10.1
pyOpenSSL==24.3.0
pytest==8.3.4
pytest-cov==6.0.0
pytest-randomly==3.16.0
python-dateutil==2.9.0.post0
pytz==2024.2
PyYAML==6.0.2
requests==2.32.3
requirements-parser==0.11.0
rich==13.9.4
shellingham==1.5.4
six==1.17.0
smmap==5.0.2
sniffio==1.3.1
-e git+ssh://git@github.com/snowflakedb/snowflake-cli.git@8cc48be4534a5531b9ff6f4a82aebdc891e94239#egg=snowflake_cli
snowflake-connector-python==3.12.4
snowflake-snowpark-python==1.25.0
snowflake.core==1.0.2
sortedcontainers==2.4.0
syrupy==4.8.0
tomli==2.2.1
tomli_w==1.1.0
tomlkit==0.13.2
trove-classifiers==2025.1.7.14
typer==0.12.5
types-setuptools==75.6.0.20241223
typing_extensions==4.12.2
tzlocal==5.2
urllib3==2.2.3
userpath==1.9.2
uv==0.5.16
virtualenv==20.28.1
zipp==3.21.0
zstandard==0.23.0

What did you do?

Try this stream with execute_stream() in connection.py:

-- middle comment;
select 1;  select 2; -- comment ; still comment
-- extra comment;
-- extra comment2;"

Basically anything with a comment at the end of the stream will cause this error:

Empty SQL statement.

If you look closely at the code, it seems that the issue is in split_statements() call, which is resulting in:

[('-- middle comment;\nselect 1;', False), ('select 2; -- comment ; still comment', False), ('-- extra comment;\n-- extra comment2;', None)]

It splits the statements correctly, except that the last statement is not really a statement as it includes comments only.

What did you expect to see?

Expected for the code to execute successfully.

Can you set logging to DEBUG and collect the logs?

import logging
import os

for logger_name in ('snowflake.connector',):
    logger = logging.getLogger(logger_name)
    logger.setLevel(logging.DEBUG)
    ch = logging.StreamHandler()
    ch.setLevel(logging.DEBUG)
    ch.setFormatter(logging.Formatter('%(asctime)s - %(threadName)s %(filename)s:%(lineno)d - %(funcName)s() - %(levelname)s - %(message)s'))
    logger.addHandler(ch)
@sfc-gh-sghosh
Copy link

Hello @sfc-gh-melnacouzi ,

Thanks for raising the issue.
We are able to reproduce the issue. will look into it.

Meanwhile as a workaround, you can either use parameter remove_comments=True or remove the empty lines from the multi SQL statement before processing.

Example: using remove_comments=True

`sql_script = """
-- This is a middle comment;
select count() from mycsvtable;
select count(
) from mycsvtable1;
-- Comment still comment;
-- Extra comment;
-- Another comment;
"""

try:

sql_stream = StringIO(sql_script)


with conn.cursor() as cur:
    for result_cursor in conn.execute_stream(sql_stream,remove_comments=True):
        for result in result_cursor:
            print(f"Result: {result}")

finally:
conn.close()`

Exmaple : by removing the empty lines

`sql_script = """
-- This is a middle comment;
select count() from mycsvtable;
select count(
) from mycsvtable1;
-- Comment still comment;
-- Extra comment;
-- Another comment;
"""

try:
cleaned_script = "\n".join(
line for line in sql_script.splitlines()
if line.strip() and not line.strip().startswith("--")
)

sql_stream = StringIO(cleaned_script)


with conn.cursor() as cur:
    for result_cursor in conn.execute_stream(sql_stream,remove_comments=False):
        for result in result_cursor:
            print(f"Result: {result}")

finally:
conn.close()`

@sfc-gh-sghosh sfc-gh-sghosh added status-triage_done Initial triage done, will be further handled by the driver team and removed needs triage labels Jan 16, 2025
@sfc-gh-melnacouzi
Copy link
Author

Thank you @sfc-gh-sghosh , we are aware of the workaround - I wanted to log this for other users so they don't get into the same issue.

@sfc-gh-sghosh
Copy link

Hello @sfc-gh-melnacouzi ,

We discussed this internally with the team and concluded that it's not recommended to use the feature; the recommendation is either to parse the SQL string before using it or please use it with the parameter remove_commetns=True. We can only improve this feature, fixing this will result in another example that will break it, so the recommendations if you want to use this feature please use with remove_comments=True or else please parse the SQL string before using it.

Regards,
Sujan

@sfc-gh-melnacouzi
Copy link
Author

Thanks Sujan. If remove_comments has to be True for the function to function properly, I suggest always removing the comments. Otherwise, this could cause issues for other users as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants