feat: add connector for CouchbaseDB #29225
Conversation
dosubot
bot
added
data:connect
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #29225 +/- ##
==========================================
+ Coverage 60.48% 70.34% +9.85%
==========================================
Files 1931 1970 +39
Lines 76236 78585 +2349
Branches 8568 8973 +405
==========================================
+ Hits 46114 55279 +9165
+ Misses 28017 21108 -6909
- Partials 2105 2198 +93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| Any | ||
| ) | ||
|
|
||
| class CouchbaseEngineSpec(BaseEngineSpec): |
There was a problem hiding this comment.
you can you add BasicParametersMixin to this since using MYSQL dialect
https://github.com/apache/superset/blob/master/superset/db_engine_specs/base.py#L2276
|
Thanks @ayush33143314 for adding the engine. Would you also mind:
|
|
Thank you for this! Would it also be possible to add a docs entry here? |
Hi , yes, I am preparing the doc, I will submit doc along with tests in next patchset. Sorry for this patch. It is still in progress. |
|
Ahh, ok, thanks! Feel free to add [WIP] to the title and/or convert it to a draft PR if you need a while to keep working on it, just so others don't assume it's ready for review like I did. Thanks again! |
|
@hughhhh @john-bodley Please review my latest patch, and guide me if I need to make any change. Thank you |
|
Looking forward to this PR being merged |
| @classmethod | ||
| def epoch_to_dttm(cls) -> str: | ||
| return "MILLIS_TO_STR({col} * 1000, '111')" | ||
|
|
||
| @classmethod | ||
| def epoch_ms_to_dttm(cls) -> str: | ||
| return "MILLIS_TO_STR({col}, '111')" |
There was a problem hiding this comment.
I tried reading up on the docs, and I didn't fully understand how this format is valid:
MILLIS_TO_STR: https://docs.couchbase.com/cloud/n1ql/n1ql-language-reference/datefun.html#fn-date-millis-to-str- Date format: https://docs.couchbase.com/cloud/n1ql/n1ql-language-reference/datefun.html#date-string
To be clear, the idea of these methods is to provide an expression that can convert an epoch (seconds since 1970-01-01 00:00:00 in UTC) into a native temporal format. But if it works it works - if this is the case, maybe adding a comment as to why the format '111' works here would be helpful.
There was a problem hiding this comment.
Thank you so much for your comments,
If format is unspecified or incorrect it gives combined date and time in result , On querying
SELECT MILLIS_TO_STR(1463284740000,'111'); it gives me "2016-05-15T03:59:00Z" in result . I had this '111' just for notation . But I have removed this in my new patch , because MILLIS_TO_STR(1463284740000) also gives me same results.
| ("Date", "DATETIME(DATE_FORMAT_STR(STR_TO_UTC('2019-01-02'), 'iso8601'))"), | ||
| ( | ||
| "DateTime", | ||
| "DATETIME(DATE_FORMAT_STR(STR_TO_UTC('2019-01-02T03:04:05'), 'iso8601'))", | ||
| ), |
There was a problem hiding this comment.
It would be nice to have a few more typical formats featured here, like VARCHAR, TEXT, INT, as I assume those will be commonly used.
There was a problem hiding this comment.
I have added test case for other data types also.
ayush33143314
force-pushed
the
master
branch
2 times, most recently
from
0a11aff
to
d2f3da0
Compare
There was a problem hiding this comment.
One small last comment, we can probably do improvements in follow-up PRs. Also, since CounchbaseDB doesn't seem to have any dedicated temporal types, have you been able to get it to work fully with temporal charts? If not, you may benefit from specifying column_type_mutators on your spec to handle the conversion after query results come in.
SUMMARY
Add support for couchbase db. https://www.couchbase.com/products/analytics/
TESTING INSTRUCTIONS
mannual Testing
ADDITIONAL INFORMATION