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

Provide loadBeforeEx #485

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Provide loadBeforeEx #485

wants to merge 1 commit into from

Conversation

navytux
Copy link

@navytux navytux commented Nov 11, 2021

loadBeforeEx is like loadBefore, but simpler, provides better
information for object delete records and can be more efficiently
implemented by many storages: zopefoundation/ZODB#323

On RelStorage loadBefore is currently implemented via 3 SQL queries:

  1. check whether object record exists at all
  2. retrieve object state
  3. retrieve serial of next object revision

Compared to that loadBeforeEx is implemented via only one SQL query "2"
from the above - "retrieve object state". It is exactly the same query
that loadBefore uses and after the patch loadBefore actually invokes
loadBeforeEx for step 2.

This change was outlined in

zopefoundation/ZODB#318 (comment) and
zopefoundation/ZODB#318 (comment)

and as explained in the first link this patch is also semantically coupled with

#484

This patch passes tests with both ZODB5 and with ZODB5+zopefoundation/ZODB#323:

  • when ran with ZODB5 it verifies that loadBefore implementation does
    not become broken.

  • when ran with ZODB5+loadAt zopefoundation/ZODB#323 it
    verifies that loadBeforeEx implementation is correct.

For tests to pass with
ZODB5+zopefoundation/ZODB#323 we also need
zopefoundation/zc.zlibstorage#11 because without
that fix zc.zlibstorage does not decompress data on loadBeforeEx.

loadBeforeEx is like loadBefore, but simpler, provides better
information for object delete records and can be more efficiently
implemented by many storages: zopefoundation/ZODB#323

On RelStorage loadBefore is currently implemented via 3 SQL queries:

  1) check whether object record exists at all
  2) retrieve object state
  3) retrieve serial of next object revision

Compared to that loadBeforeEx is implemented via only one SQL query "2"
from the above - "retrieve object state". It is exactly the same query
that loadBefore uses and after the patch loadBefore actually invokes
loadBeforeEx for step 2.

This change was outlined in

zopefoundation/ZODB#318 (comment) and
zopefoundation/ZODB#318 (comment)

and as explained in the first link this patch is also semantically coupled with

zodb#484

This patch passes tests with both ZODB5 and with ZODB5+zopefoundation/ZODB#323:

- when ran with ZODB5 it verifies that loadBefore implementation does
  not become broken.

- when ran with ZODB5+zopefoundation/ZODB#323 it
  verifies that loadBeforeEx implementation is correct.

For tests to pass with
ZODB5+zopefoundation/ZODB#323 we also need
zopefoundation/zc.zlibstorage#11 because without
that fix zc.zlibstorage does not decompress data on loadBeforeEx.
@navytux
Copy link
Author

navytux commented Nov 11, 2021

Regarding test failure: I tend to believe that the sole failue of PYTHON=C:\Python38-x64 is due to some flakiness and is unrelated to hereby patch becuase of the following reasons:

  • That relstorage.adapters.sqlite.tests.test_sqlite.Sqlite3RSDestHFZodbConvertTests_sqlite3 passes locally for me, and it passed in all other runs: PYTHON=C:\Python27-x64, PYTHON=C:\Python36-x64 and PYTHON=C:\Python37-x64.
  • When run with regular ZODB5 (without loadAt zopefoundation/ZODB#323) hereby patch semantic is only slight code movement.

Kirill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant