Comments (9)
@alexolog for some reason I think some conversation disappeared. But you're right — I wasn't aware of the operator[]
requirement back when I wrote that code. So the result iterator is going to have to be a bidirectional iterator for now.
We'll probably need a deprecation period for the existing operator[]
in the result iterator — which means we can't fully resolve this in 8.0. :-( On the one hand I see why the concept has that requirement (given how the classic C definition of the operator is just a combination of *
and +
). On the other hand it really gets in the way of the "referential transparency" that made so much sense in a two-dimensional iteration.
OTOH the row iterator is easy — that didn't have an operator[]
. So in the short term I'm thinking...
- Degradate the result iterators to "bidirectional."
- Give the row iterators an
operator[]
.
from libpqxx.
I thought I did the work for this, sometime long ago.
@alexolog do you know exactly which concepts are missing, and how I implement them?
from libpqxx.
I think we should be able to get some of the information by trying to instantiate a template that expects a std::random_access_iterator
, but passing it a pqxx::const_row_iterator
or a pqxx::const_result_iterator
. There should be an error, and at least with some compilers, the error will tell us "this type doesn't satisfy the random_access_iterator
concept because..."
@alexolog did you see a helpful error like that while compiling your code? (Switching between gcc and clang may help — I think they produce different error messages.
from libpqxx.
I thought I did the work for this, sometime long ago.
@alexolog do you know exactly which concepts are missing, and how I implement them?
Looking at LegacyRandomAccessIterator, we can see the requirements:
template< class I >
concept random_access_iterator =
std::bidirectional_iterator<I> &&
std::derived_from</*ITER_CONCEPT*/<I>, std::random_access_iterator_tag> &&
std::totally_ordered<I> &&
std::sized_sentinel_for<I, I> &&
requires(I i, const I j, const std::iter_difference_t<I> n) {
{ i += n } -> std::same_as<I&>;
{ j + n } -> std::same_as<I>;
{ n + j } -> std::same_as<I>;
{ i -= n } -> std::same_as<I&>;
{ j - n } -> std::same_as<I>;
{ j[n] } -> std::same_as<std::iter_reference_t<I>>;
};
It appears you have most of them, and the only one missing is operator[]
. Implement that one and you're golden.
from libpqxx.
I think we should be able to get some of the information by trying to instantiate a template that expects a
std::random_access_iterator
, but passing it apqxx::const_row_iterator
or apqxx::const_result_iterator
. There should be an error, and at least with some compilers, the error will tell us "this type doesn't satisfy therandom_access_iterator
concept because..."@alexolog did you see a helpful error like that while compiling your code? (Switching between gcc and clang may help — I think they produce different error messages.
See the example code I included in the original post.
We can find the iterator category by using if constexpr
instead of SFINAE.
Both result and row iterator types satisfy bidirectional_iterator
and below, but not random_access_iterator
or above.
from libpqxx.
Implementing operator[]
isn't so easy. The result iterator already has an operator, but it does something different.
from libpqxx.
Implementing operator[]
should be easy since
Giving it different semantics (or omitting it altogether) means that the iterator won't be treated as a random-access iterator by libraries (including the standard library), which will, at best, choose to invoke operator++
in a loop instead of the more efficient operator+
(e.g., std::distance
), or flatly refuse to accept it in some cases (e.g., std::flat_set<>
or std::sort
)
This is of course a valid design choice, but you cannot have your cake and eat it too. So the options are:
- (re)implement
operator[]
with the expected semantics to have the iterators modelstd::random_access_iterator
, or - declare the iterators as bidirectional (
using iterator_category = std::bidirectional_iterator_tag;
) and document them as such.
(Issue renamed accordingly)
from libpqxx.
We should be able to resolve this together with #770. Rewrite the iterator types so that they're no longer closely intertwined with the row/field types; give row
and const_result_iterator
different operator[]
s with different return types; and also make row
and field
keep their result
object alive but no longer make the result/row iterators keep their result alive.
from libpqxx.
Tempting: it now looks like a "divorce" between result::const_iterator::operator[]
and row::operator[]
probably wouldn't break a lot of code!
I tried commenting out the operator[]
on the iterator, and the only code that failed to compile was in tests specifically meant to exercise this kind of thing. The "natural" code I'd written in other tests remained unaffected.
So loath as I am to break compatibility like this, I probably could implement random_access_iterator
in libpqxx 8.0.
(For now though, going with the demotion to bidirectional_iterator
.)
from libpqxx.
Related Issues (20)
- std::optional<std::chrono::year_month_day> to DATE error if optional is empty HOT 4
- Encoding problems with bytea HOT 22
- Undefined refences when compiling example from the documentation HOT 10
- After compilation, there are only pqxx files but no pqxx.hxx files. Is this normal? HOT 31
- Rtti usage on demand HOT 6
- linking issue when using torch+pqxx in cmake. HOT 11
- Can't use libpqxx 7.9.0 from vcpkg in a Visual Studio 2022 C++ 17 project HOT 6
- Allow parsing arrays without connection HOT 21
- Any example to deal with time or date? HOT 2
- ERROR: Unsupported server version; 9.0 is the minimum. HOT 4
- Readthedocs /stable documentation page is empty. HOT 7
- CMakeDetermineCompileFeatures throwing errors HOT 24
- `stream_to` uses `quote_name` as 2 argument function HOT 6
- std::ranges undefined? HOT 7
- building fail on github actions using cmake 3.30 HOT 5
- Cross compile of the pqxx in c++ for arm7 HOT 4
- The linking issue with the compiled artifact libpqxx.so HOT 7
- query() for prepared statements HOT 4
- Possible to combine "on clonflict do nothing" and "stream_to"? HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from libpqxx.