Apache DataSketches plugin for Trino #6643

ShashwatArghode · 2021-01-19T11:15:23Z

Plugin to query Apache Datasketches (https://datasketches.apache.org/docs/Theta/ThetaSketchFramework.html). This includes merge and estimate function for theta sketch.

Praveen2112

Thanks for working on this. Can we have some tests for like validating the result when we hit it to Trino. And a docs describing the functions would also be helpful.

...ino-datasketches/src/main/java/io/trino/plugin/datasketches/state/SketchStateSerializer.java

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Estimate.java

Praveen2112 · 2021-01-19T11:36:42Z

.../trino-datasketches/src/main/java/io/trino/plugin/datasketches/state/SketchStateFactory.java

+ return GroupedSketchState.class;
+ }
+
+ public static class GroupedSketchState


Since they are more details to capture, can we move it to a new file ? WDYT ?

I think keeping the structure consistent with other state factories would make more sense. Ex. HyperLogLog, LongApproximateMostFrequent, NumericHistogram etc.

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Union.java

Praveen2112 · 2021-01-19T11:44:25Z

Please change the commit message to Trino

ShashwatArghode · 2021-06-04T03:35:19Z

Apologies for the delay in getting back to this. I have addressed all the comments and added the doc as well. Thanks for reviewing the PR, do let me know if it needs more changes.

plugin/trino-datasketches/pom.xml

ebyhr · 2021-08-12T06:18:16Z

docs/src/main/sphinx/connector/datasketches.rst

@@ -0,0 1,36 @@
+======================
+DataSketches Connector


I think this plugin is set of functions as Teradata functions and Machine learning functions. I would move to functions directory.

I have made the suggested changes to the doc. Please let me know if the there is any issue with sphinx syntax or anything needs to be changed

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/SketchUDFPlugin.java

docs/src/main/sphinx/connector/datasketches.rst

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Estimate.java

docs/src/main/sphinx/connector/datasketches.rst

plugin/trino-datasketches/pom.xml

ebyhr · 2021-08-12T06:39:46Z

...n/trino-datasketches/src/test/java/io/trino/plugin/datasketches/theta/MergeEstimateTest.java

+ union.update(sketch2);
+ Sketch unionResult = union.getResult();
+
+ assertEquals(unionResult.getEstimate(), estimate, 0);


Could you also add tests using Trino queries? You can refer to TestMLQueries or any other function tests.

Will look into this as soon as I get some time.

Looked into TestMLQueries and adding test cases using Trino queries, but, for that, we will either need to support the creation of datasketches in Presto or register Hive UDFs for test cases for inserting Sketch in the database in binary format. We can add this when we add support for the creation of Sketches in Presto.

As we talked in Slack, I assume it's testable without such supports. We can prepare Sketches data in Java library and then cast to varbinary format at this time.

BytesWritable from hadoop turned out to be the key to insert the sketches in binary format in the database. Added the test case.

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Union.java

docs/src/main/sphinx/connector.rst

ShashwatArghode · 2021-09-08T22:15:10Z

@ebyhr, Do let me know if any more changes are needed to this.

bitsondatadev · 2022-10-21T20:58:40Z

👋 @ShashwatArghode - this PR has become inactive. If you're still interested in working on it, please let us know, and we can try to get reviewers to help with that.

We're working on closing out old and inactive PRs, so if you're too busy or this has too many merge conflicts to be worth picking back up, we'll be making another pass to close it out in a few weeks.

ShashwatArghode · 2022-10-25T18:17:49Z

Hi @bitsondatadev,
All the old review comments have been addressed, we can try to get reviewers to move this forward. I will resolve the conflicts in the meantime.

findepi · 2022-10-26T11:06:05Z

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/state/SketchState.java

