-
Notifications
You must be signed in to change notification settings - Fork 16
Add trino query #378
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 trino query #378
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for TRINO queries by introducing new factory methods in the TDJobRequest class and updating the TDJob Type enumeration.
- Added three overloaded versions of newTrinoQuery in TDJobRequest.java.
- Updated TDJob.java to include the new TRINO type for job requests.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/com/treasuredata/client/model/TDJobRequest.java | Adds overloaded newTrinoQuery methods for constructing TRINO job requests. |
| src/main/java/com/treasuredata/client/model/TDJob.java | Adds TRINO enumerator to the job type enum to support TRINO queries. |
Comments suppressed due to low confidence (2)
src/main/java/com/treasuredata/client/model/TDJobRequest.java:113
- Add Javadoc comments to explain the purpose and differences between the overloaded newTrinoQuery methods, similar to the existing query methods.
public static TDJobRequest newTrinoQuery(String database, String query)
src/main/java/com/treasuredata/client/model/TDJobRequest.java:122
- Ensure there are corresponding tests that validate the behavior of each newTrinoQuery overload for proper functionality.
public static TDJobRequest newTrinoQuery(String database, String query, String resultOutput)
tung-vu-td
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add some tests 😌
tung-vu-td
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
nmpennypacker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed for data exfiltrations
No description provided.