Skip to content

feat: Remove pk index naming from postgres destination#20438

Merged
kodiakhq[bot] merged 6 commits intomainfrom
feature/eng-694-feat-remove-suffix-from-the-primary-key-in-the-postgres
Mar 27, 2025
Merged

feat: Remove pk index naming from postgres destination#20438
kodiakhq[bot] merged 6 commits intomainfrom
feature/eng-694-feat-remove-suffix-from-the-primary-key-in-the-postgres

Conversation

@przste-go
Copy link
Copy Markdown

@przste-go przste-go commented Mar 25, 2025

Summary

⚠️ If you're contributing to a plugin please read this section of the contribution guidelines 🧑‍🎓 before submitting this PR ⚠️

This PR aims to solve issues with long table names in Postgresql database. All identifiers cannot be longer than 63 characters limit. Due to custom naming scheme of primary keys if table had more than 58 characters adding _cqpk caused DDLs to fail. To prevent that 2 things have been changed:

  1. Primary keys no longer use _cqpk instead we fallback to default postgres naming scheme of _pkey which gets auto-incremented in case of conflicting name. This change is backwards compatible as it's part of create if not exists DDL.
  2. If table name is longer than 58 characters - name is being hashed when creating perfomance index _cqpi. This change is also backwards compatible as table names longer than 58 characters couldn't be created due to _cqpk primary key identifier preventing DDL statements to execute.

@cq-bot
Copy link
Copy Markdown
Contributor

cq-bot commented Mar 25, 2025

@przste-go przste-go marked this pull request as ready for review March 26, 2025 08:06
@przste-go przste-go requested review from a team and maaarcelino March 26, 2025 08:06
Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests on migration existing tables with the old naming convention?

@przste-go
Copy link
Copy Markdown
Author

Can we add some tests on migration existing tables with the old naming convention?

Sure, I think that can be done.

@przste-go
Copy link
Copy Markdown
Author

@erezrokah Added the test checking if old index names are untouched after migration.

Comment on lines +412 to +415
err := c.conn.QueryRow(ctx, `select tco.constraint_name from information_schema.table_constraints tco join information_schema.key_column_usage kcu
on kcu.constraint_name = tco.constraint_name
where tco.constraint_type = 'PRIMARY KEY'
and kcu.table_name = $1`, tableName).Scan(&pkConstraintName)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a nitpick but since the other queries are uppercased, let's uppercase this one as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 🚀

@murarustefaan murarustefaan requested a review from erezrokah March 27, 2025 10:10
@przste-go przste-go added the automerge Automatically merge once required checks pass label Mar 27, 2025
@kodiakhq kodiakhq bot merged commit 5314c7a into main Mar 27, 2025
16 checks passed
@kodiakhq kodiakhq bot deleted the feature/eng-694-feat-remove-suffix-from-the-primary-key-in-the-postgres branch March 27, 2025 10:54
kodiakhq bot pushed a commit that referenced this pull request Mar 27, 2025
🤖 I have created a release *beep* *boop*
---


## [8.8.0](plugins-destination-postgresql-v8.7.12...plugins-destination-postgresql-v8.8.0) (2025-03-27)


### Features

* Remove pk index naming from postgres destination ([#20438](#20438)) ([5314c7a](5314c7a))


### Bug Fixes

* **deps:** Update module github.com/apache/arrow-go/v18 to v18.2.0 ([#20410](#20410)) ([ee081fb](ee081fb))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.74.2 ([#20434](#20434)) ([8db20d6](8db20d6))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/plugin/destination/postgresql automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants