ArrayOfStringsSummary copy function failing in Union between Tuple and Theta Sketch

[cboumalh]

Hi all,

Not sure if this a bug or a feature, but the if we try to union a tuple sketch (string array summary) and a theta sketch using ArrayOfStringsSummaryFactory().newSummary() for the summary argument in the union function, we get an error here since the stringArr is null. Is this fine?

Thank you!

Chris

[leerho]

Could you please provide a runnable snippet of code that reproduces this error.

[cboumalh]

Sure, code is below (6.2.X)

import org.apache.datasketches.tuple.strings.{ArrayOfStringsSummary, ArrayOfStringsSummaryFactory, ArrayOfStringsSummarySetOperations}
import org.apache.datasketches.tuple.{Union}
import org.apache.datasketches.theta.UpdateSketch;
import org.apache.datasketches.tuple.UpdatableSketchBuilder;


val thetaBldr = UpdateSketch.builder();
val thetaSk = thetaBldr.build();

val ufactory = new ArrayOfStringsSummaryFactory()
val tupleBldr = new UpdatableSketchBuilder[Array[String], ArrayOfStringsSummary](ufactory);
val tupleSk = tupleBldr.build()

thetaSk.update("a");
thetaSk.update("b");
thetaSk.update("c");

tupleSk.update("a", Array("x", "y"))
tupleSk.update("b", Array("z"))
tupleSk.update("e", Array("x", "z"))

val stringArraySetOps = new ArrayOfStringsSummarySetOperations()

val union = new Union(stringArraySetOps);

union.union(tupleSk)

// I'm assuming we can call the update method here on the summary returned by .newSummary()
// but would we want to just have it default to something that does not throw an 
// error (similar to double and integer defaults)?
union.union(thetaSk, ufactory.newSummary());

Error:

java.lang.NullPointerException: Cannot invoke "[Ljava.lang.String;.clone()" because "nodesArr" is null

[leerho]

@cboumalh,
First, I don't appreciate that what you submitted was not even Java, much less even runnable from Java. So I had to translate it to Java and try to figure out what you were trying to do. Please don't do that!

The problem you were having was in your last line. The default ArrayOfStringsSummary returned by the ArrayOfStringsSummaryFactory has a stringArr value of null. If you look at the code you can see that all the factory was doing was returning a new summary. So instead of using the factory you can just supply a new summary initialized with the string array of your choice. That fixes the null pointer exception.

The other problem with your code is that the union does nothing.

The ThetaUnion and TupleUnion operate first in the hash domain. Your ThetaSketch is created with the hashes of a,b, and c. And your TupleSketch is created with the same three hashes. So when you union the ThetaSketch into the TupleSketch, the sketch sees that the all the hash values are the same so nothing happens.

Now in the code below, uncomment line 19. This adds a new hash to the ThetaSketch that does not exist with the TupleSketch.
Now run the code again. You will see that the ThetaSketch hashes have been associated with a different string array by line 28.

package org.apache.datasketches.tuple.strings;

import org.apache.datasketches.theta.UpdatableThetaSketch;
import org.apache.datasketches.theta.UpdatableThetaSketchBuilder;
import org.apache.datasketches.tuple.TupleSketchIterator;
import org.apache.datasketches.tuple.TupleUnion;
import org.testng.annotations.Test;

public class ArrayOfStringsSummary_Issue699 {
  UpdatableThetaSketch thetaSk = UpdatableThetaSketch.builder().build();
  ArrayOfStringsTupleSketch tupleSk = new ArrayOfStringsTupleSketch();
  TupleUnion<ArrayOfStringsSummary> union = new TupleUnion<>(new ArrayOfStringsSummarySetOperations());

  @Test
  void go() {
    thetaSk.update("a");
    thetaSk.update("b");
    thetaSk.update("c");
    //thetaSk.update("d");   //line 19.  uncomment
    
    tupleSk.update("a", new String[] {"x", "y"});
    tupleSk.update("b", new String[] {"z"});
    tupleSk.update("c", new String[] {"x", "z"});
    System.out.println("Print Summary before union");
    printSummaries(tupleSk.iterator());
    
    union.union(tupleSk);
    union.union(thetaSk, new ArrayOfStringsSummary(new String[] {"u", "v"})); //line 28
    
    System.out.println("Print Summary after union");
    printSummaries(union.getResult().iterator());
  }
  
  static void printSummaries(TupleSketchIterator<ArrayOfStringsSummary> it) {
    while (it.next()) {
      String[] strArr = it.getSummary().getValue();
      for (String s : strArr) {
        System.out.print(s + ", ");
      }
      System.out.println();
    }
    System.out.println();
  }
}

This code is written with the new datasketches-java version 9.0.0 and Java 25

[leerho]

The output from the above before line 19 is uncommented:

Print Summary before union
z, 
x, y, 
x, z, 

Print Summary after union
z, 
x, y, 
x, z, 

The output with line 19 uncommented:

Print Summary before union
z, 
x, y, 
x, z, 

Print Summary after union
z, 
x, y, 
x, z, 
u, v, 

[cboumalh]

Hi @leerho, apologies about the language! It should have been with Java I agree. Just want to point out that the Tuple and Theta sketches I created are not with the same three hashes :) One is a, b, c the other is a, b, e. I guess my questions are these:

1. Why do we initialize it to null instead of a empty string array (lazily maybe)?
2. Would we want to consider having stringArr set to an empty string array by default to avoid the NullPointerException without having to update the array?
3. Would we want to consider modes of aggregation for the string arrays (concat, union, intersection, ...)?

Thanks again for the response!

[leerho]

@cboumalh,
You are correct. Setting the private class member "stringArr = new String[] {}" is simpler. I have fixed ArrayOfStringsSummary accordingly in /main/ and will add a test similar to what I posted before (with your correction) into /test/. It will show up in a release eventually. Since there is a simple workaround, I would like to wait and see if other bugs show up before I commit to a new release. :)

[cboumalh]

@leerho, Thank you for the quick fix! I also wanted to follow up on my third question from my previous comment that might have been missed. If this is something worth considering, I'm happy to open a separate feature request issue with more detailed use cases and proposals. Or if the current ArrayOfStringsSummarySetOperations already supports this in some way (or is planning to), please let me know.

Thanks again for your help!

[leerho]

As for your Question #3.

Remember what I mentioned earlier. "The ThetaUnion and TupleUnion operate first in the hash domain." And the Set Operations are designed primarily in the hash domain. In your example, The ThetaSketch has hashes (keys) a, b, c, and the TupleSketch has hashes a, b, e. So the union of those two sketches would be a sketch with keys a, b, c, e. The intersection would be a sketch with keys a, b.

But what to do with the Summaries if the hashes(keys) are the same. This is very case specific and hard to generalize especially with String arrays and can be ambiguous.

Setting up different "Modes", as we did in /tuple/aninteger/ might make sense, but it is simple enough where you can create your own *SetOperations class that defines the modes you need.

[cboumalh]

This makes sense, I was just curious. Thank you for the help with this and all the work the team has done with this library.

[leerho]

Thank you for your interest in the library and helping us perfect it. :)