You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "abhioncbr (via GitHub)" <gi...@apache.org> on 2023/05/13 03:44:18 UTC
[GitHub] [pinot] abhioncbr opened a new pull request, #10763: [multistage]: Supporting Json data type & better error messages
abhioncbr opened a new pull request, #10763:
URL: https://github.com/apache/pinot/pull/10763
- handling the `json` data type in a multistage query engine. as per the [comment](https://github.com/apache/pinot/issues/10205#issuecomment-1543278577)
- returning the meaningful error msg for unsupported type.
- returning the meaningful error message of SQL parsing errors.
As per the issue https://github.com/apache/pinot/issues/10205
cc @walterddr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr merged pull request #10763: [multistage]: better error messages.
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #10763:
URL: https://github.com/apache/pinot/pull/10763
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10763: [multistage]: Supporting Json data type & better error messages
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10763:
URL: https://github.com/apache/pinot/pull/10763#issuecomment-1546522327
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10763](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (683cf9e) into [master](https://app.codecov.io/gh/apache/pinot/commit/fe98bb0783213504c77be397b1750de3962000ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (fe98bb0) will **decrease** coverage by `56.78%`.
> The diff coverage is `7.69%`.
```diff
@@ Coverage Diff @@
## master #10763 +/- ##
=============================================
- Coverage 70.45% 13.68% -56.78%
+ Complexity 5646 439 -5207
=============================================
Files 2153 2099 -54
Lines 115396 113249 -2147
Branches 17368 17180 -188
=============================================
- Hits 81305 15497 -65808
- Misses 28449 96481 +68032
+ Partials 5642 1271 -4371
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `13.68% <7.69%> (-0.05%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...t/controller/api/resources/PinotQueryResource.java](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90UXVlcnlSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (-59.45%)` | :arrow_down: |
| [.../java/org/apache/pinot/query/type/TypeFactory.java](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvdHlwZS9UeXBlRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-70.38%)` | :arrow_down: |
| [...requesthandler/MultiStageBrokerRequestHandler.java](https://app.codecov.io/gh/apache/pinot/pull/10763?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvTXVsdGlTdGFnZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `28.57% <100.00%> (-39.10%)` | :arrow_down: |
... and [1701 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10763/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #10763: [multistage]: Supporting Json data type & better error messages
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10763:
URL: https://github.com/apache/pinot/pull/10763#discussion_r1193012876
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -108,6 +108,9 @@ public String handlePostSql(String requestJsonStr, @Context HttpHeaders httpHead
}
LOGGER.debug("Trace: {}, Running query: {}", traceEnabled, sqlQuery);
return executeSqlQuery(httpHeaders, sqlQuery, traceEnabled, queryOptions, "/sql");
+ } catch (ProcessingException pe) {
+ LOGGER.error("Caught exception while processing get request {}", pe.getMessage());
Review Comment:
this is a POST request not a get request
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java:
##########
@@ -81,14 +81,14 @@ private RelDataType toRelDataType(FieldSpec fieldSpec) {
return fieldSpec.isSingleValueField() ? createSqlType(SqlTypeName.DECIMAL)
: createArrayType(createSqlType(SqlTypeName.DECIMAL), -1);
case JSON:
- // TODO: support JSON, JSON should be supported using a special RelDataType as it is not a simple String,
- // nor can it be easily parsed as a STRUCT.
+ return createSqlType(SqlTypeName.VARCHAR);
Review Comment:
let's separate the support of the JSON type into a separate PR. and for this PR let's only contribute the proper error message with unsupported datatype propagation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] abhioncbr commented on a diff in pull request #10763: [multistage]: Supporting Json data type & better error messages
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #10763:
URL: https://github.com/apache/pinot/pull/10763#discussion_r1193019666
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java:
##########
@@ -81,14 +81,14 @@ private RelDataType toRelDataType(FieldSpec fieldSpec) {
return fieldSpec.isSingleValueField() ? createSqlType(SqlTypeName.DECIMAL)
: createArrayType(createSqlType(SqlTypeName.DECIMAL), -1);
case JSON:
- // TODO: support JSON, JSON should be supported using a special RelDataType as it is not a simple String,
- // nor can it be easily parsed as a STRUCT.
+ return createSqlType(SqlTypeName.VARCHAR);
Review Comment:
Make sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kishoreg commented on pull request #10763: [multistage]: Supporting Json data type & better error messages
Posted by "kishoreg (via GitHub)" <gi...@apache.org>.
kishoreg commented on PR #10763:
URL: https://github.com/apache/pinot/pull/10763#issuecomment-1546516472
LGTM.. lets add a test case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org