feat: Upgrade velox submodule to y-scope/velox@7ae5119.

[anlowee]

Description

This PR is to update the Velox submodule to sync the merged PR y-scope/velox#29 to have the refactored Clp*Cursor in Velox codebase.

Checklist

• The PR satisfies the contribution guidelines.
• This is a breaking change and that has been indicated in the PR title, OR this isn't a
  breaking change.
• Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Passed the CI.

Summary by CodeRabbit

• Tests

  • Added two new query tests to expand coverage for timestamp-based filtering and message-text matching, ensuring consistent results under additional filter conditions; also applied a minor formatting normalization in test assertions.

• Chores

  • Updated the Velox submodule reference to the latest commit to stay aligned with upstream changes.

[coderabbitai]

Walkthrough

Updates test-only code: changes the datetime function name casing in an existing SELECT and adds two assertQuery cases to TestPrestoNativeClpGeneralQueries that filter by a FROM_UNIXTIME threshold and by msg LIKE '%user%', each ordering by id and limiting to one row. Also advances the Velox submodule pointer. No production API changes.

Changes

Cohort / File(s)	Summary of Changes
Test additions & formatting presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java	Adjusted function name casing to FORMAT_DATETIME(...) in a SELECT and added two new assertQuery invocations that select msg, formatted timestamp, id, attr, tags from test_e2e with t.dollar_sign_date > FROM_UNIXTIME(1679441694.576) (one with additional msg LIKE '%user%'), using ORDER BY id LIMIT 1 and asserting a specific single-row result.
Submodule update: Velox pointer presto-native-execution/velox	Advanced submodule reference from ab8f446... to 7ae5119...; metadata-only change (no source code edits).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• kirkrodrigues

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check	⚠️ Warning	The description does not follow the required template and is missing critical sections such as Motivation and Context, Impact, Test Plan, and Release Notes, instead using custom headings and an incomplete placeholder in the Description section.	Please update the pull request description to match the repository template by adding detailed Motivation and Context, Impact analysis, a clear Test Plan, and either the Release Notes or the "== NO RELEASE NOTE ==" declaration.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title is concise, uses the Conventional Commits format in imperative form, and clearly highlights the main change of upgrading the Velox submodule reference, which is the primary purpose of the pull request.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc04bd and e3188c2.

📒 Files selected for processing (2)

• presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java (1 hunks)
• presto-native-execution/velox (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: prestocpp-linux-build-for-test
• GitHub Check: maven-checks (17.0.13)
• GitHub Check: maven-checks (8.0.442)

🔇 Additional comments (2)

presto-native-execution/velox (1)

1-1: Submodule bump verified: correct remote, commit and CLP-related changes
presto-native-execution/velox is pinned to https://github.com/y-scope/velox.git@7ae51198424559a7ed6d245ce4c4214ef7047d9d on branch presto-0.293-clp-connector; CLP/IR commits (#19, #22, #25, #29) are present.

presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java (1)

129-173: LGTM on added coverage.

Nice end-to-end assertions that exercise timestamp and message filters against the CLP dataset, and good use of String.format with escaped percent signs.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (3)

presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java (3)

129-150: Stabilize timestamp filter; avoid DOUBLE drift; add deterministic tie-breaker.

Using FROM_UNIXTIME(double) is time-zone and rounding sensitive, which risks flakiness. Prefer a TIMESTAMP literal and add a secondary ORDER BY key.

Apply:

-                        " WHERE t.dollar_sign_date > FROM_UNIXTIME(1679441694.576)" +
-                        " ORDER BY id" +
+                        " WHERE t.dollar_sign_date > TIMESTAMP '2023-03-22 12:34:54.576'" +
+                        " ORDER BY id, t.dollar_sign_date" +

Optionally verify there aren’t other DOUBLE-based timestamp filters and whether tests pin a session time zone:

#!/bin/bash
rg -n --type=java 'from_unixtime\(' presto-native-execution/src/test/java || true
rg -n -C2 --type=java '(TimeZoneKey|getTimeZoneKey|setTimeZone\(|System\.setProperty\("user\.timezone")' presto-native-execution/src/test/java || true

---

151-173: Mirror the timestamp stabilisation and sorting fix here.

Same flakiness risk; apply the same predicate and ordering treatment.

Apply:

-                        " WHERE t.dollar_sign_date > FROM_UNIXTIME(1679441694.576)" +
+                        " WHERE t.dollar_sign_date > TIMESTAMP '2023-03-22 12:34:54.576'" +
                         " AND msg like '%%user%%'" +
-                        " ORDER BY id" +
+                        " ORDER BY id, t.dollar_sign_date" +

---

160-160: Use ILIKE if case-insensitive match is intended.

More resilient to message casing changes.

Apply:

-                        " AND msg LIKE '%%user%%'" +
+                        " AND msg ILIKE '%%user%%'" +

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3188c2 and 0451d13.

📒 Files selected for processing (1)

• presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

• GitHub Check: test (17.0.13, :presto-main-base)
• GitHub Check: test (17.0.13, :presto-tests -P ci-only-resource-manager)
• GitHub Check: test (17.0.13, :presto-tests -P ci-only-distributed-non-hash-gen)
• GitHub Check: test (8.0.442, :presto-tests -P ci-only-plan-determinism)
• GitHub Check: test (8.0.442, :presto-tests -P presto-tests-general)
• GitHub Check: test (17.0.13, :presto-tests -P presto-tests-execution-memory)
• GitHub Check: test (8.0.442, :presto-main-base)
• GitHub Check: test (8.0.442, :presto-tests -P ci-only-distributed-non-hash-gen)
• GitHub Check: test (8.0.442, :presto-tests -P presto-tests-execution-memory)
• GitHub Check: test (8.0.442, :presto-tests -P ci-only-tpch-distributed-queries)
• GitHub Check: prestocpp-linux-build-for-test
• GitHub Check: maven-checks (17.0.13)
• GitHub Check: maven-checks (8.0.442)
• GitHub Check: prestissimo-worker-images-build