taocpp / taopq

C++ client library for PostgreSQL
Boost Software License 1.0
264 stars 40 forks source link

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

Closed Archercober closed 6 months ago

Archercober commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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!