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

table.convert() method should clean up after itself #323

Closed
simonw opened this issue Aug 25, 2021 · 1 comment
Closed

table.convert() method should clean up after itself #323

simonw opened this issue Aug 25, 2021 · 1 comment
Labels
wontfix This will not be worked on

Comments

@simonw
Copy link
Owner

simonw commented Aug 25, 2021

It currently works like this:

def convert_value(v):
bar.update(1)
if not v:
return v
return fn(v)
self.db.register_function(convert_value)
sql = "update [{table}] set {sets}{where};".format(
table=self.name,
sets=", ".join(
[
"[{output_column}] = convert_value([{column}])".format(
output_column=output or column, column=column
)
for column in columns
]
),
where=" where {}".format(where) if where is not None else "",
)

It's registering a function called convert_value() and then failing to de-register that function once it has finished.

It might even be possible for two queries running against the same connection to clobber each other's convert_value() functions, leading to incorrect behaviour.

So two fixes: firstly it should register the function with a unique name (maybe add a random suffix). Secondly, it should de-register that function once it has finished.

@simonw simonw added the bug Something isn't working label Aug 25, 2021
@simonw simonw changed the title table.convert() method should clean up after itself table.convert() method should clean up after itself Aug 25, 2021
@simonw
Copy link
Owner Author

simonw commented Aug 25, 2021

As far as I can tell the Python sqlite3 module doesn't actually have a mechanism for de-registering a custom SQL function.

This means that if I implement a mechanism whereby each call to .convert() registers a new SQL function with a random suffix (convert_value_23424() for example) those functions will stay registered - and if .convert() is called a large number of times the number of obsolete custom function registrations will grow without bounds.

For that reason, I'm going to wontfix this issue.

@simonw simonw closed this as completed Aug 25, 2021
@simonw simonw added wontfix This will not be worked on and removed bug Something isn't working labels Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant