ARROW-5862: [Java] Provide dictionary builder #4813

liyafan82 · 2019-07-05T10:54:16Z

The dictionary builder servers for the following scenario which is frequently encountered in practice when dictionary encoding is involved: the dictionary values are not known a priori, so they are determined dynamically, as new data arrive continually.

In particular, when a new value arrives, it is tested to check if it is already in the dictionary. If so, it is simply neglected, otherwise, it is added to the dictionary.

When all values have been evaluated, the dictionary can be considered complete. So encoding can start afterward.

The code snippet using a dictionary builder should be like this:

DictonaryBuilder dictionaryBuilder = ...
dictionaryBuilder.startBuild();
...
dictionaryBuild.addValue(newValue);
...
dictionaryBuilder.endBuild();

codecov-io · 2019-07-06T16:16:32Z

Codecov Report

Merging #4813 into master will increase coverage by 17.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #4813       +/-   ##
==========================================
+ Coverage   72.34%   89.7%   +17.35%     
==========================================
  Files         831     670      -161     
  Lines      110383   99383    -11000     
  Branches     1418       0     -1418     
==========================================
+ Hits        79853   89148     +9295     
+ Misses      30168   10235    -19933     
+ Partials      362       0      -362

Impacted Files	Coverage Δ
cpp/src/arrow/testing/gtest_util.h	`97.36% <0%> (-2.64%)`	⬇️
cpp/src/arrow/compute/kernel.h	`61.16% <0%> (-1.53%)`	⬇️
cpp/src/arrow/result.h	`91.3% <0%> (-0.37%)`	⬇️
python/pyarrow/tests/test_parquet.py	`96.33% <0%> (-0.07%)`	⬇️
cpp/src/arrow/table.h	`100% <0%> (ø)`	⬆️
cpp/src/arrow/flight/server.h	`100% <0%> (ø)`	⬆️
cpp/src/arrow/csv/column-builder.h	`100% <0%> (ø)`	⬆️
cpp/src/arrow/table_builder.h	`100% <0%> (ø)`	⬆️
cpp/src/plasma/plasma.h	`100% <0%> (ø)`	⬆️
cpp/src/gandiva/configuration.h	`100% <0%> (ø)`	⬆️
... and 760 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 167cea0...d6038e4. Read the comment docs.

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

java/algorithm/src/main/java/org/apache/arrow/algorithm/dictionary/DictionaryBuilder.java

emkornfield

I'd prefer to collapse the the class hierarchy and refactor once we have a second example. It seems plausible that you could potentially make use of composition to achieve different algorithmic complexities rather then inheritance.

liyafan82 · 2019-07-24T07:45:33Z

@emkornfield I have opened a separate issue (ARROW-6021) to track the problem of moving copySafe method to ValueVector, because

The changes are not minor, so placing them in a separate issue makes it easier to review the changes.
I have realized that the requirement is not only needed by this single feature. So I hope this problem can be resolved sooner.

Let's hold this PR until ARROW-6021 is resolved. Thank you so much.

liyafan82 · 2019-08-01T10:51:36Z

@emkornfield @pravindra
As the dependency (ARROW-6021) is resolved, I have revised this PR. Would you please take a look?

emkornfield · 2019-08-09T03:02:43Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

Lets make all of the fields private for now?

Sure. Sounds reasonable.

emkornfield · 2019-08-09T03:03:11Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

you might want to clarify whether this class is intended to be used more then one

Sure. Revised.

emkornfield · 2019-08-09T03:05:27Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

this is probably better named populateSortedDictionary.

Good suggestion. Revised accordingly.

emkornfield · 2019-08-09T03:06:07Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

at some point we might want to explore the possibility of making use of an off-heap datastructure if possible (but not for this PR).

Good suggestion. We will provide an ArrowBuf based binary search tree.

emkornfield · 2019-08-09T03:07:05Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

you might want a guard here to ensure this and getSortedDictionary aren't called until endBuild is called.

Good suggestion. We use a bool flag to mark the end of dictionary building. Please take a look.

emkornfield · 2019-08-09T03:07:29Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

have a code sample in the javadoc here might make it easier for users to understand the the intended call pattern.

Good suggestion. Sample code is added to the Javadoc.

emkornfield

Please see comments.

emkornfield · 2019-08-10T05:48:27Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

Can you clarify that this dictionary is NOT sorted.

Sure. Good suggestion.

emkornfield · 2019-08-10T05:48:53Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

please include details on retrieving/populating the dictionary.

Revised. Thanks.

emkornfield · 2019-08-10T05:50:13Z

A couple of documentation clarifications and then I think this can be merged.

emkornfield · 2019-08-10T21:14:32Z

Thinking about this a little bit more, why wouldn't we encode at the same time we are building the dictionary?

liyafan82 · 2019-08-11T08:51:26Z

Thinking about this a little bit more, why wouldn't we encode at the same time we are building the dictionary?

@emkornfield
Good question and good suggestion. This is called "online encoding", for which building dictionary and encoding are carried out simultaneously.

This is suitable for scenarios when the dictionary is used as is (for some cases, the dictionary may need to be sorted by content or frequency, before encoding).

So let's first support this basic scenario, and then support the online version in a separate issue.

emkornfield · 2019-08-13T05:20:15Z

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

is it important to keep the state of startBuild and endBuild. Is seems like we can set ValueCount whenever getDictionary is called if users want an incremental dictionary?

Sounds good. Revised accordingly. Thanks.

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

...rc/test/java/org/apache/arrow/algorithm/dictionary/TestSearchTreeBasedDictionaryBuilder.java

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java

emkornfield · 2019-08-16T02:53:30Z

+1, thank you.

The dictionary builder servers for the following scenario which is frequently encountered in practice when dictionary encoding is involved: the dictionary values are not known a priori, so they are determined dynamically, as new data arrive continually. In particular, when a new value arrives, it is tested to check if it is already in the dictionary. If so, it is simply neglected, otherwise, it is added to the dictionary. When all values have been evaluated, the dictionary can be considered complete. So encoding can start afterward. The code snippet using a dictionary builder should be like this: DictonaryBuilder<IntVector> dictionaryBuilder = ... dictionaryBuilder.startBuild(); ... dictionaryBuild.addValue(newValue); ... dictionaryBuilder.endBuild(); Closes apache#4813 from liyafan82/fly_0705_build and squashes the following commits: 2007b87 <liyafan82> Provide dictionary builder Authored-by: liyafan82 <fan_li_ya@foxmail.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>

fsaintjacques added the Component: Java label Jul 12, 2019

emkornfield reviewed Jul 19, 2019

View reviewed changes

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java Outdated Show resolved Hide resolved

emkornfield reviewed Jul 19, 2019

View reviewed changes

java/algorithm/src/main/java/org/apache/arrow/algorithm/dictionary/DictionaryBuilder.java Outdated Show resolved Hide resolved

emkornfield requested changes Jul 19, 2019

View reviewed changes

kszucs force-pushed the master branch 2 times, most recently from ed180da to 85fe336 Compare July 22, 2019 19:29

liyafan82 force-pushed the fly_0705_build branch from 64b803c to 6be773b Compare August 1, 2019 10:49

emkornfield reviewed Aug 9, 2019

View reviewed changes

emkornfield requested changes Aug 9, 2019

View reviewed changes

liyafan82 force-pushed the fly_0705_build branch from 6be773b to 215606c Compare August 9, 2019 03:41

emkornfield reviewed Aug 10, 2019

View reviewed changes

liyafan82 force-pushed the fly_0705_build branch from 215606c to d6038e4 Compare August 10, 2019 13:59

emkornfield reviewed Aug 13, 2019

View reviewed changes

liyafan82 force-pushed the fly_0705_build branch 2 times, most recently from d2f58a9 to 37878a4 Compare August 13, 2019 10:52

emkornfield reviewed Aug 14, 2019

View reviewed changes

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java Outdated Show resolved Hide resolved

emkornfield reviewed Aug 14, 2019

View reviewed changes

...rc/test/java/org/apache/arrow/algorithm/dictionary/TestSearchTreeBasedDictionaryBuilder.java Outdated Show resolved Hide resolved

liyafan82 force-pushed the fly_0705_build branch from 37878a4 to 84ea87f Compare August 14, 2019 07:34

emkornfield reviewed Aug 15, 2019

View reviewed changes

...hm/src/main/java/org/apache/arrow/algorithm/dictionary/SearchTreeBasedDictionaryBuilder.java Outdated Show resolved Hide resolved

[ARROW-5862][Java] Provide dictionary builder

2007b87

liyafan82 force-pushed the fly_0705_build branch from 84ea87f to 2007b87 Compare August 15, 2019 09:53

emkornfield closed this in 1f5ebd0 Aug 16, 2019

asfimport mentioned this pull request Aug 16, 2019

[Java] Provide dictionary builder #16801

Closed

ARROW-5862: [Java] Provide dictionary builder #4813

ARROW-5862: [Java] Provide dictionary builder #4813

Uh oh!

Conversation

liyafan82 commented Jul 5, 2019

Uh oh!

codecov-io commented Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

liyafan82 commented Jul 24, 2019

Uh oh!

liyafan82 commented Aug 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emkornfield commented Aug 10, 2019

Uh oh!

emkornfield commented Aug 10, 2019

Uh oh!

liyafan82 commented Aug 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emkornfield commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Jul 6, 2019 •

edited

Loading