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>