Hi,
On 10/6/22 8:39 AM, Michael Paquier wrote:
On Mon, Sep 19, 2022 at 08:29:22AM +0200, Drouvot, Bertrand wrote:
Please find attached v6 taking care of the remarks mentioned above.
+ case T_VariableSetStmt:
+ {
+ VariableSetStmt *stmt = (VariableSetStmt *) node;
+
+ /* stmt->name is NULL for RESET ALL */
+ if (stmt->name)
+ {
+ APP_JUMB(stmt->kind);
+ APP_JUMB_STRING(stmt->name);
+ JumbleExpr(jstate, (Node *) stmt->args);
+ }
+ }
+ break;
Hmm. If VariableSetStmt->is_local is not added to the jumble, then
aren't "SET foo = $1" and "SET LOCAL foo = $1" counted as the same
query?
Good catch, thanks!
While at it let's also jumble "SET SESSION foo =". For this one, we
would need to record another bool in VariableSetStmt: I'll create a
dedicated patch for that.
I am not seeing SAVEPOINT, RELEASE, ROLLBACK .. TO SAVEPOINT
mentioned on this thread. Would these be worth considering in what
gets compiled? That would cover the remaining bits of
TransactionStmt. The ODBC driver abuses of savepoints, for example,
so this could be useful for monitoring purposes in such cases.
Agree. I'll look at those too.
As of the code stands, it could be cleaner to check
IsJumbleUtilityAllowed() in compute_utility_query_id(), falling back
to a default in JumbleQuery(). Not that what your patch does is
incorrect, of course.
Will look at it.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
