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>

Reply via email to