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

Index fixes #405

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Index fixes #405

merged 3 commits into from
Dec 3, 2024

Conversation

LKajan
Copy link
Contributor

@LKajan LKajan commented Dec 2, 2024

  • Make ordering index composite with the parent. The ordering makes sense only within the parent.
  • Make short name nullable - In my understanding short names are "mandartory" only in regulation_groups of land_use_areas.
  • Make short names unique only within a plan, not globally

@LKajan LKajan requested a review from Rikuoja December 2, 2024 12:06
Order of just "ordering" column is not meaningful. Ordering takes always
place inside a parent feature which should always be included in th
queries.
Short names are unique only within the plan not globally.
unique: bool


ordering_index_by_table = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rikuoja, I don't quite like defining the expected indexes manually like this. Do you have ideas how to avoid this?

Copy link
Contributor

@Rikuoja Rikuoja left a comment

Choose a reason for hiding this comment

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

Maybe just hard-code the ordering index test with some table names instead, makes it more readable.

Index definition includes all the information about the index so
there's no need to test other columns from the pg_indexes table.
Copy link
Contributor

@Rikuoja Rikuoja left a comment

Choose a reason for hiding this comment

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

Looks good!

@LKajan LKajan merged commit ea8c5da into main Dec 3, 2024
2 checks passed
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.

2 participants