Uploaded image for project: 'Couchbase C client library libcouchbase'
  1. Couchbase C client library libcouchbase
  2. CCBC-1429

Fix Positional Params (Query & Analytics)

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • Major
    • 3.2.1
    • 3.2.0
    • library
    • None
    • 3
    • SDK32: Eventing Mgmt

    Description

      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.

      Resolution

      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.

      Acceptance Criteria

      • 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.

      Workaround

      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.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              avsej Sergey Avseyev
              ray.cardillo Ray Cardillo
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty