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.

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

feat: expose createMigrationBuilder to allow using this lib as sql builder #1285

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wenerme
Copy link
Contributor

@wenerme wenerme commented Oct 10, 2024

fix #1283

@wenerme wenerme requested a review from Shinigami92 as a code owner October 10, 2024 02:40
@wenerme wenerme changed the title feat: expose createMigrationBuilder to allow using this lib as sql bu… feat: expose createMigrationBuilder to allow using this lib as sql builder Oct 10, 2024
Copy link

github-actions bot commented Oct 10, 2024

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 92.08% (🎯 90%)
⬆️ +0.05%
3221 / 3498
🟢 Statements 92.08% (🎯 90%)
⬆️ +0.05%
3221 / 3498
🟢 Functions 95.55% (🎯 90%)
⬇️ -0.33%
258 / 270
🟢 Branches 90.48% (🎯 85%)
⬆️ +0.14%
827 / 914
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/index.ts 100% 100% 100% 100%
src/migrationBuilder.ts 95.07% 93.75% 83.33% 95.07% 354-358, 497-499, 501-502, 549-550, 552-553
Unchanged Files
src/db.ts 84.29% 89.28% 87.5% 84.29% 75-77, 107, 110-119, 121-122, 162-164
src/migration.ts 73.33% 86.79% 66.66% 73.33% 137-140, 161-163, 165-177, 216-222, 227-231, 234, 236-240, 242-249, 252, 255-260, 262-263, 320-321, 347-349, 357-358, 375-378, 407-408
src/runner.ts 75.43% 59.25% 90% 75.43% 45, 65-66, 75-76, 125-126, 168-171, 180-184, 197, 201-202, 204-206, 208-214, 233, 235-241, 244, 257, 269, 271-277, 279-282, 285-288, 298-299, 308-310, 319, 321, 323-330
src/sqlMigration.ts 90% 100% 80% 90% 45-46, 48-49
src/types.ts 100% 100% 100% 100%
src/operations/sql.ts 100% 100% 100% 100%
src/operations/casts/createCast.ts 100% 100% 100% 100%
src/operations/casts/dropCast.ts 100% 100% 100% 100%
src/operations/casts/index.ts 100% 100% 100% 100%
src/operations/domains/alterDomain.ts 100% 100% 100% 100%
src/operations/domains/createDomain.ts 100% 100% 100% 100%
src/operations/domains/dropDomain.ts 100% 100% 100% 100%
src/operations/domains/index.ts 100% 100% 100% 100%
src/operations/domains/renameDomain.ts 100% 100% 100% 100%
src/operations/domains/shared.ts 100% 100% 100% 100%
src/operations/extensions/createExtension.ts 100% 100% 100% 100%
src/operations/extensions/dropExtension.ts 100% 100% 100% 100%
src/operations/extensions/index.ts 100% 100% 100% 100%
src/operations/extensions/shared.ts 100% 100% 100% 100%
src/operations/functions/createFunction.ts 95.58% 94.44% 100% 95.58% 71-73
src/operations/functions/dropFunction.ts 100% 100% 100% 100%
src/operations/functions/index.ts 100% 100% 100% 100%
src/operations/functions/renameFunction.ts 100% 100% 100% 100%
src/operations/functions/shared.ts 100% 100% 100% 100%
src/operations/grants/grantOnSchemas.ts 100% 100% 100% 100%
src/operations/grants/grantOnTables.ts 100% 100% 100% 100%
src/operations/grants/grantRoles.ts 100% 100% 100% 100%
src/operations/grants/index.ts 100% 100% 100% 100%
src/operations/grants/revokeOnSchemas.ts 100% 100% 100% 100%
src/operations/grants/revokeOnTables.ts 100% 100% 100% 100%
src/operations/grants/revokeRoles.ts 100% 100% 100% 100%
src/operations/grants/shared.ts 95.65% 85.71% 100% 95.65% 71
src/operations/indexes/createIndex.ts 96.29% 95.23% 100% 96.29% 74-75
src/operations/indexes/dropIndex.ts 100% 100% 100% 100%
src/operations/indexes/index.ts 100% 100% 100% 100%
src/operations/indexes/shared.ts 88% 86.95% 100% 88% 22, 32-35, 47
src/operations/materializedViews/alterMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/createMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/dropMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/index.ts 100% 100% 100% 100%
src/operations/materializedViews/refreshMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/renameMaterializedView.ts 100% 100% 100% 100%
src/operations/materializedViews/renameMaterializedViewColumn.ts 100% 100% 100% 100%
src/operations/materializedViews/shared.ts 100% 87.5% 100% 100%
src/operations/operators/addToOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/createOperator.ts 100% 100% 100% 100%
src/operations/operators/createOperatorClass.ts 100% 83.33% 100% 100%
src/operations/operators/createOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/dropOperator.ts 100% 100% 100% 100%
src/operations/operators/dropOperatorClass.ts 100% 100% 100% 100%
src/operations/operators/dropOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/index.ts 100% 100% 100% 100%
src/operations/operators/removeFromOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/renameOperatorClass.ts 100% 100% 100% 100%
src/operations/operators/renameOperatorFamily.ts 100% 100% 100% 100%
src/operations/operators/shared.ts 85.71% 75% 100% 85.71% 24-25, 38
src/operations/policies/alterPolicy.ts 100% 100% 100% 100%
src/operations/policies/createPolicy.ts 100% 100% 100% 100%
src/operations/policies/dropPolicy.ts 100% 100% 100% 100%
src/operations/policies/index.ts 100% 100% 100% 100%
src/operations/policies/renamePolicy.ts 100% 100% 100% 100%
src/operations/policies/shared.ts 100% 100% 100% 100%
src/operations/roles/alterRole.ts 100% 100% 100% 100%
src/operations/roles/createRole.ts 100% 75% 100% 100%
src/operations/roles/dropRole.ts 100% 100% 100% 100%
src/operations/roles/index.ts 100% 100% 100% 100%
src/operations/roles/renameRole.ts 100% 100% 100% 100%
src/operations/roles/shared.ts 98.07% 76.92% 100% 98.07% 78
src/operations/schemas/createSchema.ts 100% 100% 100% 100%
src/operations/schemas/dropSchema.ts 100% 100% 100% 100%
src/operations/schemas/index.ts 100% 100% 100% 100%
src/operations/schemas/renameSchema.ts 100% 100% 100% 100%
src/operations/sequences/alterSequence.ts 94.11% 75% 100% 94.11% 23
src/operations/sequences/createSequence.ts 100% 100% 100% 100%
src/operations/sequences/dropSequence.ts 100% 100% 100% 100%
src/operations/sequences/index.ts 100% 100% 100% 100%
src/operations/sequences/renameSequence.ts 100% 100% 100% 100%
src/operations/sequences/shared.ts 78.57% 68.75% 100% 78.57% 41, 43-44, 49-50, 63-64, 69-70
src/operations/tables/addColumns.ts 100% 80% 100% 100%
src/operations/tables/addConstraint.ts 100% 100% 100% 100%
src/operations/tables/alterColumn.ts 88.57% 76.47% 100% 88.57% 75, 82-85, 87-89
src/operations/tables/alterTable.ts 100% 100% 100% 100%
src/operations/tables/createTable.ts 86.2% 66.66% 100% 86.2% 44-48, 65, 75-76
src/operations/tables/dropColumns.ts 100% 100% 100% 100%
src/operations/tables/dropConstraint.ts 100% 100% 100% 100%
src/operations/tables/dropTable.ts 100% 100% 100% 100%
src/operations/tables/index.ts 100% 100% 100% 100%
src/operations/tables/renameColumn.ts 100% 100% 100% 100%
src/operations/tables/renameConstraint.ts 100% 100% 100% 100%
src/operations/tables/renameTable.ts 100% 100% 100% 100%
src/operations/tables/shared.ts 83.03% 80.3% 80% 83.03% 141-142, 195-199, 224, 228-229, 248-249, 274-275, 278-285, 288-289, 426-431, 433-436, 438-448, 450-451
src/operations/triggers/createTrigger.ts 86.41% 68.18% 100% 86.41% 53-54, 66-67, 70-71, 74-77, 113
src/operations/triggers/dropTrigger.ts 100% 100% 100% 100%
src/operations/triggers/index.ts 100% 100% 100% 100%
src/operations/triggers/renameTrigger.ts 100% 100% 100% 100%
src/operations/triggers/shared.ts 100% 100% 100% 100%
src/operations/types/addTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/addTypeValue.ts 100% 100% 100% 100%
src/operations/types/createType.ts 100% 100% 100% 100%
src/operations/types/dropType.ts 100% 100% 100% 100%
src/operations/types/dropTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/index.ts 100% 100% 100% 100%
src/operations/types/renameType.ts 100% 100% 100% 100%
src/operations/types/renameTypeAttribute.ts 100% 100% 100% 100%
src/operations/types/renameTypeValue.ts 100% 100% 100% 100%
src/operations/types/setTypeAttribute.ts 100% 100% 100% 100%
src/operations/views/alterView.ts 100% 100% 100% 100%
src/operations/views/alterViewColumn.ts 100% 100% 100% 100%
src/operations/views/createView.ts 100% 100% 100% 100%
src/operations/views/dropView.ts 100% 100% 100% 100%
src/operations/views/index.ts 100% 100% 100% 100%
src/operations/views/renameView.ts 100% 100% 100% 100%
src/operations/views/shared.ts 100% 66.66% 100% 100%
src/utils/PgLiteral.ts 88.88% 100% 75% 88.88% 37-38
src/utils/StringIdGenerator.ts 100% 100% 100% 100%
src/utils/createSchemalize.ts 100% 100% 100% 100%
src/utils/createTransformer.ts 100% 100% 100% 100%
src/utils/decamelize.ts 100% 100% 100% 100%
src/utils/escapeValue.ts 100% 100% 100% 100%
src/utils/formatLines.ts 100% 100% 100% 100%
src/utils/formatParams.ts 100% 100% 100% 100%
src/utils/getMigrationTableSchema.ts 100% 100% 100% 100%
src/utils/getSchemas.ts 100% 100% 100% 100%
src/utils/identity.ts 100% 100% 100% 100%
src/utils/index.ts 100% 100% 100% 100%
src/utils/intersection.ts 100% 100% 100% 100%
src/utils/makeComment.ts 100% 100% 100% 100%
src/utils/quote.ts 100% 100% 100% 100%
src/utils/toArray.ts 100% 100% 100% 100%
src/utils/types.ts 100% 100% 100% 100%
Generated in workflow #1312 for commit b9a601d by the Vitest Coverage Report Action

