fix #284: mising param handling for versioned_table statement(s) in positional_args_to_dict L485 in manager.py

[indiVar0508]

Hi,

Raising PR to address #282 , could you please review it and let know of feedback on comments in code or any other changes needed

Before Change pytest run summary

346 failed, 308 passed, 14 skipped, 5398 warnings, 90 errors in 439.71s

After Change pytest run summary

346 failed, 308 passed, 14 skipped, 5399 warnings, 90 errors in 422.88s

Thanks

[marksteward]

I'm not convinced by these fixes. I think the function needs rewriting really, as INSERT can take many forms under SQLA these days.

[indiVar0508]

separated the PR for #282 in #295

I'm not convinced by these fixes. I think the function needs rewriting really, as INSERT can take many forms under SQLA these days.

Was trying to make an attempt to write a new version for this method but have couple of doubts, so far i made WIP changes which look like this

    def positional_args_to_dict(self, op, statement, params, tablename):
        """
        On some drivers (eg sqlite) generated INSERT statements use positional
        args instead of key value dictionary. This method converts positional
        args to key value dict.

        :param statement: SQL statement string
        :param params: tuple or dict of statement parameters

        returns: dict with parameters
        """
        if isinstance(params, tuple) and params:
            parameters = {}
            table = self.metadata.tables[tablename]
            if op == Operation.DELETE:
                columns = table.primary_key.columns.values()  ## what happens if not all primary key are not used while making a delete query with SQLA
            else:
                columns = table.columns.values()
            for index, column in enumerate(columns):
                parameters[column] = params[index]
            return parameters
        return params if params else {}

I had a couple doubts

1. I am facing an issue which i think exists in existing function in master also, that if we try to delete an assoc table entry for one specific primary key and i have two or more primary key(s) defined in assoc table i get index out of bound error

book_author_table = Table( 'book_author', Base.metadata, Column('book_id', Integer, ForeignKey('book.id'), primary_key=True, nullable=False), Column('author_id', Integer, ForeignKey('author.id'), primary_key=True, nullable=False), ... ) book_author_table.delete(whereclause=book_author_table.book_id==XYZ)

parameters[column.name] = params[index] IndexError: tuple index out of range

and is it ok to always pull primary_key lookup to delete a association entry as a deletion can also happen based on create date or any other non-primary column e.g "Delete all assoc entries prior to 2020 or delete all assoc entries where event was = ABC"

2. another doubt that i had was existing method detects the table name using regex is it ok if we use the same table_name from track_association_operations for a given set of params.

[indiVar0508]

Closing this as #295 addresses this issue