-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
refactor(pinot) regression on area chart (DB engine error) #25749
Comments
@martin-raymond I'm wondering it might be a bug in Apache Calcite side because it can't correctly calculate expression There is a workaround might be useful, 1) try to create a derived column based on the startDate in Pinot as Hope it helps you. |
we tried that @zhaoyongjie, but with pinot 1.0.0 and superset 3.0.1, the table does not regenerated properly in superset (error on the field timestamp in the schema) |
@martin-raymond Could you please post error screenshot or log? |
@zhaoyongjie yes, here it is :
|
@zhaoyongjie we discussed with the pinot team and the problem should be in superset and not in pinot or apache calcite |
@martin-raymond Do you try my suggestion that removing "ms" tag and create a timestamp column instead of integer column as time column in Superset dataset? |
@martin-raymond |
you want me to try to add a calculated column in the dataset in superset directly ? the timestamp type is not present |
@martin-raymond |
Yes, from your screenshot the dateHeureDebut is Long.
Not True, try the expression |
ok @zhaoyongjie . that's not it. there is no error, but the result is not right. Here is the query executed in pinot : The syntax is right, but the result is not the result expected |
@zhaoyongjie we tried to use a calculated column as you said superset seems to not be able to display the result from pinot. here are the logs when we try to create a chart :
(we tried DATETIME and STRING, same result) There is no error, just nothing is displayed in superset (no result) |
@martin-raymond column type should be "timestamp" rather than "string", you don't need to fill out the format datetime input box. |
@zhaoyongjie yes we tried TIMESTAMP, STRING, with a format datetime, without a format, same results in the end |
Could you please post screenshot of explore? |
@zhaoyongjie what do you mean by "explore" ? |
@martin-raymond Data Explore page, the other words that open the chart in Superset. |
The query :
which works fine in pinot @zhaoyongjie that is what you wanted ? |
Please clear up the "Contribution Mode" and try it again. |
Aucun is the french word for "no contribution mode" so it is already cleared up @zhaoyongjie |
It's weird. Could you click the three dots on the right corner of Explore page, and check out what Query is? and please check the logs of Superset, does superset acturally send a query? |
Hi @zhaoyongjie and @ege-st do you have any news on the issue ? After discussion with the pinot team, the problem is definitly in superset, and from our perspective, it is a huge issue because it prevents us to be able to use the 3.0.0 for our clients |
@martin-raymond I was wondering we should checkout all the list for this case
BTW, we have deployed this patch and run Pinot about 3 month, everything works fine. |
@zhaoyongjie thx for your answer yes i confirm all that, i just re-tried everything from scratch, same results |
Could you post screenshot as following
|
The first column of results seems like TEXT instead of DATATIME from the screenshot of SQLLab. Please check out 1) Pinot side 2) SQLalchemy driver of Pinot 3) or what did you change in Superset codebase? @martin-raymond |
pinot seems fine. what do you want me to check there ? it worked weel until the new request from 3.0.0 |
What did version of SQLAlchemy Driver use? try following command in your superset cluser. pip list | grep pinot |
pinotdb : 0.3.8 |
https://pypi.org/project/pinotdb/
The latest version is 5.1.0, try to update it.
…On Wed, Nov 8, 2023 at 4:38 PM martin-raymond ***@***.***> wrote:
pinotdb : 0.3.8
—
Reply to this email directly, view it on GitHub
<#25749 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPMKUVJNPMETVVPENBIUGLYDORRHAVCNFSM6AAAAAA6OJMWLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBSGE2DIOJSGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Best regards,
Yongjie
|
@zhaoyongjie well, it seems to have solved the issue (but we have to use a calculated column as you adviced). thank you very much @zhaoyongjie for your help. i will keep you informed here if it works on every charts, then i will close my issue. |
i confirm all our charts are now ok. thx very much @zhaoyongjie |
@zhaoyongjie the issue is still ongoing. The workaround is theoretically correct, but in practice, on our production data, the volume is so significant that casting to a timestamp, applied to the temproel field through the time range filter, results in processing times multiplied by 40, making our service unusable. This is ultimately due to a faulty query in Superset 3.0.1. Therefore, the problem persists. |
It's been quite a while since this thread was revisited, but it's one of the best support threads I've read on here in a long time! @martin-raymond are you still up against this, and @zhaoyongjie do you have any new theories? I'm wondering if this is still an issue in Superset 3.1 / 4.0. If there is a faulty query being generated, maybe you can share what it looks like now? |
Hi Evan, we are using Superset 3.1 and Pinot stable version on the
Production environment, the corresponding query works well, so if no more
issues are reported, might close this one.
On Mon 8. Apr 2024 at 22:58, Evan Rusackas ***@***.***> wrote:
It's been quite a while since this thread was revisited, but it's one of
the best support threads I've read on here in a long time! @martin-raymond
<https://github.com/martin-raymond> are you still up against this, and
@zhaoyongjie <https://github.com/zhaoyongjie> do you have any new
theories? I'm wondering if this is still an issue in Superset 3.1 / 4.0. If
there is a faulty query being generated, maybe you can share what it looks
like now?
—
Reply to this email directly, view it on GitHub
<#25749 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPMKUVOR5HY4YCT22DXY4TY4MAHLAVCNFSM6AAAAAA6OJMWLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGYZTAMRSGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Best regards,
Yongjie
|
Agreed. I'll be optimistic here and just close this one. If @martin-raymond is still facing this issue, we can revisit and/or reopen as necessary. |
we changed all of our temporal field in our pinot table, so we skipped the problem completely. the thread can stay closed, thx |
We are using a pinot table as a dataset, with a temporal column startDate in epoch_ms. Before the 3.0.0.rc4 version, we were using it to create the followin area chart :
Since the release of the 3.0.0.rc4 version, our chart is not loading :
DB engine Erreur : could not convert string to Timestamp
Before 3.0.0.rc4, the query generated by the simple area chart was :
SELECT DATETIMECONVERT(startDate, '1:MILLISECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '1:DAYS'), count(*) FROM "dataset" WHERE startDate >= 1694908800000 AND startDate < 1697500800000 GROUP BY DATETIMECONVERT(startDate, '1:MILLISECONDS:EPOCH', '1:MILLISECONDS:EPOCH', '1:DAYS') ORDER BY count(*) DESC LIMIT 10000;
(1694908800000 and 1697500800000 are just examples of our time filter)
After the 3.0.0.rc4, the query that causes the error is :
SELECT CAST(DATE_TRUNC('day', CAST(DATETIMECONVERT((startDate/1000), '1:SECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS') AS TIMESTAMP)) AS TIMESTAMP), count(*) FROM "dataset" WHERE startDate >= 1694908800000 AND startDate < 1697500800000 GROUP BY CAST(DATE_TRUNC('day', CAST(DATETIMECONVERT((startDate/1000), '1:SECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS') AS TIMESTAMP)) AS TIMESTAMP) ORDER BY count(*) DESC LIMIT 10000;
The part :
DATETIMECONVERT((startDate/1000), '1:SECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS')
seems odd because it literally said "convert seconds in seconds", and we think it might causes the crash.
We think the problem is caused by : #24942
Thx for your help
3.0.0.rc4
The text was updated successfully, but these errors were encountered: