emit absolute iris in w3c result formats - #1630
Conversation
bplatz
left a comment
There was a problem hiding this comment.
One inline comment: the datatype half of this fix stops one format short of where the same bug lives.
Merge note: this and #1629 both add module lines to fluree-db-api/tests/grp_query_sparql.rs (different anchors, so probably clean) and both remove entries from testsuite-sparql/tests/registers/mod.rs (different consts). Whichever lands second should re-run the full suite rather than trusting its own pre-merge green — this PR also changes what context the harness renders results with, which #1629's newly-greened agg-count-rows-distinct runs through.
| /// Whether a **string-backed** literal may be serialized without its `datatype`. | ||
| /// | ||
| /// `w3c_strict` selects the rule for the W3C result serializations (SPARQL | ||
| /// Results JSON, CSV, TSV — see [`crate::FormatterConfig::absolute_iris`]). |
There was a problem hiding this comment.
SPARQL Results XML belongs in this list. sparql_xml.rs:330 still calls is_inferable_datatype, so it has exactly the bug the doc comment above describes: <literal> content is text, nothing about the datatype is recoverable from the serialized form, and STRDT("2", xsd:integer) comes back as a plain literal — a different RDF term than the one the query produced.
The description says "SPARQL-XML was already correct," which is true for the IRI half (it never compacted) but not for this half. Its own test inferable_datatype_omitted_long currently pins the wrong behavior.
Either route sparql_xml through omit_datatype_for_string_literal(dt, true) here, or say explicitly in the description that XML is knowingly left on the loose rule — as written the PR reads as though all four W3C formats are now consistent, and they aren't.
There was a problem hiding this comment.
You're right, and it's worse in XML than it was in SRJ — thank you for catching this, it would have shipped a half-done boundary.
The SRJ gate only ever fired on the FlakeValue::String arm, so a stored integer escaped it by landing as FlakeValue::Long and taking a different arm that always wrote the datatype. XML has a single gate at sparql_xml.rs:330, above the match val, so it applied to every value kind: a stored xsd:long serialized as a bare <literal>42</literal>, not just the STRDT shape. So the format I'd described as "already correct" was actually the one leaking the most.
I've routed it through the shared rule. A few notes on what that touched:
- Renamed the helper to
may_omit_datatype(datatype.rs). The oldomit_datatype_for_string_literalname would have been actively misleading at the XML call site, since XML gates all value kinds through it, and the doc comment now names all four W3C formats rather than three. FormatterConfig::sparql_xml()now setsabsolute_iris, andformat_results_stringthreads it into the XML compactor. That flag is inert for IRIs here —write_sid_ref(sparql_xml.rs:252) streams namespace prefix + name and never consults the compactor, which is exactly why XML was already right on the IRI half — so it only carries the datatype rule. I went that way rather than hard-codingtrueat the call site sosparql_xml()stops being the one W3C constructor that doesn't declare itself one.inferable_datatype_omitted_longis noww3c_profile_keeps_datatype_on_inferable_types, asserting the datatype is emitted, plus the string-backedxsd:integer(STRDT) shape. I also had it assert that the loose profile still omits, so the test measures the flag rather than a constant.inferable_datatype_omitted_stringstays as-is — a literal with no datatype and noxml:langgenuinely is anxsd:string, so that's the one omission that's exact rather than lossy.- The
it_w3c_result_formats.rsSTRDT matrix now runs the XML writer alongside SRJ, and the four-prologue matrix asserts the stored literal'sdatatypesurvives in XML too.
I checked the new assertions actually bite: reverting just the absolute_iris line on sparql_xml() fails them with <literal>2</literal> for both STRDT("2", xsd:integer) and the stored xsd:integer — i.e. exactly the term-identity change you described.
Full testsuite-sparql re-run after this commit was 1420 total / 1231 passed / 189 ignored / 0 failed — unchanged, i.e. the XML change moves no W3C test on its own. (The post-rebase number is different and better; see the merge-note reply below.) fluree-db-api is 3133 / 0, and server and CLI are green too, since the server serves sparql-results+xml.
I've also updated the PR description — the "SPARQL-XML was already correct" line now says it was correct on the IRI half only, and there's a paragraph on the datatype half covering all four formats. You were right that as written it read as though the four were consistent when they weren't.
bplatz
left a comment
There was a problem hiding this comment.
Left some feedback below to look at before merge
46358b7 to
27fbced
Compare
|
@bplatz — on the merge note: You called this exactly right, and #1629 landed first, so this PR is the lands-second party — I've rebased onto Result at the rebased head: 1420 total / 1232 passed / 188 ignored / 0 failed, against 1231 / 189 before the rebase. That +1 passed / −1 ignored is The textual merge was clean, and I checked both overlap points rather than trusting that: |
When a SPARQL query declares a prefix, the lowerer builds a JSON-LD-style context from the prologue and the formatters compact against it. That's the intended behavior for our JSON-LD-flavored outputs and CLI display — but it also reached
application/sparql-results+json, CSV, and TSV, where the specs define aurivalue as the absolute IRI and the formats carry no prefix map for a consumer to expand with. ABASE-only prologue was worse still: relative IRIs in the output. SPARQL-XML was already correct on this half — it never compacted node IRIs.FormatterConfiggrows anabsolute_irisprofile, true for the W3C-format constructors and threaded into the compactor as a suppress flag (same pattern as the existing graph-source flag). The CLI's SPARQL display paths explicitly construct the compacting variant, so--format json/csv/tsv keep today's output — that contract is now pinned by a CLI test rather than being emergent. In the same profile, datatype omission is narrowed toxsd:string: the old allow-list dropped tags from string-backed typed literals (e.g.STRDTresults), which changes term identity in a format whose values are always text.That datatype rule applies to all four W3C result formats, XML included. SPARQL-XML gated on the same
is_inferable_datatypeallow-list, and more broadly than SRJ did — its single gate covered every value kind, so a storedxsd:longserialized as a bare<literal>42</literal>.<literal>content is text, so nothing about the datatype survives serialization there either, and its own test pinned the wrong behavior. Both writers now route throughmay_omit_datatype, so the four formats are genuinely consistent.The W3C harness previously rendered results with an empty context, so compaction was invisible to it; it now renders with the query's own context and the full suite stays green against the fixed writer — and the datatype change greened five equality tests whose register entries had misattributed the failures. Behavior note for release: SRJ/CSV/TSV consumers that had adapted to compacted output will see absolute IRIs, and SRJ/XML consumers will see
datatypeon typed literals that previously arrived bare; every consumer that handles the fuller form is unaffected.