Skip to content

fix: pass re.DOTALL as keyword arg in Livy comment-stripping regex#195

Merged
mdrakiburrahman merged 1 commit into
microsoft:mainfrom
sdebruyn:fs-04
May 19, 2026
Merged

fix: pass re.DOTALL as keyword arg in Livy comment-stripping regex#195
mdrakiburrahman merged 1 commit into
microsoft:mainfrom
sdebruyn:fs-04

Conversation

@sdebruyn

Copy link
Copy Markdown
Contributor

Summary

Fixes #194.

_getLivySQL (in singleton_livy.py) and _strip_block_comments (in concurrent_livy.py) both call:

re.sub(r"\s*/\*(.|\n)*?\*/\s*", "\n", sql, re.DOTALL)

re.DOTALL is the integer enum value 16, and re.sub's positional signature is re.sub(pattern, repl, string, count=0, flags=0). Passing re.DOTALL positionally sets count=16, capping comment-stripping to at most 16 replacements per submitted SQL string.

This PR swaps both call sites to flags=re.DOTALL so the comment-stripping pass is no longer capped and the flag the author intended to set is actually set.

Diff

-re.sub(r"\s*/\*(.|\n)*?\*/\s*", "\n", sql, re.DOTALL)
+re.sub(r"\s*/\*(.|\n)*?\*/\s*", "\n", sql, flags=re.DOTALL)

Applied identically to both singleton_livy.py:488 and concurrent_livy.py:555.

Test plan

  • CI green on this branch
  • Manual: submit a model with >16 /* ... */ comment blocks via Livy; verify all comments are stripped before submission (was: 4 surviving comments with the original count=16 cap; now: 0).

Notes

A natural follow-up would be to extract a single _strip_sql_comments(sql) helper used by both files so this duplicated regex (and its history of being miswritten) can't drift again. Left out of this PR to keep the diff minimal and easy to backport.

@sdebruyn

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@mdrakiburrahman

Copy link
Copy Markdown
Collaborator

Same as here:

#193 (comment)

@mdrakiburrahman

Copy link
Copy Markdown
Collaborator

I think this one is simple enough where I'll run CI locally and merge if it runs green 🙂

@mdrakiburrahman mdrakiburrahman self-requested a review May 19, 2026 11:12
@mdrakiburrahman

mdrakiburrahman commented May 19, 2026

Copy link
Copy Markdown
Collaborator

I have fired CI on fs-04:

image

Will share results here after running.

@mdrakiburrahman

Copy link
Copy Markdown
Collaborator

CI ran green on the final commit in this branch:

image

@mdrakiburrahman mdrakiburrahman merged commit 46cc6a4 into microsoft:main May 19, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

\_getLivySQL\: \re.DOTALL\ passed as positional \count\ arg — comment-stripping silently capped at 16 replacements per file

2 participants