Survive lack of permissions on pointcloud_columns

[strk]

See #32972

[strk]

I'm thinking that another way to deal with the permission thing would be to use a DO block that creates a cursor and then fetch from ALL (or partial) from the cursor. Advantage of that is that permissions would be checked on a table-by-table basis instead that from the whole metadata (affects not only topology but any table that can be seen in catalogs but not read for fetching srid/type etc.). Also, it would theoretically allow for interruption of the long-running query (if FETCH is used in blocks). What do you think about this approach @jef-n ?

[strk]

I'm back with the problem of being unable to trigger a reconnection from those python-obtained connections :/

[strk]

@elpaso what about adding a parameter to providerMetadata returned object's createConnection to force the creation of a new connection rather than recycling an existing one ?

[elpaso]

@elpaso what about adding a parameter to providerMetadata returned object's createConnection to force the creation of a new connection rather than recycling an existing one ?

It makes no sense to me: QgsAbstractProviderConnection *createConnection( const QString &name ) and the other overload they both create new connections.

But as we have already discussed some time ago I suspect that you are misinterpreting what those "connections" are about: these are not low level DB connections (that are handled by the provider) but rather wrappers for QGIS settings stored connection configurations, the real backend connection is handled by the provider.

The QgsAbstractDatabaseProviderConnection provides virtual methods that are implemented in the provider and those methods use low level connections.

I strongly believe that low level connection/reconnection/errors etc. should be handled by the provider and the only interaction with client code should be exceptions thrown in case of unrecoverable errors.

[strk]

@elpaso there are cases in which the caller wants to have a new low-level connection, because the provider may be doing something at connection time (this is the case for PostgreSQL connections).
Or we may say the caller wants to "reset" a connection, for that same reason (re-read some connection-time thingies). Should this not be exposed in the API ?

[elpaso]

No: it's not client code responsibility to handle low level connections and to know what's going on inside the provider, the provider must do that.

That said, QgsPostgresProviderConnection does not store any low level connection.

see: QgsPostgresProviderConnection::executeSqlPrivate

QgsPostgresConn *conn = QgsPostgresConnPool::instance()->acquireConnection ....

[strk]

Right, so it's the pool itself that keeps those connections active. I guess I could use some env variable to disable pooling...

[strk]

There seem to be some confusion with connection pools:

git grep CONN_POOL_MAX_CONCURRENT_CONNS

@m-kuhn according to ChangeLog you added QgsApplication::maxConcurrentConnectionsPerPool() in 2018, but that function now returns a constant value and there's no way to set that value ?

[m-kuhn]

IIRC I think I just changed the max number of connections and added a couple of "backup connections" to recover in some cases. I don't think I can help here right now...

[m-kuhn]

Hmm... this seems to break CI.

ERROR: Service 'postgres' failed to build: The command '/bin/sh -c wget -O- https://github.com/pgpointcloud/pointcloud/archive/master.tar.gz | tar xz && cd pointcloud-master && ./autogen.sh && ./configure && make && make install && cd .. && rm -Rf pointcloud-master' returned a non-zero code: 1

https://travis-ci.org/qgis/QGIS/jobs/650370964?utm_medium=notification&utm_source=github_status

Any chance to switch to a package instead of a custom built module in these tests?

[strk]

As things clearly worked as of Dec 4, 2019 I guess we could fetch the pointcloud code from that date, for instance pgpointcloud/pointcloud@dda0cb9

[m-kuhn]

As it suddenly stopped working, I assume it was caused by an upgrade somewhere else (on ubuntu side? postgres docker side?) and just rolling back in time won't help.

Feel free to try whatever you think helps. As long as the CI is green by default and helps point out things that are broken in our code I'm happy.

[strk]

I'm trying that older commit with #34500 -- I would use packaged pointcloud if we had it.
BTW, wouldn't it be better to have docker built in a separate repository/CI and just use the pre-built Docker for testing QGIS itself ? Would also save a lot of time...

[m-kuhn]

BTW, wouldn't it be better to have docker built in a separate repository/CI and just use the pre-built Docker for testing QGIS itself ? Would also save a lot of time...

The docker is only rebuilt if things changed and then cached. This helps to avoid having to synchronize dependencies between different repositories and isolating code while keeping things fast. If this doesn't work as expected we need to have a look.