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

Error NC29 #193

Open
RealKoenisch opened this issue Apr 24, 2024 · 10 comments
Open

Error NC29 #193

RealKoenisch opened this issue Apr 24, 2024 · 10 comments

Comments

@RealKoenisch
Copy link

{"reqId":"wwrjPQE2Tzll6lg4zzJD","level":3,"time":"2024-04-24T19:20:38+00:00","remoteAddr":"94.219.34.169","user":"XXXX","app":"user_sql","method":"GET","url":"/settings/admin/logging","message":"Could not execute the query: SELECT COUNT(u.username) AS count FROM aase6_users u WHERE u.username LIKE :search ","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36","version":"29.0.0.19","exception":{"Exception":"Doctrine\DBAL\Exception\TableNotFoundException","Message":"An exception occurred while executing a query: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'owncloud.aase6_users' doesn't exist","Code":1146,"Trace":

The table is aase6_user. Ther could be something wrong with the parameters or binding of parameter (in PDO/Statment.php passing $params to statement is depreceated.

@kainhofer
Copy link

kainhofer commented Apr 29, 2024

I have the same problem, but I'm trying to access a view in a different mysql database (dbname=xxx_admidio) on the same server. I did a little debugging and it seems that even though the user_sql passes in the correct dbname in the $parameters to the $connectionFactory->getConnection call in lib/Query/DataQuery.php, the connection is created to the default nextcloud database...

If I add the following code after the getConnection call:

$result = $this->connection->fetchOne("SELECT DATABASE();");
$this->logger->warning("MySQL: Connected to DB ".$result);

The log output is:
"MySQL: Connected to DB xxx_nextcloud"
rather than the correct database xxx_admidio

@kainhofer
Copy link

After some more debugging, I seem to understand how the problem appeared:
In the commit nextcloud/server@79c4986, the Nextcloud server code switched from using \Doctrine\DBAL\Connection for the DB connections to Doctrine\DBAL\Connections\PrimaryReadReplicaConnection.

Apparently PrimaryReadReplicaConnection uses a different array structure for its arguments (connection params are stored in the ['primary'] sub-array instead of first-level array elements). Unfortunately, the code in ConnectionFactory::getConnection (https://github.com/nextcloud/server/blob/147426c3ca7183ad065293bd9b600e10adde4abf/lib/private/DB/ConnectionFactory.php#L129) merges in the connection parameters as first-level arguments (as was required for the old Connection class) rather than into the ['primary'] sub-array, which is now needed for the PrimaryReadReplicaConnection.

So, while the old code needed

Array(
	'host' => "localhost",
	'password' => "myPassword",
	'user' => "DBUsername",
	'dbname' => "DBUserPassword"
)

the new Primary ReadReplicaConnection now needs them in the primary sub-array:

Array(
	'primary' => Array(
		'host' => "localhost",
		'password' => "myPassword",
		'user' => "DBUsername",
		'dbname' => "DBUserPassword"
	)
)

For a difference of the params, I found this nice page:
https://blog.adamcameron.me/2023/01/php-primaryreadreplicaconnection.html

Without that page, I would still be debugging...

I suppose the correct fix would be to properly fix the ConnectionFactory to override the primary connection data instead of merging the params at top-level, where they are never used. I'll try to submit a issue with the nextcloud server repo.

@kainhofer
Copy link

As a quick fix (ONLY for NC29!!!), one can adjust the user_sql code with the following patch:

diff --git a/lib/Query/DataQuery.php b/lib/Query/DataQuery.php
index 1a7e2ae..51f3917 100644
--- a/lib/Query/DataQuery.php
+++ b/lib/Query/DataQuery.php
@@ -168,7 +168,7 @@ class DataQuery
         }

         $this->connection = $connectionFactory->getConnection(
-            $this->properties[DB::DRIVER], $parameters
+           $this->properties[DB::DRIVER], array("primary" => $parameters)
         );

         $this->logger->debug(

However, this (1) depends on the specific Nextcloud Server version, (2) brings in the logic from the underlying ConnectionFactory implementation to third-party apps (i.e. it's not transparent) for no good reason and (3) might break at any time the nextcloud server chooses to fix this issue.

@RealKoenisch
Copy link
Author

RealKoenisch commented Apr 29, 2024

Positiv: I tried your small patch - it works in NC-29.

@FischFischer
Copy link

Where / at which file did you do your changes?

@RealKoenisch
Copy link
Author

RealKoenisch commented Jun 4, 2024

Where / at which file did you do your changes?

The changes are in the fork from rotdrop. Not Master but cafevdb/stable 29
https://github.com/rotdrop/user_sql/tree/production/cafevdb/stable29

The Changes are made in "lib/Query/DataQuery.php"

The git is out of date - is there someone who can implement the changes to this version?

KR

@d-jsb
Copy link

d-jsb commented Jul 3, 2024

Any news on this? While the patches shown above allow at least to login again. Groups are not working for me with NC29
All file shares are gone for the users as they are group based in my installation :(
Is user_sql officially supported by nextcloud and if yes, why don't they check if it breaks when touching the server code? :/

@StefanSander3
Copy link

Hey @RealKoenisch,

what is the state with this issue (see last comment) and this app in general. For some time now we can not use our nextcloud instance.

@kainhofer
Copy link

It seems that "Nextcloud Hub 8 (29.0.7)" switched back to the old param structure, so the workaround from April 29 is no longer needed (and indeed breaks user_sql). I switched back to the original user_sql code and it seems to work for now.

@flow10
Copy link

flow10 commented Dec 7, 2024

Seems to be fixed by 5654799. Can someone comfirm?

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

6 participants