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

adbc.snowflake.statement.* options are not effective ["python"] #2146

Open
davlee1972 opened this issue Sep 9, 2024 · 6 comments
Open

adbc.snowflake.statement.* options are not effective ["python"] #2146

davlee1972 opened this issue Sep 9, 2024 · 6 comments
Labels
Type: bug Something isn't working

Comments

@davlee1972
Copy link

davlee1972 commented Sep 9, 2024

What happened?

I've tried changing adbc.snowflake.statement options, but none of the settings appear to have any effect with adbc_ingest().

I've tried the following:

"adbc.snowflake.statement.ingest_copy_concurrency": "0",
"adbc.snowflake.statement.ingest_copy_concurrency": "1",

"adbc.snowflake.statement.ingest_target_file_size": "100mb",
"adbc.snowflake.statement.ingest_target_file_size": "100",
"adbc.snowflake.statement.ingest_target_file_size": "100000000",

But the result always ends up with ~120 PUTs with concurrent overlapping COPY INTO(s) running in parallel..

Changing any of the above to int(s) like:
"adbc.snowflake.statement.ingest_copy_concurrency": 0,
results in an error which is expected, so the parameters are being checked, but they just don't impact anything..

  File "../miniconda3/lib/python3.9/site-packages/adbc_driver_snowflake/__init__.py", line 156, in connect
    return adbc_driver_manager.AdbcDatabase(driver=_driver_path(), **kwargs)
  File "adbc_driver_manager/_lib.pyx", line 482, in adbc_driver_manager._lib.AdbcDatabase.__init__
  File "adbc_driver_manager/_lib.pyx", line 276, in adbc_driver_manager._lib._to_bytes
ValueError: value must be str or bytes

I'm also passing a recordbatchreader into adbc_ingest()

Stack Trace

No response

How can we reproduce the bug?

No response

Environment/Setup

No response

@davlee1972 davlee1972 added the Type: bug Something isn't working label Sep 9, 2024
@davlee1972 davlee1972 changed the title adbc.snowflake.statement.* options aren't not effective ["python"] adbc.snowflake.statement.* options are not effective ["python"] Sep 10, 2024
@zeroshade
Copy link
Member

@joellubi can you take a look at this when you get a chance? thanks!

@joellubi
Copy link
Member

@davlee1972 Can you please share the code you used to pass in those parameters?

Based on the traceback the error is occurring during database initialization so my guess is those parameters are being provided as database kwargs. The ingestion parameters are defined on the statement rather than the database. They can be set as follows:

with adbc_driver_snowflake.dbapi.connect(uri) as conn, conn.cursor() as cur:
    cur.adbc_statement.set_options(**kwargs)
    cur.adbc_ingest(...)

@joellubi
Copy link
Member

FWIW I do think the way parameters handling works right now is kind of awkward. I would be for cleaning it up some way if it can help make this more intuitive.

@davlee1972
Copy link
Author

Ok, I'll try that, but it is a kinda awkward. The documentation isn't clear that these are adbc_statement options as part of the cursor since the options are show right after the database kwargs sample.

image

@joellubi
Copy link
Member

I think we should update the documentation to make this clearer, and perhaps make the interface more consistent.

@lidavidm Do you know if adding optional parameters to Connection.cursor() would be a problem for the dbapi spec?

I'm thinking for consistency to allow stmt_kwargs to be specified similar to how the db and conn kwargs already are:

with adbc_driver_snowflake.dbapi.connect(uri, db_kwargs=db_kwargs, conn_kwargs=conn_kwargs) as conn:
    with conn.cursor(stmt_kwargs=stmt_kwargs) as cur:
        cur.adbc_ingest(...)

Thoughts on this?

CC: @zeroshade

@lidavidm
Copy link
Member

I think it would be OK only as a keyword-only argument like adbc_stmt_kwargs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants