-
Notifications
You must be signed in to change notification settings - Fork 3
Add PartiQL parser (GetHistory and ShowTables) #221
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
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 implements PartiQL parser support for GetHistory and ShowTables operations. The addition enables parsing SQL-like syntax for table history queries and information schema queries, providing a more familiar interface for table management operations.
Key changes include:
- Added GetHistory and ShowTables statement classes for PartiQL operations
- Extended the PartiQL parser to recognize
SELECT history()
andSELECT * FROM information_schema.tables
syntax - Updated GetHistory contract to use a new query-based argument structure instead of direct table/key parameters
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
GetHistoryStatement.java | New statement class implementing history query functionality with table, conditions, and limit support |
ShowTablesStatement.java | New statement class for table listing operations with optional table name filtering |
PartiqlParserVisitor.java | Enhanced parser to detect and handle history queries and information schema queries |
ScalarPartiqlParser.java | Added constants for history function and information schema table names |
JacksonUtils.java | Added utility methods for condition validation and value extraction |
GetHistory.java | Refactored to use new query-based argument structure with condition validation |
Constants.java | Added new contract constants and query field definitions |
TableStoreClientError.java | Added new error codes for table alias and projection validation |
Comments suppressed due to low confidence (3)
table-store/src/main/java/com/scalar/dl/tablestore/client/partiql/parser/PartiqlParserVisitor.java:222
- [nitpick] The method name 'getPath' is ambiguous. Consider renaming to 'extractPathComponents' or 'getExpressionPath' to better indicate it returns a list of path components.
private List<String> getPath(ExprPath exprPath) {
table-store/src/main/java/com/scalar/dl/tablestore/client/util/JacksonUtils.java:73
- [nitpick] The method name 'isConditionForShowTables' could be more descriptive. Consider 'isTableNameCondition' or 'isInformationSchemaTableCondition' to better convey its specific purpose.
public static boolean isConditionForShowTables(JsonNode condition) {
table-store/src/main/java/com/scalar/dl/tablestore/client/partiql/parser/PartiqlParserVisitor.java:309
- [nitpick] The method name 'isInformationSchemaQuery' could be more specific. Consider 'isInformationSchemaTablesQuery' since it specifically checks for the 'tables' table within information_schema.
private boolean isInformationSchemaQuery(FromTableRef tableRef) {
generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/GetHistory.java
Show resolved
Hide resolved
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.
Summary of Changes
Hello @jnmt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly expands the query capabilities of the system by integrating GetHistory
and ShowTables
operations through the PartiQL parser. It involves substantial refactoring of existing contracts and the introduction of new parsing logic and data structures to support these new query types, ensuring robust validation and error handling.
Highlights
- New PartiQL Query Support: Introduced support for two new PartiQL query types:
SELECT history()
to retrieve the history of a record, andSELECT * FROM information_schema.tables
to list available tables. - Refactored GetHistory Contract: The underlying
GetHistory
contract (common/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/Constants.java
,generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/GetHistory.java
) has been refactored to use a more generalized query argument structure (QUERY_TABLE
,QUERY_CONDITIONS
,QUERY_LIMIT
) instead ofRECORD_TABLE
andRECORD_KEY
. - PartiQL Parser Enhancements: The PartiQL parser (
table-store/src/main/java/com/scalar/dl/tablestore/client/partiql/parser/PartiqlParserVisitor.java
) now recognizes and translateshistory()
function calls and queries againstinformation_schema.tables
into corresponding contract statements. - New Statement Classes: Added dedicated statement classes (
GetHistoryStatement.java
,ShowTablesStatement.java
) to represent the parsedGetHistory
andShowTables
PartiQL queries, facilitating their execution as contract calls. - Enhanced Validation and Error Handling: New validation logic and error codes (
TableStoreClientError.java
) have been added to ensure correct usage of the new query types, covering scenarios like invalid conditions, unsupported aliases, and projection limitations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request adds PartiQL parsing support for GetHistory
and ShowTables
statements. The changes are well-structured and include updates to contracts, new statement classes, and comprehensive tests. My review focuses on reducing code duplication and ensuring consistent use of constants.
generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/GetHistory.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
...c/integration-test/java/com/scalar/dl/genericcontracts/GenericContractTableEndToEndTest.java
Show resolved
Hide resolved
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.
I've added a couple comments and suggestions. PTAL!
table-store/src/main/java/com/scalar/dl/tablestore/client/error/TableStoreClientError.java
Outdated
Show resolved
Hide resolved
generic-contracts/src/test/java/com/scalar/dl/genericcontracts/table/v1_0_0/GetHistoryTest.java
Show resolved
Hide resolved
…r/TableStoreClientError.java Co-authored-by: Josh Wong <[email protected]>
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! Thank you!🙇🏻♂️
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! Thank you!
Left one minor suggestion, but you can handle it in another PR as necessary.
...ore/src/main/java/com/scalar/dl/tablestore/client/partiql/statement/GetHistoryStatement.java
Show resolved
Hide resolved
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!
Co-authored-by: Josh Wong <[email protected]>
Description
This PR adds PartiQL support for utility queries to get the table schema and histories of records. See add tests in
PartiqlParserVisitorTest
for possible and impossible query patterns.Related issues and/or PRs
Changes made
GetHistory
contract specification.Select
contract, and can validate them.Checklist
Additional notes (optional)
ShowTables
contract, we follow a standard specification called information_schema.Release notes
Added the PartiQL parser for utility queries.