apache / apache/shardingsphere

[Discussion]Design contract for SQL Server OPENQUERY encrypt rewrite

Open
#39,313 3 comments 0 reactions 0 assignees View on GitHub
db: SQLServer feature: encrypt type: discussion
Dominant language
Java
Stars
20.8k
Forks
6.9k
Avg merge
11h 38m
Merged PRs (30d)
326

Description

## Feature Request

**For English only**, other languages will not be accepted.

Please pay attention on issues you submitted, because we maybe need more details.
If no response anymore and we cannot make decision by current information, we will **close it**.

Please answer these questions before submitting your issue. Thanks!

### Is your feature request related to a problem?
Yes. PR #39156 explored encrypt rewrite for SQL Server `UPDATE OPENQUERY (...) SET ...`, but review showed that the current approach is blocked by missing design contracts rather than by more local patches.

OPENQUERY pass-through SQL is executed by the linked-server provider, not by the local SQL Server engine. The current implementation:
1. looks up Encrypt metadata mainly by the terminal table name from the inner SQL;
2. owns a SQL Server-style inner SQL scanner/rewriter inside the Encrypt module;
3. always emits SQL Server bracket-quoted physical column names;
4. does not authoritatively detect linked-server provider identity.

As a result, it may rewrite the wrong remote table, emit SQL for the wrong dialect, or silently skip Encrypt rewrite. Review retained PR #39156 as Needs Discussion and recommended opening a focused design issue before infrastructure or Encrypt integration work.

This issue is a design follow-up to https://github.com/apache/shardingsphere/issues/30227 and PR [#39156](https://github.com/apache/shardingsphere/pull/39156). Implementation should not start until the contract below is accepted.

### Describe the feature you would like.
* The supported provider boundary and how it is authoritatively detected.

**Scope**: local dialect = SQL Server, linked-server provider = SQL Server only. Heterogeneous providers (Oracle/MySQL/PostgreSQL/unknown) are out of scope.

**Authoritative detection**: ShardingSphere can not query the remote sys.servers catalog at rewrite time, so the provider identity must come from explicit user configuration (e.g. a `databaseType` declared per linked server), never inferred from the linked-server name. Any linked server absent from the configuration is treated as unknown and rejected fail-closed.
* The ownership and source of linked-server and remote-table metadata.

**Linked-server metadata ownership/source**:Candidate owners: (a) inside the Encrypt rule configuration — simplest, but not reusable by other features; (b) a standalone rule/metadata section (e.g. an openQuery or linkedServers global entry) — reusable, but adds a new top-level concept; (c) database connection metadata layer.

My proposal is (b): linked-server identity is a property of the data topology, not of the Encrypt feature, and future features (sharding, mask) may need the same mapping. The source is static YAML/DistSQL configuration maintained by the user; ShardingSphere does not attempt runtime discovery.

**Remote-table metadata ownership/source**: Remote-table metadata should be owned together with the linked-server entry, not looked up by bare terminal table name inside Encrypt.
Proposed shape: under each configured linked server, declare remote identities (catalog.schema.table) and map them to a local Encrypt logical table.
Encrypt still owns column-level encryption rules; the linked-server section only owns remote identity → Encrypt table binding.
If a remote identity cannot be resolved from this mapping, rewrite fails closed and must not fall back to EncryptRule.findEncryptTable(terminalName).

* How remote identity maps to Encrypt metadata.

Replace terminal-table-name lookup with an explicit identity model:
RemoteTableIdentity = linkedServer + catalog + schema + table

Each identity maps to at most one Encrypt logical table via configuration (e.g. MyLinkedServer + HumanResources.dbo.Department → encrypt table Department). Rewrite happens only when the full identity resolves to a configured mapping. A bare terminal name with no linked-server mapping must not fall back to EncryptRule.findEncryptTable(tableName); it fails closed. This guarantees that same-named tables on different linked servers never share Encrypt metadata accidentally.

* Whether existing parser infrastructure can be reused or a new extension contract is justified.

Option 1 — reuse the existing DatabaseType-driven ANTLR parser: once the provider is known to be SQL Server, parse the inner SQL with the existing SQL Server parser and reuse existing rewrite token infrastructure. Pros: no parallel parser, full grammar coverage, consistent maintenance. Open question: whether the current rewrite engine can operate on a nested SQL string inside an outer statement's literal argument, and how token offsets compose.

Option 2 — a dedicated extension contract (e.g. OpenQueryDialectHandler with extractRemoteTableIdentity / validateSupportedShape / rewriteProjection): justified only if Option 1 is proven impractical. Encrypt would then apply only encryption semantics (logical→physical mapping, fail-closed checks) on top of the dialect result.

My default preference is Option 1; Option 2 requires evidence that nested-statement rewriting cannot be expressed in the existing infrastructure. Either way, the hand-written scanner currently inside the Encrypt module should be retired, not extended.
1.Run a focused feasibility spike: parse the OPENQUERY inner SQL string with the existing SQL Server DatabaseType-driven parser and verify that the supported narrow `SELECT ... FROM` shape produces a usable AST (including comments, delimited identifiers, and regular identifiers with `$ / @ / #`)
2.Validate whether the existing rewrite/token infrastructure can operate on that nested statement and how token offsets compose with the outer literal argument.
3.Only if the spike exposes a real gap in the existing parser or rewrite path, open a dedicated follow-up PR for that infrastructure gap.
4.Migrate the useful boundary scenarios from PR #39156 into parser/infrastructure tests and Encrypt integration tests according to ownership; retire the hand-written Encrypt
scanner rather than growing it further.

In short: Option 1 means “prove reuse first, then implement on top of the shared parser/rewriter,” not “keep patching `EncryptOpenQueryPassThroughSQL`.”

* Fail-closed behavior, configuration compatibility, and runtime verification.

Fail-closed rules: rewrite is skipped only when the target is provably not an encrypted remote table. In every other uncertain case — unconfigured linked server, non-SQLServer provider, unmapped remote identity, unsupported inner SQL shape, encrypted predicate, encrypted SET column absent from the projection — throw UnsupportedEncryptSQLException instead of silently passing SQL through.

Configuration compatibility: the linked-server section is additive and optional. Users without it keep today's behavior (OPENQUERY untouched by Encrypt); no existing configuration changes meaning.

Runtime verification: unit tests for identity resolution and fail-closed branches; rewriter IT cases for accepted shapes; the existing test scenarios in PR #39156 will be migrated. Whether a real linked-server E2E environment is feasible in CI is an open question for the community.

Contributor guide

Open the contributing guide

Research direction

Start with PR #39156 and the existing EncryptOpenQueryPassThroughSQL path, then run the proposed feasibility spike using the SQL Server DatabaseType-driven parser for the narrow SELECT ... FROM shape. Check nested rewrite support, token-offset composition, and boundary cases such as comments and delimited identifiers. Done means the community accepts the provider, metadata, identity-mapping, parser-reuse, and fail-closed contracts before implementation begins.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, sql
Domain
backend-api-design, databases
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.