Skip to content

Select up to 10 #2553

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Select up to 10 #2553

wants to merge 2 commits into from

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 self-assigned this Apr 1, 2025
@billy1624 billy1624 requested a review from Copilot April 1, 2025 13:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces the “Select up to 10” feature by refactoring and expanding the existing selection types. Key changes include:

  • Defining a macro (def_select_struct!) to reduce repetitive struct definitions for selections from two up to ten models.
  • Replacing individual trait implementations with macros (impl_query_trait!, impl_selector_trait!, etc.) across multiple modules.
  • Updating join, combine, executor, paginator, and cursor modules to support selection operations on up to ten entities.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/query/select.rs Refactored select struct definitions and trait implementations using macros.
src/query/mod.rs Updated re-exports in the combine module to include new selection types.
src/query/join.rs Modified related join implementations to use a new macro with updated documentation.
src/query/combine.rs Replaced individual impls with macros for selecting additional entity columns.
src/executor/select.rs Introduced macros for model conversion and JSON conversion supporting new selection types.
src/executor/paginator.rs Updated paginator trait implementations via macros to support up to ten models.
src/executor/cursor.rs Refactored cursor trait implementations using macros with adjustments for multi-entity support.
Comments suppressed due to low confidence (2)

src/query/join.rs:165

  • [nitpick] The doc comments for find_also_related (and its counterpart) use terms like 'second last' and 'last' which may be ambiguous. Consider clarifying these descriptions to more explicitly indicate which related entity is being joined.
pub fn find_also_related<R>(self, r: R) -> crate::$next_struct<$($generics),*, R>

src/query/combine.rs:119

  • Using placeholders ('_') for type parameters in the prepare_select_col call might reduce clarity and could potentially lead to type inference issues. Consider providing explicit type parameters to ensure clearer code maintainability.
prepare_select_col::<$last, _, _>(&mut self, $col_prefix);

{
let mut cursor = Cursor::new(
self.query,
SeaRc::new($first_select_generics::default()),
Copy link
Preview

Copilot AI Apr 1, 2025

Choose a reason for hiding this comment

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

The macro calls default() on the entity type parameter without an explicit Default bound. Ensure that all entity types passed to this macro implement Default to avoid potential runtime issues.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@billy1624 billy1624 requested a review from tyt2y3 April 1, 2025 13:32
@guymave
Copy link

guymave commented Apr 9, 2025

yesssssss

@edmiester777
Copy link

Let's get it! Should this be draft though?

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.

SelectThree up to SelectNine
3 participants