Merge lp://staging/~zorba-coders/zorba/bug1064978 into lp://staging/zorba

This is a proposal for solving the requested issue with current bind-variables.

This new function called xqxq:set-variable($query-key as xs:anyURI, $var-name as xs:QName, $value as xs:string*)

fulfills the requested feature of casting an xs:string to the corresponding type of a defined external variable.
To be able to create this function a new function to the DynamicContext API had to be added, the function is called getValueType which returns a string of the type a given variable QName.
And then bringing this function to the xqxq module with the name xqxq:variable-type.

I am not sure about the name set-variable since is different from bind-variable. Would appreciate suggestions on how to name this new function.

1. Need some test cases of this functionality, including error conditions, for both new functions.

2. For the element() case, I think you should use the x:parse() function in the built-in XML module (http://www.zorba-xquery.com/modules/xml) with the "parse-external-parsed-entity" option. I'm not sure it'll make a difference, honestly, but it's slightly more correct.

3. I agree set-variable() isn't a great name. It should be more explicit that it is doing casting. Maybe bind-cast-variable() ?

4. It'd be nice if you could support eg. xs:integer+, and all known schema simple types. I have a suggestion - in xqxq:bind-cast-variable(), first check for object/array/element/document-node, and then for any other type, use XQXQ to let XQuery itself do the casting. You'll need to first strip the quantifier from the type string, and then construct a query that does the casting. This should work:

  variable $unquanttype := fn:replace($type, "[*?+]$", "");
  variable $caster := xq := xqxq:prepare-main-module(
     fn:concat("declare variable $val as xs:string* external; ",
               "for $v in $val return $v cast as ",
               $unquanttype));
  xqxq:bind-variable($caster, xs:QName("val"), $value);
  xqxq:evaluate($caster)

The result of that will be the $casted-value. It'd probably be a good idea to explicitly call xqxq:delete-query($caster) also.

The changes suggested by Chris have been included can you review again?

Looks good!

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug1064978-2013-04-11T00-04-50.241Z/log.html

The attempt to merge lp:~zorba-coders/zorba/bug1064978 into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275 (message):
  Validation queue job bug1064978-2013-04-11T00-04-50.241Z is finished. The
  final status was:

  184 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug1064978-2013-04-11T16-31-07.189Z/log.html

The attempt to merge lp:~zorba-coders/zorba/bug1064978 into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275 (message):
  Validation queue job bug1064978-2013-04-11T16-31-07.189Z is finished. The
  final status was:

  184 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

I don't think the bind-cast-variable should go in this module. It's clearly a convenient function that the user can write as well. As such, it would be OK but it adds an XML dependency to the module which is something that we don't want.

From the documentation, it's not clear what format the outType is. Is it a QName, EQName, or even a sequence type? In either case, it should probably be of type Item or a TypeIdentifier.

So, should I remove the xml dependency from the function? or should I remove the whole function?
If we remove the function the user should be able to create the same function by themselves using xqxq:get-type-variable
I will make the changes to getVariableType so it returns a TypeIdentifier or Item and make the documentation clearer.

> So, should I remove the xml dependency from the function? or should I remove
> the whole function?
> If we remove the function the user should be able to create the same function
> by themselves using xqxq:get-type-variable
I think we should remove the entire function. It leaves to many unanswered questions. For example,
what if the user wants to pass other parameters to the parse-fragment function or what if the user wants to validate items before passing them as arguments?

> > So, should I remove the xml dependency from the function? or should I remove
> > the whole function?
> > If we remove the function the user should be able to create the same
> function
> > by themselves using xqxq:get-type-variable
> I think we should remove the entire function. It leaves to many unanswered
> questions. For example,
> what if the user wants to pass other parameters to the parse-fragment function
> or what if the user wants to validate items before passing them as arguments?

So I made the following changes:
*Removed the implementation of bind-cast-variable
*Changed the return value of getVariableType from zorba::String to a string Item since if the function returns a TypeIdentifier getting a legible type of the variable is lost, unless there is a way to get the schema String from a typeIdentifier?

> > > So, should I remove the xml dependency from the function? or should I
> remove
> > > the whole function?
> > > If we remove the function the user should be able to create the same
> > function
> > > by themselves using xqxq:get-type-variable
> > I think we should remove the entire function. It leaves to many unanswered
> > questions. For example,
> > what if the user wants to pass other parameters to the parse-fragment
> function
> > or what if the user wants to validate items before passing them as
> arguments?
>
> So I made the following changes:
> *Removed the implementation of bind-cast-variable
> *Changed the return value of getVariableType from zorba::String to a string
> Item since if the function returns a TypeIdentifier getting a legible type of
> the variable is lost, unless there is a way to get the schema String from a
> typeIdentifier?
I think a string item doesn't make a difference. The type of a variable is a sequence type that consists of a QName and an occurrence indicator. Just having the name doesn't really help. What do you think is missing from the TypeIdentifier? It allows you to retrieve the URI, local name, and quantifier. What am I missing?

> > > > So, should I remove the xml dependency from the function? or should I
> > remove
> > > > the whole function?
> > > > If we remove the function the user should be able to create the same
> > > function
> > > > by themselves using xqxq:get-type-variable
> > > I think we should remove the entire function. It leaves to many unanswered
> > > questions. For example,
> > > what if the user wants to pass other parameters to the parse-fragment
> > function
> > > or what if the user wants to validate items before passing them as
> > arguments?
> >
> > So I made the following changes:
> > *Removed the implementation of bind-cast-variable
> > *Changed the return value of getVariableType from zorba::String to a string
> > Item since if the function returns a TypeIdentifier getting a legible type
> of
> > the variable is lost, unless there is a way to get the schema String from a
> > typeIdentifier?
> I think a string item doesn't make a difference. The type of a variable is a
> sequence type that consists of a QName and an occurrence indicator. Just
> having the name doesn't really help. What do you think is missing from the
> TypeIdentifier? It allows you to retrieve the URI, local name, and quantifier.
> What am I missing?

There are 2 things I miss information about typeIdentifier:
1. how to create a TypeIdentifier from a XQType
2. The function of xqxq is supposed to return the string of type function so getVariableType is returning the XQType toSchemaString so that the user can actually use it to cast the values of the variable, now does a typeIdentifier has a value like that? Does the localname represents that?

> > > > > So, should I remove the xml dependency from the function? or should I
> > > remove
> > > > > the whole function?
> > > > > If we remove the function the user should be able to create the same
> > > > function
> > > > > by themselves using xqxq:get-type-variable
> > > > I think we should remove the entire function. It leaves to many
> unanswered
> > > > questions. For example,
> > > > what if the user wants to pass other parameters to the parse-fragment
> > > function
> > > > or what if the user wants to validate items before passing them as
> > > arguments?
> > >
> > > So I made the following changes:
> > > *Removed the implementation of bind-cast-variable
> > > *Changed the return value of getVariableType from zorba::String to a
> string
> > > Item since if the function returns a TypeIdentifier getting a legible type
> > of
> > > the variable is lost, unless there is a way to get the schema String from
> a
> > > typeIdentifier?
> > I think a string item doesn't make a difference. The type of a variable is a
> > sequence type that consists of a QName and an occurrence indicator. Just
> > having the name doesn't really help. What do you think is missing from the
> > TypeIdentifier? It allows you to retrieve the URI, local name, and
> quantifier.
> > What am I missing?
>
> There are 2 things I miss information about typeIdentifier:
> 1. how to create a TypeIdentifier from a XQType
Take a look at CollectionImpl::getType in src/api/collectionimpl.cpp.

> 2. The function of xqxq is supposed to return the string of type function so
> getVariableType is returning the XQType toSchemaString so that the user can
> actually use it to cast the values of the variable, now does a typeIdentifier
> has a value like that? Does the localname represents that?
The problem is that there is no representation of types in the data model. To answer the question we need to investigate the use case. I think the function should either (1) return a QName item that represents the name of the type (maybe renaming the function to getVariablePrimeType) or (2) return a string using the sequence-type syntax (e.g. xs:string*, xs:integer?, or Q{http://foo.bar}my-type*).

> > > > > > So, should I remove the xml dependency from the function? or should
> I
> > > > remove
> > > > > > the whole function?
> > > > > > If we remove the function the user should be able to create the same
> > > > > function
> > > > > > by themselves using xqxq:get-type-variable
> > > > > I think we should remove the entire function. It leaves to many
> > unanswered
> > > > > questions. For example,
> > > > > what if the user wants to pass other parameters to the parse-fragment
> > > > function
> > > > > or what if the user wants to validate items before passing them as
> > > > arguments?
> > > >
> > > > So I made the following changes:
> > > > *Removed the implementation of bind-cast-variable
> > > > *Changed the return value of getVariableType from zorba::String to a
> > string
> > > > Item since if the function returns a TypeIdentifier getting a legible
> type
> > > of
> > > > the variable is lost, unless there is a way to get the schema String
> from
> > a
> > > > typeIdentifier?
> > > I think a string item doesn't make a difference. The type of a variable is
> a
> > > sequence type that consists of a QName and an occurrence indicator. Just
> > > having the name doesn't really help. What do you think is missing from the
> > > TypeIdentifier? It allows you to retrieve the URI, local name, and
> > quantifier.
> > > What am I missing?
> >
> > There are 2 things I miss information about typeIdentifier:
> > 1. how to create a TypeIdentifier from a XQType
> Take a look at CollectionImpl::getType in src/api/collectionimpl.cpp.
>
> > 2. The function of xqxq is supposed to return the string of type function so
> > getVariableType is returning the XQType toSchemaString so that the user can
> > actually use it to cast the values of the variable, now does a
> typeIdentifier
> > has a value like that? Does the localname represents that?
> The problem is that there is no representation of types in the data model. To
> answer the question we need to investigate the use case. I think the function
> should either (1) return a QName item that represents the name of the type
> (maybe renaming the function to getVariablePrimeType) or (2) return a string
> using the sequence-type syntax (e.g. xs:string*, xs:integer?, or
> Q{http://foo.bar}my-type*).

As for the purpose of the function, is to allow the user a tool so he can cast a string value into the corresponding type of external variable, that's why we need the function to return something logical in string which in this case the schema string of the type fits perfectly, so an actual type representation of the function is not that helpful if is not usable to cast the variable.

So let me see if I understand your points the first one, is to rename the function (which is appropriate) and to instead of returning the string item with the value turn it into a QName my only question would be the namespace of this QName should it be the same of the external variable? or a new namespace referring to this primetype, also I am guessing that the localname would be the schema string of the type.
As for your second suggestion I don't understand what you mean by it, are you talking about the xqxq.xq description o...

> > > > > > > So, should I remove the xml dependency from the function? or
> should
> > I
> > > > > remove
> > > > > > > the whole function?
> > > > > > > If we remove the function the user should be able to create the
> same
> > > > > > function
> > > > > > > by themselves using xqxq:get-type-variable
> > > > > > I think we should remove the entire function. It leaves to many
> > > unanswered
> > > > > > questions. For example,
> > > > > > what if the user wants to pass other parameters to the parse-
> fragment
> > > > > function
> > > > > > or what if the user wants to validate items before passing them as
> > > > > arguments?
> > > > >
> > > > > So I made the following changes:
> > > > > *Removed the implementation of bind-cast-variable
> > > > > *Changed the return value of getVariableType from zorba::String to a
> > > string
> > > > > Item since if the function returns a TypeIdentifier getting a legible
> > type
> > > > of
> > > > > the variable is lost, unless there is a way to get the schema String
> > from
> > > a
> > > > > typeIdentifier?
> > > > I think a string item doesn't make a difference. The type of a variable
> is
> > a
> > > > sequence type that consists of a QName and an occurrence indicator. Just
> > > > having the name doesn't really help. What do you think is missing from
> the
> > > > TypeIdentifier? It allows you to retrieve the URI, local name, and
> > > quantifier.
> > > > What am I missing?
> > >
> > > There are 2 things I miss information about typeIdentifier:
> > > 1. how to create a TypeIdentifier from a XQType
> > Take a look at CollectionImpl::getType in src/api/collectionimpl.cpp.
> >
> > > 2. The function of xqxq is supposed to return the string of type function
> so
> > > getVariableType is returning the XQType toSchemaString so that the user
> can
> > > actually use it to cast the values of the variable, now does a
> > typeIdentifier
> > > has a value like that? Does the localname represents that?
> > The problem is that there is no representation of types in the data model.
> To
> > answer the question we need to investigate the use case. I think the
> function
> > should either (1) return a QName item that represents the name of the type
> > (maybe renaming the function to getVariablePrimeType) or (2) return a string
> > using the sequence-type syntax (e.g. xs:string*, xs:integer?, or
> > Q{http://foo.bar}my-type*).
>
> As for the purpose of the function, is to allow the user a tool so he can cast
> a string value into the corresponding type of external variable, that's why we
> need the function to return something logical in string which in this case the
> schema string of the type fits perfectly, so an actual type representation of
> the function is not that helpful if is not usable to cast the variable.
>
> So let me see if I understand your points the first one, is to rename the
> function (which is appropriate) and to instead of returning the string item
> with the value turn it into a QName my only question would be the namespace of
> this QName should it be the same of the external variable? or a new namespace
> referring to this primetype, also I am guessing that the localname would be
> the sc...

I talked to Markos and there are some more issues here.

1. The function should probably go into the static context instead of the dynamic context.
2. It really depends on what you want to cast to. In general, the target type for casting is a sequence type (e.g. node()*, or xs:string?). As already mentioned, this cannot be returned as an item other than a string. If you did that, it pushes a lot of effort to the consumer because he needs to parse the sequence-type. There are two alternatives:
  - xqxq:variable-type-name($name as xs:QName) as xs:QName? which returns the name of type (e.g. no node()*)
  - xqxq:variable-sequence-type($name as xs:QName) as xs:string? which returns the sequence type as a string.

It really depends on exactly what the user needs. I would go with variable-type-name if that's sufficient for all FOTS use cases.

> I talked to Markos and there are some more issues here.
>
> 1. The function should probably go into the static context instead of the
> dynamic context.
But the Static context doesn't have functions to access the external variables, so if this function is to be implemented in the static context it would actually be using several functions of the dynamic context to work. Since the external variable are part of the Dynamic context isn't it better to just leave them there?

> 2. It really depends on what you want to cast to. In general, the target type
> for casting is a sequence type (e.g. node()*, or xs:string?). As already
> mentioned, this cannot be returned as an item other than a string. If you did
> that, it pushes a lot of effort to the consumer because he needs to parse the
> sequence-type. There are two alternatives:
> - xqxq:variable-type-name($name as xs:QName) as xs:QName? which returns the
> name of type (e.g. no node()*)
> - xqxq:variable-sequence-type($name as xs:QName) as xs:string? which returns
> the sequence type as a string.
>
> It really depends on exactly what the user needs. I would go with variable-
> type-name if that's sufficient for all FOTS use cases.
I will go ahead and make the change for it to return of a QName

> > I talked to Markos and there are some more issues here.
> >
> > 1. The function should probably go into the static context instead of the
> > dynamic context.
> But the Static context doesn't have functions to access the external
> variables, so if this function is to be implemented in the static context it
> would actually be using several functions of the dynamic context to work.
> Since the external variable are part of the Dynamic context isn't it better to
> just leave them there?
As I said, it doesn't belong there. If you want to have it in the dynamic context
you should only invoke it after execution.
I think you should use VarInfo* lookup_var(const store::Item* qname) const;
VarInfo contains the type of the variable.

>
> > 2. It really depends on what you want to cast to. In general, the target
> type
> > for casting is a sequence type (e.g. node()*, or xs:string?). As already
> > mentioned, this cannot be returned as an item other than a string. If you
> did
> > that, it pushes a lot of effort to the consumer because he needs to parse
> the
> > sequence-type. There are two alternatives:
> > - xqxq:variable-type-name($name as xs:QName) as xs:QName? which returns
> the
> > name of type (e.g. no node()*)
> > - xqxq:variable-sequence-type($name as xs:QName) as xs:string? which
> returns
> > the sequence type as a string.
> >
> > It really depends on exactly what the user needs. I would go with variable-
> > type-name if that's sufficient for all FOTS use cases.
> I will go ahead and make the change for it to return of a QName
Double check with Sorin whether he really doesn't need the occurrence indicator for casting.

> > > I talked to Markos and there are some more issues here.
> > >
> > > 1. The function should probably go into the static context instead of the
> > > dynamic context.
> > But the Static context doesn't have functions to access the external
> > variables, so if this function is to be implemented in the static context it
> > would actually be using several functions of the dynamic context to work.
> > Since the external variable are part of the Dynamic context isn't it better
> to
> > just leave them there?
> As I said, it doesn't belong there. If you want to have it in the dynamic
> context
> you should only invoke it after execution.
> I think you should use VarInfo* lookup_var(const store::Item* qname) const;
> VarInfo contains the type of the variable.
Oh that makes sense I will make it static context then.

>
> >
> > > 2. It really depends on what you want to cast to. In general, the target
> > type
> > > for casting is a sequence type (e.g. node()*, or xs:string?). As already
> > > mentioned, this cannot be returned as an item other than a string. If you
> > did
> > > that, it pushes a lot of effort to the consumer because he needs to parse
> > the
> > > sequence-type. There are two alternatives:
> > > - xqxq:variable-type-name($name as xs:QName) as xs:QName? which returns
> > the
> > > name of type (e.g. no node()*)
> > > - xqxq:variable-sequence-type($name as xs:QName) as xs:string? which
> > returns
> > > the sequence type as a string.
> > >
> > > It really depends on exactly what the user needs. I would go with
> variable-
> > > type-name if that's sufficient for all FOTS use cases.
> > I will go ahead and make the change for it to return of a QName
> Double check with Sorin whether he really doesn't need the occurrence
> indicator for casting.

The current implementation does return the occurrence indicator

the build doesn't work for me:

/home/mbrantner/zorba/sandbox/src/api/staticcontextimpl.cpp: In member function 'virtual zorba::Item zorba::StaticContextImpl::getVariablePrimeType(const zorba::Item&)':
/home/mbrantner/zorba/sandbox/src/api/staticcontextimpl.cpp:1808:12: error: could not convert 'type' from 'zorba::store::Item_t {aka zorba::store::ItemHandle<zorba::store::Item>}' to 'zorba::Item'

import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq';

variable $queryID := xqxq:prepare-main-module('declare variable $ext2 external; $ext2');

fn:local-name-from-QName(xqxq:variable-type-name($queryID, xs:QName('ext2')))

returns xs:anyType. Shouldn't it be anyType? I think the QName is not constructed
correctly.

xs:anyType should not be a string constant in the code. I think this should come from the typemanager.

> the build doesn't work for me:
>
> /home/mbrantner/zorba/sandbox/src/api/staticcontextimpl.cpp: In member
> function 'virtual zorba::Item
> zorba::StaticContextImpl::getVariablePrimeType(const zorba::Item&)':
> /home/mbrantner/zorba/sandbox/src/api/staticcontextimpl.cpp:1808:12: error:
> could not convert 'type' from 'zorba::store::Item_t {aka
> zorba::store::ItemHandle<zorba::store::Item>}' to 'zorba::Item'

I pushed a fix for the build.

import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq';

variable $queryID := xqxq:prepare-main-module('declare variable $ext2 as xs:integer? external; $ext2');

fn:local-name-from-QName(xqxq:variable-type-name($queryID, xs:QName('ext2')))

returns
xs:integer?.

That's not a local-name.

> import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq';
>
> variable $queryID := xqxq:prepare-main-module('declare variable $ext2
> external; $ext2');
>
> fn:local-name-from-QName(xqxq:variable-type-name($queryID, xs:QName('ext2')))
>
> returns xs:anyType. Shouldn't it be anyType? I think the QName is not
> constructed
> correctly.
What do you mean is not being constructed correctly?

>
> xs:anyType should not be a string constant in the code. I think this should
> come from the typemanager.
The thing with the variables that doesn't have a type specified in code when asked about the XQType of the variable using the function var->getType it returns a NULL pointer that is why a set a constant value of xs:anyType, now you don't want this to be constant what should I do then? create a new any type XQType variable using the TypeManager and return it's schemaString()?

This changes remove the constant return String on xs:anyType it now uses the type manager to create an anyType item type and return it's string value.

With this change the query you mentioned using local-name-from-QName works correctly.

Why don't you use XQType::getQName to get the QName directly and return it?

> Why don't you use XQType::getQName to get the QName directly and return it?
Because if the type is not atomic, if it is an element or an object it throws a zorba exception.

Please see https://bugs.launchpad.net/zorba/+bug/1064978/comments/9