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

sqlite3.Row doesn't support slice indexes #57792

Closed
xapple mannequin opened this issue Dec 11, 2011 · 9 comments
Closed

sqlite3.Row doesn't support slice indexes #57792

xapple mannequin opened this issue Dec 11, 2011 · 9 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@xapple
Copy link
Mannequin

xapple mannequin commented Dec 11, 2011

BPO 13583
Nosy @pitrou, @serhiy-storchaka
Files
  • sqlrowslice.patch: Patch for implementing slices in sqlite3.Row
  • issue13583.patch
  • sqlite3_slicing_demo.py
  • sqlite3_row_slice.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-03-31.10:36:54.897>
    created_at = <Date 2011-12-11.21:45:47.766>
    labels = ['extension-modules', 'type-feature']
    title = "sqlite3.Row doesn't support slice indexes"
    updated_at = <Date 2015-03-31.10:36:54.896>
    user = 'https://bugs.python.org/xapple'

    bugs.python.org fields:

    activity = <Date 2015-03-31.10:36:54.896>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-03-31.10:36:54.897>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2011-12-11.21:45:47.766>
    creator = 'xapple'
    dependencies = []
    files = ['23919', '35070', '35071', '38379']
    hgrepos = []
    issue_num = 13583
    keywords = ['patch', 'needs review']
    message_count = 9.0
    messages = ['149251', '149871', '217343', '217344', '233840', '237462', '239172', '239680', '239681']
    nosy_count = 6.0
    nosy_names = ['ghaering', 'pitrou', 'jesstess', 'python-dev', 'xapple', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue13583'
    versions = ['Python 3.5']

    @xapple
    Copy link
    Mannequin Author

    xapple mannequin commented Dec 11, 2011

    When using the sqlite3.Row object as a row factory, one can access the resulting rows by index (such as row[1]) or by name (such as row['b']). However, the slice functionality is lost, as doing row[0:2] raises the error:

    "slices not implemented, yet"

    Here is a patch that fixes this, I implemented it and I added the corresponding unit test.

    @xapple xapple mannequin added the type-feature A feature request or enhancement label Dec 11, 2011
    @pitrou
    Copy link
    Member

    pitrou commented Dec 19, 2011

    Thanks for the patch. Two things:

    • there is a compilation warning using gcc:

    /home/antoine/cpython/default/Modules/_sqlite/row.c: In function ‘pysqlite_row_subscript’:
    /home/antoine/cpython/default/Modules/_sqlite/row.c:128:26: attention : passing argument 1 of ‘PySlice_GetIndicesEx’ from incompatible pointer type

    • you can use assertEqual to avoid defining the error message yourself

    @jesstess
    Copy link
    Member

    Thanks for the ticket and patch, xapple!

    I updated the patch to address the compiler warning and use assertEqual.

    While testing, I noticed that slicing with steps wasn't supported, so I expanded the sqlite3.Row slicing code to support steps, and added some additional tests.

    The slicing code in Modules/_sqlite/row.c:pysqlite_row_subscript is unfortunately pretty redundant with the slicing code in Objects/tupleobject.c. It'd be better to either be able to factor the code from both into a function (but I couldn't see how to do this without making it part of the public API), or have tuple, sqlite.Row, etc. implement a shared slicing interface. Perhaps we should defer that decision to a future ticket, though.

    Note that even after this patch, sqlite.Row instances don't support negative indices.

    • This patch passes make patchcheck.
    • The full test suite passes with this patch.
    • There are no build warnings related to the patch.

    @jesstess
    Copy link
    Member

    I've also uploaded a short script that sets up an in-memory sqlite database that fetches Rows, for easy manual testing.

    @ghaering ghaering mannequin self-assigned this Jan 11, 2015
    @serhiy-storchaka
    Copy link
    Member

    May be just use PyObject_GetItem(self->data, idx)?

    @serhiy-storchaka
    Copy link
    Member

    Here is a patch with much simpler implementation.

    @serhiy-storchaka serhiy-storchaka added the extension-modules C modules in the Modules dir label Mar 7, 2015
    @serhiy-storchaka
    Copy link
    Member

    Could you look at the patch Gerhard?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 31, 2015

    New changeset e47f520eb756 by Serhiy Storchaka in branch 'default':
    Issue bpo-13583: sqlite3.Row now supports slice indexing.
    https://hg.python.org/cpython/rev/e47f520eb756

    @serhiy-storchaka
    Copy link
    Member

    The implementation can be much simpler, but in any case thank you for your patches Lucas and Jessica.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants