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.

Sign up for GitHub

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

Jump to bottom

Revert DataSize change for public-facing API #24682

Merged
merged 4 commits into from
Mar 6, 2025
Merged

Revert DataSize change for public-facing API #24682

merged 4 commits into from
Mar 6, 2025

Conversation

shangm2
Copy link
Contributor

@shangm2 shangm2 commented Mar 6, 2025 •
edited
Loading

Description

  1. this pr reverts DataSize change for public facing API, aka QueryStats and other required changes from Change from DataSize to long to remove unnecessary data creation #24582

Motivation and Context

  1. We dont want to break current API for now.

Impact

Test Plan

  1. passed verifier tests: https://www.internalfb.com/intern/presto/verifier/results/?test_id=212950
  2. UI looks good
Screenshot 2025-03-05 at 22 37 20 Screenshot 2025-03-05 at 22 38 31 Screenshot 2025-03-05 at 22 38 52

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

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

@shangm2 shangm2 requested a review from presto-oss March 6, 2025 05:20
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Mar 6, 2025
@shangm2 shangm2 force-pushed the dont_change_public_api branch from f56d8bd to 88ca7d0 Compare March 6, 2025 05:27
@shangm2 shangm2 force-pushed the dont_change_public_api branch from 88ca7d0 to fa6fa4d Compare March 6, 2025 06:35
@shangm2
Copy link
Contributor Author

shangm2 commented Mar 6, 2025

@ZacBlanco @NikhilCollooru I wonder if you guys can take a look at this. Thanks.

yhwang
yhwang approved these changes Mar 6, 2025
View reviewed changes
Copy link
Member

@yhwang yhwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for the revert of UI part.

@facebook-github-bot
Copy link
Collaborator

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

@shangm2 shangm2 merged commit 6ee450c into prestodb:master Mar 6, 2025
53 checks passed
shangm2 added a commit to shangm2/presto that referenced this pull request Mar 7, 2025
@shangm2
Revert DataSize change for public-facing API (prestodb#24682)
3f8cff1 
* Revert API change for QueryStats

* Revert related tests

* Revert UI change

* Revert presto-spark-base change
This was referenced Mar 10, 2025
Closed
Closed
jp-sivaprasad pushed a commit to jp-sivaprasad/presto that referenced this pull request Mar 10, 2025
@shangm2 @jp-sivaprasad
Revert DataSize change for public-facing API (prestodb#24682)
1c7db77 
* Revert API change for QueryStats

* Revert related tests

* Revert UI change

* Revert presto-spark-base change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yhwang yhwang yhwang approved these changes

@ZacBlanco ZacBlanco ZacBlanco approved these changes

@NikhilCollooru NikhilCollooru NikhilCollooru approved these changes

@shrinidhijoshi shrinidhijoshi Awaiting requested review from shrinidhijoshi shrinidhijoshi is a code owner

@jaystarshot jaystarshot Awaiting requested review from jaystarshot jaystarshot is a code owner

@feilong-liu feilong-liu Awaiting requested review from feilong-liu feilong-liu is a code owner

@elharo elharo Awaiting requested review from elharo elharo is a code owner

@ClarenceThreepwood ClarenceThreepwood Awaiting requested review from ClarenceThreepwood ClarenceThreepwood is a code owner

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

Assignees
No one assigned
Labels
from:Meta PR from Meta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shangm2 @facebook-github-bot @yhwang @ZacBlanco @NikhilCollooru @prestodb-ci