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

when the number of struct members is less than the number of columns in the table #74

Closed
Archercober opened this issue Apr 24, 2024 · 6 comments
Assignees
Labels

Comments

@Archercober
Copy link

Archercober commented Apr 24, 2024

For example

// an aggregate
struct user
{
   std::string name;
};

template<>
inline constexpr bool tao::pq::is_aggregate< user > = true;

int main()
{
   // open a connection to the database
   const auto conn = tao::pq::connection::create( "dbname=template1" );

   // execute statements
   conn->execute( "DROP TABLE IF EXISTS users" );
   conn->execute( "CREATE TABLE users ( name TEXT PRIMARY KEY, age INTEGER NOT NULL, planet TEXT NOT NULL )" );

   // execute previously prepared statements
   conn->execute( "insert_user", user{ "R. Daneel Olivaw", 19230, "Earth" } );

   // query and convert data even user only has on field
   for( const user u : conn->execute( "SELECT * FROM users" ) ) {
      std::cout << u.name;
   }
}

Is it possible to convert to a struct with fewer members than the number of table columns?

@d-frey d-frey self-assigned this Apr 24, 2024
@d-frey d-frey added the bug label Apr 24, 2024
@d-frey
Copy link
Member

d-frey commented Apr 24, 2024

This seems to be a bug, it works when you add a second member to user. The bug seems to occur when there is only a single member to bind to. I'll investigate when I have some time...

@d-frey
Copy link
Member

d-frey commented Apr 24, 2024

Note that I consider it to be really bad practice to use SELECT * FROM ... as this is just begging for code to break in the future when the DB schema is modified. Always list the columns (and their order) explicitly in your SELECT statements. Anyway, that is unrelated to the bug.

@d-frey
Copy link
Member

d-frey commented Apr 24, 2024

I was thinking about this more and I think there are two separate questions here:

  1. Why is an aggregate with only one value behaving differently?
  2. Do we want to convert from a result to a struct with more values than needed?

I think for question 2 the answer should be "No.", as this would usually mean that something is wrong or at least inefficient, and it is also inconsistent with how C++ is defined in other cases. Consider:

struct user
{
   std::string name;
   int age;
};

user u{ "jim", 42, "new york" };

leads to a compiler error too many initializers for 'user' (GCC) or excess elements in struct initializer(Clang).

Likewise, the following code also fails:

std::tuple t{ "jim", 42, "new york" };
const auto& [ name, age ] = t;

with the error given by Clang: type 'std::tuple<...>' decomposes into 3 elements, but only 2 names were provided and GCC providing a similar but longer error message.

That said, both cases show that you want to have the correct amount of columns to prevent accidental mismatched columns. I still need to figure out how to fix the code for this (and question 1)...

@d-frey d-frey closed this as completed in 44608b9 Apr 24, 2024
@d-frey
Copy link
Member

d-frey commented Apr 24, 2024

I fixed the single member aggregate case, but I still wonder if the code should check (obviously at runtime) for excess result columns as explained in my previous comment...

@d-frey
Copy link
Member

d-frey commented Apr 26, 2024

I think my testcase was broken, as the code already contains a check for the correct number of columns, so from my perspective everything works as expected now.

@d-frey
Copy link
Member

d-frey commented Apr 26, 2024

Found out what the problem with my original test was, I mixed up table names and never actually tested with data, hence the check in the code was never triggered. If there is data, the check was always there, so it would always catch the excess columns at runtime. The only bug was the compile-time problem for single member aggregates you found and that is fixed now. Case closed, thank you for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants