Merge lp:~mwhudson/launchpad/recipe-db-schema into lp:launchpad/db-devel

Hi there,

This branch adds the db schema for sourcepackagerecipes and associated things that was much discussed on the list. I've written a little code using this schema now (see lp:~mwhudson/launchpad/recipe-model-code) and it still feels more or less right and I would like to start landing code, so now seems like a good time to get this branch formally reviewed.

This presupposes a fairly boring way of representing recipes in the database -- but I think that this will be easier to change if we want to later.

Cheers,
mwh

Hello there!

I understood that manifests need to store the revision for each referenced branch. I don't see that detail addressed in the schema change. Or did I miss something?

I read through the "First cut at recipe db-schema patch" email thread again..

Just in case you plan to keep the revision data for manifests in the SourcePackageRecipeData.recipe text column: how do we distinguish between recipe and manifest data w/o looking at the .recipe content or checking that it was referenced via SourcePackageBuild.manifest?

Or is there no need for such a distinction?

Muharem Hrnjadovic wrote:
> I read through the "First cut at recipe db-schema patch" email thread again..
>
> Just in case you plan to keep the revision data for manifests in the SourcePackageRecipeData.recipe text column: how do we distinguish between recipe and manifest data w/o looking at the .recipe content or checking that it was referenced via SourcePackageBuild.manifest?

I guess I was expecting that you wouldn't do a big query for all
SourcePackageRecipeData rows referencing a branch and the split them
into recipe and manifest links but rather do two queries, one for
recipes referencing a branch and then one for manifests/build logs
referencing a branch. In my model code currently, there is no interface
for SourcePackageRecipeData -- it's hidden within the
SourcePackageRecipe content type.

> Or is there no need for such a distinction?

I don't really think there is.

Cheers,
mwh

Muharem Hrnjadovic wrote:
> Hello there!
>
> I understood that manifests need to store the revision for each referenced branch. I don't see that detail addressed in the schema change. Or did I miss something?

Oh whoops, forgot about that.

I think I'm gradually coming around to Jono's view that we'll have
fairly structured storage of the recipe in the db, so all this will
probably change quite a bit before we start using it in anger.

Cheers,
mwh

I'm curious why SourcePackageRecipeData is a seperate table.

If this is a 1:1 relationship, do you have a rationale for this being a separate table?

If this is not a 1:1 relationship, how do we express this in English? A SourcePackageRecipe contains multiple recipes?

I'm happy to have the non-structured recipe column provided we don't need to do anything with it besides store it, retrieve it, and send it to builders. If the appservers need to decode it we should consider a more structured format per Jono's reported views.

SourcePackageBuild.sourcepackagename is misspelled.

The SourcePackageRecipe unique index does not match its name. Do you want (owner, distroseries, sourcepackagename, name) unique or just (distroseries, sourcepackagename, name)? I prefer ALTER TABLE ADD CONSTRAINT sourcepackagerecipe__ds__spn__name__key UNIQUE (distroseries, sourcepackagename, name) to CREATE UNIQUE INDEX, as we get a little more metadata in the DB with the former syntax (functionally, it is identical though except for some DBA operations).

It is going to need more indexes - pretty much every foreign key constraint needs an index on that column. I can add these in later if you want. We also will probably need indexes on SourcePackageBuild and SourcePackageBuildUpload to support queries required by our reports - we can add them in now if this is obvious, or leave them until later if we don't know what the selection criteria will be.

Michael and I had a chat on the phone. We've worked out a schema that will have a much more structured view of recipes. Should be up for review again tomorrow.

Hi guys, I think the patch is now ready and we'd like to land it before It will still need indexes added, but I hope stub can do that :-)

 * 'name' on SPRInstruction should be NOT NULL, I think.
 * You _could_ add a constraint saying that the line number should be strictly greater than the parent's line number. Not sure if this is necessary.
 * Shouldn't SourcePackageBuild.recipe be NOT NULL?

Other than that, looks good.

* OK.
* Doesn't seem useful really
* OK.

Go again?

Wuu.

Allocated patch number is patch-2207-25-0.sql

SourcePackageRecipeDataInstruction.recipe_data should be NOT NULL

