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

Error message: sqlite3.ProgrammingError: You did not supply a value for binding # might be improved #85804

Closed
WolfgangFahl mannequin opened this issue Aug 26, 2020 · 17 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir

Comments

@WolfgangFahl
Copy link
Mannequin

WolfgangFahl mannequin commented Aug 26, 2020

BPO 41638
Nosy @serhiy-storchaka, @erlend-aasland, @WolfgangFahl
PRs
  • bpo-41638: Improve ProgrammingError message for absent parameter. #21999
  • 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 = None
    closed_at = <Date 2021-04-02.06:04:06.888>
    created_at = <Date 2020-08-26.06:16:55.379>
    labels = ['library', '3.10']
    title = 'Error message: sqlite3.ProgrammingError: You did not supply a value for binding # might be improved'
    updated_at = <Date 2021-04-02.06:04:06.888>
    user = 'https://github.com/WolfgangFahl'

    bugs.python.org fields:

    activity = <Date 2021-04-02.06:04:06.888>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-04-02.06:04:06.888>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2020-08-26.06:16:55.379>
    creator = 'WolfgangFahl'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41638
    keywords = ['patch']
    message_count = 17.0
    messages = ['375906', '375907', '375908', '375910', '375911', '375913', '375914', '375915', '375916', '375923', '376050', '376056', '376058', '376064', '376392', '390013', '390042']
    nosy_count = 4.0
    nosy_names = ['serhiy.storchaka', 'EvensF', 'erlendaasland', 'WolfgangFahl']
    pr_nums = ['21999']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue41638'
    versions = ['Python 3.10']

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    def testBindingError(self):
            '''
            test list of Records with incomplete record leading to
            "You did not supply a value for binding 2"
            '''
            listOfRecords=[{'name':'Pikachu', 'type':'Electric'},{'name':'Raichu' }]
            resultList=self.checkListOfRecords(listOfRecords,'Pokemon','name')

    Which eventually will call:

    insertCmd=entityInfo.insertCmd
    self.c.executemany(insertCmd,listOfRecords)
    self.c.commit()

    leading to the error message:

    sqlite3.ProgrammingError: You did not supply a value for binding 2.

    When many thousand records are inserted this message is not very helpful.

    you might want to improve it to:
    sqlite3.ProgrammingError: You did not supply a value for binding 2 ("type") in record #2 with a debug option that shows the actual date like:
    sqlite3.ProgrammingError: You did not supply a value for binding 2 ("type") in record #2 debuginfo: name="Raichu", type=missing

    sqlite3.ProgrammingError: You did not supply a value for binding 2.

    @WolfgangFahl WolfgangFahl mannequin added 3.10 only security fixes stdlib Python modules in the Lib dir labels Aug 26, 2020
    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    My workaround starts with:
    try:
    self.c.executemany(insertCmd,listOfRecords)
    self.c.commit()
    except sqlite3.ProgrammingError as pe:
    msg=pe.args[0]
    if "You did not supply a value for binding" in msg:
    columnIndex=int(re.findall(r'\d+',msg)[0])
    columnName=list(entityInfo.typeMap.keys())[columnIndex-1]
    raise Exception("%s\nfailed: no value supplied for column '%s'" % (insertCmd,columnName))
    else:
    raise pe

    which gives me:

    Exception: INSERT INTO Pokemon (name,type) values (:name,:type)
    failed: no value supplied for column 'type'

    but not the data yet. So i am now forced to implement another insert that does not use executemany (which i like a lot) just to get proper debug information - this is a pitty.

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    The full workaround is in WolfgangFahl/DgraphAndWeaviateTest@f1a58d7

    def testBindingError(self):
            '''
            test list of Records with incomplete record leading to
            "You did not supply a value for binding 2"
            see https://bugs.python.org/issue41638
            '''
            listOfRecords=[{'name':'Pikachu', 'type':'Electric'},{'name':'Raichu' }]
            for executeMany in [True,False]:
                try:
                    self.checkListOfRecords(listOfRecords,'Pokemon','name',executeMany=executeMany)
                    self.fail("There should be an exception")
                except Exception as ex:
                    if self.debug:
                        print(str(ex))
                    self.assertTrue('no value supplied for column' in str(ex))   

    Giving the error messages:

    INSERT INTO Pokemon (name,type) values (:name,:type)
    failed: no value supplied for column 'type'

    in mode "executeMany"

    INSERT INTO Pokemon (name,type) values (:name,:type)
    failed: no value supplied for column 'type'
    record #2={'name': 'Raichu'}

    if executeMany is not used and errorDebug is on

    The wrapper code is:

    def store(self,listOfRecords,entityInfo,executeMany=False):
            '''
            store the given list of records based on the given entityInfo
            
            Args:
              
               listOfRecords(list): the list of Dicts to be stored
               entityInfo(EntityInfo): the meta data to be used for storing
            '''
            insertCmd=entityInfo.insertCmd
            try:
                if executeMany:
                    self.c.executemany(insertCmd,listOfRecords)
                else:
                    index=0
                    for record in listOfRecords:
                        index+=1
                        self.c.execute(insertCmd,record)
                self.c.commit()
            except sqlite3.ProgrammingError as pe:
                msg=pe.args[0]
                if "You did not supply a value for binding" in msg:
                    columnIndex=int(re.findall(r'\d+',msg)[0])
                    columnName=list(entityInfo.typeMap.keys())[columnIndex-1]
                    debugInfo=""
                    if not executeMany:
                        if self.errorDebug:
                            debugInfo="\nrecord  #%d=%s" % (index,repr(record))
                    raise Exception("%s\nfailed: no value supplied for column '%s'%s" % (insertCmd,columnName,debugInfo))
                else:
                    raise pe

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 26, 2020

    Note that there is also another error:

    sqlite3.InterfaceError: Error binding parameter :series - probably unsupported type.

    with similar issue but which actively shows the binding name (but not the record# and debugInfo ...

    @EvensF
    Copy link
    Mannequin

    EvensF mannequin commented Aug 28, 2020

    I don't think this is a bug. As I have explained in the answer I have provided on StackOverflow (https://stackoverflow.com/posts/comments/112539410?noredirect=1), it seems that sqlite3 uses by default the qmark parameter substitution style for their query which uses a tuple to provide it with its values. So it makes sense that the error message only does reference the index number since the matching is done on the order where a parameter appear on the SQL query and the same order in the tuple provided.

    Here @wolfgang seems to try to use the named parameter substitution style because he is using a dictionary to provide the values to the query. You have to set it globally before doing that (sqlite3.paramstyle = 'named').

    So from my point of view there is no bug here.

    @WolfgangFahl
    Copy link
    Mannequin Author

    WolfgangFahl mannequin commented Aug 29, 2020

    Indeed this a request for improvement. Technically the software works as expected. Just thousands of programmers have extra debug effort at this tate of affairs as can be deducted from the view count in the stackoverflow questions. The extra step of looking up the column name from the binding number can be avoided as i have shown by my workaround. To hide the detail of

    set it globally before doing that (sqlite3.paramstyle = 'named').

    here in the bug report is not helpful. Part of the request is also to show the record number in executeMany - just to make lifer easier for users for sqlite3

    @EvensF
    Copy link
    Mannequin

    EvensF mannequin commented Aug 29, 2020

    Yes we could say that the documentation can be lacking but that doesn't means it's a bug in the code. If you feel that the documentation need to be improved then you should submit a patch to the documentation and not how the code should be changed.

    If you feel that the error message doesn't give enough information you could modify the source code of that module and submit a patch instead of a workaround.

    But meanwhile I would suggest that your code should check that all the information is valid before you submit it to the database. It shouldn't be really the responsibility of the database to catch this kind of errors.

    @serhiy-storchaka
    Copy link
    Member

    I agree that the error message should contain the name of the absent parameter instead of its index when parameters are supplied as a dict. It is more informative and would consistent with other error message.

    As for including the number of a record in the error message, I am not sure we should do this.

    1. For other errors (too large integer, string containing surrogate characters, error in custom adapter, etc) the error message does not contain neither parameter name nor index. If we want to attach references to parameter and record, it is much more larger problem. It may require designing a general method for attaching additional information to exceptions and writing a PEP. It is a LARGE problem.

    2. ProgrammingError is a programming error. In correctly working program you should never see such kind of errors, because you are responsible for preparing the statement and providing the consistent number of parameter values.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 8171580 by Serhiy Storchaka in branch 'master':
    bpo-41638: Improve ProgrammingError message for absent parameter. (GH-21999)
    8171580

    @erlend-aasland
    Copy link
    Contributor

    Can this be closed, Serhiy?

    @serhiy-storchaka
    Copy link
    Member

    Sure. Thanks for the reminder.

    @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
    3.10 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants