Deprecate default (built-in) sqlite3 converters and adapters

There’s been numerous issues about the built-in (or default) date and timestamp converters and adapters in the sqlite3 module; they are buggy, and it is difficult to fix them, because it will break existing code. The only sensible thing to do seems to be to deprecate the default converters and adapters and just let people write their own[1]. Now, early in a new development cycle, is a perfect time for deprecations :slight_smile: I’m not sure we want to remove them, though, so for now I’m inclined to just mark them deprecated.

:+1: / :-1: ?

See also:


  1. We can also improve the docs with a short guide on how to write a datetime converter, how to write a timestamp adapter, etc. ↩︎

3 Likes

As I’ve already mentioned on a couple of those tickets:

  • I’m +1 on deprecating the default converters and have users implement their own (which work according to their needs).

  • I’m -1 on removing the possibility to register converters and adapters, since these are useful features for cases where the default implementation does not cover the use cases you want to implement in your code, e.g. when working with user defined data types.

3 Likes

I’m not aware of these issues and… too lazy to dig into your list of issues. Would you mind to summarize issues of existing built-in converters? If you requires users to write their own converters, please add a migration documentation somewhere. Maybe even suggest an implementation in the documentation?

Would it make sense to add converters for the most common types (datetime?) in sqlite3? Sorry, I don’t know this module, maybe they already exist. I’m not suggesting to register them by default, but provide them and let users to register them manually.

To keep it short, they do not play well with timezones/UTC offsets. Note, the docs already says:

The default “timestamp” converter ignores UTC offsets in the database and always returns a naive datetime.datetime object. To preserve UTC offsets in timestamps, either leave converters disabled, or register an offset-aware converter with register_converter() .

Definitely. Three of the four default adapters/converters are simply one-liners. Adding these snippets to the docs should be sufficient for users to adapt them to their own needs. Kind of like the recipes of the itertools docs.

That is what we currently have, and this is what is creating headaches for us :slight_smile:

5 Likes

It is difficult to write a correct converter for datetime from zero. I propose to add several new standard datetime converters, partially compatible with the current one (return a naive datetime object, a local time, a UTC time, etc) or a fabric of converters, and let the user to chose which of them to register.

Of course, the current default converter should be deprecated. It cannot be fixed without breaking someone’s code anyway, so it is better to break all code and force users to be explicit.

In the face of ambiguity, refuse the temptation to guess.

How about providing recipes for such converters in the docs, like the itertools module does it?

To echo @storchaka 's point, if you’re going to provide recipes, what’s the downside of just implementing them directly with some mechanism to register them as default instead of the current ones (if I’m understanding how things work correctly)? Seems like it would be easier and less painful to convince users to switch, less chance of users making a mistake, and everyone benefits if the converter is improved to fix a defect or extended to cover a new use case (unless that’s impossible by design)?

1 Like

@erlendaasland’s call, but I think if we’re prepared to provide recipes then we may as well add them in with a couple of tests. Possibly as well as offering recipes.

It likely comes down to how stable these converters have to be. If, for a given database, they should absolutely never change under any circumstances, then we want to encourage users to copy them into their own codebase so that we can make bugfixes. But if we understand the scope well enough and believe that bugfixes will always be improvements for all existing data, then we wouldn’t need to encourage that.

3 Likes

The problem with such converters is that they will always just implement a single use case and not necessarily the one which the user needs. As in your examples, we’d need to add many of those to address just a bunch of cases and where would this end ?

Once we’ve added them, we’d run into the same situation as with the existing ones: If the implementations are missing out on important use cases, we would not be able to fix them.

IMO, the itertools approach with providing some recipes makes more sense.

And lastly: every such converter or adapter introduces a possible incompatibility with other code in your application which also uses the module, but doesn’t expect a particular data type to be converted in the same way. These converters are global and not per connection, so care has to be taken when using the feature.

3 Likes

My concerns are perfectly worded in MAL’s post:

As I already wrote in the issue (gh-90016), I think the best path forward is:

  1. Improve the docs regarding converters and adapters (introducing recipes, etc.)
  2. Deprecate the existing converters[1]
  3. Consider providing some of the recipes as “built-in” converters/adapters, but not enable them by default.[2]

  1. we still have to keep them enabled by default, though, in order to not break existing code ↩︎

  2. I’m currently -1 on this ↩︎

2 Likes

Recipes would work too. But I afraid, that correct implementation of some of these reciepes will be more than one line, but the incorrect implementation is simpler.

One of compatible conventions: accept a naive datetime, return a naive datetime. The key point is that it should raise an error for an aware datetime instead of just dropping out the timezone.

Other compatible convention: accept an aware datetime, convert it to the local time before saving, return an aware datetime with the local timezone.

Yet one compatible convention: the same, but with UTC.

The recommended way: accept an aware datetime, save it with the time zone, return an aware datetime with the saved timezone. But it is not compatible with the current format.

There may be compatible mode: always save with timezone, but when load use the specific timezone by default.

Your list already shows how complex the topic for just date/time values is (which SQLite stores as strings).

Only the application developer will know what the “right” solution is for the intended use case and can manage the consequences such a global converter/adapter will have for different parts of the application using SQLite, e.g. you may be using a package which stores sessions in SQLite, another one for application for config data and a third one for your apps native data. Each of the packages may have different expectations, so care has to be taken not to lose information or interpret data based on the wrong timezone.

1 Like

Since we all seem to agree on deprecating the current default converters/adapters, and on improving the docs, I’ll start implementing that. We’ve got the whole 3.12 alpha phase to discuss the remaining item :slight_smile:

3 Likes

Right, and we should help our users.

  1. We should help developers for which the old broken converter/adapter “just work”. Either because they only use naive datetime, or all datetime in the same timezone and restore it after reading from the DB. Migration should be so simple as adding two lines for registration an appropriate adapter and converter in the code. The skip between the deprecation and helpers should be minimal, otherwise they will start to write their own broken converters.
  2. We should help new users which do not know best practices and are not aware or underwater rocks. Reciepes work well for this. The worst case if they search in internet and found bad reciepes.

If we do not care about this, we should not bother with fixing or deprecating the broken converters. Not that I’m against anything, I’m just thinking about how this change will affect developers and what we want to achieve in long term.

And thanks Erlend again for all his work with the sqlite module. Unfortunately, I do not have enough time to help. It takes too much effort to focus.

2 Likes

I believe that Erlend’s suggestion to use recipes in the docs is a better way to raise awareness among developers for the issues related to date/time handling with SQLite – both for developers who have used the broken converters and for ones who are just entering the field and need to figure out what they really need.

Also note that date/time is only one area where such converters may help. JSON is another area where people may want to use converters to seamlessly convert dictionaries to JSON for storage and back to dictionaries again when reading data.

The recipes could include examples for this as well.

1 Like

The first PR is now up, very much a work in progress, but I need other eyes on it in order to progress. Reviews are heartly welcomed. Keep in mind that I intend to backport the doc rewrite to 3.11 and 3.10. The deprecation happens after this doc rewrite.

1 Like

… and it has now landed :flight_arrival:

The PR for the actual deprecations is now up: gh-94276

1 Like