gabotorresruiz commented on code in PR #44928: URL: https://github.com/apache/superset/pull/44928#discussion_r4212017741
########## superset/versioning/unit_of_work.py: ########## @@ -16,12 +16,39 @@ # under the License. """Runtime capture suppression at Continuum's transaction-scoped write boundary.""" +from itertools import chain + +from sqlalchemy.engine import Connection from sqlalchemy.orm import Session +from sqlalchemy_continuum import versioning_manager from sqlalchemy_continuum.operation import Operations from sqlalchemy_continuum.unit_of_work import UnitOfWork +from sqlalchemy_continuum.utils import is_versioned from superset.versioning.utils import capture_enabled +INITIAL_STATES_KEY: str = "_version_changes_initial_states" + + +def _has_versioned_work(session: Session) -> bool: + """Recognize pending parents, children, deletes, or retained pre-flush state.""" + return bool(session.info.get(INITIAL_STATES_KEY)) or any( + is_versioned(obj) for obj in chain(session.new, session.dirty, session.deleted) Review Comment: Not a blocker, more a scoping question. With `VERSIONING_CAPTURE_PREDICATE` unset, which is the OSS default, `capture_enabled` is three `config.get` calls, so there is no host lookup left to defer and `_has_versioned_work` ends up the more expensive of the two: measured per call on sqlite, `capture_enabled` is flat at about 0.9us while this helper is about 0.9us with 1 pending object, 4.7us at 50, 38us at 500 and 152us at 2000, and it runs 4 times per unrelated flush plus once at the commit. I could not see it at the commit level (a 2000 row unrelated commit measured 61 to 68ms here against 64 to 67ms on `5a2f4c8e`), and `any()` short circuits on the first versioned object so the versioned path is untouched. Is the intent that the win only lands for hosts that install a policy, or would a `predicate is None` fast path ahead of the scan be worth adding? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