+public interface SketchState
+ extends AccumulatorState
+{
+ Slice getSketch();


#14290 added a Theta sketch aggregation function: IcebergThetaSketchForStats
I forgot about this PR (and it is still waiting for "syntax review"), but anyway that function is for Iceberg plugin's internal use. Also this happens to be sketch building aggregation function that's not being added here.

Anyway, i was able to use UpdateSketch directly in the state class.
For sketch union aggregation, the state could probably hold directly onto an org.apache.datasketches.theta.Union object

findepi · 2022-10-26T11:07:14Z

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/UnionWithParams.java

+ private UnionWithParams() {}
+
+ @InputFunction
+ public static void input(@AggregationState SketchState state, @SqlType(StandardTypes.VARBINARY) Slice inputValue, @SqlType(StandardTypes.INTEGER) Integer normEntries, @SqlType(StandardTypes.BIGINT) Long seed)


What does normEntries mean?

findepi · 2022-10-26T11:08:11Z

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/UnionWithParams.java

+ {
+ state.setNominalEntries(normEntries);
+ state.setSeed(seed);
+ state.setSketch(inputValue);


Can this overwrite the state/result after previous input function invocation?

findepi · 2022-10-26T11:09:11Z

docs/src/main/sphinx/functions/datasketches.rst

+DataSketches functions
+----------------------
+
+.. function:: theta_sketch_union(sketches) -> sketch


there are two variants of theta_sketch_union, one having parameters. The parameters need to be documented.

also, should the docs say this is an aggregation function?

-> sketch

there is no "sketch" type. should this be "varbinary"?

findepi · 2022-10-26T11:11:06Z

I was just skimming. Not reviewing until the syntax is reviewed (#6643 (comment))

vgankidi · 2022-12-05T21:02:59Z

Hi @findepi , could I ask what open concerns are there on the syntax? I couldn't find relevant context from the comment above.

vgankidi · 2022-12-05T21:03:52Z

We are interested in this as well and can provide any help required to take this forward.

bitsondatadev · 2022-12-27T19:03:38Z

Hey @vgankidi, syntax needs review needs a particular review generally from @martint, @electrum, or @dain.

This just means that there needs to be a review around the consensus on how things are being done. I believe this is something we should work on defining or making a little more transparent.

mosabua · 2024-01-11T00:18:09Z

👋 @ShashwatArghode - this PR is inactive and doesn't seem to be under development. If you'd like to continue work on this at any point in the future, feel free to re-open.

Praveen2112 reviewed Jan 19, 2021

View reviewed changes

...ino-datasketches/src/main/java/io/trino/plugin/datasketches/state/SketchStateSerializer.java Outdated Show resolved Hide resolved

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Estimate.java Outdated Show resolved Hide resolved

Praveen2112 reviewed Jan 19, 2021

View reviewed changes

plugin/trino-datasketches/src/main/java/io/trino/plugin/datasketches/theta/Union.java Show resolved Hide resolved

findepi added enhancement New feature or request syntax-needs-review labels Jan 19, 2021

findepi requested a review from martint January 19, 2021 13:02

tooptoop4 mentioned this pull request Jan 19, 2021

More https://trino.io/resources.html #3472

Closed

ShashwatArghode force-pushed the trino_datasketches_plugin branch from b1d0670 to 6fec5ed Compare June 2, 2021 21:27

cla-bot bot added the cla-signed label Jun 2, 2021

ShashwatArghode force-pushed the trino_datasketches_plugin branch from 6fec5ed to 52fc9b9 Compare June 2, 2021 21:28

ShashwatArghode changed the title ~~Apache DataSketches plugin for Presto~~ Apache DataSketches plugin for Trino Jun 2, 2021

ShashwatArghode force-pushed the trino_datasketches_plugin branch from 636a2f5 to f814770 Compare June 2, 2021 22:57

findepi force-pushed the master branch from 8538e49 to 1f896ea Compare July 30, 2021 22:13

ebyhr reviewed Aug 12, 2021

View reviewed changes

ShashwatArghode force-pushed the trino_datasketches_plugin branch from f814770 to 8a09d22 Compare August 26, 2021 06:18

ShashwatArghode commented Aug 26, 2021

View reviewed changes

docs/src/main/sphinx/connector.rst Outdated Show resolved Hide resolved

ShashwatArghode force-pushed the trino_datasketches_plugin branch from 127131b to a638a5e Compare September 1, 2021 02:05

Apache DataSketches plugin for Trino

b772682

ShashwatArghode force-pushed the trino_datasketches_plugin branch from a638a5e to b772682 Compare September 1, 2021 02:07

findepi reviewed Oct 26, 2022

View reviewed changes

kokosing force-pushed the master branch from 3f05134 to 58d6356 Compare March 14, 2023 11:34

mosabua closed this Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apache DataSketches plugin for Trino #6643

Apache DataSketches plugin for Trino #6643

ShashwatArghode commented Jan 19, 2021

Praveen2112 left a comment

Praveen2112 Jan 19, 2021

ShashwatArghode Jun 2, 2021

Praveen2112 commented Jan 19, 2021

ShashwatArghode commented Jun 4, 2021

ebyhr Aug 12, 2021

ShashwatArghode Aug 26, 2021

ebyhr Aug 12, 2021

ShashwatArghode Aug 26, 2021

ShashwatArghode Aug 31, 2021

ebyhr Aug 31, 2021 •

edited

Loading

ShashwatArghode Sep 1, 2021

ShashwatArghode commented Sep 8, 2021

bitsondatadev commented Oct 21, 2022

ShashwatArghode commented Oct 25, 2022

findepi Oct 26, 2022

findepi Oct 26, 2022

findepi Oct 26, 2022

findepi Oct 26, 2022

findepi Oct 26, 2022

findepi Oct 26, 2022

findepi commented Oct 26, 2022

vgankidi commented Dec 5, 2022 •

edited

Loading

vgankidi commented Dec 5, 2022

bitsondatadev commented Dec 27, 2022

mosabua commented Jan 11, 2024

Apache DataSketches plugin for Trino #6643

Apache DataSketches plugin for Trino #6643

Conversation

ShashwatArghode commented Jan 19, 2021

Praveen2112 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Praveen2112 commented Jan 19, 2021

ShashwatArghode commented Jun 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShashwatArghode commented Sep 8, 2021

bitsondatadev commented Oct 21, 2022

ShashwatArghode commented Oct 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

findepi commented Oct 26, 2022

vgankidi commented Dec 5, 2022 • edited Loading

vgankidi commented Dec 5, 2022

bitsondatadev commented Dec 27, 2022

mosabua commented Jan 11, 2024

ebyhr Aug 31, 2021 •

edited

Loading

vgankidi commented Dec 5, 2022 •

edited

Loading