[SPARK-43986][SQL] Create error classes for HyperLogLog function call failures

[dtenedor]

What changes were proposed in this pull request?

This PR creates error classes for HyperLogLog function call failures.

Why are the changes needed?

These replace previous Java exceptions or other cases, in order to improve the user experience and bring consistency with other parts of Spark.

Does this PR introduce any user-facing change?

Yes, error messages change slightly.

How was this patch tested?

This PR also adds SQL query test files for the HLL functions.

[dtenedor]

Hi @MaxGekk @RyanBerti can you please help review this PR to improve error messages and add more test coverage? 🙏

[mkaravel]

@dtenedor Thank you for this PR. Inline comments are mostly about naming and phrasing.

My high level comment is that we have one more category of errors and this is related to passing BINARY values to hll_union, hll_union_agg, and hll_sketch_estimate that are not valid representations of sketches. At this point we rely on the DataSketches library to throw an exception for these cases, and we let that exception surface. My take on this is that we should add one more error class for these (along the lines of an illegal argument exception/error). What do you think?

[dtenedor]

Hi @MaxGekk @mkaravel I have responded to your comments, please take another look :)

[mkaravel]

@dtenedor Thank you for the fixes.
Posting a couple of minor comments.

[mkaravel]

@dtenedor Thank you for this PR. Inline comments are mostly about naming and phrasing.

My high level comment is that we have one more category of errors and this is related to passing BINARY values to hll_union, hll_union_agg, and hll_sketch_estimate that are not valid representations of sketches. At this point we rely on the DataSketches library to throw an exception for these cases, and we let that exception surface. My take on this is that we should add one more error class for these (along the lines of an illegal argument exception/error). What do you think?

@dtenedor What about the above?

[dtenedor]

@dtenedor Thank you for this PR. Inline comments are mostly about naming and phrasing.
My high level comment is that we have one more category of errors and this is related to passing BINARY values to hll_union, hll_union_agg, and hll_sketch_estimate that are not valid representations of sketches. At this point we rely on the DataSketches library to throw an exception for these cases, and we let that exception surface. My take on this is that we should add one more error class for these (along the lines of an illegal argument exception/error). What do you think?

Sorry for missing this before, I made this suggested update. It was a good idea, I think the errors are clearer now.

[dtenedor]

Thanks @mkaravel for the review! I implemented your suggestions, please take another look.

[mkaravel]

LGTM.
@dtenedor Thank you for putting the time to work on this and for addressing my comments!

I have a few nits regarding the function names we pass to some error classes (case needs to be lowercase and basically, the function names we pass need to match the pretty name of the function).

[dtenedor]

Hi @MaxGekk can we trouble you to help us merge this please when you have a moment :)

[dtenedor]

Note the only CI failure is spurious and unrelated to this PR:

[dtenedor]

Hi @MaxGekk, thanks for your careful review. I responded to your comments, please look again :)

[MaxGekk]

I think the failure below is not related to PR's changes:

systemctl restart cri-docker: Process exited with status 1
stdout:


stderr:
Job for cri-docker.service failed because the control process exited with error code.
See "systemctl status cri-docker.service" and "journalctl -xe" for details.

* 
╭─────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                             │
│    * If the above advice does not help, please let us know:                                 │
│      https://github.com/kubernetes/minikube/issues/new/choose                               │
│                                                                                             │
│    * Please run `minikube logs --file=logs.txt` and attach logs.txt to the GitHub issue.    │
│                                                                                             │
╰─────────────────────────────────────────────────────────────────────────────────────────────╯
Error: Process completed with exit code 90.

[MaxGekk]

+1, LGTM. Merging to master.
Thank you, @dtenedor and @mkaravel for review.

[dtenedor]

Thanks @MaxGekk for your reviews and merging!!