Re: Query Jumbling for CALL and SET utility statements

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