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.

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

Add AQEShuffleRead WriteFiles execs to the supportedOps and score files #963

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

cindyyuanjiang
Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang commented Apr 24, 2024

Closes #865
This PR updates the CSV and override files for all remaining execs or expressions marked as TNEW in #865.

@cindyyuanjiang cindyyuanjiang self-assigned this Apr 24, 2024
@cindyyuanjiang cindyyuanjiang added bug Something isn't working core_tools Scope the core module (scala) labels Apr 24, 2024
Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang

@parthosa I thought you worked on adding support for CheckOverflowInTableInsert before.

Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

lets add them to the score sheets -- if they are missing -- just in case we implemented classes for those execs

@amahussein amahussein changed the title Add support for remaining operators marked as TNEW Add AQEShuffleRead WriteFiles execs to the supportedOps and score files Apr 24, 2024
@parthosa
Copy link
Collaborator

@parthosa I thought you worked on adding support for CheckOverflowInTableInsert before.

My PR had not made any changes in these files because I was not able to reproduce this expression in physical plan and we concluded that this expression would never show up.

Although I think we should have these updated scores. Thanks @cindyyuanjiang for updating them.

parthosa
parthosa previously approved these changes Apr 25, 2024
Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thank you @cindyyuanjiang

Signed-off-by: cindyyuanjiang <[email protected]>
@cindyyuanjiang
Copy link
Collaborator Author

Thanks @amahussein @parthosa! I have updated the scores for the new operators.

Copy link
Collaborator

@nartal1 nartal1 left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang !

Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang
LGTME

@cindyyuanjiang cindyyuanjiang merged commit 7f02a82 into NVIDIA:dev Apr 25, 2024
15 checks passed
@cindyyuanjiang cindyyuanjiang deleted the sync-remaining-ops branch April 25, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core_tools Scope the core module (scala)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Audit newly added rows to the supported ops CSV files
4 participants