sagemath / sage

Main repository of SageMath. Now open for Issues and Pull Requests.
https://www.sagemath.org
Other
1.21k stars 417 forks source link

Large Cremona database construction failure #34516

Open 55d85b0f-bd1c-4711-b38b-f67ac4e3a89c opened 1 year ago

55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

I attempted to build the large Cremona database with sagemath 9.6 and sqlite 3.36.0 on a Fedora 36 x86_64 machine, as follows:

import sage.databases.cremona
c = sage.databases.cremona.LargeCremonaDatabase('cremona', False, True)
c._init_from_ftpdata('ecdata-2019-10-29')

The attempt failed at the very end. This is the last of the output:

Inserting allgens.90000-99999
Committing...
Traceback (most recent call last):
  File "/builddir/build/BUILD/sage-9.6/cremona.sage.py", line 10, in <module>
    c._init_from_ftpdata('ecdata-2019-10-29')
  File "/builddir/build/BUILDROOT/sagemath-9.6-4.fc38.x86_64/usr/lib64/python3.11/site-packages/sage/databases/cremona.py", line 1397, in _init_from_ftpdata
    self.vacuum()
  File "/builddir/build/BUILDROOT/sagemath-9.6-4.fc38.x86_64/usr/lib64/python3.11/site-packages/sage/databases/sql_db.py", line 2180, in vacuum
    self.__connection__.execute('VACUUM')
sqlite3.OperationalError: cannot VACUUM from within a transaction

The phrase "from within a transaction" suggests a missing commit. Indeed, unlike _init_degphi and _init_allbsd, which both print the "Committing..." message and then call self.commit(), _init_allgens just prints the message but doesn't call commit. This patch adds the missing commit. With this change, I was able to build the database successfully.

Component: elliptic curves

Keywords: cremona database

Branch/Commit: u/gh-jamesjer/large_cremona_database_construction_failure @ 0a2c5be

Issue created by migration from https://trac.sagemath.org/ticket/34516

55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

Description changed:

--- 
+++ 
@@ -1 +1,22 @@
+I attempted to build the large Cremona database with sagemath 9.6 and sqlite 3.36.0 on a Fedora 36 x86_64 machine, as follows:

+```
+import sage.databases.cremona
+c = sage.databases.cremona.LargeCremonaDatabase('cremona', False, True)
+c._init_from_ftpdata('ecdata-2019-10-29')
+```
+The attempt failed at the very end.  This is the last of the output:
+
+```
+Inserting allgens.90000-99999
+Committing...
+Traceback (most recent call last):
+  File "/builddir/build/BUILD/sage-9.6/cremona.sage.py", line 10, in <module>
+    c._init_from_ftpdata('ecdata-2019-10-29')
+  File "/builddir/build/BUILDROOT/sagemath-9.6-4.fc38.x86_64/usr/lib64/python3.11/site-packages/sage/databases/cremona.py", line 1397, in _init_from_ftpdata
+    self.vacuum()
+  File "/builddir/build/BUILDROOT/sagemath-9.6-4.fc38.x86_64/usr/lib64/python3.11/site-packages/sage/databases/sql_db.py", line 2180, in vacuum
+    self.__connection__.execute('VACUUM')
+sqlite3.OperationalError: cannot VACUUM from within a transaction
+```
+The phrase "from within a transaction" suggests a missing commit.  Indeed, unlike `_init_degphi` and `_init_allbsd`, which both print the "Committing..." message and then call `self.commit()`, `_init_allgens` just prints the message but doesn't call commit.  This patch adds the missing commit.  With this change, I was able to build the database successfully.
55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

Changed keywords from none to cremona database

55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

Branch: u/gh-jamesjer/large_cremona_database_construction_failure

55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

New commits:

0a2c5beAdd missing commit to LargeCremonaDatabase._init_allgens
55d85b0f-bd1c-4711-b38b-f67ac4e3a89c commented 1 year ago

Commit: 0a2c5be

JohnCremona commented 1 year ago

Out of interest, why were you trying to do this? The code to rebuild the database is only intended to be used when the database is extended -- at which point I do often find that some code needs changing.

Anyway, feel free to make a PR to make it work.