-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Wrong datetime formating when building the query #185
Comments
that would really help! I knew this was going to hit on some db with different autocast... |
HI, Haven't found a nice way to let SQLAlchemy dialect to convert the date time yet.
*Sorry for posting code, I'm facing some issues managing new branches and PR. |
Thanks for the workaround . |
I am still finding problem with this issue. I just updated to latest caravel. |
#283 was not the right approach. we have yet to find a solution to this. |
This should help #446, please test and report whether it fixes your issues. |
@gbrian is this fixed for you? |
Hi @xrmx, Sorry not working due to precision: Don't know the best fix, I'll suggest trim precision. |
@xrmx : If you agree we can use column data type to decide if we can use DateTime2 or trim milliseconds. I mean in case column has been defined as DateTime2 cast to DateTime2, if not just trim (but not TSQL version). |
@gbrian adding a proper python_date_format wouldn't fix that? I know it's manual and not working out of the box is lame. |
@xrmx proper way (IMO)
so
I think is this way I keep as much precision as possible with the definition the user has done. |
@gbrian a diff is easier to review :) Anyway the chain of ifs insinde the format is a no-go imho :) Also we can remove the for loop and the dict if we need to patch only mssql and oracle. Also in mssql case do we really need the CONVERT, can't we just return a slice of default instead?
|
Yeah! totally agree with the "ifs" ;) was just playing around, sorry. Sadly I don't have an older SQL Server version to test the "default" behavior so I though CONVERT will be more backward compatible: http://www.techonthenet.com/sql_server/functions/convert.php (basically will cover SQL Server 2005) Let me try again: |
@gbrian do we really need the convert for the DATETIME2? it looks like it can handle our default just fine. |
Ah, ok! yep you are right, for DATETIME2, CONVERT is not needed. Creating PR, talk later |
What is the point of formatting dates within Superset, why don't let SQLAlchemy do that? |
HI,
I've found that datetime is being custom formatted before sending to SQLAlchemy that causes incompatibility between Datetime and Datetime2 in SQLServer.
I'll check to use the SQLAlchemy dialect of the datasource to format instead.
The text was updated successfully, but these errors were encountered: