Merged.
On Tue, Jul 5, 2016 at 8:33 AM, Riyafa Abdul Hameed <[email protected]> wrote: > Hi, > > I have made a PR after squashing the commits[1] > > [1] https://github.com/apache/vxquery/pull/84 > > Thank you. > > Yours sincerely, > Riyafa > > On 5 July 2016 at 14:34, Riyafa Abdul Hameed <[email protected]> > wrote: > >> Hi, >> >> If I use jn:keys for object navigation instead of jdm:keys in [1], the >> following would be correct: >> >> >> *let $seq := ("foo", [ 1, 2, 3 ], { "a" : 1, "b" : 2 }, { "a" : 3, "c" : 4 >> })return $seq() * >> >> and would return: *("a", "b", "c")* >> >> But the jdm:keys only works for objects, hence only the following should >> be correct: >> >> >> >> *let $map := { "eyes" : "blue", "hair" : "fuchsia" }return $map()* >> which should return: *("eyes", "hair")* >> >> [1] >> https://github.com/apache/vxquery/blob/master/vxquery-core/src/main/java/org/apache/vxquery/xmlquery/translator/XMLQueryTranslator.java#L1560 >> >> Thank you. >> >> Yours sincerely, >> Riyafa >> >> On 4 July 2016 at 11:31, Till Westmann <[email protected]> wrote: >> >>> Hi Riyafa, >>> >>> responses are inline. >>> Hope this helps. >>> >>> Cheers, >>> Till >>> >>> On 3 Jul 2016, at 5:02, Riyafa Abdul Hameed wrote: >>> >>> I am not sure I am very clear about the suggestions: >>>> >>>> 1) How to implement jn:keys by using jdm:keys? >>>> >>> >>> One way to implement jn:keys($seq) would be to translate it to the JSONiq >>> expression: >>> >>> distinct-values( >>> for $item in $seq >>> return >>> typeswitch ($item) >>> case $o as object() return jdm:keys($o) >>> default return () >>> ) >>> >>> This way we would only need a runtime for jdm:keys and could re-use >>> existing functionality for everything else. However, since we are not very >>> good at "optimizing away" all these intermediate expressions, there >>> probably >>> is some value in "natively" implementing the jn:keys function. >>> >>> 2) I am not sure if it's possible to implement everything inside jn:keys >>>> because it "returns all keys of all objects in the supplied sequence" >>>> while >>>> jdm:keys returns all keys only in a single object. >>>> >>> >>> It seems that your current implementation of jn:keys already does >>> everything >>> that is needed. The case where the input is a single object does exactly >>> what >>> the implementation of jdm:keys does. If the input is a sequence, then the >>> keys >>> are extracted from each object, duplicates are eliminated and the keys are >>> returned. >>> >>> So I think that we would be ok, if we just removed jdm:keys from the code. >>> >>> >>> On 3 July 2016 at 03:02, Till Westmann <[email protected]> wrote: >>>> >>>> Hi Riyafa, >>>>> >>>>> the change looks good, I've added just one comment. >>>>> >>>>> On the question of jdm:keys vs. js:keys the specification says in [4]: >>>>> >>>>> The XQuery data model uses accessors to explain the data model. >>>>> Accessors >>>>> are >>>>> not exposed to the user and are only used for convenience in this >>>>> specification. Objects have the following accessors: jdm:keys [...] >>>>> >>>>> So jdm:keys is not supposed to be accessible to the user. Now I think >>>>> there >>>>> are 2 things that we could do: >>>>> 1) have both the jdm:keys accessor and the jn:keys function and >>>>> implement >>>>> jn:keys by using jdm:keys or >>>>> 2) just implement everything inside of the jn:keys evaluator. >>>>> >>>>> Does this make sense? >>>>> >>>>> Cheers, >>>>> Till >>>>> >>>>> [4] >>>>> >>>>> http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680641300880 >>>>> >>>>> >>>>> On 2 Jul 2016, at 1:24, Riyafa Abdul Hameed wrote: >>>>> >>>>> Hi, >>>>> >>>>>> >>>>>> I have created a PR with the suggested changes[1] except for the 3rd >>>>>> suggestions. The implementation of jdm:keys is based on [2] and >>>>>> jn:keys is >>>>>> based on [3]. >>>>>> >>>>>> [1] https://github.com/apache/vxquery/pull/80 >>>>>> [2] >>>>>> >>>>>> >>>>>> http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680641300880 >>>>>> [3] >>>>>> >>>>>> >>>>>> http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680622840960 >>>>>> >>>>>> Thank you. >>>>>> >>>>>> Yours sincerely, >>>>>> Riyafa >>>>>> >>>>>> On 2 July 2016 at 00:33, Till Westmann <[email protected]> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>>> >>>>>>> it’s really great to see the object navigation. This is really nice >>>>>>> progress! >>>>>>> >>>>>>> Looking at the commits I saw a few things that don’t seem to be quite >>>>>>> right. >>>>>>> Could you take a look if those can be fixed? >>>>>>> >>>>>>> 1) The return type for jn:null is specified to be xs:string*, that >>>>>>> seems >>>>>>> wrong. >>>>>>> 2) The return type jdm:keys is specified to be item(), that seems >>>>>>> wrong >>>>>>> as >>>>>>> well. >>>>>>> 3) There’s a lot of overlap between jdm:keys and jn:keys. Do we >>>>>>> actually >>>>>>> need jdm:keys? >>>>>>> 4) A number of the new tests don’t have a new-line at the end of the >>>>>>> file. >>>>>>> Could we add those? >>>>>>> >>>>>>> Cheers, >>>>>>> Till >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> -- >>>>>> Riyafa Abdul Hameed >>>>>> Undergraduate, University of Moratuwa >>>>>> >>>>>> Email: [email protected] >>>>>> Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/> >>>>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa> >>>>>> <http://twitter.com/Riyafa1> >>>>>> >>>>>> >>>>> >>>> >>>> -- >>>> Riyafa Abdul Hameed >>>> Undergraduate, University of Moratuwa >>>> >>>> Email: [email protected] >>>> Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/> >>>> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa> >>>> <http://twitter.com/Riyafa1> >>>> >>> >> >> >> -- >> Riyafa Abdul Hameed >> Undergraduate, University of Moratuwa >> >> Email: [email protected] >> Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/> >> <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa> >> <http://twitter.com/Riyafa1> >> > > > > -- > Riyafa Abdul Hameed > Undergraduate, University of Moratuwa > > Email: [email protected] > Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/> > <http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa> > <http://twitter.com/Riyafa1>
