You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@griffin.apache.org by "XiaoyuBD (via GitHub)" <gi...@apache.org> on 2023/02/18 15:36:12 UTC
[GitHub] [griffin] XiaoyuBD opened a new pull request, #628: JDBC Connector SQL mode
XiaoyuBD opened a new pull request, #628:
URL: https://github.com/apache/griffin/pull/628
# What changes were proposed in this pull request?
JDBC connector could configuring sql which would get sql results as datasource.
It is more flexible to use. And in some special cases (such as only a small number of columns need to be extracted), it can reduce the IO of loading data from db.
# Does this PR introduce any user-facing change?
No. The original configuration mode is not affected.
# How was this patch tested?
Unit Tests. And it has been used in our production environment for several months.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] whhe merged pull request #628: JDBC Connector SQL mode
Posted by "whhe (via GitHub)" <gi...@apache.org>.
whhe merged PR #628:
URL: https://github.com/apache/griffin/pull/628
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] whhe commented on pull request #628: JDBC Connector SQL mode
Posted by "whhe (via GitHub)" <gi...@apache.org>.
whhe commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1539286180
> LGTM, @guoyuepeng can you please merge thi, for some reason the merge pr script is not working on my system
There is no related Jira ticket, I think it's OK to use `squash and merge` on GitHub.
Besides, the ASF projects migrated to GitHub a long time ago, I think we can move all stuffs (like Jira and Wiki) to GitHub now. @guoyuepeng @chitralverma Maybe we can discuss it elsewhere.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] XiaoyuBD commented on pull request #628: JDBC Connector SQL mode
Posted by "XiaoyuBD (via GitHub)" <gi...@apache.org>.
XiaoyuBD commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1527454620
@chitralverma Hi, is there any other comments? This PR has been open for a long time.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] guoyuepeng commented on pull request #628: JDBC Connector SQL mode
Posted by "guoyuepeng (via GitHub)" <gi...@apache.org>.
guoyuepeng commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1482809334
LGTM.
@chitralverma any comments?
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] chitralverma commented on pull request #628: JDBC Connector SQL mode
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1538658841
LGTM, merging.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] chitralverma commented on a diff in pull request #628: JDBC Connector SQL mode
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on code in PR #628:
URL: https://github.com/apache/griffin/pull/628#discussion_r1112052266
##########
griffin-doc/measure/measure-configuration-guide.md:
##########
@@ -547,13 +547,15 @@ List of supported data connectors:
| Name | Type | Description | Default Values |
|:-----------|:---------|:---------------------------------------|:-------------- |
| database | `String` | database name | default |
+| sql | `String` | query sql | `Empty` |
Review Comment:
is `sql` mode mutually exclusive to the usual table db mode or does it work with them? this needs to be documented somewhere.
Also how does this work now with the `where` functionality ?
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] chitralverma commented on pull request #628: JDBC Connector SQL mode
Posted by "chitralverma (via GitHub)" <gi...@apache.org>.
chitralverma commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1437449959
@XiaoyuBD thanks for the PR.
I have some questions about the purpose of the changes.
If you dont want to query unwanted columns or even transform the data before applying checks, you can put 1 or more queries in the `pre.proc` section of any data source. This has a far bigger scope that just JDBC as it applies to all types of sources.
This will also support projection and predicate push down to the source which will allow much less IO.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] XiaoyuBD commented on a diff in pull request #628: JDBC Connector SQL mode
Posted by "XiaoyuBD (via GitHub)" <gi...@apache.org>.
XiaoyuBD commented on code in PR #628:
URL: https://github.com/apache/griffin/pull/628#discussion_r1113132637
##########
griffin-doc/measure/measure-configuration-guide.md:
##########
@@ -547,13 +547,15 @@ List of supported data connectors:
| Name | Type | Description | Default Values |
|:-----------|:---------|:---------------------------------------|:-------------- |
| database | `String` | database name | default |
+| sql | `String` | query sql | `Empty` |
Review Comment:
Thanks for the comment. If the config `sql` was provided, `database`, `tablename` and `where` can all be ignored, connector will extract the result of sql. I added that description to the MD and added the SQL mode config example.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] XiaoyuBD commented on pull request #628: JDBC Connector SQL mode
Posted by "XiaoyuBD (via GitHub)" <gi...@apache.org>.
XiaoyuBD commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1436607298
@guoyuepeng Can you review this PR? Thanks.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] XiaoyuBD commented on pull request #628: JDBC Connector SQL mode
Posted by "XiaoyuBD (via GitHub)" <gi...@apache.org>.
XiaoyuBD commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1438548244
> purpose
You're right, using **pre.proc** can also achieve the effect I described earlier. My previous description gave an inappropriate example. In fact, the main benefit of SQL mode is that it is very flexible. For data developers, they can express various datasets in SQL, including join multiple tables to extract and analyze query results.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [griffin] github-actions[bot] commented on pull request #628: JDBC Connector SQL mode
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #628:
URL: https://github.com/apache/griffin/pull/628#issuecomment-1482067789
Automated Message: This PR is being labelled as stale and will be closed in next 15 days due to lack of activity. To avoid this push new commits or ask the committers for a review/ resolution.
--
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: dev-unsubscribe@griffin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org