The sourcepackagerecipedatainstruction__name__recipe_data should be called
sourcepackagerecipedatainstruction__name__recipe_data__key. Similarly other unique constraints should end with __key.

Please change:

ALTER TABLE SourcePackageRecipeDataInstruction
ADD CONSTRAINT sourcepackagerecipedatainstruction__line_number__recipe_data
UNIQUE (line_number, recipe_data);

into the more useful to PostgreSQL variant:

ALTER TABLE SourcePackageRecipeDataInstruction ADD CONSTRAINT
sourcepackagerecipedatainstruction__recipe_data__line_number__key
UNIQUE (recipe_data, line_number);

sourcepackage is one word in Launchpad, so the .*source_package.* columns
need to be renamed .*sourcepackage.*.

As discussed on IRC and seen at http://pastebin.ubuntu.com/355335/, its better to make SourcepackageRecipeData reference recipe and build rather than the other way around, as we can then enforce the constraint that one and only one should be set.

Needed indexes:

CREATE INDEX sourcepackagerecipedata__base_branch__idx
ON SourcepackageRecipeData(base_branch);

CREATE INDEX sourcepackagerecipedatainstruction__branch__idx
ON SourcepackageRecipeDataInstruction(branch);

CREATE INDEX sourcepackagerecipe__registrant__idx
ON SourcepackageRecipe(registrant);

--CREATE INDEX sourcepackagerecipe__owner__idx
--ON SourcepackageRecipe(owner);

CREATE INDEX sourcepackagerecipebuild__distroseries__idx
ON SourcepackageRecipeBuild(distroseries);

CREATE INDEX sourcepackagerecipebuild__sourcepackagename__idx
ON SourcepackageRecipeBuild(sourcepackagename);

CREATE INDEX sourcepackagerecipebuild__build_log__idx
ON SourcepackageRecipeBuild(build_log) WHERE build_log IS NOT NULL;

CREATE INDEX sourcepackagerecipebuild__builder__idx
ON SourcepackageRecipeBuild(builder);

CREATE INDEX sourcepackagerecipebuild__requester__idx
ON SourcepackageRecipeBuild(requester);

CREATE INDEX sourcepackagerecipebuild__recipe__idx
ON SourcepackageRecipeBuild(recipe);

CREATE INDEX sourcepackagebuildupload__registrant__idx
ON SourcepackageRecipeBuildUpload(registrant);

I missed some indexes and one of the constraints is wrong.

CREATE INDEX sourcepackagerecipebuildupload__archive__idx
ON SourcepackageRecipeBuildUpload(archive);

CREATE INDEX sourcepackagerecipebuildupload__upload_log__idx
ON SourcepackageRecipeBuildUpload(upload_log) WHERE upload_log IS NOT NULL;

CREATE INDEX sourcepackagerelease__sourcepackage_recipe_build__idx
ON SourcepackageRelease(sourcepackage_recipe_build);

Should BuildSourcepackageFromRecipe.job be UNIQUE? If so:

ALTER TABLE BuildSourcepackageFromRecipe ADD CONSTRAINT
buildsourcepackagefromrecipe__job__key UNIQUE (job);

Otherwise:

CREATE INDEX buildsourcepackagefromrecipe__job__idx
ON BuildSourcepackageFromRecipe(job);

One of the constraints is wrong (NULL != NULL in SQL 3-value boolean). Drop:

ALTER TABLE SourcePackageRecipeData ADD CONSTRAINT
sourcepackagerecipedata__sourcepackage_recipe__sourcepackage_recipe_build__key
UNIQUE (sourcepackage_recipe, sourcepackage_recipe_build);

Replace with:

CREATE UNIQUE UNDEX sourcepackagerecipedata__sourcepackage_recipe__key
ON SourcepackageRecipeData(sourcepackage_recipe)
WHERE sourcepackage_recipe IS NOT NULL;
CREATE UNIQUE INDEX sourcepackagerecipedata__sourcepackage_recipe_build__key
ON SourcepackageRecipeData(sourcepackage_recipe_build)
WHERE sourcepackage_recipe_build IS NOT NULL;

W00t