Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add REST remote function #5

Closed
wants to merge 21 commits into from
Closed

Add REST remote function #5

wants to merge 21 commits into from

Conversation

Joe-Abraham
Copy link
Owner

No description provided.

duanmeng and others added 20 commits September 26, 2024 10:22
Summary:
Create a `QueryDataWriter` in `exec::TableWriter` if the query trace is enabled,
recording the input vectors batch by batch. Each operator writes its data to the
directory `$rootDir/$pipelineId/$driverId/data`. The recorded data will be used
to replay the execution of `exec::TableWriter`, which will be supported in the
follow-up.

Design notes: https://docs.google.com/document/d/1crIIeVz4tWKYQnBoHoxrv2i-4zAML9HSYLps8h5SDrc/edit#heading=h.y6j2ojtr7hm9

Part of facebookincubator#9668

Pull Request resolved: facebookincubator#10910

Reviewed By: pedroerp

Differential Revision: D63444416

Pulled By: xiaoxmeng

fbshipit-source-id: ddd74ff6dd56de7bce31ec536035b32211453364
…#11101)

Summary: Pull Request resolved: facebookincubator#11101

Reviewed By: Yuhta

Differential Revision: D63442387

fbshipit-source-id: 9166e485ec1605913c3706256a9a9f532d914a45
…hrown from one eval path (facebookincubator#11096)

Summary:
Pull Request resolved: facebookincubator#11096

Velox throws VeloxRuntimeError at a few places intentionally to fail the query
instead of generating wrong results, e.g., when timestamp is outside the supported
range. These VeloxRuntimeErrors caused failures in expression fuzzer, hence an
attempt was made to tolerate such runtime errors in fuzzer so that fuzzer can still
test the relevant functions (facebookincubator#10993).
However, it turns out that that attempt was not enough. The reason is that
VeloxRuntimeError doesn't work well with the existing mechanism of masking error
by default nulls. So the expression fuzzer still occasionally fails because of only one
of the common and simple evaluation path throwing the error. (If we later want to
make those VeloxRuntimeError not suppressable by TRY but suppressable by the
mask-error-by-default-null mechanism, we'll need to  extend the EvalErrors struct
with an additional buffer to track whether the error at an index can be suppressed
by Try and check that in TryExpr.)

This diff relaxes the fuzzer check to make fuzzer not fail when only one of the
common and simple eval path throw the UNSUPPORTED_INPUT_UNCATCHABLE
runtime error. This should unblock facebookincubator#10713.

Reviewed By: kevinwilfong

Differential Revision: D63421914

fbshipit-source-id: 54d01f7a5e7ea887018659d0eec8070fd0922b50
…cubator#11098)

Summary:
Pull Request resolved: facebookincubator#11098

Splitting the .cpp into two compilation unitst (one for min one for
max) so that compilation can be parallelized. In my development environment,
recompilations of this unit went from 1m50s to about 40s.

Reviewed By: xiaoxmeng

Differential Revision: D63433609

fbshipit-source-id: 5402a280dfa3f5e65ffa10c49102579820e1eb4c
…m comparison functions (facebookincubator#11019)

Summary:
Pull Request resolved: facebookincubator#11019

facebookincubator#11015 added support for custom types to provide
custom comparison functions, motivated by the need to support TimestampWithTimezone's
comparison semantics.

I validated the UDFs provided in Comparisons.h on top of this change.

It does not look like any further changes are needed in those files, but I added explicit tests for
TimestampWithTimezone in the various comparison UDFs.  I also added tests for the UDFs that
support Generic arguments, to ensure that when the Type underlying the Generic provides custom
comparison functions, that is respected.

I considered adding checks to the UDFs that support templated arguments to ensure the type does
not provide custom comparison functions, as these are not supported in any of them (e.g. between,
<, >, and the SIMD versions of comparison functions). However, someone would have to explicitly
register these functions for the custom type, and I don't think it's unreasonable to expect the user to
test these functions work as expected when doing so, I don't think the per-batch check is worth the
(admittedly small) overhead.

Reviewed By: xiaoxmeng, kgpai

Differential Revision: D62893143

fbshipit-source-id: b9c9e2877b3c3a951b3fca9f371feac1e8d14d8e
Summary:
The early merged pr (facebookincubator@38f9a1f) was reverted by facebookincubator#10965 due to this issue: facebookincubator#10963.

In the original impl., a patch is expected to be applied to add `-fPIC` before building lib
stemmer, but when building docker image (ghcr.io/facebookincubator/velox-dev:centos9),
that patch file is not available to use due to the copied setup script outside velox repo is
executed. See code:
https://github.com/facebookincubator/velox/blob/7f2d7adaa1544c72129fd1c8d0766755ff354455/scripts/centos.dockerfile#L22
This pr proposes the installation of lib stemmer with the above issue fixed.

Pull Request resolved: facebookincubator#10984

Reviewed By: DanielHunte

Differential Revision: D63344087

Pulled By: Yuhta

fbshipit-source-id: a1467f5392ef828efdd219d0b7f1dfe0003d00ed
Summary:
The columns in `WindowPartition::data_` are reordered to start with partition-by keys and order-by keys, whereas the columns in `WindowPartition::columns_` are in the same order as the Window operator. The variable `orderByColumn` contains the index of order-by key in the reordered columns, so this should be accessed from `data_` and not `columns_`. A unit test is added to ensure an exception is thrown when NULL values in order-by column and frame column do not match.

Pull Request resolved: facebookincubator#11075

Reviewed By: amitkdutta

Differential Revision: D63404094

Pulled By: kagamiori

fbshipit-source-id: cae3713c3c6c945001af8c2930ba08a34d574f67
Summary:
Requested here to split off from the PR.
facebookincubator#10816 (comment)

Pull Request resolved: facebookincubator#11105

Reviewed By: Yuhta, amitkdutta

Differential Revision: D63475266

Pulled By: kagamiori

fbshipit-source-id: 11c7830b1cd4b665bc7915fd59b52673a7d39537
…11032)

Summary:
Pull Request resolved: facebookincubator#11032

Building on facebookincubator#11021 this adds support for custom
comparison functions provided by custom types in Presto's IN function.

I was able to reuse the ComplexTypeInPredicate and the support for custom comparisons already
present in BaseVector.

This diff is largely just renaming ComplexTypeInPredicate to VectorSetInPredicate (to clarify it's not
just for complex types anymore) and if statement to identify the case where
providesCustomComparison() is true for the element type (and of course updating the tests).

Making TimestampWithTimeZone a special case of bigint (comparing the millis) in the future might
give a performance boost if this shows up as a bottleneck.

Reviewed By: xiaoxmeng

Differential Revision: D62994557

fbshipit-source-id: 82d3eb2c3d24118f7f555b3f739810c91c58fc32
…11084)

Summary: Pull Request resolved: facebookincubator#11084

Reviewed By: xiaoxmeng

Differential Revision: D63472474

Pulled By: tanjialiang

fbshipit-source-id: 47041957eb41069f1435fa035a4f63bd1134c62c
Summary:
Pull Request resolved: facebookincubator#11102

Track running tasks for velox runtime for runtime task stats collection.
Followup can add to periodic print out or report task stats from velox
runtime stats reporter per query system needs.

Reviewed By: tanjialiang, oerling

Differential Revision: D63444008

fbshipit-source-id: 4e533470e417951cd86456c439101160ab1a05ca
…#11018)" (facebookincubator#11079)

Summary:
This reverts commit 6d1fbf0.

I validated locally that reverting this commit fixes the velox_functions_remote_client_test failure.
Resolves facebookincubator#11071

Pull Request resolved: facebookincubator#11079

Reviewed By: kgpai

Differential Revision: D63323399

Pulled By: pedroerp

fbshipit-source-id: a239066f542e3b4867add87a5418c81b7fd3020f
Summary:
Pull Request resolved: facebookincubator#11108

Addressing a comment made on facebookincubator#11021 after it was landed

Reviewed By: xiaoxmeng

Differential Revision: D63478307

fbshipit-source-id: 6c1ea5c402455b8fe7b4f859381d1448fef6ea8d
Summary:
Pull Request resolved: facebookincubator#11110

Addressing some comments made on
facebookincubator#11021 after it was landed

Reviewed By: xiaoxmeng

Differential Revision: D63478579

fbshipit-source-id: 5e36faf945d1fac576e3c89b774a26c20dc64160
…r#10948)

Summary:
Enables experimental fuzzer test with decimal type enabled. We can turn it on
for the nightly and CI jobs once we get a few green runs.

Pull Request resolved: facebookincubator#10948

Reviewed By: xiaoxmeng

Differential Revision: D63543385

Pulled By: kagamiori

fbshipit-source-id: cf4d1d91d3cb5c8001fb4ce04a4d0c0078926b98
…cebookincubator#11117)

Summary:
Pull Request resolved: facebookincubator#11117

Previously the running task counter is decremented as part of member destruction.
Recent changes to track the running task in a global list for stats tracking but
we remove it from the list at the entry of the task destructor so there is race
condition between the task destruction thread running at the background and the
test shutdown thread. The fix is to move the task list remove at the end of task
destructor. Also note that we manually destroy class members in task destructor
so it is fine to remove the task list in destructor.

Reviewed By: spershin

Differential Revision: D63546925

fbshipit-source-id: 1fe0c8f087a06138035b4d4ef16848faf87d652d
Summary:
Pull Request resolved: facebookincubator#11097

Whenever there is lazy loading, TableScan would come out with zero
input and output bytes, and they would be attributed to the operator that in
fact loaded the lazy vector. Using the existing mechanism to save it locally
and periodically apply to the correct source operator.

Reviewed By: xiaoxmeng

Differential Revision: D63414708

fbshipit-source-id: d1bbc64fc7ec1ed952593aaeece0b02daed60038
Summary: Pull Request resolved: facebookincubator#11121

Reviewed By: xiaoxmeng

Differential Revision: D63568508

Pulled By: tanjialiang

fbshipit-source-id: fd6f1637aa8f5bc02fd4ee7e9728df01b1fc0b15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.