Skip to content

Add support for column comments - #306

Merged
susodapop merged 5 commits into
databricks:sqlalchemy-stagingfrom
cbornet:column_comment
Jan 23, 2024
Merged

Add support for column comments#306
susodapop merged 5 commits into
databricks:sqlalchemy-stagingfrom
cbornet:column_comment

Conversation

@cbornet

@cbornet cbornet commented Dec 22, 2023

Copy link
Copy Markdown

No description provided.

Signed-off-by: Christophe Bornet <cbornet@hotmail.com>
Signed-off-by: Christophe Bornet <cbornet@hotmail.com>
@cbornet

cbornet commented Jan 8, 2024

Copy link
Copy Markdown
Author

Hi @susodapop, will you have time to review this soon ?

@susodapop

Copy link
Copy Markdown
Contributor

Yep! Just getting back up to speed following vacation time away.

@cbornet

cbornet commented Jan 9, 2024

Copy link
Copy Markdown
Author

Awesome, thanks !

@susodapop susodapop left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread 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.

Copy link
Copy Markdown
Contributor

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

Comment thread src/databricks/sqlalchemy/_ddl.py
Comment thread src/databricks/sqlalchemy/_parse.py
Jesse Whitehouse added 2 commits January 23, 2024 14:56
…ressions

Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop
susodapop changed the base branch from main to sqlalchemy-staging January 23, 2024 20:27
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
@susodapop

Copy link
Copy Markdown
Contributor

I've changed the base branch for this PR to a sqlalchemy-staging branch so that I can test these changes alongside #308 and update the changelog in one fell swoop.

@susodapop
susodapop merged commit ae37007 into databricks:sqlalchemy-staging Jan 23, 2024
"type": final_col_type,
"nullable": bool(thrift_resp_row.NULLABLE),
"default": thrift_resp_row.COLUMN_DEF,
"comment": thrift_resp_row.REMARKS,

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants