Hi Robert On 21.05.25 19:10, Robert Haas wrote: > Yeah. Just to be clear, I can't really think of committing a patch in > this area because I don't know the topic well enough. I can make some > general comments on what I see as issues with this patch but serious > review would really need to come from a committer who is more familiar > with the XML specifications than I am.
I understand. And I really do appreciate that you take the time to take a look at it nevertheless. > If nobody like that shows up, > this proposal won't be able to advance. So I'll keep my fingers crossed that somebody shows up :) > Well, the sentence begins with "Another option". Let's say we were > talking about making cookies. I could say "Another option, if you > don't have butter, is to substitute Crisco." But if I do that, I > should then go on to explain further: "However, if you do this, it may > affect the flavor of the cookies and they may brown differently in the > oven. Nevertheless, it's better than not having cookies." Your patch > seemed to me to be lacking any further explanation of this kind. When > we document that there are multiple options, we should try to give > some context to help the user choose between them. In my cookie-based > example, the additional text makes it clear why I would select the > Crisco option: I might be out of butter, and something is better than > nothing. In your case, it was not clear to me why someone should > choose XMLCAST over options or the other way around. > > To be clear, I don't want you to explain it *to me*. I want you to > explain it to the reader of the documentation. Got it. In v10 I added this to the documentation to make the difference to CAST clearer: Similar to the SQL function <function>CAST</function>, this function converts an <replaceable>expression into the specified <replaceable>type</replaceable>. It is primarily used for converting between SQL values and <type>xml</type> values in a standards-compliant way. Unlike <function>CAST</function>, which may coerce SQL values into text or XML without enforcing a specific lexical representation, <function>xmlcast</function> ensures that the conversion produces or expects a canonical XML Schema lexical form appropriate for the target type. For example, an <type>interval</type> value is rendered as <literal>P1Y2M</literal> (<type>xs:duration</type>), and a <type>timestamp</type> as <literal>2023-05-19T14:30:00Z</literal> (xs:dateTime). Similarly, when converting from XML to SQL types, <function>xmlcast</function> validates that the input string conforms to the lexical format required by the corresponding SQL type. >> Says the SQL/XML:2023 standard :) >> >> SQL/XML:2023 (ISO/IEC 9075-14:2023) - “General Rules” of §6.7.3 (d.ii.1 >> and d.ii.2): > Cool. You should put that in the patch. In v10 I changed these comments to: /* * SQL date/time types must be mapped to XML Schema types when casting to XML: * - DATE -> xs:date * - TIME [WITH/WITHOUT TZ] -> xs:time * - TIMESTAMP [WITH/WITHOUT TZ] -> xs:dateTime * * These mappings are defined in SQL/XML:2023 (ISO/IEC 9075-14:2023), * Subclause 6.7 "<XML cast specification>", item 15.e.i–v. * * The corresponding XML Schema lexical formats (e.g., "2023-05-19", "14:30:00Z", * "2023-05-19T14:30:00+01:00") follow ISO 8601 and are specified in * W3C XML Schema Part 2: Primitive Datatypes §3.2.7 (dateTime) and §3.2.9 (date). */ and /* * SQL interval types must be mapped to XML Schema types when casting to XML: * - Year-month intervals -> xs:yearMonthDuration * - Day-time intervals -> xs:dayTimeDuration * * This behavior is required by SQL/XML:2023 (ISO/IEC 9075-14:2023), * Subclause 6.7 "<XML cast specification>", General Rules, item 3.d.ii.1–2. * * These XML Schema types require ISO 8601-compatible lexical representations, * such as: "P1Y2M", "P3DT4H5M", or "P1Y2M3DT4H5M6S", as defined in * W3C XML Schema Part 2: Primitve Datatypes, §3.2.6 (duration) */ >> Given that XMLCast converts values between SQL and XML and vice versa, >> my rationale was that if the target type is not a text type (such as >> TEXTOID, VARCHAROID, or NAMEOID), then the cast operand must be of type >> xml, which makes this default: safe. >> [...] >> But I can see it looks unsafe. Do you have something like this in mind? >> [...] >> default: >> elog(ERROR, "unsupported target data type for XMLCast"); >> } > Yes, exactly. Done in v10. Thanks! Jim
