While working on a new LCB travel sample backend, I was previously using 3.1.3 and then upgraded to 3.2.0 and noticed an error when trying to set the positional query params. After some investigation, I realized that the root cause was a change in behavior with the lcb_cmdquery_positional_param() and lcb_cmdanalytics_positional_param() functions.
Previously, these functions accumulated positional parameters one at a time, and built the underlying array for the query. So it was possible to just add a simple JSON string value for example (including the quotes), as our "minimal" query.c example illustrated. However, the new strategy expects a single JSON array to be passed (and formatted as a JSON array) and then if it's called again, the array is overwritten. With the new approach, if a simple JSON string value is passed, LIBC would throw an invalid parameter error because it's not a JSON array. So these behaviors are very different and the expected implementation and documentation isn't very clear.
To fix this, we need to revert the logic for the previous functions: lcb_cmdquery_positional_param() and lcb_cmdanalytics_positional_param(), but then deprecate them, introduce a new set of functions lcb_cmdquery_positional_params() and lcb_cmdanalytics_positional_params() and document both to make sure each are clear.
The documentation for the deprecated functions will state that JSON values can be passed and will be accumulated internally. The documentation for the new functions will state that a JSON array is expected and if it's called again it will overwrite the previous JSON array.
- ✅ Revert the logic for the previous functions so we will not break current users, example code, and documented sample code.
- ✅ Update the API ref documentation for the previous functions, mark them deprecated, and make sure the expected behavior is clear (see comments above).
- ✅ Add the new logic (expecting JSON arrays) with the new function name, add an example that exercises this approach, and update sample code.
- ✅ Update the API ref documentation for the new functions and make sure the expected behavior clear (see comments above).
- ✅ Add unit test coverage for BOTH the previous and the new functions to be sure we don't introduce a regression on the expected behavior until we are ready to remove the previous method in the future.
CCBC-1441: Update the docs site, examples, and samples so be sure they are using (and documenting correct use of) the new methods that expect a JSON array.
The best workaround for users affected by this bug is to switch to using named parameters instead. If they update code to match the new behavior, it's only going to change again when we fix this errant behavior and add the new method.
|For Gerrit Dashboard: CCBC-1429|
|158001,5||CCBC-1429: fixed positional parameters for query/analtyics||master||libcouchbase||Status: MERGED||+2||+1|
|158291,2||PCBC-777: workaround for query positional parameters||master||php-couchbase||Status: MERGED||+2||+1|