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

Correct FROM of some queries to the right schema #1295

Closed
wants to merge 2 commits into from
Closed

Correct FROM of some queries to the right schema #1295

wants to merge 2 commits into from

Conversation

chris-x86-64
Copy link

@chris-x86-64 chris-x86-64 commented Sep 11, 2020

What:

Relation cert_bund_advs does not exist, however, the table resides in a schema called cert, hence it should be cert.cert_bund_advs. Same goes with cert_bund_cves as well.
With this PR, FROM cert_ will be replaced with FROM cert.cert_ .

Why:

When I tried to EXPLAIN ANALYZE a query, I got the following error:

ERROR:  relation "cert_bund_advs" does not exist
LINE 1: ..., nvts.tag, (ARRAY (SELECT name::text        FROM cert_bund_...

I looked into the source code and found that the table belongs to schema cert.
Proof:

gvmd/src/manage_pg.c

Lines 3070 to 3083 in ea8242e

sql ("CREATE SCHEMA cert;");
sql ("SELECT set_config ('search_path',"
" current_setting ('search_path') || ',cert',"
" false);");
/* Create tables and indexes. */
sql ("CREATE TABLE cert.meta"
" (id SERIAL PRIMARY KEY,"
" name text UNIQUE,"
" value text);");
sql ("CREATE TABLE cert.cert_bund_advs"

With my changes, these queries should now point to the right table in the right schema.
I also suspect that this is could be the cause of a performance issue reported in a third-party Docker deployment. Secure-Compliance-Solutions-LLC/GVM-Docker#54

How:

I did a basic sed substitution.

git grep -l 'FROM cert_' | xargs sed -i 's/FROM cert_/FROM cert.cert_/g'

Not verified yet as I haven't taken time to prepare a build environment (which what makes this PR a draft).
I'm planning on adding tests later.

Checklist:

@mattmundell
Copy link
Contributor

You could also set the search_path like gvmd does, then your EXPLAIN ANALYZE would work.

@chris-x86-64
Copy link
Author

Thanks, it did work and I got the results from EXPLAIN ANALYZE.

gvmd=> set search_path to "$user",public,cert;
SET
gvmd=> explain analyze ...

I overlooked the search_path configurations in src/manage_pg.c, sorry.

gvmd/src/manage_pg.c

Lines 3035 to 3039 in ea8242e

if (manage_cert_loaded ())
sql ("SELECT set_config ('search_path',"
" current_setting ('search_path') || ',cert',"
" false);");
}

@mattmundell
Copy link
Contributor

I'm closing because this is getting old, it's possible to solve with search_path, and some cases have already been done in #1343.

By the way, I originally did not do this to allow switching databases, similar to the --database option for the main database.

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

Successfully merging this pull request may close these issues.

2 participants