test(native): Add `AbstractTestQueries` to native tests #23671

pramodsatya · 2024-09-18T01:36:02Z

Description

Adds support to run testcases from AbstractTestQueries with the C++ worker in presto-native-tests. These testcases are run with Presto's iterative optimizer disabled in TestNonIterativeDistributedQueries and with the default native hive query runner in TestTpchDistributedQueries.

Motivation and Context

Improves Presto C++ e2e test coverage with sidecar by extending production tests from presto-tests module to use the native worker.

Release Notes

== RELEASE NOTES ==

General Changes
* Add testcases from `AbstractTestQueries` to `presto-native-tests`.

steveburnett

LGTM! (docs)

Reviewed README.md, no concerns. Thanks for the doc!

tdcmeehan · 2024-09-19T13:17:52Z

Are these tests being run as part of the CI?

pramodsatya · 2024-09-19T15:49:09Z

They are not being run as part of CI currently, but we plan to setup a pipeline to run these tests on a regular cadence subsequently. Will also add a CI job to run these tests in a follow-up PR.

presto-native-tests/src/test/java/com.facebook.presto.nativetests/TestAggregations.java

steveburnett · 2024-11-11T14:57:57Z

Just a suggestions for the release note: Consider using the description of the PR in the release note entry to help a release note reader understand what this module does.

== RELEASE NOTES ==
General Changes
* Add presto-native-tests module to run end-to-end tests with Presto native workers. :pr:`23671`

pramodsatya · 2024-11-11T15:14:33Z

Just a suggestions for the release note: Consider using the description of the PR in the release note entry to help a release note reader understand what this module does.
== RELEASE NOTES ==
General Changes
* Add presto-native-tests module to run end-to-end tests with Presto native workers. :pr:`23671`

Thanks for the suggestion @steveburnett, updated accordingly.

pramodsatya · 2025-07-17T16:02:59Z

@aditi-pandit, could you please help review this PR?

aditi-pandit

@pramodsatya : Thanks for the iterations of this PR. Have the next round of comments.

presto-native-tests/pom.xml

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

aditi-pandit · 2025-07-17T19:04:30Z

presto-docs/src/main/sphinx/presto_cpp/limitations.rst

You could move the changes in limitations.rst and TestNonIterativeDistributedQueries and TestTpchDistributedQueries into a separate PR as that will be much easier to review and submit than this PR.

The changes in TestNonIterativeDistributedQueries and TestTpchDistributedQueries depend on AbstractTestQueriesNative, and the comment in limitations.rst explains the difference in approx_set results for which the test is added here. Is it fine to retain these changes here?

...-native-execution/src/test/java/com/facebook/presto/nativeworker/NativeQueryRunnerUtils.java

presto-native-tests/src/test/java/com/facebook/presto/nativetests/NativeTestsUtils.java

presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java

presto-main-base/src/main/java/com/facebook/presto/metadata/FunctionSignatureMatcher.java

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

steveburnett · 2025-07-29T14:47:09Z

Suggested changes for the release note.

== RELEASE NOTES ==

Prestissimo (Native Execution) Changes
* Add testcases from ``AbstractTestQueries`` to ``presto-native-tests``.

pramodsatya

Thanks for the feedback @aditi.pandit, addressed the comments. Could you PTAL?

pramodsatya · 2025-09-02T16:00:42Z

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

+    /// not needed and this test is disabled in Presto C++.
+    @Override
+    @Test(enabled = false)
+    public void testShowFunctions() {}


This test checks metadata for certain functions and would need to be modified for native functions, I figured since validation with Presto java is not necessary for output of SHOW FUNCTIONS this test could be disabled. I have now modified the test to validate native function signatures instead of disabling the test, would this be fine?

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

pramodsatya · 2025-09-23T21:38:05Z

presto-docs/src/main/sphinx/presto_cpp/limitations.rst

The changes in TestNonIterativeDistributedQueries and TestTpchDistributedQueries depend on AbstractTestQueriesNative, and the comment in limitations.rst explains the difference in approx_set results for which the test is added here. Is it fine to retain these changes here?

presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java

...-native-execution/src/test/java/com/facebook/presto/nativeworker/NativeQueryRunnerUtils.java

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

presto-native-tests/src/test/java/com/facebook/presto/nativetests/NativeTestsUtils.java

## Description P4HyperLogLog type was added to Velox recently in facebookincubator/velox@75ec5ba, and this type should be supported by `NativeTypeManager`. ## Motivation and Context Support P4HLL type in `NativeTypeManager`, uncovered by `native-tests` in #23671. ## Impact Support P4HLL in Presto C++ clusters with sidecar. ## Test Plan Added tests. ``` == NO RELEASE NOTE == ```

github-actions · 2025-10-31T22:40:38Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

## Description Fixes expected `orderdate` length in `testShowColumns` for `DWRF` file format. ## Motivation and Context Required by tests to be added in #23671 . ``` == NO RELEASE NOTE == ```

pramodsatya · 2025-11-21T01:02:34Z

@aditi-pandit, could you please help review this PR? I addressed the previous review comments and opened issues for failures identified.

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java

