(superset) 01/01: docs: clarifying that config.SQL_QUERY_MUTATOR does not affect cache

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch mutator_not_cache
in repository https://gitbox.apache.org/repos/asf/superset.git

commit f257e2956b20df44e805b43ce91a676aa0370f82
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Mon May 6 11:22:25 2024 -0700

    docs: clarifying that config.SQL_QUERY_MUTATOR does not affect cache
    
    I was using SQL_QUERY_MUTATOR for something specific for a customer, and had to double-check whether what's done in `config.SQL_QUERY_MUTATOR` does or doesn't affect the cache key. After checking I thought I would clarify this as a comment where it's defined.
    
    Knowing this, it could be good to allow this configuration to affect the cache key as there are use cases where you may want to mutate the SQL with say adding a specific predicate, and make sure that that predicate is captured in the cache key, but that's beyond the scope of this PR. It didn't seem trivial (nor super difficult) to add this feature, and it seemed it would require 1. changing the function signature to allow returning not only the SQL, but something to add to the cache k [...]
    
    From my understanding SQL_QUERY_MUTATOR is often use to add metadata so around the query in the form of comments, for instance passing the username of who's running the SQL, so that its captured in the database logs. For these use cases there's really no need for altering the cache key.
---
 superset/config.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/superset/config.py b/superset/config.py
index 7851938b72..6421781288 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1227,7 +1227,7 @@ DB_SQLA_URI_VALIDATOR: Callable[[URL], None] | None = None
 
 
 # A function that intercepts the SQL to be executed and can alter it.
-# The use case is can be around adding some sort of comment header
+# A common use case for this is around adding some sort of comment header to the SQL
 # with information such as the username and worker node information
 #
 #    def SQL_QUERY_MUTATOR(
@@ -1237,9 +1237,12 @@ DB_SQLA_URI_VALIDATOR: Callable[[URL], None] | None = None
 #    ):
 #        dttm = datetime.now().isoformat()
 #        return f"-- [SQL LAB] {user_name} {dttm}\n{sql}"
-# For backward compatibility, you can unpack any of the above arguments in your
+#
+# NOTE: For backward compatibility, you can unpack any of the above arguments in your
 # function definition, but keep the **kwargs as the last argument to allow new args
 # to be added later without any errors.
+# NOTE: whatever you in this function DOES NOT affect the cache key, so ideally this function
+# is "functional", as in deterministic from its input.
 def SQL_QUERY_MUTATOR(  # pylint: disable=invalid-name,unused-argument
     sql: str, **kwargs: Any
 ) -> str: