Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4070

Generated PHP for Thrift sets are incompatible with PHP

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: PHP - Compiler
    • Labels:
      None
    • Flags:
      Important

      Description

      In the `TBase` class that all Thrift generated code inherits from, there is a `_writeList` method that takes a boolean of whether or not the "list" in question is a set. The assumption here is that since PHP has no built-in Set data type, we can duplicate the uniqueness constraint of Sets by looking at the keys of a PHP associative array. Here's the associated code in `_writeList`:

      ```
      foreach ($var as $key => $val) {
      $elem = $set ? $key : $val;
      if (isset($ewrite))

      { $xfer += $output->$ewrite($elem); }

      else {
      switch ($etype) {
      case TType::STRUCT:
      $xfer += $elem->write($output);
      break;
      ```

      I want to point out again that if we're using a `TType::SET`, the `$set` variable will be `true`, and `$elem` will be the `$key`, not the `$value`. Here's the problem, from PHP array documentation (http://php.net/manual/en/language.types.array.php):

      "The key can either be an integer or a string. The value can be of any type...Arrays and objects can not be used as keys. Doing so will result in a warning: Illegal offset type."

      In other words, it is impossible to implement a Thrift set of anything other than strings or integers given the current implementation of PHP generated code.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              elreimundo David Reiman
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated: