feat: optional driver-owned table query templates - #818
Conversation
|
Matching SQL Server implementation: TabularisDB/tabularis-sqlserver-plugin#31 (stacked on the pagination fix in #30 of that repository). The new flag must be registered in the Tabularium driver-kind schema before publishing the plugin; registry code and production configuration remain untouched. |
Preview buildThe preview build of commit |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| | `connection_string` | bool | Set `false` to hide the connection string import UI for this driver. Defaults to `true` for network drivers. `file_based` and `folder_based` drivers skip the import UI automatically regardless of this flag. | | ||
| | `connection_string_example` | string | Optional placeholder example shown in the connection string import field (e.g. `"clickhouse://user:pass@localhost:9000/db"`). Also accepted as camelCase `connectionStringExample`. | | ||
| | `identifier_quote` | string | Character used to quote SQL identifiers. Use `"\""` for ANSI standard or `` "`" `` for MySQL style. | | ||
| | `table_query_templates` | bool | Opts the Generate SQL dialog into the optional `get_table_query_template` RPC for SELECT/UPDATE/DELETE previews. Defaults to `false`; older plugins and built-in drivers keep the existing host templates. See [Table Query Templates](#table-query-templates). | |
There was a problem hiding this comment.
WARNING: New capability is missing from plugins/manifest.schema.json
plugins/manifest.schema.json declares capabilities with "additionalProperties": false and lists every other capability (inline_pk, manage_tables, materialized_views, …) under capabilities.properties. This PR documents table_query_templates in the guide but does not add it to that schema, so any manifest that sets the flag is rejected by the schema the guide itself points plugin authors to (plugins/manifest.schema.json, linked from PLUGIN_GUIDE.md and from the create-plugin template README). The Tabularium-side rollout mentioned below does not cover this in-repo artifact.
Add a "table_query_templates": { "type": "boolean", "default": false, ... } entry to capabilities.properties so the documented flag validates.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Addressed in a54684c: both plugins/manifest.schema.json and plugins/tabularium-extensions.schema.json now declare the optional, default-false property. Added two schema regression tests in tests/utils/tableQueryTemplateSchema.test.ts.
| are unchanged. This is not a replacement for the existing DDL methods. | ||
| - Older hosts ignore the new manifest capability and never call the method. | ||
| Plugins need not raise `min_runtime_version` solely for this optional feature. | ||
| - The capability is a static manifest opt-in, not a connection-metadata override. |
There was a problem hiding this comment.
SUGGESTION: A plugin echoing this capability in get_connection_metadata will fail the whole metadata parse
ConnectionCapabilityOverrides (src-tauri/src/plugins/connection_metadata.rs:35) is #[serde(deny_unknown_fields)] and its apply! list does not include table_query_templates. A plugin that returns its full capability set from get_connection_metadata — which this guide now makes easy to do, since the field is documented as a capability — will make deserialization fail with an unknown-field error, failing the entire connection metadata load rather than just ignoring the key.
Consider either accepting (and ignoring) table_query_templates in the overrides struct, or noting in the guide that plugins must not return the key from get_connection_metadata.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
This is intentionally a static manifest opt-in. The get_table_query_template compatibility section already states: "The capability is a static manifest opt-in, not a connection-metadata override." Plugins must not echo the full static capability set through the restricted get_connection_metadata override object. Connection snapshots inherit table_query_templates from the installed manifest, so keeping the existing deny_unknown_fields allowlist is deliberate.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The two findings from the previous review are resolved by the latest commits: Files Reviewed (5 files)
Assumptions & Not Verified
Previous Review Summary (commit 288c20a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 288c20a)Status: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (16 files)
Assumptions & Not Verified
Reviewed by free · Input: 0 · Output: 0 · Cached: 0 |
1 participant
Summary
Backward compatibility
Validation
Coordination
Refs TabularisDB/tabularis-sqlserver-plugin#26.
Plugin pagination fix: TabularisDB/tabularis-sqlserver-plugin#30.
The matching plugin implementation is on feat/table-query-templates; this draft freezes the additive contract for joint review.
Tabularium
Before publishing the plugin, register the optional boolean table_query_templates (default false) under the driver-kind capabilities schema. Ingestion uses lenient AJV validation with removeAdditional: all, which otherwise strips the flag. No database migration, endpoint or SDK change is needed. Registry production configuration has not been modified.