Support encrypt rewrite for SQL Server OPENQUERY UPDATE with narrow SELECT ... FROM shape

[ClaireLytt]

for #30227
official document: https://learn.microsoft.com/en-us/sql/t-sql/queries/update-transact-sql?view=sql-server-ver17#p-updating-data-in-a-remote-table-by-using-the-openquery-function

Changes proposed in this pull request:

---

Before committing this PR, I'm sure that I have checked the following options:

• My code follows the code of conduct of this project.
• I have self-reviewed the commit code.
• I have (or in comment I request) added corresponding labels for the pull request.
• I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
• I have made corresponding changes to the documentation.
• I have added corresponding unit tests for my changes.
• I have updated the Release Notes of the current development version. For more details, see Update Release Note

[ClaireLytt]

Problem
UPDATE OPENQUERY (...) targets a FunctionTableSegment, not a SimpleTableSegment. Encrypt rewrite only checked TablesContext.getSimpleTables(), so OPENQUERY updates were skipped and Logic SQL equaled Actual SQL (logical columns/values were not rewritten).

Approach

1. Decorator gate — For OPENQUERY updates, detect encrypt tables by parsing the inner FROM schema.table in the OPENQUERY SQL string.
2. Generator gate — Let EncryptUpdateAssignmentTokenGenerator run when the update target is OPENQUERY.
3. Rewrite — In EncryptAssignmentTokenGenerator, rewrite both:
   • SET clause: logic column → cipher column + encrypted value
   • OPENQUERY string: logic column → cipher column (EncryptOpenQuerySQLToken)

Shared logic — EncryptOpenQueryUtils centralizes OPENQUERY detection, table matching, and schema extraction

[terrymanu]

Summary

Review Result: Not Mergeable

Feedback Mode: Change Request

Reason: The PR identifies the correct root cause—OPENQUERY is represented as a FunctionTableSegment—but the latest head still has three functional blockers and one repository-standard violation.

Issues

P1 — Parameterized assignments are not encrypted

Problem: The new token path supports SET GroupName = ?, but the parameter rewriter still resolves the table exclusively from ColumnSegmentBoundInfo#getOriginalTable() and skips the assignment when that value is empty. Function-table columns use the temporary-table binding path, which the new test also models with an empty original table. See EncryptAssignmentTokenGeneratorTest.java#L146-L156 and EncryptAssignmentParameterRewriter.java#L68-L84.

Impact: A prepared UPDATE OPENQUERY(...) SET GroupName = ? rewrites the target column to the cipher column but sends the original plaintext parameter.

Required Change: Please make assignment-parameter rewriting resolve the OPENQUERY encrypt table and schema through the same target context as literal assignments. Add a rewrite test using an outer parameter marker, including derived cipher/assisted/like parameters where configured.

P1 — Multiple assignments produce overlapping OPENQUERY tokens

