Fix: Properly quote database and schema identifiers for SQL compatibility

[mschmid09]

Summary
This PR updates the logic for constructing the elementary_database_and_schema identifier so that the database and schema are each wrapped in double quotes, e.g. "database"."schema". This ensures correct SQL syntax and avoids treating the entire string as a single identifier.

Details
Previously, the code would wrap the whole database.schema string in quotes, resulting in "database.schema", which is not valid in standard SQL.
Now, the code splits the identifier on the dot and wraps each part individually: "database"."schema".
If only a schema is present, it is wrapped as "schema".

#1986

Summary by CodeRabbit

• Bug Fixes
  • Relation identifiers now have surrounding quotes removed before use, ensuring consistent handling of database/schema names.
  • Logs now display the cleaned (unquoted) relation value for clearer diagnostics.
  • Existing fallback behavior on errors is preserved to maintain resilience.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Function now strips surrounding quotes from the retrieved relation identifier, logs the stripped value, and returns the stripped relation. On exception it logs the error, records via tracking if present, and returns the fallback <elementary_database>.<elementary_schema>.

Changes

Cohort / File(s)	Summary
Relation quote stripping elementary/monitor/data_monitoring/data_monitoring.py	After obtaining the relation string, strip surrounding quotes (if any), log the stripped value, and return the stripped relation. Error path unchanged: exceptions log the error, record via tracking if present, and return the fallback <elementary_database>.<elementary_schema>. No public API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Review the logic that strips quotes to ensure it only removes surrounding quotes and preserves internal content.
• Check logging statements and tracking/error-handling branch for unchanged behavior.

Poem

I hopped through names both clipped and bright,
I brushed away the quotes at night.
I logged the clean and kept it neat,
Returned the name to steady feet. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: properly quoting database and schema identifiers for SQL compatibility by splitting and quoting them separately.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

👋 @mschmidoev
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

elementary/monitor/data_monitoring/data_monitoring.py (2)

82-83: If embedding inside single-quoted SQL literals, escape single quotes instead

If the goal is to safely place the relation inside a single-quoted SQL string literal (e.g., in a Slack snippet), converting double quotes is unnecessary and potentially harmful. Instead, escape single quotes in the content:

-            relation = relation.replace('"', "'") if relation else relation
+            display_relation = relation.replace("'", "''") if relation else relation

This yields a string-literal-safe version while preserving any double quotes that are harmless inside single-quoted literals.

---

82-85: Scope quote-normalization for display only

We verified that the get_elementary_database_and_schema macro always returns an unquoted database.schema string, and that self.elementary_database_and_schema is used broadly (in tests, Slack messages, alert formatting, etc.). Replacing quotes on the canonical return value is a no-op today but could strip valid identifier quoting in the future. To avoid mutating the source-of-truth, only normalize for display:

In elementary/monitor/data_monitoring/data_monitoring.py (lines 82–85):

-            # Replace double quotes with single quotes for proper SQL compatibility
-            relation = relation.replace('"', "'") if relation else relation
-            logger.info(f"Elementary's database and schema: '{relation}'")
-            return relation
+            # Display-only: replace double quotes with single quotes in logs
+            display_relation = relation.replace('"', "'") if isinstance(relation, str) else relation
+            logger.info(f"Elementary's database and schema: '{display_relation}'")
+            return relation

If downstream callers (e.g. Slack templates) need a “safe” string, consider adding a derived elementary_database_and_schema_display property or performing .replace('"', "'") at the call site instead.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4288f49 and f8ed8f1.

📒 Files selected for processing (1)

• elementary/monitor/data_monitoring/data_monitoring.py (1 hunks)

[elazarlachkar]

@mschmidoev The CI is failing for some reason. Can you update your branch to the latest master?

[mschmid09]

@elazarlachkar should be good to go :)

[elazarlachkar]

Hi @mschmidoev!

The CI still has the issue we solved on master.
Can you make sure you're updated to the latest elementary-data:master? I assume you updated from the fork's master (mschmidoev:master).

Thanks, and sorry for the trouble.

[mschmid09]

Hey @elazarlachkar , no dramas, should be good now

[elazarlachkar]

Hi @mschmidoev!
The CI issue is now fixed on the latest elementary-data:master. Sorry for the trouble, can you update your branch again?

[mschmid09]

Thanks @elazarlachkar, was off last week but this PR should be up to date with master branch now!

[mschmid09]

@elazarlachkar looks like that's worked this time around! Let me know if there's anything else you need :) )

[mschmid09]

Hey @elazarlachkar just checking if there's any progress here :)

[elazarlachkar]

Hi @mschmidoev!

I really apologize for the delay here. At the end of September I shifted my focus to other things, and I missed your check in message.

I checked your changes myself, and consulted one of my colleagues here at Elementary, and added a comment I think should be addressed.

Please let me know what you think? This PR is a priority for me now.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

elementary/monitor/data_monitoring/data_monitoring.py (1)

82-85: LGTM! Implementation follows the agreed approach.

The solution correctly strips surrounding quotes from the relation to avoid case sensitivity issues and double-quoting problems, as discussed in the previous review. The implementation is clean and straightforward.

Optional: Consider the try-except-else pattern.

Static analysis suggests moving the return statement to an else block for slightly more idiomatic exception handling:

 try:
     relation = self.internal_dbt_runner.run_operation(
         "elementary_cli.get_elementary_database_and_schema", quiet=True
     )[0]
     # Strip quotes from relation to avoid case sensitivity issues and double-quoting
     strip_relation = relation.strip('"')
     logger.info(f"Elementary's database and schema: '{strip_relation}'")
-    return strip_relation
 except Exception as ex:
     logger.error("Failed to parse Elementary's database and schema.")
     if self.tracking:
         self.tracking.record_internal_exception(ex)
     return "<elementary_database>.<elementary_schema>"
+else:
+    return strip_relation

This pattern makes the success path more explicit, though the current code is perfectly valid.

Note: Identifier quoting assumptions.

The stripped relation (e.g., my_db.my_schema) will work correctly as long as the database and schema names don't contain special characters, spaces, or reserved words that require quoting in SQL. This should be fine for typical use cases, but if issues arise downstream with specific identifier names, this assumption may need revisiting.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 701198f and a4d5a1c.

📒 Files selected for processing (1)

• elementary/monitor/data_monitoring/data_monitoring.py (1 hunks)

🧰 Additional context used

🪛 Ruff (0.14.7)

elementary/monitor/data_monitoring/data_monitoring.py

85-85: Consider moving this statement to an else block

(TRY300)

[elazarlachkar]

@mschmidoev I merged the change, thanks for the PR!
It will be included in the next release (should be in the next few days).