sha174n commented on PR #44496: URL: https://github.com/apache/superset/pull/44496#issuecomment-5774077507
Pushed 3f3e82d. This changes behavior again since your approvals, @aminghadersohi @rusackas, so flagging it rather than leaving it in the diff. The comment strip I added in 9afeb83 only understood `'` and `"` quoting. A `--` inside a dollar-quoted literal is data, not a comment, so the strip was dropping the rest of the body: `DO $$ BEGIN PERFORM $q$--$q$; EXECUTE 'SET search_path TO evil'; END $$` went from `changes_search_path() == True` on master to `False` on the branch. That is a regression in a pre-existing gate, introduced by this PR, so it needed fixing here. The strip now recognises dollar-quoted regions, and peels the body's own `$$` wrapper off first so the code inside it is still scanned. Both directions are pinned by new cases in `test_changes_search_path` and `test_get_client_file_transfer_command`. Second fix in the same helper: an unterminated `/*` now has its own alternative that runs to end of text, so the lazy block-comment match no longer rescans from every `/*` in turn. That was quadratic on text a caller supplies (360 KB went from ~90s to 3ms). Last change is a fourth call site: `AlertCommand._validate_rendered_sql` already enforces the sibling read-only constraint but not this one, and an alert query is user-submitted SQL on the same premise, so the gate runs there too. UPDATING.md now names the three paths. Comment density around the new parse helpers is trimmed; no behavior in that part. -- 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]
