Add a special error message during test run when Postgres 14+ is using the older pgss

[keiko713]

Starting from Postgres 14, there is a concept called toplevel but if you're using the older version of pg_stat_statements extension, this toplevel not appearing during the stats collection, therefore it can still cause the bug that was fixed in #463

With this PR, we detect such case during the test run and warn folks to upgrade pgss.

[msakrejda]

Technically, if you're never calling the same queries from both within functions and at the top level, this doesn't affect you, so this wording may be a little strong. Maybe something more vague like

Suggested change

	logger.PrintError("Outdated pg_stat_statements extension is known to cause the incorrect data with query statistics")
	logger.PrintError("Outdated pg_stat_statements may cause incorrect query statistics")

?

Also, it might be good to mention the concrete issue in the comment.

[keiko713]

+1 on rewording 👍

Also, it might be good to mention the concrete issue in the comment.

I'm actually not really convinced what I wrote in the PR summary is correct, so I'm not so confident enough to put anything concrete in the comment 🤔 Like, I'm not sure if the mismatch of pg14 and pgss version is really the cause.
My understanding is, toplevel was introduced purely within pgss 1.9, then it shouldn't really matter which version of postgres is used, it shouldn't really cause the problem. That said, the customer's issue was happening when version mismatches and did fix with the pgss version bump, so it is definitely worth doing (also using pgss10 is better in general, it also added pg_stat_statements_info which is useful).

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6b4d23feef6e334fb85af077f2857f62ab781848

I'm happy to update the comment if you know the concrete issue though 🙈

[msakrejda]

No objection to urging an upgrade here.

Re: the root cause--I'm not sure either. Maybe we can just link to this discussion? I think it could be that adding the functionality for toplevel involved changes on the Postgres side as well, so Postgres started reporting some queries twice to pgss (once for each value of toplevel), but if you're using an older pgss that doesn't know about toplevel, it just ignores that and reports the same value twice? That seems unlikely, but it could explain this.

[keiko713]

Uhmm, after reading the below thread (or rather, you don't need to read an entire thread, the first message is enough), I feel like this has been a known issue for all Postgres versions (and pgss versions) until the solution was implemented in pgss 1.9 (shipped with pg14).
https://www.postgresql.org/message-id/20201202040516.GA43757@nol

With that, I think I'll keep your suggestion on the rewording and update the comment something mentioning that.

[lfittl]

I'm actually not really convinced what I wrote in the PR summary is correct, so I'm not so confident enough to put anything concrete in the comment 🤔 Like, I'm not sure if the mismatch of pg14 and pgss version is really the cause. My understanding is, toplevel was introduced purely within pgss 1.9, then it shouldn't really matter which version of postgres is used, it shouldn't really cause the problem.

Hopefully I'm not saying something that you already know (or said elsewhere), but here is my understanding of the issue:

With Postgres extensions the shared library and the schema definition are separate. When you upgrade to a new Postgres release, you automatically get the new shared library (since the .so file on disk changes) for all contrib extensions, but the extension's schema information in use doesn't change since that's actually stored in the database itself.

In that situation, you already get the new internal logic of pg_stat_statements that will record a separate entry if toplevel=false, causing there to be two entries for a given (database, role, queryid). However, since you didn't update the schema yet, the pg_stat_statements view will simply not return the toplevel field - thus you'll get two entries that are identical. Once you upgrade the schema the toplevel field is included in the view, allowing you to differentiate the two entries.

[keiko713]

Ah I updated comment without realizing your comment here, though after re-reading https://www.postgresql.org/message-id/20201202040516.GA43757@nol, just realized that my understanding is wrong too (I thought it was tracked differently, but the point in that discussion is that there is no distinctions, so it's "combined" before rather than 2 rows created).

In that situation, you already get the new internal logic of pg_stat_statements that will record a separate entry if toplevel=false, causing there to be two entries for a given (database, role, queryid).

This (and other things you said) make sense, though I'm also surprised that the new logic will be used before the ALTER EXTENSION. I get the part that the new shared library is automatically obtained, though my assumption was that it won't be used until ALTER EXTENSION is executed. Feels a bit wild to me. I meant, if you use a new logic without asking, why don't you also automatically update it for us 😂

Anyways, I'll re-update the comment pointing this.

[lfittl]

This (and other things you said) make sense, though I'm also surprised that the new logic will be used before the ALTER EXTENSION. I get the part that the new shared library is automatically obtained, though my assumption was that it won't be used until ALTER EXTENSION is executed. Feels a bit wild to me. I meant, if you use a new logic without asking, why don't you also automatically update it for us 😂

Yeah, agreed it doesn't really make sense from a user experience perspective :)

For additional context, here is how that's defined inside pg_stat_statements.c: https://github.com/postgres/postgres/blob/master/contrib/pg_stat_statements/pg_stat_statements.c#L1523 (i.e. there are different C functions for each extension schema version, and when you upgrade you essentially switch which C function you are using)