googleapis / python-spanner-sqlalchemy

Apache License 2.0
39 stars 28 forks source link

fix: incorrect DDL generated when using server_default (#209) #220

Closed trevor-pope closed 2 years ago

trevor-pope commented 2 years ago

Added parenthesis to enclose DEFAULT values of CREATE TABLE statements, and changed the order of NOT NULL and DEFAULT statements to match the specifications of Spanner.

Fixes #209

Could also probably add some tests for server_default.

I also realized that the intended way is to use sqlalchemy.text() to render default values, so the other problem mentioned at the bottom of #209 is not true.

google-cla[bot] commented 2 years ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

IlyaFaer commented 2 years ago

@asthamohta, okay, I've fixed everything here.

It appears now linter is checking long_description of the package. In the main python-spanner repository the README file is used as a long description.

Also it requires to use .rst format README file instead of GitHub default .md (otherwise list_setup_py will still fail). So, I've converted the README file as well.

Let's merge the PR, and I will add tests for it separately (not very convenient to work in client's fork).