Just null the ID field by default then and it should work
ocharles
reosarevok: you generally store lists in the opposite direction
list elements point to the thing that holds the list, not the thing that holds the list pointing to elements
but if we do flip that round, we can use NULL to model unknown
voiceinsideyou joined the channel
reosarevok certainly sees [none] as more useful than multiple barcodes. If only because the amount of releases with no barcode is hugely higher than the amount of releases with more than one...
reosarevok
So yeah, please make sure you don't break [none] to add this :)
(I'm yet to see one release that needs it, although I know some exist - I'm not sure it is really worth the extra complexity but if people really want it)
Leftmost
I have one, though it's buried right now.
reosarevok
Well, yes, I'm certainly not doubting they exist, and I agree it would be better to be able to store it. It's just that adding several tables for it when the most common situation by far will be that they'll map 1:1 to releases as they do now seems weird
(adding extra joins to every single release for the benefit of a few)
warp
some is a list of 0 or more actual barcodes. unknown is not none and count(some) = 0.
(woops, responding to random old backscroll lines)
reosarevok
But I guess it's at least more consistent than "store barcode if there's one, point to list if not"
nikki
reosarevok: yeah, I'm not keen on even more joins :(
ocharles
make views then
nikki
I haven't got a clue how to do that
ocharles
create view name as select ...
that's it
zag joined the channel
nikki
I don't know how to do the select
ocharles
you know how to do a select with a join :)
nikki
yes, but I don't know how to select only one barcode
I don't want two rows for a single release just because some stupid release has two
ocharles
why do you want to select only one barcode?
the array_agg
then*
JonnyJD
On the musicbrainz-server VM the "musicbrainz_test" db doesn't exist. Is there a quick script somewhere to create it? With the musicbrainz user I can't create a DB and role "root" doesn't exist either.
ocharles
JonnyJD: script/create_test_db.sh
it's in HACKING
hmm, actually, it isn't
tut
JonnyJD
No, it isn't
It probably should be in "HACKING" though.
ocharles
i'm adding it now
reosarevok
ocharles: why not just one table, (release.id, barcode), for which barcode contains either null, '' or a barcode? + a trigger like "if barcode is null, instead of new insert, update that"?
ocharles
reosarevok: that doesn't set you store multiple barcodes
oh wait, a separate table, sorry
reosarevok
ocharles: it does if release.id isn't a key
ocharles
yes, you can do that, but now you have to write triggers to enforce integrity
reosarevok
It's still lots of stuff for little gain, but if we want the little gain
ocharles
when you could just model it to express the constraints you want
warp
make barcode an array type in the database :)
reosarevok
I guess if I say "since barcodes hopefully don't have commas in them, enter them as comma-separated values" in the barcode column I'll get slapped, right? :p
ocharles
warp: that is an option
warp
reosarevok: postgresql has proper array types for lists of things.
ocharles
with a GIN you can index that too
warp
NULL is unknown, empty list is no barcode aka none, items in the list are barcodes.
reosarevok
warp: oh, ok :) (that also wasn't in my introductory course :) )
If that's doable, it sounds sensible
Leftmost
GIN?
ocharles
that does however, make it difficult to enforce that barcodes form a set, not a list
I guess if I say "add a second column for additional barcodes and leave the first one on the table as-is" I'll get shouted at, right? :p
(with that second column pointing to a second table or whatever)
Mineo: no
(but I guess you knew :p)
I guess we could do the multiple tables thing and *then* a view with arrays though?
Mineo
I was unsure if the troubles last night required the links to be changed :)
reosarevok
Mineo: ok, my actual answer is "sounds extremely unlikely" then :p
ocharles
reosarevok: yes to both questions :)
reosarevok: the view with an array is what I was suggesting above
Leftmost
Hmm. I'm guessing Ian was right and I'm not going to finish this review by today.
reosarevok
The main problem I can see with views is that they make it more annoying to graphically document the useful ways to use the schema (as in, I have no idea how it could be done)
Sure, you could document the actual tables but if nobody ever is going to want to use them that way...
(do we have any documentation about our current views, btw?)
ocharles
no
reosarevok
:(
ocharles
Leftmost: I think release(id, barcode_set nullable, ...), barcode_set(id), and barcode (barcode_set, barcode) might be the way to go.
primary keys on barcode_set.id and barcode.barcode
Leftmost
Okay.
I'm making a note of that and trying to get a bit of sleep. I'll push the changes in a few hours.
Thanks for talking it out with me.
ocharles
No worries
it's a tricky one
reosarevok
ocharles, can you look at wtf is going on with the links Mineo mentioned?
ocharles
is it urgent or can I have lunch first?
reosarevok
have lunch
:)
ocharles
ok, will look after :)
Mineo, reosarevok: oh, bah - that was me "fixing" the wiki docs while internal DNS was broken
let me just fix that
Ok, should be better now
reosarevok
Seems to still point to the wiki?
reosarevok checks whether it is cache
Doesn't seem so
ocharles
there might be a cache server side
i believe it'll fix itself in an hour
reosarevok
Ok
We'll see
:)
ocharles
yea, I think that's easiest
ocharles has changed the topic to: 🌅 SUNRISE week | http://musicbrainz.org/#devel | note about meeting time in the US: http://bit.ly/Ym40ql | Agenda: reviews, MBS-5933 (ocharles), mbs-2079 (nikki), code reviewing non-MBS projects (ocharles/warp), MBS-2411 (ocharles)
I hope you don't mind me shoving a schema change ticket in front of 2079, nikki :)
JonnyJD
should fixes for (old) update scripts in admin/sql/updates go to master or beta? (I have one for 20130125)
warp
JonnyJD: I consider update scripts to be things which run once on release. So in general I would say you need to add a new script.
JonnyJD: either way, pull requests should go to beta.
JonnyJD
Well, I am running that update script now and it errors out. Not sure if later update scripts depend on this
warp
why not just get a new dump? january is quite some time ago.
nikki
ocharles: actually I just want to know what we should do with that ticket. most of the comments (and probably most of the votes too) are for a feature we added, but technically we haven't fixed the thing that's asked for... but I'm sick to death of seeing that ticket, thinking "...didn't we already add that?" and then realising I've mistook it for something else yet again
so it's not really that important, just annoying :P
JonnyJD
Well, I don't actually want new data and I really don't want to move the whole DB again because I would run out of space in the VM with a complete update
ocharles
nikki: ok (:
hawke_1 joined the channel
warp
nikki: hm, marked is "In Progress" without being assigned seems odd.
hawke joined the channel
nikki
warp: yeah. the whole ticket is a mess. it got set to in progress when ollie was doing the relationship editor thing he started (which, note, is not what the ticket asked for either :P)
JonnyJD
warp: Anyways, I fixed the script so it works and would like to submit that upstream if anybody is interested. I don't think it is that important that a new script needs to be created.
nikki
I'm thinking it might be better to just make a new ticket for original requested behaviour and close that one and tell people to go vote for the new ticket if they still want it
warp
JonnyJD: ok, well just submit it as a pull request to beta like any other change.
nikki
so that we have some idea of how many people actually want that and how many were just like "gaaaah I can't link works for a release" (since solved by the relationship editor)
ocharles
nikki: i'm fine with that
warp
nikki: that sounds like a perfectly reasonable thing to do.
nikki
then maybe I should just do that now and take it out of the agenda :P
zag has left the channel
djce joined the channel
Jormangeud joined the channel
murdos joined the channel
voiceinsideyou joined the channel
voiceinsideyou1 joined the channel
JonnyJD
is there a known validator.nu "mirror"? http://validator.nu is offline and testing takes ages since it always waits for a timeout. (I did set an empty validator in DBDefs.pm for now)
voiceinsideyou joined the channel
warp
JonnyJD: you can run your own
ijabz joined the channel
(assuming the source repositories are still online)