rusackas commented on code in PR #44686:
URL: https://github.com/apache/superset/pull/44686#discussion_r4135890832
##########
superset/db_engine_specs/__init__.py:
##########
@@ -169,10 +169,16 @@ def get_available_engine_specs() ->
dict[type[BaseEngineSpec], set[str]]: # noq
# process-global `compiler.OPERATORS` mapping in place on import instead
# of subclassing it, which would otherwise silently change SQL rendering
# (e.g. `!=` -> `<>`) for every dialect for the rest of the process, not
- # just the misbehaving one. Snapshot/restore around each load so a
- # buggy connector can't leak global compiler state into unrelated
- # dialects just because it was enumerated here.
+ # just the misbehaving one. Others (e.g. kylinpy) rebind or extend the
+ # shared `IdentifierPreparer.reserved_words` set, which changes identifier
+ # quoting (e.g. `name` -> `"name"`) for every dialect that doesn't define
+ # its own reserved words. Snapshot/restore around each load so a buggy
+ # connector can't leak global compiler state into unrelated dialects just
+ # because it was enumerated here.
operators_snapshot = dict(sqla_compiler.OPERATORS)
Review Comment:
```suggestion
operators = sqla_compiler.OPERATORS
operators_snapshot = dict(operators)
```
##########
superset/db_engine_specs/__init__.py:
##########
@@ -208,9 +214,22 @@ def get_available_engine_specs() ->
dict[type[BaseEngineSpec], set[str]]: # noq
driver = driver.decode()
drivers[backend].add(driver)
finally:
- if sqla_compiler.OPERATORS != operators_snapshot:
- sqla_compiler.OPERATORS.clear()
- sqla_compiler.OPERATORS.update(operators_snapshot)
+ # Restore without ever emptying the shared objects, so SQL compiled
+ # concurrently on another thread never sees a transiently empty
+ # operator map or reserved-word set: drop only what the dialect
+ # added, then put back what it removed or changed.
+ operators = sqla_compiler.OPERATORS
+ if operators != operators_snapshot:
+ for key in operators.keys() - operators_snapshot.keys():
+ operators.pop(key, None)
+ operators.update(operators_snapshot)
Review Comment:
```suggestion
if sqla_compiler.OPERATORS is not operators:
sqla_compiler.OPERATORS = operators
if operators != operators_snapshot:
for key in operators.keys() - operators_snapshot.keys():
operators.pop(key, None)
operators.update(operators_snapshot)
```
--
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]