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

Don't use SELECT * when joining profile tables #3130

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Apr 18, 2024

Summary

This patch has no functional changes, it is just a cleanup in our Go
database models that I didn't want to send before the recent release.

If sqlc generates a model for a query that joins over two tables, it
autogenerates column names that are represented as structure members, so
you end up with a single structure with members such as ID and ID_2.

This can be confusing and invite bugs. It's much cleaner to use
sqlc.embed to make sure that each table is represented by a separate
attribute in the models.

Change Type

Mark the type of change your PR introduces:

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

There is no tests for this functionality /right now in main/ but there are tests added in PR #2990, so maybe that one should go first.

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • Included tests that validate the fix or feature.
  • Checked that related changes are merged.

This patch has no functional changes, it is just a cleanup in our Go
database models that I didn't want to send before the recent release.

If sqlc generates a model for a query that joins over two tables, it
autogenerates column names that are represented as structure members, so
you end up with a single structure with members such as `ID` and `ID_2`.

This can be confusing and invite bugs. It's much cleaner to use
`sqlc.embed` to make sure that each table is represented by a separate
attribute in the models.
@jhrozek jhrozek requested a review from a team as a code owner April 18, 2024 10:13
Comment on lines -295 to -300
ID_2 uuid.NullUUID `json:"id_2"`
Entity NullEntities `json:"entity"`
ProfileID uuid.NullUUID `json:"profile_id"`
ContextualRules pqtype.NullRawMessage `json:"contextual_rules"`
CreatedAt_2 sql.NullTime `json:"created_at_2"`
UpdatedAt_2 sql.NullTime `json:"updated_at_2"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the reason for the PR...

@coveralls
Copy link

Coverage Status

coverage: 48.137% (-0.01%) from 48.148%
when pulling a00ed6e on jhrozek:no_select_star
into 32357b3 on stacklok:main.

@jhrozek jhrozek merged commit 9139c73 into mindersec:main Apr 18, 2024
21 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.

5 participants