On Tue, 12 May 2009 23:04:19 +0100 Sébastien Cevey <s...@cine7.net> wrote: > - xmmsv_coll_add_order_operators could apply apply xmmsv_t list type > restriction. re:FIXME on how to throw clientlib errors, I suppose it > should print the error to stderr, since it's a bug in the client > that passed wrong arguments to the clientlib (stderr > silent error).
Aye. Todo. > - xmms_collection_query_medialist uses a _string_ query specs (JSON), > that's really nasty (but predates xmmsv_t, so that explains > it). Still, it feels like xmmsv_t should be used instead for this > kind of stuff, possibly with helper constructor functions. This really depends on the `advanced' querying to be properly implemented using xmmsv_t. Even with JSON, advanced querying is not in a good shape. (It gives even me headaches.) Also I would like a list of queries that should be made easy and/or comments on http://wiki.xmms2.xmms.se/wiki/Summer_of_Code_2008/Collections_2.0/Querying_concept , so that it can be done properly (and non-mind-boggling) this time. > - in xmmsc_coll_query_ids, why query coll -> mediaset -> order, > i.e. why apply the mediaset operator? to remove duplicates? Yes, this function retains the old behaviour of only returning distinct ids. If you don't want that you should use xmmsc_coll_query_medialist_ids. Maybe (some of) the *_medialist_*-functions should be renamed. At the time the decision was to just postpone the final naming scheme. > - don't ask the client to strtol all numeric properties from strings! > we should maybe use some heuristic server-side to determine what > type to wrap in the xmmsv_t. could use the content and/or hardcoded > or even dynamic mapping of property->type for that. We have three options: - Allow strings to be converted to ints under the hood in the clientlib - Add an extra column in the database recording the type - Chose a type based on the property's name. (Requires us to fetch this always.) - Wait for S4. Tell me which should be done please? > - what's the deal with plugin sourceranks? This is part of server-side source-preference. Properties with lower ranks are preferred over those with higher ranks. > - collparser: autofilter > * COMPARE/MATCH: applied on what properties? "all"? doesn't seem to > be just artist/album/title? All fields are indeed used. > - collparser: TOKEN is not used when doing a compare with a given > property (binary operator)? That's right. In general the collparser should be updated more thoroughly. > - collparser: # and / > * / always references active playlist, cannot ref another? > originally, the idea was that PLAYLISTNAME/SEQ selects positions > identified by SEQ in PLAYLISTNAME (defaults to active playlist). This was just for acquainting myself some with the collparser code. You're welcome to change it. > * # always uses id matching instead of IDLIST, makes sense for large > or unbound ranges, but does it for single matches ("#1")? No, but I don't think it is too much of a penalty. > * # orders each range, rather than the whole result? Yes, is that a problem? > - how are multiple ordering applied (e.g. album, artist, etc)? Stable sorting is simulated. E.g. foo | bar --------- a | c c | a a | b b | a order(foo, order(bar, coll)) will result in foo | bar --------- a | b a | c b | a c | a order(bar, order(foo, coll)): foo | bar --------- b | a c | a a | b a | c > Main Coll2.0 commit: > - why have both xmmsc_playlist_add_medialist and > xmmsc_playlist_add_idlist? _idlist should be abolished, or be a clientlib wrapper for _medialist. > - INTERSECTION_ORDER vs INTERSECTION - is that an extra operator we > agreed on doing implicitely? Yes. > - why the operand type ID? couldn't COMPARE be used for that? No. IMHO id is not a property, as it is not part of the metadata associated with the file, but rather some internal administration. If it would be regarded as a property using COMPARE on all fields would need an extra SELECT (or OR). Problems could also arise with the collations, as ids are true integers, while all values have been converted to strings. > - operation == "=", "<", etc: could use a nicer or more verbose value? This is a matter of taste. I like "=", ">", especially for "greater-than-or-equal-to" and "smaller-than-or-equal-to". Also should it then be greater or larger and lesser or smaller? > - global idlist register: > * not very nice, though probably not formally a problem.. Hmm. It might be possible to load the module separately for every mlib-session (that needs it) and pass the register as udata, but I'm not sure about that. > * is it ever pruned? Yes, at lines 500 and 681 of collquery.c. > * what do idlist ids (index in reg) correspond to? If the query generator comes across an idlist it will call idlist_register_add, which extends the idlist_register array with a pointer to the idlist and returns the position of this pointer in the array. This integer is then used in an sql-query: SELECT id FROM IdlistVirtual WHERE idlist_id = %u. IdlistVirtual now is a virtual table which is implemented in idlist_vtab.c, which accesses idlist_register[the integer] to obtain a pointer to the original idlist. > - query_ids was removed (replaced by _medialist_ids?), but QUERY_IDS > still in the code? Todo. > - would it be possible to split: > * tokens > * source prefs/rank > * collquery > * idlist vtable > into subcommits? not worth it if you need to recreate entire stubs > if they are interdependent, but this main commit is just scary. Yes, this is todo. Also the deprecated functions should be avoided. > - database: > * why Media vs MediaBackend (aliases?) ? Media has a trigger to update the goodness-column, which MediaBackend does not have. Batch commits are directly into MediaBackend, after which the goodness-column is updated just once. For single commits using the trigger should not be much of a performance penalty. > * need 4 indices for so many combinations? as we noticed, indices > take it shitloads of space, so carefully picking those that are > needed would be better. Aye. Any preference? > * performance of triggers on insertion/update/delete? Not used for batch commits. Less common things, like updating source rank in Sources-table are slow, but that has to traverse the MediaBackend table a few times anyway. > * avoid duplicated queries in schema and upgrade (reuse var) Why? What happens if you update the schema (and thus the var) and someone still needs to update? Couldn't that break the later updates of the fallthrough? > * token table: never pruned on value changes? (could be expensive) Yes. There should be some vacuuming command (or something else?) > - static void udata_set_idlist (tree_node_t *node, xmmsv_coll_t *coll); > declared twice. Oops. > - xmms_collection_is_medialist uses validate without locking the DAG, > is that okay? No. However this is not the only lock missing (the playlist object also accesses the dag without locking). Todo. > Side-notes: > - we should fix the IPC so it sends colls as a DAG, not a tree. Aye. > - use xmmsv_t for coll attributes? > (e.g. support list of values or fields to compare?) Could be cool. I'm a bit afraid of added complexity on the server-side though. Why not just use a union? > nesciens, could you also tell us what's your roadmap for working on > getting coll2 ready for merge, whether you have time to do it, etc? This is my last week with lectures. Then I have to prepare for the exams and take them. The last is on the 25th of June. I might have time to get some work done in between, but I don't want to promise anything. Erik Massop / nesciens -- _______________________________________________ Xmms2-devel mailing list Xmms2-devel@lists.xmms.se http://lists.xmms.se/cgi-bin/mailman/listinfo/xmms2-devel
