exasol / pyexasol

Exasol Python driver with low overhead, fast HTTP transport and compression
MIT License
72 stars 39 forks source link

Send snapshotTransactionsEnabled attribute only if set explicitly by user #77

Closed ben-cohen-xs closed 3 years ago

ben-cohen-xs commented 3 years ago

Hi, I work for Exasol and recently implemented a change to enable Snapshot Mode by default in Exasol 7.1.0. We also added a new syntax "ALTER SYSTEM|SESSION SET SNAPSHOT_MODE = 'OFF'|'SYSTEM TABLES';", to allow Snapshot Mode to be disabled or enabled for the system default or current session.

Could you change pyexasol so that it does not send the snapshotTransactionsEnabled attribute to WebSockets unless the user explicitly requested it using the snapshot_transactions option?

This is because the "default" for the attribute can now be True (for 'SYSTEM TABLES') or False (for 'OFF'), depending on the ALTER SYSTEM value.

This change will also be safe for earlier versions, for which the system default is False.

Test case

# Setup
# Connect to Exasol 7.1.0, with system default SYSTEM TABLES
import pyexasol
C1 = pyexasol.connect(dsn='localhost:45623', user='sys', password='exasol')
C2 = pyexasol.connect(dsn='localhost:45623', user='sys', password='exasol', snapshot_transactions=False)

# Existing behaviour: [('OFF',)]
# Desired behaviour:  [('SYSTEM TABLES',)]
print(C1.execute("select SESSION_VALUE from EXA_PARAMETERS where PARAMETER_NAME = 'SNAPSHOT_MODE'").fetchall())

# Existing behaviour: [('OFF',)]
# Desired behaviour:  [('OFF',)]
print(C2.execute("select SESSION_VALUE from EXA_PARAMETERS where PARAMETER_NAME = 'SNAPSHOT_MODE'").fetchall())
littleK0i commented 3 years ago

@ben-cohen-xs , merged and added an additional commit on top: https://github.com/badoo/pyexasol/commit/a0c71c266d78b311b4b3ee11bcb1fa37ef7d9b28

Thank you!

littleK0i commented 3 years ago

In theory, we may now consider deprecating this parameter. With all the latest updates (/*snapshot execution*/ comment, metadata requests, default snapshot mode for system tables in 7.1), I don't see any practical reasons for users to set the snapshot transaction mode.

When it was originally implemented for Exasol 6.0, it was the only way to get lock-free access to metadata. But now we have so many better options.

ben-cohen-xs commented 3 years ago

Thank you for merging this!