-
Notifications
You must be signed in to change notification settings - Fork 139
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
PPL describe command #646
PPL describe command #646
Conversation
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
This is an early draft exploring option 4 mentioned in #644 Here is the result of querying the sample flight data.
Currently the type names returned are wrong ( |
Signed-off-by: Sean Kao <seankao@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #646 +/- ##
=============================================
- Coverage 94.63% 62.76% -31.88%
=============================================
Files 279 10 -269
Lines 7517 658 -6859
Branches 556 119 -437
=============================================
- Hits 7114 413 -6701
+ Misses 349 192 -157
+ Partials 54 53 -1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Changed response formatter so that response format for ppl describe matches that of sql describe. |
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
Reverted the protocol breaking change for now. |
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
invalid index name (start with '_') Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
|
||
@Test | ||
public void testDescribeFieldsCommandShouldPass() { | ||
ParseTree tree = new PPLSyntaxParser().analyzeSyntax("describe t | fields a,b"); |
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.
What is the use case for this example? It seems little odd because if someone wants a,b as result he would specify a,b instead of describing table and filtering again.
Fields command is to project menitoned columns from the result set. A plausible usecase could be describe t | fields 2, 3 which implies give me second and third column names.
Also if someone appends other commands to describe, what is the expected behavior. I am assuming we will be calculating on the result set provided by prior describe command.
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 am assuming we will be calculating on the result set provided by prior describe command.
That's correct. When appending other commands to describe, the behavior is to query the metadata table, instead of the data table itself (as expected for the pipe syntax).
An example of the usage of fields
can be seen here
The fields do not refer to the data table's fields, but the metadata table's fields, because the result set of the describe command is a metadata table. Here you can see the full list of such fields.
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.
A plausible usecase could be describe t | fields 2, 3 which implies give me second and third column names.
Interestingly, we could support that using the following syntax:
describe t | where ORDINAL_POSITION=2 or ORDINAL_POSITION=3 | fields COLUMN_NAME
However, it doesn't quite work yet at the moment, due to type mismatch snippet 1, snippet 2. Also, most of the metadata are meaningless right now, including the order of the columns.
$ curl .... '{"query": "describe opensearch_dashboards_sample_data_flights | where ORDINAL_POSITION=0"}'
{
"error": {
"reason": "Invalid Query",
"details": "= function expected {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG],[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[TIMESTAMP,TIMESTAMP],[DATE,DATE],[TIME,TIME],[DATETIME,DATETIME],[INTERVAL,INTERVAL],[STRUCT,STRUCT],[ARRAY,ARRAY]}, but get [STRING,INTEGER]",
"type": "ExpressionEvaluationException"
},
"status": 400
}
$ curl .... '{"query": "describe opensearch_dashboards_sample_data_flights | where ORDINAL_POSITION=\"0\""}'
{
"error": {
"reason": "Invalid Query",
"details": "invalid to get integerValue from value of type STRING",
"type": "ExpressionEvaluationException"
},
"status": 400
}
Signed-off-by: Sean Kao <seankao@amazon.com>
I learned from https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/general/identifiers.rst that we actually have to support multiple indices. Most of them worked fine out of the box before my latest commit
In my last commit, I implemented this by joining the index strings, so that the following explain results matches:
I'm not sure whether this is the preferred way to do it. I tried other ways to do this, but was faced with some issues
Only the second index name is prefixed by _ODFE_SYS_TABLE_MAPPINGS" and I don't know why |
Signed-off-by: Sean Kao <seankao@amazon.com>
ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java
Outdated
Show resolved
Hide resolved
ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Sean Kao <seankao@amazon.com>
Signed-off-by: Sean Kao <seankao@amazon.com>
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.
Thanks for the change!
Description
Support
describe
command in PPLIssues Resolved
#644
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.