Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Database returned by CreateDatabase() does not contain a set database_dialect #8573

Closed
devbww opened this issue Mar 21, 2022 · 1 comment · Fixed by #9549
Closed

Database returned by CreateDatabase() does not contain a set database_dialect #8573

devbww opened this issue Mar 21, 2022 · 1 comment · Fixed by #9549
Assignees
@devbww
Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@devbww
Copy link
Contributor

devbww commented Mar 21, 2022

The "Client Library Spec for Dialect Aware Database" doc says, "Check that a database created with the DATABASE_DIALECT_UNSPECIFIED dialect has the dialect set as GOOGLE_STANDARD_SQL in the creation response, in GetDatabase, in ListDatabases and in RestoreDatabase."

My testing indicates that the Database message returned by CreateDatabase() does not contain a set GOOGLE_STANDARD_SQL database_dialect field when the CreateDatabaseRequest has an unset or explicit DATABASE_DIALECT_UNSPECIFIED database_dialect field.

Similarly, the Database message returned by RestoreDatabase() does not contain a set GOOGLE_STANDARD_SQL database_dialect field when the source database was created with an unset or explicit DATABASE_DIALECT_UNSPECIFIED database_dialect field.

GetDatabase() and ListDatabases() do report GOOGLE_STANDARD_SQL for such a database, and its restoration.

I think I agree with the doc's idea of how this should work, and in which case I'd say its a bug.
@devbww devbww added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. api: spanner Issues related to the Spanner API. labels Mar 21, 2022
@devbww devbww self-assigned this Mar 21, 2022
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Mar 23, 2022
@devbww
feat(spanner): integration tests for Database.database_dialect
57ece34 
Extend existing tests to also cover `database_dialect`by setting it
in the CreateDatabaseRequest, and expecting it in Database and Backup
responses.  Include allowances for these being invisible to the Spanner
emulator, and for pending bug googleapis#8573.

In particular, BackupIntegrationTest.BackupRestore leaves the dialect
unspecified in CreateDatabase() (but expects to see GOOGLE_STANDARD_SQL
in responses), while it is set explicitly to GOOGLE_STANDARD_SQL in
DatabaseAdminClientTest.VersionRetentionPeriodCreate, and to POSTGRESQL
in BackupExtraIntegrationTest.BackupRestoreWithCMEK.

DatabaseAdminClientTest.DatabasePostgreSQLBasics is a new test case
that also exercises the non-backup parts of an explicit POSTGRESQL
setting.

[Aside: These are only in the tests for the generated database admin
client.]

Finally, add ClientIntegrationTest.DatabaseDialect to enumerate the
`database_dialect` value from `INFORMATION_SCHEMA.DATABASE_OPTIONS`
for a default-dialect database.
@devbww devbww mentioned this issue Mar 23, 2022
Merged
devbww added a commit that referenced this issue Mar 24, 2022
@devbww
feat(spanner): integration tests for Database.database_dialect (#8589)
48d0da1 
Extend existing tests to also cover `database_dialect`by setting it
in the CreateDatabaseRequest, and expecting it in Database and Backup
responses.  Include allowances for these being invisible to the Spanner
emulator, and for pending bug #8573.

In particular, BackupIntegrationTest.BackupRestore leaves the dialect
unspecified in CreateDatabase() (but expects to see GOOGLE_STANDARD_SQL
in responses), while it is set explicitly to GOOGLE_STANDARD_SQL in
DatabaseAdminClientTest.VersionRetentionPeriodCreate, and to POSTGRESQL
in BackupExtraIntegrationTest.BackupRestoreWithCMEK.

DatabaseAdminClientTest.DatabasePostgreSQLBasics is a new test case
that also exercises the non-backup parts of an explicit POSTGRESQL
setting.

[Aside: These are only in the tests for the generated database admin
client.]

Finally, add ClientIntegrationTest.DatabaseDialect to enumerate the
`database_dialect` value from `INFORMATION_SCHEMA.DATABASE_OPTIONS`
for a default-dialect database.
devbww added a commit to devbww/google-cloud-cpp that referenced this issue May 20, 2022
@devbww
test(spanner): remove database_dialect workaround for CreateDatabase
7bf6da6 
Setting of `database_dialect` in the `CreateDatabase()` result has
been fixed.  `RestoreDatabase()`, however, remains an issue.

Part of googleapis#8573.
@devbww devbww mentioned this issue May 20, 2022
Merged
devbww added a commit that referenced this issue May 20, 2022
@devbww
test(spanner): remove database_dialect workaround for CreateDatabase (#…
087bd01 
…9029)

Setting of `database_dialect` in the `CreateDatabase()` result has
been fixed.  `RestoreDatabase()`, however, remains an issue.

Part of #8573.
@devbww
Copy link
Contributor Author

devbww commented May 20, 2022

Setting of database_dialect in the CreateDatabase() response has been fixed. RestoreDatabase() remains an issue.

devbww added a commit to devbww/google-cloud-cpp that referenced this issue Jul 21, 2022
@devbww
test(spanner): RestoreDatabase() now sets database_dialect
f5fc8cb 
Fixes googleapis#8573.
@devbww devbww mentioned this issue Jul 21, 2022
Merged
devbww added a commit that referenced this issue Jul 22, 2022
@devbww
test(spanner): RestoreDatabase() now sets database_dialect (#9549)
c03a1e9 
Fixes #8573.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@devbww devbww

Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(spanner): RestoreDatabase() now sets database_dialect devbww/google-cloud-cpp
1 participant
@devbww