Ship a valid SET search_path with replicated DDL for an empty path

[danolivo]

Summary

The auto-DDL path in spock_auto_replicate_ddl() interpolated the publisher's search_path GUC straight into the queued command with a bare %s, guarded only by strlen(search_path) > 0. That is unsafe: GetConfigOptionByName("search_path", ...) does not always return something that is valid SQL on its own. In particular set_config('search_path', ' ', false) — exactly what core's namespace regression test does — leaves a bare space in the GUC, which interpolated directly emits the invalid SET search_path TO ;. That statement fails to parse on the subscriber, so the DDL (and any rows depending on it) never lands.

spock_replicate_ddl_command() already parsed and re-quoted the path correctly, so the two code paths disagreed. This PR factors the correct logic into a single helper and routes both call sites through it.

Changes

• Add append_set_search_path(), which parses the search_path value with SplitIdentifierString() and re-emits each schema name through quote_identifier(). A value that resolves to no schemas (empty or whitespace-only) is shipped as SET search_path TO '';, so the command is always valid SQL and the subscriber ends up with the same effective (empty) path as the publisher.
• Replace the unsafe %s interpolation in spock_auto_replicate_ddl() with a call to the helper.
• Replace the duplicated open-coded loop in spock_replicate_ddl_command() with the same helper.
• Upgrade the spock_relation_open() cache-miss elog to an ereport with errdetail/errhint that point at the likely cause — a relation message arriving after a data change that references it, usually a schema mismatch — so the subscriber-side failure is diagnosable rather than a bare "cache lookup failed".

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a shared append_set_search_path static helper in spock_functions.c that parses and properly quotes the search_path GUC before appending a SET search_path TO ...; fragment to a buffer. Both spock_replicate_ddl_command and spock_auto_replicate_ddl are updated to use this helper, removing duplicated inline logic. A regression test for the empty search_path case is added to autoddl.sql. Separately, the cache-lookup failure path in spock_relation_open is upgraded from elog to structured ereport.

Changes

search_path Serialization Refactor

Layer / File(s)	Summary
append_set_search_path helper and call sites src/spock_functions.c	Adds static append_set_search_path that quotes each schema identifier and emits a complete SET search_path TO ...; fragment (or SET search_path TO ''; for empty values). Removes the old inline SplitIdentifierString block from spock_replicate_ddl_command and replaces raw-string interpolation in spock_auto_replicate_ddl with single calls to the helper.
Empty search_path regression test tests/regress/sql/autoddl.sql	Sets search_path to a bare space via set_config, creates test_ns_schema_1 and table abc, inserts a row, syncs provider and subscriber, and asserts the schema/table and row exist on the subscriber.

Relcache Lookup Structured Error

Layer / File(s)	Summary
ereport upgrade for missing remote relation src/spock_relcache.c	Replaces elog(ERROR, ...) with ereport(ERROR, errcode(ERRCODE_INTERNAL_ERROR), errmsg, errdetail, errhint) to give structured diagnostic context when a remote relation is absent from the cache.

Poem

A path once bare, just whitespace there,
Now quoted strings fill every chair 🐇
The helper splits and wraps each name,
No inline mess—the logic's tame.
And cache mishaps now speak up clear,
With hints and codes for engineers to hear! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing DDL replication by ensuring valid SET search_path commands are generated even when search_path is empty.
Description check	✅ Passed	The description clearly explains the bug, the root cause of unsafe GUC interpolation, and details the fix including the new helper function and refactoring of both code paths.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch spoc-570

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/spock_functions.c (1)

2240-2240: 💤 Low value

Use appendStringInfoString for literal strings.

appendStringInfo(buf, ", ") works but appendStringInfoString is more appropriate when there are no format specifiers—it avoids parsing the format string.

Suggested fix

		if (lc != list_head(path_list))
-			appendStringInfo(buf, ", ");
+			appendStringInfoString(buf, ", ");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spock_functions.c` at line 2240, Replace the `appendStringInfo(buf, ",
")` call with `appendStringInfoString(buf, ", ")` to avoid unnecessary format
string parsing when appending a literal string with no format specifiers. The
`appendStringInfoString` function is the appropriate choice for appending
literal strings and is more efficient than `appendStringInfo` in this case.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/spock_functions.c`:
- Line 2240: Replace the `appendStringInfo(buf, ", ")` call with
`appendStringInfoString(buf, ", ")` to avoid unnecessary format string parsing
when appending a literal string with no format specifiers. The
`appendStringInfoString` function is the appropriate choice for appending
literal strings and is more efficient than `appendStringInfo` in this case.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 96af3da6-96ee-4c8c-9603-ea16b1db026e

📥 Commits

Reviewing files that changed from the base of the PR and between 922ea93 and 7254192.

⛔ Files ignored due to path filters (2)

• tests/regress/expected/autoddl.out is excluded by !**/*.out
• tests/regress/expected/exception_row_capture.out is excluded by !**/*.out

📒 Files selected for processing (3)

• src/spock_functions.c
• src/spock_relcache.c
• tests/regress/sql/autoddl.sql

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric	Results
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}