DOC: Updates to spock content #326
Conversation
susan-pgedge
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR substantially reorganizes and expands Spock documentation: installation and two-node cluster guides rewritten for an enterprise workflow; many function reference pages added or reworked (signatures, params, RETURNS, ARGUMENTS, examples); mkdocs navigation updated; several legacy CLI/docs pages removed. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@docs/spock_functions/functions/spock_repset_add_table.md`:
- Around line 7-14: The SYNOPSIS signature for spock.repset_add_table lists
required parameters (set_name, relation, synchronize_data, columns, row_filter,
include_partitions) but the examples call the function with only set_name and
relation; fix by either adding default/optional markers to the SYNOPSIS for
synchronize_data, columns, row_filter, and include_partitions (and document
their defaults) or update every example to call spock.repset_add_table with all
required positional or named arguments (e.g., include synchronize_data, columns,
row_filter, include_partitions), and apply the same change to the other examples
referenced in the file.
In `@docs/spock_functions/functions/spock_repset_alter.md`:
- Around line 27-28: Update the documentation for spock_repset_alter to remove
the contradiction by explicitly specifying which arguments may be NULL to mean
"no change" (e.g., boolean flags like is_enabled, is_streaming, etc.) and which
argument(s) cause the function to return NULL (e.g., set_name if omitted).
Modify the sentences around the current "Any argument left as NULL will retain
its current setting" and "Returns NULL if any argument is NULL" so they read
together: list the nullable boolean parameters that are "no-op when NULL" and
state that a NULL set_name (or other required identifiers) will cause the
function to return NULL; include the symbol names set_name and the specific
boolean parameter names to make locating the behavior clear.
In `@docs/spock_functions/functions/spock_sub_wait_for_sync.md`:
- Around line 3-8: The docs for spock.sub_wait_for_sync are inconsistent: the
header shows spock.sub_wait_for_sync() with no arguments while SYNOPSIS and
ARGUMENTS (and the NULL note) mention subscription_name; pick one canonical
signature and make all sections match it. Update the header, SYNOPSIS, ARGUMENTS
list, and the NULL note in spock_sub_wait_for_sync.md so either every section
shows spock.sub_wait_for_sync() with no parameters or every section shows
spock.sub_wait_for_sync(subscription_name name) (and adjust the NULL note
accordingly); ensure the function name spock.sub_wait_for_sync and the parameter
subscription_name are referenced consistently.
In `@docs/two_node_cluster.md`:
- Line 68: Update the DSN example string so the database name parameter is
explicitly named; in the DSN assignment line starting with "dsn
:='host=<node_2_IP_address> port=<n2_port> =<db_name>');`" replace the stray "
=<db_name>" with " dbname=<db_name>" so the example reads "... port=<n2_port>
dbname=<db_name>");` ensuring the connection string uses the correct "dbname"
parameter.
- Line 96: The doc uses the wrong parameter name: replace the `subscriber_dsn :=
'host=<node_2_IP_address> port=<n2_port> dbname=<db_name>');` assignment with
the correct `provider_dsn` parameter expected by the spock API (see
spock.sub_create / spock_sub_create.md); update any occurrences of
`subscriber_dsn` to `provider_dsn` so the spock.sub_create call supplies the
proper parameter name.
🟡 Minor comments (19)
docs/spock_functions/functions/spock_sub_alter_skiplsn.md-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorRemove redundant “NAME” line or make it a proper subheading.
Line 3 repeats the section header and looks like a formatting artifact. Consider dropping it or making it a consistent subheading.docs/spock_functions/functions/spock_sub_alter_skiplsn.md-11-16 (1)
11-16:⚠️ Potential issue | 🟡 MinorInclude NULL behavior in the RETURNS bullets for consistency.
The RETURNS list omits the NULL case that’s described later. Add a bullet for “NULL if any argument is NULL” to keep the API contract scannable.Also applies to: 35-35
docs/spock_functions/functions/spock_sub_show_status.md-29-29 (1)
29-29:⚠️ Potential issue | 🟡 MinorAdd markdown header formatting to section headers.
The section headers "DESCRIPTION" (line 29), "ARGUMENTS" (line 45), and "EXAMPLE" (line 52) are missing markdown formatting (
###), unlike other section headers in this document (lines 1, 5, 9). This breaks the markdown hierarchy and may affect documentation rendering.📝 Proposed fix
-DESCRIPTION +### DESCRIPTION-ARGUMENTS +### ARGUMENTS-EXAMPLE +### EXAMPLEAlso applies to: 45-45, 52-52
docs/spock_functions/functions/spock_sub_show_status.md-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorFix inconsistent bullet character.
Line 17 uses an em dash (—) instead of a hyphen (-) like the other bullet points in this list (lines 15, 19, 21, 23, 25, 27). This creates a formatting inconsistency.
📝 Proposed fix
- — Current replication status. + - Current replication status.docs/spock_functions/functions/spock_sync_event.md-7-7 (1)
7-7:⚠️ Potential issue | 🟡 MinorFix spacing inconsistency in function signature.
The SYNOPSIS shows
spock.sync_event ()with a space before the parentheses, while the NAME section (line 3) showsspock.sync_event()without a space. For consistency, remove the space.📝 Proposed fix
-spock.sync_event () +spock.sync_event()docs/spock_functions/functions/spock_node_drop_interface.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorFix typo in RETURNS section.
Line 13 contains a typo: "false it the interface" should be "false if the interface".
📝 Proposed fix
- - false it the interface did not exist. + - false if the interface did not exist.docs/spock_functions/functions/spock_sub_disable.md-9-13 (1)
9-13:⚠️ Potential issue | 🟡 MinorAdd NULL return case to the RETURNS section.
The DESCRIPTION section (line 33) states "Returns NULL if any argument is NULL," but this return value is not documented in the RETURNS section. For completeness, add the NULL case here so users don't need to read the entire description to understand all possible return values.
📝 Proposed addition
### RETURNS - true if the subscription was successfully disabled. - false if the operation fails. + + - NULL if any argument is NULL.docs/spock_functions/functions/spock_node_create.md-12-12 (1)
12-12:⚠️ Potential issue | 🟡 MinorClarify NULL-return behavior consistency.
Line 12 states the function returns "The OID of the newly created Spock node," but line 30 states "Returns NULL if any argument is NULL." This creates ambiguity about whether the function returns an OID or NULL.
Consider clarifying the RETURNS section to explicitly mention the NULL case, for example:
"The OID of the newly created Spock node, or NULL if any argument is NULL."Also applies to: 30-30
docs/spock_functions/functions/spock_repset_create.md-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorClarify OID vs NULL-return behavior.
Line 17 states the function returns "The OID of the newly created replication set," but line 35 states "Returns NULL if any argument is NULL." This is inconsistent.
Consider updating the RETURNS section to: "The OID of the newly created replication set, or NULL if any argument is NULL."
Also applies to: 35-35
docs/spock_functions/functions/spock_repset_remove_seq.md-13-14 (1)
13-14:⚠️ Potential issue | 🟡 MinorClarify boolean vs NULL-return behavior.
Lines 13-14 indicate the function returns boolean (true/false), but line 26 states "Returns NULL if any argument is NULL." This creates ambiguity.
For consistency across the documentation set, consider updating the RETURNS section to explicitly state: "Returns true if the sequence was successfully removed, false if it was not a member, or NULL if any argument is NULL."
Also applies to: 26-26
docs/spock_functions/functions/spock_table_wait_for_sync.md-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorResolve void vs NULL-return contradiction.
Line 11 states the function returns "void (returns nothing upon successful completion)," but line 39 states "Returns NULL if any argument is NULL." A void function cannot return NULL in PostgreSQL/SQL semantics.
Consider either:
- Removing line 39 if the function raises an error on NULL arguments, or
- Clarifying that the function behavior is undefined or errors on NULL inputs rather than returning NULL.
Also applies to: 39-39
docs/spock_functions/functions/spock_repset_add_all_tables.md-12-15 (1)
12-15:⚠️ Potential issue | 🟡 MinorClarify boolean vs NULL-return behavior.
Lines 12-15 indicate the function returns boolean (true/false), but line 34 states "Returns NULL if any argument is NULL." This creates ambiguity about the return type behavior.
Consider updating the RETURNS section to explicitly state: "Returns true if all tables were successfully added, false on failure, or NULL if any argument is NULL."
Also applies to: 34-34
docs/spock_functions/functions/spock_sub_drop.md-7-8 (1)
7-8:⚠️ Potential issue | 🟡 MinorDocument the default in the SYNOPSIS.
The DESCRIPTION says
ifexistsdefaults to false, but the SYNOPSIS omitsDEFAULT false. Add the default there to keep the signature consistent.Also applies to: 21-38
docs/spock_functions/functions/spock_repset_add_table.md-99-103 (1)
99-103:⚠️ Potential issue | 🟡 MinorRename
sync_datain the warning text.The warning still uses
sync_data=truebut the parameter is nowsynchronize_data. Update the warning to match the new name.docs/spock_functions/functions/spock_sub_enable.md-7-8 (1)
7-8:⚠️ Potential issue | 🟡 MinorDocument the default in the SYNOPSIS.
immediateis described as default false, but the SYNOPSIS doesn’t showDEFAULT false. Add it for consistency and clarity.Also applies to: 25-44
docs/spock_functions/functions/spock_repset_remove_table.md-12-18 (1)
12-18:⚠️ Potential issue | 🟡 MinorFix section heading formatting for consistency.
RETURNS,DESCRIPTION,ARGUMENTS, andEXAMPLEare missing the###heading marker used elsewhere, and the returns list mixes hyphen and em dash. Standardize headings and list markers to avoid rendering/formatting drift.Also applies to: 33-49
docs/two_node_cluster.md-78-78 (1)
78-78:⚠️ Potential issue | 🟡 MinorRemove extra backtick at end of line.
Line 78 has an extra backtick (
`) at the end that should be removed for proper markdown formatting.📝 Proposed fix
- SELECT spock.repset_add_all_tables('default', ARRAY['public']);` + SELECT spock.repset_add_all_tables('default', ARRAY['public']);docs/spock_functions/functions/spock_repset_add_partition.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorFix inconsistent function name formatting.
The NAME section uses hyphens (
spock.repset-add-partition), but the actual function name uses underscores (spock.repset_add_partition). This inconsistency could confuse users.📝 Proposed fix
-`spock.repset-add-partition ()` +`spock.repset_add_partition()`docs/spock_functions/functions/spock_repset_add_seq.md-14-15 (1)
14-15:⚠️ Potential issue | 🟡 MinorFix inconsistent bullet punctuation.
Line 14 uses a hyphen (
-) while line 15 uses an em dash (—) for bullet points in the RETURNS section. Use consistent punctuation for better formatting.📝 Proposed fix
- true if the sequence was successfully added to the replication set. - — false if the sequence was already a member of the replication set. + - false if the sequence was already a member of the replication set.
🧹 Nitpick comments (16)
docs/spock_functions/functions/spock_replicate_ddl.md (1)
7-20: Clarify return type and NULL semantics in RETURNS.The RETURNS section mixes “true/false” with “set of booleans” and the NULL rule is only in DESCRIPTION. Consider explicitly stating: single command returns boolean; array returns SETOF boolean; if any argument is NULL then result is NULL (and clarify whether NULL elements in the array yield NULL for that element or NULL for the whole call). This will avoid ambiguity for callers reading only RETURNS.
Also applies to: 39-40
docs/spock_functions/functions/spock_gen_slot_name.md (2)
41-43: Consider showing the expected output in the example.The example demonstrates the function call but doesn't show what result users should expect. Including the output would make the documentation more helpful and complete.
📚 Suggested enhancement to include expected output
### EXAMPLE SELECT spock.spock_gen_slot_name('postgres', 'n1', 'sub_n2_n1'); + +Result: +``` + spock_gen_slot_name +------------------------------ + spock_postgres_n1_sub_n2_n1 +```Note: Adjust the actual output format based on what the function actually returns.
27-40: Optional: Consider adding more context to argument descriptions.While the current argument descriptions are functional, adding brief details about expected values or constraints could improve usability. For example, clarifying whether these names must match existing objects or are arbitrary strings.
docs/spock_functions/functions/spock_sync_event.md (1)
34-34: Consider removing redundant statement.This statement is redundant with the information already provided in the DESCRIPTION section (lines 19-21 and 31-32). The fact that the function inserts a sync marker into the replication stream is clearly documented above.
📝 Proposed change
-This function modifies the replication stream by inserting a sync marker. -docs/spock_functions/functions/spock_node_drop_interface.md (1)
22-31: Remove redundant information.Lines 22-24 and lines 30-31 both mention that the function doesn't modify PostgreSQL server configuration or networking settings. This information is redundant and should be consolidated.
♻️ Proposed consolidation
Consider removing lines 30-31 since the same information is already clearly stated in lines 22-24:
Other nodes that were using this interface to connect will no longer be able to use it. Ensure no active subscriptions are relying on this interface before removing it. -This function writes metadata into the Spock catalogs but does not modify -PostgreSQL server configuration or networking settings. - Returns NULL if any argument is NULL.docs/spock_functions/functions/spock_get_country.md (1)
35-38: Consider showing the expected output in the example.The example demonstrates the function call but would be more helpful if it also showed the expected result, helping users understand what the function returns.
📖 Suggested enhancement for the example
### EXAMPLE SET spock.country = 'US'; SELECT spock.get_country(); + + Expected result: + + get_country + ------------- + US + (1 row)docs/spock_functions/functions/spock_min_proto_version.md (1)
39-41: Consider showing the expected output in the example.Including sample output would help users understand what integer values to expect from this function.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.spock_min_proto_version(); + + Expected result: + + spock_min_proto_version + ------------------------- + 1 + (1 row)docs/spock_functions/functions/spock_version.md (1)
27-29: Consider showing the expected output in the example.Displaying a sample version string would help users understand the format returned by this function.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.spock_version(); + + Expected result: + + spock_version + --------------- + 3.1.0 + (1 row)docs/spock_functions/functions/spock_version_num.md (1)
31-33: Consider showing the expected output in the example.Since the description explains the XXYYZZ encoding format, showing a concrete example output would reinforce this concept for users.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.spock_version_num(); + + Expected result: + + spock_version_num + ------------------- + 30100 + (1 row)docs/spock_functions/functions/spock_max_proto_version.md (1)
37-39: Consider showing the expected output in the example.Including sample output would help users understand what integer values represent protocol versions.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.spock_max_proto_version(); + + Expected result: + + spock_max_proto_version + ------------------------- + 2 + (1 row)docs/spock_functions/functions/spock_wait_slot_confirm_lsn.md (1)
48-51: Consider showing the expected output in the example.Even though this function returns void, showing the successful completion would help users understand the expected behavior.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.wait_slot_confirm_lsn('spock_local_sync_123456_789abc', '0/3000000'); + + Expected result (after the target LSN is reached): + + wait_slot_confirm_lsn + ----------------------- + + (1 row)docs/spock_functions/functions/spock_sub_alter_interface.md (1)
46-48: Consider showing the expected output in the example.Including the expected result would help users understand what a successful interface change looks like.
📖 Suggested enhancement for the example
### EXAMPLE SELECT spock.sub_alter_interface('sub_n2_n1', 'private_net'); + + Expected result: + + sub_alter_interface + --------------------- + t + (1 row)docs/spock_functions/functions/spock_repset_add_partition.md (1)
7-10: Clarify optional parameters in the SYNOPSIS.The ARGUMENTS section (lines 40-43) and DESCRIPTION (lines 23-24) indicate that the
partitionparameter is optional with a default of NULL, but the SYNOPSIS doesn't reflect this. Consider addingDEFAULT NULLto make the optionality explicit in the signature.📋 Suggested enhancement
spock.repset_add_partition ( parent regclass, - partition regclass, - row_filter text) + partition regclass DEFAULT NULL, + row_filter text DEFAULT NULL)docs/spock_functions/functions/spock_repset_add_seq.md (1)
7-10: Consider showing DEFAULT value in SYNOPSIS.The ARGUMENTS section (line 49) mentions "The default is false" for
synchronize_data, but the SYNOPSIS doesn't include theDEFAULTkeyword. For consistency with other updated functions likespock_repset_add_all_seqs(which showsDEFAULT false), consider adding it.📋 Suggested enhancement
spock.repset_add_seq ( set_name name, relation regclass, - synchronize_data boolean) + synchronize_data boolean DEFAULT false)docs/spock_functions/functions/spock_sub_create.md (2)
53-56: Format replication set names consistently.The accepted replication set names (
default,default_insert_only,ddl_sql) should be formatted with backticks or code formatting for consistency with other parameter values in the documentation.📋 Suggested enhancement
replication_sets An array of replication set names to subscribe to. Accepted values - are default, default_insert_only, ddl_sql. + are `default`, `default_insert_only`, and `ddl_sql`.
7-10: Consider adding DEFAULT keywords to SYNOPSIS for clarity.Similar to the comment on other files, the ARGUMENTS section describes default values for many parameters (
synchronize_structure: false,synchronize_data: false,forward_origins: '{}', etc.), but the SYNOPSIS doesn't show these defaults. While this might be a documentation style choice, adding DEFAULT keywords would improve clarity and match the style used inspock_repset_add_all_seqs.md.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mkdocs.yml`:
- Line 86: The nav entry label mistakenly includes the file extension; update
the mkdocs.yml navigation entry that currently reads "spock.gen_slot_name.md():
spock_functions/functions/spock_gen_slot_name.md" to remove ".md" from the label
so it matches other function labels (use "spock.gen_slot_name()") while keeping
the target path "spock_functions/functions/spock_gen_slot_name.md" unchanged.
🧹 Nitpick comments (1)
mkdocs.yml (1)
126-126: Consider adding parentheses for consistency with other function entries.Most function entries include parentheses (e.g.,
spock.version(),spock.sync_event()), but this entry is missing them. Adding()would maintain consistency across the navigation.Proposed fix
- - spock.wait_slot_confirm_lsn: spock_functions/functions/spock_wait_slot_confirm_lsn.md + - spock.wait_slot_confirm_lsn(): spock_functions/functions/spock_wait_slot_confirm_lsn.md
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/spock_functions/functions/spock_repset_drop.md`:
- Around line 7-8: Update the function synopsis for spock.repset_drop to show
the default for the ifexists parameter (e.g., change "spock.repset_drop
(set_name name, ifexists boolean)" to include "ifexists boolean DEFAULT false")
and ensure the ARGUMENTS section matches this default; apply the same change to
the other similar function signatures referenced around lines 47-51 so all
synopses explicitly document "DEFAULT false" for ifexists.
- Around line 33-36: Update the wording in the spock_repset_drop documentation:
change the phrase “error handling behavior” to the hyphenated form
“error-handling behavior” in the description of the ifexists parameter so the
sentence reads cleanly when describing spock_repset_drop’s ifexists behavior.
In `@docs/spock_functions/functions/spock_repset_remove_partition.md`:
- Line 7: Update the SYNOPSIS line for the function
spock.repset_remove_partition to show the optional partition argument by adding
"DEFAULT NULL" to the signature (i.e., change "spock.repset_remove_partition
(parent regclass, partition regclass)" to include the default for partition).
This aligns the SYNOPSIS with the ARGUMENTS and DESCRIPTION which state the
partition parameter defaults to NULL and clarifies callers that partition is
optional.
In `@docs/spock_functions/functions/spock_seq_sync.md`:
- Around line 3-8: The synopsis shows the SQL function signature with an extra
space before the parentheses; update the documentation to use consistent
formatting by changing the signature from "spock.sync_seq (relation regclass)"
to "spock.sync_seq(relation regclass)" so it matches other SQL signatures
(reference: spock.sync_seq in the SYNOPSIS section).
🧹 Nitpick comments (2)
docs/spock_functions/functions/spock_seq_sync.md (1)
9-12: Format boolean literal as code.Consider formatting
trueas inline code (true) to distinguish the literal value in RETURNS.docs/spock_functions/functions/spock_repset_remove_partition.md (1)
51-54: Enhance the EXAMPLE section to demonstrate NULL behavior.The DESCRIPTION extensively documents the NULL partition behavior (lines 19-21, 32-34), but the EXAMPLE only shows the specific partition case. Consider adding an example demonstrating how to remove all partitions by passing NULL.
📚 Proposed enhancement to add NULL case example
### EXAMPLE +Remove a specific partition from replication: + SELECT spock.repset_remove_partition('public.mytable', 'public.mytable_202012'); + +Remove all partitions of a table from replication: + +SELECT spock.repset_remove_partition('public.mytable', NULL);
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@docs/spock_functions/functions/spock_max_proto_version.md`:
- Around line 40-44: The example shows the wrong function: it calls
spock.spock_min_proto_version() on a page documenting
spock.spock_max_proto_version(); update the example to call
spock.spock_max_proto_version() instead (replace the function name in the SQL
example and ensure the displayed result row matches the actual max protocol
version returned by spock_max_proto_version()).
In `@docs/spock_functions/functions/spock_node_add_interface.md`:
- Around line 43-47: The example PostgreSQL DSN in the
spock_node_add_interface.md text contains an extra space after "port=" which can
lead to a malformed connection string; update the example DSN in that markdown
(the line showing "host=10.1.2.5 port= 5432 user=rocky") to remove the stray
space so it reads "host=10.1.2.5 port=5432 user=rocky" to ensure users can copy
a valid connection string.
In `@docs/spock_functions/functions/spock_node_drop_interface.md`:
- Line 13: Fix the typo in the documentation string "false it the interface did
not exist." by replacing "it" with "if" so the sentence reads "false if the
interface did not exist."; update the single occurrence in
spock_node_drop_interface.md (the sentence containing "false it the interface
did not exist.") accordingly.
In `@docs/spock_functions/functions/spock_repset_add_all_seqs.md`:
- Around line 57-60: In the EXAMPLE paragraph for repset_add_all_seqs, remove
the fragment "The:" and the extra space, and rewrite the sentence so it reads
smoothly (e.g., "For example, the first command adds the sequences in the
'public' schema to the 'default' replication set:"); update the example text
referencing repset_add_all_seqs to use this tightened sentence so the fragment
is eliminated and spacing corrected.
In `@docs/two_node_cluster.md`:
- Around line 66-69: The SQL example for creating a node in the spock extension
contains a stray trailing backtick after the closing parenthesis in the SELECT
spock.node_create(...) line; open the docs/two_node_cluster.md and remove the
extra backtick so the statement ends with ");" (i.e., SELECT spock.node_create
(node_name := 'n2', dsn := 'host=<node_2_IP_address> port=<n2_port>
dbname=<db_name>');) ensuring the fenced code block contains the corrected SQL.
- Around line 77-79: The SQL code block contains a stray trailing backtick after
the closing parenthesis in the call to spock.repset_add_all_tables('default',
ARRAY['public']); — remove the extra backtick so the statement ends with ');'
(locate the SQL line invoking spock.repset_add_all_tables and delete the
trailing ` character).
In `@mkdocs.yml`:
- Around line 120-125: The nav entries in mkdocs.yml for spock.sub_show_table
and spock.wait_slot_confirm_lsn are missing the trailing parentheses used
elsewhere; update those labels to `spock.sub_show_table()` and
`spock.wait_slot_confirm_lsn()` (keeping the same target paths
spock_functions/functions/spock_sub_show_table.md and
spock_functions/functions/spock_wait_slot_confirm_lsn.md) so naming is
consistent with other function entries like spock.sync_event() and
spock.version().
🧹 Nitpick comments (4)
docs/spock_functions/functions/spock_node_drop_interface.md (1)
20-31: Redundant statement about PostgreSQL server configuration.Lines 22-24 and 30-31 both state that the function doesn't modify PostgreSQL server configuration. Consider removing one instance to avoid repetition.
Proposed fix
This function deletes the interface definition from the Spock catalogs. It does not modify PostgreSQL server configuration or networking settings; it only removes Spock's logical representation of the interface. Other nodes that were using this interface to connect will no longer be able to use it. Ensure no active subscriptions are relying on this interface before removing it. -This function writes metadata into the Spock catalogs but does not modify -PostgreSQL server configuration or networking settings. - This command must be executed by a superuser.docs/spock_functions/functions/spock_repset_alter.md (1)
7-13: Consider clarifying optional parameter syntax in the synopsis.The synopsis shows all parameters without indicating which are optional, but the ARGUMENTS section states that boolean parameters have "default is NULL" and the DESCRIPTION mentions NULL arguments retain current settings. Consider either:
- Adding
DEFAULT NULLto the synopsis for the boolean parameters, or- Adding a note in the synopsis section that the boolean parameters are optional
This would help readers understand whether they need to pass explicit
NULLvalues or can omit trailing parameters.docs/spock_functions/functions/spock_replicate_ddl.md (2)
5-9: Synopsis should reflect both single-command and array signatures.The synopsis shows only
command text[], but the DESCRIPTION (lines 28-29) states the function "accepts either a single DDL command as text or an array of DDL commands," and the examples demonstrate both forms (single text at lines 68-69, 76-77 vs. array at lines 84-85).Consider either documenting both overloaded signatures or adding a note that single text values are implicitly converted to arrays.
📝 Suggested documentation pattern
If the function is overloaded, show both signatures:
spock.replicate_ddl (command text, ...) spock.replicate_ddl (command text[], ...)Or if it's implicit conversion, add a note:
spock.replicate_ddl (command text[], ...) Note: A single text value can be passed and will be treated as a single-element array.
7-8: Consider indicating default parameter values in the synopsis.The ARGUMENTS section documents default values for
replication_sets(line 51),search_path(line 56), androle(line 61), and the examples demonstrate calling the function with fewer arguments (lines 68-69). However, the synopsis doesn't indicate which parameters are optional or their defaults.Consider adding
DEFAULTclauses to help readers understand the minimal required signature:spock.replicate_ddl ( command text[], replication_sets text[] DEFAULT '{ddl_sql}', search_path text DEFAULT current_setting('search_path'), role text DEFAULT current_user )
mkdocs.yml
Outdated
| - spock.sub_show_table: spock_functions/functions/spock_sub_show_table.md | ||
| - spock.sub_sync(): spock_functions/functions/spock_sub_sync.md | ||
| - spock.sync_event(): spock_functions/functions/spock_sync_event.md | ||
| - spock.sub_wait_for_sync(): spock_functions/functions/spock_sub_wait_for_sync.md | ||
| - spock.version(): spock_functions/functions/spock_version.md | ||
| - spock.version_num(): spock_functions/functions/spock_version_num.md | ||
| - spock.wait_slot_confirm_lsn: spock_functions/functions/spock_wait_slot_confirm_lsn.md |
There was a problem hiding this comment.
Inconsistent naming: missing parentheses on function labels.
Two navigation entries are missing the () suffix that other function entries use:
- Line 120:
spock.sub_show_table - Line 125:
spock.wait_slot_confirm_lsn
Proposed fix
- - spock.sub_show_table: spock_functions/functions/spock_sub_show_table.md
+ - spock.sub_show_table(): spock_functions/functions/spock_sub_show_table.md
- spock.sync_event(): spock_functions/functions/spock_sync_event.md
- spock.sub_wait_for_sync(): spock_functions/functions/spock_sub_wait_for_sync.md
- spock.version(): spock_functions/functions/spock_version.md
- spock.version_num(): spock_functions/functions/spock_version_num.md
- - spock.wait_slot_confirm_lsn: spock_functions/functions/spock_wait_slot_confirm_lsn.md
+ - spock.wait_slot_confirm_lsn(): spock_functions/functions/spock_wait_slot_confirm_lsn.md📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - spock.sub_show_table: spock_functions/functions/spock_sub_show_table.md | |
| - spock.sub_sync(): spock_functions/functions/spock_sub_sync.md | |
| - spock.sync_event(): spock_functions/functions/spock_sync_event.md | |
| - spock.sub_wait_for_sync(): spock_functions/functions/spock_sub_wait_for_sync.md | |
| - spock.version(): spock_functions/functions/spock_version.md | |
| - spock.version_num(): spock_functions/functions/spock_version_num.md | |
| - spock.wait_slot_confirm_lsn: spock_functions/functions/spock_wait_slot_confirm_lsn.md | |
| - spock.sub_show_table(): spock_functions/functions/spock_sub_show_table.md | |
| - spock.sync_event(): spock_functions/functions/spock_sync_event.md | |
| - spock.sub_wait_for_sync(): spock_functions/functions/spock_sub_wait_for_sync.md | |
| - spock.version(): spock_functions/functions/spock_version.md | |
| - spock.version_num(): spock_functions/functions/spock_version_num.md | |
| - spock.wait_slot_confirm_lsn(): spock_functions/functions/spock_wait_slot_confirm_lsn.md |
🤖 Prompt for AI Agents
In `@mkdocs.yml` around lines 120 - 125, The nav entries in mkdocs.yml for
spock.sub_show_table and spock.wait_slot_confirm_lsn are missing the trailing
parentheses used elsewhere; update those labels to `spock.sub_show_table()` and
`spock.wait_slot_confirm_lsn()` (keeping the same target paths
spock_functions/functions/spock_sub_show_table.md and
spock_functions/functions/spock_wait_slot_confirm_lsn.md) so naming is
consistent with other function entries like spock.sync_event() and
spock.version().
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@docs/spock_functions/functions/spock_gen_slot_name.md`:
- Around line 7-25: Update the documentation for spock.spock_gen_slot_name:
remove the extra space before the opening parenthesis in the synopsis line
(change "spock.spock_gen_slot_name (dbname..." to
"spock.spock_gen_slot_name(dbname...") and correct the grammar in the
DESCRIPTION paragraph by changing "This function that performs a calculation
without accessing the database or modifying any data." to "This function
performs a calculation without accessing the database or modifying any data."
Also tidy heading/whitespace to match the rest of the docs for consistency.
In `@docs/spock_functions/functions/spock_replicate_ddl.md`:
- Around line 7-8: Update the function synopsis for spock.replicate_ddl to
reflect both single-text and text-array invocation forms (e.g., show command as
"command text | text[]" or show two overload signatures) so it matches the
description and examples; update all other affected synopsis lines (the similar
entries referenced at lines 28-30 and 68-77) to use the same dual-form notation
and ensure the parameter list still includes replication_sets text[],
search_path text, role text to match the implementation.
In `@docs/spock_functions/functions/spock_repset_add_partition.md`:
- Line 3: Update the NAME section to use the actual function identifier with
underscores: replace `spock.repset-add-partition ()` with
`spock.repset_add_partition ()` so it matches the SYNOPSIS and the real function
name (spock.repset_add_partition) for consistency.
In `@docs/spock_functions/functions/spock_repset_add_seq.md`:
- Around line 14-15: Fix inconsistent bullet punctuation in the RETURNS section
for spock_repset_add_seq: replace the em dash (—) on the second bullet with a
standard hyphen (-) so both lines use the same bullet character, ensuring
uniform formatting of the two return descriptions for spock_repset_add_seq in
docs/spock_functions/functions/spock_repset_add_seq.md.
In `@docs/spock_functions/functions/spock_repset_add_table.md`:
- Around line 100-102: The warning text uses the incorrect parameter name
`sync_data`; update it to the actual parameter `synchronize_data` (used in the
SYNOPSIS and ARGUMENTS) so the message reads e.g. "Using `synchronize_data=true`
with a valid `row_filter` is usually a one_time operation..." and scan the
surrounding text in spock_repset_add_table.md for any other occurrences of
`sync_data` and replace them with `synchronize_data` to keep names consistent.
In `@docs/spock_functions/functions/spock_repset_remove_table.md`:
- Around line 14-15: The RETURNS section uses inconsistent bullet characters;
replace the em dash in the second bullet with a standard hyphen so both lines
use the same bullet styling (e.g., change the em dash in the second line of the
RETURNS block for spock_repset_remove_table.md to a hyphen to match the first
line).
- Around line 12-49: The headings in the Spock function docs (RETURNS,
DESCRIPTION, ARGUMENTS, EXAMPLE) are missing Markdown heading syntax; update
each of those header lines to use the same pattern as other docs by prefixing
them with "### " (e.g., change "RETURNS" to "### RETURNS", "DESCRIPTION" to "###
DESCRIPTION", "ARGUMENTS" to "### ARGUMENTS", and "EXAMPLE" to "### EXAMPLE") so
the file matches the formatting used in spock_repset_add_seq.md and
spock_repset_add_all_tables.md.
In `@docs/spock_functions/functions/spock_sub_alter_interface.md`:
- Around line 9-34: The RETURNS section of the spock_sub_alter_interface
documentation is inconsistent with the DESCRIPTION: update the RETURNS block for
the spock_sub_alter_interface function to include NULL as a possible return
(e.g., "true if the interface was successfully changed; false if the operation
fails; NULL if any argument is NULL"), or if NULL is not actually returned by
the implementation, remove the "Returns NULL..." sentence from DESCRIPTION; make
the change in the spock_sub_alter_interface doc so RETURNS and DESCRIPTION
consistently reflect the function's actual behavior.
In `@docs/spock_functions/functions/spock_sub_create.md`:
- Around line 12-14: Update the RETURNS section for spock_sub_create to
explicitly state whether the function returns an oid or a nullable oid (e.g.,
"oid or NULL") and reconcile the text on "Returns NULL if any required argument
is NULL" by clearly listing which arguments are considered required in the
ARGUMENTS section (mark them as REQUIRED next to their names) and then stating
the exact behavior (e.g., "Returns NULL if any REQUIRED argument is NULL;
otherwise returns the OID of the newly created subscription" or, alternatively,
"Raises an error if any REQUIRED argument is NULL" if that's the intended
behavior). Ensure you reference the function name spock_sub_create and update
both the RETURNS paragraph and the ARGUMENTS entries so readers can see which
parameters are required and what happens when they are NULL.
In `@docs/spock_functions/functions/spock_sub_remove_repset.md`:
- Around line 9-14: Update the RETURNS section for spock_sub_remove_repset to
include NULL as a possible return value: add a bullet like "NULL if any argument
is NULL" so it matches the behavior described later ("Returns NULL if any
argument is NULL.") and ensures the documentation for spock_sub_remove_repset
accurately lists true, false, and NULL.
🧹 Nitpick comments (7)
docs/spock_functions/functions/spock_sub_remove_repset.md (1)
7-7: Formatting inconsistency: space before opening parenthesis.The SYNOPSIS section (line 7) includes a space between the function name and the opening parenthesis:
spock.sub_remove_repset (, while the EXAMPLE section (line 45) omits this space:spock.sub_remove_repset(. Consider standardizing the formatting for consistency throughout the documentation.♻️ Option 1: Remove space in SYNOPSIS (matches typical SQL usage)
-spock.sub_remove_repset (subscription_name name, replication_set name) +spock.sub_remove_repset(subscription_name name, replication_set name)♻️ Option 2: Add space in EXAMPLE (matches SYNOPSIS style)
- SELECT spock.sub_remove_repset('sub_n2_n1', 'custom_repset'); + SELECT spock.sub_remove_repset ('sub_n2_n1', 'custom_repset');Also applies to: 45-45
docs/spock_functions/functions/spock_seq_sync.md (1)
9-12: Consider documenting failure behavior in the RETURNS section.The RETURNS section currently only describes the success case (
true). Consider clarifying what happens on failure—does the function returnfalse, or does it raise an error? This would help users understand proper error handling.docs/spock_functions/functions/spock_sub_create.md (3)
7-10: Clarify which parameters are required vs optional in the SYNOPSIS.The synopsis lists all ten parameters but doesn't indicate which are optional. Given that the ARGUMENTS section specifies defaults for many parameters and the first example (line 97-98) only passes two arguments, most parameters appear to be optional. Consider adding
DEFAULTannotations or square brackets to distinguish required from optional parameters, following PostgreSQL documentation conventions.
73-73: Consider clarifying default value notation for complex types.The default values for arrays (lines 73, 93) use
'{}'and for interval (line 78) uses'0'. While these are valid PostgreSQL text representations, consider adding brief clarifications:
- For
'{}': note this is an empty array (you already do this on line 93 but not line 73)- For
'0': consider specifying the unit (e.g.,'0 seconds') or usingINTERVAL '0'for clarityThis would help readers unfamiliar with PostgreSQL's type notation.
Also applies to: 78-78, 93-93
97-104: Consider using named parameters consistently in examples.The first example (lines 97-98) uses positional parameters while the second (lines 100-104) uses named parameters. Given that the function accepts ten parameters, using named parameters consistently would improve clarity and make it easier for users to extend the examples. This is especially helpful when most parameters are optional and have defaults.
docs/spock_functions/functions/spock_repset_add_all_tables.md (1)
53-55: Consider more concise wording in the example description.The phrases "all of the tables" appear twice and could be shortened to "all tables" for a more concise style.
✍️ Proposed refinement
-Specify the names of one or more schemas in an array to add all of the -tables in the schemas to the specified replication set; the following -command adds all of the tables in the public schema to the default +Specify the names of one or more schemas in an array to add all tables +in the schemas to the specified replication set; the following +command adds all tables in the public schema to the default replication set:docs/spock_functions/functions/spock_repset_add_table.md (1)
7-14: Consider documenting parameter defaults in the SYNOPSIS.The ARGUMENTS section specifies defaults for
synchronize_data(line 60: "Default is false") andinclude_partitions(line 75: "Default is true"), but the SYNOPSIS doesn't reflect these defaults. While the examples correctly use named parameter syntax to omit optional parameters, explicitly showing defaults in the signature would improve clarity.📋 Optional enhancement for clarity
spock.repset_add_table ( set_name name, relation regclass, - synchronize_data boolean, + synchronize_data boolean DEFAULT false, columns text[], row_filter text, - include_partitions boolean + include_partitions boolean DEFAULT true )
| spock.spock_gen_slot_name (dbname name, provider_node name, | ||
| subscription name) | ||
|
|
||
| ### RETURNS | ||
|
|
||
| The generated replication slot name as a name type. | ||
|
|
||
| ### DESCRIPTION | ||
|
|
||
| Generates a standardized replication slot name for a Spock subscription. | ||
|
|
||
| This function creates a consistent naming convention for replication slots | ||
| based on the database name, provider node name, and subscription name. The | ||
| generated name follows Spock's internal naming scheme to ensure uniqueness | ||
| and traceability. | ||
|
|
||
| This function that performs a calculation without accessing the | ||
| database or modifying any data. It can be used to predict what slot name | ||
| Spock will generate for a given subscription configuration. |
There was a problem hiding this comment.
Tighten wording and formatting for clarity.
Minor polish: remove the extra space before ( in the synopsis and fix the grammar in “This function that performs…”.
✏️ Suggested edits
-spock.spock_gen_slot_name (dbname name, provider_node name,
+spock.spock_gen_slot_name(dbname name, provider_node name,
subscription name)
@@
-This function that performs a calculation without accessing the
+This function performs a calculation without accessing the
database or modifying any data. It can be used to predict what slot name
Spock will generate for a given subscription configuration.🤖 Prompt for AI Agents
In `@docs/spock_functions/functions/spock_gen_slot_name.md` around lines 7 - 25,
Update the documentation for spock.spock_gen_slot_name: remove the extra space
before the opening parenthesis in the synopsis line (change
"spock.spock_gen_slot_name (dbname..." to "spock.spock_gen_slot_name(dbname...")
and correct the grammar in the DESCRIPTION paragraph by changing "This function
that performs a calculation without accessing the database or modifying any
data." to "This function performs a calculation without accessing the database
or modifying any data." Also tidy heading/whitespace to match the rest of the
docs for consistency.
| spock.replicate_ddl (command text[], replication_sets text[], search_path text, | ||
| role text) |
There was a problem hiding this comment.
Align synopsis with single-command usage.
The synopsis only shows command text[], but the description and examples clearly show single text usage too. Please reflect both forms in the synopsis (e.g., overloads or command text | text[]) to avoid confusion.
Also applies to: 28-30, 68-77
🤖 Prompt for AI Agents
In `@docs/spock_functions/functions/spock_replicate_ddl.md` around lines 7 - 8,
Update the function synopsis for spock.replicate_ddl to reflect both single-text
and text-array invocation forms (e.g., show command as "command text | text[]"
or show two overload signatures) so it matches the description and examples;
update all other affected synopsis lines (the similar entries referenced at
lines 28-30 and 68-77) to use the same dual-form notation and ensure the
parameter list still includes replication_sets text[], search_path text, role
text to match the implementation.
| - true if the sequence was successfully added to the replication set. | ||
| — false if the sequence was already a member of the replication set. |
There was a problem hiding this comment.
Fix inconsistent bullet character in RETURNS section.
Line 14 uses a hyphen (-) for the bullet point, but line 15 uses an em dash (—). This inconsistency should be corrected for uniform formatting.
📝 Proposed fix
- true if the sequence was successfully added to the replication set.
- — false if the sequence was already a member of the replication set.
+ - false if the sequence was already a member of the replication set.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - true if the sequence was successfully added to the replication set. | |
| — false if the sequence was already a member of the replication set. | |
| - true if the sequence was successfully added to the replication set. | |
| - false if the sequence was already a member of the replication set. |
🤖 Prompt for AI Agents
In `@docs/spock_functions/functions/spock_repset_add_seq.md` around lines 14 - 15,
Fix inconsistent bullet punctuation in the RETURNS section for
spock_repset_add_seq: replace the em dash (—) on the second bullet with a
standard hyphen (-) so both lines use the same bullet character, ensuring
uniform formatting of the two return descriptions for spock_repset_add_seq in
docs/spock_functions/functions/spock_repset_add_seq.md.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/spock_functions/functions/spock_wait_slot_confirm_lsn.md`:
- Around line 50-54: Update the example in the docs so the slot name in the
descriptive text and the SQL call match (use the same slot identifier for both),
e.g., change either `spock_local_sync_346_79a` or
`spock_local_sync_123456_789abc` so they are identical; also reformat the SQL
example for readability by placing the entire SELECT call on one line or using a
clear two-line indentation for the function and its arguments referencing
spock.wait_slot_confirm_lsn so the example is consistent and easy to read.
ibrarahmad
left a comment
ibrarahmad
left a comment
There was a problem hiding this comment.
I have added some comments, and please check that
coderabbitai comments too.
| to a database named 'inventory': | ||
|
|
||
| `spock.node_create ('n1', 'host=10.1.2.5 user=rocky dbname=demo')` | ||
|
|
There was a problem hiding this comment.
Just curious why we have removed that?
There was a problem hiding this comment.
I'm not totally sure what 'that' refers to (the lines in the file don't line up any more), but if it's the example of the command, I've moved all of the examples to the end of the function files and added commentary so we explain what the example is doing. I'm trying to make our online help more like real documentation...
|
|
||
| spock.node_drop (node_name name, ifexists boolean DEFAULT false) | ||
|
|
||
| ### RETURNS |
There was a problem hiding this comment.
I think we need same level of info for node_create()
There was a problem hiding this comment.
We'll have the same sections for each command - a synopsis, a returns section, the properties section (where I list the defaults - this DEFAULT shouldn't be here for readability), a description, and an example.
| ## NAME | ||
|
|
||
| `spock.replicate_ddl()` | ||
| spock.replicate_ddl() |
There was a problem hiding this comment.
This is also a psql command like you did changes for node_create and node_drop
| spock.repset_add_all_seqs() | ||
|
|
||
| ### SYNOPSIS | ||
|
|
There was a problem hiding this comment.
This also a psql command and have same sytax you use for node_create and node_drop, check for all other commands
There was a problem hiding this comment.
I'm not sure I understand this comment... what is different here?
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/install_spock.md (1)
71-73:⚠️ Potential issue | 🟡 MinorUse the standard
make installtarget.
make-installwill fail on typical builds.🐛 Proposed fix
-`make-install` +`make install`
🤖 Fix all issues with AI agents
In `@docs/configuring.md`:
- Around line 205-209: Update the inline code formatting in the
"spock.save_resolutions" section so only the table name is backticked: change
the string currently rendered as `spock.resolutions table` to
`spock.resolutions` table; edit the text around the `spock.save_resolutions`
paragraph (look for the heading or the literal `spock.save_resolutions` and the
phrase mentioning the resolutions table) and replace the combined backticks with
backticks that wrap only the `spock.resolutions` identifier.
- Around line 39-56: The GUC heading levels jump from h2 to h4 (e.g., the
headings for `spock.allow_ddl_from_functions` and `spock.batch_inserts`) which
violates markdown heading order; update those GUC headings (and all similar
`spock.*` GUC headings) to use h3 (###) instead of h4 (####) so they increment
properly under the section title, ensuring consistent heading hierarchy
throughout the file.
In `@docs/two_node_cluster.md`:
- Around line 88-91: Update the placeholder in the SQL snippet for
spock.sub_create to match the standard underscore format used elsewhere: replace
the `<n1 port>` placeholder with `<n1_port>` (so the provider_dsn uses
host=<node_1_IP_address> port=<n1_port> dbname=<db_name>), ensuring consistency
with other placeholders like `<n1_port>` in the docs.
- Around line 3-12: Remove the stray line that contains only ":" between the
paragraph starting with "After installing Postgres..." and the following fenced
code block; delete that lone ":" so the paragraph flows directly into the
"```sql" code fence (i.e., remove the orphan colon line).
Removed default value for 'ifexists' parameter in spock.node_drop function.
Removed parameter list from spock.node_info() synopsis.
2 participants
I'll use this branch to manage updates to the documentation; my primary initial focus is on the online help, but I'm finding updates required in other content that will be rolled in as well.
The publish_spock branch in pgedge-docs is set up to publish this content on Cloudflare...