Skip to content

835 Add built-in named record types to static context#1991

Merged
ndw merged 2 commits into
qt4cg:masterfrom
michaelhkay:835_built-in-named-record-types
May 20, 2025
Merged

835 Add built-in named record types to static context#1991
ndw merged 2 commits into
qt4cg:masterfrom
michaelhkay:835_built-in-named-record-types

Conversation

@michaelhkay
Copy link
Copy Markdown
Contributor

This PR adds six built-in named record types to the static context of every application:

Record [key-value-pair]
Record [load-xquery-module-record]
Record [parsed-csv-structure-record]
Record [random-number-generator-record]
Record [schema-type-record]
Record [uri-structure-record]

These are now listed in Appendix C of F&O

Issue 835 requests a review of the names of these records; perhaps putting them in one place will make that review easier. Personally, I am happy with the names as currently defined.

@michaelhkay michaelhkay added XPath An issue related to XPath XQuery An issue related to XQuery XQFO An issue related to Functions and Operators Enhancement A change or improvement to an existing feature Tests Needed Tests need to be written or merged labels May 11, 2025
@michaelhkay
Copy link
Copy Markdown
Contributor Author

Fix #835

@michaelhkay
Copy link
Copy Markdown
Contributor Author

Note also (and see also issue #1484) we are inconsistent as to whether these record types are extensible or not. We ought to be systematic about this.

@ChristianGruen
Copy link
Copy Markdown
Contributor

We could discuss…

  • The necessity to add all these record types to the static context: What do we gain by that?
  • Naming: Only one record type has no -record suffix
  • Possibly missing types: The spec mentions value records, and there is not type for the result of fn:divide-decimals.

@michaelhkay
Copy link
Copy Markdown
Contributor Author

Yes, all very good points.

On the first point, what we gain is that variables can be declared with these types, and if variables are declared with a precise type then we are more likely to detect errors early.

I decided to put forward a proposal that listed the named record types as currently defined, so that we could see them in one place and review them.

I think the other question that is worth discussing is whether the types should be extensible. Some of them are, some aren't, and there are arguments both ways.

@michaelhkay
Copy link
Copy Markdown
Contributor Author

michaelhkay commented May 11, 2025

Also worth noting: some of the generated test cases in misc-BuiltInKeywords rely on these types being in the static context, e.g.

<test-case name="Keywords-fn-random-number-generator-1">
      <description>Test of keyword arguments in fn:random-number-generator</description>
      <created by="generate-keyword-test-set.xsl" on="2025-05-06"/>
      <environment ref="ka"/>
      <test>fn:random-number-generator(seed := ?) instance of function(xs:anyAtomicType?) as fn:random-number-generator-record</test>
      <result>
         <assert-true/>
      </result>
   </test-case>

And that particular example can't be expressed any other way, because fn:random-number-generator-record is a recursive type so it can't be replaced by its expansion.

@michaelhkay michaelhkay added Tests Added Tests have been added to the test suites and removed Tests Needed Tests need to be written or merged labels May 11, 2025
@michaelhkay michaelhkay force-pushed the 835_built-in-named-record-types branch from 3c52f6b to 7228e57 Compare May 17, 2025 14:16
@ChristianGruen
Copy link
Copy Markdown
Contributor

It seems (as far as I can judge) that the addition of the record types is driven a lot by the tests. Maybe we should rather extend the test suite (drivers) to test record types, or rewrite the few corresponding tests.

Apart from that, we should ask ourselves if the record may become important in practice. My guess is that key-value-pair will be the only relevant candidate, and that others, like parsed-csv-structure-record, can hardly be memorized.

@michaelhkay
Copy link
Copy Markdown
Contributor Author

michaelhkay commented May 19, 2025

It seems (as far as I can judge) that the addition of the record types is driven a lot by the tests.

No, I don't think that's true. The fact that we had failing tests was the trigger, but it seems entirely reasonable to me that if the function signatures use named record types then those names should be available to applications. It's based more on the idea that you're likely to want to bind variables to values used as the argument or result of a function, and it's better to allow such variables to declare their type using a system-defined name rather than repeating the type definition, which could easily get out of sync with the spec. Not quite encapsulation, but a little bit of abstraction.

@ndw
Copy link
Copy Markdown
Contributor

ndw commented May 20, 2025

At meeting 122, the CG agreed to merge this PR.

@ndw ndw merged commit a1853a9 into qt4cg:master May 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement A change or improvement to an existing feature Tests Added Tests have been added to the test suites XPath An issue related to XPath XQFO An issue related to Functions and Operators XQuery An issue related to XQuery

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants