-
Notifications
You must be signed in to change notification settings - Fork 153
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
SNOW-227101: fix to datetime columns being created without timezone #202
base: main
Are you sure you want to change the base?
Conversation
@Nazarii do you know when this commit can be merged? and also is a reason that take out these TIMESTAMP_TZ from v1.1.8? |
@WeiGuHS no idea, this PR has no reviewers assigned, the same with the original ticket. I have no permissions to assign PR, I'll try to ping other contributors to review. It looks like the support of |
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
I have read the CLA Document and I hereby sign the CLA |
Not sure why but this PR shows there are conflicts. Diff does not show any merge conflicts or maybe there are some other conflicts? |
We will revisit this issue shortly. |
@sfc-gh-mkeller any news on this? conflicts comes from GEOGRAPHY being added at the same spot in the file as your modifications def visit_TIMESTAMP(self, type_, **kw):
++<<<<<<< HEAD
+ return "TIMESTAMP"
+
+ def visit_GEOGRAPHY(self, type_, **kw):
+ return "GEOGRAPHY"
++=======
+ is_local = kw.get('is_local', False)
+ timezone = kw.get('timezone', type_.timezone)
+ return "TIMESTAMP %s%s" % (
+ (timezone and "WITH" or "WITHOUT") + (
+ is_local and " LOCAL" or "") + " TIME ZONE",
+ "(%d)" % type_.precision if getattr(type_, 'precision',
+ None) is not None else ""
+ )
++>>>>>>> 9620a0a (SNOW-227101: fix to datetime columns being created without timezone (TIMESTAMP_NTZ).)``` |
ad9c66d
to
9620a0a
Compare
…TIMESTAMP_NTZ). Commit 28e8031 introduced a bug where all sqlalchemy.DateTime(timezone=True) columns were created as DATETIME in Snowflake which is an alias to TIMESTAMP_NTZ. This commit reverts datetime logic back so if the timezone is defined in datetime column it will create TIMESTAMP_TZ and TIMESTAMP_NTZ otherwise. (cherry picked from commit e180b23)
Original ticket.
Commit 28e8031 introduced a bug where all
sqlalchemy.DateTime(timezone=True)
columns were created asDATETIME
in Snowflake which is an alias toTIMESTAMP_NTZ
.This commit reverts datetime logic back so if the timezone is defined in datetime column it will create
TIMESTAMP_TZ
andTIMESTAMP_NTZ
otherwise.This bug was introduced in v1.1.18