fix(sql): PostgreSQL compatibility for executions and partitions #3812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robzienert
approved these changes
Jul 10, 2020
| @@ -14,6 +14,6 @@ databaseChangeLog: | |||
| nullable: false | |||
| - column: | |||
| name: expiry | |||
| type: bigint(13) | |||
| type: bigint | |||
There was a problem hiding this comment.
Won't validCheckSum be required here as well?
There was a problem hiding this comment.
Indeed it would be - I've added the original checksum to the migration.
Comment on lines
-8
to
+36
| sql: CREATE TABLE `deleted_executions` (id INT AUTO_INCREMENT, execution_id char(26) NOT NULL, execution_type ENUM("pipeline", "orchestration") NOT NULL, deleted_at DATETIME NOT NULL, CONSTRAINT deleted_executions_pk PRIMARY KEY (id)) | ||
| sql: ALTER TABLE `deleted_executions` MODIFY COLUMN `execution_type` ENUM("pipeline", "orchestration") NOT NULL |
nicolasff
suggested changes
Jul 13, 2020
| .run { | ||
| when (jooq.dialect()) { | ||
| SQLDialect.POSTGRES -> { | ||
| onConflict(DSL.field("id")) |
There was a problem hiding this comment.
This should be lock_name, not id which is not a field in this table. lock_name is the primary key.
There was a problem hiding this comment.
This is fixed, thank you.
Includes the following changes: - Removing bigint display qualifiers - these are MySQL specific, and display-only - Avoid using `forceIndex` on PostgreSQL - it does not support that feature - Remove MySQL-specific column backticks for `partition`, and replace with `DSL.name` throughout - A number of queries used MySQL-specific upsert features. These have been expanded with PostgreSQL-compatible varieties. - Add new PostgreSQL module with runtimeOnly driver dependency - Add PostgreSQL tests for orca-sql
3e21970 to
b5f5d12
Compare
|
@robzienert Thanks for the review! This is still dependent on spinnaker/keiko#176 - would it be possible to get eyes on that sometime as well? |
marchello2000
approved these changes
Jul 14, 2020
|
merged keiko, it should produce a release https://github.com/spinnaker/keiko/releases/tag/v3.8.0 |
|
Thanks @marchello2000 ! I'll wait for the next Kork release, and then this PR should (hopefully) test correctly. |
KathrynLewis
pushed a commit
to KathrynLewis/orca
that referenced
this pull request
Jan 31, 2021
…nnaker#3812) * fix(sql): PostgreSQL compatibility for executions and partitions Includes the following changes: - Removing bigint display qualifiers - these are MySQL specific, and display-only - Avoid using `forceIndex` on PostgreSQL - it does not support that feature - Remove MySQL-specific column backticks for `partition`, and replace with `DSL.name` throughout - A number of queries used MySQL-specific upsert features. These have been expanded with PostgreSQL-compatible varieties. - Add new PostgreSQL module with runtimeOnly driver dependency - Add PostgreSQL tests for orca-sql * Update kork/keiko pins to include required dependency changes Co-authored-by: Adam Jordens <adam@jordens.org> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes the following changes:
forceIndexon PostgreSQL - it does not support that featurepartition, and replace withDSL.namethroughoutDepends on:
spinnaker/kork#716
spinnaker/keiko#176