-
Notifications
You must be signed in to change notification settings - Fork 110
Add support for column comments #306
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
Add support for column comments #306
Conversation
Signed-off-by: Christophe Bornet <[email protected]>
76ece75
to
7537b93
Compare
Signed-off-by: Christophe Bornet <[email protected]>
7537b93
to
bf2f18b
Compare
Hi @susodapop, will you have time to review this soon ? |
Yep! Just getting back up to speed following vacation time away. |
Awesome, thanks ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution! Looks great. I'm going to push a few test additions. Plus we need an entry in the changelog.
src/databricks/sqlalchemy/_ddl.py
Outdated
autoincrement=True on a column. See comments in test_suite.py. We may implement implicit | ||
IDENTITY using this feature in the future, similar to the Microsoft SQL Server dialect. | ||
""" | ||
# Emit a log message if a user attempts to set autoincrement=True on a column. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this as a docstring, since regular comments aren't surfaced in a code editor
…ressions Signed-off-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
I've changed the base branch for this PR to a |
@@ -354,6 +354,7 @@ def parse_column_info_from_tgetcolumnsresponse(thrift_resp_row) -> ReflectedColu | |||
"type": final_col_type, | |||
"nullable": bool(thrift_resp_row.NULLABLE), | |||
"default": thrift_resp_row.COLUMN_DEF, | |||
"comment": thrift_resp_row.REMARKS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to pass the ComponentReflectionTest
we need to update this line to
"comment": thrift_resp_row.REMARKS or None
No description provided.