Stream: fhirpath
Topic: Bad mistake in FHIRPath
Grahame Grieve (Jul 23 2018 at 23:57):
So I think we've made a bad mistake in the language, one that wastes a lot of my time.
Grahame Grieve (Jul 23 2018 at 23:57):
"" means a element with an illegal name
'' means a string constant
Grahame Grieve (Jul 23 2018 at 23:58):
this has wasted a lot of time of mine debugging something only to find out that I've used " not ' and I didn't notice. And I'm not the only one
Grahame Grieve (Jul 23 2018 at 23:59):
we should do something like [] (like SQL) around illegal names... and use " ' interchangeably.
Grahame Grieve (Jul 23 2018 at 23:59):
I expect that we'll have to decide it's too late to do anything about it..... but can we do anything to reduce the damage?
Jean Duteau (Jul 24 2018 at 00:02):
this is something like SQL and that always trips me up as well. i'd love it if we didn't recreate that problem and let " and ' be used for strings.
Bryn Rhodes (Jul 24 2018 at 06:23):
Would it make sense to have the compiler/interpreter/engine/processor give a warning in the case that a string resolves to an identifier?
Grahame Grieve (Jul 24 2018 at 06:29):
well, maybe. a warning where?
Grahame Grieve (Jul 27 2018 at 21:18):
but also, it's a warning if it doesn't.
Bryn Rhodes (Jul 28 2018 at 16:13):
I suppose it would go the same place that traces do, with some pointers back to context and/or source location. Not sure I follow "it's a warning if it doesn't"?
Grahame Grieve (Jul 28 2018 at 20:22):
the time to raise a warning is if "xx" doesn't match a known path, but has mapped to it anyway
Grahame Grieve (Jul 28 2018 at 20:23):
or maybe the solution is that if the engine doesn't find "xx" it falls back to treating it as a string?
Brian Postlethwaite (Jul 29 2018 at 11:31):
In my usage of it, I actually produce errors when the parse tree doesn't resolve identifiers with properties. Doesn't cater for all occasions, but usually the string doesn't exist as a property.
Brian Postlethwaite (Jul 29 2018 at 11:31):
But yes, is a source of potential issues.
Bryn Rhodes (Jul 29 2018 at 20:15):
"or maybe the solution is that if the engine doesn't find "xx" it falls back to treating it as a string?" I think this isn't well-defined. It should be an error if an identifier doesn't resolve to a valid path, whether that happens at compile-time or run-time is up to the environment, but it's still an error condition. A string could never resolve to a valid path, it's a string literal, not an identifier, so this is just a warning to say "you're using a string literal that resolved to a valid path in this context, did you mean to use an identifier instead?".
Grahame Grieve (Jul 29 2018 at 20:24):
I just think that all these approaches are wrong: we're trying to use tooling to work around a language mistake. and the problem for me is, in most places people use FHIRPath, there's not any logical place to handle these things - it's a language that lives in the cracks in most places (not like CQL)
Bryn Rhodes (Jul 29 2018 at 20:28):
It's a fair point, but what is the solution? About to get on a plane to Tanzania, but I'll give it some more thought on the plane.
Bryn Rhodes (Jul 29 2018 at 20:29):
Is changing FHIRPath to support both types of strings and introducing a new delimiter even an option at this point?
Brian Postlethwaite (Jul 29 2018 at 23:28):
If we're going to change it, now would be the time.
(I had to handle the quote change in my first version, and still migrating content)
Grahame Grieve (Jul 29 2018 at 23:31):
it feels too late, but I don't want to say that this is actually too late on this issue
Grahame Grieve (Jul 29 2018 at 23:32):
enjoy Tanzania - thanks for going
Bryn Rhodes (Aug 06 2018 at 02:53):
How about this: Change the identifier delimiter in FHIRPath to a backtick, so double-quotes wouldn't be allowed at all in FHIRPath. Then CQL can introduce a double-quoted identifier.
Bryn Rhodes (Aug 06 2018 at 02:53):
That way it doesn't clash with any existing symbol-use in FHIRPath or CQL, FHIRPath gets a non-string-looking identifier delimiter, and CQL can still have double-quoted identifiers (which is really important in the CQL space).
Grahame Grieve (Aug 06 2018 at 05:36):
by 'identifier' you mean 'element name'?
Bryn Rhodes (Aug 06 2018 at 14:18):
Element name is one possible resolution of an identifer, I mean identifier in the sense of the lexical element of the language. So everywhere we currently allow the use of quoted-identifiers would be changed to use back-ticks.
Bryn Rhodes (Aug 06 2018 at 14:19):
Probably the biggest impact would be in the use of context variables where we regularly use the quoted-identifiers to allow characters like - in the identifiers. So what is now %"us-zip" would become %`us-zip`
Bryn Rhodes (Aug 06 2018 at 14:22):
And looking at this page in the current build: http://build.fhir.org/fhirpath.html, I can see the issue occurs at least three times on that page. So with this change, the use of double-quotes would always be an error in FHIRPath.
Lloyd McKenzie (Aug 06 2018 at 14:41):
Back-tick vs. forward tick are going to be confusing to some...
Bryn Rhodes (Aug 06 2018 at 14:52):
Yes, but much less likely to be confused with string delimiters, especially in the web space where JSON is ubiquitous.
Michel Rutten (Aug 06 2018 at 14:56):
Backticks are somewhat unusual, might also be a bit hard to discern depending on font/rendering. What about the earlier proposed SQL-style square brackets? e.g. [identifier]
Bryn Rhodes (Aug 06 2018 at 14:58):
Square brackets would conflict with array indexers, as well as with the retrieve syntax in CQL.
Bryn Rhodes (Aug 06 2018 at 14:58):
And it seems like they would also be confused with array syntax in JSON.
Michel Rutten (Aug 06 2018 at 15:11):
Ah I see, thanks for explaining. Apparently the backtick character is not that unusual either:
https://en.wikipedia.org/wiki/Grave_accent#Use_in_programming
Paul Lynch (Aug 06 2018 at 17:24):
Another option (not that I object to back-ticks) would be to use braces around the identifiers, like %{us-zip}, sort of like the example in 5.6.8 (replaceMatches) in http://hl7.org/fhirpath/, and also sort of like JavaScript template literal syntax (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals). Braces make sense to me if the identifier is a variable name; I'm not sure about other cases.
Bryn Rhodes (Aug 06 2018 at 17:35):
Braces would conflict with list selector syntax.
Grahame Grieve (Aug 06 2018 at 19:04):
Square brackets would conflict with array indexers
Grahame Grieve (Aug 06 2018 at 19:04):
why?
nicola (RIO/SS) (Aug 06 2018 at 19:10):
In postgresql " used to quote identifiers and ' for strings - postgresql implements SQL standard better than others.
Grahame Grieve (Aug 15 2018 at 20:41):
@Bryn Rhodes
why?
....
Bryn Rhodes (Aug 15 2018 at 21:08):
You're right, square brackets wouldn't conflict with array indexers directly in FHIRPath, I misspoke. But they would conflict with the retrieve syntax in CQL.
Bryn Rhodes (Aug 15 2018 at 21:10):
We chose the double-quotes for identifiers and single-quotes for strings in CQL based on the fact that many (if not all) dialects of SQL support that.
Bryn Rhodes (Aug 15 2018 at 21:12):
Square brackets for identifiers is common in T-SQL flavors (Sybase, MSSQL, MySQL)
Bryn Rhodes (Aug 15 2018 at 21:15):
Actually, MySQL uses backticks (and has an option to turn on double-quotes).
Grahame Grieve (Aug 16 2018 at 10:28):
so.... where does that leave us?
Bryn Rhodes (Aug 16 2018 at 12:41):
Well, it seems to me we have two options, 1) leave it, or 2) change the identifier delimiter in FHIRPath to a backtick. FHIR has far and away the most usage of FHIRPath, so what is the practical impact of the change? Are there any actual invariants that use double-quoted identifiers?
Grahame Grieve (Aug 16 2018 at 20:26):
if there are in the spec, then they are in error
Grahame Grieve (Aug 16 2018 at 20:26):
there might be a few in implementers structure maps.
Bryn Rhodes (Aug 16 2018 at 20:46):
So it seems like if we're going to do it, now is the only windows. Straw poll?
Grahame Grieve (Aug 16 2018 at 20:47):
I think so.
Grahame Grieve (Aug 16 2018 at 20:51):
@all Straw Poll: FHIRPath says that:
Paths may use double quotes to include characters in path parts that would otherwise be interpreted as keywords or operators, e.g.: Message."PID-1"
This is confusing for implementers who often use " for strings. We could fix this by changing from " to `, and saying that strings can either be delimited by ' or ".
Please indicate your response to this as yes or no by responding to this post with either :thumbs_up: or :thumbs_down: (you can find that by scrolling further down the list of emojis)
Grahame Grieve (Aug 16 2018 at 20:51):
(use emoji by clicking on the down arrow on the top right against the post, and choose "Add emoji reaction"
Bryn Rhodes (Aug 16 2018 at 20:52):
Friendly amendment, strings would only be delimited by a single-quote. Double-quotes would be invalid syntax.
Grahame Grieve (Aug 16 2018 at 20:52):
you can vote for the amendment as well if you want. I'm... abstaining on the amendment (not sure....)
Bryn Rhodes (Aug 16 2018 at 20:53):
Allowing both single and double-quoted delimited strings in FHIRPath would represent a significant misalignment with CQL.
Grahame Grieve (Aug 16 2018 at 20:53):
oh?
Bryn Rhodes (Aug 16 2018 at 20:53):
Yes, if FHIRPath allowed double-quoted strings, I could no longer differentiate them as identifiers in CQL sytnax.
Grahame Grieve (Aug 16 2018 at 20:54):
ok. guess I'll vote for the amendment after all
Bryn Rhodes (Aug 16 2018 at 20:54):
Thank you :)
Paul Lynch (Aug 16 2018 at 21:54):
I don't suppose making the same change in CQL is something to be considered? Then there would not be a misalignment if double-quotes were allowed on strings.
Grahame Grieve (Aug 16 2018 at 21:57):
not to be considered - a massive set of existing content that would be invalidated, including explicit ballot syntax rules
Brian Postlethwaite (Aug 17 2018 at 00:34):
Backtick and single quote are so close to the same , and with some editors the font makes it even worse.
But I do support the removal of double quotes as valid syntax to force the issue.
Brian Postlethwaite (Aug 17 2018 at 00:35):
We did come to the conclusion that using the square brackets wasn't a conflict with array scopers right?
Bryn Rhodes (Aug 17 2018 at 00:37):
That's correct, it's not in conflict with array indexers within FHIRPath itself, but it would conflict with Retrieve syntax in CQL (because brackets can start an expression in CQL).
Brian Postlethwaite (Aug 17 2018 at 00:37):
Thanks, a CQL clash, ok.
Last updated: Apr 12 2022 at 19:14 UTC
