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

Bindings type checking too strict #373

Closed
rogerbinns opened this issue Sep 27, 2022 · 1 comment
Closed

Bindings type checking too strict #373

rogerbinns opened this issue Sep 27, 2022 · 1 comment
Assignees

Comments

@rogerbinns
Copy link
Owner

Bindings for cursor.execute/executemany can conceptually either be a dictionary (lookup by key) or a sequence (lookup by position).

Currently PyDict_Check is used to determine if it is a dictionary, otherwise PySequence_Fast is used for positional access.

Because PyDict_Check only looks for dict subclasses, it means that UserDict won't work. The apparent solution of PyMapping_Check returns True for UserDict, but also for lists and tuples.

For reference, this is what the builtin _sqlite3 module does

Treat as not a dictionary if:

PyTuple_CheckExact(parameters) || PyList_CheckExact(parameters) 
|| (!PyDict_Check(parameters) && PySequence_Check(parameters))

Otherwise treat as dictionary if:

PyDict_Check(parameters)

Otherwise error

@rogerbinns
Copy link
Owner Author

Some tests with various types that shows we can't tell dict/list apart:

dict_check 0 sequence_check 1 mapping_check 1: collections.UserDict
dict_check 0 sequence_check 1 mapping_check 1: list()
dict_check 1 sequence_check 0 mapping_check 1: dict()
dict_check 0 sequence_check 1 mapping_check 1: class with only __getitem__

collections.abc.Mapping does seem to be useful. isinstance(T, collections.abc.Mapping)

True: dict
True: collections.UserDict
False: list
False: set
False: class with only __getitem__

It looks like this is the most useful, especially as custom classes can register with collections.abc.Mapping

@rogerbinns rogerbinns self-assigned this Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant