#4317 Feature/variable length bytes offline dictionary #4318

buchireddy · 2019-06-14T20:35:51Z

PR for the feature mentioned in #4317

…onfig. (#1)

…e#4041) (#4) * Use hex string as the representation of byte array for queries * Adding test for indexing hexstring * Address comments

* Support Bytes type in sorted column * Fixing type casting issue for BYTES type values during realtime segment persistence (apache#3992) * Adding BYTES support for sorted column from query side with predicate.

…ilet plate code

Reducing the number of objects created during sorting and removing bo…

* Fixing byte[] as groupby value * Default PinotAdmin Commands to not use System.exit(...). (apache#4110) * Default PinotAdmin Commands to not use System.exit(...). * Adding comments for pinot-admin cmd

#11) * Adding map reader writer for batch and realtime. Supports map<int,int> * Fixing license header

* Adding pinot query to broker request converter * Addressing review comments and fixing bug in handling filter expression * Adding license header

* Adding AST parser to PinotQuery insider BrokerRequest * Fixing Predicate implementation

…lKind (#20)

… is not set by config (#10) * Adding config to use hostname over ip as part of pinot default instancd id * Exit 1 when caught exception in Pinot Admin command. (apache#4065) * Exit 1 when caught exception in Pinot Admin command. * Only exit(1) for failure requets * Infer hostname for controller in possible

* Adding validation mode to compare broker request and pinot query * Fixing the logic to assign ids to sub filter in the current code * Logging PQL when conversion fails * Adding SelectionSort and make standardizeExpression to take one more paramter to single quote non-string literal inside functions * ignore test of having queries * Minor clean. Warning on mismatch instead of throwing exception * Adding license header * Addressing review comments

* Adding support for udfs in Selection * Addressing review comments * Adding license headers * Adding support for bytes type in order by

…dex.

variable length byte dictionary. In the next iteration, we'll be deciding dynamically whether to use fixed or variable length impl.

…rfTest.java

Also fixed other review comments from Kishore.

for a column configurable through TableConfig.

- IndexLoadingConfig - RealtimeSegmentConfig

causing a race condition and unit test failure.

snleee · 2019-06-14T20:55:18Z

@buchireddy Can you rebase the code from apache/incubator-pinot's master branch? Also, it seems that your pr includes some other commits from winedepot branch. If would be great if you can cherry pick the commits so that this pr only includes the code changes related to variable length byte & string support for dictionary.

buchireddy · 2019-06-14T22:25:18Z

@snleee Closing this PR since I don't want to merge all other changes from winedepot fork. I've open #4321 with the intended changes only.

mcvsubbu · 2019-06-14T22:33:57Z

@buchireddy did you consider the use of MutableOffheapByteArrayStore? I am not able to readily say whether your implementation is based on that or is something parallel.

buchireddy · 2019-06-17T06:09:14Z

@mcvsubbu I did look at MutableOffheapByteArrayStore and took some concepts from it while implementing VarLengthBytesValueReaderWriter too. However, I haven't used the former fully because of the following reasons:

In order to support a seamless transition from fixed length to variable length offline dictionary, we need a special magic header to be added at the beginning, which needs some changes to MutableOffheapByteArrayStore.
MutableOffheapByteArrayStore is handling the buffer expansions, multiple buffers logic because it needs to be mutable but the immutable one doesn't need all those.

So, it's a trade-off b/w simple code vs avoiding the code duplication (though we anyways need a new class for variable length dictionary class implementation). I'm open for debate and for further brainstorming.

P.S: This is my first time going through Pinot code so please let me know if I'm missing any aspects here.

mcvsubbu · 2019-06-17T13:54:25Z

Mutable byte array store can be modified to have a header, no problem. It is currently used as a volatile store for comsuming segments, so changing it to include a header is not backward incompatible.

Why not modify it to have the header and also derive an unmutable class from it to be used during segment generation?

kishoreg · 2019-06-17T15:26:48Z

Let’s move the discussion to the issue #4317.

Seunghyun Lee and others added 30 commits February 14, 2019 21:50

[maven-release-plugin] prepare release release-0.1.0-rc0

bbf29dc

[maven-release-plugin] prepare for next development iteration

e21e49d

Adding simple avro msg decoder which imports avro schema from table c…

0312e53

…onfig. (#1)

Adding script to build and publish docker image (#2)

98fe9ba

Fixing license header

d6c6fbe

Fixing license header for readme

68c819f

Exit 1 when caught exceptions in Pinot Admin command.

2995f9c

Use hex string as the representation of byte array for queries (apach…

80c633b

…e#4041) (#4) * Use hex string as the representation of byte array for queries * Adding test for indexing hexstring * Address comments

Enable travis on develop branch (#5)

8b1c145

Support sorted column for bytes (#8)

d08d26d

* Support Bytes type in sorted column * Fixing type casting issue for BYTES type values during realtime segment persistence (apache#3992) * Adding BYTES support for sorted column from query side with predicate.

Reducing the number of objects created during sorting and removing bo…

67c9277

…ilet plate code

Merge pull request #9 from winedepot/optimize-sorting-phase-rt

a46b692

Reducing the number of objects created during sorting and removing bo…

Fixing byte[] as groupby value (#12)

df53919

* Fixing byte[] as groupby value * Default PinotAdmin Commands to not use System.exit(...). (apache#4110) * Default PinotAdmin Commands to not use System.exit(...). * Adding comments for pinot-admin cmd

Adding map reader writer for batch and realtime. Supports map<int,int… (

52b784e

#11) * Adding map reader writer for batch and realtime. Supports map<int,int> * Fixing license header

Adding query.thrift - enhancing broker request (#15)

9751e3c

Adding pinot query to broker request converter (#16)

45e8967

* Adding pinot query to broker request converter * Addressing review comments and fixing bug in handling filter expression * Adding license header

Adding AST parser to PinotQuery insider BrokerRequest (#17)

eb2202a

* Adding AST parser to PinotQuery insider BrokerRequest * Fixing Predicate implementation

Make filter expression in PinotQuery use calcite function names in Sq…

028848f

…lKind (#20)

Adding more filter operators into FilterKind (#21)

f5498d8

Adding support for udfs in Selection (#22)

261638e

* Adding support for udfs in Selection * Addressing review comments * Adding license headers * Adding support for bytes type in order by

Adding a variable length bytes value reader for offline dictionary in…

4181951

…dex.

Make variables private.

a17f005

Minor fixes for selection-udf. Adding test cases (#25)

30fe389

Cleaned up the changes a bit and removed the hacks to use the

e5c4432

variable length byte dictionary. In the next iteration, we'll be deciding dynamically whether to use fixed or variable length impl.

Minor cleanup.

b35fce6

Adding stats about the string values length in the StringDictionaryPe…

2cef0ad

…rfTest.java

Improved the magic header for VariableLength dictionary.

7e5f422

Also fixed other review comments from Kishore.

Making the feature of whether to use variable length dictionary

9c3d549

for a column configurable through TableConfig.

buchireddy and others added 7 commits June 5, 2019 18:03

Use the variable length dictionary config in realtime segments path too.

95e2e08

Fixed a bug that was caught by the unit tests.

b5278b6

Adding variable length dictionary config in more places like:

a9daafb

- IndexLoadingConfig - RealtimeSegmentConfig

Use separate file names for different tests in a single class. This was

f0468df

causing a race condition and unit test failure.

Dont set null value for varLengthDictionaryColumns

9ca37bb

Fixed review comments.

48d5a04

Merge branch 'develop' into feature/var_length_bytes

50efdf4

buchireddy closed this Jun 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

#4317 Feature/variable length bytes offline dictionary #4318

#4317 Feature/variable length bytes offline dictionary #4318

Uh oh!

buchireddy commented Jun 14, 2019

Uh oh!

snleee commented Jun 14, 2019 •

edited

Loading

Uh oh!

buchireddy commented Jun 14, 2019

Uh oh!

mcvsubbu commented Jun 14, 2019

Uh oh!

buchireddy commented Jun 17, 2019

Uh oh!

mcvsubbu commented Jun 17, 2019

Uh oh!

kishoreg commented Jun 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

#4317 Feature/variable length bytes offline dictionary #4318

#4317 Feature/variable length bytes offline dictionary #4318

Uh oh!

Conversation

buchireddy commented Jun 14, 2019

Uh oh!

snleee commented Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buchireddy commented Jun 14, 2019

Uh oh!

mcvsubbu commented Jun 14, 2019

Uh oh!

buchireddy commented Jun 17, 2019

Uh oh!

mcvsubbu commented Jun 17, 2019

Uh oh!

kishoreg commented Jun 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

snleee commented Jun 14, 2019 •

edited

Loading