Opened 6 years ago

Closed 6 years ago

#14629 closed enhancement (fixed)

Upgrade and clean up sqlite package

Reported by: jdemeyer Owned by: jdemeyer
Priority: major Milestone: sage-5.10
Component: packages: standard Keywords: sqlite package
Cc: Merged in: sage-5.10.beta5
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/sqlite-3.7.17.spkg (spkg diff)

apply: 14629_sqlite.patch to the Sage library.

sqlite-3.7.17 (Jeroen Demeyer, 24 May 2013)

  • #14629: upgrade to version 3.7.17
  • Use standard template for SPKG.txt and spkg-install, use $MAKE instead of make
  • Remove Fortran and C++ stuff from spkg-install
  • Compile with -DSQLITE_WITHOUT_ZONEMALLOC on OS X <= 10.4

Attachments (2)

14629_sqlite.patch (1.2 KB) - added by jdemeyer 6 years ago.
sqlite-3.7.17.diff (5.1 KB) - added by jdemeyer 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work
sage -t devel/sage/sage/databases/cremona.py
**********************************************************************
File "devel/sage/sage/databases/cremona.py", line 636, in sage.databases.cremona.MiniCremonaDatabase.__iter__
Failed example:
    it.next().label()
Expected:
    '11a1'
Got:
    '1001a1'
**********************************************************************
File "devel/sage/sage/databases/cremona.py", line 638, in sage.databases.cremona.MiniCremonaDatabase.__iter__
Failed example:
    it.next().label()
Expected:
    '11a2'
Got:
    '1001b1'
**********************************************************************
File "devel/sage/sage/databases/cremona.py", line 640, in sage.databases.cremona.MiniCremonaDatabase.__iter__
Failed example:
    it.next().label()
Expected:
    '11a3'
Got:
    '1001b2'
**********************************************************************
File "devel/sage/sage/databases/cremona.py", line 642, in sage.databases.cremona.MiniCremonaDatabase.__iter__
Failed example:
    it.next().label()
Expected:
    '14a1'
Got:
    '1001b3'
**********************************************************************
File "devel/sage/sage/databases/cremona.py", line 645, in sage.databases.cremona.MiniCremonaDatabase.__iter__
Failed example:
    it.next().label()
Expected:
    '45a3'
Got:
    '1014g2'
**********************************************************************

Changed 6 years ago by jdemeyer

comment:3 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Changed 6 years ago by jdemeyer

comment:4 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:5 follow-up: Changed 6 years ago by fbissey

Funny that. We did face the upgrade in sage-on-gentoo in sage 5.8 and we had the same issue. Christopher was supposed to open a ticket with his solution but he never did.

--- sage/databases/cremona.py.orig	2013-02-19 10:33:56.000000000 +0000
+++ sage/databases/cremona.py	2013-03-18 16:51:58.950097433 +0000
@@ -634,19 +634,19 @@
 
             sage: it = CremonaDatabase().__iter__()
             sage: it.next().label()
-            '11a1'
+            '1001a1'
             sage: it.next().label()
-            '11a2'
+            '1001b1'
             sage: it.next().label()
-            '11a3'
+            '1001b2'
             sage: it.next().label()
-            '14a1'
+            '1001b3'
             sage: skip = [it.next() for _ in range(100)]
             sage: it.next().label()
-            '45a3'
+            '1014g2'
         """
         for c in self.__connection__.cursor().execute('SELECT curve FROM ' \
-            + 't_curve'):
+            + 't_curve ORDER BY curve'):
             yield self.elliptic_curve(c[0])
 
     def __getitem__(self, N):

Anyway, your solution looks good too. One concern we had: is it fast?

comment:6 in reply to: ↑ 5 Changed 6 years ago by jdemeyer

Replying to fbissey:

One concern we had: is it fast?

I have no idea, but I assume that SQLite is sufficiently fast. There are other place too where ORDER BY is used. In any case, your patch is not good because it assumes that there is no curve of conductor 10001 for example. This assumption is false if the large Cremona database has been installed.

comment:7 Changed 6 years ago by fbissey

  • Status changed from needs_review to positive_review

Roger that, of course we don't currently package the large Cremona database so it was easy to miss. Looks all good to me, positive review.

comment:8 Changed 6 years ago by jdemeyer

  • Reviewers set to François Bissey

comment:9 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.10.beta5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.