Java<->C++ Integration tests for dictionary-encoded vectors

[asfimport]

Reporter: Wes McKinney / @wesm
Assignee: Wes McKinney / @wesm

Related issues:

• Arrow 0.5.0 release (blocks)
• [C++] Implement JSON round trip for DictionaryArray (is blocked by)
• [Java] Add JSON support for dictionary vectors (is blocked by)

_{Note: This issue was originally created as ARROW-692. Please see the migration documentation for further details.}

[asfimport]

Bryan Cutler / @BryanCutler:
here is an example of JSON data with a dictionary I'll be working towards, let me know if there was something else in mind @wesm @julienledem

{
  "schema": {
    "fields": [
      {
        "name": "foo",
        "type": {"name": "utf8"},
        "nullable": true, "children": [],
        "typeLayout": {
          "vectors": [
            {"type": "VALIDITY", "typeBitWidth": 1},
            {"type": "OFFSET", "typeBitWidth": 32}
            {"type": "DATA", "typeBitWidth": 8}
          ]
        },
        "dictionary": {
          "id": 0, 
          "indexType": {"name": "int", "bitWidth": 8, "isSigned": false}, 
          "isOrdered": false
        }
      }
    ]
  },
  "dictionaries": [
    {
      "id": 0,
      "data": {
        "count": 3,
        "columns": [
          {
            "name": "foo",
            "count": 3,
            "VALIDITY": [1, 1, 1],
            "OFFSET": [0, 3, 6, 9], 
            "DATA": ["foo", "bar", "baz"]
          }
        ]
      }
    }
  ],
  "batches": [
    {
      "count": 6,
      "columns": [
        {
          "name": "foo",
          "count": 6,
          "VALIDITY": [1, 1, 0, 1, 1, 1],
          "DATA": [0, 1, 0, 1, 2]
        }
      ]
    }
  ]
}

[asfimport]

Wes McKinney / @wesm:
Sounds good. Keep in mind that dictionaries may be used more than once. I will try to include some data generation as part of writing the C++ portion of this

[asfimport]

Bryan Cutler / @BryanCutler:
Updated the sample JSON - the Field schema should be the dictionary type (utf8 here), and not the index type

@wesm and @julienledem I have a couple questions:

1. The "name" field in the dictionary is meaningless right? It's not part of the RecordBatch message. In Arrow Java, when writing it will be whatever name the user initializes the dictionary vector as. When reading, the dictionary vector will be the first Field name that has a dictionary encoding. Would it be better to overwrite any name to something standard like "DICT#" where # is the dictionary id?

2. Does it make sense for the dictionary field to be nullable? In Java the dictionary field nullable flag will be whatever the first field using that encoding is. Should nullable only be allowed to be false and enforce this when setting the dictionary field? Of course the encoded index field can be nullable.

[asfimport]

Julien Le Dem / @julienledem:
@BryanCutler

1. yes, the name is not important here. another option is to name it after the field it is used for but it is true that a dictionary can be used for multiple fields. I don't have a strong opinion there. The id field is already there so forcing it in the name is not that useful. The name field is optional in the schema so we can also say it is omitted for dictionaries.
2. i'd say a dictionary is always not nullable. nulls are stored in the validity vector of the index field.

[asfimport]

Wes McKinney / @wesm:
@BryanCutler I'm working on this on the C++ side. I'm wishing I had put more thought into sharing abstractions between the JSON reader/writer and the binary IPC reader/writer, as it probably would have saved me a lot of time, but it's too late now.

As a small detail, do you have any objection to nesting the dictionaries in the "schema" object? That turned out to make things simpler for me, but if it makes things harder on the Java side I can change the code.

Otherwise I think we are aligned; I will add the integration test fixtures in a follow up patch and I reckon there could be a small round of format reconciliation over small issues (but maybe we'll get lucky!)

[asfimport]

Bryan Cutler / @BryanCutler:
@wesm By dictionaries, you are asking about the dictionary record batches right? Adding the dictionaries to the schema object would make things more difficult on the Java side. Right now the Schema is written and read back as a single object (generator.writeObjectField("schema", schema);) with the class members that go into JSON having an annotation that lets the generator/parser know. So to put the dictionaries into the Schema, I would need to either re-write the JSON parts to not use annotations or add another member in the Schema class to hold the dictionaries - but that doesn't seem right. There might be someway to manipulate the built-in parser to do this, but I'm not sure. Would it be too difficult to keep the format above with dictionaries on the same level as the schema?

[asfimport]

Wes McKinney / @wesm:
So the way that schemas are being reconstructed in Java, at least last time I checked, doesn't seem quite right to me (the schema being modified post-facto). In C++, the dictionaries are held in the schema, and the schema is immutable – it proceeds in a first pass to find the dictionary types, then reads the dictionaries and stores them in a "DictionaryMemo" object, and then does a second pass of the schema metadata to reconstruct the schema. We were discussing this in @elahrvivaz's original patches. Seeing if @julienledem has any thoughts on this

One of the artifacts of the way that Java is set up right now is that dictionary-encoding in nested subfields is not supported, but we need to support that (e.g. List of Dictionary-encoded String). That's probably a more invasive refactoring

I will change my patch #750 to put the dictionaries at the top level of the JSON object; that's not such a big deal.

[asfimport]

Bryan Cutler / @BryanCutler:
Thanks Wes, that will help things on the Java side. Currently, Java also stores the dictionaries in a similar DictionaryMemo called "DictionaryProvider". This is an interface that is implemented by the ArrowFileReader and ArrowJsonReader, so these end up storing the dictionaries and allow to lookup one from a given id. The Schema is modified post-facto to change the type of the fields between the index type and dictionary type. So there is a little difference, but I don't know if there would be any impact to usage. I'll have to look into it more to see about support for nested subfields.

[asfimport]

Wes McKinney / @wesm:
It would be simpler for the schema to be immutable, and therefore constructed in a single pass. That makes it easier to support nested subfields.

As an example, see the "list of encoded string" field in https://gist.github.com/wesm/5100e41173a3b5e53437b7a887e4383a#file-gistfile1-json, we have

{
        "name": "list of encoded string",
        "nullable": true,
        "type": {
          "name": "list"
        },
        "children": [
          {
            "name": "item",
            "nullable": true,
            "type": {
              "name": "utf8"
            },
            "dictionary": {
              "id": 1,
              "indexType": {
                "name": "int",
                "bitWidth": 8,
                "isSigned": true
              },
              "isOrdered": false
            },
            "children": [],
            "typeLayout": {
              "vectors": [
                {
                  "type": "VALIDITY",
                  "typeBitWidth": 1
                },
                {
                  "type": "DATA",
                  "typeBitWidth": 8
                }
              ]
            }
          }
        ],
        "typeLayout": {
          "vectors": [
            {
              "type": "VALIDITY",
              "typeBitWidth": 1
            },
            {
              "type": "OFFSET",
              "typeBitWidth": 32
            }
          ]
        }
      }

[asfimport]

Wes McKinney / @wesm:
PR: #800

[asfimport]

Wes McKinney / @wesm:
Issue resolved by pull request 800
#800