From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Subject: bug#32190: [PATCH] database: Merge Derivations into Builds table. Date: Wed, 15 Aug 2018 20:57:00 +0200 Message-ID: <87mutn5ugz.fsf@lassieur.org> References: <87k1ptirr0.fsf@lassieur.org> <20180804160303.20451-1-clement@lassieur.org> <87h8jwvkg8.fsf@elephly.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:60063) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fq1b7-0003wD-P0 for bug-guix@gnu.org; Wed, 15 Aug 2018 15:36:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fq1b0-0005M8-0x for bug-guix@gnu.org; Wed, 15 Aug 2018 15:36:46 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:47046) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fq1az-0005LH-SM for bug-guix@gnu.org; Wed, 15 Aug 2018 15:36:41 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fq0zZ-0007kg-SD for bug-guix@gnu.org; Wed, 15 Aug 2018 14:58:01 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-reply-to: <87h8jwvkg8.fsf@elephly.net> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ricardo Wurmus Cc: 32190@debbugs.gnu.org Hi Ricardo, Ricardo Wurmus writes: > I see that you=E2=80=99ve mentioned your changes to =E2=80=9Cbuild-packag= es=E2=80=9D in a later > email. > > I have two general questions about this: why was the change from =E2=80= =9Cid=E2=80=9D to > =E2=80=9Crowid=E2=80=9D necessary? And: could you please also update the= documentation > so that is reflects the changes? I removed the 'id' column from the Builds table because it was used as the primary key, and I wanted the primary key to be 'derivation'. But we still need to display the build id in the web interface, and the API allows to get the builds by id. We can't use the 'id' column anymore because AUTOINCREMENT isn't allowed without PRIMARY KEY[1], but we can use the rowid[2] implicit column instead. I updated the documentation on my personal branch[3], but I didn't send it to the ml, because I have sent too many things already ;) [1]: https://www.sqlite.org/autoinc.html [2]: https://www.sqlite.org/rowidtable.html [3]: https://git.lassieur.org/cgit/cuirass.git/ >> diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm >> index b4b1652..7788ac9 100644 >> --- a/src/cuirass/database.scm >> +++ b/src/cuirass/database.scm [...] >> (define (db-add-build db build) >> "Store BUILD in database DB. BUILD eventual outputs are stored >> in the OUTPUTS table." [...] > This procedure is called when a build is scheduled, isn=E2=80=99t it? Yes. > The docstring says =E2=80=9CBUILD eventual outputs are stored in the OUTP= UTS > table.=E2=80=9D =E2=80=93 does this mean the names of the *expected* out= put > directories are recorded in Outputs? They don=E2=80=99t exist at this po= int, > right? Indeed, it's just the output paths written in the derivation files. >> + (lambda (key who code message . rest) >> + ;; If we get a unique-constraint-failed error, that means we have >> + ;; already inserted the same build. That happens when several jo= bs >> + ;; produce the same derivation, and we can ignore it. >> + (if (=3D code SQLITE_CONSTRAINT_PRIMARYKEY) >> + #f >> + (apply throw key who code rest))))) > > Okay. > > Can we prevent this from happening in the first place? I feel a little > uncomfortable about performing an operation that we expect to cause > primary key errors regularly. We would need to manually check that the derivation doesn't already exist. That would be an extra query so I think it would be less efficient and it would require more code. I believe the 'constraint' is a pretty good mechanism to describe things that shouldn't happen in the database, I don't know any better way. >> diff --git a/src/schema.sql b/src/schema.sql >> index eb0f7e9..0452495 100644 >> --- a/src/schema.sql >> +++ b/src/schema.sql > [=E2=80=A6] >> --- Builds are not in a one to one relationship with derivations in orde= r to >> --- keep track of non deterministic compilations. > > Is this comment still correct considering that the derivation is now the > primary key of the Builds table? No, I forgot to remove it. >> CREATE TABLE Builds ( >> - id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, >> - derivation TEXT NOT NULL, >> + derivation TEXT NOT NULL PRIMARY KEY, >> evaluation INTEGER NOT NULL, >> + job_name TEXT NOT NULL, >> + system TEXT NOT NULL, >> + nix_name TEXT NOT NULL, >> log TEXT NOT NULL, >> status INTEGER NOT NULL, >> timestamp INTEGER NOT NULL, >> starttime INTEGER NOT NULL, >> stoptime INTEGER NOT NULL, >> - FOREIGN KEY (derivation) REFERENCES Derivations (derivation), >> FOREIGN KEY (evaluation) REFERENCES Evaluations (id) >> ); > > Thanks again! :-) you're welcome! Thanks for reviewing. Cl=C3=A9ment