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

db: Rework index config with generated index names #10589

Merged
merged 8 commits into from
Mar 28, 2024

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented Mar 27, 2024

Changes

This introduces generated index names when passing an array to indexes. The object configuration syntax is now deprecated. See changelog

  • Update dbConfig with a transformation step to generate index names by table and column names
  • Introduce a ResolvedDbConfig type for migration queries
  • Misc: add a mapObject utility to remap object values with type safety

Testing

Add tests for new array format, and move old tests to "legacy"

Docs

withastro/docs#7639

Copy link

changeset-bot bot commented Mar 27, 2024

🦋 Changeset detected

Latest commit: 064d001

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@bholmesdev
Copy link
Contributor Author

Kept to a patch since the old config object is deprecated, but still works! Made a ticket to remove in the next minor.

@@ -0,0 +1,31 @@
---
"@astrojs/db": patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, this is non-breaking, the old syntax is still supported for now, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen to people's existing indices? Will they be dropped and recreated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old syntax is supported! And it shouldn't drop and recreate even when refactoring to the new syntax (unless you switch to using generated names of course). We handle everything with a Zod transform that matches the original object format. I'll add a unit test to confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess what i'm asking is, currently the user can name them whatever they want. For example foobar_index. But now with the automated index names, is it not going to try to add the new index and not know about the previous index?

I guess you're saying that when you do push, it will see the old index and the new index and drop the old one (assuming it doesn't match) and create a new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: confirmed there are no migrations when upgrading to the new syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthewp Yep, it will drop and recreate the indexes if you don't pass a name option. If you pass a name to the new indexes that matches the old name, it'll run without any migrations 👍

@bholmesdev bholmesdev merged commit ed1031b into main Mar 28, 2024
13 checks passed
@bholmesdev bholmesdev deleted the feat/indexes-config-rework branch March 28, 2024 18:09
@astrobot-houston astrobot-houston mentioned this pull request Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants