search

OpenDDS / OpenDDS

OpenDDS is an open source C++ implementation of the Object Management Group (OMG) Data Distribution Service (DDS). OpenDDS also supports Java bindings through JNI.
http://www.opendds.org
Other
1.29k stars 465 forks source link

Quote query condition string parameters or not? #2449

Closed jwillemsen closed 3 years ago

jwillemsen commented 3 years ago

We identified a difference in how OpenDDS and RTI Connext DDS handle string parameters. When we have a query as "name = %0" where name is a string, should the string passed as query parameter for %0 be single quoted or not?

RTI Connext DDS expects that the query parameter passed for %0 should have quotes, see https://community.rti.com/comment/5669#comment-5669 and https://community.rti.com/examples/polling-query-condition.

OpenDDS seems to expect that the query parameter passed for %0 should not have quotes, will create a PR with a test extension

The OMG DDS 1.4 specification says in B2 page 167 the following:

STRING - Any series of characters encapsulated in single quotes, except a new-line character or a right quote. A string starts with a left or right quote, but ends with a right quote.

This difference is a challenge in writing portable user code, especially when the user shouldn't be aware of the concrete DDS vendor. Is there a specific reason why OpenDDS decided to not use single quotes for strings, or is this an oversight?

mitza-oci commented 3 years ago

This value shouldn't have single quotes in it, unless the intent is to compare to an actual single quote in the data sample:

param[0] = "'hello world'"; // NO
param[0] = "hello world"; // YES

The filter parameters themselves are strings, not expressions in the filter language. This is consistent with SQL usage.

jwillemsen commented 3 years ago

My testing showed that with OpenDDS it should be "hello world" but seems RTI requires "'hello world'" which is very very annoying. I have also opened an issue at RTI to get their feedback on this.

jwillemsen commented 3 years ago

See https://community.rti.com/static/documentation/connext-dds/6.0.1/doc/api/connext_dds/api_cpp/group__DDSQueryAndFilterSyntaxModule.html for the RTI documentation, they clearly state that single quotes should be used around string parameters.

mitza-oci commented 3 years ago

Is there a specific reason why OpenDDS decided to not use single quotes for strings, or is this an oversight?

OpenDDS is only based on the spec, not on RTI's implementation decisions. The spec doesn't have any requirement to treat the content of a filter parameter as a certain special kind of expression, it's just a string.

jwillemsen commented 3 years ago

We opened an issue at RTI and an issue at the OMG for the DDS spec itself, this should be clear to users, the current mismatch between implementations makes portability of user code harder. I will update my PR to use "data_B"

mitza-oci commented 3 years ago

If it needs to change in OpenDDS it could be controlled by an option that defaults to the current behavior. I'd rather not have to maintain that, but it should be a simple change to deal with locally. Discovery may be a bit more complex.

jwillemsen commented 3 years ago

Looks OpenSplice also doesn't use the single quote, see https://github.com/ADLINK-IST/opensplice/blob/629d6bf19b0df658723d41a56b5452f43e97d065/examples/dcps/WaitSet/java/src/WaitSetDataSubscriber.java#L72. At the moment I have a concrete feedback from RTI I will add it here

jwillemsen commented 3 years ago

This looks something to me that should be documented in the OpenDDS dev guide, add an example using a string and add a note that some other DDS vendors do require the single quotes, that is something anyone migrating from RTI to OpenDDS should be aware of.

jwillemsen commented 3 years ago

RTI reported back that they also now find the specification ambiguous and that this should be addressed in the next DDS standard revision. When this becomes a bigger problems for the user code we should look at making the quoting an option. They have an internal issue with ID CORE-11236.

mitza-oci commented 3 years ago

This looks something to me that should be documented in the OpenDDS dev guide, add an example using a string and add a note that some other DDS vendors do require the single quotes, that is something anyone migrating from RTI to OpenDDS should be aware of.

Yes, we can add to the DevGuide. All three Content Subscription features (query condition, content-filtered topic, multitopic) can use these parameters which the spec calls "query parameters" or "expression parameters" depending on the context -- but they're the same either way.

mitza-oci commented 3 years ago

This is now in the Developer's Guide.

jwillemsen commented 3 years ago

Thanks for adding this to the DevGuide

jrw972 commented 2 months ago

https://github.com/omg-dds/dds-rtps/issues/37

jrw972 commented 2 months ago

https://issues.omg.org/browse/DDS15-319