Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1040

ColumnValue and TypedValue are unnecessarily both repeated

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.0
    • Fix Version/s: 1.6.0
    • Component/s: avatica
    • Labels:
      None

      Description

      Carter Shanklin sent me some nice notes from working on a Python interface to the Phoenix QueryServer. He saw this strange thing in the code he had to write:

      count = response.results[0].first_frame.rows[0].value[0].value[0].number_value
      

      The value[0].value[0] is strange. Looking at the protobuf definition, it seems like both ColumnValue and TypedValue are repeated which is causing this. Only one of them should be repeated, not both. This creates the equivalent of a 2dim array inside each row instead of just a 1dim array.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.6.0 (2016-01-22).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.6.0 (2016-01-22).
          Hide
          elserj Josh Elser added a comment -

          Thanks, Julian.

          Show
          elserj Josh Elser added a comment - Thanks, Julian.
          Show
          julianhyde Julian Hyde added a comment - Revised fix http://git-wip-us.apache.org/repos/asf/calcite/commit/361096ba .
          Hide
          julianhyde Julian Hyde added a comment -

          No! I'm in the middle of fixing the previous change.

          Show
          julianhyde Julian Hyde added a comment - No! I'm in the middle of fixing the previous change.
          Hide
          elserj Josh Elser added a comment - - edited

          Ack, thanks for the catch! I can push a change if you'd like?

          Show
          elserj Josh Elser added a comment - - edited Ack, thanks for the catch! I can push a change if you'd like?
          Hide
          julianhyde Julian Hyde added a comment -

          Minor nit: Please remember to use descriptive verb phrase for method javadoc: "Determines if this message ..." not "Determine if this message ...".

          Show
          julianhyde Julian Hyde added a comment - Minor nit: Please remember to use descriptive verb phrase for method javadoc: "Determines if this message ..." not "Determine if this message ...".
          Hide
          elserj Josh Elser added a comment -

          Fixed in https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=942693e67d1916264c98e0e803040e7f1f4311e3

          Took the high path and made sure that, when present, we still do the same (busted) parsing on the protobuf. This should make sure that we don't break 1.6 clients talking to 1.5 servers (the big reason we went protobuf to start).

          Included tests for the 'old style' records, as well as added some arrays being serialized/deserialized to make sure they were still working correctly.

          Docs also included (will probably just wait for the 1.6.0 release to happen instead of updating immediately). scalar_value will be set when the column is a scalar, and array_value when it's an array.

          Show
          elserj Josh Elser added a comment - Fixed in https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=commit;h=942693e67d1916264c98e0e803040e7f1f4311e3 Took the high path and made sure that, when present, we still do the same (busted) parsing on the protobuf. This should make sure that we don't break 1.6 clients talking to 1.5 servers (the big reason we went protobuf to start). Included tests for the 'old style' records, as well as added some arrays being serialized/deserialized to make sure they were still working correctly. Docs also included (will probably just wait for the 1.6.0 release to happen instead of updating immediately). scalar_value will be set when the column is a scalar, and array_value when it's an array.

            People

            • Assignee:
              elserj Josh Elser
              Reporter:
              cartershanklin Carter Shanklin
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development