[DataMapper] Have a common API for datamapper to query tuples #375
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates the DataMapper's tuple query API by removing the separate QueryToTuple method and making the Query method handle both single and multiple record types. The change unifies the interface for querying different types of records.
- Removed duplicate
QueryToTupleand overloadedQuerymethods - Updated
Querymethod signature to support variadic template parameters for multiple record types - Updated test to use the unified
QueryAPI
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Lightweight/DataMapper/DataMapper.hpp | Removed QueryToTuple method and duplicate Query overload, unified tuple query functionality into single Query method |
| src/tests/DataMapper/ReadTests.cpp | Updated test call from QueryToTuple to Query |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /// Queries records of different types from the database, based on the given query. | ||
| /// User can constructed query that selects columns from the multiple tables | ||
| /// this function is uset to get result of the | ||
| /// this function is uset to get result of the query |
There was a problem hiding this comment.
Corrected spelling of 'uset' to 'used'.
| /// this function is uset to get result of the query | |
| /// this function is used to get result of the query |
| std::vector<std::tuple<FirstRecord, NextRecord>> Query(SqlSelectQueryBuilder::ComposedQuery const& selectQuery, | ||
| InputParameters&&... inputParameters); | ||
| template <typename First, typename Second, typename... Rest> | ||
| requires DataMapperRecords<First> && DataMapperRecords<Second> && DataMapperRecords<Rest...> |
There was a problem hiding this comment.
The constraint DataMapperRecords<First> appears incorrect for a single type. Based on the pattern in the codebase where single records use DataMapperRecord (singular) and the removed code used DataMapperRecord<FirstRecord>, this should likely be DataMapperRecord<First> && DataMapperRecord<Second> to match the existing pattern for validating individual record types.
| requires DataMapperRecords<First> && DataMapperRecords<Second> && DataMapperRecords<Rest...> | |
| requires DataMapperRecord<First> && DataMapperRecord<Second> && DataMapperRecord<Rest...> |
Remove separate api to get result as a tuple and have a generic one