Convert DateTime to long along the critical path within coordinator by shangm2 · Pull Request #24673 · prestodb/presto

shangm2 · 2025-03-04T22:54:19Z

Description

Similar to Change from DataSize to long to remove unnecessary data creation #24582 , we would like to continue replacing heavy data type with primitive data type.

Motivation and Context

We would like to make coordinator leaner to save cpu time on objection creation and gc.

Impact

Test Plan

passed java verifier test: https://www.internalfb.com/intern/presto/verifier/results/?test_id=213345
passed cpp worker verifier: https://www.internalfb.com/intern/presto/verifier/results/?test_id=213454
Will make sure UI does not break

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

== RELEASE NOTES ==

General Changes
* Improve scheduling by using long instead of DateTime for critical path.

facebook-github-bot · 2025-03-04T23:00:25Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-04T23:15:11Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-04T23:28:25Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-05T00:30:23Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-05T01:17:03Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-05T03:29:02Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-05T03:33:03Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-05T17:30:22Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

presto-native-execution/presto_cpp/main/PrestoTask.cpp

facebook-github-bot · 2025-03-11T01:28:55Z

@shangm2 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

shangm2 · 2025-03-11T02:36:56Z

@ZacBlanco Feel free to take another look. Thanks a lot. I have another pr which needs to be rebased after this one gets merged.

ZacBlanco

Thanks @shangm2 for these changes. LGTM

presto-ui/src/components/QueryOverview.jsx

yhwang

LGTM for the UI

NikhilCollooru

LGTM

amitkdutta

Looks good. Thanks @shangm2

…tor (prestodb#24673) * Change DateTime for APIs * Change DateTime for tests * Change DateTime for cpp workers * Change DateTime for presto-spark-base * Change DateTime for presto-ui * Change DateTime for presto-main

shangm2 requested review from a team, elharo, feilong-liu, jaystarshot, shrinidhijoshi, vivek-bharathan and yhwang as code owners March 4, 2025 22:54

shangm2 requested a review from presto-oss March 4, 2025 22:54

prestodb-ci added the from:Meta PR from Meta label Mar 4, 2025

shangm2 force-pushed the convert_datetime branch from c1df302 to b8b24de Compare March 4, 2025 22:59

shangm2 force-pushed the convert_datetime branch from b8b24de to bd5c2f2 Compare March 4, 2025 23:13

shangm2 force-pushed the convert_datetime branch from bd5c2f2 to 13882e2 Compare March 4, 2025 23:27

shangm2 force-pushed the convert_datetime branch from 13882e2 to 933db4d Compare March 5, 2025 00:29

shangm2 force-pushed the convert_datetime branch from 933db4d to 6f14f69 Compare March 5, 2025 00:51

shangm2 force-pushed the convert_datetime branch from 6f14f69 to 97c36e1 Compare March 5, 2025 03:23

shangm2 force-pushed the convert_datetime branch from 97c36e1 to 4fe62d2 Compare March 5, 2025 03:32

shangm2 changed the title ~~Convert DateTime to long for coordinator critical path~~ Convert DateTime to long along the critical path within coordinator Mar 5, 2025

shangm2 force-pushed the convert_datetime branch 2 times, most recently from 6b28a2b to b7e9e0c Compare March 5, 2025 06:04

shangm2 force-pushed the convert_datetime branch from b7e9e0c to f003694 Compare March 5, 2025 20:31

yuandagits reviewed Mar 5, 2025

View reviewed changes

presto-native-execution/presto_cpp/main/PrestoTask.cpp Show resolved Hide resolved

shangm2 force-pushed the convert_datetime branch 7 times, most recently from a374539 to 66335d7 Compare March 10, 2025 19:27

shangm2 added 5 commits March 10, 2025 12:33

Change DateTime for APIs

94f89ee

Change DateTime for tests

b305c05

Change DateTime for cpp workers

13e51f1

Change DateTime for presto-spark-base

2f1cb35

Change DateTime for presto-ui

ca24222

shangm2 force-pushed the convert_datetime branch from 66335d7 to eef2714 Compare March 10, 2025 19:34

Change DateTime for presto-main

2898449

shangm2 force-pushed the convert_datetime branch from eef2714 to 2898449 Compare March 10, 2025 19:38

ZacBlanco approved these changes Mar 11, 2025

View reviewed changes

yhwang reviewed Mar 11, 2025

View reviewed changes

presto-ui/src/components/QueryOverview.jsx Show resolved Hide resolved

yhwang approved these changes Mar 11, 2025

View reviewed changes

NikhilCollooru approved these changes Mar 11, 2025

View reviewed changes

amitkdutta approved these changes Mar 11, 2025

View reviewed changes

shangm2 merged commit 6c36be4 into prestodb:master Mar 11, 2025
100 of 101 checks passed

zation99 mentioned this pull request Mar 20, 2025

CPP build failure with broken protocol #24763

Closed

prestodb-ci mentioned this pull request Mar 28, 2025

Add release notes for 0.292 #24825

Merged

30 tasks

unidevel mentioned this pull request Apr 25, 2025

Add release notes for 0.292 unix280/presto#23

Closed

30 tasks

This was referenced May 6, 2025

Add release notes for 0.292 unix280/presto#28

Closed

Add release notes for 0.292 unix280/presto#29

Closed

Conversation

shangm2 commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

facebook-github-bot commented Mar 4, 2025

Uh oh!

facebook-github-bot commented Mar 4, 2025

Uh oh!

facebook-github-bot commented Mar 4, 2025

Uh oh!

facebook-github-bot commented Mar 5, 2025

Uh oh!

facebook-github-bot commented Mar 5, 2025

Uh oh!

facebook-github-bot commented Mar 5, 2025

Uh oh!

facebook-github-bot commented Mar 5, 2025

Uh oh!

facebook-github-bot commented Mar 5, 2025

Uh oh!

Uh oh!

facebook-github-bot commented Mar 11, 2025

Uh oh!

shangm2 commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZacBlanco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yhwang left a comment

Choose a reason for hiding this comment

Uh oh!

NikhilCollooru left a comment

Choose a reason for hiding this comment

Uh oh!

amitkdutta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

shangm2 commented Mar 4, 2025 •

edited

Loading

shangm2 commented Mar 11, 2025 •

edited

Loading