Skip to content

gh-111789: Use PyDict_GetItemRef() in sqlite - #111829

Merged
serhiy-storchaka merged 4 commits into
python:mainfrom
serhiy-storchaka:use-dict_getitemref-in-sqlite
Nov 10, 2023
Merged

gh-111789: Use PyDict_GetItemRef() in sqlite#111829
serhiy-storchaka merged 4 commits into
python:mainfrom
serhiy-storchaka:use-dict_getitemref-in-sqlite

Conversation

@serhiy-storchaka

@serhiy-storchakaserhiy-storchaka commented Nov 7, 2023

Copy link
Copy Markdown
Member

Comment threadModules/_sqlite/cursor.c Outdated
current_param = PyObject_GetItem(parameters, binding_name_obj);
}
PyObject *current_param;
(void)PyMapping_GetOptionalItem(parameters, binding_name_obj, &current_param);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyMapping_GetOptionalItem() includes a special case for dict, so there is no performance loss.

@erlend-aaslanderlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks; the resulting code is cleaner, thus positively affecting readability and maintainability.

@serhiy-storchaka

Copy link
Copy Markdown
MemberAuthor

It can be even more simplified if use PyMapping_GetOptionalItemString().

@serhiy-storchaka
serhiy-storchaka enabled auto-merge (squash) November 10, 2023 20:29
@serhiy-storchaka
serhiy-storchaka merged commit afac3c9 into python:mainNov 10, 2023
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Use new C API functions PyDict_GetItemRef() and
PyMapping_GetOptionalItemString().
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Use new C API functions PyDict_GetItemRef() and
PyMapping_GetOptionalItemString().
@serhiy-storchaka
serhiy-storchaka deleted the use-dict_getitemref-in-sqlite branch July 1, 2026 16:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@serhiy-storchaka@erlend-aasland