pramodsatya requested review from aditi-pandit, majetideepak and mknegi September 18, 2024 01:36

steveburnett previously approved these changes Sep 18, 2024

View reviewed changes

pramodsatya dismissed steveburnett’s stale review via acaa071 September 19, 2024 04:04

pramodsatya force-pushed the presto-native-tests branch 2 times, most recently from acaa071 to 8ec8208 Compare September 19, 2024 04:56

pramodsatya marked this pull request as ready for review September 19, 2024 04:57

pramodsatya requested review from a team, ClarenceThreepwood, elharo, feilong-liu and jaystarshot as code owners September 19, 2024 04:57

pramodsatya requested a review from presto-oss September 19, 2024 04:57

mknegi previously approved these changes Sep 19, 2024

View reviewed changes

jaystarshot reviewed Sep 19, 2024

View reviewed changes

presto-native-tests/src/test/java/com.facebook.presto.nativetests/TestAggregations.java Outdated Show resolved Hide resolved

pramodsatya dismissed mknegi’s stale review via 32b2ad6 October 1, 2024 03:48

pramodsatya force-pushed the presto-native-tests branch 2 times, most recently from 32b2ad6 to f614cb6 Compare October 7, 2024 21:16

pdabre12 mentioned this pull request Nov 1, 2024

Add a native function namespace manager #23358

Merged

6 tasks

pramodsatya force-pushed the presto-native-tests branch 3 times, most recently from 11d653c to 47ecc65 Compare November 11, 2024 07:49

tdcmeehan self-assigned this Nov 11, 2024

zuyu mentioned this pull request Nov 15, 2024

ClassCastException in testArrayCumSum: Double cannot be cast to Float #23754

Closed

pramodsatya mentioned this pull request Jul 3, 2025

Refactor AbstractTestQueries testcases for native-tests #25486

Merged

pramodsatya force-pushed the presto-native-tests branch 3 times, most recently from b86a828 to ad26257 Compare July 15, 2025 16:26

pramodsatya force-pushed the presto-native-tests branch from ad26257 to 1e5f3cd Compare July 16, 2025 02:11

pramodsatya mentioned this pull request Jul 16, 2025

feat: Add nonDecimalNumericTypeOnly constraint to type parameters facebookincubator/velox#14049

Open

pramodsatya marked this pull request as ready for review July 16, 2025 03:22

pramodsatya requested review from czentgr and unidevel as code owners July 16, 2025 03:22

pramodsatya force-pushed the presto-native-tests branch 2 times, most recently from 02c08ea to ad69521 Compare July 17, 2025 16:02

pramodsatya requested a review from aditi-pandit July 17, 2025 16:03

aditi-pandit reviewed Jul 17, 2025

View reviewed changes

pramodsatya force-pushed the presto-native-tests branch from ad69521 to eeda180 Compare September 23, 2025 22:00

pramodsatya commented Sep 23, 2025

View reviewed changes

pramodsatya mentioned this pull request Oct 27, 2025

fix: Add P4HyperLogLog to NativeTypeManager #26444

Merged

pramodsatya force-pushed the presto-native-tests branch from eeda180 to 94746ea Compare October 31, 2025 21:56

pramodsatya changed the title ~~[native] Add AbstractTestQueries to native tests~~ test(native): Add AbstractTestQueries to native tests Oct 31, 2025

pramodsatya mentioned this pull request Oct 31, 2025

fix(testing): Fix orderdate length in testShowColumns #26504

Merged

pramodsatya added 2 commits November 18, 2025 22:24

refactor(testing): Refactor testcases for native-tests

5834550

test(native): Add AbstractTestQueries to native-tests

e8b547d

pramodsatya force-pushed the presto-native-tests branch from 94746ea to e8b547d Compare November 21, 2025 01:00

github-code-quality bot found potential problems Nov 21, 2025

View reviewed changes

...to-native-tests/src/test/java/com/facebook/presto/nativetests/AbstractTestQueriesNative.java Dismissed Show dismissed Hide dismissed

test(native): Add AbstractTestQueries to native tests #23671

Are you sure you want to change the base?

test(native): Add AbstractTestQueries to native tests #23671

Uh oh!

Conversation

pramodsatya commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Release Notes

Uh oh!

steveburnett left a comment

Choose a reason for hiding this comment

Uh oh!

tdcmeehan commented Sep 19, 2024

Uh oh!

pramodsatya commented Sep 19, 2024

Uh oh!

Uh oh!

steveburnett commented Nov 11, 2024

Uh oh!

pramodsatya commented Nov 11, 2024

Uh oh!

pramodsatya commented Jul 17, 2025

Uh oh!

aditi-pandit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aditi-pandit Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

pramodsatya Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steveburnett commented Jul 29, 2025

Uh oh!

pramodsatya left a comment

Choose a reason for hiding this comment

Uh oh!

pramodsatya Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pramodsatya Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

pramodsatya commented Nov 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

test(native): Add `AbstractTestQueries` to native tests #23671

test(native): Add `AbstractTestQueries` to native tests #23671

pramodsatya commented Sep 18, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2025 •

edited

Loading