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

fix: serialize AnalyzeExec #1756

Merged
merged 3 commits into from
Sep 13, 2023
Merged

fix: serialize AnalyzeExec #1756

merged 3 commits into from
Sep 13, 2023

Conversation

adhish20
Copy link
Contributor

Fixes #1752

@adhish20 adhish20 linked an issue Sep 13, 2023 that may be closed by this pull request
RPC: AnalyzeExec not serialized #1752
Closed
@adhish20 adhish20 requested a review from scsmithr September 13, 2023 12:21
@adhish20
Copy link
Contributor Author

should we submit a change to data fusion to make the verbose param pub in AnalyzeExec? Currently, I hardcoded it to true

@adhish20 adhish20 requested a review from vrongmeal September 13, 2023 12:24
scsmithr
scsmithr reviewed Sep 13, 2023
View reviewed changes
Copy link
Member

@scsmithr scsmithr left a comment

Choose a reason for hiding this comment

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

should we submit a change to data fusion to make the verbose param pub in AnalyzeExec? Currently, I hardcoded it to true

Yes

Can we also add an slt test to ensure that this doesn't error with rpc?

@adhish20 adhish20 merged commit 9e0e4b3 into main Sep 13, 2023
@adhish20 adhish20 deleted the adhish/analyzeexec branch September 13, 2023 14:21
@adhish20 adhish20 restored the adhish/analyzeexec branch September 20, 2023 11:24
@adhish20 adhish20 deleted the adhish/analyzeexec branch September 20, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scsmithr scsmithr scsmithr left review comments

@vrongmeal vrongmeal Awaiting requested review from vrongmeal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RPC: AnalyzeExec not serialized
2 participants
@adhish20 @scsmithr