Problem: The generator creates one full-range EncryptOpenQuerySQLToken for every encrypted assignment (EncryptAssignmentTokenGenerator.java#L90-L135). Tokens for the same string literal have identical ranges, and the rewrite engine retains only the first overlapping token (AbstractSQLBuilder.java#L45-L55).

Impact: For two encrypted columns, only one inner projection is rewritten while both outer assignments use physical column names. Configurations with assisted or like-query columns have the same mismatch because those physical columns are added to the outer assignment but not the OPENQUERY rowset. SQL Server permits multiple SET assignments and requires updated columns to exist in the target rowset (Microsoft UPDATE documentation).

Required Change: Please generate one composed inner-query token per OPENQUERY target from the complete physical-column mapping. Cover two encrypted assignments and a column with assisted/like-query derived columns.

P1 — Regex rewriting can change pass-through query semantics

Problem: Table discovery uses FROM\s+([^\s,;]+), and column rewriting replaces every matching word in the raw pass-through string (EncryptOpenQueryUtils.java#L41-L116, EncryptAssignmentTokenGenerator.java#L131-L135). OPENQUERY’s second argument is an opaque query executed by the linked server, and Microsoft’s examples include quoted predicates (Microsoft OPENQUERY documentation).

For example, rewriting GroupName also changes the literal in WHERE Note = ''GroupName''. The table matcher also fails on a valid delimited identifier such as [Human Resources].[Department].

Impact: Valid SQL can select a different remote rowset or silently bypass encryption-table detection.

Required Change: Please replace raw regex rewriting with a structure-aware, quote-aware mechanism appropriate to the supported linked-server dialect. If support is intentionally limited to a narrower query shape, enforce that boundary and document it. Add cases for string literals, delimited identifiers, multipart names, and predicates on encrypted columns.

P2 — New methods use Optional parameters

Problem: The new internal flow passes Optional<TableSegment> and Optional<OpenQueryContext> as method parameters (EncryptAssignmentTokenGenerator.java#L90-L128). The repository coding standard explicitly forbids Optional method parameters and requires minimal access control (CODE_OF_CONDUCT.md#L61-L65).

Impact: The patch does not meet the project’s production-code quality gate and makes the normal/OpenQuery paths less explicit.

Required Change: Please keep Optional as a return/local representation and express the two execution paths without optional parameters. Also use the minimum required visibility for the new overload.

Review Details

• Review Focus: Code Correctness Review. CI not reviewed by request.
• Reviewed Scope: All 13 files in features/encrypt/core, test/it/rewriter, and RELEASE-NOTES.md at head d04c4f8d45adf970516b1f4dc3c3fdc9e2a5e39f; base master SHA 6ecd9c15b0ea089c6036fa4d1776098a19b03cb1; merge-base cb50781bc2a08ba20d7b9a66ec5e017fc2bf17c5. The local triple-dot file list matched GitHub /pulls/39156/files.
• Not Reviewed Scope: GitHub Actions/check runs and live SQL Server/OLE DB execution. These omissions do not affect the static blockers above.
• Verification: Public PR metadata, files, issue comments, and reviews were refreshed successfully; the head and file list remained unchanged. Local source-path and token-overlap analysis completed successfully. Maven was not run because an isolated latest-head checkout was unavailable; the findings do not depend on build or CI results.
• Release Note / User Docs: The release-note entry is present. User documentation is missing for the implementation’s current query-shape restrictions; it is required if support remains intentionally limited.

[ClaireLytt]

P1 — Parameterized assignments are not encrypted
→ Code: EncryptAssignmentParameterRewriter.java
→ Unit tests: EncryptAssignmentTokenGeneratorTest
→ update.xml: add new tests for SET GroupName = ? and expands to cipher / assisted / like parameters

P1 — Multiple assignments produce overlapping OPENQUERY tokens
→ Code: EncryptAssignmentTokenGenerator.java
→ Unit tests: EncryptAssignmentTokenGeneratorTest
→ update.xml:

• two encrypted columns rewritten together
• derived cipher/assisted/like columns

P1 — Regex rewriting can change pass-through query semantics
→ Code: EncryptOpenQueryPassThroughSQL.java — structure/quote-aware parse + SELECT-list-only rewrite; unsupported shapes throw UnsupportedEncryptSQLException. EncryptOpenQueryUtils.java / EncryptAssignmentTokenGenerator.java delegate to it (no raw whole-string regex replace).
→ Unit tests: EncryptOpenQueryPassThroughSQLTest — multipart / delimited names; WHERE on encrypted columns preserved; string literal Note = 'GroupName' not rewritten; unsupported SELECT literal/expression, space-delimited id, three-part name, JOIN rejected; derived-column rewrite. EncryptAssignmentTokenGeneratorTest#assertGenerateSQLTokenWithOpenQueryPreservesWhereClauseColumnRef / #assertGenerateSQLTokenWithUnsupportedOpenQuerySelectLiteralExpectsException.
→ update.xml:

• *_predicate_on_encrypted_column_for_literals — WHERE column name left as-is
• *_string_literal_predicate_for_literals — literal containing column name not rewritten
• *_delimited_multipart_table_name_for_literals / *_for_parameters — [HumanResources].[Department] supported

P2 — New methods use Optional parameters
Fixed. Split into two overloads without optional/null parameters: generateSQLTokens(tables, set) for normal UPDATE and generateSQLTokens(tables, set, openQueryTable) for OPENQUERY. The caller keeps the table segment as a local and routes by path. The OPENQUERY overload is package-private.

[terrymanu]

Summary

Review Result: Not Mergeable

Feedback Mode: Change Request

Reason: The latest head fixes the earlier parameter-rewrite and overlapping-token defects, but the new pass-through parser still permits encryption bypasses and incorrect physical SQL. The result is code-scope only; CI was not reviewed by request.

Issues

P1 — Encrypted predicates remain on logical columns

Problem: rewrite transforms only the SELECT list and appends the entire remainder unchanged (EncryptOpenQueryPassThroughSQL.java#L116-L123). The new test explicitly expects WHERE GroupName IS NOT NULL to remain logical while the projection becomes group_name_cipher (EncryptOpenQueryPassThroughSQLTest.java#L67-L75).

The changed configuration defines GroupName with only group_name_cipher, not a plaintext query column (query-with-cipher.yaml#L161-L180). Normal encrypt rewriting replaces predicate columns with their cipher or assisted-query columns (EncryptPredicateColumnTokenGenerator.java#L112-L122). OPENQUERY executes its pass-through query on the linked server, so the outer rewrite cannot repair that unchanged predicate later (Microsoft OPENQUERY documentation).

Impact: Predicates on encrypted columns can fail against the physical table, query an optional stale plaintext column, or select a different rowset from normal encrypt semantics before the update is applied.

Required Change: Please either rewrite predicates using the full EncryptTable query semantics—including encrypted literal values—or reject pass-through predicates that reference encrypted logical columns. Replace the current “preserves encrypted predicate” expectations with physical-predicate or explicit-unsupported assertions.

P1 — Identifier handling can silently bypass encryption or emit invalid physical SQL

Problem: Target discovery accepts only bracketed or alphanumeric/underscore identifier parts (EncryptOpenQueryPassThroughSQL.java#L258-L282). For a valid SQL Server target such as "HumanResources"."Department", discovery returns empty, findEncryptTable returns empty (EncryptOpenQueryUtils.java#L76-L82), and the decorator returns before validation or rewriting (EncryptSQLRewriteContextDecorator.java#L54-L57). SQL Server supports both bracketed and double-quoted delimited identifiers (Microsoft identifier documentation).

The emitter also writes cipher, assisted, and like-query names without identifier quoting (EncryptOpenQueryPassThroughSQL.java#L359-L363). Encrypt configuration validation requires these names to be non-empty but does not restrict them to regular identifiers (EncryptRuleConfigurationChecker.java#L64-L77).

Impact: A valid delimited target can be forwarded without encryption, while configured physical names containing spaces, reserved words, or special characters produce invalid pass-through SQL.

Required Change: Please use one authoritative identifier model for target discovery, validation, and emission. It should support or deliberately reject valid delimiter forms only after identifying the encrypt target, handle escaped delimiters, and quote physical column names for the supported remote dialect—or reject configurations that cannot be emitted safely. Add end-to-end cases for double-quoted targets, escaped bracket identifiers, and physical names requiring delimiters.

P1 — The narrow query-shape gate accepts unsupported multi-source and set queries

Problem: validateRemainder rejects only text containing JOIN (EncryptOpenQueryPassThroughSQL.java#L158-L163). Therefore, examples such as these pass validation:

• SELECT GroupName FROM dbo.Department, dbo.Other
• SELECT GroupName FROM dbo.Department UNION SELECT GroupName FROM dbo.Other
• SELECT GroupName FROM dbo.Department CROSS APPLY ...

The implementation rewrites only the first SELECT list and appends the remainder unchanged. SQL Server officially supports comma-separated table sources and APPLY in FROM, as well as UNION query expressions (Microsoft FROM documentation, Microsoft UNION documentation).

Impact: Shapes outside the claimed narrow boundary can receive a partial logical-to-physical rewrite instead of the promised explicit unsupported error, producing mixed rowset semantics or provider-dependent failures.

Required Change: Please validate the complete pass-through structure before changing parameters or generating tokens. Explicitly reject every table-source, set-operation, and trailing-clause form outside the supported grammar, and add counterexample tests for comma sources, APPLY, UNION, EXCEPT, and INTERSECT.

P2 — The user-facing support boundary is undocumented

Problem: The release note claims general OPENQUERY update support (RELEASE-NOTES.md#L79), while the implementation rejects SELECT expressions, SELECT literals, space-containing identifiers, three-part table names, and JOINs (EncryptOpenQueryPassThroughSQL.java#L36-L46). Neither encrypt limitations document was updated.

Impact: Users cannot determine the accepted query shape, linked-server/provider boundary, or migration limitations without encountering runtime exceptions or incorrect rewrites.

Required Change: Please narrow the release-note wording and document the exact supported OPENQUERY shape and rejected predicates, identifiers, table sources, set operations, and remote-dialect assumptions in both limitations.en.md and limitations.cn.md.

Multi-Round Comparison

• Fixed: Outer parameter markers now resolve the OPENQUERY encrypt table, including assisted/like derived parameters, with integration coverage.
• Fixed: Multiple assignments now produce one composed OPENQUERY token.
• Partially fixed: Raw whole-string regex replacement was removed and string literals are preserved, but encrypted predicates and unsupported identifier/query shapes remain incorrect.
• Fixed: The assignment generator no longer passes Optional parameters, and the OPENQUERY overload now has package-private visibility.
• Newly introduced: The custom pass-through parser’s detector, validator, and emitter do not share a complete fail-closed identifier/query model.

Review Details

• Review Focus: Code Correctness Review. CI not reviewed by request.
• Reviewed Scope: All 16 changed files in features/encrypt/core, test/it/rewriter, and RELEASE-NOTES.md at head f115808b981a886bb39f593200871c1b30099a31; base master SHA 6ecd9c15b0ea089c6036fa4d1776098a19b03cb1; local merge-base cb50781bc2a08ba20d7b9a66ec5e017fc2bf17c5. The final local triple-dot file list matched GitHub /pulls/39156/files.
• Not Reviewed Scope: GitHub Actions/check runs and live SQL Server/OLE DB provider execution. These omissions do not affect the source-path and contract blockers above.
• Verification: GitHub PR metadata, files, commits, comments, and reviews were refreshed after the head changed and again before publication. The exact-head review inventory completed successfully with all 16 files matched. Maven was not run because the active checkout was not the PR head and differed in every changed file; the findings are proven directly by the latest-head source paths and official SQL Server contracts.
• Release Note / User Docs: Release-note entry present but broader than the implementation. Required English and Chinese limitations documentation is missing.

[ClaireLytt]

This PR adds encryption rewrite support for SQL Server UPDATE OPENQUERY(...) statements.

It detects encrypted tables inside FunctionTableSegment, rewrites logical columns and assignment values to physical encrypted columns, and supports literal and parameterized assignments, multiple columns, assisted-query columns, and like-query columns.

It also introduces structure-aware parsing for the inner SELECT ... FROM query, safely handles quoted identifiers, preserves string literals, and explicitly rejects unsupported query shapes. Comprehensive unit and SQL rewrite integration tests cover the new behavior and edge cases.

I may open a follow-up issue to address the OPENQUERY scenarios that aren't supported in this PR.

[terrymanu]

Summary

• Review Result: Not Mergeable
• Feedback Mode: Change Request
• Reason: The current revision follows the correct root-cause direction, but four P1 correctness gaps can still reject valid OPENQUERY statements, bypass encryption rewrite, or emit malformed/unsupported pass-through SQL.

Issues

P1 — OPENQUERY text is parsed and emitted at the wrong escaping layer

Problem:

SQL Server supplies the pass-through query as a T-SQL string literal. StringLiteralValue removes the outer quotes but retains doubled inner apostrophes, and the SQL Server visitor forwards that value unchanged.

The new scanners operate as if they received decoded inner SQL. Consequently, production text such as WHERE Note = ''GroupName'' exposes GroupName to the scanner as an identifier outside a string. Likewise, ''UNION ALL'' is misclassified as a set operation. The current direct parser tests use already-decoded single quotes and therefore do not cover the production representation.

The reverse path has the same boundary problem: quotePhysicalColumnName permits apostrophes, while EncryptOpenQuerySQLToken.toString() wraps rewritten SQL without doubling inserted apostrophes. A physical name such as foo'bar therefore produces a malformed outer literal.

This escaping requirement is visible in Microsoft’s OPENQUERY examples, which use doubled apostrophes inside the outer literal.

Impact:

Valid supported queries containing string predicates can be rejected, and valid physical identifiers containing apostrophes can produce syntactically broken rewritten SQL.

Required Change:

Model the boundary explicitly: decode one T-SQL string-literal layer before lexing the pass-through query and escape it exactly once when emitting the outer literal. Add production-path rewrite tests using doubled predicate literals and a physical column name containing an apostrophe.

P1 — Target-table discovery can fail open on valid delimited identifiers

Problem:

findFromKeywordIndexIfPresent tracks string literals but not bracketed or double-quoted identifiers. For valid SQL such as:

SELECT [FROM], GroupName FROM dbo.Department

it treats the first [FROM] as the clause delimiter. Current-head diagnostics confirm that findTableName then returns empty. EncryptOpenQueryUtils.findEncryptTable cannot identify the encrypt table, so the decorator exits without rewriting.

Bracketed reserved words are valid delimited identifiers according to Microsoft’s identifier rules.

Impact:

A valid encrypted target silently bypasses encryption rewrite instead of being rewritten or rejected, leaving logical columns and plaintext assignments in the SQL sent downstream.

Required Change:

Use one delimiter-aware lexical model for both target discovery and later validation, covering strings, brackets, double quotes, and escaped delimiters. Once an encrypted OPENQUERY target is identified, every unsupported shape must fail closed. Add a full decorator/rewrite regression test with [FROM] in the projection.

P1 — The documented narrow single-SELECT grammar is not enforced

Problem:

The documentation limits support to SELECT <columns> FROM ... [WHERE ...] and explicitly excludes expressions and multiple statements. However, validateColumnIdentifier accepts numeric and keyword expressions such as 1 or NULL, while validateRemainder rejects only a subset of table-source and set-operation forms.

Current-head diagnostics confirm that all of these are accepted:

SELECT 1 FROM dbo.Department
SELECT GroupName FROM dbo.Department; DELETE FROM dbo.Other
SELECT GroupName FROM dbo.Department ORDER BY DepartmentID

This contradicts the documented supported shape and exclusions.

Impact:

The rewrite can generate an update against a rowset that does not expose the required physical column, or forward additional clauses/statements after partially rewriting only the first projection.

Required Change:

Validate the complete supported single-statement grammar before rewriting: every projection item must be an allowed identifier, and only explicitly supported table hints and an optional WHERE remainder may follow the table reference. Reject statement terminators, additional clauses, numeric/keyword expressions, and all unrecognized trailing forms. Cover these counterexamples through the full rewrite boundary.

P1 — Validation and pass-through rewrite are skipped when SET has no encrypt column

Problem:

generateSQLTokens invokes the composed pass-through rewrite only when hasEncryptAssignment becomes true. Because validation of the inner query is performed inside that rewrite, a statement such as:

UPDATE OPENQUERY(server,
  'SELECT GroupName, DepartmentID FROM dbo.Department WHERE GroupName IS NOT NULL')
SET DepartmentID = 5

is neither rewritten nor rejected, even though the documented contract explicitly rejects encrypted predicates.

Impact:

Logical encrypted columns and predicates can be sent unchanged to the remote table, causing invalid SQL or incorrect row selection.

Required Change:

Validate and rewrite the pass-through target whenever an encrypted target table is identified, independently of whether the outer assignment itself targets an encrypted column. If non-encrypted assignments are outside the supported contract, reject them explicitly instead of silently skipping the rewrite. Add integration coverage for this branch.

Multi-Round Comparison

• Fixed from the first review round: parameterized assignment rewriting, multiple-assignment token composition, derived-query parameter handling, and Optional method parameters.
• Fixed or improved from the second round: explicit documentation and release notes were added; comma sources, joins, apply operators, set operations, and encrypted predicates are rejected when the parser is reached.
• Partially fixed: replacing regex rewriting with structured scanning is the right direction, but escaping-layer handling, identifier-aware discovery, complete grammar validation, and fail-closed behavior remain incomplete.
• The issues above apply to the current head only; resolved earlier findings are not repeated as blockers.

Review Details

• Review Focus: Code Correctness Review. CI not reviewed by request.
• Reviewed Scope: all 20 changed files at head 6e5ec36170022fbc1b53010fde73ecf439420141, compared with base 6ecd9c15b0ea089c6036fa4d1776098a19b03cb1; authenticated GitHub and local changed-file inventories matched.
• Root-Cause Assessment: the PR correctly recognizes that OPENQUERY is represented as a function table and therefore bypasses the existing simple-table encrypt rewrite path. The implementation reaches that boundary, but the current parser/emitter and conditional validation do not preserve it safely.
• Verification:
  • ./mvnw -pl features/encrypt/core -DskipITs -Dtest=EncryptOpenQueryPassThroughSQLTest,EncryptOpenQueryUtilsTest,EncryptAssignmentTokenGeneratorTest,EncryptAssignmentTokenGeneratorOpenQueryUnsupportedShapeTest,EncryptUpdateAssignmentTokenGeneratorTest,EncryptSQLRewriteContextDecoratorTest test -B -T1C — exit 0, 68 tests.
  • ./mvnw -pl features/encrypt/core,test/it/rewriter -Dtest=EncryptSQLRewriterIT -Dsurefire.failIfNoSpecifiedTests=false test -B -T1C — exit 0, 208 tests.
  • Latest-head diagnostic probes reproduced the escaping mismatch, [FROM] discovery bypass, incomplete grammar acceptance, and unescaped emitted apostrophe.
• Not Reviewed: GitHub Actions/check logs and live SQL Server/OLE DB provider execution. The verdict is code-scope only and does not depend on those items because each blocker is reproducible in the current Java paths before provider execution.
• Release Note / User Docs: Present and appropriately narrowed, but the implementation does not yet enforce the documented boundary.
• Evidence Gaps: None affecting the verdict.