FirebirdSQL / python3-driver

Firebird driver for Python that uses new Firebird API
https://www.firebirdsql.org/en/devel-python-driver/
MIT License
26 stars 10 forks source link

It is not possible to start a transaction without specifying an isolation level #30

Closed ant-zuev closed 2 months ago

ant-zuev commented 9 months ago

default_tpb with Isolation.SNAPSHOT is used when we do not define a custom tpb. But with this, I can't initialize a tpb without specifying an isolation level. In the TPB class, the default isolation is SNAPSHOT: https://github.com/FirebirdSQL/python3-driver/blob/b40e0d95814b239fb24a3bcb9e8f80da74d91ed1/src/firebird/driver/core.py#L292 The Connection class also uses the SNAPSHOT type by default: https://github.com/FirebirdSQL/python3-driver/blob/b40e0d95814b239fb24a3bcb9e8f80da74d91ed1/src/firebird/driver/core.py#L1618 According to the documentation, the server already uses the default SNAPSHOT isolation type: https://firebirdsql.org/file/documentation/chunk/en/refdocs/fblangref40/fblangref40-transacs.html#fblangref40-transacs-settransac-iso

pcisar commented 9 months ago

I really don't understand your problem. The isolation level is key and mandatory parameter of transaction parameters. If you don't specify it, then default SNAPSHOT is used (which is consistent with ISQL behavior that uses this isolation level for default transactions). The is no such thing in Firebird as transactions without isolation.

ant-zuev commented 9 months ago

I really don't understand your problem. The isolation level is key and mandatory parameter of transaction parameters. If you don't specify it, then default SNAPSHOT is used (which is consistent with ISQL behavior that uses this isolation level for default transactions). The is no such thing in Firebird as transactions without isolation.

The isolation level is not a mandatory parameter of the API function. If we don't specify the isolation level when calling an API method, the server uses the SNAPSHOT value by default when creating a transaction. What is the point of specifying SNAPSHOT again in the driver? ISQL does not define a default isolation type for DML operations.

pcisar commented 9 months ago

I see. But still don't understand your problem. For application there is no difference between specifying SNAPSHOT by default in TPB and by not specifying isolation in TPB at all. The sole difference is absence of isolation node in TPB. And I prefer explicit over implicit, hence the default is enforced in driver. Why do you need TPB without Isolation node is beyond me (you are the first who asked for it), but you can always construct TPB as you wish and use it where needed. You don't need to use TPB class at all.

dyemanov commented 9 months ago

RedDatabase has recently implemented the default isolation level being configurable rather than hardcoded (SNAPSHOT) as in Firebird. And driver's hidden override prevents from relying on the server-level default (when the isolation level is absent in TPB). It would be quite a pity to maintain a python-driver fork just because of this issue.

pcisar commented 9 months ago

Did they really? I would love to know the rationale behind this novel approach how to fuck up the application via (mis)configuration. But again. TPB class and tpb() function (that uses TPB class internally) are optional and not needed to be used by app developer. All tpb's used in driver are defined as bytes. Developers can specify their own tpb's everywhere it's used. Anywhere you can specify the custom transaction parameter block in functions like transaction(), default tpb in Connection class etc., you have to pass bytes. Hence you can pass any tpb binary content you wish. TPB class is just a helper for developer's convenience. Sure, if developer would use driver defaults, these defaults may not respect RedDatabase server defaults.

Anyway, I'm not against the idea to adjust the driver to "fix" the issue for RedDatabase, but I'm not convinced yet that it's worth the trouble. Actually, I see the current need for developer to explicitly create tpb's without isolation tag contained to take advantage of new RedDatabase configuration option as better - because it's explicit and thus the app developer should know why and when he is using it. And I see it as minor inconvenience, as it's possible to define such tpb bytes on single place in app and use it everywhere.

BTW, I would certainly adjust the driver IF such feature would be introduced into Firebird itself (are there such plans?).

ant-zuev commented 9 months ago

Anywhere you can specify the custom transaction parameter block in functions like transaction(), default tpb in Connection class etc., you have to pass bytes. Hence you can pass any tpb binary content you wish. TPB class is just a helper for developer's convenience. Sure, if developer would use driver defaults, these defaults may not respect RedDatabase server defaults.

This may not be consistent with the driver's approach either, but there is also a problem with an empty tpb. For example, the begin() method of TransactionManager class does not allow to use such tpb: https://github.com/FirebirdSQL/python3-driver/blob/b40e0d95814b239fb24a3bcb9e8f80da74d91ed1/src/firebird/driver/core.py#L2430 If I want to pass tpb without an isolation type, I have to pass at least one parameter.

pcisar commented 9 months ago

Well, that's a valid complaint, so I'm going to reopen the issue. However, the fix is simple (your can test it locally), just change the start_transaction(tpb or self.default_tpb) to start_transaction(self.default_tpb if tpb is None else tpb).