@wenerme wenerme force-pushed the features/expose-builder branch from 25573f2 to 9df3929 Compare October 10, 2024 03:53
@Shinigami92 Shinigami92 added c: feature Request for new feature p: 1-normal Nothing urgent labels Oct 13, 2024
@Shinigami92 Shinigami92 added this to the v7.x milestone Oct 13, 2024
@Shinigami92
Copy link
Collaborator

Sorry for the late response, but life... you know

What is the purpose of this PR? You wrote about not having file accesses. Potentially we need to go another route and provide an option to not write files and just output to stdio for example 🤔

Also we definitely need tests...

@wenerme
Copy link
Contributor Author

wenerme commented Nov 5, 2024

I want to use node-pg-migrate as dsl builder instead migration tool

  1. I want to get the sql and provide the sql to knex/kysely, let them execute the migration
  2. I want to get the final sql as a reference, instead of the dsl in code

@wenerme wenerme force-pushed the features/expose-builder branch from 9df3929 to b9a601d Compare November 5, 2024 08:01
Copy link
Collaborator

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I cannot accept this PR right now, because it exposes something fully new I personally don't want to maintain in the long run. Also it would throw errors on a second run when e.g. a select/query would get executed, like calling for a lock.

We would need to implement it in a totally different like adding a new option parameter to RunnerOption and then use the logger to do something like a dry-run.

export async function runner(options: RunnerOption): Promise<RunMigration[]> {

runner is getting also called from the CLI, so that would also then be a new option to the CLI.

@wenerme
Copy link
Contributor Author

wenerme commented Nov 12, 2024

Would it make sense to expose this under unstable_createMigrationBuilder and return MigrationBuilder instead of MigrationBuilderImpl for now? If a dry-run mode is implemented for the runner, the builder could later incorporate additional methods for SQL generation. Feel free to close this issue if you feel the timing isn’t right.

@Shinigami92
Copy link
Collaborator

I don't feel safe with that approach. Even unstable would be used and required some day and depend on.
I personally don't like to expose MigrationBuilderImpl because it could be renamed to MigrationBuilder (like the interface right now) in a later major and merged with it's interface.

What I could suggest right now as a quick solution for you:
try out patch-package and patch node-pg-migrate by exposing MigrationBuilderImpl and then write the createMigrationBuilder in your own codebase.

I know this is an unsatisfying solution, but it is a solution 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose migration builder impl, make node-pg-migrate can used as a schema builder library
2 participants