Code review comment for lp:~zorba-coders/zorba/bug900677

William Candillon (wcandillon) wrote :

> > > > Should the actual atomic type be present in the attribute element?
> > > What's the use case for this? The type is declared in the index which
> should
> > > be know by the user. It could be done but it's not clear how this would
> look
> > > like. Do you want the QName of the atomic type of the sequence type?
> > >
> > > > If you have "by @id" where @id can be 1 or "1", you might want to create
> > two
> > > > index keys?
> > > I don't understand this.
> > Is it possible to have an index key as xs:anyAtomicType?
> > If yes, you might have an index key of value xs:string("1") and another key
> of
> > value xs:integer(1).
> > Is this use case possible?
> >
>
> It is not for value indexes, but it is for general indexes. Currently, the
> keys() function is not implemented for general indexes. If it is implemented
> for general indexes, then the type info should be included.
>
>
> > >
> > > > If a key returns (), you might want to distinguish with empty string for
> > > > probing later?
> > > I have made a fix such that the value attribute is only present if the
> entry
> > > is not the empty sequence.
> > Cool.
> > Consider the following index key declaration:
> > by xs:string(@team) as xs:string,
> > xs:string(@country) as xs:string?,
> > xs:string(@league) as xs:string?;
> >
> > If keys() returns:
> > <keys>
> > <key value="Foo" />
> > <key value="Bar" />
> > </keys>
> > It's not good because I cannot probe the index with this info.
> > Does it makes sense?
> >
> Yes, William is right. I think we need an extra attribute per key item. The
> attribute name would be something like "is_empty", and its value "true" or
> "false". The is_empty attribute could be optional; if it is not there, "false"
> is implied.
>
> By the way, I really dislike "attribute" as the tag name of an element.
> Something like "key_item" would have been much better.
>
> > >
> > >
> > > > By using keys(), I got the following output:
> > > > <key xmlns="http://www.zorba-
> > > > xquery.com/modules/store/static/indexes/dml"><attribute
> > > > value="hello"></attribute><attribute value="4"></attribute></key>
> > > >
> > > > Is the namespace useful or does it just make the life of the user
> harder?
> > > It's consistent with the keys function of the maps module. We can remove
> it
> > > but should remove it for both then (which is a backwards incompatible
> > change).
> > I would fix it here and fix it for map() in 3.0.
> > Except if the local name attribute means something.
> >
>
> I don't have a strong preference for this. Either way looks ok to me.
So sorry it's a mistake from me, I meant to write about the local-name attribute, I'm fine with the namespace.

« Back to merge proposal