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

Array types with other containers #769

Closed
alexolog opened this issue Dec 15, 2023 · 6 comments
Closed

Array types with other containers #769

alexolog opened this issue Dec 15, 2023 · 6 comments

Comments

@alexolog
Copy link

alexolog commented Dec 15, 2023

We are trying use a Postgres array with an std::span, by modelling it on the following code in include/pqxx/internal/conversions.hxx:

template<typename T, typename... Args>
struct string_traits<std::vector<T, Args...>>
        : internal::array_string_traits<std::vector<T, Args...>>
{};

Unfortunately, since array_string_traits is in the internal namespace, we ended up doing something like this:

template<typename T>
struct string_traits<std::span<T>> : internal::array_string_traits<std::span<T>>
{};

Which makes our code dependent on internal implementation.

Is there another, recommended way of doing it?

If not, please consider moving the template out of the internal namespace.

@jtv
Copy link
Owner

jtv commented Dec 17, 2023

Actually, that looks like it should just be in libpqxx as standard! (At least, for C++20 or better.)

Here's the solution I would propose:

  • You submit your workaround as a PR.
  • (Of course don't forget to write a one-line entry in NEWS, with a suffix o f(#769) to link it to this ticket.)
  • You apply your workaround in your own application, with a note to remove it once it's no longer needed.
  • I merge your PR, so you are properly credited in the git changelog.
  • Once the patch shows up in your next libpqxx upgrade, the definitions are identical so the compiler shouldn't mind the duplication.
  • At some point you remove the workaround code from your application.

Does that look good?

One painful note: array_string_traits supports conversion to a string, but not parsing an array from a string. That's because the parser needs to be aware of the connection's client encoding. And that's something that the existing conversion API cannot provide — it was designed to support conversion even when there's no connection. (There's no llibpqxx representation of an encoding by itself, by deliberate choice.) If you need to parse arrays you receive from the database, use pqxx::array_parser.

@alexolog
Copy link
Author

The actual work was done by my coworker and I wouldn't want to misappropriate it. I will ask him if he's willing to do the PR.

@jtv
Copy link
Owner

jtv commented Feb 10, 2024

@alexolog It's merged. Sorry it took so long. In theory that takes care of the blockers for a 7.9.0 release.

Now we give it a little bit of time for people to report problems with the latest changes; deal with those; and... release.

@jtv
Copy link
Owner

jtv commented Feb 18, 2024

@alexolog, @fallenworld1 — I've released 7.9.0 with your work in it. Will close this ticket soon.

If you try it and find a problem, by all means file a new ticket!

@alexolog
Copy link
Author

One painful note: array_string_traits supports conversion to a string, but not parsing an array from a string. That's because the parser needs to be aware of the connection's client encoding. And that's something that the existing conversion API cannot provide — it was designed to support conversion even when there's no connection. (There's no llibpqxx representation of an encoding by itself, by deliberate choice.) If you need to parse arrays you receive from the database, use pqxx::array_parser.

Why not allow the operation with the caveat that either the encoding will have to be explicitly specified by the user or no encoding conversion will be performed?

I opened #841 for it

@jtv
Copy link
Owner

jtv commented May 27, 2024

@alexolog we'll continue conversation in #841 then. But note that no encoding conversion is involved here. It's just that libpqxx needs to be aware in various situations of where the next character begins. Without that, for example, it might stumble when it sees a byte that happens to be the same numeric value as an ASCII quote or backslash, but is actually just a byte inside a multi-byte character value. It's a painful problem with real security implications.

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

No branches or pull requests

2 participants