Skip to content

SONK-3386: Fix encoding issue (#1)#33956

Closed
isaac-jaynes-imperva wants to merge 1 commit into
apache:masterfrom
isaac-jaynes-imperva:fix-apache-drill
Closed

SONK-3386: Fix encoding issue (#1)#33956
isaac-jaynes-imperva wants to merge 1 commit into
apache:masterfrom
isaac-jaynes-imperva:fix-apache-drill

Conversation

@isaac-jaynes-imperva

Copy link
Copy Markdown
Contributor

Don't format mongo commands with a '/' if using drill+sadrill.
Needed in addition to JohnOmernik/sqlalchemy-drill#98 to make MongoDB + SqlAlchemyDrill + Superset to work properly

@korbit-ai korbit-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Functionality Incorrect Logic for Schema Modification Condition ▹ view 🧠 Incorrect
Design Schema Path Conversion Logic Not Properly Encapsulated ▹ view 🧠 Not in scope
Files scanned
File Path Reviewed
superset/db_engine_specs/drill.py

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

) -> tuple[URL, dict[str, Any]]:
if schema:
uri = uri.set(database=parse.quote(schema.replace(".", "/"), safe=""))
if uri.get_driver_name() == 'sadrill' and uri.get_backend_name() != 'drill':

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this break other, non-mongo queries?

Comment on lines +90 to +91
if uri.get_driver_name() == 'sadrill' and uri.get_backend_name() != 'drill':
schema = schema.replace(".", "/")

This comment was marked as resolved.

@rusackas

Copy link
Copy Markdown
Member

@cgivre I just stumbled across this... any thoughts on this one?

@isaac-jaynes-imperva isaac-jaynes-imperva closed this by deleting the head repository Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants