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

refactor: deduplicate and abstract SQL schema building #9987

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

r1tsuu
Copy link
Member

@r1tsuu r1tsuu commented Dec 15, 2024

What?

Abstracts SQL schema building, significantly reducing code duplication for SQLite / Postgres

db-sqlite lines count From:

 wc -l **/*.ts
      62 src/connect.ts
      32 src/countDistinct.ts
       9 src/createJSONQuery/convertPathToJSONTraversal.ts
      86 src/createJSONQuery/index.ts
      15 src/defaultSnapshot.ts
       6 src/deleteWhere.ts
      21 src/dropDatabase.ts
      15 src/execute.ts
     178 src/index.ts
     139 src/init.ts
      19 src/insert.ts
      19 src/requireDrizzleKit.ts
     544 src/schema/build.ts
      27 src/schema/createIndex.ts
      38 src/schema/getIDColumn.ts
      13 src/schema/idToUUID.ts
      28 src/schema/setColumnID.ts
     787 src/schema/traverseFields.ts
      18 src/schema/withDefault.ts
     248 src/types.ts
    2304 total

To:

wc -l **/*.ts
      62 src/connect.ts
      32 src/countDistinct.ts
       9 src/createJSONQuery/convertPathToJSONTraversal.ts
      86 src/createJSONQuery/index.ts
      15 src/defaultSnapshot.ts
       6 src/deleteWhere.ts
      21 src/dropDatabase.ts
      15 src/execute.ts
     180 src/index.ts
      39 src/init.ts
      19 src/insert.ts
      19 src/requireDrizzleKit.ts
     149 src/schema/buildDrizzleTable.ts
      32 src/schema/setColumnID.ts
     258 src/types.ts
     942 total

Builds abstract schema in shared drizzle package that later gets converted by SQLite/ Postgres specific implementation into drizzle schema.
This, apparently can also help here #9953. It should be very trivial to implement a MySQL adapter with this as well.

@r1tsuu r1tsuu force-pushed the refactor/deduplicate-drizzle branch 2 times, most recently from 39c9fd8 to 1fd7da7 Compare December 15, 2024 11:41
Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

This is very exciting to me. I wanted to do this when we first built SQLite and opted to speed past it. I love that you cleaned up our tech debt here.

I have a few questions after reviewing from mobile.

packages/db-sqlite/src/init.ts Outdated Show resolved Hide resolved
packages/drizzle/src/schema/withDefault.ts Outdated Show resolved Hide resolved
packages/drizzle/src/types.ts Outdated Show resolved Hide resolved
packages/drizzle/src/types.ts Show resolved Hide resolved
…hema building, fix schema regression, buildDrizzleRelations build all, extract some columns to their types, jsdocs, rename isLocale to locale, refactor: db specfiic geometry default value sanitization
@r1tsuu r1tsuu force-pushed the refactor/deduplicate-drizzle branch from bd09358 to 827dc45 Compare December 16, 2024 05:56
Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

Great work!

@r1tsuu
Copy link
Member Author

r1tsuu commented Dec 16, 2024

I'll do some extra migrations regression testing just in case and then merge

@r1tsuu
Copy link
Member Author

r1tsuu commented Dec 16, 2024

With the website template when updated to 3.7.1-canary.2ce70a3 I didn't get schema changes.

@r1tsuu r1tsuu merged commit 727fba7 into main Dec 16, 2024
65 checks passed
@r1tsuu r1tsuu deleted the refactor/deduplicate-drizzle branch December 16, 2024 15:17
Copy link
Contributor

🚀 This is included in version v3.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants